feat(pixelflow-search): budget saturation against LIVE e-classes, not allocated slots

[jppittman]

What this is

Two commits: the measurement that found the defect, and the fix it recommended, A/B'd on the same corpus.

EGraph::union (graph.rs) merges through the union-find and never removes an entry from self.classes. So num_classes() — self.classes.len() — is ALLOCATED class slots and only ever grows, while the live graph is class_ids(): canonical AND non-empty. The production budget check was self.classes.len() > max_classes. Consequence: the classical tier's 5 000-class cap trips at a median 1 352 live classes — 3 648 classes of slack below its own budget, spent on ghosts.

The honest tension, which the A/B settles rather than assumes: the cap's stated purpose is memory protection, and memory is consumed by allocated classes — so counting allocated was defensible for memory while being wrong for quality. The fix is to stop making one number mean two populations.

The change

• EGraph::live_classes — canonical AND non-empty, exactly class_ids().count(), maintained in O(1) at the only three sites that can move it (add, union, rebuild_budgeted's drain/refill). Each site computes its delta from the emptiness it actually observes, not from "canonical implies non-empty" — which is false precisely inside rebuild's mem::take window, where congruence closure runs. Underflow is checked_sub().expect(), never a wrap.
• ClassCeiling::{Live, Allocated} / ClassCeilings { live, allocated } — the live budget is the search knob; the allocated ceiling is the memory guard, held at the existing HARD_CLASS_LIMIT (100 000, from fix(pixelflow-search): the over-budget e-graph cannot be mistaken for a class id (L1) #1107). They are grouped in a struct so a call site cannot silently swap them. SaturationStop::ClassCap and ScanStop::ClassCap now carry which ceiling fired; telemetry emits class_cap_live vs class_cap_allocated.
• Budget::Explicit and Limits name both ceilings; OptimizerStats reports both counts. That is what lets the A/B run the old policy exactly (classes: HARD_CLASS_LIMIT, allocated_classes: preset.max_classes reduces every check to the old classes.len() > max_classes) through the public API, in the same process as the new one.

Drift is checked, not argued: debug_assert_live_count() holds the counter against class_ids().count() at the top of every saturation round, and tests/live_class_count.rs asserts it after every mutation — including interleaved rule batches with partial rebuilds, which is the path that exercises the drain window.

How the two ceilings relate to HARD_CLASS_LIMIT: unchanged in value and role. It was the clamp on the single cap; it is now the clamp on the allocated ceiling, and production sets the allocated ceiling to exactly it. On this corpus it never fires (max allocated observed: 10 660, 10.7% of the guard) — the live budget is what bounds every run.

A/B — 206 real kernels + 200 synthetics, Optimizer::production(), CostModel::latency_prior()

Both arms run back to back in one process. Deterministic metrics reproduced bit-identically across three runs. Full data: docs/results/2026-09-02-class-cap-live-ab.{md,csv,json}.

group	n	cost median	cost p90	improved	regressed	live median	allocated median	alloc worst	bytes-proxy median	apps median	wall median	capped before→after
glyph16	95	+2.09%	+6.27%	75	1	1703→2453	4718→7225	2.14x	804 KB→1.22 MB	2.03x	1.48x	89→88
glyph32	95	+2.09%	+6.27%	75	1	1703→2453	4718→7225	2.14x	804 KB→1.22 MB	2.03x	1.47x	89→88
shader	12	+0.00%	+8.82%	5	1	311→406	1794→3648	2.57x	304 KB→618 KB	1.61x	1.33x	9→9
psychedelic	1	−5.35%	−5.35%	0	1	946→2561	4847→9885	2.04x	823 KB→1.67 MB	1.90x	1.70x	1→1
cellgrid	3	+0.00%	+0.00%	0	0	1261→1763	4583→8034	1.75x	779 KB→1.36 MB	1.67x	1.41x	3→3
synthetic	200	+0.00%	+0.00%	3	6	67→67	161→161	2.51x	27 KB→27 KB	1.00x	0.98x	40→37
REAL (all)	206	+2.03%	+6.27%	155	4	1379→2172	4688→7220	2.57x	798 KB→1.22 MB	2.00x	1.47x	191→189
REAL cap-hit	191	+2.14%	+6.45%	155	4	1703→2453	4718→7298	2.57x	804 KB→1.23 MB	2.06x	1.48x	191→189
ALL	406	+0.00%	+4.60%	158	10	420→562	2884→4724	2.57x	495 KB→807 KB	1.54x	1.24x	231→226

Memory (allocated classes, nodes and memo entries are all monotone across a run, so end-of-run is peak): median allocated 4 688 → 7 220 on real kernels, worst-case ratio 2.57x; the largest graph any kernel allocates goes 5 226 → 10 660 classes, i.e. 878 KB → 1.83 MB by the documented byte proxy. The 100 000-class guard fired on zero kernels. The docs/.../class-cap-ghosts.md recommendation warned of a possible ~12x worst case; measured, it is 2.57x, because the live budget stops the run long before the ghost ratio compounds.

Compile-time cost — this is the part #1109 taught us to put in the body rather than discover later. Rule applications, the deterministic proxy, go 2.00x median / 3.35x p90 on real kernels (61 015 → 107 314 at the max). Aggregate wall clock over the real corpus is 7.78 s → 23.71 s (3.05x) — context only, the machine was loaded (load avg ~5), never a metric.

Why this is DRAFT, not armed for auto-merge

The pre-agreed gate was: auto-merge only if no real kernel regresses in cost and compile-time cost is under 2x. Both fail:

kernel	group	before	after	delta
glyph16:U+004B	glyph16	1047	1121	+74
glyph32:U+004B	glyph32	1047	1121	+74
psychedelic	psychedelic	766	807	+41
shader:julia_set	shader	716	728	+12

The regressions are not a bug in the counter — they are the honest shape of the change. Extraction is a greedy DP over a static cost prior, not an optimum, so a larger e-graph does not monotonically produce a cheaper term: more equalities can move the greedy choice onto a worse branch. Four of 206 real kernels land that way; 155 improve.

So the decision this PR is asking for is a trade, stated plainly: +2.03% median extracted cost on real kernels, for ~2x saturation work and ~2x peak e-graph, with 4/206 kernels getting worse. That is JP's call, not a green-CI call.

Three ways to shrink the ask, if the trade as measured is too expensive:

1. Keep the live budget but lower max_classes for the classical tier, so the effective search is closer to today's and the fix is purely a correctness one (the budget then means what it says). Cheap to measure with the same harness.
2. Land the counter and the typed ceilings now (which are correct regardless) but keep production budgeting on Allocated behind Budget::Explicit, deferring the policy flip.
3. Chase the 4 regressions first — they are an extraction-quality question (greedy DP vs. cost prior), not a budget question, and they will recur for any change that grows the graph.

Gates

cargo build --workspace, cargo test --workspace, cargo clippy --workspace --all-targets -D warnings, cargo fmt --check, and both no-default-features checks (pixelflow-ir, pixelflow-core) all pass.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[github-actions]

🤖 Hi @jppittman, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

[jppittman]

HOLD for JP — not armed for auto-merge, by the pre-agreed gate.

The gate was: arm squash auto-merge only if no real kernel regresses in extracted cost AND the compile-time cost is under 2x. Measured, both fail — 4 of 206 real kernels regress (largest +74 on glyph16/32:U+004B), and rule applications go 2.00x median / 3.35x p90 on real kernels (aggregate wall 3.05x, loaded, context only).

So this is a judgement call, and the numbers are all in the body. The short version:

• The defect is real and confirmed in code: the budget counted allocated class slots, and union never frees one, so a 5 000-class budget stopped at a median 1 352 live classes.
• The fix is correct and cheap: an O(1) live counter maintained at the three sites that move it, checked against class_ids().count() by debug_assert on every saturation round and after every mutation in the new test. The typed ClassCeiling::{Live, Allocated} split is right regardless of which policy we ship.
• The payoff is +2.03% median extracted cost on real kernels (155 of 206 improve), and the price is ~2x saturation work and ~2x peak e-graph (worst kernel 878 KB → 1.83 MB; the 100 000-class memory guard never fires).
• The 4 regressions are not a counter bug — they are greedy extraction over a static cost prior on a bigger graph, which is not monotone.

Three smaller asks are listed at the end of the body if the trade as measured is too expensive; option 2 in particular lands the counter and the typed ceilings (correct either way) while deferring the policy flip.

[jppittman]

Orchestrator synthesis. The HOLD is right, and the reason is bigger than this PR.

What this measured. Live-counted budget: real kernels median +2.03% extracted cost (p90 +6.27%), 155 improved / 4 regressed, live classes at stop 1,379 → 2,172, memory 2.57x worst case (the ghost doc feared 12x), HARD_CLASS_LIMIT never fired. Price: rule applications 2.00x median / 3.35x p90.

The pattern across three independent changes tonight. Every change that gives saturation more room shows the same signature — most kernels improve, a minority regress, and the psychedelic shader is in the regressing minority every time:

change	improved	regressed	psychedelic
numeric-first rule order (#1101)	146/204	21/204	1.044 (worse)
remove the cap-break (#1109 arm B)	140/204	16/204	unchanged
live-counted cap (this PR)	155/206	4/206	−5.35% (worse)

This PR's own concerns section names the mechanism, and it is correct: "extraction is a greedy DP over a static cost prior, so a larger e-graph is not monotonically better — more equalities can move the greedy choice onto a worse branch. Any change that grows the graph will hit this."

So the binding constraint is extraction, not the saturation budget. Issue #1111 established that extract_dag is not argmin twice over — it sums tree costs (sharing is never priced) and it is a single DFS rather than a fixpoint, so a class whose child is still on the stack is scored at CYCLE_COST and never revisited — plus it reports a total_cost computed before repair_choices_well_founded mutates the choices. Every "give saturation more room" change is currently gated by that, and pays 2-3x compile time to buy a couple of percent through a lossy chooser.

Recommendation: hold this PR (as it is), and size the extraction gap first — how much cost is left on the table by the greedy DP versus an exact reference on kernels small enough to solve exactly. If the extraction gap is large, fixing it may deliver more than 2% at no compile-time cost, and it would very likely turn these three regression minorities into improvements. If it is small, then the 2%-for-2x trade here is the real trade and JP can take it or leave it on its own terms.

Not armed; the pre-agreed gate failed on both clauses (4 regressions, 2.00x compile) and it should stay failed until the sequencing question is settled.

🤖 Held by Claude Code