fix(compact): count the logged session prefix toward token pressure
ds-review-bot critical: compactIfNeeded estimated pressure from the derived history + system prompt only, but every loop-built request also carries EpochHeader.messagePrefix in front of the history — a deployment at the window edge would under-estimate by exactly the prefix, skip compaction, and ship an over-window request. BasicCompactService now gates on estimatePressure(): the session prefix read from the log's folded header + the derived history + the system prompt. The fold is exact from the instance's second request on (and from a resumed instance's first — the previous instance logged its prefix); it is absent only before a fresh session's first request, where the history is a single prompt and compaction is moot. Compaction itself still shrinks history only — a prefix that alone approaches the window is a configuration error no compactor fixes, same as the documented single-unit-overflow stance.
This commit is contained in:
@@ -8,7 +8,7 @@ This is the implementation tier of the compaction capability — see the [interf
|
||||
|
||||
The abstract contract states only WHAT compaction does; this backend owns every HOW decision:
|
||||
|
||||
- **Token estimation** — `estimateContentTokens()`: chars divided by the `charsPerToken` config (default 4) with per-block structural overhead (`text`/`reasoning` = `ceil(len/charsPerToken) + 4`, `tool-call` from name + arguments, `tool-result` recursive, unknown blocks via JSON length).
|
||||
- **Token estimation** — `estimateContentTokens()`: chars divided by the `charsPerToken` config (default 4) with per-block structural overhead (`text`/`reasoning` = `ceil(len/charsPerToken) + 4`, `tool-call` from name + arguments, `tool-result` recursive, unknown blocks via JSON length). The pressure gate estimates the NEXT request via `estimatePressure()`: the logged session prefix (`EpochHeader.messagePrefix` from the header fold — the `agent/session-prefix` product rides every request in front of the history, so omitting it would under-estimate pressure by exactly the prefix) + the derived history + the system prompt.
|
||||
- **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 is a direct one-shot `ctx.llm.stream()` call — NOT a loop step, so it does not run `agent/request` (that seam shapes the loop's conversation requests); the model comes from `summarizationModel` falling back to the agent's own, and per-call routing happens at `llm/stream` like any other direct call. `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 (tool-call, tool-result, plugin-added types) contributes a type-tagged placeholder (`[tool-call: name(args)]`, `[tool-result: …]`, …) so the summarizer is told what existed rather than silently dropping it.
|
||||
|
||||
@@ -187,7 +187,7 @@ export class BasicCompactService extends CompactService {
|
||||
try {
|
||||
const result = await this.compactIfNeeded(agent, fullSystemPrompt, signal)
|
||||
if (result) {
|
||||
const after = this.estimateTokens(agent.session.deriveMessages(), fullSystemPrompt)
|
||||
const after = this.estimatePressure(agent.session, fullSystemPrompt)
|
||||
ctx.logger.info(
|
||||
`compaction: shadowed ${result.shadowedSeqs.length} surface nodes ` +
|
||||
`(seqs ${result.shadowedRange.start}-${result.shadowedRange.end}, ` +
|
||||
@@ -359,11 +359,19 @@ export class BasicCompactService extends CompactService {
|
||||
// ---- Core API (implements the abstract contract) ----
|
||||
|
||||
/**
|
||||
* The sole token-pressure gate: estimate the current surface-derived history,
|
||||
* and if it exceeds the threshold (`contextWindow * thresholdRatio`), compact
|
||||
* The sole token-pressure gate: estimate the NEXT request's pressure — the
|
||||
* logged session prefix + the surface-derived history + the system prompt
|
||||
* ({@link estimatePressure}) — and if it exceeds the threshold
|
||||
* (`contextWindow * thresholdRatio`), compact
|
||||
* the oldest surface nodes outside the `retainTokens` budget. The auto-
|
||||
* compaction listener delegates here rather than pre-checking, so this is the
|
||||
* only place the decision lives.
|
||||
* only place the decision lives. The prefix counts because every request
|
||||
* carries it in front of the history (`EpochHeader.messagePrefix`) even
|
||||
* though it is not derived history — omitting it would under-estimate by
|
||||
* exactly the prefix and let a deployment at the window edge skip
|
||||
* compaction, then ship an over-window request. Compaction itself can only
|
||||
* shrink HISTORY: a prefix that alone approaches the window is a
|
||||
* configuration error no compactor fixes.
|
||||
*
|
||||
* Retention is a UNIFORM tail→head walk over the whole surface — turn
|
||||
* boundaries play NO role. Walking node-by-node from the tail and summing
|
||||
@@ -393,7 +401,7 @@ export class BasicCompactService extends CompactService {
|
||||
const threshold = Math.floor(this.config.contextWindow * this.config.thresholdRatio)
|
||||
let result: CompactionResult | null = null
|
||||
for (let attempt = 0; attempt <= this.config.compactionRetries; attempt++) {
|
||||
const totalTokens = this.estimateTokens(session.deriveMessages(), fullSystemPrompt)
|
||||
const totalTokens = this.estimatePressure(session, fullSystemPrompt)
|
||||
if (totalTokens < threshold) return result
|
||||
|
||||
const range = this._compactableRange(session)
|
||||
@@ -407,7 +415,7 @@ export class BasicCompactService extends CompactService {
|
||||
result = await this.compactRegion(session, range.start, range.end, agent, signal)
|
||||
}
|
||||
|
||||
const totalTokens = this.estimateTokens(session.deriveMessages(), fullSystemPrompt)
|
||||
const totalTokens = this.estimatePressure(session, fullSystemPrompt)
|
||||
if (totalTokens < threshold) return result
|
||||
|
||||
throw new Error(
|
||||
@@ -416,6 +424,23 @@ export class BasicCompactService extends CompactService {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Estimated token pressure of the NEXT request: the logged session prefix
|
||||
* (`EpochHeader.messagePrefix` from the header fold — request-only messages
|
||||
* the loop sends in front of the derived history), the derived history, and
|
||||
* the system prompt. The fold is exact from the loop instance's second
|
||||
* request on (and from a resumed instance's first — the previous instance
|
||||
* logged its prefix); it is absent only before a fresh session's first
|
||||
* request, where the history is a single prompt and compaction is moot.
|
||||
* @param session - the session whose next request is being estimated.
|
||||
* @param fullSystemPrompt - the assembled system prompt (counts toward pressure).
|
||||
* @returns the estimated token total the next request will carry.
|
||||
*/
|
||||
estimatePressure(session: Session, fullSystemPrompt: string): number {
|
||||
const sessionPrefix = session.requestHeader()?.messagePrefix ?? []
|
||||
return this.estimateTokens([...sessionPrefix, ...session.deriveMessages()], fullSystemPrompt)
|
||||
}
|
||||
|
||||
override async compactRegion(
|
||||
session: Session,
|
||||
start: number,
|
||||
|
||||
@@ -557,6 +557,29 @@ describe('BasicCompactService.compactIfNeeded', () => {
|
||||
expect(result!.shadowedSeqs.length).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('counts the logged session prefix toward pressure (every request carries it in front of the history)', async () => {
|
||||
const svc = createTestService({ contextWindow: 200, thresholdRatio: 0.5, retainTokens: 10 })
|
||||
const session = multiTurnSession(3, 1) // 6 derived messages ≈ 84 estimated tokens — under the 100 threshold alone
|
||||
expect(await compactIfNeeded(svc, session, '', 'm', SIGNAL)).toBeNull()
|
||||
|
||||
// The loop records the composed agent/session-prefix product on the
|
||||
// request header; it rides every request, so pressure must include it.
|
||||
session.append('request/header', {
|
||||
header: {
|
||||
config: { model: 'm' },
|
||||
messagePrefix: [
|
||||
{ role: 'user', content: [{ type: 'text', text: `opener one.${LONG_FIXTURE_TEXT}` }] },
|
||||
{ role: 'user', content: [{ type: 'text', text: `opener two.${LONG_FIXTURE_TEXT}` }] },
|
||||
],
|
||||
},
|
||||
reason: 'initial',
|
||||
})
|
||||
const result = await compactIfNeeded(svc, session, '', 'm', SIGNAL)
|
||||
expect(result).not.toBeNull()
|
||||
// The prefix itself is NOT history: compaction shadowed surface nodes only.
|
||||
expect(session.requestHeader()?.messagePrefix).toHaveLength(2)
|
||||
})
|
||||
|
||||
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`.
|
||||
|
||||
Reference in New Issue
Block a user