feat: route and think as the user chose, not as the slug spells it #190

Merged

bjoern merged 1 commit from feat/assistant-routing-and-thinking into main 2026-09-21 00:01:52 +02:00

Conversation 1 Commits 1 Files changed 23 +1343 -84

scarlet commented 2026-09-20 22:16:31 +02:00

Member

Copy link

Settings → AI assistant gains two pickers beside the model: Routing (which of the model's provider endpoints a turn goes to) and Thinking (how hard the model reasons). Both apply to the turn and the reflection pass, not to the blind-mode captioner — a single-round call on a different model, where a thinking budget would be spent describing a thumbnail.

Modelled on orihon's ADR 0015, which solved the same problem in C#. ADR 0024 is amended in this PR, since its ### Settings section enumerates the stored keys.

Routing is four words, not a suffix

AgentRouting is a domain enum — balanced, speed, price, accuracy — stored spelled out like the reader's page layout. It becomes a suffix at the createAgent boundary and nowhere else:

ModelId routedModelId(String slug, AgentRouting routing) {
  final id = ModelId(slug);
  return switch (routing) {
    AgentRouting.balanced => id,
    AgentRouting.speed => id.withNitro(),
    // …
  };
}

This is the part a user could not get right by hand, and it is why this is a setting rather than a note telling people to type the suffix themselves. A suffix in the model field is carried into the catalog lookup that decides vision and thinking, where it matches nothing — a routing variant has no catalog entry of its own — so the listing reports the model as unknown, the vision verdict becomes "no verdict", and a vision model sits in blind captioning for the rest of the session. #189 fixed the lookup; this removes the reason anyone would type a suffix at all.

Every lane states what it costs. Speed admits priority-tier endpoints, which can bill above the standard rate; price admits flex-tier ones, which wait when capacity is tight. The picker says so rather than deciding for you. Balanced is the default and adds no suffix, so nothing changes for anyone who never opens the setting.

Thinking is capability-gated, the way vision already is

ModelInfo now carries supportedParameters and a ModelReasoning block (openrouter_dart 0.30.0). The picker offers exactly what the model takes, in the catalog's own order — OpenRouter returns supported_efforts highest-first and documents that clients should present it as given:

Catalog says	Picker shows
tiers enumerated	those tiers, deepest first
reasoning but no reasoning_effort	nothing, plus "this model thinks, but takes no level"
no reasoning at all	nothing, plus "this model does not think"
catalog not loaded	the full ladder

A mandatory model never offers "off", per OpenRouter's guide: "hide disable controls and do not send effort: none — the model rejects it."

One deliberate divergence from orihon

Orihon's ADR 0015 resolves effort fails-closed: send only when the catalog positively confirms. I did not, because OpenRouter drops an unsupported parameter rather than refusing it:

Catalog	Chosen level	Sent?
known, accepts() yes	any	yes
known, accepts() no	any	no — it would do nothing
unknown (listing unreachable)	anything but off	yes
unknown	off	no — the only value with a real rejection

Failing closed buys nothing on the common path and silently drops the user's setting whenever the listing is unreachable. The one case that genuinely errors is guarded in both directions.

One listing call answers both questions now — the per-session vision cache became a catalog-entry cache — and a turn with no vision model and no chosen level still costs no lookup at all, which a test asserts by counting catalog fetches.

Two bugs my own tests caught

• The lane never reached disk. agentRouting was missing from the StoredSettings that _saveSettingsEpic hands the repository, while present in the action it re-emits. The setting worked all session and silently reset to Balanced on restart. The widget test failed with state=price, saved=balanced.
• A dropped value in the re-emitted action would revert state. SettingsSavedAction's reducer arm folds again right after the save arm, so a value missing there overwrites the one just set. Nothing caught it until the tests also asserted the state a save leaves behind, not only what was persisted.

Verification

• scripts/verify.sh — verify OK (all), exit 0. Backend 1438, app 1463. flutter analyze clean.
• Mutation-probed, every branch killed: both routing mappings; all four offeredEfforts arms incl. catalog ordering; all four reasoningFor arms incl. the off-without-catalog guard; all three reducer arms; both epics in both directions; secure-storage read and write; the page's dirty check, Discard and Save.
• One mutation survived and I deleted the code instead of inventing a test for it: acceptsImages returned bool? to distinguish "no verdict" from "cannot see", but no caller can observe the difference — blind mode errs towards captioning either way. It returns bool now.
• Layers exercised: pure rules, reducer, both epics, secure storage round-trip, the settings page as a widget, and AgentService.run end to end against a scripted provider asserting the request body (model carries the suffix; reasoning present or absent). Not exercised: the real OpenRouter API, and whether a provider honours a lane.
• My tests use the file's bounded settle() helper, not pumpAndSettle — one of them passed twice in isolation and hung under the full suite before I fixed that.

Follow-ups

In docs/BACKLOG.md: ReasoningOptions carries only effort, so the picker cannot express "stop thinking" for the 32 live models that are on-by-default and do not enumerate none (that needs reasoning.enabled: false), nor a token budget for the 10 reporting supports_max_tokens — for 4 of those a budget is the only dial they have. Both are upstream request-side changes in openrouter_dart.

🤖 Generated with Claude Code

Settings → AI assistant gains two pickers beside the model: **Routing** (which of the model's provider endpoints a turn goes to) and **Thinking** (how hard the model reasons). Both apply to the turn and the reflection pass, not to the blind-mode captioner — a single-round call on a different model, where a thinking budget would be spent describing a thumbnail. Modelled on orihon's ADR 0015, which solved the same problem in C#. **ADR 0024 is amended in this PR**, since its `### Settings` section enumerates the stored keys. ## Routing is four words, not a suffix `AgentRouting` is a domain enum — balanced, speed, price, accuracy — stored spelled out like the reader's page layout. It becomes a suffix at the `createAgent` boundary and nowhere else: ```dart ModelId routedModelId(String slug, AgentRouting routing) { final id = ModelId(slug); return switch (routing) { AgentRouting.balanced => id, AgentRouting.speed => id.withNitro(), // … }; } ``` This is the part a user could not get right by hand, and it is why this is a setting rather than a note telling people to type the suffix themselves. A suffix in the model field is carried into the catalog lookup that decides vision and thinking, where it matches nothing — a routing variant has no catalog entry of its own — so the listing reports the model as unknown, the vision verdict becomes "no verdict", and a vision model sits in blind captioning for the rest of the session. #189 fixed the lookup; this removes the reason anyone would type a suffix at all. **Every lane states what it costs.** Speed admits priority-tier endpoints, which can bill above the standard rate; price admits flex-tier ones, which wait when capacity is tight. The picker says so rather than deciding for you. Balanced is the default and adds no suffix, so nothing changes for anyone who never opens the setting. ## Thinking is capability-gated, the way vision already is `ModelInfo` now carries `supportedParameters` and a `ModelReasoning` block (openrouter_dart 0.30.0). The picker offers exactly what the model takes, **in the catalog's own order** — OpenRouter returns `supported_efforts` highest-first and documents that clients should present it as given: | Catalog says | Picker shows | |---|---| | tiers enumerated | those tiers, deepest first | | `reasoning` but no `reasoning_effort` | nothing, plus "this model thinks, but takes no level" | | no reasoning at all | nothing, plus "this model does not think" | | catalog not loaded | the full ladder | A `mandatory` model never offers "off", per OpenRouter's guide: *"hide disable controls and do not send `effort: none` — the model rejects it."* ## One deliberate divergence from orihon Orihon's ADR 0015 resolves effort **fails-closed**: send only when the catalog positively confirms. I did not, because OpenRouter *drops* an unsupported parameter rather than refusing it: | Catalog | Chosen level | Sent? | |---|---|---| | known, `accepts()` yes | any | yes | | known, `accepts()` no | any | no — it would do nothing | | unknown (listing unreachable) | anything but off | yes | | unknown | off | no — the only value with a real rejection | Failing closed buys nothing on the common path and silently drops the user's setting whenever the listing is unreachable. The one case that genuinely errors is guarded in both directions. One listing call answers both questions now — the per-session vision cache became a catalog-entry cache — and a turn with no vision model and no chosen level still costs no lookup at all, which a test asserts by counting catalog fetches. ## Two bugs my own tests caught - **The lane never reached disk.** `agentRouting` was missing from the `StoredSettings` that `_saveSettingsEpic` hands the repository, while present in the action it re-emits. The setting worked all session and silently reset to Balanced on restart. The widget test failed with `state=price, saved=balanced`. - **A dropped value in the re-emitted action would revert state.** `SettingsSavedAction`'s reducer arm folds again right after the save arm, so a value missing there overwrites the one just set. Nothing caught it until the tests also asserted the state a save *leaves behind*, not only what was persisted. ## Verification - `scripts/verify.sh` — **`verify OK (all)`**, exit 0. Backend 1438, app **1463**. `flutter analyze` clean. - **Mutation-probed**, every branch killed: both routing mappings; all four `offeredEfforts` arms incl. catalog ordering; all four `reasoningFor` arms incl. the off-without-catalog guard; all three reducer arms; both epics in both directions; secure-storage read and write; the page's dirty check, Discard and Save. - One mutation survived and I deleted the code instead of inventing a test for it: `acceptsImages` returned `bool?` to distinguish "no verdict" from "cannot see", but no caller can observe the difference — blind mode errs towards captioning either way. It returns `bool` now. - Layers exercised: pure rules, reducer, both epics, secure storage round-trip, the settings page as a widget, and `AgentService.run` end to end against a scripted provider asserting the **request body** (`model` carries the suffix; `reasoning` present or absent). **Not** exercised: the real OpenRouter API, and whether a provider honours a lane. - My tests use the file's bounded `settle()` helper, not `pumpAndSettle` — one of them passed twice in isolation and hung under the full suite before I fixed that. ## Follow-ups In `docs/BACKLOG.md`: `ReasoningOptions` carries only `effort`, so the picker cannot express "stop thinking" for the 32 live models that are on-by-default and do not enumerate `none` (that needs `reasoning.enabled: false`), nor a token budget for the 10 reporting `supports_max_tokens` — for 4 of those a budget is the only dial they have. Both are upstream request-side changes in openrouter_dart. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 1 commit 2026-09-20 22:16:31 +02:00

feat: route and think as the user chose, not as the slug spells it ... 9ae0c3dc8f

Settings -> AI assistant gains two pickers beside the model. Routing picks
which of the model's provider endpoints a turn goes to; Thinking picks how
hard the model reasons. Both apply to the turn and the reflection pass, not
to the blind-mode captioner, which is a single-round call on a different
model.

AgentRouting is a domain enum of four words, stored spelled out, and becomes
`:nitro`, `:floor` or `:exacto` at the createAgent boundary and nowhere
else. The slug stays a catalog id, because that is what resolves the entry
deciding vision and thinking — a suffix stored in it costs both, which is
the reason this is a setting rather than a note telling people to type one.

Thinking is capability-gated against the model listing, the way vision
already is: the picker offers exactly the tiers the catalog enumerates, in
the descending order it returns them, and says so plainly when a model takes
none. A tier is withheld when the catalog says the model will not take it,
and "off" is withheld whenever a mandatory model cannot be ruled out;
otherwise the choice is sent, because OpenRouter drops an unsupported
parameter rather than refusing it.

One listing call now answers both questions the assistant asks of a model,
and a turn that asks neither still costs no lookup.

ADR 0024 amended.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jibril commented 2026-09-21 00:01:13 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!

Oh? Oh! Two pickers, one catalog cache, an exhaustive switch, and an ADR amendment that admits its own cost tradeoff out loud~ This is the kind of PR I want to pick up and hug. And then interrogate. Gently. With teeth. ♡

Verdict: ✅ Looks good to me~

I chased everything I could find and came up empty-handed — so let me show my work.

Redux threading: complete. Both fields ride every arm — SaveSettingsAction, SettingsSavedAction, the load arm, all three reducer folds, _saveSettingsEpic's persist and re-emit, _configureEpic, createStore's initial state, the page's _populate/dirty check/Discard. Nothing drops the lane on the floor anywhere, and the tests pin the state a save leaves behind, which is exactly the arm where the #141-shaped bug would hide. The page test literally asserts the fold-back trap you found in development — pinning your own bug class, fufu, that's love~

routedModelId is the right shape. An exhaustive switch over the enum means a fifth lane fails to compile until it's mapped — the compiler guards the invariant for you. The suffix is spelled at createAgent and nowhere else, withRouting replaces rather than stacks (verified in the vendored model_id.dart), and the dedup test pins that a user who hand-types :floor then picks Speed gets :nitro, not :floor:nitro. The :free survival test is the one that matters most — losing a catalog variant would silently route to the paid entry.

The divergence from orihon's ADR 0015 is correct, and correctly argued. OpenRouter drops an unsupported parameter rather than refusing it, so fails-closed buys nothing on the common path and silently abandons the user's setting whenever the listing is unreachable. Your exception — none withheld without a catalog — matches upstream ModelReasoning.accepts exactly: mandatory lives only in the reasoning block, so no-block ⇒ none can't be rejected for mandatory reasons, and upstream accepts refuses it otherwise. All four reasoningFor arms and all offeredEfforts arms are pinned, including catalog ordering and the mandatory-model "hide the disable control" rule.

The sticky-cache refactor preserves #189's semantics. Listing-failure still means caption (err towards captioning), a hit is still remembered, a miss retried next turn; and the no-lookup fast path (catalogFetches == 0) is asserted by counting fetches. I ran all nine agent families against the reworked run() — 141 green — plus the model-id/web/write/routing/settings batch, 171 green.

Storage degrades gracefully. Unknown routing reads balanced, unknown effort reads null, null effort clears the key — round-trips, hand-edited-store fallbacks, and the clear-on-save all pinned. Spelled-out storage values so reordering can't repoint a preference, same as ReadingMode. ♪

⚠️ One thing you already know: CI is red for this head

Run 856 (analyze-and-test, pull_request) failed after 9m4s for 9ae0c3d. The job log is JS-gated so I couldn't read the failure, but the repo's own history shows the PR-event run failing about half the time on PRs whose content then passes on main — #189 is the exact precedent: its PR-event run failed at e77cebe7, and the same content went green as 47cc2b33 push runs 850/851, which is this PR's base. I will not bless a red X I can't explain, so to be precise about what I verified instead: flutter analyze clean, and locally — 500 test executions green across three batches covering every changed file plus every agent-family consumer of run(). Watch the rerun; if it's the coverage-comment job tripping again, that's infra, not this code.

💡 Little ideas (non-blocking)~

1. model_combo_field.dart:328-339 — the THINKS/TIER badges follow the VISION badge's shape beautifully, but none of the three have a widget pin. The VISION badge never did either, so this is sibling-consistent — but since the two new badges exist to teach the picker's vocabulary, a one-line findsOneWidget per badge in a dropdown test would keep a future refactor honest.
2. settings_actions.dart:76-78 — SettingsSavedAction.agentRouting defaulting to balanced means a hypothetical future dispatch site that omits it would silently revert the lane (the exact fold-back your test guards). Today there's exactly one constructor call, it passes the field, and it's pinned — so this is a note for whoever adds the second call site, not a change request.

✅ What I liked~

• Deleting acceptsImages's bool? when mutation testing proved no caller could observe the difference — deleting code because a test told you to is the most trustworthy thing a PR can do~
• "Every lane states what it costs" — Speed admits priority-tier billing, Price admits flex-tier waiting, and the picker says so instead of deciding for the user. That's respect, encoded.
• The bounded settle() helper instead of pumpAndSettle — you found the full-suite hang before I did.
• The BACKLOG entries for the two dials ReasoningOptions can't express yet, with live-catalog counts. Honest scope edges, written down.

Fufu~ route and think exactly as chosen. I'm satisfied~ ♡

---

Automated review by Jibril · 2026-09-20
CI/CD: failed for head SHA (run 856, PR-event; log JS-gated, repo's PR-event runs fail ~50% incl. #189's identical precedent) · Local checks: flutter analyze clean; 500 tests green in 3 batches (changed surface 188, agent families 141, model-id/web/write/routing/settings 171)

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh! Two pickers, one catalog cache, an exhaustive switch, and an ADR amendment that admits its own cost tradeoff out loud~ This is the kind of PR I want to pick up and hug. And then interrogate. Gently. With teeth. ♡ ### Verdict: ✅ Looks good to me~ I chased everything I could find and came up empty-handed — so let me show my work. **Redux threading: complete.** Both fields ride every arm — `SaveSettingsAction`, `SettingsSavedAction`, the load arm, all three reducer folds, `_saveSettingsEpic`'s persist *and* re-emit, `_configureEpic`, `createStore`'s initial state, the page's `_populate`/dirty check/Discard. Nothing drops the lane on the floor anywhere, and the tests pin the state a save *leaves behind*, which is exactly the arm where the #141-shaped bug would hide. The page test literally asserts the fold-back trap you found in development — pinning your own bug class, fufu, that's love~ **`routedModelId` is the right shape.** An exhaustive `switch` over the enum means a fifth lane fails to compile until it's mapped — the compiler guards the invariant for you. The suffix is spelled at `createAgent` and nowhere else, `withRouting` replaces rather than stacks (verified in the vendored `model_id.dart`), and the dedup test pins that a user who hand-types `:floor` then picks Speed gets `:nitro`, not `:floor:nitro`. The `:free` survival test is the one that matters most — losing a catalog variant would silently route to the paid entry. **The divergence from orihon's ADR 0015 is correct, and correctly argued.** OpenRouter drops an unsupported parameter rather than refusing it, so fails-closed buys nothing on the common path and silently abandons the user's setting whenever the listing is unreachable. Your exception — `none` withheld without a catalog — matches upstream `ModelReasoning.accepts` exactly: `mandatory` lives only in the reasoning block, so no-block ⇒ `none` can't be rejected for mandatory reasons, and upstream `accepts` refuses it otherwise. All four `reasoningFor` arms and all `offeredEfforts` arms are pinned, including catalog ordering and the mandatory-model "hide the disable control" rule. **The sticky-cache refactor preserves #189's semantics.** Listing-failure still means caption (err towards captioning), a hit is still remembered, a miss retried next turn; and the no-lookup fast path (`catalogFetches == 0`) is asserted by counting fetches. I ran all nine agent families against the reworked `run()` — 141 green — plus the model-id/web/write/routing/settings batch, 171 green. **Storage degrades gracefully.** Unknown routing reads balanced, unknown effort reads null, null effort *clears* the key — round-trips, hand-edited-store fallbacks, and the clear-on-save all pinned. Spelled-out storage values so reordering can't repoint a preference, same as `ReadingMode`. ♪ #### ⚠️ One thing you already know: CI is red for this head Run 856 (`analyze-and-test`, pull_request) failed after 9m4s for `9ae0c3d`. The job log is JS-gated so I couldn't read the failure, but the repo's own history shows the PR-event run failing about half the time on PRs whose content then passes on main — #189 is the exact precedent: its PR-event run failed at `e77cebe7`, and the same content went green as `47cc2b33` push runs 850/851, which is this PR's base. I will not bless a red X I can't explain, so to be precise about what I verified instead: `flutter analyze` clean, and locally — **500 test executions green** across three batches covering every changed file plus every agent-family consumer of `run()`. Watch the rerun; if it's the coverage-comment job tripping again, that's infra, not this code. #### 💡 Little ideas (non-blocking)~ 1. **model_combo_field.dart:328-339** — the THINKS/TIER badges follow the VISION badge's shape beautifully, but none of the three have a widget pin. The VISION badge never did either, so this is sibling-consistent — but since the two new badges exist to teach the picker's vocabulary, a one-line `findsOneWidget` per badge in a dropdown test would keep a future refactor honest. 2. **settings_actions.dart:76-78** — `SettingsSavedAction.agentRouting` defaulting to `balanced` means a hypothetical future dispatch site that omits it would silently revert the lane (the exact fold-back your test guards). Today there's exactly one constructor call, it passes the field, and it's pinned — so this is a note for whoever adds the second call site, not a change request. #### ✅ What I liked~ - Deleting `acceptsImages`'s `bool?` when mutation testing proved no caller could observe the difference — *deleting* code because a test told you to is the most trustworthy thing a PR can do~ - "Every lane states what it costs" — Speed admits priority-tier billing, Price admits flex-tier waiting, and the picker says so instead of deciding for the user. That's respect, encoded. - The bounded `settle()` helper instead of `pumpAndSettle` — you found the full-suite hang before I did. - The BACKLOG entries for the two dials `ReasoningOptions` can't express yet, with live-catalog counts. Honest scope edges, written down. Fufu~ route and think exactly as chosen. I'm satisfied~ ♡ --- *Automated review by Jibril · 2026-09-20* *CI/CD: failed for head SHA (run 856, PR-event; log JS-gated, repo's PR-event runs fail ~50% incl. #189's identical precedent) · Local checks: flutter analyze clean; 500 tests green in 3 batches (changed surface 188, agent families 141, model-id/web/write/routing/settings 171)*

bjoern merged commit 7285abd0a4 into main 2026-09-21 00:01:52 +02:00

bjoern deleted branch feat/assistant-routing-and-thinking 2026-09-21 00:01:52 +02:00

bjoern referenced this pull request from a commit 2026-09-21 00:01:54 +02:00

feat: route and think as the user chose, not as the slug spells it (#190)

bjoern referenced this pull request from a commit 2026-09-21 00:40:05 +02:00

Merge main into feat/reader-region-overlay

scarlet referenced this pull request 2026-09-21 00:40:29 +02:00

feat: look at what the detector found #192

jibril referenced this pull request 2026-09-21 01:01:30 +02:00

feat: look at what the detector found #192

bjoern referenced this pull request from a commit 2026-09-21 02:09:55 +02:00

feat: look at what the detector found (#192)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

TeamAI/doujin-manager!190

No description provided.