Repository navigation
🐛 fix(review): report each withheld not-hive-authored head once - #14887
Conversation
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> (cherry picked from commit b9bce6c) Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[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 |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
There was a problem hiding this comment.
Reviewed (docs-currency) — approve, one low advisory.
Read at head 73c3062. The behavior change in withholdFix (src/pkg/review/dispatch.go:1399-1425) and the reworded auditWithheldReviewFixes doc (src/cmd/hive/main.go:11146-11149) now agree, and TestWithheldFixExclusionReportedOncePerHead pins it. Two adjacent comments still describe the old every-cycle contract:
- src/pkg/review/dispatch.go:485-488 — the
FixExclusionsfield doc says it holds "changes-requested PRs whose fix kick was deliberately not added to FixKicks this cycle". After this change an already-withheld not-hive-authored head's kick is still not added, but it no longer appears inFixExclusions(and so not in thefix_excluded/fix_excluded_reasonssummary at main.go:10908-10909). Worth a clause like the one onWithheldFixes(dispatch.go:481-483): refusals already inState.Withheldare not repeated. - src/cmd/hive/main.go:11151-11153 — "One WARN per changes_requested head that ends the cycle without a fix kick" is now only true for non-policy exclusions; an already-withheld head ends the cycle without a kick and without a line.
Neither blocks: the linked #14882/#14883 asked for exactly this dedup, and the changelog fragment (changelog.d/fixed-withheld-fix-exclusion-once.md) states the new behavior accurately.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Style/maintainability — fix is correct and minimal; one advisory, non-blocking.
src/pkg/review/fix_push_gate_test.go:273-290—TestWithheldFixExclusionReportedOncePerHeadre-builds the same two-cycle scenario thatTestHumanPRFixWithheldWhenFixHumanPRsOffalready runs at lines 71-80 (comment there: "Reported once per head: the next cycle finds the state entry and produces nothing new to audit"). The only new assertion islen(second.FixExclusions) != 0; adding that one check to the existingagainblock would keep the once-per-head contract pinned in one place. The new test also setspr.HiveAttributed = true(line 275) with no comment on why —humanPR("sha1")alone already reacheswithholdFix(as the existing test shows), so the flag reads as load-bearing when it is not. Every other test in this file carries a doc comment; this one does not.
— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
withholdFix re-recorded an already-withheld head's exclusion every eval cycle, producing the 6-in-10m repetition behind #14870 and contradicting auditWithheldReviewFixes' doc ("The planner reports each head once"). Now the plan entry is recorded once per head (when the State.Withheld entry is first created) and the doc comment describes that. Adds a regression test running the planner twice over the same head.
Supersedes #14886 (opened against v6 by mistake; v5 is the fix base and forward-merges into v6).
Closes #14882
Closes #14883
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88