diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cb3e9c4..f676aed6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,14 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ## [Unreleased] +### Agent + +- **Compaction keeps scored work, not retry loops.** Errored tool results are no + longer auto-pinned; identical errors collapse to one representative. Anchors + are scored (writes, successful task completions, plan updates) and pair + closures count against `maxAnchorTurns`. The LLM summary is workflow-aware + and skips degenerate assistant text. + ### Plugins - **`run_shell` no longer defaults to a 15s timeout.** Omitted timeout arms no diff --git a/src/context-compactor.test.ts b/src/context-compactor.test.ts index 0ff5a3a9..40b9ea41 100644 --- a/src/context-compactor.test.ts +++ b/src/context-compactor.test.ts @@ -335,6 +335,248 @@ describe("createPruningCompactor — image aging", () => { }); }); +describe("createPruningCompactor — error anchoring (CL-6906)", () => { + function assistantErrorCall(id: string, name: string): ConversationTurn { + return makeTurn({ + role: "assistant", + content: [{ type: "tool_call", id, name, arguments: {} }], + }); + } + function errorResult(callId: string, text: string): ConversationTurn { + return makeTurn({ + role: "user", + content: [{ type: "tool_result", callId, content: [{ type: "text", text }], isError: true }], + }); + } + function padding(n: number, prefix: string): ConversationTurn[] { + return Array.from({ length: n }, (_, i) => + makeTurn({ + role: i % 2 === 0 ? "assistant" : "user", + content: [{ type: "text", text: `${prefix}${i}` }], + }), + ); + } + + test("a lone errored tool_result no longer anchors on its own", async () => { + const turns: ConversationTurn[] = [ + makeTurn({ role: "user", content: [{ type: "text", text: "the initiating task" }] }), + ...padding(3, "before"), + assistantErrorCall("e1", "run_shell"), + errorResult("e1", "Error: exit code 1 " + "x".repeat(100)), + ...padding(8, "after"), + ]; + const compactor = createPruningCompactor({ + keepRecentTurns: 6, + maxAnchorTurns: 8, + summaryMaxChars: 2000, + }); + const { output } = await compactor.apply(turns, mockStrategyCtx); + // The lone error's own turn score (3) sits below the anchor threshold (5), + // so its body must not survive verbatim outside the recent window. + const survivedVerbatim = output.some((t) => + t.content.some((b) => b.type === "tool_result" && b.callId === "e1"), + ); + expect(survivedVerbatim).toBe(false); + }); + + test("two distinct errors on one turn still clear the anchor threshold", async () => { + const turns: ConversationTurn[] = [ + makeTurn({ role: "user", content: [{ type: "text", text: "the initiating task" }] }), + ...padding(3, "before"), + makeTurn({ + role: "assistant", + content: [ + { type: "tool_call", id: "d1", name: "run_shell", arguments: {} }, + { type: "tool_call", id: "d2", name: "grep", arguments: {} }, + ], + }), + makeTurn({ + role: "user", + content: [ + { + type: "tool_result", + callId: "d1", + content: [{ type: "text", text: "Error: build failed" }], + isError: true, + }, + { + type: "tool_result", + callId: "d2", + content: [{ type: "text", text: "Error: no matches found" }], + isError: true, + }, + ], + }), + ...padding(8, "after"), + ]; + const compactor = createPruningCompactor({ + keepRecentTurns: 6, + maxAnchorTurns: 8, + summaryMaxChars: 2000, + }); + const { output } = await compactor.apply(turns, mockStrategyCtx); + const kept = output.find((t) => + t.content.some((b) => b.type === "tool_result" && b.callId === "d1"), + ); + expect(kept).toBeDefined(); + expect(kept?.content.some((b) => b.type === "tool_result" && b.callId === "d2")).toBe(true); + }); + + test("repeated identical errors collapse to one representative before anchor selection", async () => { + // "old" repeats the same (tool, error-text) signature that recurs again + // later ("recur"); combined with a distinct error on the same turn, the + // uncollapsed score (3 + 3 = 6) would clear the threshold, but the + // collapsed score (0 + 3 = 3) must not. + const sharedErrorText = "Error: type mismatch on line 12, expected string"; + const turns: ConversationTurn[] = [ + makeTurn({ role: "user", content: [{ type: "text", text: "the initiating task" }] }), + ...padding(3, "before"), + makeTurn({ + role: "assistant", + content: [ + { type: "tool_call", id: "old", name: "edit_file_check", arguments: {} }, + { type: "tool_call", id: "uniq", name: "grep", arguments: {} }, + ], + }), + makeTurn({ + role: "user", + content: [ + { + type: "tool_result", + callId: "old", + content: [{ type: "text", text: sharedErrorText }], + isError: true, + }, + { + type: "tool_result", + callId: "uniq", + content: [{ type: "text", text: "Error: distinct failure here" }], + isError: true, + }, + ], + }), + ...padding(4, "mid"), + assistantErrorCall("recur", "edit_file_check"), + errorResult("recur", sharedErrorText), + ...padding(8, "after"), + ]; + const compactor = createPruningCompactor({ + keepRecentTurns: 6, + maxAnchorTurns: 8, + summaryMaxChars: 2000, + }); + const { output, record } = await compactor.apply(turns, mockStrategyCtx); + expect(record.decisions["repeatedErrorCount"]).toBe(1); + // The combined turn's score drops below threshold once "old" is + // collapsed, so neither of its results survives verbatim. + const oldSurvived = output.some((t) => + t.content.some((b) => b.type === "tool_result" && b.callId === "old"), + ); + const uniqSurvived = output.some((t) => + t.content.some((b) => b.type === "tool_result" && b.callId === "uniq"), + ); + expect(oldSurvived).toBe(false); + expect(uniqSurvived).toBe(false); + }); +}); + +describe("createPruningCompactor — maxAnchorTurns caps pairing pulls (CL-6906)", () => { + test("bounds the total scored-anchor pull even when many high-score pairs are scattered through history", async () => { + const turns: ConversationTurn[] = [ + makeTurn({ role: "user", content: [{ type: "text", text: "the initiating task" }] }), + ]; + // 10 write-pair call/result turns, well separated from each other and from + // the recent window. A single edit_file scores 3 (below the threshold of + // 5); two writes on the same assistant turn score 6, so each pair + // independently clears the scored-anchor bar. + for (let i = 0; i < 10; i++) { + turns.push( + makeTurn({ + role: "assistant", + content: [ + { + type: "tool_call", + id: `edit${i}a`, + name: "edit_file", + arguments: { path: `f${i}a.ts` }, + }, + { + type: "tool_call", + id: `edit${i}b`, + name: "edit_file", + arguments: { path: `f${i}b.ts` }, + }, + ], + }), + makeTurn({ + role: "user", + content: [ + { + type: "tool_result", + callId: `edit${i}a`, + content: [{ type: "text", text: `edited f${i}a.ts` }], + }, + { + type: "tool_result", + callId: `edit${i}b`, + content: [{ type: "text", text: `edited f${i}b.ts` }], + }, + ], + }), + makeTurn({ role: "assistant", content: [{ type: "text", text: `note ${i}` }] }), + makeTurn({ role: "user", content: [{ type: "text", text: `ask ${i}` }] }), + ); + } + for (let i = 0; i < 6; i++) { + turns.push( + makeTurn({ + role: i % 2 === 0 ? "assistant" : "user", + content: [{ type: "text", text: `recent${i}` }], + }), + ); + } + + const maxAnchorTurns = 4; + const compactor = createPruningCompactor({ + keepRecentTurns: 6, + maxAnchorTurns, + summaryMaxChars: 2000, + }); + const { record } = await compactor.apply(turns, mockStrategyCtx); + // The initiating task (1 turn, no partners) is kept outside the cap; the + // scored/pair-partner pull must stay within maxAnchorTurns. + const anchorTurnCount = record.decisions["anchorTurnCount"] as number; + expect(anchorTurnCount - 1).toBeLessThanOrEqual(maxAnchorTurns); + // With a budget of 4 and each edit pair costing 2 (call + result), exactly + // two pairs (the most recent two) fit; a third would overshoot and must + // be rejected as a whole, not split. + expect(anchorTurnCount).toBe(1 + 4); + }); +}); + +describe("createPruningCompactor — summarize receives the workflow context (CL-6906)", () => { + test("passes cfg.summaryContext() through to summarize as the second argument", async () => { + let capturedCtx: unknown = "not called"; + const workflowCtx = { workflow: { name: "build", stepIndex: 2, total: 7 } }; + const compactor = createPruningCompactor({ + keepRecentTurns: 1, + summaryMaxChars: 500, + summaryContext: () => workflowCtx, + summarize: async (_turns, ctx) => { + capturedCtx = ctx; + return "summary text"; + }, + }); + const turns: ConversationTurn[] = [ + makeTurn({ role: "assistant", content: [{ type: "text", text: "a" }] }), + makeTurn({ role: "assistant", content: [{ type: "text", text: "b" }] }), + makeTurn({ role: "user", content: [{ type: "text", text: "recent" }] }), + ]; + await compactor.apply(turns, mockStrategyCtx); + expect(capturedCtx).toBe(workflowCtx); + }); +}); + describe("buildContextEnvelope", () => { test("includes active task label", () => { const result = buildContextEnvelope({ diff --git a/src/session/compactor.ts b/src/session/compactor.ts index 038fcfce..81a5dc31 100644 --- a/src/session/compactor.ts +++ b/src/session/compactor.ts @@ -20,6 +20,7 @@ import type { StrategyBlob, } from "@intx/types/runtime"; import { ageImageBlocks } from "./attachment-store.js"; +import type { SummaryContext } from "./summarizer.js"; // --------------------------------------------------------------------------- // Task boundary decision @@ -191,10 +192,16 @@ export function buildContextEnvelope(envelope: ContextEnvelope): string { export interface CompactorConfig { keepRecentTurns: number; summaryMaxChars: number; - summarize?: (turns: ConversationTurn[]) => Promise; - // Max older turns to pull forward as anchors (file edits, task updates, - // errors) before the summary stub. Pulled from the end of the older set - // so the most-recent anchors survive. + summarize?: (turns: ConversationTurn[], ctx?: SummaryContext) => Promise; + /** + * Read at compaction time and passed to `summarize` so the summary can + * carry live workflow state (which workflow/step was active when the + * compacted turns were dropped). + */ + summaryContext?: () => SummaryContext | undefined; + // Max older turns to pull forward as anchors (file edits, task updates) + // before the summary stub. Selected from the end of the older set so the + // most-recent anchors survive; pair partners count against the cap too. maxAnchorTurns: number; } @@ -223,6 +230,18 @@ const ANCHOR_SCORE_THRESHOLD = 5; // Tool names whose results are path-keyed for re-read dedup during compaction. const READ_TOOLS = new Set(["read_file"]); +// Replayable query tools deduped by full-argument identity: a later identical +// grep/search_files/list_dir call reflects newer workspace state, so an older +// identical result is stale the same way an older read_file body is. +// run_shell is deliberately excluded — the same command is not idempotent +// (builds, tests, mutations), so an older run_shell result can be the only +// record of a genuinely distinct outcome. +const QUERY_TOOLS = new Set(["grep", "search_files", "list_dir"]); + +function isReplayableResultTool(name: string): boolean { + return READ_TOOLS.has(name) || QUERY_TOOLS.has(name); +} + // Call-id index for stub rendering (name + path). Dedup keys live on `readKey`. interface ToolCallInfo { name: string; @@ -274,6 +293,37 @@ function readIdentityFromArguments(raw: unknown): { path: string; readKey: strin return { path, readKey }; } +// Deterministic key for structurally equal arguments regardless of key order. +function stableStringify(value: unknown): string { + if (Array.isArray(value)) return `[${value.map(stableStringify).join(",")}]`; + if (value !== null && typeof value === "object") { + const rec = value as Record; + const entries = Object.keys(rec) + .sort() + .map((k) => `${JSON.stringify(k)}:${stableStringify(rec[k])}`); + return `{${entries.join(",")}}`; + } + const scalar = JSON.stringify(value); + return scalar === undefined ? "undefined" : scalar; +} + +/** + * Dedup identity for a query tool call: tool name + canonicalized arguments. + * Only byte-identical (modulo key order) calls share a key, so a grep for a + * different pattern or a list of a different directory never supersedes. + */ +function queryIdentityFromArguments(name: string, raw: unknown): string | undefined { + let args: unknown = raw ?? {}; + if (typeof args === "string") { + try { + args = JSON.parse(args) as unknown; + } catch { + return undefined; + } + } + return `${name}\0${stableStringify(args)}`; +} + // callId → tool name/path for readable stubs. Inverse of path-to-reads. function buildCallIndex(turns: readonly ConversationTurn[]): Map { const index = new Map(); @@ -286,6 +336,10 @@ function buildCallIndex(turns: readonly ConversationTurn[]): Map): Set { const superseded = new Set(); @@ -370,20 +426,133 @@ function buildPairIndex(turns: ConversationTurn[]): Map { return pairs; } -// Score a turn by its anchor importance. Turns that write files, update -// tasks, or contain errors are load-bearing regardless of age. -function anchorScore(turn: ConversationTurn): number { +// Errored results score BELOW the anchor threshold on purpose: a lone failure +// is context for the summary, not an anchor. Scoring errors at or above the +// threshold preserved every iteration of a failing-edit retry loop verbatim +// past the summary boundary, crowding the kept context with the loop while +// the substance was summarized away. Two distinct errors on one turn still +// clear the threshold. +const ERRORED_RESULT_SCORE = 3; + +// Whitespace-collapsed error-text prefix length compared when deciding two +// errored results are the same failure repeating. Long enough to separate +// distinct errors, short enough that trailing variable detail (line numbers, +// retry counters) does not defeat the collapse. +const ERROR_SIGNATURE_PREFIX_CHARS = 120; + +type ToolResultBlock = Extract; + +function erroredResultSignature( + block: ToolResultBlock, + callIndex: ReadonlyMap, +): string { + const info = callIndex.get(block.callId); + const name = info === undefined ? "" : info.name; + const text = block.content + .flatMap((c) => (c.type === "text" ? [c.text] : [])) + .join("") + .replace(/\s+/g, " ") + .slice(0, ERROR_SIGNATURE_PREFIX_CHARS); + return `${name}\0${text}`; +} + +/** + * Call ids of errored results whose (tool name, error-text prefix) signature + * recurs on a later turn in the same set. Every occurrence but the last is + * returned, collapsing a retry loop's repeats to one representative — the + * most recent failure, which is the state the agent must resume from. + */ +function repeatedErroredResultCallIds( + turns: readonly ConversationTurn[], + callIndex: ReadonlyMap, +): Set { + const lastSeen = new Map(); + const repeated = new Set(); + for (const turn of turns) { + for (const block of turn.content) { + if (block.type !== "tool_result" || block.isError !== true) continue; + const signature = erroredResultSignature(block, callIndex); + const previous = lastSeen.get(signature); + if (previous !== undefined) repeated.add(previous); + lastSeen.set(signature, block.callId); + } + } + return repeated; +} + +// Score a turn by its anchor importance. Turns that write files or update +// tasks are load-bearing regardless of age. Errored results whose failure +// signature repeats later contribute nothing — only the last occurrence of a +// recurring error counts (see repeatedErroredResultCallIds). +function anchorScore(turn: ConversationTurn, suppressedErrorCallIds: ReadonlySet): number { let score = 0; for (const block of turn.content) { if (block.type === "tool_call") { if (block.name === "edit_file" || block.name === "write_file") score += 10; else if (block.name === "manage_tasks") score += 7; } - if (block.type === "tool_result" && block.isError === true) score += 5; + if ( + block.type === "tool_result" && + block.isError === true && + !suppressedErrorCallIds.has(block.callId) + ) { + score += ERRORED_RESULT_SCORE; + } } return score; } +// Turn index → pair-partner turn indices, derived from the pair index, so +// closure walks touch each pair once instead of rescanning all pairs per step. +function buildPartnerIndex(pairs: ReadonlyMap): Map { + const partners = new Map(); + const link = (a: number, b: number): void => { + const list = partners.get(a); + if (list === undefined) partners.set(a, [b]); + else list.push(b); + }; + for (const { callIdx, resultIdx } of pairs.values()) { + if (callIdx === undefined || resultIdx === undefined || callIdx === resultIdx) continue; + link(callIdx, resultIdx); + link(resultIdx, callIdx); + } + return partners; +} + +/** + * Older-region turn indices a candidate anchor drags along: itself plus its + * tool_call/tool_result partners, transitively, minus turns already kept + * (recent window or previously anchored). Selecting anchors closure-at-a-time + * is what lets maxAnchorTurns bound the total pull: a pair is either taken + * whole or not at all, so no partner ever needs an over-budget rescue. + */ +function pairClosure( + start: number, + partnerIndex: ReadonlyMap, + keepFrom: number, + kept: ReadonlySet, +): Set { + const closure = new Set(); + const queue = [start]; + while (queue.length > 0) { + const idx = queue.pop(); + if (idx === undefined || idx >= keepFrom || kept.has(idx) || closure.has(idx)) continue; + closure.add(idx); + const partners = partnerIndex.get(idx); + if (partners !== undefined) queue.push(...partners); + } + return closure; +} + +function addPairClosure( + start: number, + partnerIndex: ReadonlyMap, + keepFrom: number, + kept: Set, +): void { + for (const idx of pairClosure(start, partnerIndex, keepFrom, kept)) kept.add(idx); +} + // Index of the first turn carrying the user's own words. This is the // initiating task; it must survive compaction so the agent never loses what // it was asked to do, even when it falls far outside the recent window. @@ -518,7 +687,7 @@ export function createPruningCompactor(config: Partial = {}): C return { name: "pruning-compactor", - version: "1.3.1", + version: "1.4.0", async apply( turns: ConversationTurn[], _ctx: StrategyContext, @@ -553,38 +722,55 @@ export function createPruningCompactor(config: Partial = {}): C const recentTurns = aged.turns.slice(keepFrom); const olderTurns = aged.turns.slice(0, keepFrom); - // Pull high-importance turns forward regardless of age. Take from the - // tail of the older set so the most recent anchors survive. - const scoredOlder = olderTurns.map((t, i) => ({ turn: t, index: i, score: anchorScore(t) })); - const anchorIndices = new Set( - scoredOlder - .filter(({ score }) => score >= ANCHOR_SCORE_THRESHOLD) - .slice(-cfg.maxAnchorTurns) - .map(({ index }) => index), - ); + const pairs = buildPairIndex(aged.turns); + const partnerIndex = buildPartnerIndex(pairs); - // Always keep the initiating task verbatim, outside the maxAnchorTurns - // cap. Losing the oldest user turn is how the agent forgets what it was - // asked to do; correctness outranks the size target here. - const initiatingIdx = firstUserTurnIndex(olderTurns); - if (initiatingIdx >= 0) anchorIndices.add(initiatingIdx); + // Repeated identical errors collapse to their last occurrence before + // scoring, so a failing retry loop contributes one representative + // instead of scoring every iteration. + const repeatedErrors = repeatedErroredResultCallIds(olderTurns, callIndex); + const scoredOlder = olderTurns.map((t, i) => ({ + index: i, + score: anchorScore(t, repeatedErrors), + })); // Keep tool_call/tool_result pairs together across the keep/summarize - // boundary. A turn that survives (anchored, or in the recent window) whose - // partner would be summarized leaves a dangling tool_call or an orphaned - // tool_result, which the inference layer rejects. Pull the older partner - // forward as an anchor so the surviving sequence stays well-formed. - // Pairing wins over maxAnchorTurns: correctness outranks the size target. - const pairs = buildPairIndex(aged.turns); - const isKept = (idx: number): boolean => idx >= keepFrom || anchorIndices.has(idx); + // boundary: a surviving turn whose partner is summarized leaves a + // dangling tool_call or an orphaned tool_result, which the inference + // layer rejects. Partners of recent-window turns are mandatory pulls + // and are counted against maxAnchorTurns first, so the cap bounds the + // total turns pulled forward past the summary. + const anchorIndices = new Set(); for (const { callIdx, resultIdx } of pairs.values()) { if (callIdx === undefined || resultIdx === undefined) continue; - if (isKept(callIdx) && !isKept(resultIdx) && resultIdx < keepFrom) - anchorIndices.add(resultIdx); - else if (isKept(resultIdx) && !isKept(callIdx) && callIdx < keepFrom) - anchorIndices.add(callIdx); + if (callIdx >= keepFrom && resultIdx < keepFrom) + addPairClosure(resultIdx, partnerIndex, keepFrom, anchorIndices); + else if (resultIdx >= keepFrom && callIdx < keepFrom) + addPairClosure(callIdx, partnerIndex, keepFrom, anchorIndices); + } + + // Pull high-importance turns forward regardless of age, most recent + // first so the freshest anchors survive. Each candidate is taken with + // its pair partners, whole closure or not at all, and only while the + // combined pull stays within maxAnchorTurns. + let anchorBudget = Math.max(0, cfg.maxAnchorTurns - anchorIndices.size); + for (let i = scoredOlder.length - 1; i >= 0; i--) { + const candidate = scoredOlder[i]; + if (candidate === undefined) continue; + if (candidate.score < ANCHOR_SCORE_THRESHOLD || anchorIndices.has(candidate.index)) + continue; + const closure = pairClosure(candidate.index, partnerIndex, keepFrom, anchorIndices); + if (closure.size > anchorBudget) continue; + for (const idx of closure) anchorIndices.add(idx); + anchorBudget -= closure.size; } + // Always keep the initiating task verbatim, outside the maxAnchorTurns + // cap. Losing the oldest user turn is how the agent forgets what it was + // asked to do; correctness outranks the size target here. + const initiatingIdx = firstUserTurnIndex(olderTurns); + if (initiatingIdx >= 0) addPairClosure(initiatingIdx, partnerIndex, keepFrom, anchorIndices); + // Ascending original order keeps the concatenated [anchors, recent] // sequence globally index-ordered, so every result still follows its call. const sortedAnchorIndices = [...anchorIndices].sort((a, b) => a - b); @@ -599,7 +785,7 @@ export function createPruningCompactor(config: Partial = {}): C const summary = cfg.summarize !== undefined - ? await cfg.summarize(summarizedTurns) + ? await cfg.summarize(summarizedTurns, cfg.summaryContext?.()) : buildTurnSummary(summarizedTurns, cfg.summaryMaxChars, anchorTurns.length); // A user-role turn survives every adapter unchanged. A system-role turn @@ -645,6 +831,7 @@ export function createPruningCompactor(config: Partial = {}): C summaryLength: summary.length, agedImageCount: aged.agedImageCount, supersededReadCount: supersededReads.size, + repeatedErrorCount: repeatedErrors.size, }, }, ...(aged.blobs.length > 0 ? { blobs: aged.blobs } : {}), diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index 2bcca516..60d05285 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -263,4 +263,26 @@ describe("createSessionPruningCompactor", () => { expect(typeof pruning.apply).toBe("function"); expect(typeof llm.apply).toBe("function"); }); + + test("forwards summaryContext to summarize in llm mode", async () => { + const ctx = { workflow: { name: "build", stepIndex: 1, total: 3 } }; + let captured: unknown; + const summarize = async (_turns: unknown, c?: unknown) => { + captured = c; + return "summary"; + }; + const llm = createSessionPruningCompactor({ + compactionMode: "llm", + summarize, + summaryContext: () => ctx, + }); + const now = Date.now(); + const turns = Array.from({ length: 8 }, (_, i) => ({ + role: i % 2 === 0 ? "user" : "assistant", + content: [{ type: "text", text: `t${i}` }], + timestamp: now, + })); + await llm.apply(turns as never, { state: {} as never, trigger: "test" }); + expect(captured).toBe(ctx); + }); }); diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index feace9ef..7220c44f 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -41,6 +41,7 @@ import type { Approval, GrantScope } from "../permission/types.js"; import type { ReasoningEffort } from "../provider/reasoning-effort.js"; import type { SubAgentProvider } from "../subagent/index.js"; import { COMPACTOR_KEEP_RECENT_TURNS, createPruningCompactor } from "./compactor.js"; +import type { SummaryContext } from "./summarizer.js"; import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; // --------------------------------------------------------------------------- @@ -264,7 +265,8 @@ const SESSION_COMPACTOR_SUMMARY_MAX_CHARS = 2500; export interface SessionPruningCompactorArgs { compactionMode: "llm" | "pruning"; - summarize: (turns: ConversationTurn[]) => Promise; + summarize: (turns: ConversationTurn[], ctx?: SummaryContext) => Promise; + summaryContext?: () => SummaryContext | undefined; telemetry?: Telemetry; } @@ -274,6 +276,7 @@ export function createSessionPruningCompactor(args: SessionPruningCompactorArgs) keepRecentTurns: COMPACTOR_KEEP_RECENT_TURNS, summaryMaxChars: SESSION_COMPACTOR_SUMMARY_MAX_CHARS, ...(args.compactionMode !== "pruning" ? { summarize: args.summarize } : {}), + ...(args.summaryContext ? { summaryContext: args.summaryContext } : {}), }); const telemetry = args.telemetry ?? NOOP_TELEMETRY; return { diff --git a/src/session/summarizer.ts b/src/session/summarizer.ts index c1449b6e..3a4f803f 100644 --- a/src/session/summarizer.ts +++ b/src/session/summarizer.ts @@ -9,9 +9,14 @@ import { runInference, type Dependencies } from "@intx/inference"; import { createDefaultDependencies } from "@intx/inference/providers"; +import { getLogger } from "@intx/log"; import type { ConversationTurn, InferenceSource } from "@intx/types/runtime"; +import { LOG_NAMESPACE_ROOT } from "../branding.js"; +import { detectRepetition } from "../subagent/repetition.js"; import { buildTurnSummary } from "./compactor.js"; +const logger = getLogger([LOG_NAMESPACE_ROOT, "session", "summarizer"]); + // What the agent was doing when compaction fired. Lets the summary preserve // the workflow contract ("we are at step 3/7 of /build") rather than dropping // it into the compacted region. @@ -70,7 +75,13 @@ export function condenseTurns(turns: ConversationTurn[]): string { if (turn.role === "user") { userMessages.push(block.text.slice(0, 400)); } else if (turn.role === "assistant" && block.text.length > 0) { - assistantSnippets.push(block.text.slice(0, 300)); + // Compaction often fires mid-degeneration, when the tail of the + // history is the model looping one phrase. Seeding the summary from + // those turns hands the looped text to the summarizer verbatim, so + // repetition-flagged turns are dropped from the excerpt entirely. + if (detectRepetition(block.text) === null) { + assistantSnippets.push(block.text.slice(0, 300)); + } } } if (block.type === "tool_call") { @@ -190,7 +201,10 @@ export function createModelSummarizer( const maxChars = options.maxChars ?? 4000; return async (turns, ctx) => { - const fallback = (): string => buildTurnSummary(turns, maxChars); + // The marker tells the model (and anyone reading a transcript) that the + // compacted region is a lossy stats stub, not a real handoff summary. + const fallback = (reason: string): string => + `[Model summary unavailable (${reason}); deterministic fallback]\n${buildTurnSummary(turns, maxChars)}`; try { const promptTurns: ConversationTurn[] = [ { @@ -206,10 +220,16 @@ export function createModelSummarizer( ]; const signal = options.getSignal?.() ?? new AbortController().signal; const text = await complete(promptTurns, options.getSource(), signal); - if (text.length === 0) return fallback(); + if (text.length === 0) { + logger.warn("compaction summary call returned empty text; using deterministic fallback"); + return fallback("empty model output"); + } return text.length > maxChars ? text.slice(0, maxChars) : text; - } catch { - return fallback(); + } catch (error) { + logger.warn("compaction summary call failed; using deterministic fallback: {error}", { + error: error instanceof Error ? error.message : String(error), + }); + return fallback("summary call failed"); } }; } diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 125d6569..c0d8f633 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -238,7 +238,7 @@ import { skillDirsFromEnabledPlugins, } from "../session/runtime-assembly.js"; import { createAttachmentRehydrateTransform } from "../session/attachment-store.js"; -import { createModelSummarizer } from "../session/summarizer.js"; +import { createModelSummarizer, type SummaryContext } from "../session/summarizer.js"; import { COMMAND_NAME, ID_PREFIX, LOG_NAMESPACE_ROOT } from "../branding.js"; import { deliverAgentMessage } from "./deliver-agent-message.js"; @@ -1457,28 +1457,23 @@ export async function runTUI(initialConfig: Config): Promise { // Compaction summarizer: produces a structured, workflow-aware handoff via a // one-shot call on the live model, falling back to the deterministic summary - // on any failure. The workflow context is read at call time so a compaction - // mid-/build or mid-/plan preserves which step we are on. + // on any failure. Workflow state is read at compaction time so a pass + // mid-/build or mid-/plan still names the active step. const compactionSummarize = createModelSummarizer({ getSource: () => liveSource, deps: inferenceDeps, }); - const summarizeForCompaction = ( - turns: Parameters[0], - ): Promise => { + const summaryContext = (): SummaryContext | undefined => { const status = workflowController.status(); - return compactionSummarize(turns, { - ...(status.active - ? { - workflow: { - ...(status.name !== undefined ? { name: status.name } : {}), - stepLabel: status.label, - stepIndex: status.stepIndex, - total: status.total, - }, - } - : {}), - }); + if (!status.active) return undefined; + return { + workflow: { + ...(status.name !== undefined ? { name: status.name } : {}), + stepLabel: status.label, + stepIndex: status.stepIndex, + total: status.total, + }, + }; }; // Mutable reference so the compaction summarize callback reads the live mode @@ -1510,7 +1505,8 @@ export async function runTUI(initialConfig: Config): Promise { compactors: { "pruning-compactor": createSessionPruningCompactor({ compactionMode: liveCompactionMode, - summarize: summarizeForCompaction, + summarize: compactionSummarize, + summaryContext, telemetry: liveTelemetry, }), }, diff --git a/tests/unit/compactor-pairing.test.ts b/tests/unit/compactor-pairing.test.ts index c6cc1ae3..f4bb1b73 100644 --- a/tests/unit/compactor-pairing.test.ts +++ b/tests/unit/compactor-pairing.test.ts @@ -364,3 +364,131 @@ describe("pruning compactor stubs superseded file reads (CL-4374)", () => { expect(older).toMatch(/omitted|chars/); }); }); + +// grep/search_files/list_dir are replayable the same way read_file is: an +// identical later call reflects newer workspace state, so an older identical +// result is stubbed the same way an older full-file read is (CL-6906). +function assistantQuery(id: string, name: string, args: Record): ConversationTurn { + return { + role: "assistant", + content: [{ type: "tool_call", id, name, arguments: args }], + timestamp: 1, + }; +} + +describe("pruning compactor extends superseded-result stubbing to query tools (CL-6906)", () => { + test("stubs an older successful grep call repeated with byte-identical arguments", async () => { + const oldBody = "OLD_MATCHES_" + "a".repeat(200); + const newBody = "NEW_MATCHES_" + "b".repeat(200); + const args = { pattern: "TODO", path: "src" }; + const turns: ConversationTurn[] = [ + userText("start"), + userText("a"), + userText("b"), + userText("c"), + assistantQuery("g1", "grep", args), + userReadResult("g1", oldBody), + assistantQuery("g2", "grep", args), + userReadResult("g2", newBody), + userText("d"), + userText("e"), + ]; + const compactor = createPruningCompactor({ keepRecentTurns: 6, maxAnchorTurns: 2 }); + const { output } = await compactor.apply(turns, {} as never); + const older = resultText(output, "g1"); + expect(resultText(output, "g2")).toBe(newBody); + expect(older).toBeDefined(); + expect(older).not.toBe(oldBody); + expect(older).toMatch(/omitted|chars/); + }); + + test("stubs an older successful search_files call with argument key order irrelevant", async () => { + const oldBody = "OLD_SEARCH_" + "a".repeat(200); + const newBody = "NEW_SEARCH_" + "b".repeat(200); + const turns: ConversationTurn[] = [ + userText("start"), + userText("a"), + userText("b"), + userText("c"), + assistantQuery("s1", "search_files", { query: "widget", limit: 20 }), + userReadResult("s1", oldBody), + // Same arguments, different key order — must still be treated as identical. + assistantQuery("s2", "search_files", { limit: 20, query: "widget" }), + userReadResult("s2", newBody), + userText("d"), + userText("e"), + ]; + const compactor = createPruningCompactor({ keepRecentTurns: 6, maxAnchorTurns: 2 }); + const { output } = await compactor.apply(turns, {} as never); + expect(resultText(output, "s2")).toBe(newBody); + expect(resultText(output, "s1")).not.toBe(oldBody); + }); + + test("stubs an older successful list_dir call repeated on the same path", async () => { + const oldBody = "OLD_LISTING_" + "a".repeat(200); + const newBody = "NEW_LISTING_" + "b".repeat(200); + const args = { path: "src/components" }; + const turns: ConversationTurn[] = [ + userText("start"), + userText("a"), + userText("b"), + userText("c"), + assistantQuery("l1", "list_dir", args), + userReadResult("l1", oldBody), + assistantQuery("l2", "list_dir", args), + userReadResult("l2", newBody), + userText("d"), + userText("e"), + ]; + const compactor = createPruningCompactor({ keepRecentTurns: 6, maxAnchorTurns: 2 }); + const { output } = await compactor.apply(turns, {} as never); + expect(resultText(output, "l2")).toBe(newBody); + expect(resultText(output, "l1")).not.toBe(oldBody); + }); + + test("does not supersede a grep call with different arguments", async () => { + const body1 = "MATCHES_TODO_" + "a".repeat(200); + const body2 = "MATCHES_FIXME_" + "b".repeat(200); + const turns: ConversationTurn[] = [ + userText("start"), + userText("a"), + userText("b"), + userText("c"), + assistantQuery("g1", "grep", { pattern: "TODO", path: "src" }), + userReadResult("g1", body1), + assistantQuery("g2", "grep", { pattern: "FIXME", path: "src" }), + userReadResult("g2", body2), + userText("d"), + userText("e"), + ]; + const compactor = createPruningCompactor({ keepRecentTurns: 6, maxAnchorTurns: 2 }); + const { output } = await compactor.apply(turns, {} as never); + expect(resultText(output, "g1")).toBe(body1); + expect(resultText(output, "g2")).toBe(body2); + }); + + test("never supersedes run_shell results, even with byte-identical commands", async () => { + // The same shell command is not idempotent (builds, tests, mutations can + // each produce a genuinely different outcome), so run_shell is excluded + // from replayable-result stubbing entirely. + const oldBody = "OLD_SHELL_OUTPUT_" + "a".repeat(200); + const newBody = "NEW_SHELL_OUTPUT_" + "b".repeat(200); + const args = { command: "npm test" }; + const turns: ConversationTurn[] = [ + userText("start"), + userText("a"), + userText("b"), + userText("c"), + assistantQuery("sh1", "run_shell", args), + userReadResult("sh1", oldBody), + assistantQuery("sh2", "run_shell", args), + userReadResult("sh2", newBody), + userText("d"), + userText("e"), + ]; + const compactor = createPruningCompactor({ keepRecentTurns: 6, maxAnchorTurns: 2 }); + const { output } = await compactor.apply(turns, {} as never); + expect(resultText(output, "sh1")).toBe(oldBody); + expect(resultText(output, "sh2")).toBe(newBody); + }); +}); diff --git a/tests/unit/summarizer.test.ts b/tests/unit/summarizer.test.ts index 39f80d30..9e9b18f2 100644 --- a/tests/unit/summarizer.test.ts +++ b/tests/unit/summarizer.test.ts @@ -50,6 +50,25 @@ test("condenseTurns extracts files, tools, and links", () => { expect(out).toContain("https://example.com/ticket/42"); }); +test("condenseTurns drops a degenerate repeated assistant tail from the excerpt (CL-6906)", () => { + // The live detector needs 16 consecutive repeats of an 8+ char window + // (DEFAULT_REPETITION_CONFIG). Ten copies of this phrase was enough + // before that raise; twenty still trips it after. + const loopPhrase = "we need to check whether the cache key already accounts for locale. "; + const loopText = loopPhrase.repeat(20); + const healthyNote = "Looked at src/auth.ts and found the missing null check."; + const degenerateTurns: ConversationTurn[] = [ + { role: "user", content: [{ type: "text", text: "please continue" }], timestamp: 1 }, + { role: "assistant", content: [{ type: "text", text: healthyNote }], timestamp: 2 }, + { role: "assistant", content: [{ type: "text", text: loopText }], timestamp: 3 }, + ]; + const out = condenseTurns(degenerateTurns); + // The looping tail is dropped entirely rather than handed to the summarizer. + expect(out).not.toContain("cache key already accounts for locale"); + // A healthy assistant note elsewhere in the same drop still survives. + expect(out).toContain(healthyNote); +}); + test("buildSummaryPrompt injects active workflow context", () => { const prompt = buildSummaryPrompt(turns(), { workflow: { name: "build", stepLabel: "Implement", stepIndex: 2, total: 7 }, @@ -95,3 +114,34 @@ test("model summarizer falls back when the model returns empty text", async () = const result = await summarize(turns()); expect(result).toContain("Tools called"); }); + +test("model summarizer marks a failure fallback as distinguishable from a real summary (CL-6906)", async () => { + const summarize = createModelSummarizer({ + getSource: () => source, + complete: async () => { + throw new Error("model unreachable"); + }, + }); + const result = await summarize(turns()); + expect(result).toContain("[Model summary unavailable"); + expect(result).toContain("summary call failed"); +}); + +test("model summarizer marks an empty-output fallback as distinguishable from a real summary (CL-6906)", async () => { + const summarize = createModelSummarizer({ + getSource: () => source, + complete: async () => "", + }); + const result = await summarize(turns()); + expect(result).toContain("[Model summary unavailable"); + expect(result).toContain("empty model output"); +}); + +test("model summarizer does not mark a real summary with the fallback marker", async () => { + const summarize = createModelSummarizer({ + getSource: () => source, + complete: async () => "## What Happened\n- read src/auth.ts", + }); + const result = await summarize(turns()); + expect(result).not.toContain("[Model summary unavailable"); +});