Skip to content

fix: include all models in provider model listing, not just canonical…#8001

Open
octogonz wants to merge 1 commit intoblock:mainfrom
octogonz:fix-model-list-filtering
Open

fix: include all models in provider model listing, not just canonical…#8001
octogonz wants to merge 1 commit intoblock:mainfrom
octogonz:fix-model-list-filtering

Conversation

@octogonz
Copy link

Summary

Our company uses a custom proxy that implements an Anthropic-compatible API. When connecting Goose, it correctly calls the /v1/models endpoint to enumerate available models. But several models like glm-5 and kimi-k2 were mysteriously missing from the dropdown. The models worked fine when manually specified in config.yaml, yet the UI refused to show them.

Details

After debugging, it turns out fetch_recommended_models was filtering out any model that couldn't be mapped to a canonical model ID in Goose's bundled registry. The code tried to look up each model name in the registry to get metadata like release dates and tool support—but if the lookup failed, the model was discarded entirely.

This is inconsistent with how the runtime actually works. When you configure glm-5 in config.yaml, the ModelConfig::with_canonical_limits function gracefully handles missing canonical metadata—it just skips filling in defaults. The model runs perfectly fine without it.

So the listing code was treating the registry as required while the execution code treated it as optional. If you think about it, why should a model be blocked from appearing in a dropdown just because Goose doesn't know its release date? The metadata is nice to have, not a prerequisite for using the model.

Fix

Changed fetch_recommended_models to include all models, using the canonical registry only for optional sorting metadata when available. Models with known release dates sort newest-first; unknown models sort alphabetically after them. Now all the relevant models appear in the combo box.

