Address PR #415 R2 review (2 P1 sibling-surface drift)

R2 surfaced two cascading P1 findings — both sibling-surface drift from R0's
mandate substitution and assessment-criteria tightening that didn't propagate
to all reviewer-facing surfaces.

R2 P1 #1 — adapted prompt still instructed impossible tool-using audits.
The Mandate substitution removed shell-grep claims, but pr_review.md's Rules
section (line 139-142) and Re-review Scope (line 207) still said the
"Mandate above authorizes broader audits (sibling surfaces, pattern-wide
greps, reciprocal checks, transitive deps)". Both reviewers are now
single-shot with no shell access, so these references were misleading and
contradicted the substituted Mandate. Rewrote both bullets to scope audits
to the loaded context and explicitly call out single-shot constraints.

R2 P1 #2 — P2-blocking verdict bar wasn't mirrored across siblings. The
Assessment Criteria now says ✅ requires no unmitigated P0/P1/P2, but three
sibling surfaces still treated P2 as compatible with ✅:
- pr_review.md Re-review Scope (line 211-212): "If all previous P1+ findings
  are resolved, the assessment should be ✅ even if new P2/P3 items are
  noticed."
- openai_review.py compile_prompt previous-review block (line 1166-1170):
  same wording, injected into every re-review prompt.
- ai-review-local.md (line 400-418): "For ⛔ or ⚠️ (P0/P1 findings)" branch
  and "For ✅ with P2/P3 findings only" branch.

Updated all three to mirror the new rule: P2 blocks ✅ even on re-review;
the ⚠️ branch covers P0/P1/P2; the ✅ branch covers P3 only.

Tests added:
- TestAdaptReviewCriteria.test_no_tool_using_audit_claims_in_either_mode
  (asserts "pattern-wide greps", "Transitive workflow deps", "transitive
  deps", "Mandate above authorizes" are absent from adapted prompt in both
  ci_mode values)
- TestAdaptReviewCriteria.test_re_review_scope_uses_new_p2_blocking_rule
  (asserts old P2-carve-out wording is gone and new "block ✅ just like P1"
  wording is present)
- TestCompilePrompt.test_previous_review_block_uses_new_p2_blocking_rule
  (asserts compile_prompt's previous-review framing uses "P0/P1/P2 findings
  have been addressed" and "no new unmitigated P2 findings exist")
- TestSkillDocAPIConsistency.test_skill_doc_uses_new_p2_blocking_verdict_bar
  (asserts ai-review-local.md verdict decision tree uses P0/P1/P2 ⚠️ branch
  and P3-only ✅ branch)

189 tests pass (was 185).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: igerber

.claude/commands/ai-review-local.md

@@ -397,15 +397,15 @@ Review passed with no findings. Suggested next steps:
 - /submit-pr — commit and open a pull request
 ```
 
-**For ⛔ or ⚠️ (P0/P1 findings)**:
+**For ⛔ or ⚠️ (P0/P1/P2 findings)**:
 ```
 Options:
 1. Enter plan mode to address findings (Recommended)
 2. Re-run with --full-registry for deeper methodology context
 3. Skip — I'll address these manually
 ```
 
-**For ✅ with P2/P3 findings only**:
+**For ✅ with P3 findings only**:
 ```
 Options:
 1. Address findings before submitting
@@ -414,8 +414,8 @@ Options:
 
 **If user chooses to address findings**: Parse the findings from the review output.
 The review context is already in the conversation. Start addressing the findings
-directly — for P0/P1 issues use `EnterPlanMode` for a structured approach; for P2/P3
-issues, fix them directly since they are minor.
+directly — for P0/P1/P2 issues use `EnterPlanMode` for a structured approach; for
+P3 issues, fix them directly since they are minor.
 
 After fixes are committed, the user re-runs `/ai-review-local` for a follow-up review.
 On re-review, the script automatically activates delta-diff mode (comparing only

.claude/scripts/openai_review.py

@@ -1164,10 +1164,13 @@ def compile_prompt(
         if previous_review:
             sections.append(
                 "This is a follow-up review. The previous review's findings are included "
-                "below. Focus on whether previous P0/P1 findings have been addressed. "
+                "below. Focus on whether previous P0/P1/P2 findings have been addressed. "
                 "New findings on unchanged code should be marked \"[Newly identified]\". "
-                "If all previous P1+ findings are resolved, the assessment should be "
-                "\u2705 even if new P2/P3 items are noticed.\n"
+                "If all previous P1+ findings are resolved AND no new unmitigated P2 "
+                "findings exist (per the Assessment Criteria above), the assessment should "
+                "be \u2705. New unmitigated P2 findings (claim-vs-test mismatches, "
+                "public-API docstring drift, missing rendering surfaces) keep the verdict "
+                "at \u26a0\ufe0f Needs changes \u2014 they block \u2705 just like P1.\n"
             )
             if structured_findings:
                 sections.append("### Full Previous Review\n")

.github/codex/prompts/pr_review.md

@@ -137,9 +137,12 @@ This project tracks deferred technical debt in `TODO.md` under "Tech Debt from C
   are P0/P1 and are not deferrable.
 
 Rules:
-- Review the changes introduced by this PR (diff). The Single-Pass Completeness
-  Mandate above authorizes broader audits (sibling surfaces, pattern-wide greps,
-  reciprocal checks, transitive deps) — do those upfront rather than deferring.
+- Review the changes introduced by this PR (diff). Conduct the audits listed in
+  the Single-Pass Completeness section above (sibling surfaces, reciprocal
+  checks, claim-vs-shipped) on the loaded context — do those upfront rather
+  than deferring. You are a single-shot reviewer with no shell access, so audit
+  only what is visible in the loaded prompt; do not claim audits that require
+  greps, file loads, or tool use beyond the provided context.
 - Provide a single Markdown report with:
   - Overall assessment (see Assessment Criteria below)
   - Executive summary (3–6 bullets)
@@ -204,12 +207,14 @@ When this is a re-review (the PR has prior AI review comments):
   to distinguish from moving goalposts. Limit these to clear, concrete issues — not
   speculative concerns or stylistic preferences.
 - New code added since the last review IS in scope for new findings — apply the
-  Single-Pass Completeness Mandate's audits (sibling surfaces, pattern-wide greps,
-  reciprocal checks) to that new code in this re-review pass. For UNCHANGED code,
-  the existing [Newly identified] convention from the bullet above still applies:
+  Single-Pass Completeness audits (sibling surfaces, reciprocal checks, claim-vs-shipped)
+  to that new code in this re-review pass, scoped to the loaded context. For UNCHANGED
+  code, the existing [Newly identified] convention from the bullet above still applies:
   new P1+ findings MAY be raised but must be marked "[Newly identified]".
-- If all previous P1+ findings are resolved, the assessment should be ✅ even if new
-  P2/P3 items are noticed.
+- If all previous P1+ findings are resolved AND no new unmitigated P2 findings exist
+  (per the Assessment Criteria above), the assessment should be ✅. Newly identified
+  unmitigated P2 findings (claim-vs-test mismatches, public-API docstring drift, missing
+  rendering surfaces) keep the verdict at ⚠️ Needs changes — they block ✅ just like P1.
 
 ## Known Anti-Patterns
 

tests/test_openai_review.py

@@ -311,6 +311,63 @@ def test_claim_vs_shipped_audit_in_both_modes(self, review_mod):
                     f"Surface '{surface}' missing in ci_mode={ci_mode}"
                 )
 
+    def test_no_tool_using_audit_claims_in_either_mode(self, review_mod):
+        """The adapted prompt (post-substitution) must NOT instruct the
+        single-shot reviewer to do tool-using audits anywhere — neither in the
+        Mandate (substituted) nor in the Rules section nor in Re-review Scope.
+        Both reviewers are now single-shot; references to 'pattern-wide greps'
+        or 'transitive deps' as required audits are misleading and were the
+        source of PR #415 R2 P1 (sibling-surface drift).
+        """
+        assert _SCRIPT_PATH is not None
+        repo_root = _SCRIPT_PATH.parent.parent.parent
+        prompt_path = repo_root / ".github" / "codex" / "prompts" / "pr_review.md"
+        if not prompt_path.exists():
+            pytest.skip("pr_review.md not found")
+        source = prompt_path.read_text()
+        for ci_mode in (False, True):
+            adapted = review_mod._adapt_review_criteria(source, ci_mode=ci_mode)
+            # These tool-using-audit phrases must not appear ANYWHERE in the
+            # adapted prompt (Mandate, Rules, or Re-review Scope sections).
+            for phrase in (
+                "pattern-wide greps",
+                "Transitive workflow deps",
+                "transitive deps",
+                "Mandate above authorizes",
+            ):
+                assert phrase not in adapted, (
+                    f"Tool-using-audit phrase '{phrase}' leaked into "
+                    f"adapted prompt (ci_mode={ci_mode})"
+                )
+
+    def test_re_review_scope_uses_new_p2_blocking_rule(self, review_mod):
+        """Re-review Scope must mirror the tightened ✅ rule: P2 blocks ✅
+        even on re-review. Sibling-surface fix from PR #415 R2 P1."""
+        assert _SCRIPT_PATH is not None
+        repo_root = _SCRIPT_PATH.parent.parent.parent
+        prompt_path = repo_root / ".github" / "codex" / "prompts" / "pr_review.md"
+        if not prompt_path.exists():
+            pytest.skip("pr_review.md not found")
+        source = prompt_path.read_text()
+        # The old wording said "✅ even if new P2/P3 items are noticed".
+        # The new wording must explicitly say P2 blocks. Check both source
+        # and adapted (post-substitution) since this section isn't substituted.
+        old_p2_carve_out = (
+            "If all previous P1+ findings are resolved, the assessment should "
+            "be ✅ even if new P2/P3 items are noticed"
+        )
+        assert old_p2_carve_out not in source, (
+            "Re-review Scope still has the old P2-carve-out wording; must be "
+            "tightened to match Assessment Criteria"
+        )
+        # New wording must say P2 blocks, in both source and adapted
+        for ci_mode in (False, True):
+            adapted = review_mod._adapt_review_criteria(source, ci_mode=ci_mode)
+            assert "no new unmitigated P2 findings exist" in adapted, (
+                f"P2-blocking wording missing in ci_mode={ci_mode}"
+            )
+            assert "block ✅ just like P1" in adapted
+
 
 # ---------------------------------------------------------------------------
 # compile_prompt
@@ -346,6 +403,26 @@ def test_includes_previous_review(self, review_mod):
         assert "Previous review findings here." in result
         assert "follow-up review" in result
 
+    def test_previous_review_block_uses_new_p2_blocking_rule(self, review_mod):
+        """The previous-review framing in compile_prompt must mirror the
+        tightened ✅ rule: P2 blocks ✅ even on re-review. Sibling-surface fix
+        from PR #415 R2 P1.
+        """
+        result = review_mod.compile_prompt(
+            criteria_text="Criteria.",
+            registry_content="Registry.",
+            diff_text="diff content",
+            changed_files_text="M\tfoo.py",
+            branch_info="main",
+            previous_review="Previous review findings here.",
+        )
+        # Old (stale) wording must NOT appear
+        assert "✅ even if new P2/P3 items are noticed" not in result
+        # New wording must explicitly state P2 blocks
+        assert "P0/P1/P2 findings have been addressed" in result
+        assert "no new unmitigated P2 findings exist" in result
+        assert "block ✅ just like P1" in result
+
     def test_no_previous_review_block_when_none(self, review_mod):
         result = review_mod.compile_prompt(
             criteria_text="C.",
@@ -1996,6 +2073,33 @@ def test_skill_doc_does_not_reference_chat_completions(self):
             "script uses Responses API at openai_review.py:ENDPOINT"
         )
 
+    def test_skill_doc_uses_new_p2_blocking_verdict_bar(self):
+        """Skill doc's verdict-handling decision tree must mirror the
+        tightened ✅ rule: P2 triggers ⚠️, not ✅. Sibling-surface fix from
+        PR #415 R2 P1.
+        """
+        assert _SCRIPT_PATH is not None
+        repo_root = _SCRIPT_PATH.parent.parent.parent
+        doc_path = repo_root / ".claude" / "commands" / "ai-review-local.md"
+        if not doc_path.exists():
+            pytest.skip("ai-review-local.md not found")
+        text = doc_path.read_text()
+        # Old (stale) wording must NOT appear
+        assert "**For ⛔ or ⚠️ (P0/P1 findings)**" not in text, (
+            "Skill doc still uses old P0/P1-only ⚠️ branch; tighten to P0/P1/P2"
+        )
+        assert "**For ✅ with P2/P3 findings only**" not in text, (
+            "Skill doc still has '✅ with P2/P3 findings only' branch; under "
+            "the new rule, ✅ allows only P3"
+        )
+        # New wording must appear
+        assert "**For ⛔ or ⚠️ (P0/P1/P2 findings)**" in text
+        assert "**For ✅ with P3 findings only**" in text
+        assert (
+            "for P0/P1/P2 issues use `EnterPlanMode` for a structured approach"
+            in text
+        )
+
 
 class TestExtractResponseText:
     def test_prefers_output_text_field(self, review_mod):