Stop the coverage-map bot pushing a no-op commit on every refresh

[sbryngelson]

Description

coverage-refresh.yml has been pushing a test: refresh coverage map [skip ci] commit on nearly every master push (7/17, 7/19, 7/20 ×2, 7/21, 7/23, 7/25, 7/26 …). It has a git diff --quiet tests/coverage_map.json.gz guard, but that guard can never pass. Three independent causes:

1. param_hash embedded the runner's absolute checkout path. TestCaseBuilder.to_case() absolutizes file-valued params so a test runs from any cwd. For the 8 STL cases that puts the checkout path into stl_models(1)%model_filepath, and hence into the key. Cloning master into two directories and computing all 651 keys from each:

BEFORE: 8 of 651 tests hash differently across the two checkouts
          2D -> IBM -> STL, 3D -> IBM -> STL, 3D -> ICPP -> STL, ...
AFTER:  0 of 651

Those 8 entries churned out of the map on every refresh. In 4 of the last 7 map commits, 100% of added keys had byte-identical coverage lists to a removed key — the same tests, rehashed. They also mismatched on PR runners, dropping those tests to rung 5 (unmapped → always run). coverage_key() now hashes the repo-relative form.

2. save_map wrote a non-deterministic gzip header. gzip.open() stamps the current time and source filename, so identical maps still produced different bytes. Header bytes from consecutive commits: ...088f6b5f6a..., ...08486a5a6a..., ...08c5bb5c6a.... Now mtime=0, empty filename.

3. The guard compared bytes, not coverage. _meta (built_at, git_sha) moves on every rebuild, so git diff always reported a change. Now compared via .github/scripts/coverage_map_changed.py (exit 0 = changed, 10 = unchanged, anything else = error).

unchanged is deliberately not 1: an uncaught Python exception exits 1, and a missing interpreter exits 127, so a crash in the comparison would otherwise be indistinguishable from "nothing to push" — rebuilt map silently discarded, job green, a dead guard looking exactly like a quiet repo. Every code outside {0, 10} fails the workflow.

Replayed over 7/17–7/26: 7 pushes → 3, and every survivor is a real change (n_tests 610 → 622). Commit 96358736 was +0 −0 against its predecessor — a provably pointless push.

Health-check follow-on

Once the bot stops pushing on no-op refreshes, built_at freezes during quiet stretches and the "stale after 10 days" rule misfires — the largest gap between coverage-relevant commits in the last 2.5 months is 8.9 days, already close.

Replaced with the question that rule was approximating: does the map's git_sha contain the last commit touching src/**/*.fpp or cases.py? The map already records git_sha, so this is one git merge-base --is-ancestor call — no API, no token, no heartbeat commits. It also detects a dead refresh on the next relevant push rather than 10 days later. The wall-clock rule stays as the fallback when git can't answer (built_after_last_change=None), so existing behavior is preserved where it still applies. coverage-health.yml gains fetch-depth: 0, which the ancestry query needs.

Type of change

• Bug fix

Testing

• 58/58 in toolchain/mfc/test/test_coverage_unit.py (19 new, written failing first; all 39 pre-existing pass unchanged).
• Two-checkout key comparison: 8 → 0 divergent keys.
• Commit guard exercised on all three paths — identical map → skip; new entry → push; only _meta changed → skip while git diff still reports a change (the everyday case). These build a throwaway git repo and run the script as a subprocess, so they assert the exit codes the workflow actually branches on rather than a mock of them.
• Every guard exit code driven through the real run: block (git stubbed): 0 → push; 10 → discard rebuilt map; 1, 2 and 127 → ::error and job failure. Codes 1 and 127 took the silent-skip path before.
• Health check both directions: current map → healthy, exit 0; map pinned behind the last .fpp commit → STALE, exit 1, despite built_at being today.
• ruff check and ruff format --check clean on all touched files; both workflow YAMLs parse. (Two pre-existing ruff failures under .github/scripts/ are untouched — they are already on master.)

Expected on merge: the first refresh legitimately rewrites the map once, as the 8 STL keys move to canonical form.

Checklist

• I added or updated tests for new behavior
• I updated documentation if user-facing behavior changed

[Copilot]

Pull request overview

This PR fixes the coverage-map refresh bot repeatedly pushing no-op commits by making coverage keys and map serialization deterministic, and by switching the commit guard from byte-level diffs to entry-level comparisons. It also improves the “stale map” health check to use git ancestry (last relevant source change) instead of wall-clock age when determinable.

Changes:

• Canonicalize in-repo absolute parameter paths to repo-relative form when hashing coverage keys (prevents checkout-path churn).
• Write tests/coverage_map.json.gz with a deterministic gzip header (mtime=0, empty filename) and add an entry-based change detector for the refresh workflow.
• Update the health check workflow/script to validate freshness via git history (requires full fetch depth).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
toolchain/mfc/test/coverage.py	Adds path canonicalization for hashing, deterministic gzip writing, and extends map_health to accept git-based freshness input.
toolchain/mfc/test/case.py	Switches coverage_key() to hash canonicalized params (repo-relative paths).
toolchain/mfc/test/test_coverage_unit.py	Adds targeted unit tests for canonicalization, deterministic gzip header, entry comparison, and new map_health behavior.
.github/workflows/coverage-refresh.yml	Replaces byte-diff guard with a script that compares coverage entries and resets the working tree when unchanged.
.github/workflows/coverage-health.yml	Fetches full git history to support ancestry-based freshness checks.
.github/scripts/coverage_map_changed.py	New helper to decide whether a rebuilt map is substantively changed (entries).
.github/scripts/check_coverage_map_health.py	Adds git-based “built after last relevant change” computation and threads it into map_health.

[github-actions]

Claude Code Review

Head SHA: 0730715

Files changed:

• 7
• .github/scripts/check_coverage_map_health.py
• .github/scripts/coverage_map_changed.py
• .github/workflows/coverage-health.yml
• .github/workflows/coverage-refresh.yml
• toolchain/mfc/test/case.py
• toolchain/mfc/test/coverage.py
• toolchain/mfc/test/test_coverage_unit.py

Findings:

• coverage_map_changed.py's own docstring defines the exit-code contract as 0 = changed, 1 = unchanged, 2 = error, but only the explicit entries_equal(...) branch calls sys.exit(1); there is no top-level exception handling, so any unhandled exception (e.g. an ImportError from the sys.path.insert + from mfc.test.coverage import ... line, a subprocess.run failure, or a bug in a future edit) also terminates the process with Python's default exit code 1. In coverage-refresh.yml, the consuming shell logic only special-cases changed -eq 2 as an error (new lines ~49-52); every other non-zero code, including an accidental 1 from a genuine crash, falls into the else branch, which silently runs git checkout -- tests/coverage_map.json.gz and reports nothing wrong. This is exactly the "silent staleness" failure mode check_coverage_map_health.py's built_after_last_change check in this same PR is designed to detect faster — but a broken coverage_map_changed.py would look identical to a legitimate no-op run, so the map could silently stop refreshing indefinitely without either workflow raising an alarm.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.04%. Comparing base (83bf84c) to head (a41ff7b).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1683   +/-   ##
=======================================
  Coverage   61.04%   61.04%           
=======================================
  Files          83       83           
  Lines       20978    20978           
  Branches     3099     3099           
=======================================
  Hits        12807    12807           
  Misses       6126     6126           
  Partials     2045     2045           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.