From d817e26e620728f4e2ca8d27befdff07a30f881f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 29 Sep 2026 16:32:02 -0700 Subject: [PATCH] fix(session): surface compaction summarizer stub fallback A failed or empty summary used to keep prior context while the compact cycle still looked like a successful reduction. The fold now substitutes the statistics-only stub, names the failure kind to the operator, and skips the success compaction event. --- src/context-compactor.test.ts | 66 +++++++++++++++++++++--- src/session/compactor.ts | 76 +++++++++++++++++++--------- src/session/runtime-assembly.test.ts | 56 ++++++++++++++++++++ src/session/runtime-assembly.ts | 9 +++- src/session/summarizer.test.ts | 9 ++++ src/session/summarizer.ts | 24 +++++---- src/telemetry/product-events.test.ts | 30 +++++++++++ src/tui/runner/session.ts | 2 +- 8 files changed, 228 insertions(+), 44 deletions(-) diff --git a/src/context-compactor.test.ts b/src/context-compactor.test.ts index 9fa4075da..efc0855bd 100644 --- a/src/context-compactor.test.ts +++ b/src/context-compactor.test.ts @@ -801,7 +801,7 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => { expect(result.record.reason).toBe("no compaction needed"); }); - test("failing summarizer keeps prior context; a later success writes one handoff", async () => { + test("failing summarizer folds with a statistics-only stub; a later success writes one handoff", async () => { const source: InferenceSource = { id: "test", provider: "openai", @@ -825,10 +825,14 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => { }); const turns = grow([], 16, "fail"); const result1 = await compactor.apply(turns, mockStrategyCtx); - expect(result1.output).toBe(turns); - expect(result1.record.reason).toBe("summarize failed"); - expect(firstText(defined(result1.output[0]))).not.toContain( - COMPACTED_PREFIX, + expect(result1.output).not.toBe(turns); + expect(result1.record.reason).toContain("statistics-only stub"); + expect(result1.record.decisions.summarizeFailed).toBe(1); + expect(result1.record.decisions.summarizeFailureKind).toBe("failed"); + expect(compactedTurns(result1.output)).toHaveLength(1); + const stubHandoff = defined(defined(result1.blobs)[0]); + expect(new TextDecoder().decode(stubHandoff.bytes)).toContain( + "Turns compacted:", ); const result2 = await compactor.apply( @@ -839,8 +843,11 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => { // CL-8744: the narrative lives in the fat handoff file, not the prompt. // The live output carries only the thin spine plus its pointer. expect(allText(result2.output)).not.toContain("UNIQUE_SUCCESS_SUMMARY"); - const handoffBlob = defined(defined(result2.blobs)[0]); - expect(handoffBlob.contentType).toBe("text/markdown"); + const handoffBlob = defined( + defined(result2.blobs).find( + (blob) => blob.contentType === "text/markdown", + ), + ); expect(new TextDecoder().decode(handoffBlob.bytes)).toContain( "UNIQUE_SUCCESS_SUMMARY", ); @@ -849,6 +856,51 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => { ); expect(hasConsecutiveSameRole(result2.output)).toBe(false); }); + + test("empty summarizer output folds with a statistics-only stub", async () => { + const source: InferenceSource = { + id: "test", + provider: "openai", + model: "test-model", + 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({ + keepRecentTurns: 2, + summaryMaxChars: 500, + summarize, + }).apply(grow([], 16, "empty"), mockStrategyCtx); + expect(result.record.decisions.summarizeFailed).toBe(1); + 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 () => { + const err = new Error("aborted by lifecycle"); + err.name = "AbortError"; + const turns = grow([], 16, "abort"); + const result = await smallCompactor({ + keepRecentTurns: 2, + summaryMaxChars: 500, + summarize: async () => { + throw err; + }, + }).apply(turns, mockStrategyCtx); + expect(result.output).toBe(turns); + expect(result.record.reason).toBe("summarize failed"); + expect(result.record.decisions.summarizeFailed).toBe(1); + expect(result.record.decisions.summarizedTurnCount).toBeUndefined(); + }); }); describe("buildTurnSummary via createPruningCompactor", () => { diff --git a/src/session/compactor.ts b/src/session/compactor.ts index 461470907..2f565c0b0 100644 --- a/src/session/compactor.ts +++ b/src/session/compactor.ts @@ -16,7 +16,11 @@ import type { } from "@intx/types/runtime"; import { ageImageBlocks } from "./attachment-store.js"; import { buildHandoffFold, COMPACTED_PREFIX } from "./compaction-handoff.js"; -import type { SummaryContext } from "./summarizer.js"; +import { + classifySummarizerFailure, + type SummarizerFailureClass, + type SummaryContext, +} from "./summarizer.js"; import { extractContinuationFacts, verifyOrRepair, @@ -1188,33 +1192,47 @@ export function createPruningCompactor( ? operatorCtx : { ...operatorCtx, priorSummary: priorSummaryForFold }; let summary: string; + let summarizeFallback: SummarizerFailureClass | undefined; + const stubSummary = (): string => + buildTurnSummary( + summarizedTurns, + cfg.summaryMaxChars, + anchorTurns.length, + ); try { summary = cfg.summarize !== undefined ? await cfg.summarize(summarizedTurns, summaryCtx) - : buildTurnSummary( - summarizedTurns, - cfg.summaryMaxChars, - anchorTurns.length, - ); - } catch { - return { - output: turns, - record: { - strategy: this.name, - version: this.version, - parameters: { - keepRecentTurns: cfg.keepRecentTurns, - compactionShape: shape, - ...extraInstructionParameter(cfg), + : stubSummary(); + } catch (error) { + const failureClass = classifySummarizerFailure(error); + // A lifecycle abort is operator intent, not a lossy fallback: keep the + // prior context so the wrapCompactor race cannot land a stub fold. + if (failureClass === "aborted") { + return { + output: turns, + record: { + strategy: this.name, + version: this.version, + parameters: { + keepRecentTurns: cfg.keepRecentTurns, + compactionShape: shape, + ...extraInstructionParameter(cfg), + }, + reason: "summarize failed", + decisions: { + summarizeFailed: 1, + agedImageCount: aged.agedImageCount, + }, }, - reason: "summarize failed", - decisions: { - summarizeFailed: 1, - agedImageCount: aged.agedImageCount, - }, - }, - }; + }; + } + summarizeFallback = failureClass; + summary = stubSummary(); + } + if (summary.trim().length === 0) { + summarizeFallback = summarizeFallback ?? "empty"; + summary = stubSummary(); } if (summary.trim().length === 0) { return { @@ -1337,7 +1355,11 @@ export function createPruningCompactor( compactionShape: shape, ...extraInstructionParameter(cfg), }, - reason: `compacted ${summarizedTurns.length} turns, anchored ${anchorTurns.length}, keeping ${tailTurns.length} tail`, + reason: `compacted ${summarizedTurns.length} turns, anchored ${anchorTurns.length}, keeping ${tailTurns.length} tail${ + summarizeFallback !== undefined + ? ` (statistics-only stub: ${summarizeFallback})` + : "" + }`, decisions: { summarizedTurnCount: summarizedTurns.length, anchorTurnCount: anchorTurns.length, @@ -1352,6 +1374,12 @@ export function createPruningCompactor( agedImageCount: aged.agedImageCount, supersededReadCount: supersededReads.size, repeatedErrorCount: repeatedErrors.size, + ...(summarizeFallback !== undefined + ? { + summarizeFailed: 1, + summarizeFailureKind: summarizeFallback, + } + : {}), ...verifyDecisions, }, }, diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index dd0ec0da8..fa7416a42 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -32,6 +32,7 @@ import { import type { SubAgentSourcesConfig } from "./runtime-assembly.js"; import type { Settings } from "../config/settings.js"; import type { Telemetry } from "../telemetry/index.js"; +import { createModelSummarizer } from "./summarizer.js"; import { generateSessionId, initSessionDir, sessionDir } from "./index.js"; import type { PluginModule } from "../plugins/loader.js"; @@ -563,6 +564,61 @@ describe("createSessionPruningCompactor", () => { }); }); +describe("createSessionPruningCompactor stub fallback", () => { + test("a summarizer stub fallback folds without success telemetry or onFolded", async () => { + const captured: { event: string }[] = []; + const folds: { turnsBefore: number; turnsAfter: number }[] = []; + const telemetry: Telemetry = { + enabled: true, + installationId: "test", + capture: (event) => { + captured.push({ event }); + }, + captureIntentional: () => false, + flush: async () => undefined, + discard: () => undefined, + }; + const notices: string[] = []; + 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"); + }, + onFailure: (text) => notices.push(text), + }); + const compactor = createSessionPruningCompactor({ + summarize, + telemetry, + onFolded: (info) => folds.push(info), + compactionShape: { tailBudgetTokens: 1 }, + }); + const now = Date.now(); + const many = Array.from({ length: 8 }, (_, i) => ({ + role: i % 2 === 0 ? "user" : "assistant", + content: [{ type: "text", text: `t${i}` }], + timestamp: now, + })); + const result = await compactor.apply(many as never, { + state: {} as never, + trigger: "test", + }); + expect(result.record.decisions.summarizeFailed).toBe(1); + expect(result.record.reason).toContain("statistics-only stub"); + expect(folds).toEqual([]); + expect(captured).toEqual([]); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("statistics-only stub"); + expect(notices[0]).toContain("failed"); + }); +}); + describe("buildCompactionContinuationMessage", () => { test("builds a content-less system inbound that re-enters the reactor", () => { const message = buildCompactionContinuationMessage(); diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index cde34fb4d..4d5a7bb18 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -475,8 +475,13 @@ 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. - if (result.record.decisions.summarizedTurnCount !== undefined) { + // 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, diff --git a/src/session/summarizer.test.ts b/src/session/summarizer.test.ts index 6eb31320a..704f5027a 100644 --- a/src/session/summarizer.test.ts +++ b/src/session/summarizer.test.ts @@ -103,11 +103,16 @@ 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 () => { @@ -261,6 +266,8 @@ test("a second 401 fails: retry budget is spent once", async () => { 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"); }); @@ -356,6 +363,8 @@ 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"); diff --git a/src/session/summarizer.ts b/src/session/summarizer.ts index d09a8e4e4..cfdf3003c 100644 --- a/src/session/summarizer.ts +++ b/src/session/summarizer.ts @@ -4,9 +4,9 @@ // replaces older turns with a summary. A deterministic stats blob ("Turns: N, // 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. Failure is fatal to that -// compact cycle: the caller retains the prior context instead of substituting -// a statistics-only stub. +// 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. import { type } from "arktype"; import { runInference, type Dependencies } from "@intx/inference"; @@ -388,7 +388,9 @@ function inferenceErrorCause(error: unknown): InferenceError | undefined { return parsed instanceof type.errors ? undefined : parsed; } -function classifySummarizerFailure(error: unknown): SummarizerFailureClass { +export function classifySummarizerFailure( + error: unknown, +): SummarizerFailureClass { const cause = inferenceErrorCause(error); if (cause !== undefined) { if (cause.category === "aborted") return "aborted"; @@ -425,13 +427,15 @@ function failureNotice( error: Error, ): string { const firstLine = error.message.split("\n", 1)[0]?.trim() ?? ""; - const reason = - firstLine.length > 0 + const detail = + firstLine.length > 0 && firstLine !== failureClass ? firstLine.length > 140 ? `${firstLine.slice(0, 140)}...` : firstLine - : failureClass; - return `Compaction summary failed — keeping prior context (${reason})`; + : undefined; + const kind = + detail !== undefined ? `${failureClass}: ${detail}` : failureClass; + return `Compaction summary failed — using a statistics-only stub (${kind})`; } export interface ModelSummarizerOptions { @@ -467,8 +471,8 @@ 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 keep the prior context - * instead of replacing it with a statistics-only stub. + * or a failed call throws so the compact cycle can substitute a + * statistics-only stub and surface that fallback to the operator. */ export function createModelSummarizer( options: ModelSummarizerOptions, diff --git a/src/telemetry/product-events.test.ts b/src/telemetry/product-events.test.ts index bc6117d62..9d8f75c67 100644 --- a/src/telemetry/product-events.test.ts +++ b/src/telemetry/product-events.test.ts @@ -695,6 +695,36 @@ test("compaction fires only when turns were actually folded away", async () => { expect(captured[0]?.properties.turns_before).toBe(60); }); +test("compaction does not fire when the summarizer fell back to a stub", async () => { + const { telemetry, events } = harness(); + const { createModelSummarizer } = await import("../session/summarizer.js"); + const compactor = createSessionPruningCompactor({ + summarize: createModelSummarizer({ + getSource: () => + ({ + id: "test", + provider: "openai", + model: "test-model", + credentialId: "test", + }) as never, + complete: async () => { + throw new Error("model unreachable"); + }, + }), + telemetry, + compactionShape: { tailBudgetTokens: 1 }, + }); + + const longHistory = Array.from({ length: 60 }, (_, i) => ({ + role: (i % 2 === 0 ? "user" : "assistant") as "user" | "assistant", + content: [{ type: "text" as const, text: `turn ${i}` }], + timestamp: i, + })); + await compactor.apply(longHistory, {} as never); + + expect(await events()).toEqual([]); +}); + // --------------------------------------------------------------------------- // The allowlist itself: unknown events and unknown keys never reach the wire // --------------------------------------------------------------------------- diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index c96f92066..039178118 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -599,7 +599,7 @@ export async function assembleTUISession( resolveLiveSessionSources(state.config, state.sessionId); // Compaction summarizer: structured handoff via the live model. Failure - // keeps prior context rather than substituting a stats stub. Workflow state + // substitutes a statistics-only stub and tells the operator. 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