From 1808570933d866ec5c7ceff83181e0456f3ddaf6 Mon Sep 17 00:00:00 2001 From: Hypatia May Date: Mon, 29 Jun 2026 16:56:44 +0800 Subject: [PATCH] fix(compact): harden summarization convergence Use maxTokens as the provider generation cap and remove the confusing stored-summary max config. Strip reasoning blocks before storing compaction summaries, reject non-shrinking summaries, and retry bounded re-compaction when the surface remains over threshold. Add config validation for numeric and type-shaped knobs plus unit and real-API e2e coverage for reasoning-capable summarization. --- docs/core-data-structures/compaction.md | 2 +- .../2026-06-18-compaction-capability-seam.md | 2 +- examples/coding-agent/tests/compaction.e2e.ts | 13 +- packages/compact/compact-basic/README.md | 7 +- packages/compact/compact-basic/src/index.ts | 148 +++++++----- packages/compact/compact-basic/src/types.ts | 70 +++--- .../compact-basic/tests/compact-basic.spec.ts | 220 +++++++++++++----- .../tests/compact-loop-repro.spec.ts | 6 +- 8 files changed, 319 insertions(+), 149 deletions(-) diff --git a/docs/core-data-structures/compaction.md b/docs/core-data-structures/compaction.md index a1ca8978a6..05d1ce6c2e 100644 --- a/docs/core-data-structures/compaction.md +++ b/docs/core-data-structures/compaction.md @@ -52,4 +52,4 @@ interface CompactionResult { `CompactService` (`ctx.compact`, abstract — defined in [`packages/compact/compact/src/index.ts`](../../packages/compact/compact/src/index.ts)) declares two abstract methods: `compactIfNeeded(agent, turn, step, fullSystemPrompt, signal)` checks token pressure and compacts an older range if the history is too large (returning `null` when nothing needs it), and `compactRegion(session, start, end, agent, turn, step, signal?)` forcibly summarizes surface nodes `[start, end]` into a single replacement node. `compactIfNeeded`'s parameters are all required — the loop's `agent/pre-step` checkpoint supplies the agent, lifecycle context, assembled `fullSystemPrompt`, and turn `signal`. A backend summarizing via `ctx.llm.stream()` must forward `signal` into the call's `GenerateOptions.signal`, so an abort or dispose tears down the in-flight summarization. The entire strategy — token estimation, retention policy, event sequencing, summarization — is a HOW decision owned by the implementation. -Auto-compaction runs on the serial `agent/pre-step` loop seam (fired once per step, after `turn/start` and BEFORE the step opens and its request history is derived), not the `agent/request` waterfall: compaction mutates the session surface in place — with its log-only `compact/*` records landing cleanly outside any step — and the loop derives the request from the already-compacted surface. Retention is turn-agnostic — the only structural guard is tool-pairing balance (a compacted region's edges are balanced cuts on the surface, so it never splits a step's tool-calls from their results), so a single runaway turn that alone exceeds the window compacts its own early closed steps rather than being retained verbatim. The backend that ships this (`dsh-compact-basic`) documents the retention walk, the approximate convergence invariant, and the crash/recoverable failure taxonomy. +Auto-compaction runs on the serial `agent/pre-step` loop seam (fired once per step, after `turn/start` and BEFORE the step opens and its request history is derived), not the `agent/request` waterfall: compaction mutates the session surface in place — with its log-only `compact/*` records landing cleanly outside any step — and the loop derives the request from the already-compacted surface. Retention is turn-agnostic — the only structural guard is tool-pairing balance (a compacted region's edges are balanced cuts on the surface, so it never splits a step's tool-calls from their results), so a single runaway turn that alone exceeds the window compacts its own early closed steps rather than being retained verbatim. The backend that ships this (`dsh-compact-basic`) documents the retention walk, summary shrink validation, bounded re-compaction, and the crash/recoverable failure taxonomy. diff --git a/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md b/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md index ba6f6c6ae6..29371a3abb 100644 --- a/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md +++ b/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md @@ -66,7 +66,7 @@ A runaway turn thus compacts exactly like any other history: its early *closed* ### Approximate convergence invariant -`resolveConfig` **rejects** (throws at construction) any config where `summarizationMaxTokens + retainTokens >= contextWindow * thresholdRatio`. The invariant bounds the two variable parts of post-compaction history — the bounded summary plus the retained recent tail — but it is intentionally approximate: checkpoint framing, per-message role overhead, system-prompt size, and the char/4 estimator's error can still leave a narrow accepted config near the threshold. The bound is **strict** (`>=` rejects, not `>`): the token-pressure gate declines only when the estimate is `< threshold`, so a post-compaction history sitting *exactly* at the threshold would re-trigger on the very next check — equality is a leak, not a safe boundary. `summarizationMaxTokens` stays an explicit *quality* knob (terse summaries); the invariant only forbids setting it so high it breaks the structural budget. The sole residual is the single-unit-overflow case above (a backward-rounded oversized step can push the retained tail over budget) — which is exactly the out-of-scope concern, not a thrash bug. Per the pre-release reject-don't-migrate stance, a config that cannot satisfy the structural bound is a bug at the call site, not something to silently clamp. +`resolveConfig` validates numeric knobs but does NOT reject based on a pretend summary-length invariant. Convergence is dynamic: provider output caps can be spent on hidden or surfaced reasoning tokens, and the model may emit a summary of unpredictable size. `maxTokens` is only the provider-side generation cap for the summarization call; reasoning blocks are stripped before the checkpoint is stored. If a compacted surface is still over threshold, `compactIfNeeded()` re-compacts the head checkpoint up to `compactionRetries` extra times, but each committed summary must be smaller than the content it shadows. The sole residual is the single-unit-overflow case above (a backward-rounded oversized step can push the retained tail over budget) — which is exactly the out-of-scope concern, not a thrash bug. ### Surface replacement: `compact/*` events are log-only; one `user/message` carries the summary diff --git a/examples/coding-agent/tests/compaction.e2e.ts b/examples/coding-agent/tests/compaction.e2e.ts index 17186055b1..a300aac69a 100644 --- a/examples/coding-agent/tests/compaction.e2e.ts +++ b/examples/coding-agent/tests/compaction.e2e.ts @@ -40,18 +40,17 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('compaction: a long session compa await writeFile(join(workdir, `file${i}.txt`), `This is file number ${i}. `.repeat(40)) } - // Tiny window so a couple of steps crosses the threshold. The convergence - // invariant requires summarizationMaxTokens + retainTokens to be strictly - // BELOW the threshold = floor(contextWindow * thresholdRatio) = - // floor(2400 * 0.5) = 1200; 600 + 500 = 1100 < 1200. The summary cap - // stays high enough for the live model to emit the required checkpoint - // sections; a truncated checkpoint fails closed and leaves no summary. + // Tiny window so a couple of steps crosses the threshold. The generation + // cap is deliberately larger than the final checkpoint because + // reasoning-capable APIs count reasoning tokens against the provider output + // budget even though those blocks are stripped before the checkpoint is + // stored. ctx = await codingHarness(workdir, { compact: { contextWindow: 2400, thresholdRatio: 0.5, retainTokens: 500, - summarizationMaxTokens: 600, + maxTokens: 2048, }, persistenceRoot: './.sessions', }) diff --git a/packages/compact/compact-basic/README.md b/packages/compact/compact-basic/README.md index 2b13666033..5d4f2c755e 100644 --- a/packages/compact/compact-basic/README.md +++ b/packages/compact/compact-basic/README.md @@ -10,8 +10,8 @@ The abstract contract states only WHAT compaction does; this backend owns every - **Token estimation** — `estimateContentTokens()`: char/4 with per-block structural overhead (`text`/`reasoning` = `ceil(len/4) + 4`, `tool-call` from name + arguments, `tool-result` recursive, `image` = 85, unknown blocks via JSON length). - **Retention policy** — `compactIfNeeded()` walks the surface nodes tail→head summing per-node token estimates, and retains the smallest tail-run of WHOLE units (a closed step, or a single no-step node such as a pre-step `user/message` or inter-step `steering/message`) whose total reaches `retainTokens`; everything older is compacted. Retention is **turn-agnostic** — turn boundaries play no role, so a single runaway turn that alone exceeds the window compacts its OWN early closed steps rather than being retained verbatim (the failure mode that motivated dropping turn-protection: a tool-heavy turn must stay compactable or the harness dies exactly when compaction is needed). The only structural guard is **tool-pairing balance**: the compacted region's edges are balanced cuts on the surface (no unanswered tool-call crosses either edge), so it never splits a step's `assistant/message` tool-calls from their `tool/result`s. When the only compactable content left is an un-splittable open tail step, it declines (returns `null`) and retries once an older step closes. **Single-unit overflow is out of scope, by design**: if one retained unit (a single closed step, or a large pasted `user/message`) ALONE exceeds the budget, compaction cannot help and the call may go out over-budget — bounding an individual unit's size is a separate concern. `compactRegion()` enforces tool-pairing balance strictly, throwing on a boundary that would split a step. `dsh-session` exports `isToolPairingBalanced` for the check. -- **Single-pass convergence** — `resolveConfig()` rejects (throws) any config where `summarizationMaxTokens + retainTokens >= contextWindow * thresholdRatio`. The invariant guarantees the post-compaction history (the bounded summary plus the retained recent tail) is structurally below the threshold, so a compaction never immediately triggers another: consecutive re-compaction is impossible by construction. The bound is strict (`>=` rejects) because the token-pressure gate declines only when the estimate is `< threshold` — a post-compaction history sitting exactly at the threshold would re-trigger. -- **Summarization** — `summarize()`: a `GenerateOptions` request assembled via `BlockAssembler` with a fixed system prompt that asks for a structured checkpoint (Primary Request and Intent · Key Technical Concepts · Files and Code · Errors and Fixes · Pending Tasks · Current Work · Next Step · Critical Context), every section mandatory, exact paths/commands/identifiers preserved. The request runs through the `agent/request` waterfall before `ctx.llm.stream()`, so router agents that choose the concrete model there also route compaction summaries. The compacted region is flattened to a plain-text transcript first: text and reasoning contribute their text, and every non-text block (image, tool-call, tool-result, plugin-added types) contributes a type-tagged placeholder (`[image]`, `[tool-call: name(args)]`, …) so the summarizer is told what existed rather than silently dropping it. +- **Dynamic convergence** — no static summary-length config pretends to bound what the model will write. If framing/estimator/system overhead leaves the compacted surface above threshold, `compactIfNeeded()` re-compacts the head checkpoint up to `compactionRetries` extra times; if it still cannot get below threshold, it throws. A summary whose estimated stored size is not smaller than the shadowed content fails closed before it mutates the surface. +- **Summarization** — `summarize()`: a `GenerateOptions` request assembled via `BlockAssembler` with a fixed system prompt that asks for a structured checkpoint (Primary Request and Intent · Key Technical Concepts · Files and Code · Errors and Fixes · Pending Tasks · Current Work · Next Step · Critical Context), every section mandatory, exact paths/commands/identifiers preserved. The request runs through the `agent/request` waterfall before `ctx.llm.stream()`, so router agents that choose the concrete model there also route compaction summaries. `maxTokens` is the provider-side generation cap; reasoning blocks from reasoning-capable APIs are stripped before the checkpoint is stored. The compacted region is flattened to a plain-text transcript first: text and reasoning contribute their text, and every non-text block (image, tool-call, tool-result, plugin-added types) contributes a type-tagged placeholder (`[image]`, `[tool-call: name(args)]`, …) so the summarizer is told what existed rather than silently dropping it. - **Checkpoint framing** — the raw summary is not landed directly. `compactRegion()` wraps it in a checkpoint preamble (so a resuming model reads it as a checkpoint, not a fresh user request, and builds on the captured context rather than restating it) plus `…` tags. Because region compaction can be invoked manually, a surface may hold several checkpoints, so the framing does not claim everything after it is recent or verbatim. The tags make a prior checkpoint detectable in the transcript on the next compaction cycle: the summarization prompt then instructs the model to merge it in place (preserve still-true facts, drop stale ones) rather than re-summarize it verbatim — a cheap incremental merge that needs no extra log/event machinery. The unframed summary stays on the `compact/summary` provenance event. - **Surface mutation** — `compactRegion()` appends the `compact/start` → `compact/summary` → `compact/end` log records and the single `user/message` replace node carrying the framed summary (see the interface README). - **Auto-compaction** — an `agent/pre-step` listener delegates to `compactIfNeeded()` before every step (not just a turn's first — a tool-heavy turn grows the surface mid-turn, so a runaway turn still compacts, and per-step firing is the only moment to rescue it before overflow). `agent/pre-step` is a serial (awaited, in-order, no-veto) surface-mutation checkpoint that fires after `turn/start` and BEFORE the step opens (`step/start`) and its request history is derived, so compaction mutates the surface — with its log-only `compact/*` records landing cleanly outside any step — and the loop derives once from the result: no double-derive, and the listener cannot see (or need to rewrite) an already-assembled `messages` array. The listener owns no threshold logic of its own (the single token-pressure check lives in `compactIfNeeded()`). @@ -27,7 +27,8 @@ The abstract contract states only WHAT compaction does; this backend owns every | `thresholdRatio` | `0.8` | Compact when estimated usage exceeds this fraction of the window. | | `retainTokens` | `20480` | Tokens of recent context to keep intact. | | `summarizationModel` | `''` | Model for summarization (empty → use the agent's model). | -| `summarizationMaxTokens` | `2048` | Max tokens for the summary response. | +| `maxTokens` | `8192` | Provider generation cap for the summarization call; may include reasoning tokens. | +| `compactionRetries` | `1` | Extra compaction attempts after the first if the compacted surface remains over threshold. | | `auto` | `true` | Register the `agent/pre-step` auto-compaction listener. Set `false` for manual-only. | ## Usage diff --git a/packages/compact/compact-basic/src/index.ts b/packages/compact/compact-basic/src/index.ts index e474be7cd9..7d84b52511 100644 --- a/packages/compact/compact-basic/src/index.ts +++ b/packages/compact/compact-basic/src/index.ts @@ -290,7 +290,7 @@ export class BasicCompactService extends CompactService { content: [{ type: 'text', text: `Summarize this conversation history:\n\n${text}\n\nSummary:` }], }], system: SUMMARIZE_SYSTEM_PROMPT, - maxTokens: this.config.summarizationMaxTokens, + maxTokens: this.config.maxTokens, } // exactOptionalPropertyTypes: only set `signal` when present — assigning // `undefined` to an optional `signal?: AbortSignal` is a type error. @@ -306,7 +306,12 @@ export class BasicCompactService extends CompactService { const error = finishError(assembler.finish) if (error) throw error - return assembler.message().content + const summary = this._stripReasoning(assembler.message().content) + if (!summary.some(block => block.type === 'text' && block.text.trim().length > 0)) { + throw new Error('summarization produced no non-reasoning summary content') + } + + return summary } // ---- Core API (implements the abstract contract) ---- @@ -345,62 +350,28 @@ export class BasicCompactService extends CompactService { signal: AbortSignal, ): Promise { const session = agent.session - const messages = session.deriveMessages() - const totalTokens = this.estimateTokens(messages, fullSystemPrompt) - const threshold = Math.floor(this.config.contextWindow * this.config.thresholdRatio) - if (totalTokens < threshold) return null + let result: CompactionResult | null = null + for (let attempt = 0; attempt <= this.config.compactionRetries; attempt++) { + const totalTokens = this.estimateTokens(session.deriveMessages(), fullSystemPrompt) + if (totalTokens < threshold) return result - const nodes = session.surface.nodes - if (nodes.length === 0) return null + const range = this._compactableRange(session) + if (range === null) { + if (result === null) return null + break + } - const events = session.events - const retainBudget = this.config.retainTokens - - // Walk tail→head summing per-node token estimates. `keepFromIdx` is the - // index of the OLDEST node we retain verbatim; everything strictly older - // (`[0, keepFromIdx - 1]`) is the compactable range. - let accumulated = 0 - let keepFromIdx = nodes.length // nothing retained yet - for (let i = nodes.length - 1; i >= 0; i--) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const node = nodes[i]! - const event = events[node.seq] - /* v8 ignore next -- node.seq is a surface-node seq, always a valid log index by construction */ - if (event) accumulated += this.estimateEventTokens(event) - keepFromIdx = i - if (accumulated >= retainBudget) break + result = await this.compactRegion(session, range.start, range.end, agent, turn, step, signal) } - // The whole surface fits the retain budget — nothing to compact. - if (keepFromIdx === 0) return null + const totalTokens = this.estimateTokens(session.deriveMessages(), fullSystemPrompt) + if (totalTokens < threshold) return result - // Round the cutoff to a tool-pairing boundary: if the cut before - // `nodes[keepFromIdx]` is unbalanced (an unanswered tool-call sits before - // it — i.e. it is mid-step), extend the retained side head-ward until the - // cut is balanced, so the compacted range ends without splitting an - // assistant↔result pair. A node that belongs to no step is already a - // balanced (free) boundary. Decline if no balanced cut exists at or below - // `keepFromIdx` (the compactable range is only an un-splittable open tail - // step — retry once it closes). - while (keepFromIdx > 0) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - if (isToolPairingBalanced(nodes, events, nodes[keepFromIdx]!.seq)) break - keepFromIdx -= 1 - } - if (keepFromIdx === 0) return null - - // The compacted range is [head … keepFromIdx - 1], anchored at the head. - // The cutoff node `nodes[keepFromIdx - 1]` is necessarily a balanced END: - // the retained start `nodes[keepFromIdx]` opens on a balanced cut, and that - // same cut is the cut AFTER `nodes[keepFromIdx - 1]` — so no separate end - // check is needed. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const firstSeq = nodes[0]!.seq - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const cutoffSeq = nodes[keepFromIdx - 1]!.seq - - return this.compactRegion(session, firstSeq, cutoffSeq, agent, turn, step, signal) + throw new Error( + `compaction still above threshold after ${this.config.compactionRetries + 1} compaction attempts ` + + `(${totalTokens} estimated tokens >= threshold ${threshold})`, + ) } override async compactRegion( @@ -482,7 +453,12 @@ export class BasicCompactService extends CompactService { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion shadowedTokenCount += this.estimateEventTokens(session.events[seq]!) } - + const summaryTokenCount = this.estimateContentTokens(summary) + if (summaryTokenCount >= shadowedTokenCount) { + throw new Error( + `summary is not smaller than the shadowed content (${summaryTokenCount} estimated tokens >= ${shadowedTokenCount})`, + ) + } // --- Provenance record (log-only) --- const summaryEvent = session.append('compact/summary', { summary, @@ -580,6 +556,72 @@ export class BasicCompactService extends CompactService { return false } + /** Resolve the next head-anchored compactable surface range, or `null`. */ + private _compactableRange(session: Session): { start: number; end: number } | null { + const nodes = session.surface.nodes + if (nodes.length === 0) return null + + const events = session.events + const retainBudget = this.config.retainTokens + + // Walk tail→head summing per-node token estimates. `keepFromIdx` is the + // index of the OLDEST node we retain verbatim; everything strictly older + // (`[0, keepFromIdx - 1]`) is the compactable range. + let accumulated = 0 + let keepFromIdx = nodes.length // nothing retained yet + for (let i = nodes.length - 1; i >= 0; i--) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const node = nodes[i]! + const event = events[node.seq] + /* v8 ignore next -- node.seq is a surface-node seq, always a valid log index by construction */ + if (event) accumulated += this.estimateEventTokens(event) + keepFromIdx = i + if (accumulated >= retainBudget) break + } + + // The whole surface fits the retain budget — nothing to compact. + if (keepFromIdx === 0) return null + + // Round the cutoff to a tool-pairing boundary: if the cut before + // `nodes[keepFromIdx]` is unbalanced (an unanswered tool-call sits before + // it — i.e. it is mid-step), extend the retained side head-ward until the + // cut is balanced, so the compacted range ends without splitting an + // assistant↔result pair. A node that belongs to no step is already a + // balanced (free) boundary. Decline if no balanced cut exists at or below + // `keepFromIdx` (the compactable range is only an un-splittable open tail + // step — retry once it closes). + while (keepFromIdx > 0) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + if (isToolPairingBalanced(nodes, events, nodes[keepFromIdx]!.seq)) break + keepFromIdx -= 1 + } + if (keepFromIdx === 0) return null + + // The compacted range is [head … keepFromIdx - 1], anchored at the head. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const firstSeq = nodes[0]!.seq + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const cutoffSeq = nodes[keepFromIdx - 1]!.seq + return { start: firstSeq, end: cutoffSeq } + } + + /** Remove reasoning blocks from model-produced summary content before storing it. */ + private _stripReasoning(blocks: readonly ContentBlock[]): ContentBlock[] { + const stripped: ContentBlock[] = [] + for (const block of blocks) { + switch (block.type) { + case 'reasoning': + break + case 'tool-result': + stripped.push({ ...block, content: this._stripReasoning(block.content) }) + break + default: + stripped.push(block) + } + } + return stripped + } + /** * The turn number of the currently OPEN turn — a `turn/start` not yet * followed by its `turn/end` — or `null` if the session has no open turn. diff --git a/packages/compact/compact-basic/src/types.ts b/packages/compact/compact-basic/src/types.ts index 13365b7ed1..8c4753c84f 100644 --- a/packages/compact/compact-basic/src/types.ts +++ b/packages/compact/compact-basic/src/types.ts @@ -19,8 +19,10 @@ export interface BasicCompactConfig { retainTokens?: number /** Model to use for summarization (default '' — uses the agent's model). */ summarizationModel?: string - /** Maximum tokens for the summarization response (default 2048). */ - summarizationMaxTokens?: number + /** Provider generation cap for the summarization call (default 8192). */ + maxTokens?: number + /** Extra compaction attempts when the first compacted surface is still over threshold (default 1). */ + compactionRetries?: number /** Enable automatic compaction on the `agent/pre-step` seam (default true). */ auto?: boolean } @@ -34,40 +36,52 @@ export const DEFAULTS: ResolvedConfig = { thresholdRatio: 0.8, retainTokens: 20480, summarizationModel: '', - summarizationMaxTokens: 2048, + maxTokens: 8192, + compactionRetries: 1, auto: true, } /** - * Apply defaults to a partial config and enforce the approximate convergence - * invariant. + * Apply defaults to a partial config and reject nonsensical numeric knobs. * - * `summarizationMaxTokens + retainTokens` must be strictly BELOW the compaction - * threshold (`contextWindow * thresholdRatio`). The invariant bounds the two - * variable pieces of post-compaction history — the summary and the retained - * recent tail — but it is intentionally approximate: checkpoint framing, - * per-message role overhead, system-prompt size, and the char/4 estimator's - * error can still leave a narrow accepted config near the threshold. The bound - * is strict (`>=` rejects) because `compactIfNeeded` declines only when the - * estimate is `< threshold`: a post-compaction history sitting EXACTLY at the - * threshold would re-trigger on the next check. Pre-release we reject rather - * than clamp: a config that cannot satisfy even this structural bound is a bug - * at the call site, not something to silently paper over. - * - * @throws if `summarizationMaxTokens + retainTokens >= contextWindow * thresholdRatio`. + * Convergence is not a static config invariant: provider generation caps can be + * spent on hidden or surfaced reasoning tokens, and the model may emit a summary + * of unpredictable size. The backend instead enforces convergence dynamically: + * each committed summary must be smaller than the content it shadows, and + * `compactIfNeeded` may re-compact up to `compactionRetries` extra times before + * throwing if the surface still exceeds the threshold. */ export function resolveConfig(config: BasicCompactConfig): ResolvedConfig { const resolved = { ...DEFAULTS, ...config } - const threshold = Math.floor(resolved.contextWindow * resolved.thresholdRatio) - const postCompactionFloor = resolved.summarizationMaxTokens + resolved.retainTokens - if (postCompactionFloor >= threshold) { - throw new Error( - `BasicCompactConfig: summarizationMaxTokens (${resolved.summarizationMaxTokens}) + ` - + `retainTokens (${resolved.retainTokens}) = ${postCompactionFloor} is not below the compaction ` - + `threshold contextWindow * thresholdRatio = ${threshold}; post-compaction history would ` - + 'stay at/over threshold and re-compact endlessly. Lower retainTokens/summarizationMaxTokens ' - + 'or raise contextWindow/thresholdRatio.', - ) + + assertPositiveInteger('contextWindow', resolved.contextWindow) + assertRatio('thresholdRatio', resolved.thresholdRatio) + assertNonNegativeInteger('retainTokens', resolved.retainTokens) + assertPositiveInteger('maxTokens', resolved.maxTokens) + assertNonNegativeInteger('compactionRetries', resolved.compactionRetries) + if (typeof resolved.summarizationModel !== 'string') { + throw new Error('BasicCompactConfig: summarizationModel must be a string.') + } + if (typeof resolved.auto !== 'boolean') { + throw new Error('BasicCompactConfig: auto must be a boolean.') } return resolved } + +function assertPositiveInteger(name: string, value: number): void { + if (!Number.isInteger(value) || value <= 0) { + throw new Error(`BasicCompactConfig: ${name} (${value}) must be a positive integer.`) + } +} + +function assertNonNegativeInteger(name: string, value: number): void { + if (!Number.isInteger(value) || value < 0) { + throw new Error(`BasicCompactConfig: ${name} (${value}) must be a non-negative integer.`) + } +} + +function assertRatio(name: string, value: number): void { + if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0 || value > 1) { + throw new Error(`BasicCompactConfig: ${name} (${value}) must be a number in (0, 1].`) + } +} diff --git a/packages/compact/compact-basic/tests/compact-basic.spec.ts b/packages/compact/compact-basic/tests/compact-basic.spec.ts index fbbb8258c5..73fc6246e6 100644 --- a/packages/compact/compact-basic/tests/compact-basic.spec.ts +++ b/packages/compact/compact-basic/tests/compact-basic.spec.ts @@ -17,14 +17,18 @@ const SIGNAL = new AbortController().signal * predictable token estimate, for deterministic unit tests of the algorithm. */ class TestCompactService extends BasicCompactService { + private readonly summaryOutputs = new WeakSet() /** Track calls to summarize for test assertions. */ summarizeCalls: { text: string; model: string }[] = [] /** The fixed summary to return. */ mockSummary: ContentBlock[] = [{ type: 'text', text: 'Test summary of compacted content.' }] + /** Per-call summaries; when set, each summarize() call shifts one value. */ + mockSummaryQueue: ContentBlock[][] = [] /** If set, summarize() throws this error. */ summarizeError: Error | null = null override estimateContentTokens(blocks: readonly ContentBlock[]): number { + if (this.summaryOutputs.has(blocks)) return blocks.length * 2 // 10 tokens per block — predictable for retention/threshold math. return blocks.length * 10 } @@ -33,18 +37,15 @@ class TestCompactService extends BasicCompactService { const model = this.config.summarizationModel || agent.options.model || '' this.summarizeCalls.push({ text, model }) if (this.summarizeError) throw this.summarizeError - return this.mockSummary + const summary = this.mockSummaryQueue.shift() ?? this.mockSummary + this.summaryOutputs.add(summary) + return summary } } -/** - * Create a test service with a throwaway context (auto disabled — no model). - * A small `summarizationMaxTokens` baseline keeps the convergence invariant - * (`summarizationMaxTokens + retainTokens <= contextWindow * thresholdRatio`) - * satisfied for the tiny windows these tests use; a test may override it. - */ +/** Create a test service with a throwaway context (auto disabled — no model). */ function createTestService(config: BasicCompactConfig = {}): TestCompactService { - return new TestCompactService(new Context(), { auto: false, summarizationMaxTokens: 1, ...config }) + return new TestCompactService(new Context(), { auto: false, ...config }) } /** @@ -182,7 +183,7 @@ describe('BasicCompactService step-alignment (never split a tool-call/result pai // region always ends on a step boundary, so no step's tool-call is split // from its result. retainTokens=55 keeps the recent tail; the older steps // compact intact. - const svc = createTestService({ contextWindow: 200, thresholdRatio: 0.5, retainTokens: 55 }) + const svc = createTestService({ contextWindow: 280, thresholdRatio: 0.5, retainTokens: 55 }) const session = toolTurnSession(3) const result = await compactIfNeeded(svc, session, '', 'm', SIGNAL) @@ -520,7 +521,7 @@ describe('BasicCompactService.compactIfNeeded', () => { }) it('walks tail→head and retains nodes within token budget', async () => { - const svc = createTestService({ contextWindow: 100, thresholdRatio: 0.2, retainTokens: 15 }) + const svc = createTestService({ contextWindow: 350, thresholdRatio: 0.2, retainTokens: 15 }) const session = multiTurnSession(5, 1) // 10 surface nodes = ~100 tokens const result = await compactIfNeeded(svc, session, '', 'm', SIGNAL) @@ -531,13 +532,12 @@ describe('BasicCompactService.compactIfNeeded', () => { }) it('returns null when the whole surface fits the retain budget (over threshold by role/system overhead)', async () => { - // threshold = floor(470*0.1) = 47. The 4 surface nodes weigh 10 each (raw 40 + // threshold = floor(480*0.1) = 48. The 4 surface nodes weigh 10 each (raw 40 // for the retention walk), but the derived estimate adds 4 role tokens per - // message → 56 ≥ 47, so the threshold check passes and the walk runs. The + // message → 56 ≥ 48, so the threshold check passes and the walk runs. The // walk accumulates all 40 < retainTokens (45) without crossing the budget, - // so keepFromIdx reaches 0 and compaction declines. The invariant holds: - // summarizationMaxTokens (1) + retainTokens (45) = 46 < threshold 47. - const svc = createTestService({ contextWindow: 470, thresholdRatio: 0.1, retainTokens: 45 }) + // so keepFromIdx reaches 0 and compaction declines. + const svc = createTestService({ contextWindow: 480, thresholdRatio: 0.1, retainTokens: 45 }) const session = multiTurnSession(2, 1) expect(await compactIfNeeded(svc, session, '', 'm', SIGNAL)).toBeNull() }) @@ -553,7 +553,7 @@ describe('BasicCompactService.compactIfNeeded', () => { // verbatim (protectedIdx = first open-turn node = 0), so compactIfNeeded // returned null and shadowedSeqs would be empty — the runaway turn could // never compact and the next model call would overflow the window. - const svc = createTestService({ contextWindow: 300, thresholdRatio: 0.1, retainTokens: 25 }) + const svc = createTestService({ contextWindow: 800, thresholdRatio: 0.1, retainTokens: 25 }) const s = new Session(SessionId('runaway')) // ONE open turn with 5 closed steps; each step is [asst(tool-call), result]. s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) @@ -598,7 +598,7 @@ describe('BasicCompactService.compactIfNeeded', () => { // never stranded. retainTokens=25 leaves a couple of retained nodes after // the first compaction (so the surface is [summary, …retained], not just // [summary]). - const svc = createTestService({ contextWindow: 300, thresholdRatio: 0.1, retainTokens: 25 }) + const svc = createTestService({ contextWindow: 800, thresholdRatio: 0.1, retainTokens: 25 }) const s = multiTurnSession(4, 1) // turns 1-4 closed, turn 5 open (no surface yet) const first = await compactIfNeeded(svc, s, '', 'm', SIGNAL) @@ -623,6 +623,45 @@ describe('BasicCompactService.compactIfNeeded', () => { const turn5UserSeq = s.events.find(e => e.type === 'user/message' && e.data.content.some(b => b.type === 'text' && b.text === 'turn 5 work'))!.seq expect(second!.shadowedSeqs).not.toContain(turn5UserSeq) }) + + it('re-compacts smaller summaries until the post-compaction surface drops below threshold', async () => { + const svc = createTestService({ + contextWindow: 100, + thresholdRatio: 0.5, + retainTokens: 10, + compactionRetries: 2, + }) + svc.mockSummaryQueue = [ + Array.from({ length: 4 }, (_, index) => ({ type: 'text', text: `first ${index}` })), + [{ type: 'text', text: 'second' }], + ] + const session = multiTurnSession(4, 1) + + const result = await compactIfNeeded(svc, session, '', 'm', SIGNAL) + + expect(result).not.toBeNull() + expect(svc.summarizeCalls).toHaveLength(2) + expect(session.events.filter(e => e.type === 'compact/summary')).toHaveLength(2) + expect(svc.estimateTokens(session.deriveMessages(), '')).toBeLessThan(50) + }) + + it('throws after the configured re-compaction attempts still leave the surface above threshold', async () => { + const svc = createTestService({ + contextWindow: 100, + thresholdRatio: 0.5, + retainTokens: 10, + compactionRetries: 1, + }) + svc.mockSummaryQueue = [ + Array.from({ length: 4 }, (_, index) => ({ type: 'text', text: `first ${index}` })), + Array.from({ length: 3 }, (_, index) => ({ type: 'text', text: `second ${index}` })), + ] + const session = multiTurnSession(4, 1) + + await expect(compactIfNeeded(svc, session, '', 'm', SIGNAL)) + .rejects.toThrow(/still above threshold after 2 compaction attempts/) + expect(svc.summarizeCalls).toHaveLength(2) + }) }) describe('BasicCompactService replay equivalence', () => { @@ -753,31 +792,31 @@ describe('BasicCompactService HMR safety', () => { }) }) -describe('BasicCompactService convergence invariant (config)', () => { - it('throws when summarizationMaxTokens + retainTokens exceeds the threshold', () => { - // threshold = floor(1000 * 0.5) = 500; 200 + 400 = 600 is not below 500 → reject. - expect(() => new BasicCompactService(new Context(), { - auto: false, contextWindow: 1000, thresholdRatio: 0.5, retainTokens: 400, summarizationMaxTokens: 200, - })).toThrow(/not below the compaction threshold/) +describe('BasicCompactService config validation', () => { + it('rejects invalid numeric config values', () => { + expect(() => new BasicCompactService(new Context(), { auto: false, contextWindow: 0 })).toThrow(/contextWindow .* positive integer/) + expect(() => new BasicCompactService(new Context(), { auto: false, thresholdRatio: 0 })).toThrow(/thresholdRatio .* \(0, 1\]/) + expect(() => new BasicCompactService(new Context(), { auto: false, thresholdRatio: 1.1 })).toThrow(/thresholdRatio .* \(0, 1\]/) + expect(() => new BasicCompactService(new Context(), { auto: false, retainTokens: -1 })).toThrow(/retainTokens .* non-negative integer/) + expect(() => new BasicCompactService(new Context(), { auto: false, maxTokens: 0 })).toThrow(/maxTokens .* positive integer/) + expect(() => new BasicCompactService(new Context(), { auto: false, compactionRetries: -1 })) + .toThrow(/compactionRetries .* non-negative integer/) + expect(() => new BasicCompactService(new Context(), { auto: false, summarizationModel: 1 } as unknown as BasicCompactConfig)) + .toThrow(/summarizationModel must be a string/) + expect(() => new BasicCompactService(new Context(), { auto: 'no' } as unknown as BasicCompactConfig)) + .toThrow(/auto must be a boolean/) }) - it('rejects the boundary case (sum equals the threshold — would re-trigger)', () => { - // threshold = floor(1000 * 0.5) = 500; 100 + 400 = 500 is NOT below 500, so - // post-compaction history would sit exactly at threshold and re-compact. + it('accepts a large retain budget because convergence is enforced dynamically', () => { expect(() => new BasicCompactService(new Context(), { - auto: false, contextWindow: 1000, thresholdRatio: 0.5, retainTokens: 400, summarizationMaxTokens: 100, - })).toThrow(/not below the compaction threshold/) - }) - - it('accepts the case just below the threshold', () => { - // threshold = floor(1000 * 0.5) = 500; 99 + 400 = 499 < 500 → allowed. - expect(() => new BasicCompactService(new Context(), { - auto: false, contextWindow: 1000, thresholdRatio: 0.5, retainTokens: 400, summarizationMaxTokens: 99, + auto: false, + contextWindow: 1000, + thresholdRatio: 0.5, + retainTokens: 900, })).not.toThrow() }) - it('the default config satisfies the invariant', () => { - // 2048 + 20480 = 22528 ≤ floor(128000 * 0.8) = 102400. + it('the default config is valid', () => { expect(() => new BasicCompactService(new Context(), { auto: false })).not.toThrow() }) }) @@ -797,6 +836,41 @@ class ScriptedAdapter extends LlmAdapter { } } +/** An adapter that emits arbitrary content blocks, preserving reasoning/text shape. */ +class BlocksAdapter extends LlmAdapter { + lastOptions: GenerateOptions | null = null + constructor(private blocks: readonly ContentBlock[]) { + super() + } + + async * stream(options: GenerateOptions): AsyncIterable { + this.lastOptions = options + for (const [index, block] of this.blocks.entries()) { + yield { type: 'block-start', index, blockType: block.type } + switch (block.type) { + case 'text': + yield { type: 'text-delta', index, text: block.text } + break + case 'reasoning': + yield { type: 'reasoning-delta', index, text: block.text } + break + default: + yield { type: 'block-end', index, block } + } + } + yield { type: 'finish', reason: { kind: 'stop' } } + } +} + +/** Wire a real LlmService + arbitrary-block adapter into a context. */ +async function ctxWithBlocks(blocks: readonly ContentBlock[], model = 'test-model'): Promise<{ ctx: Context; adapter: BlocksAdapter }> { + const ctx = new Context() + await ctx.plugin(LlmService) + const adapter = new BlocksAdapter(blocks) + ctx.llm.registerAdapter([model], adapter) + return { ctx, adapter } +} + /** Wire a real LlmService + scripted adapter into a context. */ async function ctxWithModel(summaryText: string, model = 'test-model'): Promise<{ ctx: Context; adapter: ScriptedAdapter }> { const ctx = new Context() @@ -858,7 +932,7 @@ function summarize(svc: BasicCompactService, text: string, model: string) { describe('BasicCompactService.summarize (real ctx.llm.stream)', () => { it('summarizes via the registered adapter and returns its content', async () => { const { ctx, adapter } = await ctxWithModel('SUMMARY TEXT') - const svc = new BasicCompactService(ctx, { auto: false, summarizationMaxTokens: 512 }) + const svc = new BasicCompactService(ctx, { auto: false, maxTokens: 512 }) const summary = await summarize(svc, 'User: hi\n\nAssistant: hello', 'test-model') expect(summary).toEqual([{ type: 'text', text: 'SUMMARY TEXT' }]) @@ -869,6 +943,37 @@ describe('BasicCompactService.summarize (real ctx.llm.stream)', () => { expect(adapter.lastOptions!.messages[0]!.content[0]).toMatchObject({ type: 'text' }) }) + it('uses maxTokens as the summarization provider cap', async () => { + const { ctx, adapter } = await ctxWithModel('SUMMARY TEXT') + const svc = new BasicCompactService(ctx, { + auto: false, + maxTokens: 50, + }) + + await summarize(svc, 'User: hi', 'test-model') + + expect(adapter.lastOptions!.maxTokens).toBe(50) + }) + + it('strips reasoning blocks from the stored summary', async () => { + const { ctx } = await ctxWithBlocks([ + { type: 'reasoning', text: 'private chain of thought' }, + { type: 'text', text: 'PUBLIC SUMMARY' }, + ]) + const svc = new BasicCompactService(ctx, { auto: false }) + + const summary = await summarize(svc, 'User: hi', 'test-model') + + expect(summary).toEqual([{ type: 'text', text: 'PUBLIC SUMMARY' }]) + }) + + it('throws when stripping reasoning leaves no summary text', async () => { + const { ctx } = await ctxWithBlocks([{ type: 'reasoning', text: 'private only' }]) + const svc = new BasicCompactService(ctx, { auto: false }) + + await expect(summarize(svc, 'User: hi', 'test-model')).rejects.toThrow(/no non-reasoning summary content/) + }) + it('throws when no model is provided', async () => { const { ctx } = await ctxWithModel('x') const svc = new BasicCompactService(ctx, { auto: false }) @@ -930,6 +1035,17 @@ describe('BasicCompactService.summarize (real ctx.llm.stream)', () => { // The raw summary is wrapped in the checkpoint framing on the surface. expect(session.deriveMessages()[0]!.content).toContainEqual({ type: 'text', text: 'CONDENSED' }) }) + + it('rejects a summary that is not smaller than the shadowed content', async () => { + const svc = createTestService({ auto: false }) + const session = multiTurnSession(2, 1) + const nodes = session.surface.nodes + svc.mockSummary = Array.from({ length: 20 }, (_, index) => ({ type: 'text', text: `large ${index}` })) + + await expect(compactRegion(svc, session, nodes[0]!.seq, nodes[1]!.seq, 'm')) + .rejects.toThrow(/summary is not smaller than the shadowed content/) + expect(session.events.some(e => e.type === 'compact/summary')).toBe(false) + }) }) describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => { @@ -940,7 +1056,7 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => it('compacts (mutating the surface) when over threshold', async () => { const { ctx } = await ctxWithModel('SUMMARY') - void new BasicCompactService(ctx, { contextWindow: 200, thresholdRatio: 0.5, retainTokens: 20, summarizationMaxTokens: 50 }) + void new BasicCompactService(ctx, { contextWindow: 200, thresholdRatio: 0.5, retainTokens: 20 }) const session = multiTurnSession(5, 1) // 10 surface nodes const agent = stubAgent(session, 'test-model') const before = session.surface.nodes.length @@ -956,7 +1072,7 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => it('compacts mid-turn on steps after the first (the surface grows within a turn)', async () => { const { ctx } = await ctxWithModel('SUMMARY') - void new BasicCompactService(ctx, { contextWindow: 100, thresholdRatio: 0.5, retainTokens: 10, summarizationMaxTokens: 30 }) + void new BasicCompactService(ctx, { contextWindow: 100, thresholdRatio: 0.5, retainTokens: 10 }) const session = multiTurnSession(3, 1) // over the 0.5 threshold const agent = stubAgent(session, 'test-model') @@ -981,7 +1097,7 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => // surface is untouched (the loop derives the full history). const ctx = new Context() await ctx.plugin(LlmService) - void new BasicCompactService(ctx, { contextWindow: 300, thresholdRatio: 0.1, retainTokens: 10, summarizationMaxTokens: 1 }) + void new BasicCompactService(ctx, { contextWindow: 300, thresholdRatio: 0.1, retainTokens: 10 }) const session = multiTurnSession(3, 1) const agent = stubAgent(session, 'missing-model') const before = session.surface.nodes.length @@ -994,7 +1110,7 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => it('does not register the listener when auto is false', async () => { const { ctx } = await ctxWithModel('SUMMARY') - void new BasicCompactService(ctx, { auto: false, contextWindow: 100, thresholdRatio: 0.1, retainTokens: 5, summarizationMaxTokens: 1 }) + void new BasicCompactService(ctx, { auto: false, contextWindow: 100, thresholdRatio: 0.1, retainTokens: 5 }) const session = multiTurnSession(3, 1) const agent = stubAgent(session, 'test-model') @@ -1008,7 +1124,7 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => options.model = 'routed-model' return next() }) - void new BasicCompactService(ctx, { contextWindow: 200, thresholdRatio: 0.5, retainTokens: 20, summarizationMaxTokens: 50 }) + void new BasicCompactService(ctx, { contextWindow: 200, thresholdRatio: 0.5, retainTokens: 20 }) const session = multiTurnSession(5, 1) const agent = stubAgent(session) @@ -1025,7 +1141,6 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => contextWindow: 200, thresholdRatio: 0.5, retainTokens: 20, - summarizationMaxTokens: 50, }) const session = multiTurnSession(5, 1) const agent = stubAgent(session, 'test-model') @@ -1139,14 +1254,16 @@ describe('BasicCompactService edge cases', () => { expect(svc.estimateContentTokens([unknown])).toBeGreaterThan(0) }) - it('compacts once without re-checking a post-compaction threshold', async () => { + it('auto-compaction reports bounded retry exhaustion after committing a smaller summary', async () => { const { ctx } = await ctxWithModel('SUMMARY') - // Even with a window so tiny the post-compaction history still exceeds the - // threshold, the agnostic listener does NOT re-gate or warn — it compacts - // once (the single check lives in compactIfNeeded) and proceeds. const warnings: string[] = [] ctx.logger.warn = ((msg: string) => void warnings.push(msg)) as typeof ctx.logger.warn - void new BasicCompactService(ctx, { contextWindow: 300, thresholdRatio: 0.1, retainTokens: 5, summarizationMaxTokens: 5 }) + void new BasicCompactService(ctx, { + contextWindow: 300, + thresholdRatio: 0.1, + retainTokens: 5, + compactionRetries: 0, + }) const session = multiTurnSession(4, 1) const agent = stubAgent(session, 'test-model') @@ -1154,8 +1271,7 @@ describe('BasicCompactService edge cases', () => { expect(session.events.some(e => e.type === 'compact/summary')).toBe(true) // The surface was mutated; the head message is the framed summary checkpoint. expect(session.deriveMessages()[0]!.content).toContainEqual({ type: 'text', text: 'SUMMARY' }) - // No cascade warning is emitted. - expect(warnings.length).toBe(0) + expect(warnings.some(w => w.includes('still above threshold after 1 compaction attempts'))).toBe(true) }) it('rejects compaction when no turn is open (compaction events must be turn-enclosed)', async () => { @@ -1225,7 +1341,7 @@ describe('BasicCompactService edge cases', () => { const { ctx } = await ctxWithModel('SUMMARY') const warnings: string[] = [] ctx.logger.warn = ((msg: string) => void warnings.push(msg)) as typeof ctx.logger.warn - const svc = new TestCompactService(ctx, { contextWindow: 300, thresholdRatio: 0.1, retainTokens: 10, summarizationMaxTokens: 10 }) + const svc = new TestCompactService(ctx, { contextWindow: 300, thresholdRatio: 0.1, retainTokens: 10 }) svc.summarizeError = 'boom' as unknown as Error const session = multiTurnSession(3, 1) const agent = stubAgent(session, 'test-model') @@ -1243,7 +1359,7 @@ describe('BasicCompactService edge cases', () => { // A large system prompt pushes the listener's estimate over threshold, but // retainTokens is huge so compactIfNeeded walks everything and returns null. // threshold = floor(2000*0.1) = 200; invariant: 5 + 150 = 155 ≤ 200. - const svc = new TestCompactService(ctx, { contextWindow: 2000, thresholdRatio: 0.1, retainTokens: 150, summarizationMaxTokens: 5 }) + const svc = new TestCompactService(ctx, { contextWindow: 2000, thresholdRatio: 0.1, retainTokens: 150 }) const session = multiTurnSession(2, 1) const agent = stubAgent(session, 'test-model') const bigSystem = 'x'.repeat(900) // ceil(900/4)=225 > threshold 200 diff --git a/packages/compact/compact-basic/tests/compact-loop-repro.spec.ts b/packages/compact/compact-basic/tests/compact-loop-repro.spec.ts index e12861a43a..dddb636b21 100644 --- a/packages/compact/compact-basic/tests/compact-loop-repro.spec.ts +++ b/packages/compact/compact-basic/tests/compact-loop-repro.spec.ts @@ -91,14 +91,12 @@ async function harness(toolSteps: number): Promise<{ ctx: Context; compact: Repr }, })) // Tiny window so a couple of tool steps cross the threshold and compaction - // fires within the runaway turn. Convergence invariant holds: - // summarizationMaxTokens(1) + retainTokens(20) = 21 <= floor(60*0.5) = 30. + // fires within the runaway turn. const compact = new ReproCompactService(ctx, { auto: true, - contextWindow: 60, + contextWindow: 64, thresholdRatio: 0.5, retainTokens: 20, - summarizationMaxTokens: 1, }) return { ctx, compact } }