From adb793bee1ba4a994f2bcc75c517a0080d778a76 Mon Sep 17 00:00:00 2001 From: JD Davis Date: Fri, 26 Jun 2026 23:34:34 -0500 Subject: [PATCH] ci: harden PR governance and model cache checks (#1401) ## Description Hardens two routine PR-review pain points from the recent open-PR sweep: - PR Governance reruns could keep validating the stale `pull_request_target` event body even after the live PR description had been fixed. - Main CI model-cache misses could surface as dozens of unrelated memory-test failures instead of one clear cache-preflight failure. This intentionally avoids PyPI/package-bloat and release/nightly workflow changes so the PR stays scoped to review and CI stabilization. ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Documentation - [x] Refactor - [x] Tests only ## Changes Made - Added `--body-file` support to `scripts/pr-governance.py` so workflows can validate the current PR body rather than stale rerun payloads. - Updated PR Governance to fetch the live PR body via the GitHub API before validating template fields. - Added a CI preflight script that loads the default sentence-transformer model in offline mode and verifies the expected embedding dimension. - Wired that preflight into the sharded CI job before pytest starts, turning missing/corrupt Hugging Face caches into one early, actionable failure. - Added workflow/script regression tests for the live-body override and model-cache preflight placement. ## Testing - [x] Unit tests - [x] Lint/static checks - [ ] Integration tests - [ ] Manual testing ### Test Output ```text uv run --with pytest --with pytest-asyncio python -m pytest scripts/tests/test_pr_governance.py scripts/tests/test_pr_health_workflow.py scripts/tests/test_ci_workflow.py -q 9 passed in 0.04s uv run ruff check scripts/pr-governance.py scripts/ci/verify_hf_model_cache.py scripts/tests/test_pr_governance.py scripts/tests/test_pr_health_workflow.py scripts/tests/test_ci_workflow.py All checks passed! python -m py_compile scripts\ci\verify_hf_model_cache.py scripts\pr-governance.py # passed ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.13.3, isolated worktree `C:\git\headroom\.worktrees\stabilization-hardening`. - Exact command / steps: Ran the focused governance/workflow tests, ruff on touched Python files, and `py_compile` for the executable scripts. - Observed result: Governance tests prove a stale event body can be overridden by the live PR body; workflow tests prove CI validates live PR body and runs the Hugging Face offline-cache preflight before pytest shards. - Not tested: Full GitHub CI before PR creation; that will run on this PR. The new Hugging Face preflight itself is intentionally not run locally because it depends on the CI-warmed offline model cache. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review --- .github/workflows/ci.yml | 7 +++ .github/workflows/pr-health.yml | 22 +++++++--- scripts/ci/verify_hf_model_cache.py | 56 ++++++++++++++++++++++++ scripts/pr-governance.py | 31 ++++++++++++- scripts/tests/test_ci_workflow.py | 16 +++++++ scripts/tests/test_pr_governance.py | 41 +++++++++++++++++ scripts/tests/test_pr_health_workflow.py | 2 + 7 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 scripts/ci/verify_hf_model_cache.py create mode 100644 scripts/tests/test_ci_workflow.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b2a542f25..db769a540 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -210,6 +210,13 @@ jobs: cp "${SITE}/headroom/"_core*.so headroom/ python -c "from headroom._core import DiffCompressor; print('headroom._core OK')" + - name: Verify offline HuggingFace model cache + env: + HF_HUB_OFFLINE: "1" + TRANSFORMERS_OFFLINE: "1" + HF_HUB_DISABLE_TELEMETRY: "1" + run: python scripts/ci/verify_hf_model_cache.py + # Coverage upload: without this, codecov only receives reports from # the two native-e2e workflows (3 CLI test files total), so head # coverage reads ~6% and codecov/patch fails for ANY diff not diff --git a/.github/workflows/pr-health.yml b/.github/workflows/pr-health.yml index 4cd0ec6e7..ee274bbd2 100644 --- a/.github/workflows/pr-health.yml +++ b/.github/workflows/pr-health.yml @@ -25,13 +25,21 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 steps: - - uses: actions/checkout@v6 - with: - ref: ${{ github.event.pull_request.base.sha }} - - - name: Validate PR template - id: validate - run: python3 scripts/pr-governance.py --event "$GITHUB_EVENT_PATH" --report .pr-governance-report.json + - uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.base.sha }} + + - name: Fetch current PR body + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + gh api "repos/${REPO}/pulls/${PR_NUMBER}" --jq '.body // ""' > .pr-body.md + + - name: Validate PR template + id: validate + run: python3 scripts/pr-governance.py --event "$GITHUB_EVENT_PATH" --body-file .pr-body.md --report .pr-governance-report.json - name: Append governance summary run: | diff --git a/scripts/ci/verify_hf_model_cache.py b/scripts/ci/verify_hf_model_cache.py new file mode 100644 index 000000000..6750ab7ac --- /dev/null +++ b/scripts/ci/verify_hf_model_cache.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +"""Verify that CI can load the default embedding model offline. + +The main test shards run with TRANSFORMERS_OFFLINE=1. If the Hugging Face cache +misses or is partially restored, many unrelated memory tests fail later with +network/cache errors. This preflight keeps that failure mode early and specific. +""" + +from __future__ import annotations + +import os +import sys + + +def main() -> int: + os.environ.setdefault("HF_HUB_OFFLINE", "1") + os.environ.setdefault("TRANSFORMERS_OFFLINE", "1") + os.environ.setdefault("HF_HUB_DISABLE_TELEMETRY", "1") + + from headroom.models.config import ML_MODEL_DEFAULTS + + model_name = ML_MODEL_DEFAULTS.sentence_transformer + expected_dim = ML_MODEL_DEFAULTS.sentence_transformer_dim + + try: + from sentence_transformers import SentenceTransformer + + model = SentenceTransformer(model_name, local_files_only=True) + embedding = model.encode(["headroom cache preflight"], convert_to_numpy=True) + except Exception as exc: + print( + f"::error::Hugging Face offline model cache is not usable for {model_name!r}: {exc}", + file=sys.stderr, + ) + print( + "The prefetch-model job or fallback download must populate " + "~/.cache/huggingface before offline test shards run.", + file=sys.stderr, + ) + return 1 + + actual_dim = int(embedding.shape[-1]) + if actual_dim != expected_dim: + print( + "::error::Loaded embedding model has unexpected dimension: " + f"{actual_dim} != {expected_dim}", + file=sys.stderr, + ) + return 1 + + print(f"offline Hugging Face model cache OK: {model_name} ({actual_dim} dims)") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/pr-governance.py b/scripts/pr-governance.py index 9f4d426f5..cfda86b3a 100644 --- a/scripts/pr-governance.py +++ b/scripts/pr-governance.py @@ -261,6 +261,24 @@ def validate_pull_request(event: dict[str, Any]) -> GovernanceReport: ) +def validate_pull_request_body(event: dict[str, Any], body: str | None = None) -> GovernanceReport: + """Validate a PR event, optionally replacing the event payload body. + + GitHub reruns use the original event payload. That makes a governance rerun + keep validating an old PR body even after maintainers fix the live body. + The workflow fetches the current body via the API and passes it here so the + check reflects what reviewers see on the PR page. + """ + if body is None: + return validate_pull_request(event) + + event_copy = dict(event) + pull_request = dict(event["pull_request"]) + pull_request["body"] = body + event_copy["pull_request"] = pull_request + return validate_pull_request(event_copy) + + def emit_outputs(report: GovernanceReport) -> None: output_path = os.environ.get("GITHUB_OUTPUT") lines = [ @@ -284,13 +302,24 @@ def parse_args(argv: list[str]) -> argparse.Namespace: parser.add_argument( "--event", type=Path, required=True, help="Path to the GitHub event payload JSON." ) + parser.add_argument( + "--body-file", + type=Path, + help=( + "Optional file containing the current PR body. Use this in GitHub Actions " + "so reruns validate the live PR body instead of the stale event payload." + ), + ) parser.add_argument("--report", type=Path, required=True, help="Path to write the JSON report.") return parser.parse_args(argv) def main(argv: list[str] | None = None) -> int: args = parse_args(argv or sys.argv[1:]) - report = validate_pull_request(load_event(args.event)) + body_override = ( + args.body_file.read_text(encoding="utf-8") if args.body_file is not None else None + ) + report = validate_pull_request_body(load_event(args.event), body_override) args.report.write_text(json.dumps(report.to_dict(), indent=2), encoding="utf-8") emit_outputs(report) return 0 diff --git a/scripts/tests/test_ci_workflow.py b/scripts/tests/test_ci_workflow.py new file mode 100644 index 000000000..20c873d46 --- /dev/null +++ b/scripts/tests/test_ci_workflow.py @@ -0,0 +1,16 @@ +"""Tests for CI workflow hardening contracts.""" + +from __future__ import annotations + +from pathlib import Path + + +def test_sharded_ci_verifies_offline_huggingface_cache_before_pytest() -> None: + workflow = Path(".github/workflows/ci.yml").read_text(encoding="utf-8") + + verify_step = "Verify offline HuggingFace model cache" + pytest_step = "Run test shard ${{ matrix.shard }}/4" + + assert verify_step in workflow + assert "python scripts/ci/verify_hf_model_cache.py" in workflow + assert workflow.index(verify_step) < workflow.index(pytest_step) diff --git a/scripts/tests/test_pr_governance.py b/scripts/tests/test_pr_governance.py index d913d0d00..810edd0e3 100644 --- a/scripts/tests/test_pr_governance.py +++ b/scripts/tests/test_pr_governance.py @@ -3,6 +3,7 @@ from __future__ import annotations import importlib.util +import json import sys from pathlib import Path @@ -97,6 +98,46 @@ def test_validate_pull_request_accepts_crlf_test_output_code_block() -> None: assert report.problems == [] +def test_validate_pull_request_body_override_uses_live_body() -> None: + module = _load_module() + stale_event_body = "" + + report = module.validate_pull_request_body(_event(stale_event_body), VALID_BODY) + + assert report.valid is True + assert report.ready_for_review is True + assert report.problems == [] + + +def test_cli_body_file_override_uses_live_body(tmp_path: Path, monkeypatch) -> None: + module = _load_module() + event_path = tmp_path / "event.json" + body_path = tmp_path / "body.md" + report_path = tmp_path / "report.json" + event_path.write_text( + json.dumps(_event("")), + encoding="utf-8", + ) + body_path.write_text(VALID_BODY, encoding="utf-8") + monkeypatch.delenv("GITHUB_OUTPUT", raising=False) + + exit_code = module.main( + [ + "--event", + str(event_path), + "--body-file", + str(body_path), + "--report", + str(report_path), + ] + ) + + assert exit_code == 0 + report = json.loads(report_path.read_text(encoding="utf-8")) + assert report["valid"] is True + assert report["ready_for_review"] is True + + def test_validate_pull_request_allows_draft_without_ready_checkboxes() -> None: module = _load_module() body = VALID_BODY.replace( diff --git a/scripts/tests/test_pr_health_workflow.py b/scripts/tests/test_pr_health_workflow.py index 9920397ad..24e13f22a 100644 --- a/scripts/tests/test_pr_health_workflow.py +++ b/scripts/tests/test_pr_health_workflow.py @@ -8,6 +8,8 @@ from pathlib import Path def test_incomplete_pr_template_is_reported_without_failing_job() -> None: workflow = Path(".github/workflows/pr-health.yml").read_text(encoding="utf-8") + assert "Fetch current PR body" in workflow + assert "--body-file .pr-body.md" in workflow assert "Report incomplete PR body" in workflow assert "PR template validation found missing fields" in workflow assert "Fail when the PR body is incomplete" not in workflow