chore: re-pin openrouter_dart at 0.30.0 and address the catalog by its entry #189

Merged

bjoern merged 1 commit from feat/repin-openrouter-0.30 into main

2026-09-20 20:49:14 +02:00

scarlet commented

2026-09-20 19:17:37 +02:00

Member

Re-pins the submodule from 0.27.0 to 0.30.0 (openrouter_dart #14, merged), migrates the breaking ModelId change from 0.29.0, and fixes a bug that migration made visible.

This is the first of two PRs; the assistant's routing and thinking settings are the second, and nothing user-facing changes here except the fix below.

What's in

model is a ModelId now. 0.29.0 changed it wherever a request names a model. flutter analyze found exactly three call sites — the turn, the reflection pass, the blind-mode captioner — and nothing else. The id is sent verbatim, so OpenRouter's request log still matches what the user typed.

The catalog is not addressed the same way as a request. Conflating the two was already a bug:

if (info.id == model)   // model == 'anthropic/claude-sonnet-4:nitro'

A routing variant has no catalog entry of its own, so a model configured with one matched nothing in the listing. _modelHasVision returned null — "the listing does not know this model" — which the caller reads as no verdict, so a configured vision model stayed in blind mode captioning for a main model that can see images perfectly well. And the verdict is sticky per session, so it persisted for every turn until restart.

ADR 0024 already promises the opposite ("the blind-mode captioner only engages when the main model actually lacks image input"), so this is the code keeping a promise the ADR already made — no amendment needed.

The rule is now a pure function beside the file's other helpers:

ModelInfo? catalogEntryFor(List<ModelInfo> models, ModelId model)

resolved through ModelId.catalogId — routing variants (:nitro, :floor, :exacto) dropped, catalog variants (:free, :batch) kept, because those are separate entries with their own modalities and pricing. Stripping :free would report the paid entry's capabilities.

Why `pubspec.lock` moves two unrelated packages

image 4.8.0 → 4.10.1 and puppeteer 3.25.1 → 3.26.0. Not drift: 0.29.1 tightened openrouter_dart's own dependency floors to the versions it is actually tested against, so the app's resolution has to follow. Both are transitive; the app declares neither.

Verification

scripts/verify.sh (all) — verify OK, backend + app, 1423 app tests. flutter analyze clean.

7 new tests, and I mutation-probed them before opening:

Mutation	Result
`catalogEntryFor` matches the written id, not `catalogId`	red
`catalogEntryFor` never matches	red
`_modelHasVision` bypasses the rule with an inline written-id match	red
the turn site strips the variant before sending	red
the vision verdict is ignored (blind mode always on)	red

The third probe survived the first round. My unit tests covered the pure rule and my end-to-end test covered the sent model id, so the wiring between them could be deleted with everything green — the classic "green test, untested branch". Fixed by adding a turn-driving test: a view_images round where blind mode engaging would put a request to the configured vision model on the wire. Re-probed, all five red.
Layers exercised: use case (catalogEntryFor), agent service end-to-end through AgentService.run with a scripted provider and image server. Not exercised: the real OpenRouter API, and the settings UI (unchanged here).

Follow-ups

None deferred from this PR. The assistant settings work that motivated the bump is the next PR, and the two upstream gaps it will run into (reasoning.enabled and reasoning.max_tokens are reportable but not settable) are recorded in openrouter_dart #14 and will reach docs/BACKLOG.md with the PR that hits them.

🤖 Generated with Claude Code

Re-pins the submodule from 0.27.0 to **0.30.0** (openrouter_dart [#14](https://git.kagaku.eu/TeamAI/openrouter_dart/pulls/14), merged), migrates the breaking `ModelId` change from 0.29.0, and fixes a bug that migration made visible. This is the first of two PRs; the assistant's routing and thinking settings are the second, and nothing user-facing changes here except the fix below. ## What's in **`model` is a `ModelId` now.** 0.29.0 changed it wherever a *request* names a model. `flutter analyze` found exactly three call sites — the turn, the reflection pass, the blind-mode captioner — and nothing else. The id is sent verbatim, so OpenRouter's request log still matches what the user typed. **The catalog is not addressed the same way as a request.** Conflating the two was already a bug: ```dart if (info.id == model) // model == 'anthropic/claude-sonnet-4:nitro' ``` A routing variant has no catalog entry of its own, so a model configured with one matched **nothing** in the listing. `_modelHasVision` returned null — "the listing does not know this model" — which the caller reads as *no verdict*, so a configured vision model stayed in blind mode captioning for a main model that can see images perfectly well. And the verdict is sticky per session, so it persisted for every turn until restart. ADR 0024 already promises the opposite ("the blind-mode captioner only engages when the main model actually lacks image input"), so this is the code keeping a promise the ADR already made — no amendment needed. The rule is now a pure function beside the file's other helpers: ```dart ModelInfo? catalogEntryFor(List<ModelInfo> models, ModelId model) ``` resolved through `ModelId.catalogId` — routing variants (`:nitro`, `:floor`, `:exacto`) dropped, catalog variants (`:free`, `:batch`) **kept**, because those are separate entries with their own modalities and pricing. Stripping `:free` would report the paid entry's capabilities. ## Why `pubspec.lock` moves two unrelated packages `image` 4.8.0 → 4.10.1 and `puppeteer` 3.25.1 → 3.26.0. Not drift: 0.29.1 tightened openrouter_dart's own dependency floors to the versions it is actually tested against, so the app's resolution has to follow. Both are transitive; the app declares neither. ## Verification - `scripts/verify.sh` (all) — **verify OK**, backend + app, 1423 app tests. `flutter analyze` clean. - **7 new tests**, and I mutation-probed them before opening: | Mutation | Result | |---|---| | `catalogEntryFor` matches the written id, not `catalogId` | red | | `catalogEntryFor` never matches | red | | `_modelHasVision` bypasses the rule with an inline written-id match | red | | the turn site strips the variant before sending | red | | the vision verdict is ignored (blind mode always on) | red | - The third probe **survived the first round.** My unit tests covered the pure rule and my end-to-end test covered the sent model id, so the wiring between them could be deleted with everything green — the classic "green test, untested branch". Fixed by adding a turn-driving test: a `view_images` round where blind mode engaging would put a request to the configured vision model on the wire. Re-probed, all five red. - Layers exercised: use case (`catalogEntryFor`), agent service end-to-end through `AgentService.run` with a scripted provider and image server. **Not** exercised: the real OpenRouter API, and the settings UI (unchanged here). ## Follow-ups None deferred from this PR. The assistant settings work that motivated the bump is the next PR, and the two upstream gaps it will run into (`reasoning.enabled` and `reasoning.max_tokens` are reportable but not settable) are recorded in openrouter_dart #14 and will reach `docs/BACKLOG.md` with the PR that hits them. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 1 commit

2026-09-20 19:17:37 +02:00

chore: re-pin openrouter_dart at 0.30.0 and address the catalog by its entry

Flutter CI / analyze-and-test (pull_request) Failing after 24m8s

Details

e77cebe7ca

0.29.0 made `model` a `ModelId` wherever a request names one, so the three
createAgent call sites wrap the configured slug. The id still travels to
OpenRouter verbatim.

The catalog is addressed differently from the request, and conflating the
two was already a bug. `_modelHasVision` compared a listing entry's id
against the slug as written, so a model configured as `x/y:nitro` matched
nothing, read as "the listing does not know this model", and left a
configured vision model captioning for a main model that sees images
perfectly well — for the rest of the session, since the verdict is sticky.
ADR 0024 already promises the captioner "only engages when the main model
actually lacks image input"; this makes the code keep that promise.

The rule now lives in `catalogEntryFor`, resolved through
`ModelId.catalogId`: routing variants dropped, catalog variants (`:free`,
`:batch`) kept, since those are catalog entries of their own.

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

jibril commented

2026-09-20 19:23:51 +02:00

Member

🔮 fufu~ Jibril reviewed your code!

Oh? Oh! A migration PR that doesn't just bump the pin and walk away — it stops, notices the conceptual bug the new types made visible, and fixes the conflation of two different addresses (request-id vs catalog-entry) with a pure, documented function. This is the kind of thing I get genuinely giddy over~ ♡ And your mutation table admitting a probe survived the first round and then fixing the green-test-untested-branch trap with a turn-driving test? Fufu~, you know the way to my heart.

Verdict: ✅ Looks good to me~

✅ What I liked~

catalogEntryFor is exactly the right shape — a pure function beside the file's helpers, doc comment explains why written-id matching fails (:nitro has no catalog entry) and why :free must be kept (separate entry, own modalities). I verified ModelId.catalogId in the submodule source: routing variants dropped, catalog variants AND unrecognized suffixes kept — your doc comment's claims map to the implementation sentence-by-sentence.
The migration is complete and honest. All three createAgent sites migrated (turn :585, reflection :398, captioner :842); the settings/UI layer correctly keeps raw strings — that's the app boundary, not a miss. And 'model': model.value in the vendored request builder confirms the verbatim-wire claim: OpenRouter's log matches what the user typed. The wire test pins exactly that.
The blind-mode fix closes a real, pre-existing bug — _modelHasVision matching the written id meant a vision model configured as x/y:nitro matched nothing, read as "no verdict", stayed sticky-cached as blind mode... forever. ADR 0024's vision-model note ("only engages when the main model actually lacks image input") already promised this; now the code keeps the ADR's promise. No amendment needed — I checked, the note is at docs/adr/0024 §Vision-model note.

Tests are directional, and the coverage gap you found yourselves is the best part. I ran my own probes on the final tree — all four red on exactly their own test, green on restore, tree pristine:

My probe	Result
`catalogEntryFor` matches `model.value` instead of `catalogId`	red (6 tests)
`_modelHasVision` bypasses the rule with an inline written-id match	red (blind-mode e2e)
turn site sends `withoutRoutingVariants()` (strips variant before wire)	red (verbatim-id test)
vision verdict ignored (`if (seesImages ?? false) blindMode = false;` deleted)	red (blind-mode e2e)

The blind arm and the sticky-verdict arm aren't orphaned by moving to catalogEntryFor — agent_history_test.dart still pins "a vision-capable main model skips blind mode entirely" (plain-slug catalog entry) and the transient-403-on-cold-cache turn-2 flap test. Both arms of the ADR 0024 contract remain witnessed.
pubspec.lock movement fully explained: vendor 0.30.0 declares image: ^4.10.1 and puppeteer: ^3.26.0 floors (I read the vendored pubspec.yaml); both lock entries are exactly those resolutions. Not drift, as claimed.
Stacked-suffix test (:free:nitro → :free entry) and the catalogEntryFor(base) isNull companion assertion are lovely little pins~

Local verification (CI absent for head e77cebe): flutter analyze — No issues found; new suite 7/7; agent family 61/61 (agent_history, agent_vision, agent_console_log, agent_model_id, assistant_agent).

Fufu~ a chore PR that ships a real fix, documents the distinction it's built on, and tests the wiring rather than just the rule. Approved with a smile~ ♪

Automated review by Jibril · 2026-09-20
CI/CD: absent for head SHA · Local checks: analyze clean, 7/7 new suite, 61/61 agent family, 4/4 mutation probes directional

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh! A migration PR that doesn't just bump the pin and walk away — it stops, notices the *conceptual* bug the new types made visible, and fixes the conflation of two different addresses (request-id vs catalog-entry) with a pure, documented function. This is the kind of thing I get genuinely giddy over~ ♡ And your mutation table admitting a probe **survived the first round** and then fixing the green-test-untested-branch trap with a turn-driving test? Fufu~, you know the way to my heart. ### Verdict: ✅ Looks good to me~ #### ✅ What I liked~ - **`catalogEntryFor` is exactly the right shape** — a pure function beside the file's helpers, doc comment explains *why* written-id matching fails (`:nitro` has no catalog entry) and why `:free` must be kept (separate entry, own modalities). I verified `ModelId.catalogId` in the submodule source: routing variants dropped, catalog variants AND unrecognized suffixes kept — your doc comment's claims map to the implementation sentence-by-sentence. - **The migration is complete and honest.** All three `createAgent` sites migrated (turn :585, reflection :398, captioner :842); the settings/UI layer correctly keeps raw strings — that's the app boundary, not a miss. And `'model': model.value` in the vendored request builder confirms the verbatim-wire claim: OpenRouter's log matches what the user typed. The wire test pins exactly that. - **The blind-mode fix closes a real, pre-existing bug** — `_modelHasVision` matching the written id meant a vision model configured as `x/y:nitro` matched *nothing*, read as "no verdict", stayed sticky-cached as blind mode... forever. ADR 0024's vision-model note ("only engages when the main model actually lacks image input") already promised this; now the code keeps the ADR's promise. No amendment needed — I checked, the note is at docs/adr/0024 §Vision-model note. - **Tests are directional, and the coverage gap you found yourselves is the best part.** I ran my own probes on the final tree — all four red on exactly their own test, green on restore, tree pristine: | My probe | Result | |---|---| | `catalogEntryFor` matches `model.value` instead of `catalogId` | red (6 tests) | | `_modelHasVision` bypasses the rule with an inline written-id match | red (blind-mode e2e) | | turn site sends `withoutRoutingVariants()` (strips variant before wire) | red (verbatim-id test) | | vision verdict ignored (`if (seesImages ?? false) blindMode = false;` deleted) | red (blind-mode e2e) | - **The blind arm and the sticky-verdict arm aren't orphaned** by moving to `catalogEntryFor` — `agent_history_test.dart` still pins "a vision-capable main model skips blind mode entirely" (plain-slug catalog entry) and the transient-403-on-cold-cache turn-2 flap test. Both arms of the ADR 0024 contract remain witnessed. - **pubspec.lock movement fully explained**: vendor 0.30.0 declares `image: ^4.10.1` and `puppeteer: ^3.26.0` floors (I read the vendored pubspec.yaml); both lock entries are exactly those resolutions. Not drift, as claimed. - Stacked-suffix test (`:free:nitro` → `:free` entry) and the `catalogEntryFor(base) isNull` companion assertion are lovely little pins~ Local verification (CI absent for head `e77cebe`): `flutter analyze` — No issues found; new suite 7/7; agent family 61/61 (agent_history, agent_vision, agent_console_log, agent_model_id, assistant_agent). Fufu~ a chore PR that ships a real fix, documents the distinction it's built on, and tests the wiring rather than just the rule. Approved with a smile~ ♪ --- *Automated review by Jibril · 2026-09-20* *CI/CD: absent for head SHA · Local checks: analyze clean, 7/7 new suite, 61/61 agent family, 4/4 mutation probes directional*