feat(review): exclude the parallel reviewer from the prompt - #38
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # Matched by name against the app slug rather than by a second constant: a GitHub App's bot | ||
| # login is its slug plus "[bot]", and its checks are the slug and slug-prefixed names, so | ||
| # $skip still carries the one value all five programs agree on. Both rollup shapes are | ||
| # matched -- a CheckRun by .name, a StatusContext by .context -- because which of the two an | ||
| # app posts is the app's choice, not ours. | ||
| # | ||
| # Silent while other checks remain, and stated when the exclusion empties the block: a list | ||
| # claims nothing about being every check, but "No checks reported." on a PR that has some is | ||
| # a false claim, and this is the block whose emptiness the README warns gets read as green. | ||
| # One definition of "belongs to the other reviewer", composed into both programs below rather |
There was a problem hiding this comment.
super nit: (not blocking) This paragraph now describes the rule the next one replaces, and the two run together.
Lines 562-566 still say the match is "by name … a CheckRun by .name, a StatusContext by .context", which is exactly the name-only, case-sensitive rule cycle 2 established is not enough — a maintainer stopping here reads the current theirs as narrower than it is, and reads it as complete. Lines 577-585 say the true rule; the earlier paragraph is the superseded draft of it. Worth folding the two into one, or at least amending 565-566 to "both names on the entry, downcased".
Also missing the # separator between "…gets read as green." (570) and "One definition of…" (571), so the silent/stated asymmetry and the shared-owner-test rationale render as one paragraph. Every other break in this block has one.
Same class: the block says "all five programs agree on" $skip while CHECKS_JQ and FAILING_JOBS_JQ now reach it through CHECK_OWNER_JQ, and :302 still says "three programs below" (as does extract_const's comment in tests/lib.sh:45). The README's removal list is the one that has to be right and it is; these are just counts that stopped counting.
There was a problem hiding this comment.
Cycle 2's two gaps are closed and pinned. CHECK_OWNER_JQ is one rule composed into both check programs, matching .name/.context and .workflowName downcased, with the . != "" guard keeping an empty slug from matching everything; the {name: "review", workflowName: "Pullfrog"} and Pullfrog-Approval fixture entries cover both shapes that leaked before, and the failing-job assertion covers the log-fetch arm separately. reviews-drift-with-other-bot.json pins the case the exclusion makes ordinary — a drifted login beside the excluded one still fires. The orphaned-reply relabel and its fixture reply (2105 → 2103) do what the thread asked. Verified the composed programs against the fixtures by reading them; CI was still queued at review time, so I am not claiming the suite result.
One super nit inline, comments only.
1 participant
Keeps Pullfrog's review output out of Claude's prompt for the comparison trial (#37 adds the Pullfrog workflow): its inline comments and review body reach
<prior_review_comments>and the PR conversation through two author-unfiltered reads, which costs the comparison its independence and costs the diff blocks the bytes.Also excludes it from the review-cycle drift predicate, which would otherwise report a reviewer-identity change on the first review of every PR in a trial repo. Must merge before #37.