fix(review): gate_check_run's byte-identical no-op closes #6724's remaining gap#7517
Conversation
…aining gap (#7514) surfaceContentChanged treated gate_check_run's mere presence in a pass's published outputs as an unconditional change, even when the gate's actual conclusion was byte-identical to the last completed check for that exact head SHA -- #6724 already covered comment/label this way but its own test file flagged gate_check_run as "a separate, not-yet-implemented scope." recordPublishedGateCheckSummary already upserts keyed on [repoFullName, headSha, name], so a prior row for the exact current head SHA only exists if this SAME commit was already gate-checked before -- a genuinely new commit has no prior row and always falls through to "changed." Reads the prior conclusion once, before this pass writes anything, and compares it against whichever conclusion this pass actually recorded at each of the 3 gate-check publish sites -- not re-derived, so the comparison can't drift from what was really written. check_run (the separate advisory check) has no equivalent persisted prior state today, so its presence is still left unconditionally counted, same as before -- extending that would need new persistent storage and is a larger, separate change. Also corrects one existing #6724 test that turned out to rely on an unrelated type-label mechanism's own activity rather than the label output it claimed to test (confirmed via direct instrumentation: decision.willLabel was false throughout that test; the label POSTs came from the separate, unconditional resolvePrTypeLabel path). Closes #7514
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7517 +/- ##
==========================================
- Coverage 88.48% 88.46% -0.02%
==========================================
Files 720 720
Lines 75710 75718 +8
Branches 22537 22540 +3
==========================================
- Hits 66989 66986 -3
- Misses 7677 7684 +7
- Partials 1044 1048 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-20 20:08:58 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
The self-monitoring "review burst" anomaly detector has repeatedly flagged the same PRs (
JSONbored/loopover,/metagraphed,/awesome-claude) for publishing 6+ review surfaces within 2 hours with no real state change (Sentryops_anomaly, LOOPOVER-21/22/23/1W).#6724added byte-identical no-op detection for the comment and label outputs, but its own test file explicitly flaggedgate_check_runas "a separate, not-yet-implemented scope" for any repo with the gate check enabled — the common case. This closes that gap.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.Closes #7514
Validation
git diff --checknpm run typechecknpm run test:coveragelocally — 2 new end-to-end tests intest/unit/queue.test.tscover the no-op suppression (identical gate conclusion, same head SHA, across two fullprocessJobpasses) and the critical safety property (a genuinely new commit — different head SHA — is never suppressed, even with an identical AI verdict); the fullqueue.test.tsfile (176 tests) passesIf any required check was skipped, explain why:
Safety
UI Evidencesection. (N/A — no visible UI change.)Notes
Design, matching
#6724's own conservative philosophy ("this guard is only ever allowed to skip a PROVABLE no-op, never to guess one"):recordPublishedGateCheckSummaryalready upserts the gate-check summary keyed on[repoFullName, headSha, name]— so a prior row for the exact current head SHA only exists if this SAME commit was already gate-checked before. A genuinely new commit has no prior row and always falls through to "changed."priorGateCheckConclusion, near the top ofmaybePublishPrPublicSurface), and compared against whichever conclusion this pass actually recorded (finalGateCheckConclusion, captured at each of the 3 gate-check publish call sites) — not re-derived, avoiding any risk of the comparison drifting from what was really written.check_run(the separate advisory check, distinct from the gate one) has no equivalent persisted "prior state" today, so its presence is left unconditionally counted as a change, same as before — extending that is a larger, separate change (would need new persistent storage) and is out of scope here.#6724test ("a comment-unchanged pass that newly applies a previously-missing label...") turned out to rely on an unrelated type-label mechanism's own activity (confirmed via direct instrumentation:decision.willLabelwasfalsethroughout; every label POST in that test came from the separateresolvePrTypeLabelpath, which runs unconditionally and was never part ofsurfaceContentChanged's tracked outputs, before or after this change). Renamed and corrected that test's expectations to reflect the more accurate behavior; the type-label mechanism's own actions still happen exactly as before, this only affects the internal "did the tracked review surface change" bookkeeping.