Repository navigation
✨ feat(advice): say why the human-gated queue waits and how to help - #14495
hivecommons-hive[bot] merged 4 commits into
Conversation
The weekly owner advice's needs-human items said "59 of 94 open PRs are your turn (1 INSPECT, 24 DECIDE, 34 REVIEW)" and listed five items whose notes could run to a paragraph (a parked hive review prints every reviewer's summary). The verb split says what kind of action is owed, not why the PRs are stuck or who can unstick them. clear-human-gate-prs and unblock-human-issues now group the your-turn items by the stored ownership reason code, in plain words with where each group sits (Recommendation.Why), name the action for the largest groups (Recommendation.Help), and keep each item note to one clause. The digest and the dashboard card render one idea per line, with the raw signals last (folded away on the card). A dashboard test keeps every ownership reason code worded in the advisor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1 Signed-off-by: Danathar <Danathar@users.noreply.github.com>
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[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 |
kubestellar-prow
Bot
added
the
size/XL
hivecommons-hive
Bot
added
author/clanker
…ady says "REVIEW — Waiting on a person's review: an approving review is required by branch protection (0 given)" repeated "Waiting on a person's review" on most listed items. The note now keeps what follows it: "REVIEW — an approving review is required by branch protection (0 given)", "REVIEW — a reviewer requested changes". The inbox and the CSV keep the whole reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1 Signed-off-by: Danathar <Danathar@users.noreply.github.com>
hivecommons-hive
Bot
added
stage/072-returned-to-contributor
|
@Danathar this PR is back in your court ( Failing checks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Your relay session received this as a |
hivecommons-hive
Bot
added
stage/050-review-pending
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Read the full diff against v6 plus the producers of the reason strings it parses. The grouping/rendering is sound and the output is a clear improvement, but one string match misses the most common "changes requested" wording, which sends owners the opposite advice for those PRs.
Finding (medium) — src/pkg/hiveadvisor/human_gate.go:96
gateReasonFor splits review_decision_required on strings.Contains(reason, "requested changes"). The reason text comes from github.PullRequest.BranchProtectionBlockReason via ownership_gates.go:98 ("Waiting on a person's review: "+reason), and that producer has two spellings:
src/pkg/github/merge_block_reason.go:103—"changes requested by " + whowhenChangesRequestedByis populated (the normal case once the hub has reviewer data)src/pkg/github/merge_block_reason.go:105—"a reviewer requested changes"only as the fallback
Only the fallback contains "requested changes". A PR with changes requested by @alice falls through to the generic gateReasons["review_decision_required"] entry, so it is counted under "waiting for an approving review that branch protection requires" and the Help line says "approve PRs in …" — for a PR that actually needs the author to address review feedback. The test fixtures (human_gate_test.go:49) only use the fallback wording, which is why this passes.
Suggested fix: match both orderings, e.g.
case strings.Contains(reason, "requested changes"), strings.Contains(reason, "changes requested"):and add a fixture with changes requested by @someone so the /changes bucket is exercised on the common path. (The code-owner branch is fine: merge_block_reason.go:151,153 both emit code-owner.)
Verified OK
- Reason-code coverage: every key in
ownershipReasonVerbs(pkg/dashboard/ownership.go:81) has wording ingateReasons, and the newhive_advice_readable_test.gopins that; the dashboard test shards are green. whereText/topRepos/joinAndand themaxWhyGroups+1fold rule behave as the comments describe;copyRecommendationsnow deep-copiesWhy.index.htmlpasseswhy,helpand item notes throughescapeHtml; the raw signals moved into a<details>rather than being dropped.advisory.mdandapi-reference.mdwere updated for the new fields.- The red
test (agent 4/5)/coveragechecks areTestPollTmuxOutputForAgent_WiresThrashBreakerinpkg/agent, which this PR does not touch — that is the v6 breakage #14494 forward-ports the fix for, not something to chase here.
Note for whoever triages: the body cites #11507 as background but has no Closes/Refs issue; that's the hive's own call for a feature PR, flagged only so nobody expects an issue to auto-close.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read pkg/hiveadvisor, pkg/advisory/advisory.go:1458-1500, pkg/dashboard/ownership*.go and the renderHiveAdvice block in index.html at v6 392df48 alongside this diff. The change does what the body says and I found no defects. Looks correct to me.
Verified
humanGateWhy(owns, repos)indexesrepos[i]for eachowns[i]; both slices are appended together insummarizePRs(queue.go:329) andsummarizeIssues(:387), so they stay in lockstep. Grouping is by count only, so the later oldest-first sort ofyourTurnItemsdoes not affect it.- Every code in the new
gateReasonstable exists in the dashboard: the 25ownershipReasonVerbskeys (pkg/dashboard/ownership.go:81-106, enforced by the newTestHiveAdviceWordsEveryOwnershipReason) plus the review-hold (ownership_review_holds.go), gate (ownership_gates.go) and run/approval (ownership_runs.go) codes. Unknown codes fall through to"other/<first clause>"rather than being dropped. review_decision_requiredsplit: matches therequested changes/code-ownersubstrings thatHumanReviewBlocksMergereasons use; anything else keeps the generic wording.sig()takesany(hiveadvisor.go:273), so movingverbSummaryinto averbssignal is type-safe;copyRecommendationsdeep-copies the newWhyslice.- Renderer:
why,helpand item notes all pass throughescapeHtml(index.html:27303-27311); the Markdown writer only reorders existing content — item notes were already written raw before this change. shortOwnershipNotestrips only the leading"<clause>: "forreview_decision_requiredand otherwise truncates at a clause boundary or 99 runes +…; the full reason remains in the CSV export as the comment says.- docs:
advisory.mdand the/api/hive-advicerow inapi-reference.mddescribe the newwhy/helpfields; thepkg/dashboard/api.go:356citation is untouched and server.go is not edited, so the citation checker is unaffected.
CI
The red test (agent 4/5) / coverage jobs fail on pkg/agent TestPollTmuxOutputForAgent_WiresThrashBreaker (thrash_poller_wiring_test.go:110), which this PR does not touch — that is the v6 baseline failure #14494 is forward-porting the fix for. The hub shard that runs pkg/hiveadvisor, pkg/advisory and pkg/dashboard passed. Expect green once #14494 lands and this is rebased or re-run.
Note for the maintainer: no issue is linked; the body's before/after quote is the spec, and the diff matches it.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
hivecommons-hive
Bot
added
stage/072-returned-to-contributor
|
hive/shared-ci: the 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
hivecommons-hive
Bot
added
stage/060-reviewed
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
hivecommons-hive
Bot
added
stage/072-returned-to-contributor
|
Shared-CI incident #14506 is resolved: the 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
hive/awaiting-ci: branch synced with v6 (picks up #14494, which fixed the 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
The review-decision split matched only "requested changes", the fallback wording BranchProtectionBlockReason uses when it does not know who asked. When it does, the reason reads "changes requested by @alice", so those PRs were counted as waiting for an approval and the help line said "approve PRs in ..." for PRs whose author has review feedback to address. Match both wordings, and test every review-decision reason the producer can give. Found by the hive's reviewer-2 on this PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1 Signed-off-by: Danathar <Danathar@users.noreply.github.com>
There was a problem hiding this comment.
Read hiveadvisor/human_gate.go, queue.go, hiveadvisor.go and advisory.go writeAdviceSection at 9fa97a0; this looks correct to me.
humanGateWhy(human_gate.go:122-181): the group sort and themaxWhyGroupsfold (len(sorted) > maxWhyGroups+1) match the comment; everyhelptemplate containing%sends in" in %s", so theSprintf/TrimSuffixbranch cannot leave a stray verb.sig("verbs", verbSummary(...))is fine:sigtakesany(hiveadvisor.go:279).copyRecommendationscopies the newWhyslice (hiveadvisor.go:301), so the cached result is not aliased.
One wording note, not a defect: queue.go:512 says "the oldest are listed first", but yourTurnItems are ordered by IdleDays desc (queue.go:311, :369), i.e. longest idle, not oldest by creation. Same semantics as the previous "decide the oldest first" text, so no change needed unless you want "longest waiting".
go build ./pkg/hiveadvisor/ ./pkg/advisory/ ./pkg/dashboard/ passes locally.
— 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 v6 tree. Looks correct to me.
src/pkg/hiveadvisor/queue.go:329,387—yourTurnOwnandyourTurnReposare appended in the same statement, sohumanGateWhy'srepos[i]indexing (human_gate.go:138) is always in bounds.human_gate.go:115-125— unknown reason codes still group underother/<first clause>, so a newownershipReasonVerbscode degrades to a readable line rather than being dropped;hive_advice_readable_test.gopins the known set againstpkg/dashboard/ownership.go:81.static/index.html:27304-27311— every new interpolation (why,help,note) goes throughescapeHtml.hiveadvisor.go:301—copyRecommendationsdeep-copies the newWhyslice, so the frozen-week snapshot does not alias.advisory.go— signals moved to the end under "Numbers behind this"; still emitted, so nothing a digest consumer greps for is lost. Builds clean at 7c3b967.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the diff against the tree at 9fa97a0, including the producers of the strings it matches on. Looks correct to me.
- The
review_decision_requiredsplit inhuman_gate.go(gateReasonFor) keys on"changes requested","requested changes"and"code-owner"/"code owner". Those are exactly the reasonsmerge_block_reason.goproduces (:103"changes requested by " + who,:105"a reviewer requested changes",:151"… requires code-owner review"), wrapped byownership_gates.go:98as"Waiting on a person's review: …"— and the lowercase/Containsmatch survives that prefix. gateReasonscovers everyOwnershipReason*constant value inpkg/dashboard(I diffed the two sets: no code on either side is missing), andTestHiveAdviceWordsEveryOwnershipReasonkeeps that true in CI.yourTurnOwn/yourTurnReposare appended in the same branch asyourTurnItems(queue.go:322-330,:380-388), so the parallel sliceshumanGateWhyindexes stay aligned.go build ./pkg/hiveadvisor/ ./pkg/advisory/ ./pkg/dashboard/is clean at this head. Body matches the diff (why / how-to-help / first list, signals folded under "Numbers behind this").
One non-blocking observation: gateReasonFor groups unknown codes by "other/" + firstClause(o.Reason), so a future code with a per-item variable reason would fan out into one group per item rather than one group per code. The dashboard test above means it cannot happen silently, so I would not change anything here.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
Summary
The weekly owner advice's needs-human item was hard to read. On the projectbluefin hive it said:
The verb split says what kind of action is owed, not why the PRs are stuck or who can unstick them, and one item's note ran to a paragraph.
With this change, the same queue (the 59 your-turn PRs read from that hive on 2026-10-27, through the new code) renders in the digest as:
The dashboard card shows the same parts in the same order, with the numbers folded into a "Numbers behind this" disclosure. Issues ("Unblock the human queue") get the same treatment.
Change
pkg/hiveadvisorRecommendation.Why): the your-turn items are grouped by the ownership reason code the dashboard already stored for each one (📝 spec: [v6] dashboard "your turn" signal — ☐ marker, inbox and one server-side ownership decision (from RFC #11504) #11507), worded in plain language, largest group first, each with where it sits (whereText: the three largest repositories, then "N elsewhere").review_decision_requiredsplits three ways, because each needs a different person: a reviewer's requested changes (either wordingBranchProtectionBlockReasongives: "changes requested by @x" or "a reviewer requested changes"), a code owner's review, or approvals branch protection requires. The five largest groups are named; any more fold into one "N more for other reasons" line (a single leftover group is shown instead).Recommendation.Help): the action for the three largest groups. It names repositories only when two of them hold most of the group, so a group spread across five repos doesn't send people to the first two alphabetically.shortOwnershipNote): verb plus reason, cut to the reason's first clause past 100 characters ("Hive's review is parked for a person"), or truncated with "…" when there is no clause. A review-decision note drops "Waiting on a person's review:", which REVIEW already says ("REVIEW — a reviewer requested changes").NeedsHumanItems, which the inbox reads, keeps the full reason.verbs.DescribesReasonCodelets the dashboard check that every reason code it can produce has wording. A code without wording still groups, under its own reason text.Renderers
advisory.writeAdviceSection): one idea per line. The headline is followed by "Why they wait:" sub-items, "How to help:", "First:" with one item per line, "Full list:", and last "Numbers behind this:" (signals used to trail the headline in parentheses). This applies to every recommendation, not only the needs-human ones.renderHiveAdvice): the same order. Why and items are lists, help is one line, and signals sit in a closed<details>. The new classes use the existing spacing and type tokens.Docs
src/docs/advisory.mddescribes Why, How to help, the short item note and "Numbers behind this".src/docs/api-reference.mdlists thewhyandhelpfields.Testing
From
src/under/var/tmp,-count=1,TZ=UTC:go test ./pkg/hiveadvisor/... ./pkg/advisory/... ./pkg/dashboard/...andgo test ./cmd/hive/ -run 'Advis|Advice': allok(hiveadvisor, advisory, dashboard, dashboard/collect, dashboard/theme, dashboard/webstatic, cmd/hive).go build ./pkg/... ./cmd/...succeeds;go vetandgofmtare clean on the changed packages.check-docs-links.py,check-docs-citations.pyandcheck-changelog-fragment.sh v6pass.New tests:
TestHumanGateAdviceSaysWhyAndHowToHelpTestHumanGateWhyFoldsSmallGroupsTestHumanGateWhyShowsASingleLeftoverGroupTestHumanGateHelpNamesReposOnlyWhereMostItemsAreTestHumanGateWhySplitsEveryReviewDecisionWordingBranchProtectionBlockReasongives a review decision lands in its group: both requested-changes wordings, both code-owner wordings, the approval count.TestHumanGateWhyWordsUnknownCodesFromTheirReasonTestWhereTextNamesTheLargestRepos,TestShortOwnershipNoteCutsAClauselessReasonTestAdviceSectionRendersWhyAndHowToHelp(pkg/advisory)TestHiveAdviceWordsEveryOwnershipReason(pkg/dashboard)ownershipReasonVerbshas wording in the advisor.TestHiveAdviceCardReadsAsWhyHelpAndItems(pkg/dashboard)renderHiveAdvicein node: section order, list markup, help line, folded signals, cleared advice unchanged.Three existing tests pinned the old wording and now check the new headline, the
verbssignal, the Why lines for the issue scenario and the one-item-per-line digest format.Mutation checks
Each part was broken in turn and restored from a backup:
Not verified
review_queuetool, not on a running dashboard.TestAgentClosedFindingIsNotReportedResolved(pkg/advisory, untouched) fails when the local date differs from the UTC date, which is why the runs above setTZ=UTCas CI does. It fails the same way on v6 without this change.Contributor checklist
changelog.d/changed-readable-human-gate-advice.md.v6.src/docs/advisory.md,src/docs/api-reference.md).🤖 Generated with Claude Code
https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1