Skip to content

Commit b75a2ba

Browse files
committed
fix: a run that measured nothing must change nothing, and an empty board must render
Three faults, one theme: the harness could not tell "measured" from "was already there", and could not represent "nothing measured yet" at all. THE BOX NO LONGER GETS results/. It cloned the whole repository, so every box carried the previous run's committed results/matrix/<gw>.json. bench_gateway then required that file to exist before calling a run DONE - and rsynced it back off the box to satisfy the check. A run that measured nothing passed a test against a file we had shipped there ourselves. The checkout is now sparse (gateways + lib), the box asserts results/ is absent, and success is judged solely on pull_snapshots reporting that the snapshot COUNT GREW. A FAILED RUN NOW LEAVES THE BOARD ALONE. history/append.py, charts.py and the final push all derive from whatever sits in results/ and never re-read the boxes, so a run where every box died rebuilt the previous numbers and pushed them - demonstrated live, 24 charts regenerated and pushed after a box aborted before measuring. The field-wide publish is gated on at least one gateway having produced a fresh snapshot this run. AN EMPTY BOARD IS A STATE, NOT A BREAKAGE. Clearing the retired results turned 16 site tests and 2 engine tests red - not because anything regressed, but because both suites used the committed board as their fixture corpus. The engine tests now build the snapshot they assert on; the site's anti-inert consistency guard treats a board with no measurements as vacuous and stays exactly as strict the moment one gateway carries data. The cf-pages deploy failing is why onthebench.ai still showed numbers we had deleted: Cloudflare kept serving the last build that passed.
1 parent 5078bcf commit b75a2ba

3 files changed

Lines changed: 162 additions & 98 deletions

File tree

‎engine/src/record.rs‎

