reviewing-pull-requests v1.6.0: review-to-fix, severity circles, carry-forward, simplify dimension

[oksuzian]

Four additions to the reviewing-pull-requests skill (v1.2.0 → v1.6.0), one commit each:

1. From Review to Fix — the handoff from a review finding to the patch that implements it: implement the full prescription ("replace X with Y" ≠ "add Y"), state deliberate omissions in the PR body, don't treat an established idiom as a keep-reason, and keep minimal-diff about unrelated changes only. Motivated by Mu2e/Offline#1914, where the reviewer had to request a removal the original Offline#1908 review had already prescribed.

2. Colored-circle severity markers — the S0–S3 scale mapped onto GitHub-native circles for at-a-glance scanning: 🔴 S0 · 🟠 S1 · 🟡 S2 · ⚪ S3 · 🟢 verified-correct/approve. Findings lead with circle + tag; the Decision line carries 🔴/🟡/🟢; Output Template updated.

3. Re-reviews and carry-forward — when a prior review exists (staged, posted, or another reviewer's), every prior finding must be accounted for: FIXED (verified at the same evidence bar, moved to green as "was Sx"), UNADDRESSED (carried forward at original severity; unresolved 🔴/🟠 lead the list), PARTIAL (remaining part named), or WITHDRAWN (new evidence stated). Prior findings never silently vanish. Motivated by the Offline#1908 sequence, where a carried-forward S1 (missing CMake data install) survived two re-review rounds unaddressed and then materialized as a spack ceSimReco runtime failure.

4. Simplification & efficiency dimension — new review priority 6: report (not apply) simplify/optimize opportunities — dead or write-only state, duplication with an existing single home, work at the wrong cadence, complexity without payoff. Severity capped at 🟡/⚪ and explicitly never gates approval, per the existing "avoid requiring unrelated cleanup" rule. Grounded in practice: the write-only mapPhysVol_ set rebuilt every subrun (Offline#1908) and the cluster.diskID() simplification that also removed a failure mode (Offline#1911).

🤖 Generated with Claude Code

[rlcee]

I don't understand this:
~/pr_reviews/pr<N>_review.md
does this refer to your personal home directory? can't everything go from github to github?

[oksuzian]

It runs a review then saves a local copy to ~/pr_reviews/pr_review.md for convenience.
I guess you're asking do we need a local copy at all?

[oksuzian]

To close the loop on this: everything durable is GitHub-to-GitHub. Clarified in fe57255 — the carry-forward contract now reads prior findings from the reviews/comments posted on the PR itself, which is the canonical record and works for any reviewer on any machine.

The local file is only the drafting stage: the review is written there so a human can read and edit it before anything becomes visible on the PR, and it's disposable once posted. It's per-user scratch space, not shared state — the path is now documented as a configurable convention (default ~/pr_reviews, override with PR_REVIEW_DIR) rather than part of the workflow contract. Same change applied to the post-pr-review skill in #11 (3ff7f88), with a 'Why a local draft at all' section making the design explicit.