Repository navigation
Tolerate incomplete model listings from OpenAI-compatible endpoints, and bump openai_dive to 1.4.3 - #117
Merged
Conversation
openai_dive deserializes GET /models into ListModelResponse, where the top-level `object` field and each Model's `object` / `owned_by` are mandatory Strings. Several openai-compatible gateways omit them, so the whole listing failed with an opaque "missing field `object`" even though the payload contained perfectly usable model ids. This broke `shai auth` model selection and default_model() for those endpoints. Add providers::models::list_models_compat, which fetches the listing with the client's own http client and only requires `id` per entry, defaulting object to "model" and leaving owned_by/created empty when absent. It also accepts a bare top-level array, skips entries without a usable id, and reports non-2xx responses with the status and a truncated body instead of a parse error. Used by the four providers that went through the strict listing: openai, openai_compatible, ovhcloud and ollama. Upgrading openai_dive would not help here: 1.4.3 still declares those fields as mandatory.
1.3.1 -> 1.4.3 across shai-llm, shai-core, shai-cli and shai-http. Breaking changes handled: - ChatMessage::Assistant, DeltaChatMessage::Assistant and DeltaChatMessage::Untagged gained a `reasoning` field alongside the existing `reasoning_content`. All initializers now set it. - shared::Usage lost input_tokens, input_tokens_details, output_tokens and output_tokens_details; the Responses API got its own ResponseUsage type carrying them instead. Every site we had set those four to None, so they are simply dropped from the chat Usage literals. The Responses API formatter now builds a ResponseUsage, which also makes its emitted usage object spec-correct: it previously reported chat-style prompt_tokens / completion_tokens inside a Responses payload. - ResponseObject gained top_logprobs, and items::FunctionToolCall made `id` and `status` optional. Providers are split on how they report reasoning: some use `reasoning`, some `reasoning_content`. extract_think_content now normalizes the former onto the latter, which is what the agent and the TUI read, so reasoning from those models is displayed instead of silently dropped. The existing <think> extraction still takes precedence. Added tests covering the parts of the upgrade that could regress silently: 1.4.x switched several enums from rename_all = "lowercase" to "snake_case", so role tags, tool types and tool-choice values are asserted to serialize unchanged, along with the reasoning normalization and <think> extraction. Verified against a live openai-compatible endpoint: model listing, chat with function calling, tool-call parsing and token usage all work, for FunctionCall, FunctionCallRequired and StructuredOutput alike. No test regressions: the same 36 pre-existing failures (live tests needing credentials, plus an unrelated ringbuffer panic) before and after.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
shai authfails to list models against some OpenAI-compatible endpoints with:The cause is deserialization strictness, not the endpoint.
openai_divedeclares:Three of those fields are mandatory
String. Plenty of gateways return a perfectly usable{"data":[{"id":"vendor/some-model"}, ...]}with no top-level"object":"list", no per-entry"object":"model", and sometimes noowned_by. serde aborts on the first missing one, so thewhole listing is rejected over metadata shai never reads — it only needs the ids. This breaks
model selection in
shai authanddefault_model()for those providers.Fix
New
shai_llm::providers::models::list_models_compat, which fetches/modelswith the client'sown http client (the pattern
mistral.rsalready used, sinceClient::getispub(crate)) andrequires only
idper entry:objectto"model", leavesowned_by/createdempty when absentidinstead of failing the whole listingWired into the four providers that went through the strict listing:
openai,openai_compatible,ovhcloudandollama.mistral,openrouterandanthropicalreadyhad their own conversions and are untouched.
Note that bumping
openai_divedoes not fix this on its own — 1.4.3 still declares thosefields as mandatory.
Also: bump openai_dive 1.3.1 -> 1.4.3
Second commit, across all four crates. Breaking changes handled:
ChatMessage::Assistantand bothDeltaChatMessagevariants gainedreasoningshared::Usagelostinput_tokens,output_tokensand their_detailsUsageliterals — every site set them toNoneResponseUsagecarrying those fieldsresponse/formatter.rsbuilds aResponseUsageResponseObjectgainedtop_logprobs;items::FunctionToolCallmadeidandstatusoptionalThe
ResponseUsagesplit also fixes a latent spec bug:/v1/responseswas reportingchat-style
prompt_tokens/completion_tokensinside a Responses payload, where the speccalls for
input_tokens/output_tokens.Providers are split on how they report reasoning — some send
reasoning, somereasoning_content. On 1.3.1 the former did not exist in the struct at all, so it was silentlydiscarded.
extract_think_contentnow normalizesreasoningontoreasoning_content, which iswhat the agent and the TUI read, so reasoning from those models is displayed rather than
dropped. The existing
<think>extraction still takes precedence.Testing
objectfields, a standard OpenAI listing, a barearray, entries without an
id, and an unparsable body. No network or credentials needed.rename_all = "lowercase"to"snake_case", so role tags, tool types and tool-choice valuesare asserted to serialize unchanged (they do — every variant shai uses is a single word),
alongside the reasoning normalization and
<think>extraction.object: model listing,chat with function calling, tool-call parsing and token usage all work, for
FunctionCall,FunctionCallRequiredandStructuredOutputalike. Also exercised end to end through therelease binary in headless mode — listing, chat, tool call, tool execution, reasoning display.
credentials, plus an unrelated
ringbufferpanic infc::tests::test_clear_history).cargo build --releaseclean; the one warning is pre-existing and unrelated(
shell/pty.rs:193, a rustc function-cast lint).Out of scope, noticed along the way
fc::tests::test_clear_historypanics insideringbuffer-0.16and fails onmaintoo.LlmClient::default_model()returnsOk("")whenSHAI_MODELis set but empty, instead offalling back to the provider.