fix(routing): survive upstream 410 Gone + retire dead Ollama Cloud model - #8
Merged
Conversation
Ollama Cloud answers 410 Gone for models it has retired (e.g. qwen3-coder:480b). The classifier had no case for 410, so it fell through to 'other' (retryable: false, block: none): the fallback chain aborted with a 502 instead of trying the next model, and no runtime-health block was ever recorded, so auto-route kept picking the dead model on every request. 410 now classifies like 404 (model_unavailable, retryable, skipModel), which makes auto-route skip it and continue the chain, records the 24h runtime-health block, and lets pinned requests fail honestly.
Live-verified 2026-07-22: upstream returns 410 Gone for this model. With intelligence_rank 2 it sat at the top of the auto-route chain, so every 'model: auto' request died on it before the classifier fix. Disabled, not deleted, per V18 precedent. gemma4:31b, gpt-oss:120b and gpt-oss:20b were probed the same day and still answer 200, so they stay enabled.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (24)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds Google Gemini audio transcription, expands provider error classification, disables a retired Ollama model through migration V19, seeds Google transcription routing, and makes server host binding configurable. ChangesGoogle audio transcription
Provider errors and model retirement
Configurable server host binding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GoogleProvider
participant GeminiAPI
Client->>GoogleProvider: Submit audio transcription request
GoogleProvider->>GeminiAPI: Send inlineData audio to generateContent
GeminiAPI-->>GoogleProvider: Return candidate transcript
GoogleProvider-->>Client: Return text or JSON result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
server/src/__tests__/services/provider-errors.test.ts (1)
5-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the 24-hour cooldown and cover both message variants.
This test only checks that
keyCooldownMsis positive and exercises “organization has been restricted”; the new branch also supports “organization is restricted” and promises a 24-hour cooldown. Assert24 * 60 * 60 * 1000and add coverage for the second form.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/__tests__/services/provider-errors.test.ts` around lines 5 - 17, Update the provider-error tests around classifyProviderError to assert keyCooldownMs equals 24 * 60 * 60 * 1000 instead of only being positive, and add a case covering the “organization is restricted” message variant. Preserve the existing auth, retryable, skipModel, and canRetryProviderFailure expectations for both variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/src/__tests__/db/ollama-410-v19.test.ts`:
- Around line 14-17: Update the beforeAll hook around initDb to be asynchronous
and await its returned promise, ensuring database initialization and any
failures complete within Vitest’s setup lifecycle before tests run.
In `@server/src/index.ts`:
- Around line 15-16: Update the startup logging around the server URL messages
to derive a normalized display host separate from the bind HOST: bracket IPv6
literals when they are usable destinations, and for wildcard binds such as
0.0.0.0 or :: avoid emitting a clickable proxy endpoint URL. Preserve the
server-running message while ensuring it does not advertise wildcard addresses
as client destinations.
In `@server/src/providers/google.ts`:
- Around line 762-771: Update the transcription response handling around
GeminiResponse and the extracted transcript to inspect
candidates[0].finishReason before returning success. Treat MAX_TOKENS as an
error by throwing instead of returning the partial transcript, while preserving
the existing no-text validation and successful response formatting for other
finish reasons.
---
Nitpick comments:
In `@server/src/__tests__/services/provider-errors.test.ts`:
- Around line 5-17: Update the provider-error tests around classifyProviderError
to assert keyCooldownMs equals 24 * 60 * 60 * 1000 instead of only being
positive, and add a case covering the “organization is restricted” message
variant. Preserve the existing auth, retryable, skipModel, and
canRetryProviderFailure expectations for both variants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 324b5b1f-5512-4dc8-a08a-0ee0a135328b
📒 Files selected for processing (7)
server/src/__tests__/db/ollama-410-v19.test.tsserver/src/__tests__/providers/google.test.tsserver/src/__tests__/services/provider-errors.test.tsserver/src/db/index.tsserver/src/index.tsserver/src/providers/google.tsserver/src/services/provider-errors.ts
Comment on lines
+14
to
+17
| beforeAll(() => { | ||
| process.env.ENCRYPTION_KEY = '0'.repeat(64); | ||
| initDb(':memory:'); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching ollama test / db init:"
fd -i 'ollama-410-v19\.test\.ts|index\.ts$' server/src 2>/dev/null || true
echo
echo "Target test excerpt:"
if [ -f server/src/__tests__/db/ollama-410-v19.test.ts ]; then
nl -ba server/src/__tests__/db/ollama-410-v19.test.ts | sed -n '1,80p'
fi
echo
echo "Find initDb declarations/usages:"
rg -n "function initDb|const initDb|initDb\(" server/src --glob '*.ts' || true
echo
echo "Relevant db index excerpts:"
for f in $(fd -i 'index\.ts$' server/src/db 2>/dev/null); do
echo "--- $f"
nl -ba "$f" | sed -n '1,220p'
done
echo
echo "Git status/diff stat:"
git diff --stat || true
git diff -- server/src/__tests__/db/ollama-410-v19.test.ts server/src/db/index.ts | sed -n '1,220p' || trueRepository: aliihsaad/LLM_HUB
Length of output: 376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target test excerpt:"
if [ -f server/src/__tests__/db/ollama-410-v19.test.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' server/src/__tests__/db/ollama-19.test.ts 2>/dev/null | sed -n '1,120p' || true
awk '{printf "%6d\t%s\n", NR, $0}' server/src/__tests__/db/ollama-410-v19.test.ts 2>/dev/null | sed -n '1,140p' || true
fi
echo
echo "Find initDb declarations/usages:"
rg -n "function initDb|const initDb|initDb\(" server/src --glob '*.ts' || true
echo
echo "Relevant db index excerpts:"
if [ -f server/src/db/index.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' server/src/db/index.ts | sed -n '1,260p'
fi
echo
echo "Relevant server index excerpts around db initialization imports:"
if [ -f server/src/index.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' server/src/index.ts | sed -n '1,220p'
fi
echo
echo "Git diff stat:"
git diff --stat || true
git diff -- server/src/__tests__/db/ollama-410-v19.test.ts server/src/db/index.ts server/src/index.ts | sed -n '1,260p' || trueRepository: aliihsaad/LLM_HUB
Length of output: 19783
Await the asynchronous database initialization.
initDb returns a promise, but this synchronous beforeAll hook does not wait for it. The current setup may run the synchronous seed/migrate phase before the first async import, while later initialization and failures can still occur outside Vitest’s setup lifecycle.
Proposed fix
- beforeAll(() => {
+ beforeAll(async () => {
process.env.ENCRYPTION_KEY = '0'.repeat(64);
- initDb(':memory:');
+ await initDb(':memory:');
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| beforeAll(() => { | |
| process.env.ENCRYPTION_KEY = '0'.repeat(64); | |
| initDb(':memory:'); | |
| }); | |
| beforeAll(async () => { | |
| process.env.ENCRYPTION_KEY = '0'.repeat(64); | |
| await initDb(':memory:'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/__tests__/db/ollama-410-v19.test.ts` around lines 14 - 17, Update
the beforeAll hook around initDb to be asynchronous and await its returned
promise, ensuring database initialization and any failures complete within
Vitest’s setup lifecycle before tests run.
Comment on lines
+15
to
+16
| console.log(`Server running on http://${HOST}:${PORT}`); | ||
| console.log(`Proxy endpoint: http://${HOST}:${PORT}/v1/chat/completions`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not advertise every bind host as a directly usable URL.
For HOST=0.0.0.0 or HOST=::, the logged URL is not a reliable client destination; IPv6 literals also require brackets. Use a separate, normalized display host and avoid emitting a clickable endpoint for wildcard binds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/index.ts` around lines 15 - 16, Update the startup logging around
the server URL messages to derive a normalized display host separate from the
bind HOST: bracket IPv6 literals when they are usable destinations, and for
wildcard binds such as 0.0.0.0 or :: avoid emitting a clickable proxy endpoint
URL. Preserve the server-running message while ensuring it does not advertise
wildcard addresses as client destinations.
Comment on lines
+762
to
+771
| const data = await res.json() as GeminiResponse; | ||
| const transcript = extractText(data.candidates?.[0]?.content?.parts)?.trim(); | ||
| if (!transcript) { | ||
| throw new Error('Google transcription returned no text'); | ||
| } | ||
|
|
||
| const wantsText = request.response_format === 'text'; | ||
| return { | ||
| body: wantsText ? transcript : { text: transcript }, | ||
| contentType: wantsText ? 'text/plain; charset=utf-8' : 'application/json', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not return truncated Gemini output as a successful transcript.
finishReason can be MAX_TOKENS, but this method returns any extracted text as success. Long audio can therefore produce a partial transcript with HTTP 200. Check candidate.finishReason before returning; the API documents MAX_TOKENS as output truncation. (ai.google.dev)
Proposed guard
const data = await res.json() as GeminiResponse;
- const transcript = extractText(data.candidates?.[0]?.content?.parts)?.trim();
+ const candidate = data.candidates?.[0];
+ if (candidate?.finishReason && candidate.finishReason !== 'STOP') {
+ throw new Error(`Google transcription stopped with ${candidate.finishReason}`);
+ }
+ const transcript = extractText(candidate?.content?.parts)?.trim();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const data = await res.json() as GeminiResponse; | |
| const transcript = extractText(data.candidates?.[0]?.content?.parts)?.trim(); | |
| if (!transcript) { | |
| throw new Error('Google transcription returned no text'); | |
| } | |
| const wantsText = request.response_format === 'text'; | |
| return { | |
| body: wantsText ? transcript : { text: transcript }, | |
| contentType: wantsText ? 'text/plain; charset=utf-8' : 'application/json', | |
| const data = await res.json() as GeminiResponse; | |
| const candidate = data.candidates?.[0]; | |
| if (candidate?.finishReason && candidate.finishReason !== 'STOP') { | |
| throw new Error(`Google transcription stopped with ${candidate.finishReason}`); | |
| } | |
| const transcript = extractText(candidate?.content?.parts)?.trim(); | |
| if (!transcript) { | |
| throw new Error('Google transcription returned no text'); | |
| } | |
| const wantsText = request.response_format === 'text'; | |
| return { | |
| body: wantsText ? transcript : { text: transcript }, | |
| contentType: wantsText ? 'text/plain; charset=utf-8' : 'application/json', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/providers/google.ts` around lines 762 - 771, Update the
transcription response handling around GeminiResponse and the extracted
transcript to inspect candidates[0].finishReason before returning success. Treat
MAX_TOKENS as an error by throwing instead of returning the partial transcript,
while preserving the existing no-text validation and successful response
formatting for other finish reasons.
Source: MCP tools
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
1 participant
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
model: "auto"returned 502 on every request (VPS and local): auto-route's top-ranked candidateqwen3-coder:480b(Ollama Cloud, intelligence_rank 2) was retired upstream and now answers 410 Gone.classifyProviderErrorhad no case for 410, so it fell through toother(retryable: false, block: none):Other pinned models' fallback chains also terminated on it.
Fix
c545ff9— classify410like404:model_unavailable, retryable, skipModel. Auto-route now skips the dead model and continues the chain, records the 24h runtime-health block, and pinned requests fail honestly instead of wandering.317a135— V19 migration disables the retiredqwen3-coder:480brow (V18 precedent). Probed same-day:gemma4:31b,gpt-oss:120b,gpt-oss:20bstill answer 200 and stay enabled.Also ports the two VPS-local commits upstream so nothing diverges (
90a11b9loopback bind — dist artifacts stripped,160d60eGoogle audio transcription fallback; both running in production since Jul 8/12).Validation (TDD)
other/false/false) + V19 migration assertions — then the fix; both now pass.npm run buildclean.Summary by CodeRabbit
New Features
HOSTenvironment variable.Bug Fixes
Maintenance