mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-08-29 04:26:38 +00:00
fix(agent-loop): compose the session prefix before pre-step; hand it to the pressure gate
ds-review-bot critical (follow-up): on the first step of a resumed or seeded/forked instance, auto-compaction ran before runStep composed this instance's prefix, so the gate read the PREVIOUS instance's logged prefix from the header fold — a contributor that grew across resume/fork (skills added, AGENTS.md grown: exactly the environment-dependent case) could under-gate and ship an over-window first request. The loop now composes agent/session-prefix before the instance's first agent/pre-step (still once per instance; runStep just reads the cache), and agent/pre-step carries the composed prefix to its listeners. CompactService.compactIfNeeded gains the sessionPrefix parameter; BasicCompactService.estimatePressure gates on the handed value — the header-fold read is gone, so the estimate is exact at every step including a resumed/forked instance's first. Composition moving before the boundary snapshot also means a composing listener's session append now joins the CURRENT request (documented on the seam). New coverage: composition precedes pre-step and the seam receives the composed prefix; cancel and disposal landing inside the composition window drop the step cleanly; the compact gate test hands the prefix directly.
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). 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.
|
||||
- **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 session prefix (the `agent/session-prefix` product — composed by the loop BEFORE the pre-step seam and handed through it, so the gate counts the prefix this instance will actually send in front of the history, never a stale logged one) + 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.
|
||||
|
||||
@@ -183,11 +183,11 @@ export class BasicCompactService extends CompactService {
|
||||
// log-only `compact/*` records and the replacement node cleanly outside a
|
||||
// step, so a crash mid-compaction leaves an inert orphan the turn-repair
|
||||
// closes — never a half-open step.
|
||||
ctx.on('agent/pre-step', async (agent: Agent, _turn: number, _step: number, fullSystemPrompt: string, signal: AbortSignal) => {
|
||||
ctx.on('agent/pre-step', async (agent: Agent, _turn: number, _step: number, fullSystemPrompt: string, sessionPrefix: readonly Message[], signal: AbortSignal) => {
|
||||
try {
|
||||
const result = await this.compactIfNeeded(agent, fullSystemPrompt, signal)
|
||||
const result = await this.compactIfNeeded(agent, fullSystemPrompt, sessionPrefix, signal)
|
||||
if (result) {
|
||||
const after = this.estimatePressure(agent.session, fullSystemPrompt)
|
||||
const after = this.estimatePressure(agent.session, fullSystemPrompt, sessionPrefix)
|
||||
ctx.logger.info(
|
||||
`compaction: shadowed ${result.shadowedSeqs.length} surface nodes ` +
|
||||
`(seqs ${result.shadowedRange.start}-${result.shadowedRange.end}, ` +
|
||||
@@ -360,7 +360,7 @@ export class BasicCompactService extends CompactService {
|
||||
|
||||
/**
|
||||
* The sole token-pressure gate: estimate the NEXT request's pressure — the
|
||||
* logged session prefix + the surface-derived history + the system prompt
|
||||
* 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-
|
||||
@@ -369,7 +369,11 @@ export class BasicCompactService extends CompactService {
|
||||
* 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
|
||||
* compaction, then ship an over-window request. The loop composes the
|
||||
* prefix BEFORE the pre-step seam and hands it through, so the gate sees
|
||||
* this instance's actual prefix (never a previous instance's logged one —
|
||||
* a resumed/forked instance whose contributor grew is gated on the grown
|
||||
* value from its very first step). Compaction itself can only
|
||||
* shrink HISTORY: a prefix that alone approaches the window is a
|
||||
* configuration error no compactor fixes.
|
||||
*
|
||||
@@ -395,13 +399,14 @@ export class BasicCompactService extends CompactService {
|
||||
override async compactIfNeeded(
|
||||
agent: Agent,
|
||||
fullSystemPrompt: string,
|
||||
sessionPrefix: readonly Message[],
|
||||
signal: AbortSignal,
|
||||
): Promise<CompactionResult | null> {
|
||||
const session = agent.session
|
||||
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.estimatePressure(session, fullSystemPrompt)
|
||||
const totalTokens = this.estimatePressure(session, fullSystemPrompt, sessionPrefix)
|
||||
if (totalTokens < threshold) return result
|
||||
|
||||
const range = this._compactableRange(session)
|
||||
@@ -415,7 +420,7 @@ export class BasicCompactService extends CompactService {
|
||||
result = await this.compactRegion(session, range.start, range.end, agent, signal)
|
||||
}
|
||||
|
||||
const totalTokens = this.estimatePressure(session, fullSystemPrompt)
|
||||
const totalTokens = this.estimatePressure(session, fullSystemPrompt, sessionPrefix)
|
||||
if (totalTokens < threshold) return result
|
||||
|
||||
throw new Error(
|
||||
@@ -425,19 +430,16 @@ 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.
|
||||
* Estimated token pressure of the NEXT request: the session prefix
|
||||
* (`EpochHeader.messagePrefix` — request-only messages the loop sends in
|
||||
* front of the derived history, composed before the pre-step seam and
|
||||
* handed to the gate), the derived history, and the system prompt.
|
||||
* @param session - the session whose next request is being estimated.
|
||||
* @param fullSystemPrompt - the assembled system prompt (counts toward pressure).
|
||||
* @param sessionPrefix - the instance's composed session prefix (counts toward pressure).
|
||||
* @returns the estimated token total the next request will carry.
|
||||
*/
|
||||
estimatePressure(session: Session, fullSystemPrompt: string): number {
|
||||
const sessionPrefix = session.requestHeader()?.messagePrefix ?? []
|
||||
estimatePressure(session: Session, fullSystemPrompt: string, sessionPrefix: readonly Message[]): number {
|
||||
return this.estimateTokens([...sessionPrefix, ...session.deriveMessages()], fullSystemPrompt)
|
||||
}
|
||||
|
||||
|
||||
@@ -557,27 +557,22 @@ 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 () => {
|
||||
it('counts the 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)
|
||||
// The loop composes the agent/session-prefix product before the pre-step
|
||||
// seam and hands it to the gate; it rides every request, so pressure must
|
||||
// include it — the same history now crosses the threshold.
|
||||
const sessionPrefix: Message[] = [
|
||||
{ role: 'user', content: [{ type: 'text', text: `opener one.${LONG_FIXTURE_TEXT}` }] },
|
||||
{ role: 'user', content: [{ type: 'text', text: `opener two.${LONG_FIXTURE_TEXT}` }] },
|
||||
]
|
||||
const result = await compactIfNeeded(svc, session, '', 'm', SIGNAL, sessionPrefix)
|
||||
expect(result).not.toBeNull()
|
||||
// The prefix itself is NOT history: compaction shadowed surface nodes only.
|
||||
expect(session.requestHeader()?.messagePrefix).toHaveLength(2)
|
||||
expect(sessionPrefix).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('returns the first compaction result when a zero-retry pass converges after the loop', async () => {
|
||||
@@ -1005,8 +1000,9 @@ function compactIfNeeded(
|
||||
fullSystemPrompt: string,
|
||||
model: string,
|
||||
signal: AbortSignal,
|
||||
sessionPrefix: readonly Message[] = [],
|
||||
) {
|
||||
return svc.compactIfNeeded(stubAgent(session, model), fullSystemPrompt, signal)
|
||||
return svc.compactIfNeeded(stubAgent(session, model), fullSystemPrompt, sessionPrefix, signal)
|
||||
}
|
||||
|
||||
function compactRegion(
|
||||
@@ -1174,7 +1170,7 @@ describe('BasicCompactService.summarize (real ctx.llm.stream)', () => {
|
||||
describe('BasicCompactService auto-compaction (agent/pre-step listener)', () => {
|
||||
/** Fire the agent/pre-step serial checkpoint as the loop does. */
|
||||
function firePreStep(ctx: Context, agent: Agent, step: number, fullSystemPrompt: string): Promise<unknown> {
|
||||
return ctx.serial('agent/pre-step', agent, 1, step, fullSystemPrompt, SIGNAL)
|
||||
return ctx.serial('agent/pre-step', agent, 1, step, fullSystemPrompt, [], SIGNAL)
|
||||
}
|
||||
|
||||
it('compacts (mutating the surface) when over threshold', async () => {
|
||||
@@ -1276,7 +1272,7 @@ describe('BasicCompactService auto-compaction (agent/pre-step listener)', () =>
|
||||
const session = multiTurnSession(5, 1)
|
||||
const agent = stubAgent(session, 'agent-model')
|
||||
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, '', SIGNAL)
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, '', [], SIGNAL)
|
||||
|
||||
expect(adapter.lastOptions?.model).toBe('routed-model')
|
||||
expect(session.events.some(e => e.type === 'compact/summary')).toBe(true)
|
||||
@@ -1415,7 +1411,7 @@ describe('BasicCompactService edge cases', () => {
|
||||
const session = multiTurnSession(4, 1)
|
||||
const agent = stubAgent(session, 'test-model')
|
||||
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, '', SIGNAL)
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, '', [], SIGNAL)
|
||||
expect(session.events.some(e => e.type === 'compact/summary')).toBe(true)
|
||||
// The surface was mutated; the head message is the framed summary checkpoint.
|
||||
expect(session.deriveMessages()[0]!.content).toContainEqual({ type: 'text', text: 'SUMMARY' })
|
||||
@@ -1495,7 +1491,7 @@ describe('BasicCompactService edge cases', () => {
|
||||
const agent = stubAgent(session, 'test-model')
|
||||
const before = session.surface.nodes.length
|
||||
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, '', SIGNAL)
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, '', [], SIGNAL)
|
||||
// The failure was swallowed; the surface is untouched and a warning logged.
|
||||
expect(session.surface.nodes.length).toBe(before)
|
||||
expect(session.events.some(e => e.type === 'compact/summary')).toBe(false)
|
||||
@@ -1512,7 +1508,7 @@ describe('BasicCompactService edge cases', () => {
|
||||
const agent = stubAgent(session, 'test-model')
|
||||
const bigSystem = 'x'.repeat(900) // ceil(900/4)=225 > threshold 200
|
||||
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, bigSystem, SIGNAL)
|
||||
await ctx.serial('agent/pre-step', agent, 1, 1, bigSystem, [], SIGNAL)
|
||||
expect(session.events.some(e => e.type === 'compact/start')).toBe(false)
|
||||
expect(svc.summarizeCalls.length).toBe(0)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user