Skip to content

Commit d3c12be

Browse files
Merge pull request #1582 from corbitsdev/cl-11823-post-one-human-readable-review-from-every-review-pass
feat(review): post one plain-language review
2 parents 5e1631f + 6dbd302 commit d3c12be

1 file changed

Lines changed: 18 additions & 11 deletions

File tree

  • plugins/corbits-skills/skills/review

‎plugins/corbits-skills/skills/review/SKILL.md‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,29 +52,36 @@ Deeper work (file-by-file behavior, architecture, coherence) can go to sub-agent
5252

5353
Raise what helps the author. Flag real defects and architecture that would constrain future work. Skip hypothetical problems in cases the code need not handle, stylistic nits, and vague remarks like "could be cleaner".
5454

55-
- Every finding has a `path:line` and a concrete failure mode. Severity is the review action, not adjectives.
55+
- Every finding is one of **blocking** (breaks the contract or the behavior), **should-fix** (a real defect that should change in this diff), or **informational** (true, but not something this diff has to change; the same rank as file-for-later). Severity is the review action, not an adjective.
56+
- Cite a line range, `path:start-end`, not a single line unless the failure is one line. A range is the lines a reader opens to see the failure.
5657
- Describe the branch as it stands now, in present tense, from `git diff <base>...HEAD`, not the journey or the PR description.
57-
- Write to a colleague: propose rather than command, say why, describe consequences instead of "wrong" or "broken", and suggest the one-liner when the fix is small.
58+
- Write the description for a person: what goes wrong, and the consequence. No finder jargon, no pass names.
5859
- **Cite the check.** Every affirmative claim ("tests pass", "no regressions") names the command or file range that proved it. Otherwise run the check, strike the claim, or narrow it to what you examined ("read `lock.go:42-68`, no acquire-while-holding cycle there").
5960

6061
## 6. Report and post
6162

62-
Summarize with the findings. When the target has an open GitHub PR, post the finished review on it; a review that only lives in chat is not done. Post when the operator asked for a PR review, when `/implement`'s whole-branch gate runs on a branch with an open PR, or when a record on the PR is the purpose. Do not post a private local read, and do not reopen the analysis while writing the body.
63+
Summarize with the findings. When the target has an open GitHub PR, post the finished review on it; a review that only lives in chat is not done. Post when the operator asked for a PR review, when `/implement`'s whole-branch gate runs on a branch with an open PR, or when a record on the PR is the purpose. Do not post a private local read. Do not implement fixes. Do not reopen the analysis while writing the body.
64+
65+
Wait until every pass Dispatch started has reported. Passes do not post to GitHub and do not approve, comment, or request changes.
66+
67+
Then post exactly one review with `gh pr review`. One verdict for the whole diff. Use one of `--approve`, `--comment`, or `--request-changes`, and swap that flag to match the verdict:
6368

6469
```bash
6570
gh pr review <number-or-url> --comment --body-file <file>
6671
```
6772

68-
Use `--approve` or `--request-changes` for the verdict. Shape:
69-
70-
```markdown
71-
## Review · <Approve | Comment | Request changes>
73+
The body is one short paragraph of what the branch does now, then a bullet list. Nothing else. Do not name the passes. Do not use finder jargon.
7274

73-
<one line: what the branch does>
75+
Each bullet is the severity, then the description, then the line range:
7476

75-
### Findings
77+
```markdown
78+
<what the branch does now, in plain language>
7679

77-
- `path/to/file.ts:12` — concrete problem and why it matters
80+
- **blocking** — <what goes wrong> `path/to/file.ts:12-18`
81+
- **should-fix** — <what goes wrong> `path/to/file.ts:40-44`
82+
- **informational** — <what is true, and why this diff need not change it> `path/to/file.ts:7`
7883
```
7984

80-
A clean review says "No findings." after the one line. No throat-clearing, praise to soften a finding, emoji, or "nit:" taste. When more than one lens produced a distinct judgment (reviewer, planner waiver, warden), each posts its own labelled review, and only the lens that owns the merge verdict may approve. Paste the review URL back to the operator. Posting does not change Linear state. Remove the PR worktree afterward.
85+
Request changes when any bullet is blocking or should-fix. Comment when every bullet is informational. Approve when there are no bullets, and say "No findings." after the paragraph. No throat-clearing, praise, emoji, or "nit:".
86+
87+
The note back to the operator is that same body plus the review URL. Do not write a second, more technical summary in chat. Posting does not change Linear state. Remove the PR worktree afterward.

0 commit comments

Comments
 (0)