Skip to content

fix: publish the benchmark snapshot again and receive already-present packs as git does - #295

Merged
scolladon merged 29 commits into
mainfrom
fix/bench-snapshot-summary-adr-lint
Sep 5, 2026
Merged

fix: publish the benchmark snapshot again and receive already-present packs as git does#295
scolladon merged 29 commits into
mainfrom
fix/bench-snapshot-summary-adr-lint

Conversation

@scolladon

@scolladon scolladon commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Background

main has been red on benchmark-snapshot since the 2026-08-29 fixture fix landed, at the very last step: benchmark-action/github-action-benchmark rejects reports/benchmarks/snapshot.json because 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 its throws option 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 bench exited 0 every time, and only the publish action could notice.

Two smaller things were true alongside it. The bench summary renderer required both a tsgit and an isomorphic-git entry 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]
  end
Loading

A 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.

  1. The bench measures. test/bench/fetch-pack.bench.ts builds 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.
  2. Silence is no longer green. test/bench/support/bench-dsl.ts passes throws: true on every bench it registers, so a warmup throw fails the file; tooling/test/integration/bench-warmup-throw.test.ts spawns 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 carry timeout-minutes.
  3. The converter refuses. tooling/bench-to-snapshot.ts exports assertEveryBenchmarkValued, called on the publish path only, naming every sample-less scenario; the comparison tool keeps its own missing verdict.
  4. The summary renders. tooling/bench-summarize.ts exports 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 and n/a; the missing marker now means exactly one thing.
  5. The receive path matches git. src/application/primitives/fetch-pack.ts settles the quarantine copy (rename, adopt or refuse) and internal/write-pack-artifacts.ts writes 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_MISMATCH joins the error union; the dead FILE_EXISTS tolerance in fetch-missing goes. test/integration/pack-receive-idempotence-interop.test.ts runs git and tsgit through the identical, planted, missing-siblings and corrupt-index cases on one fixture.
  6. Lint housekeeping. Five ADR status lines and anchors take the lint's exact form, and two live citations point at the ADRs that now govern.

Unit tests live in tooling/test/unit/ and test/unit/; test/bench/** and tooling/** 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

  • Decisions: ADR-800 (zero-sample guard in both layers), ADR-801 (bench jobs time-bounded), ADR-802 (converter guard on the publish path only), ADR-803 (fresh memory context per iteration), ADR-804 as amended after review (per-artefact compare-and-complete, refuse on mismatch), ADR-805 to ADR-807 (lint form and the two citations), ADR-808 and ADR-809 (summariser shape and rendering).
  • Design: docs/design/bench-snapshot-summary-adr-lint.md.
  • Divergences from the brief: the brief asserted the FILE_EXISTS refusal 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.
  • Pinned behaviours: identical re-receive keeps inode and mtime; planted pack, same-size one-byte-off pack, superset pack, zero-byte index, directory occupant and unreadable occupant are refused with the artefact named and no quarantine file left; missing siblings and the promisor sentinel are recreated; two concurrent receives both succeed; a pack over 1 MiB is compared in 1 MiB windows; a warmup throw fails a spawned bench run; a sample-less entry is refused by name before anything is published; tsgit-only rows render with their numbers; the lint prints only its OK line.
  • Test plan: npm run validate green 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: one bench.yml dispatch 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: see the collapsed section.
Run record
  • resolve: brief file; the previous run's post-merge verdict was the trigger; the mutation harness and the refactoring pass were waived for this chore by explicit flag (WAIVER: validation; WAIVER: refactoring) — the mutation harness was then run standalone on the src/ diff at the user's ruling.
  • workspace: fix/bench-snapshot-summary-adr-lint from main@18dff9ca; isolate policy: proceed.
  • design and decisions: 10 candidates; the design's git pin overturned the brief's claim that the FILE_EXISTS refusal 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.
  • planning: 7 parts; plan-lint green; 5 plan-level candidates adopted as recommended.
  • implementation: 7 of 7 parts landed first-spawn (the seventh a live dependency bump the registry drifted into); GATE(implementation): validate green (24 scripts).
  • review: four dimensions, three cycles. Cycle 1: 3 HIGH + 9 MEDIUM + 13 LOW; the HIGHs overturned the ratified path-only adoption — real git compares content for pack artefacts and recreates missing siblings — re-ratified by the user as per-artefact compare-and-complete, the fix diff shown before commit. Cycle 2: 9 MEDIUM + 15 LOW (quarantine leak on a throwing compare, unbounded occupant read, unproven windowing, wireit inputs, an audit exclusion of mine that was wrong). Cycle 3: perf and security converged; 4 code/test MEDIUMs applied and verified by gates. Seven fix commits. GATE(review): validate green after each round; two registry drifts and one chunk budget fixed in their own commits; one interop flake classified with evidence; the serial-run remedy found to deadlock two suites that build inside their setup.
  • validation (standalone): 86 mutants, 66 killed, 17 compiler-rejected, 3 survived → two killed by a read spy and a requested-length sum, one marked equivalent with proof; confirmation run: 100 percent, 67 killed, none surviving, 17 compiler-rejected mutants excluded, 4 ignored..
  • documentation: two pages refreshed; spelling and doc links green.
  • propose: npm outdated shows only the six documented exceptions; push and PR creation confirmed by the user.
  • integrate: one bench.yml dispatch on the branch green (sweep under throws, 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/test is held at 1.62.1 with a documented check:deps exception; merged on the user's standing instruction once every blocking check was green.

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated — a new cross-tool interop file against real git, and a spawn test for the bench verdict
  • All tests pass (npm run validate)
  • Coverage at 100% — the coverage gate scopes domain and adapters; the domain error change is covered
  • No surviving mutants — scoped standalone run at 100 percent; the CI mutation job is non-blocking

Checklist

  • Follows conventional commits
  • Faithful to canonical git (or divergence recorded in an ADR) — receive posture pinned against git 2.55.0 in the interop file
  • No any types without justification
  • Kebab-case file names
  • Domain layer has no outward imports
  • Bundle size within budget — primitives chunk budget raised 61 to 62 kB for the compare path, per precedent

@pkg-pr-new

pkg-pr-new Bot commented Sep 5, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@scolladon/tsgit@295

commit: 4d9f235

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Docs drift — informational only

The following commands/primitives changed in this PR without a matching
docs/use/* update. After one cycle of tuning this gate will start blocking;
please consider adding the docs update in the same PR.

  • src/application/commands/fetch-missing.ts → expected docs/use/commands/fetch-missing.md or a row update in docs/use/commands/README.md

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 [skip-docs-gate] PR-label escape hatch.

@scolladon
scolladon merged commit dedd61b into main Sep 5, 2026
42 checks passed
@scolladon
scolladon deleted the fix/bench-snapshot-summary-adr-lint branch September 5, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant