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.
This commit is contained in:
@@ -21,7 +21,7 @@ export interface BasicCompactConfig {
|
|||||||
summarizationModel?: string
|
summarizationModel?: string
|
||||||
/** Maximum tokens for the summarization response (default 2048). */
|
/** Maximum tokens for the summarization response (default 2048). */
|
||||||
summarizationMaxTokens?: number
|
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
|
auto?: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -42,29 +42,31 @@ export const DEFAULTS: ResolvedConfig = {
|
|||||||
* Apply defaults to a partial config and enforce the single-pass convergence
|
* Apply defaults to a partial config and enforce the single-pass convergence
|
||||||
* invariant.
|
* invariant.
|
||||||
*
|
*
|
||||||
* `summarizationMaxTokens + retainTokens` must not exceed the compaction
|
* `summarizationMaxTokens + retainTokens` must be strictly BELOW the compaction
|
||||||
* threshold (`contextWindow * thresholdRatio`). The invariant guarantees that
|
* threshold (`contextWindow * thresholdRatio`). The invariant guarantees that
|
||||||
* after a compaction the derived history — the (bounded) summary plus the
|
* after a compaction the derived history — the (bounded) summary plus the
|
||||||
* retained recent tail — is structurally BELOW the threshold, so the very next
|
* 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
|
* pre-step check passes and a second compaction cannot fire on the same
|
||||||
* content. Without it, a too-large summary budget or retain budget would leave
|
* content. The bound is strict (`>=` rejects) because `compactIfNeeded` declines
|
||||||
* the post-compaction history still over threshold, triggering compaction again
|
* only when the estimate is `< threshold`: a post-compaction history sitting
|
||||||
* and again. Pre-release we reject rather than clamp: a config that cannot
|
* EXACTLY at the threshold would re-trigger on the next check. Without the
|
||||||
* guarantee convergence is a bug at the call site, not something to silently
|
* invariant, a too-large summary or retain budget would leave the
|
||||||
* paper over.
|
* 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 {
|
export function resolveConfig(config: BasicCompactConfig): ResolvedConfig {
|
||||||
const resolved = { ...DEFAULTS, ...config }
|
const resolved = { ...DEFAULTS, ...config }
|
||||||
const threshold = Math.floor(resolved.contextWindow * resolved.thresholdRatio)
|
const threshold = Math.floor(resolved.contextWindow * resolved.thresholdRatio)
|
||||||
const postCompactionFloor = resolved.summarizationMaxTokens + resolved.retainTokens
|
const postCompactionFloor = resolved.summarizationMaxTokens + resolved.retainTokens
|
||||||
if (postCompactionFloor > threshold) {
|
if (postCompactionFloor >= threshold) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`BasicCompactConfig: summarizationMaxTokens (${resolved.summarizationMaxTokens}) + `
|
`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 `
|
+ `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.',
|
+ 'or raise contextWindow/thresholdRatio.',
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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 () => {
|
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
|
// 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,
|
// walk accumulates all 40 < retainTokens (45) without crossing the budget,
|
||||||
// so keepFromIdx reaches 0 and compaction declines. The invariant holds:
|
// so keepFromIdx reaches 0 and compaction declines. The invariant holds:
|
||||||
// summarizationMaxTokens (1) + retainTokens (45) = 46 ≤ threshold 46.
|
// summarizationMaxTokens (1) + retainTokens (45) = 46 < threshold 47.
|
||||||
const svc = createTestService({ contextWindow: 460, thresholdRatio: 0.1, retainTokens: 45 })
|
const svc = createTestService({ contextWindow: 470, thresholdRatio: 0.1, retainTokens: 45 })
|
||||||
const session = multiTurnSession(2, 1)
|
const session = multiTurnSession(2, 1)
|
||||||
expect(await svc.compactIfNeeded(session, '', 'm', SIGNAL)).toBeNull()
|
expect(await svc.compactIfNeeded(session, '', 'm', SIGNAL)).toBeNull()
|
||||||
})
|
})
|
||||||
@@ -739,16 +739,24 @@ describe('BasicCompactService HMR safety', () => {
|
|||||||
|
|
||||||
describe('BasicCompactService convergence invariant (config)', () => {
|
describe('BasicCompactService convergence invariant (config)', () => {
|
||||||
it('throws when summarizationMaxTokens + retainTokens exceeds the threshold', () => {
|
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(), {
|
expect(() => new BasicCompactService(new Context(), {
|
||||||
auto: false, contextWindow: 1000, thresholdRatio: 0.5, retainTokens: 400, summarizationMaxTokens: 200,
|
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)', () => {
|
it('rejects the boundary case (sum equals the threshold — would re-trigger)', () => {
|
||||||
// threshold = floor(1000 * 0.5) = 500; 100 + 400 = 500 ≤ 500 → allowed.
|
// 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(), {
|
expect(() => new BasicCompactService(new Context(), {
|
||||||
auto: false, contextWindow: 1000, thresholdRatio: 0.5, retainTokens: 400, summarizationMaxTokens: 100,
|
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()
|
})).not.toThrow()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user