Skip to content

Commit a340589

Browse files
committed
test(vnext): prove isolated parser worker placement
1 parent 79dc271 commit a340589

18 files changed

Lines changed: 1851 additions & 0 deletions

.github/workflows/test.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ jobs:
9999
- name: 🧪 Browser Tests
100100
run: pnpm run test:browser
101101

102+
- name: 🧵 Worker Placement Evidence
103+
run: pnpm run test:worker-placement
104+
102105
package:
103106
env:
104107
EXPECTED_PNPM_VERSION: ${{ matrix.pnpm-version }}

docs/adr/0003-node-sql-parser-adapter.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ follow-up decision records one of:
167167
That decision must include browser measurements, cancellation behavior,
168168
worker/module failure recovery, and the effect of many mounted editors.
169169

170+
[ADR 0004](./0004-isolated-parser-execution.md) selects a dedicated,
171+
service-owned browser worker and defines the remaining evidence gates. It does
172+
not authorize session wiring until those gates and in-worker semantic
173+
normalization pass.
174+
170175
The current production loader supports pure Node only. A future browser
171176
integration must invoke parsing from a dedicated worker whose global object is
172177
not shared with application code, the legacy parser, or another installed copy.
Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,313 @@
1+
# ADR 0004: Isolated Browser Parser Execution
2+
3+
Status: accepted for implementation, session wiring gated by evidence
4+
Date: 2026-07-25
5+
6+
## Context
7+
8+
ADR 0003 keeps the `node-sql-parser` adapter internal and unwired. Its parser
9+
is synchronous, so an `AbortSignal` cannot interrupt it while it occupies the
10+
JavaScript thread. Even a late result that is correctly discarded can make an
11+
editor unresponsive.
12+
13+
Moving the current adapter object into a worker is not valid. Parser requests,
14+
authorities, ranges, artifacts, and analyses are authenticated by
15+
package-owned, realm-local `WeakSet` and `WeakMap` state. Structured cloning
16+
would produce unauthenticated copies. The backend AST is also retained in a
17+
realm-local weak map and cannot become a cross-realm semantic API.
18+
19+
The distributed dialect builds introduce separate constraints:
20+
21+
- The Node loader uses `node:module` and intentionally rejects any realm with
22+
`self` or `window`.
23+
- The browser builds are CommonJS/UMD files which a consumer bundler must
24+
transform.
25+
- Loading a build may write `NodeSQLParser` or `global` on its realm.
26+
- A browser worker can be terminated for a wall-clock deadline, but browsers
27+
do not expose an enforceable per-worker heap limit.
28+
- One worker per editor would multiply parser memory across marimo's many
29+
mounted editors.
30+
31+
This decision concerns local browser placement. Node `worker_threads`, native
32+
providers, remote providers, and public packaging are separate decisions.
33+
34+
## Decision
35+
36+
### Browser-first placement
37+
38+
Interactive browser parsing will use a dedicated module worker. The existing
39+
pure-Node inline adapter remains internal evidence and batch-test
40+
infrastructure. It is not a fallback when browser worker construction,
41+
loading, or execution fails.
42+
43+
Browser placement is accepted with an explicit residual risk: input, queue,
44+
response, cache, and lifetime can be bounded, but transient parser allocation
45+
cannot be capped before the browser itself terminates an over-consuming
46+
worker. The current 16 KiB input ceiling remains an upper safety bound, not an
47+
interactive performance claim. Production session wiring remains blocked
48+
until adversarial memory, latency, failure-recovery, and many-editor gates
49+
pass.
50+
51+
### Ownership and scheduling
52+
53+
Each `SqlLanguageService` will lazily own at most one dedicated parser worker.
54+
All sessions opened by that service share it. The worker is neither a
55+
`SharedWorker` nor a module-global singleton.
56+
57+
The first executor is single-lane:
58+
59+
- At most one request is posted at a time.
60+
- The host queue is bounded independently by request count and retained UTF-16
61+
text units.
62+
- A service owns construction, listeners, timers, termination, and disposal.
63+
- No worker pool or idle shutdown is introduced without profile evidence.
64+
- Service disposal terminates the worker and settles every pending consumer.
65+
66+
Ordinary caller cancellation and supersession settle the consumer promptly
67+
without relying on a worker message that cannot run during synchronous
68+
parsing. The executor may drain and discard that active result. A hard
69+
wall-clock deadline, worker crash, malformed protocol, or service disposal
70+
terminates the generation. The placement benchmark must compare drain versus
71+
restart under rapid edits before the executor policy is frozen.
72+
73+
The safety deadline is separate from product latency targets. A deadline
74+
failure never upgrades parser authority and an active request is not
75+
automatically replayed after a crash or timeout.
76+
77+
### Realm and loading boundary
78+
79+
The worker is created with a same-origin URL:
80+
81+
```ts
82+
new Worker(
83+
new URL("./node-sql-parser-browser-worker.js", import.meta.url),
84+
{ name: "codemirror-sql-parser", type: "module" },
85+
);
86+
```
87+
88+
Blob, data, and evaluated workers are not used. Hosts must allow the emitted
89+
worker URL in their Content Security Policy, normally through
90+
`worker-src 'self'`. The worker asset response also receives a restrictive
91+
policy because a worker has its own execution context.
92+
93+
The constructor shape and its literal options stay static. Current
94+
[Vite worker handling](https://vite.dev/guide/features#web-workers) recognizes
95+
the URL only when `new URL(..., import.meta.url)` appears directly inside the
96+
worker constructor. The
97+
[platform worker contract](https://developer.mozilla.org/en-US/docs/Web/API/Worker/Worker)
98+
also requires a same-origin entry URL and JavaScript response media type.
99+
100+
The browser worker has a browser-specific loader. It does not weaken or reuse
101+
the pure-Node realm gate. It dynamically imports literal, dialect-specific
102+
paths only:
103+
104+
```text
105+
node-sql-parser/build/postgresql.js
106+
node-sql-parser/build/bigquery.js
107+
```
108+
109+
The worker verifies that `self === globalThis` and that no DOM window exists.
110+
It snapshots and restores the exact `NodeSQLParser` and `global` descriptors
111+
around dialect loading. Cleanup failure poisons that worker generation.
112+
113+
### Private wire protocol
114+
115+
The worker protocol is package-private, versioned, closed, and decoded from
116+
`unknown` on both sides. It transports plain evidence, never authenticated
117+
syntax objects.
118+
119+
The initial request contains only:
120+
121+
- Protocol version
122+
- Host correlation ID
123+
- Grammar ID: PostgreSQL or BigQuery
124+
- Exact untrimmed statement text
125+
126+
DuckDB uses the PostgreSQL grammar. Target-dialect policy stays in the main
127+
realm.
128+
129+
The initial response contains only one closed outcome:
130+
131+
- Parsed normalized statement kind
132+
- Syntax rejection
133+
- Bounded unsupported reason
134+
- Bounded failure code plus retryability
135+
136+
Messages do not contain:
137+
138+
- Public document revisions or session identities
139+
- Parser authorities or dialect handles
140+
- `AbortSignal`, `Error`, stack, or raw backend message values
141+
- Source text echoed in a response
142+
- Absolute document ranges
143+
- Raw ASTs or generic payload bags
144+
145+
The host requires the current protocol version and correlation ID, validates
146+
all keys and closed values, and copies accepted data into new frozen objects.
147+
It then constructs an authentic `SqlParserAnalysis` with the exact pending
148+
request text and the host-owned authority. PostgreSQL and BigQuery rejection
149+
remain uncovered constructs; DuckDB rejection remains compatibility rejection.
150+
Worker isolation does not strengthen the compatibility-only evidence recorded
151+
by ADR 0003.
152+
153+
Old-generation events are ignored by generation-owned listeners. A malformed,
154+
duplicate, unsolicited, or mismatched response kills the generation and
155+
settles the active operation exactly once without exposing raw event data.
156+
157+
### Semantic reuse
158+
159+
Raw backend ASTs will not cross the worker boundary and the first protocol will
160+
not introduce remote AST handles or worker-local AST leases.
161+
162+
Before production session wiring, the worker request will parse once and run
163+
adapter-owned semantic decoders in the same realm. It will return only the
164+
bounded, validated relation facts required by the first completion slice.
165+
This keeps backend shapes private, avoids reparsing once for syntax and again
166+
for relations, and makes cached main-realm evidence measurable.
167+
168+
Worker-local AST caching is deferred until profiling demonstrates that
169+
reparsing is material enough to justify leases, byte accounting, generation
170+
invalidation, and release semantics.
171+
172+
### Packaging boundary
173+
174+
Core and `/vnext` imports must remain SSR-safe and contain no parser grammar or
175+
worker asset. A future optional integration entry may create the worker lazily,
176+
but it will expose an opaque language-service module factory rather than the
177+
protocol, worker URL, transport, pool, or backend AST.
178+
179+
The initial supported bundler claim is limited to packed-consumer fixtures that
180+
run in CI. Source-workspace success is not packaging evidence.
181+
182+
## Evidence required before session wiring
183+
184+
A production-shaped fixture built from the exact `npm pack` archive must prove:
185+
186+
- Core-only import emits no parser or worker bytes.
187+
- PostgreSQL and BigQuery emit separate worker chunks.
188+
- The all-dialect build is absent.
189+
- Worker creation is lazy.
190+
- Both grammars execute in a real browser.
191+
- Main-window parser globals remain unchanged.
192+
- Core import remains SSR-safe.
193+
- A same-origin module worker runs under a strict CSP.
194+
- Worker startup, cold import, warm parse, and message round-trip samples are
195+
recorded.
196+
- Raw and gzip worker sizes are recorded.
197+
198+
The executor and semantic slices additionally require:
199+
200+
- Main-thread long-task and event-loop responsiveness evidence.
201+
- Malformed message, crash, timeout, late-event, and restart tests.
202+
- Rapid-edit drain-versus-restart measurements.
203+
- One, ten, and fifty editor scenarios.
204+
- Retained worker, listener, timer, and memory checks after disposal.
205+
- Adversarial statements at the accepted input ceiling.
206+
207+
The current product envelopes remain:
208+
209+
- No routine main-thread task over 50 ms.
210+
- Warm active-statement analysis p95 under 16 ms.
211+
- Local completion p95 under 50 ms.
212+
213+
Safety timeouts are not evidence that these product targets are met.
214+
215+
### Initial packed-consumer baseline
216+
217+
The placement harness introduced with this decision builds the exact packed
218+
archive, consumes it from an isolated Vite 8 fixture, serves the production
219+
output with a same-origin CSP, and runs it in Chromium. Its first local
220+
Node 24 / Chromium 149 / arm64 macOS sample recorded:
221+
222+
| Output | Raw | gzip |
223+
| --- | ---: | ---: |
224+
| Core-only fixture | 24,462 B | 7,477 B |
225+
| PostgreSQL grammar plus worker entry | 318,628 B | 66,211 B |
226+
| BigQuery grammar plus worker entry | 222,769 B | 49,492 B |
227+
| Complete worker fixture | 567,271 B | 123,798 B |
228+
229+
The core module trace contained no `node-sql-parser` module. No dialect
230+
resource loaded before explicit construction. PostgreSQL and BigQuery were
231+
emitted as separate assets and both parsed successfully without changing the
232+
main-window parser sentinel.
233+
234+
Two consecutive cold/warm runs measured:
235+
236+
| Dialect | Cold request range | Warm parse | Warm round trip |
237+
| --- | ---: | ---: | ---: |
238+
| PostgreSQL | 17.0–32.6 ms | 0.2 ms | 0.2–0.3 ms |
239+
| BigQuery | 10.1–10.8 ms | 0.3 ms | 0.3 ms |
240+
241+
These numbers establish packaging feasibility and initial size guards. They
242+
are not percentile claims. Stable latency decisions require repeated,
243+
cross-platform samples over the representative and adversarial corpus.
244+
245+
The checked-in harness fails above 68 KiB gzip for the PostgreSQL assets,
246+
50 KiB for BigQuery, or 124 KiB / 590 KiB for the complete worker fixture in
247+
gzip/raw form. These ceilings include small measurement headroom and are
248+
placement-spike guards, not the final optional-integration bundle budget.
249+
250+
## Implementation sequence
251+
252+
1. Add this ADR and the packed-consumer browser placement harness.
253+
2. Extract a realm-neutral backend engine and add strict protocol codecs.
254+
3. Add the minimal browser worker and single-lane executor.
255+
4. Add in-worker normalized relation extraction.
256+
5. Add the pure statement coordinator, bounded cache, in-flight sharing, and
257+
atomic session ownership.
258+
6. Ship relation completion as the first public consuming vertical slice.
259+
260+
Every production step is a medium change and receives two independent,
261+
commit-bound adversarial reviews.
262+
263+
## Consequences
264+
265+
- Synchronous parser CPU work cannot block the editor main thread.
266+
- Fifty editors on one service do not imply fifty parser workers.
267+
- Realm-local authenticity remains an internal safety boundary.
268+
- Worker failure is explicit and never falls back to unsafe inline parsing.
269+
- The raw AST remains replaceable and private.
270+
- A serial worker may create head-of-line blocking; measurement, queue bounds,
271+
and hard deadlines make that tradeoff visible before considering a pool.
272+
- Browser heap exhaustion cannot be fully contained and remains a documented
273+
residual risk.
274+
- Browser and Node interactive execution can evolve independently.
275+
276+
## Rejected alternatives
277+
278+
### Run the parser on the browser main thread
279+
280+
Late-result rejection preserves correctness but cannot restore responsiveness
281+
while synchronous parsing runs.
282+
283+
### Clone normalized syntax objects from the worker
284+
285+
Structured cloning loses the package-owned realm authentication required by
286+
the syntax contract.
287+
288+
### Send raw ASTs or AST handles
289+
290+
Raw ASTs expose backend coupling and can be very large. Remote handles add
291+
leases, eviction, crash invalidation, and release semantics before a semantic
292+
consumer exists.
293+
294+
### One worker per editor
295+
296+
This multiplies grammar and runtime memory and conflicts with the many-editor
297+
release target.
298+
299+
### `SharedWorker` or a module-global singleton
300+
301+
Both weaken service ownership and disposal isolation. `SharedWorker` also
302+
narrows runtime and CSP compatibility.
303+
304+
### A generic worker or provider transport
305+
306+
The first need is one parser with a small closed protocol. A general framework
307+
would stabilize abstractions before there is evidence from a second workload.
308+
309+
### A worker pool
310+
311+
A pool increases grammar duplication, memory, scheduling, and cancellation
312+
complexity. It can be reconsidered only if a measured serial bottleneck
313+
outweighs those costs.

docs/vnext/node-sql-parser-adapter.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,7 @@ without importing a backend.
114114
For that reason, this adapter remains unwired. A worker-versus-main-thread ADR,
115115
with browser latency, memory, hostile-input, timeout, and recovery evidence, is
116116
required before interactive sessions may call it.
117+
118+
[ADR 0004](../adr/0004-isolated-parser-execution.md) chooses isolated
119+
browser-worker execution and records the packaging, performance, memory, and
120+
semantic-reuse gates that still block session wiring.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"test:coverage": "vitest run --config vitest.config.ts --coverage",
2424
"test:coverage:changed": "node ./scripts/changed-coverage.mjs",
2525
"test:browser": "vitest run --config vitest.browser.config.ts",
26+
"test:worker-placement": "node ./scripts/worker-placement.mjs",
2627
"test:integrity": "node ./scripts/check-test-integrity.mjs",
2728
"bench:parser-adapter": "vitest bench --run src/vnext/__tests__/node-sql-parser-adapter.bench.ts",
2829
"bench:statement-index": "vitest bench --run src/vnext/__tests__/statement-index.bench.ts",

0 commit comments

Comments
 (0)