diff --git a/CLAUDE.md b/CLAUDE.md index 1380c927..f305b226 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,6 +76,7 @@ every project it is installed in. | PR body | marker blocks, one owner each: status · problem · solution · ac · history · visual · risk | | Ticket | Problem · Context · Fix · Acceptance Criteria · Risk · Blocked by (+ Evidence for bugs) | | Comment | `[role] [label] status` · Did · ✅ Done · ❌ Not done · Next · ≤ 8 lines | +| PR author notes | Purpose · What changed · Why it matters, in native file/inline review comments; follow `skills/super-board/references/pr-author-notes.md` | - NEVER rewrite a whole PR body: `scripts/super-board-pr-body.sh` rewrites one block. - ALWAYS change a format in `writing-standard.md` first, then the templates and `tests/test_writing_format.py`. diff --git a/skills/super-board/references/agents-md-block.md b/skills/super-board/references/agents-md-block.md index 0fc48670..2f93808a 100644 --- a/skills/super-board/references/agents-md-block.md +++ b/skills/super-board/references/agents-md-block.md @@ -33,7 +33,8 @@ Board: Backlog · Ready · Building · QA · Review · Blocked · Done. Labels r | PR body | blocks: status · Problem · Solution · AC + proof · history · Before\|After · Risk — `super-board-pr-body.sh` | | Ticket | Problem · Context · Fix · AC · Risk · Blocked by → `docs/agents/issue-tracker.md` | | Comment | `[role] [label] status` · Did · ✅ Done · ❌ Not done · Next · ≤ 8 lines | +| PR author notes | file + critical inline review comments: Purpose · What changed · Why it matters → `pr-author-notes.md` | - NEVER chain steps with arrows. One step per line, lettered under Where. - NEVER link screenshots. Embed a raw URL pinned to a sha. -- DON'T list files in comments. DON'T write "Not verified" or "Next" in a PR body. +- DON'T repeat file lists in status comments. DON'T write "Not verified" or "Next" in a PR body. diff --git a/skills/super-board/references/pr-author-notes.md b/skills/super-board/references/pr-author-notes.md new file mode 100644 index 00000000..8723f33d --- /dev/null +++ b/skills/super-board/references/pr-author-notes.md @@ -0,0 +1,88 @@ +# PR author notes + +Make each changed file understandable in GitHub's **Files changed** view. Use the +**Purpose / What changed / Why it matters** format in [writing-standard.md](writing-standard.md#author-notes-on-a-pr). +Keep each label to one short sentence. Describe what the file does before its details. + +## When to write them + +The PR author posts notes after opening a PR, including a partial draft, and refreshes +them after each push. The lane that changes code owns the refresh for its changed files; +it also checks that every file in the current PR has a summary. This includes QA fixes +and UI-refine PRs. Do it before the lane handoff. No new setup question is needed. + +Write one native file-level comment per changed file. Add only a few inline comments +where a reader needs the reason behind a critical change. Both use the three labels. +Tests explain what behavior they protect; scripts explain when they run; config and docs +explain who follows the changed rule. Do not edit source files just to add these notes. + +## Read before writing + +1. Read the PR's repository, base and full current head SHA. Fetch the matching diff and + all pages of changed files and review comments, including replies. Confirm that the + local code you inspect matches that head. A failed or incomplete read is not an empty + list: stop posting and report the missing read. +2. Draft notes from that diff and the file's actual job. Use only paths on the current PR. + For inline notes, choose a line or small range in the current diff, with the correct + old/new side. Never guess line numbers from an earlier checkout. +3. Match existing author notes by path, subject type and stable marker key. Only change + notes whose author is the current authenticated identity and whose marker identifies + this workflow's note. Preserve all other comments, including human edits and replies; + a marker by itself does not prove ownership. If ownership or an edit is unclear, leave + it intact and report the conflict instead of creating a second copy. +4. Re-read the head before writing. If it changed, discard the draft anchors and start + from the new diff. Follow the existing GitHub quota and halt rules. After posting, + read back the notes and head to confirm what actually appeared. + +## Use GitHub review comments + +Use the [review-comment API](https://docs.github.com/en/rest/pulls/comments#create-a-review-comment-for-a-pull-request): +`POST /repos/{owner}/{repo}/pulls/{pull_number}/comments`. +Send `body`, `path` and the full current `commit_id` for every new note. + +- File summary: set `subject_type` to `file`; omit line numbers. Do not attach a fake + line-1 comment to stand in for a whole-file note. +- Critical inline note: omit `subject_type` and send `line` and `side`. Use `LEFT` + for a deleted line and `RIGHT` for an added line. Add `start_line` and `start_side` + only for a range that the current diff supports. +- Write the JSON payload to a file and pass it to `gh api --input `. Keep prose + out of shell interpolation. A review submission must use `event: COMMENT`, never + `APPROVE` or `REQUEST_CHANGES`. Direct review comments do not grant approval. + +GitHub's [review API](https://docs.github.com/en/rest/pulls/reviews#create-a-review-for-a-pull-request) +can group line comments in a `COMMENT` review. Its documented `comments` entries do not +include `subject_type`; use the individual review-comment endpoint for file summaries. +Do not submit an existing pending review that may contain someone else's draft comments. + +## Refresh without noise + +- Same note, still true at a valid current anchor: leave it alone. Do not publish one + copy per lane, polling pass or commit. +- Changed explanation, same valid anchor: PATCH only the owned comment's `body` using + `/repos/{owner}/{repo}/pulls/comments/{comment_id}`. Keep its marker/key. PATCH cannot + move a comment to another line or commit. +- Outdated anchor, removed file or renamed path: preserve the old discussion. Mark only + an owned, unedited note as superseded, and link to its replacement when one is needed + on a current changed file. Never delete a thread or resolve human replies to tidy up. +- Timeout or uncertain write result: read the comments again before doing anything else. + Match the intended marker, path, anchor and body. If its outcome is still unknown, stop + and report it. Never blindly replay a create or submit request. + +Generated files, vendored files and binary assets still need useful context. Say what +produced or changed them, with only the details you can verify. If GitHub cannot accept +a file comment, use one clearly marked fallback PR comment covering the affected paths +with the same three labels and the reason. Reuse that owned fallback on later runs. +Do not invent inline anchors for files with no visible diff or silently skip them. + +## Handoff and review + +Report the confirmed number of file notes and inline notes, plus any grouped fallback +or failed note. If posting fails, keep the PR draft (or leave an existing PR's state +unchanged), preserve the work, and report the incomplete handoff. Do not claim notes +were posted or move the card forward as though the handoff finished. + +The Reviewer reads the code before these author explanations. A note by itself is not +a defect finding and must not cause a rebuild or count as approval. Read its replies: +a human question or change request still follows the normal review/blocking flow. +Never exempt a whole thread because its first comment has the author-note marker. +Do not auto-resolve these conversations or bypass GitHub's conversation-resolution rules. diff --git a/skills/super-board/references/run.md b/skills/super-board/references/run.md index 4b703868..f96b303c 100644 --- a/skills/super-board/references/run.md +++ b/skills/super-board/references/run.md @@ -198,7 +198,7 @@ check in `risk` too. ## PR review-comment threads — prefix + resolution protocol -Reviewer always uses **line-level review comments** (resolvable threads). Every thread MUST start with the lane that owns the fix — `[builder]`, `[qa]` or `[reviewer]` — then a label (`[blocker]`, `[issue]`, `[suggestion]`, `[nit]`, `[question]`, `[praise]`; writing-standard.md § 4). Read `[QA]` and `[review]` on older PRs as `[qa]` and `[reviewer]`. +Reviewer findings use **line-level review comments** (resolvable threads). Each finding MUST start with the lane that owns the fix — `[builder]`, `[qa]` or `[reviewer]` — then a label (`[blocker]`, `[issue]`, `[suggestion]`, `[nit]`, `[question]`, `[praise]`; writing-standard.md § 4). Read `[QA]` and `[review]` on older PRs as `[qa]` and `[reviewer]`. [PR author notes](pr-author-notes.md) have their own format; read all replies before treating a thread as explanation only. Examples: @@ -213,7 +213,7 @@ e2e/streaming/ttfb.spec.ts:18 [qa] [issue] spec asserts status only — add a |---|---|---| | Builder (Building → QA) | All `[builder]` threads on this PR | Stay in Building, fix, then exit | | Tester (QA → Review) | All `[qa]` threads on this PR | Stay in QA, fix, then exit | -| Reviewer (approving merge) | ALL threads on this PR | Bounce: `[builder]` open → Ready; `[qa]` open → QA | +| Reviewer (approving merge) | All findings and human requests; any conversations GitHub requires resolved | Bounce: `[builder]` open → Ready; `[qa]` open → QA; unresolved human decision → Blocked | Threads are resolved via `gh api graphql` `resolveReviewThread` mutation when the fix is committed. @@ -284,7 +284,7 @@ column), never built unchecked. `qa` cards are not pre-flighted: nothing is buil move the card to Blocked `❓` — never push on past the cap (super-build → "Keep each PR small"). 5. Commit + push (always). Commits follow writing-standard.md § 1 (`✨ [feat] chat: stream replies` + short bullets). 6. Open draft PR linked to the issue with the PR description template (title in commit-subject format, every block filled). -7. Post the `[builder] [report]` PR comment (see "Commenting cadence"). +7. Follow [PR author notes](pr-author-notes.md), then post the `[builder] [report]` PR comment (see "Commenting cadence"). 8. Post a short status comment on the issue with the PR URL. 9. Clean up worktree. Keep branch + PR open. 10. Move card Building → QA. @@ -298,7 +298,7 @@ column), never built unchecked. `qa` cards are not pre-flighted: nothing is buil (A Reviewer bounce reaches you as those same `[builder]` threads, listed in the latest `` comment. A finding the Reviewer re-opened as `not fixed` was resolved without a fix last time — fix the code, not just the thread.) 5. Commit + push to same branch. 6. Verify ALL `[builder]` threads are resolved. If not, return to step 3. -7. Rewrite `status`, `solution`, `history` blocks; post `[builder] [report]` PR + issue comments. Move Building → QA. Clean up worktree. +7. Refresh [PR author notes](pr-author-notes.md), rewrite `status`, `solution`, `history` blocks; post `[builder] [report]` PR + issue comments. Move Building → QA. Clean up worktree. ### Tester (first pass — repo-backed) @@ -347,7 +347,11 @@ If a screenshot file is >5MB, downscale to ≤1920px wide before committing; Git ### Reviewer 1. Worktree `.claude/worktrees/issue--review/` from current state of `issue--`. -2. **Gate 1** — scan PR threads. If ANY unresolved: +2. **Gate 1** — scan PR threads and their replies. [PR author notes](pr-author-notes.md) + alone are explanations, not findings. Human questions or change requests in those + threads still follow the normal review/blocking flow; the marker exempts no replies. + Never auto-resolve them or bypass GitHub's conversation-resolution requirements. + If ANY unresolved finding: - `[builder]` open → comment, move card Review → Ready. - `[qa]` open → comment, move card Review → QA. - Both open → bounce to whichever is older; the other gets picked up later. @@ -582,7 +586,8 @@ Claim uses a **GitHub Issue assignee mutex** — atomic compare-and-set via `gh │ move card to Blocked, continue with next card. └─ Present → proceed. 3. Do the lane's work (build / QA / review). -4. Comment evidence on issue + PR (writing-standard.md § 4) and rewrite your PR body blocks. +4. If this lane opened a PR or pushed changes, refresh [PR author notes](pr-author-notes.md). + Comment evidence on issue + PR (writing-standard.md § 4) and rewrite your PR body blocks. 5. Move card to next column (or Blocked with the full §4 template; dropped on purpose → closed, Done, 🤷 comment). 6. RELEASE CLAIM (`gh issue edit --remove-assignee super-board-bot[bot]`) and remove descriptive label. ``` diff --git a/skills/super-board/references/writing-standard.md b/skills/super-board/references/writing-standard.md index 17973968..7480335c 100644 --- a/skills/super-board/references/writing-standard.md +++ b/skills/super-board/references/writing-standard.md @@ -15,7 +15,8 @@ Every template in this pack points here. Change the format here first, then the commit sha. NEVER link to them, NEVER upload to a public image host. - NEVER write a "Not verified" section or a "Next" section in a PR body. An unchecked AC with its one-line reason replaces both. -- NEVER list changed files in a comment. GitHub already shows them. +- DON'T repeat a changed-file list in status comments. Explain each file in its own PR + review comment, using "Author notes" below. - DON'T restate the ticket. DON'T narrate ("I have successfully…"). ## 1 · Commit @@ -273,6 +274,31 @@ refuse a body that misses a required section. ## 4 · Comment +### Author notes on a PR + +On every PR opened by this pack, add a native **file-level review comment** for each changed +file. Use the same three labels, one short sentence each, in plain English: + +```markdown + +**Purpose:** This file checks whether a code change may be merged. +**What changed:** It asks for fresh approval if the AI edits the code after approval. +**Why it matters:** Your earlier approval cannot allow code you have not reviewed. +``` + +Add a few **inline review comments** at important changed sections using the same labels. +Explain a safety check, a hard-to-see choice, or a changed behavior; skip obvious lines. +Use a stable key for each inline topic, such as `key=fresh-approval`, instead of +`key=file-summary`. The key and path identify the note on later runs. + +These notes live on the pull request, never as comments inserted into source files. They +explain the author's work; they are not reviewer findings, approval, or proof a test passed. +They use the three-label format instead of the lane-status format below. Do not copy the +PR description into every note. See [PR author notes](pr-author-notes.md) for when to post, +how to use GitHub's file and line comments, and how to refresh notes without duplicates. + +### Lane status and findings + ``` [] [