Repository navigation
test(g500-ladder): the phase metric policy table assumes work proportional to data, which the redesign removes #1433
Description
Summary
PHASE_METRIC_POLICIES in crates/graphforge-api/tests/scale_g500_ladder.rs encodes the pre-redesign architecture, in which nearly every lifecycle phase did work proportional to the data. The write-path redesign exists to stop doing proportional work. So as it lands, rows in this table fail one by one — and each failure needs a judgement that cannot be made generically.
Four pull requests hit this in one night, each on a different row:
| PR | row and field | observed 1x/2x/4x | verdict |
|---|---|---|---|
| #1412 | recovery_reauthentication.read_calls |
69, 68, 70 | legitimate regime change — fixed, see below |
| #1413 | cas_install_read_write.write_bytes |
no positive growth | install-by-rename makes writes ~O(1) |
| #1425 | hydration_verification.read_bytes |
87475, 87479, 87479 | a real defect — see the warning below |
| #1430 | fsync_synchronization.fsync_calls |
2195, 2089, 2063 | merge tree deleted, fewer files to fsync |
Verified by checking out each branch and running the failing test, not by assuming a shared cause.
The warning that makes this urgent
A flat metric is not automatically a regime change. On #1425 it was a genuine data-integrity bug.
#1425's hydration_verification.read_bytes went flat because the PR removed the only content verification of Topology-role graph objects on the open path. An independent review proved a one-byte corruption of topology/nodes.parquet — same inode, same length — is silently accepted, and a query then returns a count over corrupted data. The table's assertion caught that.
So the reflex "the redesign removes proportional work, therefore flat is expected" is true often enough to be dangerous. It is exactly the argument that would have waved #1425 through. Every row needs a judgement, and the judgement has to distinguish stopped doing redundant work from stopped doing necessary work — which look identical at a growth assertion.
What #1412 established, as a worked example
Root cause traced causally rather than inferred: temporary instrumentation at every increment site, run, then reverted with no production code touched. The 69/68/70 is the sum of three byte-independent contributors — fixed 14 from supersession-read accounting, ~27-31 from authenticating a published successor's manifest and files, fixed 27 from the interrupted-import recovery drill. The plus-or-minus-one traces to compact_parent_inventory's manifest-tree packing moving by one page between rungs. read_bytes still scales, so SCALE still holds on that field.
Its replacement policy, BoundedObjectCalls, keeps the per-rung buffered bounds and adds two properties instead of deleting monotonicity:
- Cross-rung spread capped at 20% of the minimum. Reverting to per-byte authentication moves calls roughly in step with bytes, and bytes must grow substantially under
SCALE, so a revert breaches it. - An absolute floor. Spread alone is blind to a component dropped uniformly from every rung, because a uniform drop keeps spread small. The floor is sized from the three known contributors: losing any one drops the total below it.
Both proven by mutation, with two deliberate mutations added to the policy test — one where calls track byte growth, one where calls are held uniformly constant below the floor.
Proposed shape
Make each row declare its regime explicitly rather than defaulting to proportional:
- (a) data-proportional — today's
ScaleBearing/BufferedCalls, unchanged. - (b) structure-bounded — tied to a real, small, mostly-fixed count (objects, files, partitions, commit boundaries), with a spread tolerance and an absolute floor.
- (c) fixed-protocol — today's
FixedProtocol, genuinely constant.
The condition that makes this worth doing rather than harmful: a (b) floor must be justified from an actual structural quantity the redesign still guarantees — a partition count, a file count, a "one fsync per commit boundary" invariant. A floor reverse-engineered from one PR's observed numbers is the same trap in a new shape, and would make every future regression explainable.
Acceptance
- Every row changed states which regime it declares and why, anchored to a structural quantity rather than to observed output.
- Each new bound is proven by mutation: a deliberate regression must fail it. State what that regression is for each row.
- perf(storage): memoize and cheapen the open-path generation inventory sweep #1425's row is restored, not widened. Its flat bytes are a real coverage loss; the content check must come back before that row is re-declared.
- No row is relaxed without a named replacement property.
Not in scope
#1412 is already fixed and landed separately; this generalises it. The per-row judgement for #1413, #1425 and #1430 belongs to whoever owns those changes, but the table shape belongs here.
Activity
DecisionNerd commentedon Sep 17, 2026
Correction: the #1430 fsync row was a REAL regression, not a regime change
This issue lists PR #1430's
fsync_synchronization.fsync_calls=[2195, 2089, 2063]with the verdict "merge tree deleted, fewer files to fsync". That reading was wrong, and it is worth correcting here because it is exactly the trap this issue exists to warn about.The agent measured
mainon the same rungs rather than assuming:mainS5's counts were not fewer. They were flat and roughly 10x higher at 1x. The assertion was catching a genuine durability regression.
Cause. Each partition costs a durable spill with its own barrier and receipt in every family. Cutting the full recorded 256 partitions regardless of input size made durability cost a constant — a 2,000-row graph paying a 67M-row graph's price — so shaping's barrier count stopped tracking the work it protects.
Fixed at the cause, not the assertion. The cut is now
min(partition_count, max(1, staged_identity_records / 16)). The floor isBALANCE_MIN_MEAN_ROWSdeliberately: below it the balance check is disabled, so cutting past it would create partitions the balance assertion cannot validate. It does not make the partition count machine-dependent — the bound is a pure function of the record count in the chunk receipts, so identical input cuts identically on any host, and at 67.1M identities the recorded 256 is still binding.Result:
[1143, 1579, 2063]— monotone.ScaleBearingpasses untouched and no test policy was changed.Residual, reported rather than hidden: at ladder rungs the absolute count is still ~5x main's, because those rungs sit below the crossover. The merge tree's barriers grew with chunk count; partitioning's are bounded by partition count, so the crossover is 256 chunks — S20 and above. Every production scale does strictly fewer barriers; only small graphs pay more. fsync is 0.25% of ingest and an explicit non-goal of #1387.
Why this matters for the table work
Two of the four rows catalogued here have now turned out to be real defects, not regime changes.
hydration_verification.read_byteson perf(storage): memoize and cheapen the open-path generation inventory sweep #1425 went flat because the only content check covering Topology-role objects had been removed — a one-byte corruption was silently accepted and a query returned a count over corrupted data.fsync_synchronization.fsync_callson perf(storage): range-partition shaping and delete the external merge tree #1430, above.So the reflex reading — the redesign removes proportional work, therefore flat is expected — has now been wrong on half the observed cases. Both times the assertion was right and the interpretation was wrong.
The acceptance criterion in this issue stands and is reinforced: a bound must be anchored to a structural quantity the redesign still guarantees, never reverse-engineered from observed output. Anyone restructuring a row should first measure
mainon the same rungs, as was done here, before deciding a row's regime changed at all.DecisionNerd commentedon Sep 17, 2026
Correction: the #1425 row was misattributed, and this one is mine
This issue records #1425's
hydration_verification.read_bytes=[87475, 87479, 87479]as a real defect — the removed Topology content check — and uses it as one of "two of four rows that turned out to be real defects rather than legitimate flattening."That attribution is wrong.
PR #1435 restores the content check #1425 removed. It carries both commits:
4a92035b(the O1 memoization, kept) and3e624070(the restoration). And it produces the identical flat values:Byte for byte the same as #1425's. Restoring the check changed this row not at all.
Main is green on this test, so the row grows on main. By elimination the flatness comes from the commit #1435 keeps, not the one it reverses: O1's memoization of
graph_files_inventory(). That call previously ran 3-4 times per open; memoized, it runs once. The repeated reads were what scaled, so removing the repetition flattened the row.So this row is a legitimate regime change after all — the phase genuinely stopped doing proportional work, which is exactly the category this issue exists to handle properly.
What does NOT change
The corruption was real and is not in question. It was proven by mutation, independently of any metric: a one-byte corruption of
topology/nodes.parquet— same inode, same declared length — was accepted at open on #1425, and an ordinaryMATCH (n) RETURN count(n)returned a count over corrupted data. The baseline commit refused it. #1435 restores the refusal and shipshardlinked_topology_payload_corruption_is_refused, which fails against #1425 and passes with the fix.Two separate things were conflated: a data-integrity defect, and a metric row going flat. Both were present on #1425. Only the first was caused by the removed check.
The tally in this issue, corrected
recovery_reauthentication.read_callsBoundedObjectCallsfsync_synchronization.fsync_callshydration_verification.read_bytescas_install_read_write.write_bytesSo one confirmed real defect among the resolved rows, not two.
The lesson stands, and is sharper for this
The rule this issue insists on — measure the baseline on the same rungs before declaring a row's regime changed — is exactly what would have caught my error. #1430's agent did that and found main at
210, 278, 414against its own flat2195, 2089, 2063, proving a real regression. Nobody did it for this row; I inferred the cause from a coincidence of timing, because the corruption and the flat metric appeared in the same pull request.A row going flat at the same moment a defect is discovered is not evidence the defect caused it.
test(g500-ladder): declare hydration_verification.read_bytes bounded,…
test: move encoded-corruption refusal to the CAS install and re-size …
test: move encoded-corruption refusal to the CAS install and re-size …
DecisionNerd commentedon Sep 29, 2026
Rename request folded into #1623: the ladder phase currently labelled
validationis the stage+seal construction path, not a check. #1623 owns the rename in the policy table and receipts; coordinate the table redesign so the new name lands once.test(g500-ladder): floor CAS installs and shape merge bytes (#1433)
fix(storage): restore the content check CurateLabs#1425 removed for T…
perf(storage): stop re-hashing the encoded inventory three times per …
test(g500-ladder): declare each phase metric's regime explicitly (Cur…