fix(together): parse model list response - #10576
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65935aee7f
ℹ️ 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".
| let mut model_ids: Vec<String> = models | ||
| .iter() | ||
| .filter_map(|m| m.get("id").and_then(|v| v.as_str()).map(str::to_string)) | ||
| .collect(); |
There was a problem hiding this comment.
When a newly supported top-level array response is syntactically an array but its entries do not contain string id fields (for example [{}] or a provider schema change), this filter_map drops every item and returns Ok([]). For Together/custom OpenAI providers using dynamic_models, fetch_supported_models then accepts the empty API result and does not fall back to the static model list because fallback only happens on EndpointNotFound, so a malformed response is exposed as an empty supported-models list instead of a request error. Please fail when no model ids can be parsed from a non-empty models array.
Useful? React with 👍 / 👎.
65935ae to
8b113c3
Compare
michaelneale
changed the title
8b113c3 to
ba883de
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba883de63d
ℹ️ 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".
| let mut model_ids: Vec<String> = models | ||
| .iter() | ||
| .filter_map(|m| m.get("id").and_then(|v| v.as_str()).map(str::to_string)) | ||
| .collect(); |
There was a problem hiding this comment.
When Together's dynamic_models path starts using this top-level array parser, every returned id is exposed as a supported model even though Together's /models schema includes non-chat types such as image, embedding, moderation, and rerank. fetch_supported_models is used directly for the supported-model list, and this provider only sends selected models to chat/responses endpoints, so those non-LLM entries can be offered to users and then fail at runtime. Please filter the array to chat-compatible types before collecting ids for top-level Together-style responses.
Useful? React with 👍 / 👎.
|
I don't think the test failure is related to the changes here. Merging. |
* main: fix(desktop): allow removing images when editing user messages (#9979) Fetch cursor-agent models from CLI (#10367) fix(cerebras): preserve thinking inline for models that reject reasoning_content (#10774) perf: make CLI streaming render incremental instead of O(n^2) (#10487) Moving to issues as the new PRs (#10818) ci: defer privileged recipe scans for forks (#10825) docs: adopt issue-first contribution workflow (#10819) Add Azure AI Foundry multi-LLM provider (#10622) fix(together): parse model list response (#10576) docs: remove retired site resources (#10817) feat: streaming shell output while commands run (#10808)
* main: (224 commits) fix(desktop): allow removing images when editing user messages (aaif-goose#9979) Fetch cursor-agent models from CLI (aaif-goose#10367) fix(cerebras): preserve thinking inline for models that reject reasoning_content (aaif-goose#10774) perf: make CLI streaming render incremental instead of O(n^2) (aaif-goose#10487) Moving to issues as the new PRs (aaif-goose#10818) ci: defer privileged recipe scans for forks (aaif-goose#10825) docs: adopt issue-first contribution workflow (aaif-goose#10819) Add Azure AI Foundry multi-LLM provider (aaif-goose#10622) fix(together): parse model list response (aaif-goose#10576) docs: remove retired site resources (aaif-goose#10817) feat: streaming shell output while commands run (aaif-goose#10808) feat(acp): allow session/new to request a Hidden session via _meta.hidden (aaif-goose#10811) chore: removed stale text pnpm workspace (aaif-goose#10809) feat(otel): emit GenAI semantic convention attributes (aaif-goose#10700) fix: avoid double loading full conversation on session open (aaif-goose#10794) feat(providers): forward images and MCP embedded-resource blobs in Anthropic and Google formats (aaif-goose#10340) Deprecate and remove ui/text TUI (aaif-goose#10799) Upgrade to rmcp 3.0 (aaif-goose#10789) fix(release): signing environment (aaif-goose#10797) feat(tools): collapse const-union enums in tool schemas (aaif-goose#10577) ...
3 participants
Together’s
/v1/modelsendpoint returns a top-level array rather than OpenAI’sdataenvelope. Accept both model-list response shapes in the OpenAI-compatible parser.Tests:
cargo test -p goose-providers --lib@DOsinga @jamadeo I hate this - but not sure of a better way at the moment. Together just doesn't use "data" like every other openai model listing.