fix(observability): report the model that actually ran on the AI failure path, and surface degraded requests - #10192
Merged
Merged
Conversation
…ure path, and surface degraded requests resolveModel discards the Workers-AI model id the core passes, because it is meaningless off-Workers. The success path captured usage.model (correct), but the failure path had no usage to read and fell back to the raw argument -- the very placeholder resolveModel threw away. Every failed AI call was therefore labelled with a model that never ran: on the live project all 33 failures reported `@cf/*` ids at a 100% error rate for a deployment that has never called Workers AI, while the real failing providers (the claude-code CLI, ollama) read 0%. Each adapter now reports, via a new optional telemetryModel on SelfHostAi, exactly what its own run() would resolve -- derived from the same hoisted expression, so the label and the request cannot drift. runProviderWithOtel and withAiGenerationCapture resolve it once, up front, and use it on both paths; the OTel span attribute carried the identical lie and reads from the same value. A `@cf/` id can no longer reach telemetry, and an unconfigured provider resolves to `<provider>-default` rather than a bare "default" that named no provider at all. Two degradation paths also left nothing in PostHog, so a provider failing or being skipped for every request looked exactly like a provider with no traffic: a circuit-breaker skip emitted only a Prometheus counter, and chain exhaustion only a console line (an $exception the AI views do not read). Both now emit selfhost_ai_degraded, carrying the shared $ai_trace_id so they join the real attempts around them. Deliberately not an $ai_generation: no model ran, so none is credited with the failure and the spend/token aggregates stay a record of real calls. A CLI provider declining an embed is routing working as designed and stays exempt at both sinks. Closes #10186
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
loopover-ui | 7051e8c | Jul 31 2026, 12:01 PM |
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Bundle ReportChanges will increase total bundle size by 1.76kB (0.02%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Summary
$ai_modelreported a model that never ran on every failed AI call.resolveModelexists because the core passes a Workers-AI model id that is meaningless off-Workers — it discards any@cf/-prefixed value and substitutes the provider's own. The success path capturedusage.model(the real resolved model, correct); the failure path had nousageto read and fell back to the raw argument, i.e. exactly the placeholderresolveModelhad thrown away.The result was a perfect false signal. On the live project all 33 failed AI events were mislabelled: a breakdown by model showed a set of Cloudflare model ids at a 100% error rate for a deployment that has never called Workers AI, while the models that were actually failing — the
claude-codeCLI and self-hosted ollama — read 0%, because their failures were attributed elsewhere. This is the same class of bug the codebase already fixed once in a different sink (src/services/ai-review.ts: "self-host claude-code reviews were mis-logged as the Workers-AI model ids, which hid outages").The fix
Each adapter now reports what its own
run()would resolve, through a new optionaltelemetryModelonSelfHostAi. In every adapter the resolution is hoisted into one expression that bothrun()andtelemetryModelcall, so the label and the actual request cannot drift. The OpenAI-compatible adapter branches on the sameoptions.textdiscriminatorrundoes, so an embedding request is labelled with the embed model (bge-m3:latest) rather than the chat model it never touches.runProviderWithOtelandwithAiGenerationCaptureresolve it once, before the call, and use it on both paths.usage.modelstill wins on success — it is what the provider reported actually running — and the two now agree for a given configured provider. The OTel span attribute carried the identical lie and reads from the same value.Consequences, per the issue's deliverables: a
@cf/value can no longer reach$ai_modelfrom the self-host path, and an unconfigured provider resolves to<provider>-defaultinstead of a bare"default"that named no provider at all (codex resolves to""on purpose — the CLI picks the account default — socodex-defaultkeeps that attributable).Degraded requests that reached no model
Two paths left nothing in PostHog, so a provider failing or being skipped for every request looked exactly like a provider with no traffic:
$ai_generationstream simply stops.$exception, an event the AI observability views do not read.Both now emit
selfhost_ai_degraded, carrying the shared$ai_trace_idso a degraded request joins the real attempts around it in the same trace. Deliberately not an$ai_generationwith$ai_is_error: true: no model ran, so booking it against one would credit that model with a failure it never had — the same false signal this issue is about — and would drag zero-token, zero-cost rows into the spend aggregates.A CLI-subscription provider declining an embed (
claude_code_no_embed) is routing working as designed, not a degradation, and stays exempt at both sinks — one exemption rule rather than two that can disagree. A chain with no embed-capable member at all is a real misconfiguration, but its fail-loud signal isshouldWarnRagEmbedUnavailable's boot warning, not one event per chunk batch.Closes #10186
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
test:coveragerun: 100% of the changed lines AND branches insrc/selfhost/ai.tsandsrc/selfhost/posthog.tsare covered, verified line-by-line against the lcov report rather than read off a summary percentage.ui:lintwas run asprettier --checkon the one changed MDX file (clean).Tests added
Four existing tests asserted the old
"default"label, which is precisely the behaviour the issue declares wrong; they were updated to the corrected expectation with the reasoning inline, not deleted. On top of that:@cf/placeholder never reaches$ai_model— with an adapter that implementstelemetryModel, and with one that does not (where it must still degrade to<provider>-defaultrather than pass through).telemetryModeloutranks a truthy non-@cf/model argument (the livevisual-visioncase), and a blank one is ignored.createOpenAiCompatibleAiadapter and a 400 response.circuit_openafter the breaker latches, asserting the skip is not also booked as a generation.chain_exhaustednaming every provider tried and attributing to the one that ran last, with both per-provider failures still captured individually alongside it."unknown"fallback.capturePostHogAiDegradationdirectly, for every branch — inactive client, trace present/absent,providersabsent/empty/non-empty, error present/absent,Error/non-Error, blank provider/model, repo group present/absent, and that no content field is ever attached.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.The new event carries the same metadata-only posture as
$ai_generation: provider/model ids, a reason, and an already-redacted error string. The repo group reuses the valueoperationalPropertiesalready processed, so under the shared central key it inherits HMAC anonymization and the fail-closed drop for free — a raw private repo name cannot reach the group index by this path. A test asserts no content field is ever attached.UI Evidence
Not applicable — no visible UI or frontend change. The one docs change is a prose/
FeatureRowaddition to the existing self-hosting operations page, whose accuracy is pinned by the drift test rather than by a screenshot.Notes
apps/loopover-ui/content/docs/self-hosting-operations.mdxdocumented the AI observability surface and would have become false: it now describes the failure-path model guarantee and the newselfhost_ai_degradedevent, and the drift test pins the exported event name and both reasons against the code — the same guard the monitor-heartbeat event already has, so the page cannot silently drift from what is emitted.The
$ai_trace_id/repo-group logic #10187 introduced was extracted into two small shared helpers so the new event lands in the same trace and inherits the same fail-closed grouping, rather than carrying a second copy of that fallback that could drift.