Skip to content

Commit 88f9c6f

Browse files
authored
docs(vnext): accept parser-independent relation completion (#182)
## Summary - accept ADR 0005 for parser-independent relation completion - keep the first completion slice independent of parser acceptance and parser-worker startup - define a bounded partial-`SELECT` query-site recognizer and narrow CTE visibility model - define one provider-owned catalog boundary, canonical suffix rendering, epochs, cache identity, deadlines, cancellation, and deterministic composition - define atomic text/context/embedded-region transactions and service-originated revision notifications - define CodeMirror ownership, rich-info decoration, loading refresh intent, and marimo 1/10/50-view acceptance evidence - keep provider/completion declarations provisional until the vertical slice, two provider shapes, packed marimo integration, and hostile lifecycle suites pass ## Key decisions - incomplete relation completion does not depend on `node-sql-parser` - catalog matching/addressability belongs to the provider and scope; dialect runtime owns SQL decoding and role-aware rendering - catalog edits replace the authenticated whole typed path with a provider-proven canonical suffix - arbitrary embedded regions are grammar barriers; recovered sites remain explicitly heuristic and incomplete - one atomic shared-work owner set covers request consumers and refresh observers - hard safety deadlines are separate from the 40 ms default interactive catalog response budget - terminal and soft loading results carry bounded one-shot completion-intent leases - higher epochs discovered by responses or invalidations preserve active/menu/no-menu completion intent exactly once - completion search misses never prove unknown-object diagnostics; authoritative resolution is separate ## Validation - patch rebased onto merged executor checkpoint `873c926` - all TypeScript configurations pass - oxlint passes with zero findings - test integrity passes - 1,248 Node tests pass plus 1 expected negative - three specialist adversarial review loops covered SQL/API correctness, marimo ergonomics, and concurrency/performance - final exact-head approvals are recorded for `d66ea25` ## Follow-up The first implementation change is the runtime-free marimo-shaped type fixture. No production completion slice starts until that fixture compiles. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Accepts ADR 0005 to provide parser‑independent relation completion with clear catalog/provider contracts, CTE visibility, and deterministic lifecycle. Completion now works for incomplete FROM/JOIN without waiting for `node-sql-parser` or worker startup, and keeps `CodeMirror` integration separate. - New Features - Decoupled relation completion from parser/worker; added bounded partial-SELECT site and CTE-visibility recognizers. - Defined catalog provider boundary: scope/search paths in; canonical paths with `completionPathStart`, epochs, and coverage out. - Added atomic text/context/embedded-region transactions and `onDidChange` session notifications for service-originated revisions. - Clarified dialect-owned identifier decoding/quoting and role-aware rendering; no generic folding/dedup in the service. - Specified deterministic ranking, latest-wins cancellation, deadlines (queue/exec), cache identity, and one-shot refresh intent. - Documented `CodeMirror` adapter boundary, safe coalesced refresh on `catalog-availability`, and no worker construction for this slice. - Migration - Pass a complete embedded-region set on open/update; regions, text, and context update atomically. - Implement the new catalog provider contract (or use `createInMemoryRelationCatalog`); do not emit SQL text—return decoded paths and roles. - Do not gate completion on parser readiness; keep the parser worker and scoped semantics for future slices. - Subscribe to `session.onDidChange` and coalesce refresh on `catalog-availability`; respect the provided completion-intent lease. <sup>Written for commit d66ea25. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/182?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
1 parent 873c926 commit 88f9c6f

5 files changed

Lines changed: 862 additions & 52 deletions

docs/adr/0001-language-service-and-session.md

Lines changed: 70 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ debounce, dispatch, and session disposal.
3737

3838
Shared providers contain no request-local mutable state.
3939

40-
## Stable introductory shape
40+
## Target introductory shape
41+
42+
The service/session lifecycle in this section is stable direction. Catalog,
43+
search-path, embedded-region, completion, and service-originated notification
44+
spellings remain provisional until ADR 0005's stabilization gate.
4145

4246
The target consumer shape is:
4347

@@ -64,6 +68,7 @@ const service = createSqlLanguageService<MarimoSqlContext>({
6468

6569
const session = service.openDocument({
6670
text: "SELECT * FROM users",
71+
embeddedRegions: [],
6772
context: {
6873
dialect: duckdb.id,
6974
engine: "__marimo__",
@@ -104,11 +109,19 @@ interface SqlDocumentContext {
104109
readonly dialect: SqlDialectId;
105110
readonly catalog?: {
106111
readonly scope: string;
107-
readonly searchPath?: readonly string[];
112+
readonly searchPath?: readonly (readonly {
113+
readonly value: string;
114+
readonly quoted: boolean;
115+
}[])[];
108116
};
109117
}
110118
```
111119

120+
Search paths are ordered component paths rather than dot-joined strings.
121+
Consequently a quoted identifier containing a dot is never confused with two
122+
path components. ADR 0005 keeps the exact public spelling provisional until
123+
the catalog vertical slice and consumer fixtures pass.
124+
112125
The service resolves dialect IDs through its immutable registry and rejects
113126
unknown or duplicate IDs. Duplicate registration is rejected even when the
114127
definitions appear equivalent. The built-in dialect factories return frozen
@@ -130,28 +143,38 @@ complete operation without mutation. The accepted snapshot is recursively
130143
frozen and is the only context observed by providers. Caller mutation therefore
131144
cannot change an in-flight request. Providers never read ambient editor state.
132145

133-
The session updates text and context atomically:
146+
The session updates text, context, and embedded regions atomically:
134147

135148
```ts
136149
session.update({
137-
kind: "document",
138150
baseRevision: session.revision,
139151
document: {
140152
kind: "changes",
141153
changes: [{ from: 14, to: 19, insert: "customers" }],
142154
},
143155
context: nextContext,
156+
embeddedRegions: nextEmbeddedRegions,
144157
});
145158
```
146159

160+
A transaction contains `baseRevision` and any non-empty subset of `document`,
161+
`context`, and `embeddedRegions`. A document mutation requires the complete
162+
region set for its resulting text. Without a document mutation, an explicit
163+
complete region set can change alone or together with context; omission
164+
preserves the current regions. An explicit empty set clears them. Opening a
165+
document accepts the same optional complete set, with omission or an empty set
166+
meaning identity source.
167+
147168
A full-text replacement is available for simple consumers. Incremental changes
148169
are ordered, non-overlapping, absolute UTF-16 ranges in the current document.
149-
The service validates the base revision and every range before mutating state.
170+
The service validates the base revision, every edit, context, and the complete
171+
resulting region set before mutating state. The transaction either commits one
172+
revision or changes nothing; callers never need a fake no-op document
173+
replacement to change template metadata.
150174

151-
A context-only update is valid and also requires `baseRevision`. Accepted
152-
updates always create a new revision, including an `A → B → A` text cycle, so a
153-
previous result never becomes current again. A stale base rejects the complete
154-
update without partial mutation.
175+
Accepted updates always create a new revision, including an `A → B → A` text
176+
cycle, so a previous result never becomes current again. A stale base rejects
177+
the complete update without partial mutation.
155178

156179
## Revision and applicability
157180

@@ -311,11 +334,26 @@ interface SqlDocumentSession<Context extends SqlDocumentContext> {
311334
request?: SqlFormatRequest,
312335
) => Promise<SqlRequestResult<readonly SqlTextEdit[]>>;
313336

337+
readonly onDidChange: (
338+
listener: (event: {
339+
readonly revision: SqlRevision;
340+
readonly reason:
341+
| "catalog"
342+
| "catalog-availability"
343+
| "provider-configuration";
344+
}) => void,
345+
) => { readonly dispose: () => void };
346+
314347
readonly isCurrent: (revision: SqlRevision) => boolean;
315348
readonly dispose: () => void;
316349
}
317350
```
318351

352+
`onDidChange` reports service-originated revision advances that do not come
353+
from the host's own document transaction. State changes before notification;
354+
listener failures are isolated; and both subscription and session disposal are
355+
idempotent. ADR 0005 specifies the first catalog use.
356+
319357
The walking skeleton exposes only implemented features. Adding a method is
320358
backward-compatible; returning placeholder success from an unimplemented method
321359
is not.
@@ -325,13 +363,13 @@ is not.
325363
Providers are narrow and async-only. Pure range, change, identifier, and
326364
statement-index primitives remain synchronous.
327365

328-
Provider requests contain:
329-
330-
- Immutable source snapshot with distinctly named `originalText`,
331-
`analysisText`, and source mapping
332-
- Explicit document context
333-
- Provider configuration identity
334-
- `AbortSignal`
366+
Each provider receives only the minimum feature-specific immutable projection.
367+
Document-oriented validators may receive a distinctly named `originalText` or
368+
`analysisText`, source mapping, and their declared context projection. Catalog
369+
search receives only the catalog/query projection in ADR 0005. No provider
370+
automatically receives the full source snapshot or arbitrary host context.
371+
Provider configuration identity remains service-owned, and `AbortSignal` is
372+
passed separately.
335373

336374
Each validator declares both `granularity: "document" | "statement"` and
337375
`input: "original" | "analysis"`. Regardless of input form, returned public
@@ -353,7 +391,8 @@ validates ranges and result limits before arbitration.
353391

354392
Authority and arbitration are feature-specific:
355393

356-
- Completion merges, deduplicates, and deterministically ranks.
394+
- Completion composes and deterministically ranks under its declared identity
395+
rules; it never applies a generic catalog fold or deduplication policy.
357396
- Diagnostics use coverage and authority; they are not blindly unioned.
358397
- Hover can select or attribute sections.
359398
- Definitions use precedence, confidence, and provenance.
@@ -363,15 +402,12 @@ Provider completion order cannot affect final ordering.
363402

364403
## Parser-neutral artifacts
365404

366-
The walking skeleton normalizes only evidence needed by its first vertical
367-
slice:
368-
369-
- Statement ranges and state
370-
- Tokens
371-
- Diagnostics
372-
- Normalized relation references
373-
- Cursor-context facts
374-
- Exact completion replacement range
405+
The parser walking skeleton normalizes only bounded syntax evidence it can
406+
prove, initially statement kind, syntax acceptance/rejection, authenticated
407+
source locations, and explicit availability/limitations. Parser-independent
408+
statement indexing and relation completion keep their own smaller artifacts.
409+
The parser does not manufacture relation references, cursor-context facts, or
410+
completion ranges for ADR 0005.
375411

376412
It does not expose `ast?: unknown` or attempt a universal SQL AST.
377413

@@ -387,9 +423,10 @@ Catalog access is lazy and bounded. Providers search and resolve within an
387423
explicit scope, prefix, object-kind set, and service-clamped result limit.
388424
Responses carry stable entity IDs, provider epoch, pagination, and coverage.
389425

390-
Empty-complete and empty-loading are distinct. A miss supports a definite
391-
unknown-object diagnostic only when the response proves complete coverage for
392-
the searched scope.
426+
Empty-complete and empty-loading are distinct. A complete-empty completion
427+
search proves only that the exact search produced no candidates. Only a
428+
separate authoritative resolution contract with explicit complete resolution
429+
coverage may support a definite unknown-object diagnostic.
393430

394431
Catalog subscriptions are scoped and disposable. An invalidation identifies
395432
provider ID, affected scope, and:
@@ -421,9 +458,10 @@ coverage.
421458

422459
The initial internal source primitive implements identity snapshots and this
423460
length-preserving masking only. It does not expose a transformer or source-map
424-
SPI. Sessions create a new source snapshot for document updates and reuse the
425-
existing snapshot for context-only updates. Non-identity generated/reordered
426-
source remains deferred until a concrete consumer validates the segment model.
461+
SPI. Sessions create a new source snapshot from the complete post-update region
462+
set for document or region updates and reuse the existing snapshot only when
463+
text and regions are unchanged. Non-identity generated/reordered source remains
464+
deferred until a concrete consumer validates the segment model.
427465

428466
Current session edits use identity sources, so validated original-document
429467
changes are also trusted analysis-coordinate changes. A future transformed

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,16 @@ settles the active operation exactly once without exposing raw event data.
183183
Raw backend ASTs will not cross the worker boundary and the first protocol will
184184
not introduce remote AST handles or worker-local AST leases.
185185

186-
Before production session wiring, the worker request will parse once and run
187-
adapter-owned semantic decoders in the same realm. It will return only the
188-
bounded, validated relation facts required by the first completion slice.
189-
This keeps backend shapes private, avoids reparsing once for syntax and again
190-
for relations, and makes cached main-realm evidence measurable.
186+
The first relation-completion slice is parser-independent under
187+
[ADR 0005](0005-parser-independent-relation-completion.md). Protocol v1 remains
188+
syntax-only. Incomplete relation completion must not wait for worker startup,
189+
parser acceptance, timeout, or recovery.
190+
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.
191196

192197
Worker-local AST caching is deferred until profiling demonstrates that
193198
reparsing is material enough to justify leases, byte accounting, generation
@@ -220,7 +225,7 @@ prove:
220225
recorded.
221226
- Raw and gzip worker sizes are recorded.
222227

223-
The semantic and session-integration slices additionally require:
228+
Parser session-integration and future semantic slices additionally require:
224229

225230
- Main-thread long-task and event-loop responsiveness evidence.
226231
- Malformed message, crash, timeout, late-event, and restart tests.
@@ -290,10 +295,11 @@ optional-integration bundle budget.
290295
1. Add this ADR and the packed-consumer browser placement harness.
291296
2. Extract a realm-neutral backend engine and add strict protocol codecs.
292297
3. Add the minimal browser worker and private bounded single-lane executor.
293-
4. Add in-worker normalized relation extraction.
294-
5. Add the pure statement coordinator, bounded cache, in-flight sharing, and
295-
atomic session ownership.
296-
6. Ship relation completion as the first public consuming vertical slice.
298+
4. Add the pure statement coordinator, bounded cache, in-flight sharing, and
299+
authenticated syntax integration.
300+
5. Ship parser-independent relation completion under ADR 0005.
301+
6. Design in-worker scoped semantics and protocol v2 before a scope-dependent
302+
feature consumes parser evidence.
297303

298304
Every production step is a medium change and receives two independent,
299305
commit-bound adversarial reviews.

0 commit comments

Comments
 (0)