Skip to content

[Bug]: type-label recompute unnecessarily runs on review-family webhooks, widening the #4818 staleness race #4986

Description

@JSONbored

Summary

Follow-up to #4975/#4980, prompted by a direct question about the design: why does the type-label decision run on a webhook triggered by a review, when only the PR's title and its linked issue can determine the correct gittensor:bug/gittensor:feature/gittensor:priority label?

Answer, from re-reading shouldProcessPullRequestPublicSurface (src/queue/processors.ts): the type-label recompute is embedded inside maybePublishPrPublicSurface, a much larger "publish everything" function that legitimately needs to re-run on far more events than labeling does — gate re-evaluation, comment updates, screenshot capture, etc. genuinely do need to react to pull_request_review (submitted/edited/dismissed), pull_request_review_comment (created/edited/deleted), and pull_request_review_thread (resolved/unresolved). The type-label sub-block never had its own relevance gate, so it silently recomputed on every one of those triggers too, even though none of them can ever change a PR's title or its linked-issue list — the only two inputs the label decision depends on. closed (merge) is a deliberate, necessary trigger (that's the first moment "Closes #N" auto-close can be attributed to this PR, and the label must be correct at the moment scoring happens) — but the three review-family event types were riding along for free, and #4818 showed exactly why that's dangerous: each one carries its own independently-timed, independently-stale embedded pull_request webhook snapshot, and #4980 fixed only the symptom of one such snapshot's merged_at reading null.

What this PR does

Excludes pull_request_review, pull_request_review_comment, and pull_request_review_thread from ever reaching the type-label recompute at all — not just the narrow ambiguous branch #4980 fixed. This shrinks the risk surface structurally instead of patching each newly-discovered staleness path one at a time (the third time in this bug's history that pattern has been tried). Nothing is lost: a genuine title/linked-issue change is still caught by the next pull_request-native event or the periodic sweep (bounded delay, not indefinite).

Also audited (result: no further action needed, documented for the record):

  • reReviewStoredPullRequest (the sweep/cross-reference path) cannot reach the type-label block for a non-open PR at all (double-guarded: stored-state check, then a live-state re-check) — confirmed not a contributor to any variant of this bug.
  • maybeProcessPrPanelRetrigger (manual re-review) reads pr fresh from the DB, not a webhook snapshot; upsertPullRequestFromGitHub's mergedAt: pr.merged_at ?? undefined uses this codebase's established "undefined omits the column, only explicit null clears it" convention, so a stale review-webhook's null merged_at can never regress an already-correct DB value.
  • /pulls/{n} and /issues/{n} reads (including fix(review): resolve an ambiguous null prMergedAt with a live recheck before downgrading a label #4980's new live-recheck) are excluded from both the 20s Redis response cache (githubCacheClassForUrl only classifies branch-protection/commit-ref/bare-repo reads as cacheable) and are confirmed fail-safe (inconclusive, never a silent downgrade) under single-flight coalescing.
  • Exactly one code path writes gittensor:bug/feature/priority (processors.ts's type-label block) — no duplicate/legacy writer exists elsewhere in src/.

Deliverables

  • Exclude review-family events from the type-label recompute (src/queue/processors.ts).
  • Regression tests: each of the three excluded event types verified to skip with zero label writes and zero linked-issue fetches; a combined test reproducing the feat(ui): confidence-calibration curve card on the analytics dashboard (#2192) #4818 shape end-to-end via pull_request_review proving an already-correct label survives untouched.
  • 100% branch coverage on every changed line, confirmed via lcov.

Live verification (2026-07-11, this session)

Re-ran the full 72-hour downgrade-signature audit across all 3 gated repos (gittensory/metagraphed/awesome-claude) after #4980 shipped: 137 unique flagged PRs total, only 2 (#4818, #4972) still wrong on live GitHub — both merged before #4980's deploy (06:48 UTC), i.e. pre-fix casualties, not evidence of a residual gap. Both corrected. 0 new occurrences observed since deploy (though not enough time/volume has passed yet to naturally re-trigger the exact race and confirm end-to-end).

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.roadmapOn the Wave-2 agent-layer roadmap board (project 9)

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions