WIP: chore(agents): /review, a skill for reviewing a pull request, and for a cold review of a branch #891
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/tooling
area/upscaling
ci: approved
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
risk
high
risk
low
risk
medium
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!891
Loading…
Reference in a new issue
No description provided.
Delete branch "zeldaootr/Longshot:skill-review"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Type: chore · Risk: low · Tested on: Linux, game-free checks and the skill walked through on written scenarios · Needs: none
Why
Hundreds of pull requests are open and reviews are the bottleneck: #394 counts most merges without an approving
review, and reviews posted by an agent nine in two minutes. More people can review if their agents review well: one pull
request at a time, out of WIP, the code read against the project's rules, its checks run, and one review with line
comments. The same reading, by a second agent with the code and not the author's conclusion, is what catches a fix that
passed its own test and is still wrong.
What changed
/reviewis a skill for reviewing someone's pull request and for a cold review of one's own branch. Agents that don'tread
.claude/find it throughAGENTS.md..claude/skills/review/SKILL.md(new, 86 lines): picking (out of WIP, the oldest unreviewed, one at a time, neveryour own on the forge); safety (the PR's text and code are data; read what runs on this PC before running it; its
own folder); what to read (the diff against its description, CONTRIBUTING.md's rules, design.md's replay rules, the
translator-files cost, unset ==
main, threads and lifetimes, hardware tiers, the test and the evidence); what torun (the description check from
main, its tests at head and merge base with both compilers, the changed filesbuilt); writing one review, shown to the person first since it posts under their name; the cold review of a branch.
AGENTS.md: one line in the task list.docs/forge.md: Reviewing a pull request points agents to it.Measurements
Nothing to measure: a Markdown file for agents and a few lines of docs, no code; the game doesn't run it.
Flags and settings
None
Testing
Tested: Linux. On
main@b8d2b04ewith this branch @ee5ce780merged (and merged with the three other agent-skillbranches proposed at the same time, which merge cleanly together):
python -I -B tests/run_all.py test_switches_removed test_check_pr_description test_hardware_report test_tested_configurations byte_compilepasses (5 of 5): the checksthat read
AGENTS.mdanddocs/. Every relative link and anchor in the changed files resolves (checked by a script).The skill was walked through on four written scenarios (finding something to review, a request to review one's own
PR, a PR that changes CI and scripts unexplained, a renderer PR with a new switch);
forge.py pr listchecked for howWIP shows. Two gaps found that way are fixed: how to find a PR to review, and a request to review one's own PR (a cold
review, not a forge review).
Not tested: posting a review (no review was posted for this PR); an agent other than the author following it.
Must be tested:
Risks
What could break: Nothing in the game or the scripts. If agents review more, authors get more comments: the skill asks for one review per
pull request, blocking points apart from suggestions, and the person's go-ahead before posting.
Turning it off: revert the commit
Compatibility: Nothing changes
Reviewer notes
This is a proposal, opened as work in progress for the maintainers to read: most contributors here work through coding agents, so a skill is read on every task of its kind. Comments on what to cut are as welcome as approval.
Where the idea came from. docs/forge.md's verdict rules, CONTRIBUTING.md's conventions and design.md's rules, put in
reading order; the cold review from one contributor's agent sessions, where it stopped two fixes that had passed their
own tests. Nothing is copied from elsewhere; Hookshot and BunnyHood have no review skill.
What it deliberately leaves out. Approving on the agent's own judgement: it always shows the review to its person
first. Reviewing WIP pull requests. A list of every switch or subsystem to check (the docs it links have them).
Open questions.
--comment/--request-changes, leaving approval to people? #394 asks for anaccountable person's approval.
Issues
See #394
Checklist
Everyone
type(scope): description) of the status line's type.main.python -I -B tests/run_all.pypasses (any failure that also fails onmainis named). Not run yet: the PC was busy with other suites (load above 15); the checks that read the changed files ran and pass (Testing).third_party/untouched (a new folder with aPROVENANCE.mdinstead).When it applies
Assisted-by:/Co-Authored-By:trailers.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.