Skip to content

Commit 0e2f0dc

Browse files
committed
fix(session): defer stub fallback notice until the fold commits
A failed summary used to tell the operator a statistics-only stub was in use before verify ran. Verify abort keeps prior context, so that notice was a lie. The wrapper now fires the notice and cache-prefix prune only after turns actually fold, skips success telemetry on a stub, and exec writes the same notice to stderr.
1 parent 24f7772 commit 0e2f0dc

9 files changed

Lines changed: 217 additions & 73 deletions

‎src/context-compactor.test.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -848,11 +848,9 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => {
848848
baseURL: "http://localhost:1",
849849
credentialId: "test",
850850
};
851-
const notices: string[] = [];
852851
const summarize = createModelSummarizer({
853852
getSource: () => source,
854853
complete: async () => "",
855-
onFailure: (text) => notices.push(text),
856854
});
857855
const result = await smallCompactor({
858856
summaryMaxChars: 500,
@@ -862,9 +860,6 @@ describe("createPruningCompactor — consolidated handoff (CL-7521)", () => {
862860
expect(result.record.decisions.summarizeFailureKind).toBe("empty");
863861
expect(result.record.reason).toContain("statistics-only stub: empty");
864862
expect(compactedTurns(result.output)).toHaveLength(1);
865-
expect(notices).toHaveLength(1);
866-
expect(notices[0]).toContain("statistics-only stub");
867-
expect(notices[0]).toContain("empty");
868863
});
869864

870865
test("an aborted summarizer keeps prior context instead of stubbing", async () => {

‎src/exec/runner.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,10 @@ export async function runExec(config: Config): Promise<ExecResult> {
943943
currentStorage?.readBlob.bind(currentStorage),
944944
),
945945
telemetry: liveTelemetry,
946+
// Operator-visible once a statistics-only stub actually replaces turns.
947+
onFailure: (text) => {
948+
stderr.write(`${text}\n`);
949+
},
946950
onFolded: () => {
947951
// Fold restarts the cached prefix — drop idle execute-promoted
948952
// schemas rather than carrying them forever, and persist so a

‎src/session/compaction-lifecycle.test.ts‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,6 @@ describe("createCompactionLifecycle", () => {
246246
}) as never,
247247
getSignal: () => lifecycle.getSignal(),
248248
telemetry,
249-
onFailure: (text) => {
250-
notices.push(text);
251-
},
252249
complete: (_promptTurns, _source, signal) =>
253250
new Promise<string>((_resolve, reject) => {
254251
signal.addEventListener(
@@ -297,8 +294,7 @@ describe("createCompactionLifecycle", () => {
297294
expect(telemetryEvents).toEqual([]);
298295
});
299296

300-
test("a genuine summarizer failure still notifies and emits telemetry", async () => {
301-
const notices: string[] = [];
297+
test("a genuine summarizer failure still emits telemetry", async () => {
302298
const captured: {
303299
event: string;
304300
properties?: Record<string, unknown> | undefined;
@@ -322,16 +318,11 @@ describe("createCompactionLifecycle", () => {
322318
credentialId: "test",
323319
}) as never,
324320
telemetry,
325-
onFailure: (text) => {
326-
notices.push(text);
327-
},
328321
complete: async () => {
329322
throw new Error("model unreachable");
330323
},
331324
});
332325
await expect(summarize(turns(5))).rejects.toThrow("model unreachable");
333-
expect(notices).toHaveLength(1);
334-
expect(notices[0]).toContain("Compaction summary failed");
335326
const failures = captured.filter((e) => e.event === "summarizer_failure");
336327
expect(failures).toHaveLength(1);
337328
expect(failures[0]?.properties?.["error_kind"]).toBe("failed");

‎src/session/runtime-assembly.test.ts‎

Lines changed: 117 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -567,9 +567,10 @@ describe("createSessionPruningCompactor", () => {
567567
});
568568

569569
describe("createSessionPruningCompactor stub fallback", () => {
570-
test("a summarizer stub fallback folds without success telemetry or onFolded", async () => {
570+
test("a summarizer stub fallback folds without success telemetry and still runs onFolded", async () => {
571571
const captured: { event: string }[] = [];
572-
const folds: { turnsBefore: number; turnsAfter: number }[] = [];
572+
const folds: { turnsBefore: number; turnsAfter: number; stub: boolean }[] =
573+
[];
573574
const telemetry: Telemetry = {
574575
enabled: true,
575576
installationId: "test",
@@ -593,12 +594,12 @@ describe("createSessionPruningCompactor stub fallback", () => {
593594
complete: async () => {
594595
throw new Error("model unreachable");
595596
},
596-
onFailure: (text) => notices.push(text),
597597
});
598598
const compactor = createSessionPruningCompactor({
599599
summarize,
600600
telemetry,
601601
onFolded: (info) => folds.push(info),
602+
onFailure: (text) => notices.push(text),
602603
compactionShape: { tailBudgetTokens: 1 },
603604
});
604605
const now = Date.now();
@@ -613,11 +614,123 @@ describe("createSessionPruningCompactor stub fallback", () => {
613614
});
614615
expect(result.record.decisions.summarizeFailed).toBe(1);
615616
expect(result.record.reason).toContain("statistics-only stub");
616-
expect(folds).toEqual([]);
617+
expect(folds).toEqual([
618+
{ turnsBefore: 8, turnsAfter: result.output.length, stub: true },
619+
]);
617620
expect(captured).toEqual([]);
618621
expect(notices).toHaveLength(1);
619622
expect(notices[0]).toContain("statistics-only stub");
620623
expect(notices[0]).toContain("failed");
624+
expect(notices[0]).toContain("model unreachable");
625+
});
626+
627+
test("verify abort after a failed summary keeps prior context and fires no stub notice", async () => {
628+
const notices: string[] = [];
629+
const folds: { stub: boolean }[] = [];
630+
const summarize = createModelSummarizer({
631+
getSource: () =>
632+
({
633+
id: "test",
634+
provider: "openai",
635+
model: "test-model",
636+
baseURL: "http://localhost:1",
637+
credentialId: "test",
638+
}) as never,
639+
complete: async () => {
640+
throw new Error("model unreachable");
641+
},
642+
});
643+
const now = Date.now();
644+
const turns = [
645+
{
646+
role: "user" as const,
647+
content: [
648+
{
649+
type: "text" as const,
650+
text: "Migrate the auth module to opaque tokens",
651+
},
652+
],
653+
timestamp: now,
654+
},
655+
{
656+
role: "assistant" as const,
657+
content: [
658+
{
659+
type: "tool_call" as const,
660+
id: "c1",
661+
name: "run_shell",
662+
arguments: { command: "bun test auth" },
663+
},
664+
],
665+
timestamp: now,
666+
},
667+
{
668+
role: "user" as const,
669+
content: [
670+
{
671+
type: "tool_result" as const,
672+
callId: "c1",
673+
isError: true,
674+
content: [
675+
{ type: "text" as const, text: "token refresh assertion failed" },
676+
],
677+
},
678+
],
679+
timestamp: now,
680+
},
681+
{
682+
role: "assistant" as const,
683+
content: [
684+
{ type: "text" as const, text: "Working through the failure" },
685+
],
686+
timestamp: now,
687+
},
688+
{
689+
role: "user" as const,
690+
content: [
691+
{
692+
type: "text" as const,
693+
text: "Confirm there are no errors remaining in auth",
694+
},
695+
],
696+
timestamp: now,
697+
},
698+
{
699+
role: "assistant" as const,
700+
content: [
701+
{ type: "text" as const, text: "Continuing the auth work now" },
702+
],
703+
timestamp: now,
704+
},
705+
{
706+
role: "user" as const,
707+
content: [
708+
{
709+
type: "text" as const,
710+
text: "There are no errors remaining in the suite",
711+
},
712+
],
713+
timestamp: now,
714+
},
715+
{
716+
role: "assistant" as const,
717+
content: [
718+
{ type: "text" as const, text: "I will keep going from here" },
719+
],
720+
timestamp: now,
721+
},
722+
];
723+
const result = await createSessionPruningCompactor({
724+
summarize,
725+
onFolded: (info) => folds.push(info),
726+
onFailure: (text) => notices.push(text),
727+
compactionShape: { tailBudgetTokens: 1 },
728+
}).apply(turns as never, { state: {} as never, trigger: "test" });
729+
expect(result.output).toBe(turns);
730+
expect(result.record.reason).toBe("verify failed — keeping prior context");
731+
expect(result.record.decisions.summarizedTurnCount).toBeUndefined();
732+
expect(folds).toEqual([]);
733+
expect(notices).toEqual([]);
621734
});
622735
});
623736

‎src/session/runtime-assembly.ts‎

Lines changed: 73 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ import {
5858
createPruningCompactor,
5959
type CompactionShape,
6060
} from "./compactor.js";
61-
import type { SummaryContext } from "./summarizer.js";
61+
import {
62+
classifySummarizerFailure,
63+
summarizerStubFallbackNotice,
64+
type SummaryContext,
65+
} from "./summarizer.js";
6266
import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js";
6367
import { COMPACTION_ABORTED_REASON } from "./compaction-lifecycle.js";
6468

@@ -418,7 +422,17 @@ export interface SessionPruningCompactorArgs {
418422
readPriorHandoff?: () => Promise<string | undefined>;
419423
telemetry?: Telemetry;
420424
/** Fires only when turns were actually folded away — not on no-ops. */
421-
onFolded?: (info: { turnsBefore: number; turnsAfter: number }) => void;
425+
onFolded?: (info: {
426+
turnsBefore: number;
427+
turnsAfter: number;
428+
/** True when the fold used a statistics-only stub, not an LLM summary. */
429+
stub: boolean;
430+
}) => void;
431+
/**
432+
* Operator-visible notice for a statistics-only stub that actually replaced
433+
* turns. Verify abort (keeping prior context) does not fire this.
434+
*/
435+
onFailure?: (text: string) => void;
422436
/**
423437
* True when the lifecycle has discarded (or will discard) the in-flight
424438
* compact — e.g. bound to the session compaction lifecycle's signal. A
@@ -434,10 +448,39 @@ export interface SessionPruningCompactorArgs {
434448
compactionShape?: Partial<CompactionShape>;
435449
}
436450

451+
function stubFallbackNoticeFromError(error: unknown): string | undefined {
452+
const err = error instanceof Error ? error : new Error(String(error));
453+
const kind = classifySummarizerFailure(err);
454+
if (kind === "aborted") return undefined;
455+
return summarizerStubFallbackNotice(kind, err);
456+
}
457+
458+
function stubFallbackNoticeFromRecord(
459+
pending: string | undefined,
460+
kind: unknown,
461+
): string {
462+
if (pending !== undefined) return pending;
463+
const label = typeof kind === "string" && kind.length > 0 ? kind : "failed";
464+
return `Compaction summary failed — using a statistics-only stub (${label})`;
465+
}
466+
437467
/** Shared pruning-compactor defaults for the main session agent. */
438468
export function createSessionPruningCompactor(
439469
args: SessionPruningCompactorArgs,
440470
): Compactor {
471+
let pendingStubNotice: string | undefined;
472+
const innerSummarize = args.summarize;
473+
const summarize =
474+
innerSummarize === undefined
475+
? undefined
476+
: async (turns: ConversationTurn[], ctx?: SummaryContext) => {
477+
try {
478+
return await innerSummarize(turns, ctx);
479+
} catch (error) {
480+
pendingStubNotice = stubFallbackNoticeFromError(error);
481+
throw error;
482+
}
483+
};
441484
const compactor = createPruningCompactor({
442485
summaryMaxChars: SESSION_COMPACTOR_SUMMARY_MAX_CHARS,
443486
// CL-9489 budgeted-tail shape: explicit defaults (same object the record
@@ -447,7 +490,7 @@ export function createSessionPruningCompactor(
447490
...DEFAULT_TAIL_COMPACTION_SHAPE,
448491
...args.compactionShape,
449492
},
450-
...(args.summarize !== undefined ? { summarize: args.summarize } : {}),
493+
...(summarize !== undefined ? { summarize } : {}),
451494
...(args.summaryContext ? { summaryContext: args.summaryContext } : {}),
452495
...(args.readPriorHandoff !== undefined
453496
? { readPriorHandoff: args.readPriorHandoff }
@@ -457,6 +500,7 @@ export function createSessionPruningCompactor(
457500
return {
458501
...compactor,
459502
async apply(turns, ctx) {
503+
pendingStubNotice = undefined;
460504
const turnsBefore = turns.length;
461505
const startedAt = Date.now();
462506
const result = await compactor.apply(turns, ctx);
@@ -473,21 +517,33 @@ export function createSessionPruningCompactor(
473517
// summarizedTurnCount is only set on the branch that actually folded
474518
// turns away. The other branch is a no-op (or image aging alone), and
475519
// reporting it as compaction would drag the duration and turn-count
476-
// averages toward the runs where nothing happened. A statistics-only
477-
// stub fold is not a successful LLM reduction: the operator notice
478-
// owns that path, and emitting the success event would relabel it.
479-
if (
480-
result.record.decisions.summarizedTurnCount !== undefined &&
481-
result.record.decisions.summarizeFailed !== 1
482-
) {
483-
telemetry.capture("compaction", {
484-
mode: "llm",
485-
duration_ms: Date.now() - startedAt,
486-
turns_before: turnsBefore,
487-
turns_after: result.output.length,
488-
});
489-
args.onFolded?.({ turnsBefore, turnsAfter: result.output.length });
520+
// averages toward the runs where nothing happened.
521+
if (result.record.decisions.summarizedTurnCount === undefined) {
522+
return result;
523+
}
524+
const stub = result.record.decisions.summarizeFailed === 1;
525+
// Stub folds still break the cached prefix, so prune still runs.
526+
// Success telemetry and the TUI fold flash must not relabel a stub.
527+
args.onFolded?.({
528+
turnsBefore,
529+
turnsAfter: result.output.length,
530+
stub,
531+
});
532+
if (stub) {
533+
args.onFailure?.(
534+
stubFallbackNoticeFromRecord(
535+
pendingStubNotice,
536+
result.record.decisions.summarizeFailureKind,
537+
),
538+
);
539+
return result;
490540
}
541+
telemetry.capture("compaction", {
542+
mode: "llm",
543+
duration_ms: Date.now() - startedAt,
544+
turns_before: turnsBefore,
545+
turns_after: result.output.length,
546+
});
491547
return result;
492548
},
493549
};

0 commit comments

Comments
 (0)