greyson/playpen
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Build in the review tool.

Browse Source
This commit is contained in:
Greyson Parrelli
2026-08-24 09:24:01 -04:00
parent 4eef6a28d3
commit 407dd81512
55 changed files with 12932 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.claude/skills/leave-review/SKILL.md
+234
View File
@@ -0,0 +1,234 @@
---
name: leave-review
description: Review the diff open in this playpen tab's review pane and leave your own line-level comments there (as Claude), instead of printing a terminal report. Can delegate the analysis to another review skill — pr-review, security-review, or any project-specific reviewer — and post its findings as inline comments. Use when the user says "review the diff", "leave a review", "review my changes in the pane", "review this with <skill>", or runs /leave-review.
---
# Leave a review in this tab's review pane
Playpen serves a GitHub-style review UI for the repository the current tab is
working in. Normally the user leaves comments there and an agent addresses them —
the `address-review` skill. This skill runs the loop the other way: **you** review
the diff on screen and leave the comments, anchored to real lines, so the user
reads them in the same pane they'd read a colleague's review in — reply, resolve,
or hand them back to be fixed.
**This skill reviews. It does not fix.** Do not edit a single file while running
it, even for something trivial or obviously right — a review whose findings have
already been silently applied is impossible to read. Fixing is `address-review`'s
job, and your comments land in its queue automatically (see step 7).
## 1. Set the target
```bash
BASE="$PLAYPEN_REVIEW_URL" # e.g. http://127.0.0.1:8420/t/tab3
curl -s "$BASE/api/repo"
```
`$PLAYPEN_REVIEW_URL` is exported into every terminal pane and names **this
tab's** review, so there is nothing to choose and no way to leave your review on
someone else's branch.
- `{"open":true,…}` with a `context` object — good, go to step 2.
- `{"open":false}` or a 409 — this tab has no review pane. Tell the user to open
one (**Ctrl+Shift+D**) and stop. Don't review a different tab.
- `$PLAYPEN_REVIEW_URL` empty, or connection refused — list the tabs with
`curl -s http://127.0.0.1:8420/api/tabs` and match a tab's `path` against
`git rev-parse --show-toplevel`. If nothing matches, stop and say so.
## 2. Find out which diff to review
The base ref, the **uncommitted** toggle, and any single commit picked out of the
range are chosen in the pane, and the pane publishes that selection to the
server. It's the `context` field from step 1:
```json
"context": { "base": "main", "uncommitted": true, "commit": "" }
```
Review **that** diff. It's what the user is looking at, and it's the only diff
whose line numbers the pane can place a comment on.
- `"context": null` — nothing has been on screen yet. Ask the user to open the
review pane, then re-read it. Don't guess a base.
- A non-empty `"commit"` means they're reading one commit of the range on its
own. Review that commit; the numbers only mean anything there.
- If the user asks for a different diff ("review just the last commit"), you can
pass your own `base`/`uncommitted`/`commit` — but say plainly that those
comments will only appear once they switch the pane to match.
## 3. Get the diff
```bash
curl -s "$BASE/api/diff?base=main&uncommitted=true"
```
The `patch` field is the exact bytes the pane renders, and `files` is the
per-file summary — read that first, for scale. `commits` lists the commits the
range spans, so a user asking about "the second commit" can be answered by
re-fetching with `&commit=<sha>`.
Use this rather than running `git diff` yourself: identical bytes means identical
line numbers, which is what makes an anchor land.
A very large change set comes back with `"oversized": true` and no patch. Add
`&force=1` to get it anyway, or narrow to one commit.
Read the surrounding code with your normal tools — the patch alone is rarely
enough to tell a real bug from a fine one. If the patch is empty, say so and stop.
## 4. Pick the lens — and delegate when there's a skill for it
This skill is the delivery mechanism; the *judgment* can come from a skill that
already knows this codebase. Before reviewing anything yourself, check the skills
available to you for one that fits this repo or what the user asked for:
- **The user named one** — "review this with pr-review", "do a security review",
"/leave-review pr-review". Invoke it with the Skill tool. Their choice wins.
- **A project-specific reviewer exists** for this repo (e.g. `pr-review` for
Signal-Android conventions) — prefer it over your own generic pass, and say
which one you used.
- **Several apply** — run them in turn (conventions pass, then security pass) and
merge the findings, dropping duplicates.
- **None fits** — review it yourself against the rubric below.
When you invoke a review skill, follow its instructions as written, but note up
front that its findings are going to be posted as inline comments rather than
printed — so you need, for each finding, a **file path and a line number in this
diff**, plus the rule name it fired. Then continue at step 5 with its report as
your finding list. If it produces a terminal report anyway, that's fine: its
numbered findings are exactly the input you need.
Two limits:
- **Only delegate to skills that review.** Some skills change code as part of
their job (`simplify`, for instance, applies its own fixes). Don't invoke one of
those here — it would edit the diff out from under the review. If the user asked
for one by name, say why you're not running it and offer its rubric as a lens
instead.
- **If a delegate doesn't apply** (a Signal-Android reviewer in an unrelated repo,
say) don't force it. Fall back to your own pass and mention the swap.
### Rubric for your own pass
Restrict findings to **added and modified lines** — don't review code the diff
didn't touch, unless the change made it newly wrong. In rough priority order:
1. **Correctness** — logic that doesn't do what the code around it clearly
intends; off-by-one, inverted condition, wrong variable, missed case.
2. **Error handling** — swallowed errors, unchecked returns, an error path that
leaves state half-updated, panics on input the caller controls.
3. **Resources and lifetimes** — leaks, missing close/cancel, work that outlives
what it belongs to.
4. **Concurrency** — data races, state mutated without the lock its neighbours
take, deadlock ordering.
5. **Interface and contract** — a caller that can now be silently wrong; a
behaviour change not reflected in the doc comment right above it.
6. **Tests** — a new branch with real failure modes and no test; a test that
would pass with the bug still in.
7. **Fit** — code that ignores an existing helper, layering, or naming pattern
the file establishes.
8. **Cruft** — commented-out code, narration comments, debug logging, a stray
TODO with no owner.
## 5. Turn findings into comments
Choose the anchor per finding, most specific that will actually render:
| Finding | Anchor |
| --- | --- |
| A specific line | `"level":"line"`, `file`, `line` |
| A construct spanning lines | add `endLine` |
| A deleted line (the change removed something needed) | add `"side":"old"` |
| The file as a whole, or a line not in the patch | `"level":"file"`, `file` |
| Cross-cutting: architecture, a missing test file, the change set as a whole | `"level":"review"` |
**Verify every line number against the patch before you post it.** Read the hunk
header — `@@ -old,n +new,m @@` — and count: the number you pass must be a line the
patch actually shows on that side (an added `+` line or a context line). A comment
on a line the pane doesn't render is invisible; when in doubt, widen to a range or
drop to a file-level comment. Never post a line number you inferred from your
memory of the file.
Three details that decide whether a comment lands where you meant:
- `file` takes the path as the diff names it: the **new** path, or the old one
for a deleted file.
- A range hangs its thread off the **end** line, GitHub-style. Put the range
around the construct and let it anchor at the bottom.
- Prefer `"side":"new"`. Old-side line numbers are positions in the base
revision, so they stop meaning anything the moment the user changes the base
ref, and the comment goes outdated. Only use `old` when the finding really is
about a line the change deleted.
**Don't repeat what's already been said.** Fetch the existing threads first:
```bash
curl -s "$BASE/api/comments"
```
Skip anything the user already raised, and anything **you** raised on an earlier
pass — including threads they resolved. Re-posting a resolved finding is the
fastest way to make this skill not worth running twice.
Write each body like a comment in a thread someone has to read:
- Lead with what's wrong, in one sentence. Then why it matters, then the fix.
- 2–4 sentences. Show the fix as code when that's shorter than describing it.
- Open with a severity label so the rail can be triaged: `Blocking:`,
`Should fix:`, `Nit:`, or `Question:`.
- When a delegate skill produced it, name the rule at the end in parentheses —
e.g. `(pr-review: LogTagInlined)` — so the user can trace it.
- **Bodies render as plain text, not markdown.** Newlines and indentation are
preserved, so an indented line or two is how you show suggested code. Backticks
around an identifier read fine; `**bold**` and ``` fences just show up as
punctuation, so skip them.
- No praise-only comments, no restating what the diff does, no "consider possibly
maybe". One issue per comment; group unrelated nits in one file-level comment
rather than five line comments.
- Aim for **at most ~10–12 comments**. Past that, keep the serious ones and roll
the tail into a single review-level comment. A wall of comments reads as noise
and buries the two that mattered.
## 6. Post them
```bash
curl -s -X POST "$BASE/api/comments" -H 'Content-Type: application/json' -d '{
"level":"line","file":"src/review/Store.zig","side":"new","line":84,"endLine":91,
"author":"claude",
"body":"Should fix: save() runs while the write lock is held, so a slow disk blocks every reader for the length of the write. Snapshot the list under the lock and write outside it."
}'
```
- `base`, `uncommitted` and `commit` default to the selection the pane published,
so leave them out unless you deliberately reviewed another diff.
- `"author":"claude"` is the point: the comment shows up as **Claude**'s in the
pane, and lands as an **open thread** rather than a draft of the user's. Drafts
are theirs alone — never post as `"author":"user"`.
- Post the most serious findings first — the comments rail is ordered by creation.
- Finish with **one review-level comment** as the review body: what you reviewed
(base ref, file count), which lens you used, and the two or three themes worth
the user's attention. Without it they can't tell what was checked versus what
came up clean.
Everything appears in the pane immediately over its live connection.
## 7. Summarize in the terminal
Report: the lens used, how many comments you left and where (`file:line — one-line
summary` each), and anything you deliberately didn't comment on. Then tell them
the two ways forward:
- Read them in the pane — reply, or resolve the ones they disagree with.
- Or say **"address the review"** — your comments are open threads, so they're in
the same queue the user's comments go into, and `address-review` will fix them
the same way. Mention this: it's the whole loop, and it's not obvious that your
own findings come back to you as work.
## Notes
- Your comments count toward the review's **open** count, the same as the user's —
it means "threads awaiting someone", not "awaiting Claude".
- If the user edits code after you review, your anchors drift. That's expected;
they hit **↻ Refresh diff** and the next pass re-anchors.
- Reviewing a large diff: work file group by file group and post as you go, so a
long pass still leaves usable comments if it's interrupted.