fix(test): address PR #120 benchmark review (gate, CI, baselines)

Author: clean6378-max-it

.github/workflows/tests.yml

@@ -105,7 +105,7 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           python -m pip install -r requirements-lock.txt
-          python -m pip install 'pytest>=8,<9' 'hypothesis>=6.100,<7'
+          python -m pip install 'pytest>=8,<9' 'pytest-benchmark==4.0.0' 'hypothesis>=6.100,<7'
 
       - name: Run unittest suite
         run: python -m unittest discover tests -v
@@ -218,6 +218,7 @@ jobs:
   # ── Performance benchmarks: summary cache (issue #7) ───────────────────────
   benchmarks:
     name: Performance benchmarks (gated)
+    needs: [unittest]
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2
@@ -233,7 +234,7 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           python -m pip install -r requirements-lock.txt
-          python -m pip install 'pytest>=8,<9' 'pytest-benchmark>=4,<5'
+          python -m pip install 'pytest>=8,<9' 'pytest-benchmark==4.0.0'
 
       - name: Run summary-cache benchmarks
         run: >

benchmarks/baselines.json

@@ -1,6 +1,6 @@
 {
-  "_note": "Gated means from ubuntu-latest CI benchmark-results.json (PR #7 summary-cache benchmarks). Refresh via: pytest tests/benchmarks/ --benchmark-only --benchmark-json=benchmark-results.json -o addopts= then update these values.",
-  "updated": "2026-06-25T12:00:00Z",
+  "_note": "Gated means from ubuntu-latest CI benchmark-results.json (PR #120, run 28123677675). Refresh: pytest tests/benchmarks/ --benchmark-only --benchmark-json=benchmark-results.json -o addopts=",
+  "updated": "2026-06-24T19:20:27Z",
   "machine": "Linux",
   "groups": {
     "summary-cache": {

pyproject.toml

@@ -37,6 +37,8 @@ dev = [
 ]
 
 [tool.pytest.ini_options]
+pythonpath = ["."]
+addopts = "--benchmark-disable"
 testpaths = ["tests"]
 markers = [
     "benchmark: performance benchmarks (pytest-benchmark)",

scripts/check_benchmark_regression.py

@@ -14,6 +14,11 @@ class BenchmarkDataError(ValueError):
     """Raised when benchmark JSON input is malformed or missing required fields."""
 
 
+def normalize_benchmark_name(name: str) -> str:
+    """Strip pytest node prefix so baselines match short or full benchmark names."""
+    return str(name).rsplit("::", 1)[-1]
+
+
 def load_results(results_path: str | Path) -> dict[str, float]:
     path = Path(results_path)
     try:
@@ -34,13 +39,13 @@ def load_results(results_path: str | Path) -> dict[str, float]:
         if not isinstance(entry, dict):
             raise BenchmarkDataError(f"{path} benchmarks[{index}] must be an object")
         try:
-            name = entry["name"]
+            raw_name = entry["name"]
             mean = float(entry["stats"]["mean"])
         except (KeyError, TypeError, ValueError) as exc:
             raise BenchmarkDataError(
                 f"{path} benchmarks[{index}] missing 'name' or 'stats.mean'"
             ) from exc
-        name = str(name)
+        name = normalize_benchmark_name(str(raw_name))
         if name in results:
             raise BenchmarkDataError(f"{path} duplicate benchmark name {name!r}")
         results[name] = mean
@@ -67,13 +72,17 @@ def load_baseline_means(baselines_path: str | Path) -> dict[str, float]:
     means: dict[str, float] = {}
     for group_name, value in groups.items():
         if not isinstance(value, dict):
-            continue
+            raise BenchmarkDataError(
+                f"{path} groups[{group_name!r}] must be an object of benchmark means"
+            )
         for name, mean in value.items():
-            name = str(name)
-            if name in means:
-                raise BenchmarkDataError(f"{path} duplicate benchmark name {name!r} across groups")
+            bench_name = normalize_benchmark_name(str(name))
+            if bench_name in means:
+                raise BenchmarkDataError(
+                    f"{path} duplicate benchmark name {bench_name!r} across groups"
+                )
             try:
-                means[name] = float(mean)
+                means[bench_name] = float(mean)
             except (TypeError, ValueError) as exc:
                 raise BenchmarkDataError(
                     f"{path} groups[{group_name!r}][{name!r}] is not a numeric mean"

tests/benchmarks/__init__.py

@@ -2,19 +2,13 @@
 
 from __future__ import annotations
 
-import os
-import sys
 from pathlib import Path
 from typing import Any
 
 import pytest
 
-REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
-if REPO_ROOT not in sys.path:
-    sys.path.insert(0, REPO_ROOT)
-
-from services import summary_cache  # noqa: E402
-from services.summary_cache import fingerprint_workspace_storage  # noqa: E402
+from services import summary_cache
+from services.summary_cache import fingerprint_workspace_storage
 
 
 def make_workspace_entries(workspace_root: Path, count: int) -> list[dict[str, Any]]:
@@ -38,7 +32,11 @@ def make_workspace_entries(workspace_root: Path, count: int) -> list[dict[str, A
 
 @pytest.fixture
 def summary_cache_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
-    """Redirect summary-cache files to an isolated temp directory."""
+    """Redirect summary-cache files to an isolated temp directory.
+
+    Patches ``CACHE_DIR`` (also used by tab-summary paths via ``_tab_summaries_path``)
+    plus the projects/composer-map file constants used by current benchmarks.
+    """
     cache_dir = tmp_path / "cache"
     cache_dir.mkdir()
     monkeypatch.setattr(summary_cache, "CACHE_DIR", cache_dir)
@@ -86,4 +84,5 @@ def workspace_fingerprint(synthetic_workspace: tuple[str, list[dict[str, Any]]])
 
 @pytest.fixture
 def stale_fingerprint(workspace_fingerprint: dict[str, Any]) -> dict[str, Any]:
-    return {**workspace_fingerprint, "global_db_mtime_ns": 9_999_999_999}
+    """Return a fingerprint guaranteed to differ from the stored one."""
+    return {**workspace_fingerprint, "rules_digest": "deadbeefdeadbeef"}

tests/benchmarks/conftest.py

@@ -13,6 +13,7 @@
     set_cached_projects,
 )
 
+
 @pytest.mark.benchmark(group="summary-cache")
 def test_summary_cache_hit(
     benchmark,

tests/benchmarks/test_summary_cache_bench.py

@@ -0,0 +1,208 @@
+"""Tests for scripts/check_benchmark_regression.py."""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+
+from scripts.check_benchmark_regression import (
+    BenchmarkDataError,
+    check_regression,
+    load_baseline_means,
+    load_results,
+    normalize_benchmark_name,
+)
+
+GATED_BENCH = "test_summary_cache_hit"
+
+
+def _write_results(path, benchmarks: list[dict]) -> None:
+    path.write_text(
+        json.dumps({"benchmarks": benchmarks}, indent=2),
+        encoding="utf-8",
+    )
+
+
+def _write_baselines(path, groups: dict[str, dict[str, float]]) -> None:
+    path.write_text(
+        json.dumps({"groups": groups}, indent=2),
+        encoding="utf-8",
+    )
+
+
+def test_normalize_benchmark_name_strips_module_prefix() -> None:
+    full = "tests/benchmarks/test_summary_cache_bench.py::test_summary_cache_hit"
+    assert normalize_benchmark_name(full) == "test_summary_cache_hit"
+    assert normalize_benchmark_name("test_summary_cache_hit") == "test_summary_cache_hit"
+
+
+def test_load_results_normalizes_full_node_id(tmp_path) -> None:
+    path = tmp_path / "results.json"
+    _write_results(
+        path,
+        [
+            {
+                "name": "tests/benchmarks/test_summary_cache_bench.py::test_summary_cache_hit",
+                "stats": {"mean": 0.0001},
+            }
+        ],
+    )
+
+    assert load_results(path)["test_summary_cache_hit"] == pytest.approx(0.0001)
+
+
+def test_missing_baseline_warns_without_failing(
+    tmp_path, capsys: pytest.CaptureFixture[str]
+) -> None:
+    results = tmp_path / "results.json"
+    baselines = tmp_path / "baselines.json"
+    _write_results(
+        results,
+        [
+            {"name": "test_new_bench", "stats": {"mean": 0.01}},
+            {"name": GATED_BENCH, "stats": {"mean": 0.0001}},
+        ],
+    )
+    _write_baselines(
+        baselines,
+        {"summary-cache": {GATED_BENCH: 0.0001}},
+    )
+
+    assert check_regression(results, baselines) == 0
+    out = capsys.readouterr().out
+    assert "WARN: 'test_new_bench' has no baseline yet" in out
+
+
+def test_regression_over_threshold_fails(tmp_path, capsys: pytest.CaptureFixture[str]) -> None:
+    results = tmp_path / "results.json"
+    baselines = tmp_path / "baselines.json"
+    _write_results(
+        results,
+        [{"name": GATED_BENCH, "stats": {"mean": 0.00025}}],
+    )
+    _write_baselines(
+        baselines,
+        {"summary-cache": {GATED_BENCH: 0.0002}},
+    )
+
+    assert check_regression(results, baselines) == 1
+    out = capsys.readouterr().out
+    assert "REGRESSION" in out
+
+
+def test_within_threshold_passes(tmp_path) -> None:
+    results = tmp_path / "results.json"
+    baselines = tmp_path / "baselines.json"
+    _write_results(
+        results,
+        [{"name": GATED_BENCH, "stats": {"mean": 0.00022}}],
+    )
+    _write_baselines(
+        baselines,
+        {"summary-cache": {GATED_BENCH: 0.0002}},
+    )
+
+    assert check_regression(results, baselines) == 0
+
+
+def test_load_results_rejects_malformed_json(tmp_path) -> None:
+    path = tmp_path / "bad.json"
+    path.write_text("{not json", encoding="utf-8")
+    with pytest.raises(BenchmarkDataError, match="invalid JSON"):
+        load_results(path)
+
+
+def test_load_results_requires_benchmarks_array(tmp_path) -> None:
+    path = tmp_path / "results.json"
+    path.write_text("{}", encoding="utf-8")
+    with pytest.raises(BenchmarkDataError, match="'benchmarks' array"):
+        load_results(path)
+
+
+def test_load_results_rejects_missing_file(tmp_path) -> None:
+    with pytest.raises(BenchmarkDataError, match="cannot read"):
+        load_results(tmp_path / "missing.json")
+
+
+def test_zero_baseline_skips_ratio_check(tmp_path, capsys: pytest.CaptureFixture[str]) -> None:
+    results = tmp_path / "results.json"
+    baselines = tmp_path / "baselines.json"
+    _write_results(
+        results,
+        [{"name": GATED_BENCH, "stats": {"mean": 0.00025}}],
+    )
+    _write_baselines(
+        baselines,
+        {"summary-cache": {GATED_BENCH: 0.0}},
+    )
+
+    assert check_regression(results, baselines) == 0
+    assert f"baseline for '{GATED_BENCH}' is zero" in capsys.readouterr().out
+
+
+def test_exactly_at_threshold_passes(tmp_path) -> None:
+    results = tmp_path / "results.json"
+    baselines = tmp_path / "baselines.json"
+    _write_results(
+        results,
+        [{"name": GATED_BENCH, "stats": {"mean": 0.00024}}],
+    )
+    _write_baselines(
+        baselines,
+        {"summary-cache": {GATED_BENCH: 0.0002}},
+    )
+
+    assert check_regression(results, baselines) == 0
+
+
+def test_missing_current_result_fails(tmp_path, capsys: pytest.CaptureFixture[str]) -> None:
+    results = tmp_path / "results.json"
+    baselines = tmp_path / "baselines.json"
+    _write_results(results, [])
+    _write_baselines(
+        baselines,
+        {"summary-cache": {GATED_BENCH: 0.0002}},
+    )
+
+    assert check_regression(results, baselines) == 1
+    out = capsys.readouterr().out
+    assert "MISSING" in out
+    assert "no current result for gated baseline" in out
+
+
+def test_main_reports_benchmark_data_error(tmp_path, capsys: pytest.CaptureFixture[str]) -> None:
+    from scripts.check_benchmark_regression import main
+
+    bad = tmp_path / "bad.json"
+    bad.write_text("{}", encoding="utf-8")
+    baselines = tmp_path / "baselines.json"
+    _write_baselines(baselines, {"summary-cache": {GATED_BENCH: 0.0002}})
+
+    assert main([str(bad), str(baselines)]) == 2
+    assert "ERROR:" in capsys.readouterr().err
+
+
+def test_duplicate_baseline_name_raises(tmp_path) -> None:
+    baselines = tmp_path / "baselines.json"
+    _write_baselines(
+        baselines,
+        {
+            "summary-cache": {GATED_BENCH: 0.0002},
+            "export": {GATED_BENCH: 0.0003},
+        },
+    )
+
+    with pytest.raises(BenchmarkDataError, match="duplicate benchmark name"):
+        load_baseline_means(baselines)
+
+
+def test_load_baseline_means_rejects_non_dict_group(tmp_path) -> None:
+    baselines = tmp_path / "baselines.json"
+    baselines.write_text(
+        json.dumps({"groups": {"summary-cache": "not-a-dict"}}),
+        encoding="utf-8",
+    )
+
+    with pytest.raises(BenchmarkDataError, match="must be an object"):
+        load_baseline_means(baselines)