From 6cac3e6476d8490d5e9842330b66048c72a2e552 Mon Sep 17 00:00:00 2001 From: Hypatia May Date: Fri, 26 Jun 2026 13:51:20 +0800 Subject: [PATCH] fix(compact): reject threshold-equality config to keep compaction convergent (CBR-002) Codex round 1 CBR-002: `resolveConfig` rejected only `summarizationMaxTokens + retainTokens > threshold` (allowing equality), but `compactIfNeeded` declines only when the estimate is `< threshold`. At exact equality the post-compaction history sits at the threshold and re-triggers on the very next check. Make the bound strict (`>=` rejects), so post-compaction history is guaranteed strictly below the threshold. Updated the boundary test (the sum-equals-threshold case is now rejected, not accepted) and added an "accepts just below the threshold" case; nudged one unrelated config that incidentally sat at the equality boundary. --- packages/compact/compact-basic/src/types.ts | 28 ++++++++++--------- .../compact-basic/tests/compact-basic.spec.ts | 24 ++++++++++------ 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/packages/compact/compact-basic/src/types.ts b/packages/compact/compact-basic/src/types.ts index 7273150d8e..b7261eb093 100644 --- a/packages/compact/compact-basic/src/types.ts +++ b/packages/compact/compact-basic/src/types.ts @@ -21,7 +21,7 @@ export interface BasicCompactConfig { summarizationModel?: string /** Maximum tokens for the summarization response (default 2048). */ summarizationMaxTokens?: number - /** Enable automatic compaction on the `agent/request` waterfall (default true). */ + /** Enable automatic compaction on the `agent/pre-step` seam (default true). */ auto?: boolean } @@ -42,29 +42,31 @@ export const DEFAULTS: ResolvedConfig = { * Apply defaults to a partial config and enforce the single-pass convergence * invariant. * - * `summarizationMaxTokens + retainTokens` must not exceed the compaction + * `summarizationMaxTokens + retainTokens` must be strictly BELOW the compaction * threshold (`contextWindow * thresholdRatio`). The invariant guarantees that * after a compaction the derived history — the (bounded) summary plus the - * retained recent tail — is structurally BELOW the threshold, so the very next - * pre-request check passes and a second compaction cannot fire on the same - * content. Without it, a too-large summary budget or retain budget would leave - * the post-compaction history still over threshold, triggering compaction again - * and again. Pre-release we reject rather than clamp: a config that cannot - * guarantee convergence is a bug at the call site, not something to silently - * paper over. + * retained recent tail — is structurally below the threshold, so the very next + * pre-step check passes and a second compaction cannot fire on the same + * content. 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. Without the + * invariant, a too-large summary or retain budget would leave the + * post-compaction history at/over threshold, triggering compaction again and + * again. Pre-release we reject rather than clamp: a config that cannot guarantee + * convergence is a bug at the call site, not something to silently paper over. * - * @throws if `summarizationMaxTokens + retainTokens > contextWindow * thresholdRatio`. + * @throws if `summarizationMaxTokens + retainTokens >= contextWindow * thresholdRatio`. */ 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) { + if (postCompactionFloor >= threshold) { throw new Error( `BasicCompactConfig: summarizationMaxTokens (${resolved.summarizationMaxTokens}) + ` - + `retainTokens (${resolved.retainTokens}) = ${postCompactionFloor} exceeds the compaction ` + + `retainTokens (${resolved.retainTokens}) = ${postCompactionFloor} is not below the compaction ` + `threshold contextWindow * thresholdRatio = ${threshold}; post-compaction history would ` - + 'stay over threshold and re-compact endlessly. Lower retainTokens/summarizationMaxTokens ' + + 'stay at/over threshold and re-compact endlessly. Lower retainTokens/summarizationMaxTokens ' + 'or raise contextWindow/thresholdRatio.', ) } diff --git a/packages/compact/compact-basic/tests/compact-basic.spec.ts b/packages/compact/compact-basic/tests/compact-basic.spec.ts index 4c91f11173..3a6d9a0360 100644 --- a/packages/compact/compact-basic/tests/compact-basic.spec.ts +++ b/packages/compact/compact-basic/tests/compact-basic.spec.ts @@ -530,13 +530,13 @@ describe('BasicCompactService.compactIfNeeded', () => { }) it('returns null when the whole surface fits the retain budget (over threshold by role/system overhead)', async () => { - // threshold = floor(460*0.1) = 46. The 4 surface nodes weigh 10 each (raw 40 + // threshold = floor(470*0.1) = 47. The 4 surface nodes weigh 10 each (raw 40 // for the retention walk), but the derived estimate adds 4 role tokens per - // message → 56 ≥ 46, so the threshold check passes and the walk runs. The + // message → 56 ≥ 47, 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 46. - const svc = createTestService({ contextWindow: 460, thresholdRatio: 0.1, retainTokens: 45 }) + // summarizationMaxTokens (1) + retainTokens (45) = 46 < threshold 47. + const svc = createTestService({ contextWindow: 470, thresholdRatio: 0.1, retainTokens: 45 }) const session = multiTurnSession(2, 1) expect(await svc.compactIfNeeded(session, '', 'm', SIGNAL)).toBeNull() }) @@ -739,16 +739,24 @@ 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 > 500 → reject. + // 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(/exceeds the compaction threshold/) + })).toThrow(/not below the compaction threshold/) }) - it('accepts the boundary case (sum equals the threshold)', () => { - // threshold = floor(1000 * 0.5) = 500; 100 + 400 = 500 ≤ 500 → allowed. + 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. 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, })).not.toThrow() })