Contribution review gate

Contribution Review Gate

A repeatable procedure for reviewing the user's own contribution PRs (self-review) and routing findings by severity. The point is to keep feature PRs minimal and honestly scoped while still surfacing real problems: post the things that must block or inform the maintainer now, and quietly bank the cleanup for later so it doesn't rot on a soon-closed PR.

This skill encodes the procedure and conventions. It does not replace judgment — you still read the code and decide severity every run. Treat the steps as a harness, not a script.

Before you start: recall context

The specifics evolve — active cleanup drafts, the finding taxonomy, the exact review etiquette, per-repo conventions. Do not hardcode those here. Recall them from memory / the long-term store at the start of each run (e.g. the review-etiquette note and the project's findings/deep-scan note). Read them; don't duplicate them into this file.

Inputs

• One or more PR numbers, and the repo (default to the fork's upstream; confirm if ambiguous).
• Confirm each PR is the user's own — this is self-review. If it's someone else's, stop and ask; the gate and etiquette below assume self-review.

The severity rubric

Classify every finding. The gate hinges on the P2 boundary, so calibrate it deliberately.

• P1 — correctness / data-loss / corruption / crash. A concrete input produces a wrong document, destroys existing data, or throws.
• P2 — a meaningful behavior or UX regression or gap: an explicit user action silently no-ops, loses state, or leaves the editor in a broken-but-not-obvious state; a robustness hole a real user will hit. Not data-loss, but clearly "this got worse / this is broken."
• P3 — nit / redundancy / consistency / phantom undo entry / test-guard brittleness. Real, but cosmetic or defense-in-depth.
• QUESTION — needs the author's design decision (e.g. "is this branch even reachable?", "did you intend Escape to cancel?"). Not a defect on its own.

For every P1/P2 you must be able to state a concrete failure scenario: exact inputs/steps → wrong result. If you can't construct one, it isn't P1/P2 — drop it or downgrade.

Procedure

1. Gather

For each PR: gh pr view <n> --json state,author,baseRefName,headRefName,headRefOid,isDraft,url, the full diff (git diff <base>...<headOid>), the PR body, and existing review state (gh api repos/OWNER/REPO/pulls/<n>/reviews and .../comments). Fetch the head so you can read files at that exact commit (git show <headOid>:<path>).

2. Review — trace each finding to its origin

Read the diff and the surrounding source it depends on — the helpers it calls, the callers, and the engine/runtime contracts it relies on — at the PR head. A routing/refactor change is only as correct as the invariants of what it plugs into.

For each candidate finding, don't stop at where it hurts — trace to where it started:

1. Symptom — where it's observable (the failing action / wrong output).
2. Consumption — where the bad value is used.
3. Production / first violation — the earliest file:line where a previously-true invariant goes false. Follow the value backward through callers, conversions, and any cached/derived representation to get there.

State the finding's mechanism at the first-violation file:line, not the symptom site — that file:line is also what the inline comment should anchor to, and what §3's refutation attacks. The recurring failure mode is a boundary contract the new path silently inherited: what the old call did afterward, what the snapshot recorder actually captures, what coordinate space the numbers live in. Verify the contract, not just the call. The concrete catalog of these traps (with the real P1/P2 cases) lives in clean-code-gate/references/contract-traps.md — consult it when a finding involves re-routed mutations, undo/snapshot, or index/coordinate spaces.

Classify each finding per the rubric. Recommending the fix direction for a confirmed finding is clean-code-gate's job — trace and verify here; hand the "where should the fix land" question to that skill.

3. Adversarially verify EVERY P2+ candidate before you accept it

This is the quality gate that protects the author's credibility. A false P2 posted on a public PR is worse than silence — it costs more trust than the finding could ever have earned. For each P1/P2 candidate, run an independent refutation pass (spawn a subagent, or trace it yourself with fresh skepticism) that reads the actual source and tries to prove the finding wrong. Confirm every crux link to file:line. Only findings that survive the refutation get posted.

The usual ways a candidate dies: the path is unreachable (a dispatcher prefix / canExecute / edit-mode guard blocks it), a clamp or existing guard prevents the bad state, it's pre-existing (the PR mirrors old behavior and doesn't introduce or widen it), or the impact is actually cosmetic. When two independent reviewers reach the same mechanism but disagree on reachability, the reachability question is the finding — resolve it against source before calling it P2.

4. Apply the gate

• P2 or higher → post on the PR. Submit a COMMENT-type review (gh api -X POST repos/OWNER/REPO/pulls/<n>/reviews, event=COMMENT), with each finding as an inline comment anchored to the exact changed line (path + line + side:RIGHT; the line must be within a diff hunk). Write in the repo's review language. For each: state the mechanism (traced, with file:line), a concrete repro, and a concrete fix direction. Follow the review etiquette from memory — the load-bearing rules are: no top-level "thanks for the review", never guess the review's provenance/tooling, facts only.

• Below P2 → do NOT post on the source PR. Add it to the right root-cause cluster in a scoped cleanup draft PR. Prefer 2–4 drafts split by domain/root-cause, not one per source PR — so every instance of one root cause (e.g. all redundant-emit findings) collects in one place and can be fixed together. QUESTIONs go in a "needs author decision" section of the draft, not as code. Update the draft's checklist body (gh pr edit <draft> --body-file ...).

Why the split: the feature PR stays minimal and honestly scoped (which maintainers reward), the cleanup accumulates as live code + a checklist rather than as notes on a PR nobody reopens, and related fixes land as one focused change instead of scattered nits.

5. Draft & thread hygiene

• Draft PRs live on the fork branch off the latest base (git fetch first; rebase if it drifts — cleanups touch hot files). Open the draft with --draft; its body is the clustered checklist. A PR needs at least one commit — a small, obviously-safe first fix is a good anchor (and start the accumulator), but verify it (tsc / tests) before committing.
• If a P3 was previously posted on a PR and you're now moving it to a draft, delete the stale inline thread: gh api -X DELETE repos/OWNER/REPO/pulls/comments/{id}.
• Submitted COMMENTED review bodies cannot be deleted (only pending reviews can; only inline comments are removable). So post P3-heavy self-reviews as inline-only from the start and keep the top-level body minimal — otherwise a summary listing the findings lingers after you clean up.

6. Report, and confirm when it counts

Summarize per PR: severity counts, what was posted where, what moved to which draft, and any QUESTIONs. The user may grant standing authorization to run the whole gate autonomously ("do it that way") — honor that. Absent it, or when a severity is borderline or the post is unusually consequential, show the drafted comment and confirm before posting. Public posts are close to irreversible (notifications fire); a false or sloppy one is the expensive failure mode here.

After the run

Persist what's reusable to memory: the review outcome (what posted / what banked), any new root-cause cluster, and — importantly — any new gotcha the review surfaced (a class of bug, a reachability subtlety, an API limitation). The point of the loop is that next cycle starts smarter.