chore(experiments): make RESULTS.md a baseline instead of a test output - #197
Merged
Merged
Conversation
E2 overwrote the committed experiments/RESULTS.md on every run, so any
`pnpm -r test` left the working tree dirty with machine-local wall-clock
timings. The file was simultaneously a checked-in artifact and a test output,
which meant the recorded result silently became "whatever the last person to
run the suite measured".
Splits the two roles:
- each run writes its own table to the gitignored experiments/.results/
(override with SH_E2_RESULTS_DIR);
- the committed RESULTS.md becomes a baseline that E2 asserts against, so a
change moving the read counts has to be acknowledged rather than quietly
rewriting the record. Refresh deliberately with SH_E2_UPDATE_BASELINE=1.
Only the columns that actually reproduce are compared. Measured, rather than
assumed, by running E2 twice locally and diffing against the CI run captured
earlier today:
- entries + ratio: identical on both (deterministic synthetic fixtures)
- backendBytes: 7482/28508/140908/706909 locally vs 7486/28512/140912/
706913 on CI — a consistent +4, so serialization is
environment-sensitive and cannot be asserted
- backendMs / checkpointMs: vary run to run even on one machine
Hence deterministicView() covers n, both entries columns, and the ratio at the
1 decimal the table carries. Adds parseE2Table() to read the baseline back,
with unit tests for the round-trip, the drift-insensitivity, and that a real
entries change is still caught. Verified end to end: three consecutive E2 runs
leave RESULTS.md untouched, and perturbing a baseline entries cell fails E2.
The baseline refresh in this commit moves only the ms columns — every asserted
column is unchanged from what was already committed.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
experiments/RESULTS.mdwas both a checked-in artifact and a test output: E2 rewrote it on every run, so anypnpm -r testleft the working tree dirty with machine-local wall-clock timings, and the recorded result silently became "whatever the last person to run the suite measured".This splits the two roles:
experiments/.results/(SH_E2_RESULTS_DIR=<dir>to redirect).RESULTS.mdbecomes a baseline E2 asserts against, so a change that moves the read counts has to be acknowledged rather than quietly rewriting the record. Refresh deliberately withSH_E2_UPDATE_BASELINE=1 pnpm -C experiments test e2-reconstruction-cost.Which columns a baseline can actually assert
Measured rather than assumed — E2 run twice locally, diffed against the CI run captured earlier today:
backendBytescheckpointBytesbackendMs/checkpointMsSo
deterministicView()coversn, both entries columns, and the ratio at the 1 decimal the table carries. Had I compared bytes, this would have gone green locally and failed on CI.Changes
experiments/src/report.ts— addsparseE2Table()(reads the baseline back, tolerating surrounding prose and unrelated tables) anddeterministicView(), with the measurement rationale documented at the type.experiments/test/e2-reconstruction-cost.test.ts— writes to the gitignored dir; compares against the baseline unlessSH_E2_UPDATE_BASELINE=1.experiments/test/report.test.ts— new, 8 cases: round-trip, prose/other-table tolerance, throwing rather than silently returning nothing on a malformed table, drift-insensitivity, that a real entries change is caught, and that the committed baseline itself parses and round-trips (catching a hand-edit that breaks the shape E2 reads)..gitignore,experiments/README.md— the ignored path and the refresh workflow.Verification
RESULTS.mduntouched (git diff --quietclean) while each refreshes the gitignored copy — the actual point of the change.6→7) fails E2 with a clear diff; restoring it passes and leaves the file byte-identical.pnpm -r test: experiments 51 → 59 passed, 0 failures repo-wide.make typecheckclean.Note on style
experiments/test/report.test.tsuses double quotes to match the other 12 files in the package..prettierrcdeclaressingleQuote: true, but nothing inexperiments/follows it and prettier is gated neither in CI nor (working) pre-commit — so I matched the surrounding code rather than leave one odd file out. Worth a separate repo-wide formatting pass if you want the config actually honoured; I didn't fold that into this change.