MOD-18822 Fail the benchmark job on any KPI floor breach and report it for triage - #1642

redisbench-admin judges each `kpis` floor on a single sample, and the
harness's own run-to-run spread is 22-49% above ~100k rps against floors
set 5% low. Four consecutive nightlies on the identical commit a3b2edb
recorded 20 breaches between them, so the gate currently fails quiet
nights and would pass a real slowdown.

Move the verdict into the repo: run-remote keeps measuring but no longer
decides (continue-on-error), and check_kpis.py compares each result
against its floor and fails only when the previous run saw the same
benchmark breach. The verdict is carried between runs in the Actions
cache, keyed by ref, shard and run id, so a PR run cannot inherit
master's breaches and each of the three shards judges its own subset.

Replaying the four nights through the new gate reproduces the 8 and 12
breaches on 09-19 and 09-21 and exits 0 on all four, per shard and
merged; a 30% slowdown injected into two consecutive nights is reported
on the first and fails the job on the second.

json_nummultby_num_2 (MOD-18654, Won't Do) has no floor, so it is
listed as ungated rather than failed -- otherwise the job stays red
every night and its status still means nothing. A run where no
benchmark produced a result does fail, so continue-on-error cannot hide
a broken harness.

This is a stopgap for the gate, not for the measurement: a regression
smaller than the harness's spread stays invisible. Narrowing that spread
is MOD-18823; per-benchmark margins should follow from its findings.

A benchmark under its floor is under a baseline already set 5% below a
measured value, so any breach is worth a ticket -- whether or not it
repeats the next night. The previous commit reported a first-time breach
only as a log line, which is too easy to lose.

Findings now leave the run in a form the nightly analysis can triage
from: kpi-state/findings.json, uploaded as an artifact, and a table in
the run summary listing every breach worst-shortfall-first. Nothing
files a ticket automatically; that decision stays with the nightly
analysis.

Job status is unchanged and still fails only on a breach the previous
run saw too. Failing on a single one would put the job back to red on
roughly half the nights -- 20 breaches in four nights of identical code
-- which is what MOD-18822 is about.

A result under its floor is a result more than 5% below the baseline,
and the policy for one is red job, ticket filed, floor left alone --
first occurrence, not on the second. So the two-consecutive-runs rule
and all the state it needed are gone: no Actions cache, no previous
verdict to restore or save, and the check's exit code now follows
directly from this run's results.

What stays is the part that makes a breach actionable. redisbench-admin
fails the step for a floor breach and for a benchmark that never ran
with the same exit code, so telling them apart meant grepping the log;
the check names each one, writes the breaches to kpi-state/findings.json
with their shortfall for the nightly analysis to file from, and puts the
same table in the run summary. A run where nothing measured at all still
fails, so handing it the verdict cannot hide a broken harness, and
json_nummultby_num_2 (MOD-18654, Won't Do, no floor) is reported as
ungated rather than deciding the job.

Replaying the four nights of 09-18..09-21: 09-19 and 09-21 fail with 8
and 12 breaches, 09-18 and 09-20 pass, per shard and merged. Those two
nights ran the same commit as the two that passed, so the tickets filed
from them are about the harness's own 22-49% spread -- MOD-18823 -- which
is the thing that has to shrink for this gate to be quiet.

Since run-remote continues on error, an errored benchmark left no trace:
its result file is simply absent, and absent is indistinguishable from
"this shard was never given that benchmark", so a timeout or a single
test error after some successes left the job green. Before this PR
run-remote's own exit code covered that case.

The run log is the only place that records a benchmark was meant to run
and did not, so capture it (tee) and read the
"Failed to run remote benchmark for test '<name>'" lines from it. Each
one is a finding and fails the job, except the names passed as
--known-broken -- json_nummultby_num_2 (MOD-18654, Won't Do) and nothing
else.

Reported by Cursor Bugbot on the PR.

Two more holes of the same shape as the last one: with run-remote under
continue-on-error, anything that leaves a benchmark merely absent read
as "this shard was never given it" and passed.

measured() discarded the reason result_metric returns when a result file
exists but cannot be used -- ambiguous duplicates, or no known
throughput metric in it. That benchmark looked unmeasured and its floor
was never applied; it is now reported unreadable and fails the job.

A step timeout or a crash partway through leaves no log line at all for
the benchmarks that never started, so the log cannot catch it. The
gate now also takes run-remote's step outcome: a non-success that
nothing in the log accounts for -- no breach, no failed-to-run line, not
even a known-broken one -- fails the job as an incomplete run. A timeout
on a night when a known-broken benchmark also errored still reads as
accounted-for; closing that needs each shard's expected benchmark list,
which redisbench-admin does not expose (neither i % 3 nor contiguous
thirds of the sorted yml list reproduces the real 14/13/15 split).

Both reported by Cursor Bugbot. Four-night replay unchanged.

The benchmark gate writes its table on each of the three shard jobs,
which is not where anyone looks. The nightly summary is, and it carried
only the job name: "benchmark - job failed", with no indication of which
benchmark or by how much.

So the summary job now restates the breaches, in the same table the
benchmark job renders, and sends them to Slack as one section of their
own -- a floor breach is a performance finding, not a failed test, so it
does not belong folded into the failed-OS list. No new download: the
existing step already pulls every artifact of the run, kpi findings
included. The three shards each report the same ungated benchmark, hence
unique_by.

Rendered against last night's run (35780480546), the Slack section reads:

  *Benchmark KPI floors:*
  • *json_set_key_empty* - 134217 vs floor 163624 (-18%)
  • *json_vs_hashes_hset_key_simple* - 136977 vs floor 165889 (-17.4%)
  • *json_set_sclr_pass_100_json* - 116244 vs floor 129489 (-10.2%)
  • *json_vs_hashes_json.set_key_simple* - 133769 vs floor 135841 (-1.5%)

The block is built with jq rather than interpolated into the payload
format string, so a benchmark name or a percentage cannot break it, and
the step output is passed in as an env var rather than expanded inside
the script.

Brings in #1643 (MOD-18654), which bounds json_nummultby_num_2 so the
benchmark runs again -- the premise this branch was written against.

Two findings on the previous commit, both about the same blind spot: an
ungated benchmark is reported by every shard, but only the shard that
was given it has a number.

The summary merged those copies with unique_by, which keeps whichever
came first, so the nightly table and the Slack block could say "no
result" for a benchmark another shard had timed. It now dedupes on
(benchmark, status) preferring the copy with a measured value. This
only became visible with #1643: before it, json_nummultby_num_2 had no
result on any shard.

And verdict() classified an unusable result before checking for a
floor, so a floor-less benchmark with an unreadable result produced an
UNREADABLE row with floor=None, which the summary table formats as a
number -- TypeError, after findings.json was already written. Ungated is
now decided first: with no floor there is nothing for an unusable result
to be measured against. Covered in --self-test, which also renders the
step summary for that row rather than only checking the verdict.

Four-night replay unchanged. Reported by Cursor Bugbot.

Both of these are the asymmetry the previous commit fixed in the nightly
summary and left in the gate's own table.

An ungated benchmark is listed but is not a finding, and step_summary
appended the file-a-ticket footer whenever the list was non-empty -- and
with a floor-less benchmark in the set it is never empty. So a clean run
told the reader to file a ticket, which on push-to-integ is the only KPI
report they see. The footer now follows the rows that need a ticket, and
an otherwise clean run says its floors were met.

And a benchmark that failed to run was reported twice when it also had
no floor: once as errored from the run log, once as ungated from the
verdict, both asking for a ticket, and the nightly merge groups by
(benchmark, status) so both survived. The errored row is the actionable
one and now the only one.

Rendered for both cases: the 09-18 artifacts produce the ungated row
plus "Every gated benchmark met its floor", 09-21 produces the 12
breaches plus the footer. Reported by Cursor Bugbot.