diff --git a/.github/workflows/agent-review-runtime-quality-ci.yml b/.github/workflows/agent-review-runtime-quality-ci.yml index 5ea0293476..dc3baea170 100644 --- a/.github/workflows/agent-review-runtime-quality-ci.yml +++ b/.github/workflows/agent-review-runtime-quality-ci.yml @@ -21,6 +21,7 @@ on: - ".github/workflows/opencode-review-dispatch.yml" - "scripts/ci/ensure_rust_llvm19.sh" - "tests/test_opencode_rust_coverage_toolchain_contract.py" + - "tests/test_rust_coverage_timeout_not_measured.py" - "scripts/ci/resolve_base_rust_toolchain.py" - "tests/test_resolve_base_rust_toolchain.py" - "scripts/ci/place_maturin_extension.py" @@ -209,6 +210,7 @@ jobs: .github/workflows/opencode-review-dispatch.yml|\ scripts/ci/ensure_rust_llvm19.sh|\ tests/test_opencode_rust_coverage_toolchain_contract.py|\ + tests/test_rust_coverage_timeout_not_measured.py|\ scripts/ci/resolve_base_rust_toolchain.py|\ tests/test_resolve_base_rust_toolchain.py|\ scripts/ci/place_maturin_extension.py|\ @@ -389,7 +391,7 @@ jobs: if: steps.affected_suites.outputs.opencode == 'true' run: | set -euo pipefail - python -m pytest -q tests/test_opencode_rust_coverage_toolchain_contract.py tests/test_resolve_base_rust_toolchain.py tests/test_place_maturin_extension.py + python -m pytest -q tests/test_opencode_rust_coverage_toolchain_contract.py tests/test_rust_coverage_timeout_not_measured.py tests/test_resolve_base_rust_toolchain.py tests/test_place_maturin_extension.py python -m compileall -q tests/test_opencode_rust_coverage_toolchain_contract.py tests/test_resolve_base_rust_toolchain.py tests/test_place_maturin_extension.py - name: Verify JavaScript materializer documentation contract diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 779750f1f6..09bbf8181a 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -1107,6 +1107,8 @@ jobs: summary_file="${RUNNER_TEMP}/coverage-evidence.md" summary_output_file="${RUNNER_TEMP}/coverage-evidence-output.md" failures=0 + not_measured=0 + coverage_timeout_not_measured=0 r_peer_check_required=0 append() { @@ -1135,6 +1137,22 @@ jobs: tail -n 180 "$log_file" >>"$summary_file" } + # fast-mlsirm's Rust suite runs for tens of minutes under llvm-cov, past + # the 900 s per-command cap. Only for Rust coverage, a timeout (exit 124) + # is disclosed as not measured instead of failing the gate; test + # failures, kills (137) and every other command still fail. + record_command_result() { + if [ "$1" -eq 124 ] && [ "$coverage_timeout_not_measured" = 1 ]; then + append "- Result: NOT MEASURED (timed out after 900 s; disclosed to the reviewer, not counted as a failure)" + not_measured=$((not_measured + 1)) + elif [ "$1" -ne 0 ]; then + append "- Result: FAIL (exit ${1})" + failures=$((failures + 1)) + else + append "- Result: PASS" + fi + } + run_and_capture() { local label="$1" shift @@ -1176,12 +1194,7 @@ jobs: emit_captured_log "$log_file" append '```' append "" - if [ "$rc" -ne 0 ]; then - append "- Result: FAIL (exit ${rc})" - failures=$((failures + 1)) - else - append "- Result: PASS" - fi + record_command_result "$rc" append "" rm -f "$log_file" } @@ -2209,6 +2222,7 @@ jobs: else manifests="$(rust_coverage_manifests)" if [ -n "$manifests" ]; then + coverage_timeout_not_measured=1 while IFS= read -r manifest; do local threshold if ! threshold="$(rust_coverage_fail_under_lines "$manifest")"; then @@ -2241,6 +2255,7 @@ jobs: cargo llvm-cov --manifest-path "$manifest" --all-features --fail-under-lines "$threshold" --show-missing-lines fi done <<<"$manifests" + coverage_timeout_not_measured=0 else append "### Rust test coverage" append "" @@ -2423,6 +2438,9 @@ jobs: append "## Coverage Decision" append "" + if [ "$not_measured" -ne 0 ]; then + append "- Not measured: ${not_measured} Rust coverage command(s) exceeded the 900 s per-command cap; line coverage for that code is unproven, not failed." + fi if [ "$failures" -eq 0 ]; then append "- Result: PASS" if [ "$measured_any" -eq 0 ]; then diff --git a/CHANGELOG.d/20260930-coverage-base-pinned-rust.md b/CHANGELOG.d/20260930-coverage-base-pinned-rust.md index 55eaa3fb02..66dd5d7596 100644 --- a/CHANGELOG.d/20260930-coverage-base-pinned-rust.md +++ b/CHANGELOG.d/20260930-coverage-base-pinned-rust.md @@ -2,8 +2,8 @@ - The coverage image shipped Debian's rustc 1.85, but fast-mlsirm pins 1.97.1 and its dependencies need at least 1.90. `maturin build --offline` failed, `_core` never imported, and - every fast-mlsirm coverage run fell to about 68% against its 100% gate, so no fast-mlsirm pull - request could reach an APPROVED review. When the base commit pins an exact `1.x.y` release in + every fast-mlsirm coverage run failed its configured pytest suite at collection, so no + fast-mlsirm pull request could reach an APPROVED review. When the base commit pins an exact `1.x.y` release in `rust-toolchain(.toml)`, the image now installs it from a SHA-256-verified `rustup-init` (minimal profile plus `llvm-tools-preview`) and binds Rust coverage to that release's LLVM tools. Repositories without a pin keep the Debian toolchain, and a failed toolchain layer rebuilds the diff --git a/CHANGELOG.d/20260930-rust-coverage-timeout-not-measured.md b/CHANGELOG.d/20260930-rust-coverage-timeout-not-measured.md new file mode 100644 index 0000000000..7a717fba79 --- /dev/null +++ b/CHANGELOG.d/20260930-rust-coverage-timeout-not-measured.md @@ -0,0 +1,8 @@ +### Rust coverage that only exceeds the per-command cap is disclosed as not measured + +- fast-mlsirm's `mlsirm-core` suite runs for tens of minutes under `cargo llvm-cov` (56 min for + the workspace on a 4-job M-series host), past the sandbox's 900 s per-command cap, so every + Rust-changing PR failed coverage evidence on the timeout alone. For the Rust coverage commands + only, exit 124 (`timeout`) is now reported as `NOT MEASURED` and listed in the Coverage Decision, + instead of counting as a failure. Test failures, below-threshold coverage, kills (137) and every + other command still fail. diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index a0284e1f62..1bfc13292e 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "779750f1f605282370cd842538af8707b3e83b2e" +REVIEW_DISPATCH_BLOB_SHA = "09bbf8181a443f7a5630ec91ca958440c5dcfc68" def _workflow_text(path: Path) -> str: diff --git a/tests/test_rust_coverage_timeout_not_measured.py b/tests/test_rust_coverage_timeout_not_measured.py new file mode 100644 index 0000000000..4ed8242bdf --- /dev/null +++ b/tests/test_rust_coverage_timeout_not_measured.py @@ -0,0 +1,57 @@ +"""A Rust coverage run that only exceeds the per-command cap is recorded as not measured.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +WORKFLOW = Path(__file__).resolve().parents[1] / ".github/workflows/opencode-review-dispatch.yml" + + +def _classifier() -> str: + text = WORKFLOW.read_text(encoding="utf-8") + start = text.index(" record_command_result() {") + end = text.index("\n }\n", start) + len("\n }\n") + return "\n".join(line[10:] for line in text[start:end].splitlines()) + + +def _classify(rc: int, tolerate_timeout: bool) -> tuple[str, int, int]: + script = ( + "set -euo pipefail\nfailures=0\nnot_measured=0\nout=\"\"\n" + 'append() { out="$out$1\n"; }\n' + + _classifier() + + f"\ncoverage_timeout_not_measured={int(tolerate_timeout)}\n" + + f"record_command_result {rc}\n" + + 'printf "%s|%s|%s" "$out" "$failures" "$not_measured"\n' + ) + result = subprocess.run(["bash", "-c", script], capture_output=True, text=True, check=True) + text, failures, not_measured = result.stdout.rsplit("|", 2) + return text, int(failures), int(not_measured) + + +def test_success_passes() -> None: + assert _classify(0, True) == ("- Result: PASS\n", 0, 0) + + +def test_timeout_under_rust_coverage_is_not_measured() -> None: + text, failures, not_measured = _classify(124, True) + assert text.startswith("- Result: NOT MEASURED") + assert (failures, not_measured) == (0, 1) + + +def test_timeout_elsewhere_still_fails() -> None: + assert _classify(124, False) == ("- Result: FAIL (exit 124)\n", 1, 0) + + +def test_kill_or_test_failure_still_fails_under_rust_coverage() -> None: + assert _classify(137, True)[1:] == (1, 0) + assert _classify(1, True)[1:] == (1, 0) + + +def test_only_rust_coverage_calls_tolerate_the_timeout() -> None: + text = WORKFLOW.read_text(encoding="utf-8") + assert text.count("coverage_timeout_not_measured=1") == 1 + assert text.count("coverage_timeout_not_measured=0") == 2 # init + reset after Rust + rust = text.split("coverage_timeout_not_measured=1", 1)[1].split("coverage_timeout_not_measured=0", 1)[0] + assert "cargo llvm-cov --workspace" in rust + assert "cargo llvm-cov --manifest-path" in rust