Skip to content

Replace the falsification suite with the time-series suite, under bench/ - #4

Merged
bfulton merged 16 commits into
mainfrom
claude/supdb-architecture-review-6mkcis
Sep 6, 2026
Merged

Replace the falsification suite with the time-series suite, under bench/#4
bfulton merged 16 commits into
mainfrom
claude/supdb-architecture-review-6mkcis

Conversation

@bfulton

@bfulton bfulton commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Net result: the old suite — the six experiment binaries, claims.json, results/, figures/, the external comparators, the browser test and the plan files — is gone from this repository, and the rewritten suite lives at bench/ as its own cargo workspace. For your review — not merged.

The history is honest about how it got here. The first commits split the old suite out to supdb-bench and pointed CI at it; the rewrite happened there (supdb-bench#6, reviewed and green); the last commit brings the rewritten suite back, because at ten source files, one binary and a runs/ directory the second repository cost more than it guarded — a paired PR for every change touching both, a submodule pointer and a branch override so CI tested the right engine, and the engine's release profile silently ignored under the other workspace.

What is in bench/

bench run --scale quick|full measures the two floors, then every arm over a ladder of store sizes, and writes one row of raw per-rep samples under bench/runs/<scale>/<utc>-<sha7>.json. bench gate ROW compares a row to the last ten rows of its machine class. bench figures draws every figure from the latest row of each class. bench machine prints the host as a row records it. bench/DESIGN.md is the specification.
workloads over the ladder 1, 3, 10, 30 … × 10⁴ load, load-shuffled, read, scan, ycsb-A … ycsb-F
floors, per machine, no engine wal-floor (framed 1,000-record batches, one fdatasync each), scan-floor (one mmap sequential walk)
arms, durable per batch supdb, supdb-noadvice, lmdb, rocksdb-tuned
arms, buffered supdb-ingest, lmdb-nosync, rocksdb-nosync
quick top rung 300 000 keys, 5 reps — about three minutes; gates every PR
full top rung where the store is 1.5× the machine's memory, 7 reps; on a quiet machine, by hand
gate regressed if the row's CI is entirely worse than every CI in the window; flagged if entirely better; no band below three prior rows
figures one SVG per (workload, quantity, guarantee): curves over size with a CI band, the message as the title, floor and memory rules, direct labels, no legend

What this changes here

  • bench/ is its own workspace, depending on the engine by path. The engine's cargo build and cargo test never pay for the comparators' C++ build and its lockfile never lists them. Cargo takes profiles from the root of the workspace being built, so the engine's release profile is repeated in bench/Cargo.toml with the reason.
  • A row records one sha. One commit now names the engine and the suite that measured it.
  • scripts/check.sh gains bench (the suite's build, test and lint; in the default set) and quick (one measurement, the gate against bench/runs/, the figures; not in the default set because a timing run needs the machine to itself).
  • CI runs bench in the three-platform matrix with a libclang step for bindgen, and quick in a job of its own that uploads the row and figures as artifacts. The job that cloned supdb-bench is gone. The three workflows that ran the suite move here (apple-silicon detect, quiet-bench, runner-smoke) with paths adjusted, and .localmostrc returns to the root.
  • The documents describe one repository. README's benchmark section says what is measured and what the curves show in words, with no embedded figure until a full row is committed and drawn. CLAUDE.md gets a bench/ section and bench/CLAUDE.md carries that side's rules.
  • Claim ids stay in code comments and design notes. They name the previous suite's claims, in supdb-bench's history, and CLAUDE.md says so; scrubbing about two hundred of them is a separate decision.

Verified

Engine: sh scripts/check.sh build test lint, 80 tests, clippy -D warnings clean, formatted. Suite: its own build, test and lint clean, 33 tests; a smoke run writes a row with the single sha, the gate and figures read it back; every workflow run: block parses. The suite's own CI was green on all five checks at supdb-bench's head before the move, including the cold RocksDB build on macOS.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2

@claude
Describe the engine in its own terms, not a consumer's
4c9c509 
The engine's own source, its tests, the browser reader and the design
notes explained themselves by naming one application built on top: a
segment was "a day index", a posting list was "what it writes", a size
budget was "its client". A reader of the store should not have to know
about a downstream program to understand what a run of fixed-width
values is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
@claude
Make the browser suite measure the format, not one application
6fc7e87 
The suite's workload was an HTTP access log: seven named fields, a day of
lines, a posting per line per field. None of that is a property of the
store, and it had spread -- the engine's own source and the browser reader
explained themselves by naming it.

The model is now an inverted index over rows: anonymous attributes on a
cardinality ladder from 8 to 5,000, four-byte row ordinals, Zipf-skewed.
The ladder is the part that matters and is kept exactly: three orders of
magnitude of run length in one file, so a single fixture covers a run of
hundreds of thousands of postings and a run of tens. `w1-daysize` is
`w1-indexsize`, and its two claims are stated about an index rather than
about a day of traffic.

Renaming the attributes moved where they sort, and that exposed four
checks that were testing the fixture's geometry rather than the reader:

- W4.1 required every probe to plan a fetch, but an inline run correctly
  plans nothing. The non-vacuity guard moves to the probe set, and is now
  stronger: each shape must exercise both the block-backed and the inline
  path.
- W5.3 required each range's reads to equal its plan. A plan names a
  16 KiB checksum piece, so a walk over a piece the open already made
  resident reads less. Containment is the property -- never read outside
  the plan -- and tightness is what W5.2 and W5.6 measure. Its evidence
  was also identical whether it passed or failed; it now names the miss.
- W6.5 required exactly one postings wave, so a shape needing none was
  reported as a failure rather than as a path the run did not take.
  Rule 3: it reports not_exercised.
- The browser test's refusal check assumed range 0 was non-resident. It
  now probes every range and requires that one refuses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
@claude
Run the browser suite in CI, and re-record what it measures
61cac3e 
The browser suite's five experiments were not in `scripts/check.sh`, so
nobody ran them and their committed records drifted from the engine. The
fixture whose index they recorded at 686,506 bytes measures 1,112,132
today. `verify` was comparing claims against the old recording and
reporting green.

It was kept out by its own exit code: the binary exited non-zero whenever
a finding failed, so a run containing a known-failing finding looked like
a crashed program. In this project a recorded failure is a valid state and
`verify` is what adjudicates it, so the suite now exits 0 and writes its
record like every other one, and `suites` runs it.

What the fresh records show, all from one cause: `Db` inlines a run up to
`Options::inline_bytes` exactly as `SegmentWriter` does. Around 450 KB of
a 7.4 MB fixture that these claims assumed was in blocks is in the key
section.

- W5.5 fails -> holds. 6.1% against a 10% bound, from 12.7%. The sparse
  open got smaller and the whole open it is measured against got bigger.
- W5.6 holds -> fails. The worst field's plans are 1.10 of their bound,
  from 0.52: a key section carrying values spans more pages, so the
  page-boundary slack is paid over a range that outgrew the bound's
  per-key term. Recorded as failing rather than recalibrated.
- W6.1 holds -> fails. Store and segment both open in two waves, so the
  contrast the claim asserts is gone. W6.2 and W6.4 still hold, so what
  R7.1 was built for is intact.
- W6.5 was pointed at the rarest key of all, which now has no block. It
  takes the smallest run that did not fit inline, and holds: 65 postings,
  one wave, the chunks the run spans rather than the block they share.

CLAUDE.md said `Store` never inlines. Three findings were built on that
difference; the note now says what the code does and why to distrust it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
@claude
Move the falsification suite out to supdb-bench
26459b1 
The engine keeps the library, its tests, and the browser reader with its
wasm build. The suite -- the measurement substrate, the six experiment
binaries, claims.json, results, figures, the external comparators, the
browser test and the plan files -- moves to supdb-bench, which carries
this repository as a submodule and depends on it by path.

web/build.sh now builds the reader and prints its four sizes; recording
them against a budget is a claim and lives with the claims. It also asks
cargo where its target directory is rather than assuming, because as a
submodule the workspace root is a level up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
@claude
Documentation and CI for an engine repository
b1b8833 
The README is a database README: what it is, a benchmark section with the
two figures and directions with claim ids, usage, and one sentence for the
falsification discipline with one link. CLAUDE.md keeps the invariants a
change here must not break and the shapes the bugs come in, and drops
everything about profiles, claims and comparing engines -- that moved with
the suite. CONTRIBUTING.md leads with the fact that the tests runnable here
are not the whole gate.

check.sh is build, test, lint and wasm. ci.yml keeps the platform matrix
and adds a required `bench` job that clones supdb-bench, puts this
checkout where the submodule would be, and runs the suite against it. It
needs no secrets, so it works on fork pull requests, and it runs on pull
requests and tags rather than on pushes to main. The workflows that drove
the suite on the self-hosted runners move to supdb-bench with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI lite review requested due to automatic review settings September 5, 2026 20:24
@bfulton Claude

bfulton commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

bench is red, and it is the ordering dependency rather than a defect in this branch:

error: could not find `Cargo.toml` in `/home/runner/work/supdb/supdb/supdb-bench`

The job clones supdb-bench at main, which is still the placeholder README, so there is no crate to build. It resolved BENCH_REF: main, BENCH_REF_SOURCE: default — correct behaviour, nothing to fix here.

There is nothing to port into this PR either: the fix lives in the other repository. bfulton/supdb-bench#1 carries the suite, and this goes green once that merges. That PR's own CI is already green on browser and committed-results-are-current with the rest running.

Not re-running the check — a re-run would fail identically until bench main changes. I am watching this PR and will confirm once bench has landed.

Generated by Claude Code

Copilot AI reviewed Sep 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Documentation/build-script messaging still references moved or removed suite artifacts (e.g., results/w3-bundle.*.json / src/bin/browser.rs), and web/build.sh needs a small robustness guard for clearer CI failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refactors supdb to be “engine + tests + browser reader” only, moving the falsification/benchmarking suite (plans, results, figures, external comparators, and related scripts) into supdb-bench, and updating docs/comments/scripts accordingly.

Changes:

  • Removes the in-repo suite artifacts (plan markdown, recorded results, figure HTML, AWS runner tooling, and external comparator crate) now owned by supdb-bench.
  • Simplifies the crate layout (drops workspace + bench module) and updates engine/browser-reader documentation to remove suite-specific references.
  • Updates the wasm reader build script to be submodule/workspace-root aware and to emit size measurements as plain text.
File summaries
File Description
ycsb-plan.md Removed (moved to supdb-bench).
web/worker.mjs Updates comments to remove suite-specific naming.
web/test/run.sh Removed (browser test moved to supdb-bench).
web/test/page.html Removed (browser test moved to supdb-bench).
web/supdb.mjs Updates comments/docs to remove suite-specific naming.
web/README.md Updates prose, but still needs alignment with moved-out suite/build outputs.
web/build.sh Uses cargo metadata target dir (workspace/submodule-safe) and prints size metrics.
walreuse-plan.md Removed (moved to supdb-bench).
walfloor-plan.md Removed (moved to supdb-bench).
walcrc-plan.md Removed (moved to supdb-bench).
tests/segwriter.rs Comment-only updates to remove suite-specific naming.
tests/blob.rs Comment-only updates to remove suite-specific naming.
tailbound-plan.md Removed (moved to supdb-bench).
tail-plan.md Removed (moved to supdb-bench).
syncpolicy-plan.md Removed (moved to supdb-bench).
src/wasmapi.rs Updates ABI docs to remove suite-specific naming.
src/lib.rs Removes bench module exposure and updates module-level commentary.
src/db.rs Comment-only updates to remove suite-specific naming.
src/bytes.rs Comment-only updates to remove suite-specific naming.
src/blob.rs Comment-only updates to remove suite-specific naming.
src/bench/json.rs Removed (bench module moved to supdb-bench).
shape-plan.md Removed (moved to supdb-bench).
segwrite-plan.md Removed (moved to supdb-bench).
segsize-plan.md Removed (moved to supdb-bench).
segroute-plan.md Removed (moved to supdb-bench).
sealwait-plan.md Removed (moved to supdb-bench).
scripts/claimrefs.sh Removed (suite/claims checks moved to supdb-bench).
scanmerge-plan.md Removed (moved to supdb-bench).
scanfloor-plan.md Removed (moved to supdb-bench).
results/w4-ranges.full.json Removed (moved to supdb-bench).
results/w4-ranges.ci.json Removed (moved to supdb-bench).
results/w3-bundle.full.json Removed (moved to supdb-bench).
results/w3-bundle.ci.json Removed (moved to supdb-bench).
results/w1-daysize.full.json Removed (moved to supdb-bench).
results/w1-daysize.ci.json Removed (moved to supdb-bench).
results/f8-checksums.full.json Removed (moved to supdb-bench).
results/f8-checksums.dev.json Removed (moved to supdb-bench).
results/f8-checksums.ci.json Removed (moved to supdb-bench).
results/f64-indexsum.full.json Removed (moved to supdb-bench).
results/f64-indexsum.ci.json Removed (moved to supdb-bench).
results/f60-sealwait.full.json Removed (moved to supdb-bench).
results/f60-sealwait.ci.json Removed (moved to supdb-bench).
results/f48-syncpolicy.full.json Removed (moved to supdb-bench).
results/f48-syncpolicy.ci.json Removed (moved to supdb-bench).
results/f47-parwal.full.json Removed (moved to supdb-bench).
results/f47-parwal.ci.json Removed (moved to supdb-bench).
results/f45-scanfloor.full.json Removed (moved to supdb-bench).
results/f45-scanfloor.ci.json Removed (moved to supdb-bench).
results/f42-load.full.json Removed (moved to supdb-bench).
results/f42-load.ci.json Removed (moved to supdb-bench).
results/f10-pair-mph-paged-vs-mph-pagedfixed.full.json Removed (moved to supdb-bench).
results/f10-pair-hash-paged-vs-hash-pagedfixed.full.json Removed (moved to supdb-bench).
results/f1-outofcore.full.json Removed (moved to supdb-bench).
results/c4-crash.full.json Removed (moved to supdb-bench).
results/c4-crash.ci.json Removed (moved to supdb-bench).
results/c1-decoders.full.json Removed (moved to supdb-bench).
results/c1-decoders.ci.json Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-next-pair.run3.env.txt Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-next-pair.run2.env.txt Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-next-pair.run1.env.txt Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-durable-pair.run4-valuelog.env.txt Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-durable-pair.run3-valuelog.env.txt Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-durable-pair.run2.env.txt Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-buffered-read.run2.env.txt Removed (moved to supdb-bench).
results/apple-silicon/ext-kv-buffered-read.run1.env.txt Removed (moved to supdb-bench).
results/apple-silicon/env.run2.txt Removed (moved to supdb-bench).
results/apple-silicon/env.run1.txt Removed (moved to supdb-bench).
results/apple-silicon/env-durable-pair.run1.txt Removed (moved to supdb-bench).
results/apple-silicon/env-buffered-pair.run2.txt Removed (moved to supdb-bench).
results/apple-silicon/env-buffered-pair.run1.txt Removed (moved to supdb-bench).
promote-plan.md Removed (moved to supdb-bench).
profile-plan.md Removed (moved to supdb-bench).
parwal-plan.md Removed (moved to supdb-bench).
merge-plan.md Removed (moved to supdb-bench).
madvise-plan.md Removed (moved to supdb-bench).
loadlevers-plan.md Removed (moved to supdb-bench).
inline-plan.md Removed (moved to supdb-bench).
filter-plan.md Removed (moved to supdb-bench).
figures/index.html Removed (moved to supdb-bench).
figures/full/index.html Removed (moved to supdb-bench).
fanout-plan.md Removed (moved to supdb-bench).
drain-plan.md Removed (moved to supdb-bench).
docs/engine.md Updates wording to remove suite-specific naming.
dict-plan.md Removed (moved to supdb-bench).
crash-plan.md Removed (moved to supdb-bench).
CONTRIBUTING.md Adds contribution workflow explaining the split and CI gating via supdb-bench.
compaction-plan.md Removed (moved to supdb-bench).
Cargo.toml Removes workspace + suite bins from this repo’s manifest.
bulkseal-plan.md Removed (moved to supdb-bench).
bench/profile.sh Removed (moved to supdb-bench).
bench/external/Cargo.toml Removed (moved to supdb-bench).
bench/aws/reap.sh Removed (moved to supdb-bench).
bench/aws/iam/setup.sh Removed (moved to supdb-bench).
bench/aws/iam/bench-policy.json Removed (moved to supdb-bench).
bench/aws/bootstrap.sh Removed (moved to supdb-bench).
.gitignore Updates browser-reader artifact notes (still needs alignment with moved test harness).
.github/workflows/runner-smoke.yml Removed (runner workflows moved to supdb-bench).
Review details
  • Files reviewed: 91/246 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread web/build.sh
Comment thread web/README.md Outdated
@claude
Give the wasm build its settings instead of a second profile
94393c9 
Cargo takes profiles from the workspace root, so the engine's
`[profile.wasm]` was ignored when it was built as a submodule of
supdb-bench, and the copy that had to live in the other manifest was two
definitions of one thing with nothing holding them equal. The settings
now travel with the only script that wants them, as --config overrides;
the module comes out byte-identical.

The bench job's fallback branch is gone: supdb-bench carries its own
check.sh, and a missing one is a broken checkout rather than a reason to
measure something else.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 5, 2026 20:45
@claude
Fail the wasm build where it stops knowing, and correct web/README
bf31f09 
The target directory is parsed out of `cargo metadata`; an empty parse
meant the script no longer knew where it was looking, and it would have
surfaced later as a missing module -- blaming the build for a failure in
the question before it. It exits there instead.

web/README still described build.sh as recording w3-bundle against a
budget in a binary this repository no longer has, and pointed at results,
claims and a browser test that moved. It describes what is here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI reviewed Sep 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A few updated comments/docs still refer to non-existent binaries/paths and the old web/build.sh JSON-results flow, which should be corrected to match the new engine-only layout.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 91/246 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread web/build.sh
Copilot AI review requested due to automatic review settings September 5, 2026 20:49
Copilot AI reviewed Sep 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Reviewed changes are primarily a mechanical suite extraction + CI/build script updates, with only a minor doc-comment typo to fix.

Review details
  • Files reviewed: 91/246 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@claude
Stop pointing at files this repository no longer has
344c973 
The suite moved, and eight references to it did not. Four are in code
comments and crate docs that name `results/` and `claims.json` as bare
paths, which now resolve to nothing here; they name supdb-bench instead,
and the two design briefs say so once in their opening rather than at
every citation.

The crate docs also carried two standing ratios for the durable load and
the point read. A figure in the present tense is a claim about how things
are, so it belongs where `verify` gates it -- the claim ids were already
beside the numbers, and are now the whole of it.

`.gitignore` had three rules for outputs nothing here produces: a vendor
directory under a `bench/` that moved, the `results-ci`/`figures-ci` trees
that `check.sh suites` wrote before that group left, and `web/test/out`.
`*.dat` went with the harnesses that wrote them. What is left is the
target directories, the profiler droppings and the wasm module.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 5, 2026 20:57
Copilot AI reviewed Sep 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes cross-repo CI gating and removes/migrates a large suite surface area, which can’t be fully validated from this PR alone without the paired supdb-bench merge/run.

Review details
  • Files reviewed: 92/248 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@claude
Send the runner sandbox policy where the runners are
70e733d 
`.localmostrc` declares every path and host a benchmark job on a
self-hosted Mac may reach. Strict mode means the declaration is the whole
surface, so the file is only worth reading beside the jobs it governs --
and those moved to supdb-bench with the suite they measure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 5, 2026 21:06
Copilot AI reviewed Sep 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It makes large structural and CI-adjacent changes (removing the in-repo suite and shifting gates to an external repo) that should be validated with a final human review of the cross-repo integration.

Review details
  • Files reviewed: 92/249 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@bfulton Claude

bfulton commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

bench is red, and it is not this pull request's.

x ext-kv/EXT.47: expected 'holds', recorded 'fails' --
  supdb 63678449 entries/s vs supdb-noadvice 68133583 entries/s
  (less 0.935x, p=0.0367, rel_iqr 1.5%/2.0%)

This head — 70e733d, unchanged — passed the same job at 21:24 and failed it at 22:47. Nothing moved in between except bench main, and that change touched f68-prefetch's detail text, not the scan path.

EXT.47 compares supdb against supdb-noadvice on the ordered scan and asserts they are not resolvably different, gated on clearing the 5% MIN_EFFECT. It was argued from full runs at 1.038 and 0.995. At ci the arms are small enough that runner variance reaches that floor. Three local ci runs on an idle machine give 0.963, 0.995 and 0.975; its partner EXT.46 gives 0.968, 1.040 and 1.025, and that 1.040 is a point from resolving as a win the claim does not assert. The failing run's rel_iqr of 1.5%/2.0% says both arms were stable within it — so this is the arms' relative speed differing between runs, not jitter inside one.

The fix cannot be ported here: the claim lives in the other repository. It is bfulton/supdb-bench#4, which pins EXT.46 and EXT.47 to full where their evidence was taken, and records the spread that earned the pin. Once that merges I will re-run this job; nothing in this branch needs to change.

Generated by Claude Code

@claude
Bring the benchmark suite back, under bench/
0c3f9cc 
The suite left this repository when it was large: 183 claims, a results
archive, plan files, browser tests and a launcher, and keeping the ruler
out of the thing it measured seemed worth a second repository. Rewritten
as a time series it is ten source files, one binary and a runs/ directory,
and the second repository cost more than it guarded: a paired pull
request for every change that touched both, a submodule pointer and a
branch override so CI tested the right engine, and the engine's release
profile silently ignored under the other workspace.

bench/ is its own cargo workspace, depending on the engine by path, so
the engine's build and test never pay for the comparators' C++ and its
lockfile never lists them; the release profile is repeated there with the
reason. A row records one sha, since one commit now names the engine and
the suite that measured it. scripts/check.sh gains the `bench` group (the
suite's build, test and lint, in the default set) and `quick` (one
measurement, gated against bench/runs/, with the figures; not in the
default set because a timing run needs the machine to itself). CI runs
`bench` in the three-platform matrix and `quick` in a job of its own,
and the job that cloned the other repository is gone. The three
workflows that ran the suite move here with their paths adjusted, and
.localmostrc returns to the root beside the jobs it governs.

The documents that described two repositories describe one. Claim ids
in code comments and design notes stay: they name the previous suite's
claims, in the supdb-bench repository's history, and CLAUDE.md says so.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 6, 2026 03:53
@bfulton bfulton changed the title Split the falsification suite out to supdb-bench Replace the falsification suite with the time-series suite, under bench/ Sep 6, 2026
Copilot AI reviewed Sep 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new bench build script and libclang helper have correctness/robustness issues (stale SUPDB_SHA due to insufficient rerun triggers; apt-get install without apt-get update fallback) that should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 108/267 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread bench/build.rs
Comment thread bench/scripts/libclang.sh
@claude
bench: re-stamp the sha on same-branch commits, update apt before ins…
11d4d21 
…tall

The build script watched only .git/HEAD, which a commit on the same
branch does not touch -- the branch's ref file or packed-refs changes
instead -- so the sha in a row could name the previous commit. It now
watches HEAD, the ref HEAD points at, and packed-refs.

libclang.sh's Debian fallback ran apt-get install on whatever package
lists the image had; it updates them first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 6, 2026 03:58
Copilot AI reviewed Sep 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new bench build/workflow lint utilities have concrete correctness gaps (notably build-script rerun tracking against a non-existent packed-refs path, and workflow run: coverage), and should be adjusted before landing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

bench/scripts/workflows.sh:56

  • The script claims it checks every workflow run: block, but the awk extractor only collects block-scalars (run: |). Most workflows here use single-line run: entries (e.g. run: sh scripts/check.sh ... in ci.yml), so they currently bypass both the bash-parse and ${{ }}-interpolation checks.
  • Files reviewed: 108/267 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread bench/build.rs
@claude
Remove every claim id and experiment name from the tree
dceabba 
The previous suite's claim ids (EXT.23, F43.4, W4.1, R6.3 and the rest),
its experiment names (f49, c4-crash, w1-daysize), its plan files and
its results paths were cited in about two hundred places: the design
brief, the index note, the crate docs, doc comments, test comments, the
web reader's README and its modules, and the suite's own CLAUDE.md. The
checker they pointed at is gone, and an id whose checker is gone is a
pointer to nothing that comes back as a number.

Each citation is rewritten so the measured fact and the reasoning stay
in words and the pointer goes: "measured at full scale, the ordered load
trailed LMDB" rather than an id in parentheses. Verdict vocabulary --
holds, fails, refuted, recorded as -- goes with them. Where a sentence
was only a pointer it is deleted. No number in the text changed, and the
only non-comment line changed is one assert message.

Also from Copilot's review: the build script watches packed-refs and the
branch ref only when they exist, since cargo treats a watched path that
does not exist as changed on every build; and the workflow lint now
checks one-line `run:` entries, which are most of them, not only block
scalars.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 6, 2026 04:09
Copilot AI reviewed Sep 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new bench/scripts/libclang.sh Linux probe can exit successfully on a versioned libclang-*.so even though the script’s own stated failure mode requires a libclang.so, risking CI breakage unless that condition is tightened.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 106/274 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread bench/scripts/libclang.sh
Comment thread bench/Cargo.toml Outdated
@claude
bench: say which profile settings are repeated and which differ
4abb949 
The comment claimed the release profile was the engine's settings
repeated, while the engine asks for full debug info and this asks for
line tables. Every setting that decides code generation is the same;
the debug level differs on purpose and the comment says so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 6, 2026 04:15
Copilot AI reviewed Sep 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It makes wide, repo-spanning changes to benchmarking infrastructure and CI-adjacent tooling (new bench/ workspace + removal of legacy suite artifacts), which warrants final human verification end-to-end.

Review details
  • Files reviewed: 106/274 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@claude
An exact head reserve, and a batch writer that uses it
f9c9f9b 
A segment's head reserve holds the block table, the key section's
checksum row and copies of the fence and the directory, and it has to be
sized before the first key is written. A streaming caller can only
guess, and both ways of guessing wrong are invisible: too small and the
pieces that do not fit go after the data, costing the sparse reader a
round trip, and too large and every segment carries zeroes it will never
use. So a floor under the guess was wrong in both directions.

`reserve::for_lengths` computes it from the key and run lengths a caller
that gathered its input already has. It restates none of the layout: it
plans the key section with `flatindex::plan_inline`, the call the writer
makes, over placeholder keys of the caller's lengths, and takes the
block table's size from a new `flatindex::block_table_len` that
`encode_blocks` now allocates by. The only arithmetic here is the cut
into blocks, which is the writer's rule and four lines.

The answer is minimal, not merely sufficient, and a test says so by
binary-searching the smallest reserve a reader can still open and seek
from and holding the estimate to it. What is over is twelve bytes: the
checksum row is cut on the object's pages, so its length depends on
where the section lands and that depends on the answer, and it is taken
at its worst alignment.

`reserve::from_totals` bounds it for a caller that knows only totals. It
asks for the longest key and the longest run as well, because without
them there is no bound: the fence copies whole keys, and a run that does
not fit closes a block early.

The four pieces are returned separately, because the last is a decision.
The directory copy is four bytes a key and buys a lookup that plans with
no second wave; `Reserve::without_directory` prices it.

`SegmentWriter::write_sorted` does the whole thing for input in hand. A
test holds it to the streaming writer byte for byte.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 6, 2026 04:30
Copilot AI reviewed Sep 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It’s a large repo-wide suite/CI/workflow restructuring with extensive deletions and new benchmarking infrastructure that warrants final human validation beyond static review.

Review details
  • Files reviewed: 106/275 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@claude
write_sorted: a field per setter, so compression is reachable
e2ffa4e 
The batch writer took `inline_max` as a bare argument and had nowhere to
put the rest, so it never called `set_compress` or `set_sync_every`. A
caller asking for a compressed segment got a plain one and nothing said
so -- which is the failure `SegmentOptions` refuses a compression field
to avoid, reproduced one layer up. Its note names the precedent: a
setting nothing reads does nothing quietly, and that is how a test came
to check the plain path while claiming to check the compressed one.

`SegmentWrite` carries the per-file settings, one field per setter on
the writer, so the next setter's absence is a compile error here rather
than a quiet default. It also carries whether the head reserve holds a
copy of the hash directory, which was previously always yes: four bytes
a key for a lookup that plans from the probe.

Three tests. One asks the file rather than the flag -- a compressed
segment of compressible values must be smaller than the plain one, and
must still read back the values that went in. It fails against the bug
it was written for, with the two sizes identical. One holds each setting
to the streaming writer byte for byte. One pins that compression does
not move the reserve, since blocks are cut on the staged payload before
anything compresses it, and that is worth a file on disk rather than an
argument.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 6, 2026 12:20
Copilot AI reviewed Sep 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It restructures and replaces the repository’s entire benchmarking/test artifact system (including new workspace + CI-facing scripts), which warrants final human verification across platforms.

Review details
  • Files reviewed: 106/275 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@claude
Take the wall clock out of the byte-for-byte segment comparisons
dfb8373 
The macOS runner failed the new batch-writer test on the `sync_every`
arm, which is the one setting that cannot change a byte: it moves when
fdatasync happens, not what is written. The cause was not the setting
and not the writer. A segment's superblock records SystemTime::now() in
seconds as its third field, and the FNV-1a over the fields that follows
covers it, in each of the two slots -- so two writes of identical input
differ in eighteen-odd bytes whenever they straddle a tick. Comparing
whole files is a test that passes on a fast machine and fails on a slow
one, and it did: green here, red there, on an arm where nothing about
the setting touches the file.

The comparisons now normalise those four words away and compare
everything else, superblock offsets included, since those are what a
mis-applied setting would actually move.

Worth writing down for whoever measures this next: the timestamp means
two builds of the same input are not the same bytes, so a segment
cannot be content-addressed or cache-validated across builds. That is a
property of the format rather than of this change, and nothing here
depends on it either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2wxgRU31dUrqaY8KwfsA2
Copilot AI review requested due to automatic review settings September 6, 2026 12:27
Copilot AI reviewed Sep 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The change set is a large benchmark/CI/tooling restructure with substantial deletions and new suite wiring that merits final human verification end-to-end.

Review details
  • Files reviewed: 106/275 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@bfulton
bfulton merged commit c8e8d94 into main Sep 6, 2026
8 checks passed
@bfulton
bfulton deleted the claude/supdb-architecture-review-6mkcis branch September 6, 2026 12:44
@bfulton bfulton mentioned this pull request Sep 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bfulton @claude