Skip to content

Commit 9a40257

Browse files
hyperpolymathclaude
andcommitted
perf(d-3 followup): harden compare.exs against schema drift (standards#99)
Phase D-3 follow-up under the single-lane HCG tier-2 channel (standards#91). PR #26 (D-4 bootstrap) deferred this as a "separate defensive D-3 follow-up, not coupled to D-4 collection": when bench/baseline.json `_status` is flipped to `active`, a scenario present in results.json but absent from baseline.json (a new harness scenario landed without rebaseline) silently passed the gate, and a scenario present in baseline.json but absent from results.json (the harness dropped a scenario without rebaselining) was never even checked. Both directions of schema drift now fail-closed in active mode and surface as informational "scaffold (would fail: ...)" rows in scaffold-placeholder mode so a rebaseline PR previews the active-mode verdict before the gate is armed. The comparator now iterates the union of scenario names across results and baseline rather than the results map alone, and uses a single `enforce: bool` opt to pivot between scaffold and active mode (replaces the previous `nil` sentinel). check_regression/5 also has a latent crash fixed in the process — when baseline values are TODO sentinels (or any non-number), num/1 returns nil and the `or` chain raises BadBooleanError; the inner `&&` short-circuit already returns nil for unknowns, so the outer joins are switched from `or` to `||` to match. Previously this was masked by scaffold mode never reaching check_regression at all (the `nil` sentinel skipped it); the new flow exposes that path in scaffold mode too. docs/perf-contract.md gains a "Schema drift" section explaining the two directions, the active vs scaffold display difference, and the fail-closed semantic. The behaviour pivots on `_status` in bench/baseline.json — no code change is needed to arm the schema checks once Phase D-4 maintainer-only rebaseline + active flip lands. Smoke-tested locally against synthetic results/baseline fixtures (four cases: active+drift→regressed, scaffold+drift→ok-with-warnings, active+clean→ok, active+TODO-sentinels→ok-no-crash). Build is not verified end-to-end — the session environment has Elixir 1.14 only, no Elixir 1.19 / OTP 28 toolchain — but Code.format_string!/1 reports the file is already formatted and Code.string_to_quoted!/1 round-trips under 1.14. Repo CI (`Perf Regression`, governance, hypatia-scan, dogfood-gate, codeql, scorecard) is the verification gate. Refs hyperpolymath/standards#91 Refs hyperpolymath/standards#99 NOT Closes #99: joint-close is owner-only; D-4 baseline collection plus the `_status` flip to active still pend under #99 after this lands. Same posture as PRs #14, #22, #26. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 430ef58 commit 9a40257

2 files changed

Lines changed: 85 additions & 21 deletions

File tree

bench/compare.exs

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ defmodule Bench.Compare do
4747
defp compare(results, baseline) do
4848
status = Map.get(baseline, "_status", "unknown")
4949
tolerance = Map.get(baseline, "tolerance", %{})
50+
baseline_scenarios = Map.get(baseline, "scenarios", %{})
5051

5152
IO.puts("# Phase D — Performance Regression Report")
5253
IO.puts("")
@@ -59,15 +60,18 @@ defmodule Bench.Compare do
5960
"> SCAFFOLD MODE — bench/baseline.json has not been populated yet " <>
6061
"(Phase D-4 collects the real baseline). This run is informational " <>
6162
"only; the gate is **non-blocking** until baseline.json `_status` " <>
62-
"is flipped to `active`."
63+
"is flipped to `active`. Schema drift (a scenario present in " <>
64+
"results.json but absent from baseline.json, or vice versa) is " <>
65+
"surfaced inline as `scaffold (would fail: ...)` so a rebaseline " <>
66+
"PR previews the active-mode verdict before the gate is armed."
6367
)
6468

6569
IO.puts("")
66-
emit_table(results, nil, tolerance)
70+
emit_table(results, baseline_scenarios, tolerance, enforce: false)
6771
System.halt(0)
6872

6973
"active" ->
70-
emit_table(results, baseline["scenarios"], tolerance)
74+
emit_table(results, baseline_scenarios, tolerance, enforce: true)
7175
|> case do
7276
:ok -> System.halt(0)
7377
:regressed -> System.halt(1)
@@ -80,34 +84,70 @@ defmodule Bench.Compare do
8084
end
8185

8286
# ── Pretty-print + regression check ────────────────────────────────────────
87+
#
88+
# Iterates the UNION of scenario names from results and baseline so neither
89+
# schema-drift direction is silent:
90+
# • results-only scenario → "MISSING IN BASELINE" (new harness scenario
91+
# landed without a rebaseline; the regression gate has no anchor for it).
92+
# • baseline-only scenario → "MISSING IN RESULTS" (the harness dropped a
93+
# scenario the baseline still claims; the gate must not silently pass).
94+
# Both directions fail-closed when `enforce: true` (active mode) and surface
95+
# as informational `scaffold (would fail: ...)` rows when `enforce: false`
96+
# (scaffold-placeholder mode) — see docs/perf-contract.md § Schema drift.
97+
98+
defp emit_table(results, baseline_scenarios, tolerance, opts) do
99+
enforce = Keyword.fetch!(opts, :enforce)
83100

84-
defp emit_table(results, baseline_scenarios, tolerance) do
85101
# Benchee JSON shape: top-level "statistics" -> per-scenario map.
86102
stats = Map.get(results, "statistics", %{})
87103

104+
result_names = stats |> Map.keys() |> MapSet.new()
105+
baseline_names = baseline_scenarios |> Map.keys() |> MapSet.new()
106+
all_names = result_names |> MapSet.union(baseline_names) |> Enum.sort()
107+
88108
IO.puts("| Scenario | p50 (µs) | p95 (µs) | p99 (µs) | Status |")
89109
IO.puts("|----------|----------|----------|----------|--------|")
90110

91-
Enum.reduce(stats, :ok, fn {name, scenario_stats}, acc ->
92-
p50 = percentile_us(scenario_stats, "50")
93-
p95 = percentile_us(scenario_stats, "95")
94-
p99 = percentile_us(scenario_stats, "99")
111+
Enum.reduce(all_names, :ok, fn name, acc ->
112+
in_results = MapSet.member?(result_names, name)
113+
in_baseline = MapSet.member?(baseline_names, name)
114+
115+
{p50, p95, p99} =
116+
if in_results do
117+
scenario_stats = Map.fetch!(stats, name)
118+
119+
{percentile_us(scenario_stats, "50"), percentile_us(scenario_stats, "95"),
120+
percentile_us(scenario_stats, "99")}
121+
else
122+
{nil, nil, nil}
123+
end
124+
125+
{raw_status, drift?} =
126+
cond do
127+
in_results and not in_baseline ->
128+
{"MISSING IN BASELINE", true}
129+
130+
in_baseline and not in_results ->
131+
{"MISSING IN RESULTS", true}
95132

96-
status =
97-
case baseline_scenarios do
98-
nil ->
99-
"scaffold"
133+
true ->
134+
base = Map.fetch!(baseline_scenarios, name)
135+
regressed? = check_regression(base, p50, p95, p99, tolerance) == "REGRESSED"
136+
{if(regressed?, do: "REGRESSED", else: "ok"), regressed?}
137+
end
100138

101-
map ->
102-
base = Map.get(map, name)
103-
check_regression(base, p50, p95, p99, tolerance)
139+
display_status =
140+
cond do
141+
enforce -> raw_status
142+
drift? -> "scaffold (would fail: #{raw_status})"
143+
true -> "scaffold"
104144
end
105145

106-
IO.puts("| #{name} | #{fmt(p50)} | #{fmt(p95)} | #{fmt(p99)} | #{status} |")
146+
IO.puts("| #{name} | #{fmt(p50)} | #{fmt(p95)} | #{fmt(p99)} | #{display_status} |")
107147

108148
cond do
109149
acc == :regressed -> :regressed
110-
status == "REGRESSED" -> :regressed
150+
enforce and drift? -> :regressed
111151
true -> acc
112152
end
113153
end)
@@ -126,8 +166,6 @@ defmodule Bench.Compare do
126166
defp fmt(n) when is_float(n), do: :erlang.float_to_binary(n, decimals: 2)
127167
defp fmt(n), do: to_string(n)
128168

129-
defp check_regression(nil, _, _, _, _), do: "no baseline"
130-
131169
defp check_regression(base, p50, p95, p99, tolerance) do
132170
bp50 = num(base["p50_us"])
133171
bp95 = num(base["p95_us"])
@@ -138,8 +176,8 @@ defmodule Bench.Compare do
138176
t99 = Map.get(tolerance, "p99_max_ratio", 1.50)
139177

140178
breached =
141-
(bp50 && p50 && p50 > bp50 * t50) or
142-
(bp95 && p95 && p95 > bp95 * t95) or
179+
(bp50 && p50 && p50 > bp50 * t50) ||
180+
(bp95 && p95 && p95 > bp95 * t95) ||
143181
(bp99 && p99 && p99 > bp99 * t99)
144182

145183
if breached, do: "REGRESSED", else: "ok"

docs/perf-contract.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,32 @@ The CI gate (`.github/workflows/perf-regression.yml`) fails a PR when
8181
Tolerances are looser as the percentile gets noisier — Phase D-4 will
8282
tighten these once intra-run variance is characterised.
8383

84+
## Schema drift
85+
86+
The comparator iterates the **union** of scenario names from
87+
`bench/results.json` and `bench/baseline.json`. Either direction of
88+
schema drift fails the build in `active` mode:
89+
90+
- **`MISSING IN BASELINE`** — a scenario in `results.json` (the harness
91+
just emitted it) has no entry in `baseline.json`. A new scenario
92+
landed without a rebaseline; the gate has no anchor for it and
93+
cannot meaningfully report regression. Rebaseline before merging.
94+
- **`MISSING IN RESULTS`** — a scenario in `baseline.json` is absent
95+
from `results.json` (the harness skipped or dropped it). Either the
96+
harness regressed silently, or a scenario was removed without
97+
rebaselining the file. The gate must not silently pass.
98+
99+
Both directions are surfaced inline in the markdown table (in the
100+
`Status` column). In `scaffold-placeholder` mode they appear as
101+
informational `scaffold (would fail: MISSING IN BASELINE)` /
102+
`scaffold (would fail: MISSING IN RESULTS)` rows so a rebaseline PR
103+
previews the eventual active-mode verdict before the gate is armed;
104+
the build still passes. In `active` mode they appear as bare
105+
`MISSING IN BASELINE` / `MISSING IN RESULTS` and exit the comparator
106+
with status 1. Behaviour pivots on the single `_status` flag in
107+
`bench/baseline.json` — no code change is needed to arm the schema
108+
checks once the gate goes live.
109+
84110
## Baseline lifecycle
85111

86112
The baseline lives in `bench/baseline.json`. Its `_status` field gates

0 commit comments

Comments
 (0)