diff --git a/skills/cross-model-review/SKILL.md b/skills/cross-model-review/SKILL.md index 40f13fe..d1f1fb1 100644 --- a/skills/cross-model-review/SKILL.md +++ b/skills/cross-model-review/SKILL.md @@ -105,6 +105,29 @@ objection is not re-litigated — but a *reproduced* correctness or safety defec stays must-fix (§3 triage): it may be deferred only with the owner's explicit acceptance, never closed by the reviewers on their own. +**Pre-existing is a disposition you establish, not assume — reproduce the +finding on the merge-base too.** On a behavior-preserving refactor branch a +*reproduced* defect can still be one the branch faithfully carried over from +master, not one it introduced; recording it against the branch is a false +regression that blocks a clean refactor for a fault it did not create. Before +you count a reproduced finding as the branch's defect, re-run it against the +merge-base: present on both → `pre-existing-tracked`, spun off as its own +fix, never bloating the refactor; gone on master and live on the branch → a real +regression this branch owns. Reviewers do not see master (§2 packet is your +inlined lines), so this classification is yours to make, not theirs — a +reviewer's severity label ("not merge-ready", "CRITICAL") is a claim about +the code in front of it, blind to whether the branch introduced the behavior. +And applicability is itself a **runtime/deployment-state** question, not a +source one: a migration or identifier case-collision finding that no live +data can trigger — empty store, writer never deployed — is an irreproducible +false positive (§4), not a regression this branch owns. Reproduce against +what actually shipped (query the collection, check the deploy), not synthetic +data the bug reproduces on but the running system never holds. +neg: filing a reviewer's "P1, not merge-ready" against a refactor for a quirk +that reproduces identically on the merge-base — the review still pays off (spin +the quirk off as its own fix), but the branch is clean and the label was +baseline-blind. + **A proposed fix is a suggestion, not a patch.** Reproducing a finding licenses the finding, not its remedy — these are separate judgments, and a reviewer writes the remedy against only the lines you inlined (§2), so a real @@ -232,6 +255,16 @@ the reason recorded. The landed side is verifiable in-repo (PR #32's commits); the held side was never pushed, so the comparison itself is contributor-reported — the rule ships with an in-body `unprobed` marker per the README covenant's second branch. +The §3 baseline-classification and runtime-state adjudication rules +(2026-07-24) are class-distilled from a mining pass over the owner's own +sessions (no code taken): a moira-web behavior-preserving refactor where +codex's "not merge-ready" and agy's "CRITICAL" both dissolved under +reproduction as pre-existing master behaviors the branch faithfully preserved +(0 regressions, 4 pre-existing quirks spun off), and a TG-bot review where +codex's identifier-migration and case-collision findings were non-applicable +because the store was empty and the writer had never deployed. The lesson +is the general one — pre-existing-vs-regression is a baseline diff, and +applicability is a runtime-state question — not the specific findings. Re-verify line: model families, CLI availability, "flagship" identity, and effort tiers are volatile — re-discover at session time; never trust a model name or tier