From 30716900d8166c5cc470e70a4a5308269563c4a0 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 17:40:17 -0700 Subject: [PATCH 1/7] fix(document-api): unify insert target/ref handling and restore CLI offset compat --- apps/cli/src/__tests__/cli.test.ts | 54 ++++++-- apps/cli/src/__tests__/conformance/harness.ts | 11 +- .../src/__tests__/conformance/scenarios.ts | 10 +- apps/cli/src/cli/operation-params.ts | 15 ++- apps/cli/src/lib/invoke-input.ts | 37 ++--- apps/cli/src/lib/operation-args.ts | 4 +- .../reference/_generated-manifest.json | 2 +- apps/docs/document-api/reference/index.mdx | 2 +- apps/docs/document-api/reference/insert.mdx | 127 +++++++++++++----- apps/docs/document-engine/sdks.mdx | 4 +- packages/document-api/src/README.md | 14 +- .../src/contract/contract.test.ts | 36 ++++- .../src/contract/operation-definitions.ts | 8 +- packages/document-api/src/contract/schemas.ts | 42 +++++- packages/document-api/src/delete/delete.ts | 20 ++- packages/document-api/src/format/format.ts | 41 ++---- packages/document-api/src/index.test.ts | 49 ++++--- packages/document-api/src/index.ts | 10 +- packages/document-api/src/insert/insert.ts | 106 ++++++++++----- .../src/overview-examples.test.ts | 6 +- packages/document-api/src/replace/replace.ts | 19 +-- .../document-api/src/selection-mutation.ts | 32 +++-- .../src/types/mutation-plan.types.ts | 7 +- packages/document-api/src/types/query.ts | 2 +- .../src/types/structural-input.ts | 23 +++- packages/document-api/src/write/write.ts | 16 +-- packages/sdk/langs/node/README.md | 2 +- .../helpers/adapter-utils.ts | 73 ++++------ .../helpers/deterministic-node-id.test.ts | 64 +++++++++ ...le-node-id.ts => deterministic-node-id.ts} | 29 ++-- .../helpers/node-address-resolver.test.ts | 52 ++++++- .../helpers/node-address-resolver.ts | 14 +- .../helpers/selection-target-resolver.ts | 38 +++++- .../helpers/table-node-id.test.ts | 29 ---- .../plan-engine/compiler.ts | 13 +- .../insert-structured-wrapper.test.ts | 8 +- .../plan-engine/plan-wrappers.ts | 124 ++++++++++------- .../structural-write-engine.test.ts | 5 +- .../document-api-adapters/write-adapter.ts | 63 ++++++--- 39 files changed, 791 insertions(+), 420 deletions(-) create mode 100644 packages/super-editor/src/document-api-adapters/helpers/deterministic-node-id.test.ts rename packages/super-editor/src/document-api-adapters/helpers/{table-node-id.ts => deterministic-node-id.ts} (59%) delete mode 100644 packages/super-editor/src/document-api-adapters/helpers/table-node-id.test.ts diff --git a/apps/cli/src/__tests__/cli.test.ts b/apps/cli/src/__tests__/cli.test.ts index 3c361d4527..2718736fc8 100644 --- a/apps/cli/src/__tests__/cli.test.ts +++ b/apps/cli/src/__tests__/cli.test.ts @@ -1028,7 +1028,42 @@ describe('superdoc CLI', () => { expect(verifyEnvelope.data.result.total).toBeGreaterThan(0); }); - test('insert with --block-id and --offset targets a specific block position', async () => { + test('insert with --block-id and --offset targets a specific block position (legacy compat)', async () => { + const insertSource = join(TEST_DIR, 'insert-blockid-legacy-offset-source.docx'); + const insertOut = join(TEST_DIR, 'insert-blockid-legacy-offset-out.docx'); + await copyFile(SAMPLE_DOC, insertSource); + + const target = await firstTextRange(['find', insertSource, '--type', 'text', '--pattern', 'Wilde']); + + const insertResult = await runCli([ + 'insert', + insertSource, + '--block-id', + target.blockId, + '--offset', + '0', + '--value', + 'CLI_BLOCKID_LEGACY_OFFSET_INSERT', + '--out', + insertOut, + ]); + + expect(insertResult.code).toBe(0); + + const verifyResult = await runCli([ + 'find', + insertOut, + '--type', + 'text', + '--pattern', + 'CLI_BLOCKID_LEGACY_OFFSET_INSERT', + ]); + expect(verifyResult.code).toBe(0); + const verifyEnvelope = parseJsonOutput>(verifyResult); + expect(verifyEnvelope.data.result.total).toBeGreaterThan(0); + }); + + test('insert with --block-id and --start/--end targets a specific block position', async () => { const insertSource = join(TEST_DIR, 'insert-blockid-offset-source.docx'); const insertOut = join(TEST_DIR, 'insert-blockid-offset-out.docx'); await copyFile(SAMPLE_DOC, insertSource); @@ -1041,7 +1076,9 @@ describe('superdoc CLI', () => { insertSource, '--block-id', target.blockId, - '--offset', + '--start', + '0', + '--end', '0', '--value', 'CLI_BLOCKID_OFFSET_INSERT_1597', @@ -1064,7 +1101,7 @@ describe('superdoc CLI', () => { expect(verifyEnvelope.data.result.total).toBeGreaterThan(0); }); - test('insert with --block-id alone defaults offset to 0', async () => { + test('insert with --block-id alone defaults start/end to 0', async () => { const insertSource = join(TEST_DIR, 'insert-blockid-only-source.docx'); const insertOut = join(TEST_DIR, 'insert-blockid-only-out.docx'); await copyFile(SAMPLE_DOC, insertSource); @@ -1093,15 +1130,19 @@ describe('superdoc CLI', () => { expect(resolvedTarget?.range.end).toBe(0); }); - test('insert with --offset but no --block-id returns INVALID_ARGUMENT', async () => { + test('insert with --start but no --block-id returns validation error', async () => { const insertSource = join(TEST_DIR, 'insert-offset-no-blockid-source.docx'); const insertOut = join(TEST_DIR, 'insert-offset-no-blockid-out.docx'); await copyFile(SAMPLE_DOC, insertSource); + // --start/--end without --block-id are not normalized into a target. + // They pass through as unknown fields and are rejected by validation. const result = await runCli([ 'insert', insertSource, - '--offset', + '--start', + '5', + '--end', '5', '--value', 'should-fail', @@ -1110,9 +1151,6 @@ describe('superdoc CLI', () => { ]); expect(result.code).toBe(1); - const envelope = parseJsonOutput(result); - expect(envelope.error.code).toBe('INVALID_ARGUMENT'); - expect(envelope.error.message).toContain('Unknown field'); }); test('insert with --type html inserts HTML content into the document', async () => { diff --git a/apps/cli/src/__tests__/conformance/harness.ts b/apps/cli/src/__tests__/conformance/harness.ts index 3a0cb384f0..4684dfcd63 100644 --- a/apps/cli/src/__tests__/conformance/harness.ts +++ b/apps/cli/src/__tests__/conformance/harness.ts @@ -375,9 +375,10 @@ export class ConformanceHarness { ): Promise<{ docPath: string; changeId: string; target: TextRangeAddress }> { const sourceDoc = await this.copyFixtureDoc(`${label}-source`); const target = await this.firstTextRange(sourceDoc, stateDir); - const collapsedTarget: TextRangeAddress = { - ...target, - range: { start: target.range.start, end: target.range.start }, + const selectionTarget = { + kind: 'selection', + start: { kind: 'text', blockId: target.blockId, offset: target.range.start }, + end: { kind: 'text', blockId: target.blockId, offset: target.range.start }, }; const outDoc = this.createOutputPath(`${label}-with-tracked-change`); @@ -386,7 +387,7 @@ export class ConformanceHarness { 'insert', sourceDoc, '--target-json', - JSON.stringify(collapsedTarget), + JSON.stringify(selectionTarget), '--value', 'TRACKED_CONFORMANCE_TOKEN', '--change-mode', @@ -412,7 +413,7 @@ export class ConformanceHarness { throw new Error(`Tracked-change fixture did not produce a tracked change id for ${label}`); } - return { docPath: outDoc, changeId, target: collapsedTarget }; + return { docPath: outDoc, changeId, target }; } async firstTwoBlockAddresses( diff --git a/apps/cli/src/__tests__/conformance/scenarios.ts b/apps/cli/src/__tests__/conformance/scenarios.ts index e4c38b72d3..4dcdad2a6d 100644 --- a/apps/cli/src/__tests__/conformance/scenarios.ts +++ b/apps/cli/src/__tests__/conformance/scenarios.ts @@ -2084,15 +2084,19 @@ export const SUCCESS_SCENARIOS = { 'doc.insert': async (harness: ConformanceHarness): Promise => { const stateDir = await harness.createStateDir('doc-insert-success'); const docPath = await harness.copyFixtureDoc('doc-insert'); - const target = await harness.firstTextRange(docPath, stateDir); - const collapsed = { ...target, range: { start: target.range.start, end: target.range.start } }; + const textRange = await harness.firstTextRange(docPath, stateDir); + const selectionTarget = { + kind: 'selection', + start: { kind: 'text', blockId: textRange.blockId, offset: textRange.range.start }, + end: { kind: 'text', blockId: textRange.blockId, offset: textRange.range.start }, + }; return { stateDir, args: [ 'insert', docPath, '--target-json', - JSON.stringify(collapsed), + JSON.stringify(selectionTarget), '--value', 'CONFORMANCE_INSERT', '--out', diff --git a/apps/cli/src/cli/operation-params.ts b/apps/cli/src/cli/operation-params.ts index 3e45246d84..64e66a6b5d 100644 --- a/apps/cli/src/cli/operation-params.ts +++ b/apps/cli/src/cli/operation-params.ts @@ -478,11 +478,6 @@ const TEXT_TARGET_FLAT_PARAMS: CliOperationParamSpec[] = [ { name: 'end', kind: 'flag', type: 'number', description: 'End offset within the block (character index).' }, ]; -const INSERT_FLAT_PARAMS: CliOperationParamSpec[] = [ - { name: 'blockId', kind: 'flag', flag: 'block-id', type: 'string', description: 'Block ID of the target paragraph.' }, - { name: 'offset', kind: 'flag', type: 'number', description: 'Character offset within the block for insertion.' }, -]; - const LIST_TARGET_FLAT_PARAMS: CliOperationParamSpec[] = [ { name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string', description: 'Node ID of the target list item.' }, ]; @@ -536,7 +531,15 @@ const EXTRA_CLI_PARAMS: Partial> = { }, ], // Text-range operations: flat flags (--block-id, --start, --end) as shortcuts for --target-json - 'doc.insert': [...INSERT_FLAT_PARAMS], + 'doc.insert': [ + ...TEXT_TARGET_FLAT_PARAMS, + { + name: 'offset', + kind: 'flag', + type: 'number', + description: 'Character offset for insertion (alias for --start/--end with same value).', + }, + ], 'doc.replace': [...TEXT_TARGET_FLAT_PARAMS], 'doc.delete': [...TEXT_TARGET_FLAT_PARAMS], 'doc.styles.apply': [ diff --git a/apps/cli/src/lib/invoke-input.ts b/apps/cli/src/lib/invoke-input.ts index c481588b79..4048b7e739 100644 --- a/apps/cli/src/lib/invoke-input.ts +++ b/apps/cli/src/lib/invoke-input.ts @@ -93,7 +93,12 @@ const FORMAT_TARGET_OPERATIONS = CLI_DOC_OPERATIONS.filter((operationId): operat * The CLI still supports legacy single-block text range flags/JSON inputs and * upgrades them to the equivalent SelectionTarget before dispatch. */ -const SELECTION_TARGET_OPERATIONS = new Set(['replace', 'delete', ...FORMAT_TARGET_OPERATIONS]); +const SELECTION_TARGET_OPERATIONS = new Set([ + 'insert', + 'replace', + 'delete', + ...FORMAT_TARGET_OPERATIONS, +]); /** * Operations that still accept a text-range target (textAddressSchema): @@ -104,11 +109,7 @@ const SELECTION_TARGET_OPERATIONS = new Set(['replace', ' */ const TEXT_ADDRESS_TARGET_OPERATIONS = new Set(['comments.create', 'comments.patch']); -/** - * Insert is a text-range operation but uses `offset` instead of `start`/`end` - * to specify a zero-width insertion point. - */ -const INSERT_OPERATION: CliExposedOperationId = 'insert'; +// INSERT_OPERATION removed — insert now uses SelectionTarget via SELECTION_TARGET_OPERATIONS. /** * List operations that accept a list-item target (listItemAddressSchema): @@ -196,14 +197,16 @@ function normalizeFlatTargetFlags(operationId: CliExposedOperationId, apiInput: return apiInput; } - // --- Selection-based text mutations (replace, delete, format.*) --- + // --- Selection-based text mutations (insert, replace, delete, format.*) --- if (SELECTION_TARGET_OPERATIONS.has(operationId)) { const blockId = apiInput.blockId; if (typeof blockId === 'string') { - const start = typeof apiInput.start === 'number' ? apiInput.start : 0; - const end = typeof apiInput.end === 'number' ? apiInput.end : 0; + // Legacy --offset for insert: expand to collapsed start/end + const hasOffset = typeof apiInput.offset === 'number'; + const start = typeof apiInput.start === 'number' ? apiInput.start : hasOffset ? (apiInput.offset as number) : 0; + const end = typeof apiInput.end === 'number' ? apiInput.end : hasOffset ? (apiInput.offset as number) : 0; assertLegacySelectionTargetSupported(operationId, { range: { start, end } }); - const { blockId: _, start: _s, end: _e, ...rest } = apiInput; + const { blockId: _, start: _s, end: _e, offset: _o, ...rest } = apiInput; return { ...rest, target: textAddressToSelectionTarget({ blockId, range: { start, end } }), @@ -227,20 +230,6 @@ function normalizeFlatTargetFlags(operationId: CliExposedOperationId, apiInput: return apiInput; } - // --- Insert operation (uses offset for zero-width insertion point) --- - if (operationId === INSERT_OPERATION) { - const blockId = apiInput.blockId; - if (typeof blockId === 'string') { - const offset = typeof apiInput.offset === 'number' ? apiInput.offset : 0; - const { blockId: _, offset: _o, ...rest } = apiInput; - return { - ...rest, - target: { kind: 'text', blockId, range: { start: offset, end: offset } }, - }; - } - return apiInput; - } - // --- Block delete (nodeType + nodeId → block target) --- if (operationId === 'blocks.delete') { const nodeType = apiInput.nodeType; diff --git a/apps/cli/src/lib/operation-args.ts b/apps/cli/src/lib/operation-args.ts index b7f89825dc..fd8c5286da 100644 --- a/apps/cli/src/lib/operation-args.ts +++ b/apps/cli/src/lib/operation-args.ts @@ -97,7 +97,9 @@ function acceptsLegacyTextAddressTarget( ): boolean { if (param.name !== 'target' || !isTextAddressLike(value)) return false; const docApiId = toDocApiId(operationId); - return docApiId === 'replace' || docApiId === 'delete' || docApiId?.startsWith('format.') === true; + return ( + docApiId === 'insert' || docApiId === 'replace' || docApiId === 'delete' || docApiId?.startsWith('format.') === true + ); } /** diff --git a/apps/docs/document-api/reference/_generated-manifest.json b/apps/docs/document-api/reference/_generated-manifest.json index 1afcaccb33..f7b08f0c09 100644 --- a/apps/docs/document-api/reference/_generated-manifest.json +++ b/apps/docs/document-api/reference/_generated-manifest.json @@ -986,5 +986,5 @@ } ], "marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}", - "sourceHash": "34852ca3a36fdbc4e68902541e89a9602b49b5031aacef23008f9c1f8d3f4677" + "sourceHash": "351babb1a7ce5b45286059e8f1931b2eac16a2bbf54209783a6c63d34f77aae1" } diff --git a/apps/docs/document-api/reference/index.mdx b/apps/docs/document-api/reference/index.mdx index c7c323919d..abc0d47cf3 100644 --- a/apps/docs/document-api/reference/index.mdx +++ b/apps/docs/document-api/reference/index.mdx @@ -70,7 +70,7 @@ The tables below are grouped by namespace. | markdownToFragment | editor.doc.markdownToFragment(...) | Convert a Markdown string into an SDM/1 structural fragment. | | info | editor.doc.info(...) | Return document summary info including word, character, paragraph, heading, table, image, comment, tracked-change, SDT-field, list, and page counts, plus outline and capabilities. | | clearContent | editor.doc.clearContent(...) | Clear all document body content, leaving a single empty paragraph. | -| insert | editor.doc.insert(...) | Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. | +| insert | editor.doc.insert(...) | Insert content into the document. Two input shapes: text-based (value + type) inserts inline content at a SelectionTarget or ref position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target/ref is omitted, content appends at the end of the document. Text mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. | | replace | editor.doc.replace(...) | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts a BlockNodeAddress (replaces whole block), SelectionTarget (expands to full covered block boundaries), or ref plus SDFragment content. | | delete | editor.doc.delete(...) | Delete content at a contiguous document selection. Accepts a SelectionTarget or mutation-ready ref. Supports cross-block deletion and optional block-edge expansion via behavior mode. | diff --git a/apps/docs/document-api/reference/insert.mdx b/apps/docs/document-api/reference/insert.mdx index ee1abf30e3..42559880e4 100644 --- a/apps/docs/document-api/reference/insert.mdx +++ b/apps/docs/document-api/reference/insert.mdx @@ -1,7 +1,7 @@ --- title: insert sidebarTitle: insert -description: "Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block." +description: "Insert content into the document. Two input shapes: text-based (value + type) inserts inline content at a SelectionTarget or ref position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target/ref is omitted, content appends at the end of the document. Text mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block." --- {/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */} @@ -10,7 +10,7 @@ description: "Insert content into the document. Two input shapes: legacy string- ## Summary -Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. +Insert content into the document. Two input shapes: text-based (value + type) inserts inline content at a SelectionTarget or ref position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target/ref is omitted, content appends at the end of the document. Text mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. - Operation ID: `insert` - API member path: `editor.doc.insert(...)` @@ -22,20 +22,33 @@ Insert content into the document. Two input shapes: legacy string-based (value + ## Expected result -Returns an SDMutationReceipt with applied status; resolution reports a TextAddress for legacy text insertion or a BlockNodeAddress for structural insertion. Receipt reports NO_OP if the insertion point is invalid or content is empty. +Returns an SDMutationReceipt with applied status; resolution reports the inserted TextAddress for text insertion or a BlockNodeAddress for structural insertion. Receipt reports NO_OP if the insertion point is invalid or content is empty. ## Input fields -### Variant 1 (target.kind="text") +### Variant 1.1 (target.kind="selection") + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `target` | SelectionTarget | yes | SelectionTarget | +| `target.end` | SelectionPoint | yes | SelectionPoint | +| `target.kind` | `"selection"` | yes | Constant: `"selection"` | +| `target.start` | SelectionPoint | yes | SelectionPoint | +| `type` | enum | no | `"text"`, `"markdown"`, `"html"` | +| `value` | string | yes | | + +### Variant 1.2 (required: ref, value) + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `ref` | string | yes | | +| `type` | enum | no | `"text"`, `"markdown"`, `"html"` | +| `value` | string | yes | | + +### Variant 1.3 (required: value) | Field | Type | Required | Description | | --- | --- | --- | --- | -| `target` | TextAddress | no | TextAddress | -| `target.blockId` | string | no | | -| `target.kind` | `"text"` | no | Constant: `"text"` | -| `target.range` | Range | no | Range | -| `target.range.end` | integer | no | | -| `target.range.start` | integer | no | | | `type` | enum | no | `"text"`, `"markdown"`, `"html"` | | `value` | string | yes | | @@ -175,30 +188,84 @@ Returns an SDMutationReceipt with applied status; resolution reports a TextAddre { "oneOf": [ { - "additionalProperties": false, - "properties": { - "target": { - "$ref": "#/$defs/TextAddress", - "description": "Insertion point: {kind:'text', blockId:'...', range:{start, end}}." + "oneOf": [ + { + "additionalProperties": false, + "properties": { + "target": { + "$ref": "#/$defs/SelectionTarget", + "description": "Selection target: {kind:'selection', start:{kind:'text', blockId, offset}, end:{kind:'text', blockId, offset}}." + }, + "type": { + "description": "Content format: 'text' (default), 'markdown', or 'html'.", + "enum": [ + "text", + "markdown", + "html" + ], + "type": "string" + }, + "value": { + "description": "Text content to insert.", + "type": "string" + } + }, + "required": [ + "target", + "value" + ], + "type": "object" }, - "type": { - "description": "Content format: 'text' (default), 'markdown', or 'html'.", - "enum": [ - "text", - "markdown", - "html" + { + "additionalProperties": false, + "properties": { + "ref": { + "description": "Handle ref string returned by a prior search/query result.", + "type": "string" + }, + "type": { + "description": "Content format: 'text' (default), 'markdown', or 'html'.", + "enum": [ + "text", + "markdown", + "html" + ], + "type": "string" + }, + "value": { + "description": "Text content to insert.", + "type": "string" + } + }, + "required": [ + "ref", + "value" ], - "type": "string" + "type": "object" }, - "value": { - "description": "Text content to insert.", - "type": "string" + { + "additionalProperties": false, + "properties": { + "type": { + "description": "Content format: 'text' (default), 'markdown', or 'html'.", + "enum": [ + "text", + "markdown", + "html" + ], + "type": "string" + }, + "value": { + "description": "Text content to insert.", + "type": "string" + } + }, + "required": [ + "value" + ], + "type": "object" } - }, - "required": [ - "value" - ], - "type": "object" + ] }, { "additionalProperties": false, diff --git a/apps/docs/document-engine/sdks.mdx b/apps/docs/document-engine/sdks.mdx index f796d3c0cf..6f5faf2c39 100644 --- a/apps/docs/document-engine/sdks.mdx +++ b/apps/docs/document-engine/sdks.mdx @@ -382,7 +382,7 @@ The SDKs expose all operations from the [Document API](/document-api/overview) p | `doc.markdownToFragment` | `markdown-to-fragment` | Convert a Markdown string into an SDM/1 structural fragment. | | `doc.info` | `info` | Return document summary info including word, character, paragraph, heading, table, image, comment, tracked-change, SDT-field, list, and page counts, plus outline and capabilities. | | `doc.clearContent` | `clear-content` | Clear all document body content, leaving a single empty paragraph. | -| `doc.insert` | `insert` | Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. | +| `doc.insert` | `insert` | Insert content into the document. Two input shapes: text-based (value + type) inserts inline content at a SelectionTarget or ref position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target/ref is omitted, content appends at the end of the document. Text mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. | | `doc.replace` | `replace` | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts a BlockNodeAddress (replaces whole block), SelectionTarget (expands to full covered block boundaries), or ref plus SDFragment content. | | `doc.delete` | `delete` | Delete content at a contiguous document selection. Accepts a SelectionTarget or mutation-ready ref. Supports cross-block deletion and optional block-edge expansion via behavior mode. | | `doc.blocks.list` | `blocks list` | List top-level blocks in document order with IDs, types, and text previews. Supports pagination via offset/limit and optional nodeType filtering. | @@ -832,7 +832,7 @@ The SDKs expose all operations from the [Document API](/document-api/overview) p | `doc.markdown_to_fragment` | `markdown-to-fragment` | Convert a Markdown string into an SDM/1 structural fragment. | | `doc.info` | `info` | Return document summary info including word, character, paragraph, heading, table, image, comment, tracked-change, SDT-field, list, and page counts, plus outline and capabilities. | | `doc.clear_content` | `clear-content` | Clear all document body content, leaving a single empty paragraph. | -| `doc.insert` | `insert` | Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. | +| `doc.insert` | `insert` | Insert content into the document. Two input shapes: text-based (value + type) inserts inline content at a SelectionTarget or ref position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target/ref is omitted, content appends at the end of the document. Text mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. | | `doc.replace` | `replace` | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts a BlockNodeAddress (replaces whole block), SelectionTarget (expands to full covered block boundaries), or ref plus SDFragment content. | | `doc.delete` | `delete` | Delete content at a contiguous document selection. Accepts a SelectionTarget or mutation-ready ref. Supports cross-block deletion and optional block-edge expansion via behavior mode. | | `doc.blocks.list` | `blocks list` | List top-level blocks in document order with IDs, types, and text previews. Supports pagination via offset/limit and optional nodeType filtering. | diff --git a/packages/document-api/src/README.md b/packages/document-api/src/README.md index cd4d20f803..5328f71c10 100644 --- a/packages/document-api/src/README.md +++ b/packages/document-api/src/README.md @@ -44,7 +44,7 @@ lives in adapter layers that map engine behavior into discovery envelopes and ot - `find` always returns `SDFindResult` with `items: SDNodeResult[]`. - Each item has `{ node, address }`, where `address` is a `NodeAddress`. - For precise mutation targeting, use `query.match`, which returns a canonical `SelectionTarget`, block addresses, and block/range metadata. -- `insert` accepts either legacy text input with an optional `TextAddress` target or structural content with an optional `BlockNodeAddress` target. Omitting `target` inserts at the end of the document. +- `insert` accepts text input with an optional `SelectionTarget` or `ref`, or structural content with an optional `BlockNodeAddress` target. Omitting both `target` and `ref` appends text at the end of the document. - Structural creation is exposed under `create.*` (for example `create.paragraph`), separate from text mutations. ## Adapter Error Convention @@ -241,22 +241,22 @@ Return document summary metadata (block count, word count, character count). ### `insert` -Insert content into the document. Legacy string input inserts at an optional `TextAddress`. Structural content inserts relative to an optional `BlockNodeAddress` using `placement`. When `target` is omitted, content appends at the end of the document. +Insert content into the document. Text input inserts at an optional `SelectionTarget` or `ref`, or appends at the end of the document when both are omitted. Structural content inserts relative to an optional `BlockNodeAddress` using `placement`. Supports dry-run and tracked mode. -- **Input**: `InsertInput` (`{ value, type?, target?: TextAddress } | { content, target?: BlockNodeAddress, placement?, nestingPolicy? }`) +- **Input**: `InsertInput` (`{ value, type?, target: SelectionTarget } | { value, type?, ref: string } | { value, type? } | { content, target?: BlockNodeAddress, placement?, nestingPolicy? }`) - **Options**: `MutationOptions` (`{ changeMode?, dryRun? }`) - **Output**: `SDMutationReceipt` - **Mutates**: Yes - **Idempotency**: non-idempotent -- **Failure codes**: see the generated reference docs for the full legacy vs. structural failure surface +- **Failure codes**: see the generated reference docs for the full text vs. structural failure surface ### `replace` Replace content at a contiguous selection. Text replacement accepts `SelectionTarget` or `ref`. Structural replacement accepts `BlockNodeAddress`, `SelectionTarget`, or `ref` with `content`. Supports dry-run and tracked mode. -- **Input**: `ReplaceInput` (`{ target?: SelectionTarget, ref?: string, text } | { target?: BlockNodeAddress | SelectionTarget, ref?: string, content, nestingPolicy? }`) +- **Input**: `ReplaceInput` (`{ target: SelectionTarget, text } | { ref: string, text } | { target: BlockNodeAddress | SelectionTarget, content, nestingPolicy? } | { ref: string, content, nestingPolicy? }`) - **Options**: `MutationOptions` (`{ changeMode?, dryRun? }`) - **Output**: `SDMutationReceipt` - **Mutates**: Yes @@ -267,7 +267,7 @@ Replace content at a contiguous selection. Text replacement accepts `SelectionTa Delete content at a contiguous selection. Accepts either an explicit `SelectionTarget` or a mutation-ready `ref`. Supports dry-run and tracked mode. -- **Input**: `DeleteInput` (`{ target?: SelectionTarget, ref?: string, behavior?: 'selection' | 'exact' }`) +- **Input**: `DeleteInput` (`{ target: SelectionTarget, behavior?: 'selection' | 'exact' } | { ref: string, behavior?: 'selection' | 'exact' }`) - **Options**: `MutationOptions` (`{ changeMode?, dryRun? }`) - **Output**: `TextMutationReceipt` - **Mutates**: Yes @@ -326,7 +326,7 @@ Insert a new heading node at a specified location with a given level (1-6). Retu Apply explicit inline style changes (bold, italic, underline, strike) to a contiguous selection using directive semantics (`'on'`, `'off'`, `'clear'`). Accepts a `SelectionTarget` or `ref`. Supports dry-run and tracked mode. Availability depends on the corresponding marks being registered in the editor schema. -- **Input**: `StyleApplyInput` (`{ target?: SelectionTarget, ref?: string, inline: { bold?, italic?, underline?, strike? } }`) +- **Input**: `StyleApplyInput` (`{ target: SelectionTarget, inline: { bold?, italic?, underline?, strike? } } | { ref: string, inline: { bold?, italic?, underline?, strike? } }`) - **Options**: `MutationOptions` (`{ changeMode?, dryRun? }`) - **Output**: `TextMutationReceipt` - **Mutates**: Yes diff --git a/packages/document-api/src/contract/contract.test.ts b/packages/document-api/src/contract/contract.test.ts index 0a00a05d46..c060b4e182 100644 --- a/packages/document-api/src/contract/contract.test.ts +++ b/packages/document-api/src/contract/contract.test.ts @@ -75,10 +75,16 @@ describe('document-api contract catalog', () => { } }); - it('declares insert input as a legacy-text or structural-content union', () => { + it('declares insert input as a text or structural-content union', () => { const schemas = buildInternalContractSchemas(); const insertInputSchema = schemas.operations.insert.input as { oneOf?: Array<{ + oneOf?: Array<{ + type?: string; + properties?: Record; + required?: string[]; + additionalProperties?: boolean; + }>; type?: string; properties?: Record; required?: string[]; @@ -89,13 +95,29 @@ describe('document-api contract catalog', () => { expect(Array.isArray(insertInputSchema.oneOf)).toBe(true); expect(insertInputSchema.oneOf).toHaveLength(2); - const [legacyVariant, structuralVariant] = insertInputSchema.oneOf!; + const [textVariant, structuralVariant] = insertInputSchema.oneOf!; + + expect(Array.isArray(textVariant.oneOf)).toBe(true); + expect(textVariant.oneOf).toHaveLength(3); + + const [textTargetVariant, textRefVariant, textUntargetedVariant] = textVariant.oneOf!; + + expect(textTargetVariant.type).toBe('object'); + expect(Object.keys(textTargetVariant.properties!).sort()).toEqual(['target', 'type', 'value']); + expect(textTargetVariant.required).toEqual(['target', 'value']); + expect(textTargetVariant.additionalProperties).toBe(false); + expect((textTargetVariant.properties!.target as { $ref?: string }).$ref).toBe('#/$defs/SelectionTarget'); + + expect(textRefVariant.type).toBe('object'); + expect(Object.keys(textRefVariant.properties!).sort()).toEqual(['ref', 'type', 'value']); + expect(textRefVariant.required).toEqual(['ref', 'value']); + expect(textRefVariant.additionalProperties).toBe(false); + expect((textRefVariant.properties!.ref as { type?: string }).type).toBe('string'); - expect(legacyVariant.type).toBe('object'); - expect(Object.keys(legacyVariant.properties!).sort()).toEqual(['target', 'type', 'value']); - expect(legacyVariant.required).toEqual(['value']); - expect(legacyVariant.additionalProperties).toBe(false); - expect((legacyVariant.properties!.target as { $ref?: string }).$ref).toBe('#/$defs/TextAddress'); + expect(textUntargetedVariant.type).toBe('object'); + expect(Object.keys(textUntargetedVariant.properties!).sort()).toEqual(['type', 'value']); + expect(textUntargetedVariant.required).toEqual(['value']); + expect(textUntargetedVariant.additionalProperties).toBe(false); expect(structuralVariant.type).toBe('object'); expect(Object.keys(structuralVariant.properties!).sort()).toEqual([ diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index c61d4df72a..1f6731cf92 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -419,13 +419,13 @@ export const OPERATION_DEFINITIONS = { memberPath: 'insert', description: 'Insert content into the document. Two input shapes: ' + - 'legacy string-based (value + type) inserts inline content at a text position within an existing block; ' + + 'text-based (value + type) inserts inline content at a SelectionTarget or ref position within an existing block; ' + 'structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. ' + - 'When target is omitted, content appends at the end of the document. ' + - 'Legacy mode supports text (default), markdown, and html content types via the `type` field. ' + + 'When target/ref is omitted, content appends at the end of the document. ' + + 'Text mode supports text (default), markdown, and html content types via the `type` field. ' + 'Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block.', expectedResult: - 'Returns an SDMutationReceipt with applied status; resolution reports a TextAddress for legacy text insertion or a BlockNodeAddress for structural insertion. Receipt reports NO_OP if the insertion point is invalid or content is empty.', + 'Returns an SDMutationReceipt with applied status; resolution reports the inserted TextAddress for text insertion or a BlockNodeAddress for structural insertion. Receipt reports NO_OP if the insertion point is invalid or content is empty.', requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'non-idempotent', diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index 2e6de31b13..3fd3f68f24 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -102,6 +102,42 @@ function targetLocatorWithPayload( }; } +/** + * Like {@link targetLocatorWithPayload}, but also allows an untargeted branch + * where neither `target` nor `ref` is present. + */ +function optionalTargetLocatorWithPayload( + payloadProperties: Record, + payloadRequired: readonly string[] = [], +): JsonSchema { + return { + oneOf: [ + objectSchema( + { + target: { + ...ref('SelectionTarget'), + description: + "Selection target: {kind:'selection', start:{kind:'text', blockId, offset}, end:{kind:'text', blockId, offset}}.", + }, + ...payloadProperties, + }, + ['target', ...payloadRequired], + ), + objectSchema( + { + ref: { + type: 'string', + description: 'Handle ref string returned by a prior search/query result.', + }, + ...payloadProperties, + }, + ['ref', ...payloadRequired], + ), + objectSchema({ ...payloadProperties }, [...payloadRequired]), + ], + }; +} + /** Shared output/success/failure shape for ImagesMutationResult operations. */ function imagesMutationSchemaSet(inputSchema: JsonSchema): OperationSchemaSet { return { @@ -1509,12 +1545,8 @@ const nestingPolicySchema: JsonSchema = { const insertInputSchema: JsonSchema = { oneOf: [ - objectSchema( + optionalTargetLocatorWithPayload( { - target: { - ...textAddressSchema, - description: "Insertion point: {kind:'text', blockId:'...', range:{start, end}}.", - }, value: { type: 'string', description: 'Text content to insert.' }, type: { type: 'string', diff --git a/packages/document-api/src/delete/delete.ts b/packages/document-api/src/delete/delete.ts index 28790b2065..8537114b99 100644 --- a/packages/document-api/src/delete/delete.ts +++ b/packages/document-api/src/delete/delete.ts @@ -5,7 +5,7 @@ * string from discovery APIs (`query.match`, `find`). */ -import type { SelectionTarget, DeleteBehavior } from '../types/address.js'; +import type { SelectionTarget, DeleteBehavior, TargetLocator } from '../types/address.js'; import type { TextMutationReceipt } from '../types/receipt.js'; import type { MutationOptions } from '../types/mutation-plan.types.js'; import type { SelectionMutationAdapter } from '../selection-mutation.js'; @@ -18,7 +18,7 @@ import { isSelectionTarget } from '../validation/selection-target-validator.js'; // Public input type // --------------------------------------------------------------------------- -export interface DeleteInput { +export type DeleteInput = TargetLocator & { /** Explicit selection target. Exactly one of `target` or `ref` is required. */ target?: SelectionTarget; /** Mutation-ready ref from `query.match` or `find`. */ @@ -29,7 +29,7 @@ export interface DeleteInput { * - `'exact'`: delete only the exact resolved range. */ behavior?: DeleteBehavior; -} +}; // --------------------------------------------------------------------------- // Validation @@ -99,13 +99,9 @@ export function executeDelete( ): TextMutationReceipt { validateDeleteInput(input); - return adapter.execute( - { - kind: 'delete', - target: input.target, - ref: input.ref, - behavior: input.behavior ?? 'selection', - }, - normalizeMutationOptions(options), - ); + const request = input.target + ? { kind: 'delete' as const, target: input.target, behavior: input.behavior ?? 'selection' } + : { kind: 'delete' as const, ref: input.ref!, behavior: input.behavior ?? 'selection' }; + + return adapter.execute(request, normalizeMutationOptions(options)); } diff --git a/packages/document-api/src/format/format.ts b/packages/document-api/src/format/format.ts index 2336ff3b09..20ac2d94d1 100644 --- a/packages/document-api/src/format/format.ts +++ b/packages/document-api/src/format/format.ts @@ -7,7 +7,7 @@ import type { MutationOptions } from '../types/mutation-plan.types.js'; import { normalizeMutationOptions } from '../write/write.js'; -import type { SelectionTarget } from '../types/address.js'; +import type { SelectionTarget, TargetLocator } from '../types/address.js'; import type { TextMutationReceipt } from '../types/receipt.js'; import type { SelectionMutationAdapter } from '../selection-mutation.js'; import { DocumentApiValidationError } from '../errors.js'; @@ -30,10 +30,7 @@ export type FormatItalicInput = FormatInlineAliasInput<'italic'>; export type FormatUnderlineInput = FormatInlineAliasInput<'underline'>; /** Input payload for `format.strikethrough`. */ -export interface FormatStrikethroughInput { - target?: SelectionTarget; - ref?: string; -} +export type FormatStrikethroughInput = TargetLocator; /** * Keys where `value` may be omitted — booleans (defaults to `true`) and @@ -52,19 +49,19 @@ type ImplicitTrueKey = * omission defaults to `true` for ergonomic "turn on" calls. */ export type FormatInlineAliasInput = K extends ImplicitTrueKey - ? { target?: SelectionTarget; ref?: string; value?: InlineRunPatch[K] } - : { target?: SelectionTarget; ref?: string; value: InlineRunPatch[K] }; + ? TargetLocator & { target?: SelectionTarget; ref?: string; value?: InlineRunPatch[K] } + : TargetLocator & { target?: SelectionTarget; ref?: string; value: InlineRunPatch[K] }; /** * Input payload for `format.apply`. * * Accepts either `target` (SelectionTarget) or `ref` (string) — exactly one required. */ -export interface StyleApplyInput { +export type StyleApplyInput = TargetLocator & { target?: SelectionTarget; ref?: string; inline: InlineRunPatch; -} +}; /** * Named alias for MutationOptions on format.apply. @@ -156,15 +153,10 @@ export function executeStyleApply( options?: MutationOptions, ): TextMutationReceipt { validateStyleApplyInput(input); - return adapter.execute( - { - kind: 'format', - target: input.target, - ref: input.ref, - inline: input.inline, - }, - normalizeMutationOptions(options), - ); + const request = input.target + ? { kind: 'format' as const, target: input.target, inline: input.inline } + : { kind: 'format' as const, ref: input.ref!, inline: input.inline }; + return adapter.execute(request, normalizeMutationOptions(options)); } // --------------------------------------------------------------------------- @@ -212,13 +204,8 @@ export function executeInlineAlias( const value = normalizeInlineAliasValue(key, (input as { value?: InlineRunPatch[K] }).value); const inline = { [key]: value } as InlineRunPatch; validateInlineRunPatch(inline); - return adapter.execute( - { - kind: 'format', - target: input.target, - ref: input.ref, - inline, - }, - normalizeMutationOptions(options), - ); + const request = input.target + ? { kind: 'format' as const, target: input.target, inline } + : { kind: 'format' as const, ref: input.ref!, inline }; + return adapter.execute(request, normalizeMutationOptions(options)); } diff --git a/packages/document-api/src/index.test.ts b/packages/document-api/src/index.test.ts index 2ae6560a36..4619ef69cb 100644 --- a/packages/document-api/src/index.test.ts +++ b/packages/document-api/src/index.test.ts @@ -622,14 +622,13 @@ describe('createDocumentApi', () => { lists: makeListsAdapter(), }); - const insertTarget = { kind: 'text', blockId: 'p1', range: { start: 0, end: 2 } } as const; const selectionTarget = { kind: 'selection' as const, start: { kind: 'text' as const, blockId: 'p1', offset: 0 }, end: { kind: 'text' as const, blockId: 'p1', offset: 2 }, }; api.insert({ value: 'Hi' }); - api.insert({ target: insertTarget, value: 'Yo' }); + api.insert({ target: selectionTarget, value: 'Yo' }); api.replace({ target: selectionTarget, text: 'Hello' }, { changeMode: 'tracked' }); api.delete({ target: selectionTarget }); @@ -638,10 +637,10 @@ describe('createDocumentApi', () => { { kind: 'insert', text: 'Hi' }, { changeMode: 'direct', dryRun: false }, ); - expect(writeAdpt.write).toHaveBeenNthCalledWith( - 2, - { kind: 'insert', target: insertTarget, text: 'Yo' }, - { changeMode: 'direct', dryRun: false }, + // Targeted insert now routes through selectionMutation adapter + expect(selectionAdpt.execute).toHaveBeenCalledWith( + { kind: 'insert', target: selectionTarget, ref: undefined, text: 'Yo' }, + { expectedRevision: undefined, changeMode: 'direct', dryRun: false }, ); expect(selectionAdpt.execute).toHaveBeenCalledWith( { kind: 'replace', target: selectionTarget, ref: undefined, text: 'Hello' }, @@ -1180,9 +1179,13 @@ describe('createDocumentApi', () => { expect(result.success).toBe(true); }); - it('accepts canonical target', () => { + it('accepts canonical SelectionTarget', () => { const api = makeApi(); - const target = { kind: 'text', blockId: 'p1', range: { start: 0, end: 0 } } as const; + const target = { + kind: 'selection' as const, + start: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + end: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + }; const result = api.insert({ target, value: 'hello' }); expect(result.success).toBe(true); }); @@ -1193,7 +1196,7 @@ describe('createDocumentApi', () => { const api = makeApi(); expectValidationError( () => api.insert({ target: null, value: 'hello' } as any), - 'target must be a text address object', + 'target must be a SelectionTarget object', ); }); @@ -1201,7 +1204,7 @@ describe('createDocumentApi', () => { const api = makeApi(); expectValidationError( () => api.insert({ target: { kind: 'text', blockId: 'p1' }, value: 'hello' } as any), - 'target must be a text address object', + 'target must be a SelectionTarget object', ); }); @@ -1295,8 +1298,8 @@ describe('createDocumentApi', () => { ); }); - it('maps insert({ target, value }) to internal write request with text field', () => { - const writeAdpt = makeWriteAdapter(); + it('maps insert({ target, value }) to selection mutation adapter', () => { + const selectionAdpt = makeSelectionMutationAdapter(); const api = createDocumentApi({ find: makeFindAdapter(FIND_RESULT), get: makeGetAdapter(), @@ -1305,18 +1308,22 @@ describe('createDocumentApi', () => { info: makeInfoAdapter(), capabilities: makeCapabilitiesAdapter(), comments: makeCommentsAdapter(), - write: writeAdpt, - selectionMutation: makeSelectionMutationAdapter(), + write: makeWriteAdapter(), + selectionMutation: selectionAdpt, trackChanges: makeTrackChangesAdapter(), create: makeCreateAdapter(), lists: makeListsAdapter(), }); - const target = { kind: 'text', blockId: 'p1', range: { start: 0, end: 2 } } as const; + const target = { + kind: 'selection' as const, + start: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + end: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + }; api.insert({ target, value: 'hello' }); - expect(writeAdpt.write).toHaveBeenCalledWith( - { kind: 'insert', target, text: 'hello' }, - { changeMode: 'direct', dryRun: false }, + expect(selectionAdpt.execute).toHaveBeenCalledWith( + { kind: 'insert', target, ref: undefined, text: 'hello' }, + { expectedRevision: undefined, changeMode: 'direct', dryRun: false }, ); }); @@ -1413,7 +1420,11 @@ describe('createDocumentApi', () => { lists: makeListsAdapter(), }); - const target = { kind: 'text', blockId: 'p1', range: { start: 0, end: 0 } } as const; + const target = { + kind: 'selection' as const, + start: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + end: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + }; api.insert({ target, value: '**bold**', type: 'markdown' }); expect(writeAdpt.insertStructured).toHaveBeenCalledWith( { target, value: '**bold**', type: 'markdown' }, diff --git a/packages/document-api/src/index.ts b/packages/document-api/src/index.ts index 86868d194e..2331fac7ee 100644 --- a/packages/document-api/src/index.ts +++ b/packages/document-api/src/index.ts @@ -11,7 +11,11 @@ export * from './inline-semantics/index.js'; export type { HistoryAdapter, HistoryApi } from './history/history.js'; export type { DiffAdapter, DiffApi } from './diff/diff.js'; export * from './diff/diff.types.js'; -export type { SelectionMutationAdapter, SelectionMutationRequest } from './selection-mutation.js'; +export type { + SelectionMutationAdapter, + SelectionMutationRequest, + SelectionInsertRequest, +} from './selection-mutation.js'; export type { RangeAnchor, DocumentEdgeAnchor, @@ -1340,7 +1344,7 @@ export { DocumentApiValidationError } from './errors.js'; export { textReceiptToSDReceipt, buildStructuralReceipt } from './receipt-bridge.js'; export type { StructuralReceiptParams } from './receipt-bridge.js'; export { isBlockNodeAddress } from './validation-primitives.js'; -export type { InsertInput, InsertContentType, LegacyInsertInput } from './insert/insert.js'; +export type { InsertInput, InsertContentType, TextInsertInput, LegacyInsertInput } from './insert/insert.js'; export { isStructuralInsertInput } from './insert/insert.js'; export type { ReplaceInput, TextReplaceInput } from './replace/replace.js'; export { isStructuralReplaceInput } from './replace/replace.js'; @@ -1818,7 +1822,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { }, }, insert(input: InsertInput, options?: MutationOptions): SDMutationReceipt { - return executeInsert(adapters.write, input, options); + return executeInsert(adapters.selectionMutation, adapters.write, input, options); }, replace(input: ReplaceInput, options?: MutationOptions): SDMutationReceipt { return executeReplace(adapters.selectionMutation, adapters.write, input, options); diff --git a/packages/document-api/src/insert/insert.ts b/packages/document-api/src/insert/insert.ts index d6483c46f6..9deb408944 100644 --- a/packages/document-api/src/insert/insert.ts +++ b/packages/document-api/src/insert/insert.ts @@ -1,53 +1,61 @@ import { executeWrite, normalizeMutationOptions, type MutationOptions, type WriteAdapter } from '../write/write.js'; -import type { TextAddress, TextMutationReceipt, SDMutationReceipt } from '../types/index.js'; +import type { SelectionTarget, TargetLocator, SDMutationReceipt } from '../types/index.js'; import type { SDInsertInput } from '../types/structural-input.js'; import type { SDFragment } from '../types/fragment.js'; import { PLACEMENT_VALUES } from '../types/placement.js'; import { DocumentApiValidationError } from '../errors.js'; import { isRecord, - isTextAddress, isBlockNodeAddress, assertNoUnknownFields, validateNestingPolicyValue, } from '../validation-primitives.js'; +import { isSelectionTarget } from '../validation/selection-target-validator.js'; import { validateDocumentFragment } from '../validation/fragment-validator.js'; import { textReceiptToSDReceipt } from '../receipt-bridge.js'; +import type { SelectionMutationAdapter } from '../selection-mutation.js'; // --------------------------------------------------------------------------- -// Legacy string-based input shape +// Text insert input shape (uses SelectionTarget/ref) // --------------------------------------------------------------------------- -/** Content format for the legacy insert operation payload. */ +/** Content format for the text insert operation payload. */ export type InsertContentType = 'text' | 'markdown' | 'html'; -/** Legacy string-based input for the insert operation. */ -export interface LegacyInsertInput { - /** Optional insertion target. When omitted, inserts at the end of the document. */ - target?: TextAddress; - /** The content to insert. Interpreted according to {@link LegacyInsertInput.type}. */ +type OptionalInsertLocator = TargetLocator | { target?: undefined; ref?: undefined }; + +/** Text-based input for the insert operation. */ +export type TextInsertInput = OptionalInsertLocator & { + /** Optional insertion target (SelectionTarget). When omitted, inserts at the end of the document. */ + target?: SelectionTarget; + /** Optional mutation ref returned by a prior find/query. Mutually exclusive with target. */ + ref?: string; + /** The content to insert. Interpreted according to {@link TextInsertInput.type}. */ value: string; /** Content format. Defaults to `'text'` when omitted. */ type?: InsertContentType; -} +}; + +/** @deprecated Use {@link TextInsertInput} instead. */ +export type LegacyInsertInput = TextInsertInput; // --------------------------------------------------------------------------- -// Discriminated union: legacy string shape OR structural SDFragment shape +// Discriminated union: text string shape OR structural SDFragment shape // --------------------------------------------------------------------------- /** * Input payload for the `doc.insert` operation. * - * Discrimination: presence of `content` (structural) vs `value` (legacy string). + * Discrimination: presence of `content` (structural) vs `value` (text string). * These are mutually exclusive — providing both is an error. */ -export type InsertInput = LegacyInsertInput | SDInsertInput; +export type InsertInput = TextInsertInput | SDInsertInput; // --------------------------------------------------------------------------- // Allowlists for strict field validation // --------------------------------------------------------------------------- -const LEGACY_INSERT_ALLOWED_KEYS = new Set(['value', 'type', 'target']); +const TEXT_INSERT_ALLOWED_KEYS = new Set(['value', 'type', 'target', 'ref']); const STRUCTURAL_INSERT_ALLOWED_KEYS = new Set(['content', 'target', 'placement', 'nestingPolicy']); const VALID_INSERT_TYPES: ReadonlySet = new Set(['text', 'markdown', 'html']); @@ -65,7 +73,7 @@ export function isStructuralInsertInput(input: InsertInput): input is SDInsertIn // --------------------------------------------------------------------------- /** - * Validates InsertInput as either legacy or structural shape. + * Validates InsertInput as either text or structural shape. * * Validation order: * 0. Input shape guard (must be non-null plain object) @@ -84,7 +92,7 @@ function validateInsertInput(input: unknown): asserts input is InsertInput { if (hasValue && hasContent) { throw new DocumentApiValidationError( 'INVALID_INPUT', - 'Insert input must provide either "value" (legacy) or "content" (structural), not both.', + 'Insert input must provide either "value" (text) or "content" (structural), not both.', { fields: ['value', 'content'] }, ); } @@ -93,7 +101,7 @@ function validateInsertInput(input: unknown): asserts input is InsertInput { if (!hasValue && !hasContent) { throw new DocumentApiValidationError( 'INVALID_INPUT', - 'Insert input must provide either "value" (legacy string) or "content" (SDFragment).', + 'Insert input must provide either "value" (text string) or "content" (SDFragment).', { fields: ['value', 'content'] }, ); } @@ -101,13 +109,13 @@ function validateInsertInput(input: unknown): asserts input is InsertInput { if (hasContent) { validateStructuralInsertInput(input); } else { - validateLegacyInsertInput(input); + validateTextInsertInput(input); } } -/** Validates the legacy string-based insert input shape. */ -function validateLegacyInsertInput(input: Record): void { - // Union conflict rule 4: structural-only fields with legacy shape +/** Validates the text-based insert input shape. */ +function validateTextInsertInput(input: Record): void { + // Union conflict rule 4: structural-only fields with text shape if ('placement' in input && input.placement !== undefined) { throw new DocumentApiValidationError( 'INVALID_INPUT', @@ -123,17 +131,33 @@ function validateLegacyInsertInput(input: Record): void { ); } - assertNoUnknownFields(input, LEGACY_INSERT_ALLOWED_KEYS, 'insert'); + assertNoUnknownFields(input, TEXT_INSERT_ALLOWED_KEYS, 'insert'); - const { target, value, type } = input; + const { target, ref, value, type } = input; - if (target !== undefined && !isTextAddress(target)) { - throw new DocumentApiValidationError('INVALID_TARGET', 'target must be a text address object.', { + // Mutual exclusivity: target and ref + if (target !== undefined && ref !== undefined) { + throw new DocumentApiValidationError( + 'INVALID_INPUT', + 'Insert input must provide either "target" or "ref", not both.', + { fields: ['target', 'ref'] }, + ); + } + + if (target !== undefined && !isSelectionTarget(target)) { + throw new DocumentApiValidationError('INVALID_TARGET', 'target must be a SelectionTarget object.', { field: 'target', value: target, }); } + if (ref !== undefined && typeof ref !== 'string') { + throw new DocumentApiValidationError('INVALID_TARGET', `ref must be a string, got ${typeof ref}.`, { + field: 'ref', + value: ref, + }); + } + if (typeof value !== 'string') { throw new DocumentApiValidationError('INVALID_TARGET', `value must be a string, got ${typeof value}.`, { field: 'value', @@ -152,7 +176,7 @@ function validateLegacyInsertInput(input: Record): void { /** Validates the structural SDFragment insert input shape. */ function validateStructuralInsertInput(input: Record): void { - // Union conflict rule 3: legacy-only "type" field with structural content + // Union conflict rule 3: text-only "type" field with structural content if ('type' in input && input.type !== undefined) { throw new DocumentApiValidationError( 'INVALID_INPUT', @@ -192,25 +216,39 @@ function validateStructuralInsertInput(input: Record): void { // Execution // --------------------------------------------------------------------------- -export function executeInsert(adapter: WriteAdapter, input: InsertInput, options?: MutationOptions): SDMutationReceipt { +export function executeInsert( + selectionAdapter: SelectionMutationAdapter, + writeAdapter: WriteAdapter, + input: InsertInput, + options?: MutationOptions, +): SDMutationReceipt { validateInsertInput(input); // Structural content path — returns SDMutationReceipt directly if (isStructuralInsertInput(input)) { - return adapter.insertStructured(input, normalizeMutationOptions(options)); + return writeAdapter.insertStructured(input, normalizeMutationOptions(options)); } - // Legacy string path - const { target, value } = input; + // Text string path + const { target, ref, value } = input; const contentType = input.type ?? 'text'; // For non-text content types, delegate to the adapter's structured insert path. if (contentType !== 'text') { - return adapter.insertStructured(input, normalizeMutationOptions(options)); + return writeAdapter.insertStructured(input, normalizeMutationOptions(options)); + } + + // Text path with target/ref → route through SelectionMutationAdapter + if (target || ref) { + const request = target + ? { kind: 'insert' as const, target, text: value } + : { kind: 'insert' as const, ref: ref!, text: value }; + const textReceipt = selectionAdapter.execute(request, normalizeMutationOptions(options)); + return textReceiptToSDReceipt(textReceipt); } - // Text path: use the existing write pipeline, wrap TextMutationReceipt → SDMutationReceipt - const request = target ? { kind: 'insert' as const, target, text: value } : { kind: 'insert' as const, text: value }; - const textReceipt = executeWrite(adapter, request, options); + // Text path without target/ref → target-less insert at document end + const request = { kind: 'insert' as const, text: value }; + const textReceipt = executeWrite(writeAdapter, request, options); return textReceiptToSDReceipt(textReceipt); } diff --git a/packages/document-api/src/overview-examples.test.ts b/packages/document-api/src/overview-examples.test.ts index 8d2a93e823..ab0d28ec24 100644 --- a/packages/document-api/src/overview-examples.test.ts +++ b/packages/document-api/src/overview-examples.test.ts @@ -581,7 +581,11 @@ describe('overview.mdx examples', () => { // Mirrors the exact code block from overview.mdx § "Dry-run preview" it('insert with dryRun true', () => { const doc = makeApi(); - const target = { kind: 'text' as const, blockId: 'p1', range: { start: 0, end: 3 } }; + const target = { + kind: 'selection' as const, + start: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + end: { kind: 'text' as const, blockId: 'p1', offset: 0 }, + }; const preview = doc.insert({ target, value: 'hello' }, { dryRun: true }); // preview.success tells you whether the insert would succeed diff --git a/packages/document-api/src/replace/replace.ts b/packages/document-api/src/replace/replace.ts index 7e3e63f2b2..2243034682 100644 --- a/packages/document-api/src/replace/replace.ts +++ b/packages/document-api/src/replace/replace.ts @@ -10,7 +10,7 @@ */ import type { MutationOptions } from '../types/mutation-plan.types.js'; -import type { SelectionTarget } from '../types/address.js'; +import type { SelectionTarget, TargetLocator } from '../types/address.js'; import type { SDMutationReceipt } from '../types/sd-contract.js'; import type { SDReplaceInput } from '../types/structural-input.js'; import type { SDFragment } from '../types/fragment.js'; @@ -33,11 +33,11 @@ import { textReceiptToSDReceipt } from '../receipt-bridge.js'; // --------------------------------------------------------------------------- /** Text replacement input — uses SelectionTarget / ref. */ -export interface TextReplaceInput { +export type TextReplaceInput = TargetLocator & { target?: SelectionTarget; ref?: string; text: string; -} +}; // --------------------------------------------------------------------------- // Discriminated union: text shape OR structural SDFragment shape @@ -216,14 +216,9 @@ export function executeReplace( // Text replacement path — route through SelectionMutationAdapter const textInput = input as TextReplaceInput; - const textReceipt = selectionAdapter.execute( - { - kind: 'replace', - target: textInput.target, - ref: textInput.ref, - text: textInput.text, - }, - normalizeMutationOptions(options), - ); + const request = textInput.target + ? { kind: 'replace' as const, target: textInput.target, text: textInput.text } + : { kind: 'replace' as const, ref: textInput.ref!, text: textInput.text }; + const textReceipt = selectionAdapter.execute(request, normalizeMutationOptions(options)); return textReceiptToSDReceipt(textReceipt); } diff --git a/packages/document-api/src/selection-mutation.ts b/packages/document-api/src/selection-mutation.ts index fe2fb7f5f2..8e2e62f6c3 100644 --- a/packages/document-api/src/selection-mutation.ts +++ b/packages/document-api/src/selection-mutation.ts @@ -1,13 +1,14 @@ /** * Selection-based mutation adapter — the single execution interface for - * `delete`, `replace` (text path), and `format.apply` in the new + * `delete`, `replace` (text path), `insert` (target/ref path), and `format.apply` in the new * SelectionTarget / ref model. * - * This replaces the WriteAdapter for delete/replace and the legacy format - * adapter for format.apply. All three operations route through the plan engine. + * This replaces the WriteAdapter for delete/replace, handles ref/selection-based + * text insertion, and replaces the legacy format adapter for format.apply. + * All four request kinds route through the plan engine. */ -import type { SelectionTarget, DeleteBehavior } from './types/address.js'; +import type { SelectionTarget, DeleteBehavior, TargetLocator } from './types/address.js'; import type { TextMutationReceipt } from './types/receipt.js'; import type { MutationOptions } from './types/mutation-plan.types.js'; import type { InlineRunPatch } from './format/inline-run-patch.js'; @@ -16,28 +17,39 @@ import type { InlineRunPatch } from './format/inline-run-patch.js'; // Adapter request types // --------------------------------------------------------------------------- -export type SelectionDeleteRequest = { +export type SelectionDeleteRequest = TargetLocator & { kind: 'delete'; target?: SelectionTarget; ref?: string; behavior: DeleteBehavior; }; -export type SelectionReplaceRequest = { +export type SelectionReplaceRequest = TargetLocator & { kind: 'replace'; target?: SelectionTarget; ref?: string; text: string; }; -export type SelectionFormatRequest = { +export type SelectionInsertRequest = TargetLocator & { + kind: 'insert'; + target?: SelectionTarget; + ref?: string; + text: string; +}; + +export type SelectionFormatRequest = TargetLocator & { kind: 'format'; target?: SelectionTarget; ref?: string; inline: InlineRunPatch; }; -export type SelectionMutationRequest = SelectionDeleteRequest | SelectionReplaceRequest | SelectionFormatRequest; +export type SelectionMutationRequest = + | SelectionDeleteRequest + | SelectionReplaceRequest + | SelectionInsertRequest + | SelectionFormatRequest; // --------------------------------------------------------------------------- // Adapter interface @@ -45,8 +57,8 @@ export type SelectionMutationRequest = SelectionDeleteRequest | SelectionReplace /** * Adapter that the super-editor plan engine implements for selection-based - * mutations. All three core mutation ops (delete, replace-text, format.apply) - * go through this single interface. + * mutations. Delete, replace-text, insert-with-locator, and format.apply go + * through this single interface. */ export interface SelectionMutationAdapter { execute(request: SelectionMutationRequest, options?: MutationOptions): TextMutationReceipt; diff --git a/packages/document-api/src/types/mutation-plan.types.ts b/packages/document-api/src/types/mutation-plan.types.ts index 70a0975db2..1e1fcb91f6 100644 --- a/packages/document-api/src/types/mutation-plan.types.ts +++ b/packages/document-api/src/types/mutation-plan.types.ts @@ -94,12 +94,7 @@ export type TextRewriteStep = { export type TextInsertStep = { id: string; op: 'text.insert'; - where: { - by: 'select'; - select: TextSelector | NodeSelector; - within?: BlockNodeAddress; - require: 'first' | 'exactlyOne'; - }; + where: StepWhere; args: { position: 'before' | 'after'; content: { text: string }; diff --git a/packages/document-api/src/types/query.ts b/packages/document-api/src/types/query.ts index 29e3bee061..78696559c4 100644 --- a/packages/document-api/src/types/query.ts +++ b/packages/document-api/src/types/query.ts @@ -73,7 +73,7 @@ export interface MatchContext { * Text ranges matching the query, expressed as block-relative offsets. * For cross-paragraph matches, this will include one range per block. * - * These ranges can be passed as targets to mutation operations. + * Block-relative ranges for display/discovery. Use `context.target` (SelectionTarget) for mutations. */ textRanges?: TextAddress[]; } diff --git a/packages/document-api/src/types/structural-input.ts b/packages/document-api/src/types/structural-input.ts index c27e50714e..ab20df8f8b 100644 --- a/packages/document-api/src/types/structural-input.ts +++ b/packages/document-api/src/types/structural-input.ts @@ -33,13 +33,24 @@ export interface SDInsertInput { // --------------------------------------------------------------------------- /** Structural shape for the replace operation. */ -export interface SDReplaceInput { - /** Target to replace. BlockNodeAddress replaces the entire block; SelectionTarget replaces a contiguous selection. */ - target?: BlockNodeAddress | SelectionTarget; - /** Opaque ref string (alternative to `target`). */ - ref?: string; +type StructuralReplaceLocator = + | { + /** Target to replace. BlockNodeAddress replaces the entire block; SelectionTarget replaces a contiguous selection. */ + target: BlockNodeAddress | SelectionTarget; + /** Opaque ref string (alternative to `target`). */ + ref?: undefined; + } + | { + /** Opaque ref string (alternative to `target`). */ + ref: string; + /** Target to replace. BlockNodeAddress replaces the entire block; SelectionTarget replaces a contiguous selection. */ + target?: undefined; + }; + +/** Structural shape for the replace operation. */ +export type SDReplaceInput = StructuralReplaceLocator & { /** Structural content to replace with. */ content: SDFragment; /** Nesting policy. Defaults to { tables: 'forbid' }. */ nestingPolicy?: NestingPolicy; -} +}; diff --git a/packages/document-api/src/write/write.ts b/packages/document-api/src/write/write.ts index 6ba2a0ce60..65e06a9851 100644 --- a/packages/document-api/src/write/write.ts +++ b/packages/document-api/src/write/write.ts @@ -1,5 +1,4 @@ -import type { TextAddress, TextMutationReceipt, SDMutationReceipt } from '../types/index.js'; -import type { BlockRelativeLocator } from './locator.js'; +import type { TextMutationReceipt, SDMutationReceipt } from '../types/index.js'; import type { InsertInput } from '../insert/insert.js'; import type { ReplaceInput } from '../replace/replace.js'; @@ -31,18 +30,15 @@ export interface MutationOptions extends RevisionGuardOptions { } /** - * Text insertion request — the only write-kind that still routes through - * the WriteAdapter. Delete and replace now use SelectionMutationAdapter. + * Text insertion request — target-less insert at document end. + * + * Targeted inserts now route through `SelectionMutationAdapter`. This + * request type only handles the no-target fallback (append to document end). */ export type InsertWriteRequest = { kind: 'insert'; - /** - * Optional insertion target. - * When omitted, inserts at the end of the document. - */ - target?: TextAddress; text: string; -} & Partial; +}; /** * Alias for `InsertWriteRequest`. Retained because super-editor adapter-utils diff --git a/packages/sdk/langs/node/README.md b/packages/sdk/langs/node/README.md index 721f6fe0f3..6aff4e2f25 100644 --- a/packages/sdk/langs/node/README.md +++ b/packages/sdk/langs/node/README.md @@ -44,7 +44,7 @@ console.log(info.counts); const results = await client.doc.find({ type: 'text', pattern: 'termination' }); await client.doc.replace({ - target: results.items[0].context.textRanges[0], + target: results.items[0].context.target, text: 'expiration', }); diff --git a/packages/super-editor/src/document-api-adapters/helpers/adapter-utils.ts b/packages/super-editor/src/document-api-adapters/helpers/adapter-utils.ts index 5ec133f37a..e899902a51 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/adapter-utils.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/adapter-utils.ts @@ -237,67 +237,48 @@ export function insertParagraphAtEnd( } /** - * Resolves the write target for a mutation request. + * Resolves the write target for a target-less insert request. * - * When the request is a target-less insert, falls back to the document-end - * insertion point via {@link resolveDefaultInsertTarget}. Otherwise resolves - * the explicit target address. + * Falls back to the document-end insertion point via {@link resolveDefaultInsertTarget}. + * Targeted inserts now route through SelectionMutationAdapter, not WriteAdapter. * * For structural-end resolutions (doc ends in non-text blocks), the returned * `ResolvedWrite` has `structuralEnd: true` and the caller is responsible for * creating a writable host before insertion. */ export function resolveWriteTarget(editor: Editor, request: WriteRequest): ResolvedWrite | null { - const requestedTarget = request.target; + const fallback = resolveDefaultInsertTarget(editor); + if (!fallback) return null; - if (request.kind === 'insert' && !request.target) { - const fallback = resolveDefaultInsertTarget(editor); - if (!fallback) return null; - - if (fallback.kind === 'structural-end') { - const pos = fallback.insertPos; - const syntheticRange: ResolvedTextTarget = { from: pos, to: pos }; - const syntheticTarget: TextAddress = { kind: 'text', blockId: '', range: { start: 0, end: 0 } }; - return { - requestedTarget, - effectiveTarget: syntheticTarget, - range: syntheticRange, - resolution: buildTextMutationResolution({ - requestedTarget, - target: syntheticTarget, - range: syntheticRange, - text: '', - }), - structuralEnd: true, - }; - } - - const text = readTextAtResolvedRange(editor, fallback.range); + if (fallback.kind === 'structural-end') { + const pos = fallback.insertPos; + const syntheticRange: ResolvedTextTarget = { from: pos, to: pos }; + const syntheticTarget: TextAddress = { kind: 'text', blockId: '', range: { start: 0, end: 0 } }; return { - requestedTarget, - effectiveTarget: fallback.target, - range: fallback.range, + requestedTarget: undefined, + effectiveTarget: syntheticTarget, + range: syntheticRange, resolution: buildTextMutationResolution({ - requestedTarget, - target: fallback.target, - range: fallback.range, - text, + requestedTarget: undefined, + target: syntheticTarget, + range: syntheticRange, + text: '', }), + structuralEnd: true, }; } - const target = request.target; - if (!target) return null; - - const range = resolveTextTarget(editor, target); - if (!range) return null; - - const text = readTextAtResolvedRange(editor, range); + const text = readTextAtResolvedRange(editor, fallback.range); return { - requestedTarget, - effectiveTarget: target, - range, - resolution: buildTextMutationResolution({ requestedTarget, target, range, text }), + requestedTarget: undefined, + effectiveTarget: fallback.target, + range: fallback.range, + resolution: buildTextMutationResolution({ + requestedTarget: undefined, + target: fallback.target, + range: fallback.range, + text, + }), }; } diff --git a/packages/super-editor/src/document-api-adapters/helpers/deterministic-node-id.test.ts b/packages/super-editor/src/document-api-adapters/helpers/deterministic-node-id.test.ts new file mode 100644 index 0000000000..f484538607 --- /dev/null +++ b/packages/super-editor/src/document-api-adapters/helpers/deterministic-node-id.test.ts @@ -0,0 +1,64 @@ +import { buildFallbackBlockNodeId, isVolatileRuntimeBlockId } from './deterministic-node-id.js'; + +describe('deterministic-node-id', () => { + describe('isVolatileRuntimeBlockId', () => { + it('treats UUID-like sdBlockIds as volatile runtime ids', () => { + expect(isVolatileRuntimeBlockId('7701a615-4ad8-45b5-922c-2a32114df4c8')).toBe(true); + }); + + it('does not treat descriptive ids as volatile runtime ids', () => { + expect(isVolatileRuntimeBlockId('table-1')).toBe(false); + }); + }); + + describe('buildFallbackBlockNodeId', () => { + it('builds deterministic table fallback ids from traversal paths', () => { + expect(buildFallbackBlockNodeId('table', 42, [3])).toBe('table-auto-4fff82cf'); + expect(buildFallbackBlockNodeId('table', 42, [3])).toBe('table-auto-4fff82cf'); + }); + + it('builds deterministic table-cell fallback ids from traversal paths', () => { + expect(buildFallbackBlockNodeId('tableCell', 88, [3, 1, 2])).toBe('cell-auto-5e34d2b2'); + expect(buildFallbackBlockNodeId('tableCell', 88, [3, 1, 2])).toBe('cell-auto-5e34d2b2'); + }); + + it('falls back to position when a traversal path is unavailable', () => { + expect(buildFallbackBlockNodeId('table', 12)).toBe('table-auto-c3f1b3e8'); + }); + + it('builds deterministic paragraph fallback ids from traversal paths', () => { + const id = buildFallbackBlockNodeId('paragraph', 10, [0]); + expect(id).toMatch(/^para-auto-[0-9a-f]{8}$/); + // Stable across calls + expect(buildFallbackBlockNodeId('paragraph', 10, [0])).toBe(id); + }); + + it('builds deterministic heading fallback ids from traversal paths', () => { + const id = buildFallbackBlockNodeId('heading', 20, [1]); + expect(id).toMatch(/^heading-auto-[0-9a-f]{8}$/); + expect(buildFallbackBlockNodeId('heading', 20, [1])).toBe(id); + }); + + it('builds deterministic listItem fallback ids from traversal paths', () => { + const id = buildFallbackBlockNodeId('listItem', 30, [2]); + expect(id).toMatch(/^list-auto-[0-9a-f]{8}$/); + expect(buildFallbackBlockNodeId('listItem', 30, [2])).toBe(id); + }); + + it('returns undefined for non-eligible types', () => { + expect(buildFallbackBlockNodeId('tableRow', 0, [0])).toBeUndefined(); + expect(buildFallbackBlockNodeId('image', 0, [0])).toBeUndefined(); + expect(buildFallbackBlockNodeId('sdt', 0, [0])).toBeUndefined(); + expect(buildFallbackBlockNodeId('tableOfContents', 0, [0])).toBeUndefined(); + }); + + it('produces different ids for different types at the same position', () => { + const paraId = buildFallbackBlockNodeId('paragraph', 10, [0]); + const headingId = buildFallbackBlockNodeId('heading', 10, [0]); + const listId = buildFallbackBlockNodeId('listItem', 10, [0]); + expect(paraId).not.toBe(headingId); + expect(paraId).not.toBe(listId); + expect(headingId).not.toBe(listId); + }); + }); +}); diff --git a/packages/super-editor/src/document-api-adapters/helpers/table-node-id.ts b/packages/super-editor/src/document-api-adapters/helpers/deterministic-node-id.ts similarity index 59% rename from packages/super-editor/src/document-api-adapters/helpers/table-node-id.ts rename to packages/super-editor/src/document-api-adapters/helpers/deterministic-node-id.ts index 81e4977909..92332911d0 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/table-node-id.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/deterministic-node-id.ts @@ -1,10 +1,13 @@ import type { BlockNodeType } from '@superdoc/document-api'; -type TableLikeBlockNodeType = 'table' | 'tableCell'; +type FallbackEligibleBlockNodeType = 'table' | 'tableCell' | 'paragraph' | 'heading' | 'listItem'; -const TABLE_LIKE_PREFIX: Readonly> = { +const FALLBACK_PREFIX: Readonly> = { table: 'table-auto', tableCell: 'cell-auto', + paragraph: 'para-auto', + heading: 'heading-auto', + listItem: 'list-auto', }; const UUID_LIKE_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; @@ -19,9 +22,12 @@ function stableHash(input: string): string { return (hash >>> 0).toString(16).padStart(8, '0'); } -function toTableLikeBlockNodeType(nodeType: BlockNodeType): TableLikeBlockNodeType | undefined { +function toFallbackEligibleBlockNodeType(nodeType: BlockNodeType): FallbackEligibleBlockNodeType | undefined { if (nodeType === 'table') return 'table'; if (nodeType === 'tableCell') return 'tableCell'; + if (nodeType === 'paragraph') return 'paragraph'; + if (nodeType === 'heading') return 'heading'; + if (nodeType === 'listItem') return 'listItem'; return undefined; } @@ -35,7 +41,7 @@ function serializeTraversalPath(path: readonly number[] | undefined, pos: number /** * Returns true when an sdBlockId looks like a runtime-generated UUID. * - * Table and table-cell sdBlockIds are frequently generated at editor startup, + * Block-level sdBlockIds are frequently generated at editor startup, * so UUID-like values are not safe to expose as public document-api node IDs. */ export function isVolatileRuntimeBlockId(id: string | undefined): boolean { @@ -43,22 +49,25 @@ export function isVolatileRuntimeBlockId(id: string | undefined): boolean { } /** - * Builds a deterministic public fallback ID for table-like nodes that lack a + * Builds a deterministic public fallback ID for block nodes that lack a * schema-valid persisted identity. * * The traversal path is preferred because it stays stable across reopen of the * same unchanged document while remaining independent of runtime-generated * `sdBlockId` UUIDs. */ -export function buildFallbackTableNodeId( +export function buildFallbackBlockNodeId( nodeType: BlockNodeType, pos: number, path?: readonly number[], ): string | undefined { - const tableLikeType = toTableLikeBlockNodeType(nodeType); - if (!tableLikeType) return undefined; + const eligibleType = toFallbackEligibleBlockNodeType(nodeType); + if (!eligibleType) return undefined; - const prefix = TABLE_LIKE_PREFIX[tableLikeType]; + const prefix = FALLBACK_PREFIX[eligibleType]; const source = serializeTraversalPath(path, pos); - return `${prefix}-${stableHash(`${tableLikeType}:${source}`)}`; + return `${prefix}-${stableHash(`${eligibleType}:${source}`)}`; } + +/** @deprecated Use {@link buildFallbackBlockNodeId} instead. */ +export const buildFallbackTableNodeId = buildFallbackBlockNodeId; diff --git a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts index 3518455106..5346e3102b 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts @@ -297,13 +297,61 @@ describe('buildBlockIndex', () => { expect(index.candidates[0].nodeId).toBe('p1'); }); - it('skips paragraph candidates when no explicit id attrs exist', () => { + it('assigns deterministic fallback id to paragraphs with no explicit id attrs', () => { const index = indexFromNodes({ typeName: 'paragraph', attrs: {}, offset: 7, }); - expect(index.candidates).toHaveLength(0); + expect(index.candidates).toHaveLength(1); + expect(index.candidates[0].nodeId).toMatch(/^para-auto-[0-9a-f]{8}$/); + }); + + it('assigns deterministic fallback id when sdBlockId is a volatile UUID', () => { + const index = indexFromNodes({ + typeName: 'paragraph', + attrs: { sdBlockId: '7701a615-4ad8-45b5-922c-2a32114df4c8' }, + offset: 7, + }); + expect(index.candidates).toHaveLength(1); + expect(index.candidates[0].nodeId).toMatch(/^para-auto-[0-9a-f]{8}$/); + // Volatile sdBlockId registered as alias + expect(index.byId.get('paragraph:7701a615-4ad8-45b5-922c-2a32114df4c8')).toBeDefined(); + }); + + it('keeps non-volatile sdBlockId as primary id for paragraphs', () => { + const index = indexFromNodes({ + typeName: 'paragraph', + attrs: { sdBlockId: 'my-stable-id' }, + offset: 7, + }); + expect(index.candidates[0].nodeId).toBe('my-stable-id'); + }); + }); + + describe('ID resolution — heading fallback', () => { + it('assigns deterministic fallback id to headings with no explicit id attrs', () => { + const index = indexFromNodes({ + typeName: 'paragraph', + attrs: { paragraphProperties: { styleId: 'Heading1' } }, + offset: 5, + }); + expect(index.candidates).toHaveLength(1); + expect(index.candidates[0].nodeType).toBe('heading'); + expect(index.candidates[0].nodeId).toMatch(/^heading-auto-[0-9a-f]{8}$/); + }); + }); + + describe('ID resolution — listItem fallback', () => { + it('assigns deterministic fallback id to listItems with no explicit id attrs', () => { + const index = indexFromNodes({ + typeName: 'paragraph', + attrs: { paragraphProperties: { numberingProperties: { numId: 1, ilvl: 0 } } }, + offset: 5, + }); + expect(index.candidates).toHaveLength(1); + expect(index.candidates[0].nodeType).toBe('listItem'); + expect(index.candidates[0].nodeId).toMatch(/^list-auto-[0-9a-f]{8}$/); }); }); diff --git a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts index 60a300df8d..ca1b6e14fa 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts @@ -5,7 +5,7 @@ import type { BlockNodeAddress, BlockNodeType, NodeAddress, NodeType } from '@su import type { ParagraphAttrs } from '../../extensions/types/node-attributes.js'; import { toId } from './value-utils.js'; import { resolvePublicTocNodeId } from './toc-node-id.js'; -import { buildFallbackTableNodeId, isVolatileRuntimeBlockId } from './table-node-id.js'; +import { buildFallbackBlockNodeId, isVolatileRuntimeBlockId } from './deterministic-node-id.js'; import { DocumentApiAdapterError } from '../errors.js'; /** Superset of all possible ID attributes across block node types. */ @@ -117,7 +117,7 @@ function resolveLegacyTableIdentity(attrs: BlockIdAttrs): string | undefined { return toId(attrs.paraId) ?? toId(attrs.blockId) ?? toId(attrs.id) ?? toId(attrs.uuid); } -function resolveRuntimeTableIdentity( +function resolveRuntimeBlockIdentity( nodeType: BlockNodeType, attrs: BlockIdAttrs, pos: number, @@ -127,7 +127,7 @@ function resolveRuntimeTableIdentity( if (sdBlockId && !isVolatileRuntimeBlockId(sdBlockId)) { return sdBlockId; } - return buildFallbackTableNodeId(nodeType, pos, path); + return buildFallbackBlockNodeId(nodeType, pos, path); } export function resolveBlockNodeId( @@ -139,9 +139,9 @@ export function resolveBlockNodeId( if (node.type.name === 'paragraph') { const attrs = node.attrs as ParagraphAttrs | undefined; // paraId (imported from DOCX) is the primary identity for paragraphs. This - // preserves historical IDs across DOCX round-trips, while sdBlockId remains - // a fallback for freshly created nodes. - return toId(attrs?.paraId) ?? toId(attrs?.sdBlockId); + // preserves historical IDs across DOCX round-trips. Non-volatile sdBlockId + // is preferred over deterministic fallback for freshly created nodes. + return toId(attrs?.paraId) ?? resolveRuntimeBlockIdentity(nodeType, (attrs ?? {}) as BlockIdAttrs, pos, path); } if (nodeType === 'tableOfContents') { @@ -162,7 +162,7 @@ export function resolveBlockNodeId( // UUID sdBlockId exists, expose a deterministic fallback instead so session // addresses remain reusable across fresh document opens. if (typeName === 'table' || typeName === 'tableCell' || typeName === 'tableHeader') { - return resolveLegacyTableIdentity(attrs) ?? resolveRuntimeTableIdentity(nodeType, attrs, pos, path); + return resolveLegacyTableIdentity(attrs) ?? resolveRuntimeBlockIdentity(nodeType, attrs, pos, path); } // NOTE: Migration surface for the stable-addresses plan. diff --git a/packages/super-editor/src/document-api-adapters/helpers/selection-target-resolver.ts b/packages/super-editor/src/document-api-adapters/helpers/selection-target-resolver.ts index 95c222abbf..0e08690c56 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/selection-target-resolver.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/selection-target-resolver.ts @@ -9,7 +9,12 @@ import type { SelectionTarget, SelectionPoint, SelectionEdgeNodeAddress } from '@superdoc/document-api'; import type { Editor } from '../../core/Editor.js'; import { getBlockIndex } from './index-cache.js'; -import { isTextBlockCandidate, type BlockCandidate, type BlockIndex } from './node-address-resolver.js'; +import { + isTextBlockCandidate, + findBlockByNodeIdOnly, + type BlockCandidate, + type BlockIndex, +} from './node-address-resolver.js'; import { resolveTextRangeInBlock } from './text-offset-resolver.js'; import { DocumentApiAdapterError } from '../errors.js'; @@ -165,12 +170,25 @@ function findTextBlockByNodeId(index: BlockIndex, nodeId: string): BlockCandidat ); } - return matches[0]; + if (matches.length === 1) return matches[0]; + + // Alias-aware fallback (same as findTextBlockCandidates in adapter-utils.ts). + // This ensures IDs returned by create operations remain resolvable in + // SelectionTarget lookups even if the canonical nodeId differs from the alias. + try { + const resolved = findBlockByNodeIdOnly(index, nodeId); + if (isTextBlockCandidate(resolved)) return resolved; + } catch (e) { + if (e instanceof DocumentApiAdapterError && e.code === 'AMBIGUOUS_TARGET') throw e; + } + + return undefined; } /** * Finds a block candidate by nodeType and nodeId. - * Uses the block index's byId map for O(1) lookup. + * Uses the block index's byId map for O(1) lookup, with alias-aware + * fallback via nodeId-only search for stale types or aliased IDs. */ function findBlockByTypeAndId(index: BlockIndex, nodeType: string, nodeId: string): BlockCandidate | undefined { const key = `${nodeType}:${nodeId}`; @@ -179,5 +197,17 @@ function findBlockByTypeAndId(index: BlockIndex, nodeType: string, nodeId: strin throw new DocumentApiAdapterError('AMBIGUOUS_TARGET', `Multiple blocks share key "${key}".`, { nodeType, nodeId }); } - return index.byId.get(key); + const exact = index.byId.get(key); + if (exact) return exact; + + // Alias-aware fallback: handles stale nodeType (e.g. paragraph re-typed to + // heading) and aliased IDs (e.g. volatile sdBlockId replaced by deterministic + // fallback). Same pattern as findTextBlockByNodeId. + try { + return findBlockByNodeIdOnly(index, nodeId); + } catch (e) { + if (e instanceof DocumentApiAdapterError && e.code === 'AMBIGUOUS_TARGET') throw e; + } + + return undefined; } diff --git a/packages/super-editor/src/document-api-adapters/helpers/table-node-id.test.ts b/packages/super-editor/src/document-api-adapters/helpers/table-node-id.test.ts deleted file mode 100644 index 4e9c79017f..0000000000 --- a/packages/super-editor/src/document-api-adapters/helpers/table-node-id.test.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { buildFallbackTableNodeId, isVolatileRuntimeBlockId } from './table-node-id.js'; - -describe('table-node-id', () => { - describe('isVolatileRuntimeBlockId', () => { - it('treats UUID-like sdBlockIds as volatile runtime ids', () => { - expect(isVolatileRuntimeBlockId('7701a615-4ad8-45b5-922c-2a32114df4c8')).toBe(true); - }); - - it('does not treat descriptive ids as volatile runtime ids', () => { - expect(isVolatileRuntimeBlockId('table-1')).toBe(false); - }); - }); - - describe('buildFallbackTableNodeId', () => { - it('builds deterministic table fallback ids from traversal paths', () => { - expect(buildFallbackTableNodeId('table', 42, [3])).toBe('table-auto-4fff82cf'); - expect(buildFallbackTableNodeId('table', 42, [3])).toBe('table-auto-4fff82cf'); - }); - - it('builds deterministic table-cell fallback ids from traversal paths', () => { - expect(buildFallbackTableNodeId('tableCell', 88, [3, 1, 2])).toBe('cell-auto-5e34d2b2'); - expect(buildFallbackTableNodeId('tableCell', 88, [3, 1, 2])).toBe('cell-auto-5e34d2b2'); - }); - - it('falls back to position when a traversal path is unavailable', () => { - expect(buildFallbackTableNodeId('table', 12)).toBe('table-auto-c3f1b3e8'); - }); - }); -}); diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts b/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts index 9e40805ce3..5f3ce0b3fe 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts @@ -675,7 +675,18 @@ function resolveTextRef(editor: Editor, index: BlockIndex, step: MutationStep, r } function resolveBlockRef(editor: Editor, index: BlockIndex, step: MutationStep, ref: string): CompiledTarget[] { - const candidate = index.candidates.find((c) => c.nodeId === ref); + let candidate = index.candidates.find((c) => c.nodeId === ref); + // Alias-aware fallback: if the ref is an sdBlockId registered as an alias + // (e.g., volatile UUID replaced by a deterministic para-auto-* primary ID), + // try the byId map which includes alias entries. + if (!candidate && index.byId) { + for (const [key, c] of index.byId) { + if (key.endsWith(`:${ref}`)) { + candidate = c; + break; + } + } + } if (!candidate) return []; const blockText = getBlockText(editor, candidate); diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/insert-structured-wrapper.test.ts b/packages/super-editor/src/document-api-adapters/plan-engine/insert-structured-wrapper.test.ts index 9ec08b66f2..8bdd9e43d2 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/insert-structured-wrapper.test.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/insert-structured-wrapper.test.ts @@ -163,8 +163,12 @@ describe('insertStructuredWrapper — markdown', () => { const result = insertStructuredWrapper(editor, { value: 'X', type: 'markdown', - target: { kind: 'text', ...target }, - }); + target: { + kind: 'selection', + start: { kind: 'text', blockId: target.blockId, offset: target.range.start }, + end: { kind: 'text', blockId: target.blockId, offset: target.range.end }, + }, + } as any); expect(result.success).toBe(false); expect(result.failure?.code).toBe('INVALID_TARGET'); diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts index b2f77d84ae..8de53b222a 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts @@ -176,35 +176,8 @@ function toTextAddress(target: TextAddress | BlockNodeAddress): TextAddress { // Locator normalization (same validation as the old adapters) // --------------------------------------------------------------------------- -function normalizeWriteLocator(request: WriteRequest): WriteRequest { - const hasBlockId = request.blockId !== undefined; - const hasOffset = request.offset !== undefined; - - if (hasOffset && request.target) { - throw new DocumentApiAdapterError('INVALID_TARGET', 'Cannot combine target with offset on insert request.', { - fields: ['target', 'offset'], - }); - } - if (hasOffset && !hasBlockId) { - throw new DocumentApiAdapterError('INVALID_TARGET', 'offset requires blockId on insert request.', { - fields: ['offset', 'blockId'], - }); - } - if (!hasBlockId) return request; - if (request.target) { - throw new DocumentApiAdapterError('INVALID_TARGET', 'Cannot combine target with blockId on insert request.', { - fields: ['target', 'blockId'], - }); - } - - const effectiveOffset = request.offset ?? 0; - const target: TextAddress = { - kind: 'text', - blockId: request.blockId!, - range: { start: effectiveOffset, end: effectiveOffset }, - }; - return { kind: 'insert', target, text: request.text }; -} +// normalizeWriteLocator removed — WriteRequest is now target-less only. +// Targeted inserts route through SelectionMutationAdapter. type FormatOperationInput = { target?: TextAddress | SelectionTarget; @@ -334,23 +307,19 @@ function validateWriteRequest(request: WriteRequest, resolved: ResolvedWrite): R } /** - * Write wrapper for insert operations only. + * Write wrapper for target-less insert operations only. * - * Delete and replace now route through `selectionMutationWrapper` via - * `SelectionMutationAdapter`. This wrapper handles the legacy insert path - * that still uses `TextAddress`-based `InsertWriteRequest`. + * Targeted inserts now route through `selectionMutationWrapper` via + * `SelectionMutationAdapter`. This wrapper handles the no-target fallback + * path that inserts at the document end. */ export function writeWrapper(editor: Editor, request: WriteRequest, options?: MutationOptions): TextMutationReceipt { - const normalizedRequest = normalizeWriteLocator(request); - - const resolved = resolveWriteTarget(editor, normalizedRequest); + const resolved = resolveWriteTarget(editor, request); if (!resolved) { - throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'Mutation target could not be resolved.', { - target: normalizedRequest.target, - }); + throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'Mutation target could not be resolved.', {}); } - const validationFailure = validateWriteRequest(normalizedRequest, resolved); + const validationFailure = validateWriteRequest(request, resolved); if (validationFailure) { return { success: false, resolution: resolved.resolution, failure: validationFailure }; } @@ -367,7 +336,7 @@ export function writeWrapper(editor: Editor, request: WriteRequest, options?: Mu // since raw `tr.insert(pos, textNode)` cannot place text between blocks. if (resolved.structuralEnd) { const insertPos = resolved.range.from; - const text = normalizedRequest.text ?? ''; + const text = request.text ?? ''; const receipt = executeDomainCommand( editor, (): boolean => { @@ -386,7 +355,7 @@ export function writeWrapper(editor: Editor, request: WriteRequest, options?: Mu id: stepId, op: 'text.insert', where: STUB_WHERE, - args: { position: 'before', content: { text: normalizedRequest.text ?? '' } }, + args: { position: 'before', content: { text: request.text ?? '' } }, } as unknown as MutationStep; const target = toCompiledTarget(stepId, 'text.insert', resolved); @@ -558,6 +527,14 @@ function buildSelectionStepDef(stepId: string, request: SelectionMutationRequest }, } as unknown as MutationStep; + case 'insert': + return { + id: stepId, + op: 'text.insert', + where, + args: { position: 'before', content: { text: request.text } }, + } as unknown as MutationStep; + case 'format': return { id: stepId, @@ -599,6 +576,27 @@ export function selectionMutationWrapper( // document state without mutating anything. const compiled = compilePlan(editor, [step]); + // Insert requires a collapsed (point) target. Reject non-collapsed ranges + // and multi-segment spans so the receipt is never misleading about the + // actual insertion point. + if (request.kind === 'insert') { + const compiledStep = compiled.mutationSteps.find((s) => s.step.id === stepId); + const target = compiledStep?.targets[0]; + if (target) { + // Multi-segment refs (span) are never valid for point inserts. + // Non-collapsed range/selection targets are also rejected. + const isInvalidInsertTarget = target.kind === 'span' || target.absFrom !== target.absTo; + if (isInvalidInsertTarget) { + const resolution = buildSelectionResolutionFromCompiled(compiled, stepId); + return { + success: false, + resolution, + failure: { code: 'INVALID_TARGET', message: 'Insert operations require a collapsed target range.' }, + }; + } + } + } + // Enforce expectedRevision even on dry-run — callers need to know if the // document has drifted since their last query, regardless of execution. checkRevision(editor, options?.expectedRevision); @@ -814,7 +812,7 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut // Legacy markdown/html path const contentType = input.type ?? 'text'; - const { value, target } = input; + const { value, target, ref } = input as { value: string; target?: SelectionTarget; ref?: string; type?: string }; // Tracked mode not supported for structured content const mode = options?.changeMode ?? 'direct'; @@ -830,14 +828,38 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut let effectiveTarget: TextAddress; if (target) { - const range = resolveTextTarget(editor, target); - if (!range) { - throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'Structured insert target could not be resolved.', { - target, - }); + const resolved = resolveSelectionTarget(editor, target); + resolvedRange = { from: resolved.absFrom, to: resolved.absTo }; + // Derive backward-compatible TextAddress from the start point + const startPoint = target.start; + const blockId = startPoint.kind === 'text' ? startPoint.blockId : startPoint.node.nodeId; + const offset = startPoint.kind === 'text' ? startPoint.offset : 0; + effectiveTarget = { kind: 'text', blockId, range: { start: offset, end: offset } }; + } else if (ref) { + // Resolve ref via a dummy compile step to get the absolute position + const dummyStepId = uuidv4(); + const dummyStep = { + id: dummyStepId, + op: 'text.insert', + where: { by: 'ref' as const, ref }, + args: { position: 'before', content: { text: '' } }, + } as unknown as MutationStep; + const compiled = compilePlan(editor, [dummyStep]); + const compiledStep = compiled.mutationSteps.find((s) => s.step.id === dummyStepId); + const compiledTarget = compiledStep?.targets[0]; + if (!compiledTarget) { + throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'Structured insert ref could not be resolved.', { ref }); } - resolvedRange = range; - effectiveTarget = target; + if (compiledTarget.kind === 'span') { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + 'Insert operations require a single-block ref. Multi-segment refs are not supported.', + { ref }, + ); + } + resolvedRange = { from: compiledTarget.absFrom, to: compiledTarget.absTo }; + const resolution = buildSelectionResolutionFromCompiled(compiled, dummyStepId); + effectiveTarget = resolution.target; } else { const fallback = resolveDefaultInsertTarget(editor); if (!fallback) { @@ -857,7 +879,7 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut } const resolution = buildTextMutationResolution({ - requestedTarget: target, + requestedTarget: effectiveTarget, target: effectiveTarget, range: resolvedRange, text: readTextAtResolvedRange(editor, resolvedRange), diff --git a/packages/super-editor/src/document-api-adapters/structural-write-engine/structural-write-engine.test.ts b/packages/super-editor/src/document-api-adapters/structural-write-engine/structural-write-engine.test.ts index 3521a56821..c409aacdab 100644 --- a/packages/super-editor/src/document-api-adapters/structural-write-engine/structural-write-engine.test.ts +++ b/packages/super-editor/src/document-api-adapters/structural-write-engine/structural-write-engine.test.ts @@ -1284,8 +1284,9 @@ describe('replaceStructuredWrapper — multi-block and locator forms', () => { // Single-block: no selectionTarget needed. expect(result.resolution!.selectionTarget).toBeUndefined(); - // The target should report full block (offset 0), not the partial offset. - expect(result.resolution!.target.blockId).toBe(ids[0]); + // The target should report a valid block ID (may be a deterministic + // fallback ID if the sdBlockId was a volatile UUID). + expect(result.resolution!.target.blockId).toBeTruthy(); }); it('multi-segment text: ref replaces all segments and includes selectionTarget', () => { diff --git a/packages/super-editor/src/document-api-adapters/write-adapter.ts b/packages/super-editor/src/document-api-adapters/write-adapter.ts index 3b58053a08..264007aa17 100644 --- a/packages/super-editor/src/document-api-adapters/write-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/write-adapter.ts @@ -1,19 +1,32 @@ import { v4 as uuidv4 } from 'uuid'; import type { Editor } from '../core/Editor.js'; -import type { - MutationOptions, - ReceiptFailure, - TextAddress, - TextMutationReceipt, - WriteRequest, -} from '@superdoc/document-api'; +import type { MutationOptions, ReceiptFailure, TextAddress, TextMutationReceipt } from '@superdoc/document-api'; import { DocumentApiAdapterError } from './errors.js'; import { ensureTrackedCapability } from './helpers/mutation-helpers.js'; import { applyDirectMutationMeta, applyTrackedMutationMeta } from './helpers/transaction-meta.js'; import { checkRevision } from './plan-engine/revision-tracker.js'; -import { insertParagraphAtEnd, resolveWriteTarget, type ResolvedWrite } from './helpers/adapter-utils.js'; +import { + insertParagraphAtEnd, + resolveTextTarget, + resolveWriteTarget, + type ResolvedWrite, +} from './helpers/adapter-utils.js'; +import { buildTextMutationResolution, readTextAtResolvedRange } from './helpers/text-mutation-resolution.js'; import { toCanonicalTrackedChangeId } from './helpers/tracked-change-resolver.js'; +/** + * Legacy insert request with optional TextAddress target and block-relative locator. + * The public InsertWriteRequest is now target-less; this extends it for + * backward-compat test callers that still exercise the targeted path. + */ +type LegacyInsertWriteRequest = { + kind: 'insert'; + target?: TextAddress; + text: string; + blockId?: string; + offset?: number; +}; + type LegacyReplaceWriteRequest = { kind: 'replace'; target?: TextAddress; @@ -31,20 +44,28 @@ type LegacyDeleteWriteRequest = { end?: number; }; -type LegacyWriteRequest = WriteRequest | LegacyReplaceWriteRequest | LegacyDeleteWriteRequest; +type LegacyWriteRequest = LegacyInsertWriteRequest | LegacyReplaceWriteRequest | LegacyDeleteWriteRequest; function resolveLegacyWriteTarget(editor: Editor, request: LegacyWriteRequest): ResolvedWrite | null { - if (request.kind === 'insert') { + // Target-less insert → default document-end insertion + if (request.kind === 'insert' && !request.target) { return resolveWriteTarget(editor, request); } - if (!request.target) return null; + // Targeted request (insert with target, replace, delete) → resolve TextAddress directly + const target = request.target; + if (!target) return null; - return resolveWriteTarget(editor, { - kind: 'insert', - target: request.target, - text: '', - }); + const range = resolveTextTarget(editor, target); + if (!range) return null; + + const text = readTextAtResolvedRange(editor, range); + return { + requestedTarget: target, + effectiveTarget: target, + range, + resolution: buildTextMutationResolution({ requestedTarget: target, target, range, text }), + }; } function validateWriteRequest(request: LegacyWriteRequest, resolvedTarget: ResolvedWrite): ReceiptFailure | null { @@ -269,12 +290,14 @@ function toFailureReceipt(failure: ReceiptFailure, resolvedTarget: ResolvedWrite }; } -export function writeAdapter(editor: Editor, request: WriteRequest, options?: MutationOptions): TextMutationReceipt { +export function writeAdapter( + editor: Editor, + request: LegacyWriteRequest, + options?: MutationOptions, +): TextMutationReceipt { checkRevision(editor, options?.expectedRevision); - // Keep the internal helper backwards-compatible for direct test callers - // that still exercise legacy replace/delete paths outside the public adapter. - const legacyRequest = request as LegacyWriteRequest; + const legacyRequest = request; // Normalize friendly locator fields (blockId + offset) into canonical TextAddress // before resolution. This is the adapter-layer normalization per the contract. From 997f2a76e70fba18000cecf5c72728c55bb2435f Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 20:27:07 -0700 Subject: [PATCH 2/7] fix: preserve paragraph runtime ids and reject empty insert refs --- .../document-api/src/insert/insert.test.ts | 43 +++++++++++++++++++ packages/document-api/src/insert/insert.ts | 4 +- .../helpers/node-address-resolver.test.ts | 13 +++--- .../helpers/node-address-resolver.ts | 38 +++++++++++++--- .../plan-engine/plan-wrappers.ts | 4 ++ 5 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 packages/document-api/src/insert/insert.test.ts diff --git a/packages/document-api/src/insert/insert.test.ts b/packages/document-api/src/insert/insert.test.ts new file mode 100644 index 0000000000..336435afe6 --- /dev/null +++ b/packages/document-api/src/insert/insert.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect, vi } from 'vitest'; +import { executeInsert, type InsertInput } from './insert.js'; +import type { SelectionMutationAdapter } from '../selection-mutation.js'; +import type { WriteAdapter } from '../write/write.js'; + +// --------------------------------------------------------------------------- +// Stub adapters — validation runs before any adapter method is called, so +// these stubs only need to exist (they should never be invoked in error cases). +// --------------------------------------------------------------------------- + +function createStubSelectionAdapter(): SelectionMutationAdapter { + return { execute: vi.fn() } as unknown as SelectionMutationAdapter; +} + +function createStubWriteAdapter(): WriteAdapter { + return { write: vi.fn(), insertStructured: vi.fn(), replaceStructured: vi.fn() } as unknown as WriteAdapter; +} + +// --------------------------------------------------------------------------- +// ref validation +// --------------------------------------------------------------------------- + +describe('executeInsert: ref validation', () => { + it('rejects empty string ref', () => { + const input: InsertInput = { value: 'hello', ref: '' } as unknown as InsertInput; + expect(() => executeInsert(createStubSelectionAdapter(), createStubWriteAdapter(), input)).toThrow( + 'ref must be a non-empty string', + ); + }); + + it('rejects non-string ref', () => { + const input = { value: 'hello', ref: 42 } as unknown as InsertInput; + expect(() => executeInsert(createStubSelectionAdapter(), createStubWriteAdapter(), input)).toThrow( + 'ref must be a non-empty string', + ); + }); + + it('does not reject undefined ref (untargeted insert is valid)', () => { + const writeAdapter = createStubWriteAdapter(); + (writeAdapter.write as ReturnType).mockReturnValue({ success: true, ref: 'r1' }); + expect(() => executeInsert(createStubSelectionAdapter(), writeAdapter, { value: 'hello' })).not.toThrow(); + }); +}); diff --git a/packages/document-api/src/insert/insert.ts b/packages/document-api/src/insert/insert.ts index 9deb408944..d137d490a9 100644 --- a/packages/document-api/src/insert/insert.ts +++ b/packages/document-api/src/insert/insert.ts @@ -151,8 +151,8 @@ function validateTextInsertInput(input: Record): void { }); } - if (ref !== undefined && typeof ref !== 'string') { - throw new DocumentApiValidationError('INVALID_TARGET', `ref must be a string, got ${typeof ref}.`, { + if (ref !== undefined && (typeof ref !== 'string' || ref === '')) { + throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a non-empty string.', { field: 'ref', value: ref, }); diff --git a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts index 5346e3102b..eef8ec6a13 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts @@ -307,16 +307,19 @@ describe('buildBlockIndex', () => { expect(index.candidates[0].nodeId).toMatch(/^para-auto-[0-9a-f]{8}$/); }); - it('assigns deterministic fallback id when sdBlockId is a volatile UUID', () => { + it('keeps volatile sdBlockId as primary id for session stability', () => { + // Volatile (UUID-like) sdBlockIds are preferred over deterministic + // fallback IDs because the fallback hashes nodeType + traversal path, + // which shifts when siblings are inserted/moved or a paragraph is + // restyled to heading/list-item. The UUID stays stable for the session. + const volatileUuid = '7701a615-4ad8-45b5-922c-2a32114df4c8'; const index = indexFromNodes({ typeName: 'paragraph', - attrs: { sdBlockId: '7701a615-4ad8-45b5-922c-2a32114df4c8' }, + attrs: { sdBlockId: volatileUuid }, offset: 7, }); expect(index.candidates).toHaveLength(1); - expect(index.candidates[0].nodeId).toMatch(/^para-auto-[0-9a-f]{8}$/); - // Volatile sdBlockId registered as alias - expect(index.byId.get('paragraph:7701a615-4ad8-45b5-922c-2a32114df4c8')).toBeDefined(); + expect(index.candidates[0].nodeId).toBe(volatileUuid); }); it('keeps non-volatile sdBlockId as primary id for paragraphs', () => { diff --git a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts index ca1b6e14fa..af28382396 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts @@ -117,7 +117,15 @@ function resolveLegacyTableIdentity(attrs: BlockIdAttrs): string | undefined { return toId(attrs.paraId) ?? toId(attrs.blockId) ?? toId(attrs.id) ?? toId(attrs.uuid); } -function resolveRuntimeBlockIdentity( +/** + * Resolves a runtime block identity for **table-like** nodes. + * + * Non-volatile sdBlockId is preferred; otherwise a deterministic fallback + * (hashed from nodeType + traversal path) is used. This is correct for tables + * because they keep a stable nodeType across mutations — their sdBlockId may + * change when ProseMirror replaces the node during property edits. + */ +function resolveTableRuntimeIdentity( nodeType: BlockNodeType, attrs: BlockIdAttrs, pos: number, @@ -130,6 +138,25 @@ function resolveRuntimeBlockIdentity( return buildFallbackBlockNodeId(nodeType, pos, path); } +/** + * Resolves a runtime block identity for **paragraph-like** nodes + * (paragraph, heading, listItem). + * + * Always prefers sdBlockId — even a volatile (UUID-like) one — because the + * deterministic fallback hashes nodeType + traversal path, both of which shift + * during ordinary edits: sibling inserts/moves change the path, and restyles + * (paragraph → heading/listItem) change the nodeType. The sdBlockId stays + * stable for the session lifetime. + */ +function resolveParagraphRuntimeIdentity( + nodeType: BlockNodeType, + attrs: BlockIdAttrs, + pos: number, + path?: TraversalPath, +): string | undefined { + return toId(attrs.sdBlockId) ?? buildFallbackBlockNodeId(nodeType, pos, path); +} + export function resolveBlockNodeId( node: ProseMirrorNode, pos: number, @@ -138,10 +165,9 @@ export function resolveBlockNodeId( ): string | undefined { if (node.type.name === 'paragraph') { const attrs = node.attrs as ParagraphAttrs | undefined; - // paraId (imported from DOCX) is the primary identity for paragraphs. This - // preserves historical IDs across DOCX round-trips. Non-volatile sdBlockId - // is preferred over deterministic fallback for freshly created nodes. - return toId(attrs?.paraId) ?? resolveRuntimeBlockIdentity(nodeType, (attrs ?? {}) as BlockIdAttrs, pos, path); + // paraId (imported from DOCX) is the primary identity for paragraphs — + // preserves historical IDs across DOCX round-trips. + return toId(attrs?.paraId) ?? resolveParagraphRuntimeIdentity(nodeType, (attrs ?? {}) as BlockIdAttrs, pos, path); } if (nodeType === 'tableOfContents') { @@ -162,7 +188,7 @@ export function resolveBlockNodeId( // UUID sdBlockId exists, expose a deterministic fallback instead so session // addresses remain reusable across fresh document opens. if (typeName === 'table' || typeName === 'tableCell' || typeName === 'tableHeader') { - return resolveLegacyTableIdentity(attrs) ?? resolveRuntimeBlockIdentity(nodeType, attrs, pos, path); + return resolveLegacyTableIdentity(attrs) ?? resolveTableRuntimeIdentity(nodeType, attrs, pos, path); } // NOTE: Migration surface for the stable-addresses plan. diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts index 8de53b222a..bd970c79c9 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts @@ -827,6 +827,10 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut let resolvedRange: ResolvedTextTarget; let effectiveTarget: TextAddress; + if (ref !== undefined && ref === '') { + throw new DocumentApiAdapterError('INVALID_TARGET', 'ref must be a non-empty string.', { ref }); + } + if (target) { const resolved = resolveSelectionTarget(editor, target); resolvedRange = { from: resolved.absFrom, to: resolved.absTo }; From 71b72f44c560fbb1735690eb5a5aa543e66a2032 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 21:10:52 -0700 Subject: [PATCH 3/7] fix: selection insert target validation and alias ref ambiguity --- packages/document-api/src/delete/delete.ts | 4 +- packages/document-api/src/format/format.ts | 4 +- .../document-api/src/insert/insert.test.ts | 103 ++++++++++++++++-- packages/document-api/src/insert/insert.ts | 13 +++ packages/document-api/src/replace/replace.ts | 4 +- .../helpers/node-address-resolver.ts | 70 ++++++++---- .../plan-engine/compiler.ts | 27 +++-- .../plan-engine/plan-wrappers.ts | 42 ++++++- 8 files changed, 215 insertions(+), 52 deletions(-) diff --git a/packages/document-api/src/delete/delete.ts b/packages/document-api/src/delete/delete.ts index 8537114b99..8d73383126 100644 --- a/packages/document-api/src/delete/delete.ts +++ b/packages/document-api/src/delete/delete.ts @@ -72,8 +72,8 @@ function validateDeleteInput(input: unknown): asserts input is DeleteInput { }); } - if (hasRef && typeof ref !== 'string') { - throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a string.', { + if (hasRef && (typeof ref !== 'string' || ref === '')) { + throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a non-empty string.', { field: 'ref', value: ref, }); diff --git a/packages/document-api/src/format/format.ts b/packages/document-api/src/format/format.ts index 20ac2d94d1..3a619ba494 100644 --- a/packages/document-api/src/format/format.ts +++ b/packages/document-api/src/format/format.ts @@ -115,8 +115,8 @@ function validateTargetLocator(input: Record, operation: string }); } - if (hasRef && typeof input.ref !== 'string') { - throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a string.', { + if (hasRef && (typeof input.ref !== 'string' || input.ref === '')) { + throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a non-empty string.', { field: 'ref', value: input.ref, }); diff --git a/packages/document-api/src/insert/insert.test.ts b/packages/document-api/src/insert/insert.test.ts index 336435afe6..ded73687ea 100644 --- a/packages/document-api/src/insert/insert.test.ts +++ b/packages/document-api/src/insert/insert.test.ts @@ -16,23 +16,41 @@ function createStubWriteAdapter(): WriteAdapter { return { write: vi.fn(), insertStructured: vi.fn(), replaceStructured: vi.fn() } as unknown as WriteAdapter; } +function exec(input: unknown): void { + executeInsert(createStubSelectionAdapter(), createStubWriteAdapter(), input as InsertInput); +} + +// --------------------------------------------------------------------------- +// Input shape discrimination +// --------------------------------------------------------------------------- + +describe('executeInsert: input shape', () => { + it('rejects non-object input', () => { + expect(() => exec(null)).toThrow('non-null object'); + expect(() => exec('hello')).toThrow('non-null object'); + expect(() => exec(42)).toThrow('non-null object'); + }); + + it('rejects input with neither value nor content', () => { + expect(() => exec({})).toThrow('either "value"'); + }); + + it('rejects input with both value and content', () => { + expect(() => exec({ value: 'hello', content: { blocks: [] } })).toThrow('not both'); + }); +}); + // --------------------------------------------------------------------------- -// ref validation +// Text insert: ref validation // --------------------------------------------------------------------------- describe('executeInsert: ref validation', () => { it('rejects empty string ref', () => { - const input: InsertInput = { value: 'hello', ref: '' } as unknown as InsertInput; - expect(() => executeInsert(createStubSelectionAdapter(), createStubWriteAdapter(), input)).toThrow( - 'ref must be a non-empty string', - ); + expect(() => exec({ value: 'hello', ref: '' })).toThrow('ref must be a non-empty string'); }); it('rejects non-string ref', () => { - const input = { value: 'hello', ref: 42 } as unknown as InsertInput; - expect(() => executeInsert(createStubSelectionAdapter(), createStubWriteAdapter(), input)).toThrow( - 'ref must be a non-empty string', - ); + expect(() => exec({ value: 'hello', ref: 42 })).toThrow('ref must be a non-empty string'); }); it('does not reject undefined ref (untargeted insert is valid)', () => { @@ -41,3 +59,70 @@ describe('executeInsert: ref validation', () => { expect(() => executeInsert(createStubSelectionAdapter(), writeAdapter, { value: 'hello' })).not.toThrow(); }); }); + +// --------------------------------------------------------------------------- +// Text insert: target/ref mutual exclusivity +// --------------------------------------------------------------------------- + +describe('executeInsert: target/ref mutual exclusivity', () => { + it('rejects input with both target and ref', () => { + const target = { + kind: 'selection', + start: { kind: 'text', blockId: 'b1', offset: 0 }, + end: { kind: 'text', blockId: 'b1', offset: 0 }, + }; + expect(() => exec({ value: 'hello', target, ref: 'r1' })).toThrow('either "target" or "ref", not both'); + }); +}); + +// --------------------------------------------------------------------------- +// Text insert: content type validation +// --------------------------------------------------------------------------- + +describe('executeInsert: content type validation', () => { + it('rejects invalid content type', () => { + expect(() => exec({ value: 'hello', type: 'pdf' })).toThrow('text, markdown, html'); + }); + + it('rejects non-string content type', () => { + expect(() => exec({ value: 'hello', type: 123 })).toThrow('text, markdown, html'); + }); +}); + +// --------------------------------------------------------------------------- +// Text insert: value validation +// --------------------------------------------------------------------------- + +describe('executeInsert: value validation', () => { + it('rejects non-string value', () => { + expect(() => exec({ value: 42 })).toThrow('value must be a string'); + }); +}); + +// --------------------------------------------------------------------------- +// Text insert: unknown fields +// --------------------------------------------------------------------------- + +describe('executeInsert: unknown fields', () => { + it('rejects unknown fields on text input', () => { + expect(() => exec({ value: 'hello', bogus: true })).toThrow('bogus'); + }); +}); + +// --------------------------------------------------------------------------- +// Structural insert: validation +// --------------------------------------------------------------------------- + +describe('executeInsert: structural input validation', () => { + it('rejects structural input with text-only "type" field', () => { + expect(() => exec({ content: { blocks: [] }, type: 'markdown' })).toThrow('"type" field is only valid'); + }); + + it('rejects invalid placement value', () => { + expect(() => exec({ content: { blocks: [] }, placement: 'middle' })).toThrow('before, after'); + }); + + it('rejects text-only fields on structural input', () => { + expect(() => exec({ content: { blocks: [] }, ref: 'r1' })).toThrow(); + }); +}); diff --git a/packages/document-api/src/insert/insert.ts b/packages/document-api/src/insert/insert.ts index d137d490a9..dddc56c522 100644 --- a/packages/document-api/src/insert/insert.ts +++ b/packages/document-api/src/insert/insert.ts @@ -216,6 +216,19 @@ function validateStructuralInsertInput(input: Record): void { // Execution // --------------------------------------------------------------------------- +/** + * Executes an insert operation, routing to the appropriate adapter path. + * + * - Text inserts with `target` or `ref` route through the SelectionMutationAdapter. + * - Structural inserts (SDFragment) and non-text content types route through WriteAdapter. + * - Text inserts without a locator append at the document end via WriteAdapter. + * + * @param selectionAdapter - Adapter for target/ref-based text mutations. + * @param writeAdapter - Adapter for structural and untargeted writes. + * @param input - Insert payload (text string or structural SDFragment). + * @param options - Optional mutation options (changeMode, dryRun, expectedRevision). + * @returns Receipt indicating success/failure and mutation metadata. + */ export function executeInsert( selectionAdapter: SelectionMutationAdapter, writeAdapter: WriteAdapter, diff --git a/packages/document-api/src/replace/replace.ts b/packages/document-api/src/replace/replace.ts index 2243034682..a672686a1b 100644 --- a/packages/document-api/src/replace/replace.ts +++ b/packages/document-api/src/replace/replace.ts @@ -95,8 +95,8 @@ function validateTargetLocator(input: Record, operation: string }); } - if (hasRef && typeof input.ref !== 'string') { - throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a string.', { + if (hasRef && (typeof input.ref !== 'string' || input.ref === '')) { + throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a non-empty string.', { field: 'ref', value: input.ref, }); diff --git a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts index af28382396..c7adfef0be 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts @@ -157,6 +157,20 @@ function resolveParagraphRuntimeIdentity( return toId(attrs.sdBlockId) ?? buildFallbackBlockNodeId(nodeType, pos, path); } +/** + * Resolves the public document-api nodeId for a block-level ProseMirror node. + * + * ID resolution strategy varies by block family: + * - **Paragraphs**: paraId → sdBlockId → deterministic fallback + * - **Tables/cells**: legacy attrs → non-volatile sdBlockId → deterministic fallback + * - **Other blocks**: blockId → id → paraId → uuid → sdBlockId + * + * @param node - The ProseMirror node. + * @param pos - Absolute document position of the node. + * @param nodeType - The mapped block node type. + * @param path - Optional traversal path for deterministic fallback IDs. + * @returns The resolved nodeId, or `undefined` if none could be determined. + */ export function resolveBlockNodeId( node: ProseMirrorNode, pos: number, @@ -346,10 +360,42 @@ export function findBlockByIdStrict(index: BlockIndex, address: BlockNodeAddress return candidate; } +/** + * Resolves a nodeId against alias entries in the block index (e.g., sdBlockId + * registered as an alias for a deterministic primary ID). + * + * @param index - The block index to search. + * @param nodeId - The node ID to resolve via alias lookup. + * @returns The single matching candidate, or `undefined` if no alias matches. + * @throws {DocumentApiAdapterError} `AMBIGUOUS_TARGET` when multiple blocks share the alias. + */ +export function resolveBlockAlias(index: BlockIndex, nodeId: string): BlockCandidate | undefined { + if (!index.byId) return undefined; + + const aliasMatches = new Map(); + for (const [key, candidate] of index.byId) { + if (!key.endsWith(`:${nodeId}`)) continue; + aliasMatches.set(`${candidate.nodeType}:${candidate.nodeId}`, candidate); + } + + if (aliasMatches.size > 1) { + throw new DocumentApiAdapterError('AMBIGUOUS_TARGET', `Multiple blocks share nodeId "${nodeId}" via aliases.`, { + nodeId, + count: aliasMatches.size, + matches: Array.from(aliasMatches.values()).map((candidate) => ({ + nodeType: candidate.nodeType, + nodeId: candidate.nodeId, + })), + }); + } + + return aliasMatches.size === 1 ? Array.from(aliasMatches.values())[0] : undefined; +} + /** * Finds a block candidate by raw nodeId without requiring a nodeType. * - * This is needed for create operations that position relative to _any_ block type. + * Falls back to alias resolution when no primary match exists. * * @param index - The block index to search. * @param nodeId - The node ID to resolve. @@ -370,26 +416,8 @@ export function findBlockByNodeIdOnly(index: BlockIndex, nodeId: string): BlockC } // No primary match — check alias entries (e.g., sdBlockId for paragraph-like nodes). - const aliasMatches = new Map(); - for (const [key, candidate] of index.byId) { - if (!key.endsWith(`:${nodeId}`)) continue; - aliasMatches.set(`${candidate.nodeType}:${candidate.nodeId}`, candidate); - } - - if (aliasMatches.size === 1) { - return Array.from(aliasMatches.values())[0]!; - } - - if (aliasMatches.size > 1) { - throw new DocumentApiAdapterError('AMBIGUOUS_TARGET', `Multiple blocks share nodeId "${nodeId}" via aliases.`, { - nodeId, - count: aliasMatches.size, - matches: Array.from(aliasMatches.values()).map((candidate) => ({ - nodeType: candidate.nodeType, - nodeId: candidate.nodeId, - })), - }); - } + const alias = resolveBlockAlias(index, nodeId); + if (alias) return alias; throw new DocumentApiAdapterError('TARGET_NOT_FOUND', `Block with nodeId "${nodeId}" was not found.`, { nodeId }); } diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts b/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts index 5f3ce0b3fe..3270603ac2 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/compiler.ts @@ -32,7 +32,12 @@ import { getBlockIndex } from '../helpers/index-cache.js'; import { getRevision } from './revision-tracker.js'; import { executeTextSelector } from '../find/text-strategy.js'; import { executeBlockSelector } from '../find/block-strategy.js'; -import { isTextBlockCandidate, type BlockCandidate, type BlockIndex } from '../helpers/node-address-resolver.js'; +import { + isTextBlockCandidate, + resolveBlockAlias, + type BlockCandidate, + type BlockIndex, +} from '../helpers/node-address-resolver.js'; import { resolveTextRangeInBlock } from '../helpers/text-offset-resolver.js'; import { resolveSelectionTarget, resolveSelectionPointPosition } from '../helpers/selection-target-resolver.js'; import { expandDeleteSelection } from '../helpers/expand-delete-selection.js'; @@ -675,18 +680,18 @@ function resolveTextRef(editor: Editor, index: BlockIndex, step: MutationStep, r } function resolveBlockRef(editor: Editor, index: BlockIndex, step: MutationStep, ref: string): CompiledTarget[] { - let candidate = index.candidates.find((c) => c.nodeId === ref); + const primaryMatches = index.candidates.filter((c) => c.nodeId === ref); + if (primaryMatches.length > 1) { + throw planError('AMBIGUOUS_TARGET', `Multiple blocks share nodeId "${ref}".`, step.id, { + ref, + count: primaryMatches.length, + }); + } + // Alias-aware fallback: if the ref is an sdBlockId registered as an alias // (e.g., volatile UUID replaced by a deterministic para-auto-* primary ID), - // try the byId map which includes alias entries. - if (!candidate && index.byId) { - for (const [key, c] of index.byId) { - if (key.endsWith(`:${ref}`)) { - candidate = c; - break; - } - } - } + // try the shared resolveBlockAlias helper which enforces ambiguity checks. + const candidate = primaryMatches[0] ?? resolveBlockAlias(index, ref); if (!candidate) return []; const blockText = getBlockText(editor, candidate); diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts index bd970c79c9..8f8b6d085b 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts @@ -567,6 +567,20 @@ export function selectionMutationWrapper( if (mode === 'tracked') ensureTrackedInlinePropertySupport(inlineKeys); } + // Text inserts require a position inside a textblock. Node-edge targets + // (e.g., "before paragraph/table/image") resolve to block boundaries where + // tr.insert() would place a text node at the doc/block level instead of + // inside a textblock. Reject them up front before compilation. + if (request.kind === 'insert' && request.target) { + const hasNodeEdge = request.target.start.kind === 'nodeEdge' || request.target.end.kind === 'nodeEdge'; + if (hasNodeEdge) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + 'Text inserts do not support nodeEdge targets. Use a text-offset target inside a textblock.', + ); + } + } + const stepId = uuidv4(); const where = buildSelectionWhere(request); const step = buildSelectionStepDef(stepId, request, where); @@ -576,15 +590,14 @@ export function selectionMutationWrapper( // document state without mutating anything. const compiled = compilePlan(editor, [step]); - // Insert requires a collapsed (point) target. Reject non-collapsed ranges - // and multi-segment spans so the receipt is never misleading about the - // actual insertion point. + // Insert requires a collapsed (point) target inside a textblock. + // Reject spans, non-collapsed ranges, and positions that resolve to + // block boundaries (e.g., refs to images/tables) so executeTextInsert() + // never attempts tr.insert() outside a textblock. if (request.kind === 'insert') { const compiledStep = compiled.mutationSteps.find((s) => s.step.id === stepId); const target = compiledStep?.targets[0]; if (target) { - // Multi-segment refs (span) are never valid for point inserts. - // Non-collapsed range/selection targets are also rejected. const isInvalidInsertTarget = target.kind === 'span' || target.absFrom !== target.absTo; if (isInvalidInsertTarget) { const resolution = buildSelectionResolutionFromCompiled(compiled, stepId); @@ -594,6 +607,21 @@ export function selectionMutationWrapper( failure: { code: 'INVALID_TARGET', message: 'Insert operations require a collapsed target range.' }, }; } + + // Verify the resolved position is inside a textblock — refs to + // non-text leaf blocks (image, table, sdt) resolve to block + // boundaries where a text node cannot be placed. + if (target.kind === 'range') { + const resolved = editor.state.doc.resolve(target.absFrom); + if (!resolved.parent.isTextblock) { + const resolution = buildSelectionResolutionFromCompiled(compiled, stepId); + return { + success: false, + resolution, + failure: { code: 'INVALID_TARGET', message: 'Text insert target must be inside a textblock.' }, + }; + } + } } } @@ -602,8 +630,12 @@ export function selectionMutationWrapper( checkRevision(editor, options?.expectedRevision); // Dry-run: compile and resolve, but do NOT execute. + // Mirror apply-time validation so callers do not get false-positive previews. if (options?.dryRun) { const resolution = buildSelectionResolutionFromCompiled(compiled, stepId); + if (request.kind === 'insert' && !request.text) { + return { success: false, resolution, failure: { code: 'NO_OP', message: 'Insert text is empty.' } }; + } return { success: true, resolution }; } From 95718cfc8a13568d84f31ede94e6570a2a8b2e7f Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 21:35:27 -0700 Subject: [PATCH 4/7] fix: document-api ref-based insert target resolution --- .../plan-engine/plan-wrappers.ts | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts index 8f8b6d085b..0f16bb8c04 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts @@ -590,21 +590,23 @@ export function selectionMutationWrapper( // document state without mutating anything. const compiled = compilePlan(editor, [step]); - // Insert requires a collapsed (point) target inside a textblock. - // Reject spans, non-collapsed ranges, and positions that resolve to - // block boundaries (e.g., refs to images/tables) so executeTextInsert() - // never attempts tr.insert() outside a textblock. + // Insert validation: reject multi-segment spans and non-textblock targets. + // Single-block range refs (absFrom < absTo) are valid — executeTextInsert() + // inserts at position: 'before' (absFrom), so the range width is irrelevant. if (request.kind === 'insert') { const compiledStep = compiled.mutationSteps.find((s) => s.step.id === stepId); const target = compiledStep?.targets[0]; if (target) { - const isInvalidInsertTarget = target.kind === 'span' || target.absFrom !== target.absTo; - if (isInvalidInsertTarget) { + // Multi-segment refs (span) are never valid for point inserts. + if (target.kind === 'span') { const resolution = buildSelectionResolutionFromCompiled(compiled, stepId); return { success: false, resolution, - failure: { code: 'INVALID_TARGET', message: 'Insert operations require a collapsed target range.' }, + failure: { + code: 'INVALID_TARGET', + message: 'Insert operations require a single-block target, not a multi-segment span.', + }, }; } @@ -893,7 +895,9 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut { ref }, ); } - resolvedRange = { from: compiledTarget.absFrom, to: compiledTarget.absTo }; + // Collapse to the start position — refs resolve to non-collapsed ranges + // but insert semantics is "insert before", not "replace range". + resolvedRange = { from: compiledTarget.absFrom, to: compiledTarget.absFrom }; const resolution = buildSelectionResolutionFromCompiled(compiled, dummyStepId); effectiveTarget = resolution.target; } else { @@ -923,8 +927,10 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut const { from, to } = resolvedRange; - // Insert semantics are point-only for doc.insert, regardless of content type. - if (from !== to) { + // Explicit targets with a non-collapsed range indicate a text selection — + // that's a replace operation, not an insert. Refs are already collapsed + // to their start position in the ref branch above. + if (target && from !== to) { return { success: false, resolution, From af95f886e7aa62321608ed8013ff83562775ffdf Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 21:58:09 -0700 Subject: [PATCH 5/7] fix: insert receipt targets for structured content --- .../plan-engine/plan-wrappers.ts | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts index 0f16bb8c04..fe9a2090ed 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts @@ -71,6 +71,7 @@ import { resolveSelectionTarget } from '../helpers/selection-target-resolver.js' import { getBlockIndex } from '../helpers/index-cache.js'; import { findBlockByNodeIdOnly, + findBlockByPos, isTextBlockCandidate, type BlockCandidate, type BlockIndex, @@ -81,6 +82,22 @@ import { getInlinePropertyCapabilityIssue, getTrackedInlinePropertySupportIssue // Helpers // --------------------------------------------------------------------------- +/** + * Computes the block-relative text offset for a `nodeEdge` `edge: 'after'` point. + * Returns the flattened text length of the block so the receipt target reflects + * the end of the anchor block rather than offset 0. + */ +function nodeEdgeAfterOffset(editor: Editor, nodeType: SelectionEdgeNodeType, nodeId: string): number { + const index = getBlockIndex(editor); + const key = `${nodeType}:${nodeId}`; + const block = index.byId.get(key); + if (!block || !block.node.isTextblock) return 0; + const contentStart = block.pos + 1; + const contentEnd = block.end - 1; + if (contentEnd <= contentStart) return 0; + return editor.state.doc.textBetween(contentStart, contentEnd, '', '\ufffc').length; +} + /** Check whether the editor has a DOM document available for HTML parsing. */ function editorHasDom(editor: Editor): boolean { const opts = (editor as any).options; @@ -871,7 +888,16 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut // Derive backward-compatible TextAddress from the start point const startPoint = target.start; const blockId = startPoint.kind === 'text' ? startPoint.blockId : startPoint.node.nodeId; - const offset = startPoint.kind === 'text' ? startPoint.offset : 0; + let offset: number; + if (startPoint.kind === 'text') { + offset = startPoint.offset; + } else if (startPoint.edge === 'after') { + // For edge: 'after', compute the block's text length so the receipt + // reflects the end of the anchor block, not offset 0. + offset = nodeEdgeAfterOffset(editor, startPoint.node.nodeType, startPoint.node.nodeId); + } else { + offset = 0; + } effectiveTarget = { kind: 'text', blockId, range: { start: offset, end: offset } }; } else if (ref) { // Resolve ref via a dummy compile step to get the absolute position @@ -899,7 +925,15 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut // but insert semantics is "insert before", not "replace range". resolvedRange = { from: compiledTarget.absFrom, to: compiledTarget.absFrom }; const resolution = buildSelectionResolutionFromCompiled(compiled, dummyStepId); - effectiveTarget = resolution.target; + // Collapse the resolution target to match the collapsed resolvedRange — + // the original resolution may reflect the full matched range, but insert + // semantics is a point insert at the start, not a range replacement. + const refTarget = resolution.target; + effectiveTarget = { + kind: 'text', + blockId: refTarget.blockId, + range: { start: refTarget.range.start, end: refTarget.range.start }, + }; } else { const fallback = resolveDefaultInsertTarget(editor); if (!fallback) { From b67c57c69c178f4d50f64311e3c46051f2dd5a87 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 22:50:05 -0700 Subject: [PATCH 6/7] fix: behavior tests --- tests/behavior/helpers/document-api.ts | 13 ++++++++--- .../programmatic-tracked-change.spec.ts | 23 ++++--------------- ...-adjacent-different-ids-resolution.spec.ts | 22 ++++-------------- 3 files changed, 18 insertions(+), 40 deletions(-) diff --git a/tests/behavior/helpers/document-api.ts b/tests/behavior/helpers/document-api.ts index a849b0c70e..278f972128 100644 --- a/tests/behavior/helpers/document-api.ts +++ b/tests/behavior/helpers/document-api.ts @@ -197,6 +197,13 @@ export async function findFirstSelectionTarget( return context?.target ?? null; } +export function collapseSelectionTargetToStart(target: SelectionTarget): SelectionTarget { + return { + ...target, + end: target.start, + }; +} + export async function addComment(page: Page, input: { target: TextAddress; text: string }): Promise { await page.evaluate((payload) => (window as any).editor.doc.comments.create(payload), input); } @@ -282,7 +289,7 @@ export async function listComments( export async function insertText( page: Page, - input: { value: string; target?: TextAddress; type?: 'text' | 'markdown' | 'html' }, + input: { value: string; target?: SelectionTarget; ref?: string; type?: 'text' | 'markdown' | 'html' }, options: { changeMode?: ChangeMode; dryRun?: boolean } = {}, ): Promise { return page.evaluate(({ payload, opts }) => (window as any).editor.doc.insert(payload, opts), { @@ -293,7 +300,7 @@ export async function insertText( export async function replaceText( page: Page, - input: { target: TextAddress; text: string }, + input: { target: SelectionTarget; text: string }, options: { changeMode?: ChangeMode; dryRun?: boolean } = {}, ): Promise { return page.evaluate(({ payload, opts }) => (window as any).editor.doc.replace(payload, opts), { @@ -304,7 +311,7 @@ export async function replaceText( export async function deleteText( page: Page, - input: { target: TextAddress }, + input: { target: SelectionTarget }, options: { changeMode?: ChangeMode; dryRun?: boolean } = {}, ): Promise { return page.evaluate(({ payload, opts }) => (window as any).editor.doc.delete(payload, opts), { diff --git a/tests/behavior/tests/comments/programmatic-tracked-change.spec.ts b/tests/behavior/tests/comments/programmatic-tracked-change.spec.ts index 1fd87daa9f..a70299bac7 100644 --- a/tests/behavior/tests/comments/programmatic-tracked-change.spec.ts +++ b/tests/behavior/tests/comments/programmatic-tracked-change.spec.ts @@ -2,15 +2,15 @@ import type { Page } from '@playwright/test'; import { test, expect } from '../../fixtures/superdoc.js'; import { assertDocumentApiReady, + collapseSelectionTargetToStart, deleteText, - findFirstTextRange, findFirstSelectionTarget, getDocumentText, insertText, listTrackChanges, replaceText, } from '../../helpers/document-api.js'; -import type { TextAddress, SelectionTarget, TextMutationReceipt } from '../../helpers/document-api.js'; +import type { SelectionTarget, TextMutationReceipt } from '../../helpers/document-api.js'; test.use({ config: { toolbar: 'full', comments: 'panel', trackChanges: true } }); @@ -27,13 +27,6 @@ async function assertTrackChangeTypeCount( .toBeGreaterThanOrEqual(minimumCount); } -function requireTextTarget(target: TextAddress | null, pattern: string): TextAddress { - if (target == null) { - throw new Error(`Could not find a text target for pattern "${pattern}".`); - } - return target; -} - function requireSelectionTarget(target: SelectionTarget | null, pattern: string): SelectionTarget { if (target == null) { throw new Error(`Could not find a selection target for pattern "${pattern}".`); @@ -96,16 +89,8 @@ test('direct insert via document-api', async ({ superdoc }) => { await superdoc.type('Hello World'); await superdoc.waitForStable(); - const target = requireTextTarget(await findFirstTextRange(superdoc.page, 'World'), 'World'); - - // insert requires a collapsed target range in the write adapter. - const insertionTarget: TextAddress = { - ...target, - range: { - start: target.range.start, - end: target.range.start, - }, - }; + const target = requireSelectionTarget(await findFirstSelectionTarget(superdoc.page, 'World'), 'World'); + const insertionTarget = collapseSelectionTargetToStart(target); const receipt = await insertText(superdoc.page, { value: 'Beautiful ', target: insertionTarget }); assertMutationSucceeded('insertText', receipt); diff --git a/tests/behavior/tests/comments/tracked-change-adjacent-different-ids-resolution.spec.ts b/tests/behavior/tests/comments/tracked-change-adjacent-different-ids-resolution.spec.ts index 514cf0a720..9535c3593b 100644 --- a/tests/behavior/tests/comments/tracked-change-adjacent-different-ids-resolution.spec.ts +++ b/tests/behavior/tests/comments/tracked-change-adjacent-different-ids-resolution.spec.ts @@ -2,13 +2,13 @@ import type { Page } from '@playwright/test'; import { test, expect } from '../../fixtures/superdoc.js'; import { assertDocumentApiReady, + collapseSelectionTargetToStart, deleteText, findFirstSelectionTarget, - findFirstTextRange, insertText, listTrackChanges, } from '../../helpers/document-api.js'; -import type { SelectionTarget, TextAddress, TextMutationReceipt, TrackChangeType } from '../../helpers/document-api.js'; +import type { SelectionTarget, TextMutationReceipt, TrackChangeType } from '../../helpers/document-api.js'; test.use({ config: { toolbar: 'full', comments: 'panel', trackChanges: true } }); @@ -26,14 +26,6 @@ type TrackedSegment = { type: TrackChangeType; }; -function requireTextTarget(target: TextAddress | null, pattern: string): TextAddress { - if (target != null) { - return target; - } - - throw new Error(`Could not find a text target for pattern "${pattern}".`); -} - function requireSelectionTarget(target: SelectionTarget | null, pattern: string): SelectionTarget { if (target != null) { return target; @@ -61,14 +53,8 @@ async function createAdjacentTrackedDeleteAndInsert(superdoc: SuperDocHarness) { const deleteReceipt = await deleteText(superdoc.page, { target: deleteTarget }, { changeMode: 'tracked' }); assertMutationSucceeded('deleteText', deleteReceipt); - const beforeB = requireTextTarget(await findFirstTextRange(superdoc.page, 'B'), 'B'); - const insertTarget: TextAddress = { - ...beforeB, - range: { - start: beforeB.range.start, - end: beforeB.range.start, - }, - }; + const beforeB = requireSelectionTarget(await findFirstSelectionTarget(superdoc.page, 'B'), 'B'); + const insertTarget = collapseSelectionTargetToStart(beforeB); const insertReceipt = await insertText( superdoc.page, { value: 'X', target: insertTarget, type: 'text' }, From 6cd92eb353f802b50f29e4e973c5e9b326c4e937 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 20 Mar 2026 08:01:22 -0700 Subject: [PATCH 7/7] chore: fix test import --- .../src/core/commands/insertContent.test.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/core/commands/insertContent.test.js b/packages/super-editor/src/core/commands/insertContent.test.js index e6d3e7b56d..77a9fae593 100644 --- a/packages/super-editor/src/core/commands/insertContent.test.js +++ b/packages/super-editor/src/core/commands/insertContent.test.js @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, beforeAll } from 'vitest'; +import { describe, it, expect, vi, beforeEach, beforeAll, afterEach } from 'vitest'; import { insertContent } from './insertContent.js'; import * as contentProcessor from '../helpers/contentProcessor.js'; @@ -169,6 +169,7 @@ describe('insertContent (integration) list export', () => { let helpers = null; let exportHelpers = null; let cachedDocxData = null; + let activeEditor = null; const getListParagraphs = (result) => { const body = result.elements?.find((el) => el.name === 'w:body'); @@ -200,9 +201,15 @@ describe('insertContent (integration) list export', () => { const setupEditor = () => { const { docx, media, mediaFiles, fonts } = cachedDocxData; const { editor } = helpers.initTestEditor({ content: docx, media, mediaFiles, fonts, mode: 'docx' }); + activeEditor = editor; return editor; }; + afterEach(() => { + activeEditor?.destroy(); + activeEditor = null; + }); + const exportFromEditorContent = async (editor) => { const content = editor.getJSON().content || []; return await exportHelpers.getExportedResultWithDocContent(content); @@ -354,6 +361,7 @@ describe('insertContent (integration) list export', () => { describe.skipIf(!process.env.CI)('insertContent (integration) horizontal rule', () => { let helpers = null; let cachedDocxData = null; + let activeEditor = null; beforeAll(async () => { vi.resetModules(); @@ -365,9 +373,15 @@ describe.skipIf(!process.env.CI)('insertContent (integration) horizontal rule', const setupEditor = () => { const { docx, media, mediaFiles, fonts } = cachedDocxData; const { editor } = helpers.initTestEditor({ content: docx, media, mediaFiles, fonts, mode: 'docx' }); + activeEditor = editor; return editor; }; + afterEach(() => { + activeEditor?.destroy(); + activeEditor = null; + }); + const countHorizontalRules = (editor) => { let count = 0; const content = editor.getJSON().content || [];