Benchmark subproject: suite format, reference runner, JMH export

.claude/skills/compare-jmh/SKILL.md

@@ -0,0 +1,55 @@
+---
+name: compare-jmh
+description: Compare two or more JMH benchmark result files (*.jmh.json, e.g. produced by run-benchmark's --out directory) side by side using the JMH Visualizer (jmh.morethan.io), a client-side-only comparison tool. Use when the user wants to compare/diff benchmark runs, view results in JMH Visualizer, or check for performance regressions between hooks or tuning variants.
+---
+
+# Compare JMH results
+
+Drives the Playwright browser tools to upload `.jmh.json` files into
+https://jmh.morethan.io/ (data stays in-browser — the tool's own banner says
+so; nothing is sent to a server) and reads back the rendered diff.
+
+## Collect files
+
+REQUIRED, at least two. Accept either:
+- explicit `.jmh.json` paths, or
+- a directory (e.g. a `run-benchmark` `--out` dir) — glob `*.jmh.json` inside
+  it non-recursively.
+
+More than ~4 files becomes a multi-run comparison rather than a pairwise
+diff; the tool handles it, just say so when reporting back.
+
+## Procedure
+
+1. If the `mcp__playwright__browser_*` tools aren't already loaded, fetch
+   them: `ToolSearch("select:mcp__playwright__browser_navigate,mcp__playwright__browser_snapshot,mcp__playwright__browser_click,mcp__playwright__browser_file_upload,mcp__playwright__browser_take_screenshot")`.
+2. `browser_navigate` to `https://jmh.morethan.io/`.
+3. `browser_snapshot` to get the "Choose File" button's ref, then
+   `browser_click` it — this opens a file-chooser modal.
+4. `browser_file_upload` with the full list of absolute file paths in one
+   call (uploads them all at once, producing the comparison view directly;
+   no need to add one at a time).
+5. `browser_take_screenshot` with `fullPage: true`, **omitting `filename`**.
+   The Playwright tool sandbox only allows writing under the repo (its
+   default `.playwright-mcp/` or the repo root) — passing an absolute path
+   outside the repo (e.g. the session scratchpad) is denied. Let it use its
+   default `.playwright-mcp/page-<timestamp>.png`.
+6. Read the screenshot and transcribe the actual numbers into the chat
+   response as a table (case, scores per file, % delta) — don't just say
+   "see the screenshot." Call out the "Declined Benchmarks" / "Improved
+   Benchmarks" sections by name if present.
+7. The visualizer hides deltas under 5% by default (the "Ignoring deviations
+   below N%" slider) — mention this if the user asks about small
+   differences, since it means near-noise cases may be omitted from the
+   rendered tables even though they were uploaded.
+8. `.playwright-mcp/` is excluded via `.git/info/exclude` (a local, uncommitted
+   ignore — it will never end up in a commit), but it still accumulates on
+   disk across sessions with nothing pruning it. After reading the
+   screenshot, delete it (and any snapshot `.yml` files this run produced)
+   with Bash `rm` so it doesn't grow unbounded.
+
+## After
+
+Report the comparison table and where the screenshot was saved (before you
+delete it). Don't editorialize about whether a regression is "real" unless
+asked — just report the numbers.

.claude/skills/implement-issue/SKILL.md

@@ -0,0 +1,76 @@
+---
+name: implement-issue
+description: Use when asked to implement, fix, or work a specific GitHub issue by number in this repo (e.g. "implement issue 42", "let's do issue #17", "pick up issue 90", "continue issue 8"). Drives the issue end-to-end from a bare issue number to a squash-merged PR: branch, explore, OpenSpec artifacts, implementation, verification, PR, refine-through-PR, then archive and merge on the user's approval.
+---
+
+# Implement issue
+
+Drives one GitHub issue end to end, from a bare issue number to a merge-ready
+PR. Unlike a pre-planned change, nothing exists yet when this starts: the
+issue itself is the brief, and exploration is what turns it into an OpenSpec
+change. Read the issue in full (body + every comment) before doing anything
+else — the number alone is not enough context to branch, let alone implement.
+
+## Procedure — every numbered step is REQUIRED, in this order
+
+1. **Gate.** `gh issue view <number> --json number,title,body,state,url,labels,comments`.
+   If `state` is not `OPEN`, stop and ask the user before doing anything (a
+   closed issue may already be resolved, or intentionally shelved). Check for
+   existing work: `git branch -a | grep "issue/<number>_"` and
+   `gh pr list --search "<number>" --state all`. If a branch or PR already
+   references this issue, stop and ask whether to resume it instead of
+   starting fresh.
+2. **Branch.** Derive `<slug>` from the issue title: lowercase, kebab-case,
+   strip punctuation, cap at ~40 characters. `git fetch origin && git
+   checkout -b issue/<number>_<slug> origin/staging/benchmark`.
+3. **Explore.** `/opsx:explore`, grounded in the issue — walk through the
+   body and every comment, pull in anything it links to (files, other
+   issues, PRs), and resolve what it leaves implicit (repro steps, root
+   cause, acceptance criteria) with the user before writing any artifact.
+   Nothing is pre-planned here the way it is for other changes: the issue is
+   the only brief there is, so this step is where the real scoping happens.
+4. **Artifacts.** `openspec new change "issue-<number>-<slug>"` (same slug as
+   the branch), then `/opsx:continue issue-<number>-<slug>` until proposal,
+   specs/design, and tasks all exist. The proposal's Why section must cite
+   the issue (`Closes #<number>` plus its URL) and reflect what exploration
+   surfaced — not a restatement of the issue title.
+5. **Implement.** `/opsx:apply issue-<number>-<slug>`. Test-first, per this
+   repo's constitution (Principle III): the failing test goes in `tests/`
+   before the fix that makes it pass.
+6. **Verify.** `bun test`, `bun run validate`, `bun run check-fmt` — all
+   green. Before attributing a failure to your change, reproduce it on a
+   clean `staging/benchmark`.
+7. **PR.** Push, then `gh pr create --base staging/benchmark`. Body must
+   include `Closes #<number>`, a summary of the change, and verification
+   results (test / validate / fmt all green). The PR opens before
+   refinement so refinement and review happen against it.
+8. **Refine.** Run the `simplify` skill, then the `code-review` skill, over
+   the PR diff; apply the accepted findings and push the updates to the PR
+   branch. The PR is not review-ready until both skills have run.
+9. **Verify.** Re-run `bun test`, `bun run validate`, `bun run check-fmt` —
+   all green after refinement — then `/opsx:verify issue-<number>-<slug>` to
+   confirm the implementation matches the change artifacts. Update the PR
+   body if the verification results changed.
+10. **Stop for review.** Report the PR URL and a one-line summary, then ask
+    the user to review. Do not proceed past this step until the user
+    approves. Never start another issue in the same session unless
+    explicitly asked to.
+11. **Archive & merge.** Only once the user approves: `/opsx:archive
+    issue-<number>-<slug>`, and push the archive commit to the PR branch —
+    the archive and synced specs ship in the same PR, not a follow-up. Then
+    squash-merge the PR (`gh pr merge --squash`).
+
+## Red flags
+
+- "The issue is small, I can skip exploration" — step 3 is what turns a bare
+  number into a real brief; skipping it means guessing at scope.
+- "The diff is small, simplify/code-review would be overkill" — run them;
+  step 8 is unconditional.
+- "I'll refine before opening the PR" — the PR opens first (step 7);
+  refinement happens against it (step 8).
+- "I'll archive after the PR merges" — archive ships in the PR, pushed
+  before the squash-merge (step 11).
+- "Tests are green, I'll merge now" — never merge until the user approves
+  at step 10; the merge is theirs to authorise, not tests'.
+- "The issue title is clear enough for the proposal" — the proposal's Why
+  must reflect exploration, not just restate the title.

.claude/skills/run-benchmark/SKILL.md

@@ -0,0 +1,69 @@
+---
+name: run-benchmark
+description: Run the SQL on FHIR performance benchmark against one or more staging hooks under benchmark/staging-hooks/ (flatquack, flatquack-internal, pathling-cli, pathling-server), routing each hook through the harness CLI and collecting reports/JMH files into one .local/ run directory. Use when the user wants to run, benchmark, or measure a staging hook, compare engines or tuning variants, or asks to run performance numbers for this repo's benchmark subproject.
+---
+
+# Run benchmark
+
+Wraps `benchmark/tools/harness/cli.js` behind one script so mixed hook
+selections don't need manual command construction. Every hook runs through the
+same harness CLI; the script only chooses which measurement route:
+
+- **Registry scenarios (default):** each hook runs once per declared scenario
+  via `cli.js run --scenario <id>`; a scenario a hook doesn't declare is
+  skipped, not an error.
+- **Off-registry definition (`--definition <file>`):** each hook runs once via
+  `cli.js run --definition <file>` — a custom plan such as the flatquack
+  internal warm-table-sink
+  (`staging-hooks/flatquack-internal/warm-table-sink.json`). Repeating `--hook`
+  fans the definition out across implementation identities for an A/B overlay.
+  Mutually exclusive with `--scenario`.
+
+## Collect parameters
+
+Ask only for what the user hasn't already given.
+
+- **Hook(s)** — REQUIRED, no default, multiple allowed. A path under
+  `benchmark/staging-hooks/`, or a bare directory name (`flatquack`) — but
+  only when that directory has exactly one `hook*.json`. `flatquack-internal`
+  has several tuning variants (`hook.json`, `hook.threads1.json`,
+  `hook.duckdb141.json`, `hook.duckdb152.json`); always name the file
+  explicitly there, never guess which one.
+- **Benchmark** — default `clinical-wide` (`benchmark/clinical-wide.json`).
+  Bare name or explicit path both work. Single-valued per run.
+- **Size** — default `m`. Single-valued per run.
+- **Scenario(s)** — default both `preloaded_repeated` and `end_to_end`; a
+  scenario a hook doesn't declare is skipped, not an error. Multiple allowed.
+  Mutually exclusive with `--definition`.
+- **Definition** — OPTIONAL, single-valued. A path to an off-registry scenario
+  definition file (e.g.
+  `benchmark/staging-hooks/flatquack-internal/warm-table-sink.json`). When
+  given, every selected hook runs that custom plan once instead of the registry
+  scenarios. The flatquack-internal hooks measure nothing without it, so name
+  the definition explicitly when running them.
+
+## Run
+
+```
+bun run .claude/skills/run-benchmark/scripts/run-benchmark.js \
+  --benchmark <name-or-path> --size <s> \
+  --hook <path> [--hook <path> ...] \
+  [--scenario <name> ... | --definition <file>] \
+  [--out <dir>] [--data <root>] [--strict] [--only <ids>] [--exclude <ids>]
+```
+
+`--strict`/`--only`/`--exclude` are optional passthrough to the harness CLI and
+apply in both modes. `--scenario` and `--definition` are mutually exclusive —
+passing both is an error.
+
+Omit `--out` for the default
+`.local/benchmark-runs/<timestamp>-<benchmark>-<size>/` — every report and
+JMH file from the invocation lands in that one directory. The script prints
+a per-hook/scenario summary (ok / skipped / failed) and exits non-zero if
+anything failed.
+
+## After running
+
+Report the output directory and the printed summary verbatim — don't
+re-derive pass/skip/fail counts by hand. If something failed, read the
+printed stderr tail before proposing a fix; don't guess.