From ddbbf5348803c4aae2af8f21d711a22b52a50806 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 21 May 2026 12:17:53 -0500 Subject: [PATCH 1/6] feat(goals): per-goal auto-compaction threshold override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an optional `autoCompactionThresholdPct` field to the goal record so each goal can pin its own auto-compaction behavior independently of the workspace's per-model slider. Lives as a tri-state mirroring `budgetCents` / `turnCap`: omitted = no override (model-level slider applies), `null` = explicit clear, 0–100 integer = explicit percent (`100` = compaction disabled for this goal). Why: in a goal-driven flow the user may prioritize cost control (aggressive compaction) over fidelity, or vice-versa, on a per-goal basis without retuning the global model slider. Backward-compatible by construction — legacy records and goals that never set the field continue to behave exactly as before. This commit lands the data model + behavior. Tests follow in a separate commit to keep the diff reviewable. --- src/browser/components/ChatPane/ChatPane.tsx | 15 +- src/browser/features/RightSidebar/GoalTab.tsx | 231 +++++++++++++++++- .../features/RightSidebar/RightSidebar.tsx | 21 ++ .../RightSidebar/Tabs/tabRegistry.tsx | 2 + src/browser/utils/chatCommands.ts | 10 + .../utils/goals/resolveGoalSetIntent.ts | 23 ++ src/browser/utils/slashCommands/registry.ts | 64 +++++ src/browser/utils/slashCommands/types.ts | 12 +- src/common/orpc/schemas/goal.ts | 32 +++ src/common/types/goal.ts | 7 + src/node/orpc/router.ts | 2 + src/node/services/agentSession.ts | 45 ++++ src/node/services/compactionMonitor.ts | 39 ++- src/node/services/workspaceGoalService.ts | 67 +++++ 14 files changed, 557 insertions(+), 13 deletions(-) diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index 2597ce8353..11aabc335c 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -376,17 +376,28 @@ export const ChatPane: React.FC = (props) => { [deferredMessages] ); + // Resolve the effective threshold for the warning banner: a per-goal + // override (set on the active goal's `autoCompactionThresholdPct`) + // wins over the workspace-level slider for both the backend and the + // renderer. Keeping the two in lockstep matters because the banner + // says "compaction is imminent" — if the renderer used the slider + // value but the backend used the goal override, the user would see + // either spurious or missing warnings around the threshold boundary. + const effectiveAutoCompactionThreshold = + workspaceState.goal?.autoCompactionThresholdPct != null + ? workspaceState.goal.autoCompactionThresholdPct / 100 + : autoCompactionThreshold / 100; const autoCompactionResult = useMemo( () => checkAutoCompaction( workspaceUsage, pendingModel, use1M, - autoCompactionThreshold / 100, + effectiveAutoCompactionThreshold, undefined, providersConfig ), - [workspaceUsage, pendingModel, use1M, providersConfig, autoCompactionThreshold] + [workspaceUsage, pendingModel, use1M, providersConfig, effectiveAutoCompactionThreshold] ); // Show warning when: shouldShowWarning flag is true AND not currently compacting. diff --git a/src/browser/features/RightSidebar/GoalTab.tsx b/src/browser/features/RightSidebar/GoalTab.tsx index e33cb9b9fc..1a0e055b73 100644 --- a/src/browser/features/RightSidebar/GoalTab.tsx +++ b/src/browser/features/RightSidebar/GoalTab.tsx @@ -42,11 +42,18 @@ import { useGoalBoard } from "@/browser/features/RightSidebar/useGoalBoard"; * UI and `/goal` paths agree on the create vocabulary. `budgetCents` is a * tri-state: `undefined` means "apply default", `null`/`0` means "no * budget", and a positive number is an explicit cents value. + * + * `autoCompactionThresholdPct` follows the same tri-state shape: + * - `undefined` → no override; the workspace's per-model slider applies. + * - `null` → explicit clear of an existing override. + * - integer 0–100 → percent of context window at which to compact + * (`100` = compaction disabled specifically for this goal). */ export interface GoalCreateIntent { objective: string; budgetCents?: number | null; turnCap?: number | null; + autoCompactionThresholdPct?: number | null; } interface GoalTabProps { @@ -75,6 +82,13 @@ interface GoalTabProps { onUpdateObjective?: (objective: string) => Promise | void; onUpdateBudget?: (budgetCents: number | null) => Promise | void; onUpdateTurnCap?: (turnCap: number | null) => Promise | void; + /** + * Persist a change to the goal's per-goal auto-compact override. `null` + * clears the override (the workspace's per-model slider applies again); + * a number 0–100 sets the percent. Optional so read-only storybook + * stories can omit it (they hide the Edit affordance via `canEdit`). + */ + onUpdateAutoCompactionThresholdPct?: (pct: number | null) => Promise | void; onClear?: () => Promise | void; /** * Create a brand-new goal for the workspace. Used by the empty-state form @@ -90,7 +104,37 @@ interface GoalTabProps { const parseBudgetInput = parseGoalBudgetInputCents; const parseTurnCapInput = parseGoalTurnCapInput; -type EditingField = "objective" | "budget" | "turnCap"; +type EditingField = "objective" | "budget" | "turnCap" | "compactThreshold"; + +/** + * Parse the user-typed value from the auto-compact inline editor / create + * form into the tri-state the rest of the goal pipeline expects. + * + * - blank, "default", "—" → `null` (clear the override; workspace slider applies) + * - "off" / "disable" / "disabled" → `100` (explicit per-goal disable) + * - integer 0–100 → that integer + * - anything else → `undefined` (parse error; caller surfaces a message) + * + * Defined alongside the budget / turn-cap parsers in + * `@/common/utils/goals/budgetParser` to keep the slash command + UI + * forms in lockstep — but this one stays inline because the slash-command + * path is not yet plumbed (follow-up). + */ +function parseGoalAutoCompactionThresholdInput(raw: string): number | null | undefined { + const trimmed = raw.trim().toLowerCase(); + if (trimmed.length === 0 || trimmed === "default" || trimmed === "—") { + return null; + } + if (trimmed === "off" || trimmed === "disable" || trimmed === "disabled") { + return 100; + } + // Strip an optional trailing "%" so "50%" parses the same as "50". + const numericPart = trimmed.endsWith("%") ? trimmed.slice(0, -1).trim() : trimmed; + if (!/^\d+$/.test(numericPart)) return undefined; + const value = Number.parseInt(numericPart, 10); + if (!Number.isFinite(value) || value < 0 || value > 100) return undefined; + return value; +} export function GoalTab(props: GoalTabProps) { const [isSummaryInputOpen, setIsSummaryInputOpen] = useState(false); @@ -171,6 +215,20 @@ export function GoalTab(props: GoalTabProps) { setEditingField("turnCap"); }; + const openCompactThresholdEditor = (origin: HTMLElement | null) => { + originRef.current = origin; + // Mirror the budget/turnCap pattern: seed the input with the current + // explicit value, or leave blank when there's no override so the + // placeholder ("default — workspace setting applies") is visible. + // `100` round-trips as "100" rather than "off"; the parser accepts + // both, so this keeps the value the user sees aligned with what + // they'd see in a /goal --compact 100 invocation. + const current = props.goal?.autoCompactionThresholdPct; + setEditValue(current == null ? "" : String(current)); + setError(null); + setEditingField("compactThreshold"); + }; + const closeEditor = () => { setEditingField(null); setError(null); @@ -211,6 +269,16 @@ export function GoalTab(props: GoalTabProps) { return; } await props.onUpdateTurnCap?.(turnCap); + } else if (editingField === "compactThreshold") { + const submittedValue = editInputRef.current?.value ?? editValue; + const pct = parseGoalAutoCompactionThresholdInput(submittedValue); + if (pct === undefined) { + setError( + 'Enter a whole number 0–100, type "off" to disable, or blank to use the workspace default.' + ); + return; + } + await props.onUpdateAutoCompactionThresholdPct?.(pct); } closeEditor(); } catch (caught) { @@ -548,25 +616,63 @@ export function GoalTab(props: GoalTabProps) { {formatGoalElapsed(props.goal.startedAtMs)} + {/* Auto-compact override (per-goal). Shows the user's current + knob position: "Default" when the goal has no override (the + workspace per-model slider applies), "Off" when the goal + explicitly disables compaction (`100`), and the percent + value otherwise. Editable via the same inline editor that + drives Budget / Turn cap so the create / edit vocabulary is + consistent across all goal fields. */} + openCompactThresholdEditor(event.currentTarget)} + /> - {(editingField === "budget" || editingField === "turnCap") && ( + {(editingField === "budget" || + editingField === "turnCap" || + editingField === "compactThreshold") && (
event.currentTarget.select()} @@ -585,7 +691,12 @@ export function GoalTab(props: GoalTabProps) {

{editingField === "budget" ? "Use $5, 500c, 0, or blank for no budget." - : "Use a positive whole number, or blank for no cap."} + : editingField === "turnCap" + ? "Use a positive whole number, or blank for no cap." + : // The auto-compact help text spells out the tri-state + // so the user understands what blank means without + // having to guess against the model-level slider. + 'Use 0–99 to compact at that % of context. Type 100 or "off" to disable for this goal. Blank uses the workspace default.'}

+ )} +
+
+
+ {primary} +
+
{helper}
+
+
+ ); +} + interface GoalCreateFormProps { onCreate: (intent: GoalCreateIntent) => Promise | void; workspaceId?: string; @@ -1010,6 +1182,11 @@ function GoalCreateForm(props: GoalCreateFormProps) { const objectiveRef = useRef(null); const budgetRef = useRef(null); const turnCapRef = useRef(null); + // Auto-compact threshold uses the same ref-driven pattern: blank means + // "no override (workspace setting applies)", a number / "off" means + // explicit per-goal override. Matches the slash-command path the + // follow-up will plumb (`/goal --compact N|off`). + const compactThresholdRef = useRef(null); // Effective defaults shown as placeholder text. We seed the inputs with // `defaultValue` rather than `value` so the user can clear them; the @@ -1067,6 +1244,21 @@ function GoalCreateForm(props: GoalCreateFormProps) { intent.turnCap = parsedTurnCap; } + // Auto-compact threshold: leave omitted when blank so the goal + // gets no override (workspace setting applies). Explicit values + // ("off", 100, 0–99, "default") flow through the shared parser. + const compactRaw = (compactThresholdRef.current?.value ?? "").trim(); + if (compactRaw.length > 0) { + const parsedPct = parseGoalAutoCompactionThresholdInput(compactRaw); + if (parsedPct === undefined) { + setError( + 'Enter a whole number 0–100, type "off" to disable, or blank to use the workspace default.' + ); + return; + } + intent.autoCompactionThresholdPct = parsedPct; + } + await props.onCreate(intent); // Clear the form on success so a returning user sees a blank slate // (if for some reason the goal didn't take, e.g., the workspace @@ -1075,6 +1267,7 @@ function GoalCreateForm(props: GoalCreateFormProps) { if (objectiveRef.current) objectiveRef.current.value = ""; if (budgetRef.current) budgetRef.current.value = ""; if (turnCapRef.current) turnCapRef.current.value = ""; + if (compactThresholdRef.current) compactThresholdRef.current.value = ""; } catch (caught) { setError(caught instanceof Error ? caught.message : "Goal creation failed"); } finally { @@ -1158,6 +1351,30 @@ function GoalCreateForm(props: GoalCreateFormProps) { + {/* Per-goal auto-compact override. Spans the full width because the + helper text is the educational part (most users won't reach for + this); shrinking to a tile-sized input would compete with + Budget / Turn cap visually for no real value. */} +
+ + + + 0–99 to compact at that % of context, 100 / "off" to disable for this goal. + +
+ {props.workspaceId != null && (
diff --git a/src/browser/features/RightSidebar/RightSidebar.tsx b/src/browser/features/RightSidebar/RightSidebar.tsx index 10538f1ab3..76807c2152 100644 --- a/src/browser/features/RightSidebar/RightSidebar.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.tsx @@ -302,6 +302,14 @@ interface RightSidebarTabsetNodeProps { onGoalUpdateObjective: (objective: string) => Promise; onGoalUpdateBudget: (budgetCents: number | null) => Promise; onGoalUpdateTurnCap: (turnCap: number | null) => Promise; + /** + * Persist the active goal's per-goal auto-compact override. `null` + * clears the override (workspace per-model slider applies); a number + * 0–100 sets the percent. Wired through `setGoalWithSingleConflictRetry` + * just like budget / turn-cap edits so the optimistic-concurrency + * loop covers all three. + */ + onGoalUpdateAutoCompactionThresholdPct: (pct: number | null) => Promise; onGoalClear: () => Promise; /** * Create a brand-new goal from the GoalTab's in-tab form. Matches the @@ -465,6 +473,7 @@ const RightSidebarTabsetNode: React.FC = (props) => onUpdateObjective: props.onGoalUpdateObjective, onUpdateBudget: props.onGoalUpdateBudget, onUpdateTurnCap: props.onGoalUpdateTurnCap, + onUpdateAutoCompactionThresholdPct: props.onGoalUpdateAutoCompactionThresholdPct, onClear: props.onGoalClear, onCreate: props.onGoalCreate, }, @@ -701,6 +710,9 @@ const RightSidebarComponent: React.FC = ({ objective?: string; budgetCents?: number | null; turnCap?: number | null; + // Tri-state: `undefined` = no change, `null` = clear the override, + // explicit number = set per-goal threshold percent. + autoCompactionThresholdPct?: number | null; completionSummary?: string; // `editInPlace` is forwarded verbatim to `setGoal`; it tells the backend // to mutate the existing goal record instead of archiving+recreating. @@ -739,6 +751,14 @@ const RightSidebarComponent: React.FC = ({ await setGoalWithSingleConflictRetry({ turnCap }); }; + const handleGoalUpdateAutoCompactionThresholdPct = async (pct: number | null) => { + // No model-pricing pre-check (unlike `handleGoalUpdateBudget`): + // the per-goal auto-compact override doesn't gate on budget pricing + // — it only changes when the compaction loop triggers. The shared + // retry helper handles the optimistic-concurrency loop. + await setGoalWithSingleConflictRetry({ autoCompactionThresholdPct: pct }); + }; + const handleGoalUpdateObjective = async (objective: string) => { // The inline objective editor matches the budget / turn-cap editors: // mutate the current goal in place rather than archiving + recreating @@ -1624,6 +1644,7 @@ const RightSidebarComponent: React.FC = ({ onGoalUpdateObjective={handleGoalUpdateObjective} onGoalUpdateBudget={handleGoalUpdateBudget} onGoalUpdateTurnCap={handleGoalUpdateTurnCap} + onGoalUpdateAutoCompactionThresholdPct={handleGoalUpdateAutoCompactionThresholdPct} onGoalClear={handleGoalClear} onGoalCreate={handleGoalCreate} onAutoFocusConsumed={() => setAutoFocusTerminalSession(null)} diff --git a/src/browser/features/RightSidebar/Tabs/tabRegistry.tsx b/src/browser/features/RightSidebar/Tabs/tabRegistry.tsx index 395b6e7168..604f68c03f 100644 --- a/src/browser/features/RightSidebar/Tabs/tabRegistry.tsx +++ b/src/browser/features/RightSidebar/Tabs/tabRegistry.tsx @@ -94,6 +94,7 @@ export interface TabPanelContext { onUpdateObjective: (objective: string) => Promise; onUpdateBudget: (budgetCents: number | null) => Promise; onUpdateTurnCap: (turnCap: number | null) => Promise; + onUpdateAutoCompactionThresholdPct: (pct: number | null) => Promise; onClear: () => Promise; onCreate: (intent: GoalCreateIntent) => Promise; }; @@ -151,6 +152,7 @@ const TAB_RENDERERS = { onUpdateObjective={ctx.goal.onUpdateObjective} onUpdateBudget={ctx.goal.onUpdateBudget} onUpdateTurnCap={ctx.goal.onUpdateTurnCap} + onUpdateAutoCompactionThresholdPct={ctx.goal.onUpdateAutoCompactionThresholdPct} onClear={ctx.goal.onClear} onCreate={ctx.goal.onCreate} /> diff --git a/src/browser/utils/chatCommands.ts b/src/browser/utils/chatCommands.ts index ad93701f29..155bd4f4e9 100644 --- a/src/browser/utils/chatCommands.ts +++ b/src/browser/utils/chatCommands.ts @@ -657,6 +657,10 @@ interface GoalSetCommandIntent { status?: PublicSetGoalStatus | null; budgetCents?: number | null; turnCap?: number | null; + // Per-goal auto-compact override forwarded verbatim to `setGoal`. + // Same tri-state as `budgetCents` / `turnCap`: omitted = no change, + // `null` = clear override, number = set explicit percent. + autoCompactionThresholdPct?: number | null; completionSummary?: string | null; } @@ -699,6 +703,12 @@ function resolveSlashGoalSetIntent( objective: parsed.objective, ...(Object.hasOwn(parsed, "budgetCents") ? { budgetCents: parsed.budgetCents ?? null } : {}), ...(Object.hasOwn(parsed, "turnCap") ? { turnCap: parsed.turnCap ?? null } : {}), + // Forward the per-goal auto-compact override only when the + // slash command actually included `--compact`. `Object.hasOwn` + // is the precision the rest of the tri-state pipeline uses. + ...(Object.hasOwn(parsed, "autoCompactionThresholdPct") + ? { autoCompactionThresholdPct: parsed.autoCompactionThresholdPct ?? null } + : {}), }, defaults ); diff --git a/src/browser/utils/goals/resolveGoalSetIntent.ts b/src/browser/utils/goals/resolveGoalSetIntent.ts index d11b8e9638..771dd7e0b7 100644 --- a/src/browser/utils/goals/resolveGoalSetIntent.ts +++ b/src/browser/utils/goals/resolveGoalSetIntent.ts @@ -50,12 +50,27 @@ export interface GoalSetIntentInput { objective: string; budgetCents?: number | null; turnCap?: number | null; + /** + * Per-goal auto-compaction threshold override. Tri-state same as + * `budgetCents` / `turnCap`: `undefined` = no opinion (no field on + * the resolved intent), `null` = explicit clear, number = explicit + * percent. No workspace-level default exists for this field yet, so + * the resolver just passes the value through verbatim instead of + * filling in a default. + */ + autoCompactionThresholdPct?: number | null; } export interface GoalSetIntent { objective: string; budgetCents: number | null; turnCap: number | null; + // Only present on the resolved intent when the caller actually opted + // in (slash command flag, create-form input, or inline editor). Keeps + // omitted-from-input distinguishable from null-from-input so the + // downstream `setGoal` payload doesn't accidentally clear an existing + // per-goal override on plain `/goal ` invocations. + autoCompactionThresholdPct?: number | null; } /** @@ -93,6 +108,14 @@ export function resolveGoalSetIntent( objective: input.objective, budgetCents, turnCap, + // Pass through verbatim — including the explicit `null` clear — so + // the slash command's `--compact default` reaches `setGoal`. Omit + // the key entirely when the caller didn't set it; otherwise a + // plain `/goal ` would unintentionally clear an existing + // override on every replace. + ...(input.autoCompactionThresholdPct !== undefined + ? { autoCompactionThresholdPct: input.autoCompactionThresholdPct } + : {}), }; } diff --git a/src/browser/utils/slashCommands/registry.ts b/src/browser/utils/slashCommands/registry.ts index f8b9b15d97..deee785714 100644 --- a/src/browser/utils/slashCommands/registry.ts +++ b/src/browser/utils/slashCommands/registry.ts @@ -458,6 +458,41 @@ function parseGoalTurnCap(value: unknown): number | null { return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : null; } +/** + * Parse the `--compact ` flag for `/goal`. Returns: + * - integer 0–100 — explicit per-goal auto-compact threshold percent + * - `null` — sentinel for "off" / "default" / "off" / "clear", which + * the handler maps to either explicit-disabled (`100`) or + * clear-override (`null`) below. + * - `undefined` — parse error; caller returns `command-invalid-args`. + * + * `"off"` is mapped to `100` (per-goal disabled) at the handler level so + * the slash command surfaces the same semantics the inline editor uses. + * `"default"` / `"none"` map to a literal `null` so the user can clear + * an existing override without opening the GoalTab. + */ +function parseGoalCompactionThreshold( + value: unknown +): { kind: "value"; pct: number } | { kind: "clear" } | { kind: "off" } | null { + if (typeof value !== "string" && typeof value !== "number") { + return null; + } + const raw = String(value).trim().toLowerCase(); + if (raw === "default" || raw === "none" || raw === "clear") { + return { kind: "clear" }; + } + if (raw === "off" || raw === "disable" || raw === "disabled") { + return { kind: "off" }; + } + const numericPart = raw.endsWith("%") ? raw.slice(0, -1).trim() : raw; + if (!/^\d+$/.test(numericPart)) return null; + const parsed = Number.parseInt(numericPart, 10); + if (!Number.isFinite(parsed) || parsed < 0 || parsed > 100) { + return null; + } + return { kind: "value", pct: parsed }; +} + const GOAL_USAGE = `/goal ${SLASH_COMMAND_HINTS.goal}`; const GOAL_BUDGET_USAGE = "/goal budget "; @@ -566,6 +601,7 @@ const goalCommandDefinition: SlashCommandDefinition = { let budgetCents: number | undefined; let turnCap: number | undefined; + let autoCompactionThresholdPct: number | null | undefined; let objectiveStartIndex = 0; if (headerTokens[0] === "-b") { @@ -608,6 +644,31 @@ const goalCommandDefinition: SlashCommandDefinition = { objectiveStartIndex += 2; } + // `--compact ` is order-sensitive like `--turns`: it must + // appear before the objective text begins. Accepted values: 0–100 + // integer percent, "off"/"disable" (= 100), "default"/"none"/"clear" + // (= null). The handler maps these into the discriminated tri-state + // the rest of the goal pipeline already understands. + if (headerTokens[objectiveStartIndex] === "--compact") { + const rawCompact = headerTokens[objectiveStartIndex + 1]; + const parsedCompact = parseGoalCompactionThreshold(rawCompact); + if (parsedCompact == null) { + return { + type: "command-invalid-args", + command: "goal", + input: String(rawCompact), + usage: GOAL_USAGE, + }; + } + autoCompactionThresholdPct = + parsedCompact.kind === "value" + ? parsedCompact.pct + : parsedCompact.kind === "off" + ? 100 + : null; + objectiveStartIndex += 2; + } + // Only a leading budget/turn flag prefix is command syntax; everything // after that is user-authored goal text so objectives can mention // flag-looking strings (including deprecated-looking budget flags). @@ -631,6 +692,9 @@ const goalCommandDefinition: SlashCommandDefinition = { if (turnCap !== undefined) { result.turnCap = turnCap; } + if (autoCompactionThresholdPct !== undefined) { + result.autoCompactionThresholdPct = autoCompactionThresholdPct; + } return result; }, diff --git a/src/browser/utils/slashCommands/types.ts b/src/browser/utils/slashCommands/types.ts index 0b38a4e0c0..bd470b7102 100644 --- a/src/browser/utils/slashCommands/types.ts +++ b/src/browser/utils/slashCommands/types.ts @@ -39,7 +39,17 @@ export type ParsedCommand = | { type: "idle-compaction"; hours: number | null } | { type: "heartbeat-set"; minutes: number | null } | { type: "goal-show" } - | { type: "goal-set"; objective: string; budgetCents?: number | null; turnCap?: number | null } + | { + type: "goal-set"; + objective: string; + budgetCents?: number | null; + turnCap?: number | null; + // Per-goal auto-compaction threshold override, mirroring the + // create-form/inline editor semantics. `null` = clear override, + // 0–100 = explicit percent (100 disables for this goal), omitted + // = no opinion (workspace setting applies). + autoCompactionThresholdPct?: number | null; + } | { type: "goal-budget"; budgetCents: number | null } | { type: "goal-pause" } | { type: "goal-resume" } diff --git a/src/common/orpc/schemas/goal.ts b/src/common/orpc/schemas/goal.ts index cf0e74d9fd..c6ed392576 100644 --- a/src/common/orpc/schemas/goal.ts +++ b/src/common/orpc/schemas/goal.ts @@ -47,6 +47,25 @@ export const GoalRecordV1Schema = z.object({ budgetLimitOriginKind: GoalBudgetLimitOriginKindSchema.optional(), requireUserAcknowledgmentSinceMs: z.number().int().nonnegative().nullable(), lastContinuationFiredAtMs: z.number().int().nonnegative().nullable().optional(), + /** + * Per-goal override for the auto-compaction threshold, expressed as an + * integer percentage of the model's max context window. + * + * - `null` / `undefined` (default) → no override; the workspace's + * per-model auto-compact slider governs as today. + * - `0` – `99` → compact at this percent of context for this goal, + * regardless of the model-level setting. + * - `100` → auto-compaction is explicitly disabled for this goal. + * + * Optional + nullable so legacy persisted goal records keep loading + * without migration; new writes set it explicitly. Lives on the goal + * because goal-based flows often have different cost-vs-fidelity + * preferences than the workspace's default model setting — a long + * research goal might want aggressive compaction for cost, while a + * short refactor goal might prefer no compaction so the agent keeps + * the full plan in view. + */ + autoCompactionThresholdPct: z.number().int().min(0).max(100).nullable().optional(), completionSummary: z.string().optional(), createdAtMs: z.number().int().nonnegative(), updatedAtMs: z.number().int().nonnegative(), @@ -60,6 +79,12 @@ export const GoalSnapshotSchema = z.object({ costCents: z.number().int().nonnegative(), turnsUsed: z.number().int().nonnegative(), turnCap: z.number().int().positive().nullable(), + // Surface the per-goal auto-compact override to the renderer so the + // ChatPane warning banner and the GoalTab tile both see the same + // effective threshold. Same semantics as `GoalRecordV1Schema.autoCompactionThresholdPct`: + // `null`/`undefined` = no override (model-level slider applies); + // 0–99 = explicit threshold for this goal; 100 = disabled for this goal. + autoCompactionThresholdPct: z.number().int().min(0).max(100).nullable().optional(), completionSummary: z.string().optional(), startedAtMs: z.number().int().nonnegative(), pendingPersistence: z.boolean().optional(), @@ -115,6 +140,11 @@ export const GoalSetInputSchema = z.object({ status: PublicGoalStatusSchema.nullish(), budgetCents: z.number().int().nonnegative().nullable().optional(), turnCap: z.number().int().positive().nullable().optional(), + // Optional per-goal auto-compact override. Same tri-state as + // `budgetCents` / `turnCap`: `undefined` = no change, `null` = clear + // the override (fall back to model-level setting), explicit number = + // set the per-goal threshold percent (0–100, 100 = disabled). + autoCompactionThresholdPct: z.number().int().min(0).max(100).nullable().optional(), completionSummary: z.string().nullish(), expectedGoalId: z.string().uuid().nullish(), // When true and a current goal already exists, an objective update mutates @@ -198,6 +228,7 @@ export const GoalBoardAddUpcomingInputSchema = z.object({ objective: z.string().min(1), budgetCents: z.number().int().nonnegative().nullable().optional(), turnCap: z.number().int().positive().nullable().optional(), + autoCompactionThresholdPct: z.number().int().min(0).max(100).nullable().optional(), }); export const GoalBoardArchiveInputSchema = z.object({ @@ -238,4 +269,5 @@ export const GoalBoardUpdateUpcomingInputSchema = z.object({ objective: z.string().min(1).optional(), budgetCents: z.number().int().nonnegative().nullable().optional(), turnCap: z.number().int().positive().nullable().optional(), + autoCompactionThresholdPct: z.number().int().min(0).max(100).nullable().optional(), }); diff --git a/src/common/types/goal.ts b/src/common/types/goal.ts index 0cdc1d51fc..3c83f00e9f 100644 --- a/src/common/types/goal.ts +++ b/src/common/types/goal.ts @@ -124,6 +124,13 @@ export function toGoalSnapshot(goal: GoalRecordV1): GoalSnapshot { costCents: goal.costCents, turnsUsed: goal.turnsUsed, turnCap: goal.turnCap, + // Carry the per-goal auto-compact override through to the renderer + // so the ChatPane warning banner and the GoalTab tile see the same + // effective threshold. Only include the key when set; `null` is + // meaningful (explicit clear) so it must round-trip when present. + ...(goal.autoCompactionThresholdPct !== undefined + ? { autoCompactionThresholdPct: goal.autoCompactionThresholdPct } + : {}), ...(goal.completionSummary != null ? { completionSummary: goal.completionSummary } : {}), startedAtMs: goal.createdAtMs, }; diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index c32aed23ab..fe4af1e59d 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -4174,6 +4174,7 @@ export const router = (authToken?: string) => { objective: input.objective, budgetCents: input.budgetCents, turnCap: input.turnCap, + autoCompactionThresholdPct: input.autoCompactionThresholdPct, }) ); }), @@ -4220,6 +4221,7 @@ export const router = (authToken?: string) => { objective: input.objective, budgetCents: input.budgetCents, turnCap: input.turnCap, + autoCompactionThresholdPct: input.autoCompactionThresholdPct, }) ); }), diff --git a/src/node/services/agentSession.ts b/src/node/services/agentSession.ts index 8e36ccfde0..9911f2a93a 100644 --- a/src/node/services/agentSession.ts +++ b/src/node/services/agentSession.ts @@ -2548,6 +2548,12 @@ export class AgentSession { await this.seedUsageStateFromHistory(); const providersConfigForCompaction = this.getProvidersConfigForCompaction(); + // Pick up the active goal's per-goal auto-compact override (if any) + // so a goal explicitly tuned for aggressive cost control or for + // "no compaction, full fidelity" beats the workspace-level slider + // for *this* send. Falls back to `null` (no override) when there's + // no active goal or the goal didn't set the field. + const goalThresholdOverride = await this.resolveGoalAutoCompactionThresholdOverride(); const compactionResult = this.compactionMonitor.checkBeforeSend({ model: modelForStream, usage: this.getUsageState(), @@ -2557,6 +2563,7 @@ export class AgentSession { providersConfigForCompaction ), providersConfig: providersConfigForCompaction, + thresholdOverride: goalThresholdOverride, }); // On-send compaction uses the configured threshold directly so we compact @@ -2875,6 +2882,36 @@ export class AgentSession { this.compactionMonitor.setThreshold(threshold); } + /** + * Look up the active goal's per-goal auto-compact threshold and return + * it as a decimal 0–1 suitable for passing to `CompactionMonitor`'s + * `thresholdOverride`, or `null` when there is no override. + * + * Failure modes are intentionally silent: the goal file might be + * missing, corrupt, or the service might not be wired up for a test + * harness. In every such case we fall back to "no override" so the + * workspace-level threshold continues to govern. Compaction is on the + * hot path for every send + every usage delta; a thrown error here + * would interrupt the stream pipeline. + */ + private async resolveGoalAutoCompactionThresholdOverride(): Promise { + const goalService = this.workspaceGoalService; + if (!goalService) return null; + try { + const goal = await goalService.getGoal(this.workspaceId); + const pct = goal?.autoCompactionThresholdPct; + if (pct == null) return null; + if (!Number.isFinite(pct)) return null; + // Schema already pins this into [0, 100], but defend against a + // legacy/corrupt record bypassing parse by clamping again before + // converting to the decimal scale the monitor consumes. + const clampedPct = Math.max(0, Math.min(100, pct)); + return clampedPct / 100; + } catch { + return null; + } + } + private getUsageState(): AutoCompactionUsageState | undefined { return this.lastUsageState; } @@ -4403,6 +4440,13 @@ export class AgentSession { const streamContext = this.activeStreamContext; const streamOptions = streamContext?.options; + // Same per-goal override as `checkBeforeSend`. Reading the goal on + // every usage-delta event is cheap (a single in-memory file read + // in the common case) and avoids racing the renderer's perception + // of the threshold: if the user edited the override mid-stream, + // the next usage delta will already see it. + const midStreamGoalThresholdOverride = + await this.resolveGoalAutoCompactionThresholdOverride(); const shouldInterruptForCompaction = this.compactionMonitor.checkMidStream({ model: modelForUsage, usage: payload.usage, @@ -4412,6 +4456,7 @@ export class AgentSession { streamContext?.providersConfig ?? null ), providersConfig: streamContext?.providersConfig ?? null, + thresholdOverride: midStreamGoalThresholdOverride, }); if (shouldInterruptForCompaction) { diff --git a/src/node/services/compactionMonitor.ts b/src/node/services/compactionMonitor.ts index b4f6c10b77..00a0a3f874 100644 --- a/src/node/services/compactionMonitor.ts +++ b/src/node/services/compactionMonitor.ts @@ -28,6 +28,16 @@ interface CheckBeforeSendParams { usage: AutoCompactionUsageState | undefined; use1MContext: boolean; providersConfig: ProvidersConfigMap | null; + /** + * Optional per-call threshold override (decimal 0–1). When provided and + * finite, replaces `this.threshold` for the duration of this check — + * the monitor's own threshold (set by the renderer's per-model slider) + * is untouched. Callers use this to layer the active goal's per-goal + * `autoCompactionThresholdPct` on top of the workspace setting without + * mutating the monitor's persistent state. A value `>= 1` disables + * compaction for this check just like `setThreshold(1)` does. + */ + thresholdOverride?: number | null; } interface CheckMidStreamParams { @@ -35,6 +45,8 @@ interface CheckMidStreamParams { usage: LanguageModelV2Usage; use1MContext: boolean; providersConfig: ProvidersConfigMap | null; + /** See `CheckBeforeSendParams.thresholdOverride`. */ + thresholdOverride?: number | null; } /** @@ -69,7 +81,7 @@ export class CompactionMonitor { params.usage, params.model, params.use1MContext, - this.threshold, + this.resolveEffectiveThreshold(params.thresholdOverride), undefined, params.providersConfig ); @@ -93,8 +105,9 @@ export class CompactionMonitor { return false; } + const effectiveThreshold = this.resolveEffectiveThreshold(params.thresholdOverride); // Threshold 1.0 means auto-compaction is disabled. - if (this.threshold >= 1) { + if (effectiveThreshold >= 1) { return false; } @@ -119,7 +132,7 @@ export class CompactionMonitor { ); const usagePercent = (usageTokens / contextLimit) * 100; - const forceThresholdPercent = this.threshold * 100 + FORCE_COMPACTION_BUFFER_PERCENT; + const forceThresholdPercent = effectiveThreshold * 100 + FORCE_COMPACTION_BUFFER_PERCENT; if (usagePercent < forceThresholdPercent) { return false; @@ -153,4 +166,24 @@ export class CompactionMonitor { getThreshold(): number { return this.threshold; } + + /** + * Pick the threshold that should govern this single check. A finite, + * positive override (passed by the caller, typically derived from the + * active goal's `autoCompactionThresholdPct`) wins over the monitor's + * own per-workspace value. We intentionally keep validation loose here: + * `null` / `undefined` / non-finite / non-positive values fall back to + * `this.threshold` instead of throwing, because the override is + * sourced from optional persisted state and a malformed entry must + * never brick the compaction pipeline mid-stream. + */ + private resolveEffectiveThreshold(override: number | null | undefined): number { + if (override == null) return this.threshold; + if (!Number.isFinite(override)) return this.threshold; + if (override <= 0) return this.threshold; + // Clamp the upper bound so an out-of-range "200%" override behaves the + // same way `setThreshold(1)` would: compaction disabled, not skewed + // higher than the model's actual context. + return Math.min(override, 1); + } } diff --git a/src/node/services/workspaceGoalService.ts b/src/node/services/workspaceGoalService.ts index 88438a9008..fd0a688311 100644 --- a/src/node/services/workspaceGoalService.ts +++ b/src/node/services/workspaceGoalService.ts @@ -101,6 +101,14 @@ export interface SetGoalInput { status?: GoalStatus | null; budgetCents?: number | null; turnCap?: number | null; + /** + * Per-goal auto-compaction threshold override (integer percent 0–100). + * Tri-state matching `budgetCents` / `turnCap`: + * - `undefined` (key omitted) → no change. + * - `null` → clear the override; model-level slider applies again. + * - explicit number → set the per-goal threshold percent (100 = disabled). + */ + autoCompactionThresholdPct?: number | null; completionSummary?: string | null; expectedGoalId?: string | null; requireUserAcknowledgmentSinceMs?: number | null; @@ -183,6 +191,10 @@ interface PendingGoalMutation { objective: string; budgetCents?: number | null; turnCap?: number | null; + // Mirrors `SetGoalInput.autoCompactionThresholdPct`. Queued so a mid-stream + // edit of the per-goal threshold rides through `applyPendingAfterStreamEnd` + // → `setGoalImmediately` and lands on the persisted record. + autoCompactionThresholdPct?: number | null; status?: GoalStatus | null; completionSummary?: string | null; expectedGoalId?: string | null; @@ -448,6 +460,11 @@ export class WorkspaceGoalService { objective: string; budgetCents: number | null; turnCap: number | null; + // Per-goal auto-compact override (percent, 0–100). `undefined` = + // no override, fall back to model-level setting. Lifted to a creation + // arg so the slash command, sidebar create form, and palette can all + // seed it on first set without a follow-up edit round-trip. + autoCompactionThresholdPct?: number | null; status?: GoalStatus | null; completionSummary?: string | null; }): GoalRecordV1 { @@ -467,6 +484,12 @@ export class WorkspaceGoalService { budgetLimitInjectedForGoalId: null, requireUserAcknowledgmentSinceMs: null, lastContinuationFiredAtMs: null, + // Persist `null` explicitly when the caller asked to clear the + // override on creation; omit when undefined so the schema's + // optional/nullable contract treats the goal as "no override". + ...(input.autoCompactionThresholdPct !== undefined + ? { autoCompactionThresholdPct: input.autoCompactionThresholdPct } + : {}), ...(input.completionSummary != null ? { completionSummary: input.completionSummary.trim() } : {}), @@ -1323,6 +1346,12 @@ export class WorkspaceGoalService { ? { budgetCents: normalizeGoalBudgetCents(input.budgetCents) } : {}), ...(Object.hasOwn(input, "turnCap") ? { turnCap: input.turnCap ?? null } : {}), + // `Object.hasOwn` rather than truthy-check so `null` (explicit + // clear of the override) survives the patch — matches how budget + // and turnCap distinguish "omitted" from "clear me". + ...(Object.hasOwn(input, "autoCompactionThresholdPct") + ? { autoCompactionThresholdPct: input.autoCompactionThresholdPct ?? null } + : {}), ...(Object.hasOwn(input, "requireUserAcknowledgmentSinceMs") ? { requireUserAcknowledgmentSinceMs: input.requireUserAcknowledgmentSinceMs ?? null } : {}), @@ -1662,6 +1691,13 @@ export class WorkspaceGoalService { objective, budgetCents: input.budgetCents ?? null, turnCap: input.turnCap ?? null, + // Mirror createGoal's tri-state: only forward the override + // key when the caller actually set it (including explicit + // null = clear), so the optimistic record matches what the + // pending mutation will persist. + ...(Object.hasOwn(input, "autoCompactionThresholdPct") + ? { autoCompactionThresholdPct: input.autoCompactionThresholdPct ?? null } + : {}), status: input.status, completionSummary: input.completionSummary, }); @@ -1681,6 +1717,13 @@ export class WorkspaceGoalService { ? { budgetCents: input.budgetCents ?? null } : {}), ...(Object.hasOwn(input, "turnCap") ? { turnCap: input.turnCap ?? null } : {}), + // Round-trip the per-goal auto-compact override through the + // mid-stream queue so a deferred drain in + // `applyPendingAfterStreamEnd` writes the threshold the user + // requested. `null` = clear; omitted = no change. + ...(Object.hasOwn(input, "autoCompactionThresholdPct") + ? { autoCompactionThresholdPct: input.autoCompactionThresholdPct ?? null } + : {}), ...(input.status != null ? { status: input.status } : {}), ...(input.completionSummary != null ? { completionSummary: input.completionSummary } @@ -1811,6 +1854,10 @@ export class WorkspaceGoalService { input.completionSummary != null || Object.hasOwn(input, "budgetCents") || Object.hasOwn(input, "turnCap") || + // Per-goal auto-compact override is a mutable field too — a + // same-objective setGoal that only changes the threshold must + // trip the "apply" branch instead of being treated as a no-op. + Object.hasOwn(input, "autoCompactionThresholdPct") || Object.hasOwn(input, "requireUserAcknowledgmentSinceMs"); const previousStatus = current.status; let updated = hasMutableChange ? this.applyMutableFields(current, input) : current; @@ -1862,6 +1909,12 @@ export class WorkspaceGoalService { objective, budgetCents: input.budgetCents ?? null, turnCap: input.turnCap ?? null, + // Forward the per-goal auto-compact override on replace too so a + // /goal command (or palette create) that includes `--compact N` + // lands the value on the fresh record. `null` = explicit clear. + ...(Object.hasOwn(input, "autoCompactionThresholdPct") + ? { autoCompactionThresholdPct: input.autoCompactionThresholdPct ?? null } + : {}), status: input.status, completionSummary: input.completionSummary, }); @@ -2712,6 +2765,7 @@ export class WorkspaceGoalService { objective: string; budgetCents?: number | null; turnCap?: number | null; + autoCompactionThresholdPct?: number | null; }): Promise { this.assertParentWorkspace(input.workspaceId); const objective = input.objective.trim(); @@ -2723,6 +2777,12 @@ export class WorkspaceGoalService { objective, budgetCents: input.budgetCents ?? null, turnCap: input.turnCap ?? null, + // Carry the per-goal auto-compact override onto the upcoming + // record so it survives the eventual `promoteUpcomingGoal` → + // `setGoal` path without forcing the renderer to re-submit it. + ...(Object.hasOwn(input, "autoCompactionThresholdPct") + ? { autoCompactionThresholdPct: input.autoCompactionThresholdPct ?? null } + : {}), // `paused` is the placeholder status for upcoming goals — they // are not actively running and not yet acknowledged by the // agent. The promote path replaces this with `active` after a @@ -2758,6 +2818,7 @@ export class WorkspaceGoalService { objective?: string; budgetCents?: number | null; turnCap?: number | null; + autoCompactionThresholdPct?: number | null; }): Promise { this.assertParentWorkspace(input.workspaceId); if (input.objective?.trim().length === 0) { @@ -2773,6 +2834,12 @@ export class WorkspaceGoalService { objective: input.objective === undefined ? existing.objective : input.objective.trim(), budgetCents: input.budgetCents === undefined ? existing.budgetCents : input.budgetCents, turnCap: input.turnCap === undefined ? existing.turnCap : input.turnCap, + // Tri-state matches the rest of the patch fields: `undefined` = + // preserve, explicit `null` = clear, number = set. + autoCompactionThresholdPct: + input.autoCompactionThresholdPct === undefined + ? existing.autoCompactionThresholdPct + : input.autoCompactionThresholdPct, updatedAtMs: Date.now(), }); const nextUpcoming = [...board.upcoming]; From 56d33a29098c77299d79a1e33f402ce878e2671d Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 21 May 2026 12:21:51 -0500 Subject: [PATCH 2/6] tests(goals): cover per-goal auto-compact override end-to-end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the data model + behavior added in the previous commit: * CompactionMonitor: per-call `thresholdOverride` wins over internal threshold for both `checkBeforeSend` and `checkMidStream`; override of `1.0` disables for that call; null/NaN/non-finite/non-positive falls back to the monitor's own threshold (defensive against corrupt persisted goal records). * WorkspaceGoalService: `setGoal` persists the field, `applyMutableFields` edit-in-place can clear it (null) without dropping unrelated state, 100 round-trips distinct from null (per-goal disable vs. no override), `addUpcomingGoal` / `updateUpcomingGoal` honor the field, and legacy goal records without the field still load. * /goal slash command: integer / off / disable / default / none / clear / 75%, composes with -b and --turns in documented order, rejects out-of-range values, treats non-leading occurrences as goal text. * GoalTab UI: tile renders Default / Off / N% across the tri-state, inline editor submits the percent, "off" → 100, blank → null, out-of-range surfaces an error, Edit affordance is gated on the handler being supplied. --- .../features/RightSidebar/GoalTab.test.tsx | 151 +++++++++++++++++ src/browser/utils/slashCommands/goal.test.ts | 92 ++++++++++ src/node/services/compactionMonitor.test.ts | 121 +++++++++++++ .../services/workspaceGoalService.test.ts | 159 ++++++++++++++++++ 4 files changed, 523 insertions(+) diff --git a/src/browser/features/RightSidebar/GoalTab.test.tsx b/src/browser/features/RightSidebar/GoalTab.test.tsx index 44e6ce660d..c4d6aed7d5 100644 --- a/src/browser/features/RightSidebar/GoalTab.test.tsx +++ b/src/browser/features/RightSidebar/GoalTab.test.tsx @@ -403,6 +403,157 @@ describe("GoalTab", () => { await waitFor(() => expect(onUpdateTurnCap).toHaveBeenCalledWith(15)); }); + test("auto-compact tile shows 'Default' when no per-goal override is set", () => { + const { getByText, queryByText } = render( + + ); + + // Null override → fall back to the workspace setting. The tile + // must distinguish "no override" from "explicit value" so users + // can tell at a glance whether the model-level slider is in effect. + expect(getByText("Auto-compact")).toBeTruthy(); + expect(getByText("Default")).toBeTruthy(); + expect(getByText("workspace setting")).toBeTruthy(); + expect(queryByText("Off")).toBeNull(); + }); + + test("auto-compact tile shows 'Off' at the per-goal-disabled sentinel", () => { + const { getByText, queryByText } = render( + + ); + + // 100 = explicit per-goal disable. Distinct from null (default) + // and from 0–99 (numeric threshold). The "Off" label is the + // user-visible cue that this specific goal won't auto-compact. + expect(getByText("Off")).toBeTruthy(); + expect(getByText("compaction disabled")).toBeTruthy(); + expect(queryByText("Default")).toBeNull(); + }); + + test("auto-compact tile renders an explicit numeric override", () => { + const { getByText } = render( + + ); + + expect(getByText("50%")).toBeTruthy(); + expect(getByText("of context window")).toBeTruthy(); + }); + + test("edits auto-compact threshold inline and persists the percent", async () => { + const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); + const { getByLabelText, getByText } = render( + + ); + + fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); + const input = getByLabelText("Goal auto-compact threshold percent"); + await waitFor(() => expect(document.activeElement).toBe(input)); + fireEvent.input(input, { target: { value: "60" } }); + fireEvent.click(getByText("Save threshold")); + + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(60)); + }); + + test("editing auto-compact with 'off' submits the per-goal-disabled sentinel (100)", async () => { + const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); + const { getByLabelText, getByText } = render( + + ); + + fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); + const input = getByLabelText("Goal auto-compact threshold percent"); + fireEvent.input(input, { target: { value: "off" } }); + fireEvent.click(getByText("Save threshold")); + + // "off" must round-trip to 100, not to null — these are different + // states (explicit disable vs. clear override). The parser path + // and the test path must agree. + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(100)); + }); + + test("editing auto-compact with a blank value clears the override (null)", async () => { + const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); + const { getByLabelText, getByText } = render( + + ); + + fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); + const input = getByLabelText("Goal auto-compact threshold percent"); + fireEvent.input(input, { target: { value: "" } }); + fireEvent.click(getByText("Save threshold")); + + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(null)); + }); + + test("auto-compact editor surfaces an error for out-of-range input", async () => { + const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); + const { getByLabelText, getByText, queryByText } = render( + + ); + + fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); + const input = getByLabelText("Goal auto-compact threshold percent"); + fireEvent.input(input, { target: { value: "150" } }); + fireEvent.click(getByText("Save threshold")); + + // 150 is out of range → handler should not be called and the + // editor must surface the help text as a visible error message. + await waitFor(() => expect(queryByText(/0[–-]100/)).toBeTruthy()); + expect(onUpdateAutoCompactionThresholdPct).not.toHaveBeenCalled(); + }); + + test("auto-compact tile hides the Edit affordance when the handler is omitted", () => { + // Read-only stories / storybook variants render without + // `onUpdateAutoCompactionThresholdPct`. The tile still shows the + // value, but the Edit button is gated so users can't open an + // editor that has nowhere to submit. + const { queryByLabelText, getByText } = render( + + ); + + expect(getByText("50%")).toBeTruthy(); + expect(queryByLabelText("Edit goal auto-compact threshold")).toBeNull(); + }); + test("opens completion summary input, traps focus, submits, and restores focus", async () => { const onSetStatus = mock(() => Promise.resolve(undefined)); const { getByLabelText, getByText, queryByLabelText } = render( diff --git a/src/browser/utils/slashCommands/goal.test.ts b/src/browser/utils/slashCommands/goal.test.ts index d08718c77a..01b7592c20 100644 --- a/src/browser/utils/slashCommands/goal.test.ts +++ b/src/browser/utils/slashCommands/goal.test.ts @@ -175,4 +175,96 @@ describe("/goal slash command", () => { command: "goal", }); }); + + // ──────────────────────────────────────────────────────────────── + // /goal --compact + // + // Mirrors the inline editor / create-form vocabulary so the slash, + // palette, and tab paths agree on what the user can type. Order- + // sensitive: must appear after `--turns` (or after `-b`) and before + // the objective. + // ──────────────────────────────────────────────────────────────── + test("parses --compact with an integer percent", () => { + expect(parseCommand("/goal --compact 50 Ship the slice")).toEqual({ + type: "goal-set", + objective: "Ship the slice", + autoCompactionThresholdPct: 50, + }); + // `100` (per-goal disabled) is distinct from "off" but both must + // resolve to the same value so /goal --compact 100 and /goal + // --compact off produce identical persisted state. + expect(parseCommand("/goal --compact 100 Long context")).toEqual({ + type: "goal-set", + objective: "Long context", + autoCompactionThresholdPct: 100, + }); + }); + + test("parses --compact off / disable / disabled as 100 (per-goal disabled)", () => { + for (const word of ["off", "disable", "disabled"]) { + expect(parseCommand(`/goal --compact ${word} Stay long`)).toEqual({ + type: "goal-set", + objective: "Stay long", + autoCompactionThresholdPct: 100, + }); + } + }); + + test("parses --compact default / none / clear as null (clear override)", () => { + for (const word of ["default", "none", "clear"]) { + expect(parseCommand(`/goal --compact ${word} Use workspace setting`)).toEqual({ + type: "goal-set", + objective: "Use workspace setting", + autoCompactionThresholdPct: null, + }); + } + }); + + test("accepts a trailing percent sign", () => { + expect(parseCommand("/goal --compact 75% Tight context")).toEqual({ + type: "goal-set", + objective: "Tight context", + autoCompactionThresholdPct: 75, + }); + }); + + test("composes with -b and --turns in the documented order", () => { + // The parser walks `-b → --turns → --compact → objective` exactly + // in that sequence; pin that order here so a future refactor can't + // silently break it. + expect(parseCommand("/goal -b $5 --turns 10 --compact 60 Run the full plan")).toEqual({ + type: "goal-set", + objective: "Run the full plan", + budgetCents: 500, + turnCap: 10, + autoCompactionThresholdPct: 60, + }); + }); + + test("rejects --compact with invalid values", () => { + expect(parseCommand("/goal --compact 200 oops")).toMatchObject({ + type: "command-invalid-args", + command: "goal", + }); + expect(parseCommand("/goal --compact abc oops")).toMatchObject({ + type: "command-invalid-args", + command: "goal", + }); + // Missing value falls through to "abc" as the rejected token — + // the test below uses an explicit dangling flag form. + expect(parseCommand("/goal --compact -1 oops")).toMatchObject({ + type: "command-invalid-args", + command: "goal", + }); + }); + + test("treats non-leading --compact tokens as goal text", () => { + // Mirrors the same defensive behavior `--budget` already has — + // only the leading flag prefix is command syntax; anything after + // the objective starts is preserved verbatim. + expect(parseCommand("/goal Ship it --compact 50 stays prose")).toEqual({ + type: "goal-set", + objective: "Ship it --compact 50 stays prose", + }); + }); }); diff --git a/src/node/services/compactionMonitor.test.ts b/src/node/services/compactionMonitor.test.ts index 0f84fb6a33..05f6f2ee93 100644 --- a/src/node/services/compactionMonitor.test.ts +++ b/src/node/services/compactionMonitor.test.ts @@ -199,4 +199,125 @@ describe("CompactionMonitor", () => { monitor.setThreshold(0.55); expect(monitor.getThreshold()).toBe(0.55); }); + + // ──────────────────────────────────────────────────────────────── + // thresholdOverride lane (per-goal auto-compact override). + // + // Behavioral contract these tests pin down: + // 1. A finite, in-range override replaces this.threshold for the + // single call. The monitor's own threshold is untouched. + // 2. Override `>= 1` (per-goal disabled / clamp) suppresses + // compaction for that call, even if the monitor's own threshold + // would normally fire. + // 3. `null` / `undefined` / non-finite / non-positive override + // falls back to `this.threshold` so a stale or corrupt persisted + // goal record cannot brick the compaction loop. + // 4. `checkBeforeSend` surfaces the effective threshold via + // `thresholdPercentage` so the on-send branch in `AgentSession` + // reads the right number. + // ──────────────────────────────────────────────────────────────── + test("checkBeforeSend honors a per-call thresholdOverride for the active goal", () => { + const { monitor } = createMonitor(); + + // 75% usage with a 0.5 (=50%) override should force-compact even + // though the monitor's own default (70%) would not yet have hit + // the buffer-driven force threshold. + const result = monitor.checkBeforeSend({ + model: BETA_SONNET_MODEL, + usage: { lastContextUsage: createUsageDisplay(150_000) }, + use1MContext: false, + providersConfig: null, + thresholdOverride: 0.5, + }); + expect(result.thresholdPercentage).toBe(50); + expect(result.shouldForceCompact).toBe(true); + + // The monitor's own threshold must NOT have been mutated by the + // override — it stays at the default 70% for subsequent calls + // without an override. + expect(monitor.getThreshold()).toBe(DEFAULT_THRESHOLD); + const followUp = monitor.checkBeforeSend({ + model: BETA_SONNET_MODEL, + usage: { lastContextUsage: createUsageDisplay(150_000) }, + use1MContext: false, + providersConfig: null, + }); + expect(followUp.thresholdPercentage).toBe(70); + }); + + test("checkBeforeSend treats an override of 1.0 as disabled for that call", () => { + const { monitor } = createMonitor(); + + // 90% usage would force-compact under the default 70% threshold, + // but an override of 1.0 (per-goal disabled) must zero out the + // result and skip the compaction signal entirely. + const result = monitor.checkBeforeSend({ + model: BETA_SONNET_MODEL, + usage: { lastContextUsage: createUsageDisplay(180_000) }, + use1MContext: false, + providersConfig: null, + thresholdOverride: 1, + }); + expect(result.shouldForceCompact).toBe(false); + expect(result.shouldShowWarning).toBe(false); + }); + + test("checkMidStream honors per-call thresholdOverride", () => { + const { monitor, statusEvents } = createMonitor(); + + // 100k / 200k = 50%. Under the monitor's default (70% + 5% buffer + // = 75% force), this would NOT fire. With an override of 0.4 + // (40% + 5% buffer = 45% force), 50% must trigger. + expect( + monitor.checkMidStream({ + model: BETA_SONNET_MODEL, + usage: createMidStreamUsage(100_000), + use1MContext: false, + providersConfig: null, + thresholdOverride: 0.4, + }) + ).toBe(true); + expect(statusEvents).toHaveLength(1); + expect(statusEvents[0]).toMatchObject({ usagePercent: 50 }); + }); + + test("checkMidStream override of 1.0 short-circuits even when usage is high", () => { + const { monitor, statusEvents } = createMonitor(); + + // 95% usage would normally force-compact (under the 70% default + // threshold the force point is 75%). Override 1 = per-goal off. + expect( + monitor.checkMidStream({ + model: BETA_SONNET_MODEL, + usage: createMidStreamUsage(190_000), + use1MContext: false, + providersConfig: null, + thresholdOverride: 1, + }) + ).toBe(false); + expect(statusEvents).toHaveLength(0); + }); + + test("invalid threshold overrides fall back to the monitor's own threshold", () => { + const { monitor } = createMonitor(); + + // null, NaN, negatives, and 0 must all degrade gracefully to the + // monitor's own threshold rather than throwing — these can come + // from a corrupt persisted goal record and must not interrupt + // the on-send / mid-stream pipelines. + for (const bad of [null, Number.NaN, Number.POSITIVE_INFINITY, -0.1, 0]) { + const result = monitor.checkBeforeSend({ + model: BETA_SONNET_MODEL, + usage: { lastContextUsage: createUsageDisplay(150_000) }, + use1MContext: false, + providersConfig: null, + thresholdOverride: bad, + }); + expect(result.thresholdPercentage).toBe(70); + } + }); }); + +// Pulled out as a constant for readability in the override-vs-default +// assertions; matches the monitor's initial value. +const DEFAULT_THRESHOLD = 0.7; diff --git a/src/node/services/workspaceGoalService.test.ts b/src/node/services/workspaceGoalService.test.ts index 71269bb78b..62fdabb5d4 100644 --- a/src/node/services/workspaceGoalService.test.ts +++ b/src/node/services/workspaceGoalService.test.ts @@ -3239,4 +3239,163 @@ describe("WorkspaceGoalService", () => { expect(patched?.budgetCents).toBeNull(); }); }); + + // ──────────────────────────────────────────────────────────────── + // Per-goal auto-compaction threshold override. + // + // The field is tri-state: omitted (key not on the input) = no change, + // `null` = clear, integer 0–100 = explicit percent. Pinned here for + // every mutator that has its own assignment path so a future refactor + // can't silently drop one. Legacy/loading compatibility is covered by + // a schema-level test in the GoalRecordV1Schema describe block at the + // bottom of this file. + // ──────────────────────────────────────────────────────────────── + describe("autoCompactionThresholdPct", () => { + test("setGoal persists the per-goal override and applyMutableFields can clear it", async () => { + const created = await setGoalOk(service, { + workspaceId, + objective: "Long research goal", + autoCompactionThresholdPct: 40, + }); + expect(created.autoCompactionThresholdPct).toBe(40); + + // applyMutableFields branch: editInPlace replace with the same + // objective should patch the threshold without touching budget / + // turnCap / costs. + const updated = await setGoalOk(service, { + workspaceId, + objective: created.objective, + autoCompactionThresholdPct: 80, + }); + expect(updated.goalId).toBe(created.goalId); + expect(updated.autoCompactionThresholdPct).toBe(80); + + // Explicit `null` clears the override; the field round-trips as + // `null` on the persisted record so the renderer's tile shows + // "Default" again. + const cleared = await setGoalOk(service, { + workspaceId, + objective: created.objective, + autoCompactionThresholdPct: null, + }); + expect(cleared.autoCompactionThresholdPct).toBeNull(); + }); + + test("setGoal without the key leaves the existing override intact", async () => { + const created = await setGoalOk(service, { + workspaceId, + objective: "Aggressive compaction goal", + autoCompactionThresholdPct: 30, + }); + + // An unrelated mutation (budget change) must not silently drop + // the per-goal threshold — this is the key reason `applyMutableFields` + // uses `Object.hasOwn` instead of a truthy check. + const budgetUpdated = await setGoalOk(service, { + workspaceId, + objective: created.objective, + budgetCents: 1000, + }); + expect(budgetUpdated.autoCompactionThresholdPct).toBe(30); + expect(budgetUpdated.budgetCents).toBe(1000); + }); + + test("setGoal value 100 round-trips (per-goal disable, distinct from clear)", async () => { + // 100 (per-goal disabled) and `null` (no override) are visually + // similar in the UI but mean very different things on the backend: + // 100 must override the workspace setting to disable compaction + // for this specific goal, while `null` falls through. The + // persistence layer must preserve that distinction. + const created = await setGoalOk(service, { + workspaceId, + objective: "No-compaction goal", + autoCompactionThresholdPct: 100, + }); + expect(created.autoCompactionThresholdPct).toBe(100); + + const reloaded = await service.getGoal(workspaceId); + expect(reloaded?.autoCompactionThresholdPct).toBe(100); + }); + + test("addUpcomingGoal seeds the override on the queued record", async () => { + const queued = await service.addUpcomingGoal({ + workspaceId, + objective: "Queued with threshold", + autoCompactionThresholdPct: 60, + }); + expect(queued.autoCompactionThresholdPct).toBe(60); + + const board = await service.getGoalBoard(workspaceId); + const found = board.entries.find( + (entry) => entry.section === "upcoming" && entry.goal.goalId === queued.goalId + ); + expect(found?.goal.autoCompactionThresholdPct).toBe(60); + }); + + test("updateUpcomingGoal patches the override without touching unrelated fields", async () => { + const queued = await service.addUpcomingGoal({ + workspaceId, + objective: "Patchable", + budgetCents: 500, + autoCompactionThresholdPct: 50, + }); + + // Patch only the threshold; budget / objective must stay put. + const patched = await service.updateUpcomingGoal({ + workspaceId, + goalId: queued.goalId, + autoCompactionThresholdPct: 90, + }); + expect(patched?.autoCompactionThresholdPct).toBe(90); + expect(patched?.budgetCents).toBe(500); + expect(patched?.objective).toBe("Patchable"); + + // And `null` clears the override on an upcoming goal too. + const cleared = await service.updateUpcomingGoal({ + workspaceId, + goalId: queued.goalId, + autoCompactionThresholdPct: null, + }); + expect(cleared?.autoCompactionThresholdPct).toBeNull(); + expect(cleared?.budgetCents).toBe(500); + }); + }); + + describe("autoCompactionThresholdPct legacy compatibility", () => { + test("legacy goal records without the field still parse and load", async () => { + // Simulate a legacy persisted goal record by writing a raw + // goal.json that pre-dates the field. The schema must accept + // it (the field is optional + nullable) and `getGoal` must + // return the parsed record with the field absent. This is + // the back-compat guarantee that lets us roll out the feature + // without a migration step. + const sessionDir = config.getSessionDir(workspaceId); + await fs.mkdir(sessionDir, { recursive: true }); + const legacyRecord = { + version: 1, + goalId: "11111111-1111-4111-8111-111111111111", + objective: "Legacy goal", + status: "active", + budgetCents: null, + turnCap: null, + costCents: 0, + costMicroCents: 0, + turnsUsed: 0, + attributedChildren: [], + budgetLimitInjectedForGoalId: null, + requireUserAcknowledgmentSinceMs: null, + createdAtMs: 1_700_000_000_000, + updatedAtMs: 1_700_000_000_000, + }; + await fs.writeFile(path.join(sessionDir, "goal.json"), JSON.stringify(legacyRecord, null, 2)); + + const loaded = await service.getGoal(workspaceId); + expect(loaded).not.toBeNull(); + expect(loaded?.goalId).toBe(legacyRecord.goalId); + // No `autoCompactionThresholdPct` key means "no override" — the + // renderer/backend treat this the same as `null` (workspace + // setting applies). + expect(loaded?.autoCompactionThresholdPct).toBeUndefined(); + }); + }); }); From 307c817c162d4eaef55557dfa83806b00d1d4e23 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 21 May 2026 12:23:38 -0500 Subject: [PATCH 3/6] fix(goals): escape ASCII quotes in create-form helper to satisfy lint --- src/browser/features/RightSidebar/GoalTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/features/RightSidebar/GoalTab.tsx b/src/browser/features/RightSidebar/GoalTab.tsx index 1a0e055b73..82a5791978 100644 --- a/src/browser/features/RightSidebar/GoalTab.tsx +++ b/src/browser/features/RightSidebar/GoalTab.tsx @@ -1371,7 +1371,7 @@ function GoalCreateForm(props: GoalCreateFormProps) { defaultValue="" /> - 0–99 to compact at that % of context, 100 / "off" to disable for this goal. + 0–99 to compact at that % of context, 100 / "off" to disable for this goal.
From 0dfbdc7699f623473e8501e06e480a5626d51d81 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 21 May 2026 12:34:20 -0500 Subject: [PATCH 4/6] fix(goals): honor 0% per-goal compaction override (Codex P2) `resolveEffectiveThreshold` was rejecting `override <= 0` and falling back to the workspace threshold, but the schema admits `0` as a valid per-goal value (the aggressive extreme of the slider). That created a renderer/backend mismatch: the GoalTab tile shows "0%" and the GoalTab editor accepts the value, but the backend silently behaved as if the override was absent. Narrow the corruption guard to negative values only and document why `0` deserves an explicit pass. Add a dedicated test pinning the contract for both `checkBeforeSend` and `checkMidStream`. Picked up via PR #3357 Codex review. --- src/node/services/compactionMonitor.test.ts | 55 +++++++++++++++++++-- src/node/services/compactionMonitor.ts | 17 +++++-- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/node/services/compactionMonitor.test.ts b/src/node/services/compactionMonitor.test.ts index 05f6f2ee93..f68e619f01 100644 --- a/src/node/services/compactionMonitor.test.ts +++ b/src/node/services/compactionMonitor.test.ts @@ -301,11 +301,16 @@ describe("CompactionMonitor", () => { test("invalid threshold overrides fall back to the monitor's own threshold", () => { const { monitor } = createMonitor(); - // null, NaN, negatives, and 0 must all degrade gracefully to the - // monitor's own threshold rather than throwing — these can come - // from a corrupt persisted goal record and must not interrupt - // the on-send / mid-stream pipelines. - for (const bad of [null, Number.NaN, Number.POSITIVE_INFINITY, -0.1, 0]) { + // null, NaN, ±Infinity, and negatives must all degrade gracefully + // to the monitor's own threshold rather than throwing — these can + // come from a corrupt persisted goal record and must not interrupt + // the on-send / mid-stream pipelines. `0` is intentionally NOT in + // this list — see the dedicated "0% override compacts on every + // check" test below. The schema admits 0 as the aggressive extreme, + // so treating it as invalid would create a renderer/backend + // mismatch (the UI shows "0%" but the backend would behave like + // the override was absent). + for (const bad of [null, Number.NaN, Number.POSITIVE_INFINITY, -0.1]) { const result = monitor.checkBeforeSend({ model: BETA_SONNET_MODEL, usage: { lastContextUsage: createUsageDisplay(150_000) }, @@ -316,6 +321,46 @@ describe("CompactionMonitor", () => { expect(result.thresholdPercentage).toBe(70); } }); + + test("override of 0% is honored (Codex P2: renderer/backend must agree)", () => { + const { monitor, statusEvents } = createMonitor(); + + // Codex review on PR #3357 caught that `resolveEffectiveThreshold` + // was rejecting `override <= 0`, but the schema admits `0` as a + // valid per-goal value (the aggressive extreme of the slider). The + // contract is: if the UI lets the user set 0 and persist it, the + // backend must honor it. A silent fallback to the workspace setting + // creates a renderer/backend mismatch the user has no way to + // diagnose. This test pins both code paths. + const beforeSend = monitor.checkBeforeSend({ + model: BETA_SONNET_MODEL, + usage: { lastContextUsage: createUsageDisplay(20_000) }, + use1MContext: false, + providersConfig: null, + thresholdOverride: 0, + }); + // The critical assertion: the effective threshold is 0%, not the + // workspace default (70%). 10% usage is above the 0% + 5% buffer + // force point so shouldForceCompact must trip too. + expect(beforeSend.thresholdPercentage).toBe(0); + expect(beforeSend.usagePercentage).toBe(10); + expect(beforeSend.shouldForceCompact).toBe(true); + + // Mid-stream: 20k tokens = 10% of the 200k context, above the + // 0% + 5% buffer force point. Without the fix this would be false + // (override <= 0 was treated as invalid and the monitor's default + // 70% threshold would govern). + expect( + monitor.checkMidStream({ + model: BETA_SONNET_MODEL, + usage: createMidStreamUsage(20_000), + use1MContext: false, + providersConfig: null, + thresholdOverride: 0, + }) + ).toBe(true); + expect(statusEvents).toHaveLength(1); + }); }); // Pulled out as a constant for readability in the override-vs-default diff --git a/src/node/services/compactionMonitor.ts b/src/node/services/compactionMonitor.ts index 00a0a3f874..4cecb1394d 100644 --- a/src/node/services/compactionMonitor.ts +++ b/src/node/services/compactionMonitor.ts @@ -169,18 +169,25 @@ export class CompactionMonitor { /** * Pick the threshold that should govern this single check. A finite, - * positive override (passed by the caller, typically derived from the + * in-range override (passed by the caller, typically derived from the * active goal's `autoCompactionThresholdPct`) wins over the monitor's - * own per-workspace value. We intentionally keep validation loose here: - * `null` / `undefined` / non-finite / non-positive values fall back to - * `this.threshold` instead of throwing, because the override is + * own per-workspace value. We intentionally keep validation loose + * here: `null` / `undefined` / non-finite / negative values fall back + * to `this.threshold` instead of throwing, because the override is * sourced from optional persisted state and a malformed entry must * never brick the compaction pipeline mid-stream. + * + * `0` is honored as "compact at 0% context" (i.e. compact on every + * send) — the schema admits it as a valid extreme of the aggressive + * end of the slider, and treating it as a fallback would silently + * make the renderer and backend disagree on what the override means + * (renderer renders "0%", backend behaves as workspace default). */ private resolveEffectiveThreshold(override: number | null | undefined): number { if (override == null) return this.threshold; if (!Number.isFinite(override)) return this.threshold; - if (override <= 0) return this.threshold; + // Only negative values are corrupt; `0` is a valid per-goal extreme. + if (override < 0) return this.threshold; // Clamp the upper bound so an out-of-range "200%" override behaves the // same way `setThreshold(1)` would: compaction disabled, not skewed // higher than the model's actual context. From d3f7bcb1023226456c14e6a4f0225693ac06b70c Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 21 May 2026 18:10:01 -0500 Subject: [PATCH 5/6] refactor(goals): slider-driven per-goal auto-compact tile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the text-input editor for the per-goal auto-compaction override with an embedded slider tile and reshape the active-goal grid to be container-query responsive. UX: - Default mode (no override): tile shows 'Default — workspace setting' with a 'Customize' button. Slider is hidden because we don't know the workspace per-model threshold at this layer and a ghost thumb would imply a value that isn't really in effect. - Override mode (value is a number): native range slider with thumb at the override value. Drag commits on release (mouseUp/touchEnd/ keyUp), not on every drag step, so backend mutation rate stays bounded by user gestures. 'Use default' clears back to null. - The right end of the slider is labeled 'Off' to mirror the workspace per-model slider — dragging there commits 100 (per-goal disabled). Layout: - Active-goal grid now uses '[container-type:inline-size]' + '@md:' so the sidebar's dynamic width (per AGENTS.md) drives the breakpoint, not the viewport. Narrow stays 2-col (Budget full row, Turns | Elapsed, Auto-compact full row). Wider goes to 3-col with Budget | Turns | Auto-compact on one row and Elapsed as a thin full-width row below. - BudgetTile internal copy stacks vertically at the wider breakpoint so its cost / remaining pair still fits in ~1/3 width. - GoalCreateForm reuses the same slider, with local state instead of the previous text-input + parser path. Drop: - 'compactThreshold' EditingField variant + the inline text editor. - parseGoalAutoCompactionThresholdInput (no longer used inside GoalTab; the slash-command parser is separate). Tests: - Replace text-input drive tests with slider-drive tests: Default mode / Off display / numeric override display; Customize commits 70; drag commits snapped value on mouseUp; drag-to-100 commits 100 (Off); mouseUp without change does NOT commit (anti-spam guard); Use default clears to null; canEdit gating hides Customize and disables the slider. - Use 'onInput' + 'onChange' both — happy-dom only synthesizes the native input event for range inputs, so onInput is required for tests, and onChange satisfies React's controlled-input contract. Validation: typecheck \u2713 lint \u2713 fmt-check \u2713 storybook-build \u2713 bun test src/node/services/ src/browser/features/RightSidebar/ src/browser/utils/slashCommands/ src/browser/utils/goals/ — 3554 pass. --- _Generated with `mux` \u2022 Model: `anthropic:claude-opus-4-7` \u2022 Thinking: `xhigh` \u2022 Cost: `$53.97`_ --- .../features/RightSidebar/GoalTab.test.tsx | 172 +++++-- src/browser/features/RightSidebar/GoalTab.tsx | 481 +++++++++++------- 2 files changed, 415 insertions(+), 238 deletions(-) diff --git a/src/browser/features/RightSidebar/GoalTab.test.tsx b/src/browser/features/RightSidebar/GoalTab.test.tsx index c4d6aed7d5..8fa2bd7595 100644 --- a/src/browser/features/RightSidebar/GoalTab.test.tsx +++ b/src/browser/features/RightSidebar/GoalTab.test.tsx @@ -403,8 +403,19 @@ describe("GoalTab", () => { await waitFor(() => expect(onUpdateTurnCap).toHaveBeenCalledWith(15)); }); - test("auto-compact tile shows 'Default' when no per-goal override is set", () => { - const { getByText, queryByText } = render( + // ──────────────────────────────────────────────────────────────── + // GoalAutoCompactSlider tile (per-goal auto-compaction override). + // + // The tile has two visual modes — "Default" (no override; renders a + // Customize button) and "Override" (slider visible; renders Use + // default + Off label). These tests pin the user-visible behavior of + // both modes plus the mode transitions; rendering literals like + // "workspace setting" / "of context window" are asserted because they + // map 1:1 to the tri-state contract the backend honors, NOT as copy + // tests — change the contract and the test should re-validate. + // ──────────────────────────────────────────────────────────────── + test("auto-compact tile renders Default mode with a Customize button when no override is set", () => { + const { getByText, queryByText, getByLabelText, queryByLabelText } = render( { ); // Null override → fall back to the workspace setting. The tile - // must distinguish "no override" from "explicit value" so users - // can tell at a glance whether the model-level slider is in effect. + // must distinguish "no override" from "explicit value", and must + // NOT show the slider — we have no concrete value to anchor it + // against in Default mode (see the component's docblock). expect(getByText("Auto-compact")).toBeTruthy(); expect(getByText("Default")).toBeTruthy(); expect(getByText("workspace setting")).toBeTruthy(); expect(queryByText("Off")).toBeNull(); + expect(getByLabelText("Customize goal auto-compact threshold")).toBeTruthy(); + // Slider is hidden in Default mode. + expect(queryByLabelText("Goal auto-compact threshold percent")).toBeNull(); }); - test("auto-compact tile shows 'Off' at the per-goal-disabled sentinel", () => { - const { getByText, queryByText } = render( + test("auto-compact tile shows the slider with Off labeling at the per-goal-disabled sentinel", () => { + const { getAllByText, getByText, getByLabelText, queryByText } = render( { ); // 100 = explicit per-goal disable. Distinct from null (default) - // and from 0–99 (numeric threshold). The "Off" label is the - // user-visible cue that this specific goal won't auto-compact. - expect(getByText("Off")).toBeTruthy(); + // and from 0–99 (numeric threshold). At this value both the + // primary value display AND the right-end slider label read "Off" + // — the redundancy is intentional (the right-end label is also a + // drag target). Using `getAllByText` documents that contract. + expect(getAllByText("Off").length).toBe(2); expect(getByText("compaction disabled")).toBeTruthy(); expect(queryByText("Default")).toBeNull(); + const slider = getByLabelText("Goal auto-compact threshold percent") as HTMLInputElement; + expect(slider.value).toBe("100"); }); - test("auto-compact tile renders an explicit numeric override", () => { - const { getByText } = render( + test("auto-compact tile renders the slider at an explicit numeric override", () => { + const { getByText, getByLabelText } = render( { expect(getByText("50%")).toBeTruthy(); expect(getByText("of context window")).toBeTruthy(); + const slider = getByLabelText("Goal auto-compact threshold percent") as HTMLInputElement; + expect(slider.value).toBe("50"); }); - test("edits auto-compact threshold inline and persists the percent", async () => { + test("clicking Customize commits an override at the global default percent (70)", async () => { const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); - const { getByLabelText, getByText } = render( + const { getByLabelText } = render( { /> ); - fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); - const input = getByLabelText("Goal auto-compact threshold percent"); - await waitFor(() => expect(document.activeElement).toBe(input)); - fireEvent.input(input, { target: { value: "60" } }); - fireEvent.click(getByText("Save threshold")); + // Customize is the explicit "I want to override the workspace + // setting" affordance. It seeds the override at 70 so the slider + // (which only renders in Override mode) has a concrete value to + // drag from — the user is one click away from a usable slider + // rather than having to type a starting number. + fireEvent.click(getByLabelText("Customize goal auto-compact threshold")); - await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(60)); + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(70)); }); - test("editing auto-compact with 'off' submits the per-goal-disabled sentinel (100)", async () => { + test("dragging the slider commits the snapped value on release", async () => { const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); - const { getByLabelText, getByText } = render( + const { getByLabelText } = render( ); - fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); - const input = getByLabelText("Goal auto-compact threshold percent"); - fireEvent.input(input, { target: { value: "off" } }); - fireEvent.click(getByText("Save threshold")); + const slider = getByLabelText("Goal auto-compact threshold percent"); + // React unifies range-input `change` and `input` events into one + // `onChange` handler driven by the native `input` event (so the + // value updates continuously during drag like a text input). Use + // `fireEvent.input` here — `fireEvent.change` doesn't actually + // trigger React's onChange for `type="range"`. + fireEvent.input(slider, { target: { value: "50" } }); + expect(onUpdateAutoCompactionThresholdPct).not.toHaveBeenCalled(); - // "off" must round-trip to 100, not to null — these are different - // states (explicit disable vs. clear override). The parser path - // and the test path must agree. - await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(100)); + fireEvent.mouseUp(slider); + + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(50)); }); - test("editing auto-compact with a blank value clears the override (null)", async () => { + test("dragging the slider to the right end commits the Off sentinel (100)", async () => { const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); - const { getByLabelText, getByText } = render( + const { getByLabelText } = render( ); - fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); - const input = getByLabelText("Goal auto-compact threshold percent"); - fireEvent.input(input, { target: { value: "" } }); - fireEvent.click(getByText("Save threshold")); + const slider = getByLabelText("Goal auto-compact threshold percent"); + fireEvent.input(slider, { target: { value: "100" } }); + fireEvent.mouseUp(slider); - await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(null)); + // 100 must reach the backend as the explicit per-goal-disabled + // sentinel, distinct from `null` (workspace setting applies). The + // monitor honors 100 by short-circuiting compaction; if we sent + // null instead, the workspace's per-model slider would govern and + // the user would see compaction fire unexpectedly. + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(100)); }); - test("auto-compact editor surfaces an error for out-of-range input", async () => { + test("releasing the slider without changing value does not commit", async () => { const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); - const { getByLabelText, getByText, queryByText } = render( + const { getByLabelText } = render( ); - fireEvent.click(getByLabelText("Edit goal auto-compact threshold")); - const input = getByLabelText("Goal auto-compact threshold percent"); - fireEvent.input(input, { target: { value: "150" } }); - fireEvent.click(getByText("Save threshold")); + // MouseUp without a preceding change must NOT trigger a backend + // mutation — otherwise tapping the thumb without dragging would + // emit a spurious `goal_replaced` event each time. This is the + // invariant the `if (draft !== value)` guard in `commit()` enforces. + fireEvent.mouseUp(getByLabelText("Goal auto-compact threshold percent")); - // 150 is out of range → handler should not be called and the - // editor must surface the help text as a visible error message. - await waitFor(() => expect(queryByText(/0[–-]100/)).toBeTruthy()); + // Give any errant async commit a chance to fire. + await new Promise((resolve) => setTimeout(resolve, 10)); expect(onUpdateAutoCompactionThresholdPct).not.toHaveBeenCalled(); }); - test("auto-compact tile hides the Edit affordance when the handler is omitted", () => { + test("clicking Use default clears the override to null", async () => { + const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); + const { getByLabelText } = render( + + ); + + fireEvent.click(getByLabelText("Use workspace default for goal auto-compact")); + + // `null` (not `undefined`) is the documented "clear the per-goal + // override" sentinel — the persisted record stores `null`, which + // `applyMutableFields` honors via `Object.hasOwn`. Sending + // undefined would leave the override intact. + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(null)); + }); + + test("auto-compact tile hides Customize / Use default when the handler is omitted", () => { // Read-only stories / storybook variants render without // `onUpdateAutoCompactionThresholdPct`. The tile still shows the - // value, but the Edit button is gated so users can't open an - // editor that has nowhere to submit. - const { queryByLabelText, getByText } = render( + // current value, but the affordances are gated so users can't open + // a workflow that has nowhere to submit. + const { queryByLabelText, getByText, rerender } = render( ); - expect(getByText("50%")).toBeTruthy(); - expect(queryByLabelText("Edit goal auto-compact threshold")).toBeNull(); + expect(queryByLabelText("Use workspace default for goal auto-compact")).toBeNull(); + const slider = queryByLabelText( + "Goal auto-compact threshold percent" + ) as HTMLInputElement | null; + expect(slider?.disabled).toBe(true); + + // Default mode (null) also gates: no Customize button when there's + // no handler. The tile renders read-only. + rerender( + + ); + expect(getByText("Default")).toBeTruthy(); + expect(queryByLabelText("Customize goal auto-compact threshold")).toBeNull(); }); test("opens completion summary input, traps focus, submits, and restores focus", async () => { diff --git a/src/browser/features/RightSidebar/GoalTab.tsx b/src/browser/features/RightSidebar/GoalTab.tsx index 82a5791978..b1183ba6b9 100644 --- a/src/browser/features/RightSidebar/GoalTab.tsx +++ b/src/browser/features/RightSidebar/GoalTab.tsx @@ -23,6 +23,11 @@ import { parseGoalBudgetInputCents, parseGoalTurnCapInput, } from "@/common/utils/goals/budgetParser"; +import { + AUTO_COMPACTION_THRESHOLD_MAX, + AUTO_COMPACTION_THRESHOLD_MIN, + DEFAULT_AUTO_COMPACTION_THRESHOLD_PERCENT, +} from "@/common/constants/ui"; import { APIContext } from "@/browser/contexts/API"; import { useGoalDefaults } from "@/browser/utils/goals/useGoalDefaults"; import { cn } from "@/common/lib/utils"; @@ -104,36 +109,34 @@ interface GoalTabProps { const parseBudgetInput = parseGoalBudgetInputCents; const parseTurnCapInput = parseGoalTurnCapInput; -type EditingField = "objective" | "budget" | "turnCap" | "compactThreshold"; +type EditingField = "objective" | "budget" | "turnCap"; /** - * Parse the user-typed value from the auto-compact inline editor / create - * form into the tri-state the rest of the goal pipeline expects. - * - * - blank, "default", "—" → `null` (clear the override; workspace slider applies) - * - "off" / "disable" / "disabled" → `100` (explicit per-goal disable) - * - integer 0–100 → that integer - * - anything else → `undefined` (parse error; caller surfaces a message) - * - * Defined alongside the budget / turn-cap parsers in - * `@/common/utils/goals/budgetParser` to keep the slash command + UI - * forms in lockstep — but this one stays inline because the slash-command - * path is not yet plumbed (follow-up). + * Sentinel "this goal opts out of auto-compaction" value. Mirrors the + * workspace per-model slider, which uses `100` as the disabled marker — + * keeping the encoding identical means the renderer banner, the goal + * record, and the compaction monitor all share one disable rule. */ -function parseGoalAutoCompactionThresholdInput(raw: string): number | null | undefined { - const trimmed = raw.trim().toLowerCase(); - if (trimmed.length === 0 || trimmed === "default" || trimmed === "—") { - return null; - } - if (trimmed === "off" || trimmed === "disable" || trimmed === "disabled") { - return 100; - } - // Strip an optional trailing "%" so "50%" parses the same as "50". - const numericPart = trimmed.endsWith("%") ? trimmed.slice(0, -1).trim() : trimmed; - if (!/^\d+$/.test(numericPart)) return undefined; - const value = Number.parseInt(numericPart, 10); - if (!Number.isFinite(value) || value < 0 || value > 100) return undefined; - return value; +const PER_GOAL_COMPACT_DISABLED_PCT = 100; + +/** + * Snap a raw slider position (0–100) into the persisted-percent + * vocabulary the goal record uses: + * - dragging at or past `100 - one-step-of-headroom` → `100` (Off / + * per-goal disabled). Same "drag to the right end disables" behavior + * the workspace per-model slider uses. + * - everything else clamps into `[MIN, MAX]` and snaps to 5% + * increments, matching the workspace slider's grid so values + * entered here look like values entered there. + */ +function snapAutoCompactSliderValue(raw: number): number { + if (!Number.isFinite(raw)) return DEFAULT_AUTO_COMPACTION_THRESHOLD_PERCENT; + if (raw >= PER_GOAL_COMPACT_DISABLED_PCT) return PER_GOAL_COMPACT_DISABLED_PCT; + const clamped = Math.max( + AUTO_COMPACTION_THRESHOLD_MIN, + Math.min(AUTO_COMPACTION_THRESHOLD_MAX, raw) + ); + return Math.round(clamped / 5) * 5; } export function GoalTab(props: GoalTabProps) { @@ -215,20 +218,6 @@ export function GoalTab(props: GoalTabProps) { setEditingField("turnCap"); }; - const openCompactThresholdEditor = (origin: HTMLElement | null) => { - originRef.current = origin; - // Mirror the budget/turnCap pattern: seed the input with the current - // explicit value, or leave blank when there's no override so the - // placeholder ("default — workspace setting applies") is visible. - // `100` round-trips as "100" rather than "off"; the parser accepts - // both, so this keeps the value the user sees aligned with what - // they'd see in a /goal --compact 100 invocation. - const current = props.goal?.autoCompactionThresholdPct; - setEditValue(current == null ? "" : String(current)); - setError(null); - setEditingField("compactThreshold"); - }; - const closeEditor = () => { setEditingField(null); setError(null); @@ -269,16 +258,6 @@ export function GoalTab(props: GoalTabProps) { return; } await props.onUpdateTurnCap?.(turnCap); - } else if (editingField === "compactThreshold") { - const submittedValue = editInputRef.current?.value ?? editValue; - const pct = parseGoalAutoCompactionThresholdInput(submittedValue); - if (pct === undefined) { - setError( - 'Enter a whole number 0–100, type "off" to disable, or blank to use the workspace default.' - ); - return; - } - await props.onUpdateAutoCompactionThresholdPct?.(pct); } closeEditor(); } catch (caught) { @@ -588,16 +567,30 @@ export function GoalTab(props: GoalTabProps) { Layout now consolidates by metric so each card answers a single question: - • Budget (full-width): "Have I run out of money yet?" - shows cost / cap with remaining + a thin progress bar. - • Turns (half): "Have I run out of turns yet?" - always shows turns / cap (or `no cap`). - • Elapsed (half): wall-clock time, unchanged. + • Budget: "Have I run out of money yet?" — cost / cap with + remaining + a thin progress bar. Full-width at narrow widths + so the cost / remaining pair stays on one line; compresses + into a 1/3-width tile at wider widths where its internal + copy stacks vertically (see `BudgetTile`). + • Turns: "Have I run out of turns yet?" — always shows + turns / cap (or `no cap`). + • Auto-compact: "When will this goal compact?" — embedded + slider, "Default" mode when no per-goal override is set. + • Elapsed: wall-clock time, shown as a thin full-width row + at wider widths so it doesn't compete with the three + decision-driving tiles above. + + Container queries (`[container-type:inline-size]` + `@md:`) + scale the grid from 2 cols (narrow) to 3 cols (wider) so that, + as the sidebar is resized, Budget / Turn cap / Auto-compact + line up on one row exactly the way the user asked for. Per + `AGENTS.md`, the sidebar is dynamically resizable, so a viewport + breakpoint (`sm:` / `md:`) wouldn't have worked here. Edit affordances stay on each tile (same aria-labels as before) so the inline editor path is untouched. */} -
+
openTurnCapEditor(event.currentTarget)} /> -
+ {/* Auto-compact override (per-goal). When the goal has no + override, the tile renders a "Default — workspace setting" + row with a `Customize` button. Clicking Customize commits an + override at the global default (70%) so the slider has a + concrete value to drag from; from there the slider commits + on release. `Use default` clears back to null. Sized as a + tile peer to Turns/Elapsed at wider widths; spans both + columns at narrow widths so the slider has room to breathe. */} + props.onUpdateAutoCompactionThresholdPct?.(pct)} + /> +
Elapsed
-
+
{formatGoalElapsed(props.goal.startedAtMs)}
- {/* Auto-compact override (per-goal). Shows the user's current - knob position: "Default" when the goal has no override (the - workspace per-model slider applies), "Off" when the goal - explicitly disables compaction (`100`), and the percent - value otherwise. Editable via the same inline editor that - drives Budget / Turn cap so the create / edit vocabulary is - consistent across all goal fields. */} - openCompactThresholdEditor(event.currentTarget)} - />
- {(editingField === "budget" || - editingField === "turnCap" || - editingField === "compactThreshold") && ( + {(editingField === "budget" || editingField === "turnCap") && (
event.currentTarget.select()} @@ -691,12 +660,7 @@ export function GoalTab(props: GoalTabProps) {

{editingField === "budget" ? "Use $5, 500c, 0, or blank for no budget." - : editingField === "turnCap" - ? "Use a positive whole number, or blank for no cap." - : // The auto-compact help text spells out the tri-state - // so the user understands what blank means without - // having to guess against the model-level slider. - 'Use 0–99 to compact at that % of context. Type 100 or "off" to disable for this goal. Blank uses the workspace default.'} + : "Use a positive whole number, or blank for no cap."}

)}
-
+
{formatGoalCents(costCents)} {hasBudget && ( @@ -1086,50 +1050,159 @@ function TurnsTile(props: TurnsTileProps) { ); } -interface AutoCompactThresholdTileProps { - autoCompactionThresholdPct: number | null; +interface GoalAutoCompactSliderProps { + /** + * Tri-state per-goal override: + * - `null` → no override; the workspace per-model slider governs + * and the tile renders in "Default" mode (no slider visible, a + * `Customize` button reveals it). + * - `100` → per-goal disabled. Slider sits at the right end with + * "Off" labeling. + * - `0`–`99` (snapped to 5%) → explicit percent threshold. + */ + value: number | null; + /** When false, the tile is read-only (no Customize / no slider drag). */ canEdit: boolean; - onEdit: (event: React.MouseEvent) => void; + /** + * Commit a new override. `null` clears (back to Default mode); a + * number creates / patches the override. Called on slider release — + * NOT on every intermediate drag step — so the backend mutation rate + * stays bounded by user gestures, not pointer-move events. + */ + onChange: (pct: number | null) => Promise | void; } /** - * Auto-compact threshold (per-goal) tile. Sits next to Budget / Turns / - * Elapsed in the active-goal grid and answers "at what point does this - * goal compact?" without making the user open Settings. Display rules: + * Auto-compact threshold (per-goal) tile with an embedded slider. + * + * Two visual modes: + * + * 1. Default (`value == null`) — workspace per-model slider applies. + * Renders a compact header + `Customize` button. We deliberately + * hide the range input here because the slider has no concrete + * value to anchor against (we don't know the workspace per-model + * threshold at this layer), and a "ghost" thumb at an arbitrary + * position would suggest a value that isn't really in effect. + * + * 2. Override (`value` is a number) — slider visible with thumb at + * `value`. Releasing the thumb commits via `onChange`. A + * `Use default` button clears the override back to null. + * + * `Customize` seeds an override at `DEFAULT_AUTO_COMPACTION_THRESHOLD_PERCENT` + * (70%) so the slider has something concrete to drag from. The user + * can clear back to "Default" with one click on `Use default`, so the + * cost of an accidental Customize is one click. * - * - `null` → "Default" + helper "(workspace setting)" — the - * per-model slider in the right sidebar governs. - * - `100` → "Off" + helper "compaction disabled" — explicit per-goal - * disable so a long-context goal can run without forced - * summarization. - * - 0–99 → "{n}%" + helper "of context window". + * Snap behavior matches the workspace `ThresholdSlider`: + * - 5% increments + * - drag to the rightmost end (≥ 100) → `100` (Off / disabled) + * - drag past the enabled max (90) → clamped to 90 unless ≥ 100 * - * Kept structurally identical to `TurnsTile` (same dt/dd shape, same - * Edit affordance) so the row reads as a peer of the other tiles. + * Tile sizing: spans both columns at narrow widths so the slider has + * room to breathe; compresses to a 1/3-width tile at wider widths next + * to the Turn cap tile (see the grid container's `@md:grid-cols-3`). */ -function AutoCompactThresholdTile(props: AutoCompactThresholdTileProps) { - const { autoCompactionThresholdPct, canEdit, onEdit } = props; - const isUnset = autoCompactionThresholdPct == null; - const isOff = autoCompactionThresholdPct === 100; - const primary = isUnset ? "Default" : isOff ? "Off" : `${autoCompactionThresholdPct}%`; - const helper = isUnset - ? "workspace setting" - : isOff - ? "compaction disabled" - : "of context window"; +function GoalAutoCompactSlider(props: GoalAutoCompactSliderProps) { + const { value, canEdit, onChange } = props; + + if (value == null) { + return ( +
+
+
Auto-compact
+ {canEdit && ( + + )} +
+ {/* Match the `dl` shape of the other tiles so screen readers + announce the tile as a term/definition pair. Helper line + spells out the tri-state without dumping the workspace's + actual percent (we don't have it here). */} +
+
Default
+
workspace setting
+
+ + ); + } + + return ; +} + +interface GoalAutoCompactSliderOverrideProps { + value: number; + canEdit: boolean; + onChange: (pct: number | null) => Promise | void; +} + +/** + * Override mode of the per-goal auto-compact tile. Split into its own + * component so the local "draft" state (used while dragging) lives + * alongside the slider that needs it — and so this state is created + * fresh whenever the user re-enters override mode from Default. + * + * Why a draft instead of committing every `onChange`? `onChange` for a + * native range input fires continuously during drag (React unifies the + * native `input` event with `change`). Committing every step would + * spam `setGoal` with a backend round-trip per drag pixel. We track a + * local draft, render it in the value label so dragging feels live, + * and commit on release (`onPointerUp` / `onKeyUp`). + * + * Why no `useEffect` to sync the draft with `value`? Per the + * `react-effects` skill, syncing prop -> state via effect is an + * anti-pattern. Instead we use the "store previous prop" pattern + * (https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes) + * so the draft resets only when the *external* value actually changes + * (e.g., after a remote goal reload), not while the user is dragging. + */ +function GoalAutoCompactSliderOverride(props: GoalAutoCompactSliderOverrideProps) { + const { value, canEdit, onChange } = props; + const [draft, setDraft] = useState(value); + const [prevValue, setPrevValue] = useState(value); + if (prevValue !== value) { + setPrevValue(value); + setDraft(value); + } + + const isOff = draft === PER_GOAL_COMPACT_DISABLED_PCT; + const primary = isOff ? "Off" : `${draft}%`; + const helper = isOff ? "compaction disabled" : "of context window"; + + const commit = () => { + // Only fire the backend mutation when the slider actually moved. + // Avoids spurious `goal_replaced` history entries from someone + // clicking the thumb without dragging. + if (draft !== value) { + void onChange(draft); + } + }; + + const updateDraftFromEvent = (e: React.SyntheticEvent) => { + setDraft(snapAutoCompactSliderValue(Number(e.currentTarget.value))); + }; return ( -
+
Auto-compact
{canEdit && ( )}
@@ -1139,6 +1212,60 @@ function AutoCompactThresholdTile(props: AutoCompactThresholdTileProps) {
{helper}
+
+ + {/* "Off" label at the right end mirrors the workspace slider's + tooltip; the user can drag here to disable compaction + specifically for this goal. */} + + Off + +
); } @@ -1182,11 +1309,13 @@ function GoalCreateForm(props: GoalCreateFormProps) { const objectiveRef = useRef(null); const budgetRef = useRef(null); const turnCapRef = useRef(null); - // Auto-compact threshold uses the same ref-driven pattern: blank means - // "no override (workspace setting applies)", a number / "off" means - // explicit per-goal override. Matches the slash-command path the - // follow-up will plumb (`/goal --compact N|off`). - const compactThresholdRef = useRef(null); + // Auto-compact override uses local state (not a ref) because the + // slider component is controlled — its value drives the slider's + // thumb position. `null` means "no override" and the create intent + // omits the field on submit; a number is sent verbatim. Matches the + // tri-state vocabulary every other goal entry-point (slash command, + // inline tile editor) uses. + const [compactThresholdPct, setCompactThresholdPct] = useState(null); // Effective defaults shown as placeholder text. We seed the inputs with // `defaultValue` rather than `value` so the user can clear them; the @@ -1244,19 +1373,14 @@ function GoalCreateForm(props: GoalCreateFormProps) { intent.turnCap = parsedTurnCap; } - // Auto-compact threshold: leave omitted when blank so the goal - // gets no override (workspace setting applies). Explicit values - // ("off", 100, 0–99, "default") flow through the shared parser. - const compactRaw = (compactThresholdRef.current?.value ?? "").trim(); - if (compactRaw.length > 0) { - const parsedPct = parseGoalAutoCompactionThresholdInput(compactRaw); - if (parsedPct === undefined) { - setError( - 'Enter a whole number 0–100, type "off" to disable, or blank to use the workspace default.' - ); - return; - } - intent.autoCompactionThresholdPct = parsedPct; + // Auto-compact threshold: omit when the slider is in "Default" + // mode (null) so the goal inherits the workspace per-model + // setting. When the user has customized via the slider, send the + // explicit percent verbatim — including `100` (per-goal off) and + // `0` (the aggressive extreme; honored end-to-end per the Codex + // P2 fix in `resolveEffectiveThreshold`). + if (compactThresholdPct !== null) { + intent.autoCompactionThresholdPct = compactThresholdPct; } await props.onCreate(intent); @@ -1267,7 +1391,7 @@ function GoalCreateForm(props: GoalCreateFormProps) { if (objectiveRef.current) objectiveRef.current.value = ""; if (budgetRef.current) budgetRef.current.value = ""; if (turnCapRef.current) turnCapRef.current.value = ""; - if (compactThresholdRef.current) compactThresholdRef.current.value = ""; + setCompactThresholdPct(null); } catch (caught) { setError(caught instanceof Error ? caught.message : "Goal creation failed"); } finally { @@ -1351,29 +1475,18 @@ function GoalCreateForm(props: GoalCreateFormProps) { - {/* Per-goal auto-compact override. Spans the full width because the - helper text is the educational part (most users won't reach for - this); shrinking to a tile-sized input would compete with - Budget / Turn cap visually for no real value. */} -
- - - - 0–99 to compact at that % of context, 100 / "off" to disable for this goal. - -
+ {/* Per-goal auto-compact override. Reuses the same slider tile + the active-goal view shows so the create / edit vocabulary is + identical: "Default" mode = no override (the create intent + omits the field entirely); Override mode = explicit percent + that flows verbatim into `setGoal`. The col-span classes on + the tile are inert in a flex column, so the tile renders at + its natural full width here. */} + setCompactThresholdPct(pct)} + /> {props.workspaceId != null && (
From 30774e50f1535ebe3da3ffb70e2d09fb2057be9a Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 21 May 2026 18:18:58 -0500 Subject: [PATCH 6/6] fix(goals): dedupe slider release events on touch (Codex P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Touch-capable browsers fire `touchend` and then synthesize `mouseup` within ~50–100ms. The slider's `commit` was wired to both, so a single finger release ran the backend mutation twice: the parent's async `setGoal` hadn't resolved between the two events, so the `draft !== value` guard couldn't suppress the second call. This produced duplicate `setGoal` writes (and duplicate `goal_replaced` history entries) on touch devices. Introduce a `justCommittedRef` flag on `GoalAutoCompactSliderOverride`: - `commit()` checks the flag first; if set, it bails before calling `onChange`. - `commit()` sets the flag immediately before calling `onChange` — the synthetic `mouseup` that follows on the same tick is then short-circuited. - `updateDraftFromEvent` (the onInput/onChange path) clears the flag whenever a fresh drag/keyboard gesture starts. We use a ref instead of state because we don't want a re-render between commit() and the synthetic mouseup that's about to fire on the same tick. Per AGENTS.md's "avoid timing-based coordination" rule, the flag lifetime is tied to user intent (the next `input` event) rather than a setTimeout. Tests: two new tests pin the contract — one asserts that touchEnd + mouseUp from the same gesture commit exactly once; one asserts a follow-up gesture commits again (the flag resets correctly on next drag). Without the fix the first test fails with two calls. Validation: typecheck \u2713 lint \u2713 fmt-check \u2713 bun test src/browser/features/RightSidebar/GoalTab.test.tsx — 34 pass. --- _Generated with `mux` \u2022 Model: `anthropic:claude-opus-4-7` \u2022 Thinking: `xhigh` \u2022 Cost: `$54.20`_ --- .../features/RightSidebar/GoalTab.test.tsx | 62 +++++++++++++++++++ src/browser/features/RightSidebar/GoalTab.tsx | 29 ++++++++- 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/browser/features/RightSidebar/GoalTab.test.tsx b/src/browser/features/RightSidebar/GoalTab.test.tsx index 8fa2bd7595..adb1ba1e5e 100644 --- a/src/browser/features/RightSidebar/GoalTab.test.tsx +++ b/src/browser/features/RightSidebar/GoalTab.test.tsx @@ -544,6 +544,68 @@ describe("GoalTab", () => { await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(100)); }); + test("touch + synthesized mouseup from the same gesture commits only once (Codex P2)", async () => { + // Touch browsers emit `touchend` and then synthesize `mouseup` + // within ~50–100ms. Codex review on PR #3357 caught that without + // a per-gesture guard, both events run `commit()` before the + // async parent mutation lands — `draft !== value` is still true + // the second time around — and a single drag yields two + // `setGoal` writes (and two `goal_replaced` history entries). + // The `justCommittedRef` flag in the component dedupes these so + // one finger release ⇒ one backend write. This test pins that + // contract; without the fix the assertion fails with two calls. + const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); + const { getByLabelText } = render( + + ); + + const slider = getByLabelText("Goal auto-compact threshold percent"); + fireEvent.input(slider, { target: { value: "55" } }); + // First release: touchend (real touch endpoint). + fireEvent.touchEnd(slider); + // Second release: synthesized mouseup, same gesture. + fireEvent.mouseUp(slider); + + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(55)); + // The critical assertion: not 2, not 3 — exactly 1. + expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledTimes(1); + }); + + test("a fresh drag after a release commits again (per-gesture flag resets)", async () => { + // Counterpart to the dedup test above: the per-gesture flag must + // reset when the user starts a new drag, otherwise the slider + // would permanently lock up after a single edit. The reset + // happens in `updateDraftFromEvent` (the onInput/onChange path) + // because `input` is what fires when a fresh gesture begins. + const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); + const { getByLabelText } = render( + + ); + + const slider = getByLabelText("Goal auto-compact threshold percent"); + // First gesture + fireEvent.input(slider, { target: { value: "55" } }); + fireEvent.mouseUp(slider); + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledWith(55)); + // Second gesture — must commit again. (The component re-renders + // with value=55 between gestures in production; here we drive + // the gesture without re-render to prove the flag resets on + // input alone, not on value-prop change.) + fireEvent.input(slider, { target: { value: "40" } }); + fireEvent.mouseUp(slider); + await waitFor(() => expect(onUpdateAutoCompactionThresholdPct).toHaveBeenCalledTimes(2)); + }); + test("releasing the slider without changing value does not commit", async () => { const onUpdateAutoCompactionThresholdPct = mock(() => Promise.resolve(undefined)); const { getByLabelText } = render( diff --git a/src/browser/features/RightSidebar/GoalTab.tsx b/src/browser/features/RightSidebar/GoalTab.tsx index b1183ba6b9..c145273f35 100644 --- a/src/browser/features/RightSidebar/GoalTab.tsx +++ b/src/browser/features/RightSidebar/GoalTab.tsx @@ -1178,16 +1178,39 @@ function GoalAutoCompactSliderOverride(props: GoalAutoCompactSliderOverrideProps const primary = isOff ? "Off" : `${draft}%`; const helper = isOff ? "compaction disabled" : "of context window"; + // Per-gesture commit flag. Touch-capable browsers fire `touchend` + // and then synthesize a `mousedown`/`mouseup` pair within a short + // window (often ~50–100ms). With handlers on both events the + // backend `onChange(draft)` was being called twice for one finger + // release — the parent's async mutation hasn't resolved by the + // second event, so `draft !== value` still passes. The Codex P2 + // review flagged this as a duplicate-write bug. Use a ref-tracked + // flag (not state — we don't want a re-render between commit() and + // the synthetic mouseup that's about to fire on the same tick) and + // reset it whenever the user starts a fresh gesture (input event). + // We deliberately don't use `setTimeout` here per AGENTS.md's + // "avoid timing-based coordination" rule — the input-event reset + // ties the flag lifetime to actual user intent. + const justCommittedRef = useRef(false); + const commit = () => { - // Only fire the backend mutation when the slider actually moved. - // Avoids spurious `goal_replaced` history entries from someone - // clicking the thumb without dragging. + // Only fire the backend mutation when the slider actually moved + // (avoids spurious `goal_replaced` history entries from a thumb + // tap), and only once per gesture (the touch + synthetic mouse + // sequence above must not double-commit). + if (justCommittedRef.current) return; if (draft !== value) { + justCommittedRef.current = true; void onChange(draft); } }; const updateDraftFromEvent = (e: React.SyntheticEvent) => { + // The native `input` event fires when a fresh drag/keyboard + // gesture starts (and on every subsequent step). Clearing the + // commit-once flag here means the next release will commit + // again, exactly as the user expects. + justCommittedRef.current = false; setDraft(snapAutoCompactSliderValue(Number(e.currentTarget.value))); };