fix(models): remember the model choice per provider across sessions

[boeschj]

Summary

Fixes "if I close it and reopen, it shows astra again".

• Choosing a model now persists under model:{provider}, and catalog.resolve reads it back when no config override is set.
• The choice is per provider, so switching providers does not inherit another one's model.
• config.model_override and the environment still win, so nothing about existing precedence changes.

Extracted from #23, which is being closed. The write path calls the existing activeProviderId (commands.zig:414), which normalizes the stored provider through findOrDefault exactly the way catalog.resolve does, so both paths key off the same id.

An earlier revision of this PR hand-rolled that lookup and dropped the normalization, which broke the default flow. A later one over-corrected by caching the id on Model, which had to be updated by hand wherever the provider changed. Both are gone as of 90aff09.

Needs no new database schema. setting, setSetting and activeProvider all already exist.

Test plan

• zig build test (507 passed, 5 skipped, up from 506 on master)
• zig build clean
• zig fmt --check clean
• New test covers all three paths: a remembered model is restored, a different provider does not inherit it, and a config override still wins
• Pick a model, quit, relaunch, confirm it reopens on that model
• Switch providers and confirm each remembers its own choice

Deviations from the ticket text

No ticket. The storage key matches #23 (model:{provider}), so a selection stored by that branch is still read correctly.

Out of scope (flagged for follow-up)

• A pinned SYNTH_PROVIDER still stores the pick under the database's own provider rather than under the pin, so a pick made under a pin is not read back. That gap predates this change; activeProviderId cannot see Config, and threading it in is a larger shape than this fix.
• A remembered model that the provider no longer offers is not handled here. ensureModel currently means three different things across the three providers: ollama silently substitutes the first model and never rewrites the setting, so it re-substitutes on every launch; openai never validates a stored name at all. Worth settling that contract in its own change.
• The model picker itself is untouched. Sorting, and filtering non-chat models out of the OpenAI list, are separate follow-ups.

🤖 Generated with Claude Code

[pullfrog]

Important

The remembered model is keyed off a different provider id on write than on read, so the fix does not restore the model in the default flow — the exact bug this PR targets. One-line fix; details inline.

Reviewed changes

• Read the remembered model in catalog.resolve — when no config override is set, the model comes from the setting table under model:{entry.id}, where entry is the active provider after findOrDefault normalization (src/provider/catalog.zig:144).
• Persist the model pick in the TUI — switchModel now stores each choice under model:{activeProvider} via a new rememberModelChoice helper (src/tui/commands.zig:468).
• New resolution test — covers restore, no cross-provider inheritance, and config override winning; assertions are exact strings, so the test can fail (src/provider/catalog.zig:259).

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using openai-compatible/glm-5.3-flash:cloud ｜ 𝕏}

[boeschj]

Good catch, and it was right: the fix did not work in the flow it targets. Fixed in 0ab75f3.

I traced it before fixing. resolve derives the entry as findOrDefault(if (config.provider_override.len > 0) config.provider_override else chosen) (catalog.zig:121), so it reads model:{normalized id}. My write path used raw db.activeProvider(), which is setting("provider") and returns "" until connectProvider has stored one. Fresh install writes model:, resolve reads model:ollama, nothing is restored.

The SYNTH_PROVIDER divergence you flagged is worse than not-remembering, which is why I did not accept it. With a pin set, the write went to whatever the database's own provider row named, so picking a model under SYNTH_PROVIDER=openai would stamp an OpenAI model name onto model:ollama. Unsetting the pin later would then hand Ollama a model it cannot run. That is cross-provider corruption, and it is exactly what the existing test asserts must not happen.

Answering your open question: yes, a pin should move where the pick is stored. A pinned provider's remembered model should be reachable, and config.model_override winning is a separate axis: SYNTH_PROVIDER without SYNTH_MODEL is a real combination, and the pick made there should survive a restart.

On the shape of the fix. activeProviderId at commands.zig:414 normalizes the empty case, but it cannot see config.provider_override and Model has no config. Rather than duplicate the normalization at a second call site and hope the two stay in step, Model.provider_id now carries the id resolve already settled on: main.zig passes active.entry.id at construction, and connectProvider updates it alongside its setActiveProvider. Write and read agree by construction, and both override and default flows fall out of it. I deliberately did not key off Backend, which stores label rather than id, since that is the reverse-mapping trap this PR was written to avoid in the first place.

