Skip to content

fix: a route change must not carry a model's reasoning effort with it - #39

Merged
linhdmn merged 1 commit into
mainfrom
fix/ladder-effort
Oct 5, 2026
Merged

linhdmn merged 1 commit into
mainfrom
fix/ladder-effort

Conversation

@linhdmn

@linhdmn linhdmn commented Oct 5, 2026

Copy link
Copy Markdown
Member

Root cause

A desktop session's durable log (~/.dsh/sessions/--Users-linh.doan-work-harvey-freepeak--/session-d9fffefc-.../session.v4.jsonl.zstd) records the whole failure in one row:

{"type": "model/selection", "data": {"provider": "deepseek-official", "model": "opencode/space-bunny-free", "reasoningEffort": "high"}}
...
{"type": "turn/end", "data": {"reason": {"kind": "error", "error": {
  "message": "provider \"onegw\" model \"execution\" does not support reasoning effort \"high\"",
  "code": "UNSUPPORTED_REASONING_EFFORT"}}}}

One step, one turn, no assistant message. The model was never reached.

Two independent causes, both required for that exact row:

  1. src/plugin.ts:2368 — the merge kept the inherited effort. The agent/request handler returned { ...resolved, ...routed }. Spread keeps every key routed does not mention, so the ladder's rewrite of provider/model left reasoningEffort: 'high' in place. The harness's dsh-llm refuses any explicit effort a model does not advertise — resolveCallWithInfo throws UNSUPPORTED_REASONING_EFFORT before provider I/O, when info.reasoning === undefined. onegw/execution, declared in the profile with no reasoningEfforts, resolves to exactly that.
  2. src/plugin.ts:683 — routeForStep dropped the rung's own effort. Route.reasoningEffort and ControllerSpec.ladder[].reasoningEffort are advertised in the type and in spec.ts, then never read. A rung that configured an effort silently inherited the session's instead, so there was no way to say "this route takes no effort" even in principle.

A third, contributing cause — the reason the failure was so total rather than intermittent — is the profile itself: ~/.dsh/profiles/desktop/cordis.patch.yml declares onegw/execution and onegw/planning with no reasoningEfforts. dsh-llm-pi-ai's resolveModelReasoning reads entry.reasoningEfforts; omitted, it falls back to base?.reasoning ?? false where base is the installed pi-ai catalog entry, and these role aliases exist nowhere in that catalog. So reasoning: false — the model advertises no effort at all, and every explicit effort is refused, including 'off'.

The fix

Both halves in src/plugin.ts:

  • routeForStep hands the rung's own effort over, branded via ReasoningEffortId because LlmCallConfig.reasoningEffort is a branded type.
  • The agent/request handler strips the inherited effort before applying the rung — the same rule the harness's own model-selection applies when its selected effort is absent (it destructures reasoningEffort out of resolved for the identical reason).

Tested at test/routing-effort.test.ts, driving the real handler: fails 2 of 3 assertions on the old code.

What I changed outside the repo

~/.dsh/profiles/desktop/cordis.patch.yml — declared reasoningEfforts for both onegw models (off: null, low, high, max), so the role aliases can carry an effort if the ladder is ever pointed back at them. The shipped ladder in the repo now names the profile's own models (deepseek-official/opencode/space-bunny-free at high, escalating to xai/grok-4.5), with prices keyed to match.

Verification

  • make test — 688 pass, 0 fail
  • make typecheck — clean
  • .githooks/pre-commit --all — clean
  • Profile YAML parsed: execution/planning now report {"off":null,"low":"low","high":"high","max":"max"}

Not verified: a live desktop turn against the rebuilt plugin. The profile's plugin copy is a file: link to the repo's lib/, which the worktree builds separately — reinstall or merge and rebuild before driving the UI.

Collateral

While re-basing a check I disturbed two old stashes in the main checkout. stash@{0} (2026-09-29, src/dashboard-page.ts, +205 lines) is preserved as branch rescue/stash-optimize-loop-20260929. stash@{1} is intact. The working tree is back to origin/main for src/.


A desktop session picked `opencode/space-bunny-free` at effort `high`; the
ladder rewrote the route to `onegw/execution` on step one, the effort rode
along, and every turn died with `UNSUPPORTED_REASONING_EFFORT` before the
model was reached. The session log for that turn has exactly one step and one
turn/end carrying the error — the loop never spoke to a model at all.

Two halves, both required:

- `routeForStep` dropped `Route.reasoningEffort`, the field the ladder
  advertises. A rung that configures an effort silently got the session's
  instead. It now hands its own over (branded, since `LlmCallConfig` requires
  `ReasoningEffortId`).
- The `agent/request` merge was `{ ...resolved, ...routed }`, which keeps
  every key `routed` does not mention. An effort belongs to a model, so the
  merge now drops the inherited one before applying the rung — the same rule
  the harness's own `model-selection` applies when its selected effort is
  absent.

The shipped `cordis.patch.yml` gains the second cause: its ladder pointed at
onegw role aliases while the profile routed `space-bunny-free` in the UI, and
a hand-written pi-ai model with no `reasoningEfforts` reports no reasoning
capability at all. The ladder now names the profile's own models, with a
`reasoningEffort` the first rung's model actually offers, and prices both
keys (an unpriced route falls through to `unpricedFallback`, so a mismatched
key prices a lie).

test/routing-effort.test.ts drives the real `agent/request` handler: it fails
2 of 3 assertions on the old code and passes on the new, and it covers both
halves because either alone still loses the session. Local `make test` 688
green, `make typecheck` clean. Verified against the deployed desktop profile:
`llm-pi-ai` now reports reasoning for onegw/execution and onegw/planning.
@linhdmn
linhdmn merged commit b4a3d39 into main Oct 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant