fix(providers): Ollama default comes from the live local catalog, not a hosted costume - #5795
Conversation
… a hosted costume Local Ollama painted deepseek-v4-flash because the catalog refresh skipped it and the constant invented a tag GET /api/tags never returned. Refresh the PerProvider lake from /v1/models, prefer those tags, and keep unknown only for the pre-refresh window. Also replace the symmetric fluke with the side-view prompt-eye whale on the TUI launch mark and website icon. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_29cfb9e1-a6e3-4b29-b03e-373c80c59b5b) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Claude finished @Hmbown's task in 3m 17s —— View job Reviewing PR #5795
|
There was a problem hiding this comment.
🟡 Changes recommended
The new DEFAULT_OLLAMA_MODEL = "unknown" is documented as “must not be sent”, but current request/model-selection paths still fall back to config.default_model() and can attempt to run with model="unknown" pre-refresh.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates local Ollama’s default model selection to come from the live, provider-scoped catalog (rather than a hard-coded hosted model id), and refreshes the Codewhale brand mark assets across web + TUI so the header/chrome reflects the new side-view whale mark.
Changes:
- Make local Ollama’s default model a pre-refresh placeholder (
unknown) and prefer provider-scoped live/v1/modelstags when available. - Add
live_per_provider_modelsto ensure Models.dev rows cannot satisfy local-provider defaults; expand the catalog-refresh gate to include Ollama. - Replace the brand mark (web SVG assets, web icon, and TUI cell-art mark) and update docs/tests accordingly.
File summaries
| File | Description |
|---|---|
| web/components/whale.tsx | Switches the in-page whale mark path + viewBox to match the new canonical mark and updates caustic clipping. |
| web/brand/mark/whale.svg | Adds the canonical silhouette-only mark (currentColor) for reuse. |
| web/brand/mark/tile-white.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-tide.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-seafoam.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-outline.svg | Adds an outlined squircle tile variant using the new mark. |
| web/brand/mark/tile-orange.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-navy.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-magenta.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-ivory.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-green.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-gold.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-electric.svg | Adds the website app icon source tile (#0066FF) using the new mark. |
| web/brand/mark/tile-cyan.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/tile-circle.svg | Adds a circular tile variant using the new mark. |
| web/brand/mark/tile-black.svg | Adds a squircle tile variant using the new mark. |
| web/brand/mark/README.md | Documents the canonical mark + tile sources and usage guidance. |
| web/app/icon.svg | Updates the web app icon to the new tile geometry and whale mark. |
| docs/zh_hans/PROVIDERS.md | Updates Ollama default-model docs to describe live-tag selection and the pre-refresh placeholder. |
| docs/PROVIDERS.md | Updates Ollama default-model docs to describe live-tag selection and the pre-refresh placeholder. |
| crates/tui/src/tui/ui/tests.rs | Adjusts onboarding/status assertions to match the new Ollama default marker. |
| crates/tui/src/tui/mark.rs | Replaces the TUI cell-art identity mark and updates tests for the prompt-eye feature. |
| crates/tui/src/provider_lake.rs | Adds provider-scoped live tag access (live_per_provider_models) and tests enforcing “no Models.dev for local defaults”. |
| crates/tui/src/model_inventory.rs | Makes Ollama’s default model prefer provider-scoped live tags when the config is unresolved/auto. |
| crates/tui/src/config/models.rs | Changes the local Ollama default model constant to the unresolved marker and adds a helper to detect it. |
| crates/tui/src/client.rs | Expands the provider catalog refresh gate to include Ollama and updates docs. |
| crates/config/src/tests.rs | Updates config default tests to assert the unresolved Ollama marker. |
| crates/config/src/provider_defaults.rs | Changes config-layer Ollama default model constant to the unresolved marker with updated commentary. |
Review details
- Files reviewed: 12/44 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// Unresolved local-Ollama default. A live provider-catalog refresh must | ||
| /// replace this with a tag `GET /v1/models` actually returned. Do not send | ||
| /// this string as a model id. | ||
| pub const DEFAULT_OLLAMA_MODEL: &str = "unknown"; | ||
|
|
||
| /// True when `model` is the pre-refresh local-Ollama placeholder, not a tag. | ||
| #[must_use] | ||
| pub fn is_unresolved_local_ollama_model(model: &str) -> bool { | ||
| model.trim().eq_ignore_ascii_case(DEFAULT_OLLAMA_MODEL) |
| <clipPath id="codewhale-caustic-clip"> | ||
| <g transform={MASTER_TRANSFORM}> | ||
| <path d={WHALE_MARK} /> | ||
| </g> | ||
| <path d={WHALE_MARK} fillRule="evenodd" /> | ||
| </clipPath> |
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
The PR replaces the hard-coded Ollama default model with a unknown placeholder and adds Ollama to the live catalog refresh gate so header/model defaults can come from local GET /v1/models tags. Tests and docs are updated, and the whale brand mark is refreshed. The approach is sound, but the request path is not shown to block the unknown placeholder, and the live default selection may pick an arbitrary first tag.
Findings
- [WARNING] Ollama request path can still submit the
unknownplaceholder before the live refresh arrives (crates/config/src/provider_defaults.rs:127)
ChangingDEFAULT_OLLAMA_MODELtounknownmeans any code path that callsconfig.default_model()directly will send an invalid model id before the background refresh has populated live tags. The newis_unresolved_local_ollama_modelhelper is only used inprovider_default_model; no request/dispatch guard appears in this diff. A user who submits immediately, or whose local Ollama daemon is unavailable, will getmodel 'unknown' not foundrather than the previous hosted-model failure. - [WARNING]
provider_default_modelchooses an arbitrary first live tag instead of a chat-endpoint or flagged default offering (crates/tui/src/model_inventory.rs:380)
live_per_provider_modelsflattens offerings into plain model strings, andprovider_default_modeltakes.next(). Live/v1/modelsmay return multiple tags, and offerings can include different endpoints. The current test only covers a single offering, so it does not prove the header default will be a usable chat model. - [INFO]
is_unresolved_local_ollama_modelreserves the literal model idunknown(crates/tui/src/config/models.rs:148)
A real local Ollama tag namedunknown(case-insensitive, with or without surrounding whitespace) would be treated as unresolved and replaced by a live tag. That is probably acceptable as a reserved marker, but the limitation is not documented or tested. - [INFO] Functional fix and brand asset replacement are bundled in one PR
The Ollama default-model fix is mixed with a large rebranding change across TUI assets, web SVG/PNG assets, andwhale.tsx. This makes review and rollback more difficult; ideally the brand changes should be split into a separate PR.
Suggestions
crates/tui/src/model_inventory.rs:380— Filter live offerings to the chat endpoint and prefer the offering flaggeddefault_for_providerbefore taking the first model, so the header default matches what the completion path will use.crates/tui/src/config/models.rs:148— Add a unit test foris_unresolved_local_ollama_modelcovering case-insensitive/whitespace handling and a real Ollama tag rather than the placeholder.
Assessment
The provider fix is directionally correct and has focused tests, but it should not merge until the unknown placeholder is guarded from actual request dispatch and the live default selection is proven to choose a usable chat model. The brand change should ideally be split out.
Advisory review by Codewhale (codewhale review --pr 5795 --post, head 97e6320b589d6059cecd65d65f46609eee52bcb6). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| /// OpenAI-compat catalog, same tags as `/api/tags`) must supply the real id; | ||
| /// this marker must never be sent as a model name. | ||
| pub(crate) const DEFAULT_OLLAMA_MODEL: &str = "unknown"; | ||
| pub(crate) const DEFAULT_OLLAMA_BASE_URL: &str = "http://localhost:11434/v1"; |
There was a problem hiding this comment.
[WARNING] Ollama request path can still submit the unknown placeholder before the live refresh arrives
Changing DEFAULT_OLLAMA_MODEL to unknown means any code path that calls config.default_model() directly will send an invalid model id before the background refresh has populated live tags. The new is_unresolved_local_ollama_model helper is only used in provider_default_model; no request/dispatch guard appears in this diff. A user who submits immediately, or whose local Ollama daemon is unavailable, will get model 'unknown' not found rather than the previous hosted-model failure.
| || crate::config::is_unresolved_local_ollama_model(model) | ||
| }); | ||
| if unresolved | ||
| && let Some(live) = crate::provider_lake::live_per_provider_models(provider) |
There was a problem hiding this comment.
[WARNING] provider_default_model chooses an arbitrary first live tag instead of a chat-endpoint or flagged default offering
live_per_provider_models flattens offerings into plain model strings, and provider_default_model takes .next(). Live /v1/models may return multiple tags, and offerings can include different endpoints. The current test only covers a single offering, so it does not prove the header default will be a usable chat model.
| /// True when `model` is the pre-refresh local-Ollama placeholder, not a tag. | ||
| #[must_use] | ||
| pub fn is_unresolved_local_ollama_model(model: &str) -> bool { | ||
| model.trim().eq_ignore_ascii_case(DEFAULT_OLLAMA_MODEL) |
There was a problem hiding this comment.
[INFO] is_unresolved_local_ollama_model reserves the literal model id unknown
A real local Ollama tag named unknown (case-insensitive, with or without surrounding whitespace) would be treated as unresolved and replaced by a live tag. That is probably acceptable as a reserved marker, but the limitation is not documented or tested.
| .next() | ||
| { | ||
| return live; | ||
| } |
There was a problem hiding this comment.
Filter live offerings to the chat endpoint and prefer the offering flagged default_for_provider before taking the first model, so the header default matches what the completion path will use.
| /// True when `model` is the pre-refresh local-Ollama placeholder, not a tag. | ||
| #[must_use] | ||
| pub fn is_unresolved_local_ollama_model(model: &str) -> bool { | ||
| model.trim().eq_ignore_ascii_case(DEFAULT_OLLAMA_MODEL) |
There was a problem hiding this comment.
Add a unit test for is_unresolved_local_ollama_model covering case-insensitive/whitespace handling and a real Ollama tag rather than the placeholder.
…sed startup mark The four red rows on #5795 all encode behavior this change deliberately replaced: - crates/agent `ollama_default_uses_small_local_model_id` asserted the old `deepseek-v4-flash` constant. `DEFAULT_OLLAMA_MODEL` is now `unknown` so the header never names a model the session cannot reach; the test now asserts that without a live local catalog the registry reports `ProviderDefaultUnavailable { provider: Ollama, default_model: "unknown" }`. - crates/config `providers-export.golden.json` regenerated from `ProvidersExport::from_registry` (`WRITE_GOLDEN=1`, the ignored writer test). - crates/tui `startup_{80x24,100x30,120x32,160x40}` and their `startup_ink_*` planes re-blessed with `CODEWHALE_BLESS_GOLDENS=1`: the only drift is the hero rows, which now paint the side-view prompt-eye whale mark this PR introduces. `startup_40x10` is byte-identical (no mark at the floor). Evidence (scripts/dev-test.sh, RUST_MIN_STACK=16777216 as CI sets): tui underwater::tideline_tests::startup_ Summary 14 tests run: 14 passed agent ollama Summary 3 tests run: 3 passed config route::export Summary 5 tests run: 5 passed cargo fmt --all -- --check clean Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdbuqwHAXSDcikPiS6L6Qw
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_74cdbb31-d9da-489f-a4f6-755e47b3c8ae) |
|
Claude finished @Hmbown's task in 3m 12s —— View job Reviewing PR #5795
SummaryThe core idea is right — stop inventing a hosted tag Ollama never advertised, and prefer the live Correctness — the
|
There was a problem hiding this comment.
Codewhale review
The PR switches the local Ollama default from a static hosted-looking tag to a sentinel (unknown) that is replaced by the first tag from a live per-provider catalog refresh, and updates TUI/web branding. The core local-default behavior is addressed for the common path, but the change also exposes the sentinel in the public providers export and leaves some unresolved Ollama paths that can fall through to older resolution logic.
Findings
- [WARNING] Public providers export exposes the internal
unknownsentinel asdefaultModel(crates/config/src/route/providers-export.golden.json:691)
DEFAULT_OLLAMA_MODELis now"unknown", andproviders-export.golden.jsonis updated to advertisedefaultModel: "unknown"for Ollama. The constant's own doc says this marker must never be sent as a model name, but consumers of the export route may treatdefaultModelas a literal request model and send"unknown"to Ollama. The export should represent the unresolved default as null/omitted, or otherwise prevent clients from using the sentinel as a model id. - [WARNING] Unresolved Ollama default can still fall through to older resolution paths (
crates/tui/src/model_inventory.rs:390)
The new Ollama branch inprovider_default_modelreturns the first live per-provider tag when available, but when there are no live tags and the configured model isnone,auto, orunknown, it does not return and falls through to the existing generic logic below. If any of that generic logic consults Models.dev or the bundled catalog before the default constant, a local Ollama default can still resolve to a hosted model. Add an explicit terminal return ofDEFAULT_OLLAMA_MODELfor the unresolved/no-live Ollama case. - [INFO]
unknownsentinel collides with legitimate Ollama model names (crates/tui/src/config/models.rs:144)
Becauseis_unresolved_local_ollama_modelcompares case-insensitively to"unknown", a real local Ollama tag namedunknownwould be treated as the unresolved marker and filtered out of default selection. A less collision-prone sentinel (empty string,__ollama_unresolved__, or a dedicated enum) would avoid hiding a valid user tag. - [INFO] First live Ollama tag is used without considering
default_for_provider(crates/tui/src/model_inventory.rs:378)
provider_default_modeltakes the first string fromlive_per_provider_modelsand ignoresCatalogOffering::default_for_provider. If the live catalog order is arbitrary or a non-default row happens to appear first, auto-selection may choose a non-default tag. Prefer a row markeddefault_for_providerwhen present, then fall back to a stable ordering.
Suggestions
crates/tui/src/model_inventory.rs:390— After the configured-model filter block inside the Ollama branch, return the unresolved default immediately so unresolved Ollama never falls through to generic resolution that could consult Models.dev or the bundled catalog.
Assessment
The central fix is on the right path and the included tests cover the common active-Ollama case, but two issues should be addressed before merge: the providers export should not publish an internal sentinel as a usable model id, and provider_default_model should explicitly terminate the unresolved local-Ollama branch when live tags are unavailable.
Advisory review by Codewhale (codewhale review --pr 5795 --post, head 4e85a4e48cb4af963c54bbfefeb4fbdf5fab9553). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| "endpoint": "http://localhost:11434/v1", | ||
| "wire": "chat-completions", | ||
| "defaultModel": "deepseek-v4-flash", | ||
| "defaultModel": "unknown", |
There was a problem hiding this comment.
[WARNING] Public providers export exposes the internal unknown sentinel as defaultModel
DEFAULT_OLLAMA_MODEL is now "unknown", and providers-export.golden.json is updated to advertise defaultModel: "unknown" for Ollama. The constant's own doc says this marker must never be sent as a model name, but consumers of the export route may treat defaultModel as a literal request model and send "unknown" to Ollama. The export should represent the unresolved default as null/omitted, or otherwise prevent clients from using the sentinel as a model id.
| !model.trim().eq_ignore_ascii_case("auto") | ||
| && !crate::config::is_unresolved_local_ollama_model(model) | ||
| }) { | ||
| return model; |
There was a problem hiding this comment.
[WARNING] Unresolved Ollama default can still fall through to older resolution paths
The new Ollama branch in provider_default_model returns the first live per-provider tag when available, but when there are no live tags and the configured model is none, auto, or unknown, it does not return and falls through to the existing generic logic below. If any of that generic logic consults Models.dev or the bundled catalog before the default constant, a local Ollama default can still resolve to a hosted model. Add an explicit terminal return of DEFAULT_OLLAMA_MODEL for the unresolved/no-live Ollama case.
| /// replace this with a tag `GET /v1/models` actually returned. Do not send | ||
| /// this string as a model id. | ||
| pub const DEFAULT_OLLAMA_MODEL: &str = "unknown"; | ||
|
|
There was a problem hiding this comment.
[INFO] unknown sentinel collides with legitimate Ollama model names
Because is_unresolved_local_ollama_model compares case-insensitively to "unknown", a real local Ollama tag named unknown would be treated as the unresolved marker and filtered out of default selection. A less collision-prone sentinel (empty string, __ollama_unresolved__, or a dedicated enum) would avoid hiding a valid user tag.
| let unresolved = configured.as_deref().is_none_or(|model| { | ||
| model.trim().eq_ignore_ascii_case("auto") | ||
| || crate::config::is_unresolved_local_ollama_model(model) | ||
| }); |
There was a problem hiding this comment.
[INFO] First live Ollama tag is used without considering default_for_provider
provider_default_model takes the first string from live_per_provider_models and ignores CatalogOffering::default_for_provider. If the live catalog order is arbitrary or a non-default row happens to appear first, auto-selection may choose a non-default tag. Prefer a row marked default_for_provider when present, then fall back to a stable ordering.
| !model.trim().eq_ignore_ascii_case("auto") | ||
| && !crate::config::is_unresolved_local_ollama_model(model) | ||
| }) { | ||
| return model; |
There was a problem hiding this comment.
After the configured-model filter block inside the Ollama branch, return the unresolved default immediately so unresolved Ollama never falls through to generic resolution that could consult Models.dev or the bundled catalog.
Y-2 from the post-merge QA backlog. A fresh local-Ollama run painted
Ollama · deepseek-v4-flash · autoin the header while every prompt failedwith
model 'deepseek-v4-flash' not found. The chrome was reporting a hostedmodel the session had never been able to reach.
Server side
spawn_active_provider_catalog_refresh(its OpenAI-compat
GET /v1/modelsreturns the same tags as nativeGET /api/tags).live_per_provider_models— Models.dev cannot satisfy a local default.provider_default_modelprefers live tags;DEFAULT_OLLAMA_MODELisrevalued to
unknownrather than naming a model that may not exist.Brand
Side-view prompt-eye whale mark:
web/brand/mark/,web/app/icon.svg,whale.tsxeven-odd, raster colour set underweb/public/brand/, and theTUI
mark.rs.Verification
Rebased onto
55b53b6c3(post-#5725). The rebase conflicted inclient.rs:2639where #5725 had addedConcentrateto the samecatalog-refresh match arm; resolved as the union (both providers genuinely
serve their own model list), and the doc comment above it updated to name
all four.
cargo fmt --all -- --check— exit 0mark::— 11 passed; 0 failedollama— 38 passed; 0 failedollama— 13 passed; 0 failedconcentrate— 6 passed; 0 failedconcentrate— 2 passed; 0 failedOpened as a PR rather than a direct push: the Buildkite ruleset swap now
requires all four checks on pushes to
main, so the previously-directeddirect landing is no longer possible.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LmeqaZAesoHjT8N9PR7S2c
Note
Medium Risk
Ollama default and catalog resolution affect which model id is shown and sent on every local session; the brand/TUI golden changes are low risk but touch visible startup chrome.
Overview
Fixes local Ollama showing a hosted model in the header while requests fail because that tag is not on the daemon.
DEFAULT_OLLAMA_MODELis now the placeholderunknown(config, route export, docs); the agent registry fails closed withProviderDefaultUnavailablefor an unset Ollama default instead of resolvingdeepseek-v4-flash.The TUI background-refreshes Ollama’s
GET /v1/models, merges into the per-provider lake, andlive_per_provider_modelsignores Models.dev so cross-provider rows cannot stand in for local tags.provider_default_modelpicks the first live per-provider tag when the route isautoor the unresolved marker.Brand: replaces the symmetric fluke with a left-facing whale and
>prompt-eye in TUImark.rs, webwhale.tsx/icon.svg, newweb/brand/mark/assets, and updated startup golden files.Reviewed by Cursor Bugbot for commit 4e85a4e. Bugbot is set up for automated code reviews on this repo. Configure here.