Skip to content

fix(observability): surface RAG/enrichment/inline review failures to Sentry (#1618)#1619

Merged
JSONbored merged 1 commit into
mainfrom
feat/review-step-sentry-telemetry
Jun 27, 2026
Merged

fix(observability): surface RAG/enrichment/inline review failures to Sentry (#1618)#1619
JSONbored merged 1 commit into
mainfrom
feat/review-step-sentry-telemetry

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Three review-pipeline failure points were caught + swallowed and never reached Sentry (the central forwarder captures level:error/fatal structured logs). When the reviewer's context backends degrade, the review silently gets worse with no signal — exactly the diagnosis pain behind the recent self-host outage. This surfaces them at error (no forwarder change needed; each remains fail-safe — only visibility changes):

  • RAG retrieval (retrieveContext, rag.ts) — logged rag_retrieve_error via console.log with no level field → a down qdrant/embedder degraded reviews to diff-only, invisibly. Now level:error review_context_fetch_failed (contextType:rag), keeping the ev tag for log continuity.
  • REES enrichment (buildReviewEnrichment catch) — swallowed fetch/timeout/parse errors. Now level:error review_context_fetch_failed (contextType:enrichment).
  • Inline review comments (postInlineReviewComments catch) — logged the post failure at warn (forwarder skips it). Now error.

Each fix is exercised by a regression test asserting it surfaces at level:error. Grounding-wire and rag-wire's outer catches are intentionally left as-is — their inner code is fail-safe so those catches are unreachable; the real RAG failure point is inside retrieveContext, which is where the fix lands.

Closes #1618.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused — observability only (review-context failure surfacing); no behavior change.
  • This follows CONTRIBUTING.md; no site/, CNAME, or Pages.
  • I linked an issue.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch ≥97% — each changed catch is exercised by a test asserting the level:error review_context_fetch_failed / inline_comments_post_failed log (rag via a throwing vector query, enrichment via a rejecting fetch, inline via a 500 review POST).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • Regression tests for each new error-surfacing path

If any required check was skipped, explain why:

  • No artifact regeneration / migration: pure logging-level change, no API/schema/binding/DB change.

Safety

  • No secrets/wallets/hotkeys/trust-scores/private rankings exposed — the logs carry repo/PR/contextType + a bounded error message (the error strings are infra failures, not PR content; the existing redaction/.slice bounds apply).
  • Public GitHub text stays sanitized — these are server logs, not GitHub surfaces.
  • Auth/CORS/session changes include negative-path tests — N/A.
  • API/OpenAPI/MCP behavior updated/tested — N/A.
  • UI changes use live data — N/A (no UI change).
  • Visible UI changes include UI Evidence — N/A.
  • Public docs/changelogs updated — N/A.

Notes

@dosubot dosubot Bot added the size:S label Jun 27, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@0700d3c). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1619   +/-   ##
=======================================
  Coverage        ?   95.51%           
=======================================
  Files           ?      204           
  Lines           ?    22051           
  Branches        ?     7966           
=======================================
  Hits            ?    21062           
  Misses          ?      413           
  Partials        ?      576           
Files with missing lines Coverage Δ
src/review/enrichment-wire.ts 100.00% <100.00%> (ø)
src/review/inline-comments.ts 100.00% <100.00%> (ø)
src/review/rag.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…Sentry (#1618)

Three review-context failure points were caught + swallowed and never reached
the central Sentry forwarder (which captures level:error/fatal structured logs),
so a degrading reviewer (qdrant/REES down, inline-post 422) had no signal:

- retrieveContext (rag.ts) logged rag_retrieve_error via console.log with no
  level field — now level:error review_context_fetch_failed (contextType rag),
  keeping the ev tag for log continuity.
- buildReviewEnrichment swallowed fetch/timeout/parse errors — now level:error
  review_context_fetch_failed (contextType enrichment).
- postInlineReviewComments logged the post failure at warn — now error.

Each remains fail-safe (the review still proceeds); only visibility changes.
Regression tests pin that each surfaces at level:error. The grounding-wire and
rag-wire OUTER catches are left as-is — their inner code is fail-safe so those
catches are unreachable; the real RAG failure point is inside retrieveContext.
@JSONbored
JSONbored force-pushed the feat/review-step-sentry-telemetry branch from c57e8e5 to 2638adc Compare June 27, 2026 19:35
@dosubot dosubot Bot added the size:S label Jun 27, 2026
@JSONbored
JSONbored merged commit 031cb7b into main Jun 27, 2026
18 checks passed
@JSONbored
JSONbored deleted the feat/review-step-sentry-telemetry branch June 27, 2026 19:39
JSONbored added a commit that referenced this pull request Jun 27, 2026
…1625)

fetchRepoTree and listStoredChunkPaths logged their catch failures via
console.log with no level, so the central Sentry forwarder (level:error/fatal
only) never saw them — a broken RAG index population (GitHub tree fetch down,
D1 read error) silently degraded retrieval quality with no signal.

Promote both to level:error (event rag_index_tree_error / rag_list_paths_error,
keeping the ev tag for log continuity), mirroring the rag.ts pattern from #1619.
Both stay fail-safe; only visibility changes. Covered by the existing tree-throws
and stored-paths-read-error tests, now asserting the level:error log.
JSONbored added a commit that referenced this pull request Jun 28, 2026
…the event slug (#1636)

forwardStructuredLogToSentry titled each issue with only obj.event and buried
the real failure in a 'log' context blob — so operators had to open each issue
to learn what broke. Now:
- title leads with the failure: "<event>: <message ?? error>" (e.g.
  orb_broker_unavailable: The operation was aborted due to timeout)
- index filterable tags for the dimensions operators search/group by (repo,
  installationId, pull, pr, project, kind, deliveryId), present values only
- fingerprint by event so recurrences collapse into one issue instead of
  fragmenting on the variable detail now in the title

Pairs with the source-side instrumentation (#1605/#1619/#1625/#1627/#1628):
those make failures reach Sentry; this makes each issue legible at a glance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review pipeline silently degrades on RAG / enrichment / inline-comment failures (not surfaced to Sentry)

1 participant