review: drop the structured-output nudge; FIXME the context-global registry constraint
Two human review directives: - No re-prompt. A structured child that finishes a turn cleanly without calling structured_output settles error to the parent immediately — readResult already carried that mapping; the nudge loop only delayed it. Deletes the loop, its cancellation-window guard, STRUCTURED_OUTPUT_NUDGE, and the structuredNudgeRetries Config on both backends. - FIXME in the structured module doc: per-agent/per-session tool registry and prompt assembly would dissolve the final-assembly enforcement dance (the placeholder tool, the swap, the strip, the global-registration lifetime).
This commit is contained in:
@@ -10,14 +10,14 @@ Runs a child as a child [`Agent`](../../core/agent) on the same cordis context (
|
||||
|
||||
1. computes child depth = `depthOf(parent) + 1`; if `request.maxDepth` is set and exceeded, throws `SubagentDepthError` (the `depthLimit` capability); a `request.outputSchema` is asserted against the supported subset (`assertSupportedOutputSchema` from [dsh-tools](../../core/tools/README.md)) before any child exists;
|
||||
2. creates a child via `ctx.agents.create` with a fresh `AgentId`/`SessionId`, the parent's `cwd` + `parentSession` lineage, the optional `options.seed` (fork's completed-turn prefix; omitted for a fresh child), and `agentOptions` (the child inherits the **parent's model** by default — a child with no model can't run — overridable via `request.agentOptions.model`; the deployment persona needs no inheritance — it is a context-wide prompt section);
|
||||
3. drives the one-shot: `child.send(prompt)` then `await child.whenIdle()` (ordering matters — `send` enqueues synchronously, so `whenIdle` observes the queued work and resolves on the child's `running → idle` transition, never before the turn starts); a structured child that finished a turn CLEANLY without calling `structured_output` is re-prompted (a nudge — a fresh turn) up to `options.structuredNudgeRetries` times;
|
||||
3. drives the one-shot: `child.send(prompt)` then `await child.whenIdle()` (ordering matters — `send` enqueues synchronously, so `whenIdle` observes the queued work and resolves on the child's `running → idle` transition, never before the turn starts); there is deliberately NO re-prompt for a structured child that finished cleanly without calling `structured_output` — the shortfall maps to an `error` result for the parent;
|
||||
4. reads the result, scoped to the child's OWN events (everything at or after `seedLength`, so a seeded child that produced no message of its own never returns the seeded parent's last message): the last `assistant/message` content (deep-cloned — the log is frozen) and the last `turn/end.reason` mapped to a `SubagentStopReason`. A structured run surfaces the captured value as `result.structured`; a structured child that finished cleanly WITHOUT ever capturing settles `error` (a clean finish without the demanded result is a failure, not a success with a missing field).
|
||||
|
||||
`dispose()` delegates to `AgentHandle.dispose()` (stop loop → await quiescence → remove session); `cancel()` cancels the child's in-flight turn. A cancel landing before any `turn/end` (the pre-turn window) still settles `aborted`, honoring the cancel contract rather than the generic no-turn `error`.
|
||||
|
||||
### `InProcessRunOptions`
|
||||
|
||||
`{ providerName: string; seed?: SessionEvent[]; structuredNudgeRetries: number }` — the per-backend inputs: the provider name (for error context), the optional child-session seed, and the structured-run nudge budget (REQUIRED, resolved from the backend's validated Config — the driver never fills it with a hidden default).
|
||||
`{ providerName: string; seed?: SessionEvent[] }` — the per-backend inputs: the provider name (for error context) and the optional child-session seed.
|
||||
|
||||
### Structured output: `acquireStructuredRuntime(ctx): StructuredAcquisition`
|
||||
|
||||
|
||||
@@ -22,7 +22,6 @@ import { assertSupportedOutputSchema } from '@deepseek-ai/dsh-tools'
|
||||
import type { SubagentResult, SubagentRun, SubagentStartRequest, SubagentStopReason } from '@deepseek-ai/dsh-subagent'
|
||||
import {
|
||||
acquireStructuredRuntime,
|
||||
STRUCTURED_OUTPUT_NUDGE,
|
||||
type StructuredAcquisition,
|
||||
} from './structured.ts'
|
||||
|
||||
@@ -30,7 +29,6 @@ export {
|
||||
acquireStructuredRuntime,
|
||||
STRUCTURED_OUTPUT_TOOL,
|
||||
STRUCTURED_OUTPUT_INSTRUCTION,
|
||||
STRUCTURED_OUTPUT_NUDGE,
|
||||
type StructuredAcquisition,
|
||||
} from './structured.ts'
|
||||
|
||||
@@ -90,13 +88,6 @@ export interface InProcessRunOptions {
|
||||
* parent's log (FORK), or `undefined` for a fresh child (SPAWN).
|
||||
*/
|
||||
readonly seed?: SessionEvent[]
|
||||
/**
|
||||
* How many times a structured run re-prompts a child that finished a turn
|
||||
* cleanly WITHOUT calling `structured_output` (see the structured module).
|
||||
* REQUIRED, resolved from the backend's validated Config — per the explicit-
|
||||
* defaulting rule, the driver never fills it with a hidden fallback.
|
||||
*/
|
||||
readonly structuredNudgeRetries: number
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -178,7 +169,7 @@ export function startInProcessRun(
|
||||
let cancelled = false
|
||||
// An accessor, not an inline read: `cancelled` mutates from closures (the
|
||||
// abort listener, run.cancel), which control-flow narrowing cannot see — an
|
||||
// inline `!cancelled` in the nudge condition reads as always-true.
|
||||
// inline read at the result mapping would narrow to the initializer.
|
||||
const isCancelled = (): boolean => cancelled
|
||||
const requestCancel = (reason: string): void => {
|
||||
cancelled = true
|
||||
@@ -196,28 +187,9 @@ export function startInProcessRun(
|
||||
if (request.signal?.aborted) return { output: [], stopReason: 'aborted' }
|
||||
child.send(request.prompt)
|
||||
await child.whenIdle()
|
||||
if (structured) {
|
||||
// Nudge loop: a child that finished a turn CLEANLY without calling
|
||||
// structured_output gets re-prompted, up to the backend-configured
|
||||
// retry count. An errored/aborted turn is not nudged — its failure is
|
||||
// the honest result (a cancelled turn ends `aborted`, and a pre-turn
|
||||
// cancel leaves no `turn/end` at all, so neither reads `completed`).
|
||||
// `!cancelled` closes the remaining window: a cancel landing AFTER a
|
||||
// clean turn end clears nothing — `child.cancel()` only kills
|
||||
// queued/running work — so without it the next `send` would spend a
|
||||
// fresh post-cancellation turn; the condition re-evaluates after
|
||||
// every `whenIdle()`, so a mid-nudge cancel stops the loop at the
|
||||
// next boundary too.
|
||||
let nudges = options.structuredNudgeRetries
|
||||
while (
|
||||
!isCancelled() && structured.captured(child) === undefined && nudges > 0
|
||||
&& lastOwnTurnEnd(child, seedLength)?.data.reason.kind === 'completed'
|
||||
) {
|
||||
nudges -= 1
|
||||
child.send([{ type: 'text', text: STRUCTURED_OUTPUT_NUDGE }])
|
||||
await child.whenIdle()
|
||||
}
|
||||
}
|
||||
// Deliberately NO re-prompt when a structured child finishes cleanly
|
||||
// without calling structured_output: readResult maps that to `error` —
|
||||
// the shortfall goes to the parent instead of buying extra model turns.
|
||||
return readResult(child, seedLength, isCancelled(), structured ? { captured: structured.captured(child) } : undefined)
|
||||
} finally {
|
||||
request.signal?.removeEventListener('abort', onAbort)
|
||||
@@ -241,12 +213,6 @@ export function startInProcessRun(
|
||||
}
|
||||
}
|
||||
|
||||
/** The child's OWN last `turn/end` event (events at or after `seedLength`), if any. */
|
||||
function lastOwnTurnEnd(child: Agent, seedLength: number): SessionEvent<'turn/end'> | undefined {
|
||||
return child.session.events.slice(seedLength)
|
||||
.findLast((e): e is SessionEvent<'turn/end'> => e.type === 'turn/end')
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a settled child's terminal result from its session log, scoped to the
|
||||
* child's OWN events (everything at or after `seedLength` — fork seeds the
|
||||
|
||||
@@ -21,6 +21,13 @@
|
||||
* returning a replacement assembly — see the waterfall composition caveat in
|
||||
* docs/architecture.md.)
|
||||
*
|
||||
* FIXME: the whole enforcement dance above exists because the tool registry
|
||||
* and prompt assembly are context-global. If they become per-agent or
|
||||
* per-session scoped, a structured run just registers its own schema'd tool on
|
||||
* the child's scope and this module reduces to the capture tool plus the
|
||||
* turn-stop — no placeholder, no final-assembly swap, no strip-for-everyone-
|
||||
* else, no global-registration lifetime dance.
|
||||
*
|
||||
* A companion `agent/turn-continuation` listener stops a child's turn once its
|
||||
* output is captured — without it, the loop's default "had tool calls ⇒
|
||||
* continue" buys a wasted extra model step per structured child. It is also
|
||||
@@ -65,11 +72,6 @@ export const STRUCTURED_OUTPUT_INSTRUCTION
|
||||
+ `\`${STRUCTURED_OUTPUT_TOOL}\` tool with arguments matching its parameter schema exactly. `
|
||||
+ 'Do not finish with a plain text answer: only the tool call counts as your result.'
|
||||
|
||||
/** The nudge sent when a structured child finishes cleanly without calling the tool. */
|
||||
export const STRUCTURED_OUTPUT_NUDGE
|
||||
= `You finished without calling \`${STRUCTURED_OUTPUT_TOOL}\`. `
|
||||
+ `Call \`${STRUCTURED_OUTPUT_TOOL}\` now with your final result matching its parameter schema.`
|
||||
|
||||
/** One structured run's state: the schema to enforce and the captured value, once recorded. */
|
||||
interface RunState {
|
||||
readonly schema: StructuredOutputSchema
|
||||
|
||||
@@ -32,7 +32,7 @@ const SCHEMA: StructuredOutputSchema = {
|
||||
* structured runtime at apply, exactly as shipped). The mock model script
|
||||
* drives the child's structured_output calls.
|
||||
*/
|
||||
async function setup(script: Script, options?: { nudges?: number; withFork?: boolean }) {
|
||||
async function setup(script: Script, options?: { withFork?: boolean }) {
|
||||
const ctx = new Context()
|
||||
const adapter = new MockAdapter(script)
|
||||
await ctx.plugin(LlmService)
|
||||
@@ -43,9 +43,9 @@ async function setup(script: Script, options?: { nudges?: number; withFork?: boo
|
||||
await ctx.plugin(Invariants)
|
||||
await ctx.plugin(AgentLoop, { agents: [] })
|
||||
await ctx.plugin(SubagentService)
|
||||
const fiber = await ctx.plugin(spawn, { providerName: 'spawn', structuredNudgeRetries: options?.nudges ?? 1 })
|
||||
const fiber = await ctx.plugin(spawn, { providerName: 'spawn' })
|
||||
const forkFiber = options?.withFork
|
||||
? await ctx.plugin(fork, { providerName: 'fork', structuredNudgeRetries: options?.nudges ?? 1 })
|
||||
? await ctx.plugin(fork, { providerName: 'fork' })
|
||||
: undefined
|
||||
ctx.llm.registerAdapter(['mock'], adapter)
|
||||
const parent = ctx.agentLoop.create(AgentId('parent'), { model: 'mock' })
|
||||
@@ -215,47 +215,25 @@ describe('in-process structured output', () => {
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('nudges a child that finished cleanly without calling the tool, then captures', async () => {
|
||||
const { ctx, parent } = await setup([
|
||||
textResponse('here is my answer in prose'),
|
||||
toolCallResponse('c1', STRUCTURED_OUTPUT_TOOL, { answer: 3 }),
|
||||
])
|
||||
const run = ctx.subagents.start('spawn', structuredRequest(parent))
|
||||
const result = await run.result
|
||||
expect(result.structured).toEqual({ answer: 3 })
|
||||
expect(result.stopReason).toBe('completed')
|
||||
// The nudge is a real user-visible message in the child's log.
|
||||
const child = ctx.agents.get(run.id)!
|
||||
const users = child.session.events.filter(e => e.type === 'user/message')
|
||||
expect(users.length).toBe(2)
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('settles error when the nudges run out without a capture', async () => {
|
||||
it('a clean finish without a capture is an immediate error to the parent — deliberately NO re-prompt', async () => {
|
||||
const { ctx, parent, adapter } = await setup([
|
||||
textResponse('prose only'),
|
||||
textResponse('still prose'),
|
||||
], { nudges: 1 })
|
||||
textResponse('here is my answer in prose'),
|
||||
textResponse('MUST NOT BE CONSUMED'),
|
||||
])
|
||||
const run = ctx.subagents.start('spawn', structuredRequest(parent))
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('error')
|
||||
expect(result.structured).toBeUndefined()
|
||||
expect(adapter.requests.length).toBe(2)
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('zero nudge retries fails immediately after the first clean prose finish', async () => {
|
||||
const { ctx, parent, adapter } = await setup([textResponse('prose')], { nudges: 0 })
|
||||
const run = ctx.subagents.start('spawn', structuredRequest(parent))
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('error')
|
||||
// Exactly one model request and one user message: no nudge turn exists.
|
||||
expect(adapter.requests.length).toBe(1)
|
||||
const child = ctx.agents.get(run.id)!
|
||||
expect(child.session.events.filter(e => e.type === 'user/message').length).toBe(1)
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('a child that errored is NOT nudged (its failure is the honest result)', async () => {
|
||||
it('an errored child keeps its honest error result (no capture expected)', async () => {
|
||||
// Script exhaustion on the first call → the child turn errors.
|
||||
const { ctx, parent, adapter } = await setup([], { nudges: 3 })
|
||||
const { ctx, parent, adapter } = await setup([])
|
||||
const run = ctx.subagents.start('spawn', structuredRequest(parent))
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('error')
|
||||
@@ -263,22 +241,17 @@ describe('in-process structured output', () => {
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('a cancel landing after a clean turn end stops the nudge loop: no post-cancellation turn is spent', async () => {
|
||||
const { ctx, parent, adapter } = await setup([textResponse('prose, no capture')], { nudges: 3 })
|
||||
it('a cancel landing after a clean capture-less turn settles aborted, not error', async () => {
|
||||
const { ctx, parent } = await setup([textResponse('prose, no capture')])
|
||||
const run = ctx.subagents.start('spawn', structuredRequest(parent))
|
||||
const child = ctx.agents.get(run.id)!
|
||||
// Cancel synchronously inside the first turn's end recording — after the
|
||||
// turn reads `completed`, before the nudge continuation resumes. The turn
|
||||
// state alone cannot see this cancel (`child.cancel()` only clears
|
||||
// queued/running work), so without the loop's own cancelled check the
|
||||
// next send would spend a fresh child turn after the caller cancelled.
|
||||
// Cancel synchronously inside the turn's end recording: the cancel
|
||||
// contract outranks the schema shortfall, so the result maps to aborted.
|
||||
ctx.on('session/event', (session, event) => {
|
||||
if (session === child.session && event.type === 'turn/end') run.cancel('cancelled between turn end and nudge')
|
||||
if (session === child.session && event.type === 'turn/end') run.cancel('cancelled at turn end')
|
||||
})
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('aborted')
|
||||
// Exactly one model request: the nudge turn never ran.
|
||||
expect(adapter.requests.length).toBe(1)
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
|
||||
@@ -51,7 +51,7 @@ describe('depthOf', () => {
|
||||
describe('startInProcessRun', () => {
|
||||
it('drives a fresh child (no seed) to completion and returns its output', async () => {
|
||||
const { ctx, parent } = await setup([textResponse('driver child answer')])
|
||||
const run = startInProcessRun(ctx, { prompt: [{ type: 'text', text: 'do X' }], parent }, { providerName: 'spawn', structuredNudgeRetries: 1 })
|
||||
const run = startInProcessRun(ctx, { prompt: [{ type: 'text', text: 'do X' }], parent }, { providerName: 'spawn' })
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('completed')
|
||||
expect(text(result.output)).toBe('driver child answer')
|
||||
@@ -61,7 +61,7 @@ describe('startInProcessRun', () => {
|
||||
|
||||
it('throws SubagentDepthError when the child would exceed maxDepth', async () => {
|
||||
const { ctx, parent } = await setup([])
|
||||
expect(() => startInProcessRun(ctx, { prompt: [{ type: 'text', text: 'p' }], parent, maxDepth: 0 }, { providerName: 'spawn', structuredNudgeRetries: 1 }))
|
||||
expect(() => startInProcessRun(ctx, { prompt: [{ type: 'text', text: 'p' }], parent, maxDepth: 0 }, { providerName: 'spawn' }))
|
||||
.toThrow(SubagentDepthError)
|
||||
})
|
||||
|
||||
@@ -73,7 +73,7 @@ describe('startInProcessRun', () => {
|
||||
parent.send([{ type: 'text', text: 'parent q' }])
|
||||
await parent.whenIdle()
|
||||
const seed = parent.session.events.slice()
|
||||
const run = startInProcessRun(ctx, { prompt: [{ type: 'text', text: 'child q' }], parent }, { providerName: 'fork', structuredNudgeRetries: 1, seed })
|
||||
const run = startInProcessRun(ctx, { prompt: [{ type: 'text', text: 'child q' }], parent }, { providerName: 'fork', seed })
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('completed')
|
||||
expect(text(result.output)).toBe('seeded child reply')
|
||||
|
||||
Reference in New Issue
Block a user