Skip to content

fix(catalog): demote bundled Models.dev snapshot (#4188) - #4248

Merged
Hmbown merged 1 commit into
mainfrom
codex/v0868-fix-4188-demote-bundled
Jul 8, 2026
Merged

fix(catalog): demote bundled Models.dev snapshot (#4188)#4248
Hmbown merged 1 commit into
mainfrom
codex/v0868-fix-4188-demote-bundled

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Demote models_dev.bundled.json / ProviderLake docs so the bundled asset is an offline/stale fallback, not competing curated truth after live Models.dev (Fetch and cache live Models.dev catalog into ProviderLake #4187).
  • Document model_completion_names_for_provider() as a compatibility fallback only for CodeWhale-only / unbundled providers; pickers/inventory/subagents stay lake-backed.
  • Add tests for live > bundled > legacy precedence, CodeWhale-only defaults (openai-codex), and alias-normalized dedupe (moonshotai/moonshot).
  • Fix models_dev_live network-failure test to avoid clippy::await_holding_lock under -Dwarnings.

Closes #4188.

Test plan

  • cargo fmt --all --check
  • cargo test -p codewhale-tui --bin codewhale-tui --locked 'provider_lake::'
  • cargo test -p codewhale-tui --bin codewhale-tui --locked 'models_dev_live::'
  • cargo test -p codewhale-config --locked bundled_asset_
  • RUSTFLAGS=-Dwarnings cargo clippy -p codewhale-tui -p codewhale-config --locked --tests (with playbook allows)
  • CI green on PR

Made with Cursor

…4188)

Prefer live Models.dev over the bundled seed and keep hardcoded completion names only for CodeWhale-only / unbundled providers, with tests for precedence and alias dedupe.
Copilot AI review requested due to automatic review settings July 8, 2026 19:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 2m 40s —— View job


Review: demote bundled Models.dev snapshot to offline fallback (#4188)

  • Gather context (PR description, changed files list)
  • Read diff against origin/main
  • Review catalog.rs / provider_lake.rs precedence logic
  • Review model_reference.rs / models_dev_live.rs / subagent/mod.rs changes
  • Check tests added for coverage
  • Post final review

Note: cargo/cargo test invocations were blocked by the sandbox in this environment (permission required, none available to grant), so this review is diff-read-only — I could not independently re-run the PR's test plan. Everything below is from static reading.

Summary

This PR is almost entirely doc-comments, _meta JSON text, and new/converted tests — the only executable-logic diff is in crates/tui/src/provider_lake.rs test module and the async→sync conversion in crates/tui/src/models_dev_live.rs. merged_snapshot() / all_catalog_models_for_provider() in provider_lake.rs are behaviorally unchanged from before this PR (live > bundled merge on (provider, wire_model_id), legacy fallback only when the merged catalog is empty for that provider) — this PR just documents that precedence more clearly and adds regression tests for it. Good alignment with the #2608 provider-safety invariant: nothing here infers a provider from a wire-id prefix or freeform text; precedence is purely catalog-source based (CatalogSource::Bundled vs Live), which is the correct axis.

Things that check out

  • crates/tui/src/provider_lake.rs:279-296 (codewhale_only_providers_keep_legacy_defaults) correctly distinguishes "CodeWhale-only provider with a legacy default" (openai-codex) from "provider intentionally empty everywhere" (ollama) — good edge-case coverage, not just the happy path.
  • crates/tui/src/provider_lake.rs:367-421 (live_over_bundled_over_legacy_precedence_and_alias_dedupe) exercises identity-override (live row replacing a bundled row with the same wire id) and alias-collapse (moonshotaimoonshotai... actually moonshot) in the same test — matches the PR's stated goal.
  • crates/tui/src/models_dev_live.rs:591-621: converting network_failure_keeps_prior_rows from #[tokio::test] async fn to #[test] fn + a dedicated current_thread runtime + block_on is the right fix for clippy::await_holding_lock — the MutexGuard from lock_test_env() is now held across a synchronous function body with no real .await point, matching the pattern already used by the neighboring publish_from_path_updates_provider_lake test (line 497) rather than inventing a new convention.
  • crates/config/src/catalog/tests.rs:542-565 (bundled_asset_meta_describes_offline_fallback_not_competing_truth) is a lightweight guard against someone reverting the demotion language in the asset's _meta.role field later; matches the new role key added in models_dev.bundled.json.

Minor observations (non-blocking)

  • The new bundled_asset_meta_describes_offline_fallback_not_competing_truth test asserts on lowercase substring matches in a free-text _meta.role string ("not a competing", "live"). It's an intentional low-cost tripwire rather than testing real behavior, and it's a little brittle to copy edits — that seems like an accepted tradeoff here for keeping the doc/behavior in sync, not a bug.
  • catalog_provider_id() (crates/tui/src/provider_lake.rs:76-82) and the legacy-fallback branch in all_catalog_models_for_provider() are unchanged by this PR; the new tests only cover them at the margins exercised here (OpenaiCodex, Ollama, Moonshot, Together, Deepseek). No gap introduced by this PR, just noting the fallback logic itself wasn't touched.

No correctness bugs, no provider/model-safety violations, and no unnecessary duplication found in the diff. Test additions are meaningful (they encode the stated precedence and alias-dedupe invariants rather than just re-asserting existing behavior).

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.

Demote curated bundled model data after Models.dev live catalog lands

2 participants