Skip to content

perf(review): early-exit the re-review when a swept PR is already closed on GitHub - #2000

Merged
JSONbored merged 1 commit into
mainfrom
claude/regate-terminal-exit
Jul 1, 2026
Merged

perf(review): early-exit the re-review when a swept PR is already closed on GitHub#2000
JSONbored merged 1 commit into
mainfrom
claude/regate-terminal-exit

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

When the re-gate sweep re-reviews a stored-open PR, it only checked the stored state, then spent GitHub budget resyncing files, running the readiness/CI reads, and re-reviewing — even for a PR that is already closed/merged on GitHub (a closed webhook the self-host relay dropped while it was down).

reReviewStoredPullRequest now early-exits on the live terminal state, right after the single memoized fetchLivePullRequest:

  • if the live PR is non-open, upsert the live payload to reconcile the stored row, then return — before refreshPullRequestDetails (the /pulls/{n}/files reads), prReadyForReview (the /commits/{sha}/… CI reads), and the review/publish path;
  • reconciling the row means the next sweep skips the PR outright via the existing stored-state guard.

Fail-open: only a live non-open state early-exits — a fetch hiccup leaves live undefined and the sweep proceeds with the stored open PR exactly as before.

This is the rec #13 half of the #1942 audit ("a job already known to be stale exits before expensive GitHub reads"). Advances #1936. (Rec #12 — the currentRegateBacklog COUNT — is a separate follow-up: that path has since moved onto the queue snapshot() binding, so it needs re-scoping rather than the audit's original edit.)

Scope

  • Conventional Commit title.
  • One focused branch in reReviewStoredPullRequest; no signature or config change.
  • Behavior-preserving for open PRs; the only new behavior is skipping the pipeline for already-closed ones.

Validation

  • git diff --check
  • npm run typecheck
  • npm run test:coverage — new sweep test: a PR closed on GitHub reconciles the stored row to closed, and makes no /files or /commits reads (early-exit); the existing drift / no-drift resync tests still pass.
  • npm run test:ci
  • npm audit --audit-level=moderate

Safety

…sed on GitHub

reReviewStoredPullRequest only checked the STORED state, then spent GitHub budget
resyncing files + running readiness/CI reads + re-reviewing even for a PR already
closed/merged on GitHub (a dropped `closed` webhook while the relay was down).

After the single memoized fetchLivePullRequest, early-exit on a live NON-open
state: upsert the live payload to reconcile the stored row, then return before the
files + CI reads + review. Reconciling the row lets the next sweep skip it via the
existing stored-state guard. Fail-open: only a live non-open state early-exits.

Advances #1936.
@dosubot dosubot Bot added the size:XS label Jul 1, 2026
@loopover-orb

loopover-orb Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-01 07:22:24 UTC

2 files · 1 AI reviewer · no blockers · readiness 86/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change adds the right live-state guard in reReviewStoredPullRequest: after the memoized PR fetch, a live non-open PR is reconciled into storage and the expensive file/CI/review path is skipped. The guard is narrowly scoped to terminal live state and preserves the existing fail-open behavior when the live fetch is unavailable. The accompanying test drives the dropped-webhook scenario and verifies the stored row becomes closed and the budget-heavy reads are not made.

Nits — 5 non-blocking
  • nit: test/unit/queue.test.ts:840 returns Response.json({}) for every unexpected fetch, which can mask a future accidental GitHub/API call in a test whose purpose is to prove no extra reads happen.
  • nit: src/queue/processors.ts:1816 swallows reconciliation failures silently, so a DB/upsert regression would leave the stored row stale without any diagnostic signal.
  • test/unit/queue.test.ts:840 should fail on unexpected fetch URLs after the known /access_tokens and /pulls/7 cases so the early-exit budget assertion stays strict.
  • src/queue/processors.ts:1816 should at least emit a structured debug/error log on a swallowed upsert failure, or document why this deliberately mirrors the adjacent drift-path behavior.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 68 registered-repo PR(s), 58 merged, 209 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 68 PR(s), 209 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 68 PR(s), 209 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.

🟩 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 gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 1, 2026
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2000      +/-   ##
==========================================
- Coverage   95.66%   95.66%   -0.01%     
==========================================
  Files         220      220              
  Lines       24453    24456       +3     
  Branches     8868     8869       +1     
==========================================
+ Hits        23394    23395       +1     
  Misses        436      436              
- Partials      623      625       +2     
Files with missing lines Coverage Δ
src/queue/processors.ts 89.75% <100.00%> (-0.11%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jul 1, 2026
@JSONbored
JSONbored merged commit 57be754 into main Jul 1, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/regate-terminal-exit branch July 1, 2026 07:25
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
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.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant