Skip to content

Commit e95be82

Browse files
authored
feat(vnext): complete SQL editor overhaul (#202)
## Summary Completes the final batched vNext SQL language-service overhaul milestone. - adds authenticated parser-neutral query binding models and bounded `node-sql-parser` normalization in the isolated worker protocol - adds public batched column and namespace catalog providers with stable provenance, epochs, bounded caches, cancellation, partial/loading/failure states, and hostile-response validation - integrates relation, namespace, and physical-column completion into document sessions and the CodeMirror adapter - adds exact statement gutter behavior and insertion-point completion gating for embedded marimo expressions while preserving external sources - adds correlated outer-scope column completion with conservative derived-table isolation - expands the marimo compile-time migration fixture and architecture/authority documentation ## Review hardening All 27 Cubic comments were reproduced and addressed. Two final adversarial reviews found and closed seven additional issues: - revoked proxies, decoder exceptions, exact response variants, structural identity comparisons, and blank namespace path components - generation-linearized cancellation and disposal in both catalog coordinators, including reentrant abort listeners - source-authenticated identifiers, aliases, CTE scope, derived ranges, compound blocks, and `ON`/`USING` visibility - exact statement bounds, one end-to-end response budget, the 64-relation deterministic partial cap, incomplete aliases, and line-comment boundaries - stale completion-gate callbacks, external-source preservation, and failure-safe browser cleanup CTE and derived-table output-column inference remains an explicit next-major cutover gap; vNext does not incorrectly route those logical relations to the physical catalog provider. ## Correctness and performance - original-document UTF-16 ranges and exact replacement edits - explicit partial/unsupported results instead of guessed semantics - revision, cancellation, owner disposal, scope, dialect, and epoch checks - no raw parser AST crosses or persists beyond the adapter/worker boundary - complete-only bounded catalog caches - deterministic ordering and deduplication - maximum-shape query-model validation reduced from about 888 ms to about 6.4 ms - framework-independent packed core: 187,989 raw bytes and 50,590 gzip bytes, within the unchanged 184 KiB raw and 50 KiB gzip limits ## Verification - 2,286 unit tests passed; 1 intentional expected failure - repository coverage: - 96.58% statements - 96.71% lines - 98.01% functions - 94.43% branches - changed vNext coverage: - 97.79% statements - 98.08% lines - 99.46% functions - 96.62% branches - every changed runtime file passes the 95% per-file statements/branches/functions/lines gate - strict TypeScript configurations, oxlint, and test-integrity checks pass - 14 browser tests pass in Chromium - build, demo build, packed-consumer smoke, parser-isolation/CSP checks, and isolated-worker placement audit pass Advances the next-major implementation milestone tracked in #169.
1 parent b0a76da commit e95be82

63 files changed

Lines changed: 17840 additions & 333 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,11 @@ A direct object or one-element array is accepted. Zero or multiple roots are
145145
`failed/malformed-output`.
146146

147147
The public normalized artifact exposes only its closed statement kind and full
148-
statement-relative range. The backend AST is retained in a module-private
149-
`WeakMap` keyed by the authenticated artifact. It is neither enumerable nor
150-
returned through the syntax contract. Future relation extraction must decode
151-
and validate backend nodes inside the adapter boundary rather than exposing
152-
the raw AST to core features.
148+
statement-relative range. The adapter consumes the backend AST immediately
149+
through the bounded query-binding decoder and retains only an immutable,
150+
backend-neutral binding model in a module-private `WeakMap` keyed by the
151+
authenticated artifact. The raw AST is not retained, enumerable, or returned
152+
through the syntax contract.
153153

154154
### Cancellation and execution placement
155155

@@ -201,7 +201,7 @@ This decision does not define:
201201
- Stable parser or provider APIs
202202
- Session cache keys or eviction
203203
- Document-level syntax diagnostics
204-
- Semantic relation, scope, column, or type models
204+
- Public semantic relation, scope, column, or type models
205205
- A worker protocol
206206
- Native DuckDB or remote validation providers
207207
- Dremio parsing

docs/adr/0004-isolated-parser-execution.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,10 @@ The initial request contains only:
141141
DuckDB uses the PostgreSQL grammar. Target-dialect policy stays in the main
142142
realm.
143143

144-
The initial response contains only one closed outcome:
144+
Protocol v2 contains only one closed outcome:
145145

146-
- Parsed normalized statement kind
146+
- Parsed normalized statement kind and a bounded query-binding DTO, or `null`
147+
when the statement has no supported query-binding evidence
147148
- Syntax rejection
148149
- Bounded unsupported reason
149150
- Bounded failure code; retryability is derived from that code
@@ -183,16 +184,17 @@ settles the active operation exactly once without exposing raw event data.
183184
Raw backend ASTs will not cross the worker boundary and the first protocol will
184185
not introduce remote AST handles or worker-local AST leases.
185186

186-
The first relation-completion slice is parser-independent under
187+
The first relation-completion slice remains parser-independent under
187188
[ADR 0005](0005-parser-independent-relation-completion.md). Protocol v1 remains
188-
syntax-only. Incomplete relation completion must not wait for worker startup,
189+
historical and protocol v2 adds the internal query-binding DTO. Incomplete
190+
relation completion must not wait for worker startup,
189191
parser acceptance, timeout, or recovery.
190192

191-
A future scope-dependent semantic slice will parse once and run adapter-owned
192-
semantic decoders in the worker realm. It will move the closed protocol
193-
atomically to v2 and return a bounded scoped IR, with normalized syntax and
194-
semantic availability represented independently. It will not return flat
195-
relation names, raw ASTs, source text, or absolute document ranges.
193+
The scope decoder runs once in the worker realm immediately after parsing.
194+
It returns query blocks, relation/alias bindings, persistent scopes, typed
195+
visibility regions, independent coverage, and closed issues. It does not
196+
return flat relation names, raw ASTs, source text, or absolute document ranges.
197+
The host revalidates the complete DTO before accepting the response.
196198

197199
Worker-local AST caching is deferred until profiling demonstrates that
198200
reparsing is material enough to justify leases, byte accounting, generation
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# ADR 0006: Internal query binding model
2+
3+
## Status
4+
5+
Accepted for the vNext implementation. The model remains package-internal until a
6+
column-completion vertical slice has validated it across the supported dialect
7+
corpora.
8+
9+
## Context
10+
11+
Column completion needs more than a parser's flat table list. It must distinguish
12+
query blocks, relation declarations, aliases, correlation, and clause-specific
13+
visibility. The parser adapter also has strict trust boundaries: raw parser ASTs
14+
are private, parser results can be partial, and cached analysis must belong to
15+
the exact statement and parser authority that produced it.
16+
17+
The model must remain cheap enough for interactive use and must not make the
18+
existing parser-independent relation completion path wait for parsing.
19+
20+
## Decision
21+
22+
Add an internal, backend-neutral query binding model with:
23+
24+
- statement-relative UTF-16 ranges;
25+
- query blocks and relation bindings identified by model-local array indexes;
26+
- a persistent scope chain in which each node adds at most one binding;
27+
- ordered, globally non-overlapping visibility regions that point to a scope;
28+
- independent query-block, relation-binding, and visibility coverage;
29+
- closed issue and unknown-source reason sets;
30+
- private authentication and exact statement/parser-authority ownership.
31+
32+
Globally non-overlapping regions deliberately exclude nested query text from
33+
their parent region. A decoder splits a parent region around nested queries.
34+
This permits binary-search lookup without duplicating the visible binding set.
35+
Persistent scopes then reconstruct bindings in declaration order using linear
36+
space. In particular:
37+
38+
- a `SELECT` list can point to the completed `FROM` scope even though it appears
39+
earlier in the source;
40+
- a join condition points to the scope containing prior relations and its
41+
current right-hand relation;
42+
- a correlated child block can enter through a parent scope;
43+
- an ordinary derived table enters through a scope that excludes same-level
44+
siblings; a future proven `LATERAL` decoder may opt into that scope.
45+
46+
Aliases are the visible qualifier when present and hide a named relation's base
47+
name. Identifier equality is supplied by the dialect runtime; the model does
48+
not lowercase or otherwise reinterpret identifiers.
49+
50+
Construction accepts untrusted plain data and validates it before issuing an
51+
immutable authenticated model. It rejects accessors, sparse or oversized
52+
arrays, malformed UTF-16 identifiers, invalid or unrelated ranges, forward
53+
parent edges, repeated bindings in a scope chain, overlapping regions, and
54+
unknown bindings paired with complete relation coverage.
55+
56+
Resolution returns:
57+
58+
- a visible binding list with complete or partial coverage;
59+
- one resolved binding, known ambiguity, or authoritative no-match;
60+
- unavailable instead of no-match when coverage is partial.
61+
62+
## Ownership and invalidation
63+
64+
The future parser service owns bounded parse/model caching and in-flight
65+
deduplication. A document session owns the mapping from its current statement
66+
slots to immutable models. The cache key includes exact statement text,
67+
dialect, parser authority/conformance, and implementation version.
68+
Statement-relative coordinates allow an unchanged statement to be reused after
69+
an edit shifts its absolute document position. Source, dialect, or parser
70+
authority changes invalidate the model; catalog epoch and completion context
71+
changes do not.
72+
73+
The interactive statement limit is initially 16 KiB. The model also caps query
74+
blocks at 256, relation bindings at 1,024, scopes and regions at 2,048 each,
75+
nesting by the block limit, issues at 256, path depth at 8, and identifier
76+
components at 256 UTF-16 code units.
77+
78+
## Degradation
79+
80+
A decoder may claim complete evidence only for directly conforming parser output
81+
and constructs it understands. Compatibility parses can supply useful partial
82+
evidence but cannot prove absence. Unknown AST subtrees become an unknown source
83+
or a closed issue and downgrade the relevant coverage; they are never silently
84+
dropped under complete coverage.
85+
86+
Invalid, failed, opaque, incomplete, or resource-limited analysis becomes
87+
partial or unavailable. Parser-independent relation completion remains
88+
available. Dremio remains partial or unavailable until it has an owned semantic
89+
corpus.
90+
91+
## Consequences
92+
93+
The model can support lazy batched column loading without exposing a parser AST
94+
or CodeMirror's eager `SQLNamespace`. Its indexes are not stable identities
95+
across revisions. Projection inference, output columns, star expansion, types,
96+
expression binding, semantic diagnostics, navigation, rename, formatting, and
97+
vendor constructs such as `PIVOT`, `UNNEST`, table functions, and unproven
98+
`LATERAL` semantics are explicitly outside this slice.
99+
100+
The node-sql-parser integration now supplies a strict AST-to-plain-data decoder,
101+
normalizes inside the worker, and checks direct-versus-worker parity. Public
102+
semantic APIs remain deferred until a column-completion vertical slice proves
103+
the model against consumer and dialect corpora.

docs/vnext/capability-charter.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ count and estimated retained bytes.
239239

240240
Provisional compressed bundle budgets:
241241

242-
- Framework-independent core with relation completion: 48 KiB
242+
- Framework-independent core with relation, namespace, and column completion:
243+
50 KiB
243244
- Core plus CodeMirror adapter, excluding parser and dialect data: 75 KiB
244245
- Each ordinary dialect module: 25 KiB
245246
- Optional `node-sql-parser` chunk: no regression from its recorded baseline

docs/vnext/codemirror-adapter.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,51 @@ const support = sqlEditor({
7070
});
7171
```
7272

73+
## Statement boundaries and gutter
74+
75+
The support object exposes synchronous structural queries without exposing the
76+
adapter-owned session:
77+
78+
```ts
79+
const current = support.statementBoundaryAt(view, {
80+
affinity: "left",
81+
position: view.state.selection.main.head,
82+
});
83+
const visible = support.statementBoundariesIntersecting(view, {
84+
from: view.viewport.from,
85+
to: view.viewport.to,
86+
});
87+
```
88+
89+
Both methods return `null` for foreign, destroyed, or unsynchronized views.
90+
Catalog-backed hosts can invalidate the adapter-owned session without exposing
91+
it:
92+
93+
```ts
94+
const revision = support.invalidateCatalog(view);
95+
```
96+
97+
This returns `null` for foreign or destroyed views. A live view advances its
98+
revision, cancels stale catalog work, and forces relation, column, and namespace
99+
providers to be consulted again.
100+
101+
An opt-in structural gutter marks only lines intersecting scanner-owned SQL
102+
`code` spans. It never parses or copies statement text:
103+
104+
```ts
105+
const support = sqlEditor({
106+
initialContext,
107+
service,
108+
statementGutter: {
109+
hideWhenNotFocused: true,
110+
showInactive: true,
111+
},
112+
});
113+
```
114+
115+
The gutter uses `--cm-sql-statement-color` and
116+
`--cm-sql-statement-inactive-opacity` CSS variables. It is disabled by default.
117+
73118
## Atomic inputs
74119

75120
Context and document changes can share one CodeMirror transaction:
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# vNext Column Catalog Authority
2+
3+
Status: internal vertical-slice contract
4+
5+
Column discovery is lazy, provider-owned, and batched. A completion request
6+
sends at most 64 unresolved relation references in one provider invocation.
7+
The deterministic first batch is useful but explicitly partial when more
8+
visible relations match; completion reports `query-binding-partial` and never
9+
claims the omitted relations were searched. Each reference carries a
10+
caller-local `requestKey`, a decoded identifier path, and no unauthenticated
11+
entity identity. The provider resolves paths against the supplied catalog
12+
scope, search paths, and dialect.
13+
14+
The provider returns stable relation and column entity IDs. Every accepted
15+
column contains:
16+
17+
- a canonical `SqlIdentifierComponent`;
18+
- bounded provider-rendered `insertText`;
19+
- a stable column entity ID and ordinal; and
20+
- immutable provenance containing provider, scope, epoch, relation, and column
21+
identities.
22+
23+
Responses describe each requested relation independently as:
24+
25+
- `ready` with complete or partial column coverage;
26+
- `loading`; or
27+
- `failed` with a normalized code and retry policy.
28+
29+
Missing, extra, conflicting, oversized, accessor-backed, or malformed data is
30+
rejected at the provider boundary. Relations and columns have deterministic
31+
code-unit order. Duplicate request keys and conflicting stable IDs fail closed;
32+
identical duplicate columns are deduplicated.
33+
34+
## Epoch and cache behavior
35+
36+
Cold requests use `expectedEpoch: null`. A response always supplies an epoch.
37+
An owner remembers that observation, and later null-epoch requests reuse only
38+
cache entries from the observed epoch. Explicit expected epochs never reuse
39+
entries from another epoch. The cache is LRU-bounded by relation entries.
40+
Only complete ready results are cached. Partial, loading, and failed results
41+
remain visible to the caller but are eligible for another provider request.
42+
43+
Relation-catalog subscription events invalidate the session's relation, column,
44+
and namespace observations together. Hosts without a subscribed relation
45+
provider call `session.invalidateCatalog()` when schema authority changes. The
46+
method advances the session revision, cancels retained completion work, rebuilds
47+
all catalog owners, and emits one `catalog` change event.
48+
49+
## Lifecycle
50+
51+
One owner represents one document/session authority for a scope and dialect.
52+
Starting a newer request supersedes and aborts that owner's prior request.
53+
Explicit cancellation settles as cancelled. Owner or coordinator disposal
54+
aborts outstanding work and settles it as unavailable/disposed. Provider
55+
rejections, throws, and malformed responses are contained; late settlements
56+
cannot publish or populate the cache.
57+
58+
The coordinator batches a request once, never once per relation. Cache hits and
59+
misses are composed deterministically while only misses are sent to the
60+
provider.
61+
62+
Provider-declared loading receives at most one automatic retry for the same
63+
document, context, and completion position. A persistent loading response is
64+
then returned without a refresh token; hosts resume it through catalog
65+
invalidation instead of an unbounded polling loop.

0 commit comments

Comments
 (0)