Skip to content

feat(gate): tell a queued PR its position in the merge train, not just the PR ahead of it - #9954

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
feat/merge-train-queue-position
Jul 30, 2026
Merged

feat(gate): tell a queued PR its position in the merge train, not just the PR ahead of it#9954
loopover-orb[bot] merged 1 commit into
mainfrom
feat/merge-train-queue-position

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #9952

The problem

The contributor-facing wait comment named the immediate blocker and nothing else:

Queued in the merge train behind #4, which touches overlapping work and was opened first.

"Behind #4" and "behind #4 and six others" are very different waits. A queue that will not say which one it is reads as a stall rather than a wait — which is exactly how it was read in practice.

The fix

The information was already computed and discarded: shouldWaitForOlderSiblings builds the full sorted list of viable overlapping older siblings and returned only viable[0]. It now returns the whole list, and the comment reports position:

Queued in the merge train at position 4, behind 3 overlapping PRs opened before this one (#1, #2, #3). The nearest is #1.

Single-blocker wording is unchanged — rendering a one-item queue as "1 PR ahead: #4" is noise.

The queue inherits every eviction rule

queueAhead is derived from the same filtered list as blockingPr, so a draft, a git-conflicted PR, a manual-review hold and a non-overlapping sibling are all absent from it. A contributor must never be told they are behind PRs that are not actually in front of them — pinned by an explicit invariant test that puts one of each in the sibling set and asserts a one-item queue.

Also asserted: queueAhead[0] === blockingPr, since the comment quotes both and they must agree.

Not included

The comment is posted once per blocker and never updated as the queue drains, so someone who reads it at position 5 cannot learn they are now at position 2 without watching the train. Editing in place (marker comment + update) is the natural follow-up, left out deliberately to keep this change small and verifiable — noted on the issue.

Full suite green (25,869 passing). Changed-line coverage: zero uncovered statements, zero uncovered branches.

…t the PR ahead of it

The wait comment named the immediate blocker and nothing else:

  Queued in the merge train behind #4, which touches overlapping work and was
  opened first.

"Behind #4" and "behind #4 and six others" are very different waits, and a
queue that will not say which one it is reads as a stall rather than a wait --
which is how it was read in practice.

The information was already computed and thrown away: shouldWaitForOlderSiblings
builds the full sorted list of viable overlapping older siblings and returned
only viable[0]. It now returns the whole list, and the comment reports the
position:

  Queued in the merge train at position 4, behind 3 overlapping PRs opened
  before this one (#1, #2, #3). The nearest is #1.

The single-blocker wording is unchanged -- rendering a one-item queue as "1 PR
ahead: #4" is noise.

The queue inherits every eviction rule the blocker choice already had, which
the tests pin: a draft, a conflicted PR, a manual-review hold and a
non-overlapping sibling are all absent from it. A contributor must never be
told they are behind PRs that are not actually in front of them.

Closes #9952
@loopover-orb

loopover-orb Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-30 20:27:29 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR extends MergeTrainDecision with a queueAhead array (built from the already-computed viable-siblings list) and updates the wait comment to report queue position when more than one PR is ahead, falling back to the original single-blocker wording otherwise. The change is correctly threaded end-to-end: shouldWaitForOlderSiblings now returns the full sorted list instead of discarding it after taking viable[0], the executor branches on queueAhead.length, and tests cover the eviction invariants (draft/dirty/manual-review-held siblings never inflate the queue) plus the blockingPr/queueAhead[0] agreement invariant. It closes #9952 directly and is narrowly scoped.

Nits — 4 non-blocking
  • nit: the ternary in agent-action-executor.ts (~line 771) building the wait comment is a bit dense with inline comments interleaved — could be extracted into a small helper function for readability.
  • nit: consider a test asserting the exact wording when queueAhead.length === 0 is impossible (i.e. blocker always implies queueAhead.length >= 1) is implicitly covered, but an explicit type-level comment could help future readers.
  • Consider extracting the wait-comment string construction in agent-action-executor.ts into a small named function (e.g. buildMergeTrainWaitComment) to keep the call site shorter and make future wording tweaks easier to test in isolation.
  • 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.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9952
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: 9 registered-repo PR(s), 8 merged, 280 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 9 PR(s), 280 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR returns the full sorted viable-sibling list from shouldWaitForOlderSiblings and rewrites the wait comment to state the queue position and list overlapping PRs ahead, exactly matching the issue's requested wording, while preserving single-blocker phrasing when only one PR is ahead as requested; tests cover position reporting and that evicted siblings don't inflate the count.

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: Python, TypeScript, Ruby, Go, MDX, Shell, Solidity, JavaScript
  • Official Gittensor activity: 9 PR(s), 280 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: 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.
🧪 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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.00%. Comparing base (3689891) to head (3c7dd35).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9954      +/-   ##
==========================================
- Coverage   91.88%   91.00%   -0.88%     
==========================================
  Files         928      928              
  Lines      113671   113673       +2     
  Branches    27412    27412              
==========================================
- Hits       104441   103451     -990     
- Misses       7931     9118    +1187     
+ Partials     1299     1104     -195     
Flag Coverage Δ
backend 94.11% <100.00%> (-1.56%) ⬇️

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

Files with missing lines Coverage Δ
src/review/merge-train.ts 100.00% <100.00%> (ø)
src/services/agent-action-executor.ts 96.92% <100.00%> (+0.01%) ⬆️

... and 3 files with indirect coverage changes

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit afa08af into main Jul 30, 2026
8 checks passed
@loopover-orb
loopover-orb Bot deleted the feat/merge-train-queue-position branch July 30, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The merge-train wait comment names the blocker but never the queue position

1 participant