Tests. commands.zig has no test infrastructure, so I pinned the invariant on the catalog side instead: a fresh database with no provider row must resolve to default_id, a model stored under that id must read back, and a pinned provider must resolve under the pin. That is 508 passing, up from 506 on master. zig build and zig fmt --check clean.

Worth noting for your next pass: the sole remaining gap is that nothing test-covers the TUI write path itself, only the invariant it has to satisfy.

[pullfrog]

✅ No new issues found.

Reviewed changes — re-reviewed the two new commits (752d841..0ab75f3) against the prior pullfrog review. The key-mismatch finding is fixed cleanly:

• Introduced Model.provider_id — holds the normalized catalog id, defaulting to catalog.default_id, seeded from catalog.resolve's entry.id at construction (src/main.zig:204).
• Rewired rememberModelChoice onto it — the write path now stores under model:{self.provider_id} instead of the raw db.activeProvider() value, so write and read key off the same id (src/tui/commands.zig:478).
• Re-synced the id on provider switch — connectProvider updates self.provider_id = entry.id after a successful connect, and it is the only production path that swaps the TUI's provider, so the id cannot go stale (src/tui/commands.zig:327).
• Added a fresh-database test — pins the default flow end to end: a fresh DB resolves to default_id, a pick stored under that id is read back, and provider_override moves both the key and the id (src/provider/catalog.zig:290). Assertions are exact strings, so the test can fail.

The provider_override divergence raised in the prior review is resolved too: the write key now follows provider_id, which carries the same normalized id resolve settles on, whether that came from the database row or the config pin. Resume paths (restoreModel) never swap providers, so no other sync site is missing.

Author-flagged follow-ups (stale remembered model vs the provider's current catalog, picker sorting/filtering) remain explicitly out of scope.

  ^{｜ View workflow run ｜ Using openai-compatible/glm-5.3-flash:cloud ｜ 𝕏}

[boeschj]

Simplified in 90aff09, after Jordan pointed out I had diverged from working code for no good reason.

activeProviderId (commands.zig:414) already normalizes the stored provider through findOrDefault, which is exactly what catalog.resolve does on the read side. It was there the whole time and the original branch called it. I hand-rolled the lookup instead, dropped the normalization, and shipped the bug you caught. Then I over-corrected by caching the id in a new Model.provider_id field threaded through main.zig.

Both are now gone. The write path is one call to the existing helper.

I also want to retract the "agree by construction" claim I made in my previous comment. Model.provider_id was cached state that connectProvider had to remember to update, so it could drift; activeProviderId re-reads the database every time and cannot. The cached version was the more fragile of the two, not the less.

The SYNTH_PROVIDER gap is now listed as a known follow-up rather than fixed here. It predates this PR, activeProviderId cannot see Config, and threading config into the TUI is a bigger shape than this change warrants. The test that covers it says so explicitly rather than implying the write path handles it.

508 tests, zig build and zig fmt --check clean.

[pullfrog]

✅ No new issues found.

Reviewed changes — re-reviewed the one new commit (0ab75f3..90aff09) against the prior pullfrog review:

• Dropped the cached Model.provider_id field — rememberModelChoice now derives the provider id from the database through the existing activeProviderId helper at write time (src/tui/commands.zig:477), so write and read key off the same findOrDefault-normalized id with no field to keep in sync.
• Removed the sync sites that existed only for the cache — the Model{} seeding in main.zig and the re-assignment in connectProvider are gone; connectProvider already writes the DB row before switchTo calls switchModel, so the write path always sees the fresh id.
• Updated the fresh-database test — the provider_override case now documents the write-side gap in prose, keeping the assertion that a pinned read comes back empty.

Verified statically (no Zig toolchain here): no remaining references to the removed field; the arena-scoped key is copied by setSetting's exec before the arena frees; activeProviderId's no-database default (catalog.default_id) is unreachable from rememberModelChoice, which returns first when there is no database. The pin-flow note below matches what the test comment now documents: with config.provider_override set to a provider other than the database row, a pick made via bare /model (no reconnect) is stored under the row's id rather than the pin's, so it is not read back on the next pinned launch. Picks made through the model picker (switchTo) reconnect first, update the row, and key correctly.

  ^{｜ View workflow run ｜ Using openai-compatible/glm-5.3-flash:cloud ｜ 𝕏}

[aw1875]

nothing blocking but just a few notes

[aw1875]

db.setting returns an empty string for missing and empty, so a corrupted write silently re-falls back every launch

[aw1875]

if this fails, there's no error shown, so the next launch just reopens on the old model