fix(llm-health): quarantine models the provider rejects instead of retrying them on every call (#4949) by Yigtwxx · Pull Request #5458 · koala73/worldmonitor

Yigtwxx · 2026-07-22T17:22:58Z

Summary

isProviderAvailable() probes new URL(apiUrl).origin with a bare GET and treats any HTTP response as healthy — 200, 404 and 403 alike. It never sees creds.model, so a model ID the provider does not serve passes the gate on every request, fails with http_4xx, and falls through to the next provider. That is one wasted round-trip per call, indefinitely, and nothing in the logs points at the model as the cause.

The signal needed to stop it was already being collected and discarded. Both provider loops read the error body for diagnostics and log the model beside it:

const errBody = await readBoundedErrorBody(resp, 300).catch(() => '');
console.warn(`[llm:${providerName}] HTTP ${resp.status} model=${creds.model} body=${errBody}`);
record(false, { reason: `http_${resp.status}` });
continue;   // <- the "this model is dead" evidence ends here

This feeds it back into the gate. After two consecutive rejections that explicitly name the model, the origin|model pair is quarantined for 10 minutes and skipped before the request is built.

Design notes

No new network calls. The issue's other candidate was a startup/selftest probe per configured model, but Edge functions are cold-start-per-request and have no startup hook — which is why warmHealthCache() is uncalled today. The reactive path costs nothing extra: it reuses a body the code already reads.

Detection is deliberately narrow. Only 400/404 and a body naming the model (not a valid model, no such model, model ... does not exist, model 'x' not found, …). 401/403/429/5xx are credentials, rate limits and outages — provider-wide, transient, and silent about the model ID — so they never quarantine. An unreadable body keeps the previous behaviour, so the fail-safe is the status quo rather than a wrongly quarantined model. Two consecutive rejections are required because a 400 can also be a malformed request; any success clears the counter.

Model usability is kept separate from origin reachability rather than folded into isProviderAvailable(). One is synchronous and local, the other probes the network. The sync check runs first, so a quarantined model costs neither a completion nor a probe, and the two skip reasons stay distinguishable in the logs.

Threshold and cooldown follow src/utils/circuit-breaker.ts (DEFAULT_MAX_FAILURES = 2). The 10-minute quarantine is longer than that module's 5-minute network cooldown because an unroutable model ID is a configuration error rather than a transient fault — but it stays bounded, so a provider that re-lists a model recovers without a redeploy.

LlmCallEvent is untouched. Adding a field there would require updating the CJS mirror in scripts/lib/llm-telemetry.cjs and the exact key-set assertions in tests/seeder-llm-telemetry.test.mjs and tests/forecast-llm-telemetry.test.mjs. Observability comes instead from a distinct [llm-health] Model quarantined … warn line and getLlmModelHealthStatus().

Verification

The load-bearing test is behavioural and was written first — tests/shared-llm.test.mts, "stops re-sending the prompt to a model the provider rejects as unknown". OpenRouter answers 400 "ghost/ghost-model-v9 is not a valid model ID", Groq answers 200, and callLlm runs four times:

Against origin/main: fails — 4 !== 2, "got 4 attempts across 4 calls". The dead model is re-sent every single time.
Against this branch: passes — 2 attempts, then quarantined.

Then:

tests/llm-health.test.mts (new — the module had no test file at all): 11/11 pass. Covers status gating, the unknown-model bodies OpenRouter / OpenAI-style / Ollama actually return, request-shaped 400s that must not quarantine, consecutive-vs-cumulative counting, quarantine expiry restoring a full failure budget, origin+model scoping, and malformed-URL / empty-model tolerance.
Ten LLM-adjacent suites together — llm-health, shared-llm, llm-usage-telemetry, redis-caching, seeder-llm-telemetry, forecast-llm-telemetry, llm-sanitize, summarize-reasoning, brief-llm, brief-llm-core: 336/336 pass.
npm run test:data (full suite): the set of failing test names on this branch is identical to the set on origin/main — both captured on the same machine and diffed with comm over the sorted names, empty in both directions. Those failures are pre-existing and unrelated to this change (OpenAPI contract, docs/i18n, pricing and Docker suites).
npm run typecheck and npm run typecheck:api — both clean.
npx biome lint over the six changed files — 4 warnings, all noUnusedFunctionParameters on pre-existing lines of tests/shared-llm.test.mts; origin/main reports the same 4, so no new findings.
lint:boundaries, lint:safe-html, check-unicode-safety and the edge-function esbuild bundle check — all pass.

tests/helpers/llm-health-stub.ts gains no-op isModelUsable / recordModelFailure / recordModelSuccess. It is injected into summarize-article via the import map in tests/redis-caching.test.mjs, so the new imports would otherwise break that suite.

Out of scope

The seeder chains (scripts/lib/llm-chain.cjs, scripts/seed-forecasts.mjs, scripts/regional-snapshot/*) have no health gate at all and cannot import server/_shared/ — covering them needs a scripts/ twin and belongs in its own change.
warmHealthCache(), getLlmHealthStatus() and reprobeAll() are currently uncalled, and /api/llm-health exists only in the Tauri sidecar. getLlmModelHealthStatus() is additive on purpose, so wiring an endpoint stays a separate decision.
State is module-level and therefore per-isolate, matching the existing 60s reachability cache. It cuts the repeat rate sharply but is not globally consistent, and the code does not claim otherwise.

Type of change

Bug fix

Affected areas

AI Insights / World Brief
API endpoints (/api/*)

Checklist

Tested on worldmonitor.app variant — server-side provider gate, covered by unit and behavioural tests; not variant-specific
Tested on tech.worldmonitor.app variant (if applicable) — N/A
New RSS feed domains added to api/rss-proxy.js allowlist (if adding feeds) — N/A, no new feeds
No API keys or secrets committed
TypeScript compiles without errors (npm run typecheck)

Documentation Alignment Checklist

N/A — no methodology, API/MCP contract, generated doc, Redis key or example output changes. This only adds a gate in front of the existing provider chain; request and response shapes are unchanged.

vercel · 2026-07-22T17:23:03Z

@Yigtwxx is attempting to deploy a commit to the World Monitor Team on Vercel.

A member of the Team first needs to authorize it.

greptile-apps · 2026-07-22T17:30:12Z

Greptile Summary

This PR introduces a reactive model-level quarantine layer on top of the existing provider reachability gate. After two consecutive HTTP 400/404 responses whose body matches a known "model not found" pattern, the origin|model pair is quarantined for 10 minutes and skipped synchronously before any network call is made.

llm-health.ts gains isModelRejection, isModelUsable, recordModelFailure, and recordModelSuccess; a new modelCache map tracks per-model failure counts and quarantine timestamps alongside the existing origin-reachability cache.
llm.ts (callLlm and callLlmReasoningStream) and summarize-article.ts each add the model gate ahead of the existing origin probe and feed HTTP error bodies back to recordModelFailure after reading them.
tests/llm-health.test.mts (new) covers 11 unit cases; tests/shared-llm.test.mts adds one end-to-end behavioral test confirming the repeated-send problem is fixed.

Confidence Score: 4/5

Safe to merge. The quarantine gate is deliberately narrow (400/404 + body pattern match, 2-strike threshold, 10-minute cooldown) and the fail-safe in every uncertain path is the pre-existing behaviour of retrying on every call.

The core logic in llm-health.ts is correct and well-tested. The two callsites in llm.ts are symmetric and consistent. The summarize-article integration is consistent with how isProviderAvailable was already wired. The only things that held the score back are the missing test teardown in shared-llm.test.mts (which today is harmless because the new test is the last in the file) and the sub-threshold entries that getLlmModelHealthStatus silently includes.

tests/shared-llm.test.mts — the new behavioral test mutates globalThis.fetch and env vars without restoring them, making any future test appended after it fragile.

Important Files Changed

Filename	Overview
server/_shared/llm-health.ts	Core quarantine logic — well-designed, status-code-gated, pattern-matched rejection detection. Sub-threshold entries (failures=1, quarantinedUntil=0) are persisted in modelCache indefinitely until a success or expiry clears them, and getLlmModelHealthStatus surfaces them, which may surprise monitoring consumers.
server/_shared/llm.ts	Correct integration in both callLlm and callLlmReasoningStream: model gate runs before origin probe, failure/success recorded symmetrically. Minor: the streaming path uses resp.text() (unbounded body read) while callLlm uses the pre-existing readBoundedErrorBody; both were already there before this PR.
server/worldmonitor/news/v1/summarize-article.ts	isModelUsable and recordModelFailure/Success added correctly. The check lives inside the cachedFetchJsonWithMeta fetcher (consistent with the pre-existing isProviderAvailable placement), meaning a quarantined model returns null and triggers a NEG_SENTINEL cache entry for 120 s — the same path taken when the provider is unreachable.
tests/llm-health.test.mts	New file with 11 focused unit tests covering the full quarantine lifecycle, status-code guards, pattern-matching correctness, consecutive-vs-cumulative counting, expiry with fresh budget, origin+model scoping, and edge-case tolerance.
tests/shared-llm.test.mts	New behavioral test confirms exactly 2 POST attempts to OpenRouter before quarantine, then groq serves the remaining calls. globalThis.fetch and env var mutations are not restored in an afterEach, relying on this being the final test in the file.
tests/helpers/llm-health-stub.ts	No-op stubs for the new exports added correctly; prevents breakage in suites that inject this stub via import map.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming LLM request] --> B[getProviderCredentials]
    B --> C{creds found?}
    C -- No --> Z[skip / return null]
    C -- Yes --> D{isModelUsable?\nsync, no network}
    D -- No: quarantined --> Z
    D -- Yes --> E{isProviderAvailable?\nasync, cached probe}
    E -- No: offline --> Z
    E -- Yes --> F[POST to provider API]
    F --> G{resp.ok?}
    G -- No: 4xx/5xx --> H[readBoundedErrorBody]
    H --> I{isModelRejection?\nstatus 400/404 + pattern match}
    I -- Yes --> J[recordModelFailure\nfailures++]
    J --> K{failures >= 2?}
    K -- Yes --> L[quarantinedUntil = now + 10 min\nlog warn]
    K -- No --> M[continue to next provider]
    L --> M
    I -- No --> M
    G -- Yes: content --> N[recordModelSuccess\nfailures = 0 / cache deleted]
    N --> O[return result]

_{Reviews (1): Last reviewed commit: "fix(llm-health): quarantine models the p..." | Re-trigger Greptile}

Yigtwxx · 2026-07-22T17:33:30Z

Addressed both review points in 52547d7.

Sub-threshold entries persisting. This was a real defect, and slightly worse than "surprising for monitoring consumers": readModelEntry only expired records that were already quarantined, so a record sitting at failures: 1 had no expiry at all. That meant modelCache retained an entry for every model ever rejected once, and the threshold effectively read "two rejections ever" rather than two related ones — a rejection today plus another a month later would have quarantined a healthy model. Lone failures now age out on the same window as the quarantine, and getLlmModelHealthStatus() reads through the same accessor as the gate so it prunes rather than reports stale state. New test: "forgets a lone rejection once the failure window passes".

Test teardown. One correction: globalThis.fetch and the env vars are restored — tests/shared-llm.test.mts has a file-level afterEach (added long before this PR) that resets both, and the new test relies on it exactly as the other 15 do. But the underlying concern was right for a different piece of state: the health gate's caches are module-level and the new test left a quarantine entry behind. That afterEach now calls __testing__.reset(), so the file no longer depends on this being the last test.

Re-verified: llm-health 12/12, the ten LLM-adjacent suites 337/337, npm run typecheck and npm run typecheck:api clean, biome unchanged at the 4 pre-existing warnings.

extractHeadBaseRefs fell back to a[data-hovercard-type="repository"] links when .commit-ref was not in the DOM. Those hrefs are /owner/repo -- they carry no /tree/<branch> segment, so parseRef's parts.slice(3) returned "". The result looked complete (right owner, right repo) but named no branch, and callers built https://github.com/<owner>/<repo>/blob//<path>?raw=1. GitHub collapses the double slash, reads the first path segment as a ref, and 404s; the HTML error page then became the thrown error's message, so the console showed a whole rendered document instead of a cause. Fork PRs hit this reliably: both the base repo and the head fork get a hovercard link, so the fallback always found its two anchors. Same-repo PRs largely did not, which is why this looked repo-specific. parseRef now requires a /tree/ segment and a non-empty owner/repo/branch, the hovercard strategy is gone (it cannot name a branch by construction), and a strategy is accepted only when both ends parse completely. fetchHeadFileContent refuses to build a URL from incomplete refs, and the boot-time prefetch -- which races the PR header into the DOM -- skips the round instead of fetching against a branch it could not identify. Verified on koala73/worldmonitor#5458 (fork PR, previously reproducing): five consecutive runs make no malformed request and prefetch succeeds with 4 changed files. Smoke suite 89 checks, 0 failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Port of koala73#5458 / worldmonitor#4949. Track consecutive provider-side model rejections and temporarily quarantine unusable model IDs so the LLM chain skips them instead of retrying a dead configuration on every call. Co-authored-by: Lukas Döring <lukasdoering@users.noreply.github.com>

The health gate probes only `new URL(apiUrl).origin` with a bare GET, and treats any HTTP response as healthy. A model ID the provider does not serve therefore passes the gate on every request, fails with http_4xx, and falls through to the next provider — one wasted round-trip per call, forever, with nothing in the logs naming the model as the cause. Both provider loops already read the error body for diagnostics and log the model alongside it, so the evidence was present and discarded. Feed it back instead: after two consecutive rejections that explicitly name the model, quarantine that origin+model pair for 10 minutes and skip it before the request is built. Detection is deliberately narrow — 400/404 plus a body that names the model. 401/403/429/5xx are credentials, rate limits and outages: provider-wide, transient, and silent about the model ID. An unreadable body keeps the old behaviour, so the failure mode is the status quo rather than a wrongly quarantined model. Model usability is kept separate from origin reachability rather than folded into isProviderAvailable(): the former is synchronous and local, the latter probes the network, and separate log lines keep the two skip reasons distinguishable.

A record with failures below the threshold had no expiry, so it survived until a success cleared it. Two consequences: `modelCache` retained an entry for every model ever rejected once, and "two consecutive rejections" carried no time bound — a rejection today plus one a month later would quarantine. Expire lone failures on the same window as the quarantine, and read getLlmModelHealthStatus() through the same accessor as the gate so it prunes aged-out records instead of reporting them as live state. Also reset the health caches in the shared-llm afterEach: they are module-level, so the quarantine set otherwise leaks into later tests.

Yigtwxx · 2026-07-28T14:00:28Z

Rebased onto current main (b832237d4); the diff is unchanged.

One merged commit since this PR opened touches a file it also touches: #5661 edited the import block and the premium gate in summarize-article.ts. The model gate here sits inside the cachedFetchJsonWithMeta fetcher, so the two do not overlap — the rebase applied without conflict.

Re-verified on the new base: llm-health 12/12, shared-llm 16/16, the LLM-adjacent suites and both typechecks clean. All required gates are green; the Vercel check is fork deploy authorization, not a build failure.

github-actions Bot added the trust:caution Brin: contributor trust score caution label Jul 22, 2026

greptile-apps Bot reviewed Jul 22, 2026

View reviewed changes

Comment thread tests/shared-llm.test.mts

Comment thread server/_shared/llm-health.ts

lukasdoering mentioned this pull request Jul 24, 2026

fix(llm-health): quarantine rejected models instead of retrying lukasdoering/geospy-v2#21

Draft

1 task

Yigtwxx added 2 commits July 28, 2026 16:50

Yigtwxx force-pushed the fix/llm-health-model-aware-gate branch from 52547d7 to 4c3a4a0 Compare July 28, 2026 13:55

koala73 and others added 2 commits July 30, 2026 07:26

Merge branch 'main' into fix/llm-health-model-aware-gate

68607b5

fix(llm): harden model health fallback

bab8207

koala73 merged commit 2f72eaf into koala73:main Jul 30, 2026
24 of 25 checks passed

Navigation Menu

fix(llm-health): quarantine models the provider rejects instead of retrying them on every call (#4949) - #5458

fix(llm-health): quarantine models the provider rejects instead of retrying them on every call (#4949)#5458
koala73 merged 4 commits into
koala73:mainfrom
Yigtwxx:fix/llm-health-model-aware-gate

Yigtwxx commented Jul 22, 2026

Uh oh!

vercel Bot commented Jul 22, 2026

Uh oh!

greptile-apps Bot commented Jul 22, 2026

Uh oh!

Uh oh!

Uh oh!

Yigtwxx commented Jul 22, 2026

Uh oh!

Yigtwxx commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Yigtwxx commented Jul 22, 2026

Summary

Design notes

Verification

Out of scope

Type of change

Affected areas

Checklist

Documentation Alignment Checklist

Uh oh!

vercel Bot commented Jul 22, 2026

Uh oh!

greptile-apps Bot commented Jul 22, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Yigtwxx commented Jul 22, 2026

Uh oh!

Yigtwxx commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants