Skip to content

Commit 1ca5751

Browse files
authored
feat(vnext): make embedded regions atomic session inputs (#184)
## Summary - make embedded regions a public, atomic input to document open and update transactions - require every text mutation to carry the complete post-edit region set while preserving omission and explicit-clear semantics for non-document updates - mask embedded regions before statement analysis, with separate source sequencing and conservative cache reuse - align runtime decoding with TypeScript structural typing by copying only declared own data fields and leaving host metadata opaque - migrate the provisional completion contract and packed consumers to the real session transaction types ## Correctness and consumer behavior - text, context, dialect, regions, revision, sequences, and statement cache commit or roll back together - stale revisions are rejected before candidate payload inspection; disposal dominates hostile/reentrant failures - region ranges use resulting-document UTF-16 coordinates and include complete non-SQL delimiters such as marimo's `{df}` - source snapshots and normalized regions are owned and frozen; same-value source transactions still advance revision/source sequence - incremental statement indexing remains limited to identity-to-identity edits; changed masked analysis rebuilds lazily - structurally richer marimo region/envelope values compile and run without invoking or retaining extra host fields - removed outer `kind` and contradictory document fields remain explicitly forbidden in both types and runtime ## Validation - `pnpm run typecheck` - `pnpm exec oxlint` - `pnpm run test:integrity` - `pnpm test` — 1,264 passed, 1 expected failure - `pnpm run test:browser` — 7 passed - `pnpm run test:package` with typed and runtime nonempty embedded regions - `pnpm run test:worker-placement` - `pnpm run demo` - changed coverage — 98.70% statements, 97.42% branches, 100% functions, 98.69% lines - pre-rebase review/fix/re-review loop across SQL/API, marimo ergonomics, and concurrency/performance perspectives - fresh exact-head approval from all three reviewers at `ff35130197fcf69a10d42d195c8dd6324f6d5ed9` Part of #169. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Make embedded regions a first-class, atomic session input in vNext. Document edits now require the complete post-edit region set, masking happens before analysis, and cache reuse is tied to a separate source sequence. Part of #169. - **New Features** - Embedded regions are accepted on open and update; document mutations must include the full resulting `embeddedRegions`. Region-only and context-only updates are supported. - Region ranges use resulting-document UTF-16 and include full non-SQL delimiters (e.g., marimo `{df}`); masking is length-preserving and runs before statement analysis. - Unchanged text+regions reuse the immutable source and statement index via a separate source sequence; only identity-to-identity edits reuse incrementally. Changed masked analysis invalidates the cache unless analysis text is equal. - Inputs are structural: only declared own data fields are read; host metadata and extras stay opaque. Context accessors and non-enumerable properties are rejected. Stale revisions are rejected before payload inspection; disposal dominates hostile/reentrant failures. - Optional `document`/`embeddedRegions`/`context` fields: `undefined` is treated as omission at runtime for compatibility with and without `exactOptionalPropertyTypes`. - **Migration** - Update `session.update` calls: - Context-only: `{ baseRevision, context }` (no `kind`; `context` must be defined by types). - Document mutation: `{ baseRevision, document: {...}, embeddedRegions: [...] }` (regions required). - Region-only: `{ baseRevision, embeddedRegions: [...] }`. - Open with regions: `service.openDocument({ text, context, embeddedRegions })`. - Use `SqlEmbeddedRegion` and `SqlDocumentUpdate` from `@marimo-team/codemirror-sql/vnext`; stop importing `SqlEmbeddedRegion` from `./source`. <sup>Written for commit db6eeee. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/184?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 1ee9a05 commit 1ca5751

15 files changed

Lines changed: 991 additions & 317 deletions

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,9 @@ region set for its resulting text. Without a document mutation, an explicit
163163
complete region set can change alone or together with context; omission
164164
preserves the current regions. An explicit empty set clears them. Opening a
165165
document accepts the same optional complete set, with omission or an empty set
166-
meaning identity source.
166+
meaning identity source. For compatibility with consumers regardless of their
167+
`exactOptionalPropertyTypes` setting, `undefined` on optional state fields has
168+
the same meaning as omission.
167169

168170
A full-text replacement is available for simple consumers. Incremental changes
169171
are ordered, non-overlapping, absolute UTF-16 ranges in the current document.
@@ -463,11 +465,12 @@ set for document or region updates and reuse the existing snapshot only when
463465
text and regions are unchanged. Non-identity generated/reordered source remains
464466
deferred until a concrete consumer validates the segment model.
465467

466-
Current session edits use identity sources, so validated original-document
467-
changes are also trusted analysis-coordinate changes. A future transformed
468-
source must provide its own validated analysis-coordinate changes for
469-
incremental statement indexing. Without them, changed analysis text
470-
invalidates the index and is rebuilt lazily.
468+
Identity-to-identity session edits can use validated original-document changes
469+
as trusted analysis-coordinate changes. Masked-source edits do not make that
470+
claim: unchanged analysis can reuse its index, while changed analysis
471+
invalidates the index and rebuilds lazily. A future transformed source must
472+
provide its own validated analysis-coordinate changes before incremental
473+
indexing can consume them.
471474

472475
Edits crossing an ambiguous or unmapped boundary are rejected. Mapping failure
473476
is unavailable analysis, not invalid SQL.

docs/vnext/session-primitives.md

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ const session = service.openDocument({
3535
});
3636

3737
const revision = session.update({
38-
kind: "document",
3938
baseRevision: session.revision,
4039
document: {
4140
kind: "changes",
4241
changes: [{ from: 14, to: 19, insert: "customers" }],
4342
},
43+
embeddedRegions: [],
4444
});
4545

4646
if (session.isCurrent(revision)) {
@@ -52,7 +52,9 @@ service.dispose();
5252
```
5353

5454
Use `{ kind: "replace", text }` for full replacement and
55-
`{ kind: "context", baseRevision, context }` for a context-only update.
55+
`{ baseRevision, context }` for a context-only update. Every document mutation
56+
also supplies the complete embedded-region set for its resulting text. A
57+
region-only transaction can replace or clear that set without a fake text edit.
5658

5759
## Dialect registration
5860

@@ -74,7 +76,12 @@ does not itself infer lexical behavior.
7476
- `baseRevision` is checked by identity and cannot come from another session.
7577
- Incremental ranges are ordered, non-overlapping, half-open UTF-16 offsets in
7678
the pre-update document.
77-
- Text and context are validated completely before the session snapshot changes.
79+
- Text, context, and embedded regions are validated completely before the
80+
session snapshot changes.
81+
- Public envelopes, edits, changes, and regions are structural inputs. The
82+
service copies only their declared own data fields; additional host metadata
83+
remains opaque and is never invoked. Removed or contradictory discriminants
84+
are explicitly forbidden rather than treating every object as exact.
7885
- Context is structured-cloned and recursively frozen. Accepted values are
7986
finite primitives, arrays, and string-keyed plain objects. Cycles and shared
8087
references are retained. Accessors, symbols, functions, class instances,
@@ -86,13 +93,15 @@ does not itself infer lexical behavior.
8693
platform errors are not exposed as causes.
8794

8895
The internal statement index is built lazily per session. Context-only updates
89-
reuse it when the lexical-profile identity is unchanged. A no-op document
90-
update or same-text replacement advances the public revision and document
91-
sequence but can reuse the index. Trusted incremental identity-source changes
92-
update a populated cache; a changed replacement, profile change, or changed
93-
transformed source without analysis-coordinate changes clears it for a later
94-
full build. Invalid updates leave both the session snapshot and cache
95-
unchanged, and disposal clears the cache.
96+
reuse it when the lexical-profile identity is unchanged. Source-changing
97+
transactions use a separate source sequence, so region-only changes cannot
98+
reuse a stale index. A no-op document update or same-text replacement advances
99+
the public revision and document sequence but can reuse the index. Trusted
100+
incremental identity-source changes update a populated cache; a changed
101+
replacement, profile change, or changed masked source without verified
102+
analysis-coordinate changes clears it for a later full build. Invalid updates
103+
leave both the session snapshot and cache unchanged, and disposal clears the
104+
cache.
96105

97106
The safety envelope currently permits at most 10,000 changes in one update, a
98107
16 Mi-code-unit document, 1,000 registered dialects, and bounded context graph

docs/vnext/source-coordinates.md

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ are valid. `SqlTextChange` extends this shape with `insert` and continues to use
1717
pre-update document coordinates.
1818

1919
The service internally owns an immutable source snapshot with separately named
20-
`originalText` and `analysisText`. A document update creates a new identity
21-
snapshot; a context-only update reuses the same source object. This separation
22-
allows later analysis transforms without making providers depend on editor
23-
state or ambiguous coordinate spaces.
20+
`originalText` and `analysisText`. A source transaction builds the complete
21+
post-update masked snapshot; a context-only update reuses the same source
22+
object. This separation keeps providers independent of editor state and
23+
ambiguous coordinate spaces.
2424

2525
## Length-preserving masking
2626

@@ -35,29 +35,52 @@ The first internal transform masks explicit embedded regions:
3535
- `analysisText.length` always equals `originalText.length`, so range mapping is
3636
identity-preserving while still returning a fresh validated range.
3737

38+
`SqlEmbeddedRegion` is the public document-coordinate input:
39+
40+
```ts
41+
const session = service.openDocument({
42+
text: "SELECT * FROM {df}",
43+
context,
44+
embeddedRegions: [{ from: 14, to: 18, language: "python" }],
45+
});
46+
47+
session.update({
48+
baseRevision: session.revision,
49+
document: {
50+
kind: "changes",
51+
changes: [{ from: 15, to: 17, insert: "next_df" }],
52+
},
53+
embeddedRegions: [{ from: 14, to: 23, language: "python" }],
54+
});
55+
```
56+
57+
The half-open interval covers the complete non-SQL fragment, including its
58+
template delimiters. For marimo, `[14, 18)` masks all of `{df}`; masking only
59+
`df` would leave `{}` to be analyzed as SQL. A document transaction always
60+
supplies the complete region set in coordinates of the resulting text.
61+
3862
At most 10,000 regions and 16 Mi UTF-16 code units are accepted. Masking uses
3963
bounded 64 Ki-code-unit chunks, including for newline-dense input, rather than
4064
a per-code-unit or per-newline array.
4165

42-
The source snapshot, embedded-region model, masking factory, and mapping
43-
functions remain internal. This slice intentionally does not publish a source
44-
transformer or source-map SPI. Generated or reordered source requires a
45-
versioned segment-map design and evidence from real consumers before becoming
46-
public.
66+
The source snapshot, masking factory, and mapping functions remain internal.
67+
Only the document-facing `SqlEmbeddedRegion` input shape is public. This slice
68+
intentionally does not publish a source transformer or source-map SPI.
69+
Generated or reordered source requires a versioned segment-map design and
70+
evidence from real consumers before becoming public.
4771

4872
The internal [statement index](./statement-index.md) scans `analysisText` and
4973
uses its length-preserving offsets without publishing analysis-coordinate
5074
ranges.
5175

5276
Statement-index reuse depends on `analysisText` value and internal lexical
53-
profile identity, not source-object identity. A document update therefore
54-
still creates a fresh source snapshot and public revision even when equal
55-
analysis text permits reuse. The index does not retain either the old or
56-
current source text.
57-
58-
Current sessions create identity sources, so their validated document changes
59-
are also trusted analysis-coordinate changes. The masking primitive is not yet
60-
attached to session updates. A future transformed-source pipeline must produce
61-
validated analysis-coordinate changes before it can use incremental indexing;
62-
otherwise a changed analysis document invalidates the cache and receives a
63-
fresh full build on demand.
77+
profile identity, not source-object identity. Every accepted source transaction
78+
creates a public revision, while an unchanged text-and-region value may reuse
79+
the immutable source snapshot and index. The index does not retain either the
80+
old or current source text.
81+
82+
Sessions accept complete length-preserving embedded-region sets on open and
83+
source transactions. Original document changes are trusted as analysis changes
84+
only for identity-to-identity updates. A changed masked source invalidates the
85+
cache unless its analysis text is unchanged; it then receives a fresh full
86+
build on demand.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@
1313
},
1414
"scripts": {
1515
"dev": "vite",
16-
"typecheck": "pnpm run typecheck:src && pnpm run typecheck:tests && pnpm run typecheck:vnext-tests && pnpm run typecheck:demo",
16+
"typecheck": "pnpm run typecheck:src && pnpm run typecheck:tests && pnpm run typecheck:vnext-tests && pnpm run typecheck:vnext-tests:loose-optional && pnpm run typecheck:demo",
1717
"typecheck:src": "tsc --project tsconfig.json --noEmit",
1818
"typecheck:tests": "tsc --project tsconfig.tests.json",
1919
"typecheck:vnext-tests": "tsc --project tsconfig.vnext-tests.json",
20+
"typecheck:vnext-tests:loose-optional": "tsc --project tsconfig.vnext-tests.loose-optional.json",
2021
"typecheck:demo": "tsc --project tsconfig.demo.json",
2122
"lint": "oxlint --fix",
2223
"test": "vitest run --config vitest.config.ts",

scripts/package-smoke.mjs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,13 @@ const service = createSqlLanguageService({
199199
});
200200
const session = service.openDocument({
201201
context: { dialect: "duckdb" },
202-
text: "SELECT 1",
202+
embeddedRegions: [{ from: 14, language: "python", to: 18 }],
203+
text: "SELECT * FROM {df}",
203204
});
204205
session.update({
205-
kind: "document",
206+
embeddedRegions: [{ from: 14, language: "python", to: 23 }],
206207
baseRevision: session.revision,
207-
document: { kind: "replace", text: "SELECT 2" },
208+
document: { kind: "replace", text: "SELECT * FROM {next_df}" },
208209
});
209210
service.dispose();
210211
`,
@@ -227,6 +228,7 @@ import {
227228
createSqlLanguageService,
228229
duckdbDialect,
229230
type SqlDocumentContext,
231+
type SqlEmbeddedRegion,
230232
type SqlTextRange,
231233
} from "@marimo-team/codemirror-sql/vnext";
232234
import commonKeywords from "@marimo-team/codemirror-sql/data/common-keywords.json" with { type: "json" };
@@ -244,15 +246,19 @@ const parser = new NodeSqlParser();
244246
const service = createSqlLanguageService<HostContext>({
245247
dialects: [duckdbDialect()],
246248
});
249+
const embeddedRegions: readonly SqlEmbeddedRegion[] = [
250+
{ from: 14, language: "python", to: 18 },
251+
];
247252
const session = service.openDocument({
248253
context: { dialect: "duckdb", engine: "local" },
249-
text: "SELECT 1",
254+
embeddedRegions,
255+
text: "SELECT * FROM {df}",
250256
});
251257
const range: SqlTextRange = { from: 0, to: 6 };
252258
session.update({
253-
kind: "document",
259+
embeddedRegions: [{ from: 14, language: "python", to: 18 }],
254260
baseRevision: session.revision,
255-
document: { kind: "changes", changes: [{ from: 7, insert: "2", to: 8 }] },
261+
document: { kind: "changes", changes: [] },
256262
});
257263
258264
void extensions;
@@ -305,13 +311,14 @@ const service = vnext.createSqlLanguageService({
305311
});
306312
const session = service.openDocument({
307313
context: { dialect: "duckdb" },
308-
text: "SELECT 1",
314+
embeddedRegions: [{ from: 14, language: "python", to: 18 }],
315+
text: "SELECT * FROM {df}",
309316
});
310317
const originalRevision = session.revision;
311318
const updatedRevision = session.update({
312-
kind: "document",
319+
embeddedRegions: [{ from: 14, language: "python", to: 23 }],
313320
baseRevision: originalRevision,
314-
document: { kind: "replace", text: "SELECT 2" },
321+
document: { kind: "replace", text: "SELECT * FROM {next_df}" },
315322
});
316323
if (session.isCurrent(originalRevision) || !session.isCurrent(updatedRevision)) {
317324
throw new Error("The packaged vNext session violated revision identity");

0 commit comments

Comments
 (0)