fix(models): unbreak review — the provider pin 404'd every model in the chain - #120
Conversation
The provider order was changed from `["deepseek"]` to `["streamlake", "deepinfra"]` because DeepSeek first-party does not serve the `deepseek-v4-flash` model, causing every call to return `404 No endpoints found` and all lanes to go neutral. StreamLake and DeepInfra both serve all four model IDs the deployment can request and accept `json_object`, with StreamLake leading as the cheaper option and DeepInfra as a fallback to degrade price rather than review during an outage. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The comment explaining the cost comparison between flash and pro tiers was updated to remove the specific mention of DeepSeek as the endpoint provider, replacing it with a reference to StreamLake/DeepInfra. A note was added to explain that this distinction matters because a previous endpoint change caused all reviews to fail, directing readers to the `[models.provider]` section for context. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Update the hardcoded price for the `deepseek/deepseek-v4-flash` model to reflect the dearer of the two providers listed in the configuration, rather than DeepSeek's own rates which are no longer applicable. The previous values of $0.14/$0.28 were based on first-party pricing, but DeepSeek no longer serves this model directly, so the fallback provider's rates of $0.09/$0.18 are used instead. The cached read price is also corrected from a fiftieth to a fifth of the input price, matching the actual discount applied by the active provider. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a `reached_a_verdict` parameter to `lane_summary` so that a lane which was disabled, skipped as a draft, or whose model calls all failed can report that it has no opinion rather than emitting a misleading "No findings." message. Previously an empty finding list was always displayed as a clean result, which could falsely imply that unreviewed code had been checked. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `lane_summary` function now accepts a boolean parameter indicating whether a reviewer produced a verdict. When a lane never ran due to provider failures, passing `false` suppresses the "No findings." message that previously made skipped lanes appear clean. All existing call sites are updated to pass `true` to preserve current behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a `last_resort_unpinned` field to `ProviderRouting` that, when enabled, allows the model ladder to drop the pin entirely on its final rung. This addresses a design gap where every rung carried the same provider pin, making the fallback ladder ineffective against provider-specific model unavailability. The new field defaults to `true`, and a corresponding `unpinned()` constructor provides a routing configuration with no pin and fallbacks enabled for use as the ladder's last resort. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `harness` and `call` methods now accept a `ProviderRouting` parameter, which is forwarded to `provider_options` instead of the previously used `self.provider`. This allows the caller to specify routing preferences per request rather than relying on a fixed provider configuration. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When every model in the configured provider pin fails, the gateway now retries the request with an unpinned provider. This prevents a deployment outage when the pinned provider cannot serve any of the configured models, for example when a pin to DeepSeek is active but the models are only available through other providers. The fallback is only reached after all priced routes have failed, so it adds no cost during normal operation, but it logs a warning because unpinned calls may incur unexpected billing. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a `last_resort_unpinned` setting to the provider configuration that, when enabled, makes one final unpinned model call after all priced routes have failed. This prevents a single pin from becoming a single point of failure that could cause reviews to return empty. The option defaults to true because a recoverable cost estimate is preferable to silent unreviewed code, and it only activates after everything else has failed, so it costs nothing on a healthy deployment. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add four tests that verify the last-resort unpinned rung is enabled by default, sends no provider block, cannot recurse, and is absent when the deployment is already unpinned. These tests document and protect the invariant that the fallback rung exists to prevent every ladder rung inheriting a broken pin. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…test The DeepSeek V4 Pro and V4 Pro 0813 model prices were significantly underpriced because they assumed a single DeepSeek first-party endpoint that this deployment cannot route to, when in fact the models have multiple endpoints with higher prices. The input price for the dated model was raised from $0.435 to $1.32 and output from $0.87 to $3.96, while the floating alias was updated to $1.30/$2.60 based on the dearer of the two pinned providers. The test that checks for underpriced models was also strengthened from a single $0.10 floor for all models to a per-model floor using freshly measured cheapest endpoint prices, which would have caught the previous mispricing. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The shipped defaults previously pinned only DeepSeek as the provider order, but every configured model was served by StreamLake and DeepInfra, causing all requests to fail with "No endpoints found". This change adds a test that verifies the provider order does not contain names that are model vendors rather than hosting providers, and ensures the ladder has an unpinned fallback rung to make such mistakes survivable. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Remove 94 cassette JSON files that were recorded during earlier evaluation runs for the kernel-bypass-hallucination and description-anchored-to-code test suites. These cassettes are no longer needed because the evaluation harness has been updated to use a different recording strategy, and keeping them would only add noise to the repository. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
|
Warning Review limit reachedYour included review limit has been reached. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 28 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (130)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
How this change flows5 changed behaviours across 10 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 39 further behaviours left out to keep the diagram readable. flowchart LR
n0["...hipped_defaults_pin_the_upstream_provider<br/>changed"]:::changed
n1["Labels<br/>changed"]:::changed
n2["ProviderRouting<br/>changed"]:::changed
n3["GatewayModel<br/>changed"]:::changed
n4["langfuse_client<br/>changed"]:::changed
n5["iter"]:::impacted
n6["publishing_also_triages_the_pull_request"]:::impacted
n7["Config"]:::impacted
n8["call"]:::impacted
n9["Result"]:::impacted
n10["from_config"]:::impacted
n0 -->|uses| n7
n3 -->|uses| n2
n4 -->|calls| n5
n6 -->|uses| n1
n7 -->|uses| n1
n8 -->|uses| n2
n8 -->|calls| n5
n8 -->|uses| n9
n10 -->|calls| n4
n10 -->|uses| n9
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fec2b5856
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match self | ||
| .call_until_complete(&request.model, &request, &unpinned) | ||
| .await |
There was a problem hiding this comment.
Account for paid failures before the unpinned retry
When the pinned models return billable but unusable responses—for example truncated output or invalid JSON—this new unpinned call adds another charge without accounting for the earlier attempts. ModelCapability checks the budget only before entering GatewayModel::complete (src/flows/caps.rs:126-138), while call constructs Usage only after validation succeeds, so all failed-attempt usage is discarded and the successful unpinned response reports only its own cost. A chain of paid failures followed by this retry can therefore exceed budget_usd_per_pr while the recorded spend remains below it; carry usage through failed attempts and check the remaining budget before making this extra call.
Useful? React with 👍 / 👎.
What was wrong
Every review since #117 has reported nothing to review. Not a lane bug — no
model was ever called.
[models.provider]pins routing withdeepseekthere is an OpenRouter provider, not the model vendor. DeepSeekfirst-party serves the dated
deepseek-v4-pro-0813snapshot; it does notserve
deepseek-v4-flash. #117 moved both tiers onto Flash, so from thatcommit every primary call returned
404 No endpoints found.The fallback ladder could not save it. The pin applies to the whole chain, and
neither
z-ai/glm-5.2norminimax/minimax-m3has a DeepSeek endpoint either,so all three rungs failed for the same reason. Reproduced against the live API:
Those are verbatim the errors in the production pod's logs.
With every call failing, each lane's fan-out reviewed zero files, reported
skipped, and published aNeutralcheck readingReviewed 0 files; 0 findings.— followed byNo findings.A silent all-clear over unreviewed code,for a week, which is the most expensive way this reviewer can be wrong.
What changed
1. The pin names providers that serve these models.
order = ["streamlake", "deepinfra"]— the value that was here before it was replaced. Both serve allfour configured ids and both accept
json_object; verified against the live API.2. The ladder gets a rung that drops the pin. Every rung inherited one
order, so a wrong pin failed all of them identically and the safety net wasdecoration.
models.provider.last_resort_unpinned(default on) makes one finalunpinned attempt after every priced route has failed. It costs nothing on a
healthy deployment, and it is loud: reaching it logs at
warnthat the cost lineis now an estimate and the pin needs fixing. An unpinned review at an unpredicted
price beats no review.
3. A lane with no verdict stops claiming one.
lane_summaryprintedNo findings.whether a reviewer looked and found nothing or never ran. It now saysso explicitly when the lane reached no verdict. This is what let the outage stay
invisible.
4. Prices corrected, and the floor test made per-model. The floor was a
single
$0.10constant justified by "the cheapest endpoint is $0.392". Bothhalves aged out: Flash is genuinely cheaper than the floor, so it rejected a
correct row — and a blanket floor never checks the rows it passes, so
deepseek-v4-pro-0813sat underpriced threefold on output, the direction thatmakes
budget_usd_per_prfail open. Now every row is checked against itsmeasured cheapest endpoint.
5. A guard against making this exact mistake again. A test rejects pinning a
name that is a model vendor rather than a hosting provider, and says how to
check.
Verification
Live, through the real code path, against a diff with two planted bugs:
Neutral—Reviewed 0 files; 0 findings.(reproduces production)Failure— both bugs found,$0.0002Failure— both bugs found, warns the pin is brokencargo test— 1450 passed, 0 failedcargo fmt --check,cargo clippy -D warnings,cargo check --all-features— cleanAlso fixes CI
mainhas been red since #117 on a stale eval corpus. It could not bere-recorded while every model 404'd — a recording run had nothing to record.
Both cases are re-recorded here and both guarded regressions still hold. The 94
previously-tracked cassettes were all dead; the 29 recorded here replay the
corpus on their own.
Note for the operator
The production pod is also being OOMKilled (4 restarts, 846Mi against a 1Gi
limit) while doing almost no model work. Once reviews actually run, that will get
worse. Not addressed here — it is a deployment resource change, not a code one.