-
Notifications
You must be signed in to change notification settings - Fork 6k
fix(together): parse model list response #10576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -468,15 +468,7 @@ impl OpenAiProvider { | |
| return Err(ProviderError::Authentication(msg.to_string())); | ||
| } | ||
|
|
||
| let data = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { | ||
| ProviderError::UsageError("Missing data field in JSON response".into()) | ||
| })?; | ||
| let mut models: Vec<String> = data | ||
| .iter() | ||
| .filter_map(|m| m.get("id").and_then(|v| v.as_str()).map(str::to_string)) | ||
| .collect(); | ||
| models.sort(); | ||
| Ok(models) | ||
| parse_model_ids(&json) | ||
| } | ||
|
|
||
| /// llama.cpp and Ollama expose the actual allocated context window in the | ||
|
|
@@ -496,6 +488,22 @@ impl OpenAiProvider { | |
| } | ||
| } | ||
|
|
||
| fn parse_model_ids(json: &serde_json::Value) -> Result<Vec<String>, ProviderError> { | ||
| let models = json | ||
| .get("data") | ||
| .and_then(|value| value.as_array()) | ||
| .or_else(|| json.as_array()) | ||
| .ok_or_else(|| { | ||
| ProviderError::RequestFailed("Missing models array in JSON response".into()) | ||
| })?; | ||
| 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(); | ||
|
Comment on lines
+499
to
+502
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Together's Useful? React with 👍 / 👎. |
||
| model_ids.sort(); | ||
| Ok(model_ids) | ||
| } | ||
|
|
||
| /// Extract `meta.n_ctx` for `model_name` from a `/v1/models` response body. | ||
| fn parse_n_ctx_from_models(json: &serde_json::Value, model_name: &str) -> Option<usize> { | ||
| let data = json.get("data")?.as_array()?; | ||
|
|
@@ -1152,6 +1160,36 @@ mod tests { | |
| assert_eq!(models_path, "openai/v1/models"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_model_ids_accepts_openai_response() { | ||
| let response = json!({"data": [{"id": "model-b"}, {"id": "model-a"}]}); | ||
|
|
||
| assert_eq!(parse_model_ids(&response).unwrap(), ["model-a", "model-b"]); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_model_ids_accepts_together_response() { | ||
| let response = json!([ | ||
| {"id": "meta-llama/Llama-3.3-70B-Instruct-Turbo", "type": "chat"}, | ||
| {"id": "Qwen/Qwen3-Coder-480B-A35B-Instruct-FP8", "type": "code"} | ||
| ]); | ||
|
|
||
| assert_eq!( | ||
| parse_model_ids(&response).unwrap(), | ||
| [ | ||
| "Qwen/Qwen3-Coder-480B-A35B-Instruct-FP8", | ||
| "meta-llama/Llama-3.3-70B-Instruct-Turbo" | ||
| ] | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_model_ids_rejects_unknown_response() { | ||
| let response = json!({"models": []}); | ||
|
|
||
| assert!(parse_model_ids(&response).is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn unknown_path_falls_back_to_default_models_path() { | ||
| let models_path = OpenAiProvider::map_base_path("custom/path", "models", "v1/models"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a newly supported top-level array response is syntactically an array but its entries do not contain string
idfields (for example[{}]or a provider schema change), thisfilter_mapdrops every item and returnsOk([]). For Together/custom OpenAI providers usingdynamic_models,fetch_supported_modelsthen accepts the empty API result and does not fall back to the static model list because fallback only happens onEndpointNotFound, 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 👍 / 👎.