Skip to content

fix(observability): fix Sentry fingerprinting for 3 fragmented issue groups - #5099

Merged
JSONbored merged 1 commit into
mainfrom
fix/5010-sentry-fingerprint-consolidation
Jul 11, 2026
Merged

fix(observability): fix Sentry fingerprinting for 3 fragmented issue groups#5099
JSONbored merged 1 commit into
mainfrom
fix/5010-sentry-fingerprint-consolidation

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Three distinct root causes were each fragmented across multiple separate Sentry issues, diluting their apparent severity (each fragment individually looked small) and defeating "existing issue" alert-trigger logic (each fragment is technically a separate "new" issue the first time it appears).

  1. REES's `captureAnalyzerDegradation` fingerprinted by WHICH analyzer hit a condition (`context.analyzer`), not WHY (`context.partialReason`) — so `"analyzer_timeout"` fragmented into a separate issue per analyzer (installScript, nativeBuild, license, ...), even though they share one root cause: the shared, dynamically-shrinking per-analyzer time budget under heavy concurrent fan-out (confirmed while investigating GITTENSORY-V/18/S/13 — all four turned out to already be fixed or inherent third-party flakiness, but the fragmentation itself was still real and worth fixing for future occurrences). Now groups by `partialReason`, falling back to analyzer name only when `partialReason` is absent. A reason that IS inherently analyzer-specific (e.g. `"bundlephobia-size_http_error"`) still gets its own issue, since the reason string already encodes that specificity.
  2. `captureError`/`captureReviewFailure`'s optional `eventName` (added earlier this session for Sentry issue titles, Sentry issue titles are generic ("Error", "HttpError") instead of describing what failed #5058) now also sets an explicit fingerprint when provided, mirroring `forwardStructuredLogToSentry`'s existing `scope.setFingerprint(["gittensory-log", event])` discipline. This is what actually fixes GITTENSORY-5/10 ("PR public-surface publish failed") and GITTENSORY-C/W ("AI review did not produce public notes") going forward — both pairs were fragmented because Sentry's default stack-trace-based grouping treated the same message captured from two different functions as two different issues.

Closes #5010

Scope

Validation

  • `git diff --check`
  • `npm run actionlint`
  • `npm run typecheck`
  • `npm run test:coverage` (full unsharded `npm run test:ci`, all green)
  • `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`
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — added regression tests for the new grouping-by-reason behavior (and the fallback-to-analyzer-name case) in REES's `sentry-degradation.test.ts`, and for the new fingerprint-set-when-eventName-provided / not-set-when-absent behavior in `selfhost-sentry.test.ts`. Also ran the full REES suite (`npm run rees:test`, 1318 tests) since this touches `review-enrichment/`.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A.)
  • UI changes use live API data or real empty/error/loading states. (N/A.)
  • Visible UI changes include a `UI Evidence` section. (N/A — backend-only.)
  • Public docs/changelogs are updated where needed. (N/A.)

