docs: ADR 0049 supersedes 0025 on compute locality - #1292
Conversation
|
Companion OSA design note: OpenScience-Collective/osa#351 ( |
📦 Package PreviewYour package is ready to test! bun install -g nemar-cli@PR1292Version: 🤖 Updated on each push. |
|
eegprep side of this decision is tracked as sccn/eegprep#324 (extras split, BLAS routing, ONNX and quantized ICLabel, Pyodide CI). |
Compute runs in the browser by default with eegprep as the engine, OSA owns the execution runtime, and only HPC submission is gated. Restates the broker decision from 0025 and marks 0025 superseded. Index test: 7 of 8 pass; the gapless-numbering check fails until epic #1272 lands ADR 0047 and 0048 on dev.
python-picard is pure Python with an optional numexpr fast path; the browser blockers in eegprep's declared dependencies are oct2py, pyedflib, and psutil. Notes the no-BLAS numpy build in Pyodide and the torch-free ICLabel need.
af244ff to
6fd3419
Compare
* chore: bump backend hono to ^4.11.4 Prerequisite for the MCP SDK's @modelcontextprotocol/hono peer (^4.11.4); previously pinned ^4.6.0. typecheck and the backend suite are green at this version. * fix: z.record requires an explicit key schema The single-argument z.record(valueSchema) form is invalid under zod 4 (z.record(keySchema, valueSchema) is required); the two-argument form is valid under zod 3 too, so this is a safe fix regardless of which major version the repo ends up on. Found while gating the phase 1 MCP contracts (issue #1293) against a zod 4 bump. * test: add live zarr v3 index and catalog fixtures Captured with curl -sL -A "nemar-cli/mcp-phase1" (issue #1293): - zarr-index-nm000329-slice.json: first 3 stores of the live index, keeping events_parquet and top-level doi/license/citation. - zarr-index-on003392-meg-sss-slice.json: the one live MEG store carrying an ADR 0028 sss object. - zarr-catalog-slice.json: 3 entries from zarr.nemar.org/catalog.json. - zarr-array-level0.zarr.json / zarr-array-view1.zarr.json: the level-0 and view/1 array metadata documents for nm000329's eeg_250hz group, matching the epic's verified geometry. * feat: zarr index v3 and MCP server contracts Issue #1293, phase 1 of epic #1065. shared/contract/zarr-index.ts is a .passthrough() zod reader mirroring shared/zarr-index.schema.json (the producer's closed contract stays the JSON Schema; this is a client-side lower bound on it, matching dataset.ts's convention). shared/contract/mcp.ts is the MCP wire vocabulary ADR 0049 fixes: the provenance envelope every tool response carries, buildReadRecipe() (recipe-first read_window, per the plan), and one input/output schema pair per tool (search_datasets, describe_dataset, list_recordings, get_events, render_overview, read_window), including the taste cap (60s x 64 channels) read_window's schema enforces. Both contract-*.unit.test.ts files exercise the public schema/function surface against real fixtures (on008083, and the two live slices), including three deliberate mutation tests (dropped source_commit, format_version 1, non-hex commit) proving the zod reader rejects what the Ajv-compiled JSON Schema also rejects. * feat: MCP transport and decode-path spike Issue #1293 step 5. Answers the two things the phase 1 plan left open: does @modelcontextprotocol/server@2.0.0 + hono@2.0.0 serve both protocol eras under workerd (wrangler dev), and which blosc/ zstd decode path (decision 7) actually runs there. Both eras of the transport work: server/discover, tools/list, tools/call in the 2026-07-28 envelope, the legacy initialize handshake, a legacy tools/call with no envelope, the -32020 rejection (disagreeing Mcp-Name, missing Mcp-Method), and GET/ DELETE 405 all behave as documented -- see smoke.sh. numcodecs' WASM Blosc codec does not run under workerd (dynamic WebAssembly.instantiate() on a runtime-fetched buffer is disallowed by the embedder); the pure-JS decoder in decode.ts (blosc2 header parse + fzstd + unshuffle) works and matches a Python-derived ground truth exactly. Path (b) is the chosen decode primitive. fixtures/chunk.bin is one real inner chunk of nm000329's level-0 array, captured via two range requests against the shard's trailing index; README.md documents the byte-level derivation and the full measurement set (bundle size, wall time). Replaced by phase 2 (#1294) and deleted in that PR. * docs: MCP server design doc Issue #1293. Purpose and constraints, the 2026-07-28 protocol revision and dual-era behavior (with the exact SEP-2243 header/ envelope mechanics verified against a real wrangler dev run, not just the spec summary), host and routing (phase 2 scope), the six- tool surface with cost class and cache behavior per tool, the envelope and read-recipe field tables, compute-minimization rules and the cache key scheme, the analytics event shape, the pinned dependency table with the spike's verdicts, the client compatibility table, and open items for phases 2-5. ADR 0050 is omitted from this PR per decision 11: origin/dev does not yet carry ADR 0049 (PR #1292 is still open). The decision text lives in this document instead; phase 5 files the ADR once #1049 has landed. * docs: cache the rendered PNG, reject over-cap taste The design doc drifted from plan decision 4 (cache the rendered overview PNG keyed by width, not the decoded level) and from its own schema (an over-cap taste is rejected by the input schema, not downgraded to a recipe). The zarrita snippet now uses the documented FetchStore form; zarrita's open.v3 takes a store, not a URL string. * fix: guard the spike decoder and assert HTTP status in smoke.sh decodePathB now refuses a short buffer, a non-int16 typesize, a zero blocksize (blosc2 marks a single block with blocksize == nbytes, never 0), a cbytes that disagrees with the input, offsets or lengths past the end of the chunk, and an odd decoded byte length that Int16Array would silently floor. Offsets and lengths are read unsigned. smoke.sh checks the HTTP status on every call, keeps bodies in a mktemp dir it removes on exit, and kills workerd along with wrangler. Smoke run: 10/10 PASS. * fix: contract review findings for the MCP wire schemas - a catalog row's null doi/license is authoritative over the index's stale hoisted copy; the index is consulted only without a row - fillTemplate fills layout templates in one pass and throws on a template that lacks a placeholder the caller needs - buildReadRecipe takes Zarr's own array metadata (data_type), with a zarrArrayMetadataSchema reader for the served arrays' zarr.json - a taste requires channels: the schema cannot know n_channels and a live MEG store has 320 - sss is present exactly when derived is true, enforced on the store reader and the envelope; sss models the converter's known keys - ranges enforce end >= start; flagToBoolean is the one 0|1|null to boolean conversion; min_participants dropped (no server-side filter) - tests: 26 -> 51, covering every tool input default and cap, both read_window output branches, the catalog document, the real zarr.json fixtures, and each fallback branch of the envelope * docs: align the design doc with the reviewed contracts search_datasets loses min_participants and names the two catalog routes it wraps; read_window's taste requires channels; the envelope table states the catalog-over-index rule; the recipe table splits dtype from codecs; section 12 records how phase 2 registers tools across the zod 3 / zod 4 split (zod4 npm alias plus a JSON Schema drift test).
|
Blocker cleared and verified, marking ready. The body's "do not merge before epic #1272" held because ADRs 0047 and 0048 existed only on that epic's branch, so the index test's gapless check failed. Epic #1272 is now closed and both ADRs are on Worth noting for anyone reading the checks: Merging now, which is the acceptance. ADR 0050 (the MCP server's own decisions, including the no-WASM-in-the-Worker rule) can take its number in epic #1065 phase 5 once this lands. |
test/adr-index.unit.test.ts validates gapless numbering, README index agreement, and superseded-target existence. Nothing in the path filters matched .context/**, so it never ran when those files changed: #1292 and #1330 both showed a fully green check list with the one test that could have failed skipped, and both were verified by hand instead. Same shape as the shared/** gap this file already documents, same fix. Wired into unit-pure, with a self-reference so editing the filter re-runs it.
* docs: file ADR 0050 and add a Python client verification ADR 0050 formalizes the rule two phases learned the hard way: no WebAssembly in the Worker bundle, ever, for any package this server depends on. Both failures (numcodecs instantiating WASM from a runtime fetch, and hyparquet-compressors compiling hysnappy's WASM at module load and so crashing isolate startup for every request) were invisible to bun test and caught only under real workerd, which is why the smoke script is the gate rather than an optimization. It could not be filed until ADR 0049 landed on dev, since the index test enforces gapless numbering; the content staged in the design doc is now a pointer to the ADR. scripts/mcp/verify_streamable_http.py drives a target host with the official Python MCP SDK: the negotiated revision, all six tools with their cache hint, then a chain from search through describe and list_recordings into the three recording tools and both read_window modes, against a recording it discovers rather than one hardcoded here. mcp-smoke.sh checks our own framing with curl and says nothing about whether a real SDK can talk to us, and OSA speaks this SDK, so this is the client that has to work. It is also where the docs page will copy its examples from. Its SDK usage was validated against a local MCPServer harness rather than discovered during a cutover, which caught two things worth having: any check failure inside the client's task group came back as a bare "ExceptionGroup: unhandled errors" with none of the message that explains it, and an over-cap refusal can arrive either as a tool error or as a raised protocol error, so the check accepts both. Design doc records the verified SDK version and the two assumptions in #1297 that turned out to be wrong: there is no For-agents guide to extend, and OSA has no MCP runtime at all while its two existing NEMAR tools call an endpoint that now 404s. * ci: run the ADR index test when ADRs change test/adr-index.unit.test.ts validates gapless numbering, README index agreement, and superseded-target existence. Nothing in the path filters matched .context/**, so it never ran when those files changed: #1292 and #1330 both showed a fully green check list with the one test that could have failed skipped, and both were verified by hand instead. Same shape as the shared/** gap this file already documents, same fix. Wired into unit-pure, with a self-reference so editing the filter re-runs it. * fix: harden the MCP verification script Review of scripts/mcp/verify_streamable_http.py found the script could not connect at all and could report a pass it had not earned. Fixed: - The default URL omitted the /mcp transport path. GET / is a descriptor and POST / answers Not Found, which during a cutover reads like an unprovisioned hostname. A bare host is now normalized and reported. - The over-cap probe wrapped its call in `except Exception`, so a 429 or a 502 became a PASS. Removed: both a tool error and a schema rejection arrive as is_error, so an exception there is a transport failure. - The probe sat exactly AT all three caps, which the schema accepts (`>`). Now one second over the duration cap with one channel, so exactly one cap is crossed, and the refusal text must name it. - The envelope check tested 3 of 14 required fields. Now all 14, plus non-null where the contract forbids null, and required only where the contract makes it required: it is .optional() on three tools, so failing hard everywhere reported missing provenance against a valid legacy index. - The subject was `results[0]`, now scanned for a v3 index with a usable group. has_zarr means converted, not v3, so a legacy hit is legitimate. - Read `results` only; `datasets` is catalog.json's field. - Notes are counted and repeated in the closing line, so a degraded run cannot look clean. Adds the wire-level test the script's cap assertion depends on: an over-cap taste answers isError with a text block naming the cap and the recipe remedy. Validated against a local MCPServer stand-in with twelve fault injections; all ten defects fail with the right message and the two honest degradations exit 0 with a note. * fix: correct ADR 0050's platform claim and the docs target An accuracy review of ADR 0050 and the design doc's phase 5 entry found four wrong claims, two of which would misdirect future work. All verified before changing, two by re-running the experiment. ADR 0050 said there is no flag permitting dynamic WASM compilation, so the restriction was the platform's and not configurable. Inverted. Measured under wrangler dev --local: compiling at MODULE LOAD is permitted by allow_eval_during_startup, which is the default from compatibility_date 2025-06-01, so the phase 3 shape is blocked only because this worker pins 2024-12-01. Compiling inside a request stays disallowed under every combination tested. So the decision survives but gains a real consequence: a routine compatibility_date bump silently removes the guard that caught the phase 3 failure, which is why the rule is written down and smoke-gated rather than left to the runtime. The flag's docs never mention WebAssembly, so the ADR now says the behavior is measured, not documented. ADR 0050 also framed both failures as dependencies that broke the server with a green suite. Neither was ever in backend/package.json: numcodecs was the spike's path (a), and hyparquet-compressors was removed before merge. The lesson holds, the history does not, and an ADR outranks the design doc. Design doc: the For-agents guide #1297 names does exist, published at docs platform/for-agents.md since 2026-09-03, so the docs work is a section on that page and #1297's premise was right. develop/zarr-contract.md is gone, split into six pages under platform/zarr/. Both errors came from reading a stale local checkout instead of origin/main. Also: #1324 has three non-handler exports, not two strings; the fixtures are real bytes where a real store exercises the path and synthetic only where the catalog has no such shape; the subpath-import option is named; and the custom-domain zone-edit caveat is moved next to the sequencing it gates.
Summary
New ADR 0049: compute runs in the browser by default, eegprep is the engine in every lane once its extras split ships, OSA owns the execution runtime, and only HPC submission is gated behind identity and the service tier. ADR 0025 is marked superseded; the parts of it that still hold (stateless recipe-first MCP broker on Workers, bulk bytes direct to S3) are restated in 0049 so it is the single current statement.
Why 0025 needed superseding rather than amending: its eegprep premise was wrong (eegprep 0.3.0 is a pure-Python wheel; the only
oct2pyimport is lazy, inside the EEGLAB comparison bridge), its hosting statement was wrong (OSA runs at api.osc.earth), and the product vision now has three stages with a login boundary the ADR did not address.Companion design note on the OSA side: OpenScience-Collective/osa
.context/browser-execution-tool-design.md(PR linked in a comment below).Files
.context/decisions/0049-compute-runs-in-the-browser-osa-owns-the-runtime-only-hpc-is-gated.md(new).context/decisions/0025-inference-compute-runs-on-device-mcp-is-a-stateless-broker.md(status, date, "Why it was superseded").context/decisions/README.md(index)Testing
bun test test/adr-index.unit.test.ts: 7 of 8 pass. The failing check is "ADR numbers are unique and gapless from 0001": ADR 0047 and 0048 exist only on the epic #1272 branch, so this PR stays red on that one check until that epic merges to dev, then goes green on rebase without changes.Notes
acceptedso the index test's "superseded target exists and is not proposed" reading stays consistent.