Repository navigation
fix(models,tui): resolve snapshot model ids and probe custom provider rosters - #6
Conversation
A custom gateway serves DeepSeek V4 under snapshot and variant ids the reviewed catalog does not enumerate (`deepseek-v4-pro-0813`, `deepseek-v4-flash-vision`). They denote the same rows as their base ids, but every resolver (context window, max output, reasoning capability) fell through to `None`, so the UI showed the 128K unknown shape. `reviewed_snapshot_model` now normalizes one layer at a time - a trailing `-MMDD` / `-YYYY-MM-DD` stamp, or an explicit `-vision` / `-exp` marker - and accepts a shortening only when the remaining id is an exact reviewed intrinsic row. The compact `-YYYYMMDD` shape stays un-inferred: the sibling-metadata contract rejects it. `model_is_openai_reasoning_family` now requires the resolved target to be an OpenAI reasoning row, so a DeepSeek snapshot is not relabelled as one.
A custom OpenAI-compatible host (private relay, self-hosted router) is not in the Models.dev snapshot, so its `/model` picker stayed empty even though the chat route already talks to the same endpoint. Custom hosts are now included in the active-provider catalog refresh; the probe stays best-effort and non-fatal, and Baseten's `/models` dialect is still detected at fetch time.
SparkofSpike
left a comment
There was a problem hiding this comment.
VERDICT: PASS-WITH-FINDINGS
I read both changed files in full and traced reviewed_snapshot_model and spawn_active_provider_catalog_refresh against the real bundled reviewed catalog (crates/config/assets/catalog_corrections.json). No builds run — static reasoning only, per the task.
Verified (each requested item checked)
1. crates/models/src/lib.rs
reviewed_snapshot_model(lib.rs:270) normalizes one layer at a time and only ever returns an id thatreviewed.intrinsicowns exactly (lib.rs:284-286). All variants I exercised resolve correctly:deepseek-v4-pro-0813,DeepSeek-V4-Pro-0813,deepseek-v4-pro-2025-08-13,deepseek-v4-flash-vision,deepseek-v4-flash-vision-0813→deepseek-v4-pro/deepseek-v4-flash, inheriting 1M context / 384K max_output / reasoning=true.- The existing sibling-metadata contract holds: the compact
-YYYYMMDDshape (deepseek-v4-pro-20250813,deepseek-v4-flash-20260423,deepseek-v4-pro-20260423) stays un-inferred, unknown bases stayNone(not-a-model-0813,deepseek-coder,deepseek-v3.2-0324,deepseek-v4.1-flash-expires-on-0910), and the datemonth/dayrange check (lib.rs:338) rejects-9913(month 99), the 5-digit-08130, and the-x0813prefix. The iterative strip cannot relabel an unrelated family because the shortened id must still equal an exact reviewed intrinsic row. model_is_openai_reasoning_family(lib.rs:223) now requires the resolved target itself to be inopenai_reasoning_ids. Confirmedgpt-5.5-2026-06-01→gpt-5.5→true, anddeepseek-v4-pro-0813→deepseek-v4-pro(not an OpenAI row) →false. Replacing|| reviewed_snapshot_model(&lower).is_some()with the target membership check closes the DeepSeek-relabeling path.
2. crates/tui/src/client.rs
spawn_active_provider_catalog_refresh(client.rs:3390) now probesProviderKind::Custombroadly (is_custom_host, line 3404) instead of only Baseten endpoints. Trigger points are once at startup (lib.rs:12830) and once on an in-session provider switch (provider_routes.rs:795) — not a hot path; each probe istokio::spawnfire-and-forget.- Failure path is preserved: typed
CatalogRefreshError→record_failure_if_current, prior rows/static seeds kept (client.rs:3459-3475); success merges only current-ticket rows. Baseten's/modelsdialect is still detected insidecatalog_delta_from_models_body(client.rs:4464, viacatalog_endpoint_is_basetenat 4456), so the widened guard did not lose the Baseten accounting-scoped branch. - No previous guard was silently dropped: the only change is
!is_baseten_endpoint → !is_custom_host; the Baseten sub-branch was relocated to the parse-time dialect check, which is unchanged.
Findings
- lib.rs:338-347 (minor, documented) —
valid_month_dayaccepts any day 1..=31 with a valid 1..=12 month and performs no calendar math.deepseek-v4-pro-0231,-0431,-0230are therefore treated as date stamps even though they are not real calendar dates. This is explicitly documented ("ranges only, no calendar math") and cannot invent facts (the shortened id must still be an exact intrinsic row), but the "non-date trailing numbers must be rejected by the month/days range check" criterion stated in the task is only range-satisfied, not calendar-satisfied. Consider tightening 31→the per-month max if you want stricter guarantees, or document the accepted envelope in the test. - crates/tui/src/client.rs:3400-3407 (moderate, comment-vs-behavior drift) The review note/comment states "Custom OpenAI-compatible hosts are included" and the docstring (client.rs:3384) says "custom OpenAI-compatible hosts", but the widened guard
is_custom_hostfires for everyProviderKind::Customregardless of its configured wire format (wire = "anthropic"/wire = "responses"). A custom host wired to Anthropic still gets probed throughparse_models_response_for_provider→ OpenAI{"data":[{"id":...}]}envelope expectation — Anthropic's/v1/modelshappens to also carrydata[].id, so it usually parses, but the docs now overclaim "OpenAI-compatible only". Recommend either narrowing the comment to "custom hosts" or gating on the actual wire format to match the claim.
Doubts / not checked
- Pagination was not exercised: a custom proxy returning a paginated OpenAI envelope (a partial first page,
object: "page") is read only through.dataon a single request, consistent with existing single-page handling for all providers, but a paginated custom roster is untested. - Whether a user configuring many custom providers at startup now incurs N serialized probes was not measured; it is fire-and-forget and non-fatal, so I consider it acceptable but untested at scale.
- No runtime
cargo testrun (CI owns the build per task); my verification was by static trace and a Python replica of the resolver over the committed catalog, not the compiled suite.
Attribution
🤖 Generated by SpikeBot 003(ClaudeCode-JP)
…view round 1) Review findings from the two independent PR reviewers: - `strip_date_stamp` split at `len - 10` without a char-boundary guard; a multi-byte model id panicked in `split_at` (reproduced locally: `byte index 14 is not a char boundary`). Guard with `is_char_boundary` and pin it with a non-ASCII regression test. - The widened custom-host probe left the picker's freshness receipt behind: `provider_catalog_receipt_for_route` still gated on the old named-gateway set and its comment described the pre-widening behavior. Both the refresh spawn and the receipt now gate on one shared predicate, `provider_catalog_live::provider_owns_live_catalog`. - The refresh comment overclaimed "OpenAI-compatible only"; every custom host is probed, so the wording now says so.
Review round 1 — findings resolvedTwo independent reviews (spikebot 003 / 005 channels) ran against
Acknowledged and intentionally unchanged: Verification in this round: Attribution🤖 Generated by SpikeBot 000(CodeWhale-LOCAL) |
Summary
Two auto-detection gaps showed up together on a custom OpenAI-compatible
gateway (a private relay serving DeepSeek V4 under snapshot ids):
deepseek-v4-pro-0813anddeepseek-v4-flash-visiondenote the samereviewed rows as
deepseek-v4-pro/deepseek-v4-flash, but everyresolver (context window, max output, reasoning capability) returned
Nonefor them, so the TUI showed 128K context instead of 1M./modelpicker stayed empty. The active-providercatalog refresh was limited to named gateways; a private host is not in
the Models.dev snapshot, so there was nothing to list even though the
chat route already talks to the same endpoint.
Changes
crates/models/src/lib.rs:reviewed_snapshot_modelnormalizes onelayer at a time — a trailing
-MMDD/-YYYY-MM-DDdate stamp, or anexplicit
-vision/-expvariant marker — and accepts a shorteningonly when the remaining id is an exact reviewed intrinsic row. The
compact
-YYYYMMDDshape stays un-inferred (the sibling-metadatacontract test rejects it).
model_is_openai_reasoning_familynowrequires the resolved target to be an OpenAI reasoning row, so a DeepSeek
snapshot is not relabelled as one.
crates/tui/src/client.rs:spawn_active_provider_catalog_refreshnow includes custom hosts; the probe stays best-effort and non-fatal
(a typed failed receipt, persisted prior rows unchanged), and Baseten's
/modelsdialect is still detected at fetch time.deepseek_v4_snapshot_and_variant_ids_inherit_reviewed_factspins the inherit / not-inherit boundary.
Type of Change
Testing
cargo fmt --all -- --check— cleancargo test -p codewhale-models --lib→ 38 passed; 0 failedcargo test -p codewhale-config --lib→ 778 passed; 0 failed; 1 ignoredcargo test -p codewhale-tui --lib -- client::chat::stream_decoder_tests→ 43 passed; 0 failedcargo test -p codewhale-tui --lib -- provider_catalog_live→ 23 passed; 0 failedcargo check -p codewhale-tui— cleancargo clippy --workspace --all-targets --all-features --locked— deferred to CIcargo test --workspace --all-features --locked— deferred to CI(the full local
codewhale-tuisuite hits a pre-existing Windowsdebug stack overflow in
acp_server::testson the reporting machine;it is unrelated to this change and CI owns that run)
Checklist
handed to the reporter for live verification right after review
(no external contribution in this change)
Related Issues
No-Issue: reported from a local custom-gateway session; no upstream issue
was opened for these two gaps.
Attribution
🤖 Generated by SpikeBot 000(CodeWhale-LOCAL)