Swap canonical model from openrouter to models.dev #6625
Conversation
…ovider * 'main' of github.com:block/goose: increase worker threads for ci (#6614) docs: todo tutorial update (#6613) Added goose doc map md file for goose agent to find relevant doc easily. (#6598) add back goose branding to home (#6617) fix: actually set the working dir for extensions from session (#6612) Multi chat (#6428) Lifei/fixed accumulated token count (#6587) Dont show MCP UI/Apps until tool is approved (#6492) docs: max tokens config (#6596) User configurable templates (#6420) docs: http proxy environment variables (#6594) feat: exclude subagent tool from code_execution filtering (#6531)
…ovider * 'main' of github.com:block/goose: PR Code Review (#6043) fix(docs): use dynamic import for globby ESM module (#6636) chore: trigger CI Document tab completion (#6635) Install goose-mcp crate dependencies (#6632) feat(goose): standardize agent-session-id for session correlation (#6626) chore: tweak release docs (#6571) fix(goose): propagate session_id across providers and MCP (#6584)
There was a problem hiding this comment.
Pull request overview
This PR swaps the canonical model data source from OpenRouter to models.dev. The key change simplifies model lookups by using (provider, model) keyed data instead of parsing model IDs. The PR also implements sorting by release date and updates pricing to be per-million tokens.
Changes:
- Switched canonical model API from OpenRouter to models.dev
- Refactored registry to use (provider, model) tuple keys instead of single ID strings
- Updated pricing fields from
prompt/completiontoinput/outputwith per-million token rates - Added release date sorting for recommended models
- Implemented
fetch_supported_modelsfor xAI provider
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/desktop/src/api/types.gen.ts | Updated comment to clarify costs are in USD |
| ui/desktop/openapi.json | Updated OpenAPI spec comments for cost fields |
| crates/goose/tests/providers.rs | Added openrouter to default truncation test exception |
| crates/goose/src/providers/xai.rs | Added fetch_supported_models implementation |
| crates/goose/src/providers/utils.rs | Added "maximum prompt length" to context exceeded detection |
| crates/goose/src/providers/canonical/registry.rs | Changed registry from HashMap to HashMap<(String, String)> |
| crates/goose/src/providers/canonical/name_builder.rs | Simplified lookup logic for non-meta providers |
| crates/goose/src/providers/canonical/model.rs | Restructured model to match models.dev schema |
| crates/goose/src/providers/canonical/mod.rs | Updated to parse canonical IDs for registry lookup |
| crates/goose/src/providers/canonical/data/canonical_mapping_report.json | Updated with new mapping data |
| crates/goose/src/providers/canonical/build_canonical_models.rs | Rewrote to fetch from models.dev instead of OpenRouter |
| crates/goose/src/providers/base.rs | Added release date sorting for recommended models |
| crates/goose-server/src/routes/config_management.rs | Converted pricing from per-million to per-token |
| crates/goose-cli/src/session/output.rs | Converted pricing from per-million to per-token |
crates/goose/src/providers/base.rs
Outdated
| let is_text_capable = canonical_id | ||
| .split_once('/') | ||
| .and_then(|(p, m)| registry.get(p, m)) | ||
| .map(|m| m.modalities.input.contains(&"text".to_string())) | ||
| .unwrap_or(false); | ||
|
|
||
| if !is_text_capable { | ||
| return None; | ||
| } | ||
|
|
||
| let release_date = canonical_id | ||
| .split_once('/') | ||
| .and_then(|(p, m)| registry.get(p, m)) | ||
| .and_then(|canonical_model| canonical_model.release_date.clone()); |
There was a problem hiding this comment.
The registry is looked up twice for each model in the filter_map - once to check is_text_capable (lines 462-466) and again to get release_date (lines 472-475). This is inefficient. Consider looking up the canonical model once and reusing it.
| let is_text_capable = canonical_id | |
| .split_once('/') | |
| .and_then(|(p, m)| registry.get(p, m)) | |
| .map(|m| m.modalities.input.contains(&"text".to_string())) | |
| .unwrap_or(false); | |
| if !is_text_capable { | |
| return None; | |
| } | |
| let release_date = canonical_id | |
| .split_once('/') | |
| .and_then(|(p, m)| registry.get(p, m)) | |
| .and_then(|canonical_model| canonical_model.release_date.clone()); | |
| let (provider, model_name) = match canonical_id.split_once('/') { | |
| Some(parts) => parts, | |
| None => return None, | |
| }; | |
| let canonical_model = match registry.get(provider, model_name) { | |
| Some(m) => m, | |
| None => return None, | |
| }; | |
| if !canonical_model | |
| .modalities | |
| .input | |
| .contains(&"text".to_string()) | |
| { | |
| return None; | |
| } | |
| let release_date = canonical_model.release_date.clone(); |
|
|
||
| pub fn maybe_get_canonical_model(provider: &str, model: &str) -> Option<CanonicalModel> { | ||
| let registry = CanonicalModelRegistry::bundled().ok()?; | ||
| map_to_canonical_model(provider, model, registry)?; |
There was a problem hiding this comment.
This function calls map_to_canonical_model twice with the same arguments. The first call on line 26 is unused - its result is ignored. This appears to be leftover debug code that should be removed.
| map_to_canonical_model(provider, model, registry)?; |
…ovider * 'main' of github.com:block/goose: fix: Manual compaction does not update context window. (#6682) Removed the Acceptable Usage Policy (#6204) Document spellcheck toggle (#6721) fix: docs workflow cleanup and prevent cancellations (#6713) Docs: file bug directly (#6718) fix: dispatch ADD_ACTIVE_SESSION event before navigating from "View All" (#6679) Speed up Databricks provider init by removing fetch of supported models (#6616) fix: correct typos in documentation and Justfile (#6686) docs: frameDomains and baseUriDomains for mcp apps (#6684)
…ovider * 'main' of github.com:block/goose: fix slash and @ keyboard navigation popover background color (#6550) fix[format/openai]: return error on empty msg. (#6511) Fix: ElevenLabs API Key Not Persisting (#6557) Logging uplift for model training purposes (command injection model) [Small change] (#6330) fix(goose): only send agent-session-id when a session exists (#6657) BERT-based command injection detection in tool calls (#6599) chore: [CONTRIBUTING.md] add Hermit to instructions (#6518) fix: update Gemini context limits (#6536) Document r slash command (#6724) Upgrade GitHub Actions to latest versions (#6700)
Remove session_id parameters from fetch_supported_models and fetch_recommended_models calls to match the updated Provider trait signature that doesn't include session_id. - xai.rs: Remove session_id param, pass None to response_get - build_canonical_models.rs: Remove session_id arg from fetch_recommended_models call
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
Yeah seems good. My only concern is the whole tokens per millions and where convert this. We should fix this at the beginning or the end
| model: query.model.clone(), | ||
| input_token_cost: input_cost, | ||
| output_token_cost: output_cost, | ||
| // Canonical model costs are per million tokens, convert to per-token |
There was a problem hiding this comment.
needing this comment suggests that we need to refactor this more either at the beginning or at the end - let's have one unit we pass around
There was a problem hiding this comment.
My followup pr deletes the pricing API and replaces it with a full canonical model fetch (or at least just the fields the client needs, but gives us a place to layer on more). will keep as cost / mtokens until we render.
|
|
||
| /// Output modalities (e.g., ["text"]) | ||
| #[serde(default)] | ||
| pub output: Vec<String>, |
There was a problem hiding this comment.
can we make these enums instead of string, ignoring the ones we don't know about?
| pub output: Vec<String>, | ||
| } | ||
|
|
||
| /// Pricing/cost information for a model |
| /// Cost per image | ||
| /// Maximum output/completion tokens | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub image: Option<f64>, |
There was a problem hiding this comment.
do we not have this anymore? we're probably not using it anyway, but still
There was a problem hiding this comment.
Yeah don't have it anymore. Definitely weren't using it, I had just pulled all the fields in from openrouter, but still a slight loss.
| } | ||
|
|
||
| /// Canonical representation of a model | ||
| /// Canonical representation of a model (mirrors models.dev API structure) |
There was a problem hiding this comment.
remove addition to comment
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub open_weights: Option<bool>, | ||
|
|
||
| /// Pricing information |
| "max_tokens", | ||
| "decrease input length", | ||
| "context limit", | ||
| "maximum prompt length", |
There was a problem hiding this comment.
did we run into this? where? the string "too long" above certainly has no place here
There was a problem hiding this comment.
hit this with x.ai testing.
There was a problem hiding this comment.
A couple of these definitely feel over generic, but I'm a little hesitant to change them. iirc too long was the databricks issue but don't remember the full error message.
crates/goose/src/providers/xai.rs
Outdated
| } | ||
| } | ||
|
|
||
| // Fallback to known models if API doesn't return what we expect |
There was a problem hiding this comment.
I'm not sure we want to do that. I just merged something that has can't reach the provider - if this happens either the provider is down or the user is using some alternate host. we should just bubble up the error or return [] - whatever it is we do elsewhere
| models_with_dates.sort_by(|a, b| match (&a.1, &b.1) { | ||
| (Some(date_a), Some(date_b)) => date_b.cmp(date_a), | ||
| (Some(_), None) => std::cmp::Ordering::Less, | ||
| (None, Some(_)) => std::cmp::Ordering::Greater, | ||
| (None, None) => a.0.cmp(&b.0), | ||
| }); |
There was a problem hiding this comment.
The sorting logic has an issue. When comparing models with release dates, date_b.cmp(date_a) will sort lexicographically, not chronologically. Since dates are in "YYYY-MM-DD" format, this works correctly (newer dates sort first), but it would be clearer to add a comment explaining this relies on ISO 8601 date format.
| .get("limit") | ||
| .and_then(|l| l.get("context")) | ||
| .and_then(|v| v.as_u64()) | ||
| .unwrap_or(128_000) as usize, |
There was a problem hiding this comment.
The default context limit of 128,000 tokens is arbitrary and may be misleading for models that don't have this data. Consider using a smaller, more conservative default (e.g., 4096 or 8192), or making this field required if context limits are essential for functionality.
| .unwrap_or(128_000) as usize, | |
| .unwrap_or(8_192) as usize, |
* 'main' of github.com:block/goose: (62 commits) Swap canonical model from openrouter to models.dev (#6625) Hook thinking status (#6815) Fetch new skills hourly (#6814) copilot instructions: Update "No prerelease docs" instruction (#6795) refactor: centralize audience filtering before providers receive messages (#6728) update doc to remind contributors to activate hermit and document minimal npm and node version (#6727) nit: don't spit out compaction when in term mode as it fills up the screen (#6799) fix: correct tool support detection in Tetrate provider model fetching (#6808) Session manager fixes (#6809) fix(desktop): handle quoted paths with spaces in extension commands (#6430) fix: we can default gooseignore without writing it out (#6802) fix broken link (#6810) docs: add Beads MCP extension tutorial (#6792) feat(goose): add support for AWS_BEARER_TOKEN_BEDROCK environment variable (#6739) [docs] Add OSS Skills Marketplace (#6752) feat: make skills available in codemode (#6763) Fix: Recipe Extensions Not Loading in Desktop (#6777) Different approach to determining final confidence level of prompt injection evaluation outcomes (#6729) fix: read_resource_tool deadlock causing test_compaction to hang (#6737) Upgrade error handling (#6747) ...
…sion-session * 'main' of github.com:block/goose: feat: platform extension migrator + code mode rename (#6611) feat: CLI flag to skip loading profile extensions (#6780) Swap canonical model from openrouter to models.dev (#6625) Hook thinking status (#6815) Fetch new skills hourly (#6814)
* main: docs: usage data collection (#6822) feat: platform extension migrator + code mode rename (#6611) feat: CLI flag to skip loading profile extensions (#6780) Swap canonical model from openrouter to models.dev (#6625) Hook thinking status (#6815) Fetch new skills hourly (#6814) copilot instructions: Update "No prerelease docs" instruction (#6795) refactor: centralize audience filtering before providers receive messages (#6728) update doc to remind contributors to activate hermit and document minimal npm and node version (#6727) nit: don't spit out compaction when in term mode as it fills up the screen (#6799) fix: correct tool support detection in Tetrate provider model fetching (#6808) Session manager fixes (#6809) fix(desktop): handle quoted paths with spaces in extension commands (#6430) fix: we can default gooseignore without writing it out (#6802) fix broken link (#6810) docs: add Beads MCP extension tutorial (#6792) feat(goose): add support for AWS_BEARER_TOKEN_BEDROCK environment variable (#6739)
Summary
Makes the lookups a lot more straightforward since models.dev has data keyed by (provider, model). Gets rid of lots of hacky text parsing. Still leaving in some level of text parsing for 'meta-providers' like databricks and bedrock. Sort by release date (works great!).
Testing
Tested Model filtering, sorting cost with:
Coming in Followup