From 9f153da8f4c11e7d34d2e59b067bbe16cbe82db2 Mon Sep 17 00:00:00 2001 From: Codex Automation Date: Mon, 3 Aug 2026 21:53:17 -0500 Subject: [PATCH 1/4] fix(sync): address generated review findings --- scripts/check_deliberate_break.py | 10 ++++-- scripts/langchain/pr_verifier.py | 6 +++- tests/scripts/test_check_deliberate_break.py | 36 ++++++++++++++----- .../test_pr_verifier_structured_output.py | 10 ++++++ 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/scripts/check_deliberate_break.py b/scripts/check_deliberate_break.py index 451933fe1..05b17fcb8 100644 --- a/scripts/check_deliberate_break.py +++ b/scripts/check_deliberate_break.py @@ -15,6 +15,7 @@ import tempfile from collections.abc import Iterator from dataclasses import dataclass +from importlib import metadata from io import BytesIO from pathlib import Path @@ -32,7 +33,8 @@ r"\b(assert|expect\(|pytest\.raises\(|assert\.)\b", ) DEFAULT_TIMEOUT_SECONDS = 120 -PYTEST_RUNTIME_DEPENDENCIES = ("pyyaml==6.0.3",) +PYTEST_RUNTIME_VERSION = "6.0.3" +PYTEST_RUNTIME_DEPENDENCIES = (f"pyyaml=={PYTEST_RUNTIME_VERSION}",) @dataclass(frozen=True) @@ -140,8 +142,10 @@ def _ensure_pytest_runtime_deps() -> None: Actions ``action_required`` approval wait on workflow-touching PRs. """ try: - import yaml # noqa: F401 - except ImportError: + installed_version = metadata.version("PyYAML") + except metadata.PackageNotFoundError: + installed_version = None + if installed_version != PYTEST_RUNTIME_VERSION: subprocess.run( [ sys.executable, diff --git a/scripts/langchain/pr_verifier.py b/scripts/langchain/pr_verifier.py index 1a3425406..91c2f2155 100755 --- a/scripts/langchain/pr_verifier.py +++ b/scripts/langchain/pr_verifier.py @@ -683,7 +683,11 @@ def _text_from_response_content(content: object) -> str | None: text_blocks = [ block["text"] for block in content - if isinstance(block, dict) and isinstance(block.get("text"), str) + if ( + isinstance(block, dict) + and isinstance(block.get("text"), str) + and block["text"].strip() + ) ] if text_blocks: # Concatenate without a separator: a provider may split one JSON diff --git a/tests/scripts/test_check_deliberate_break.py b/tests/scripts/test_check_deliberate_break.py index c115931a2..9934e10b5 100644 --- a/tests/scripts/test_check_deliberate_break.py +++ b/tests/scripts/test_check_deliberate_break.py @@ -1,4 +1,3 @@ -import builtins import os import subprocess import sys @@ -260,20 +259,22 @@ def test_cli_skips_without_marker(tmp_path) -> None: assert "skipped: no deliberate-break marker" in completed.stdout -def test_runtime_dependency_installer_uses_locked_pyyaml(monkeypatch) -> None: - real_import = builtins.__import__ +@pytest.mark.parametrize("installed_version", [None, "6.0.2"]) +def test_runtime_dependency_installer_uses_locked_pyyaml( + monkeypatch, installed_version +) -> None: calls: list[tuple[object, dict[str, object]]] = [] - def missing_yaml(name, *args, **kwargs): - if name == "yaml": - raise ImportError("PyYAML missing") - return real_import(name, *args, **kwargs) + def package_version(_name): + if installed_version is None: + raise deliberate_break.metadata.PackageNotFoundError + return installed_version def record_install(*args, **kwargs): calls.append((args, kwargs)) return subprocess.CompletedProcess(args[0], 0, "", "") - monkeypatch.setattr(builtins, "__import__", missing_yaml) + monkeypatch.setattr(deliberate_break.metadata, "version", package_version) monkeypatch.setattr(deliberate_break.subprocess, "run", record_install) deliberate_break._ensure_pytest_runtime_deps() @@ -300,6 +301,25 @@ def record_install(*args, **kwargs): ] +def test_runtime_dependency_installer_accepts_exact_locked_pyyaml(monkeypatch) -> None: + calls: list[object] = [] + + monkeypatch.setattr( + deliberate_break.metadata, + "version", + lambda _name: deliberate_break.PYTEST_RUNTIME_VERSION, + ) + monkeypatch.setattr( + deliberate_break.subprocess, + "run", + lambda *args, **kwargs: calls.append((args, kwargs)), + ) + + deliberate_break._ensure_pytest_runtime_deps() + + assert calls == [] + + def _sound_spec(repo: Path) -> tuple[str, object]: _write_app(repo, 0) base = _commit(repo, "base behavior") diff --git a/tests/scripts/test_pr_verifier_structured_output.py b/tests/scripts/test_pr_verifier_structured_output.py index 34e351eb9..4e1f3a10a 100644 --- a/tests/scripts/test_pr_verifier_structured_output.py +++ b/tests/scripts/test_pr_verifier_structured_output.py @@ -226,6 +226,16 @@ def test_text_from_response_content_concatenates_split_text_blocks_without_separ assert pr_verifier._coerce_response_content(blocks) == encoded +def test_text_from_response_content_ignores_blank_text_blocks() -> None: + blocks = [ + {"type": "text", "text": " \n"}, + {"type": "thinking", "signature": "still thinking"}, + ] + + assert pr_verifier._text_from_response_content(blocks) is None + assert pr_verifier._coerce_response_content(blocks) == json.dumps(blocks, default=str) + + def test_evaluate_pr_valid_output_no_repair(monkeypatch: pytest.MonkeyPatch) -> None: payload = _valid_payload() good = json.dumps(payload) From 7e97e6d8e4f1a82b73e7786f1f2cc777b51a53c9 Mon Sep 17 00:00:00 2001 From: Codex Automation Date: Mon, 3 Aug 2026 21:58:55 -0500 Subject: [PATCH 2/4] style: format deliberate-break regression test --- tests/scripts/test_check_deliberate_break.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/scripts/test_check_deliberate_break.py b/tests/scripts/test_check_deliberate_break.py index 9934e10b5..1f0849d2a 100644 --- a/tests/scripts/test_check_deliberate_break.py +++ b/tests/scripts/test_check_deliberate_break.py @@ -260,9 +260,7 @@ def test_cli_skips_without_marker(tmp_path) -> None: @pytest.mark.parametrize("installed_version", [None, "6.0.2"]) -def test_runtime_dependency_installer_uses_locked_pyyaml( - monkeypatch, installed_version -) -> None: +def test_runtime_dependency_installer_uses_locked_pyyaml(monkeypatch, installed_version) -> None: calls: list[tuple[object, dict[str, object]]] = [] def package_version(_name): From 22fe516df551e705e1cf75d1d63a2931ac6694e4 Mon Sep 17 00:00:00 2001 From: Codex Automation Date: Mon, 3 Aug 2026 22:00:05 -0500 Subject: [PATCH 3/4] fix(benchmark): reject non-integer category floors --- tests/tools/test_evaluate_model_benchmark.py | 11 +++++++++++ tools/evaluate_model_benchmark.py | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_evaluate_model_benchmark.py b/tests/tools/test_evaluate_model_benchmark.py index 7e81a6a3c..a61c06368 100644 --- a/tests/tools/test_evaluate_model_benchmark.py +++ b/tests/tools/test_evaluate_model_benchmark.py @@ -293,6 +293,17 @@ def test_override_floor_below_one_is_rejected(floor): evaluator.evaluate_benchmark(_thin_payload(), policy) +@pytest.mark.parametrize("floor", [1.9, "2", None, True]) +def test_override_floor_must_be_an_integer(floor): + policy = _policy() + policy["profiles"]["verifier-balanced"]["approval_stage"][ + "minimum_cases_per_category_overrides" + ] = {"review-thread-debt": floor} + + with pytest.raises(ValueError, match="must be integers"): + evaluator.evaluate_benchmark(_thin_payload(), policy) + + def test_override_for_unknown_category_is_rejected(): policy = _policy() policy["profiles"]["verifier-balanced"]["approval_stage"][ diff --git a/tools/evaluate_model_benchmark.py b/tools/evaluate_model_benchmark.py index e9a497801..c18724f1f 100644 --- a/tools/evaluate_model_benchmark.py +++ b/tools/evaluate_model_benchmark.py @@ -165,7 +165,11 @@ def evaluate_benchmark(payload: dict[str, Any], policy: dict[str, Any]) -> dict[ category_floors: dict[str, int] = {} for category in required_categories: floor = raw_overrides.get(category, minimum_per_category) - floor = int(floor) + if type(floor) is not int: + raise ValueError( + "minimum_cases_per_category_overrides values must be integers " + f"(got {floor!r} for {category!r})" + ) if floor < 1: raise ValueError( "minimum_cases_per_category_overrides values must be >= 1 " From 76c9383ea7c4c4e7f90e7ecc17c70139640004de Mon Sep 17 00:00:00 2001 From: Codex Automation Date: Mon, 3 Aug 2026 22:03:54 -0500 Subject: [PATCH 4/4] fix(verifier): preserve split-block whitespace --- scripts/langchain/pr_verifier.py | 8 ++------ tests/scripts/test_pr_verifier_structured_output.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/scripts/langchain/pr_verifier.py b/scripts/langchain/pr_verifier.py index 91c2f2155..afe379cdf 100755 --- a/scripts/langchain/pr_verifier.py +++ b/scripts/langchain/pr_verifier.py @@ -683,13 +683,9 @@ def _text_from_response_content(content: object) -> str | None: text_blocks = [ block["text"] for block in content - if ( - isinstance(block, dict) - and isinstance(block.get("text"), str) - and block["text"].strip() - ) + if isinstance(block, dict) and isinstance(block.get("text"), str) ] - if text_blocks: + if any(block.strip() for block in text_blocks): # Concatenate without a separator: a provider may split one JSON # document across blocks, and an inserted newline inside a string # literal would make the reassembled payload invalid JSON. diff --git a/tests/scripts/test_pr_verifier_structured_output.py b/tests/scripts/test_pr_verifier_structured_output.py index 4e1f3a10a..0bf4d9bfe 100644 --- a/tests/scripts/test_pr_verifier_structured_output.py +++ b/tests/scripts/test_pr_verifier_structured_output.py @@ -226,6 +226,16 @@ def test_text_from_response_content_concatenates_split_text_blocks_without_separ assert pr_verifier._coerce_response_content(blocks) == encoded +def test_text_from_response_content_preserves_blank_blocks_between_text() -> None: + blocks = [ + {"type": "text", "text": '{"summary":"not'}, + {"type": "text", "text": " "}, + {"type": "text", "text": 'safe"}'}, + ] + + assert pr_verifier._text_from_response_content(blocks) == '{"summary":"not safe"}' + + def test_text_from_response_content_ignores_blank_text_blocks() -> None: blocks = [ {"type": "text", "text": " \n"},