From d6da8ca29aa027df8928f0cdb60e583899b17fb5 Mon Sep 17 00:00:00 2001 From: Hypatia May Date: Fri, 26 Jun 2026 13:51:01 +0800 Subject: [PATCH] fix(compact): decide step-alignment from surface tool-pairing, fire compaction pre-step (CBR-001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 1 CBR-001: a head-anchored compaction checkpoint was mis-classified by the log-position step-alignment scan, so a second auto-compaction over a checkpoint-headed surface silently failed. Root cause: `isStepAlignedStart/End` scanned the LOG by seq, but a `replace` op lands a checkpoint at a high log seq whose SURFACE position is the head — its log neighbours (the open step's assistant/message) are not its surface neighbours, so the forward scan wrongly reported mid-step. Fix, per the agreed direction: - Replace the two log-position predicates with one surface-anchored helper `isToolPairingBalanced(nodes, events, beforeSeq)` in `dsh-session` (renamed step-boundary.ts → tool-pairing.ts). A cut is balanced when no unanswered tool-call precedes it on the surface; a region is collapsible iff both edges are balanced cuts. The open-tail and free-node cases fall out of the same counter. It also throws on a corrupt surface (a tool/result with no matching call). - Move compaction off the in-step seam to a new "pre-step" seam fired after turn/start and before step/start, so a compaction's log-only compact/* records and its replacement node land cleanly OUTSIDE any step (the honest structure crash-safety relies on). Renamed the event agent/pre-request → agent/pre-step and switched its dispatch from parallel → serial (listeners mutate the surface as a side effect; serial isolates them so concurrent appends can't interleave). Extended the catalog generator to accept @mode serial. Regression coverage: a real-loop test driving an auto-compaction asserts the landed checkpoint is a balanced cut on both sides; unit tests pin the checkpoint case, the mid-step injection case, multi-call steps, and the corrupt-surface guard. Proven red on the old log-position logic. --- docs/cordis-catalog/events-and-services.md | 26 +- packages/compact/compact-basic/package.json | 3 + packages/compact/compact-basic/src/index.ts | 127 +++---- .../compact-basic/tests/compact-basic.spec.ts | 86 +++-- .../tests/compact-loop-repro.spec.ts | 155 +++++++++ packages/compact/compact/src/index.ts | 25 +- packages/core/agent-loop/src/loop.ts | 82 +++-- packages/core/agent-loop/tests/cancel.spec.ts | 30 ++ packages/core/agent-loop/tests/loop.spec.ts | 73 +++- packages/core/agent/src/types.ts | 43 ++- packages/core/session/src/index.ts | 2 +- packages/core/session/src/step-boundary.ts | 97 ------ packages/core/session/src/tool-pairing.ts | 100 ++++++ .../core/session/tests/step-boundary.spec.ts | 172 ---------- .../core/session/tests/tool-pairing.spec.ts | 314 ++++++++++++++++++ pnpm-lock.yaml | 9 + scripts/gen-cordis-catalog.ts | 16 +- 17 files changed, 912 insertions(+), 448 deletions(-) create mode 100644 packages/compact/compact-basic/tests/compact-loop-repro.spec.ts delete mode 100644 packages/core/session/src/step-boundary.ts create mode 100644 packages/core/session/src/tool-pairing.ts delete mode 100644 packages/core/session/tests/step-boundary.spec.ts create mode 100644 packages/core/session/tests/tool-pairing.spec.ts diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 7514a5fe41..5ace95c782 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -11,7 +11,7 @@ The **harness tier** below (the `@deepseek-ai/dsh-*` packages) is the vocabulary ## Events -Dispatch modes: **emit** (fire-and-forget), **waterfall** (each listener gets `next()` and may transform or veto — see [waterfall semantics](../architecture.md#cordis-waterfall-semantics-important)), **parallel** (awaited fan-out, no veto). The harness declares 25 events across 6 scopes. +Dispatch modes: **emit** (fire-and-forget), **waterfall** (each listener gets `next()` and may transform or veto — see [waterfall semantics](../architecture.md#cordis-waterfall-semantics-important)), **parallel** (awaited fan-out, no veto), **serial** (awaited, in registration order, no veto). The harness declares 25 events across 6 scopes. ### `agent/*` @@ -49,21 +49,21 @@ A step or turn errored. The loop reports a failure here (plus the logger) even w Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:242`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:249`](../../packages/core/agent/src/types.ts) -#### `agent/pre-request` — parallel +#### `agent/pre-step` — serial -Awaited surface-mutation checkpoint, fired BEFORE the step's message history is derived (and thus before agent/request). The loop awaits `ctx.parallel('agent/pre-request', …)` after assembling the system prompt but before `session.deriveMessages()`, then derives ONCE from whatever the surface now holds. This is where compaction belongs: it mutates the session surface in place (shadowing an older range with a summary node), and the single subsequent derive reflects the mutation — so there is no double-derive and no listener can see (or be expected to act on) an assembled `messages` array that does not exist yet. +Awaited pre-step surface-mutation checkpoint, fired once per step AFTER `turn/start` (and after the prior step closed) but BEFORE this step's `step/start` — so anything a listener appends lands OUTSIDE the step, between `turn/start`/`step/end` and the upcoming `step/start`. `step` is the number of the step about to start. The loop awaits `ctx.serial('agent/pre-step', …)` after assembling the system prompt, then opens the step and derives the request history ONCE from whatever the surface now holds. This is where compaction belongs: it mutates the session surface in place (shadowing an older range with a summary node) with its log-only `compact/*` records cleanly outside any step, and the single subsequent derive reflects the mutation — so there is no double-derive and no listener can see (or be expected to act on) an assembled `messages` array that does not exist yet. -Awaited (parallel), not a waterfall: a listener mutates the surface as a side effect; there is nothing to transform or veto, but the loop must wait for the mutation to complete before deriving. `system`/`model` are the assembled values a listener needs to measure pressure (system counts toward the budget) and to summarize (the model). `signal` cancels any in-flight work a listener starts (e.g. a summarization model call). +Serial (awaited, in registration order, no veto), not a waterfall: a listener mutates the surface as a side effect; there is nothing to transform or veto, but the loop must wait for the mutation to complete before opening the step and deriving, and serial isolates listeners from each other (one finishes its surface append before the next runs). `system`/`model` are the assembled values a listener needs to measure pressure (system counts toward the budget) and to summarize (the model). `signal` cancels any in-flight work a listener starts (e.g. a summarization model call). ```ts cordis-catalog -'agent/pre-request'(agent: Agent, turn: number, step: number, system: string, model: string, signal: AbortSignal): Promise | void +'agent/pre-step'(agent: Agent, turn: number, step: number, system: string, model: string, signal: AbortSignal): Promise | void ``` Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:202`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:209`](../../packages/core/agent/src/types.ts) #### `agent/queued` — emit @@ -79,7 +79,7 @@ Source: [`packages/core/agent/src/types.ts:156`](../../packages/core/agent/src/t #### `agent/request` — waterfall -Waterfall: mutate the fully-assembled GenerateOptions before the model call (hooks, model switching, tool filtering, …). Call `next()` to delegate, or return without it to short-circuit. For surface mutation that must precede history derivation (compaction), use agent/pre-request instead — by the time this fires, `options.messages` is already derived. +Waterfall: mutate the fully-assembled GenerateOptions before the model call (hooks, model switching, tool filtering, …). Call `next()` to delegate, or return without it to short-circuit. For surface mutation that must precede history derivation (compaction), use agent/pre-step instead — by the time this fires, `options.messages` is already derived. ```ts cordis-catalog 'agent/request'(agent: Agent, turn: number, step: number, options: GenerateOptions, next: () => Promise): Promise @@ -87,7 +87,7 @@ Waterfall: mutate the fully-assembled GenerateOptions before the model call (hoo Types: [Agent](../core-data-structures/core.md) · [GenerateOptions](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:211`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:218`](../../packages/core/agent/src/types.ts) #### `agent/status` — emit @@ -111,7 +111,7 @@ Steering content was injected into a running turn. Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:236`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:243`](../../packages/core/agent/src/types.ts) #### `agent/step-end` — emit @@ -135,7 +135,7 @@ Waterfall: post-process the assembled assistant Message before tool dispatch (va Types: [Agent](../core-data-structures/core.md) · [Message](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:217`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:224`](../../packages/core/agent/src/types.ts) #### `agent/step-start` — emit @@ -159,7 +159,7 @@ A raw StreamChunk arrived from the model (token-level UI/log feed). Types: [Agent](../core-data-structures/core.md) · [StreamChunk](../core-data-structures/llm-streaming.md) -Source: [`packages/core/agent/src/types.ts:231`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:238`](../../packages/core/agent/src/types.ts) #### `agent/turn-continuation` — waterfall @@ -171,7 +171,7 @@ Waterfall: override the turn-continuation decision. The default (computed by the Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:224`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:231`](../../packages/core/agent/src/types.ts) #### `agent/turn-end` — emit diff --git a/packages/compact/compact-basic/package.json b/packages/compact/compact-basic/package.json index c745fda233..c019796e0d 100644 --- a/packages/compact/compact-basic/package.json +++ b/packages/compact/compact-basic/package.json @@ -30,10 +30,13 @@ }, "devDependencies": { "@deepseek-ai/dsh-agent": "workspace:^", + "@deepseek-ai/dsh-agent-loop": "workspace:^", "@deepseek-ai/dsh-compact": "workspace:^", "@deepseek-ai/dsh-invariants": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", + "@deepseek-ai/dsh-system-prompt": "workspace:^", + "@deepseek-ai/dsh-tools": "workspace:^", "cordis": "^4.0.0-rc.6" } } diff --git a/packages/compact/compact-basic/src/index.ts b/packages/compact/compact-basic/src/index.ts index 2ee0d82133..7648a245b0 100644 --- a/packages/compact/compact-basic/src/index.ts +++ b/packages/compact/compact-basic/src/index.ts @@ -5,18 +5,18 @@ * - **Token estimation** — char/4 heuristic with per-block structural overhead. * - **Retention policy** — walk surface nodes tail→head, keep recent nodes up * to a token budget, compact everything older. The cutoff is snapped forward - * to the next step boundary so a compacted region never splits a step's - * tool-call/result pair (an open tail step is never crossed — compaction - * declines and retries once it closes). + * to the next balanced tool-pairing boundary so a compacted region never + * splits a step's tool-call/result pair (an open tail step is never crossed — + * compaction declines and retries once it closes). * - **Summarization** — `ctx.llm.stream()` assembled via `BlockAssembler` * (the single model-call surface; same path the loop uses) with a fixed * condense-the-history system prompt. * - **Surface mutation** — a single `user/message` replace node carries the * summary; `compact/*` events are log-only lock + provenance records. - * - **Auto-compaction** — an `agent/request` waterfall listener delegates to - * {@link BasicCompactService.compactIfNeeded} before EVERY model call (every - * step, so a tool-heavy turn that grows the surface mid-turn still compacts); - * it owns the sole token-pressure check. + * - **Auto-compaction** — an `agent/pre-step` listener delegates to + * {@link BasicCompactService.compactIfNeeded} before EVERY step (so a + * tool-heavy turn that grows the surface mid-turn still compacts); it owns the + * sole token-pressure check. * * A different backend (real tokenizer, template summarizer, turn-count * retention) either subclasses this and overrides the {@link @@ -33,7 +33,7 @@ import type { CompactionResult } from '@deepseek-ai/dsh-compact' import { BlockAssembler } from '@deepseek-ai/dsh-llm' import type { ContentBlock, FinishReason, GenerateOptions, Message } from '@deepseek-ai/dsh-llm' import type { Session, SessionEvent } from '@deepseek-ai/dsh-session' -import { isStepAlignedStart, isStepAlignedEnd } from '@deepseek-ai/dsh-session' +import { isToolPairingBalanced } from '@deepseek-ai/dsh-session' import type { Agent } from '@deepseek-ai/dsh-agent' import type { BasicCompactConfig, ResolvedConfig } from './types.ts' import { resolveConfig } from './types.ts' @@ -169,23 +169,26 @@ export class BasicCompactService extends CompactService { this.config = resolveConfig(config) if (this.config.auto) { - // Auto-compaction: delegate to compactIfNeeded before EVERY model call — - // every step, not just the first. This is LOAD-BEARING for runaway-turn - // survival: a tool-heavy ReAct turn appends an assistant/message and a - // tool/result per step, so the surface (and the derived token count) grows - // WITHIN a turn. The only moment to rescue a turn that alone approaches the - // window is the next step's pre-request; gating to a turn's first step - // would let a runaway turn overflow before the next turn's check. The - // listener owns NO threshold logic — compactIfNeeded is the single place - // that decides whether to compact, and its in-progress lock serializes - // concurrent attempts. + // Auto-compaction: delegate to compactIfNeeded before EVERY step. This is + // LOAD-BEARING for runaway-turn survival: a tool-heavy ReAct turn appends + // an assistant/message and a tool/result per step, so the surface (and the + // derived token count) grows WITHIN a turn. The only moment to rescue a + // turn that alone approaches the window is the next step's pre-step + // checkpoint; gating to a turn's first step would let a runaway turn + // overflow before the next turn's check. The listener owns NO threshold + // logic — compactIfNeeded is the single place that decides whether to + // compact, and its in-progress lock serializes concurrent attempts. // - // It runs on `agent/pre-request` (a parallel surface-mutation checkpoint), - // NOT `agent/request`: compaction mutates the session surface, and the loop - // derives the request `messages` AFTER this fires — so a single derive - // already reflects the compaction, with no double-derive and no need to - // rewrite an already-assembled `messages` array. - ctx.on('agent/pre-request', async (agent: Agent, _turn: number, _step: number, system: string, model: string, signal: AbortSignal) => { + // It runs on `agent/pre-step` (a serial surface-mutation checkpoint fired + // AFTER turn/start but BEFORE step/start), NOT `agent/request`: compaction + // mutates the session surface, and the loop derives the request `messages` + // AFTER this fires — so a single derive already reflects the compaction, + // with no double-derive and no need to rewrite an already-assembled + // `messages` array. Firing pre-step (outside any open step) keeps the + // 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, system: string, model: string, signal: AbortSignal) => { try { const result = await this.compactIfNeeded(agent.session, system, model, signal) if (result) { @@ -321,17 +324,19 @@ export class BasicCompactService extends CompactService { * 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 * token estimates, once the retained total reaches `retainTokens` the cutoff - * is rounded to a step-aligned boundary: if the walk stopped INSIDE a step, - * it continues head-ward past that step's `step/start` so the whole step is - * retained (never splitting a step's tool-calls from their results); if it - * stopped on a free node (a node belonging to no step), that is already a - * clean boundary. This always rounds toward retaining MORE (retained ≥ - * `retainTokens`) and is step-aligned by construction — no separate snap pass. + * is rounded to a balanced tool-pairing boundary: if the cut before the + * retained node is unbalanced (an unanswered tool-call sits before it — i.e. + * it is mid-step), the walk continues head-ward until the cut is balanced so + * the whole step is retained (never splitting a step's tool-calls from their + * results); if it stopped on a free node (a node belonging to no step), that + * cut is already balanced. This always rounds toward retaining MORE (retained + * ≥ `retainTokens`) and is boundary-safe by construction — no separate snap + * pass. * * The compacted range is always anchored at the surface HEAD (`nodes[0]`): * auto-compaction re-consolidates any prior head checkpoint into one fresh * checkpoint. Declines (`null`) when nothing is over threshold, when the whole - * surface fits the retain budget, or when no step-aligned cutoff exists in the + * surface fits the retain budget, or when no balanced cutoff exists in the * compactable range (its only content is an open tail step — retry once it * closes). */ @@ -371,24 +376,26 @@ export class BasicCompactService extends CompactService { // The whole surface fits the retain budget — nothing to compact. if (keepFromIdx === 0) return null - // Round the cutoff to a step boundary: if `keepFromIdx` sits INSIDE a step, - // extend the retained side head-ward until the boundary is a step-aligned - // start, so the compacted range ends on a clean step edge. A node that - // belongs to no step is already a valid start. Decline if no step-aligned - // start exists at or below `keepFromIdx` (the compactable range is only an - // un-splittable open tail step — retry once it closes). + // 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 (isStepAlignedStart(events, nodes[keepFromIdx]!.seq)) break + 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 step-aligned END: - // the retained start `nodes[keepFromIdx]` is a step-aligned START (a boundary - // marker sits between them in the log), and that same boundary makes the node - // before it a step-aligned end — so no separate end check is needed. + // 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 @@ -420,19 +427,24 @@ export class BasicCompactService extends CompactService { throw new Error(`compactRegion: start seq ${start} (position ${startIdx}) is after end seq ${end} (position ${endIdx}) on the surface`) } - // The region must contain whole steps, never split a step's - // assistant-message tool-calls from their tool/results (which would orphan - // one side and produce a transcript every provider rejects). A boundary is - // valid when it sits on a step edge or on a node that belongs to no step - // (pre-step user message, inter-step steering, injection context); an `end` - // inside an open (unclosed) tail step is also rejected — its tool-calls have - // no results yet. See dsh-session's step-boundary predicates. + // The region must never split a step's assistant-message tool-calls from + // their tool/results (which would orphan one side and produce a transcript + // every provider rejects). A region is safe iff BOTH its edges are balanced + // cuts: the cut before `start`, and the cut after `end`. A node that belongs + // to no step (pre-step user message, inter-step steering, injection context) + // is a balanced (free) boundary; an `end` inside an open (unclosed) tail step + // leaves the cut after it unbalanced (the open tool-call has no result yet), + // so it is rejected. See dsh-session's tool-pairing balance check. const events = session.events - if (!isStepAlignedStart(events, start)) { - throw new Error(`compactRegion: start seq ${start} is not on a step boundary (would split a step's tool-call/result pair)`) + if (!isToolPairingBalanced(nodes, events, start)) { + throw new Error(`compactRegion: start seq ${start} is not a balanced boundary (would split a step's tool-call/result pair)`) } - if (!isStepAlignedEnd(events, end)) { - throw new Error(`compactRegion: end seq ${end} is not on a step boundary (would split a step, or the step is still open)`) + // The cut after `end` is named by `end`'s surface successor, or `null` when + // `end` is the tail. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const afterEnd: number | null = nodes[endIdx]!.next + if (!isToolPairingBalanced(nodes, events, afterEnd)) { + throw new Error(`compactRegion: end seq ${end} is not a balanced boundary (would split a step, or the step is still open)`) } if (this._isCompactionInProgress(session)) { @@ -441,10 +453,11 @@ export class BasicCompactService extends CompactService { // Compaction's events (compact/* and the replacement user/message) must be // turn-enclosed: the session-log contract rejects any plugin event appended - // outside an open turn. Auto-compaction satisfies this — it runs inside the - // `agent/request` waterfall, strictly between a turn's start and end. A - // manual call on a fully-closed session has no turn to enclose the events, - // so reject rather than emit an un-enclosed run. + // outside an open turn. Auto-compaction satisfies this — it runs on the + // `agent/pre-step` seam, after `turn/start` and before `step/start`, so + // strictly inside the open turn (but outside any step). A manual call on a + // fully-closed session has no turn to enclose the events, so reject rather + // than emit an un-enclosed run. const turn = this._openTurn(session) if (turn === null) { throw new Error('compactRegion: no open turn — compaction events must be enclosed in a turn') diff --git a/packages/compact/compact-basic/tests/compact-basic.spec.ts b/packages/compact/compact-basic/tests/compact-basic.spec.ts index 255a59a5b7..4c91f11173 100644 --- a/packages/compact/compact-basic/tests/compact-basic.spec.ts +++ b/packages/compact/compact-basic/tests/compact-basic.spec.ts @@ -49,9 +49,9 @@ function createTestService(config: BasicCompactConfig = {}): TestCompactService /** * Build a multi-turn session with surface markers (simulating real agent-loop * output). Compaction always runs inside an OPEN turn (the loop fires the - * `agent/request` waterfall between a turn's start and its end), so by default - * the session is left with a trailing open turn: turns `1..turns` close, then - * one more `turn/start` opens with no matching `turn/end`. Pass + * `agent/pre-step` seam after a turn's start and before a step's start), so by + * default the session is left with a trailing open turn: turns `1..turns` + * close, then one more `turn/start` opens with no matching `turn/end`. Pass * `{ leaveOpen: false }` for a fully-closed session (e.g. to assert that manual * compaction is rejected when no turn is open). */ @@ -219,7 +219,7 @@ describe('BasicCompactService step-alignment (never split a tool-call/result pai expect(s.events.some(e => e.type === 'compact/start')).toBe(false) }) - it('compactRegion rejects a start that is not a step boundary (splits a step)', async () => { + it('compactRegion rejects a start that splits a step (unbalanced boundary)', async () => { const svc = createTestService() const session = toolTurnSession(1) const nodes = session.surface.nodes // [user, asst(tool-call), result] @@ -228,11 +228,11 @@ describe('BasicCompactService step-alignment (never split a tool-call/result pai // start = the tool/result: its issuing assistant precedes it IN THE SAME STEP, // so starting here would orphan that assistant's tool-call. end is fine (user). await expect(svc.compactRegion(session, resultSeq, resultSeq, 'm')) - .rejects.toThrow(/start seq .* is not on a step boundary/) + .rejects.toThrow(/start seq .* is not a balanced boundary/) expect(userSeq).toBeLessThan(resultSeq) // sanity: ordering as expected }) - it('compactRegion rejects an end that is not a step boundary (splits a step)', async () => { + it('compactRegion rejects an end that splits a step (unbalanced boundary)', async () => { const svc = createTestService() const session = toolTurnSession(1) const nodes = session.surface.nodes @@ -241,7 +241,7 @@ describe('BasicCompactService step-alignment (never split a tool-call/result pai // end = the assistant/message: its tool/result follows IN THE SAME STEP, so // ending here would strand that result. start is fine (the pre-step user). await expect(svc.compactRegion(session, userSeq, asstSeq, 'm')) - .rejects.toThrow(/end seq .* is not on a step boundary/) + .rejects.toThrow(/end seq .* is not a balanced boundary/) }) it('compactRegion rejects an end inside an open tail step', async () => { @@ -258,7 +258,7 @@ describe('BasicCompactService step-alignment (never split a tool-call/result pai const userSeq = nodes[0]!.seq const asstSeq = nodes[1]!.seq await expect(svc.compactRegion(s, userSeq, asstSeq, 'm')) - .rejects.toThrow(/end seq .* is not on a step boundary/) + .rejects.toThrow(/end seq .* is not a balanced boundary/) }) it('compactRegion accepts step-aligned boundaries (pre-step user → last result of a closed step)', async () => { @@ -883,10 +883,10 @@ describe('BasicCompactService.summarize (real ctx.llm.stream)', () => { }) }) -describe('BasicCompactService auto-compaction (agent/pre-request listener)', () => { - /** Fire the agent/pre-request parallel checkpoint as the loop does. */ - function firePreRequest(ctx: Context, agent: Agent, step: number, system: string, model: string): Promise { - return ctx.parallel('agent/pre-request', agent, 1, step, system, model, SIGNAL) +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, system: string, model: string): Promise { + return ctx.serial('agent/pre-step', agent, 1, step, system, model, SIGNAL) } it('compacts (mutating the surface) when over threshold', async () => { @@ -896,7 +896,7 @@ describe('BasicCompactService auto-compaction (agent/pre-request listener)', () const agent = stubAgent(session, 'test-model') const before = session.surface.nodes.length - await firePreRequest(ctx, agent, 1, '', 'test-model') + await firePreStep(ctx, agent, 1, '', 'test-model') // The surface shrank in place, and a summary checkpoint landed. expect(session.surface.nodes.length).toBeLessThan(before) @@ -913,7 +913,7 @@ describe('BasicCompactService auto-compaction (agent/pre-request listener)', () // A step-2 checkpoint (a tool-heavy turn's later step) must still compact — // the surface accumulated assistant/message + tool/result nodes since step 1. - await firePreRequest(ctx, agent, 2, '', 'test-model') + await firePreStep(ctx, agent, 2, '', 'test-model') expect(session.events.some(e => e.type === 'compact/start')).toBe(true) }) @@ -923,7 +923,7 @@ describe('BasicCompactService auto-compaction (agent/pre-request listener)', () const session = multiTurnSession(1, 1) const agent = stubAgent(session, 'test-model') - await firePreRequest(ctx, agent, 1, '', 'test-model') + await firePreStep(ctx, agent, 1, '', 'test-model') expect(session.events.some(e => e.type === 'compact/start')).toBe(false) }) @@ -937,7 +937,7 @@ describe('BasicCompactService auto-compaction (agent/pre-request listener)', () const agent = stubAgent(session, 'missing-model') const before = session.surface.nodes.length - await firePreRequest(ctx, agent, 1, '', 'missing-model') + await firePreStep(ctx, agent, 1, '', 'missing-model') // No summary landed; the surface is unchanged. expect(session.events.some(e => e.type === 'compact/summary')).toBe(false) expect(session.surface.nodes.length).toBe(before) @@ -949,7 +949,7 @@ describe('BasicCompactService auto-compaction (agent/pre-request listener)', () const session = multiTurnSession(3, 1) const agent = stubAgent(session, 'test-model') - await firePreRequest(ctx, agent, 1, '', 'test-model') + await firePreStep(ctx, agent, 1, '', 'test-model') expect(session.events.some(e => e.type === 'compact/start')).toBe(false) }) }) @@ -992,6 +992,10 @@ describe('BasicCompactService._extractText branches', () => { s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) s.append('step/start', { turn: 1, step: 1 }) s.append('user/message', { content: [{ type: 'text', text: 'run it' }], source: { kind: 'user' } }, { surfaceOp: 'append' }) + s.append('assistant/message', { + turn: 1, step: 1, + content: [{ type: 'tool-call', id: CallId('c9'), name: 'bash', arguments: '{}' }], + }, { surfaceOp: 'append' }) s.append('tool/call', { turn: 1, step: 1, callId: CallId('c9'), name: 'bash', arguments: '{}' }) s.append('tool/result', { turn: 1, step: 1, callId: CallId('c9'), @@ -1014,12 +1018,15 @@ describe('BasicCompactService edge cases', () => { const s = new Session(SessionId('toolresult')) s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) s.append('step/start', { turn: 1, step: 1 }) - // assistant/message carrying a nested tool-result block and an unknown block. + // assistant/message carrying a nested tool-result block, an unknown block, + // and the tool-call that the following tool/result answers (so the surface + // is tool-pairing balanced). s.append('assistant/message', { turn: 1, step: 1, content: [ { type: 'tool-result', toolCallId: CallId('n1'), content: [{ type: 'image', url: 'https://x/n.png' }] }, { type: 'custom-widget', payload: 'x' } as unknown as ContentBlock, + { type: 'tool-call', id: CallId('b1'), name: 'bash', arguments: '{}' }, ], }, { surfaceOp: 'append' }) // tool/result whose content is itself only non-text → bare '[tool-result]'. @@ -1059,7 +1066,7 @@ describe('BasicCompactService edge cases', () => { const session = multiTurnSession(4, 1) const agent = stubAgent(session, 'test-model') - await ctx.parallel('agent/pre-request', agent, 1, 1, '', 'test-model', SIGNAL) + await ctx.serial('agent/pre-step', agent, 1, 1, '', 'test-model', 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' }) @@ -1140,7 +1147,7 @@ describe('BasicCompactService edge cases', () => { const agent = stubAgent(session, 'test-model') const before = session.surface.nodes.length - await ctx.parallel('agent/pre-request', agent, 1, 1, '', 'test-model', SIGNAL) + await ctx.serial('agent/pre-step', agent, 1, 1, '', 'test-model', 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) @@ -1157,7 +1164,7 @@ describe('BasicCompactService edge cases', () => { const agent = stubAgent(session, 'test-model') const bigSystem = 'x'.repeat(900) // ceil(900/4)=225 > threshold 200 - await ctx.parallel('agent/pre-request', agent, 1, 1, bigSystem, 'test-model', SIGNAL) + await ctx.serial('agent/pre-step', agent, 1, 1, bigSystem, 'test-model', SIGNAL) expect(session.events.some(e => e.type === 'compact/start')).toBe(false) expect(svc.summarizeCalls.length).toBe(0) }) @@ -1166,23 +1173,37 @@ describe('BasicCompactService edge cases', () => { const svc = createTestService() const s = new Session(SessionId('empties')) s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + // Step 1: an empty-text user, an empty-reasoning assistant with NO tool-call + // (balanced: nothing to answer), and empty context/steering — all extract to + // nothing and are skipped. s.append('step/start', { turn: 1, step: 1 }) - // Empty-text text/reasoning blocks contribute nothing → message skipped. s.append('user/message', { content: [{ type: 'text', text: '' }], source: { kind: 'user' } }, { surfaceOp: 'append' }) s.append('assistant/message', { turn: 1, step: 1, content: [{ type: 'reasoning', text: '' }] }, { surfaceOp: 'append' }) - // tool/result with empty content → empty extraction → skipped. - s.append('tool/call', { turn: 1, step: 1, callId: CallId('z1'), name: 'bash', arguments: '{}' }) - s.append('tool/result', { turn: 1, step: 1, callId: CallId('z1'), content: [], isError: false }, { surfaceOp: 'append' }) s.append('context/message', { content: [], source: { kind: 'user' } }, { surfaceOp: 'append' }) s.append('steering/message', { turn: 1, content: [{ type: 'text', text: '' }], source: { kind: 'user' } }, { surfaceOp: 'append' }) s.append('step/end', { turn: 1, step: 1 }) + // Step 2: a tool exchange whose tool/result has empty content → empty + // extraction → skipped. The assistant carries the matching tool-call so the + // surface stays tool-pairing balanced; its text extracts to the tool-call + // placeholder (the one surviving line). + s.append('step/start', { turn: 1, step: 2 }) + s.append('assistant/message', { + turn: 1, step: 2, + content: [{ type: 'tool-call', id: CallId('z1'), name: 'bash', arguments: '{}' }], + }, { surfaceOp: 'append' }) + s.append('tool/call', { turn: 1, step: 2, callId: CallId('z1'), name: 'bash', arguments: '{}' }) + s.append('tool/result', { turn: 1, step: 2, callId: CallId('z1'), content: [], isError: false }, { surfaceOp: 'append' }) + s.append('step/end', { turn: 1, step: 2 }) s.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) s.append('turn/start', { turn: 2, trigger: { kind: 'message', source: { kind: 'user' } } }) const nodes = s.surface.nodes await svc.compactRegion(s, nodes[0]!.seq, nodes[nodes.length - 1]!.seq, 'm') - // Every message extracted to empty text — the conversation is empty. - expect(svc.summarizeCalls[0]!.text).toBe('') + // Every empty-content message (user text, empty reasoning, empty-content + // tool/result, empty context, empty steering) extracted to nothing and was + // skipped — the only surviving line is the assistant's tool-call (which a + // balanced surface requires to answer the tool/result). + expect(svc.summarizeCalls[0]!.text).toBe('Assistant: [tool-call: bash({})]') }) it('renders non-text blocks as type-tagged placeholders across all message kinds', async () => { @@ -1192,8 +1213,15 @@ describe('BasicCompactService edge cases', () => { s.append('step/start', { turn: 1, step: 1 }) // user/message with only an image block → '[image]' placeholder. s.append('user/message', { content: [{ type: 'image', url: 'https://x/y.png' }], source: { kind: 'user' } }, { surfaceOp: 'append' }) - // assistant/message with only an image block → '[image]' placeholder. - s.append('assistant/message', { turn: 1, step: 1, content: [{ type: 'image', url: 'https://x/z.png' }] }, { surfaceOp: 'append' }) + // assistant/message with an image block AND the tool-call its tool/result + // answers (so the surface is tool-pairing balanced) → '[image]' placeholder. + s.append('assistant/message', { + turn: 1, step: 1, + content: [ + { type: 'image', url: 'https://x/z.png' }, + { type: 'tool-call', id: CallId('e1'), name: 'bash', arguments: '{}' }, + ], + }, { surfaceOp: 'append' }) // tool/result with an image block → '[image]' placeholder. s.append('tool/call', { turn: 1, step: 1, callId: CallId('e1'), name: 'bash', arguments: '{}' }) s.append('tool/result', { turn: 1, step: 1, callId: CallId('e1'), content: [{ type: 'image', url: 'https://x/r.png' }], isError: false }, { surfaceOp: 'append' }) diff --git a/packages/compact/compact-basic/tests/compact-loop-repro.spec.ts b/packages/compact/compact-basic/tests/compact-loop-repro.spec.ts new file mode 100644 index 0000000000..e12861a43a --- /dev/null +++ b/packages/compact/compact-basic/tests/compact-loop-repro.spec.ts @@ -0,0 +1,155 @@ +import { describe, expect, it } from 'vitest' +import { Context } from 'cordis' +import LlmService from '@deepseek-ai/dsh-llm' +import type { ContentBlock, GenerateOptions, StreamChunk } from '@deepseek-ai/dsh-llm' +import { CallId, LlmAdapter } from '@deepseek-ai/dsh-llm' +import SessionStore from '@deepseek-ai/dsh-session' +import { isToolPairingBalanced } from '@deepseek-ai/dsh-session' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' +import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' +import AgentLoop, { ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' +import * as Invariants from '@deepseek-ai/dsh-invariants' +import { BasicCompactService } from '@deepseek-ai/dsh-compact-basic' +import type { SurfaceEvent } from '@deepseek-ai/dsh-session' + +/** + * CBR-001 regression: a compaction checkpoint that the REAL loop lands is a + * free surface boundary (it carries no tool-call/result pair), so it must be a + * valid region edge on BOTH sides. A surface-anchored balance check sees that; + * the abandoned log-position scan did not. + * + * The loop fires the compaction seam mid-flight, so the landed checkpoint + * `user/message{replace}` sits at a HIGH log seq positioned beside the current + * step even though its SURFACE position is the head. A log-position forward scan + * from the checkpoint reaches the step's own later `assistant/message` and + * wrongly reports the checkpoint as mid-step — refusing it as a region end. A + * SECOND compaction that re-summarizes just that head checkpoint (region end == + * checkpoint) therefore throws and is swallowed, so the surface never + * re-consolidates. + * + * This drives a real auto-compaction through the agent-loop and asserts the + * landed checkpoint balances on both sides AND that re-compacting it (end == + * checkpoint) succeeds. RED on the log-position predicates; GREEN once alignment + * is decided from surface tool-pairing balance. + */ + +const TOKENS_PER_BLOCK = 10 + +class ReproCompactService extends BasicCompactService { + override estimateContentTokens(blocks: readonly ContentBlock[]): number { + return blocks.length * TOKENS_PER_BLOCK + } + + override async summarize(): Promise { + return [{ type: 'text', text: 'CHECKPOINT SUMMARY' }] + } +} + +/** Each call emits one tool-call until exhausted, then a final text answer. */ +class StepwiseToolAdapter extends LlmAdapter { + calls = 0 + constructor(private toolSteps: number) { + super() + } + + async * stream(_options: GenerateOptions): AsyncIterable { + const n = this.calls + this.calls += 1 + if (n < this.toolSteps) { + const id = CallId(`c${n}`) + const args = `{"i":${n}}` + yield { type: 'block-start', index: 0, blockType: 'text' } + yield { type: 'block-end', index: 0, block: { type: 'text', text: `step ${n}` } } + yield { type: 'block-start', index: 1, blockType: 'tool-call' } + yield { type: 'block-end', index: 1, block: { type: 'tool-call', id, name: 'work', arguments: args } } + yield { type: 'finish', reason: { kind: 'tool-calls' } } + return + } + yield { type: 'block-start', index: 0, blockType: 'text' } + yield { type: 'block-end', index: 0, block: { type: 'text', text: 'all done' } } + yield { type: 'finish', reason: { kind: 'stop' } } + } +} + +async function harness(toolSteps: number): Promise<{ ctx: Context; compact: ReproCompactService }> { + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(Invariants, {}) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + ctx.llm.registerAdapter(['mock'], new StepwiseToolAdapter(toolSteps)) + ctx.tools.register(defineTool({ + name: 'work', + description: 'does work', + parameters: { i: { type: 'number' } }, + async execute() { + return [{ type: 'text', text: 'work result' }] + }, + })) + // Tiny window so a couple of tool steps cross the threshold and compaction + // fires within the runaway turn. Convergence invariant holds: + // summarizationMaxTokens(1) + retainTokens(20) = 21 <= floor(60*0.5) = 30. + const compact = new ReproCompactService(ctx, { + auto: true, + contextWindow: 60, + thresholdRatio: 0.5, + retainTokens: 20, + summarizationMaxTokens: 1, + }) + return { ctx, compact } +} + +function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { + return new Promise((resolve) => { + const dispose = ctx.on('agent/status', (subject, status) => { + if (subject === agent && status === 'idle') { + dispose() + resolve() + } + }) + }) +} + +describe('CBR-001: a real-loop checkpoint is a valid boundary on both sides', () => { + it('the head checkpoint the loop lands is a balanced cut on both sides', async () => { + const { ctx } = await harness(8) + try { + const agent = ctx.agentLoop.create(AgentId('repro'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'do a long multi-step task' }]) + await waitForIdle(ctx, agent) + + const events = [...agent.session.events] + // A compaction ran: at least one checkpoint landed on the surface. + const checkpoints = events.filter( + (e): e is SurfaceEvent => + e.type === 'user/message' + && typeof (e as SurfaceEvent).surfaceOp === 'object', + ) + expect(checkpoints.length).toBeGreaterThan(0) + + // The loop fired compaction mid-flight, so each landed checkpoint sits at a + // high log seq beside the step it landed in, even though its SURFACE + // position is the head of the range it shadowed. A checkpoint carries no + // tool-call/result pair (only summarized prose), so every checkpoint still + // on the surface must be a balanced cut on BOTH sides — the cut before it + // (region START) and the cut after it (region END). The abandoned + // log-position scan reported the END as mis-aligned because the forward log + // scan reached the neighbouring step's assistant/message. + const nodes = agent.session.surface.nodes + for (const cp of checkpoints) { + const node = nodes.find(n => n.seq === cp.seq) + if (!node) continue // shadowed by a later checkpoint — no longer an edge. + expect(isToolPairingBalanced(nodes, events, node.seq), + `checkpoint seq ${node.seq} must be a balanced region START`).toBe(true) + expect(isToolPairingBalanced(nodes, events, node.next), + `checkpoint seq ${node.seq} must be a balanced region END`).toBe(true) + } + } finally { + await ctx.fiber.dispose() + } + }) +}) diff --git a/packages/compact/compact/src/index.ts b/packages/compact/compact/src/index.ts index 5e63169fa1..c84c147ca7 100644 --- a/packages/compact/compact/src/index.ts +++ b/packages/compact/compact/src/index.ts @@ -73,7 +73,8 @@ export abstract class CompactService extends Service { * Non-surface context injected downstream (into the request `messages` by a * later listener) is out of this accounting by construction. * - **Head-anchored, best-effort.** Auto-compaction consolidates from the - * surface HEAD up to a step-aligned cutoff, so a prior head checkpoint is + * surface HEAD up to a balanced tool-pairing cutoff, so a prior head + * checkpoint is * re-summarized into one fresh checkpoint (the surface holds at most one * auto-generated checkpoint, always at the head). It is best-effort over * CLOSED steps: when the only compactable content left is an un-splittable @@ -106,15 +107,15 @@ export abstract class CompactService extends Service { * summarizes their content and appends a replacement surface node. Used by the * (future) `/compact` tool and internally by {@link compactIfNeeded}. * - * The region MUST contain whole steps — `start` and `end` must each sit on a - * step boundary (the first / last surface node of a step) or on a node that - * belongs to no step (a pre-step user message, inter-step steering, or an - * injection context message). A boundary that falls INSIDE a step would split - * that step's `assistant/message` tool-calls from their `tool/result`s, leaving - * the rehydrated transcript with a dangling tool-call or an orphaned - * tool-result that every provider rejects. An `end` inside an open (unclosed) - * tail step is likewise invalid — its tool-calls have no results yet. - * `dsh-session` exports `isStepAlignedStart` / `isStepAlignedEnd` for this check. + * The region MUST NOT split a step's `assistant/message` tool-calls from their + * `tool/result`s, leaving the rehydrated transcript with a dangling tool-call + * or an orphaned tool-result that every provider rejects. A region is safe iff + * both its edges are balanced cuts on the surface: the cut before `start` and + * the cut after `end` each have no unanswered tool-call before them. A node + * that belongs to no step (a pre-step user message, inter-step steering, or an + * injection context message) is a balanced (free) boundary; an `end` inside an + * open (unclosed) tail step is invalid — its tool-calls have no results yet. + * `dsh-session` exports `isToolPairingBalanced` for this check. * * @param session - the session whose surface is mutated. * @param start - inclusive seq of the first surface node to compact. @@ -128,8 +129,8 @@ export abstract class CompactService extends Service { * valid surface nodes, if `start` is positioned after `end` on the surface * (the range is a surface-POSITION span, not a numeric seq interval — a * prior replace can leave the surface non-monotonic in seq order), or if - * either boundary is not step-aligned (would split a step's tool-call/result - * pair). + * either boundary is not a balanced tool-pairing cut (would split a step's + * tool-call/result pair). */ abstract compactRegion( session: Session, diff --git a/packages/core/agent-loop/src/loop.ts b/packages/core/agent-loop/src/loop.ts index 4aeef18eec..f6b286cb28 100644 --- a/packages/core/agent-loop/src/loop.ts +++ b/packages/core/agent-loop/src/loop.ts @@ -12,6 +12,7 @@ import type { FinishReason, GenerateOptions, Message } from '@deepseek-ai/dsh-ll import { BlockAssembler, HarnessError } from '@deepseek-ai/dsh-llm' import type { Session, TurnEndReason, TurnTrigger } from '@deepseek-ai/dsh-session' import { renderPrompt } from '@deepseek-ai/dsh-system-prompt' +import type { PromptAssembly } from '@deepseek-ai/dsh-system-prompt' import type {} from '@deepseek-ai/dsh-tools' import type { ReactLoopAgent } from './agent.ts' @@ -147,9 +148,9 @@ export interface LoopHandle { * drain queued → 'turn/start' → session('user/message'…) → emit agent/turn-start * STEP loop: * drain steering → session('steering/message') ⟵ catches late steering - * session('step/start'); emit agent/step-start ⟵ append before emit (the event-sourcing RFC) * assembly = ctx.systemPrompt.assemble() ⟵ waterfall system-prompt/assemble - * await ctx.parallel('agent/pre-request') ⟵ surface mutation (compaction) BEFORE derive + * await ctx.serial('agent/pre-step') ⟵ surface mutation (compaction) OUTSIDE the step + * session('step/start'); emit agent/step-start ⟵ append before emit (the event-sourcing RFC) * req = {model, system, tools, messages: session.deriveMessages(), signal} * req = waterfall agent/request ⟵ hooks/model-switch * stream ctx.llm.stream(req) ⟵ waterfall llm/stream (raw chunks) @@ -387,20 +388,54 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle, // (or turn-start listeners on the first step) joins before the request. drainSteering(ctx, agent, turn) + // Assemble the system prompt for this step. Done HERE (before step/start) + // because the pre-step seam needs it: compaction measures token pressure + // against the system prompt (it counts toward the budget) and a listener + // also receives the model to summarize with. runStep reuses this same + // assembly for the request, so the prompt is assembled once per step. + const assembly = await ctx.systemPrompt.assemble() + const system = [renderPrompt(assembly), agent.options.systemPrompt ?? ''] + .filter(text => text.length > 0) + .join('\n\n') + + // The step's AbortController exists BEFORE the pre-step seam so a cancel() + // during the seam aborts any in-flight work a listener started (e.g. a + // compaction summarization call). Cleared on every exit path below. + const abort = new AbortController() + handle.setAbort(abort) + + // Cancel landing before the seam: a synchronous `agent/turn-start` listener + // (or the previous step's continuation listeners) can have called + // `cancel()`. Drop the about-to-start step WITHOUT running the seam — no + // step is open yet, so end the turn `aborted` directly. + if (handle.isCancelled()) { + handle.setAbort(undefined) + reason = { kind: 'aborted', reason: handle.cancelReason() } + break + } + + // Pre-step surface-mutation checkpoint (compaction), fired OUTSIDE the + // step: after `turn/start` (and the prior step's close) but before + // `step/start`, so a compaction's log-only `compact/*` records and its + // replacement node land cleanly outside any step (honest structure that + // crash-safety relies on — a dangling `compact/start` sits before the + // synthetic `turn/end` repair appends). Serial (awaited, in order, no + // veto): each listener completes its surface mutation before the next, so + // concurrent listeners cannot interleave their `session.append`s. A + // throwing listener escapes to the outer catch, which closes the (not-yet- + // open) step as a no-op and ends the turn via failTurn — a broken + // pre-step plugin ends the turn, not the loop. + await ctx.serial('agent/pre-step', agent, turn, step, system, agent.options.model ?? '', abort.signal) + session.append('step/start', { turn, step }) stepOpen = true ctx.emit('agent/step-start', agent, turn, step) - const abort = new AbortController() - handle.setAbort(abort) - - // Cancel landing in the step-start window: a synchronous `agent/turn-start` - // or `agent/step-start` listener (both fire before this point) can have - // called `cancel()`, and `runStep` would otherwise run a full extra step - // with no AbortController having observed it. Check the marker AFTER - // setAbort (so the next-iteration drain sees a clean controller) and before - // `runStep`: drop the step, end the turn `aborted`. closeStep balances the - // already-appended step/start. + // Cancel landing in the seam / step-start window: a `cancel()` during the + // pre-step seam (it aborted `abort.signal` above) OR a synchronous + // `agent/step-start` listener that cancels. Check AFTER setAbort/step-start + // and before `runStep`: drop the step, end the turn `aborted`. closeStep + // balances the already-appended step/start. if (handle.isCancelled()) { handle.setAbort(undefined) reason = { kind: 'aborted', reason: handle.cancelReason() } @@ -410,7 +445,7 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle, let stepOutcome: { hadToolCalls: boolean; finish: FinishReason } | { error: Error } try { - stepOutcome = await runStep(ctx, agent, turn, step, abort.signal) + stepOutcome = await runStep(ctx, agent, turn, step, assembly, system, abort.signal) } catch (error: unknown) { stepOutcome = { error: toError(error) } } finally { @@ -550,29 +585,22 @@ function drainSteering(ctx: Context, agent: ReactLoopAgent, turn: number): boole return messages.length > 0 } -/** One step: assemble request → stream model → record → execute tools. */ +/** One step: derive request from the (already pre-step-mutated) surface → + * stream model → record → execute tools. The caller assembles the system prompt + * and fires the `agent/pre-step` seam BEFORE opening the step, then passes the + * resulting `assembly`/`system` here, so the surface this step derives from + * already reflects any compaction. */ async function runStep( ctx: Context, agent: ReactLoopAgent, turn: number, step: number, + assembly: PromptAssembly, + system: string, signal: AbortSignal, ): Promise<{ hadToolCalls: boolean; finish: FinishReason }> { const { session, options } = agent - // --- Request assembly --- - const assembly = await ctx.systemPrompt.assemble() - const system = [renderPrompt(assembly), options.systemPrompt ?? ''] - .filter(text => text.length > 0) - .join('\n\n') - - // Surface-mutation checkpoint BEFORE deriving history: compaction shadows an - // older range with a summary node here, and the single derive below reflects - // it. Awaited (no veto) — a listener mutates the surface as a side effect. - // `model` is resolved to '' when unset; a compaction listener that needs a - // model falls back to its own config. - await ctx.parallel('agent/pre-request', agent, turn, step, system, options.model ?? '', signal) - let request: GenerateOptions = { model: options.model ?? '', messages: session.deriveMessages(), diff --git a/packages/core/agent-loop/tests/cancel.spec.ts b/packages/core/agent-loop/tests/cancel.spec.ts index 9cdaa1973b..d6394acd54 100644 --- a/packages/core/agent-loop/tests/cancel.spec.ts +++ b/packages/core/agent-loop/tests/cancel.spec.ts @@ -194,6 +194,36 @@ describe('Agent.cancel()', () => { expect(reasons).toEqual([{ kind: 'aborted', reason: 'from turn-start' }]) }) + it('cancel from a synchronous agent/step-start listener drops the step (post-step-start window)', async () => { + const adapter = new MockAdapter([textResponse('should not stream')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + // A step-start listener fires AFTER step/start is appended (and after the + // pre-step seam), so cancelling there lands in the SECOND cancel check (the + // one that must closeStep() to balance the already-open step) — distinct + // from a turn-start cancel, which is caught before the step opens. + let streamed = false + ctx.on('agent/stream-chunk', () => { streamed = true }) + const dispose = ctx.on('agent/step-start', (subject) => { + if (subject === agent) agent.cancel('from step-start') + }) + + const reasons: TurnEndReason[] = [] + ctx.on('agent/turn-end', (_a, _t, reason) => void reasons.push(reason)) + + send(agent, 'go') + await waitForIdle(ctx, agent) + dispose() + + // No step streamed, the turn ended aborted with the caller's reason, and the + // log is balanced (the open step was closed by the cancel branch). + expect(streamed).toBe(false) + expect(reasons).toEqual([{ kind: 'aborted', reason: 'from step-start' }]) + const types = agent.session.events.map(e => e.type) + expect(types.filter(t => t === 'step/start').length).toBe(types.filter(t => t === 'step/end').length) + }) + it('cancel during the continuation window ends the turn aborted and runs no further step', async () => { // A continuation-waterfall listener cancels DURING the continuation decision // (the finished step's AbortController is already cleared), and votes to diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index f74e160936..2c6f9e06e8 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -320,11 +320,11 @@ describe('agent loop', () => { expect(adapter.requests[0]!.model).toBe('other-model') }) - it('agent/pre-request fires once per step before the request is derived', async () => { + it('agent/pre-step fires once per step before the step is opened', async () => { // Two steps (a tool call, then a final text turn) → two model calls → two - // pre-request fires, each carrying the assembled system + model, BEFORE the - // request messages are derived (the request the adapter sees reflects any - // surface state at fire time). + // pre-step fires, each carrying the assembled system + model, BEFORE the + // step is opened and its request is derived (the request the adapter sees + // reflects any surface state at fire time). const adapter = new MockAdapter([ toolCallResponse('c1', 'echo', {}, 'calling echo'), textResponse('done'), @@ -337,7 +337,7 @@ describe('agent loop', () => { const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) const fires: { turn: number; step: number; model: string }[] = [] - ctx.on('agent/pre-request', (subject, turn, step, _system, model) => { + ctx.on('agent/pre-step', (subject, turn, step, _system, model) => { if (subject === agent) fires.push({ turn, step, model }) }) @@ -351,32 +351,77 @@ describe('agent loop', () => { ]) }) - it('a surface mutation in agent/pre-request is reflected in the derived request (single derive)', async () => { - // pre-request fires BEFORE deriveMessages(), so a listener that appends a - // surface node there sees it land in the SAME step's request — proving the - // loop derives once, after the checkpoint, with no stale pre-derive. + it('agent/pre-step fires BEFORE the step it precedes opens (events land outside the step)', async () => { + // A listener appending a surface node in pre-step lands it BEFORE step/start + // in the log — proving the seam fires outside the step. The node is still in + // the derived request for that step (derive happens after step/start). const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) let injected = false - ctx.on('agent/pre-request', (subject, turn) => { + ctx.on('agent/pre-step', (subject) => { if (subject === agent && !injected) { injected = true subject.session.append('context/message', { - content: [{ type: 'text', text: 'INJECTED-IN-PRE-REQUEST' }], + content: [{ type: 'text', text: 'INJECTED-IN-PRE-STEP' }], source: { kind: 'plugin', plugin: 'test' }, }, { surfaceOp: 'append' }) - void turn } }) send(agent, 'go') await waitForIdle(ctx, agent) - // The adapter's request includes the node injected during pre-request. + // The adapter's request includes the node injected during pre-step (derive + // reflects it). const text = JSON.stringify(adapter.requests[0]!.messages) - expect(text).toContain('INJECTED-IN-PRE-REQUEST') + expect(text).toContain('INJECTED-IN-PRE-STEP') + + // And the injected event sits BEFORE the first step/start in the log — + // the seam fired outside the step. + const events = agent.session.events + const injectedSeq = events.find(e => e.type === 'context/message')!.seq + const firstStepStartSeq = events.find(e => e.type === 'step/start')!.seq + expect(injectedSeq).toBeLessThan(firstStepStartSeq) + }) + + it('a throwing agent/pre-step listener ends the turn (error), not the loop', async () => { + // The seam fires before step/start, so a throw escapes to runTurn's outer + // catch: the not-yet-open step closes as a no-op, the failure surfaces via + // agent/error, and the turn ends `error` (recorded on the durable turn/end). + // The loop survives and a follow-up prompt still runs. + const adapter = new MockAdapter([textResponse('second turn ok')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + let throwOnce = true + ctx.on('agent/pre-step', () => { + if (throwOnce) { throwOnce = false; throw new Error('boom in pre-step') } + }) + + const errors: Error[] = [] + ctx.on('agent/error', (_a, _t, _s, error) => void errors.push(error)) + + send(agent, 'first') + await waitForIdle(ctx, agent) + // The first turn failed at step 1 (no model call happened), surfaced via + // agent/error, with the durable failure on turn/end.reason. + expect(errors).toHaveLength(1) + expect(errors[0]!.message).toContain('boom in pre-step') + expect(adapter.requests.length).toBe(0) + const firstTurnEnd = agent.session.events.find(e => e.type === 'turn/end') + expect(firstTurnEnd?.type === 'turn/end' && firstTurnEnd.data.reason).toMatchObject({ kind: 'error', step: 1 }) + // The step opened-and-closed count stays balanced even though it never ran. + const types = agent.session.events.map(e => e.type) + expect(types.filter(t => t === 'step/start').length).toBe(types.filter(t => t === 'step/end').length) + + // The loop survived: a second prompt runs a normal completed turn. + send(agent, 'second') + await waitForIdle(ctx, agent) + expect(adapter.requests.length).toBe(1) + const lastTurnEnd = agent.session.events.findLast(e => e.type === 'turn/end') + expect(lastTurnEnd?.type === 'turn/end' && lastTurnEnd.data.reason).toEqual({ kind: 'completed' }) }) it('cancel() mid-stream ends the turn with reason aborted', async () => { diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index 407201ea5d..5bb603f1f6 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -181,30 +181,37 @@ declare module 'cordis' { // ---- interception seams (waterfall) ---- /** - * Awaited surface-mutation checkpoint, fired BEFORE the step's message - * history is derived (and thus before {@link agent/request}). The loop - * awaits `ctx.parallel('agent/pre-request', …)` after assembling the system - * prompt but before `session.deriveMessages()`, then derives ONCE from - * whatever the surface now holds. This is where compaction belongs: it - * mutates the session surface in place (shadowing an older range with a - * summary node), and the single subsequent derive reflects the mutation — - * so there is no double-derive and no listener can see (or be expected to - * act on) an assembled `messages` array that does not exist yet. + * Awaited pre-step surface-mutation checkpoint, fired once per step AFTER + * `turn/start` (and after the prior step closed) but BEFORE this step's + * `step/start` — so anything a listener appends lands OUTSIDE the step, + * between `turn/start`/`step/end` and the upcoming `step/start`. `step` is + * the number of the step about to start. The loop awaits + * `ctx.serial('agent/pre-step', …)` after assembling the system prompt, then + * opens the step and derives the request history ONCE from whatever the + * surface now holds. This is where compaction belongs: it mutates the session + * surface in place (shadowing an older range with a summary node) with its + * log-only `compact/*` records cleanly outside any step, and the single + * subsequent derive reflects the mutation — so there is no double-derive and + * no listener can see (or be expected to act on) an assembled `messages` + * array that does not exist yet. * - * Awaited (parallel), not a waterfall: a listener mutates the surface as a - * side effect; there is nothing to transform or veto, but the loop must wait - * for the mutation to complete before deriving. `system`/`model` are the - * assembled values a listener needs to measure pressure (system counts - * toward the budget) and to summarize (the model). `signal` cancels any - * in-flight work a listener starts (e.g. a summarization model call). - * @mode parallel + * Serial (awaited, in registration order, no veto), not a waterfall: a + * listener mutates the surface as a side effect; there is nothing to + * transform or veto, but the loop must wait for the mutation to complete + * before opening the step and deriving, and serial isolates listeners from + * each other (one finishes its surface append before the next runs). + * `system`/`model` are the assembled values a listener needs to measure + * pressure (system counts toward the budget) and to summarize (the model). + * `signal` cancels any in-flight work a listener starts (e.g. a summarization + * model call). + * @mode serial */ - 'agent/pre-request'(agent: Agent, turn: number, step: number, system: string, model: string, signal: AbortSignal): Promise | void + 'agent/pre-step'(agent: Agent, turn: number, step: number, system: string, model: string, signal: AbortSignal): Promise | void /** * Waterfall: mutate the fully-assembled {@link GenerateOptions} before the * model call (hooks, model switching, tool filtering, …). Call `next()` to * delegate, or return without it to short-circuit. For surface mutation that - * must precede history derivation (compaction), use {@link agent/pre-request} + * must precede history derivation (compaction), use {@link agent/pre-step} * instead — by the time this fires, `options.messages` is already derived. * @mode waterfall */ diff --git a/packages/core/session/src/index.ts b/packages/core/session/src/index.ts index 0fb44f3299..ad6fd59dd3 100644 --- a/packages/core/session/src/index.ts +++ b/packages/core/session/src/index.ts @@ -19,7 +19,7 @@ export { isJsonValue } from './json.ts' export { interruptedTurnClosers } from './repair.ts' export type { SurfaceNode } from './surface.ts' export { isSurfaceEvent, isSurfaceEligibleType } from './surface.ts' -export { isStepAlignedStart, isStepAlignedEnd } from './step-boundary.ts' +export { isToolPairingBalanced } from './tool-pairing.ts' declare module 'cordis' { interface Context { diff --git a/packages/core/session/src/step-boundary.ts b/packages/core/session/src/step-boundary.ts deleted file mode 100644 index e8ed91d74c..0000000000 --- a/packages/core/session/src/step-boundary.ts +++ /dev/null @@ -1,97 +0,0 @@ -/** - * Step-boundary predicates over a session log: is a given surface node a SAFE - * place to start or end a region that will be collapsed (e.g. by compaction)? - * - * The invariant a consumer needs: a collapsed region must NOT partially overlap - * a step. A step's surface nodes form a contiguous run, and a region must - * contain either ALL of a step's nodes or NONE of them — otherwise it can split - * an `assistant/message`'s `tool-call` blocks from their `tool/result`s, leaving - * the rehydrated transcript with a dangling tool-call or an orphaned tool-result - * (which every provider rejects). This is the compaction-time mirror of the - * crash-recovery imbalance that {@link interruptedTurnClosers} repairs on load. - * - * Nodes that belong to NO step — a pre-step `user/message` (drained before the - * first `step/start`), inter-step `steering/message`, or an injection - * `context/message` (wrapped in a bare `turn/start → context/message → turn/end` - * with no step) — carry no tool pairing and are free boundaries on both sides. - * - * The scans classify each neighbor event into three buckets: a turn/step - * BOUNDARY marker (the region edge is clean), a SURFACE node (the region edge - * is mid-step), or NOISE to skip (`assistant/chunk`, the log-only `compact/*` - * records, and any future non-surface event). "Surface node" is decided by the - * shared {@link isSurfaceEvent} guard so the two notions can't drift. - * - * @module @deepseek-ai/dsh-session/step-boundary - */ - -import type { SessionEvent } from './types.ts' -import { isSurfaceEvent } from './surface.ts' - -/** Turn/step boundary marker types — the walls the scans stop on. */ -const BOUNDARY_TYPES = new Set(['turn/start', 'turn/end', 'step/start', 'step/end']) - -/** - * Whether the surface node at `seq` is a SAFE START for a collapsed region — - * i.e. it is the first surface node of its step, or it belongs to no step at - * all (a free inter-step / pre-step / injection node). - * - * Scans BACKWARD from `seq`, skipping noise, and stops at the first significant - * event: a turn/step boundary marker ⇒ aligned (nothing of `seq`'s step lies - * before it), a surface node ⇒ NOT aligned (a predecessor surface node sits in - * the same step, so starting here would orphan it), start-of-log ⇒ aligned. - * - * No open-step check is needed on the start side: an open (unclosed) step can - * only ever be the LAST turn's last step, never before a valid region start. - */ -export function isStepAlignedStart(events: readonly SessionEvent[], seq: number): boolean { - for (let i = seq - 1; i >= 0; i--) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const event = events[i]! - if (BOUNDARY_TYPES.has(event.type)) return true - if (isSurfaceEvent(event)) return false - } - return true -} - -/** - * Whether the surface node at `seq` is a SAFE END for a collapsed region — - * i.e. it is the last surface node of a CLOSED step, or it belongs to no step - * at all. - * - * Scans FORWARD from `seq`, skipping noise, and stops at the first significant - * event: a turn/step boundary marker ⇒ aligned (the step/turn closes after - * `seq`, or a new one begins because `seq` was inter-step), a surface node ⇒ - * NOT aligned (a later surface node sits in the same step). Reaching - * end-of-log is aligned ONLY when `seq` is not inside an OPEN step — an open - * trailing step's `tool-call`s have no `tool/result`s yet, so collapsing it - * would defer the orphan to when those results land later. {@link isInOpenStep} - * decides that via a backward scan. - */ -export function isStepAlignedEnd(events: readonly SessionEvent[], seq: number): boolean { - for (let i = seq + 1; i < events.length; i++) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const event = events[i]! - if (BOUNDARY_TYPES.has(event.type)) return true - if (isSurfaceEvent(event)) return false - } - // End of log: aligned only if `seq` is not inside a still-open step. - return !isInOpenStep(events, seq) -} - -/** - * Whether `seq` sits inside an OPEN step — a `step/start` with no later - * `step/end`. Only meaningful at the tail (the EOL branch of - * {@link isStepAlignedEnd}): scans BACKWARD for the nearest turn/step boundary. - * The nearest one being `step/start` means a step opened before `seq` and never - * closed (no `step/end` lies after `seq`, or the forward scan would not have - * reached EOL) — so `seq` is mid-open-step. Any other nearest boundary (or none) - * means `seq` is inter-step / pre-step. - */ -function isInOpenStep(events: readonly SessionEvent[], seq: number): boolean { - for (let i = seq - 1; i >= 0; i--) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const type = events[i]!.type - if (BOUNDARY_TYPES.has(type)) return type === 'step/start' - } - return false -} diff --git a/packages/core/session/src/tool-pairing.ts b/packages/core/session/src/tool-pairing.ts new file mode 100644 index 0000000000..638daaf654 --- /dev/null +++ b/packages/core/session/src/tool-pairing.ts @@ -0,0 +1,100 @@ +/** + * Tool-pairing balance over a session's SURFACE: is a given cut point in the + * surface a safe edge for a collapsed region (e.g. compaction)? + * + * The invariant a consumer needs: a collapsed region must never separate an + * `assistant/message`'s `tool-call` blocks from their answering `tool/result`s + * — that would leave the rehydrated transcript with a dangling tool-call or an + * orphaned tool-result, which every provider rejects. (This is the + * compaction-time mirror of the crash-recovery imbalance that + * {@link interruptedTurnClosers} repairs on load.) Steps were once used as a + * proxy for this bracketing, but a compaction REWRITES the surface — it lands a + * replacement node at a high log seq whose SURFACE position is the head — so a + * scan over the LOG's `step/*` markers mis-reads such a node's neighbours. The + * pairing the invariant actually protects lives in the surface nodes' own + * content (a `tool-call` block's id, a `tool/result`'s `callId`), which travels + * with the node through any reshaping, so alignment is decided over the surface + * directly. + * + * A **cut** is a gap between two adjacent surface nodes (named by the node it + * sits immediately before), or the after-tail gap (`null`). Walking the surface + * head→tail and assigning each node a delta — `+1` per `tool-call` block on an + * `assistant/message`, `-1` per `tool/result`, `0` otherwise — the depth at a + * cut is the number of still-unanswered tool calls before it. A cut is + * **balanced** when that depth is `0`. A region `[start..end]` is safe to + * collapse iff BOTH its edges are balanced cuts: the cut before `start` and the + * cut after `end`. Nodes that belong to no step (a pre-step `user/message`, an + * inter-step `steering/message`, an injection `context/message`) carry no + * pairing, contribute `0`, and so are free boundaries — exactly as before, but + * now as a consequence of the balance rather than a special case. An open + * trailing step (an assistant whose `tool/result`s have not landed yet) keeps + * the depth positive through the tail, so no cut inside it is balanced — the + * old explicit open-step check falls out of the same counter. + * + * @module @deepseek-ai/dsh-session/tool-pairing + */ + +import type { SessionEvent } from './types.ts' +import type { SurfaceNode } from './surface.ts' + +/** + * The tool-pairing delta of a surface node: how it shifts the count of + * unanswered tool calls. An `assistant/message` opens one bracket per + * `tool-call` block; a `tool/result` closes one; every other surface node + * (`user/message`, `context/message`, `steering/message`, a usage-only + * `assistant/message` with no tool-call blocks) is pairing-neutral. + */ +function nodeDelta(event: SessionEvent): number { + switch (event.type) { + case 'assistant/message': + return event.data.content.filter(block => block.type === 'tool-call').length + case 'tool/result': + return -1 + // Non-pairing surface nodes and every non-surface event contribute nothing. + default: + return 0 + } +} + +/** + * Whether the surface prefix ending at the given cut has BALANCED tool-call / + * tool-result brackets — i.e. every `tool-call` block on the surface before the + * cut has its answering `tool/result` before the cut too, so the cut is a safe + * edge for a collapsed region (it cannot split an assistant↔result pair). + * + * `nodes` is the surface linked list in head→tail order (e.g. + * `session.surface.nodes`); `events` is the session log, used to look each + * node's event up by `seq`. `beforeSeq` names the cut by the surface node it + * sits immediately before; the after-tail cut (the whole surface) is `null`, + * as is any `beforeSeq` not present on the surface. + * + * A region `[start..end]` is collapsible iff both edges are balanced cuts: call + * `isToolPairingBalanced(nodes, events, start)` for the cut before `start`, and + * `isToolPairingBalanced(nodes, events, after)` — where `after` is `end`'s + * surface successor (`SurfaceNode.next`), or `null` when `end` is the tail — + * for the cut after `end`. + * + * @throws if the surface prefix drives the unanswered-call depth negative — a + * `tool/result` with no preceding open `tool-call` on the surface. That is a + * corrupt surface (a structural invariant violation), surfaced loudly here + * rather than silently mis-classifying a boundary. + */ +export function isToolPairingBalanced( + nodes: readonly SurfaceNode[], + events: readonly SessionEvent[], + beforeSeq: number | null, +): boolean { + let depth = 0 + for (const node of nodes) { + if (node.seq === beforeSeq) return depth === 0 + // node.seq is a surface-node seq, always a valid log index by construction. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + depth += nodeDelta(events[node.seq]!) + if (depth < 0) { + throw new Error(`tool-pairing balance: tool/result at surface seq ${node.seq} has no matching tool-call (corrupt surface)`) + } + } + // Reached the after-tail cut (beforeSeq === null, or a seq not on the + // surface): the whole-surface prefix is balanced iff depth returned to 0. + return depth === 0 +} diff --git a/packages/core/session/tests/step-boundary.spec.ts b/packages/core/session/tests/step-boundary.spec.ts deleted file mode 100644 index a24a6f7596..0000000000 --- a/packages/core/session/tests/step-boundary.spec.ts +++ /dev/null @@ -1,172 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { CallId } from '@deepseek-ai/dsh-llm' -import { isStepAlignedStart, isStepAlignedEnd } from '../src/index.ts' -import type { SessionEvent } from '../src/index.ts' - -/** - * Unit coverage for the step-alignment predicates. They decide whether a - * surface node is a safe START / END for a collapsed region (compaction): a - * region must contain whole steps, never split an `assistant/message`'s - * tool-calls from their `tool/result`s. Nodes belonging to no step (pre-step - * user message, inter-step steering, injection context) are free boundaries. - * - * Builders mirror the agent loop's real append order so the fixtures are - * representative: queued user messages land BEFORE `step/start`; within a step - * the order is `assistant/message` then `tool/result`(s); injection turns are a - * bare `turn/start → context/message → turn/end` with no step. - */ - -const SURFACE = { surfaceOp: 'append' as const } - -/** A closed turn with one closed step holding an assistant + its tool result. */ -function toolStepLog(): SessionEvent[] { - return [ - { type: 'turn/start', seq: 0, time: 0, data: { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } } }, - { type: 'user/message', seq: 1, time: 1, data: { content: [{ type: 'text', text: 'go' }], source: { kind: 'user' } }, ...SURFACE }, - { type: 'step/start', seq: 2, time: 2, data: { turn: 1, step: 1 } }, - { type: 'assistant/message', seq: 3, time: 3, data: { turn: 1, step: 1, content: [ - { type: 'text', text: 'calling' }, - { type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }, - ] }, ...SURFACE }, - { type: 'tool/call', seq: 4, time: 4, data: { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: '{}' } }, - { type: 'tool/result', seq: 5, time: 5, data: { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'out' }], isError: false }, ...SURFACE }, - { type: 'step/end', seq: 6, time: 6, data: { turn: 1, step: 1 } }, - { type: 'turn/end', seq: 7, time: 7, data: { turn: 1, reason: { kind: 'completed' } } }, - ] -} - -describe('isStepAlignedStart', () => { - it('is true for a pre-step user/message (belongs to no step)', () => { - // seq 1 user/message sits before step/start at seq 2 → free boundary. - expect(isStepAlignedStart(toolStepLog(), 1)).toBe(true) - }) - - it('is true for the first surface node of a step (the assistant/message)', () => { - // Backward from seq 3 the first significant event is step/start → aligned. - expect(isStepAlignedStart(toolStepLog(), 3)).toBe(true) - }) - - it('is false for a tool/result whose assistant/message precedes it in the same step', () => { - // Backward from seq 5 the first significant event is the assistant/message - // surface node (seq 3) → starting here would orphan that assistant's call. - expect(isStepAlignedStart(toolStepLog(), 5)).toBe(false) - }) - - it('is true at start-of-log (nothing precedes)', () => { - const log: SessionEvent[] = [ - { type: 'user/message', seq: 0, time: 0, data: { content: [{ type: 'text', text: 'hi' }], source: { kind: 'user' } }, ...SURFACE }, - ] - expect(isStepAlignedStart(log, 0)).toBe(true) - }) - - it('skips noise (assistant/chunk, compact/* records) when scanning back', () => { - // A compacted region landed compact/* log-only records between the prior - // step boundary and this surface node; they must be skipped, not treated as - // walls. Backward from seq 4 skips compact/end, compact/summary, compact/start - // and stops at step/start (seq 0) → aligned. - const log: SessionEvent[] = [ - { type: 'step/start', seq: 0, time: 0, data: { turn: 1, step: 1 } }, - { type: 'compact/start', seq: 1, time: 1, data: { turn: 1 } } as unknown as SessionEvent, - { type: 'compact/summary', seq: 2, time: 2, data: { summary: [], shadowedRange: { start: 0, end: 0 }, shadowedSeqs: [], shadowedTokenCount: 0 } } as unknown as SessionEvent, - { type: 'compact/end', seq: 3, time: 3, data: { turn: 1 } } as unknown as SessionEvent, - { type: 'assistant/message', seq: 4, time: 4, data: { turn: 1, step: 1, content: [{ type: 'text', text: 'a' }] }, ...SURFACE }, - ] - expect(isStepAlignedStart(log, 4)).toBe(true) - }) -}) - -describe('isStepAlignedEnd', () => { - it('is true for the last surface node of a closed step (the tool/result)', () => { - // Forward from seq 5 the first significant event is step/end → aligned. - expect(isStepAlignedEnd(toolStepLog(), 5)).toBe(true) - }) - - it('is false for an assistant/message with a later tool/result in the same step', () => { - // Forward from seq 3 the first significant event is the tool/result surface - // node (seq 5) → ending here would strand that result. - expect(isStepAlignedEnd(toolStepLog(), 3)).toBe(false) - }) - - it('is true for a pre-step user/message (next significant event is step/start)', () => { - expect(isStepAlignedEnd(toolStepLog(), 1)).toBe(true) - }) - - it('is false at EOL when the node is inside an open (unclosed) step', () => { - // step/start then an assistant tool-call, but no step/end / tool/result yet - // (mid-flight). Ending the region on seq 3 would summarize away a tool-call - // whose result lands later → orphan. EOL + open step ⇒ not aligned. - const log: SessionEvent[] = [ - { type: 'turn/start', seq: 0, time: 0, data: { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } } }, - { type: 'step/start', seq: 1, time: 1, data: { turn: 1, step: 1 } }, - { type: 'assistant/message', seq: 2, time: 2, data: { turn: 1, step: 1, content: [ - { type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }, - ] }, ...SURFACE }, - ] - expect(isStepAlignedEnd(log, 2)).toBe(false) - }) - - it('is false at EOL when the node is inside an open step, skipping noise on the back-scan', () => { - // The open-step back-scan must skip non-boundary events (here an - // assistant/chunk) before it reaches step/start. Without the skip it would - // mis-read the chunk as the nearest "boundary" and never confirm the open step. - const log: SessionEvent[] = [ - { type: 'turn/start', seq: 0, time: 0, data: { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } } }, - { type: 'step/start', seq: 1, time: 1, data: { turn: 1, step: 1 } }, - { type: 'assistant/chunk', seq: 2, time: 2, data: { turn: 1, step: 1, chunk: { type: 'text-delta', index: 0, text: 'h' } } }, - { type: 'assistant/message', seq: 3, time: 3, data: { turn: 1, step: 1, content: [ - { type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }, - ] }, ...SURFACE }, - ] - expect(isStepAlignedEnd(log, 3)).toBe(false) - }) - - it('is true at EOL when the node is a trailing inter-step node (step already closed)', () => { - // A steering message appended after step/end, at the tail. Backward the - // nearest boundary is step/end → not in an open step → aligned. - const log: SessionEvent[] = [ - { type: 'step/start', seq: 0, time: 0, data: { turn: 1, step: 1 } }, - { type: 'assistant/message', seq: 1, time: 1, data: { turn: 1, step: 1, content: [{ type: 'text', text: 'a' }] }, ...SURFACE }, - { type: 'step/end', seq: 2, time: 2, data: { turn: 1, step: 1 } }, - { type: 'steering/message', seq: 3, time: 3, data: { turn: 1, content: [{ type: 'text', text: 's' }], source: { kind: 'user' } }, ...SURFACE }, - ] - expect(isStepAlignedEnd(log, 3)).toBe(true) - }) - - it('is true at EOL when no step ever opened (start-of-log fallback in open-step check)', () => { - // A lone surface node, no turn/step markers at all → not in an open step. - const log: SessionEvent[] = [ - { type: 'user/message', seq: 0, time: 0, data: { content: [{ type: 'text', text: 'hi' }], source: { kind: 'user' } }, ...SURFACE }, - ] - expect(isStepAlignedEnd(log, 0)).toBe(true) - }) - - it('skips noise (assistant/chunk) when scanning forward', () => { - // assistant/chunk events precede the assistant/message in a real step; the - // forward scan from an inter-step node must skip them and stop on step/start. - const log: SessionEvent[] = [ - { type: 'user/message', seq: 0, time: 0, data: { content: [{ type: 'text', text: 'go' }], source: { kind: 'user' } }, ...SURFACE }, - { type: 'step/start', seq: 1, time: 1, data: { turn: 1, step: 1 } }, - { type: 'assistant/chunk', seq: 2, time: 2, data: { turn: 1, step: 1, chunk: { type: 'text-delta', index: 0, text: 'h' } } }, - ] - // Forward from seq 0 hits step/start at seq 1 → aligned (noise after is moot). - expect(isStepAlignedEnd(log, 0)).toBe(true) - }) -}) - -describe('step-alignment on an injection turn (no step)', () => { - // An idle inject() wraps a context/message in a bare turn/start → context/message - // → turn/end with NO step/start. The context node is a free boundary both ways. - const injectionLog = (): SessionEvent[] => [ - { type: 'turn/start', seq: 0, time: 0, data: { turn: 1, trigger: { kind: 'injection', source: { kind: 'user' } } } }, - { type: 'context/message', seq: 1, time: 1, data: { content: [{ type: 'text', text: 'ctx' }], source: { kind: 'user' } }, ...SURFACE }, - { type: 'turn/end', seq: 2, time: 2, data: { turn: 1, reason: { kind: 'completed' } } }, - ] - - it('start: aligned (backward hits turn/start)', () => { - expect(isStepAlignedStart(injectionLog(), 1)).toBe(true) - }) - - it('end: aligned (forward hits turn/end)', () => { - expect(isStepAlignedEnd(injectionLog(), 1)).toBe(true) - }) -}) diff --git a/packages/core/session/tests/tool-pairing.spec.ts b/packages/core/session/tests/tool-pairing.spec.ts new file mode 100644 index 0000000000..307b0d8658 --- /dev/null +++ b/packages/core/session/tests/tool-pairing.spec.ts @@ -0,0 +1,314 @@ +import { describe, expect, it } from 'vitest' +import { CallId } from '@deepseek-ai/dsh-llm' +import { Session, SessionId, isToolPairingBalanced } from '../src/index.ts' +import type { SessionEvent, SurfaceNode } from '../src/index.ts' + +/** + * Unit coverage for the tool-pairing balance check. It decides whether a CUT in + * the surface (a gap before a given surface node, or the after-tail gap) is a + * safe edge for a collapsed region (compaction): a region must never split an + * `assistant/message`'s tool-calls from their `tool/result`s. A cut is balanced + * when no unanswered tool-call sits before it on the surface. Nodes belonging to + * no step (pre-step user message, inter-step steering, injection context) are + * pairing-neutral, so their cuts are free boundaries. + * + * The fixtures are built through a real {@link Session} so the surface linked + * list is derived exactly as production does — including the non-monotonic + * surface a `replace` op leaves (a compaction checkpoint at a high log seq + * sitting at the surface head), which is the case the abandoned log-position + * scan mis-classified. + * + * Builders mirror the agent loop's real append order: queued user messages land + * BEFORE `step/start`; within a step the order is `assistant/message` then + * `tool/result`(s); injection turns are a bare `turn/start → context/message → + * turn/end` with no step. + */ + +const SURFACE = { surfaceOp: 'append' as const } + +/** Surface nodes + log for a session, the two args the balance check takes. */ +function surfaceOf(session: Session): { nodes: readonly SurfaceNode[]; events: readonly SessionEvent[] } { + return { nodes: session.surface.nodes, events: session.events } +} + +/** The cut BEFORE the surface node at `seq` is balanced (safe region start). */ +function startBalanced(session: Session, seq: number): boolean { + const { nodes, events } = surfaceOf(session) + return isToolPairingBalanced(nodes, events, seq) +} + +/** The cut AFTER the surface node at `seq` is balanced (safe region end). */ +function endBalanced(session: Session, seq: number): boolean { + const { nodes, events } = surfaceOf(session) + const node = nodes.find(n => n.seq === seq) + if (!node) throw new Error(`seq ${seq} is not a surface node`) + return isToolPairingBalanced(nodes, events, node.next) +} + +/** Surface seq of the nth (0-based) event of a given type. */ +function seqOf(s: Session, type: SessionEvent['type'], nth = 0): number { + return s.events.filter(e => e.type === type)[nth]!.seq +} + +/** A closed turn with one closed step holding an assistant + its tool result. */ +function toolStepSession(): Session { + const s = new Session(SessionId('tool-step')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('user/message', { content: [{ type: 'text', text: 'go' }], source: { kind: 'user' } }, SURFACE) + s.append('step/start', { turn: 1, step: 1 }) + s.append('assistant/message', { + turn: 1, step: 1, + content: [ + { type: 'text', text: 'calling' }, + { type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }, + ], + }, SURFACE) + s.append('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: '{}' }) + s.append('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'out' }], isError: false }, SURFACE) + s.append('step/end', { turn: 1, step: 1 }) + s.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) + return s +} + +describe('isToolPairingBalanced — region START (cut before a node)', () => { + it('is true for a pre-step user/message (belongs to no step)', () => { + const s = toolStepSession() + expect(startBalanced(s, seqOf(s, 'user/message'))).toBe(true) + }) + + it('is true for the first surface node of a step (the assistant/message)', () => { + // The cut before the assistant is balanced — nothing unanswered precedes it. + const s = toolStepSession() + expect(startBalanced(s, seqOf(s, 'assistant/message'))).toBe(true) + }) + + it('is false for a tool/result whose assistant/message precedes it in the same step', () => { + // The cut before the tool/result has one unanswered tool-call (the + // assistant's) → starting the region here would orphan that call. + const s = toolStepSession() + expect(startBalanced(s, seqOf(s, 'tool/result'))).toBe(false) + }) + + it('is true at the surface head (nothing precedes)', () => { + const s = new Session(SessionId('lone')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('user/message', { content: [{ type: 'text', text: 'hi' }], source: { kind: 'user' } }, SURFACE) + expect(startBalanced(s, seqOf(s, 'user/message'))).toBe(true) + }) +}) + +describe('isToolPairingBalanced — region END (cut after a node)', () => { + it('is true for the last surface node of a closed step (the tool/result)', () => { + // After the tool/result the assistant's single call is answered → balanced. + const s = toolStepSession() + expect(endBalanced(s, seqOf(s, 'tool/result'))).toBe(true) + }) + + it('is false for an assistant/message with a later tool/result in the same step', () => { + // After the assistant its tool-call is still unanswered → ending here strands + // the result. + const s = toolStepSession() + expect(endBalanced(s, seqOf(s, 'assistant/message'))).toBe(false) + }) + + it('is true for a pre-step user/message', () => { + const s = toolStepSession() + expect(endBalanced(s, seqOf(s, 'user/message'))).toBe(true) + }) + + it('is false at the tail when the node is inside an open (unclosed) step', () => { + // step/start then an assistant tool-call, but no tool/result yet (mid-flight). + // The after-tail cut still has one unanswered call → not balanced. + const s = new Session(SessionId('open-step')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('step/start', { turn: 1, step: 1 }) + s.append('assistant/message', { + turn: 1, step: 1, + content: [{ type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }], + }, SURFACE) + expect(endBalanced(s, seqOf(s, 'assistant/message'))).toBe(false) + }) + + it('is true at the tail when the node is a trailing inter-step node (step already closed)', () => { + // A steering message appended after step/end, at the tail. The prior step's + // pair is balanced and steering is neutral → the after-tail cut is balanced. + const s = new Session(SessionId('trailing-steer')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('step/start', { turn: 1, step: 1 }) + s.append('assistant/message', { turn: 1, step: 1, content: [{ type: 'text', text: 'a' }] }, SURFACE) + s.append('step/end', { turn: 1, step: 1 }) + s.append('steering/message', { turn: 1, content: [{ type: 'text', text: 's' }], source: { kind: 'user' } }, SURFACE) + expect(endBalanced(s, seqOf(s, 'steering/message'))).toBe(true) + }) + + it('is true at the tail when no step ever opened', () => { + const s = new Session(SessionId('no-step')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('user/message', { content: [{ type: 'text', text: 'hi' }], source: { kind: 'user' } }, SURFACE) + expect(endBalanced(s, seqOf(s, 'user/message'))).toBe(true) + }) +}) + +describe('isToolPairingBalanced — multiple tool calls in one assistant message', () => { + // An assistant message with two tool-calls needs BOTH results before the cut + // after it is balanced — depth +2, then -1, -1. + function twoCallStep(): Session { + const s = new Session(SessionId('two-call')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('step/start', { turn: 1, step: 1 }) + s.append('assistant/message', { + turn: 1, step: 1, + content: [ + { type: 'tool-call', id: CallId('c1'), name: 'a', arguments: '{}' }, + { type: 'tool-call', id: CallId('c2'), name: 'b', arguments: '{}' }, + ], + }, SURFACE) + s.append('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: '1' }], isError: false }, SURFACE) + s.append('tool/result', { turn: 1, step: 1, callId: CallId('c2'), content: [{ type: 'text', text: '2' }], isError: false }, SURFACE) + s.append('step/end', { turn: 1, step: 1 }) + s.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) + return s + } + + it('is unbalanced after the first of two results (one call still open)', () => { + const s = twoCallStep() + expect(endBalanced(s, seqOf(s, 'tool/result', 0))).toBe(false) + }) + + it('is balanced after the second result (both calls answered)', () => { + const s = twoCallStep() + expect(endBalanced(s, seqOf(s, 'tool/result', 1))).toBe(true) + }) +}) + +describe('isToolPairingBalanced — a mid-step injection context/message', () => { + // A background task-done inject() lands a context/message INSIDE an open step, + // between the assistant (with a tool-call) and its tool/result. It is + // pairing-neutral, so the cut on EITHER side of it is unbalanced (the call is + // still open across it) — it is NOT a free boundary in this position. + function midStepInjection(): Session { + const s = new Session(SessionId('mid-inject')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('step/start', { turn: 1, step: 1 }) + s.append('assistant/message', { + turn: 1, step: 1, + content: [{ type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }], + }, SURFACE) + s.append('context/message', { content: [{ type: 'text', text: 'bg task done' }], source: { kind: 'plugin', plugin: 'tool-bash' } }, SURFACE) + s.append('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'out' }], isError: false }, SURFACE) + s.append('step/end', { turn: 1, step: 1 }) + s.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) + return s + } + + it('start cut before the mid-step context/message is unbalanced (call still open)', () => { + const s = midStepInjection() + expect(startBalanced(s, seqOf(s, 'context/message'))).toBe(false) + }) + + it('end cut after the mid-step context/message is unbalanced (call still open)', () => { + const s = midStepInjection() + expect(endBalanced(s, seqOf(s, 'context/message'))).toBe(false) + }) +}) + +describe('isToolPairingBalanced on an injection turn (no step)', () => { + // An idle inject() wraps a context/message in a bare turn/start → + // context/message → turn/end with NO step. The context node is a free boundary + // both ways (pairing-neutral, nothing open around it). + function injectionSession(): Session { + const s = new Session(SessionId('injection')) + s.append('turn/start', { turn: 1, trigger: { kind: 'injection', source: { kind: 'user' } } }) + s.append('context/message', { content: [{ type: 'text', text: 'ctx' }], source: { kind: 'user' } }, SURFACE) + s.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) + return s + } + + it('start: balanced', () => { + const s = injectionSession() + expect(startBalanced(s, seqOf(s, 'context/message'))).toBe(true) + }) + + it('end: balanced', () => { + const s = injectionSession() + expect(endBalanced(s, seqOf(s, 'context/message'))).toBe(true) + }) +}) + +describe('isToolPairingBalanced — CBR-001: a head checkpoint left by a replace op', () => { + // The case the log-position scan got wrong. After a compaction, a replacement + // user/message lands at a HIGH log seq but sits at the SURFACE head, beside + // the still-open step whose events follow it in the log. It carries no + // tool-call/result pair (just summarized prose), so it must be a balanced cut + // on BOTH sides regardless of its log neighbours. + function checkpointHeadedSession(): Session { + const s = new Session(SessionId('checkpoint')) + // A closed turn with a tool step → surface [u1, asst(call), result]. + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('step/start', { turn: 1, step: 1 }) + s.append('user/message', { content: [{ type: 'text', text: 'u1' }], source: { kind: 'user' } }, SURFACE) + s.append('assistant/message', { + turn: 1, step: 1, + content: [{ type: 'tool-call', id: CallId('c1'), name: 'bash', arguments: '{}' }], + }, SURFACE) + s.append('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'out' }], isError: false }, SURFACE) + s.append('step/end', { turn: 1, step: 1 }) + s.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) + // An OPEN turn whose step is in progress (loop fires compaction here). + s.append('turn/start', { turn: 2, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('step/start', { turn: 2, step: 1 }) + // Compaction replaces the whole turn-1 surface ([u1, asst, result]) with one + // summary user/message — appended now, so it carries a high log seq. + const u1 = seqOf(s, 'user/message') + const result = s.events.find(e => e.type === 'tool/result')!.seq + s.append('user/message', { + content: [{ type: 'text', text: 'CHECKPOINT' }], + source: { kind: 'plugin', plugin: 'compact' }, + }, { surfaceOp: { op: 'replace', start: u1, end: result } }) + // The step's own assistant/message lands AFTER the checkpoint in the log, + // still inside the open step. + s.append('assistant/message', { turn: 2, step: 1, content: [{ type: 'text', text: 'a2' }] }, SURFACE) + return s + } + + it('the head checkpoint sits at the surface head while a later surface node follows it in the log', () => { + const s = checkpointHeadedSession() + const nodes = s.surface.nodes + const checkpointSeq = nodes[0]!.seq + // The checkpoint heads the surface, yet a surface node (the open step's + // assistant) follows it in LOG order — the exact split between surface + // position and log position that the log-position scan tripped on. + const laterSurfaceInLog = s.events.find( + e => e.seq > checkpointSeq && nodes.some(n => n.seq === e.seq), + ) + expect(laterSurfaceInLog).toBeDefined() + expect(nodes[0]!.seq).toBe(checkpointSeq) + }) + + it('start cut before the head checkpoint is balanced (it is the head)', () => { + const s = checkpointHeadedSession() + expect(startBalanced(s, s.surface.nodes[0]!.seq)).toBe(true) + }) + + it('end cut after the head checkpoint is balanced (it carries no tool pair)', () => { + // This is the exact assertion the log-position scan failed: the forward log + // scan from the checkpoint reached the open step's assistant/message and + // wrongly reported mid-step. The surface balance sees a neutral node whose + // following cut closes no open call. + const s = checkpointHeadedSession() + expect(endBalanced(s, s.surface.nodes[0]!.seq)).toBe(true) + }) +}) + +describe('isToolPairingBalanced — corrupt surface guard', () => { + it('throws when a tool/result has no preceding tool-call (depth goes negative)', () => { + // A surface that opens with a tool/result (no assistant call before it) is + // structurally corrupt — surfaced loudly rather than mis-classified. + const s = new Session(SessionId('corrupt')) + s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + s.append('step/start', { turn: 1, step: 1 }) + s.append('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'x' }], isError: false }, SURFACE) + const { nodes, events } = surfaceOf(s) + expect(() => isToolPairingBalanced(nodes, events, null)).toThrow(/no matching tool-call/) + }) +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 70a1a9973a..15b61280cb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -135,6 +135,9 @@ importers: '@deepseek-ai/dsh-agent': specifier: workspace:^ version: link:../../core/agent + '@deepseek-ai/dsh-agent-loop': + specifier: workspace:^ + version: link:../../core/agent-loop '@deepseek-ai/dsh-compact': specifier: workspace:^ version: link:../compact @@ -147,6 +150,12 @@ importers: '@deepseek-ai/dsh-session': specifier: workspace:^ version: link:../../core/session + '@deepseek-ai/dsh-system-prompt': + specifier: workspace:^ + version: link:../../core/system-prompt + '@deepseek-ai/dsh-tools': + specifier: workspace:^ + version: link:../../core/tools cordis: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) diff --git a/scripts/gen-cordis-catalog.ts b/scripts/gen-cordis-catalog.ts index 7479f5306c..5bf7a344ca 100644 --- a/scripts/gen-cordis-catalog.ts +++ b/scripts/gen-cordis-catalog.ts @@ -23,8 +23,8 @@ * * The HARNESS tier (the `@deepseek-ai/dsh-*` events + services) is rendered in * full from source: signature, the `@mode` badge, and the declaration's JSDoc. - * Every harness event MUST carry an `@mode emit|waterfall|parallel` tag — the - * generator hard-errors on a missing tag, and where the signature shape is + * Every harness event MUST carry an `@mode emit|waterfall|parallel|serial` tag + * — the generator hard-errors on a missing tag, and where the signature shape is * conclusive (a trailing `next: () => …` parameter is structurally a waterfall) * it asserts the tag agrees and hard-errors on a contradiction. The INHERITED * tier (cordis core + loader/hmr/timer) is pinned vendor source a plugin author @@ -48,7 +48,7 @@ const OUT = 'docs/cordis-catalog/events-and-services.md' const FENCE = 'ts cordis-catalog' /** A dispatch mode, rendered as the badge after an event name. */ -type Mode = 'emit' | 'waterfall' | 'parallel' +type Mode = 'emit' | 'waterfall' | 'parallel' | 'serial' /** * Cross-link map: a type name that appears in a signature → the @@ -165,7 +165,7 @@ function parseJsDoc(raw: string): { doc: string; mode: Mode | null } { para = [] } for (const line of inner) { - const m = /^@mode\s+(emit|waterfall|parallel)\s*$/.exec(line) + const m = /^@mode\s+(emit|waterfall|parallel|serial)\s*$/.exec(line) if (m) { mode = m[1] as Mode; continue } if (line.startsWith('@')) { flushPara(); continue } // other tags end the prose if (line.trim() === '') { flushPara(); continue } @@ -223,11 +223,11 @@ export function collectEvents(scanRoot: string = root): EventEntry[] { const { doc, mode } = parseJsDoc(rawJsDoc(text, member)) const src = pointer(rel, sf, member) if (!mode) { - throw new Error(`gen-cordis-catalog: event '${name}' (${src}) is missing an @mode tag. Add '@mode emit|waterfall|parallel' to its JSDoc (see AGENTS.md).`) + throw new Error(`gen-cordis-catalog: event '${name}' (${src}) is missing an @mode tag. Add '@mode emit|waterfall|parallel|serial' to its JSDoc (see AGENTS.md).`) } // Conclusive structural check: a trailing `next: () => …` parameter is a - // waterfall. (emit vs parallel is not structurally distinguishable, so - // it is trusted from the tag.) + // waterfall. (emit vs parallel vs serial is not structurally + // distinguishable, so it is trusted from the tag.) const last = member.parameters.at(-1) const hasNext = !!last && last.name.getText(sf) === 'next' if (hasNext && mode !== 'waterfall') { @@ -394,7 +394,7 @@ function render(events: EventEntry[], services: ServiceEntry[]): string { '', '## Events', '', - `Dispatch modes: **emit** (fire-and-forget), **waterfall** (each listener gets \`next()\` and may transform or veto — see [waterfall semantics](../architecture.md#cordis-waterfall-semantics-important)), **parallel** (awaited fan-out, no veto). The harness declares ${events.length} events across ${new Set(events.map(e => e.scope)).size} scopes.`, + `Dispatch modes: **emit** (fire-and-forget), **waterfall** (each listener gets \`next()\` and may transform or veto — see [waterfall semantics](../architecture.md#cordis-waterfall-semantics-important)), **parallel** (awaited fan-out, no veto), **serial** (awaited, in registration order, no veto). The harness declares ${events.length} events across ${new Set(events.map(e => e.scope)).size} scopes.`, '', ] const scopes = [...new Set(events.map(e => e.scope))].sort()