…groups (#5010)

Three distinct root causes were each fragmented across multiple
separate Sentry issues, diluting their apparent severity (each
fragment individually looked small) and defeating "existing issue"
alert-trigger logic:

1. REES's captureAnalyzerDegradation fingerprinted by WHICH analyzer
   hit a condition (context.analyzer), not WHY (context.partialReason)
   -- so "analyzer_timeout" fragmented into a separate issue per
   analyzer (installScript, nativeBuild, license, ...) even though
   they share one root cause (the shared, dynamically-shrinking
   per-analyzer time budget under heavy concurrent fan-out). Now
   groups by partialReason, falling back to analyzer name only when
   partialReason is absent. A reason that IS inherently
   analyzer-specific (e.g. "bundlephobia-size_http_error") still gets
   its own issue, since the reason string already encodes that.

2. captureError/captureReviewFailure's optional eventName (added
   earlier this session for Sentry issue titles) now also sets an
   explicit fingerprint when provided, mirroring
   forwardStructuredLogToSentry's existing
   scope.setFingerprint(["gittensory-log", event]) discipline. This
   is what actually fixes GITTENSORY-5/10 ("PR public-surface publish
   failed") and GITTENSORY-C/W ("AI review did not produce public
   notes") going forward: both pairs were fragmented because Sentry's
   default stack-trace-based grouping treated the same message
   captured from two different functions as two different issues.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.20%. Comparing base (e04e847) to head (c713038).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5099   +/-   ##
=======================================
  Coverage   94.20%   94.20%           
=======================================
  Files         470      470           
  Lines       39736    39738    +2     
  Branches    14501    14503    +2     
=======================================
+ Hits        37435    37437    +2     
  Misses       1645     1645           
  Partials      656      656           
Flag Coverage Δ
shard-1 46.40% <100.00%> (-0.02%) ⬇️
shard-2 33.63% <0.00%> (-0.23%) ⬇️
shard-3 31.35% <0.00%> (-0.80%) ⬇️
shard-4 32.77% <0.00%> (+0.96%) ⬆️
shard-5 33.44% <0.00%> (-0.10%) ⬇️
shard-6 45.10% <0.00%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/sentry.ts 99.63% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 11, 2026
@loopover-orb

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 14:59:47 UTC

4 files · 2 AI reviewers · no blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/selfhost/sentry.ts (matched src/selfhost/**).

Review summary
This PR changes fingerprinting for three Sentry capture paths to consolidate fragmented issue groups: captureAnalyzerDegradation now fingerprints by partialReason (falling back to analyzer), and captureError/captureReviewFailure now set an explicit fingerprint when eventName is provided, mirroring the existing forwardStructuredLogToSentry pattern. The fallback logic (context.partialReason ?? context.analyzer) is correct and the change is narrowly scoped to the fingerprint computation without touching unrelated tagging/context logic. Tests cover both the grouped case (same partialReason from different analyzers) and the fallback case, and for captureError/captureReviewFailure both the eventName-present and eventName-absent paths are asserted against setFingerprint being called or not called.

Nits — 7 non-blocking
  • src/selfhost/sentry.ts's fingerprint arrays use plain string literals ("gittensory-error", "gittensory-review-failure") duplicated across two functions; consider a shared prefix constant to avoid drift if the naming convention changes later.
  • The external brief flags src/selfhost/sentry.ts as growing past a 400-line threshold; not caused by this PR's small diff, but worth a follow-up file split if it keeps growing.
  • Consider adding a code comment or test asserting that a partialReason like "bundlephobia-size_http_error" (analyzer-specific) is not merged across analyzers, to make the stated design intent explicit in tests, not just in the comment.
  • review-enrichment/src/sentry.ts:299-306's long inline comment could be trimmed since the PR description already documents the rationale; not required.
  • nit: review-enrichment/src/sentry.ts:captureAnalyzerDegradation uses `context.partialReason ?? context.analyzer`, so a blank string partialReason would still sanitize to `unknown` in `setFingerprint` instead of falling back to the analyzer; confirm blank partialReason is impossible or normalize it with the existing `nonBlank` helper.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 2 reviewers, synthesized
Linked issue ✅ Linked #5010
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 46 registered-repo PR(s), 38 merged, 416 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 46 PR(s), 416 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The fingerprint fixes are small, targeted, and directly close the linked issue (#5010) by correcting real Sentry grouping bugs across three call sites, backed by new tests exercising both the grouped and fallback/no-op paths.
Linked issue satisfaction

Addressed
The diff adds explicit fingerprinting for all three groups: captureAnalyzerDegradation now groups by partialReason (fixing analyzer_timeout fragmentation), and captureError/captureReviewFailure now set fingerprints from eventName (fixing both the AI-review-notes and publish-failed pairs), with tests confirming each behavior. The PR description also explicitly addresses the required root-cause inve

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 46 PR(s), 416 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
@JSONbored
JSONbored merged commit 86370f2 into main Jul 11, 2026
19 checks passed
@JSONbored
JSONbored deleted the fix/5010-sentry-fingerprint-consolidation branch July 11, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

Fix Sentry fingerprinting for 3 fragmented issue groups (analyzer_timeout, AI-review-notes, publish-failed)

1 participant