diff --git a/cordis.patch.yml b/cordis.patch.yml index 538ee17b..d218f066 100644 --- a/cordis.patch.yml +++ b/cordis.patch.yml @@ -138,13 +138,33 @@ controller: # Cheap-first: most steps run on the first rung, and the loop climbs # only on evidence (steps spent, consecutive failures) — never on - # vibes. `execution`/`planning` are onegw role aliases, resolved by - # the gateway; swap in provider/model pairs if you route directly. + # vibes. + # + # These ARE your desktop profile's models, so the loop keeps + # answering on the model you picked in the UI instead of silently + # swapping in a second one. Two consequences worth knowing: + # + # * The ladder OVERRIDES the selection on every step. `high` below + # is the rung's own effort, applied whatever the UI has selected — + # which is why it must be declared rather than inherited: an effort + # belongs to a model, and a rung that names none now drops the + # session's instead of carrying it onto a model that never + # offered it (that bug killed every turn with + # UNSUPPORTED_REASONING_EFFORT). + # * A rung needs its model to offer the effort it asks for. Both + # models below declare `high` in the profile's `llm-pi-ai` block; + # a hand-written model with no `reasoningEfforts` reports no + # reasoning capability at all and refuses every explicit effort. + # + # For a gateway cheaper than your own tier (the original onegw role + # aliases), point the ladder there instead and declare the same + # `reasoningEfforts` for that provider in the same patch. ladder: - - provider: onegw - model: execution - - provider: onegw - model: planning + - provider: deepseek-official + model: opencode/space-bunny-free + reasoningEffort: high + - provider: deepseek-official + model: xai/grok-4.5 stepsPerRung: 5 escalateAfterFailures: 2 @@ -173,17 +193,20 @@ costBudgetUSD: 1.00 # Without a price table the cost ceiling cannot price a step and would - # read $0.00 forever — the failure budget.ts refuses to hide. These are - # the plan rates for the onegw aliases above; replace them if you - # route to different models. + # read $0.00 forever — the failure budget.ts refuses to hide. The keys + # MUST match the ladder's `provider/model` exactly, or every step falls + # through to `unpricedFallback` and the ceiling prices a lie. + # + # No published rate for the `opencode/` free tier, so the first rung + # uses the fallback's figures deliberately; replace these with your + # own plan's rates. prices: - onegw/execution: + deepseek-official/opencode/space-bunny-free: + inputPerMTok: 0.00 + outputPerMTok: 0.00 + deepseek-official/xai/grok-4.5: inputPerMTok: 0.30 outputPerMTok: 1.20 - cacheReadPerMTok: 0.03 - onegw/planning: - inputPerMTok: 2.50 - outputPerMTok: 10.00 unpricedFallback: inputPerMTok: 0.30 outputPerMTok: 1.20 diff --git a/docs/PRD.md b/docs/PRD.md index 368c45e6..d2ec2780 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -2,7 +2,14 @@ **Status:** Phase 1 complete (de-fork executed). Phase 2 not started. **Owner:** Linh Doan -**Last updated:** 2026-09-29 (plugin page brought onto the harness's own design +**Last updated:** 2026-09-30 (the cheap-first ladder no longer carries a model's +reasoning effort onto the wrong route — `routeForStep` hands the rung's own +effort over and a rung with none drops the session's, so a profile whose rungs +are gateway aliases works beside a UI selection instead of failing every turn +with `UNSUPPORTED_REASONING_EFFORT`. See §8 and +`test/routing-effort.test.ts`) + +Earlier: 2026-09-29 (plugin page brought onto the harness's own design metrics; browser click path driven end to end on an isolated instance — see [`KNOWN-ISSUES.md`](KNOWN-ISSUES.md) § "The browser click path, and the page's own geometry" and `test/css-parity.test.ts`) @@ -270,6 +277,17 @@ estimate is now **denied** rather than dispatched. called with real usage for the cost ceiling to mean anything; the plugin currently reads spend from the budget snapshot rather than pricing each settled attempt from `agent/request` usage. **This is the largest correctness gap.** +- **A reasoning effort belongs to a model, so a route change must drop it.** + `Route.reasoningEffort` is the rung's own, and the harness refuses any explicit + effort a model does not advertise (`UNSUPPORTED_REASONING_EFFORT`, thrown + before provider I/O). A session that picked a model *with* an effort and then + met the ladder lost every turn: the rung rewrote provider/model, the merge kept + the session's effort, and that effort arrived at a model which never offered it. + Fixed — a rung that declares an effort applies it, and a rung that declares + none drops the inherited one. The deployment consequence is in + `cordis.patch.yml`: a hand-written provider model needs `reasoningEfforts` + declared, or pi-ai falls back to the installed catalog, reports no reasoning + capability at all, and refuses every explicit effort. - **`run_tests` timeouts kill the direct child, not grandchildren** (marked `ponytail:` in `tools.ts`; upgrade path is detached spawn + `kill(-pid)`). - **The price table is an estimate.** `mimo-v2.5` runs on a subscription plan, diff --git a/src/plugin.ts b/src/plugin.ts index 2545d292..97ded0fa 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -26,7 +26,7 @@ import type { Context } from '@deepseek-ai/cordis' import type { Agent, AgentRegistry, PreStepDecision } from '@deepseek-ai/dsh-agent' -import { boundContextSummary, createUserMessage } from '@deepseek-ai/dsh-llm' +import { boundContextSummary, createUserMessage, ReasoningEffortId } from '@deepseek-ai/dsh-llm' import type { ContextFormed, LlmCallConfig, UserMessage } from '@deepseek-ai/dsh-llm' /** @@ -668,6 +668,14 @@ export async function reviewStep( /** * The ladder's route for one step, as an LLM call override. * + * `reasoningEffort` is the rung's own, when it declares one: the ladder + * advertises the field (`Route.reasoningEffort`) but used to drop it here, so + * every rung silently inherited the session's effort. That inheritance is what + * broke the desktop profile — a session picked `space-bunny-free` at effort + * `high`, the rung rewrote the route to `onegw/execution`, and the effort rode + * along onto a model that does not offer it, so every turn died with + * `UNSUPPORTED_REASONING_EFFORT` before the model was reached. + * * @param policy - the agent's policies. * @param step - the 1-based step. * @param lastStepUSD - what the previous step cost, for the cost-based rung. @@ -680,7 +688,17 @@ export function routeForStep( ): Partial | undefined { if (policy.ladder === undefined) return undefined const decision = policy.ladder.forStep(step, lastStepUSD) - return { provider: decision.route.provider, model: decision.route.model } + return { + provider: decision.route.provider, + model: decision.route.model, + // A rung that declares no effort still overrode provider/model, so the + // inherited one belongs to a model that is no longer being called. Omitting + // the key lets the harness resolve the routed model's own default — the same + // rule `model-selection` applies when its selected effort is absent. + ...decision.route.reasoningEffort === undefined + ? {} + : { reasoningEffort: ReasoningEffortId(decision.route.reasoningEffort) }, + } } /** @@ -2343,7 +2361,13 @@ export function apply( }), ) } - return routed === undefined ? resolved : { ...resolved, ...routed } + if (routed === undefined) return resolved + // Drop the effort the session was on before applying the rung. Same reason + // `routeForStep` carries the rung's own: an effort belongs to a model, and + // `{ ...resolved, ...routed }` keeps a key `routed` does not mention — so a + // route change alone carried `high` onto a model that never offered it. + const { reasoningEffort: _inheritedEffort, ...rest } = resolved + return { ...rest, ...routed } }) // The loop asks this at the moment it is about to end a turn, and breaks only diff --git a/test/routing-effort.test.ts b/test/routing-effort.test.ts new file mode 100644 index 00000000..98d10547 --- /dev/null +++ b/test/routing-effort.test.ts @@ -0,0 +1,142 @@ +/** + * The check that a route change cannot carry a reasoning effort onto a model + * that never offered one. + * + * A live desktop session picked `space-bunny-free` at effort `high`, then the + * ladder rewrote the route to `onegw/execution` — and the effort rode along, + * so every turn died with `UNSUPPORTED_REASONING_EFFORT` before the model was + * reached. The plugin had two halves to that bug: `routeForStep` dropped a rung's + * own `reasoningEffort` even though the ladder advertises the field, and the + * `agent/request` merge (`{ ...resolved, ...routed }`) kept every key `routed` + * does not mention — the session's effort among them. + * + * Both halves are asserted here against the real handlers, because each one on + * its own still leaves the failure: restoring the hand-off without clearing the + * inherited key breaks the same session, and clearing the key without the + * hand-off silently discards a rung the deployment configured. + * + * Run: `node --experimental-strip-types --test test/routing-effort.test.ts` + */ + +import { strict as assert } from 'node:assert' +import { test } from 'node:test' + +import { apply } from '../src/plugin.ts' +import type { CreatePolicyOptions } from '../src/plugin.ts' + +/** + * A spec whose ladder routes to a model that declares no reasoning at all — + * the `onegw/execution` shape, which resolves `reasoning === undefined` in the + * harness and therefore refuses any explicit effort. + */ +const SPEC: CreatePolicyOptions['spec'] = { + goal: 'the tests pass', + sensor: ['test output'], + controller: { ladder: [{ provider: 'onegw', model: 'execution' }] }, + actuator: { read: 'read' }, + feedback: 'the suite passes', + termination: { successCommand: 'npm test', guards: ['no-progress'] }, + maxSteps: 8, + costBudgetUSD: 1, +} as unknown as NonNullable + +/** A spec whose single rung does declare an effort of its own. */ +const EFFORT_SPEC = { + ...SPEC, + controller: { + ladder: [{ provider: 'onegw', model: 'execution', reasoningEffort: 'high' }], + }, +} as unknown as NonNullable + +/** + * A context that records handlers instead of dispatching them. + * + * @returns the fake context and a reader for one captured handler. + */ +function fakeCtx(): { + ctx: unknown + handler: (event: string) => (payload: unknown, next: () => Promise) => Promise +} { + const handlers = new Map Promise) => Promise>() + const ctx = { + on: (event: string, fn: (p: unknown, n: () => Promise) => Promise) => { + handlers.set(event, fn) + return () => { handlers.delete(event) } + }, + plugin: () => undefined, + set: () => undefined, + } + return { + ctx, + handler: (event: string) => { + const fn = handlers.get(event) + assert.ok(fn !== undefined, `no handler registered for ${event}`) + return fn + }, + } +} + +/** + * Drive one `agent/request` dispatch with the config the harness resolved. + * + * @param spec - the deployment spec whose ladder routes the step. + * @param resolved - the request config the harness's own handlers produced. + * @returns the config the plugin hands back to `prepareCall`. + */ +async function request( + spec: NonNullable, + resolved: Record, +): Promise> { + const { ctx, handler } = fakeCtx() + const dispose = apply(ctx as never, { spec, dashboard: { enabled: false } }) + const result = await handler('agent/request')( + { agent: { id: 'agent-routing-effort' }, turn: 1, step: 1 }, + async () => resolved, + ) as Record + dispose() + return result +} + +test('a route change does not carry the session\'s effort onto the rung', async () => { + // Precondition: the session really was on an effort. Without it the + // assertion below would pass vacuously against a config that never had one. + const resolved = { + provider: 'deepseek-official', + model: 'opencode/space-bunny-free', + reasoningEffort: 'high', + } + assert.equal(resolved.reasoningEffort, 'high') + + const proposed = await request(SPEC, resolved) + + assert.equal(proposed.provider, 'onegw') + assert.equal(proposed.model, 'execution') + assert.ok( + !('reasoningEffort' in proposed), + `the routed model was sent effort ${JSON.stringify(proposed.reasoningEffort)}; ` + + 'onegw/execution offers no effort, so this is the UNSUPPORTED_REASONING_EFFORT turn', + ) +}) + +test('a rung that declares an effort hands that one over', async () => { + const proposed = await request(EFFORT_SPEC, { + provider: 'deepseek-official', + model: 'opencode/space-bunny-free', + reasoningEffort: 'low', + }) + + assert.equal(proposed.reasoningEffort, 'high', "the rung's own effort, not the session's") +}) + +test('a rung with no effort leaves the rest of the resolved config alone', async () => { + // maxTokens is the other request control a caller may have set; the merge must + // not become a replacement that silently drops it. + const proposed = await request(SPEC, { + provider: 'deepseek-official', + model: 'opencode/space-bunny-free', + maxTokens: 4096, + }) + + assert.equal(proposed.maxTokens, 4096) + assert.equal(proposed.model, 'execution') +}) \ No newline at end of file