fix: publish the benchmark snapshot again and receive already-present packs as git does - #295
Merged
Merged
Conversation
…the snapshot converter
…alled from main only
…ext per iteration
…ename, as git does
…aming ADR-732 and ADR-733
…t row keeps the missing marker
…use a mismatch, as git does
…ew found unproven
…ristic like its siblings
…n-regular or oversize occupants
… and pin the bounded windows
… mark the loop bound equivalent
commit: |
…OPFS on the CI runner
Docs drift — informational onlyThe following commands/primitives changed in this PR without a matching
Suppressing this is fine if the change is intentionally code-only — type-only refactor, internal-only signature change, etc. The blocking phase will add an explicit |
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.
Background
mainhas been red onbenchmark-snapshotsince the 2026-08-29 fixture fix landed, at the very last step:benchmark-action/github-action-benchmarkrejectsreports/benchmarks/snapshot.jsonbecause one entry has no value. That entry is the fetch-pack scenario added on 2026-09-04. It received the same pack into the same in-memory repository on every iteration, the second receive was refused because the pack's index was already there, and the benchmark runner reported the scenario as a pass with an empty sample list: tinybench stores a warmup error on the task and throws it only when itsthrowsoption is set, which vitest never sets, and the run phase returns early on a stored error before any event fires. The scenario had never measured anything,vitest benchexited 0 every time, and only the publish action could notice.Two smaller things were true alongside it. The bench summary renderer required both a
tsgitand anisomorphic-gitentry per scenario and printed_missing entry_in both cells otherwise, so 81 of the 95 scenarios, the tsgit-only ones, read as if they had not run. And eight findings from the decision-record lint were sitting on older ADRs and two live pages, all format mismatches and stale citations.Fixing the never-measuring bench led somewhere the brief had not: real git accepts a byte-identical pack received twice, silently, and tsgit did not. The receive path renamed its quarantine copy over the existing pack and then refused the index with
FILE_EXISTS. Review then pinned the rest of git's finalize posture: a foreign file planted at a pack's content-addressed name is refused with "differ in contents" and exit 128, a zero-byte index beside a real pack is refused the same way, and a pack whose siblings went missing gets them silently recreated. So the receive path now behaves the same way.Intuition
A benchmark that throws must fail the run, an entry without a measurement must never reach the publish action, and a pack that already sits at its content-addressed name is either this exact pack, in which case nothing is rewritten, or corruption, in which case nothing is adopted.
flowchart LR subgraph receive [receive path] Q[verified quarantine copy] -->|name free| R[rename into place] Q -->|occupant identical| K[keep it, discard the copy] Q -->|occupant differs or is not a file| X[refuse PACK_ARTIFACT_MISMATCH] R --> S[siblings: write if absent, keep if identical, refuse if different] K --> S end subgraph bench [bench harness] W[sut throws in warmup] -->|throws: true| F[run fails, file aborts] E[entry without a value] -->|converter guard| P[publish refused by scenario name] T[tsgit-only scenario] -->|renderer| M[own numbers, em dash peer, n/a speedup] endA toy version of the receive rule: after the index pass verifies the received pack, look at the destination name. If nothing is there, rename the copy in. If a file is there, compare it with the copy one bounded window at a time; identical means the copy is discarded and the existing inode and mtime are untouched, anything else means a typed refusal. Then write each sibling where its name is free, keep an identical occupant, refuse a differing one. That is exactly the sequence real git runs, and the interop test runs both tools through every branch of it on one fixture.
Code
Read it in this order.
test/bench/fetch-pack.bench.tsbuilds the pack once and receives it into a fresh memory context every iteration. Scenario title and bench name are unchanged, so the published series continues.test/bench/support/bench-dsl.tspassesthrows: trueon every bench it registers, so a warmup throw fails the file;tooling/test/integration/bench-warmup-throw.test.tsspawns a real bench run on a throwing fixture and asserts the non-zero exit. Because a run-phase throw hangs the worker, the two bench-running CI jobs now carrytimeout-minutes.tooling/bench-to-snapshot.tsexportsassertEveryBenchmarkValued, called on the publish path only, naming every sample-less scenario; the comparison tool keeps its own missing verdict.tooling/bench-summarize.tsexports its renderers, treats an entry as measured only when it carries a value and a rate, and renders a tsgit-only row with its own numbers, an em dash andn/a; the missing marker now means exactly one thing.src/application/primitives/fetch-pack.tssettles the quarantine copy (rename, adopt or refuse) andinternal/write-pack-artifacts.tswrites each sibling where free, keeps an identical occupant and refuses a differing one, with a stat check before any read so a planted directory, FIFO or oversize file cannot hang or bloat the receive. The quarantine copy is released on every exit.PACK_ARTIFACT_MISMATCHjoins the error union; the deadFILE_EXISTStolerance infetch-missinggoes.test/integration/pack-receive-idempotence-interop.test.tsruns git and tsgit through the identical, planted, missing-siblings and corrupt-index cases on one fixture.Unit tests live in
tooling/test/unit/andtest/unit/;test/bench/**andtooling/**sit outside the coverage and mutation gates, so those tests are the only mechanical guard and were written to isolate each guard clause. The receive path is under the mutation harness: a scoped Stryker run over the changed line ranges scored 100 percent after triage (86 mutants, 67 killed, 17 rejected by the type checker, one loop bound marked equivalent with its proof)..Provenance & verification
docs/design/bench-snapshot-summary-adr-lint.md.FILE_EXISTSrefusal was correct; the design pinned that git tolerates an identical re-receive, and review then pinned that git compares content for pack artefacts, so the path-only adoption the first ruling adopted was replaced by the per-artefact rule, with the user's re-ratification.npm run validategreen at every gate (24 scripts). Two full bench sweeps on this branch: 109 entries, all sampled. Mutation: 86 mutants in scope, 67 killed, 0 surviving after two kill tests and one proven-equivalent marker. Pre-merge proof: onebench.ymldispatch on this branch, the full sweep under the new guard with the new renderer: 109 entries all sampled, the fetch-pack scenario measured, the summary with zero missing entries and 81 em-dash peer cells.Run record
src/diff at the user's ruling.fix/bench-snapshot-summary-adr-lintfrommain@18dff9ca; isolate policy: proceed.FILE_EXISTSrefusal was correct; 3 forks ratified by the user (receive-path fix here, both zero-sample guards plus a job timeout, re-point both citations), 7 adopted as recommended; ADRs 800–809.npm outdatedshows only the six documented exceptions; push and PR creation confirmed by the user.bench.ymldispatch on the branch green (sweep underthrows, renderer, converter); the PR's WebKit e2e job was red twice on Playwright 1.63.0 (its WebKit 26.6 build fails OPFS on the runner; green on 1.62.1 the day before), so@playwright/testis held at 1.62.1 with a documentedcheck:depsexception; merged on the user's standing instruction once every blocking check was green.Test Plan
npm run validate)Checklist
anytypes without justification