From ceedd80b8ea836b67aea6b5fb756577c712ea9b3 Mon Sep 17 00:00:00 2001 From: firaen22 Date: Fri, 24 Jul 2026 23:21:09 +0800 Subject: [PATCH] =?UTF-8?q?cross-model-review=20=C2=A73:=20pre-existing-vs?= =?UTF-8?q?-regression=20is=20a=20baseline=20diff,=20applicability=20is=20?= =?UTF-8?q?a=20runtime-state=20question?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two adjudication gaps for reviews of behavior-preserving refactor branches, class-distilled from a mining pass over the owner's own sessions (no code taken). §3 already reproduces findings before acting and tracks a pre-existing-tracked disposition, but does not say how you ESTABLISH pre-existing: on a refactor branch a reproduced defect can be one the branch faithfully carried over from master, so reproduce on the merge-base before counting it a regression the branch owns. And a finding can be non-applicable against runtime/deployment state (empty store, never-deployed writer) even when the source reads as a bug — adjudicate against the running system, not source alone. Co-Authored-By: Claude Fable 5 --- skills/cross-model-review/SKILL.md | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) 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