From 93ffd05288390fb2d64b419bca9ea504a51fc9f8 Mon Sep 17 00:00:00 2001 From: Yeming Tang Date: Fri, 14 Aug 2026 14:12:56 +0000 Subject: [PATCH] fix(compare): prefer newest per-generation dir; skip baseline gen output in warm-cache Regression introduced by the warm-cache step in #10. `shutil.copytree` merged the baseline's *entire* per-behavior tree into the current run's `artifacts_root`, including the baseline's own per-generation output subdirectory `results///scores.jsonl`. When `assert-ai run` then produced its own generation dir, `compare_runs.py::_find_scores_jsonl` saw two `scores.jsonl` matches under `current` and used `sorted()[0]` (oldest by ISO-8601 timestamp = the baseline's copy). Paired McNemar therefore compared the baseline against itself and returned zero discordant pairs -- verdict WARN via TooFewSamples on identical data. Observed on responsibleai/assert-ci-banking-demo run 31806201202 (PR #11 demo FAIL-path): current run scored coercion at 40% policy_violation vs baseline 45%, but the report said 45% == 45% and 0 discordant. Two coupled fixes: - `_find_scores_jsonl` (compare_runs.py) and `_find_file` (detect_test_set_drift.py, write_firstrun_report.py) now return `sorted()[-1]` -- the newest match. assert-ai timestamps per-generation subdirs so lexicographic order is chronological. Semantically correct in every case: when several runs coexist under the same suite root, the current run's data is always the latest. - `Warm the artifact cache from the baseline` (action.yml) skips subdirectories matching `^\d{8}T\d{6}$` when copying. The stage cache proper lives at `results//artifacts///` and is copied normally; only the baseline's per-generation output dirs (which the current run doesn't need for cache hydration) are excluded. Belt-and-suspenders: even without the `_find_file` fix, the current run's scores.jsonl is now the only one under its tree. Regression tests: - `test_find_scores_jsonl_prefers_latest_generation`: writes two generation dirs matching the exact banking-demo repro layout, asserts the picker returns the newer one. - `test_find_scores_jsonl_prefers_top_level_when_present`: documents that legacy flat layouts (no generation subdirs) keep working. All 54 tests pass locally. --- action.yml | 22 +++++++++++++++-- scripts/compare_runs.py | 10 +++++--- scripts/detect_test_set_drift.py | 7 +++++- scripts/write_firstrun_report.py | 7 +++++- tests/test_compare_runs.py | 41 ++++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 7 deletions(-) diff --git a/action.yml b/action.yml index c23a78c..d156423 100644 --- a/action.yml +++ b/action.yml @@ -353,12 +353,30 @@ runs: run: | python <<'PY' import json + import re import shutil from pathlib import Path behaviors = json.loads(Path("assert-ai-behaviors.json").read_text(encoding="utf-8")) baseline_root = Path("assert-ai-baseline").resolve() + # Skip per-generation output subdirectories when copying the baseline. + # `assert-ai` writes each run's own inference_set.jsonl / scores.jsonl + # into `results///`, which is the *baseline's* + # historical output, not something the cache needs. If we copy it in, + # the current run's `_find_scores_jsonl` sees two candidates (baseline's + # older dir and the current run's newer dir) and downstream compare_runs + # can't tell which set is "current" -- observed on + # responsibleai/assert-ci-banking-demo run 31806201202 where PR#11's + # paired McNemar compared the baseline's scores against itself and + # produced 0 discordant pairs / WARN. + # The stage cache proper lives at `results//artifacts// + # /` and is copied normally. + _GEN_DIR_RE = re.compile(r"^\d{8}T\d{6}$") + + def _skip_generation_dirs(directory, names): + return [n for n in names if _GEN_DIR_RE.match(n)] + for b in behaviors: src = baseline_root / b["slug"] dst = Path(b["artifacts_root"]) @@ -368,8 +386,8 @@ runs: dst.mkdir(parents=True, exist_ok=True) # dirs_exist_ok=True lets Python 3.11+ merge trees; any files the # baseline doesn't cover fall through to the fresh run's own writes. - shutil.copytree(src, dst, dirs_exist_ok=True) - print(f"[warm-cache] seeded {b['slug']} from baseline") + shutil.copytree(src, dst, dirs_exist_ok=True, ignore=_skip_generation_dirs) + print(f"[warm-cache] seeded {b['slug']} from baseline (excluding baseline's per-generation output dirs)") PY - name: Export provider credentials diff --git a/scripts/compare_runs.py b/scripts/compare_runs.py index 3598f34..662cbfc 100644 --- a/scripts/compare_runs.py +++ b/scripts/compare_runs.py @@ -145,7 +145,11 @@ def _find_scores_jsonl(root: Path) -> Path: Accepts either the run directory itself (``scores.jsonl`` at the top level) or a parent directory containing one or more run subdirectories. If multiple - candidates exist, the lexicographically first one is used and reported. + candidates exist, the lexicographically LAST one is used because ``assert- + ai`` names per-generation subdirectories with an ISO-8601 timestamp + (``YYYYMMDDTHHMMSS``) that sorts chronologically -- picking the newest is + correct even when a warm-cache step copied the baseline's own generation + output into the current run's tree. """ if (root / "scores.jsonl").is_file(): return root / "scores.jsonl" @@ -154,9 +158,9 @@ def _find_scores_jsonl(root: Path) -> Path: raise FileNotFoundError(f"no scores.jsonl under {root}") if len(candidates) > 1: sys.stderr.write( - f"[compare_runs] multiple scores.jsonl under {root}; using {candidates[0]}\n" + f"[compare_runs] multiple scores.jsonl under {root}; using latest {candidates[-1]}\n" ) - return candidates[0] + return candidates[-1] def _verdict_for( diff --git a/scripts/detect_test_set_drift.py b/scripts/detect_test_set_drift.py index 9550515..b4865e6 100644 --- a/scripts/detect_test_set_drift.py +++ b/scripts/detect_test_set_drift.py @@ -36,8 +36,13 @@ def _find_file(root: Path, name: str) -> Path | None: direct = root / name if direct.is_file(): return direct + # Prefer the lexicographically LAST match: assert-ai names per-generation + # subdirectories with an ISO-8601 timestamp (`YYYYMMDDTHHMMSS`) so sorting + # by name is chronological. When the warm-cache step has copied the + # baseline's per-generation output into the current run's tree, the newer + # dir is the one produced by *this* run and is what we want. matches = sorted(p for p in root.rglob(name) if p.is_file()) - return matches[0] if matches else None + return matches[-1] if matches else None def _sha256(path: Path) -> str: diff --git a/scripts/write_firstrun_report.py b/scripts/write_firstrun_report.py index 3ade28d..0f73f9a 100644 --- a/scripts/write_firstrun_report.py +++ b/scripts/write_firstrun_report.py @@ -21,8 +21,13 @@ def _find_file(root: Path, name: str) -> Path | None: direct = root / name if direct.is_file(): return direct + # Prefer the lexicographically LAST match: assert-ai names per-generation + # subdirectories with an ISO-8601 timestamp (`YYYYMMDDTHHMMSS`) so sorting + # by name is chronological. When the warm-cache step has copied the + # baseline's per-generation output into the current run's tree, the newer + # dir is the one produced by *this* run and is what we want. matches = sorted(p for p in root.rglob(name) if p.is_file()) - return matches[0] if matches else None + return matches[-1] if matches else None def _current_stats(root: Path) -> tuple[int, float]: diff --git a/tests/test_compare_runs.py b/tests/test_compare_runs.py index be39dcc..965e0d6 100644 --- a/tests/test_compare_runs.py +++ b/tests/test_compare_runs.py @@ -386,3 +386,44 @@ def test_zero_discordant_pairs_is_json_safe_and_does_not_raise(tmp_path) -> None assert dim["paired_risk_difference"] == 0.0 assert dim["verdict"] == "TooFewSamples" json.dumps(compare_runs._json_safe(report), allow_nan=False) + + +def test_find_scores_jsonl_prefers_latest_generation(tmp_path) -> None: + """Regression: after the warm-cache step in action.yml, the current run's + artifacts_root can contain two per-generation subdirectories -- the + baseline's copy (`20260814T125136/scores.jsonl` etc.) and the current run's + own (`20260814T134642/scores.jsonl`). Picking `sorted()[0]` (oldest by ISO + timestamp) returns the baseline's copy, so paired McNemar ends up comparing + the baseline to itself and reports 0 discordant pairs even when the target + callable produced measurably different scores. Prefer `sorted()[-1]` so + the current run's own generation is what compare_runs pairs against. + Bug repro: assert-ci-banking-demo run 31806201202 (PR #11). + """ + root = tmp_path / "coercion_via_unverified_authority" / "results" / "coercion_via_unverified_authority" + older = root / "20260814T125136" + newer = root / "20260814T134642" + older.mkdir(parents=True) + newer.mkdir(parents=True) + (older / "scores.jsonl").write_text('{"marker": "baseline-copy"}\n', encoding="utf-8") + (newer / "scores.jsonl").write_text('{"marker": "current-run"}\n', encoding="utf-8") + + picked = compare_runs._find_scores_jsonl(root) + assert picked == newer / "scores.jsonl" + assert '"current-run"' in picked.read_text(encoding="utf-8"), ( + "picked the baseline's scores instead of the current run's -- paired " + "McNemar would compare the baseline to itself" + ) + + +def test_find_scores_jsonl_prefers_top_level_when_present(tmp_path) -> None: + """Top-level `scores.jsonl` still wins over nested candidates. Documents that + legacy flat layouts (no generation subdirs) keep working. + """ + root = tmp_path + (root / "scores.jsonl").write_text('{"marker": "top-level"}\n', encoding="utf-8") + nested = root / "20260814T125136" + nested.mkdir() + (nested / "scores.jsonl").write_text('{"marker": "nested"}\n', encoding="utf-8") + + picked = compare_runs._find_scores_jsonl(root) + assert picked == root / "scores.jsonl"