refactor(compaction): delete dead generic-fallback variant and prod-dead wrappers (#124603)

* refactor(compaction): delete dead generic-fallback variant and prod-dead wrappers

The CompactionSummaryResult union's generic-fallback variant lost its
only producer in b942db4d56 (summarization failures now throw
CompactionError), leaving summarizeInStages returning kind:'summary'
unconditionally, a dead consumer branch in the safeguard claiming a
degradation path that cannot execute, and a test asserting the
impossible shape. Collapse the return type to string and delete the
branch + obsolete test.

Also delete three prod-dead helpers that survived only through the
safeguard's testing export (invisible to the dead-export lint):
isOversizedForSummary (planning uses its own inline threshold since the
oversized-plan refactor), capCompactionSummaryPreservingSuffix, and
formatPreservedTurnsSection (thin wrappers over budgetCompactionSummary
/ buildPreservedTurnsSection with no prod callers). Tests point at the
surviving primitives; the details-exclusion test retargets the real
owner (estimateMessagesTokens sanitization).

* test(compaction): update staged-summary assertion to string return
This commit is contained in:
Peter Steinberger
2026-08-16 07:16:42 -07:00
committed by GitHub
parent ea77e21646
commit 4caa06b976
7 changed files with 54 additions and 173 deletions
@@ -6,7 +6,7 @@ type CompactionSafeguardTestApi = {
collectToolFailures: CallableFunction;
formatToolFailuresSection: CallableFunction;
splitPreservedRecentTurns: CallableFunction;
formatPreservedTurnsSection: CallableFunction;
buildPreservedTurnsSection: CallableFunction;
buildCompactionStructureInstructions: CallableFunction;
buildStructuredFallbackSummary: CallableFunction;
prependPreviousSummaryForRedistill: CallableFunction;
@@ -16,10 +16,9 @@ type CompactionSafeguardTestApi = {
extractOpaqueIdentifiers: CallableFunction;
auditSummaryQuality: CallableFunction;
capCompactionSummary: CallableFunction;
capCompactionSummaryPreservingSuffix: CallableFunction;
budgetCompactionSummary: CallableFunction;
formatFileOperations: CallableFunction;
computeAdaptiveChunkRatio: CallableFunction;
isOversizedForSummary: CallableFunction;
readWorkspaceContextForSummary: CallableFunction;
hasMeaningfulConversationContent: CallableFunction;
isRealConversationMessage: CallableFunction;
@@ -74,14 +74,28 @@ const actualCompactionQualityModule = await vi.importActual<typeof compactionQua
const mockAuditSummaryQuality = vi.mocked(compactionQualityModule.auditSummaryQuality);
function summaryResult(text: string) {
return { kind: "summary" as const, text };
return text;
}
// Local projections of the surviving primitives (the .text/.summary wrapper
// helpers were deleted with their prod-dead exports).
function budgetCompactionSummaryText(
body: string,
suffix: string,
maxChars = MAX_COMPACTION_SUMMARY_CHARS,
): string {
return (budgetCompactionSummary(body, suffix, maxChars) as { summary: string }).summary;
}
function preservedTurnsText(messages: AgentMessage[]): string {
return (buildPreservedTurnsSection(messages) as { text: string }).text;
}
const {
collectToolFailures,
formatToolFailuresSection,
splitPreservedRecentTurns,
formatPreservedTurnsSection,
buildPreservedTurnsSection,
buildCompactionStructureInstructions,
buildStructuredFallbackSummary,
prependPreviousSummaryForRedistill,
@@ -91,14 +105,12 @@ const {
extractOpaqueIdentifiers,
auditSummaryQuality: auditSummaryQualityOwner,
capCompactionSummary,
capCompactionSummaryPreservingSuffix,
budgetCompactionSummary,
formatFileOperations,
computeAdaptiveChunkRatio,
isOversizedForSummary,
readWorkspaceContextForSummary,
BASE_CHUNK_RATIO,
MIN_CHUNK_RATIO,
SAFETY_MARGIN,
MAX_COMPACTION_SUMMARY_CHARS,
MAX_FILE_OPS_SECTION_CHARS,
SUMMARY_TRUNCATED_MARKER,
@@ -540,7 +552,7 @@ describe("compaction-safeguard summary budgets", () => {
"\n\n<workspace-critical-rules>\n## Session Startup\nRead AGENTS.md\n</workspace-critical-rules>";
const body = "x".repeat(MAX_COMPACTION_SUMMARY_CHARS);
const capped = capCompactionSummaryPreservingSuffix(body, suffix);
const capped = budgetCompactionSummaryText(body, suffix);
expect(capped.length).toBeLessThanOrEqual(MAX_COMPACTION_SUMMARY_CHARS);
expect(capped).toContain("<workspace-critical-rules>");
@@ -554,7 +566,7 @@ describe("compaction-safeguard summary budgets", () => {
"<workspace-critical-rules>\n## Session Startup\nRead AGENTS.md\n</workspace-critical-rules>";
const body = "x".repeat(MAX_COMPACTION_SUMMARY_CHARS);
const capped = capCompactionSummaryPreservingSuffix(body, diagnosticSuffix);
const capped = budgetCompactionSummaryText(body, diagnosticSuffix);
expect(capped.length).toBeLessThanOrEqual(MAX_COMPACTION_SUMMARY_CHARS);
expect(capped).toContain("## Tool Failures");
@@ -567,10 +579,7 @@ describe("compaction-safeguard summary budgets", () => {
const bodyNoNewline = "## Exact identifiers\nNone.";
const suffixNoLeadingNewline = "## Tool Failures\n- exec: failed";
const capped = capCompactionSummaryPreservingSuffix(
bodyNoNewline,
`\n\n${suffixNoLeadingNewline}`,
);
const capped = budgetCompactionSummaryText(bodyNoNewline, `\n\n${suffixNoLeadingNewline}`);
expect(capped).toContain("None.\n\n## Tool Failures");
expect(capped).not.toMatch(/None\.## Tool Failures/);
@@ -591,7 +600,7 @@ describe("compaction-safeguard summary budgets", () => {
SUMMARY_TRUNCATED_MARKER,
);
expect(
capCompactionSummaryPreservingSuffix(
budgetCompactionSummaryText(
"",
"oversized suffix".repeat(10),
CONTEXT_TRUNCATED_MARKER.length,
@@ -608,7 +617,7 @@ describe("compaction-safeguard summary budgets", () => {
"x".repeat(MAX_COMPACTION_SUMMARY_CHARS);
const oversizedSuffix = preservedTurns + criticalTail;
const capped = capCompactionSummaryPreservingSuffix("short body", oversizedSuffix);
const capped = budgetCompactionSummaryText("short body", oversizedSuffix);
expect(capped.length).toBeLessThanOrEqual(MAX_COMPACTION_SUMMARY_CHARS);
expect(capped).toContain("<workspace-critical-rules>");
@@ -679,49 +688,6 @@ describe("computeAdaptiveChunkRatio", () => {
});
});
describe("isOversizedForSummary", () => {
const CONTEXT_WINDOW = 200_000;
it("returns false for small messages", () => {
const msg: AgentMessage = {
role: "user",
content: "Hello, world!",
timestamp: Date.now(),
};
expect(isOversizedForSummary(msg, CONTEXT_WINDOW)).toBe(false);
});
it("returns true for messages > 50% of context", () => {
// Message with ~120K tokens (60% of 200K context)
// After safety margin (1.2x), effective is 144K which is > 100K (50%)
const msg: AgentMessage = {
role: "user",
content: "x".repeat(120_000 * 4),
timestamp: Date.now(),
};
expect(isOversizedForSummary(msg, CONTEXT_WINDOW)).toBe(true);
});
it("applies safety margin", () => {
// Message at exactly 50% of context before margin
// After SAFETY_MARGIN (1.2), it becomes 60% which is > 50%
const halfContextChars = (CONTEXT_WINDOW * 0.5) / SAFETY_MARGIN;
const msg: AgentMessage = {
role: "user",
content: "x".repeat(Math.floor(halfContextChars * 4)),
timestamp: Date.now(),
};
// With safety margin applied, this should be at the boundary
// The function checks if tokens * SAFETY_MARGIN > contextWindow * 0.5
const isOversized = isOversizedForSummary(msg, CONTEXT_WINDOW);
// Due to token estimation, this could be either true or false at the boundary
expect(typeof isOversized).toBe("boolean");
});
});
describe("compaction-safeguard runtime registry", () => {
it("stores and retrieves config by session manager identity", () => {
const sm = {};
@@ -866,7 +832,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
expect(split.preservedMessages).toHaveLength(2);
expect(split.summarizableMessages).toHaveLength(2);
expect(formatPreservedTurnsSection(split.preservedMessages)).toContain(
expect(preservedTurnsText(split.preservedMessages)).toContain(
"## Recent turns preserved verbatim",
);
});
@@ -962,7 +928,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
recentTurnsPreserve: 1,
});
const section = formatPreservedTurnsSection(split.preservedMessages);
const section = preservedTurnsText(split.preservedMessages);
expect(section).toContain("- Tool result (read): recent raw output");
expect(section).toContain("- User: recent ask");
});
@@ -1002,7 +968,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
recentTurnsPreserve: 1,
});
const section = formatPreservedTurnsSection(split.preservedMessages) as string;
const section = preservedTurnsText(split.preservedMessages) as string;
expect(section.length).toBeLessThanOrEqual(MAX_SPLIT_TURN_CONTEXT_CHARS);
expect(section).toContain("[Earlier preserved messages truncated]");
@@ -1014,7 +980,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
});
it("formats preserved non-text messages with placeholders", () => {
const section = formatPreservedTurnsSection([
const section = preservedTurnsText([
{
role: "user",
content: [{ type: "image", data: "abc", mimeType: "image/png" }],
@@ -1032,7 +998,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
});
it("keeps non-text placeholders for mixed-content preserved messages", () => {
const section = formatPreservedTurnsSection([
const section = preservedTurnsText([
{
role: "user",
content: [
@@ -1048,7 +1014,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
});
it("keeps bounded preserved-turn text UTF-16 safe", () => {
const section = formatPreservedTurnsSection([
const section = preservedTurnsText([
{
role: "user",
content: `${"x".repeat(599)}🚀tail`,
@@ -1060,7 +1026,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
});
it("does not add non-text placeholders for text-only content blocks", () => {
const section = formatPreservedTurnsSection([
const section = preservedTurnsText([
{
role: "assistant",
content: [{ type: "text", text: "plain text reply" }],
@@ -1130,8 +1096,8 @@ describe("compaction-safeguard recent-turn preservation", () => {
msg.role === "user" && (msg as { content?: unknown }).content === "single user prompt",
),
).toBe(true);
expect(formatPreservedTurnsSection(split.preservedMessages)).toContain("assistant-8");
expect(formatPreservedTurnsSection(split.preservedMessages)).not.toContain("assistant-2");
expect(preservedTurnsText(split.preservedMessages)).toContain("assistant-8");
expect(preservedTurnsText(split.preservedMessages)).not.toContain("assistant-2");
});
it("trim-starts preserved section when history summary is empty", () => {
@@ -2645,54 +2611,6 @@ describe("compaction-safeguard recent-turn preservation", () => {
expect(result.compaction?.summary).not.toContain("Old duplicated section");
});
it("preserves the prior summary when staged summarization returns a generic fallback", async () => {
mockSummarizeInStages.mockReset();
mockSummarizeInStages.mockResolvedValue({
kind: "generic-fallback",
text: "Context contained 4 messages. Summary unavailable due to size limits.",
});
const sessionManager = stubSessionManager();
const model = createAnthropicModelFixture();
setCompactionSafeguardRuntime(sessionManager, {
model,
recentTurnsPreserve: 0,
});
const compactionHandler = createCompactionHandler();
const mockContext = createCompactionContext({
sessionManager,
getApiKeyMock: vi.fn().mockResolvedValue("test-key"),
});
const event = {
preparation: {
messagesToSummarize: [{ role: "user", content: "latest ask status", timestamp: 1 }],
turnPrefixMessages: [],
firstKeptEntryId: "entry-1",
tokensBefore: 1_500,
fileOps: {
read: [],
edited: [],
written: [],
},
settings: { reserveTokens: 4_000 },
previousSummary: "## Goal\nKnown context that must survive the outage.",
isSplitTurn: false,
},
customInstructions: "",
signal: new AbortController().signal,
};
const result = (await compactionHandler(event, mockContext)) as {
cancel?: boolean;
compaction?: { summary?: string };
};
expect(result.cancel).not.toBe(true);
expect(result.compaction?.summary).toContain("Known context that must survive the outage.");
expect(result.compaction?.summary).toContain("Summary unavailable due to size limits.");
});
it("falls back to LLM when provider throws a provider-side AbortError with signal not aborted", async () => {
// Reproduce the undici AbortError("This operation was aborted") shape that
// arrives when the compaction provider's HTTP connection drops mid-stream while
+5 -25
View File
@@ -34,7 +34,6 @@ import {
SAFETY_MARGIN,
SUMMARIZATION_OVERHEAD_TOKENS,
computeAdaptiveChunkRatio,
isOversizedForSummary,
resolveContextWindowTokens,
summarizeInStages,
} from "../compaction.js";
@@ -302,19 +301,13 @@ function containsRealConversation(messages: AgentMessage[]): boolean {
* Only called when no compaction provider is available or the provider failed.
*/
async function summarizeViaLLM(params: Parameters<typeof summarizeInStages>[0]): Promise<string> {
const result = await compactionSafeguardDeps.summarizeInStages({
// Summarization failure throws CompactionError (b942db4d569b) — there is no
// degraded-fallback return shape to preserve a previous summary against.
return await compactionSafeguardDeps.summarizeInStages({
...params,
messages: prependPreviousSummaryForRedistill(params),
previousSummary: undefined,
});
if (result.kind === "summary") {
return result.text;
}
// A generic fallback means redistillation never happened. Preserve the
// known summary verbatim so a temporary model outage cannot erase it.
const previousSummary = params.previousSummary?.trim();
return previousSummary ? `${previousSummary}\n\n${result.text}` : result.text;
}
/**
@@ -579,14 +572,6 @@ function capCompactionSummary(summary: string, maxChars = MAX_COMPACTION_SUMMARY
return `${truncateUtf16Safe(summary, budget)}${marker}`;
}
function capCompactionSummaryPreservingSuffix(
summaryBody: string,
suffix: string,
maxChars = MAX_COMPACTION_SUMMARY_CHARS,
): string {
return budgetCompactionSummary(summaryBody, suffix, maxChars).summary;
}
function normalizeCompactionSuffix(suffix: string | CompactionSuffix): CompactionSuffix {
return typeof suffix === "string" ? { text: suffix, contextRanges: [] } : suffix;
}
@@ -903,10 +888,6 @@ function buildPreservedTurnsSection(messages: AgentMessage[]): ContextSection {
});
}
function formatPreservedTurnsSection(messages: AgentMessage[]): string {
return buildPreservedTurnsSection(messages).text;
}
function buildSplitTurnContextSection(
messages: AgentMessage[],
onTruncated?: () => void,
@@ -1463,7 +1444,7 @@ const testing = {
collectToolFailures,
formatToolFailuresSection,
splitPreservedRecentTurns,
formatPreservedTurnsSection,
buildPreservedTurnsSection,
buildCompactionStructureInstructions,
buildStructuredFallbackSummary,
prependPreviousSummaryForRedistill,
@@ -1473,10 +1454,9 @@ const testing = {
extractOpaqueIdentifiers,
auditSummaryQuality,
capCompactionSummary,
capCompactionSummaryPreservingSuffix,
budgetCompactionSummary,
formatFileOperations,
computeAdaptiveChunkRatio,
isOversizedForSummary,
readWorkspaceContextForSummary,
hasMeaningfulConversationContent,
isRealConversationMessage,
-6
View File
@@ -226,12 +226,6 @@ export function computeAdaptiveChunkRatio(messages: AgentMessage[], contextWindo
return BASE_CHUNK_RATIO;
}
/** Returns whether one message exceeds the safe summarization context share. */
export function isOversizedForSummary(msg: AgentMessage, contextWindow: number): boolean {
const tokens = estimateMessagesTokens([msg]) * SAFETY_MARGIN;
return tokens > contextWindow * 0.5;
}
/** Builds sanitized chunks for summarization prompts. */
export function buildSummaryChunks(params: {
messages: AgentMessage[];
@@ -347,10 +347,7 @@ describe("compaction staged summarization failures", () => {
.mockResolvedValueOnce("summary of chunk 3")
.mockResolvedValue("merged: chunk 1 + chunk 2 + chunk 3");
await expect(runStagedSummary()).resolves.toEqual({
kind: "summary",
text: expect.stringContaining("merged"),
});
await expect(runStagedSummary()).resolves.toEqual(expect.stringContaining("merged"));
});
it("throws CompactionError when a later chunk fails after earlier successes", async () => {
@@ -18,7 +18,7 @@ vi.mock("./sessions/index.js", async () => {
};
});
let isOversizedForSummary: typeof import("./compaction.js").isOversizedForSummary;
let estimateMessagesTokens: typeof import("./compaction.js").estimateMessagesTokens;
let summarizeWithFallback: typeof import("./compaction.test-support.js").summarizeWithFallback;
function makeAssistantToolCall(timestamp: number): AssistantMessage {
@@ -46,7 +46,7 @@ function makeToolResultWithDetails(timestamp: number): ToolResultMessage<{ raw:
describe("compaction toolResult details stripping", () => {
beforeAll(async () => {
({ isOversizedForSummary } = await import("./compaction.js"));
({ estimateMessagesTokens } = await import("./compaction.js"));
({ summarizeWithFallback } = await import("./compaction.test-support.js"));
});
@@ -157,7 +157,7 @@ describe("compaction toolResult details stripping", () => {
expect(serialized).not.toContain("secret runtime context");
});
it("ignores toolResult.details when evaluating oversized messages", () => {
it("ignores toolResult.details when estimating compaction tokens", () => {
agentSessionMocks.estimateTokens.mockImplementation((message: unknown) => {
const record = message as { details?: unknown };
return record.details ? 10_000 : 10;
@@ -173,6 +173,8 @@ describe("compaction toolResult details stripping", () => {
timestamp: 2,
};
expect(isOversizedForSummary(toolResult, 1_000)).toBe(false);
// Sanitization strips details before estimation; the raw payload must
// never inflate compaction token pressure.
expect(estimateMessagesTokens([toolResult])).toBeLessThan(1_000);
});
});
+9 -18
View File
@@ -17,7 +17,6 @@ import {
BASE_CHUNK_RATIO,
computeAdaptiveChunkRatio,
estimateMessagesTokens,
isOversizedForSummary,
MIN_CHUNK_RATIO,
SAFETY_MARGIN,
SUMMARIZATION_OVERHEAD_TOKENS,
@@ -32,7 +31,6 @@ export {
BASE_CHUNK_RATIO,
computeAdaptiveChunkRatio,
estimateMessagesTokens,
isOversizedForSummary,
MIN_CHUNK_RATIO,
SAFETY_MARGIN,
SUMMARIZATION_OVERHEAD_TOKENS,
@@ -42,10 +40,6 @@ const log = createSubsystemLogger("compaction");
type PartialSummaryError = Error & { partialSummary?: string };
type CompactionSummaryResult =
| { kind: "summary"; text: string }
| { kind: "generic-fallback"; text: string };
const DEFAULT_SUMMARY_FALLBACK = "No prior history.";
const MERGE_SUMMARIES_INSTRUCTIONS = [
"Merge these partial summaries into a single cohesive summary.",
@@ -294,10 +288,10 @@ export async function summarizeInStages(
parts?: number;
minMessagesForSplit?: number;
},
): Promise<CompactionSummaryResult> {
): Promise<string> {
const { messages } = params;
if (messages.length === 0) {
return { kind: "summary", text: await summarizeWithFallback(params) };
return await summarizeWithFallback(params);
}
const plan = await buildStageSplitPlanWithWorker({
@@ -309,7 +303,7 @@ export async function summarizeInStages(
});
if (plan.mode === "single") {
return { kind: "summary", text: await summarizeWithFallback(params) };
return await summarizeWithFallback(params);
}
const partialSummaries: string[] = [];
@@ -342,7 +336,7 @@ export async function summarizeInStages(
if (summary === undefined) {
throw new Error("Compaction summary plan produced no summary");
}
return { kind: "summary", text: summary };
return summary;
}
// Capture once so timestamps are strictly monotonic across
@@ -376,14 +370,11 @@ export async function summarizeInStages(
? `${MERGE_SUMMARIES_INSTRUCTIONS}\n\n${custom}`
: MERGE_SUMMARIES_INSTRUCTIONS;
return {
kind: "summary",
text: await summarizeWithFallback({
...params,
messages: summaryMessages,
customInstructions: mergeInstructions,
}),
};
return await summarizeWithFallback({
...params,
messages: summaryMessages,
customInstructions: mergeInstructions,
});
}
/** Resolves a positive context-window token count from model metadata. */