Skip to content

feat: add pr-review skill, project rules, and Node pin - #31

Open
nathanredblur wants to merge 5 commits into
mainfrom
feat/pr-review-skill
Open

feat: add pr-review skill, project rules, and Node pin#31
nathanredblur wants to merge 5 commits into
mainfrom
feat/pr-review-skill

Conversation

@nathanredblur

Copy link
Copy Markdown
Collaborator

Summary

Adds a project-local Claude Code skill for reviewing this repo's pull requests, tracks the .claude/ project rules that were previously untracked, and pins Node so astro build stops failing with an engine error.

Why a skill, and why not code-review

Named pr-review rather than code-review because the Anthropic code-review plugin already provides a command by that name, and the two behave differently in the way that matters most: that one posts to the PR automatically; this one never publishes without an explicit yes.

The design centres on a single failure mode — the confident false positive. A wrong finding costs the author real time and teaches them to skim the next review, which is worse than missing a nit. So the bar for every finding is a file:line that was actually opened, and the skill is explicitly barred from claiming the build or lint fails, since it does not run them.

What's here

Path
.claude/skills/pr-review/SKILL.md The review workflow, severity scale, and an explicit list of what not to flag
scripts/pr-context.sh One call for a PR: metadata, eligibility flags, diff, and the review comments earlier PRs on the same files attracted
scripts/pr-tree.sh Reads and greps the PR's tree, not your working directory
scripts/pr-stack.py Places a PR in its stack and searches the rest of the chain
scripts/post-review.py Validates every inline comment against the real diff hunks before posting
.claude/rules/ + .claude/CLAUDE.md Project conventions, previously untracked
.claude/pr-review-evals/ The eval harness (results gitignored)

Two of those scripts exist because of mistakes made while using an earlier draft of the skill on a real PR:

  • pr-tree.sh — the working directory is usually main. Grepping src/ to reason about a branch produced "these four components have no references", which was flatly wrong: the references were on the branch. Any question about a PR's file contents now goes through the PR's own tree.
  • pr-stack.py — this repo stacks deep (main → #25 → #26 → #21 → #18 → #24 → #19 was a real chain), and position decides what a finding means. An ancestor supplies things, so "X is missing" is wrong when the parent adds X. A descendant may already fix things, which makes a finding a merge-ordering constraint rather than a defect. Sampling the stack by hand missed a PR that touched the same component.

Verification

Both gates pass on this branch:

pnpm exec biome ci ./src   → exit 0, 34 files, 3 warnings + 1 info (all pre-existing on main)
pnpm build                 → 9 pages built, complete

pnpm build needs Node >= 22.12; the mise.toml in this PR is what makes that reproducible.

The skill itself was evaluated against three real PRs from this repo, each run with and without the skill: a stacked i18n PR, a Dependabot bump, and a bare "which PRs need review?". The assertions that actually separated the two runs were about restraint rather than thoroughness — the unaided baseline asserted a strictNullChecks compilation failure with no typechecker installed, and reviewed PRs of its own choosing when given no target. Harness and criteria are in .claude/pr-review-evals/.

Not verified

  • pr-tree.sh's auto-fetch fallback never ran: every PR tested already had its head commit locally. Only the refs/pull/N/head refspec was confirmed valid against the remote.
  • No timing or token figures in the eval benchmark — the data was not available, and estimates would have been worse than the omission.
  • The eval baseline is an unaided agent, not an earlier version of the skill, so the numbers say "this beats working from scratch" and not "the revisions improved it".

Reviewing this

.claude/rules/architecture.md describes the JSON-LD components as wired from layouts/Home.astro — accurate as of #25 merging.

🤖 Generated with Claude Code

nathanredblur and others added 4 commits September 4, 2026 12:42
The repo requires Node >=22.12 but nothing in the tree pinned it, so a
contributor on an older Node hits `astro build` failing with an engine
error rather than anything actionable. `mise.toml` makes the requirement
executable for anyone using mise.

`typescript` lands as a devDependency so editors and tooling resolve the
compiler from the workspace. Note this does not make the repo
type-checked: `astro check` still needs `@astrojs/check`, and
`astro build` strips types without verifying them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three additions, all regenerable or per-user:

- `.claude/settings.local.json` is per-developer. The rest of `.claude/`
  (CLAUDE.md, rules/, skills/) is shared deliberately.
- `.claude/*-evals/iteration-*/` is skill-evaluation output — hundreds of
  KB of generated reports. The harness stays tracked so a run can be
  reproduced; only its results are ignored, the same split used for tests
  and their artifacts.
- Python bytecode from the skill scripts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
These files already guided work in this repo but were untracked, so the
conventions they encode lived on one machine. Committing them makes the
i18n model, the Biome exemptions, the styling tokens and the localized
component pattern shared context rather than folklore.

`rules/architecture.md` describes the JSON-LD components as wired from
`layouts/Home.astro`, which became true when #25 merged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A project-local skill for reviewing this repo's pull requests. Named
`pr-review` rather than `code-review` to avoid colliding with the
Anthropic plugin command of that name, which behaves differently: it
posts to the PR automatically, while this one never publishes without an
explicit yes.

The design centres on one failure mode — the confident false positive —
because a wrong finding costs the author real time and teaches them to
skim the next review. Hence: every finding must cite a `file:line` that
was actually opened, and no claim about the build or lint, which this
skill does not run.

Four scripts carry the deterministic work:

- `pr-context.sh` gathers a PR in one call: metadata, eligibility flags,
  diff, and the review comments earlier PRs on the same files attracted.
- `pr-tree.sh` reads and greps the PR's tree. This exists because the
  working directory is usually `main`, and grepping `src/` to reason
  about a branch is how a review confidently reports dead code that is
  not dead.
- `pr-stack.py` places a PR in its stack. This repo stacks deep, and
  position decides what a finding means: an ancestor supplies things, a
  descendant may already fix them.
- `post-review.py` validates every inline comment against the real diff
  hunks before posting, since GitHub rejects an entire review if one
  comment points outside them.

Validated against three real PRs with and without the skill. Full detail
in `.claude/pr-review-evals/`; the generated reports are gitignored, the
harness is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants