From 865e212a53c2f1538df19631806adecd44391ff5 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Mon, 13 Jul 2026 02:26:52 +0800 Subject: [PATCH 1/3] feat: add pure contract identity helpers Co-Authored-By: Codex --- scripts/run_codex_pr_review.py | 75 +++++++++++++++++++++ tests/fixtures/codex_pr_review_replays.json | 40 +++++++++++ tests/test_run_codex_pr_review.py | 38 +++++++++++ 3 files changed, 153 insertions(+) create mode 100644 tests/fixtures/codex_pr_review_replays.json diff --git a/scripts/run_codex_pr_review.py b/scripts/run_codex_pr_review.py index 4b04f62..28d636b 100644 --- a/scripts/run_codex_pr_review.py +++ b/scripts/run_codex_pr_review.py @@ -804,6 +804,81 @@ def parse_arbitration_output( return result +def _normalize_contract_text(value: Any) -> str: + text = _sanitize_history_text(value, 240).lower() + stop_words = { + "the", "and", "for", "from", "into", "that", "this", "with", + "must", "should", "will", "return", + } + return " ".join( + sorted(token for token in re.findall(r"[a-z0-9]+", text) if token not in stop_words) + ) + + +def _contract_subject(finding: dict[str, Any]) -> str: + text = " ".join( + str(finding.get(field) or "") for field in ("description", "suggestion") + ) + anchors = re.findall(r"`([^`]{1,120})`", text) + anchors.extend(re.findall(r"\b[A-Za-z_]\w*(?:\.[A-Za-z_]\w*)+\b", text)) + normalized = {_normalize_contract_text(anchor) for anchor in anchors} + return "|".join(sorted(anchor for anchor in normalized if anchor)) or _normalize_contract_text(text) + + +def _contract_key(finding: dict[str, Any]) -> str: + payload = json.dumps( + { + "file": _sanitize_history_path(finding.get("file")), + "category": _sanitize_history_text(finding.get("category"), 80).lower(), + "subject": _contract_subject(finding), + }, + sort_keys=True, + separators=(",", ":"), + ) + return hashlib.sha256(payload.encode("utf-8")).hexdigest()[:20] + + +def _behavior_digest(finding: dict[str, Any]) -> str: + payload = json.dumps( + { + "contract_key": _contract_key(finding), + "behavior": _normalize_contract_text( + finding.get("suggestion") or finding.get("description") + ), + }, + sort_keys=True, + separators=(",", ":"), + ) + return hashlib.sha256(payload.encode("utf-8")).hexdigest()[:20] + + +def _contract_finding(finding: dict[str, Any]) -> dict[str, str]: + normalized = { + "severity": _sanitize_history_text(finding.get("severity"), 20).lower(), + "category": _sanitize_history_text(finding.get("category"), 80).lower(), + "file": _sanitize_history_path(finding.get("file")), + "description": _sanitize_history_text(finding.get("description")), + "suggestion": _sanitize_history_text(finding.get("suggestion")), + } + normalized["contract_key"] = _contract_key(normalized) + normalized["behavior_digest"] = _behavior_digest(normalized) + normalized["fingerprint_v2"] = hashlib.sha256( + json.dumps( + { + "contract_key": normalized["contract_key"], + "behavior_digest": normalized["behavior_digest"], + }, + sort_keys=True, + separators=(",", ":"), + ).encode("utf-8") + ).hexdigest()[:20] + return normalized + + +def _fingerprint_v2(finding: dict[str, Any]) -> str: + return _contract_finding(finding)["fingerprint_v2"] + + def blocking_finding_fingerprint(findings: list[dict[str, Any]]) -> str: """Return a stable arbitration-candidate identifier despite wording drift.""" normalized: list[dict[str, str]] = [] diff --git a/tests/fixtures/codex_pr_review_replays.json b/tests/fixtures/codex_pr_review_replays.json new file mode 100644 index 0000000..c2e1bea --- /dev/null +++ b/tests/fixtures/codex_pr_review_replays.json @@ -0,0 +1,40 @@ +{ + "dispatch_pr_75": { + "repo": "org/audit-bridge", + "pr_number": 75, + "finding": { + "severity": "high", + "category": "logic", + "file": "service/adapters/dispatch.py", + "description": "`DispatchState.apply()` must distinguish pre-launch failure from provider dispatch.", + "suggestion": "Persist `DispatchState.not_started` before the provider boundary." + }, + "reworded": { + "severity": "high", + "category": "logic", + "file": "service/adapters/dispatch.py", + "description": "A local error in `DispatchState.apply()` is not proof of provider launch.", + "suggestion": "Persist `DispatchState.not_started` before the provider boundary." + } + }, + "crypto_pr_125": { + "repo": "org/crypto-pipelines", + "pr_number": 125, + "findings": [ + { + "severity": "high", + "category": "logic", + "file": "src/strategy_lifecycle/backtest_wrapper.py", + "description": "`load_preflight_panel()` must validate the requested completed day.", + "suggestion": "Require an exact scored-date match." + }, + { + "severity": "high", + "category": "logic", + "file": "src/strategy_lifecycle/backtest_wrapper.py", + "description": "`CryptoBacktestRunner.run()` must revalidate each requested window.", + "suggestion": "Do not reuse a cached runner without window validation." + } + ] + } +} diff --git a/tests/test_run_codex_pr_review.py b/tests/test_run_codex_pr_review.py index cec69a0..523477a 100644 --- a/tests/test_run_codex_pr_review.py +++ b/tests/test_run_codex_pr_review.py @@ -15,6 +15,10 @@ class RunCodexPrReviewTests(unittest.TestCase): + def _replay_fixtures(self) -> dict[str, object]: + path = Path(__file__).with_name("fixtures") / "codex_pr_review_replays.json" + return json.loads(path.read_text(encoding="utf-8")) + def _write_event(self, tmpdir: str, files: list[str]) -> str: event = { "pull_request": {"number": 7, "head": {"sha": "abc1234"}}, @@ -111,6 +115,40 @@ def test_repeated_findings_are_fingerprinted_before_arbitration(self) -> None: self.assertTrue(run_codex_pr_review.should_arbitrate(blocked=True, streak=2, repeated=True, new_head=True)) self.assertFalse(run_codex_pr_review.should_arbitrate(blocked=True, streak=2, repeated=True, new_head=False)) + def test_replay_fixtures_cover_pure_contract_identity(self) -> None: + fixtures = self._replay_fixtures() + dispatch = fixtures["dispatch_pr_75"] + crypto = fixtures["crypto_pr_125"]["findings"] + dispatch_identity = run_codex_pr_review._contract_finding(dispatch["finding"]) + reworded_identity = run_codex_pr_review._contract_finding(dispatch["reworded"]) + self.assertEqual(dispatch_identity["fingerprint_v2"], reworded_identity["fingerprint_v2"]) + self.assertNotEqual( + run_codex_pr_review._contract_key(crypto[0]), + run_codex_pr_review._contract_key(crypto[1]), + ) + opposite = dict(dispatch["finding"], suggestion="Reject provider dispatch before launch.") + self.assertNotEqual( + dispatch_identity["behavior_digest"], + run_codex_pr_review._contract_finding(opposite)["behavior_digest"], + ) + critical = dict(dispatch["finding"], severity="critical") + self.assertEqual( + dispatch_identity["fingerprint_v2"], + run_codex_pr_review._contract_finding(critical)["fingerprint_v2"], + ) + + def test_no_anchor_subject_combines_description_and_suggestion(self) -> None: + first = { + "category": "logic", "file": "service/review.py", + "description": "Reject invalid state before dispatch.", + "suggestion": "Persist the rejected state.", + } + second = dict(first, description="Accept invalid state before dispatch.") + self.assertNotEqual( + run_codex_pr_review._contract_key(first), + run_codex_pr_review._contract_key(second), + ) + def test_parse_arbitration_output_requires_supported_verdict(self) -> None: self.assertEqual( run_codex_pr_review.parse_arbitration_output('{"verdict":"clear","reason":"covered by the regression test"}'), From b2692847e9e892d034a66758ba184000bc84cfe3 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Mon, 13 Jul 2026 02:56:14 +0800 Subject: [PATCH 2/3] fix: preserve contract behavior ordering Co-Authored-By: Codex --- scripts/run_codex_pr_review.py | 7 +++---- tests/test_run_codex_pr_review.py | 28 +++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/scripts/run_codex_pr_review.py b/scripts/run_codex_pr_review.py index 28d636b..bbe3b3a 100644 --- a/scripts/run_codex_pr_review.py +++ b/scripts/run_codex_pr_review.py @@ -811,7 +811,7 @@ def _normalize_contract_text(value: Any) -> str: "must", "should", "will", "return", } return " ".join( - sorted(token for token in re.findall(r"[a-z0-9]+", text) if token not in stop_words) + token for token in re.findall(r"[a-z0-9]+", text) if token not in stop_words ) @@ -842,9 +842,8 @@ def _behavior_digest(finding: dict[str, Any]) -> str: payload = json.dumps( { "contract_key": _contract_key(finding), - "behavior": _normalize_contract_text( - finding.get("suggestion") or finding.get("description") - ), + "description": _normalize_contract_text(finding.get("description")), + "suggestion": _normalize_contract_text(finding.get("suggestion")), }, sort_keys=True, separators=(",", ":"), diff --git a/tests/test_run_codex_pr_review.py b/tests/test_run_codex_pr_review.py index 523477a..19236b1 100644 --- a/tests/test_run_codex_pr_review.py +++ b/tests/test_run_codex_pr_review.py @@ -121,7 +121,9 @@ def test_replay_fixtures_cover_pure_contract_identity(self) -> None: crypto = fixtures["crypto_pr_125"]["findings"] dispatch_identity = run_codex_pr_review._contract_finding(dispatch["finding"]) reworded_identity = run_codex_pr_review._contract_finding(dispatch["reworded"]) - self.assertEqual(dispatch_identity["fingerprint_v2"], reworded_identity["fingerprint_v2"]) + self.assertEqual(dispatch_identity["contract_key"], reworded_identity["contract_key"]) + self.assertNotEqual(dispatch_identity["behavior_digest"], reworded_identity["behavior_digest"]) + self.assertNotEqual(dispatch_identity["fingerprint_v2"], reworded_identity["fingerprint_v2"]) self.assertNotEqual( run_codex_pr_review._contract_key(crypto[0]), run_codex_pr_review._contract_key(crypto[1]), @@ -148,6 +150,30 @@ def test_no_anchor_subject_combines_description_and_suggestion(self) -> None: run_codex_pr_review._contract_key(first), run_codex_pr_review._contract_key(second), ) + self.assertNotEqual( + run_codex_pr_review._behavior_digest(first), + run_codex_pr_review._behavior_digest(second), + ) + self.assertNotEqual( + run_codex_pr_review._fingerprint_v2(first), + run_codex_pr_review._fingerprint_v2(second), + ) + + def test_ordered_contract_text_distinguishes_reversed_behavior(self) -> None: + before = { + "category": "logic", "file": "service/review.py", + "description": "Validate A before B.", + "suggestion": "Use the required order.", + } + after = dict(before, description="Validate B before A.") + self.assertEqual( + run_codex_pr_review._normalize_contract_text(before["description"]), + "validate a before b", + ) + self.assertNotEqual( + run_codex_pr_review._behavior_digest(before), + run_codex_pr_review._behavior_digest(after), + ) def test_parse_arbitration_output_requires_supported_verdict(self) -> None: self.assertEqual( From dd128c2a86e6f189cf090a0e0bc85c7e65a0dad3 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Mon, 13 Jul 2026 03:08:35 +0800 Subject: [PATCH 3/3] fix: separate raw identity from history sanitization Co-Authored-By: Codex --- scripts/run_codex_pr_review.py | 37 +++++++++++++++++++------- tests/test_run_codex_pr_review.py | 43 ++++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/scripts/run_codex_pr_review.py b/scripts/run_codex_pr_review.py index bbe3b3a..811f864 100644 --- a/scripts/run_codex_pr_review.py +++ b/scripts/run_codex_pr_review.py @@ -804,32 +804,49 @@ def parse_arbitration_output( return result -def _normalize_contract_text(value: Any) -> str: - text = _sanitize_history_text(value, 240).lower() +def _normalize_identity_text(value: Any) -> str: + text = str(value or "").lower() stop_words = { "the", "and", "for", "from", "into", "that", "this", "with", "must", "should", "will", "return", } return " ".join( - token for token in re.findall(r"[a-z0-9]+", text) if token not in stop_words + token + for token in re.findall(r"[a-z0-9_]+(?:\.[a-z0-9_]+)*", text) + if token not in stop_words ) +def _normalize_identity_anchor(value: Any) -> str: + return _normalize_identity_text(value) + + +def _normalize_contract_text(value: Any) -> str: + return _normalize_identity_text(value) + + def _contract_subject(finding: dict[str, Any]) -> str: text = " ".join( str(finding.get(field) or "") for field in ("description", "suggestion") ) anchors = re.findall(r"`([^`]{1,120})`", text) anchors.extend(re.findall(r"\b[A-Za-z_]\w*(?:\.[A-Za-z_]\w*)+\b", text)) - normalized = {_normalize_contract_text(anchor) for anchor in anchors} - return "|".join(sorted(anchor for anchor in normalized if anchor)) or _normalize_contract_text(text) + subject = { + "anchors": [ + normalized + for anchor in anchors + if (normalized := _normalize_identity_anchor(anchor)) + ], + "context": _normalize_identity_text(text), + } + return json.dumps(subject, ensure_ascii=True, separators=(",", ":")) def _contract_key(finding: dict[str, Any]) -> str: payload = json.dumps( { - "file": _sanitize_history_path(finding.get("file")), - "category": _sanitize_history_text(finding.get("category"), 80).lower(), + "file": _normalize_identity_text(finding.get("file")), + "category": _normalize_identity_text(finding.get("category")), "subject": _contract_subject(finding), }, sort_keys=True, @@ -852,6 +869,8 @@ def _behavior_digest(finding: dict[str, Any]) -> str: def _contract_finding(finding: dict[str, Any]) -> dict[str, str]: + contract_key = _contract_key(finding) + behavior_digest = _behavior_digest(finding) normalized = { "severity": _sanitize_history_text(finding.get("severity"), 20).lower(), "category": _sanitize_history_text(finding.get("category"), 80).lower(), @@ -859,8 +878,8 @@ def _contract_finding(finding: dict[str, Any]) -> dict[str, str]: "description": _sanitize_history_text(finding.get("description")), "suggestion": _sanitize_history_text(finding.get("suggestion")), } - normalized["contract_key"] = _contract_key(normalized) - normalized["behavior_digest"] = _behavior_digest(normalized) + normalized["contract_key"] = contract_key + normalized["behavior_digest"] = behavior_digest normalized["fingerprint_v2"] = hashlib.sha256( json.dumps( { diff --git a/tests/test_run_codex_pr_review.py b/tests/test_run_codex_pr_review.py index 19236b1..565b3f6 100644 --- a/tests/test_run_codex_pr_review.py +++ b/tests/test_run_codex_pr_review.py @@ -121,7 +121,7 @@ def test_replay_fixtures_cover_pure_contract_identity(self) -> None: crypto = fixtures["crypto_pr_125"]["findings"] dispatch_identity = run_codex_pr_review._contract_finding(dispatch["finding"]) reworded_identity = run_codex_pr_review._contract_finding(dispatch["reworded"]) - self.assertEqual(dispatch_identity["contract_key"], reworded_identity["contract_key"]) + self.assertNotEqual(dispatch_identity["contract_key"], reworded_identity["contract_key"]) self.assertNotEqual(dispatch_identity["behavior_digest"], reworded_identity["behavior_digest"]) self.assertNotEqual(dispatch_identity["fingerprint_v2"], reworded_identity["fingerprint_v2"]) self.assertNotEqual( @@ -175,6 +175,47 @@ def test_ordered_contract_text_distinguishes_reversed_behavior(self) -> None: run_codex_pr_review._behavior_digest(after), ) + def test_identity_preserves_anchors_predicates_and_long_tails(self) -> None: + schema = { + "category": "contract", "file": "service/review.py", + "description": "Validate `schema_v2` before persistence.", + "suggestion": "Reject invalid input.", + } + fingerprint = dict(schema, description="Validate `fingerprint_v2` before persistence.") + self.assertNotEqual( + run_codex_pr_review._contract_key(schema), + run_codex_pr_review._contract_key(fingerprint), + ) + + auth = { + "category": "security", "file": "service/review.py", + "description": "`validate()` must check the auth header.", + "suggestion": "Reject missing credentials.", + } + database = dict(auth, description="`validate()` must prevent a database leak.") + self.assertNotEqual( + run_codex_pr_review._contract_key(auth), + run_codex_pr_review._contract_key(database), + ) + + shared = "`validate()` " + ("predicate context " * 130) + first = dict(auth, description=shared + "tail_alpha_unique") + second = dict(auth, description=shared + "tail_beta_unique") + first_identity = run_codex_pr_review._contract_finding(first) + second_identity = run_codex_pr_review._contract_finding(second) + self.assertNotEqual(first_identity["fingerprint_v2"], second_identity["fingerprint_v2"]) + self.assertNotIn("tail_alpha_unique", json.dumps(first_identity)) + self.assertNotIn("tail_beta_unique", json.dumps(second_identity)) + self.assertLessEqual( + len(first_identity["description"]), + run_codex_pr_review.FINDING_HISTORY_TEXT_LIMIT, + ) + redacted = run_codex_pr_review._contract_finding( + dict(auth, description="evidence token=secret-value") + ) + self.assertIn("[REDACTED]", redacted["description"]) + self.assertNotIn("secret-value", json.dumps(redacted)) + def test_parse_arbitration_output_requires_supported_verdict(self) -> None: self.assertEqual( run_codex_pr_review.parse_arbitration_output('{"verdict":"clear","reason":"covered by the regression test"}'),