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.
2026-08-15 21:04:50 +00:00 · 2026-06-29 16:56:44 +08:00
parent 1f35a4446d
commit 1808570933
8 changed files with 319 additions and 149 deletions
--- 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<CompactionResult | null> {
    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.
--- 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].`)
+  }
+}