Repository navigation
🐛 Tracker: chevron column follows the current state, never the highest stage reached - #15657
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
There was a problem hiding this comment.
Read against v5 HEAD. The fix is correct where it takes effect — the SPA — and I have one maintainability note about the Go half.
Verified
static-src/js/tracker-row.js:25-26now checksstageTrackerUnderRepair(cur, item.reason)before themerge_gate(:28) andmerge_train(:33) promotions, so a row at 070/071/072/074 or with acheck-failing:/ci-redreason lights column 50 regardless of a stalemerge-failedgate — that is exactly the second path the body describes. 085 is excluded by thecur !== STAGE_TRACKER_NEEDS_DECISIONguard, matching the Go comment.tracker-words.js:196-204mirrorsstageTrackerTrainUnderRepair(stage_tracker_merge_train.go:256-273) case for case, including the three reason prefixes.left_trainis now threaded through both nested-PR mappers intracker-row.js, matching the newStageTrackerLinkedPR.LeftTrainfield (stage_tracker.go:651, copied at:3228).stage_tracker_fixing_column.test.jsexercises the SPA path, so the behaviour that actually renders is pinned.
Note (low, maintainability): stageTrackerItemColumn has no production caller.
src/pkg/dashboard/stage_tracker_vocabulary.go:68 is referenced only from stage_tracker_item_column_test.go; nothing in the response path calls it, and the API still ships no column field — the chevron is computed solely by stageTrackerAnchor in the SPA. The 1650-case Go ratchet therefore pins a mirror that cannot drift into the page, only away from it. Either wire it (emit the column on the item so the SPA reads it, which would also retire the duplicated predicate) or keep the rule in one place and lean on the JS test. Not blocking.
Scope note (info): adding 074 to stageTrackerTrainUnderRepair also changes merge-train membership (that function is the seating rule, per its own comment), not only the column. The body declares it ("now includes 074"), and dropping a superseded original from its train reads as intended, so I'm flagging it only so the reviewer merging this knows it is a behaviour change beyond the chevron.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the diff against the v5 tree (merge-base 5380ab4), the anchor callers, and ran the new JS test. This looks correct to me.
What I verified
src/pkg/dashboard/static-src/js/tracker-row.js:26— the under-repair check now runs before themerge_gate(:28) andmerge_train(:33) promotions, which is exactly the second path the body describes: a 070 row with a stalemerge-failedgate used to fall through to column 80. All fivestageTrackerAnchorcall sites (tracker-row.js:58,1126,1159,1312,tracker-motion.js:369) pass the item or pass'pr'for linked sub-rows;:1312passes no item, sorepairingis simply false there — no regression.tracker-words.js:195-201stageTrackerUnderRepairmirrorsstage_tracker_merge_train.go:255-273branch for branch (070/071/072/074/085 + therequired-check-failing:/check-failing:/ci-redreason prefixes). The 085 carve-out attracker-row.js:26matchesstage_tracker_vocabulary.go:74.stage_tracker.go:649-651,3228—LeftTrainis mirrored ontolinked_prs[]alongsideMergeTrain, and the two SPA record builders (tracker-row.js:952,1307) carryleft_trainthrough.node --test pkg/dashboard/stage_tracker_fixing_column.test.js: 4/4 pass on the head; withstatic/index.htmlreverted to the merge-base the file fails to load (stageTrackerUnderRepairabsent) — so the test does fail without the change.
One note, not a defect
stageTrackerItemColumn (stage_tracker_vocabulary.go:68) has no production caller — stageTrackerColumn is still the only one wired into buildStageTracker, and the new function is exercised only by stage_tracker_item_column_test.go. That's fine as a Go-side oracle for the ratchet, but the user-visible fix is entirely the SPA change; worth a one-line comment on the function saying so, so the next reader doesn't go looking for the API field it feeds.
I could not run go test here (blocked on this host), so the two Go tests are unverified by me; the JS side is.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
@clubanderson this PR is held at Why:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
|
stage 080-awaiting-merge-admission → 071-needs-author: conflicting |
…t stage reached A PR under repair (070 fixing, 071 needs-author, 072 returned-to- contributor, 074 adopted-by-hive, or a red required check) lights CI / Review even while a stale merge-failed gate or a planner-ordered merge train still rides on its record. Hive's own #15574 read "Fixing · conflicting" under Merge queue after a refused merge: the merge_gate (merging / merge-failed) promotion still won. One predicate now decides both the column and train membership: stageTrackerTrainUnderRepair (Go; stageTrackerItemColumn) and its SPA mirror stageTrackerUnderRepair (stageTrackerAnchor). left_train is mirrored onto linked_prs[]. Ratchet tests pin every PR stage × gate state × train state × reason to its column on both sides, and a payload test replays #15574. Fixes #15656 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
hive's own #15654 (000 New, filed seconds ago) nested the open #15655 drawn as "PR in progress", yet sorted at the bottom among the New rows: stageTrackerSortKey fell back to the parent's unlabelled stage when the nested PR had none, while the strip drew it at 040 covered-by-PR. stageTrackerLead / stageTrackerLeadStage are now the one rule both read. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
85de7ef to
478c80a
Compare
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Port the v5 tracker-row.js changes (hivecommons#15657, hivecommons#15664, hivecommons#15668, hivecommons#15680) into the split files: stageTrackerAnchor under-repair column (tracker-row.js), acted labels (tracker-row-strip.js), queue-merge pending copy (tracker-row-acted.js), and stageTrackerLead / left_train / subRows / review notes (tracker-row-layout.js). static/index.html regenerated and matches the auto-merge byte for byte. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
Fixes #15656
Finding
Maintainer: "why is this in merge queue if it is being fixed?" (hive's own #15574, adopted copy under #15511, chevron Fixing in the Merge-queue column, caption fell back to Fixing — … — conflicting).
At v5 HEAD (post-#15616) the bug is still present through a second path.
stageTrackerAnchorpromotes a row to column 80 onmerge_gate.state ∈ {merging, merge-failed}before it looks at the stage; #15616 only removed themerge_trainpromotion by dropping under-repair rows from the train. A PR whose merge attempt GitHub refused (conflict) regresses to 070 but keepsmerge-failedon its gate (stageTrackerMergeAttemptStaleclears it only for a moved head or a CI-pending reason), so it still anchors in Merge queue. Verified against the HEAD bundle:070 + merge-failed → 80,070 + merge_train → 80(pre-#15616 server),070 + left_train → 50.Rule
The chevron column follows the current state, never the highest stage reached. One predicate decides both column and train membership so they can never disagree:
stageTrackerTrainUnderRepair(already the train-membership rule; now includes 074 adopted-by-hive) → newstageTrackerItemColumn(it).stageTrackerUnderRepair(stage, reason)(tracker-words.js), consulted first bystageTrackerAnchor(tracker-row.js): under repair past 030 → CI / Review, before the gate / train promotions. The 085 decision park is the one state that rides no train yet stays at the gate, where the person decides.left_trainis mirrored ontolinked_prs[]so the sub-row hover says it left its train.Tests
stage_tracker_item_column_test.go:TestStageTrackerItemColumnRatchet— every PR stage × gate state × train state × reason → column (1650 cases), asserting an under-repair row is never promoted;TestStageTrackerFixingAfterMergeFailedColumn— replays 🌱 dashboard: split stage_tracker.go into 4 files #15574 throughbuildStageTracker(refused merge attempt, train front regressed to 070): gate keepsmerge-failedas history, column is CI / Review, no train, sibling's train shrinks to none,left_trainmirrored onto the issue's sub-row.stage_tracker_fixing_column.test.js: liftsstageTrackerAnchor+stageTrackerUnderRepairfrom the shippedindex.html; 🌱 dashboard: split stage_tracker.go into 4 files #15574 record in all four shapes → 50; predicate mirrors Go; ratchet over stage × gate × train × reason; docs#512 pins. Fails on the pre-fix bundle.go test ./pkg/dashboard/... -run 'Tracker|Stage|Column|Train' -count=1andnode --test src/pkg/dashboard/*.test.js(90/90) green. Bundle regenerated viago generate ./pkg/dashboard/webstatic/.Also folded in: hive#15654 (same "current live work" principle)
An issue filed seconds ago (000 New, no label yet) whose nested PR #15655 was already open rendered at the very bottom among the New rows, while its strip drew it via the PR as PR in progress · @clubanderson.
stageTrackerSortKeyfell back to the parent's unlabelled stage whenever the nested PR had no stage (linked[0].stagefalsy), but the strip drew the same row atlp.stage || 40. NewstageTrackerLead(item, stages)/stageTrackerLeadStage(lp)(tracker-row.js) are the one rule both the strip and the sort key / repo-group columns read: a covered issue's place follows its most-advanced live work — an open covering PR with no stage yet counts as 040 covered-by-PR. Test:#15654case instage_tracker_fixing_column.test.jsasserts the sort anchor equals the strip's anchor for every nested-PR stage, and that the row sorts above a New issue with no PR.Docs
dashboard-sections.md(column rule, train-state paragraph, under-repair list),api-reference/dashboard-misc.md(left_trainwhy values, mirrored ontolinked_prs[]),dashboard-copy-inventory.md(new row),changelog.d/fixed-fixing-chevron-column.md.The roll-9 spoke in the screenshot predates #15616; it needs the roll for the
merge_trainpath and this PR for themerge_gatepath.