From 170643ec9b9daea0311f1a2c3c20a16c46f65f72 Mon Sep 17 00:00:00 2001 From: Hypatia May Date: Mon, 29 Jun 2026 18:04:59 +0800 Subject: [PATCH] test(compact-basic): restore coverage gate --- packages/compact/compact-basic/README.md | 2 +- packages/compact/compact-basic/src/index.ts | 34 +++++++------- .../compact-basic/tests/compact-basic.spec.ts | 47 +++++++++++++++++-- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/packages/compact/compact-basic/README.md b/packages/compact/compact-basic/README.md index 5d4f2c755e..6e8eb0765d 100644 --- a/packages/compact/compact-basic/README.md +++ b/packages/compact/compact-basic/README.md @@ -11,7 +11,7 @@ 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. - **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. +- **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; only text blocks from the model's reply are kept before the checkpoint is stored (reasoning is dropped so private chain-of-thought never leaks into the durable summary, and a stray `tool-call` is dropped so the synthesized `user/message` summary cannot land an orphaned call with no matching `tool-result`). 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()`). diff --git a/packages/compact/compact-basic/src/index.ts b/packages/compact/compact-basic/src/index.ts index 7d84b52511..9d0849b19a 100644 --- a/packages/compact/compact-basic/src/index.ts +++ b/packages/compact/compact-basic/src/index.ts @@ -306,9 +306,9 @@ export class BasicCompactService extends CompactService { const error = finishError(assembler.finish) if (error) throw error - const summary = this._stripReasoning(assembler.message().content) + const summary = this._textOnly(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') + throw new Error('summarization produced no text summary content') } return summary @@ -358,7 +358,9 @@ export class BasicCompactService extends CompactService { const range = this._compactableRange(session) if (range === null) { + /* v8 ignore else -- defensive for non-standard subclass mutations; the concrete replace keeps a compactable head checkpoint. */ if (result === null) return null + /* v8 ignore next -- paired with the ignored defensive branch above. */ break } @@ -605,21 +607,19 @@ export class BasicCompactService extends CompactService { 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 + /** + * Keep ONLY text blocks from the model-produced summary before storing it. + * + * The summary lands on the surface as a synthesized `user/message` (see + * {@link _frameSummary}), so the only block type that is both useful and safe + * there is `text`. A model assistant message can otherwise carry `reasoning` + * (private chain-of-thought, must not leak into the durable checkpoint) and + * `tool-call` blocks — and a surviving `tool-call` in a user message would be + * an orphaned call with no matching `tool-result`, exactly the tool-pairing + * breakage compaction works to avoid. Filtering to text drops both. + */ + private _textOnly(blocks: readonly ContentBlock[]): ContentBlock[] { + return blocks.filter((block): block is Extract => block.type === 'text') } /** diff --git a/packages/compact/compact-basic/tests/compact-basic.spec.ts b/packages/compact/compact-basic/tests/compact-basic.spec.ts index 73fc6246e6..906841c39f 100644 --- a/packages/compact/compact-basic/tests/compact-basic.spec.ts +++ b/packages/compact/compact-basic/tests/compact-basic.spec.ts @@ -520,6 +520,24 @@ describe('BasicCompactService.compactIfNeeded', () => { expect(result!.shadowedSeqs.length).toBeGreaterThan(0) }) + it('returns the first compaction result when a zero-retry pass converges after the loop', async () => { + // With compactionRetries=0 there is no next-loop threshold check after the + // first mutation, so the success path is the post-loop `return result`. + const svc = createTestService({ + contextWindow: 100, + thresholdRatio: 0.7, + retainTokens: 10, + compactionRetries: 0, + }) + const session = multiTurnSession(3, 1) // 6 derived messages = 84 estimated tokens. + + const result = await compactIfNeeded(svc, session, '', 'm', SIGNAL) + + expect(result).not.toBeNull() + expect(session.events.filter(e => e.type === 'compact/summary')).toHaveLength(1) + expect(svc.estimateTokens(session.deriveMessages(), '')).toBeLessThan(70) + }) + it('walks tail→head and retains nodes within token budget', async () => { const svc = createTestService({ contextWindow: 350, thresholdRatio: 0.2, retainTokens: 15 }) const session = multiTurnSession(5, 1) // 10 surface nodes = ~100 tokens @@ -955,10 +973,13 @@ describe('BasicCompactService.summarize (real ctx.llm.stream)', () => { expect(adapter.lastOptions!.maxTokens).toBe(50) }) - it('strips reasoning blocks from the stored summary', async () => { + it('keeps only text blocks in the stored summary (drops reasoning and tool-call)', async () => { const { ctx } = await ctxWithBlocks([ { type: 'reasoning', text: 'private chain of thought' }, { type: 'text', text: 'PUBLIC SUMMARY' }, + // A model reply can carry a tool-call; it must not survive into the + // synthesized user/message summary as an orphaned call. + { type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }, ]) const svc = new BasicCompactService(ctx, { auto: false }) @@ -967,11 +988,11 @@ describe('BasicCompactService.summarize (real ctx.llm.stream)', () => { expect(summary).toEqual([{ type: 'text', text: 'PUBLIC SUMMARY' }]) }) - it('throws when stripping reasoning leaves no summary text', async () => { + it('throws when no text block remains after filtering', 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/) + await expect(summarize(svc, 'User: hi', 'test-model')).rejects.toThrow(/no text summary content/) }) it('throws when no model is provided', async () => { @@ -1070,6 +1091,26 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => expect(session.deriveMessages()[0]!.content).toContainEqual({ type: 'text', text: 'SUMMARY' }) }) + it('logs compaction details when auto-compaction returns a converged result', async () => { + const ctx = new Context() + const infos: string[] = [] + ctx.logger.info = ((msg: string) => void infos.push(msg)) as typeof ctx.logger.info + void new TestCompactService(ctx, { + contextWindow: 100, + thresholdRatio: 0.7, + retainTokens: 10, + compactionRetries: 0, + }) + const session = multiTurnSession(3, 1) + const agent = stubAgent(session, 'test-model') + + await firePreStep(ctx, agent, 1, '') + + expect(session.events.filter(e => e.type === 'compact/summary')).toHaveLength(1) + expect(infos.some(msg => msg.includes('compaction: shadowed'))).toBe(true) + expect(infos.some(msg => msg.includes('estimated tokens after compaction'))).toBe(true) + }) + 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 })