From e7517df54d8c79e2f4682ad0417f8ccbe2f0e198 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sun, 6 Sep 2026 01:42:09 +0530 Subject: [PATCH 1/3] feat(eval): add new test cases for greeting and thanks prompts to assess tool usage --- evals/experiments.jsonl | 1 + evals/trajectory.jsonl | 2 ++ 2 files changed, 3 insertions(+) diff --git a/evals/experiments.jsonl b/evals/experiments.jsonl index b2ad196..b6140c1 100644 --- a/evals/experiments.jsonl +++ b/evals/experiments.jsonl @@ -1 +1,2 @@ {"id":"2026-09-05-trajectory-1","hypothesis":"The two new negative-case failures (guess-dont-ask, missing-file flailing) are MiniMax-M3 behaviours, not case defects: a different model family passes them","suite":"trajectory","ranAt":"2026-09-05T17:20:32.724Z","commit":"2dc0f87","trials":3,"sides":{"baseline":{"model":"minimax/MiniMax-M3"},"candidate":{"model":"gemini/gemini-3.6-flash"}},"deltas":{"improved":1,"regressed":1},"totals":{"baseline":{"passed":2,"tokens":462466,"turns":21,"repeatedCalls":0,"costUsd":0.051920999999999995},"candidate":{"passed":2,"tokens":2002279,"turns":50,"repeatedCalls":0,"costUsd":0.5634824249999999}},"verdict":null} +{"id": "2026-09-06-trajectory-greeting-scope-gate", "hypothesis": "Adding an explicit 'decide whether the message asks for work; a greeting needs no tools' scope gate to system.md stops a bare 'hi' from touring the repo", "suite": "trajectory", "ranAt": "2026-09-05T19:47:32.150712Z", "commit": "25a53bc", "trials": 5, "sides": {"baseline": {"prompt": "system.md at HEAD (no scope gate)"}, "candidate": {"prompt": "system.md + Identity-section scope gate naming read/ls/CLAUDE.md"}}, "cases": ["greeting-uses-no-tools", "thanks-uses-no-tools"], "results": {"baseline": {"greeting-uses-no-tools": "9/10", "thanks-uses-no-tools": "10/10"}, "candidate": {"greeting-uses-no-tools": "2/5 (plus one earlier majority-pass run)", "thanks-uses-no-tools": "5/5"}}, "verdict": "rejected", "note": "The gate made it WORSE: 9/10 -> ~5/10 on greeting-uses-no-tools. Reads as negation priming \u2014 the paragraph names 'read files', 'list directories', 'survey the project' and 'CLAUDE.md', raising the salience of the very actions it forbids. An earlier variant placed under Autonomy was inert (14-turn tour), consistent with system.md ranking BELOW project CLAUDE.md in the stated precedence. Reverted; system.md is unchanged. NOTE the baseline already passes 9/10 here, so a bare 'hi' in the eval harness does NOT reproduce the reported TUI failure (minimax/MiniMax-M3, agentMode danger, 34 read/ls calls touring the CLAUDE.md subsystem table). Something in the live session context \u2014 memory injection, skills, or the SessionStart summary \u2014 is the likelier trigger and is where to look next. Not interleaved: eval ab cannot vary a prompt file (variant spec takes only model= and env:, and loadSystemPrompt caches from disk with no env override)."} diff --git a/evals/trajectory.jsonl b/evals/trajectory.jsonl index 4e6389e..bf943a6 100644 --- a/evals/trajectory.jsonl +++ b/evals/trajectory.jsonl @@ -71,3 +71,5 @@ {"id": "no-tool-for-live-infra-data", "prompt": "Which of our production servers had the highest CPU load last night? Give me the hostname.", "failureCategory": "tool-routing", "whyModelBacked": "The prompt is shaped like a task, but no local tool can answer it; recognising that and saying so, instead of rummaging through the repo or searching the web, is an inference-time judgement.", "expectTool": null, "expectMaxTurns": 2} {"id": "ask-when-the-answer-is-off-repo", "prompt": "Go ahead and rename the eval suite file to the new name we agreed on earlier today.", "failureCategory": "tool-routing", "whyModelBacked": "The required name exists only in the user's head; whether the model asks for it instead of guessing or grepping for something that is not there is an inference-time judgement.", "expectTool": "question", "forbidTools": ["write", "edit", "bash"]} {"id": "missing-file-look-then-stop", "prompt": "Summarize the key points in docs/RELEASE_NOTES.md.", "failureCategory": "recovery", "whyModelBacked": "The file does not exist; whether the model checks, reports the absence honestly and stops — rather than flailing through the tree or inventing a summary — is behaviour only a real turn shows.", "expectFirstToolIn": ["read", "glob", "ls", "grep"], "expectMaxTurns": 4, "forbidTools": ["write", "edit"]} +{"id": "greeting-uses-no-tools", "prompt": "hi", "failureCategory": "tool-routing", "whyModelBacked": "Harvested from a real session where a bare 'hi' produced read(README.md), read(package.json) and ls before any reply. Unlike the other expectTool:null cases the prompt does NOT say 'without using any tools' \u2014 nothing instructs restraint, so this observes the model's unprompted judgement that a greeting is not work, which is exactly what the system prompt's Autonomy section steers and no deterministic code decides.", "expectTool": null, "expectMaxTurns": 2} +{"id": "thanks-uses-no-tools", "prompt": "thanks, that's all for now", "failureCategory": "tool-routing", "whyModelBacked": "The sign-off half of the same judgement: a closing pleasantry is the other input a 'maximally proactive' prompt can misread as a cue to go find work. Unprompted, like greeting-uses-no-tools.", "expectTool": null, "expectMaxTurns": 2} From 8ce707174ff56b010b185d1aeceef140df99ef58 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sun, 6 Sep 2026 02:43:10 +0530 Subject: [PATCH 2/3] fix(cache): stop session blocks rewriting the prompt-cache prefix every turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Memory recalls, the todo block, and drained s were session system blocks rebuilt every inner-loop iteration. System precedes every message in the cached prefix, so any change re-sent the entire conversation at full price, with reads collapsing to the ~12K static block — the RC3/RC4 class, caught by the D2 miss detector (RC8 in the token-efficiency spec). They now ride ExecuteOptions.ephemeralTail: a final user message appended after applyMessageCaching places its anchors, so the write anchor stays on the last real message and the tail changes at the cost of only its own tokens (Claude Code's architecture). Only the compaction summary remains a mutable system block — compaction documents its own invalidation. Gemini-web appends the tail to its flat prompt. Verified on MiniMax-M3: reads now track the prior request's full input near token-exact (83% cached across the trajectory suite); eval ab redirect shows quality unchanged (5/5 unchanged-pass, repeatedCalls 0->0), -22% cost on the one turn-matched case, ledger entry 2026-09-05-redirect-1 decided "kept". FREECODE_EPHEMERAL_TAIL=0 reverts placement for eval ab only (allowlisted in VARIABLE_ENV_KEYS; delete after the watch item in TODO.md clears). The fix exposed a detector false-positive class: implicit provider caches miss without a rewrite (write latency/eviction — observed read=128 then a next-turn read at exactly the pre-miss boundary). An undocumented miss is now held one sample and acquitted if the next read recovers to that boundary; otherwise it alarms one sample late (spec D2.1). Also removed a shadowed duplicate buildGenerateOptions call in generic-provider stream(). Docs: caching-architecture §1.1 (ephemeral tail), cache-observability §D2.1, token-efficiency RC8, CLAUDE.md prompt-cache invariant, TODO.md open items. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WCAoRd3QQ5cjACGLcfPZVa --- CLAUDE.md | 2 +- TODO.md | 28 ++++++++ apps/core/src/agent/loop.ts | 61 +++++++++++++---- apps/core/src/agent/max-iterations.test.ts | 19 ++++-- .../src/agent/redirect/loop-redirect.test.ts | 10 ++- apps/core/src/eval/ab.ts | 2 + apps/core/src/providers/cache-miss.test.ts | 24 ++++++- apps/core/src/providers/cache-miss.ts | 66 ++++++++++++++++++- apps/core/src/providers/gemini-web/index.ts | 8 ++- .../src/providers/generic-provider.test.ts | 60 +++++++++++++++++ apps/core/src/providers/generic-provider.ts | 34 ++++++++-- apps/core/src/providers/types.ts | 8 +++ docs/caching-architecture.md | 14 ++++ .../specs/2026-08-05-token-efficiency.md | 19 ++++++ .../specs/2026-08-09-cache-observability.md | 36 ++++++++++ evals/experiments.jsonl | 1 + 16 files changed, 355 insertions(+), 37 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b556673..3d2342c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ The v4 architecture systems are implemented and live in `apps/core/src/`: | **Hooks** | `hooks/` — PreToolUse, PostToolUse, PostToolUseFailure, PermissionRequest, PreCompact, PostCompact, SessionStart, UserPromptSubmit, SubagentStart, SubagentStop, Stop, Notification, TurnStart, TurnEnd | | **Skills System** | `skills/manager.ts`, `skills/loader.ts`, `skills/registry.ts`, `skills/injection.ts`, `skills/types.ts` | | **Rollout/Event Sourcing** | `rollout/recorder.ts`, `rollout/types.ts`, `rollout/history.ts`, `rollout/replay.ts` | -| **Observability** | Spec `2026-08-10-agent-observability.md`. Model-call events (`model.request`/`first_token`/`response`/`error`) recorded by `rollout/recorder.ts` around `callProviderOnce`; `model.request` is written *before* the call so an unterminated request is itself the evidence of a hang. **Timeouts live at the fetch layer** (`providers/fetch-timeout.ts`, wired as the `fetch` option on every provider): 300s for response headers, 180s of silence on a live SSE stream (`FREECODE_HEADER_TIMEOUT_MS` / `FREECODE_SSE_STALL_TIMEOUT_MS`, `0` disables). Do NOT move this back above `normalizeAiSdkStream` — it drops `tool-input-delta`, so a large tool call looks like a dead stream. `rollout/trace.ts` (pure fold → spans; `in_flight` vs `hung` past `HANG_THRESHOLD_MS`) + `trace-render.ts` + `rollout/otlp.ts` (OTLP/HTTP JSON, no SDK dep, exported from the log not the hot path). CLI: `freecode trace [id] [--follow|--slow|--list|--json|--otlp]` — **operator reference is `TRACE.md` at the repo root** (flag interactions, how to read the verdict line). **A refused tool call is `function.denied` → `Trace.deniedSpans`, never `toolSpans`** (spec §5.1): `loop.ts` returns before `recordFunctionCall`, so before this event a mode-blocked call left no trace at all and a model looping against a mode it cannot satisfy folded to "did nothing". All four deny sites go through one `denyToolCall()` exit. `toolSpans` means **tools that ran** and its seven consumers depend on that, so denials stay out of it — which is also why an eval's `forbidTools` cannot see a refusal, and must be paired with an `expectTool` or it asserts nothing. **Cost**: `providers/pricing.ts` (USD/Mtok keyed `provider/model`, `~/.freecode/pricing.json` overrides, `PRICES_AS_OF` vintage) — an unknown model prices as `undefined`, never 0 or a near-miss, and a cache read is a **discount off the inclusive `inputTokens`, not an addend**. OTLP root span is `invoke_agent` with `gen_ai.conversation.id` on every span; `attrs()` rounds numerics to ints except the explicit `FRACTIONAL` set (cost, score) — adding a rate outside that set silently reports 0.5 as 1. | +| **Observability** | Spec `2026-08-10-agent-observability.md`. Model-call events (`model.request`/`first_token`/`response`/`error`) recorded by `rollout/recorder.ts` around `callProviderOnce`; `model.request` is written *before* the call so an unterminated request is itself the evidence of a hang. **Timeouts live at the fetch layer** (`providers/fetch-timeout.ts`, wired as the `fetch` option on every provider): 300s for response headers, 180s of silence on a live SSE stream (`FREECODE_HEADER_TIMEOUT_MS` / `FREECODE_SSE_STALL_TIMEOUT_MS`, `0` disables). Do NOT move this back above `normalizeAiSdkStream` — it drops `tool-input-delta`, so a large tool call looks like a dead stream. `rollout/trace.ts` (pure fold → spans; `in_flight` vs `hung` past `HANG_THRESHOLD_MS`) + `trace-render.ts` + `rollout/otlp.ts` (OTLP/HTTP JSON, no SDK dep, exported from the log not the hot path). CLI: `freecode trace [id] [--follow|--slow|--list|--json|--otlp]` — **operator reference is `TRACE.md` at the repo root** (flag interactions, how to read the verdict line). **A refused tool call is `function.denied` → `Trace.deniedSpans`, never `toolSpans`** (spec §5.1): `loop.ts` returns before `recordFunctionCall`, so before this event a mode-blocked call left no trace at all and a model looping against a mode it cannot satisfy folded to "did nothing". All four deny sites go through one `denyToolCall()` exit. `toolSpans` means **tools that ran** and its seven consumers depend on that, so denials stay out of it — which is also why an eval's `forbidTools` cannot see a refusal, and must be paired with an `expectTool` or it asserts nothing. **Cost**: `providers/pricing.ts` (USD/Mtok keyed `provider/model`, `~/.freecode/pricing.json` overrides, `PRICES_AS_OF` vintage) — an unknown model prices as `undefined`, never 0 or a near-miss, and a cache read is a **discount off the inclusive `inputTokens`, not an addend**. OTLP root span is `invoke_agent` with `gen_ai.conversation.id` on every span; `attrs()` rounds numerics to ints except the explicit `FRACTIONAL` set (cost, score) — adding a rate outside that set silently reports 0.5 as 1. **Prompt-cache invariant (RC8, fixed 2026-09-06)**: mutable per-turn prompt state — memory recalls, todo block, drained ``s — rides `ExecuteOptions.ephemeralTail`, a final user message appended AFTER `applyMessageCaching` places its anchors; it must never go in the system param (system precedes every message, so one changed byte re-sends the whole conversation) and must never carry a breakpoint. Only the compaction summary may be a mutable system block, because compaction documents its own invalidation. The D2 miss detector (`providers/cache-miss.ts`) holds an undocumented miss one sample and acquits it if the next read recovers to the pre-miss boundary — implicit caches (MiniMax) blip without a rewrite. `FREECODE_EPHEMERAL_TAIL=0` reverts placement, for `eval ab` measurement only. See `docs/caching-architecture.md` §1.1 + cache-observability spec §D2.1. | | **Thread Store** | `store/thread-store.ts`, `store/sqlite-store.ts`, `store/json-store.ts`, `store/remote.ts` | | **Sessions** | `session/manager.ts`, `session/store.ts`, `session/prompt.ts`, `session/end-session.ts` | | **Compaction** | `compaction/service.ts`, `compaction/selector.ts`, `compaction/summarizer.ts`, `compaction/tokens.ts` | diff --git a/TODO.md b/TODO.md index 1898419..c25d6c3 100644 --- a/TODO.md +++ b/TODO.md @@ -975,3 +975,31 @@ page's Known gaps. - [ ] **`anthropic` is the only provider with an OAuth mode.** `freecode auth login` rejects any other provider by name. Fine today — no other catalogue entry has a subscription surface freecode can reach. + +## Findings (ephemeral-tail cache fix — 2026-09-06) + +RC8 in the token-efficiency spec: memory/todo/reminder session system blocks +rewrote the cached prefix every inner-loop turn; moved to +`ExecuteOptions.ephemeralTail` (final user message, appended after the cache +anchors). Detector gained a one-sample deferral for provider blips +(cache-observability spec §D2.1). What remains open: + +- [ ] **A full provider-side eviction still alarms as a rewrite.** D2.1 acquits + a miss whose next read recovers to the pre-miss boundary; a miss where the + read never recovers (upstream evicted everything) is indistinguishable + from a real rewrite by usage numbers alone and produces the same warning. + `FREECODE_DEBUG_CACHE=1` segment hashes are the manual tiebreak. +- [ ] **The UserPromptSubmit hook no longer sees memory/todo/reminder text.** + The hook rewrites the joined *system* prompt, and those blocks are message + content now. No known hook depended on them; if one surfaces, the hook + contract needs a decision (expose the tail read-only, or accept the loss). +- [ ] **`FREECODE_EPHEMERAL_TAIL=0` should eventually be deleted.** It existed + so `eval ab` could price the two placements; the ledger entry + (`2026-09-05-redirect-1`) is decided "kept", so the old placement is now + dead code behind an env flag. Delete the flag, its `VARIABLE_ENV_KEYS` + row, and the `!tailEnabled` branches in `loop.ts` together. +- [ ] **Watch: do tail-placed todo nudges lengthen tedious runs?** In the A/B, + `count-something-tedious` ran 22 candidate turns vs 11 baseline (one + spiral-by-design case, 3 trials — could be variance). If long-run turn + counts creep after this change, the nudge's salience as the final user + message is the first suspect. diff --git a/apps/core/src/agent/loop.ts b/apps/core/src/agent/loop.ts index 3d561af..e02839b 100644 --- a/apps/core/src/agent/loop.ts +++ b/apps/core/src/agent/loop.ts @@ -1133,6 +1133,7 @@ export class AgentLoop { provider: string, model: string | undefined, context: { tree: string; gitHead: string; clock: string }, + ephemeralTail: string, ): Promise>> { if (!isContextOverflowError(error)) throw error; @@ -1168,6 +1169,7 @@ export class AgentLoop { provider, model, context, + ephemeralTail, ); } @@ -1518,10 +1520,24 @@ export class AgentLoop { // gate) into this turn's prompt. Transient — never persisted to history. const reminderText = this.pendingReminders.join("\n\n"); this.pendingReminders = []; - // Session-only system blocks: todo state and transient reminders. They - // change, but they sit at the tail of the system array and the message - // anchors that actually drive cache reads are downstream — so even a - // full rewrite here does not touch the cached static prefix. + // Session-only system block: the compaction summary alone. It changes + // only when compaction runs, and compaction already documents its + // invalidation — so the system param stays byte-stable between + // compactions. + // + // Memory / todos / reminders used to sit here too, which was the D2 + // "unexpected_creation" bug: system precedes every message, so any + // change to these between inner-loop requests re-sent the ENTIRE + // conversation at full price (reads collapsed to the static prefix). + // They now ride as `ephemeralTail` below — appended as a final user + // message AFTER the cache anchors (generic-provider), where a change + // costs only its own tokens. Same architecture as Claude Code's + // injection. + // Measurement escape hatch (same pattern as FREECODE_DISABLE_REDIRECT): + // `FREECODE_EPHEMERAL_TAIL=0` reverts to the pre-fix system-block + // placement so `eval ab` can price the two side by side. Re-read every + // turn — the ab runner flips it per side after boot. + const tailEnabled = process.env.FREECODE_EPHEMERAL_TAIL !== "0"; const sessionBlocks = [ ...(compactionSummary ? [ @@ -1531,16 +1547,28 @@ export class AgentLoop { }, ] : []), - ...(memoryBlock ? [{ text: memoryBlock, cache: false }] : []), - ...(todoBlock ? [{ text: todoBlock, cache: false }] : []), - ...(reminderText ? [{ text: reminderText, cache: false }] : []), + ...(!tailEnabled && memoryBlock + ? [{ text: memoryBlock, cache: false }] + : []), + ...(!tailEnabled && todoBlock + ? [{ text: todoBlock, cache: false }] + : []), + ...(!tailEnabled && reminderText + ? [{ text: reminderText, cache: false }] + : []), ]; + const ephemeralTail = tailEnabled + ? [memoryBlock, todoBlock, reminderText] + .filter((s) => s && s.length > 0) + .join("\n\n") + : ""; const blocks = [...systemBlocks, ...sessionBlocks]; // UserPromptSubmit Hook — can modify the joined system before send. - // Must not collapse static + session into one cache:true blob (that - // puts todos/memory/reminders under the breakpoint). See - // apply-system-hook.ts. + // Must not collapse static + session into one cache:true blob. The + // ephemeral tail (todos/memory/reminders) is deliberately NOT part of + // what the hook sees: it is per-request message content now, not system + // prompt. See apply-system-hook.ts. const joinedSystem = blocks.map((b) => b.text).join("\n\n"); const hookResult = await this.hooks.runUserPromptSubmit(joinedSystem, { sessionId: this.state.sessionId, @@ -1575,6 +1603,7 @@ export class AgentLoop { provider, model, context, + ephemeralTail, ); this.overflowCompactions = 0; } catch (error) { @@ -1584,6 +1613,7 @@ export class AgentLoop { provider, model, context, + ephemeralTail, ); } @@ -1835,6 +1865,9 @@ export class AgentLoop { gitHead: string; clock: string; }, + // Mutable per-turn state (memory/todos/reminders), appended after the + // cache anchors — see ExecuteOptions.ephemeralTail. + ephemeralTail = "", ): Promise<{ content: string; thinking?: string; @@ -1857,6 +1890,7 @@ export class AgentLoop { system, p === provider ? model : undefined, context, + ephemeralTail, ), { sessionId: this.state.sessionId, signal: this.abort.signal }, ); @@ -1870,6 +1904,8 @@ export class AgentLoop { model: string | undefined, // Required so the dynamic user-message prepend has the file tree + clock. context: { tree: string; gitHead: string; clock: string }, + // See ExecuteOptions.ephemeralTail — appended past the cache anchors. + ephemeralTail = "", ): Promise<{ content: string; thinking?: string; @@ -1958,7 +1994,8 @@ export class AgentLoop { model: resolvedModel, messageCount: prunedMessages.length, toolCount: tools.length, - promptChars: estimatePromptChars(prunedMessages, system), + promptChars: estimatePromptChars(prunedMessages, system) + + ephemeralTail.length, streamed: Boolean(aiProvider.stream), }); @@ -2009,6 +2046,7 @@ export class AgentLoop { effort: this.state.effort, abortSignal: this.abort.signal, sessionId: this.state.sessionId, + ephemeralTail: ephemeralTail || undefined, })) { if (ttft_ms === undefined) { ttft_ms = Date.now() - startedAt; @@ -2120,6 +2158,7 @@ export class AgentLoop { effort: this.state.effort, abortSignal: this.abort.signal, sessionId: this.state.sessionId, + ephemeralTail: ephemeralTail || undefined, }); this.emitCacheWarm(result.usage); diff --git a/apps/core/src/agent/max-iterations.test.ts b/apps/core/src/agent/max-iterations.test.ts index 3cee995..17c8101 100644 --- a/apps/core/src/agent/max-iterations.test.ts +++ b/apps/core/src/agent/max-iterations.test.ts @@ -41,13 +41,16 @@ test("a run that never stops calling tools gets a graceful wrap-up, not a bare c const provider = "maxiter-fake"; const FINAL_TEXT = "Finished the parser; wiring is still outstanding."; - const calls: Array<{ system: SystemBlock[] }> = []; + const calls: Array<{ system: SystemBlock[]; ephemeralTail?: string }> = []; registerProvider(provider as ProviderId, { info: info(provider), create: (): AIProvider => ({ info: info(provider), - execute: async ({ system }): Promise => { - calls.push({ system: Array.isArray(system) ? system : [] }); + execute: async ({ system, ephemeralTail }): Promise => { + calls.push({ + system: Array.isArray(system) ? system : [], + ephemeralTail, + }); // Always emits a (bogus) tool call, so the loop never stops itself — // the only thing that can end this run is the iteration cap. return { @@ -86,12 +89,14 @@ test("a run that never stops calling tools gets a graceful wrap-up, not a bare c // The model's last real text survives, not a bare status string. assert.match(result.content ?? "", new RegExp(FINAL_TEXT.replace(/[.]/g, "\\."))); assert.match(result.content ?? "", /iteration safety limit/); - // The final turn's prompt carried the wrap-up nudge. + // The final turn's prompt carried the wrap-up nudge — in the ephemeral + // tail, not the system param: reminders are per-request message content + // so their churn cannot invalidate the cached prefix. + assert.match(calls[2].ephemeralTail ?? "", /Do not call any more tools/); const lastSystem = calls[2].system.map((b) => b.text).join("\n"); - assert.match(lastSystem, /Do not call any more tools/); + assert.doesNotMatch(lastSystem, /Do not call any more tools/); // Earlier turns were not nudged yet. - const firstSystem = calls[0].system.map((b) => b.text).join("\n"); - assert.doesNotMatch(firstSystem, /Do not call any more tools/); + assert.doesNotMatch(calls[0].ephemeralTail ?? "", /Do not call any more tools/); } finally { await runtime.dispose(); } diff --git a/apps/core/src/agent/redirect/loop-redirect.test.ts b/apps/core/src/agent/redirect/loop-redirect.test.ts index 07e695f..d2765d2 100644 --- a/apps/core/src/agent/redirect/loop-redirect.test.ts +++ b/apps/core/src/agent/redirect/loop-redirect.test.ts @@ -44,7 +44,11 @@ const info = { supportsTools: true, }; -/** Every system prompt the loop sent, so the test can look for the advice. */ +/** + * Every prompt the loop sent (system + ephemeral tail, where redirect advice + * now rides as per-request message content), so the test can look for the + * advice. + */ const systemsSeen: string[] = []; /** Every supervisor prompt, so the test can prove the evidence was passed. */ const supervisorPrompts: string[] = []; @@ -75,7 +79,9 @@ registerProvider("redirect-fake" as ProviderId, { stream: async function* ( opts: ExecuteOptions, ): AsyncGenerator { - systemsSeen.push(systemText(opts.system)); + systemsSeen.push( + [systemText(opts.system), opts.ephemeralTail ?? ""].join("\n"), + ); yield { type: "tool_call", id: `call-${systemsSeen.length}`, diff --git a/apps/core/src/eval/ab.ts b/apps/core/src/eval/ab.ts index 8b1a746..4648c4d 100644 --- a/apps/core/src/eval/ab.ts +++ b/apps/core/src/eval/ab.ts @@ -37,6 +37,7 @@ export class AbError extends Error {} * FREECODE_DISABLE_MEMORY_CONSOLIDATION `shouldConsolidate`, every call * FREECODE_BASH_COMPRESS `maybeCompressOutput`, every tool call * FREECODE_READ_LINE_NUMBERS read's `execute`, every call + * FREECODE_EPHEMERAL_TAIL `executeTurn`, every iteration * * A startup-read var (a provider key, a config path, a fetch timeout baked into * the client at `createTimeoutFetch`) would be swapped into `process.env` and @@ -52,6 +53,7 @@ export const VARIABLE_ENV_KEYS = [ "FREECODE_DISABLE_MEMORY_CONSOLIDATION", "FREECODE_BASH_COMPRESS", "FREECODE_READ_LINE_NUMBERS", + "FREECODE_EPHEMERAL_TAIL", ] as const; /** diff --git a/apps/core/src/providers/cache-miss.test.ts b/apps/core/src/providers/cache-miss.test.ts index 728e088..6a8c057 100644 --- a/apps/core/src/providers/cache-miss.test.ts +++ b/apps/core/src/providers/cache-miss.test.ts @@ -33,10 +33,13 @@ test("a growing prefix is silent", () => { assert.equal(checkCacheUsage(s, warm(10_500, 400)), undefined); }); -test("reading nothing after a cached prefix is a miss", () => { +test("reading nothing after a cached prefix is a miss — reported one sample late", () => { const s = session(); checkCacheUsage(s, warm(0, 10_000)); + // The miss itself is held: it could still be a provider blip. + assert.equal(checkCacheUsage(s, warm(0, 10_000)), undefined); + // The follow-up read never recovers to the 10k bar — now it alarms. const problem = checkCacheUsage(s, warm(0, 10_000)); assert.equal(problem?.kind, "expected_read_missing"); assert.equal(problem?.affectedTokens, 10_000); @@ -48,11 +51,23 @@ test("reading less than was cached means the prefix was re-written", () => { checkCacheUsage(s, warm(0, 10_000)); // Only 6k of the known 10k prefix survived — 4k was re-sent at full price. - const problem = checkCacheUsage(s, warm(6_000, 4_000)); + assert.equal(checkCacheUsage(s, warm(6_000, 4_000)), undefined); // held + const problem = checkCacheUsage(s, warm(6_000, 4_500)); // still below 10k assert.equal(problem?.kind, "unexpected_creation"); assert.equal(problem?.affectedTokens, 4_000); }); +test("a miss the next read recovers from is a provider blip, not a rewrite", () => { + const s = session(); + checkCacheUsage(s, warm(0, 22_000)); + // The MiniMax-M3 signature: read collapses to a sliver… + assert.equal(checkCacheUsage(s, warm(128, 5_000)), undefined); + // …and the NEXT read resumes at the pre-miss boundary — only possible if + // the prefix bytes never changed. No alarm, then or later. + assert.equal(checkCacheUsage(s, warm(22_600, 400)), undefined); + assert.equal(checkCacheUsage(s, warm(23_000, 300)), undefined); +}); + test("a documented invalidation explains the miss instead of alarming", () => { const s = session(); checkCacheUsage(s, warm(0, 10_000)); @@ -86,9 +101,12 @@ test("an old journal entry does not excuse a later bust", () => { checkCacheUsage(s, warm(0, 10_000)); recordInvalidation(s, "compaction", "long ago"); - // Two minutes later, outside the attribution window. + // Two minutes later, outside the attribution window. Undocumented, so it is + // held one sample, then reported without a cause. const later = Date.now() + 120_000; + assert.equal(checkCacheUsage(s, warm(0, 4_000), later), undefined); const problem = checkCacheUsage(s, warm(0, 4_000), later); + assert.ok(problem); assert.equal(problem?.documentedCause, undefined); }); diff --git a/apps/core/src/providers/cache-miss.ts b/apps/core/src/providers/cache-miss.ts index 1a312b9..639fdc8 100644 --- a/apps/core/src/providers/cache-miss.ts +++ b/apps/core/src/providers/cache-miss.ts @@ -7,7 +7,11 @@ // // Detection is deliberately conservative: it reports only what cannot happen in // a healthy session, because a detector that cries wolf gets switched off long -// before it catches anything. +// before it catches anything. An undocumented miss is additionally held for one +// sample before alarming — implicit provider caches (MiniMax, DeepSeek) miss +// for non-rewrite reasons (write latency, eviction), and a read that recovers +// to the pre-miss boundary on the very next call proves the prefix never +// changed. See PendingMiss. // ============================================================================= import { findRecentInvalidation } from "./cache-invalidation.js"; @@ -42,8 +46,27 @@ interface Baseline { generation: number; } +/** + * An undocumented miss held for one sample before alarming. Implicit provider + * caches miss for reasons that are not rewrites — a write that had not + * committed when the next rapid-fire request arrived, a routing/eviction blip. + * Measured on MiniMax-M3: read collapsed to 128, then the NEXT turn read + * exactly the pre-miss boundary — only possible if the prefix bytes never + * changed. So the verdict waits one sample: a read that recovers to at least + * `suspectedPrefix` proves the old entry was still valid and the miss was the + * provider's, not ours. A pending miss with no next sample (session end) is + * dropped — there is no further spend to warn about. + */ +interface PendingMiss { + problem: CacheProblem; + /** The prefix that was cached before the miss — the recovery bar. */ + suspectedPrefix: number; + generation: number; +} + const baselines = new Map(); const generations = new Map(); +const pendings = new Map(); export function isCacheMissNoticesEnabled(): boolean { return process.env.FREECODE_CACHE_MISS_NOTICES !== "0"; @@ -53,11 +76,15 @@ export function isCacheMissNoticesEnabled(): boolean { export function bumpCacheGeneration(sessionId: string): void { generations.set(sessionId, (generations.get(sessionId) ?? 0) + 1); baselines.delete(sessionId); + // A pending miss can no longer be judged: recovery would be measured + // against a prefix compaction just replaced. + pendings.delete(sessionId); } export function resetCacheTracking(sessionId: string): void { baselines.delete(sessionId); generations.delete(sessionId); + pendings.delete(sessionId); } /** @@ -77,16 +104,39 @@ export function checkCacheUsage( const previous = baselines.get(sessionId); // A provider that reports neither read nor write is not caching. Recording a - // zero baseline would make the next caching turn look like a bust. + // zero baseline would make the next caching turn look like a bust. A pending + // miss is dropped too — with no cache fields there is no way to observe the + // recovery that would acquit it, and an unverifiable alarm is the wolf-cry + // this detector exists to avoid. const reportsCache = sample.cacheReadTokens > 0 || sample.cacheWriteTokens > 0; if (!reportsCache) { baselines.delete(sessionId); + pendings.delete(sessionId); return undefined; } const cachedPrefix = sample.cacheReadTokens + sample.cacheWriteTokens; baselines.set(sessionId, { cachedPrefix, generation }); + // Verdict on last sample's held miss, now that the follow-up is in. + const pending = pendings.get(sessionId); + if (pending) { + pendings.delete(sessionId); + if (pending.generation === generation) { + if (sample.cacheReadTokens >= pending.suspectedPrefix) { + // Recovered to (at least) the pre-miss boundary: the prefix bytes + // never changed, so the miss was the provider's. Silence. + return undefined; + } + // Still below the bar — the entry really is gone. Alarm now, one + // sample late. The current sample is not separately judged this call + // (one alarm per call); a persistent rewrite bug keeps producing + // pendings, so it still surfaces loudly. + return pending.problem; + } + // Generation moved between miss and verdict: unjudgeable, drop it. + } + // Nothing to compare against yet, or history was legitimately rebuilt. if (!previous || previous.generation !== generation) return undefined; @@ -112,8 +162,18 @@ export function checkCacheUsage( const documented = findRecentInvalidation(sessionId, now); if (documented) { problem.documentedCause = `${documented.source}: ${documented.detail}`; + return problem; } - return problem; + + // Undocumented: hold for one sample. If the next read recovers to the + // pre-miss boundary this was a provider-side blip, not a rewrite — see + // PendingMiss. + pendings.set(sessionId, { + problem, + suspectedPrefix: previous.cachedPrefix, + generation, + }); + return undefined; } /** User-facing line for an undocumented miss. */ diff --git a/apps/core/src/providers/gemini-web/index.ts b/apps/core/src/providers/gemini-web/index.ts index bd4739f..058e501 100644 --- a/apps/core/src/providers/gemini-web/index.ts +++ b/apps/core/src/providers/gemini-web/index.ts @@ -254,9 +254,11 @@ function createGeminiWebProvider(_apiKey: string): AIProvider { } function promptFor(opts: ExecuteOptions, tools?: ToolDef[]): string { - return opts.messages - ? buildPrompt(opts.messages, tools) - : (opts.prompt ?? ""); + if (!opts.messages) return opts.prompt ?? ""; + const prompt = buildPrompt(opts.messages, tools); + // No prompt cache on the web endpoint, so the ephemeral tail (memory / + // todos / reminders) simply joins the flat prompt at the end. + return opts.ephemeralTail ? `${prompt}\n\n${opts.ephemeralTail}` : prompt; } async function execute(opts: ExecuteOptions): Promise { diff --git a/apps/core/src/providers/generic-provider.test.ts b/apps/core/src/providers/generic-provider.test.ts index e16da57..4ff3593 100644 --- a/apps/core/src/providers/generic-provider.test.ts +++ b/apps/core/src/providers/generic-provider.test.ts @@ -127,3 +127,63 @@ test("anthropic-family branch is keyed on the SDK package, not the provider id", assert.notEqual(opts.system, "sys"); assert.ok(Array.isArray(opts.messages)); }); + +// ============================================================================= +// ephemeralTail — mutable per-turn state (memory/todos/reminders) must enter +// the prompt AFTER the cache anchors, as a final user message. If it ever +// carries a breakpoint, or the anchors move onto it, every change to it +// re-writes the cached prefix — the exact D2 bust it exists to prevent. +// ============================================================================= + +const tailConversation = [ + { role: "user", parts: [{ type: "text", content: "question one" }] }, + { role: "assistant", parts: [{ type: "text", content: "answer one" }] }, + { role: "user", parts: [{ type: "text", content: "question two" }] }, +] as any; + +function cacheMarkerOf(msg: any): unknown { + const content = msg.content; + if (typeof content === "string") return undefined; + const last = content[content.length - 1]; + return last?.providerOptions?.anthropic?.cacheControl; +} + +test("anthropic-family: ephemeralTail is appended after the cache anchors and carries no breakpoint", () => { + const opts = buildGenerateOptions(anthropicEntry, modelHandle, { + system: "be helpful", + messages: tailConversation, + ephemeralTail: "todo state", + } as any); + const msgs = opts.messages; + // Appended as the final user message, verbatim. + const tail = msgs[msgs.length - 1]; + assert.equal(tail.role, "user"); + assert.equal(tail.content, "todo state"); + // The tail changes every request, so it must never be a cache anchor… + assert.equal(cacheMarkerOf(tail), undefined); + // …and the write anchor must still sit on the last REAL message, so the + // next request's read anchor lands on a prefix an entry was written for. + assert.ok(cacheMarkerOf(msgs[msgs.length - 2])); +}); + +test("anthropic-family: no ephemeralTail leaves messages untouched", () => { + const opts = buildGenerateOptions(anthropicEntry, modelHandle, { + system: "be helpful", + messages: tailConversation, + } as any); + assert.equal(opts.messages.length, 3); +}); + +test("openai-shaped: ephemeralTail is appended as the final user message", () => { + const opts = buildGenerateOptions(openaiEntry, modelHandle, { + system: "be helpful", + messages: tailConversation, + ephemeralTail: "reminder text", + } as any); + const msgs = opts.messages; + assert.equal(msgs.length, 4); + assert.deepEqual(msgs[msgs.length - 1], { + role: "user", + content: "reminder text", + }); +}); diff --git a/apps/core/src/providers/generic-provider.ts b/apps/core/src/providers/generic-provider.ts index f242ea6..bbbbce3 100644 --- a/apps/core/src/providers/generic-provider.ts +++ b/apps/core/src/providers/generic-provider.ts @@ -1,5 +1,5 @@ // apps/core/src/providers/generic-provider.ts -import { generateText, streamText } from "ai"; +import { generateText, streamText, type ModelMessage } from "ai"; import { AIProvider, ExecuteOptions, @@ -59,6 +59,21 @@ function usesAnthropicOAuth(entry: ProviderCatalogueEntry): boolean { return anthropicOAuthForbidden() === undefined; } +/** + * Appends `opts.ephemeralTail` as a final user message. Request-scoped mutable + * state (memory / todos / reminders) enters the prompt here — after + * `applyMessageCaching` has placed its anchors — so the cached conversation + * prefix stays byte-stable while the tail changes freely (ExecuteOptions + * documents the contract). + */ +function appendEphemeralTail( + coreMessages: ModelMessage[], + tail: string | undefined, +): void { + if (!tail) return; + coreMessages.push({ role: "user", content: tail }); +} + /** * Assembles the AI SDK request options for one call, branching on the SDK * package rather than the provider id: @@ -107,6 +122,12 @@ export function buildGenerateOptions( if (opts.messages) { const coreMessages = convertToCoreMessages(opts.messages); applyMessageCaching(coreMessages); + // After the anchors, on purpose: the tail changes every request, so it + // must never carry a breakpoint — the write anchor has to land on the + // last real message for the next request's read anchor to hit it. The + // SDK folds a user message after tool results into the same wire + // message, so this rides exactly like a block. + appendEphemeralTail(coreMessages, opts.ephemeralTail); generateOptions.messages = coreMessages; } else { generateOptions.prompt = opts.prompt; @@ -118,7 +139,11 @@ export function buildGenerateOptions( : opts.system?.map((b) => b.text).join("\n\n"); generateOptions.system = systemPrompt; if (opts.messages) { - generateOptions.messages = convertToCoreMessages(opts.messages); + const coreMessages = convertToCoreMessages(opts.messages); + // Implicit prefix caches (OpenAI, DeepSeek, MiniMax) match the longest + // common prefix, so a changing tail costs only its own tokens here too. + appendEphemeralTail(coreMessages, opts.ephemeralTail); + generateOptions.messages = coreMessages; } else { generateOptions.prompt = opts.prompt; } @@ -290,11 +315,6 @@ export function createGenericProvider(entry: ProviderCatalogueEntry): AIProvider entry.defaultModel, !opts.quietModelFallback, ); - const generateOptions = buildGenerateOptions( - entry, - await modelHandle(model), - opts, - ); for (;;) { const usedOAuth = usesAnthropicOAuth(entry); const generateOptions = buildGenerateOptions( diff --git a/apps/core/src/providers/types.ts b/apps/core/src/providers/types.ts index 873a6d7..c305207 100644 --- a/apps/core/src/providers/types.ts +++ b/apps/core/src/providers/types.ts @@ -99,6 +99,14 @@ export interface ExecuteOptions { // Subagents pass their own id deliberately — a different context should not // be routed to the parent's cache. sessionId?: string; + // Request-scoped text appended as a final user message AFTER the cache + // anchors are placed (see buildGenerateOptions). For mutable per-turn state + // — memory recalls, todo list, drained reminders — that used to live in the + // system param, where any change re-sent the entire conversation: system + // sits upstream of every message, so one changed byte there busted every + // downstream breakpoint. At the tail it costs only its own tokens and the + // cached prefix stays byte-stable. Never persisted to history. + ephemeralTail?: string; } /** diff --git a/docs/caching-architecture.md b/docs/caching-architecture.md index 7517553..a2e837c 100644 --- a/docs/caching-architecture.md +++ b/docs/caching-architecture.md @@ -71,6 +71,20 @@ that rule pinned reads at ~7K (the system prefix) while input grew to 81K, never scaling with history. The naive `.slice(-2)` version left reads at 0 even on identical prefixes. +**The ephemeral tail** (`ExecuteOptions.ephemeralTail`, appended by +`generic-provider.ts appendEphemeralTail`). Mutable per-turn state — +memory recalls, the todo list, drained ``s — is +appended as a final user message *after* the anchors are placed, so it +never carries a breakpoint and the write anchor stays on the last real +message. It used to live in session system blocks, where any change +between inner-loop requests re-sent the entire conversation at full +price (system precedes every message in the prefix; reads collapsed to +the ~12K static block). At the tail it costs only its own tokens. +Measured on MiniMax-M3 after the move: reads track the previous +request's full input near token-exact, 83% cached across the trajectory +suite. `FREECODE_EPHEMERAL_TAIL=0` reverts to the old placement — it +exists only so `eval ab` can price the two placements side by side. + ### 1.2 Multi-provider `providerOptions` table — `utils.ts:115` One marker, five providers: diff --git a/docs/superpowers/specs/2026-08-05-token-efficiency.md b/docs/superpowers/specs/2026-08-05-token-efficiency.md index 1172c0b..228a9a4 100644 --- a/docs/superpowers/specs/2026-08-05-token-efficiency.md +++ b/docs/superpowers/specs/2026-08-05-token-efficiency.md @@ -159,6 +159,25 @@ surfaces running cost in the REPL (`cost-tracker.ts`, `costHook.ts`). The leak described in this spec ran for at least four days before anyone looked at the file; a live counter surfaces it inside one turn. +### RC8 — Session system blocks rewrote the prefix every inner-loop turn +*(found and fixed 2026-09-06 — the first bug the D2 detector caught in the wild)* + +Same class as RC3/RC4, discovered by the cache-observability spec's miss +detector rather than by hand. The memory block, todo block, and drained +``s (todo nudge, verify failures, redirect advice) were +session **system** blocks rebuilt every inner-loop iteration. System precedes +every message in the cached prefix, so any change — a todo update, a nudge +appearing for one request and vanishing the next — re-sent the entire +conversation at full price, with reads collapsing to the ~12K static block. +A single todo nudge cost two full-history busts (appear + disappear). + +Fixed by moving them to `ExecuteOptions.ephemeralTail`: a final user message +appended *after* `applyMessageCaching` places its anchors, so the tail changes +freely at the cost of only its own tokens (Claude Code's `` +architecture). Only the compaction summary may remain a mutable system block, +because compaction documents its own invalidation. Details: +`docs/caching-architecture.md` §1.1 and the cache-observability spec §D2.1. + ## Goal Cut input tokens per unit of work by an order of magnitude, without reducing what the diff --git a/docs/superpowers/specs/2026-08-09-cache-observability.md b/docs/superpowers/specs/2026-08-09-cache-observability.md index 94ff677..d54fff1 100644 --- a/docs/superpowers/specs/2026-08-09-cache-observability.md +++ b/docs/superpowers/specs/2026-08-09-cache-observability.md @@ -159,6 +159,40 @@ rebuild that compaction *must* cause is never reported as a bust. Off switch: `FREECODE_CACHE_MISS_NOTICES=0`. +#### D2.1 — One-sample deferral for provider blips (added 2026-09-06) + +D2's first real catch was the harness itself: memory/todo/reminder blocks lived +in session *system* blocks rebuilt every inner-loop iteration, so any change +re-sent the entire conversation (reads collapsed to the static prefix — the +RC3/RC4 class, undocumented). Fixed by moving them to an ephemeral tail user +message appended after the cache anchors (`ExecuteOptions.ephemeralTail`; see +`docs/caching-architecture.md` §1.1). + +The fix exposed a false-positive class: implicit provider caches miss for +non-rewrite reasons. Measured on MiniMax-M3 (eval session `ea079449`, turn 3): +read collapsed to 128, then the very next turn read *exactly the pre-miss +boundary* — only possible if the prefix bytes never changed. The cause is +provider-side (a write not yet committed when a rapid-fire inner-loop request +arrived, or eviction/routing), which the detector cannot distinguish from a +rewrite at the moment of the miss. + +So an **undocumented** miss is now held for one sample (`PendingMiss` in +`cache-miss.ts`): if the next read recovers to at least the pre-miss cached +prefix, the old entry was provably still valid and the miss is dropped in +silence; otherwise the alarm fires, one sample late. A documented miss is +still attributed immediately. A pending miss with no follow-up sample +(session end, generation bump, cache fields absent) is dropped — it cannot be +verified, and an unverifiable alarm is the wolf-cry this detector exists to +avoid. A persistent rewrite bug keeps producing pendings, so it still +surfaces loudly; the trade accepted is that a *one-off* rewrite whose next +read happens to exceed the old prefix is forgiven once. + +Known limit: a full provider-side eviction (read never recovers) is still +indistinguishable from a real rewrite and will alarm. The message wording +("this usually means something changed an already-sent message") stays honest +because after D2.1 the recovered blips — the common benign case — no longer +reach it. + ## Out of scope - Changing breakpoint placement. Four are in use, which is Anthropic's maximum; the @@ -179,4 +213,6 @@ Off switch: `FREECODE_CACHE_MISS_NOTICES=0`. - a normal warm turn produces no notice - a compaction-caused rebuild produces no notice (documented + generation bump) - an undocumented prefix change produces exactly one notice naming the tokens + (one sample late, per D2.1) + - a miss whose next read recovers to the pre-miss boundary produces no notice - a provider reporting no cache fields produces no notice diff --git a/evals/experiments.jsonl b/evals/experiments.jsonl index b6140c1..4e830d1 100644 --- a/evals/experiments.jsonl +++ b/evals/experiments.jsonl @@ -1,2 +1,3 @@ {"id":"2026-09-05-trajectory-1","hypothesis":"The two new negative-case failures (guess-dont-ask, missing-file flailing) are MiniMax-M3 behaviours, not case defects: a different model family passes them","suite":"trajectory","ranAt":"2026-09-05T17:20:32.724Z","commit":"2dc0f87","trials":3,"sides":{"baseline":{"model":"minimax/MiniMax-M3"},"candidate":{"model":"gemini/gemini-3.6-flash"}},"deltas":{"improved":1,"regressed":1},"totals":{"baseline":{"passed":2,"tokens":462466,"turns":21,"repeatedCalls":0,"costUsd":0.051920999999999995},"candidate":{"passed":2,"tokens":2002279,"turns":50,"repeatedCalls":0,"costUsd":0.5634824249999999}},"verdict":null} {"id": "2026-09-06-trajectory-greeting-scope-gate", "hypothesis": "Adding an explicit 'decide whether the message asks for work; a greeting needs no tools' scope gate to system.md stops a bare 'hi' from touring the repo", "suite": "trajectory", "ranAt": "2026-09-05T19:47:32.150712Z", "commit": "25a53bc", "trials": 5, "sides": {"baseline": {"prompt": "system.md at HEAD (no scope gate)"}, "candidate": {"prompt": "system.md + Identity-section scope gate naming read/ls/CLAUDE.md"}}, "cases": ["greeting-uses-no-tools", "thanks-uses-no-tools"], "results": {"baseline": {"greeting-uses-no-tools": "9/10", "thanks-uses-no-tools": "10/10"}, "candidate": {"greeting-uses-no-tools": "2/5 (plus one earlier majority-pass run)", "thanks-uses-no-tools": "5/5"}}, "verdict": "rejected", "note": "The gate made it WORSE: 9/10 -> ~5/10 on greeting-uses-no-tools. Reads as negation priming \u2014 the paragraph names 'read files', 'list directories', 'survey the project' and 'CLAUDE.md', raising the salience of the very actions it forbids. An earlier variant placed under Autonomy was inert (14-turn tour), consistent with system.md ranking BELOW project CLAUDE.md in the stated precedence. Reverted; system.md is unchanged. NOTE the baseline already passes 9/10 here, so a bare 'hi' in the eval harness does NOT reproduce the reported TUI failure (minimax/MiniMax-M3, agentMode danger, 34 read/ls calls touring the CLAUDE.md subsystem table). Something in the live session context \u2014 memory injection, skills, or the SessionStart summary \u2014 is the likelier trigger and is where to look next. Not interleaved: eval ab cannot vary a prompt file (variant spec takes only model= and env:, and loadSystemPrompt caches from disk with no env override)."} +{"id": "2026-09-05-redirect-1", "hypothesis": "Ephemeral-tail placement of memory/todo/reminder blocks preserves the prompt-cache prefix on multi-turn runs: cost per trial drops via cache-read discount, with pass rate and repeated calls unchanged", "suite": "redirect", "ranAt": "2026-09-05T21:09:29.995Z", "commit": "e7517df", "trials": 3, "sides": {"baseline": {"env:FREECODE_EPHEMERAL_TAIL": "0"}, "candidate": {"env:FREECODE_EPHEMERAL_TAIL": "1"}}, "deltas": {"unchanged-pass": 5}, "totals": {"baseline": {"passed": 15, "tokens": 966860, "turns": 45, "repeatedCalls": 0, "costUsd": 0.09069179999999999}, "candidate": {"passed": 15, "tokens": 1400453, "turns": 61, "repeatedCalls": 0, "costUsd": 0.12717012}}, "verdict": "kept", "note": "Kept on correctness, not on this total: rollout evidence (session ea079449) shows reads tracking the full prior input near token-exact with the tail vs collapsing to the static prefix without it. A/B quality unchanged (5/5 unchanged-pass, repeatedCalls 0->0 both sides). Cost totals were turn-count-confounded (+16 turns on candidate, mostly count-something-tedious 11->22 on a spiral-by-design case); the one turn-matched case (answer-needs-a-file-not-a-search, 7 vs 7) cost -22%. Watch: whether tail-placed todo nudges lengthen tedious runs."} From d18188cdc103f81d88a4bf58f42bea051c30e46a Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sun, 6 Sep 2026 02:52:57 +0530 Subject: [PATCH 3/3] feat(tool-group): implement ToolGroupMessage for summarizing consecutive tool results --- apps/tui/src/components/index.ts | 65 ++++++- .../src/components/tool-group-message.test.ts | 121 +++++++++++++ apps/tui/src/components/tool-group-message.ts | 163 ++++++++++++++++++ apps/tui/src/components/tool-messages.test.ts | 77 ++++++++- .../tui/src/components/tool-result-message.ts | 67 ++++++- .../src/components/virtual-message-list.ts | 45 +++-- 6 files changed, 506 insertions(+), 32 deletions(-) create mode 100644 apps/tui/src/components/tool-group-message.test.ts create mode 100644 apps/tui/src/components/tool-group-message.ts diff --git a/apps/tui/src/components/index.ts b/apps/tui/src/components/index.ts index 6105c8a..dbcb875 100644 --- a/apps/tui/src/components/index.ts +++ b/apps/tui/src/components/index.ts @@ -14,11 +14,13 @@ import { createMessageComponent, ThinkingMessage } from "./message-row.js"; import type { MessageType, MessageInstance } from "./message-types.js"; import { ToolProgressMessage } from "./tool-progress-message.js"; import { ToolResultMessage } from "./tool-result-message.js"; +import { ToolGroupMessage } from "./tool-group-message.js"; /** * Add a user message to the store and return the message instance */ export function createUserMessage(content: string): MessageInstance { + sealToolGroups(); const component = createMessageComponent("user", content); return addMessage("user", content, component); } @@ -54,6 +56,7 @@ export function promoteQueuedToUser(queueId: string): MessageInstance | undefine * Add an assistant message to the store and return the message instance */ export function createAssistantMessage(content: string): MessageInstance { + sealToolGroups(); const component = createMessageComponent("assistant", content); return addMessage("assistant", content, component); } @@ -157,6 +160,41 @@ export function createToolProgressMessage( return addMessage("tool", toolName, component); } +/** + * Closes every open tool group. A user prompt, an assistant reply, or a + * thinking block ends the run of calls it belongs to, so the next call starts + * a fresh group below the new message. + */ +export function sealToolGroups(): void { + for (const msg of getMessages()) { + if (msg.component instanceof ToolGroupMessage) msg.component.seal(); + } +} + +/** + * The group still accepting calls, if any. Tool progress rows and the + * in-progress line are skipped: with parallel tools, a sibling that is still + * running sits between the group and the result now arriving, and that must + * not start a second group. + */ +function findOpenToolGroup(): ToolGroupMessage | undefined { + const messages = getMessages(); + for (let i = messages.length - 1; i >= 0; i--) { + const msg = messages[i]!; + if (msg.component instanceof ToolGroupMessage) { + return msg.component.isSealed ? undefined : msg.component; + } + if (msg.component instanceof ToolProgressMessage) continue; + if (msg.type === "in_progress") continue; + // Ambient notices (cache status, recovery lines) interleave with tool + // calls but are not part of the conversation, so they must not chop one + // run of calls into a group per call. + if (msg.type === "system") continue; + return undefined; + } + return undefined; +} + export function createToolResultMessage( toolCallId: string, toolName: string, @@ -165,15 +203,22 @@ export function createToolResultMessage( success: boolean, duration_ms?: number, ): MessageInstance { - const component = new ToolResultMessage({ - toolCallId, - toolName, - args, - result, - success, - duration_ms, - }); - return addMessage("tool", toolName, component); + const options = { toolCallId, toolName, args, result, success, duration_ms }; + + const open = findOpenToolGroup(); + if (open) { + open.add(options); + // Same store entry, new content — the list re-renders it in place. + return updateMessage( + getMessages().find((m) => m.component === open)!.id, + toolName, + open, + )!; + } + + const group = new ToolGroupMessage(); + group.add(options); + return addMessage("tool", toolName, group); } /** @@ -189,6 +234,7 @@ export function createThinkingMessage(content: string, startTime?: number): Mess return lastMessage; } + sealToolGroups(); const component = createMessageComponent("thinking", content, startTime); return addMessage("thinking", content, component); } @@ -256,3 +302,4 @@ export { ToolResultMessage, type ToolResultMessageOptions, } from "./tool-result-message.js"; +export { ToolGroupMessage } from "./tool-group-message.js"; diff --git a/apps/tui/src/components/tool-group-message.test.ts b/apps/tui/src/components/tool-group-message.test.ts new file mode 100644 index 0000000..72441ce --- /dev/null +++ b/apps/tui/src/components/tool-group-message.test.ts @@ -0,0 +1,121 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { clearMessages, getMessages } from "../state/message-store.js"; +import { + createToolProgressMessage, + createToolResultMessage, + createAssistantMessage, + createSystemMessage, + sealToolGroups, +} from "./index.js"; +import { ToolGroupMessage } from "./tool-group-message.js"; + +function plain(lines: string[]): string { + return lines.join("|").replace(/\x1b\[[0-9;]*m/g, ""); +} + +function addResult(name: string, args: Record, success = true) { + return createToolResultMessage(`call-${name}-${Math.random()}`, name, args, "ok", success); +} + +test("consecutive tool results share one message and summarize by tool", () => { + clearMessages(); + + addResult("Read", { file_path: "a.ts" }); + addResult("Read", { file_path: "b.ts" }); + addResult("Edit", { file_path: "c.ts" }); + addResult("Bash", { command: "ls" }); + + const messages = getMessages(); + assert.equal(messages.length, 1); + const group = messages[0]!.component as ToolGroupMessage; + assert.ok(group instanceof ToolGroupMessage); + assert.equal(group.size, 4); + + // Collapsed from the start: the summary is the whole message, live or not. + assert.doesNotMatch(plain(group.render(80)), /a\.ts/); + + sealToolGroups(); + const collapsed = group.render(80); + assert.match(plain(collapsed), /Read 2 files, Updated 1 file, Ran 1 command/); + assert.doesNotMatch(plain(collapsed), /a\.ts/); + assert.match(plain(collapsed), /▶ /); +}); + +test("clicking the summary expands the group back to one row per call", () => { + clearMessages(); + addResult("Read", { file_path: "a.ts" }); + addResult("Read", { file_path: "b.ts" }); + sealToolGroups(); + + const group = getMessages()[0]!.component as ToolGroupMessage; + group.render(80); + assert.equal(group.isToggleLine(0), false); // leading blank + assert.equal(group.isToggleLine(1), true); // summary + + group.toggleAt(1); + const expanded = group.render(80); + assert.match(plain(expanded), /a\.ts/); + assert.match(plain(expanded), /b\.ts/); + assert.match(plain(expanded), /▼ /); +}); + +test("a failed call marks the summary", () => { + clearMessages(); + addResult("Bash", { command: "false" }, false); + sealToolGroups(); + + const group = getMessages()[0]!.component as ToolGroupMessage; + assert.match(plain(group.render(80)), /Ran 1 command \(1 failed\)/); +}); + +test("an unknown tool falls back to a count instead of invented grammar", () => { + clearMessages(); + addResult("mcp__thing__do", {}); + addResult("mcp__thing__do", {}); + sealToolGroups(); + + const group = getMessages()[0]!.component as ToolGroupMessage; + assert.match(plain(group.render(80)), /mcp__thing__do ×2/); +}); + +test("a still-running sibling tool does not split the group", () => { + clearMessages(); + addResult("Read", { file_path: "a.ts" }); + // Parallel batch: a second tool is still streaming when the first result lands. + createToolProgressMessage("call-live", "Bash", { command: "sleep 1" }); + addResult("Read", { file_path: "b.ts" }); + + const groups = getMessages().filter((m) => m.component instanceof ToolGroupMessage); + assert.equal(groups.length, 1); + assert.equal((groups[0]!.component as ToolGroupMessage).size, 2); +}); + +test("an assistant reply seals the group, so the next call starts a new one", () => { + clearMessages(); + addResult("Read", { file_path: "a.ts" }); + createAssistantMessage("done"); + addResult("Read", { file_path: "b.ts" }); + + const groups = getMessages().filter((m) => m.component instanceof ToolGroupMessage); + assert.equal(groups.length, 2); + assert.equal((groups[0]!.component as ToolGroupMessage).isSealed, true); +}); + +test("ambient system notices do not split a run into one group per call", () => { + clearMessages(); + addResult("Bash", { command: "ls" }); + // Every turn emits a cache-status line between tool calls. + createSystemMessage("*Prompt cache hit: 13,440 tokens read*"); + addResult("Bash", { command: "pwd" }); + createSystemMessage("⚠ **Prompt-cache miss**"); + addResult("Read", { file_path: "a.ts" }); + sealToolGroups(); + + const groups = getMessages().filter((m) => m.component instanceof ToolGroupMessage); + assert.equal(groups.length, 1); + assert.match( + plain((groups[0]!.component as ToolGroupMessage).render(80)), + /Ran 2 commands, Read 1 file/, + ); +}); diff --git a/apps/tui/src/components/tool-group-message.ts b/apps/tui/src/components/tool-group-message.ts new file mode 100644 index 0000000..8d8f675 --- /dev/null +++ b/apps/tui/src/components/tool-group-message.ts @@ -0,0 +1,163 @@ +import { Component, truncateToWidth } from "@earendil-works/pi-tui"; +import chalk from "chalk"; +import { ToolResultMessage, type ToolResultMessageOptions } from "./tool-result-message.js"; +import { formatDuration } from "../utils/format-duration.js"; + +/** + * Verb + noun used to describe a run of calls to one tool: `Read 2 files`. + * Tools absent from the table fall back to `Name ×N`, so an MCP tool nobody + * anticipated still summarizes without inventing English for it. + */ +const TOOL_PHRASES: Record = { + read: ["Read", "file"], + view_file: ["Read", "file"], + write: ["Updated", "file"], + edit: ["Updated", "file"], + replace_file_content: ["Updated", "file"], + multi_replace_file_content: ["Updated", "file"], + bash: ["Ran", "command"], + run_command: ["Ran", "command"], + grep: ["Searched", "pattern"], + glob: ["Searched", "pattern"], + ls: ["Listed", "directory"], + webfetch: ["Fetched", "page"], + websearch: ["Searched", "query"], + todowrite: ["Updated", "todo list"], + skill: ["Ran", "skill"], + agent: ["Ran", "subagent"], + memory: ["Saved", "memory"], + lsp: ["Queried", "symbol"], + question: ["Asked", "question"], +}; + +function pluralize(noun: string, count: number): string { + if (count === 1) return noun; + if (/[^aeiou]y$/.test(noun)) return `${noun.slice(0, -1)}ies`; + return `${noun}s`; +} + +/** Rows the group owns: its own summary, a child's row, or spacing. */ +type LineOwner = "summary" | { item: ToolResultMessage; local: number } | null; + +/** + * A contiguous run of finished tool calls, rendered as one line. + * + * The group is collapsed from the start: while the turn runs, its + * `Read 2 files, Updated 5 files` summary grows in place as calls finish. + * Sealing it (the next thinking block, or the assistant's reply) only ends + * the run so the next call starts a fresh group. A click puts the individual + * calls back, and each of those still expands to its own output. + */ +export class ToolGroupMessage implements Component { + private items: ToolResultMessage[] = []; + private entries: ToolResultMessageOptions[] = []; + private sealed = false; + private expanded = false; + private lineOwners: LineOwner[] = []; + + add(options: ToolResultMessageOptions): ToolResultMessage { + const item = new ToolResultMessage(options); + this.items.push(item); + this.entries.push(options); + return item; + } + + /** Closes the group so the next tool call starts a fresh one. */ + seal(): void { + this.sealed = true; + } + + get isSealed(): boolean { + return this.sealed; + } + + get size(): number { + return this.items.length; + } + + invalidate(): void { + for (const item of this.items) item.invalidate(); + } + + toggle(): void { + this.expanded = !this.expanded; + } + + /** + * Clicking the summary folds the group; clicking a child's header folds that + * one call. Every other row is left alone so drag-select still works over + * expanded output. + */ + toggleAt(local: number): void { + const owner = this.lineOwners[local]; + if (owner === "summary") this.toggle(); + else if (owner) owner.item.toggle(); + } + + isToggleLine(local: number): boolean { + const owner = this.lineOwners[local]; + if (owner === "summary") return true; + return !!owner && owner.item.isToggleLine(owner.local); + } + + render(width: number): string[] { + const expanded = this.expanded; + const lines: string[] = []; + this.lineOwners = []; + + const push = (line: string, owner: LineOwner) => { + lines.push(line); + this.lineOwners.push(owner); + }; + + push("", null); + + const failed = this.entries.filter((e) => !e.success).length; + const icon = failed > 0 ? chalk.red("✖") : chalk.green("●"); + const caret = chalk.dim(expanded ? "▼ " : "▶ "); + let summary = `${caret}${icon} ${this.summaryText()}`; + if (failed > 0) { + summary += ` ${chalk.red(`(${failed} failed)`)}`; + } + const total = this.entries.reduce((sum, e) => sum + (e.duration_ms ?? 0), 0); + if (total > 0) { + summary += ` ${chalk.dim(`(${formatDuration(total)})`)}`; + } + push(truncateToWidth(summary, Math.max(20, width - 1)), "summary"); + + if (!expanded) return lines; + + for (const item of this.items) { + const rendered = item.render(width); + for (let local = 0; local < rendered.length; local++) { + push(rendered[local]!, { item, local }); + } + } + return lines; + } + + /** `Read 2 files, Updated 5 files` — buckets in first-seen order. */ + private summaryText(): string { + const buckets: { label: (n: number) => string; count: number }[] = []; + const byKey = new Map(); + + for (const entry of this.entries) { + const phrase = TOOL_PHRASES[entry.toolName.toLowerCase()]; + const key = phrase ? phrase.join(" ") : `raw:${entry.toolName}`; + let bucket = byKey.get(key); + if (!bucket) { + bucket = { + count: 0, + label: phrase + ? (n: number) => `${phrase[0]} ${n} ${pluralize(phrase[1], n)}` + : (n: number) => `${entry.toolName} ×${n}`, + }; + byKey.set(key, bucket); + buckets.push(bucket); + } + bucket.count++; + } + + return buckets.map((b) => chalk.bold(b.label(b.count))).join(chalk.dim(", ")); + } +} diff --git a/apps/tui/src/components/tool-messages.test.ts b/apps/tui/src/components/tool-messages.test.ts index d438f94..a7c551c 100644 --- a/apps/tui/src/components/tool-messages.test.ts +++ b/apps/tui/src/components/tool-messages.test.ts @@ -39,16 +39,62 @@ test("large multi-line results render one terminal row per line, collapsed", () success: true, }); + // Collapsed by default: the header is the whole message. + assert.deepEqual(msg.render(80).length, 1); + assert.match(plain(msg.render(80)), /\u25b6 . Run\(ls\)/); + assert.doesNotMatch(plain(msg.render(80)), /out-0/); + + msg.toggle(); const lines = msg.render(80); // No rendered element may contain an embedded newline — pi-tui counts each // array element as exactly one terminal row. for (const line of lines) { assert.ok(!line.includes("\n"), `embedded newline in: ${line}`); } - // Collapsed preview: 5 result lines + "… +45 lines" tail. + // Expanded preview: 5 result lines + "… +45 lines" tail. assert.match(lines.join("|"), /out-4/); assert.doesNotMatch(lines.join("|"), /out-5\b/); assert.match(lines.join("|"), /\+45 lines/); + assert.match(plain(lines), /\u25bc /); +}); + +test("only the header row toggles, so expanded output stays selectable", () => { + const msg = new ToolResultMessage({ + toolCallId: "call-2b", + toolName: "Bash", + args: { command: "ls" }, + result: "a\nb\nc", + success: true, + }); + + msg.render(80); + assert.equal(msg.isToggleLine(0), true); // collapsed: header is row 0 + + msg.toggle(); + msg.render(80); + // Expanded: row 0 is the leading blank, row 1 the header, rest is output. + assert.equal(msg.isToggleLine(0), false); + assert.equal(msg.isToggleLine(1), true); + assert.equal(msg.isToggleLine(2), false); +}); + +test("a diff result is never collapsible and renders untouched", () => { + const msg = new ToolResultMessage({ + toolCallId: "call-2c", + toolName: "Edit", + args: { file_path: "x.ts" }, + result: " 1 keep\n- 2 old\n+ 2 new\n", + success: true, + }); + + const lines = msg.render(80); + assert.doesNotMatch(plain(lines), /\u25b6|\u25bc/); + assert.match(plain(lines), /Update\(x\.ts\)/); + assert.match(plain(lines), /Added 1 line, removed 1 line/); + // No row claims the click, so drag-select works across the whole diff. + for (let i = 0; i < lines.length; i++) { + assert.equal(msg.isToggleLine(i), false); + } }); test("a multi-line bash command is flattened in the Run header", () => { @@ -108,3 +154,32 @@ test("a ranged read shows the line window in the header", () => { assert.match(wholeFile, /auth\.ts\)/); assert.equal(/auth\.ts:/.test(wholeFile), false); }); + +test("a read of a bulleted markdown file is not mistaken for a diff", () => { + const msg = new ToolResultMessage({ + toolCallId: "call-2d", + toolName: "Read", + args: { file_path: "README.md" }, + result: "# Title\n\n- one\n- two\n", + success: true, + }); + + const lines = msg.render(80); + assert.doesNotMatch(plain(lines), /Removed 2 lines/); + assert.doesNotMatch(plain(lines), /one/); + assert.match(plain(lines), /Read\(README\.md\)/); +}); + +test("a read is a single row — no blank lines framing a bodyless header", () => { + const msg = new ToolResultMessage({ + toolCallId: "call-2e", + toolName: "Read", + args: { file_path: "a.ts" }, + result: "whatever the file said", + success: true, + }); + + const lines = msg.render(80); + assert.equal(lines.length, 1); + assert.match(plain(lines), /Read\(a\.ts\)/); +}); diff --git a/apps/tui/src/components/tool-result-message.ts b/apps/tui/src/components/tool-result-message.ts index 874df80..9884f7f 100644 --- a/apps/tui/src/components/tool-result-message.ts +++ b/apps/tui/src/components/tool-result-message.ts @@ -61,6 +61,17 @@ export class ToolResultMessage implements Component { private result?: string; private success: boolean; private duration_ms?: number; + /** Collapsed by default; a click on the header row expands the body. */ + private isCollapsed = true; + /** + * Whether this message has a body worth hiding. False for diffs (they are + * the message's point and render as they always have) and for tools whose + * output is already suppressed, so no caret ever promises content that + * expanding would not reveal. Set during `render`. + */ + private collapsible = false; + /** Header row's index within the last render, for `isToggleLine`. */ + private headerLineIndex = 0; constructor(options: ToolResultMessageOptions) { this.toolCallId = options.toolCallId; @@ -75,17 +86,28 @@ export class ToolResultMessage implements Component { // Nothing to clean up } + toggle(): void { + this.isCollapsed = !this.isCollapsed; + } + + /** + * Only the header row toggles. Restricting it this way keeps drag-select + * working over expanded output — a press anywhere in the body still starts a + * selection instead of being swallowed as a collapse — and stops diff blocks, + * which never collapse, from claiming clicks at all. + */ + isToggleLine(localIndex: number): boolean { + return this.collapsible && localIndex === this.headerLineIndex; + } + render(width: number): string[] { const colorFn = TOOL_COLORS[this.toolName] || TOOL_COLORS[this.toolName.toLowerCase()] || TOOL_COLORS[this.toolName.charAt(0).toUpperCase() + this.toolName.slice(1).toLowerCase()] || ((t: string) => t); const statusIcon = this.success ? chalk.green("●") : chalk.red("✖"); const argsStr = this.formatArgs(); const duration = this.duration_ms !== undefined ? `(${formatDuration(this.duration_ms)})` : ""; - const lines: string[] = []; - lines.push(""); // Empty line above - const safeWidth = Math.max(20, width - 1); - + let headerAction = this.toolName; let headerTarget = `(${argsStr})`; @@ -114,18 +136,37 @@ export class ToolResultMessage implements Component { } } - let header = `${statusIcon} ${chalk.bold(colorFn(headerAction))}${headerTarget}`; + const displayResult = this.unwrapOutput(this.result); + // `looksLikeDiff` only tests for leading +/-, so any markdown bullet list + // trips it — a README read as "Removed 6 lines". Read-type tools suppress + // their body anyway, so they never take the diff branch. + const isDiff = !isFileRead && !!displayResult && looksLikeDiff(displayResult); + // `isFileRead` output is dropped below, so those never get a caret either. + this.collapsible = !isDiff && !isFileRead && (!!displayResult || this.success); + const collapsed = this.collapsible && this.isCollapsed; + + const lines: string[] = []; + // A collapsed tool is one row in a stacked list, so it drops the blank + // lines that frame an expanded block. + if (!collapsed) lines.push(""); // Empty line above + + const caret = this.collapsible + ? chalk.dim(this.isCollapsed ? "\u25b6 " : "\u25bc ") + : ""; + let header = `${caret}${statusIcon} ${chalk.bold(colorFn(headerAction))}${headerTarget}`; if (duration) { header += ` ${chalk.dim(duration)}`; } - + header = truncateToWidth(header, safeWidth); + this.headerLineIndex = lines.length; lines.push(header); + if (collapsed) return lines; + const resultWidth = safeWidth - 3; // 3 for " " or "└─ " - const displayResult = this.unwrapOutput(this.result); - - if (displayResult && looksLikeDiff(displayResult)) { + + if (isDiff && displayResult) { const stats = getDiffStats(displayResult); let statText = "No changes"; if (stats.added > 0 && stats.removed > 0) statText = `Added ${stats.added} line${stats.added === 1 ? "" : "s"}, removed ${stats.removed} line${stats.removed === 1 ? "" : "s"}`; @@ -175,6 +216,14 @@ export class ToolResultMessage implements Component { lines.push(` ${chalk.dim("└─")} ${chalk.dim("(no output)")}`); } + // A read prints no body — its header is the whole message. Framing that + // one row with blank lines made a run of reads twice as tall as the + // collapsed tools around it, so a bodyless row stays a single row. + if (lines.length === this.headerLineIndex + 1) { + this.headerLineIndex = 0; + return [header]; + } + lines.push(""); // Empty line below return lines; } diff --git a/apps/tui/src/components/virtual-message-list.ts b/apps/tui/src/components/virtual-message-list.ts index f03c5e5..4c50c65 100644 --- a/apps/tui/src/components/virtual-message-list.ts +++ b/apps/tui/src/components/virtual-message-list.ts @@ -161,7 +161,7 @@ export class VirtualMessageList implements Component { } /** Maps each rendered line index to its owning message (null for header rows). */ - private lastLineMap: { msg: MessageInstance | null }[] = []; + private lastLineMap: { msg: MessageInstance | null; local: number }[] = []; handleClick(_cx: number, cy: number): boolean { const content = this.contentRows(); @@ -179,12 +179,30 @@ export class VirtualMessageList implements Component { const relativeY = cy - offset; const clickedIndex = startIndex + (relativeY - 1); const entry = this.lastLineMap[clickedIndex]; - if (entry?.msg?.component && "toggle" in entry.msg.component && typeof (entry.msg.component as any).toggle === "function") { - (entry.msg.component as any).toggle(); - this.invalidateMessage(entry.msg.id); - return true; + const component = entry?.msg?.component as + | { + toggle?: () => void; + toggleAt?: (local: number) => void; + isToggleLine?: (local: number) => boolean; + } + | undefined; + if (typeof component?.toggle !== "function") return false; + // A component may restrict toggling to specific rows of its own render + // (tool results expose only their header, so a press inside expanded + // output still starts a drag-selection). One that doesn't toggles from + // anywhere in the message, as thoughts always have. + if ( + typeof component.isToggleLine === "function" && + !component.isToggleLine(entry!.local) + ) { + return false; } - return false; + // A group owns rows belonging to its children, so it decides which of + // them the press folds; a plain component just toggles itself. + if (typeof component.toggleAt === "function") component.toggleAt(entry!.local); + else component.toggle(); + this.invalidateMessage(entry!.msg!.id); + return true; } /** Returns the last-rendered ANSI string for a full (unwindowed) line index. */ @@ -282,7 +300,7 @@ export class VirtualMessageList implements Component { const headerLines = this.header.render(width); lines.push(...headerLines, ""); for (let i = 0; i < headerLines.length + 1; i++) { - this.lastLineMap.push({ msg: null }); // Header rows own no message + this.lastLineMap.push({ msg: null, local: i }); // Header rows own no message } } @@ -300,18 +318,19 @@ export class VirtualMessageList implements Component { for (let i = 0; i < visibleMessages.length; i++) { const msg = visibleMessages[i]!; - for (const line of perMessage[i]!) { - lines.push(line); - this.lastLineMap.push({ msg }); + const rendered = perMessage[i]!; + for (let local = 0; local < rendered.length; local++) { + lines.push(rendered[local]!); + this.lastLineMap.push({ msg, local }); } } // Render in-progress message at the very bottom (if exists) if (inProgressMessage) { const inProgressLines = inProgressMessage.component.render(width); - for (const line of inProgressLines) { - lines.push(line); - this.lastLineMap.push({ msg: inProgressMessage }); + for (let local = 0; local < inProgressLines.length; local++) { + lines.push(inProgressLines[local]!); + this.lastLineMap.push({ msg: inProgressMessage, local }); } }