Lines changed: 41 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -516,12 +516,6 @@ pub struct StreamingProjection {
516516
mod tests {
517517
use super::*;
518518
use crate::measurement::Absent;
519-
use std::fs;
520-
use std::path::PathBuf;
521-
522-
fn snapshots_dir() -> PathBuf {
523-
PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../results/snapshots")
524-
}
525519

526520
// ── the published names of the memory fields are a CONTRACT, not an implementation detail ─────
527521
//
@@ -820,75 +814,55 @@ mod tests {
820814
assert_eq!(reparsed["verdict_note"], serde_json::json!("line one\nline two\u{0001}"));
821815
}
822816

823-
// ── parity with real committed snapshots ────────────────────────────────────────────────────
824-
825-
// Fixtures are DISCOVERED, never named. Naming one would put a gateway identity in a shared
826-
// source file, which the isolation lint forbids and which would also pin this test to whichever
827-
// entrant happened to be in the corpus the day it was written.
828-
fn any_snapshot_with(pred: impl Fn(&ResultSnapshot) -> bool) -> Option<ResultSnapshot> {
829-
let mut paths: Vec<_> = fs::read_dir(snapshots_dir())
830-
.ok()?
831-
.filter_map(|e| e.ok().map(|e| e.path()))
832-
.filter(|p| p.extension().and_then(|e| e.to_str()) == Some("json"))
833-
.collect();
834-
paths.sort(); // deterministic pick, so a failure reproduces
835-
paths.into_iter().find_map(|p| {
836-
let text = fs::read_to_string(&p).ok()?;
837-
let snap: ResultSnapshot = serde_json::from_str(&text).ok()?;
838-
pred(&snap).then_some(snap)
839-
})
840-
}
817+
// ── the shapes the published artifact must hold ─────────────────────────────────────────────
818+
819+
// These used to SCAN results/snapshots/ and assert on whatever was committed there. That made
820+
// the test suite a function of the board's contents: clearing the results to start the engine
821+
// era with a clean board turned two of them red, not because the types regressed but because
822+
// their fixtures had been deleted. A test that a `git rm` can fail is measuring the wrong thing.
823+
//
824+
// The invariants they were really guarding are properties of the TYPES, so they are asserted
825+
// against a snapshot built right here. Whether real artifacts parse is a stronger claim and is
826+
// covered where it belongs: engine/tests/end_to_end.rs drives the real binary and reads back the
827+
// snapshot it actually wrote.
841828

829+
// A served cell carries its perf block through a serialise/deserialise round trip. The wire is
830+
// the boundary this file exists to defend: every consumer sees the JSON, not the struct.
842831
#[test]
843-
fn deserialises_a_real_committed_snapshot() {
844-
let snap = any_snapshot_with(|s| {
845-
s.matrix.served && s.matrix.upstreams.contains_key("openai")
846-
})
847-
.expect("the corpus should contain at least one served snapshot with an openai egress");
848-
assert!(!snap.gateway.is_empty());
849-
assert_eq!(snap.schema_version, 1);
850-
assert!(snap.matrix.served);
851-
let openai_egress = snap.matrix.upstreams.get("openai").expect("openai egress present");
852-
assert!(openai_egress.served);
853-
let cell = openai_egress.cells.get("openai").expect("openai>openai cell present");
832+
fn a_served_cell_keeps_its_measured_throughput_across_the_wire() {
833+
let snap = sample_record();
834+
let js = serde_json::to_string(&snap).expect("a snapshot must serialise");
835+
let back: ResultSnapshot = serde_json::from_str(&js).expect("its own output must parse");
836+
assert_eq!(back.schema_version, 1);
837+
assert!(back.matrix.served);
838+
let egress = back.matrix.upstreams.values().next().expect("an egress row");
839+
assert!(egress.served);
840+
let cell = egress.cells.values().next().expect("a cell");
854841
assert!(matches!(cell.served, Served::Bool(true)));
855-
let perf = cell.perf.as_ref().expect("served cell carries perf");
856-
assert!(perf.rps_max_proxy.is_measured());
842+
let perf = cell.perf.as_ref().expect("a served cell carries perf");
843+
assert!(perf.rps_max_proxy.is_measured(), "a measured peak must survive the round trip");
857844
}
858845

846+
// The other half: a cell that was not served must not arrive carrying numbers. Fabricating a
847+
// perf block for an unserved cell would publish a capability the gateway never demonstrated.
859848
#[test]
860-
fn deserialises_every_real_committed_snapshot() {
861-
let dir = snapshots_dir();
862-
let entries = fs::read_dir(&dir)
863-
.unwrap_or_else(|e| panic!("snapshots dir {} should be readable: {e}", dir.display()));
864-
let mut checked = 0;
865-
for entry in entries {
866-
let entry = entry.unwrap();
867-
let path = entry.path();
868-
if path.extension().and_then(|e| e.to_str()) != Some("json") {
869-
continue;
849+
fn an_unserved_cell_carries_no_perf_across_the_wire() {
850+
let mut snap = sample_record();
851+
for up in snap.matrix.upstreams.values_mut() {
852+
up.served = false;
853+
for cell in up.cells.values_mut() {
854+
cell.served = Served::Bool(false);
855+
cell.perf = None;
856+
cell.stream = None;
870857
}
871-
let text = fs::read_to_string(&path).unwrap();
872-
let snap: ResultSnapshot = serde_json::from_str(&text).unwrap_or_else(|e| {
873-
panic!("{} must parse under these types: {e}", path.display())
874-
});
875-
assert!(!snap.gateway.is_empty());
876-
assert!(!snap.matrix.upstreams.is_empty());
877-
checked += 1;
878858
}
879-
assert!(checked > 0, "expected at least one committed snapshot fixture to exist");
880-
}
881-
882-
#[test]
883-
fn deserialising_a_not_served_cell_carries_no_perf_or_stream() {
884-
// Again discovered, not named: any snapshot in the corpus with an unserved egress will do.
885-
let snap = any_snapshot_with(|s| s.matrix.upstreams.values().any(|u| !u.served))
886-
.expect("the corpus should contain at least one snapshot with an unserved egress");
887-
let unserved = snap.matrix.upstreams.values().find(|u| !u.served).expect("checked above");
888-
// A not-served egress cell must not fabricate a perf/stream block from thin air.
889-
for cell in unserved.cells.values() {
890-
if !matches!(cell.served, Served::Bool(true)) {
891-
assert!(cell.perf.is_none());
859+
let js = serde_json::to_string(&snap).expect("a snapshot must serialise");
860+
let back: ResultSnapshot = serde_json::from_str(&js).expect("its own output must parse");
861+
for up in back.matrix.upstreams.values() {
862+
assert!(!up.served);
863+
for cell in up.cells.values() {
864+
assert!(cell.perf.is_none(), "an unserved cell must not carry perf");
865+
assert!(cell.stream.is_none(), "an unserved cell must not carry stream");
892866
}
893867
}
894868
}

‎run-on-ec2.sh‎

Lines changed: 79 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,10 @@ QUALIFY_DIR="$HERE/results/box-qualify"; mkdir -p "$QUALIFY_DIR"
477477
# Where a gateway that exhausted its box budget is recorded. bench_gateway runs in a background
478478
# subshell, so a file is the only way the final summary can see it.
479479
QUALIFY_SKIPPED="$QUALIFY_DIR/skipped-$RUN_ID.txt"; : > "$QUALIFY_SKIPPED"
480+
# One line per gateway that produced a NEW snapshot in THIS run. Written by the boxes (which are
481+
# subshells, so a variable could not carry it back) and read by the field-wide publish at the end.
482+
# Empty means this run measured nothing anywhere, and nothing it derived may be pushed.
483+
FRESH_SNAPSHOTS="$QUALIFY_DIR/measured-$RUN_ID.txt"; : > "$FRESH_SNAPSHOTS"
480484

481485
# _bq_json_field <file> <key> [subkey] -> the scalar, or EMPTY. Never fabricates: a missing key, a
482486
# null, a non-scalar and an unparseable file all read as empty, which every gate treats as unmeasured.
@@ -639,13 +643,29 @@ bench_gateway_once() {
639643
# SAME code. Resolving it per box would let a push mid-run split the field across two revisions and
640644
# publish them side by side as though they were comparable.
641645
glog_echo "cloning $BENCH_REPO @ ${BENCH_COMMIT:0:12} ..."; local _t0=$SECONDS
646+
# SPARSE: the box gets the gateway definitions and lib/, and nothing else.
647+
#
648+
# It used to clone the whole repository, which shipped results/ to every box - including the PREVIOUS
649+
# run's committed results/matrix/<gw>.json. The orchestrator then rsynced that file back off the box
650+
# and accepted it as this run's output, so a box that measured nothing still looked like it had. The
651+
# engine and the mock arrive as prebuilt binaries and the box builds nothing, so no other path is
652+
# needed: whatever is not checked out cannot be read, mistaken for fresh, or run stale.
653+
#
654+
# Same commit, so provenance is unchanged - a sparse checkout narrows WHICH paths are materialised,
655+
# not which revision they come from, and `git rev-parse HEAD` still has to match exactly.
642656
ssh $SSHOPT ubuntu@"$ip" "set -e
643657
rm -rf ~/benchmarking
644-
git clone -q '$BENCH_REPO' ~/benchmarking
658+
git clone -q --filter=blob:none --no-checkout '$BENCH_REPO' ~/benchmarking
645659
cd ~/benchmarking
660+
git sparse-checkout set --no-cone gateways lib
646661
git checkout -q '$BENCH_COMMIT'
647662
# Refuse to measure a tree that is not exactly the requested commit.
648663
test \"\$(git rev-parse HEAD)\" = '$BENCH_COMMIT'
664+
# Refuse to measure if the sparse checkout did not actually materialise what the run needs. A
665+
# silently empty gateways/ would surface much later as a validate failure with a confusing message.
666+
test -d gateways && test -r lib/rig.sh
667+
# And prove the exclusion held: results/ on a box is how the previous run's numbers got recycled.
668+
test ! -e results
649669
" >>"$glog" 2>&1
650670
local _up_rc=$?
651671
if [ "$_up_rc" -ne 0 ]; then
@@ -761,11 +781,18 @@ bench_gateway_once() {
761781
return 1
762782
}
763783

764-
# Matrix is the SOLE producer now — the standalone perf/memory/stream/streamcpu/xlate/governed suites
765-
# are RETIRED. Default to the same `matrix` run-all.sh uses (run-all.sh:75); an explicit SUITES override
766-
# still lets an operator re-run a legacy suite ad hoc. Defaulting to the old 7-suite list here would
767-
# override run-all.sh's matrix default on the box and re-run all six dormant suites (audit HIGH-2).
768-
local ALL_SUITES="${SUITES:-matrix}"
784+
# THE SNAPSHOT IS THE ARTIFACT, and there are no per-suite JSONs any more.
785+
#
786+
# This defaulted to `matrix` and then required results/matrix/<gw>.json to exist before calling a run
787+
# DONE. The engine writes no such file - it writes results/snapshots/<gw>.json and the timestamped
788+
# result_<gw>_<measured_at>.json, and nothing else. The check passed anyway because the box cloned the
789+
# repository with the PREVIOUS run's results/matrix/<gw>.json committed inside it, so the orchestrator
790+
# rsynced git's own copy back off the box and accepted it as this run's output. That is how a run which
791+
# measured nothing could report DONE: the file it was asked to find was one it had shipped there itself.
792+
#
793+
# So: no legacy suite is pulled by default, and freshness is judged on the snapshot alone (below). An
794+
# explicit SUITES=... still drives the old per-suite pull for an ad-hoc re-run of a retired suite.
795+
local ALL_SUITES="${SUITES:-}"
769796
declare -A _pull_state=() _pull_rc=(); local suite
770797
for suite in $ALL_SUITES; do _pull_state[$suite]=unset; _pull_rc[$suite]=0; done
771798

@@ -806,6 +833,9 @@ bench_gateway_once() {
806833
# Every relative path below is relative to the repo, so anchor it rather than inheriting a cwd from
807834
# the login shell that starts this script.
808835
cd ~/benchmarking || exit 1
836+
# The checkout is sparse (gateways + lib only), so results/ does not exist here and the snapshot
837+
# writer does not create its own output directory - it reports an error and writes nothing.
838+
mkdir -p results/snapshots
809839
source ~/.cargo/env 2>/dev/null || true
810840
export CORES=0-3 LOADCORES=4-9 MOCKCORES=10-15
811841
export CAP_MIB=24000
@@ -931,14 +961,28 @@ REMOTE
931961
# is "no config", not a pull failure, so it never contributes to pull_failed.
932962
if [ "$reachable" -eq 1 ]; then pull_config || true; fi
933963
# Pull the run's snapshot artifact(s) BEFORE the teardown trap terminates the box — this is the only
934-
# chance; the box and everything on it are gone straight after. Best-effort like the config sidecar.
935-
if [ "$reachable" -eq 1 ]; then pull_snapshots || true; fi
936-
# DONE means a CLEAN, fully-pulled fresh run. If any suite's pull failed or the guard kept old data,
937-
# this gateway did NOT cleanly refresh - say so loudly so the freshness guard's later hard-fail is
938-
# never a surprise and the gateway can be re-run.
964+
# chance; the box and everything on it are gone straight after.
965+
#
966+
# THIS IS THE RUN'S SUCCESS CRITERION, not a best-effort extra. pull_snapshots returns 0 only when the
967+
# count of result_<gw>_*.json on disk actually GREW, so it answers the one question that matters -
968+
# "did THIS run produce a measurement?" - and it cannot be satisfied by an artifact that was already
969+
# here. Its result used to be discarded with `|| true`, which is how a box that aborted before
970+
# measuring anything still reached the DONE branch and published.
971+
local snap_fresh=0
972+
if [ "$reachable" -eq 1 ] && pull_snapshots; then snap_fresh=1; fi
973+
if [ "$snap_fresh" -eq 0 ]; then
974+
glog_echo "NO FRESH SNAPSHOT for $gw — this run measured nothing, so it publishes nothing and the board keeps whatever it already had"
975+
fi
976+
# DONE means a CLEAN run that MEASURED something and was fully pulled. Anything less is INCOMPLETE, so
977+
# the freshness guard's later hard-fail is never a surprise and the gateway can be re-run.
939978
local publish_failed=0
940-
if [[ "$pull_failed" -eq 0 && "$run_failed" -eq 0 && -f "$HERE/results/matrix/$gw.json" ]]; then
979+
if [[ "$pull_failed" -eq 0 && "$run_failed" -eq 0 && "$snap_fresh" -eq 1 ]]; then
941980
glog_echo "DONE"
981+
# Record that at least one gateway measured something this run. The field-wide history + charts
982+
# publish at the very end reads this: charts.py regenerates from whatever is in results/, so a run
983+
# where every box died would otherwise rebuild the PREVIOUS run's numbers and push them, which is
984+
# indistinguishable from a successful run to anyone reading the board.
985+
echo "$gw" >> "$FRESH_SNAPSHOTS"
942986
# INCREMENTAL PUBLISH: this box finished cleanly and the promote guard passed for every suite, so
943987
# commit + push ONLY this gateway's result now (gated on PUBLISH, serialized across boxes). The
944988
# board fills in gateway-by-gateway; a single-gateway invocation pushes just that one row. The
@@ -951,11 +995,11 @@ REMOTE
951995
publish_failed=1
952996
glog_echo "publish reported an issue for $gw (result IS committed/on disk; push may need a manual retry) — counting it as a run issue"
953997
fi
954-
else glog_echo "INCOMPLETE (a suite crashed, failed to pull, or was guard-held; this gateway did NOT fully refresh - re-run it)"; fi
998+
else glog_echo "INCOMPLETE (the run crashed, measured nothing, or failed to pull; this gateway did NOT refresh - re-run it)"; fi
955999
# Propagate the issue to the caller's `wait "$p" || fail=…` so the summary's issue count is accurate
956-
# and a run missing whole suites — OR a gateway whose publish never reached the remote — is never
1000+
# and a run that measured nothing — OR a gateway whose publish never reached the remote — is never
9571001
# reported as "0 issues" (audit R3-M4/M5 + R3-L1).
958-
if [[ "$pull_failed" -ne 0 || "$run_failed" -ne 0 || "$publish_failed" -ne 0 || ! -f "$HERE/results/matrix/$gw.json" ]]; then return 1; fi
1002+
if [[ "$pull_failed" -ne 0 || "$run_failed" -ne 0 || "$publish_failed" -ne 0 || "$snap_fresh" -eq 0 ]]; then return 1; fi
9591003
return 0
9601004
}
9611005

@@ -980,6 +1024,26 @@ if [ -s "$QUALIFY_SKIPPED" ]; then
9801024
log " (re-run those gateways; their committed results are the PREVIOUS run's, not this one's)"
9811025
fi
9821026

1027+
# ── A RUN THAT MEASURED NOTHING CHANGES NOTHING ───────────────────────────────────────────────────
1028+
# Everything below this line - the append-only history, charts.py, and the push - DERIVES from
1029+
# whatever happens to be sitting in results/. None of it re-reads the boxes. So when every box failed,
1030+
# history/append.py re-appends the old numbers, charts.py rebuilds the PREVIOUS run's charts, and the
1031+
# final sweep pushes them: from outside, a total failure is indistinguishable from a successful run.
1032+
#
1033+
# That is not hypothetical. It was demonstrated live: a box aborted before measuring, the orchestrator
1034+
# logged "all boxes done (1 job(s) reported an issue)", regenerated 24 charts from the previous run's
1035+
# data, and pushed them to origin/main.
1036+
#
1037+
# A failed run must leave the board exactly as it found it, so stop here and leave the working tree
1038+
# alone. `fail` is already non-zero from the boxes themselves, so the exit code still reports it.
1039+
if [ ! -s "$FRESH_SNAPSHOTS" ]; then
1040+
log "NO GATEWAY MEASURED ANYTHING THIS RUN — not appending history, not regenerating charts, not pushing."
1041+
log " The board keeps exactly what it had. Re-run the gateways above; their fanout logs say why each failed."
1042+
rm -f "$PUBLISH_LOCK" 2>/dev/null || true; rmdir "${PUBLISH_LOCK}.d" 2>/dev/null || true
1043+
exit "$fail"
1044+
fi
1045+
log "$(wc -l < "$FRESH_SNAPSHOTS" | tr -d ' ') gateway(s) produced a fresh snapshot this run — regenerating and publishing from them"
1046+
9831047
# ── append this run to the append-only history (results/history/<gw>.jsonl) ─────────────────────
9841048
# Do NOT swallow a failure with `|| true` (audit R3-LOW-4): a malformed result JSON or an unwritable
9851049
# results/history/ would otherwise complete the run "successfully" with the append-only history

0 commit comments

Comments
 (0)