Benchmark subproject: suite format, reference runner, JMH export - #9
Open
piotrszul wants to merge 43 commits into
Open
Benchmark subproject: suite format, reference runner, JMH export#9piotrszul wants to merge 43 commits into
piotrszul wants to merge 43 commits into
Conversation
…ion, reference runner) Introduce the implementation-agnostic SQL-on-FHIR benchmark subproject: the inline benchmark file format and schema, declarative dataset recipes with content-hashed materialization (generate-then-prune via a pluggable Synthea executor), the benchmark report schema, and the sof-js reference benchmark runner (load/time/stats, verify, bless). Includes the clinical-flat benchmark with blessed expected counts, wiring into the validate/test pipeline, and the OpenSpec design/specs (archived change + synced main specs). Squashed merge of PR #1 (_benchmark -> staging/benchmark). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `clinical-flat` cases declared `type: "string"` on three columns whose FHIRPath returns a FHIR `code` (`code`, `clinical_status`, `comp_code`). This is tolerated by the sof-js reference runner (which ignores `column.type`) but is non-conformant per the spec and rejected by strongly-typed engines like Pathling. Row counts are unaffected, so the blessed `expectCount` values stay. Also document that benchmark case views should conform to the ShareableViewDefinition profile (every column carries a `type` matching the FHIR type its path returns) in the schema's `view` description and the README, noting the reference runner does not enforce it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): propose pinning Synthea -e (end date) for reproducible benchmark data Author the `pin-synthea-end-date` OpenSpec change capturing issue #4: `tools/executors/synthea.js` omits Synthea's `-e` flag, so the simulation end date silently tracks the local wall clock and the blessed datasets are an accident of the bless-machine clock. Root-caused: `-e 20260630` reproduces the committed `m` data byte-for-byte (Condition 50090 / Observation 106613). The change proposes: pin `params.endTime` and pass it as `-e`; pass `--generate.thread_count=1` for deterministic export order; move the hardcoded export toggles into recipe `params` so `recipe + version` fully determines the dataset; and re-bless expectCount once. Design.md fixes the end-date decision (`endTime = 20250101`, one year after `referenceTime`, matching `yearsOfHistory = 1`) for the human approval gate. tasks.md is test-first: the failing executor-invocation / count test precedes the synthea.js change. Artifacts only; no implementation or test code, and Synthea was not run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): pin Synthea end date and source export toggles from recipe The synthea executor omitted `-e` (simulation end date), which Synthea defaults to the machine's wall-clock date, making blessed datasets an accident of the clock rather than a property of the recipe (issue #4). It also hardcoded output-affecting export toggles, so the recipe (and its content hash) did not fully describe the dataset. - Pass `-e <params.endTime>` verbatim; never fall back to the wall clock. A missing endTime surfaces via the invariant validator, not a default. - Pass `--generate.thread_count=1` so export order is deterministic. - Source `--exporter.fhir.bulk_data`, `--exporter.hospital.fhir.export`, `--exporter.practitioner.fhir.export`, `--exporter.years_of_history` from recipe params; keep `--exporter.fhir.export=true` as an executor invariant (mode selector, not a dataset dial). - Add an invariant check rejecting a synthea recipe with no params.endTime. - Tests (written first, observed red): executor arg-list assertions incl. wall-clock independence, and the validator endTime-required rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): pin endTime=20250101 in clinical-flat recipe and re-bless Add the pinned `endTime` (20250101, one year after referenceTime, matching yearsOfHistory=1) and the moved export toggles (hospitalExport=false, practitionerExport=false, bulkData=true) to the synthea recipe params, so recipe + version fully determines the dataset. The recipe content hash changes (e4d414fb → d4f6dc2a) accordingly. Re-bless expectCount once via the reference runner's --record path against the pinned end date (not hand-edited). Blessed counts (old → new): condition flat: s 6219 → 6406, m 50090 → 48483 observation components: s 4794 → 4366, m 40983 → 39336 Cross-checked analytically: condition flat = Condition manifest count; observation components = total `component` entries. Both cases report `ok` at both sizes on a non-record run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): require all output-affecting synthea params; guard undefined-leak Review hardening for the pin-synthea-end-date change. Previously only endTime was validator-guarded; omitting any of yearsOfHistory, hospitalExport, practitionerExport, or bulkData would interpolate `undefined` into the Synthea CLI (silently read as false / wall-clock), producing the wrong dataset with no error. - validator: reject a synthea recipe missing ANY output-affecting param (endTime, yearsOfHistory, hospitalExport, practitionerExport, bulkData) with a per-param message; booleans guarded on `== null` so explicit `false` is a valid declared value (TDD: red tests added first). - executor test: assert no captured Synthea arg contains "undefined", using a fully-declared recipe — a regression guard for the leak. - specs: generalize the suite-format validator requirement + scenario to all output-affecting params. - design/proposal: replace the "expected re-bless impact" prediction with the MEASURED numbers; record that size `s` was NOT end-date-insensitive at the flattened-view level, and that 106613 was the raw resource count, not the component-flattened view count (40983 -> 39336). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(openspec): archive pin-synthea-end-date change Applies the benchmark-suite-format and benchmark-dataset-materialization spec deltas into openspec/specs/ and moves the change to archive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… checkfile) (#16) * docs(benchmark): author benchmark-contract-v2 OpenSpec change Wave 1 umbrella OpenSpec change hardening the benchmark contract: - Structured implementation identity (engine/binding/variant) (#7) - Measurement scenarios (end_to_end, preloaded_repeated) mapped to JMH SingleShotTime, defined stats shape + precise inputRows (#5) - Explicit name+version dataset identity replacing the runtime content hash; TZ=UTC byte-identical NDJSON; new build-time checkfile capability + schema owning counts, checksums, and result assertions (#6) - Report provenance: benchmark + dataset identity and resource counts (#9) - Row-count guard reframed as work-verification (not conformance); invariance claim restricted to projection-position references (#12) Authoring only: proposal, design, tasks (test-first), deltas for four existing specs plus new benchmark-checkfile-format spec. No implementation. openspec validate benchmark-contract-v2 --strict: valid. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(openspec): Gate A revisions to benchmark-contract-v2 - Add a stable per-case `id`; checkfile assertions and report results key on `id`, not the mutable title. - Make the >=7 sample minimum advisory (spec prose), not a schema minItems floor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): update public schemas for contract v2 (#5/#6/#7/#9) - benchmark.schema.json: require stable case id, remove inline expectCount, add countVariancePermitted, document explicit dataset version semantics - benchmark-report.schema.json: structured implementation (engine required, binding/variant optional), measurement.scenario enum, defined stats shape, benchmark+dataset provenance, per-size resourceCounts, per-case id - benchmark-checkfile.schema.json (NEW): committed lock artifact contract Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): flag inline expectCount and pin TZ=UTC in synthea executor (#6/#12) - validator flags a case carrying inline expectCount (generated facts belong in the checkfile); removes the former expectCount-key-vs-size check - synthea executor sets TZ=UTC in the child-process env so emitted timestamps render in UTC, making NDJSON byte-identical across environments Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): key materialized data by name/version, delete content hash (#6) Rework layout to data/<name>/<version>/<size>/ using explicit identity; remove the JS-only recipe canonicaliser, .slice(0,8) truncation and array-order hash (kills F1/F6). Materializer records per-file sha256 in the manifest and honours dataset.version. Add checkfileFor() sibling-path resolver. Byte-identity test proves TZ pinning collapses timezones to identical sha256. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): checkfile writer/reader + identity-based runner (#5/#6/#9/#12) - benchmark/tools/checkfile.js: build/write/read the committed checkfile, per-file sha256 checksums, assertionFor, verifyChecksums (strict drift check) - runner locates data by name/version (no hash), reads assertions from the checkfile, honours countVariancePermitted, blesses by WRITING the checkfile - analytic cross-check (deriveExpectedCount) gates blessed counts - report: structured implementation (engine), measurement.scenario, defined stats shape (mean/min/max/stddev/p50/p95), benchmark+dataset provenance, per-size resourceCounts, per-case id, precise inputRows Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): migrate clinical-flat to authored-intent-only (#6) Add stable case ids (condition-flat, observation-components), pin dataset identity version to "1", add distinct syntheaVersion, remove inline expectCount (relocated to the checkfile). Validator requires syntheaVersion for synthea kind. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): canonicalise NDJSON order + re-bless checkfile under TZ=UTC (#6) Synthea's bulk export emits resources in a non-deterministic LINE ORDER across runs (thread_count pins generation, not export iteration order), so raw NDJSON is only sorted-identical. The materializer now sorts NDJSON lines byte-wise so persisted bytes and their sha256 are stable across environments. Re-bless writes benchmark/clinical-flat.check.json (the committed lock): - condition-flat: s=6406 (AEST 6406, =), m=48908 (AEST 48483, +425) - observation-components: s=4366 (AEST 4366, =), m=39479 (AEST 39336, +143) Byte-identity verified: a fresh re-materialization passes strict checksum verify with no drift. Checkfile validates against benchmark-checkfile.schema.json. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(benchmark): prettier formatting; check off §10 green gate Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): recipeOf strips syntheaVersion from the executor recipe syntheaVersion is dataset identity/lock info, not a generation input, so it must not leak into the recipe passed to executors. Add it to the stripped fields (test-first). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): make syntheaVersion optional in the checkfile schema syntheaVersion was unconditionally required while additionalProperties was false, which would reject a future non-synthea checkfile. Drop it from required (keeping it a defined, optional property documented as present for synthea-generated datasets); a checkfile without it is now accepted (test-first). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): prune stale assertions on re-bless buildCheckfile unioned previous + new case ids, so a deleted/renamed case's assertion survived forever. Filter merged assertions to the case ids being blessed now (test-first). Also route checkfile file paths through layout.js helpers (resourceFile/join) for consistency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(benchmark): share sha256Of and drop unused manifest.files Extract the duplicated sha256Of into layout.js and import it in checkfile.js. Remove materialize.js's checksums/manifest.files map: it was computed but never read (the checkfile is the authoritative per-file sha256 home), which also drops materialize's last sha256Of call site. Reword the canonicalisation comment: the sort is lexicographic (ordinal, locale-independent), and localeCompare is intentionally avoided for cross-environment reproducibility. Behaviour-preserving; committed checkfile bytes unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(benchmark): update cli.test fixture to the v2 contract Add syntheaVersion to the dataset and a stable id to the case so the fixture matches the current benchmark-file shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(benchmark): correct the byte-identity mechanism in the spec delta The implementation showed --generate.thread_count=1 does not make Synthea's bulk-export line order deterministic. Byte-identity across environments is delivered by TZ=UTC (timestamp rendering) plus the materializer's deterministic, locale-independent NDJSON line canonicalization. Rework the Reproducible Synthea materialization requirement and its Export-order scenario to attribute stable, byte-identical persisted NDJSON to the line canonicalization (not thread_count alone), and add a layout scenario asserting the materializer canonicalizes line order so persisted bytes reproduce across runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(openspec): archive benchmark-contract-v2 change Applies the suite-format, report-format, dataset-materialization, and reference-runner deltas plus the new benchmark-checkfile-format spec into openspec/specs/, and moves the change to archive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Synthea writes a generation db.sqlite and per-run public/export/<epoch>/ dirs to the working directory during materialization/re-bless, separate from the --exporter.baseDirectory output. Ignore them (root-anchored) so re-bless runs don't dirty the tree. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(benchmark): author contract-v2 feedback OpenSpec change (#18) Encode the three ACCEPTED pre-implementation refinements the Pathling implementer raised against merged contract v2: 1. Authored suite identity — add a stable suite `name` + authored `version` to the suite format so `report.benchmark.{name,version}` sources directly from the inputs (report.results keying is out of scope). 2. Both measurement scenarios use `sink: csv`; the scenario distinction is purely whether the load phase is inside the timed region (sink enum unchanged). 3. Reduce the required `stats` set to {mean, stddev, min, max, median} (median replaces p50); p95/ci95 become optional; samplesMs stays required; JMH projection updated. Authoring only (specs, proposal, design, tasks). No implementation, no schema/data edits, no re-bless. openspec validate --strict passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(benchmark): fold approved results re-key into contract-v2-feedback change (#18) Fold the human-approved addition into the OpenSpec artifacts so spec and impl stay coherent: report.results is re-keyed by the stable suite name (not the mutable title), consistent with report.benchmark.name, the case id, and the dataset name/version. - report-format delta: add a MODIFIED "Report structure and status taxonomy" requirement keying results on the suite name, with a scenario; update the "Size is a result dimension" delta to state results key on the suite name. - proposal: flip the out-of-scope framing to decision #4 (re-key by name); update Acceptance Criteria and Impact. - tasks: add the re-key work test-first in section 4. Re-validates: openspec validate benchmark-contract-v2-feedback --strict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): require authored suite name + version (#18.1, #9) Add a top-level authored suite identity to the benchmark file — a stable machine name and an authored version — mirroring dataset.name/dataset.version. These are the authoritative source for report.benchmark.{name,version} and the report's results map key, so a runner no longer invents identity from a pinned tag. - benchmark.schema.json: name + version required top-level; additionalProperties stays false. - validate-benchmarks.js: invariant validator requires suite name + version (== null idiom). - clinical-flat.json: declare name "clinical-flat" and version "1". Test-first (schema + validator RED observed before impl). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): reduce required stats, csv sink both scenarios, re-key results (#18.2, #18.3, #5) Three report-shape refinements plus the approved results re-key: - Reduced stats (#18.3): required stats set becomes {mean, stddev, min, max, median}; median replaces p50; p95 and ci95 are OPTIONAL; samplesMs unchanged (no minItems floor). statsOf emits median, not p50/p95. - csv sink both scenarios (#18.2): the runner defaults sink: csv for both end_to_end and preloaded_repeated; the scenario distinction is purely the load boundary (end_to_end times load+execute+extract, preloaded_repeated times execute+extract). The sink enum is unchanged (spec-level SHALL, not schema). - Provenance (#18.1): report.benchmark.{name,version} sourced directly from the authored suite name/version, not the dataset version or a pinned tag. - Re-key results (approved addition): the results map is keyed by the stable suite name, not the mutable title. Test-first (report-schema + statsOf + buildReport RED observed before impl). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): materialize CSV inside the timed region (truthful sink:csv) The reference runner's timed evaluate path only measured evaluate() (an in-memory JS array); no CSV serialization happened, so sink: 'csv' was not truthful for sof-js and its extract cost was not comparable to a runner that writes real CSV. Since the #18.2 contract makes both scenarios sink: csv precisely for comparable extract cost, the timed region must include CSV serialization. Extract the CSV serializer (formatRows/csvEscape) out of server/sql.js into a shared src/csv.js so the $run/SQL path and the benchmark runner materialize CSV identically (single source of truth, not a reinvention). timeEvaluate now serializes the evaluated rows to CSV inside the timed region and retains the output so an optimizer cannot prune it. outputRows/count semantics are unchanged — the count still comes from the evaluated rows. Correct the now-false comment in benchmark-run.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(benchmark): fix stale sink wording in report schema Both scenarios now use sink: csv. Update the measurement.scenario description so preloaded_repeated says sink csv (not the stale table/memory), and make clear the scenario distinction is the load boundary, not the sink. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(benchmark): assert per-scenario phases; drop duplicate schema test Add per-scenario phases assertions to the csv-sink test: end_to_end => [load, execute, extract]; preloaded_repeated => [execute, extract] (the runner's correctness touch, previously untested). Remove the newly-added duplicate low-sample-count schema test, keeping the pre-existing one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(openspec): archive benchmark-contract-v2-feedback change Applies the suite-format and report-format deltas (authored suite identity, csv sink for both scenarios, reduced required stats with median, results keyed by suite name) into openspec/specs/ and moves the change to archive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(benchmark): remove p95/ci95 from stats contract entirely Tighten the benchmark report contract so a case's `stats` is EXACTLY {mean, stddev, min, max, median}. p95 and ci95 are removed as properties; additionalProperties stays false so any extra key (including p95/ci95) is rejected. The reference runner already emitted only the five fields, so no emission change was needed. Applied spec, archived change docs, and the source comment are aligned to the exact-five contract. A consumer recomputes richer percentiles and the confidence interval / scoreError from the raw samplesMs the report always carries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(benchmark): author Wave 2 OpenSpec change (bootstrap + failure isolation) Encode two accepted benchmark-hardening issues into one OpenSpec change: - #8 (G9) Per-case failure isolation: reference-runner mandates record-and-continue and partial-report validity; report-format extends the status taxonomy with `timeout` and `malformed` and adds an OPTIONAL per-case `message`. - #10 (F9) Materialization bootstrap: dataset-materialization auto-fetches the pinned Synthea jar (checksum-verified, cached), making executors.config.json an optional override, and runs the generator in an isolated working directory so db.sqlite/public/ no longer land in the repo tree. Authoring only — no implementation, tests, schema, or data files touched. Validates with `openspec validate ... --strict`. No re-bless required. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): record-and-continue failure isolation + status taxonomy (#8) Extend the per-case report status enum to {ok, count_mismatch, generation_error, execution_error, timeout, malformed} and add an OPTIONAL free-text per-case message (benchmark-report-format). timeout = exceeded a wall-clock budget (distinct from execution_error); malformed = structurally invalid inputs/outputs (distinct from generation_error). Pre-production additive enum extension (Constitution IV): the four originals remain members, so a Wave 1 report still validates. Make the reference runner (sof-js/src/benchmark-run.js) enforce per-case failure isolation: each case runs under its own boundary, a failing case is recorded with an error status (+ advisory message) and the run CONTINUES; a failure never aborts the run or voids other cases. A partial run over a caseFilter subset still emits a schema-valid report of exactly the completed cases. Bless mode keeps failing hard (runCases), by design. Refs #8, #3. Test-first (Constitution III). No re-bless: touches no data or contract-of-data; the checkfile counts/sha256/assertions are byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): auto-fetch pinned Synthea jar + isolated generator CWD (#10) Add a committed syntheaVersion -> { url, sha256 } pin (benchmark/tools/executors/synthea-releases.js) and a fetch/verify/cache helper: the materializer auto-fetches the pinned Synthea jar into the gitignored benchmark/.cache/synthea/, checksum-verifies BEFORE use (loud failure on mismatch, nothing unverified cached), and reuses the cache on subsequent runs. tools/executors.config.json becomes an OPTIONAL override: with no config the jar is auto-fetched; with a config its jar path / java binary WIN. Unit tests mock the network (never hit GitHub). The 3.2.0 pin was verified at implementation time: the v3.2.0 release asset synthea-with-dependencies.jar (sha256 57216a99...db0720) is byte-identical to the blessed local jar that produced the committed checkfile. Run Synthea in an isolated working directory (cwd = per-materialization staging temp dir, outside the repo tree) so its incidental db.sqlite and public/export/ never land in the repo root; the final NDJSON still lands at data/<name>/<version>/<size>/ and the staging dir is cleaned up. gitignore benchmark/.cache/; annotate the sample config as an optional override. Refs #10, #3. Test-first (Constitution III). No re-bless: the generator jar checksum is complementary to and independent of the checkfile NDJSON sha256; the checkfile counts/sha256/assertions are byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(benchmark): check off Wave 2 implementation tasks (#8, #10) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): failure-isolate resourceCounts and share the timed-case body buildReport recorded per-case outcomes under a boundary, but then computed dataset.resourceCounts via observeResourceCounts with no isolation — a missing declared resource file threw AFTER all cases were processed, voiding the whole cases array and breaking the #8 partial-run-valid guarantee. Wrap the count observation so it degrades to {} and the report still emits and validates. Add tests: a missing dataset resource file leaves the completed cases intact (RED before the guard), and blessCheckfile still HARD-fails on a missing case file — documenting the intentional contrast (buildReport is record-and-continue, bless via runCases is all-or-nothing). Extract the shared timeCase() body used by both runOneCase (isolated) and runCases (non-isolated); annotate runCases as the bless-only hard-fail path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(benchmark): drop executor options shim, harden cwd test, broaden schema doc - Pin the isolated-CWD test to assert cwd === outDir (the staging dir the executor is handed) instead of the trivially-true "does not start with repoRoot" check. - Remove the normalizeOptions bare-config shim from makeSyntheaExecutor; every caller now uses the { config, ... } shape (skip integration test + capture helper updated). - Broaden the execution_error schema description to cover a pre-execution input-load/preparation failure, not only an engine raise. Description-only; runtime categorization is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(openspec): archive benchmark-bootstrap-and-failure-isolation Applies the reference-runner (record-and-continue), report-format (timeout/malformed statuses + optional message), and dataset-materialization (auto-fetch pinned jar, isolated CWD) deltas into openspec/specs/ and moves the change to archive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(#23) * docs(benchmark): author OpenSpec change for #22 failure-status clarifications Encode the accepted #22 clarifications into ONE OpenSpec change (spec + README only; no schema change, no runner code change, no re-bless): - benchmark-report-format: execution_error is the always-conformant default for any load/prepare/evaluate failure; timeout/malformed are OPTIONAL best-effort refinements a runner MAY apply; classification is best-effort (a lazy strongly- typed engine may report execution_error rather than distinguish malformed). - benchmark-report-format: reword timeout so it is keyed off the runner's OWN out-of-band wall-clock budget, dropping any implication of an authored/contract budget field (none is added; a shared authored budget is deferred). - benchmark-reference-runner: clarify subset filtering (--case/caseFilter) as OPTIONAL reference-runner convenience, not an implementer obligation; add a scenario. Contract requires only per-case-independent recording + partial-report validity. - tasks.md: only implementation edit is the stale benchmark/README.md data-layout line (data/<name>/<version>/<size>/); confirm existing tests green, schema and runner unchanged, no re-bless. openspec validate --strict: valid. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * benchmark: clarify failure-status taxonomy + fix README layout (#22) Spec clarifications from the Pathling Wave-2 review (#22): - status taxonomy is available-not-required; execution_error is the always-conformant default; timeout/malformed are optional best-effort refinements (classification is best-effort on lazy/typed engines). - timeout is keyed off the runner's own out-of-band budget; the contract defines no budget field. - per-case subset filtering (--case/caseFilter) is optional runner convenience, not an implementer obligation. Also fixes the stale benchmark/README.md data-layout line to the v2 identity path data/<name>/<version>/<size>/. No schema/runner code change, no re-bless. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(benchmark): author OpenSpec change for JMH-compatible export (#11) Add the benchmark-jmh-export OpenSpec change: a NEW capability spec benchmark-jmh-format defining a JMH Visualizer-compatible JSON export as a lossy projection of the native benchmark report. Ports the reference harness (dev/sof-benchmark results-reporting) faithfully: verified-cells-only, one file per (benchmark,size,impl) triple, mode:ss / ms/op primaryMetric with score=stats.mean and scoreError/percentiles recomputed from raw samplesMs, secondaryMetrics.rows from outputRows, and benchmark name <benchmark.name>.<case.id>. Spec authoring only: no implementation, tests, .js/.json/data changes. openspec validate benchmark-jmh-export --strict is green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): add pure JMH-export projection (#11) Add sof-js/src/jmh.js: a pure, lossy projection of a conforming benchmark-report.json onto the JMH-Visualizer JSON shape. Exports only verified `ok` cells with usable samplesMs; one file per (benchmark, size, implementation) triple named <benchmark>-<size>-<impl>.jmh.json with each segment sanitized. Numeric methods and JSON shapes are ported verbatim from the reference harness dev/sof-benchmark src/sofbench/results.py: _percentiles, _score_error (normal-approx 95% half-width 1.959964*sd/sqrt(n), n<2 -> 0), _jmh_entry, write_jmh_exports grouping, sanitize_id/output_stem, and the `verified` predicate. Deliberate divergences (design.md D8): reads the native report (score = stats.mean) rather than a CellResult, drops the `sofbench.` name prefix, adds `implementation` to params, sources `rows` from outputRows. Tests use hand-computed fixtures for score, scoreError, scorePercentiles, rawData nesting, secondaryMetrics.rows, verified-cells-only filtering, n<2 guard, and sanitized per-triple file naming. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): wire JMH export CLI and runner --jmh flag (#11) Add `bun run jmh <report.json> <outdir>` (sof-js/src/jmh-cli.js) and an optional `--jmh <dir>` flag on the reference runner, both calling the same pure projectJmh/writeJmhExports function — no duplicated logic. Extend the sof-js fmt/check-fmt globs to cover the new jmh* files and check off the implementation tasks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): recompute JMH score/iterations from samplesMs for fidelity Apply consolidated review feedback to the #11 JMH export, aligning the projection more closely with the reference harness (sof-benchmark src/sofbench/results.py) and hardening it against external reports. Behavioral (test-first): - score/scoreConfidence: recompute the mean locally from samplesMs (matching statistics.fmean) instead of reading caseDef.stats.mean, so an external report whose stats.mean drifts from its own samplesMs stays centred on the same samples as rawData. Reverses the earlier design.md D8-1 "reads stats.mean" divergence; identical for conforming reports. - measurementIterations: use the per-cell samplesMs.length (as the reference does) rather than report.measurement.iterations, which can contradict the actual sample count for an external report. Cleanups: - implementationId: coerce each part with String(...) so a missing engine field can't compose the literal "undefined" into the filename/params. - percentiles: drop the redundant String(...) around toFixed (toFixed already returns a string); percentile string keys are byte-identical. - Correct the writeJmhExports return comment (absolute only when outDir is absolute) and drop the now-unused existsSync test import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(openspec): archive benchmark-jmh-export change Creates the benchmark-jmh-format capability spec (JMH-compatible export: lossy verified-cells-only projection of the native report, one file per benchmark/size/impl, single-shot primaryMetric with CI/percentiles recomputed from samplesMs, secondaryMetrics.rows) and moves the change to archive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(benchmark): shared reference harness + thin hook contract Invert the runner contract: one shared harness owns the measurement loops, status taxonomy, statistics, CSV-derived row counting, checkfile verification, report emission, and JMH export, while an implementation provides only a thin worker HOOK (line-delimited JSON over stdio: capabilities -> prepare -> run xN -> shutdown). Comparability becomes true by construction rather than by spec prose. - NEW hook contract (public): hook.json manifest + worker stdio protocol (benchmark/benchmark-hook.schema.json). - NEW shared harness under benchmark/tools/harness/ owning worker lifecycle, harness-timed wall clock, both scenarios (preloaded_repeated, end_to_end with worker restart), stats, report and JMH emission. - sof-js re-wrapped as the first reference hook (src/hook.js, hook.json); benchmark-run.js slimmed to bless mode (--record + analytic cross-check). - jmh.js / jmh-cli.js relocated under the harness; tests moved with them. - Report format hardening (additive): stats opened for extension (five fields still required), per-case `verified` flag distinguishing verified-ok from unverified-ok. - Amend the reference-runner "no runner code" promise under the pre-production waiver: implementations need no runner code; the harness ships as replaceable convenience and the hook is the thin normative contract. Hand-rolled-runner route survives as the escape hatch. Verification: benchmark suite 154/0, validate clean, check-fmt clean. sof-js suite has no new failures and fixes 4 pre-existing $run CSV/JSON tests; remaining failures (fn_boundary, validate) are pre-existing conformance gaps unrelated to this change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(benchmark): simplify harness runner and worker Behavior-preserving cleanups from a quality pass over the harness: - runner.js: factor a shared sampleLoop() so both scenarios stop repeating the samplesMs/phase/lastResp bookkeeping; drop the redundant ctx bundle (inner fns already close over runSuite scope); fold verdict and stats into finishEntry, removing the carry-and-strip post-loop map. - worker.js: replace the string-dispatched settle() with explicit resolvePending/rejectPending helpers and route the timeout through them. - csv-count.js: drop lastNewline index tracking in favour of a direct trailing-newline check (quotes are balanced at a well-formed EOF). Benchmark suite 154/0, check-fmt clean. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(benchmark): harden harness against misbehaving hooks and inputs Fixes the confirmed review findings on the harness branch, each with a failing test written first: - worker: treat spawn 'error' as terminal (WorkerCrash, exit waiters woken) so a missing hook binary fails the case, not the run - worker: escalate shutdown()/kill() past SIGTERM to SIGKILL so a signal-trapping worker can never hang the harness - runner: prepare lazily per declared resource inside the case boundary, so a missing resource file fails only the cases that query it - cli: refuse a schema-invalid suite up front (a zero measurement count otherwise emits NaN-as-null stats in a schema-violating report) - cli: --strict fails loudly when the checkfile or its size entry is missing instead of silently skipping verification - hook (sof-js): answer ok:false for an unprepared resource type instead of silently evaluating an empty dataset as ok - hook.json: engine version 1.0.0 matches package.json, pinned by a drift test (reports previously claimed a nonexistent sof-js 2.0.0) - paths: pathFrom() helper (fileURLToPath) replaces URL.pathname at all six sites, fixing checkouts under directories with spaces - bench:harness no longer cd's into benchmark/, so the documented relative --hook and suite paths resolve from the repo root Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(benchmark): stdin EPIPE hardening + single NDJSON count implementation Fixes the two below-cap review findings, each with a failing test first: - worker: an 'error' listener on child.stdin fails the in-flight command with WorkerCrash instead of letting a send racing worker death raise an unhandled EPIPE (fatal under Node, silently swallowed by Bun). The regression test drives the real worker client through the race in a node subprocess, where the crash reproduces. - runner: report resourceCounts now reuse the checkfile's exported countLines instead of a divergent local counter, so a blank NDJSON line can no longer make the report disagree with the sha256-locked checkfile counts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(benchmark): switch the hook transport from stdio line-JSON to local HTTP The hook is now a small HTTP service with two lifecycle modes declared in the manifest (exactly one): `command` (spawn mode — harness assigns HOOK_PORT, polls GET /capabilities for readiness, owns process-group termination) or `endpoint` (connect mode — operator-managed service the harness only connects to; shutdown is never sent). The protocol keeps the same five commands as endpoints; engine failures stay in the body (2xx + ok:false) while transport failures (refused/reset, non-2xx, malformed body) fail only the in-flight case. Setup failures (never-ready spawn, refused connect) abort the run loudly instead of per-case noise. Scenario semantics per mode: end_to_end spawn mode restarts the service per sample (dataset-cold by construction); connect mode trusts an untimed reset before each timed prepare+run region. prepare gains replace-semantics and the new reset command discards prepared state. The sof-js hook is re-wrapped as a Bun.serve service; the fake hook fixture becomes an HTTP server covering the transport-native misbehaviours; the README hook route documents the HTTP protocol with a Flask sketch and a curl walk-through. The openspec change (proposal/design/specs/tasks) records the transport decision; the superseded stacked change directory is removed. Closes out the PR #26 review findings that intersect the rework: the spawn error/shutdown-hang/stdin-EPIPE surfaces no longer exist, and the remaining fixes (unprepared-resource ok:false, cwd-relative paths, --strict loudness, version-drift test, per-resource prepare isolation) are carried with tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(benchmark): refuse an unknown scenario up front with the valid names A typo'd --scenario was previously caught only by capability gating, whose 'hook does not declare scenario' error misattributes a caller mistake to the hook. runSuite now whitelists the scenario before any hook is started. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * openspec: sync benchmark-harness deltas to main specs and archive the change Creates the benchmark-hook-format and benchmark-harness capabilities, updates benchmark-reference-runner (two conformant routes, CSV-derived guard, extract in the timed region) and benchmark-report-format (open stats shape, verified flag, advisory message on ok). Change archived to openspec/changes/archive/2026-07-06-benchmark-harness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(benchmark): connector SPI + zero-code CLI hooks (connector-spi)
Formalize the harness-internal connector SPI (startConnector dispatching on
the manifest's lifecycle discriminator) and add the CLI connector: a
stateless command-line implementation is now benchmarkable with a manifest
alone — an argv template with {dataDir}/{viewFile}/{outCsv} placeholders,
one fresh engine process per timed run, end_to_end only by construction.
The hook manifest schema gains the additive 'cli' branch; the HTTP protocol
and all existing manifests are unchanged (existing worker/runner tests pass
unedited as the behaviour-preservation guard).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* openspec: sync connector-spi deltas to main specs and archive the change
benchmark-harness gains the connector-SPI requirement and CLI-aware scenario
semantics + failure taxonomy; benchmark-hook-format gains the CLI hook mode
and scenario-declaration requirements and the three-way manifest lifecycle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(benchmark): align CLI connector with design D3 (review follow-ups)
Review findings on #27, fixed test-first:
- kill() now escalates SIGTERM -> SIGKILL via terminateGroup with the
same { graceMs } policy as the HTTP spawn connector; a SIGTERM-ignoring
engine no longer leaks on the runner's abandonment path. Pinned by a
new IgnoreTermMe fixture mode.
- reset is a true no-op (cold by construction): the prepared dataset
location survives reset.
- cwd/env schema descriptions now say "Spawn and CLI modes", matching
the spec and the code (description-only).
- The preloaded-refusal test asserts SuiteError (now exported), as
tasks.md claimed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(benchmark): bootstrap contract validation via staging hooks Validate the benchmark contract against three real implementations (flatquack, Pathling CLI, Pathling Server) before external adoption. This bootstrap adds only the coordination scaffolding: - umbrella design doc (structure, workflow, findings taxonomy, exit criteria, migration/teardown plan) - benchmark/staging-hooks/README.md — the scaffolding contract for the temporary in-repo glue (deleted again at migration) - three minimal OpenSpec changes (proposal-level), one per target, each to be explored/implemented in its own session and feature branch, PR-reviewed into staging/benchmark, in order Note: the repo openspec config names schema `jg-spec-driven`, which is not installed in this environment; the changes use the stock `spec-driven` schema (identical artifact sequence). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(benchmark): add validation-cycle skill driving each validation change Repo-local skill encoding the per-change workflow: ordering gate, branch off staging/benchmark, opsx explore/artifacts/apply, findings taxonomy, verification pipeline, mandatory simplify + code-review refinement, archive-in-PR, and stop-for-user-merge. Verified with a baseline/with-skill subagent pair: without it, a fresh session skips exploration, the design artifact, and the refinement skills; with it, all ten steps are followed. Shares the scaffolding lifecycle: deleted with staging-hooks/ at migration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(benchmark): point cycle entry at the validation-cycle skill Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Extends the clinical-flat size series upward with `l` = 10,000 patients, giving three points across two decades (s=100, m=1000, l=10000) — enough for a credible throughput-vs-size curve where two points were not. Materialised counts confirm the scaling the two existing points only hinted at: Observation is dead-linear (~100/patient), while Condition is sub-linear (64/patient at s, 49 at m, 46 at l). `defaultSize` stays `m`. Checkfile blessed for `l` (Condition 459,611 / Observation 1,004,953; assertions condition-flat=459,611, observation-components=390,615); s and m carried forward untouched. Data is regenerated locally, not committed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(benchmark): validate contract against flatquack CLI hook First-contact validation of the benchmark hook contract against a real, independent, non-JVM engine (flatquack, aehrc/flatquack master-fix @ 8a66e46). Zero-code CLI-mode hook under benchmark/staging-hooks/flatquack/ (manifest + declarative SQL template). Size `s` passes with checkfile-exact rows (6406/4366); size `m` is blocked upstream (aehrc/flatquack#42). Contract fixes forced by findings (test-first, delta spec synced): - Harness handed engines symlinked temp paths ({viewFile}, {outCsv}) — on macOS /var -> /private/var — breaking engines that resolve/glob-walk the path string. New shared makeEngineTempDir (tools/harness/tempdir.js) canonicalizes every engine-facing temp dir; used by the CLI connector and the runner. Regression test forces a symlinked TMPDIR. - benchmark case views were not ShareableViewDefinition-conformant (missing url/name/status/fhirVersion); added, checkfile re-blessed byte-identical. - sof-js ViewDefinition schema rejected the spec-defined fhirVersion element; added (new shared conformance case in tests/validate.json, test-first). - README doc gaps: fictitious flatquack CLI example; unmentioned --scenario end_to_end requirement for CLI hooks. Tool defects filed and worked around: aehrc/flatquack#42 (exit-teardown segfault), #43 (exit 0 on SQL failure). Findings in benchmark/staging-hooks/flatquack/FINDINGS.md. Change archived in-PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(benchmark): flatquack adapter unblocks size-m; drop fragile glob idiom Add benchmark/staging-hooks/flatquack/flatquack-hook.js, a thin per-engine adapter (the CLI-hook analog of sof-js/hook.js). Two adapter jobs, no contract change: - View isolation: copy the harness's single {viewFile} into a fresh temp dir and run flatquack's real, documented CLI (--view-path <dir> --view-pattern '*.json'), replacing the earlier `..{viewFile}` idiom that leaned on undocumented Bun-glob behaviour. Reframes finding 2 from "contract gap (deferred)" to "no contract change; thin adapter". - Output-based success: ignore flatquack's exit code and report success iff the CSV was written. Masks the aehrc/flatquack#42 teardown segfault (which killed 100% of raw size-m samples) and turns #43's silent exit-0-on-failure into an honest non-zero. The harness still counts rows from the file against the checkfile, so a truncated write fails as count_mismatch. Result: both sizes now pass with checkfile-exact counts (s 6406/4366, m 48908/39479, stable across repeats). Timing is not yet trustworthy — crashed samples carry Bun's panic overhead — until #42 is fixed; documented in FINDINGS.md. Machine-local flatquack path reduced to env.FLATQUACK_CLI in hook.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(benchmark): run flatquack under node (upstream #42 fix); trustworthy timing flatquack fixed aehrc/flatquack#42 by launching its run mode under node instead of Bun (Bun could not finalize the legacy duckdb native addon, segfaulting at teardown). Update the adapter accordingly: - flatquack-hook.js launches `node <cli> --mode run …` (was `bun run`). - Exit code is trustworthy again, so honor it: success now requires a clean exit AND a written CSV (delete any prior CSV first so a reused sample path can't mask a failure). The CSV check still guards #43 (exit 0, no output). Both sizes now pass with zero flakes and no panic overhead in the timed region, so timing — not just row counts — is trustworthy: s 6406/4366, m 48908/39479, checkfile-exact. Closes the s+m exit criterion for this target with no contract change forced by findings 2–4. Docs (FINDINGS, hook README, archived design/tasks) updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(benchmark): record flatquack timing + correct the outlier explanation Add the actual end_to_end timing (M3 Pro, node) to the flatquack FINDINGS pass record, and explain the s/condition-flat outlier correctly: it is a cold-start first-touch I/O cost (chiefly loading the 54 MB duckdb.node addon + FHIR schema from cold page cache) on the session's first process, NOT JIT warm-up — each end_to_end sample is a fresh process, and the spike vanishes once page-cached (the first sample then becomes the fastest). Note the numbers are startup-dominated and not comparable to a warm/server deployment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(benchmark): record flatquack size-l pass (clinical-flat `l` scale) Exercise the flatquack hook against the new `l` scale (PR #30; 10k population, 442 MB / 923 MB NDJSON). Both cases verified clean, zero flakes across 3 runs: condition-flat 459611 rows (median 363 ms), observation-components 390615 rows (median 1135 ms). The largest scale is the strongest test of the #42 node fix — no crashes. Timing table + pass record updated; forEach unnest (observation-components) is now visibly data-bound at `l` while the simple projection stays near the startup floor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(benchmark): canonicalize flatquack adapter's view temp dir The adapter copied {viewFile} into an uncanonicalized mkdtemp dir and pointed flatquack's --view-path at it, re-opening finding 1's symlink defect (macOS /var->/private/var) for any non-symlink-following glob. Safe only because flatquack now runs under node; wrap the temp dir in realpathSync so the guarantee no longer depends on the runtime, matching the harness's own tempdir.js. Note the (test-less) coverage gap in FINDINGS.md — the adapter is outside the contract and a regression test would need a flatquack install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Second sibling validation change (after validate-flatquack-hook). Adds a
near-zero-glue CLI-mode staging hook driving Pathling's published `pathling
view` CLI, plus a full harness pass on clinical-flat at sizes s and m
(end_to_end).
Findings (benchmark/staging-hooks/pathling-cli/FINDINGS.md):
- Clean, adapter-free pass: `pathling view` fits {dataDir}/{viewFile}/{outCsv}
directly (--departition coalesces Spark output to one headed CSV; --overwrite
for the reused outCsv across samples). s: 6406/4366, m: 48908/39479, all
checkfile-exact, reports valid against benchmark-report.schema.json.
- Doc gap (fixed here): the run-output contract never stated the CSV must be a
single file with a header row, though the harness always required it. Adds a
"Result CSV output format" requirement to benchmark-hook-format (delta spec,
synced into the main spec), a README clarification at the shared /run
altitude, and a regression test pinning the header requirement. No harness
behaviour change (the behaviour was already tested).
- "Timed region includes process startup" confirmed: JVM+Spark boot dominates
each end_to_end sample (~6-8.5s); implementation.variant `cli` records the
deployment vs a future warm server.
- Predicted column.type / ShareableViewDefinition case defect did NOT
materialize (Pathling honours the typed views; metadata already added by the
flatquack cycle). No tool defect, no benchmark-case defect.
Archived in-PR with synced specs (repo precedent: #27).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…y-bounded pipeline (#33)
* feat(benchmark): add clinical-wide benchmark with xl tier via a memory-bounded pipeline
New clinical-wide.json (Patient/Encounter/Condition/Observation over six imported
Pathling views) with s/m/l/xl tiers and a blessed clinical-wide.check.json. The
whole materialize+bless pipeline is made memory-bounded so the xl (100k, ~22GB)
tier is materializable and blessable (~300MB peak RSS end to end).
- Streaming bless (one NDJSON resource at a time) + generalized analytic
row-cardinality cross-check (nested forEach/unionAll/cross-join/where)
- Case selection: --only / --exclude on the run and bless CLIs
- Export-filtered generation (recipe.resources -> exporter included_resources)
- Memory-bounded canonicalisation via a pure-JS external merge sort
(external-sort.js), byte-identical to the prior in-memory sort so existing
checkfiles are unchanged (no re-bless)
- Remove inert --generate.thread_count=1 flag: it is not a real Synthea property;
bulk-export line order is non-deterministic and canonicalisation stabilises it
- Streaming sha256/line-count in checkfile build
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* review(benchmark): address PR #33 findings (spec delta, line-count parity tests, cardinality memo)
- SP1 (MEDIUM): add a MODIFIED 'Reproducible Synthea materialization' delta so
the main spec no longer mandates the removed --generate.thread_count=1; document
memory-bounded canonicalisation and a 'no generation thread flag' scenario
- S1/SP2 (LOW): add countLines/hashAndCountLines parity tests across empty,
unterminated, terminated and single-line files, plus a sha256 whole-file parity
test (empty => 0 for both; the reported drift-to-1 was a false positive)
- SP4 (INFO): memoize normalize(structuredClone(view)) per view (WeakMap) so bless
no longer re-normalizes once per resource at xl; behaviour-preserving
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* review(benchmark): memory-bound the harness run path (PR #33 S3)
Streaming already bounded materialize + bless (D2/D8); a measurement RUN still
slurped whole files at the three reads the review named. Extend chunked reads to
the run path so a run, not only a bless, is memory-bounded at xl (an ~9 GB file
exceeds the JS engine's max string length):
- sha256Of (layout.js): chunked hash stream instead of a whole-file buffer;
used by verifyChecksums on the run path.
- countLines + hashAndCountLines (checkfile.js): share one private scanFileBytes
chunk loop (byte-level newline count, multibyte-safe); countLines backs the
harness's observeResourceCounts.
- loadResources (sof-js/src/benchmark.js): synchronous StringDecoder chunk reader
so a UTF-8 char split across a chunk boundary is reassembled and no whole-file
string is allocated; the hook request handler stays sync and unchanged. The
parsed table preloaded_repeated keeps resident is the scenario's inherent cost
(bounds the load, not the resident dataset).
Output-preserving refactor (identical counts/hashes/arrays), like the external
merge sort — no behavioural red. New chunked paths are locked by tiny-chunkBytes
boundary tests forcing a newline and a multibyte char mid-chunk (a naive per-chunk
toString corrupts the char, which the parity assertions catch).
Adds a benchmark-harness requirement (Harness run reads resource files by
streaming) + design D9 + tasks section 13. Verified: benchmark + sof-js suites
green, validate + check-fmt clean, openspec --strict valid, and the harness e2e
run at s on clinical-wide still reports all 6 cases verified.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* review(benchmark): stream the harness run OUTPUT path too (PR #33 re-review)
The prior commit bounded the run path's INPUT reads; the output side still
buffered whole files: countCsvRows read the output CSV with readFileSync, and the
hook wrote it via one serializeCsv string. A wide xl view's CSV can exceed the JS
engine's max string length even when the row objects fit, so both now stream:
- countCsvRows (csv-count.js): byte-chunk RFC-4180 scan carrying quote state
across chunk boundaries (" and \n never occur as UTF-8 continuation bytes, so
byte-level scanning is multibyte-safe).
- serializeCsv (csv.js): refactored onto a csvLines generator (one source of the
CSV shape) so a new synchronous writeCsvFile can stream rows to disk through a
bounded buffer, byte-identical to serializeCsv + writeFileSync. The hook uses
writeCsvFile; serializeCsv keeps its whole-string API for the server $run path,
and the hook request handler stays sync.
Same honest boundary as the input load: this bounds the COUNT and the WRITE, not
the result-row array the non-streaming evaluate() returns (a streaming evaluate is
a separate, larger change). Output-preserving — no behavioural red; the new paths
are locked by tiny-chunk/tiny-buffer boundary tests (quoted embedded newline +
escaped quotes + multibyte across boundaries; writeCsvFile byte-identity incl.
empty result and escaped fields).
Extends the benchmark-harness requirement + design D9 to the output side, tasks
13.6. Verified: benchmark suite 226 pass / 1 skip / 0 fail, sof-js csv/hook/
benchmark green, validate + check-fmt clean, openspec --strict valid, and the
harness e2e run at s on clinical-wide still reports all 6 cases verified. (The one
sof-js server metadata-endpoint failure is a pre-existing static-serve env issue,
confirmed identical with these changes stashed.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(benchmark): consolidate the streaming I/O onto one chunk-scan primitive (/simplify)
Cleanup pass over the PR #33 review follow-ups; output-preserving, locked by
the existing tiny-chunkBytes boundary/parity tests.
- scanFileBytes exported from layout.js; sha256Of, countLines,
hashAndCountLines, and countCsvRows all build on it (was four hand-rolled
openSync/readSync loops)
- countLines/hashAndCountLines share one lineTally, with newlines found via
Buffer.indexOf (native memchr) instead of a per-byte JS loop
- loadResources probes the raw chunk for a newline before joining, so a line
spanning k chunks joins once instead of re-scanning the carry per chunk
- writeCsvFile accumulates lines and joins once per flush instead of
per-line string concatenation
- cardinality() memo comment documents the identity-key immutability caveat
loadResources keeps its own loop deliberately: the hook example must not
depend on harness tooling. Skipped by design: the --strict double pass
(predates this change) and an async hook handler (contradicts D9).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Deep protocol test: full five-command HTTP hook + both lifecycle modes against Pathling Server. No behavioural contract change; one doc-gap recorded (finding 10, reset-omission SHALL wording). Two manifests (server-connect / server-spawn), spawn image digest-pinned. Connect + spawn re-verified at s; all declared scenarios validated.
… (add-measurement-plans) (#34) * feat(benchmark): plan OpenSpec change add-measurement-plans Full artifact set (proposal, design, delta spec, tasks) for generalizing the harness measurement loop into a JMH-like plan-driven executor, with official scenarios as bindings, staging-scoped post-loop count/extract verification, self-describing non-conforming internal records, and a staging flatquack DuckDB-session benchmark — toward retiring the standalone dev/sof-benchmark rig. Extends the validation-cycle skill to cover this change as the fourth in its sequence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(benchmark): plan-driven measurement core + internal-tuning reuse (add-measurement-plans) Refactor the harness measurement loop into one generic single-shot executor over a declarative, closed-data MeasurementPlan; re-express the official scenarios (preloaded_repeated, end_to_end in spawn/connect/cli) as named plan bindings with zero observable change — the existing suite is the regression net and passes unmodified. Add plan-only capabilities reachable only via the runPlanSuite module entry point (never the public CLI): fork-per-trial, in-engine table sink with untimed post-loop verification (count / extract), warmup on a load-timing region. Honesty guard: a custom plan emits a lossless <stem>.internal-report.json under a non-official internal:<name> scenario with the plan embedded verbatim; the published report schema's closed enum makes it fail-closed everywhere, at zero contract cost. validatePlan rejects unsound plans (post-loop-count without a materializing table sink; a post-loop verb under invocation fork) as loud setup failures before any case runs. Validate the reuse story against a real engine via a staging flatquack DuckDB-session hook + driver (benchmark/staging-hooks/flatquack-internal/): clinical-flat sizes s and m, two implementation identities, all cases verified against the sha256-locked checkfile via the count verb; extract==count; .timer cross-check holds. No contract/harness change was forced — the one quiet round for internal reuse (FINDINGS.md). Archive the OpenSpec change in-PR; benchmark-harness spec synced (+3 / ~1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(benchmark): reuse/simplify/efficiency cleanups from /simplify - reuse: flatquack-internal-hook uses makeEngineTempDir instead of re-implementing the mkdtemp+realpath symlink-collapse recipe inline - simplify: assembleReport derives sink/warmup from ctx.plan so the two call sites no longer duplicate the plan->sink/warmup derivation - efficiency: hoist timed-region command payloads out of the measured window (built once per case, not per sample) No behavior change; benchmark suite green (263 pass), fmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(benchmark): second /simplify pass over the plan-executor diff - validatePlan now rejects invocation-fork plans whose trialSetup/ invocationSetup/warmup fields the fresh-worker-per-sample path never applies (test-first) — an embedded plan can no longer claim choreography that did not run - lifecycleMode(manifest) exported from worker.js as the single manifest->mode mapping; startConnector and the runner both dispatch through it (was duplicated as runner-local modeOf) - runSuite/runPlanSuite forward one opts object to buildContext, which now owns the checkfile/inactivityMs defaults (was three parameter lists edited in lockstep); redundant validatePlan on the hard-coded official bindings dropped (pinned by the plan test suite) - OFFICIAL_SCENARIOS derived from OFFICIAL_PHASES (was a second copy) - DuckSession: single in-flight slot instead of a pending FIFO with errStart offsets built for pipelined calls no caller makes; errBuf reset per statement (was growing for the session's lifetime) - staging hook: dead reset verb and unread verbs capability removed; countSql() collapsed to the COUNT_SQL constant - driver: dead summaries accumulator removed - executor tests 4.1-4.5 + writer share one warm run via beforeAll (was six identical suite executions per bun test) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(benchmark): close review-noted coverage gaps + taxonomy labels Applies the three actionable suggestions from the PR #34 /review pass, all additive (no production-code change): - Pin that the public harness CLI cannot request a custom (internal:) plan — --scenario resolves official bindings only; the custom-plan pathway is module-only (runPlanSuite). Closes spec-review item 1. - Test the D6 driver honesty guard: runDriver refuses a manifest whose implementation.variant lacks the `internal-` prefix, before any duckdb session. Closes spec-review item 2. - Amend the staging-hooks README findings taxonomy with the `no contract change` and `tool/environment constraint` outcomes that all four staging hooks already use de facto, with a note on the sub-reason parenthetical. Benchmark suite green: 266 pass, bun run validate, bun run check-fmt. The pre-existing sof-js server-test failures are unrelated (reproduce on the clean base). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(benchmark): add duckdb 1.4.1/1.5.2 staging-hook variants Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Moves the completed change into openspec/changes/archive/ and syncs its delta specs (benchmark-dataset-materialization, benchmark-harness, benchmark-reference-runner) into the main spec files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adapts validation-cycle's branch/explore/artifacts/implement/verify/refine/ archive/PR cycle to arbitrary numbered issues: branch as issue/<n>_<slug>, and exploration grounds itself in the issue thread instead of a pre-planned change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Matches validation-cycle: issue branches fork from staging/benchmark and their PRs land back on it, not main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Closes #36. JMH Visualizer's two-file compare mode matches entries by benchmark name plus full params equality. Since one exported .jmh.json file always corresponds to one implementation, params.implementation was constant within a file but necessarily differed across the files a compare workflow loads side by side, so every cross-file compare matched 0 entries. Drop it from params (keep size), matching the reference harness's original shape; the implementation axis remains recoverable from the file name via the unchanged outputStem/ implementationId. Updates the benchmark-jmh-format spec's "axes carry our identity" requirement accordingly (BREAKING change to this published contract, per the stable-public-contracts principle) and archives the OpenSpec change in the same commit. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…ey (#40) benchmark/tools/harness/jmh.js's grouping-key template literal embedded a literal NUL byte (0x00) as the separator between benchmarkName/size/implId, instead of the '\x00' escape sequence. Both produce the identical runtime string, but a raw NUL byte makes git (and most text tooling) classify the file as binary — diffs collapse to "Binary files ... differ" and grep refuses to search it normally, hiding real changes to this file from PR review, git log -p, and similar tooling. Replace the two raw NUL bytes with '\x00', which is plain ASCII in the source and produces the identical string at runtime. Behavior is unchanged; the file is now a normal text file for diffing/grepping. Adds a regression test asserting the source file contains no raw NUL bytes. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Wraps the harness CLI and each staging hook's custom-plan driver behind one script so mixed hook selections (standard + flatquack-internal) don't need manual command construction, and adds a companion skill to diff resulting .jmh.json files in the JMH Visualizer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
Fixes the benchmark's objectives in one place (contract-first primary goal, self-tracking/comparison as secondary, kernel-support doctrine as design principles, explicit v1 non-goals) and defines the review process, triage rubric, and pre-triage of issues #2-#8 for public-review readiness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (design doc v2) Applies the review-discussion outcomes: principle 2 reworded to canonical versioned claim vocabulary with the registry/join convergence path; observable-boundary corollary added to principle 3; full findings list (F1-F14) triaged from the onboarding cold-read, design discussion, and systematic audit; scenario-artifact sketch as Appendix A. Verified on this branch: validate + check-fmt green; both checkfile-format and jmh-format spec Purposes are TBD stubs; harness dataset check is advisory-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…F14) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rvation, input guard) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…heckfile counts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (F2, F3) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t tier (F10) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…6, F9, F11) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Smooth the output-contract splice (drop the duplicated "single CSV file with a header row" noun phrase) and de-adjacency the curl walkthrough's opening phrase (name bench:harness run explicitly instead of relying on "also", since two subsections now sit between them). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
|
Public-review readiness pass applied on top of this branch (design record: docs/superpowers/specs/2026-07-17-benchmark-public-review-design.md): README Goals/non-goals + restructure, spec Purposes/rank annotations, single-node scope + phases-reservation clarifications, default input-count guard (test-first), internal-record schema promoted to harness. bun run validate and bun run check-fmt verified green — the two unchecked test-plan boxes can be ticked. Reminder recorded in the design doc: staging-hooks/ deletion remains a merge-time step (F8). |
…quirements Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop clinical-flat.json and its checkfile, leaving clinical-wide as the sole committed suite. README Layout and the hand-driven curl example now reference clinical-wide's dataset. Historical validation records under staging-hooks/ (slated for deletion at merge) are left as-is; test fixtures that use "clinical-flat" as a synthetic name are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The README's scenario-semantics paragraph is a summary; add a pointer to the OpenSpec requirements that normatively define each scenario's claim (benchmark-report-format) and its per-mode enforcement (benchmark-harness), plus plan.js as the executable binding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs(benchmark): public-review readiness (goals, scope, harness defaults, F1-F14)
…initions (#18) * docs(benchmark): add unify-benchmark-definitions OpenSpec change Formalize the direction from the design review: govern benchmark comparability by definition identity rather than access-control, so one harness runs standard and custom benchmarks through one path. Captures the measurement definition as a versioned registry artifact (new benchmark-scenario-definition capability), derived-not-asserted phases, identity-based fail-closed comparability with official drift detection, and a single harness entry point — deliberately keeping the hook/harness normative (comparability needs a pinned procedure, per the stress-test). Includes proposal, design (decisions + alternatives + phased migration), delta specs (new benchmark-scenario-definition; modified benchmark-harness and benchmark-report-format), and a phased, test-first task list. Umbrella for #10; subsumes #11 (run --definition replaces the fenced verb). Alt 2 (demote the hook) tracked separately in #17. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(benchmark): unify standard and custom benchmarks on scenario definitions Replace access-control comparability (closed scenario enum + internal: namespace + a second report schema) with identity-based comparability over versioned scenario definitions — the performance analog of tests/. - New kernel contract benchmark-scenario-definition.schema.json + a registry under scenarios/ (preloaded_repeated, end_to_end as data artifacts). - Single harness entry point runDefinition runs official (registry-resolved) and custom (--definition file) scenarios through one path; runSuite and runPlanSuite become thin wrappers; bindingFor is registry-backed. - phases derived from the plan's timed choreography (derivePhases), never asserted; the registry enforces derive(plan) === stated phases at load. - Reports cite measurement.definition {id, version, contentHash} and embed the executed measurement.plan; custom runs are fail-closed by identity (id not in the registry). Drift detection deferred to a future results site. - BREAKING (report-schema v2, version-signalled): scenario enum relaxed to a string id; measurement.plan/definition added; internal-report.schema.json and internal-report.js retired — one report format for every run. - CLI: run --definition <file> (mutually exclusive with --scenario). - Migrated the flatquack-internal staging driver to runDefinition. Closes #10. Subsumes #11 (the custom-plan CLI need is met by run --definition). OpenSpec change archived to openspec/changes/archive/2026-07-20-unify-benchmark-definitions; delta specs synced into openspec/specs/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(benchmark): apply simplify + code-review findings - registry/runSuite: surface a broken official artifact's real error instead of masking schema/coherence/parse failures as "unknown scenario" (check registry membership first, then load and let real errors propagate). - benchmark-scenario-definition.schema.json: allow ':' in the id pattern so it matches the report scenario pattern — a namespaced custom id (internal:<name>) authored as a --definition file now validates. - registry coherence: also enforce that a definition's stated sink/verification/ warmupPolicy agree with its per-mode plans, not just phases. - runner: collapse the now-unreachable optional-definition spread in assembleReport (runDefinition always cites the definition). - tests: colon-id acceptance (schema + loadDefinitionFile) and summary-field coherence rejection. bun test 299 pass / 0 fail; validate + check-fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Open the PR before refinement (step 7) so simplify/code-review and user review happen against it. Run bun checks plus /opsx:verify before review (step 9), then archive and squash-merge only on the user's approval (step 11). Update the description and red flags to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ess CLI (#20) Removes the bespoke flatquack-internal-driver.js and folds its orchestration into the run-benchmark skill: internal-plan runs now flow through `cli.js run --definition warm-table-sink.json` (a checked-in off-registry definition), via an explicit --definition flag on run-benchmark.js. The internal- honesty guard is dropped (comparability is fail-closed by off-registry definition identity). cli.js and every published schema are unchanged. Refinement collapsed the two per-mode run functions into one invokeHarnessRun primitive and fixed a regression that pass caught: a --definition run against a non-declaring hook must fail (exit non-zero), not be silently skipped. Closes #19 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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.
Summary
This branch (
staging/benchmark) accumulates 9 previously-merged PRs from its original development in piotrszul/sql-on-fhir.js (piotrszul#2, piotrszul#8, piotrszul#10, piotrszul#11, piotrszul#15, piotrszul#16, piotrszul#17, piotrszul#18, piotrszul#20, piotrszul#21, piotrszul#22, piotrszul#23, piotrszul#24) and is now ready to land onmain.Test plan
bun run test— 203 pass / 11 fail / 2 errors insof-js; all failures are pre-existing onmain(verified via a side-by-side worktree comparison) and unrelated to this branch's changes — mostlybeforeAllbun/jest incompatibility in server tests andfn_boundary/validate.jsonFHIRPath edge cases. This branch actually fixes 5$run/$viewdefinition-exportfailures present onmain.bun run bench:testpassesbun run validate/bun run check-fmt— not re-verified in this session; recommend running in CI before merge🤖 Generated with Claude Code
Migrated from piotrszul#25 (originally filed 2026-07-03).