Testing

  • cargo test -p goose passes
  • Verified glm-5 now appears in model listing test output
  • Custom provider model selection works in the desktop UI

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec0e0e4cfa

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +553 to +555
let mut models_with_dates: Vec<(String, Option<String>)> = all_models
.iter()
.filter_map(|model| {
let canonical_id = map_to_canonical_model(provider_name, model, registry)?;

let (provider, model_name) = canonical_id.split_once('/')?;
let canonical_model = registry.get(provider, model_name)?;

if !canonical_model
.modalities
.input
.contains(&crate::providers::canonical::Modality::Text)
{
return None;
}

if !canonical_model.tool_call && !self.get_model_config().toolshim {
return None;
}

let release_date = canonical_model.release_date.clone();

Some((model.clone(), release_date))
.map(|model| {

Choose a reason for hiding this comment

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

P1 Badge Restore text-model filtering before returning recommendations

When the upstream /models response includes non-chat SKUs, this now returns them as selectable models. The OpenAI and Google providers both expose every id from their model endpoint in fetch_supported_models (crates/goose/src/providers/openai.rs:368-393, crates/goose/src/providers/google.rs:150-171), but Goose later issues chat-style requests against the chosen id. Removing the old modalities.input.contains(Text) check means embedding/image-only models can surface in the CLI/UI picker and then fail on the first request.

Useful? React with 👍 / 👎.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@octogonz I think we need to keep the modalities.input.contains(Text) check -- if the model cannot support text input then it's not usable

Comment on lines +555 to +559
.map(|model| {
let release_date = map_to_canonical_model(provider_name, model, registry)
.and_then(|canonical_id| {
let (provider, model_name) = canonical_id.split_once('/')?;
registry.get(provider, model_name)

Choose a reason for hiding this comment

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

P1 Badge Keep no-tool models out unless toolshim is enabled

This change also drops the previous tool_call gate, so fetch_recommended_models now advertises models that Goose cannot drive in the normal tool-calling path. In regular sessions reply_parts.rs:197-205 forwards real tools unless toolshim is enabled, and the OpenAI request builder attaches them whenever present (crates/goose/src/providers/formats/openai.rs:771-789); it already has to reject o1-mini specifically because Goose relies on tool calling (crates/goose/src/providers/formats/openai.rs:756-759). Returning those models here will let users pick configurations that break as soon as tools are exposed.

Useful? React with 👍 / 👎.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This also seems important; if the model doesn't support tool calling, it will be nearly unusable unless toolshim is used, and I don't see that toolshim would get enabled if we don't have any info about the model

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: abd696ef94

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@octogonz octogonz force-pushed the fix-model-list-filtering branch from abd696e to 72184b7 Compare March 20, 2026 05:06
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72184b727c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +460 to +461
fn provider_type(&self) -> ProviderType {
ProviderType::Custom

Choose a reason for hiding this comment

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

P1 Badge Plumb the real provider type into model filtering

ProviderRegistry already classifies Google, OpenRouter, Tetrate, etc. as built-in/preferred (crates/goose/src/providers/init.rs:47-69), but this new trait default makes every provider act like Custom unless it overrides provider_type(). In this patch only OpenAI and Anthropic add an override (crates/goose/src/providers/openai.rs:364-370, crates/goose/src/providers/anthropic.rs:192-198), so providers like Google still take the relaxed allows_unknown_models path even though their fetch_supported_models returns raw /models ids (crates/goose/src/providers/google.rs:150-169). Any newly launched Gemini/OpenRouter model that is missing from the bundled registry will now bypass the text/tool-support checks and be offered as selectable again.

Useful? React with 👍 / 👎.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9fcece6c63

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +617 to +619
} else if allows_unknown_models {
// Custom/declarative providers: include unknown models
Some((model.clone(), None))

Choose a reason for hiding this comment

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

P1 Badge Reject unknown custom-host models without capability data

Fresh evidence: although the Text/tool_call checks were restored for canonical matches, the new None arm still bypasses them for every model id that the bundled registry cannot map. On custom-host OpenAI/Anthropic providers, provider_type() now forces the relaxed path, while fetch_supported_models() in openai.rs/anthropic.rs still returns every /models id and stream() later sends chat/message requests against the chosen id. If a compatibility proxy exposes an embedding-only or no-tool SKU that Goose doesn't know canonically, it will now show up in the picker and fail on the first turn.

Useful? React with 👍 / 👎.

Comment on lines +368 to +370
fn provider_type(&self) -> crate::providers::base::ProviderType {
if self.name == OPEN_AI_PROVIDER_NAME && !self.is_custom_host {
crate::providers::base::ProviderType::Builtin

Choose a reason for hiding this comment

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

P2 Badge Treat official OpenAI fine-tunes as selectable models

This keeps the default-host OpenAI provider on the strict canonical-only path, so account-specific ids from /v1/models—most notably ft: fine-tunes—are still dropped before they reach the CLI/UI picker. fetch_supported_models() returns those ids, ModelConfig::with_canonical_limits() in model.rs already tolerates unknown models, and stream() forwards the raw model_name, so these models remain usable when typed manually but invisible in model selection.

Useful? React with 👍 / 👎.

@octogonz octogonz force-pushed the fix-model-list-filtering branch from 3f83f4e to 4276c7c Compare March 20, 2026 05:46
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4276c7cb6e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

custom_headers,
supports_streaming: true,
name: OPEN_AI_PROVIDER_NAME.to_string(),
is_custom_host: host != "https://api.openai.com",

Choose a reason for hiding this comment

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

P2 Badge Normalize OpenAI host before deciding it's custom

When OPENAI_HOST is set to an equivalent official URL like https://api.openai.com/ or https://api.openai.com:443, this raw string comparison flips the provider to Custom even though ApiClient::build_url() still resolves requests against the normal OpenAI API (crates/goose/src/providers/api_client.rs:325-337). fetch_recommended_models() then takes the relaxed allows_unknown_models branch for custom providers (crates/goose/src/providers/base.rs:557-607), so the built-in model picker stops applying the text/tool capability gate just because the host string was formatted differently.

Useful? React with 👍 / 👎.

@octogonz octogonz force-pushed the fix-model-list-filtering branch from 4276c7c to 10c4b86 Compare March 20, 2026 06:14
My company has a custom proxy that exposes an Anthropic-compatible API. When connecting Goose, it correctly calls the /v1/models endpoint to enumerate available models—but several models like glm-5 and kimi-k2 were mysteriously missing from the dropdown. The models worked fine when manually specified in config.yaml, yet the UI refused to show them.

After debugging, it turns out fetch_recommended_models was filtering out any model that couldn't be mapped to a canonical model ID in Goose's bundled registry. The code tried to look up each model name in the registry to get metadata like release dates and tool support—but if the lookup failed, the model was discarded entirely.

This is inconsistent with how the runtime actually works. When you configure glm-5 in config.yaml, the ModelConfig::with_canonical_limits function gracefully handles missing canonical metadata—it just skips filling in defaults. The model runs perfectly fine without it.

So the listing code was treating the registry as required while the execution code treated it as optional. If you think about it, why should a model be blocked from appearing in a dropdown just because Goose doesn't know its release date? The metadata is nice to have, not a prerequisite for using the model.

Fix:

- Changed fetch_recommended_models to branch on provider_type()

- Built-in OpenAI/Anthropic (with default host): strict filtering (require canonical metadata, text modality, tool support)

- Custom/declarative providers: relaxed filtering (allow unknown models, use canonical metadata when available for sorting)

- Alternate-host OpenAI/Anthropic (OPENAI_HOST/ANTHROPIC_HOST set): treated as custom

- Default provider_type is Custom, only OpenAI/Anthropic override to Builtin when using canonical names and default host

- Registry load errors are propagated instead of silently returning empty list

- Added regression tests for custom-include vs builtin-exclude behavior

Signed-off-by: Pete Gonzalez <4673363+octogonz@users.noreply.github.com>
@octogonz octogonz force-pushed the fix-model-list-filtering branch from 10c4b86 to 4ff33d1 Compare March 20, 2026 06:17
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ff33d1f04

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

.get_param("ANTHROPIC_HOST")
.unwrap_or_else(|_| "https://api.anthropic.com".to_string());

let is_custom_host = host != "https://api.anthropic.com";

Choose a reason for hiding this comment

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

P2 Badge Normalize Anthropic host before deciding it's custom

When ANTHROPIC_HOST is set to an equivalent official URL such as https://api.anthropic.com/ or https://api.anthropic.com:443, this raw string comparison flips the provider to Custom even though ApiClient::build_url() still resolves requests against the normal Anthropic API (crates/goose/src/providers/api_client.rs). That changes fetch_recommended_models() onto the relaxed custom-provider path in base.rs, so the CLI/UI model picker stops applying the usual canonical filtering just because the host string was formatted differently.

Useful? React with 👍 / 👎.

@DOsinga
Copy link
Collaborator

DOsinga commented Mar 20, 2026

I think you want to add this as a flag to the custom providers whether they should filter to the canonical models or not, not try to guess this yourself.

@DOsinga DOsinga added the needs_human label to set when a robot looks at a PR and can't handle it label Mar 20, 2026
DOsinga pushed a commit that referenced this pull request Mar 21, 2026
Declarative providers serving arbitrary local models (e.g. LM Studio)
have model names that won't match the canonical registry, so filtering
them out leaves an empty model list.

Add a boolean skip_canonical_filtering field to DeclarativeProviderConfig
(defaults false) and honour it in fetch_recommended_models. Set it to
true in lmstudio.json so all locally-served models are returned as-is.

Fixes the same problem addressed by #8043 and #8001.

Signed-off-by: Douwe Osinga <douwe@squareup.com>
@DOsinga
Copy link
Collaborator

DOsinga commented Mar 21, 2026

This is a real problem and the investigation here is solid. We've taken a narrower approach in #8052 — a single skip_canonical_filtering: bool field on DeclarativeProviderConfig (defaults false) wired into fetch_recommended_models. LM Studio's JSON sets it to true. No ProviderType enum needed, no changes to built-in providers. Closing this in favour of that fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs_human label to set when a robot looks at a PR and can't handle it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants