feat(harness): BYOK live provider matrix + fix null-tolerant OpenAI-compatible deserialization - #84
Conversation
…aults Mistral returns "tool_calls": null on every plain (non-tool) chat completion. `#[serde(default)]` covers only an *absent* key, so an explicit null failed the whole response with `invalid type: null, expected a sequence` — unary chat was dead against mistral-small-latest while its streaming and tool-calling paths worked. Rather than special-case one provider, encode the rule that a field which may be absent may equally be null: a `deserialize_null_default` helper applied to every deserialized wire field that already declares a default (response and chunk `choices`, `delta`, both `tool_calls`, tool-call `id`/`type`/`name`, the usage counters, and the model-listing `data`). Genuinely required fields such as `ChoiceWire::message` keep no default and stay strict, so a structurally broken response still errors. Same class of quirk as the DeepSeek/local-server shapes already normalized by `deserialize_arguments`, and fixed the same way. Regression tests use a captured Mistral-shaped body plus a nulls-everywhere response, the streaming seam, and the model-listing envelope. All four fail before this change and pass after; verified live against the real Mistral API.
Verifies every provider you hold a key for in one run: a 1-shot chat call, a streaming call, and a tool call each, printed as a `provider | PASS/FAIL(reason) | latency(ms)` board. Providers are discovered from a gitignored `providers.env` rather than a list in the test, so adding one is three lines of config and never a code change. Each entry is a preset name (the eight built-in `ProviderKind`s, plus ollama) or a bare OpenAI-compatible `base_url`; `providers.env.example` seeds 25 providers with blank keys. Operational rules, each covered by an offline unit test: - a blank key falls back to the preset's own variable (OPENAI_API_KEY, ...) and then reports SKIP without dialling; - an exported PROVIDER_* variable overrides the file, and a blank export never does, so a run can be retargeted without editing a providers.env holding real keys; - dialling is opt-in via PROVIDER_MATRIX=1, so a bare `cargo test` stays offline even with a fully configured providers.env; - PROVIDER_MATRIX_ALLOW_FAILURES=1 prints the board without failing, for when providers fail on account state (quota, billing) rather than on code. Run: PROVIDER_MATRIX=1 cargo test --test live_provider_matrix -- --nocapture
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
| Filename | Overview |
|---|---|
| tests/live_provider_matrix.rs | New live provider matrix harness. Solid overall — offline unit tests cover parsing, resolution, and precedence rules. One bug: PROVIDER_MATRIX_ALLOW_FAILURES is checked with .is_err() rather than the same `filter( |
| src/harness/providers/openai/types.rs | Adds deserialize_null_default helper and applies it to all wire fields that already declare #[serde(default)]. Fix is correct and well-documented; genuinely required fields keep no default and stay strict. |
| src/harness/providers/openai/test.rs | Adds four regression tests covering the null-tolerance fix (Mistral-shaped body, nulls-everywhere, streaming seam, model-listing envelope). All four would fail before the types.rs change. |
| providers.env.example | New template file seeding 25 providers with blank keys, correct Cerebras model (gpt-oss-120b), and Ollama tag guidance. Safe to ship — providers.env is gitignored. |
| src/harness/providers/openai/README.md | Adds a "BYOK provider matrix" section with configuration format and opt-in gate docs. Content is accurate and consistent with the implementation. |
| README.md | Adds a "Verifying real providers (BYOK)" section under Development with clear instructions and reference to the detailed README. |
| .gitignore | Adds providers.env to gitignore to prevent accidental credential commits. Correct and necessary. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[providers.env / env vars] --> B[parse_env_file]
B --> C[merge_env_overrides]
C --> D[collect_entries]
D --> E{resolve each entry}
E -->|valid + key present| F[ResolvedProvider]
E -->|blank key| G[SKIP report]
E -->|misconfigured| H[FAIL report]
F --> I[run_provider\nchat / stream / tools]
I --> J{checks pass?}
J -->|all pass| K[PASS row]
J -->|any fail| L[FAIL row]
K & L & G & H --> M[print_table]
M --> N{broken > 0?}
N -->|yes + ALLOW_FAILURES not set| O[panic]
N -->|no OR ALLOW_FAILURES=1| P[test passes]
Reviews (2): Last reviewed commit: "docs(providers): use a current Cerebras ..." | Re-trigger Greptile
The README snippet and the config-parsing test fixtures still carried `llama3.1-8b`, which Cerebras has retired and now 404s — the exact id the rest of this PR corrects in providers.env.example. Flagged in review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Part of tinyhumansai/openhuman#5146 (Stream A — BYOK provider test matrix + Mistral null-tolerance fix). |
2 participants
Summary
Adds a BYOK live provider matrix and fixes the first real provider bug it found.
Part of tinyhumansai/openhuman#5146 (Stream A) — verifying that every LLM
provider a user can bring actually works through the one OpenAI-compatible
client.
1.
tests/live_provider_matrix.rs— the harness. Verifies every provideryou hold a key for in a single run: a 1-shot chat call, a streaming call, and a
tool call each, printed as a
provider | PASS/FAIL(reason) | latency(ms)board.Providers are discovered from a gitignored
providers.envrather than a list inthe test, so adding one is three lines of config and never a code change. Each
entry is a preset name (the eight built-in
ProviderKinds, plus ollama) or abare OpenAI-compatible
base_url;providers.env.exampleseeds 25 providerswith blank keys.
Operational rules, each covered by an offline unit test:
OPENAI_API_KEY, …) andthen reports
SKIPwithout dialling;PROVIDER_*variable overrides the file, and a blank export neverdoes, so a run can be retargeted without editing a
providers.envholdingreal keys;
PROVIDER_MATRIX=1, so a barecargo teststaysoffline even with a fully configured
providers.env;PROVIDER_MATRIX_ALLOW_FAILURES=1prints the board without failing, for whenproviders fail on account state (quota, billing) rather than on code.
2.
fix(openai): tolerate an explicit JSONnullwherever a wire fielddefaults. The matrix immediately caught a real adapter bug: Mistral returns
"tool_calls": nullon every plain (non-tool) chat completion.#[serde(default)]covers only an absent key, so an explicit null failed the whole response with
invalid type: null, expected a sequence— unary chat was dead againstmistral-small-latestwhile its streaming and tool-calling paths worked.Rather than special-case one provider, the fix encodes the rule that a field
which may be absent may equally be
null: adeserialize_null_defaulthelperapplied to every deserialized wire field that already declares a default
(response and chunk
choices,delta, bothtool_calls, tool-callid/type/name, the usage counters, and the model-listingdata).Genuinely required fields such as
ChoiceWire::messagekeep no default and staystrict, so a structurally broken response still errors. This is the same class of
quirk as the DeepSeek / local-server shapes already normalized by
deserialize_arguments, and is fixed the same way — the next provider with thishabit needs no code.
3. Config corrections in
providers.env.example(found by running theboard):
llama3.1-8bis retired on Cerebras and 404s, and the Ollama entry nowcarries an explicit tag (
llama3.2:3b) since Ollama 404s on an untagged name ithas not pulled.
Live results
llama-3.3-70b-versatile)gpt-4.1-mini)mistral-small-latest)FAIL(chat: serialization error: invalid type: null, expected a sequence)SKIP— no key configuredAPI Or Behavior Changes
No public API change. Behavior: the OpenAI-compatible adapter now accepts an
explicit
nullfor any wire field that already had a default, instead offailing the whole response. This is strictly more permissive — no previously
successful parse changes its result.
Tests
New coverage: four regression tests for the null-tolerance fix (a captured
Mistral-shaped body, a nulls-everywhere response, the streaming seam, and the
model-listing envelope) — all four fail before the change and pass after —
plus eleven offline unit tests for the matrix's discovery/resolution/precedence
rules, which need no network and no credentials.
The fix is additionally confirmed live:
mistral-small-latestgoes fromFAIL(chat)toPASSagainst the real API.cargo fmt --checkcargo clippy --all-targets -- -D warningscargo clippy --all-targets --all-features -- -D warningscargo build --all-targetscargo build --all-targets --all-featurescargo testcargo test --all-featuresDocumentation
src/harness/providers/openai/README.md— new "BYOK provider matrix" section(configuration format, precedence, opt-in gate).
README.md— "Verifying real providers (BYOK)" under Development.providers.env.example— documents the format inline, with the Cerebras andOllama gotchas noted where they bite.
deserialize_null_defaultcarries the rationale and the rule for futureprovider quirks.
No real keys are committed:
providers.envis gitignored and untracked; onlythe blank-key
.exampleships.