Skip to content

Commit 1814b15

Browse files
Sravan1011claude
andcommitted
fix: guard --recreate against deleting non-benchmark indexes; fail on zero latency baseline
Addresses two review findings: - BLOCKING: --recreate deleted whatever MOSS_INDEX_NAME pointed at, so a developer with a shared or production Moss project in their env could destroy a non-benchmark index. Deletion is now guarded three ways: the derived benchmark-ci-<signature> name deletes without confirmation, an overridden name inside the benchmark-ci-* namespace additionally requires the new --force flag, and names outside that namespace are refused even with --force. - CONSIDER: the checked-in zero latency baseline made test_no_latency_regression skip on every run, leaving the latency guard silently inactive until someone manually armed it. A zero baseline now FAILS comparison runs with the arming procedure in the message. update_baseline dispatch runs are unaffected (they do not pass --baseline-file), and fork PRs skip earlier at the credentials gate, so the failure lands exactly on trusted runs that should be enforcing the guard. baseline.json note and README updated; the first trusted CI run's artifact is the natural baseline to commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 16b0343 commit 1814b15

4 files changed

Lines changed: 57 additions & 4 deletions

File tree

benchmarks/ci/README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ expected document IDs to `ground_truth.json`. Commit the updated file.
5050
After a corpus or model change, pass `--recreate` so the index is rebuilt
5151
from the current corpus before the ground truth is captured.
5252

53+
`--recreate` deletion is guarded: only the derived `benchmark-ci-<hash>`
54+
index is deleted without confirmation. If `MOSS_INDEX_NAME` overrides the
55+
name, deletion additionally requires `--force`, and indexes outside the
56+
`benchmark-ci-*` namespace are never deleted — so a stray environment
57+
variable pointing at a shared or production index cannot be destroyed.
58+
5359
> **Note**: the ground truth is a *ranking-stability reference* generated by
5460
> Moss itself at a known-good commit — not an independent relevance judgment.
5561
> The recall gate detects changes in retrieval behavior; an intentional
@@ -63,6 +69,10 @@ The harness compares the current run's metrics against `baseline.json`:
6369
- **Latency**: Fails if P95 increases by more than the threshold (default 20%)
6470
- **Recall**: Fails if Recall@5 drops by more than the threshold (default 5pp)
6571

72+
A zero latency baseline **fails** comparison runs rather than skipping — the
73+
guard cannot stay silently inactive. To arm it, commit a CI-captured
74+
baseline (see "Updating the baseline" below).
75+
6676
Thresholds are configurable via CLI flags:
6777

6878
```bash

benchmarks/ci/baseline.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"commit": "f4732a1",
33
"timestamp": "2026-07-20T05:35:00+00:00",
4-
"_note": "Recall values are measured (hardware-independent) so the recall guard is active. Latency values are intentionally zero \u2014 latency is hardware-dependent, so the guard skips until a baseline captured on CI runners replaces this file (run the Benchmark workflow, download the benchmark-results-<sha> artifact, commit it here).",
4+
"_note": "Recall values are measured (hardware-independent) so the recall guard is active. Latency values are intentionally zero and the latency guard FAILS on zero baselines in comparison runs \u2014 the first trusted CI run will be red until a CI-captured baseline is committed: run the Benchmark workflow (update_baseline=true also works), download the benchmark-results-<sha> artifact, and commit it here. Latency baselines must come from CI runners; numbers from other hardware are not comparable.",
55
"latency_ms": {
66
"p50": 0,
77
"p95": 0,

benchmarks/ci/generate_ground_truth.py

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
from bench_queries import (
3838
DOC_COUNT,
39+
INDEX_NAME_PREFIX,
3940
MODEL_ID,
4041
QUERIES,
4142
corpus_signature,
@@ -70,7 +71,32 @@ async def _create_index(client, index_name: str, corpus_slice: list[dict]) -> No
7071
print(f"Created index '{index_name}' with {result.doc_count} docs")
7172

7273

73-
async def main(recreate: bool) -> None:
74+
def _guard_deletion(index_name: str, derived_name: str, force: bool) -> None:
75+
"""Refuse to delete indexes that are not clearly benchmark-owned.
76+
77+
MOSS_INDEX_NAME is a documented override, so a developer whose
78+
environment points at a shared or production Moss project could
79+
otherwise aim --recreate at a non-benchmark index and destroy it.
80+
"""
81+
if index_name == derived_name:
82+
return # the derived benchmark index — always safe to recreate
83+
if not index_name.startswith(f"{INDEX_NAME_PREFIX}-"):
84+
print(
85+
f"Error: refusing to delete index '{index_name}' — it is outside "
86+
f"the benchmark namespace ('{INDEX_NAME_PREFIX}-*'). Unset "
87+
"MOSS_INDEX_NAME or point it at a benchmark index."
88+
)
89+
sys.exit(1)
90+
if not force:
91+
print(
92+
f"Error: MOSS_INDEX_NAME overrides the derived name "
93+
f"('{index_name}' != '{derived_name}'). Pass --force to confirm "
94+
"deleting the overridden benchmark index."
95+
)
96+
sys.exit(1)
97+
98+
99+
async def main(recreate: bool, force: bool) -> None:
74100
from moss import MossClient, QueryOptions
75101

76102
project_id = os.getenv("MOSS_PROJECT_ID")
@@ -94,6 +120,7 @@ async def main(recreate: bool) -> None:
94120
existing = {idx.name for idx in await client.list_indexes()}
95121

96122
if index_name in existing and recreate:
123+
_guard_deletion(index_name, index_name_for(signature), force)
97124
print(f"--recreate: deleting existing index '{index_name}'")
98125
await client.delete_index(index_name)
99126
existing.discard(index_name)
@@ -147,5 +174,12 @@ async def main(recreate: bool) -> None:
147174
"querying. Required after a corpus or embedding-model change so the "
148175
"ground truth reflects the current data.",
149176
)
177+
parser.add_argument(
178+
"--force",
179+
action="store_true",
180+
help="Confirm --recreate deletion when MOSS_INDEX_NAME overrides the "
181+
"derived index name. Only benchmark-namespace indexes "
182+
"(benchmark-ci-*) can be deleted even with this flag.",
183+
)
150184
args = parser.parse_args()
151-
asyncio.run(main(recreate=args.recreate))
185+
asyncio.run(main(recreate=args.recreate, force=args.force))

benchmarks/ci/test_bench_ci_moss.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,16 @@ def test_no_latency_regression(self, request, benchmark_results):
421421
_assert_baseline_compatible(baseline, benchmark_results)
422422

423423
if baseline_p95 == 0:
424-
pytest.skip("Baseline p95 is zero — cannot compute regression ratio")
424+
# A zero baseline means the latency guard has never been armed.
425+
# Skipping here would let every run pass with the guard silently
426+
# inactive — fail instead, with the arming procedure.
427+
pytest.fail(
428+
"Baseline p95 is zero — the latency guard is not armed. Run the "
429+
"Benchmark workflow with update_baseline=true, download the "
430+
"benchmark-results-<sha> artifact, and commit it as "
431+
"benchmarks/ci/baseline.json (values must come from CI runners; "
432+
"this run's artifact works too)."
433+
)
425434

426435
regression = (current_p95 - baseline_p95) / baseline_p95
427436

0 commit comments

Comments
 (0)