From add59a3336f0ab61c55499418081aab76cb550f8 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 15 Jun 2026 23:53:47 +0800 Subject: [PATCH] feat(agent-loop): surface max-tokens as a distinct turn-end reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a `max-tokens` variant to `TurnEndReasonMap` and carry the model finish reason up from `runStep` to `runTurn`, applying the rule "any max-tokens step in the turn surfaces as max-tokens" (disposed/aborted/ error still take precedence). This lets consumers distinguish a clean stop from a truncated one — the contract RFC 010's ACP bridge maps to the `max_tokens` stop reason. Also add an AGENTS.md rule: write an ADR when (and only when) a PR makes a durable, contested, surprising decision. --- AGENTS.md | 2 + docs/architecture.md | 2 + packages/agent-loop/README.md | 2 +- packages/agent-loop/src/loop.ts | 41 ++++++++++++- packages/agent-loop/tests/loop.spec.ts | 72 ++++++++++++++++++++++- packages/agent-loop/tests/mock-adapter.ts | 15 +++++ packages/session/src/types.ts | 14 +++++ packages/session/tests/session.spec.ts | 13 ++++ 8 files changed, 156 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ddda1af98e..f11b1943e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,6 +125,8 @@ In the **core** packages (`packages/llm`, `packages/tools`, `packages/agent`, `p Verbose documentation is fine **as long as docs and code stay strictly in sync**. Out-of-sync docs are worse than no docs. **When you change code, update its docs in the SAME change** — grep the package README and the module/JSDoc comments for the old behavior (config keys, defaults, error codes, wire field names, event names) and fix every hit. CI runs `yarn doc-sync` (`doc-typecheck` + `verify-event-taxonomy`), which typechecks every fenced `ts` block in `README.md`, `docs/**/*.md`, and `packages/*/README.md` and verifies the event-taxonomy table against source — but that scope does NOT cover `AGENTS.md`, `packages/AGENTS.md`, or `packages/README.md`, nor does it catch prose drift (config keys, defaults, error codes), so keeping those in sync remains on the author. Every module has a module-level doc comment explaining its role. Every exported class, interface, type, function, and non-obvious method has a JSDoc that explains semantics (not just the name) — contracts (what events fire when), disposal behavior, error behavior, and extension intent. Internal helpers get docs only where non-obvious. Prefer one-liners when one line suffices. +**Write an ADR when — and only when — a PR makes a decision that is durable, contested, and surprising.** ADRs (`docs/adr/`) record the *why* behind choices a future reader would otherwise re-litigate (the vendoring policy, event-sourcing, the schema DSL are the existing examples). A PR that introduces such a decision — a new third-party runtime dependency over the vendoring default, a cross-package contract, a security/isolation model, a deviation from a documented architecture rule — writes the ADR **in the same PR**, and links it from the relevant code/RFC. A PR whose changes are mechanical, self-evident, or already covered by an existing ADR/RFC needs none — do not manufacture an ADR for a routine change. When unsure, the test is: would a competent maintainer six months from now ask "why was it done this way?" and be unable to answer from the code alone? If yes, write it. + **Markdown is not hard-wrapped**: write one line per paragraph and let the editor soft-wrap. Hard line breaks mid-paragraph make docs harder to edit and diff — a one-word change reflows and re-diffs the whole paragraph. This applies to prose only: leave fenced code blocks, tables, and list structure intact (a wrapped list item folds to one line per bullet). Code comments / JSDoc are exempt — they stay under the linter's column limit. **Editing these instructions**: `AGENTS.md` is the real file; `CLAUDE.md` is a symlink to it (at the repo root and in `packages/`). Always edit `AGENTS.md` — never write through the `CLAUDE.md` symlink or replace it with a regular file. diff --git a/docs/architecture.md b/docs/architecture.md index 829283b729..8d94bd6da6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -165,6 +165,8 @@ forever: Error containment: a throwing `agent/turn-continuation` listener or a broken step ends the **turn** with an `error` event (appended INSIDE the turn, before `turn/end`) — never the driver loop. An adapter that ends its stream with a `finish {kind:'error'}` or `{kind:'aborted'}` chunk (the in-band error path, for adapters that can't throw mid-stream) is likewise translated into a step error, so the turn ends `error`/`aborted` instead of logging a normal `completed` assistant message. `abort()` is honored mid-stream **and** between tool calls; disposal mid-turn ends the turn with reason `disposed` and emits `agent/status('disposed')`. +Turn-end reasons: a turn ends with one `TurnEndReason` — `completed`, `aborted`, `error`, `disposed`, or `max-tokens`. `max-tokens` mirrors the model-call `FinishReason` of the same name (DeepSeek's `length`): a step that hit the output-token ceiling makes the turn end `max-tokens` rather than `completed`, by the rule *any `max-tokens` step in the turn surfaces as `max-tokens`* (a continuation plugin may run further steps after one, but the cut-short fact wins; the `disposed`/`aborted`/`error` outcomes still take precedence). This lets a consumer distinguish a clean stop from a truncated one (the ACP bridge maps it to the `max_tokens` stop reason). `TurnEndReason` is merge-extensible; `refusal` and `max_turn_requests` are the next variants to add when an adapter/loop first emits them. + A failure that happens once the turn is already closed has no in-turn position for a session `error` event (appending one after `turn/end` would put it past the persistence commit boundary, where it is dropped as a crash tail — ADR 0017). So a rejecting `session/flush` (the post-`turn/end` durability checkpoint) and a throwing `agent/turn-end` listener are reported via `agent/error` + the logger only, NOT as a session event; the turn stays balanced and the persistence backend keeps its buffered events for the next flush. **Turn-enclosure invariant**: every session event lives inside a turn (between a `turn/start` and its `turn/end`). The loop appends queued `user/message` events *after* `turn/start`, and an idle `agent.inject()` wraps its `context/message` in a one-shot `injection` turn. This makes the turn the single durability/replay boundary: a persistence backend can treat anything after the last `turn/end` as an interrupted-crash tail without risking the loss of legitimately-recorded between-turn context. The `dsh-invariants` plugin enforces it in dev (a message event outside an open turn throws). See ADR 0017. diff --git a/packages/agent-loop/README.md b/packages/agent-loop/README.md index 8dd8c14808..f8362b3cd7 100644 --- a/packages/agent-loop/README.md +++ b/packages/agent-loop/README.md @@ -64,7 +64,7 @@ forever: idle unless more queued ``` -Error containment: a throwing plugin ends the **turn**, never the loop. Dispose mid-turn emits `agent/status('disposed')` and ends with reason `disposed`. +Error containment: a throwing plugin ends the **turn**, never the loop. Dispose mid-turn emits `agent/status('disposed')` and ends with reason `disposed`. A step that hits the model's output-token ceiling makes the turn end `max-tokens` (the rule: any `max-tokens` step in the turn surfaces as `max-tokens`; `disposed`/`aborted`/`error` still take precedence) — distinct from a clean `completed` stop. ### What is NOT here diff --git a/packages/agent-loop/src/loop.ts b/packages/agent-loop/src/loop.ts index d2b1ed270b..74ccb0f71e 100644 --- a/packages/agent-loop/src/loop.ts +++ b/packages/agent-loop/src/loop.ts @@ -71,6 +71,31 @@ function errorData(err: CodedError): { message: string; code?: string } { return { message: err.message, ...typeof err.code === 'string' ? { code: err.code } : {} } } +/** + * The turn-end contribution of a step's *successful* finish, or `undefined` + * when the step finished ordinarily (a plain `completed`). + * + * {@link finishError} has already converted `error`/`aborted` finishes into + * thrown step errors, so the finishes that reach here are `stop`, + * `tool-calls`, `max-tokens`, or a future merge-extensible kind. Only + * `max-tokens` carries forward as a distinct {@link TurnEndReason}: a step that + * hit the output-token ceiling ended the turn cut-short rather than by the + * model's choice. `stop`/`tool-calls`/unknown kinds contribute nothing beyond + * the default `completed`. {@link runTurn} applies this with the rule "any + * `max-tokens` step in the turn makes the turn end `max-tokens`". + */ +function stepFinishReason(finish: FinishReason): TurnEndReason | undefined { + switch (finish.kind) { + case 'max-tokens': + return { kind: 'max-tokens' } + // stop / tool-calls / plugin-added kinds → no turn-end contribution + // beyond the default `completed`. FinishReason is merge-extensible, so a + // default (not assertNever) handles unknown kinds as ordinary success. + default: + return undefined + } +} + /** * Ambient handles the loop driver receives from the agent. Decouples the * pure function `runLoop` from the mutable LoopAgent fields, making the @@ -294,7 +319,7 @@ async function runTurn(ctx: Context, agent: LoopAgent, handle: LoopHandle, turn: const abort = new AbortController() handle.setAbort(abort) - let stepOutcome: { hadToolCalls: boolean } | { error: Error } + let stepOutcome: { hadToolCalls: boolean; finish: FinishReason } | { error: Error } try { stepOutcome = await runStep(ctx, agent, turn, step, abort.signal) } catch (error: unknown) { @@ -320,6 +345,16 @@ async function runTurn(ctx: Context, agent: LoopAgent, handle: LoopHandle, turn: break } + // The successful step's finish reason carries forward: a `max-tokens` + // step makes the whole turn end `max-tokens` (RFC 010's rule "any + // max-tokens step surfaces as max-tokens"). `stepFinishReason` returns + // `max-tokens` or `undefined`, so a later ordinary step never resets a + // max-tokens turn back to completed, and a never-truncated turn keeps the + // default `completed`. The disposal/abort/error branches above and the + // continuation-window disposal check below override this — they win. + const stepReason = stepFinishReason(stepOutcome.finish) + if (stepReason) reason = stepReason + // Steering that arrived during streaming/tool execution. const steered = drainSteering(ctx, agent, turn) @@ -423,7 +458,7 @@ async function runStep( turn: number, step: number, signal: AbortSignal, -): Promise<{ hadToolCalls: boolean }> { +): Promise<{ hadToolCalls: boolean; finish: FinishReason }> { const { session, options } = agent // --- Request assembly --- @@ -517,7 +552,7 @@ async function runStep( /* v8 ignore stop */ } - return { hadToolCalls: toolCalls.length > 0 } + return { hadToolCalls: toolCalls.length > 0, finish: assembler.finish } } /** The last turn number in a (possibly seeded) session log, or 0. */ diff --git a/packages/agent-loop/tests/loop.spec.ts b/packages/agent-loop/tests/loop.spec.ts index 94a7181771..4d8cdefe6c 100644 --- a/packages/agent-loop/tests/loop.spec.ts +++ b/packages/agent-loop/tests/loop.spec.ts @@ -6,7 +6,7 @@ import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' import AgentRegistry from '@deepseek-ai/dsh-agent' import AgentLoop, { LoopAgent } from '@deepseek-ai/dsh-agent-loop' -import { MockAdapter, textResponse, toolCallResponse } from './mock-adapter.ts' +import { MockAdapter, maxTokensResponse, textResponse, toolCallResponse } from './mock-adapter.ts' async function harness(adapter: MockAdapter) { const ctx = new Context() @@ -336,6 +336,76 @@ describe('agent loop', () => { expect(reasons).toEqual([{ kind: 'aborted', reason: 'user interrupt' }]) }) + it('surfaces max-tokens as the turn-end reason when the last step is cut off', async () => { + // A single step that ends with a max-tokens finish (no tool calls): the + // turn stops by default and ends max-tokens, not completed. + const adapter = new MockAdapter([maxTokensResponse('truncat')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create('a1', { model: 'mock' }) + + const reasons: TurnEndReason[] = [] + ctx.on('agent/turn-end', (_agent, _turn, reason) => void reasons.push(reason)) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + expect(adapter.requests).toHaveLength(1) + expect(reasons).toEqual([{ kind: 'max-tokens' }]) + // and the reason is recorded in the log's turn/end event + const turnEnd = agent.session.events.findLast(e => e.type === 'turn/end') + expect(turnEnd!.data.reason).toEqual({ kind: 'max-tokens' }) + }) + + it('a max-tokens step earlier in a turn still surfaces as max-tokens after a later completed step', async () => { + // Step 1 is cut off (max-tokens, no tool calls → would stop by default), so + // continuation must be FORCED to reach step 2 which finishes normally + // (stop). The rule "any max-tokens step surfaces as max-tokens" means the + // turn ends max-tokens even though the LAST step completed cleanly. + const adapter = new MockAdapter([ + maxTokensResponse('first half'), + textResponse('second half'), + ]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create('a1', { model: 'mock' }) + + let steps = 0 + ctx.on('agent/step-end', () => void steps++) + // Force exactly one continuation (step 1 → step 2), then defer to default + // (step 2 is a plain stop with no tool calls → stops). + ctx.on('agent/turn-continuation', async (_agent, _turn, _defaultDecision, next) => { + if (steps < 2) return true + return next() + }) + + const reasons: TurnEndReason[] = [] + ctx.on('agent/turn-end', (_agent, _turn, reason) => void reasons.push(reason)) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + expect(steps).toBe(2) + expect(adapter.requests).toHaveLength(2) + expect(reasons).toEqual([{ kind: 'max-tokens' }]) + }) + + it('a completed step after no max-tokens keeps the turn completed (max-tokens does not leak across turns)', async () => { + // Two consecutive turns: turn 1 is cut off (max-tokens), turn 2 is a clean + // stop. The per-turn reason must be independent — turn 2 ends completed. + const adapter = new MockAdapter([maxTokensResponse('cut'), textResponse('clean')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create('a1', { model: 'mock' }) + + const reasons: TurnEndReason[] = [] + ctx.on('agent/turn-end', (_agent, _turn, reason) => void reasons.push(reason)) + + send(agent, 'first') + await waitForIdle(ctx, agent) + send(agent, 'second') + await waitForIdle(ctx, agent) + + expect(reasons).toEqual([{ kind: 'max-tokens' }, { kind: 'completed' }]) + }) + it('chains queued messages into consecutive turns', async () => { const adapter = new MockAdapter([textResponse('first'), textResponse('second')]) const ctx = await harness(adapter) diff --git a/packages/agent-loop/tests/mock-adapter.ts b/packages/agent-loop/tests/mock-adapter.ts index f6cb47cda3..4aaff72415 100644 --- a/packages/agent-loop/tests/mock-adapter.ts +++ b/packages/agent-loop/tests/mock-adapter.ts @@ -12,6 +12,21 @@ export function textResponse(text: string): StreamChunk[] { ] } +/** + * Like {@link textResponse} but the stream ends with a `max-tokens` finish — + * the model was cut off at the output-token ceiling (DeepSeek's `length`). + * Used to exercise the turn-end `max-tokens` surfacing rule. + */ +export function maxTokensResponse(text: string): StreamChunk[] { + return [ + { type: 'block-start', index: 0, blockType: 'text' }, + ...Array.from(text, (char): StreamChunk => ({ type: 'text-delta', index: 0, text: char })), + { type: 'block-end', index: 0, block: { type: 'text', text } }, + { type: 'usage', usage: { inputTokens: 10, outputTokens: text.length } }, + { type: 'finish', reason: { kind: 'max-tokens' } }, + ] +} + export function toolCallResponse(rawCallId: string, name: string, args: object, text?: string): StreamChunk[] { const callId = CallId(rawCallId) const argumentsJson = JSON.stringify(args) diff --git a/packages/session/src/types.ts b/packages/session/src/types.ts index 197e251b59..b0c3ed08bb 100644 --- a/packages/session/src/types.ts +++ b/packages/session/src/types.ts @@ -93,12 +93,26 @@ export type TurnTrigger = TurnTriggerMap[keyof TurnTriggerMap] /** * Why a turn ended. * Merge-extensible sum type. + * + * `max-tokens` mirrors the model-call `FinishReasonMap` variant (DeepSeek's + * `length`): the turn ended because a step hit the output-token ceiling, not + * because the model chose to stop. The agent-loop surfaces it via the rule + * "any `max-tokens` step in the turn makes the turn end `max-tokens`" (a + * continuation plugin can run further steps after one, but the cut-short fact + * still wins). It is distinct from `completed` so a consumer (e.g. the ACP + * bridge mapping to `StopReason: 'max_tokens'`) can tell a clean stop from a + * truncated one. The next variants to add — when an adapter/loop first emits + * them — are `refusal` and `max_turn_requests` (both named by RFC 010 as ACP + * stop reasons); no current adapter produces a `refusal` finish (unknown + * DeepSeek finish reasons collapse to `error`), so it is deliberately omitted + * until one does. */ export interface TurnEndReasonMap { completed: { kind: 'completed' } aborted: { kind: 'aborted'; reason?: string } error: { kind: 'error'; message: string; code?: string } disposed: { kind: 'disposed' } + 'max-tokens': { kind: 'max-tokens' } } export type TurnEndReason = TurnEndReasonMap[keyof TurnEndReasonMap] diff --git a/packages/session/tests/session.spec.ts b/packages/session/tests/session.spec.ts index 50dc9d5907..ac40a8ea3f 100644 --- a/packages/session/tests/session.spec.ts +++ b/packages/session/tests/session.spec.ts @@ -26,6 +26,19 @@ describe('Session', () => { expect(messages[2]!.content[0]).toMatchObject({ type: 'tool-result', toolCallId: CallId('c1') }) }) + it('accepts and round-trips a max-tokens turn/end reason', () => { + // The max-tokens TurnEndReason variant carries no extra data, so it must + // append and persist like any other reason (JSON-serializable, no fields). + const session = new Session(SessionId('s1')) + session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + session.append('turn/end', { turn: 1, reason: { kind: 'max-tokens' } }) + + const turnEnd = session.events.findLast(e => e.type === 'turn/end')! + expect(turnEnd.data.reason).toEqual({ kind: 'max-tokens' }) + // survives a structuredClone (the persistence-serialization boundary) + expect(structuredClone(turnEnd.data.reason)).toEqual({ kind: 'max-tokens' }) + }) + it('renders context and steering messages as tagged synthetic user content', () => { const session = new Session(SessionId('s2')) session.append('context/message', {