Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions benchmarks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,17 @@ The `benchmarks` job on **ubuntu-latest** runs the full `tests/benchmarks/` suit

Pinned runner: `ubuntu-latest`, `--benchmark-min-rounds=5`.

Sub-millisecond cache lookups (`test_summary_cache_lookup`, `test_composer_map_cache_lookup`, `test_tab_summary_cache_lookup`) and small ZIP export (`test_post_export_zip[composers-10]`) are already listed in `EXCLUDED_FROM_GATE` because shared runners show 2–4x spread. For remaining gated benches that turn flaky, raise `--slack` at baseline refresh time or add a targeted `EXCLUDED_FROM_GATE` entry.
Sub-millisecond cache lookups (`test_summary_cache_lookup`, `test_composer_map_cache_lookup`, `test_tab_summary_cache_lookup`) are already listed in `EXCLUDED_FROM_GATE` because shared runners show 2–4x spread. For remaining gated benches that turn flaky, raise `--slack` at baseline refresh time or add a targeted `EXCLUDED_FROM_GATE` entry.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

`test_summary_cache_round_trip` is intentionally excluded from the gate: it calls `set_cached_projects` (file write) + `get_cached_projects` (file read) each round, so OS page-cache state on shared runners causes 3–5x variation between consecutive CI runs. The baseline entry is kept for observation only.

`test_post_export_zip[composers-10]` is excluded for the same reason on the small corpus: observed ~4x spread on ubuntu-latest (e.g. 0.008s vs 0.031s on consecutive runs of the same branch). That is environmental variance (cold ZIP / page-cache), not an application slowdown — refreshing `baselines.json` cannot bracket both tails within the 0.5x–1.2x gate. The baseline entry is kept for observation only. `composers-50` remains gated (~1.2x spread).
### Export ZIP benchmarks

Both `test_post_export_zip[composers-10]` and `test_post_export_zip[composers-50]` are excluded from the regression gate. ZIP export timing on shared ubuntu-latest runners swings with page-cache and first-write effects (e.g. composers-10 ~4x between runs; composers-50 exceeded 1.2x vs a Jul-15 baseline at 0.045s vs 0.028s). That is environmental variance, not an application slowdown. Baseline entries in `baselines.json` are kept for observation only; parse, search, and fingerprint benches remain gated.

### Out-of-scope CI fixes in feature PRs

Sometimes a feature PR fails **Performance benchmarks (gated)** even though the PR does not touch export, parse, or search hot paths. When the failure is pre-existing runner variance (as with `composers-10` above), the correct fix is a targeted `EXCLUDED_FROM_GATE` entry — **not** a `baselines.json` refresh bundled into the feature PR.
Sometimes **Performance benchmarks (gated)** fails on `master` even though the change does not touch export, parse, or search hot paths. When the failure is pre-existing runner variance (export ZIP benches above), the correct fix is a targeted `EXCLUDED_FROM_GATE` entry — **not** a `baselines.json` refresh unless you are doing intentional performance work on that bench.

- **In scope for the feature PR:** unblocking CI so `mypy` + unit tests + the benchmark job all pass.
- **Out of scope for the feature PR:** claiming a performance win/loss, changing export behavior, or refreshing baselines for unrelated benches.
Expand Down
17 changes: 8 additions & 9 deletions scripts/check_benchmark_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,16 @@
"test_composer_map_cache_lookup[miss]",
"test_tab_summary_cache_lookup[hit]",
"test_tab_summary_cache_lookup[miss]",
# POST /api/export ZIP over 10 composers: observed ~4x spread on ubuntu-latest
# (e.g. 0.008s vs 0.031s on the same branch). Environmental — cold page cache /
# first ZIP write — not an application regression. Baseline refresh cannot fix
# this: a high baseline passes slow runs but marks fast runs STALE (<50%).
# composers-50 (~30ms) stays gated and is stable (~1.2x spread).
# POST /api/export ZIP (10 and 50 composers): timing varies with cold page cache /
# ZIP writes on ubuntu-latest (e.g. composers-10 ~4x spread; composers-50 hit
# 1.6x vs Jul-15 baseline on master after unrelated merges). Not an application
# regression. Baseline-only fixes fail when slow and fast runs cannot both sit
# inside the 0.5x–1.2x gate.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
#
# Scope note: this exclusion is CI-gate maintenance only. It does not change
# export behavior and is unrelated to feature PRs that happen to hit the flaky
# gate (e.g. structured {error, code} bodies). Prefer EXCLUDED_FROM_GATE over
# baselines.json churn when the failure is runner variance, not a code slowdown.
# CI-gate maintenance only; prefer EXCLUDED_FROM_GATE over baselines.json churn
# when the failure is runner variance, not a code slowdown.
"test_post_export_zip[composers-10]",
"test_post_export_zip[composers-50]",
}
)

Expand Down
6 changes: 3 additions & 3 deletions tests/test_check_benchmark_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@

# Pytest benchmark node IDs (parametrize ids) that must stay in EXCLUDED_FROM_GATE.
# Keep in sync with scripts/check_benchmark_regression.py — see that file for rationale
# per exclusion. composers-10 export: ungatable CI variance on shared runners, not an
# app perf regression; safe to land in a feature PR only as CI-gate maintenance (see
# benchmarks/README.md § "Out-of-scope CI fixes in feature PRs").
# per exclusion. Export ZIP benches (composers-10/50): ungatable CI variance; see
# benchmarks/README.md § "Export ZIP benchmarks".
EXPECTED_EXCLUDED_FROM_GATE = (
"test_summary_cache_round_trip",
"test_summary_cache_lookup[hit]",
Expand All @@ -31,6 +30,7 @@
"test_tab_summary_cache_lookup[hit]",
"test_tab_summary_cache_lookup[miss]",
"test_post_export_zip[composers-10]",
"test_post_export_zip[composers-50]",
)


Expand Down
Loading