RUE-1543: propose ADR-0067 and ADR-0071 amendments on evidence and store size - #2444
RUE-1543: propose ADR-0067 and ADR-0071 amendments on evidence and store size#2444DorianListens wants to merge 5 commits into
Conversation
steveklabnik
left a comment
There was a problem hiding this comment.
The measurement and append-without-rewriting argument are compelling, but the claimed independence of the schema rollout and corpus compaction conflicts with the current reader contract. The amendment needs to resolve that compatibility requirement before it can be accepted.
| ### Relationship to the other amendment | ||
|
|
||
| If Question 1 is answered as recommended, the two amendments are | ||
| **independent**: the encoding can be adopted for new records without compacting |
There was a problem hiding this comment.
RUN_SCHEMA_VERSION is not currently a multi-version decoding axis: validate_run rejects every run whose version differs from the single current constant, and lib.rs explicitly says there is “no compatibility path, by design.” If new records switch to v2 while the old v1 records remain at the tip, derive will reject the old corpus, so adopting the encoding for new records without compacting is not independent under the current architecture. Please either make dual v1/v2 decoding and validation (including amending the no-compatibility invariant) part of this amendment, or require an atomic corpus conversion with the schema rollout, which couples these decisions.
There was a problem hiding this comment.
You're right, and the claim was worse than imprecise — it was false. Fixed in 5e9b172.
I confirmed the mechanism: validate.rs:401 compares against the single RUN_SCHEMA_VERSION constant and returns UnsupportedSchemaVersion without evaluating anything else, with the intent stated in lib.rs:139 and repeated at validate.rs:34. Bumping the constant doesn't add a version, it replaces the only one readers accept.
The consequence is also worse than I'd assumed, and it's now in the text: with v2 records written while the 1,188 v1 records remain, derive routes all of them to rejected and derives no platform — and validate-performance-stall.py reads the empty platforms list as "no plotted points yet; nothing to stall" and exits 0. The gate built to catch a stopped series can't see a totally rejected corpus, so a botched rollout is silent in exactly the way §"A series may not stall silently" forbids.
I took the first of your two routes, as a new Question 1a that is explicitly a prerequisite rather than a preference:
- Readers implement every schema version that can still be in the store;
RUN_SCHEMA_VERSIONbecomes "what the producer writes", and refusal applies to versions ahead of the reader, not behind it. Both prose invariants are amended by name. - Encoding shape dispatches on
schema_version; what must be proven keeps dispatching on the suite'sprotocol_version. These axes now cross, which is the defect that would bite next:check_boundary_evidencekeyslen == batch_sizeoff protocol v2, so against a v2-encoded record it has to checkboundary_processesrather thanboundary_evidence. - v1 support may be dropped only once no consumer can reach a v1 record — and permanently retained if compaction is declined, which is now stated as a cost of declining rather than a later surprise.
On your second route: I recorded it and explained why I didn't take it. An atomic reader-and-corpus cutover isn't achievable across a repository merge and a data-branch push, and its failure window is precisely the silent one above — so it's not implementable rather than merely less attractive.
The relationship section is corrected too: the dual reader is a prerequisite for both amendments and must land first; after it, the two are independent in outcome and ordered in execution, with both permitted sequences spelled out. ADR-0071 Amendment 1 now says it cannot be adopted alone, and the supporting note's independence section carries the correction and the measured consequence.
Statuses unchanged — both still proposal, no ruling recorded.
steveklabnik
left a comment
There was a problem hiding this comment.
Question 1a is the right correction and the right shape — readers implement every version still in the store, refusal applies forward not backward, and encoding dispatches on schema_version while what-must-be-proven keeps dispatching on protocol_version. I checked the mechanism you cite and it holds: validate_run returns UnsupportedSchemaVersion before evaluating anything (validate.rs:401), and the stall gate's empty-platforms path really does return 0 (scripts/validate-performance-stall.py:390). Saying plainly that the atomic cutover is not implementable rather than merely less attractive is the correct call.
Two things to fix before this is ruled on, both in the evidence rather than the recommendations.
The compaction numbers were measured without the version bump they depend on. 311-of-1,188 and "Epoch 2's three pins are untouched" are only true if re-encoded records keep schema_version = 1, and Q1a needs them to say 2. Whichever way you resolve it, something in the text has to change — either the address and pin counts, or Q1a.3's claim that compaction ever lets us drop v1. Detail inline; it lands on the retired-epoch silent-failure case you already identified, which makes the unindexed() gate extension a hard prerequisite rather than a recommendation.
The per-process digest assumes one worker. Digesting {runner, compiler, compiler_work} as a unit works because every boundary epoch today is worker_setting = "one". check_boundary_evidence gates work identity on exactly that and leaves output identity ungated, precisely because parallel rows are expected to disagree on compiler_work — so under the automatic-worker regime this ADR already targets, the encoding loses the guarantee that was never optional. Splitting the digest in two looks like it costs nothing worth measuring.
On the rulings themselves, for when the numbers are settled: I'm persuaded by ADR-0071 Amendment 1 (S4 over S1 — the 0.6 points buys the only per-commit critical path we keep), by RUN_SCHEMA_VERSION as the axis on the strength of the 0-of-1,188 equivalence, and by append-not-rewrite. The declined force-push is argued correctly and for the right reason: unreachable objects stay fetchable, GitHub's maintenance is not available on request, and 42 forks make "nobody else has this data" unverifiable.
The inventory work here is unusually good — the consumer inventory checks out against the source, and scaling.rs really does only ever take boundary_evidence.first(), so S4 leaves it whole.
Generated by Claude Code
| Only **311 of 1,188 addresses move**. Epochs 2 and 4 carry no boundary evidence | ||
| and are byte-identical after re-encoding, as are six epoch-5 records. The | ||
| required repository changes are exactly: | ||
|
|
||
| | Site | Change | | ||
| | --- | --- | | ||
| | `performance/manifest.toml` | re-pin 6 `[epoch.baseline] run` values (epochs 5 and 6, three platforms) and the epoch-5 `reference_run`. Epoch 2's three pins are untouched. | |
There was a problem hiding this comment.
These numbers come from a re-encode that does not touch schema_version, and Question 1a requires one that does.
schema_version is an ordinary field of RunObject (crates/rue-perf-schema/src/run.rs:457) with no skip_serializing_if, so it is part of the canonical form content_address digests. A record whose only change is 1 → 2 gets a new address. The supporting note reports the opposite outcome — "Epoch 2 (868) and epoch 4 (3) carry no evidence and are byte-identical" — and byte-identical is only reachable if the prototype left the version at 1.
So one of two things has to give, and each contradicts text currently in the amendment:
- Re-encoded records declare v2, as Q1a needs. Then all 1,188 addresses move, not 311; all nine
[epoch.baseline] runpins are re-pinned, not six; and "Epoch 2's three pins are untouched" is false. That last one matters more than the arithmetic: epoch 2 is retired, so its three pins land in exactly the silent-failure class you name two paragraphs down — a wrong baseline on a retired epoch publishes no index while still plotting every series, andunindexed()iterates live epochs only. The gate extension you make a condition of acceptance goes from prudent to load-bearing. The retired-epochs-only variant inherits the same correction. - Evidence-free records stay at v1. The numbers stand, but 877 v1 records remain at the tip permanently, so Q1a.3's condition for dropping v1 support — "after a compaction that removes the last v1 record from the tip" — is unreachable by the compaction this amendment proposes. That is a fine thing to decide, but it should be said, because Q1a.3 currently reads as though accepting Question 2 buys an eventual exit from dual-reader maintenance, and under this option it does not.
Neither resolution changes what you are recommending. Both change the breakage inventory the ruling is made against, which is the part I would be relying on.
Generated by Claude Code
There was a problem hiding this comment.
Confirmed, and I took the first resolution: re-encoded records declare schema_version = 2, and every address moves. Fixed in 8b9a937.
I verified the mechanism you name. schema_version is an ordinary pub field of RunObject (run.rs:457) with no skip_serializing_if, and the struct derives Serialize, so it is part of the canonical form content_address digests. A record whose only change is 1 → 2 gets a new name. You are also right about where the old numbers came from: the prototype left the version at 1, which is the only way epochs 2 and 4 could come out byte-identical.
The corrected inventory:
| was | now | |
|---|---|---|
| addresses moved, full re-encode | 311 | 1,188 |
| addresses moved, retired-only | 242 | 1,119 |
[epoch.baseline] run pins re-pinned |
6 | 9 |
"Epoch 2's three pins are untouched" is deleted. The 877 evidence-free records — epoch 2's 868, epoch 4's 3, six epoch-5 — are still byte-identical below the version field, which is why the equivalence result is unaffected, but that distinction does not reduce the inventory, which turns only on whether an address moved. I've said that explicitly rather than leaving both facts in the text to be reconciled.
Your second point is the one that mattered, and it lands harder than the arithmetic. Six of the nine moved pins belong to epochs 2 and 5, both collection = false. unindexed() iterates newest_epochs() (validate-performance-stall.py:135), so all six sit outside the only gate that would report a wrong baseline. The gate extension has moved from a recommendation beside the amendment to a stated condition of accepting Question 2, in both the ADR and the note. The retired-epochs-only variant carries the same correction.
On the alternative resolution — evidence-free records keep v1 — I recorded it and declined it, for two reasons now in the text. It leaves 877 v1 records at the tip permanently, so Q1a.3's condition for dropping v1 support is unreachable by the very compaction that is supposed to enable it; and it makes schema_version describe the record's payload rather than the encoder that wrote it, which is the one thing the field is supposed to mean. If you'd rather have the smaller inventory at that price, it's a one-paragraph change and I'll make it.
Statuses unchanged: both amendments still proposal.
| are invariant across the whole run, each workload observation gains a | ||
| workload-level `boundary` block for the parts invariant across its processes, | ||
| and each sample carries `boundary_processes`: one digest per process, taken |
There was a problem hiding this comment.
The digest composition is right for every epoch that exists today, and it quietly stops working for one this ADR already commits to.
check_boundary_evidence enforces two cross-process guarantees, and only one is unconditional:
- output identity (
crates/rue-perf-schema/src/validate.rs:569-580) — every process'srunner.output_sha256must agree, for all protocol-2 records; - work identity (
validate.rs:588-600) — every process'scompiler_workmust agree, but onlyif policy.worker_setting == WorkerSetting::One. The comment there says why: "Parallel rows deliberately include schedule-dependent joins, reuses, and validation paths; those are distribution evidence, not output identity."
Every boundary epoch in performance/manifest.toml today is worker_setting = "one", which is why the measurement finds exactly one distinct compiler_work per workload. Folding compiler_work into both the shared workload witness and the per-process digest turns that observed fact into a structural assumption. On an epoch whose boundary policy is two/four/eight/automatic, compiler_work varies across processes by design, so no workload-level witness can hold it, every per-process digest differs from the witness and from every other process, and a reader can no longer re-derive the guarantee that is not gated — output identity — because a digest mismatch stops distinguishing "different binary" from "different schedule".
That is not hypothetical for this ADR in particular: accepted decision 2 names the 150 ms automatic-worker target, Decision 7 requires the report across WorkerSetting::REFERENCE_MATRIX, and validate_run already carries the branch for the parallel case.
The fix looks cheap: split the per-process entry into two digests, one over {runner, compiler} and one over compiler_work, so the invariant part stays shared and the schedule-dependent part stays per-process and comparable. That is 64 more hex characters per process against a 3.6% target, and it keeps output identity re-derivable under any worker setting. If you would rather not, then the "future boundary variant … inherits the same shape and the same per-process digest rule" bullet should instead scope the shape to worker_setting = "one" and say a parallel boundary epoch needs its own encoding ruling.
Generated by Claude Code
There was a problem hiding this comment.
Agreed, and split. Fixed in 8b9a937.
Each process now carries two digests — one over {runner, compiler}, one over compiler_work — so the invariant half stays shared and re-derivable and the schedule-dependent half stays per-process and comparable.
I checked the asymmetry you cite and it is exactly as described: output identity is enforced for every protocol-2 record (validate.rs:569-580), work identity only if policy.worker_setting == WorkerSetting::One (validate.rs:588-600), with the comment giving the reason. And all eight worker_setting values in performance/manifest.toml are "one", which is precisely why the measurement found one distinct compiler_work per workload — an observed fact, not a structural one. The ADR text now says that in those terms, and names Decision 2's automatic-worker target and Decision 7's REFERENCE_MATRIX as the commitments that break it.
One correction to your cost estimate, in the unhelpful direction. It isn't free. The note already measures the quantity: S1 keeps one digest per process and S3 replaces it with a process count, so S1 − S3 = 6.7 MiB is the cost of one digest across 105,489 process entries — 66.6 bytes each, a 64-hex string plus quotes and separator. The second digest costs the same again:
| Branch total | e6 x86-64 | e6 macOS | Growth/day | |
|---|---|---|---|---|
| S4, one digest (measured) | 52.6 MiB (3.6%) | 203.8 KiB | 350.2 KiB | 11.4 MiB |
| S4, split (derived) | 59.3 MiB (4.0%) | 210.2 KiB | 412.1 KiB | 12.5 MiB |
So 0.4 points, not nothing — and the macOS record moves most because it carries 951 processes against x86-64's 99. It also moves the compaction's tip from 54.3 to 61.0 MiB, which I've propagated into ADR-0067 rather than leaving the two documents disagreeing.
I still think it's clearly worth it — 4.0% is a 25× reduction and it buys an encoding that survives an epoch this ADR already commits to — but it deserved a number rather than "cheap", so I derived one. The derived rows are labelled as arithmetic on a measured per-digest cost, not a fresh serialization; re-measuring them belongs to the implementing change alongside the derive-level A/B.
The "future boundary variant" bullet now also scopes the parallel case explicitly instead of implying the one-worker shape generalizes.
| | Option | Checkout after | Loses | | ||
| | --- | ---: | --- | | ||
| | do nothing | 1,482.9 MiB, +289/day | nothing; the trend continues | | ||
| | new encoding for new records only | 1,482.9 MiB, +11/day | nothing | | ||
| | **re-encode at the tip, no history rewrite** | **54.3 MiB** | per-process `critical_path` from the tip; still in history | |
There was a problem hiding this comment.
Minor, and not an objection: this column is checkout-after, and the append grows the thing it is being traded against. The note measures each candidate state as a standalone single commit (5.0 MiB of pack for the S4 tip), but the recommended operation lands that commit on top of the existing 402, so the branch's fetch goes from 53.69 MiB to roughly 59 MiB and stays there.
Still an easy trade at that size. But the whole argument turns on "the store's cost is a checkout cost, not a storage cost", so the one number where the storage side moves the wrong way should be stated rather than left to be inferred from the note's tables.
Generated by Claude Code
There was a problem hiding this comment.
Fair, and stated. Fixed in 8b9a937.
The column now says what it is — working-tree cost, which is what RUE-1543 is about — and the paragraph beneath the table gives the number that moves the other way: each state was measured as a standalone single commit (5.0 MiB of pack for the S4 tip), but the append lands that commit on top of the existing 402, so the branch's fetch goes from 53.69 MiB to roughly 59 MiB and stays there. The supporting note carries the same correction next to the option-2 discussion, since that's where the pack table invites the inference.
Two adjustments to your figure, both from the digest split in the other thread. The extra digest adds 6.7 MiB uncompressed, so the fetch lands under 66 MiB rather than at 59, and the checkout reduction is 24× rather than 27×. I've given the fetch as a range (10–23%) rather than a point, because I measured the pack for the one-digest encoding and not for the split, and 6.7 MiB of hex compresses somewhere between half and not at all. Flagged as not separately measured rather than presented as precise.
Still an easy trade at that size, but you're right that it's the one number where the storage side moves against the argument, so it shouldn't have been left to the note's tables.
…ore size performance-data-v1 is 1,481.2 MiB across 1,188 records and grows ~289 MiB a day. Boundary evidence is 98.6-99.3% of a record, and within one workload of one run every part of every evidence entry is byte-identical across all processes except critical_path — a macOS startup observation stores 792 identical copies of one 5.4 KiB structure. Nothing that consumes the branch reads any of it but one validation function. Two amendments, proposed and not accepted, because the rulings separate. ADR-0071 Amendment 1 rules on what the evidence must contain: retain one complete witness per workload observation plus one SHA-256 per process, which is 52.6 MiB of branch and 11.4 MiB a day instead of 1,481.2 and 288.6. The cross-process guarantees survive exactly — a reader re-derives them from the digests — and the amendment states plainly what does not: per-process critical_path histograms for processes 2..N. ADR-0067 Amendment 1 rules on the store: which versioning axis owns an encoding change, and whether published records may be re-encoded. It recommends RUN_SCHEMA_VERSION rather than a suite revision, on the evidence that with the evidence keys stripped from both sides 0 of 1,188 records differ, so no median, ratio, index, ratchet, or flag can move. And it recommends re-encoding once as an ordinary append rather than rewriting history: the branch fetches in 53.69 MiB and expands to 1,482.9 on disk, so a rewrite reclaims at most ~49 MiB of pack — and only after GitHub's own maintenance — while changing the tip reclaims 1,428.6 MiB of checkout and breaks no clone. Force-push was taken seriously and declined on measurement. The supporting note carries the numbers, the breakage inventory (311 of 1,188 addresses move; 6 baseline pins and 1 reference_run), the threat model behind content addressing, and what could not be verified. Both amendments need a maintainer ruling. Neither ADR's status changes here.
Review found the amendment's independence claim false. RUN_SCHEMA_VERSION is not a decoding axis: validate_run compares it against the single current constant and returns UnsupportedSchemaVersion without evaluating anything else, and lib.rs states the intent — "there is no compatibility path, by design". Bumping the constant does not add a version, it replaces the only one readers accept, so v2 records written while the 1,188 v1 records remain would see the whole corpus rejected. That failure is silent, which makes it worse than a broken build. Every record lands in `rejected`, derive publishes no platform, and the staleness gate reads the empty platforms list as "no plotted points yet; nothing to stall" and exits 0 — the gate built to notice a stopped series cannot see a totally rejected corpus. Adds Question 1a: readers implement every schema version that can still be in the store, encoding shape dispatches on schema_version while what must be proven keeps dispatching on the suite's protocol_version, and v1 support may be dropped only once no consumer can reach a v1 record — permanently, if compaction is declined. Both prose invariants are amended by name. Corrects the relationship section. The dual-version reader is a prerequisite for both amendments and must land first; after it, the two are independent in outcome and ordered in execution, with both orders spelled out. Records the alternative review raised — an atomic reader-and-corpus cutover — and says plainly it is not implementable across a repository merge and a data-branch push rather than merely less attractive. ADR-0071 Amendment 1 gains the matching note that it cannot be adopted alone, and the supporting note's independence section is corrected with the measured consequence. Neither ADR's status changes.
Three corrections from review, all in the evidence rather than the
recommendations.
The compaction numbers were measured against a prototype that left
`schema_version` at 1, which Question 1a does not permit. `schema_version`
is an ordinary field of `RunObject` with no `skip_serializing_if`, so it is
part of the canonical form `content_address` digests: a record whose only
change is 1 -> 2 still gets a new name. Re-encoded records therefore declare
2 and every address moves, not 311 of them. All nine `[epoch.baseline] run`
pins are re-pinned rather than six, and "Epoch 2's three pins are untouched"
is deleted because it is false. Six of the nine belong to retired epochs 2
and 5, which `unindexed()` does not reach because it iterates
`newest_epochs()`, so extending that gate becomes a condition of accepting
Question 2 rather than a recommendation beside it. The retired-epochs-only
variant inherits the correction: 1,119 addresses, not 242.
The alternative resolution — evidence-free records keep v1 — is declined and
said so: it would leave 877 v1 records at the tip permanently, making
Q1a.3's exit from dual-reader maintenance unreachable, and would make
`schema_version` describe the payload rather than the encoder that wrote it.
The per-process digest is split in two, over `{runner, compiler}` and over
`compiler_work`. Digesting them as a unit is correct only because every
boundary epoch is `worker_setting = "one"` today; `check_boundary_evidence`
gates work identity on exactly that and leaves output identity ungated, so
under the automatic-worker regime Decision 2 and Decision 7 already require,
a combined digest would differ per process and stop distinguishing a
different binary from a different schedule. The second digest costs 6.7 MiB
across the branch, measured as the S1-minus-S3 delta, taking S4 from 3.6% to
4.0% and the tip from 54.3 to 61.0 MiB. Derived rows are labelled as such;
the implementing change owes the re-measurement.
The append's effect on fetch is stated rather than left to inference: the
checkout-after column measures each candidate as a standalone commit, but
the recommended commit lands on top of the existing 402, so the branch's
fetch rises from 53.69 MiB and stays there.
Statuses unchanged: both amendments remain `proposal`.
5e9b172 to
8b9a937
Compare
|
Rebased onto current trunk (122 commits; the only conflict was a one-line Summary of what changed in the evidence — none of it changes what is recommended:
The consequential one is (1)'s second half: six of the nine moved pins are on retired epochs 2 and 5, which Both amendments remain |
steveklabnik
left a comment
There was a problem hiding this comment.
Adversarial review: I found four follow-ups that should be resolved before these amendments are treated as an implementation-ready ruling. The main concern is that the retired-epoch baseline safeguard is not wired through the current staleness-input selection path.
| `validate-performance-stall.py`'s `unindexed()` gate catches exactly this — | ||
| but iterates `newest_epochs()`, so epochs 2 and 5 are unguarded. Six of the | ||
| nine pins that must move belong to those two retired epochs, so this is where | ||
| the compaction's most likely mistake lands. **Extending that gate to every |
There was a problem hiding this comment.
This acceptance condition is not implementable by changing unindexed() alone. The current CI path runs rue-bench staleness-inputs, and crates/rue-bench/src/staleness_inputs.rs deliberately selects only the epoch containing each platform's newest point before derive; retired epoch 2/5 records are never present in the gate's $DATA. Please include the required input-selection/workflow change here (for example, select every epoch with a declared baseline, or add a separate manifest/index-level baseline-resolution check) and test it. Otherwise a wrong retired baseline remains silent after compaction.
There was a problem hiding this comment.
You're right, and this was the important one: the condition I wrote could not have been met by doing what it said. Fixed in f98daf7 (gate) and ad49563 (text).
I confirmed the mechanism. select() in staleness_inputs.rs keeps only entries whose epoch equals the epoch of that platform's newest index entry, so a retired epoch's records are never checked out into $DATA and derive never sees them. Editing unindexed() cannot inspect data that was not materialized — the rule was never the binding constraint.
I did not take the "select every epoch with a declared baseline" option, because it costs almost exactly what RUE-1542 just bought. Against the live index: epoch 2 is 868 records, epoch 5 is 248, epoch 6 is 321, epoch 4 is 3 — 1,440 total. Epochs 2, 5 and 6 all declare baselines, so that variant reads 1,437 of 1,440 records, against 321 today. That is the whole regression, reintroduced to answer a question that needs no run objects.
So I took your second suggestion. Baseline resolution is decidable from index.json alone — it carries every record's platform, epoch and address, and the step has already checked it out to decide what to read. New subcommand, run in the staleness job ahead of derive:
$ rue-bench check-baselines --manifest performance/manifest.toml --index "$DATA/index.json"
9 declared baseline(s) resolve
And against a simulated compaction that moves addresses without re-pinning:
x86_64-linux epoch 2: baseline 4ae3eb4c… does not resolve — no record in index.json carries this address
aarch64-linux epoch 2: baseline b0bbb8dd… does not resolve — no record in index.json carries this address
2 of 9 declared baseline(s) do not resolve; the epoch publishes no index and no workload ratios
Exit 3, and note those are epoch 2 — the retired epoch that is precisely what unindexed() cannot reach.
Two details worth flagging:
- The check is stricter than "the address exists somewhere in the store". A baseline must name a record of its own epoch and platform, because that is what
derive.rs:1316requires — an address that resolves under a different epoch resolves to nothing where it is looked up, and would otherwise pass an existence test while still publishing no index. Covered by two of the six tests. - It reuses
staleness_inputs' index types rather than parsingindex.jsona second way, so the two readers cannot drift.
Six unit tests, validate-ci-gate.py extended so the job cannot silently lose the step, clippy and fmt clean. It is a separate commit from the amendment text, so if you'd rather this landed on its own or later, dropping f98daf7 leaves the docs coherent — the text would just go back to describing the check as required rather than present.
| binary rather than the schedule. Under the split, only | ||
| `boundary_work_processes` is permitted to vary, and it stays comparable. | ||
|
|
||
| Correspondingly, the workload-level `compiler_work` witness above is the shared |
There was a problem hiding this comment.
The parallel case has no witness for this digest. Lines above say the reader compares every work digest to a corresponding stored witness, but this paragraph explicitly says a parallel epoch stores per-process work digests without a workload-level compiler_work witness. With only opaque hashes and no raw compiler_work, a reader cannot validate them; it can only observe that they differ, which is expected. Since G8 is intentionally not required for parallel rows, either drop boundary_work_processes there and revise the size projections, or define a concrete consumer/artifact and preimage that makes the hashes useful.
There was a problem hiding this comment.
Agreed — and this is my own objection turned around on me, which is what makes it right. Dropped in ad49563.
The note argues against encoding A that "a digest of discarded bytes turns every re-derivable guarantee into a producer assertion." Per-process work digests on a parallel epoch are exactly that: no stored preimage, no requirement that they agree, and check_boundary_evidence gating work identity on worker_setting == One means differing values are the expected outcome. A reader can observe that they differ and learn nothing.
So boundary_work_processes is now present only at worker_setting = "one", and the encoding is stated per worker setting instead of uniformly:
worker_setting = "one" |
parallel | |
|---|---|---|
workload-level compiler_work |
witness; every process must equal it | one representative sample, from a named process |
boundary_processes ({runner, compiler}) |
present; all equal the witness | present; all equal the witness |
boundary_work_processes |
present; all equal the witness | absent |
full per-process compiler_work |
workflow artifact | workflow artifact |
boundary_processes stays unconditional, which is the whole point of having split the digest: output identity is enforced for every protocol-2 record, so it remains re-derivable under any worker setting. The parallel epoch keeps one representative compiler_work per workload — same selection rule and same _source field as S4's critical_path, so there's one convention for "a sample, not a witness" rather than two — and the full per-process work goes to the artifact the amendment already recommends retaining.
On revising the size projections: they don't move. Every boundary epoch in performance/manifest.toml is worker_setting = "one", so every record in the store today carries both digests and the measured 6.7 MiB stands. A future parallel epoch is strictly cheaper than these projections, not dearer — so the numbers you'd be ruling against are still the conservative ones.
| Concretely, a run object gains a run-level `boundary` block for the parts that | ||
| are invariant across the whole run, each workload observation gains a | ||
| workload-level `boundary` block for the parts invariant across its processes, | ||
| and each sample carries **two digests per process**: one over that process's |
There was a problem hiding this comment.
The central verification contract is underspecified. Current runner/compiler evidence contains fields that are workload-specific (outputs and accepted_inputs) as well as fields you propose hoisting to run/workload blocks, so “digest over {runner, compiler}” does not define the canonical preimage after normalization. Please specify the exact field set, ordering/canonicalization, omission/default rules, and domain separation (including whether schema_version participates), then add test vectors so a reader can independently recompute the digest rather than merely trust the producer.
There was a problem hiding this comment.
Fair — "digest over {runner, compiler}" named a preimage without defining one. Specified in ad49563, under four rules.
1. The preimage is the complete original per-process pair, not the residue after hoisting. This is the part that resolves your objection: hoisting is a lossless partition, every field landing in exactly one of the run-level or workload-level block, so a reader reassembles the whole {runner, compiler} pair from those blocks and digests that. The digest therefore commits to the evidence as measured, and does not depend on where a field was hoisted — only on the partition being complete and disjoint. The implementing change must assert that round trip (reassemble, compare to the original, require equality), because a field landing in neither block or in both is otherwise invisible.
While specifying it I hit the ambiguity you were pointing at: the note's sketch said "output identity" as though it were two fields. It is four — runner.output_sha256, runner.output_size_bytes, compiler.emitted_output_sha256, compiler.emitted_output_size_bytes — all workload-level, with accepted_inputs. Written out now.
2. Canonicalization is canonical_json (canonical.rs:51), unchanged — keys sorted by Unicode scalar sequence, no insignificant whitespace, floats an error. Deliberately the same function content_address uses; a second canonicalization path would eventually disagree with the first and the digests would certify something other than what naming enforces.
3. No omission or default rules, and none may be added. Both types are deny_unknown_fields with no skip_serializing_if and no serde(default) on any field, so the preimage cannot depend on what a writer chose to emit. Adding skip_serializing_if to either later would silently change the preimage for already-published records — the accident Stored exists to prevent — so the amendment forbids it for these two types by name.
4. Domain separation, and schema_version does not participate. SHA-256(tag || canonical_json(value)), tag rue.boundary.identity.1\n for {runner, compiler} and rue.boundary.work.1\n for compiler_work. The trailing digit versions the digest scheme. schema_version is excluded on purpose: the process evidence is a property of the process, not of the record encoding carrying it, and folding it in would change every digest on a re-encode that changed nothing about what was measured. If the preimage ever changes, the tag increments.
Test vector, in the note and checkable with sha256sum. One RunnerBoundaryEvidence, canonical JSON at 464 bytes:
{"clock_boundary":"monotonic_pre_spawn_through_exit_and_output_verification","compiler_binary_sha256":"aaaa…","daemon_endpoint_supplied":false,"fresh_output_directory":true,"fresh_state_directory":true,"native_output_verified":true,"output_sha256":"bbbb…","output_size_bytes":16384,"retained_session_handle_supplied":false,"successful_exit":true}
| digest | |
|---|---|
rue.boundary.identity.1\n |
2a095434f674a8c7d4096f6c69d45273c1f811a8ac05bd590f82649170a8501e |
rue.boundary.work.1\n |
b8b977f34591df2d3d8700988c6f0dd0a0a7da1a65e9ac5818bcce75af821155 |
| no tag | 354de1ad26a990020fb8548f8e29a8b3e5618562fa91e9b256224beb68433675 |
Three names for one value, which is the property the tag buys — and it makes the domain separation checkable rather than asserted. The note gives the full byte string and the printf | sha256sum line. The vector fixes the mechanism (tag, canonicalization, hex); the real preimage is the two-key {"compiler": …, "runner": …} object over the reassembled pair, which is too large to inline usefully.
| Split, the invariant half stays shared and re-derivable under any worker | ||
| setting, and the schedule-dependent half stays per-process and comparable. | ||
|
|
||
| Keep one `critical_path` per workload observation (encoding **S4** in the |
There was a problem hiding this comment.
S4 needs a selection rule. critical_path is the one evidence member that varies across processes, yet the proposal keeps only one per workload and calls it the per-commit record. Which process supplies it—first, median, representative, or something else—and how is that choice tied to the retained witness? Without this, two valid encoders can publish different “per-commit critical paths,” so the auditing/visualization semantics are not defined.
There was a problem hiding this comment.
Right — "one per workload observation" was ambiguous for exactly the member where ambiguity costs something. Specified in ad49563.
The rule is the first process of the first sample, with the provenance recorded rather than left to convention:
workloads[i].boundary.critical_path
workloads[i].boundary.critical_path_source = { sample_index: 0, process_index: 0 }
Three reasons it is that rather than an arbitrary pick:
- It is already the project's convention. Every existing consumer takes
boundary_evidence.first()— six places inscaling.rs(lines 136, 377, 536, 581, 611, 650, 686, 716). Choosing anything else would mean the stored value disagreed with the value every report already shows. - The ordering is deterministic. I checked, because
.first()is only meaningful if it is:measure_sampleruns the batch in a serialforloop and pushes each process's evidence in spawn order (measure.rs:139-164), so index 0 names the same process on every platform and every rerun. critical_path_sourcemakes the choice auditable. Your concern was two conforming encoders publishing different values with nothing able to tell; recording the source means a reader can see which process it holds, and a mismatch against the rule is detectable. It also stops a representative sample from being misread as a witness, which is the more likely downstream error.
I considered a median and rejected it: CompilerCriticalPathEvidence is a struct of histograms and counters rather than a scalar, so a median needs a per-field rule plus a tie-break, and it would publish a value no process actually observed — worse for auditing than a named real one.
The same rule and the same _source field now carry the representative compiler_work that a parallel epoch retains (per the other thread), so the encoding has one convention for "a sample, not a witness" instead of two.
…uded A wrong `[epoch.baseline] run` fails silently: `derive` resolves the baseline by address among its epoch's own records and, on a miss, publishes no index and no workload ratios while still plotting every per-workload series. `unindexed()` reports that, but iterates `newest_epochs()`, so a retired epoch is unguarded. That gap is not closable inside `unindexed()`. `staleness-inputs` selects the epoch holding each platform's newest point and nothing else (RUE-1542), so a retired epoch's records are never materialized into the data root `derive` reads — the rule cannot inspect data that was not checked out. Restoring them is the wrong repair: selecting every epoch declaring a baseline means reading 1,437 of the store's 1,440 records against the 321 read now, which is the cost RUE-1542 removed. Baseline resolution needs no derived data. `index.json` carries every record's platform, epoch and address, and the staleness job has already checked it out to decide what to read, so the question is asked there instead: every declared baseline must name a record of its own epoch and platform. That covers live and retired epochs alike for one pass over a file the step already holds. The check is deliberately stricter than existence. An address resolving under a different epoch resolves to nothing where `derive` looks it up, so it would pass a whole-store existence test while still publishing no index. Against the store on 2026-08-18: nine declared baselines, nine resolving.
Four follow-ups from adversarial review, all making underspecified parts of
the proposal decidable.
The acceptance condition for Question 2 was not implementable as written.
"Extend `unindexed()` to every epoch declaring a baseline" cannot work,
because `staleness-inputs` keeps retired epochs out of the derived data the
gate reads. The condition is now a manifest-against-index resolution check
covering every epoch, which the preceding commit implements and wires into
the staleness job.
The per-process digest preimage is specified rather than named. It is the
complete original `{runner, compiler}` pair, reassembled from the run-level
and workload-level blocks — hoisting is a lossless partition, and the
implementing change must assert that round trip, because a field landing in
neither block or in both is otherwise invisible. Canonicalization is
`canonical_json`, unchanged, so there is no second canonicalization path to
disagree with naming. Omission rules are forbidden by name for the two
evidence types. Each digest is domain-separated by a tag whose suffix versions
the digest scheme, and `schema_version` deliberately does not participate: the
evidence is a property of the process, not of the encoding carrying it. The
supporting note carries a byte-exact vector checkable with `sha256sum`,
including the same bytes under the other tag and under none.
The parallel case loses its work digest. Storing per-process `compiler_work`
digests on an epoch where the values are expected to differ, with no witness
and no agreement requirement, certifies nothing — the objection this note
already raises against encoding A. `boundary_work_processes` is therefore
present only at `worker_setting = "one"`; `boundary_processes` stays
unconditional, which is what keeps ungated output identity re-derivable. A
parallel epoch retains one representative `compiler_work` instead. Size
projections are unchanged, since every epoch in the manifest is one-worker.
S4 gets a selection rule. `critical_path` is the member that varies across
processes, so "one per workload observation" was ambiguous: the first process
of the first sample, with `critical_path_source` recorded so a reader can tell
a representative sample from a witness. That ordering is deterministic —
`measure_sample` runs a batch serially in spawn order — and it is already the
convention every consumer uses.
Statuses unchanged: both amendments remain `proposal`.
|
All four addressed; replies inline. Two commits, deliberately separable:
Two of your four made the proposal smaller rather than larger, which is worth saying plainly. The parallel work digest was my own argument against encoding A pointed back at me — a digest of bytes you don't store, that is expected to differ, certifies nothing — so it is gone rather than justified. And the Size projections are unchanged by any of this: every boundary epoch in the manifest is If you'd rather the gate landed separately, dropping f98daf7 leaves the docs coherent — the text reverts to describing the check as required rather than present. Both amendments remain |
For review and a ruling by Steve and Dorian. Nothing here is accepted or implemented — both amendments carry
Status: proposal, and neither ADR's front matter changes.Problem statement: RUE-1543. This PR proposes what to do about it.
What the measurements found
performance-data-v1is 1,481.2 MiB across 1,188 records, growing ~289 MiB/day since protocol-v2 boundary evidence began collecting on 08-13. Evidence is 98.6–99.3% of a record.The finding that drives the recommendation: within one workload of one run, every part of every evidence entry is byte-identical across all processes except
critical_path. A macOSstartupobservation stores 792 identical copies of one 5.4 KiB structure. And the only consumer of stored evidence ischeck_boundary_evidence— the heavy semantic checks all run in the producer before storage.The second finding reframes the history question: the store's cost is a checkout cost, not a storage cost. The whole branch fetches in 53.69 MiB and expands to 1,482.9 MiB on disk (~28×). GitHub reports the entire repository at 115.8 MiB, and
git repack --window=250 --depth=100reclaims nothing.Two amendments, because the rulings separate
ADR-0071 Amendment 1 — what the evidence must contain. Retain one complete witness per workload observation plus a pair of SHA-256 digests per process — one over
{runner, compiler}, one overcompiler_work.Cross-process guarantees survive exactly — a reader re-derives "all N processes produced the same output" from N digests against the witness. What is dropped, stated plainly in the text: per-process
critical_pathhistograms for processes 2..N.ADR-0067 Amendment 1 — the store. Two questions:
RUN_SCHEMA_VERSION, not a suite revision, on the evidence that with the evidence keys stripped from both sides 0 of 1,188 records differ — so no median, ratio, index, ratchet, or flag can move. Read literally, §3 would force an epoch turn on three platforms and make compaction impossible.Rewriting history was taken seriously and declined on measurement, per the request: a force-push reclaims at most ~49 MiB of pack, only after GitHub's own maintenance (not available on request), leaves unreachable objects fetchable by SHA, and destroys the evidence permanently. The append breaks no clone — which matters, since the repo is public with 42 forks and "nobody else has this data" is not verifiable.
The amendment also states what content addressing actually buys us today, since the ADR justifies it as verifying a record without trusting its writer: there is no untrusted writer in the real path. The properties in use are idempotent republication, accident refusal, protection against a silent additive-schema rename, and a governance property — and a reviewed one-time re-encode spends only the last, in the open.
Two things to weigh in review
derivepublishes no index while still plotting every series, andunindexed()iterates live epochs only. Because re-encoded records declareschema_version = 2, all 1,188 addresses move and all nine baseline pins are re-pinned — six of them on retired epochs 2 and 5, squarely in that blind spot. Extending the gate to every epoch declaring a baseline is a stated condition of acceptance.Not verified, and said so in the text
The re-encoded corpus cannot pass
rue-bench derivetoday —deny_unknown_fieldsplus thelen == batch_sizerule reject it by construction — so equivalence was checked structurally. A derive-level A/B is acceptance evidence for the implementing change, not for this proposal.The split digest's cost is derived from a measured per-digest figure (S1 − S3 = 6.7 MiB over 105,489 process entries) rather than freshly serialized, and the pack/fetch effect of those extra bytes was not separately measured — given as a bound, not a point. Both re-measurements belong to the implementing change.
Supporting note:
docs/notes/performance-boundary-evidence-size.md(measurements, breakage inventory, full option space).