Repository navigation
Read the whole change in one review, and fail a review that did not - #20
Conversation
Reviews of the two largest pull requests in quaffed came back partial: each said it had split the change into parallel passes that had not reported back, and wrote up only what it had read itself. The session is not allowed to start other agents, so those passes cannot have run; both runs logged refused tool calls, though without full output which calls they were is not shown. Agent and Workflow are now disallowed, so the session is not offered them and reads the change itself, and the prompt says so. A partial review still posted "approve", so the review check was green over files nobody reviewed. The structured output now carries `complete`, and a review that did not cover every file in the range is posted headed "incomplete", with the verdict on what it read, and fails the run with no marker, so the next request reviews the range again. A missing field counts as incomplete, since it cannot be told apart. Tests run the posting step as it ships against a stand-in for gh, and read the session's arguments; four of the five fail against the old workflow. Signed-off-by: Gavin Lucas <Gavin@L337.org>
Claude review: changes requestedFindings1. Bug: an incomplete review's body can be read as a marker, so a range nobody finished reviewing can count as reviewed. What goes wrong:
The paused marker has the same problem, and it is likely in this repository specifically. The "did not complete" comment already has the same exposure through the Suggested fix: accept a marker only at the end of the body, e.g. 2. Bug: incomplete reviews are not counted towards
Suggested fix: put a distinct sentinel on incomplete comments, e.g. 3. Maintainability: the README says more than the change does about tools that reach other agents. This session runs the pull request's own version of the review, and it is still offered:
None of them is in 4. Maintainability (recorded state): two added lines restate the job timeout.
Both repeat 5. Preference: the incomplete comment always suggests What I checked
What I did not check
full review of |
…e rounds An incomplete review carried no marker of its own, so the parser, which took the last marker anywhere in a comment, would read one quoted in its text as the review's: a quoted marker for the head would answer the next request "already reviewed", and a quoted paused marker would count as a notice already posted, so automatic reviews would stop silently. The review text comes from a session the pull request's author can steer. A reviewed or paused marker now counts only at the very end of a comment, where this workflow writes it. An incomplete review also escaped the bound on automatic rounds, which counted only marked reviews, so a large pull request that never finishes would be reviewed in full on every push. It now ends with its own marker, which counts push-triggered ones towards the bound and moves no range. The README names the two tools disallowed rather than all tools that reach other agents, the job's timeout is no longer restated in the prompt or a comment, and the incomplete notice offers a delta request as well as a full one. Tests run the range step as it ships against stand-ins for gh and git; four of the new ones fail against the previous commit. Signed-off-by: Gavin Lucas <Gavin@L337.org>
|
All five points are fixed in cbc1f5d. Both bugs were confirmed in the parser before fixing.
Tests.
|
Claude review: approveResolved
Still open Declined New
What I checked: the whole delta across the workflow, the README and the tests, and the range step around it. I did not run the tests or actionlint, so I am relying on the author's report of 100 passing. delta review of |
Why
Full reviews of the two largest pull requests in L337-org/quaffed (#54 and #55) came back partial, and both posted "approve":
show_full_output, so which calls those were is not shown.The
review / Reviewcheck was green both times, over files nobody had reviewed.What changes
No helper agents.
AgentandWorkflowjoin--disallowedTools, so the session is not offered them. The prompt says to read the whole change itself, within the job's hour.@anthropic-ai/claude-agent-sdk@0.3.304, which the pinned action installs: the subagent tool isAgent, the olderTaskis an alias for it, andWorkflowis the multi-agent tool.An incomplete review fails. The structured output gains a required
complete. When it is nottrue, the review is still posted, but:Claude review: incomplete, with the verdict on what it read.A missing field counts as incomplete, since it cannot be told apart from a skipped part.
The README's review section and its description of this repository's CI are updated.
Tested
tests/test_claude_review.py. They run the posting step as it ships against a stand-in forgh, and read the session's arguments:AgentandWorkfloware disallowed, andcompleteis required in the schema.main.Not tested
/claude-review fulloncev2points at the release.Release
After merge, the owner publishes v2.0.2 and moves
v2. No caller needs a change.