Skip to content

fix(queue): prioritize linked-issue siblings when waking PRs on a merge - #7905

Merged
JSONbored merged 1 commit into
mainfrom
fix/sibling-wake-linked-issue-priority
Jul 21, 2026
Merged

fix(queue): prioritize linked-issue siblings when waking PRs on a merge#7905
JSONbored merged 1 commit into
mainfrom
fix/sibling-wake-linked-issue-priority

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to fix(queue): fast-track a PR whose merge/close got denied by a live staleness recheck #7886 (fast-tracks a stale-recheck-denied PR): this fixes the mechanism that should have caught that scenario in the first place. The feat(review): event-driven re-gate trigger on sibling PR merge #4005 event-driven sibling re-gate (maybeEnqueueSiblingRegateForMergedPr) exists specifically to catch "a duplicate cluster now missing its winner" the instant a merge happens, instead of waiting for the sweep. But it capped its wake set at the first MERGE_WAKE_MAX_PRS (15) entries of otherOpenPullRequests, a list ordered ascending-by-PR-number for a different purpose (duplicate-winner election, where the lowest number IS the winner). Reused here, that meant the mechanism always woke the 15 oldest open PRs in the repo — with zero relation to which PRs a given merge could actually invalidate.
  • In a repo with 15+ older open PRs already sitting around (increasingly likely, since a maintainer-only manual-review hold is never auto-cleared), a genuinely-affected, newer duplicate-cluster sibling could be silently excluded from ever being woken by this mechanism at all, leaving it to the ordinary (much slower) sweep cadence.
  • Fix: siblings that share a linked issue with the just-merged PR are now placed first, ahead of the existing oldest-first fallback used for the general "any PR could now conflict with the new base tip" case (no single relevance signal exists there, so PR age stays a reasonable heuristic). The fallback tier, and behavior when the merged PR has no linked issues, is unchanged.

Test plan

  • npm run typecheck
  • Two new tests in test/unit/queue.test.ts covering: a linked sibling winning priority over a lower-numbered unrelated one, and every linked sibling surviving the cap by displacing the highest-numbered fallback siblings
  • All 5 sibling-wake tests pass, including the 3 pre-existing ones unchanged (proving byte-identical behavior when the merged PR has no linked issues)
  • Full test/unit/queue*.test.ts suite (960 tests) passes
  • 100% line/branch coverage on every new line (verified via scoped vitest --coverage)
  • Full unsharded npm run test:coverage in progress

The #4005 sibling-wake mechanism (maybeEnqueueSiblingRegateForMergedPr) capped
its wake set at the first MERGE_WAKE_MAX_PRS entries of an ascending-by-PR-number
list built for duplicate-winner election, not relevance. In a busy repo with that
many older open PRs, a genuinely-affected duplicate-cluster sibling could be
silently excluded from ever being woken, even though catching exactly that case
("a duplicate cluster now missing its winner") is this mechanism's whole purpose.

Siblings sharing a linked issue with the merged PR are now placed first, ahead of
the existing oldest-first fallback used for the general "any PR could now
conflict with the new base tip" case.
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.74%. Comparing base (2364ad4) to head (93cd5ee).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7905      +/-   ##
==========================================
- Coverage   91.40%   89.74%   -1.66%     
==========================================
  Files         730       89     -641     
  Lines       74789    24347   -50442     
  Branches    22822     4787   -18035     
==========================================
- Hits        68358    21850   -46508     
+ Misses       5389     2232    -3157     
+ Partials     1042      265     -777     
Flag Coverage Δ
shard-1 80.43% <100.00%> (+22.45%) ⬆️
shard-2 30.22% <60.00%> (-21.01%) ⬇️
shard-3 39.25% <60.00%> (-11.90%) ⬇️

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

Files with missing lines Coverage Δ
src/queue/processors.ts 95.78% <100.00%> (+<0.01%) ⬆️

... and 641 files with indirect coverage changes

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

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-21 17:07:42 UTC

2 files · 1 AI reviewer · 2 blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This fixes a real bug in the #4005 event-driven sibling re-gate: it previously reused otherOpenPullRequests's ascending-by-number ordering (meant for duplicate-winner election) as if it were a relevance ordering, capping the wake set at the 15 oldest open PRs regardless of actual relevance to the merge. The fix partitions siblings into a linked-issue tier (via the merged PR's own linkedIssues, filtered/preserving ascending order) placed first, then a fallback tier of the remaining PRs oldest-first, sliced to MERGE_WAKE_MAX_PRS — correctly falling back to byte-identical old behavior when the merged PR has no linked issues (linkedSiblingNumbers.size===0 branch). The two new tests trace through the actual dispatch order and delay options correctly and match the described priority/displacement semantics.

Nits — 3 non-blocking
  • The doc-comment block above the function is quite long/verbose (adds ~14 lines); consider trimming to the essential rationale since the git history/PR description already preserves the incident context.
  • processors.ts:1349-ish — confirm `sibling.linkedIssues` and `PullRequestRecord.linkedIssues` are populated consistently for all callers of `upsertPullRequestFromGitHub` (not shown in this diff) so the filter doesn't silently miss real siblings on stale/partial data.
  • Consider extracting the linked/fallback partition logic into a small named helper if it's likely to be reused elsewhere (e.g., duplicate-winner election could someday want the same relevance-first ordering).

Concerns raised — review before merging

  • No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example `Closes #123`) before opening the PR.

Decision drivers

  • ❌ Code review — 2 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
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: 21 registered-repo PR(s), 14 merged, 325 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 21 PR(s), 325 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
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: 21 PR(s), 325 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 2 steps in the Signals table above.
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.
🧪 Chat with LoopOver

Ask 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.

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

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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 21, 2026
@JSONbored
JSONbored merged commit cf0cc6f into main Jul 21, 2026
14 checks passed
@JSONbored
JSONbored deleted the fix/sibling-wake-linked-issue-priority branch July 21, 2026 17:10
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant