diff --git a/.claude/commands/sieval-release.md b/.claude/commands/sieval-release.md index 3bef8769..62b6d24a 100644 --- a/.claude/commands/sieval-release.md +++ b/.claude/commands/sieval-release.md @@ -32,13 +32,27 @@ git fetch origin main # local main must match remote [ "$(git rev-parse HEAD)" = "$(git rev-parse origin/main)" ] -python -m pytest tests/unit tests/integration tests/acceptance --tb=short -q +python -m pytest tests/unit tests/integration tests/acceptance -m "not benchmark" --tb=short -q ruff check . ty check gh pr list --state open --limit 20 # list for user review ``` +Then the wall-clock gates, alone — CI deselects `benchmark`, so this is the only +place they run. Serially and without `--cov`: both other load and the coverage +tracer distort what they measure. + +```bash +SIEVAL_BENCHMARK_ARTIFACT_DIR=./outputs/benchmarks \ +python -m pytest -m benchmark -q -s +``` + +Report the `SiEval Benchmark Summary` table (printed last, after the engine's log +lines) as the release's performance baseline; the same numbers land in +gitignored `outputs/benchmarks/benchmark_summary.json`. A breach can just mean a +busy box — re-run idle before calling it a regression. + Also collect changes since last tag: ```bash @@ -47,7 +61,7 @@ git log $PREV_TAG..HEAD --format="%h %s" --no-merges git diff $PREV_TAG..HEAD --stat ``` -If working tree is dirty, not on main, tests/lint fail — stop and report. +If working tree is dirty, not on main, tests/lint/benchmark fail — stop and report. If there are open PRs — list them and ask the user whether to include or defer. ### 2. ReferenceImpl Sanity Check diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1c7c540..5a72e35f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -102,9 +102,12 @@ jobs: run: pdm install --check --frozen-lockfile ${{ env.INSTALL_GROUPS }} - name: Preflight (registration, version, deps, links, meta-drift) run: pdm run python scripts/check_preflight.py - - name: Unit tests + coverage + # `benchmark` deselected: shared runners cannot hold wall-clock + # thresholds calibrated on a dedicated box. The rest is deterministic. + - name: Unit + integration + acceptance tests + coverage run: >- - pdm run python -m pytest tests/unit -m "not stress" + pdm run python -m pytest tests/unit tests/integration tests/acceptance + -m "not stress and not benchmark" --cov --cov-report=xml -q - name: Upload coverage (advisory) if: always() diff --git a/pyproject.toml b/pyproject.toml index c2463db2..ad3600dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -226,7 +226,10 @@ also_copy = [ ] [tool.pytest] -markers = ["stress: resource-intensive tests (run with: pytest -m stress)"] +markers = [ + "stress: resource-intensive tests (run with: pytest -m stress)", + "benchmark: wall-clock throughput gates, excluded from CI (see tests/README.md)", +] testpaths = [ "tests/unit", "tests/integration", diff --git a/tests/README.md b/tests/README.md index 96d23c05..18f3ffbc 100644 --- a/tests/README.md +++ b/tests/README.md @@ -126,6 +126,10 @@ python -m pytest tests/acceptance/ -v -s SIEVAL_BENCHMARK_ARTIFACT_DIR=./outputs/benchmarks \ python -m pytest tests/acceptance/ -v -s +# What CI runs: everything deterministic, benchmarks deselected +python -m pytest tests/unit tests/integration tests/acceptance \ + -m "not stress and not benchmark" --cov -q + # Performance diagnostic benchmarks (default excludes stress) python -m pytest tests/performance/ -v @@ -135,6 +139,20 @@ python -m pytest tests/performance/ -m "not stress" -v # Run only stress tests (intentional profiling) python -m pytest tests/performance/ -m stress -v +# Run only the wall-clock throughput gates +python -m pytest -m benchmark -v -s +``` + +### Markers + +| Marker | Meaning | +| --- | --- | +| `stress` | Resource-intensive profiling runs. Excluded by default via `addopts`; opt in with `-m stress`. | +| `benchmark` | Wall-clock throughput gates, calibrated on a dedicated box. A plain local `pytest` runs them; **CI deselects them** (a shared runner cannot hold the thresholds, and `--cov` skews the latency they measure), so `/sieval-release` is where they are enforced. | + +Because these assert on *time*, treat a failure as "re-run idle" before calling it a regression. `tests/acceptance/` holds one (`test_benchmark_scenarios`); its other nine are deterministic and do run in CI. + +```bash # Single file python -m pytest tests/integration/resume/test_advanced.py -v diff --git a/tests/acceptance/performance/test_performance_acceptance.py b/tests/acceptance/performance/test_performance_acceptance.py index 43cdcc28..a107eecc 100644 --- a/tests/acceptance/performance/test_performance_acceptance.py +++ b/tests/acceptance/performance/test_performance_acceptance.py @@ -445,6 +445,7 @@ def _check_regressions( class TestBenchmarkSummary: """Acceptance tests: realistic scenarios with structured reporting.""" + @pytest.mark.benchmark @pytest.mark.anyio async def test_benchmark_scenarios(self, tmp_path: Path) -> None: """Run all benchmark scenarios and produce summary report.""" diff --git a/tests/performance/test_io_overhead.py b/tests/performance/test_io_overhead.py index 192781f6..fcc1824a 100644 --- a/tests/performance/test_io_overhead.py +++ b/tests/performance/test_io_overhead.py @@ -24,10 +24,13 @@ samples_per_second, ) - # =================================================================== # Helpers # =================================================================== +# Samples taken per timed arm, for metrics too short to measure single-shot. +_TIMING_REPEATS = 5 + + async def _write_contexts_batch( root: Path, contexts: list[TaskContext], @@ -232,6 +235,11 @@ async def test_dependency_loading_overhead(self, tmp_path: Path) -> None: Writes snapshots at each stage, then hydrates with all dependencies. Compares against single-stage-only hydration. + + Arms are compared on their fastest of ``_TIMING_REPEATS`` hydrations. + One takes ~20ms, so a single sample each put any transient stall + straight into the ratio (idle 27% vs 876% under load); contention only + ever adds time, so the minimum is the stable estimator. """ n_samples = 500 stages = [ @@ -261,42 +269,38 @@ async def test_dependency_loading_overhead(self, tmp_path: Path) -> None: saver._stage_queue.append(ctx) await saver.flush() - # Hydrate with dependencies - task_dep = _make_mock_task(n_samples) - loader_dep = TaskLoader( - task=task_dep, - root_dir=root_dep, - shard_read_concurrency=8, - ) - - timer_dep = PerfTimer() - with timer_dep: - _loaded_dep = await loader_dep.load_initial_state() - - # Compare: hydrate without dependency stages (final only) + # Compare against hydration without dependency stages (final only) root_nodep = tmp_path / "nodep_bench" final_contexts = [_make_bench_ctx(i, TaskStage.FINAL) for i in range(n_samples)] await _write_contexts_batch(root_nodep, final_contexts, shard_samples=256) - task_nodep = _make_mock_task(n_samples) - loader_nodep = TaskLoader( - task=task_nodep, - root_dir=root_nodep, - shard_read_concurrency=8, - ) - - timer_nodep = PerfTimer() - with timer_nodep: - _loaded_nodep = await loader_nodep.load_initial_state() - - overhead_pct = ( - (timer_dep.elapsed - timer_nodep.elapsed) / timer_nodep.elapsed * 100 - if timer_nodep.elapsed > 0 - else 0 - ) + async def _fastest_hydration(root: Path) -> float: + """Seconds taken by the fastest of _TIMING_REPEATS hydrations.""" + elapsed: list[float] = [] + for _ in range(_TIMING_REPEATS): + # Fresh loader: load_initial_state() populates it, so reusing + # one would not re-read the shards. + loader = TaskLoader( + task=_make_mock_task(n_samples), + root_dir=root, + shard_read_concurrency=8, + ) + timer = PerfTimer() + with timer: + loaded = await loader.load_initial_state() + # Equal counts, or the ratio compares unequal work. + assert len(loaded) == n_samples + elapsed.append(timer.elapsed) + return min(elapsed) + + dep_s = await _fastest_hydration(root_dep) + nodep_s = await _fastest_hydration(root_nodep) + + overhead_pct = (dep_s - nodep_s) / nodep_s * 100 if nodep_s > 0 else 0 print( f"PERF: dep_loading overhead={overhead_pct:.1f}% " - f"(dep={timer_dep.elapsed:.4f}s, nodep={timer_nodep.elapsed:.4f}s)" + f"(dep={dep_s:.4f}s, nodep={nodep_s:.4f}s, " + f"best of {_TIMING_REPEATS})" ) # Dependency loading adds cross-stage reads; overhead should be bounded # (4 stages means ~4x more reads, expect <300% overhead)