-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix: Remove applied rules from chat history #7343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -324,7 +324,7 @@ export const getApplicableRules = ( | |
| return applicableRules; | ||
| }; | ||
|
|
||
| export function getRuleId(rule: RuleWithSource): string { | ||
| export function getRuleId(rule: Omit<RuleWithSource, "rule">): string { | ||
| return rule.slug ?? rule.ruleFile ?? rule.name ?? rule.source; | ||
| } | ||
|
|
||
|
|
@@ -342,17 +342,17 @@ export const getSystemMessageWithRules = ({ | |
| rulePolicies?: RulePolicies; | ||
| }): { | ||
| systemMessage: string; | ||
| appliedRules: RuleWithSource[]; | ||
| appliedRules: Omit<RuleWithSource, "rule">[]; | ||
| } => { | ||
| const appliedRules = getApplicableRules( | ||
| const rules = getApplicableRules( | ||
| userMessage, | ||
| availableRules, | ||
| contextItems, | ||
| rulePolicies, | ||
| ); | ||
| let systemMessage = baseSystemMessage ?? ""; | ||
|
|
||
| for (const rule of appliedRules) { | ||
| for (const rule of rules) { | ||
| if (systemMessage) { | ||
| systemMessage += "\n\n"; | ||
| } | ||
|
|
@@ -361,6 +361,9 @@ export const getSystemMessageWithRules = ({ | |
|
|
||
| return { | ||
| systemMessage, | ||
| appliedRules, | ||
| appliedRules: rules.map(fullRule => { | ||
| const { rule, ...rest } = fullRule // destruct rule key | ||
| return rest; | ||
| }), | ||
|
Comment on lines
+364
to
+367
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit funky as well. Ideally this method would not take There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function could still just return RuleWithSource, shouldn't effect storage if omitted at the point of redux note that config should reload if a rule file is updated, so should be up to date in most cases |
||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,10 +29,33 @@ interface RuleCardProps { | |
| rule: RuleWithSource; | ||
| } | ||
|
|
||
| export const openRules = async (rule: Omit<RuleWithSource, "rule">) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could take RuleWithSource | AppliedRule |
||
| const ideMessenger = useContext(IdeMessengerContext); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will need to pass ideMessenger for this one since can't use hooks outside of top level of component |
||
| if (rule.slug) { | ||
| void ideMessenger.request("controlPlane/openUrl", { | ||
| path: `${rule.slug}/new-version`, | ||
| orgSlug: undefined, | ||
| }); | ||
| } else if (rule.ruleFile) { | ||
| ideMessenger.post("openFile", { | ||
| path: rule.ruleFile, | ||
| }); | ||
| } else if ( | ||
| rule.source === "default-chat" || | ||
| rule.source === "default-plan" || | ||
| rule.source === "default-agent" | ||
| ) { | ||
| ideMessenger.post("openUrl", DEFAULT_SYSTEM_MESSAGES_URL); | ||
| } else { | ||
| ideMessenger.post("config/openProfile", { | ||
| profileId: undefined, | ||
| element: { sourceFile: (rule as any).sourceFile }, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| const RuleCard: React.FC<RuleCardProps> = ({ rule }) => { | ||
| const dispatch = useAppDispatch(); | ||
| const ideMessenger = useContext(IdeMessengerContext); | ||
| const mode = useAppSelector((store) => store.session.mode); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unused. |
||
| const policy = useAppSelector((state) => | ||
| rule.name | ||
| ? state.ui.ruleSettings[rule.name] || DEFAULT_RULE_SETTING | ||
|
|
@@ -41,29 +64,6 @@ const RuleCard: React.FC<RuleCardProps> = ({ rule }) => { | |
|
|
||
| const isDisabled = policy === "off"; | ||
|
|
||
| const handleOpen = async () => { | ||
| if (rule.slug) { | ||
| void ideMessenger.request("controlPlane/openUrl", { | ||
| path: `${rule.slug}/new-version`, | ||
| orgSlug: undefined, | ||
| }); | ||
| } else if (rule.ruleFile) { | ||
| ideMessenger.post("openFile", { | ||
| path: rule.ruleFile, | ||
| }); | ||
| } else if ( | ||
| rule.source === "default-chat" || | ||
| rule.source === "default-plan" || | ||
| rule.source === "default-agent" | ||
| ) { | ||
| ideMessenger.post("openUrl", DEFAULT_SYSTEM_MESSAGES_URL); | ||
| } else { | ||
| ideMessenger.post("config/openProfile", { | ||
| profileId: undefined, | ||
| element: { sourceFile: (rule as any).sourceFile }, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| const handleTogglePolicy = () => { | ||
| if (rule.name) { | ||
|
|
@@ -139,12 +139,12 @@ const RuleCard: React.FC<RuleCardProps> = ({ rule }) => { | |
| <ArrowsPointingOutIcon className="h-3 w-3 text-gray-400" /> | ||
| </HeaderButtonWithToolTip>{" "} | ||
| {rule.source === "default-chat" || | ||
| rule.source === "default-agent" ? ( | ||
| <HeaderButtonWithToolTip onClick={handleOpen} text="View"> | ||
| rule.source === "default-agent" ? ( | ||
| <HeaderButtonWithToolTip onClick={() => openRules(rule)} text="View"> | ||
| <EyeIcon className="h-3 w-3 text-gray-400" /> | ||
| </HeaderButtonWithToolTip> | ||
| ) : ( | ||
| <HeaderButtonWithToolTip onClick={handleOpen} text="Edit"> | ||
| <HeaderButtonWithToolTip onClick={() => openRules(rule)} text="Edit"> | ||
| <PencilIcon className="h-3 w-3 text-gray-400" /> | ||
| </HeaderButtonWithToolTip> | ||
| )} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -282,7 +282,7 @@ export const streamNormalInput = createAsyncThunk< | |
| ...(appliedRules.length > 0 && { | ||
| rules: appliedRules.map((rule) => ({ | ||
| id: getRuleId(rule), | ||
| rule: rule.rule, | ||
| rule: "", // TODO: remove rule key from type. The contents should be present in the prompt | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a bit funky. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any specific reason to remove rules from dev data? Have you had issues with verbose dev data? |
||
| slug: rule.slug, | ||
| })), | ||
| }), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can keep the AppliedRule type and just say
type AppliedRule = Omit<RuleWithSource, "rule">to prevent havingOmits all over