From bef29b4b8b2d62e047c1f2ddafec01ef7786cbd8 Mon Sep 17 00:00:00 2001 From: Francisco <95440147+frarredondo@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:36:48 -0700 Subject: [PATCH 1/2] Add sticky coverage comment and gate statements plus branches. Split a coverage-ratchet CI check, post project (whole-package) metrics on PRs, and enforce statement/branch floors while leaving combined informational. --- .github/workflows/ci.yml | 49 +++++++-- CONTRIBUTING.md | 23 ++-- README.md | 2 +- codecov.yml | 3 +- pyproject.toml | 4 +- scripts/check_coverage_ratchet.py | 157 ++++++++++++++++++++++----- tests/test_check_coverage_ratchet.py | 121 +++++++++++++++++++++ 7 files changed, 311 insertions(+), 48 deletions(-) create mode 100644 tests/test_check_coverage_ratchet.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a195b67..79ece9d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,10 @@ concurrency: group: ci-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +permissions: + contents: read + pull-requests: write + jobs: test: name: test @@ -33,20 +37,13 @@ jobs: - name: Run unit tests with coverage run: > - uv run pytest + uv run python -m pytest --cov=podtx --cov-branch --cov-report=term-missing --cov-report=xml:coverage.xml --cov-report=json:coverage.json - - name: Enforce statement coverage ratchet - env: - # Repo Actions variable (Settings → Secrets and variables → Actions → Variables). - # Falls back to 65 in the script if unset. - COVERAGE_RATCHET_MIN: ${{ vars.COVERAGE_RATCHET_MIN }} - run: uv run python scripts/check_coverage_ratchet.py - - name: Upload coverage to Codecov uses: codecov/codecov-action@v5 with: @@ -55,3 +52,39 @@ jobs: name: unit token: ${{ secrets.CODECOV_TOKEN }} fail_ci_if_error: true + + - name: Upload coverage.json for ratchet job + uses: actions/upload-artifact@v4 + with: + name: coverage-json + path: coverage.json + if-no-files-found: error + + coverage-ratchet: + name: coverage-ratchet + needs: test + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Download coverage.json + uses: actions/download-artifact@v4 + with: + name: coverage-json + + - name: Enforce coverage ratchet (statements + branches) + id: ratchet + env: + # Repo Actions variables (Settings → Secrets and variables → Actions → Variables). + COVERAGE_RATCHET_MIN: ${{ vars.COVERAGE_RATCHET_MIN }} + COVERAGE_RATCHET_MIN_BRANCHES: ${{ vars.COVERAGE_RATCHET_MIN_BRANCHES }} + run: python3 scripts/check_coverage_ratchet.py + + - name: Sticky coverage PR comment + if: always() && github.event_name == 'pull_request' && hashFiles('coverage-ratchet.md') != '' + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: podtx-coverage-ratchet + path: coverage-ratchet.md + recreate: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6f842f8..4d9c9d4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,17 +18,24 @@ uv run pytest ### CI -Pull requests and pushes to `main` run the **`test`** GitHub Actions job (`.github/workflows/ci.yml`): +Pull requests and pushes to `main` run two GitHub Actions jobs (`.github/workflows/ci.yml`): -1. `uv sync --extra dev` -2. `uv run pytest` with line + branch coverage (`pytest-cov`) -3. Statement-coverage ratchet (`scripts/check_coverage_ratchet.py`), floor from repo Actions variable **`COVERAGE_RATCHET_MIN`** (default **65** if unset) -4. Upload `coverage.xml` to Codecov (badge in README) +1. **`test`** — `uv sync --extra dev`, then `pytest` with line + branch coverage; upload `coverage.xml` to Codecov +2. **`coverage-ratchet`** — project coverage floors for package `podtx` (whole package on the branch tip, **not** Codecov patch/diff coverage): + - Statements ≥ **`COVERAGE_RATCHET_MIN`** (default **65**) + - Branches ≥ **`COVERAGE_RATCHET_MIN_BRANCHES`** (default **45**) + - Combined is reported only (not gated) + - On pull requests, posts/updates a sticky comment with the summary -ML extras (`parakeet` / `whisper`) are not installed in CI; unit tests do not require them. Branch coverage is reported to Codecov but is not gated yet. +ML extras (`parakeet` / `whisper`) are not installed in CI; unit tests do not require them. -Raise the ratchet over time by updating **`COVERAGE_RATCHET_MIN`** under *Settings → Secrets and variables → Actions → Variables* (no code change required). Locally: `COVERAGE_RATCHET_MIN=65 uv run python scripts/check_coverage_ratchet.py`. +Raise floors over time under *Settings → Secrets and variables → Actions → Variables* (no code change required). Locally: + +```bash +COVERAGE_RATCHET_MIN=65 COVERAGE_RATCHET_MIN_BRANCHES=45 \ + uv run python scripts/check_coverage_ratchet.py +``` Repository secret `CODECOV_TOKEN` is required for Codecov uploads on protected branches. -Once the workflow has run at least once on `main`, you can mark **`test`** as a required status check on the Protect main ruleset. +Once the workflow has run at least once on `main`, mark **`test`** and **`coverage-ratchet`** as required status checks on the Protect main ruleset. diff --git a/README.md b/README.md index 0dae700..74b1a13 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,6 @@ uv sync --extra all --extra dev uv run pytest ``` -CI runs the unit suite with coverage on pull requests via the GitHub Actions job **`test`** (see [CONTRIBUTING.md](CONTRIBUTING.md)). Statement coverage is ratcheted via repo variable **`COVERAGE_RATCHET_MIN`** (default 65%); branch coverage is reported but not gated yet. +CI runs the unit suite with coverage on pull requests via jobs **`test`** and **`coverage-ratchet`** (see [CONTRIBUTING.md](CONTRIBUTING.md)). Project statement/branch floors come from repo variables **`COVERAGE_RATCHET_MIN`** (default 65%) and **`COVERAGE_RATCHET_MIN_BRANCHES`** (default 45%); combined coverage is informational. PRs also get a sticky coverage comment (whole package, not patch). See [CONTRIBUTING.md](CONTRIBUTING.md) for how to file bugs and feature requests. diff --git a/codecov.yml b/codecov.yml index c9a000d..764f5f4 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,5 +1,6 @@ # Codecov config: https://docs.codecov.com/docs/codecov-yaml -# CI enforces a statement-coverage ratchet locally; Codecov statuses are informational. +# CI enforces project statement + branch ratchets locally (sticky PR comment); +# Codecov statuses remain informational (including patch coverage). coverage: status: diff --git a/pyproject.toml b/pyproject.toml index 2f7771e..92e1c46 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,5 +52,5 @@ relative_files = true show_missing = true skip_covered = false precision = 0 -# Line/statement ratchet is enforced by scripts/check_coverage_ratchet.py -# (branch coverage is measured and uploaded, but not gated yet). +# Project statement + branch ratchets: scripts/check_coverage_ratchet.py +# (combined coverage is reported but not gated). diff --git a/scripts/check_coverage_ratchet.py b/scripts/check_coverage_ratchet.py index 24c3e0f..5bec7ab 100644 --- a/scripts/check_coverage_ratchet.py +++ b/scripts/check_coverage_ratchet.py @@ -1,11 +1,16 @@ #!/usr/bin/env python3 -"""Fail if statement (line) coverage drops below the ratchet floor. +"""Fail if project coverage drops below configured ratchet floors. -Branch coverage is measured in CI and uploaded to Codecov, but is not gated -here yet. +Gates: + - statement (line) coverage vs ``COVERAGE_RATCHET_MIN`` (default 65) + - branch coverage vs ``COVERAGE_RATCHET_MIN_BRANCHES`` (default 45) -The floor comes from env ``COVERAGE_RATCHET_MIN`` (GitHub Actions repo variable -in CI). Default is 65 to match the 2026-08 unit-suite baseline without ML extras. +Combined coverage is reported but not gated. + +Metrics are **project** coverage for package ``podtx`` on the tested commit +(whole package), not Codecov patch / diff coverage. + +Also writes ``coverage-ratchet.md`` for the CI sticky PR comment. """ from __future__ import annotations @@ -15,25 +20,120 @@ import sys from pathlib import Path -_DEFAULT_RATCHET_MIN = 65.0 +_DEFAULT_STATEMENT_MIN = 65.0 +_DEFAULT_BRANCH_MIN = 45.0 +_MARKDOWN_PATH = Path("coverage-ratchet.md") -def _ratchet_min() -> float: - raw = os.environ.get("COVERAGE_RATCHET_MIN", "").strip() +def _env_float(name: str, default: float) -> float: + raw = os.environ.get(name, "").strip() if not raw: - return _DEFAULT_RATCHET_MIN + return default try: return float(raw) except ValueError: print( - f"error: COVERAGE_RATCHET_MIN must be a number, got {raw!r}", + f"error: {name} must be a number, got {raw!r}", file=sys.stderr, ) raise SystemExit(2) from None +def _summary_line( + *, + statements: float, + statement_floor: float, + branches: float, + branch_floor: float, + combined: float, +) -> str: + return ( + f"statements={statements:.2f}% (min {statement_floor:.0f}%) | " + f"branches={branches:.2f}% (min {branch_floor:.0f}%) | " + f"combined={combined:.2f}% (informational)" + ) + + +def write_markdown_report( + path: Path, + *, + statements: float, + statement_floor: float, + branches: float, + branch_floor: float, + combined: float, + passed: bool, +) -> None: + """Write sticky-comment markdown for project (whole-package) coverage.""" + line = _summary_line( + statements=statements, + statement_floor=statement_floor, + branches=branches, + branch_floor=branch_floor, + combined=combined, + ) + status = "passed" if passed else "failed" + path.write_text( + "\n".join( + [ + "## Coverage ratchet (project)", + "", + "Whole package (`podtx`) on this PR branch — " + "**not** Codecov patch / diff coverage.", + "", + f"`{line}`", + "", + "| Metric | Value | Role |", + "|--------|------:|------|", + f"| Statements | {statements:.2f}% | Gated (min {statement_floor:.0f}%) |", + f"| Branches | {branches:.2f}% | Gated (min {branch_floor:.0f}%) |", + f"| Combined | {combined:.2f}% | Informational |", + "", + f"**Status:** {status}", + "", + ] + ), + encoding="utf-8", + ) + + +def evaluate( + totals: dict[str, object], + *, + statement_floor: float, + branch_floor: float, +) -> tuple[bool, str, list[str]]: + """Return (passed, summary_line, error_messages).""" + statements = float(totals["percent_statements_covered"]) # type: ignore[arg-type] + branches = float(totals.get("percent_branches_covered") or 0.0) # type: ignore[arg-type] + combined = float(totals.get("percent_covered") or 0.0) # type: ignore[arg-type] + + line = _summary_line( + statements=statements, + statement_floor=statement_floor, + branches=branches, + branch_floor=branch_floor, + combined=combined, + ) + errors: list[str] = [] + if statements < statement_floor: + errors.append( + f"error: statement coverage {statements:.2f}% is below ratchet " + f"{statement_floor:.0f}%" + ) + if branches < branch_floor: + errors.append( + f"error: branch coverage {branches:.2f}% is below ratchet " + f"{branch_floor:.0f}%" + ) + return (not errors, line, errors) + + def main() -> int: - floor = _ratchet_min() + statement_floor = _env_float("COVERAGE_RATCHET_MIN", _DEFAULT_STATEMENT_MIN) + branch_floor = _env_float( + "COVERAGE_RATCHET_MIN_BRANCHES", _DEFAULT_BRANCH_MIN + ) report = Path("coverage.json") if not report.is_file(): print( @@ -44,25 +144,26 @@ def main() -> int: data = json.loads(report.read_text(encoding="utf-8")) totals = data["totals"] - statements = float(totals["percent_statements_covered"]) - branches = float(totals.get("percent_branches_covered") or 0.0) - combined = float(totals.get("percent_covered") or 0.0) - - print( - f"coverage ratchet: statements={statements:.2f}% " - f"(min {floor:.0f}%) | " - f"branches={branches:.2f}% (informational) | " - f"combined={combined:.2f}% (informational)" + passed, line, errors = evaluate( + totals, + statement_floor=statement_floor, + branch_floor=branch_floor, ) - if statements < floor: - print( - f"error: statement coverage {statements:.2f}% is below ratchet " - f"{floor:.0f}%", - file=sys.stderr, - ) - return 1 - return 0 + print(f"coverage ratchet: {line}") + write_markdown_report( + _MARKDOWN_PATH, + statements=float(totals["percent_statements_covered"]), + statement_floor=statement_floor, + branches=float(totals.get("percent_branches_covered") or 0.0), + branch_floor=branch_floor, + combined=float(totals.get("percent_covered") or 0.0), + passed=passed, + ) + + for message in errors: + print(message, file=sys.stderr) + return 0 if passed else 1 if __name__ == "__main__": diff --git a/tests/test_check_coverage_ratchet.py b/tests/test_check_coverage_ratchet.py new file mode 100644 index 0000000..2bb4e67 --- /dev/null +++ b/tests/test_check_coverage_ratchet.py @@ -0,0 +1,121 @@ +from __future__ import annotations + +import importlib.util +import json +import sys +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +_SCRIPT = _ROOT / "scripts" / "check_coverage_ratchet.py" + + +def _load_ratchet(): + spec = importlib.util.spec_from_file_location("check_coverage_ratchet", _SCRIPT) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +ratchet = _load_ratchet() + + +def _totals( + *, + statements: float, + branches: float, + combined: float, +) -> dict[str, float]: + return { + "percent_statements_covered": statements, + "percent_branches_covered": branches, + "percent_covered": combined, + } + + +def test_evaluate_passes_when_both_above_floor() -> None: + passed, line, errors = ratchet.evaluate( + _totals(statements=70.0, branches=50.0, combined=65.0), + statement_floor=65.0, + branch_floor=45.0, + ) + assert passed is True + assert errors == [] + assert "statements=70.00% (min 65%)" in line + assert "branches=50.00% (min 45%)" in line + assert "combined=65.00% (informational)" in line + + +def test_evaluate_fails_on_statement_or_branch_floor() -> None: + passed, _, errors = ratchet.evaluate( + _totals(statements=64.0, branches=50.0, combined=60.0), + statement_floor=65.0, + branch_floor=45.0, + ) + assert passed is False + assert any("statement coverage" in e for e in errors) + + passed2, _, errors2 = ratchet.evaluate( + _totals(statements=70.0, branches=40.0, combined=60.0), + statement_floor=65.0, + branch_floor=45.0, + ) + assert passed2 is False + assert any("branch coverage" in e for e in errors2) + + +def test_write_markdown_report_mentions_project_not_patch(tmp_path: Path) -> None: + path = tmp_path / "coverage-ratchet.md" + ratchet.write_markdown_report( + path, + statements=69.35, + statement_floor=65.0, + branches=53.45, + branch_floor=45.0, + combined=65.49, + passed=True, + ) + text = path.read_text(encoding="utf-8") + assert "Whole package (`podtx`)" in text + assert "Codecov patch" in text + assert "statements=69.35% (min 65%)" in text + assert "branches=53.45% (min 45%)" in text + assert "**Status:** passed" in text + + +def test_main_writes_markdown_and_respects_env( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + report = tmp_path / "coverage.json" + report.write_text( + json.dumps( + { + "totals": _totals( + statements=70.0, branches=50.0, combined=65.0 + ) + } + ), + encoding="utf-8", + ) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("COVERAGE_RATCHET_MIN", "65") + monkeypatch.setenv("COVERAGE_RATCHET_MIN_BRANCHES", "45") + monkeypatch.setattr(ratchet, "_MARKDOWN_PATH", tmp_path / "coverage-ratchet.md") + + assert ratchet.main() == 0 + assert (tmp_path / "coverage-ratchet.md").is_file() + + report.write_text( + json.dumps( + { + "totals": _totals( + statements=70.0, branches=10.0, combined=50.0 + ) + } + ), + encoding="utf-8", + ) + assert ratchet.main() == 1 From ea1f618ff52cd1092ad24aacc410459857079071 Mon Sep 17 00:00:00 2001 From: Francisco <95440147+frarredondo@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:39:14 -0700 Subject: [PATCH 2/2] Keep the coverage ratchet PR comment sticky in place. Drop recreate so the action updates the existing comment instead of deleting it and posting a new one at the bottom. --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79ece9d..de79f9f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,6 +85,8 @@ jobs: if: always() && github.event_name == 'pull_request' && hashFiles('coverage-ratchet.md') != '' uses: marocchino/sticky-pull-request-comment@v2 with: + # Update the same comment in place (do not recreate — that posts a new + # comment at the bottom and is not sticky). header: podtx-coverage-ratchet path: coverage-ratchet.md - recreate: true + skip_unchanged: true