diff --git a/src/context-compactor.test.ts b/src/context-compactor.test.ts index fe14afe92..e0790f944 100644 --- a/src/context-compactor.test.ts +++ b/src/context-compactor.test.ts @@ -848,11 +848,9 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => { baseURL: "http://localhost:1", credentialId: "test", }; - const notices: string[] = []; const summarize = createModelSummarizer({ getSource: () => source, complete: async () => "", - onFailure: (text) => notices.push(text), }); const result = await smallCompactor({ summaryMaxChars: 500, @@ -862,9 +860,6 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => { expect(result.record.decisions.summarizeFailureKind).toBe("empty"); expect(result.record.reason).toContain("statistics-only stub: empty"); expect(compactedTurns(result.output)).toHaveLength(1); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("statistics-only stub"); - expect(notices[0]).toContain("empty"); }); test("an aborted summarizer keeps prior context instead of stubbing", async () => { diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 76a7a3550..ec524d443 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -943,6 +943,10 @@ export async function runExec(config: Config): Promise { currentStorage?.readBlob.bind(currentStorage), ), telemetry: liveTelemetry, + // Operator-visible once a statistics-only stub actually replaces turns. + onFailure: (text) => { + stderr.write(`${text}\n`); + }, onFolded: () => { // Fold restarts the cached prefix — drop idle execute-promoted // schemas rather than carrying them forever, and persist so a diff --git a/src/session/compaction-lifecycle.test.ts b/src/session/compaction-lifecycle.test.ts index 61ad8c684..18b981018 100644 --- a/src/session/compaction-lifecycle.test.ts +++ b/src/session/compaction-lifecycle.test.ts @@ -246,9 +246,6 @@ describe("createCompactionLifecycle", () => { }) as never, getSignal: () => lifecycle.getSignal(), telemetry, - onFailure: (text) => { - notices.push(text); - }, complete: (_promptTurns, _source, signal) => new Promise((_resolve, reject) => { signal.addEventListener( @@ -297,8 +294,7 @@ describe("createCompactionLifecycle", () => { expect(telemetryEvents).toEqual([]); }); - test("a genuine summarizer failure still notifies and emits telemetry", async () => { - const notices: string[] = []; + test("a genuine summarizer failure still emits telemetry", async () => { const captured: { event: string; properties?: Record | undefined; @@ -322,16 +318,11 @@ describe("createCompactionLifecycle", () => { credentialId: "test", }) as never, telemetry, - onFailure: (text) => { - notices.push(text); - }, complete: async () => { throw new Error("model unreachable"); }, }); await expect(summarize(turns(5))).rejects.toThrow("model unreachable"); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("Compaction summary failed"); const failures = captured.filter((e) => e.event === "summarizer_failure"); expect(failures).toHaveLength(1); expect(failures[0]?.properties?.["error_kind"]).toBe("failed"); diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index 5906396b9..459fe096d 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -567,9 +567,10 @@ describe("createSessionPruningCompactor", () => { }); describe("createSessionPruningCompactor stub fallback", () => { - test("a summarizer stub fallback folds without success telemetry or onFolded", async () => { + test("a summarizer stub fallback folds without success telemetry and still runs onFolded", async () => { const captured: { event: string }[] = []; - const folds: { turnsBefore: number; turnsAfter: number }[] = []; + const folds: { turnsBefore: number; turnsAfter: number; stub: boolean }[] = + []; const telemetry: Telemetry = { enabled: true, installationId: "test", @@ -593,12 +594,12 @@ describe("createSessionPruningCompactor stub fallback", () => { complete: async () => { throw new Error("model unreachable"); }, - onFailure: (text) => notices.push(text), }); const compactor = createSessionPruningCompactor({ summarize, telemetry, onFolded: (info) => folds.push(info), + onFailure: (text) => notices.push(text), compactionShape: { tailBudgetTokens: 1 }, }); const now = Date.now(); @@ -613,11 +614,123 @@ describe("createSessionPruningCompactor stub fallback", () => { }); expect(result.record.decisions.summarizeFailed).toBe(1); expect(result.record.reason).toContain("statistics-only stub"); - expect(folds).toEqual([]); + expect(folds).toEqual([ + { turnsBefore: 8, turnsAfter: result.output.length, stub: true }, + ]); expect(captured).toEqual([]); expect(notices).toHaveLength(1); expect(notices[0]).toContain("statistics-only stub"); expect(notices[0]).toContain("failed"); + expect(notices[0]).toContain("model unreachable"); + }); + + test("verify abort after a failed summary keeps prior context and fires no stub notice", async () => { + const notices: string[] = []; + const folds: { stub: boolean }[] = []; + const summarize = createModelSummarizer({ + getSource: () => + ({ + id: "test", + provider: "openai", + model: "test-model", + baseURL: "http://localhost:1", + credentialId: "test", + }) as never, + complete: async () => { + throw new Error("model unreachable"); + }, + }); + const now = Date.now(); + const turns = [ + { + role: "user" as const, + content: [ + { + type: "text" as const, + text: "Migrate the auth module to opaque tokens", + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { + type: "tool_call" as const, + id: "c1", + name: "run_shell", + arguments: { command: "bun test auth" }, + }, + ], + timestamp: now, + }, + { + role: "user" as const, + content: [ + { + type: "tool_result" as const, + callId: "c1", + isError: true, + content: [ + { type: "text" as const, text: "token refresh assertion failed" }, + ], + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { type: "text" as const, text: "Working through the failure" }, + ], + timestamp: now, + }, + { + role: "user" as const, + content: [ + { + type: "text" as const, + text: "Confirm there are no errors remaining in auth", + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { type: "text" as const, text: "Continuing the auth work now" }, + ], + timestamp: now, + }, + { + role: "user" as const, + content: [ + { + type: "text" as const, + text: "There are no errors remaining in the suite", + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { type: "text" as const, text: "I will keep going from here" }, + ], + timestamp: now, + }, + ]; + const result = await createSessionPruningCompactor({ + summarize, + onFolded: (info) => folds.push(info), + onFailure: (text) => notices.push(text), + compactionShape: { tailBudgetTokens: 1 }, + }).apply(turns as never, { state: {} as never, trigger: "test" }); + expect(result.output).toBe(turns); + expect(result.record.reason).toBe("verify failed — keeping prior context"); + expect(result.record.decisions.summarizedTurnCount).toBeUndefined(); + expect(folds).toEqual([]); + expect(notices).toEqual([]); }); }); diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index 1cacc40aa..0269ad8f1 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -58,7 +58,11 @@ import { createPruningCompactor, type CompactionShape, } from "./compactor.js"; -import type { SummaryContext } from "./summarizer.js"; +import { + classifySummarizerFailure, + summarizerStubFallbackNotice, + type SummaryContext, +} from "./summarizer.js"; import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; import { COMPACTION_ABORTED_REASON } from "./compaction-lifecycle.js"; @@ -418,7 +422,17 @@ export interface SessionPruningCompactorArgs { readPriorHandoff?: () => Promise; telemetry?: Telemetry; /** Fires only when turns were actually folded away — not on no-ops. */ - onFolded?: (info: { turnsBefore: number; turnsAfter: number }) => void; + onFolded?: (info: { + turnsBefore: number; + turnsAfter: number; + /** True when the fold used a statistics-only stub, not an LLM summary. */ + stub: boolean; + }) => void; + /** + * Operator-visible notice for a statistics-only stub that actually replaced + * turns. Verify abort (keeping prior context) does not fire this. + */ + onFailure?: (text: string) => void; /** * True when the lifecycle has discarded (or will discard) the in-flight * compact — e.g. bound to the session compaction lifecycle's signal. A @@ -434,10 +448,39 @@ export interface SessionPruningCompactorArgs { compactionShape?: Partial; } +function stubFallbackNoticeFromError(error: unknown): string | undefined { + const err = error instanceof Error ? error : new Error(String(error)); + const kind = classifySummarizerFailure(err); + if (kind === "aborted") return undefined; + return summarizerStubFallbackNotice(kind, err); +} + +function stubFallbackNoticeFromRecord( + pending: string | undefined, + kind: unknown, +): string { + if (pending !== undefined) return pending; + const label = typeof kind === "string" && kind.length > 0 ? kind : "failed"; + return `Compaction summary failed — using a statistics-only stub (${label})`; +} + /** Shared pruning-compactor defaults for the main session agent. */ export function createSessionPruningCompactor( args: SessionPruningCompactorArgs, ): Compactor { + let pendingStubNotice: string | undefined; + const innerSummarize = args.summarize; + const summarize = + innerSummarize === undefined + ? undefined + : async (turns: ConversationTurn[], ctx?: SummaryContext) => { + try { + return await innerSummarize(turns, ctx); + } catch (error) { + pendingStubNotice = stubFallbackNoticeFromError(error); + throw error; + } + }; const compactor = createPruningCompactor({ summaryMaxChars: SESSION_COMPACTOR_SUMMARY_MAX_CHARS, // CL-9489 budgeted-tail shape: explicit defaults (same object the record @@ -447,7 +490,7 @@ export function createSessionPruningCompactor( ...DEFAULT_TAIL_COMPACTION_SHAPE, ...args.compactionShape, }, - ...(args.summarize !== undefined ? { summarize: args.summarize } : {}), + ...(summarize !== undefined ? { summarize } : {}), ...(args.summaryContext ? { summaryContext: args.summaryContext } : {}), ...(args.readPriorHandoff !== undefined ? { readPriorHandoff: args.readPriorHandoff } @@ -457,6 +500,7 @@ export function createSessionPruningCompactor( return { ...compactor, async apply(turns, ctx) { + pendingStubNotice = undefined; const turnsBefore = turns.length; const startedAt = Date.now(); const result = await compactor.apply(turns, ctx); @@ -473,21 +517,33 @@ export function createSessionPruningCompactor( // summarizedTurnCount is only set on the branch that actually folded // turns away. The other branch is a no-op (or image aging alone), and // reporting it as compaction would drag the duration and turn-count - // averages toward the runs where nothing happened. A statistics-only - // stub fold is not a successful LLM reduction: the operator notice - // owns that path, and emitting the success event would relabel it. - if ( - result.record.decisions.summarizedTurnCount !== undefined && - result.record.decisions.summarizeFailed !== 1 - ) { - telemetry.capture("compaction", { - mode: "llm", - duration_ms: Date.now() - startedAt, - turns_before: turnsBefore, - turns_after: result.output.length, - }); - args.onFolded?.({ turnsBefore, turnsAfter: result.output.length }); + // averages toward the runs where nothing happened. + if (result.record.decisions.summarizedTurnCount === undefined) { + return result; + } + const stub = result.record.decisions.summarizeFailed === 1; + // Stub folds still break the cached prefix, so prune still runs. + // Success telemetry and the TUI fold flash must not relabel a stub. + args.onFolded?.({ + turnsBefore, + turnsAfter: result.output.length, + stub, + }); + if (stub) { + args.onFailure?.( + stubFallbackNoticeFromRecord( + pendingStubNotice, + result.record.decisions.summarizeFailureKind, + ), + ); + return result; } + telemetry.capture("compaction", { + mode: "llm", + duration_ms: Date.now() - startedAt, + turns_before: turnsBefore, + turns_after: result.output.length, + }); return result; }, }; diff --git a/src/session/summarizer.test.ts b/src/session/summarizer.test.ts index 704f5027a..f8d40678e 100644 --- a/src/session/summarizer.test.ts +++ b/src/session/summarizer.test.ts @@ -103,16 +103,11 @@ test("model summarizer throws on failure instead of substituting a stats stub", }); test("model summarizer throws when the model returns empty text", async () => { - const notices: string[] = []; const summarize = createModelSummarizer({ getSource: () => source, complete: async () => "", - onFailure: (text) => notices.push(text), }); await expect(summarize(turns())).rejects.toThrow("empty text"); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("statistics-only stub"); - expect(notices[0]).toContain("empty"); }); test("model summarizer feeds archive payloads into the prompt instead of clipped turns", async () => { @@ -250,13 +245,11 @@ test("a 401 retries once after a credential re-read", async () => { test("a second 401 fails: retry budget is spent once", async () => { let calls = 0; let refreshes = 0; - const notices: string[] = []; const summarize = createModelSummarizer({ getSource: () => source, refreshAuth: async () => { refreshes++; }, - onFailure: (text) => notices.push(text), complete: async () => { calls++; throw new Error("HTTP 401 Unauthorized"); @@ -265,10 +258,6 @@ test("a second 401 fails: retry budget is spent once", async () => { await expect(summarize(turns())).rejects.toThrow("401"); expect(calls).toBe(2); expect(refreshes).toBe(1); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("statistics-only stub"); - expect(notices[0]).toContain("auth"); - expect(notices[0]).toContain("401"); }); test("a 401 without a refresh hook is not retried", async () => { @@ -343,14 +332,12 @@ test("a timeout is never retried", async () => { expect(calls).toBe(1); }); -test("final failure throws, notices once, and reports telemetry", async () => { +test("final failure throws and reports telemetry", async () => { const { telemetry, events } = stubTelemetry(); - const notices: string[] = []; let calls = 0; const summarize = createModelSummarizer({ getSource: () => source, telemetry, - onFailure: (text) => notices.push(text), complete: async () => { calls++; throw inferenceFailure({ @@ -362,10 +349,6 @@ test("final failure throws, notices once, and reports telemetry", async () => { }); await expect(summarize(turns())).rejects.toThrow("500"); expect(calls).toBe(2); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("statistics-only stub"); - expect(notices[0]).toContain("provider"); - expect(notices[0]).toContain("500"); expect(events).toHaveLength(1); expect(events[0]?.event).toBe("summarizer_failure"); expect(events[0]?.properties?.error_kind).toBe("provider"); diff --git a/src/session/summarizer.ts b/src/session/summarizer.ts index cfdf3003c..8fc0208eb 100644 --- a/src/session/summarizer.ts +++ b/src/session/summarizer.ts @@ -5,8 +5,9 @@ // Tools called: ...") loses everything that matters for resuming work, so this // module produces a structured, workflow-aware narrative via a one-shot // inference call against the session's own model. Empty output or a failed -// call throws so the compact cycle can substitute a statistics-only stub and -// tell the operator, instead of logging the fold as a successful reduction. +// call throws so the compact cycle can substitute a statistics-only stub. +// The operator notice for that fallback is owned by the session pruning +// wrapper, which fires it only after the fold actually commits. import { type } from "arktype"; import { runInference, type Dependencies } from "@intx/inference"; @@ -420,9 +421,11 @@ export function classifySummarizerFailure( return "failed"; } -// One-line operator notice for a final failure. The reason named is the -// provider's own first line when short enough to be useful, else the class. -function failureNotice( +// One-line operator notice for a stub fold that actually committed. The +// reason named is the provider's own first line when short enough to be +// useful, else the class. Callers must not fire this until the fold lands: +// verify abort keeps prior context, so claiming a stub was used would lie. +export function summarizerStubFallbackNotice( failureClass: SummarizerFailureClass, error: Error, ): string { @@ -461,8 +464,6 @@ export interface ModelSummarizerOptions { * mean this process holds a token another already rotated. */ refreshAuth?: (() => Promise) | undefined; - /** Fires once per failed `summarize` call, after the retry budget is spent. */ - onFailure?: ((text: string) => void) | undefined; telemetry?: Telemetry | undefined; /** Primary sessions pass the evidence archive so the prompt is not a clipped stub. */ getArchive?: () => SummaryExcerptArchive | undefined; @@ -472,7 +473,9 @@ export interface ModelSummarizerOptions { * Build a `summarize(turns, ctx)` function suitable for `CompactorConfig`. * Produces a structured, workflow-aware summary via the model. Empty output * or a failed call throws so the compact cycle can substitute a - * statistics-only stub and surface that fallback to the operator. + * statistics-only stub. The operator-visible fallback notice is owned by + * the session pruning wrapper, which fires it only after that stub fold + * actually commits. */ export function createModelSummarizer( options: ModelSummarizerOptions, @@ -555,11 +558,9 @@ export function createModelSummarizer( // A lifecycle abort (interrupt/rotation mid-compact) is operator // intent, not a summarizer failure: the wrapCompactor race already // returns its no-op fold and the lifecycle emits its own - // "interrupted" notice, so a second failure-framed notice plus a - // summarizer_failure telemetry event would be noise — and the - // "failed" framing actively misleads. Stay silent here and just - // rethrow so the race resolves as an abort. Aborts observed while - // this attempt's signal is live-but-unaborted keep the notice. + // "interrupted" notice, so a summarizer_failure telemetry event + // would be noise — and a "failed" framing actively misleads. Stay + // silent here and just rethrow so the race resolves as an abort. if (failureClass === "aborted" && signal.aborted) throw err; const source = options.getSource(); telemetry.capture("summarizer_failure", { @@ -568,7 +569,6 @@ export function createModelSummarizer( error_kind: failureClass, duration_ms: Date.now() - startedAt, }); - options.onFailure?.(failureNotice(failureClass, err)); throw err; } } diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 0d770ee64..c3d1b34e4 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -602,7 +602,8 @@ export async function assembleTUISession( resolveLiveSessionSources(state.config, state.sessionId); // Compaction summarizer: structured handoff via the live model. Failure - // substitutes a statistics-only stub and tells the operator. Workflow state + // substitutes a statistics-only stub; the pruning wrapper tells the operator + // only after that fold commits. Workflow state // is read at compaction time so a pass mid-/build or mid-/plan still names // the active step. The archive, when mounted, supplies the unclipped excerpt. // CL-8220: abort-aware compaction lifecycle. The summary call is the only @@ -635,7 +636,6 @@ export async function assembleTUISession( if (state.currentAgent !== undefined) setAgentSourceUnlessClosed(state.currentAgent, fresh); }, - onFailure: (text) => state.systemNotice?.(text), }); const summaryContext = (): SummaryContext | undefined => { const status = workflowHost.status(); @@ -711,6 +711,8 @@ export async function assembleTUISession( // still completes underneath must not report telemetry or side // effects for work that never landed. isAborted: () => compactionLifecycle.getSignal().aborted, + // Stub notice waits until the fold commits (verify abort stays silent). + onFailure: (text) => state.systemNotice?.(text), // Main-session folds only — exec runner and subagents stay silent. onFolded: (info) => { // Fold restarts the cached prefix — drop idle execute-promoted @@ -727,7 +729,7 @@ export async function assembleTUISession( void state.persistRunSnapshot?.("running"); }, }); - emitter.emit("compaction", info); + if (!info.stub) emitter.emit("compaction", info); }, }), ), diff --git a/src/tui/runtime-channels.test.ts b/src/tui/runtime-channels.test.ts index 73413b441..d11350fbb 100644 --- a/src/tui/runtime-channels.test.ts +++ b/src/tui/runtime-channels.test.ts @@ -275,7 +275,7 @@ describe("compaction channel", () => { emitter.emit("compaction", { turnsBefore: 42, turnsAfter: 8 }); const painted = await frame(); // the fold reports both turn counts on one line - expect(painted).toMatch(/compact/i); + expect(painted).toMatch(/context compacted/i); expect(painted).toMatch(/42[^\n]*8/); expect(host.shell.streamLog).toEqual([]); } finally { @@ -287,7 +287,7 @@ describe("compaction channel", () => { const { host, emitter, frame, cleanup } = await mountHeadless(); try { emitter.emit("compaction", { turnsBefore: 42 }); - expect(await frame()).not.toMatch(/compact/i); + expect(await frame()).not.toMatch(/context compacted/i); expect(host.shell.streamLog).toEqual([]); } finally { cleanup();