From ab94ed9e480997ebab49269b6a865288d4bfdbfd Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 20 Feb 2026 07:53:09 -0800 Subject: [PATCH] fix(document-api): add friendlier insert at node id with offset, or pos --- apps/cli/src/__tests__/cli.test.ts | 86 +++++ .../reference/_generated-manifest.json | 2 +- apps/docs/document-api/reference/insert.mdx | 40 +++ .../generated/agent/compatibility-hints.json | 2 +- .../generated/agent/remediation-map.json | 4 +- .../generated/agent/workflow-playbooks.json | 2 +- .../manifests/document-api-tools.json | 38 ++- .../schemas/document-api-contract.json | 38 ++- packages/document-api/src/README.md | 26 +- .../src/contract/contract.test.ts | 13 + .../src/contract/operation-definitions.ts | 2 +- packages/document-api/src/contract/schemas.ts | 24 +- packages/document-api/src/errors.ts | 18 ++ packages/document-api/src/index.test.ts | 300 ++++++++++++++++++ packages/document-api/src/index.ts | 2 + packages/document-api/src/insert/insert.ts | 156 ++++++++- packages/document-api/src/write/locator.ts | 14 + packages/document-api/src/write/write.ts | 3 +- .../write-adapter.test.ts | 139 ++++++++ .../document-api-adapters/write-adapter.ts | 59 +++- 20 files changed, 932 insertions(+), 36 deletions(-) create mode 100644 packages/document-api/src/errors.ts create mode 100644 packages/document-api/src/write/locator.ts diff --git a/apps/cli/src/__tests__/cli.test.ts b/apps/cli/src/__tests__/cli.test.ts index 8663e16de..df173bf8a 100644 --- a/apps/cli/src/__tests__/cli.test.ts +++ b/apps/cli/src/__tests__/cli.test.ts @@ -296,6 +296,13 @@ describe('superdoc CLI', () => { expect(result.stdout).not.toContain(' Document path or stdin'); }); + test('describe command doc.insert includes --block-id and --offset flags', async () => { + const result = await runCli(['describe', 'command', 'doc.insert', '--output', 'pretty']); + expect(result.code).toBe(0); + expect(result.stdout).toContain('--block-id'); + expect(result.stdout).toContain('--offset'); + }); + test('call executes an operation from canonical input payload', async () => { const result = await runCli([ 'call', @@ -949,6 +956,85 @@ describe('superdoc CLI', () => { expect(verifyEnvelope.data.result.total).toBeGreaterThan(0); }); + test('insert with --block-id and --offset 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); + + // Get a real blockId from the document + const target = await firstTextRange(['find', insertSource, '--type', 'text', '--pattern', 'Wilde']); + + const insertResult = await runCli([ + 'insert', + insertSource, + '--block-id', + target.blockId, + '--offset', + '0', + '--text', + 'CLI_BLOCKID_OFFSET_INSERT_1597', + '--out', + insertOut, + ]); + + expect(insertResult.code).toBe(0); + + const verifyResult = await runCli([ + 'find', + insertOut, + '--type', + 'text', + '--pattern', + 'CLI_BLOCKID_OFFSET_INSERT_1597', + ]); + expect(verifyResult.code).toBe(0); + const verifyEnvelope = parseJsonOutput>(verifyResult); + expect(verifyEnvelope.data.result.total).toBeGreaterThan(0); + }); + + test('insert with --block-id alone defaults offset 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); + + const target = await firstTextRange(['find', insertSource, '--type', 'text', '--pattern', 'Wilde']); + + const insertResult = await runCli([ + 'insert', + insertSource, + '--block-id', + target.blockId, + '--text', + 'CLI_BLOCKID_ONLY_INSERT_1597', + '--out', + insertOut, + ]); + + expect(insertResult.code).toBe(0); + + const insertEnvelope = parseJsonOutput< + SuccessEnvelope<{ + target: TextRange; + }> + >(insertResult); + // blockId alone → offset defaults to 0 → collapsed range at start + expect(insertEnvelope.data.target.range.start).toBe(0); + expect(insertEnvelope.data.target.range.end).toBe(0); + }); + + test('insert with --offset but no --block-id returns INVALID_ARGUMENT', 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); + + const result = await runCli(['insert', insertSource, '--offset', '5', '--text', 'should-fail', '--out', insertOut]); + + expect(result.code).toBe(1); + const envelope = parseJsonOutput(result); + expect(envelope.error.code).toBe('INVALID_ARGUMENT'); + expect(envelope.error.message).toContain('offset requires blockId'); + }); + test('create paragraph writes output and adds a new paragraph with seed text', async () => { const createSource = join(TEST_DIR, 'create-paragraph-source.docx'); const createOut = join(TEST_DIR, 'create-paragraph-out.docx'); diff --git a/apps/docs/document-api/reference/_generated-manifest.json b/apps/docs/document-api/reference/_generated-manifest.json index 59d186bdd..559723654 100644 --- a/apps/docs/document-api/reference/_generated-manifest.json +++ b/apps/docs/document-api/reference/_generated-manifest.json @@ -124,5 +124,5 @@ } ], "marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}", - "sourceHash": "3e7edc551feb80b42bfa59f928a0ce1f34ecb7cf71952d5a489107089105d543" + "sourceHash": "d53e75c131bc35996800d18ff3b67961de5d6d031ba20f4a6a03106e1117acaa" } diff --git a/apps/docs/document-api/reference/insert.mdx b/apps/docs/document-api/reference/insert.mdx index ae409fbf2..85f655973 100644 --- a/apps/docs/document-api/reference/insert.mdx +++ b/apps/docs/document-api/reference/insert.mdx @@ -23,6 +23,7 @@ description: Generated reference for insert - `TARGET_NOT_FOUND` - `TRACK_CHANGE_COMMAND_UNAVAILABLE` - `CAPABILITY_UNAVAILABLE` +- `INVALID_TARGET` ## Non-applied failure codes @@ -34,7 +35,46 @@ description: Generated reference for insert ```json { "additionalProperties": false, + "allOf": [ + { + "not": { + "required": [ + "target", + "blockId" + ] + } + }, + { + "not": { + "required": [ + "target", + "offset" + ] + } + }, + { + "if": { + "required": [ + "offset" + ] + }, + "then": { + "required": [ + "blockId" + ] + } + } + ], "properties": { + "blockId": { + "description": "Block ID for block-relative targeting.", + "type": "string" + }, + "offset": { + "description": "Character offset within the block identified by blockId.", + "minimum": 0, + "type": "integer" + }, "target": { "additionalProperties": false, "properties": { diff --git a/packages/document-api/generated/agent/compatibility-hints.json b/packages/document-api/generated/agent/compatibility-hints.json index 9bc2c2058..a0db2e2a7 100644 --- a/packages/document-api/generated/agent/compatibility-hints.json +++ b/packages/document-api/generated/agent/compatibility-hints.json @@ -362,5 +362,5 @@ "supportsTrackedMode": false } }, - "sourceHash": "3e7edc551feb80b42bfa59f928a0ce1f34ecb7cf71952d5a489107089105d543" + "sourceHash": "d53e75c131bc35996800d18ff3b67961de5d6d031ba20f4a6a03106e1117acaa" } diff --git a/packages/document-api/generated/agent/remediation-map.json b/packages/document-api/generated/agent/remediation-map.json index fa77e59dc..0f3323d94 100644 --- a/packages/document-api/generated/agent/remediation-map.json +++ b/packages/document-api/generated/agent/remediation-map.json @@ -170,7 +170,7 @@ "lists.setType", "replace" ], - "preApplyOperations": [] + "preApplyOperations": ["insert"] }, { "code": "NO_OP", @@ -328,5 +328,5 @@ ] } ], - "sourceHash": "3e7edc551feb80b42bfa59f928a0ce1f34ecb7cf71952d5a489107089105d543" + "sourceHash": "d53e75c131bc35996800d18ff3b67961de5d6d031ba20f4a6a03106e1117acaa" } diff --git a/packages/document-api/generated/agent/workflow-playbooks.json b/packages/document-api/generated/agent/workflow-playbooks.json index 55237d4e4..f8916c956 100644 --- a/packages/document-api/generated/agent/workflow-playbooks.json +++ b/packages/document-api/generated/agent/workflow-playbooks.json @@ -1,6 +1,6 @@ { "contractVersion": "0.1.0", - "sourceHash": "3e7edc551feb80b42bfa59f928a0ce1f34ecb7cf71952d5a489107089105d543", + "sourceHash": "d53e75c131bc35996800d18ff3b67961de5d6d031ba20f4a6a03106e1117acaa", "workflows": [ { "id": "find-mutate", diff --git a/packages/document-api/generated/manifests/document-api-tools.json b/packages/document-api/generated/manifests/document-api-tools.json index a73c68238..ba68db104 100644 --- a/packages/document-api/generated/manifests/document-api-tools.json +++ b/packages/document-api/generated/manifests/document-api-tools.json @@ -2,7 +2,7 @@ "contractVersion": "0.1.0", "generatedAt": null, "sourceCommit": null, - "sourceHash": "3e7edc551feb80b42bfa59f928a0ce1f34ecb7cf71952d5a489107089105d543", + "sourceHash": "d53e75c131bc35996800d18ff3b67961de5d6d031ba20f4a6a03106e1117acaa", "tools": [ { "description": "Read Document API data via `find`.", @@ -1013,7 +1013,36 @@ "idempotency": "non-idempotent", "inputSchema": { "additionalProperties": false, + "allOf": [ + { + "not": { + "required": ["target", "blockId"] + } + }, + { + "not": { + "required": ["target", "offset"] + } + }, + { + "if": { + "required": ["offset"] + }, + "then": { + "required": ["blockId"] + } + } + ], "properties": { + "blockId": { + "description": "Block ID for block-relative targeting.", + "type": "string" + }, + "offset": { + "description": "Character offset within the block identified by blockId.", + "minimum": 0, + "type": "integer" + }, "target": { "additionalProperties": false, "properties": { @@ -1356,7 +1385,12 @@ ] }, "possibleFailureCodes": ["INVALID_TARGET", "NO_OP"], - "preApplyThrows": ["TARGET_NOT_FOUND", "TRACK_CHANGE_COMMAND_UNAVAILABLE", "CAPABILITY_UNAVAILABLE"], + "preApplyThrows": [ + "TARGET_NOT_FOUND", + "TRACK_CHANGE_COMMAND_UNAVAILABLE", + "CAPABILITY_UNAVAILABLE", + "INVALID_TARGET" + ], "remediationHints": [], "successSchema": { "additionalProperties": false, diff --git a/packages/document-api/generated/schemas/document-api-contract.json b/packages/document-api/generated/schemas/document-api-contract.json index faf16ed93..2b61cb063 100644 --- a/packages/document-api/generated/schemas/document-api-contract.json +++ b/packages/document-api/generated/schemas/document-api-contract.json @@ -9278,7 +9278,36 @@ }, "inputSchema": { "additionalProperties": false, + "allOf": [ + { + "not": { + "required": ["target", "blockId"] + } + }, + { + "not": { + "required": ["target", "offset"] + } + }, + { + "if": { + "required": ["offset"] + }, + "then": { + "required": ["blockId"] + } + } + ], "properties": { + "blockId": { + "description": "Block ID for block-relative targeting.", + "type": "string" + }, + "offset": { + "description": "Character offset within the block identified by blockId.", + "minimum": 0, + "type": "integer" + }, "target": { "additionalProperties": false, "properties": { @@ -9322,7 +9351,12 @@ "supportsTrackedMode": true, "throws": { "postApplyForbidden": true, - "preApply": ["TARGET_NOT_FOUND", "TRACK_CHANGE_COMMAND_UNAVAILABLE", "CAPABILITY_UNAVAILABLE"] + "preApply": [ + "TARGET_NOT_FOUND", + "TRACK_CHANGE_COMMAND_UNAVAILABLE", + "CAPABILITY_UNAVAILABLE", + "INVALID_TARGET" + ] } }, "outputSchema": { @@ -13172,5 +13206,5 @@ } }, "sourceCommit": null, - "sourceHash": "3e7edc551feb80b42bfa59f928a0ce1f34ecb7cf71952d5a489107089105d543" + "sourceHash": "d53e75c131bc35996800d18ff3b67961de5d6d031ba20f4a6a03106e1117acaa" } diff --git a/packages/document-api/src/README.md b/packages/document-api/src/README.md index c4f9aae19..9a9918d49 100644 --- a/packages/document-api/src/README.md +++ b/packages/document-api/src/README.md @@ -43,7 +43,7 @@ lives in adapter layers that map engine behavior into `QueryResult` and other AP - For text selectors (`{ type: 'text', ... }`), `matches` are containing block addresses. - Exact matched spans are returned in `context[*].textRanges` as `TextAddress`. - Mutating operations should target `TextAddress` values from `context[*].textRanges`. -- `insert` also supports omitting `target`; adapters resolve a deterministic default insertion point (first paragraph start when available). +- `insert` supports three targeting modes: canonical `TextAddress`, block-relative (`blockId` + optional `offset`), or default insertion point when all target fields are omitted. - Structural creation is exposed under `create.*` (for example `create.paragraph`), separate from text mutations. ## Adapter Error Convention @@ -94,6 +94,18 @@ if (target) { } ``` +### Workflow: Block-Relative Insert + +Insert text at a specific position within a known block, without constructing a full `TextAddress`: + +```ts +// Insert at the start of a block +editor.doc.insert({ blockId: 'paragraph-1', text: 'Hello ' }); + +// Insert at a specific character offset within a block +editor.doc.insert({ blockId: 'paragraph-1', offset: 5, text: 'world' }); +``` + ### Workflow: Tracked-Mode Insert Insert text as a tracked change so reviewers can accept or reject it: @@ -208,9 +220,17 @@ Return document summary metadata (block count, word count, character count). ### `insert` -Insert text at an optional `TextAddress` target. When `target` is omitted, the adapter resolves a deterministic default insertion point. Supports dry-run and tracked mode. +Insert text at a target location. Supports three targeting modes: + +1. **Canonical target**: `{ target: TextAddress, text }` — full address with block ID and range. +2. **Block-relative**: `{ blockId, offset?, text }` — friendly shorthand. `offset` defaults to 0 when omitted. +3. **Default insertion point**: `{ text }` — no target; adapter resolves to first paragraph start. + +Exactly one targeting mode is allowed per call. Mixing `target` with `blockId`/`offset` throws `INVALID_TARGET`. `offset` without `blockId` throws `INVALID_TARGET`. `offset` must be a non-negative integer. + +Supports dry-run and tracked mode. -- **Input**: `InsertInput` (`{ target?, text }`) +- **Input**: `InsertInput` (`{ target?, blockId?, offset?, text }`) - **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 14ee4a473..45ae8955b 100644 --- a/packages/document-api/src/contract/contract.test.ts +++ b/packages/document-api/src/contract/contract.test.ts @@ -73,6 +73,19 @@ describe('document-api contract catalog', () => { } }); + it('encodes insert locator pairing and exclusivity constraints in the input schema', () => { + const schemas = buildInternalContractSchemas(); + const insertInputSchema = schemas.operations.insert.input as { + allOf?: Array>; + }; + + expect(insertInputSchema.allOf).toEqual([ + { not: { required: ['target', 'blockId'] } }, + { not: { required: ['target', 'offset'] } }, + { if: { required: ['offset'] }, then: { required: ['blockId'] } }, + ]); + }); + it('derives OPERATION_IDS from OPERATION_DEFINITIONS keys', () => { const definitionKeys = Object.keys(OPERATION_DEFINITIONS).sort(); const operationIds = [...OPERATION_IDS].sort(); diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index d5c6f9348..816935967 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -176,7 +176,7 @@ export const OPERATION_DEFINITIONS = { supportsDryRun: true, supportsTrackedMode: true, possibleFailureCodes: ['INVALID_TARGET', 'NO_OP'], - throws: T_NOT_FOUND_TRACKED, + throws: [...T_NOT_FOUND_TRACKED, 'INVALID_TARGET'], }), referenceDocPath: 'insert.mdx', referenceGroup: 'core', diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index b643f4233..d90d2a5a1 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -618,6 +618,22 @@ const capabilitiesOutputSchema = objectSchema( ); const strictEmptyObjectSchema = objectSchema({}); +const insertInputSchema: JsonSchema = { + ...objectSchema( + { + target: textAddressSchema, + text: { type: 'string' }, + blockId: { type: 'string', description: 'Block ID for block-relative targeting.' }, + offset: { type: 'integer', minimum: 0, description: 'Character offset within the block identified by blockId.' }, + }, + ['text'], + ), + allOf: [ + { not: { required: ['target', 'blockId'] } }, + { not: { required: ['target', 'offset'] } }, + { if: { required: ['offset'] }, then: { required: ['blockId'] } }, + ], +}; const operationSchemas: Record = { find: { @@ -647,13 +663,7 @@ const operationSchemas: Record = { output: documentInfoSchema, }, insert: { - input: objectSchema( - { - target: textAddressSchema, - text: { type: 'string' }, - }, - ['text'], - ), + input: insertInputSchema, output: textMutationResultSchemaFor('insert'), success: textMutationSuccessSchema, failure: textMutationFailureSchemaFor('insert'), diff --git a/packages/document-api/src/errors.ts b/packages/document-api/src/errors.ts new file mode 100644 index 000000000..cf90b4d52 --- /dev/null +++ b/packages/document-api/src/errors.ts @@ -0,0 +1,18 @@ +/** + * Structured validation error thrown by document-api execute* functions. + * + * Consumers should prefer checking `error.code` over `instanceof` for resilience + * across package boundaries and bundling scenarios. + */ +export class DocumentApiValidationError extends Error { + readonly code: string; + readonly details?: Record; + + constructor(code: string, message: string, details?: Record) { + super(message); + this.name = 'DocumentApiValidationError'; + this.code = code; + this.details = details; + Object.setPrototypeOf(this, DocumentApiValidationError.prototype); + } +} diff --git a/packages/document-api/src/index.test.ts b/packages/document-api/src/index.test.ts index 81082bae3..92fd76834 100644 --- a/packages/document-api/src/index.test.ts +++ b/packages/document-api/src/index.test.ts @@ -736,4 +736,304 @@ describe('createDocumentApi', () => { expect(directResult).toEqual(getResult); expect(capAdpt.get).toHaveBeenCalledTimes(2); }); + + describe('insert friendly locator validation', () => { + function makeApi() { + return createDocumentApi({ + find: makeFindAdapter(QUERY_RESULT), + getNode: makeGetNodeAdapter(PARAGRAPH_INFO), + getText: makeGetTextAdapter(), + info: makeInfoAdapter(), + capabilities: makeCapabilitiesAdapter(), + comments: makeCommentsAdapter(), + write: makeWriteAdapter(), + format: makeFormatAdapter(), + trackChanges: makeTrackChangesAdapter(), + create: makeCreateAdapter(), + lists: makeListsAdapter(), + }); + } + + function expectValidationError(fn: () => void, messageMatch?: string | RegExp) { + try { + fn(); + expect.fail('Expected DocumentApiValidationError to be thrown'); + } catch (err: unknown) { + const e = err as { name: string; code: string; message: string }; + expect(e.name).toBe('DocumentApiValidationError'); + expect(e.code).toBe('INVALID_TARGET'); + if (messageMatch) { + if (typeof messageMatch === 'string') { + expect(e.message).toContain(messageMatch); + } else { + expect(e.message).toMatch(messageMatch); + } + } + } + } + + // -- Truth table: valid cases -- + + it('accepts no-target (default insertion point)', () => { + const api = makeApi(); + const result = api.insert({ text: 'hello' }); + expect(result.success).toBe(true); + }); + + it('accepts canonical target', () => { + const api = makeApi(); + const target = { kind: 'text', blockId: 'p1', range: { start: 0, end: 0 } } as const; + const result = api.insert({ target, text: 'hello' }); + expect(result.success).toBe(true); + }); + + it('accepts blockId alone (offset defaults to 0)', () => { + const api = makeApi(); + const result = api.insert({ blockId: 'p1', text: 'hello' }); + expect(result.success).toBe(true); + }); + + it('accepts blockId + offset', () => { + const api = makeApi(); + const result = api.insert({ blockId: 'p1', offset: 5, text: 'hello' }); + expect(result.success).toBe(true); + }); + + it('accepts offset of 0', () => { + const api = makeApi(); + const result = api.insert({ blockId: 'p1', offset: 0, text: 'hello' }); + expect(result.success).toBe(true); + }); + + // -- Truth table: invalid cases -- + + it('rejects target + blockId (mixed modes)', () => { + const api = makeApi(); + const target = { kind: 'text', blockId: 'p1', range: { start: 0, end: 0 } } as const; + expectValidationError( + () => api.insert({ target, blockId: 'p2', text: 'hello' }), + 'Cannot combine target with blockId', + ); + }); + + it('rejects offset without blockId', () => { + const api = makeApi(); + expectValidationError(() => api.insert({ offset: 5, text: 'hello' } as any), 'offset requires blockId'); + }); + + it('rejects target + offset without blockId', () => { + const api = makeApi(); + const target = { kind: 'text', blockId: 'p1', range: { start: 0, end: 0 } } as const; + expectValidationError( + () => api.insert({ target, offset: 5, text: 'hello' } as any), + 'Cannot combine target with offset', + ); + }); + + it('rejects null target', () => { + const api = makeApi(); + expectValidationError( + () => api.insert({ target: null, text: 'hello' } as any), + 'target must be a text address object', + ); + }); + + it('rejects malformed target objects', () => { + const api = makeApi(); + expectValidationError( + () => api.insert({ target: { kind: 'text', blockId: 'p1' }, text: 'hello' } as any), + 'target must be a text address object', + ); + }); + + // -- Numeric bounds -- + + it('rejects negative offset', () => { + const api = makeApi(); + expectValidationError(() => api.insert({ blockId: 'p1', offset: -1, text: 'hello' }), 'non-negative integer'); + }); + + it('rejects non-integer offset', () => { + const api = makeApi(); + expectValidationError(() => api.insert({ blockId: 'p1', offset: 1.5, text: 'hello' }), 'non-negative integer'); + }); + + // -- Type checks -- + + it('rejects non-string blockId', () => { + const api = makeApi(); + expectValidationError(() => api.insert({ blockId: 42, text: 'hello' } as any), 'blockId must be a string'); + }); + + it('rejects non-string text', () => { + const api = makeApi(); + expectValidationError(() => api.insert({ text: 42 } as any), 'text must be a string'); + }); + + // -- Validation error shape -- + + it('throws DocumentApiValidationError (not plain Error)', () => { + const api = makeApi(); + try { + api.insert({ offset: 5, text: 'hello' } as any); + expect.fail('Expected error'); + } catch (err: unknown) { + expect((err as Error).constructor.name).toBe('DocumentApiValidationError'); + expect((err as { code: string }).code).toBe('INVALID_TARGET'); + } + }); + + // -- Input shape guard -- + + it('rejects null input', () => { + const api = makeApi(); + expectValidationError(() => api.insert(null as any), 'non-null object'); + }); + + it('rejects numeric input', () => { + const api = makeApi(); + expectValidationError(() => api.insert(42 as any), 'non-null object'); + }); + + it('rejects undefined input', () => { + const api = makeApi(); + expectValidationError(() => api.insert(undefined as any), 'non-null object'); + }); + + // -- pos runtime rejection -- + + it('rejects pos (not yet supported)', () => { + const api = makeApi(); + expectValidationError(() => api.insert({ text: 'hi', pos: 3 } as any), 'pos locator is not yet supported'); + }); + + // -- Validation precedence: pos before unknown-field -- + + it('returns pos error before unknown-field error', () => { + const api = makeApi(); + expectValidationError( + () => api.insert({ text: 'hi', pos: 3, block_id: 'x' } as any), + 'pos locator is not yet supported', + ); + }); + + it('returns pos error before mode-exclusivity error', () => { + const api = makeApi(); + expectValidationError( + () => api.insert({ text: 'hi', pos: 3, blockId: 'x' } as any), + 'pos locator is not yet supported', + ); + }); + + // -- Unknown field rejection -- + + it('rejects unknown top-level fields', () => { + const api = makeApi(); + expectValidationError(() => api.insert({ text: 'hi', block_id: 'abc' } as any), 'Unknown field "block_id"'); + }); + + // -- Backward compatibility parity -- + + it('sends same adapter request for insert({ text }) as before', () => { + const writeAdpt = makeWriteAdapter(); + const api = createDocumentApi({ + find: makeFindAdapter(QUERY_RESULT), + getNode: makeGetNodeAdapter(PARAGRAPH_INFO), + getText: makeGetTextAdapter(), + info: makeInfoAdapter(), + capabilities: makeCapabilitiesAdapter(), + comments: makeCommentsAdapter(), + write: writeAdpt, + format: makeFormatAdapter(), + trackChanges: makeTrackChangesAdapter(), + create: makeCreateAdapter(), + lists: makeListsAdapter(), + }); + + api.insert({ text: 'hello' }); + expect(writeAdpt.write).toHaveBeenCalledWith( + { kind: 'insert', text: 'hello' }, + { changeMode: 'direct', dryRun: false }, + ); + }); + + it('sends same adapter request for insert({ target, text }) as before', () => { + const writeAdpt = makeWriteAdapter(); + const api = createDocumentApi({ + find: makeFindAdapter(QUERY_RESULT), + getNode: makeGetNodeAdapter(PARAGRAPH_INFO), + getText: makeGetTextAdapter(), + info: makeInfoAdapter(), + capabilities: makeCapabilitiesAdapter(), + comments: makeCommentsAdapter(), + write: writeAdpt, + format: makeFormatAdapter(), + trackChanges: makeTrackChangesAdapter(), + create: makeCreateAdapter(), + lists: makeListsAdapter(), + }); + + const target = { kind: 'text', blockId: 'p1', range: { start: 0, end: 2 } } as const; + api.insert({ target, text: 'hello' }); + expect(writeAdpt.write).toHaveBeenCalledWith( + { kind: 'insert', target, text: 'hello' }, + { changeMode: 'direct', dryRun: false }, + ); + }); + + it('passes blockId + offset through to adapter for normalization', () => { + const writeAdpt = makeWriteAdapter(); + const api = createDocumentApi({ + find: makeFindAdapter(QUERY_RESULT), + getNode: makeGetNodeAdapter(PARAGRAPH_INFO), + getText: makeGetTextAdapter(), + info: makeInfoAdapter(), + capabilities: makeCapabilitiesAdapter(), + comments: makeCommentsAdapter(), + write: writeAdpt, + format: makeFormatAdapter(), + trackChanges: makeTrackChangesAdapter(), + create: makeCreateAdapter(), + lists: makeListsAdapter(), + }); + + api.insert({ blockId: 'p1', offset: 5, text: 'hello' }); + expect(writeAdpt.write).toHaveBeenCalledWith( + { + kind: 'insert', + blockId: 'p1', + offset: 5, + text: 'hello', + }, + { changeMode: 'direct', dryRun: false }, + ); + }); + + it('passes blockId without offset through to adapter', () => { + const writeAdpt = makeWriteAdapter(); + const api = createDocumentApi({ + find: makeFindAdapter(QUERY_RESULT), + getNode: makeGetNodeAdapter(PARAGRAPH_INFO), + getText: makeGetTextAdapter(), + info: makeInfoAdapter(), + capabilities: makeCapabilitiesAdapter(), + comments: makeCommentsAdapter(), + write: writeAdpt, + format: makeFormatAdapter(), + trackChanges: makeTrackChangesAdapter(), + create: makeCreateAdapter(), + lists: makeListsAdapter(), + }); + + api.insert({ blockId: 'p1', text: 'hello' }); + expect(writeAdpt.write).toHaveBeenCalledWith( + { + kind: 'insert', + blockId: 'p1', + text: 'hello', + }, + { changeMode: 'direct', dryRun: false }, + ); + }); + }); }); diff --git a/packages/document-api/src/index.ts b/packages/document-api/src/index.ts index 0b9eb3de9..4025d6e48 100644 --- a/packages/document-api/src/index.ts +++ b/packages/document-api/src/index.ts @@ -178,6 +178,8 @@ export type { SetCommentInternalInput, } from './comments/comments.js'; export type { CommentInfo, CommentsListQuery, CommentsListResult } from './comments/comments.types.js'; +export { DocumentApiValidationError } from './errors.js'; +export type { InsertInput } from './insert/insert.js'; /** * Callable capability accessor returned by `createDocumentApi`. diff --git a/packages/document-api/src/insert/insert.ts b/packages/document-api/src/insert/insert.ts index b603c8331..5e5aaa8f5 100644 --- a/packages/document-api/src/insert/insert.ts +++ b/packages/document-api/src/insert/insert.ts @@ -1,9 +1,143 @@ import { executeWrite, type MutationOptions, type WriteAdapter } from '../write/write.js'; import type { TextAddress, TextMutationReceipt } from '../types/index.js'; +import { DocumentApiValidationError } from '../errors.js'; export interface InsertInput { target?: TextAddress; text: string; + /** Block ID for block-relative targeting. When provided without `offset`, offset defaults to 0. */ + blockId?: string; + /** Character offset within the block identified by `blockId`. Requires `blockId`. Must be a non-negative integer. */ + offset?: number; +} + +/** + * Strict top-level allowlist for InsertInput fields. + * Any key not in this list is rejected as an unknown field. + * PR B adds 'pos' to this list. + */ +const INSERT_INPUT_ALLOWED_KEYS = new Set(['text', 'target', 'blockId', 'offset']); + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value != null && !Array.isArray(value); +} + +function isInteger(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value); +} + +function isTextAddress(value: unknown): value is TextAddress { + if (!isRecord(value)) return false; + if (value.kind !== 'text') return false; + if (typeof value.blockId !== 'string') return false; + + const range = value.range; + if (!isRecord(range)) return false; + if (!isInteger(range.start) || !isInteger(range.end)) return false; + return range.start <= range.end; +} + +/** + * Validates InsertInput and throws DocumentApiValidationError on violations. + * + * This is the first input validation in an execute* function in the document-api — + * a new pattern. Previously all validation lived in the adapter layer. + * + * Validation order: + * 0. Input shape guard (must be non-null plain object) + * 1. `pos` runtime rejection (PR A: not yet supported) + * 2. Unknown field rejection (strict allowlist) + * 3. Target/type checks (target shape, text and blockId types) + * 4. Mode exclusivity (at most one locator mode) + * 5. Required-pair checks (offset requires blockId) + * 6. Numeric bounds (offset >= 0, integer) + */ +function validateInsertInput(input: unknown): asserts input is InsertInput { + // Step 0: Input shape guard + if (!isRecord(input)) { + throw new DocumentApiValidationError('INVALID_TARGET', 'Insert input must be a non-null object.'); + } + + const inputObj = input; + + // Step 1: pos runtime rejection (PR A — pos is not yet supported) + if ('pos' in inputObj) { + throw new DocumentApiValidationError('INVALID_TARGET', 'pos locator is not yet supported.', { + field: 'pos', + }); + } + + // Step 2: Unknown field rejection (strict allowlist) + for (const key of Object.keys(inputObj)) { + if (!INSERT_INPUT_ALLOWED_KEYS.has(key)) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `Unknown field "${key}" on insert input. Allowed fields: ${[...INSERT_INPUT_ALLOWED_KEYS].join(', ')}.`, + { field: key }, + ); + } + } + + const { target, text, blockId, offset } = inputObj; + const hasTarget = target !== undefined; + const hasBlockId = blockId !== undefined; + const hasOffset = offset !== undefined; + + // Step 3: Target/type checks + if (hasTarget && !isTextAddress(target)) { + throw new DocumentApiValidationError('INVALID_TARGET', 'target must be a text address object.', { + field: 'target', + value: target, + }); + } + + if (typeof text !== 'string') { + throw new DocumentApiValidationError('INVALID_TARGET', `text must be a string, got ${typeof text}.`, { + field: 'text', + value: text, + }); + } + + if (hasBlockId && typeof blockId !== 'string') { + throw new DocumentApiValidationError('INVALID_TARGET', `blockId must be a string, got ${typeof blockId}.`, { + field: 'blockId', + value: blockId, + }); + } + + // Step 4: Mode exclusivity — at most one locator mode + if (hasTarget && hasBlockId) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + 'Cannot combine target with blockId. Use exactly one locator mode.', + { fields: ['target', 'blockId'] }, + ); + } + if (hasTarget && hasOffset) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + 'Cannot combine target with offset. Use exactly one locator mode.', + { fields: ['target', 'offset'] }, + ); + } + + // Step 5: Required-pair checks — offset requires blockId + if (hasOffset && !hasBlockId) { + throw new DocumentApiValidationError('INVALID_TARGET', 'offset requires blockId.', { + fields: ['offset', 'blockId'], + }); + } + + // Step 6: Numeric bounds — offset must be a non-negative integer + if (hasOffset) { + if (typeof offset !== 'number' || !Number.isInteger(offset) || offset < 0) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `offset must be a non-negative integer, got ${JSON.stringify(offset)}.`, + { field: 'offset', value: offset }, + ); + } + } } export function executeInsert( @@ -11,16 +145,18 @@ export function executeInsert( input: InsertInput, options?: MutationOptions, ): TextMutationReceipt { - const request = input.target - ? { - kind: 'insert' as const, - target: input.target, - text: input.text, - } - : { - kind: 'insert' as const, - text: input.text, - }; + validateInsertInput(input); + + const { target, blockId, offset, text } = input; + + // Pass friendly locator fields through to the adapter for normalization. + // The adapter is responsible for converting blockId + offset into a canonical TextAddress. + if (blockId !== undefined) { + return executeWrite(adapter, { kind: 'insert', blockId, offset, text }, options); + } + + // Canonical target or no-target (default insertion point) + const request = target ? { kind: 'insert' as const, target, text } : { kind: 'insert' as const, text }; return executeWrite(adapter, request, options); } diff --git a/packages/document-api/src/write/locator.ts b/packages/document-api/src/write/locator.ts new file mode 100644 index 000000000..43a94315e --- /dev/null +++ b/packages/document-api/src/write/locator.ts @@ -0,0 +1,14 @@ +/** + * Shared internal locator types for friendly target resolution. + * + * These types capture the `blockId + offset` pattern (and `pos` in PR B) + * for use by write operations (insert, and later replace/delete). + * + * NOT exported from the package root — internal use only. + */ + +/** Block-relative locator: a block ID with an optional character offset. */ +export interface BlockRelativeLocator { + blockId: string; + offset?: number; +} diff --git a/packages/document-api/src/write/write.ts b/packages/document-api/src/write/write.ts index dd7abb799..8559de95d 100644 --- a/packages/document-api/src/write/write.ts +++ b/packages/document-api/src/write/write.ts @@ -1,4 +1,5 @@ import type { TextAddress, TextMutationReceipt } from '../types/index.js'; +import type { BlockRelativeLocator } from './locator.js'; export type ChangeMode = 'direct' | 'tracked'; @@ -25,7 +26,7 @@ export type InsertWriteRequest = { */ target?: TextAddress; text: string; -}; +} & Partial; export type ReplaceWriteRequest = { kind: 'replace'; diff --git a/packages/super-editor/src/document-api-adapters/write-adapter.test.ts b/packages/super-editor/src/document-api-adapters/write-adapter.test.ts index 1ce963985..b43926326 100644 --- a/packages/super-editor/src/document-api-adapters/write-adapter.test.ts +++ b/packages/super-editor/src/document-api-adapters/write-adapter.test.ts @@ -705,4 +705,143 @@ describe('writeAdapter', () => { expect(trackedReceipt.success).toBe(true); expect(insertTrackedChange).toHaveBeenCalledTimes(1); }); + + // -- blockId + offset adapter normalization -- + + it('normalizes blockId + offset to TextAddress and inserts at the correct position', () => { + const { editor, dispatch, tr } = makeEditor('Hello'); + + const receipt = writeAdapter( + editor, + { + kind: 'insert', + blockId: 'p1', + offset: 3, + text: 'X', + }, + { changeMode: 'direct' }, + ); + + expect(receipt.success).toBe(true); + expect(receipt.resolution.target).toEqual({ + kind: 'text', + blockId: 'p1', + range: { start: 3, end: 3 }, + }); + // PM position = block start (1) + offset (3) = 4 + expect(tr.insertText).toHaveBeenCalledWith('X', 4, 4); + expect(dispatch).toHaveBeenCalledTimes(1); + }); + + it('normalizes blockId without offset to offset 0', () => { + const { editor, dispatch, tr } = makeEditor('Hello'); + + const receipt = writeAdapter( + editor, + { + kind: 'insert', + blockId: 'p1', + text: 'X', + }, + { changeMode: 'direct' }, + ); + + expect(receipt.success).toBe(true); + expect(receipt.resolution.target).toEqual({ + kind: 'text', + blockId: 'p1', + range: { start: 0, end: 0 }, + }); + expect(tr.insertText).toHaveBeenCalledWith('X', 1, 1); + expect(dispatch).toHaveBeenCalledTimes(1); + }); + + it('throws TARGET_NOT_FOUND for unknown blockId via friendly locator', () => { + const { editor } = makeEditor('Hello'); + + expect(() => + writeAdapter( + editor, + { + kind: 'insert', + blockId: 'missing', + text: 'X', + }, + { changeMode: 'direct' }, + ), + ).toThrow('Mutation target could not be resolved.'); + }); + + it('throws INVALID_TARGET when blockId and target are both present (defensive adapter validation)', () => { + const { editor } = makeEditor('Hello'); + + expect(() => + writeAdapter( + editor, + { + kind: 'insert', + target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 0 } }, + blockId: 'p1', + text: 'X', + }, + { changeMode: 'direct' }, + ), + ).toThrow('Cannot combine target with blockId'); + }); + + it('throws INVALID_TARGET when offset is present without blockId (defensive adapter validation)', () => { + const { editor } = makeEditor('Hello'); + + expect(() => + writeAdapter( + editor, + { + kind: 'insert', + offset: 5, + text: 'X', + } as any, + { changeMode: 'direct' }, + ), + ).toThrow('offset requires blockId'); + }); + + it('throws INVALID_TARGET when offset is mixed with canonical target (defensive adapter validation)', () => { + const { editor } = makeEditor('Hello'); + + expect(() => + writeAdapter( + editor, + { + kind: 'insert', + target: { kind: 'text', blockId: 'p1', range: { start: 0, end: 0 } }, + offset: 3, + text: 'X', + } as any, + { changeMode: 'direct' }, + ), + ).toThrow('Cannot combine target with offset'); + }); + + it('normalizes blockId + offset for tracked inserts', () => { + const { editor, insertTrackedChange } = makeEditor('Hello'); + + const receipt = writeAdapter( + editor, + { + kind: 'insert', + blockId: 'p1', + offset: 2, + text: 'X', + }, + { changeMode: 'tracked' }, + ); + + expect(receipt.success).toBe(true); + expect(insertTrackedChange).toHaveBeenCalledTimes(1); + expect(insertTrackedChange.mock.calls[0]?.[0]).toMatchObject({ + from: 3, + to: 3, + text: 'X', + }); + }); }); 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 0d55fd8db..46a567ef6 100644 --- a/packages/super-editor/src/document-api-adapters/write-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/write-adapter.ts @@ -68,6 +68,51 @@ type ResolvedWriteTarget = { resolution: ReturnType; }; +/** + * Normalize block-relative locator fields (blockId + offset) into a canonical TextAddress. + * This runs inside the adapter layer so that the resolution uses engine-specific block lookup. + * + * Returns the original request unchanged when no friendly locator is present. + */ +function normalizeInsertLocator(request: WriteRequest): WriteRequest { + if (request.kind !== 'insert') return request; + + const hasBlockId = request.blockId !== undefined; + const hasOffset = request.offset !== undefined; + + // Defensive: reject offset mixed with canonical target. + if (hasOffset && request.target) { + throw new DocumentApiAdapterError('INVALID_TARGET', 'Cannot combine target with offset on insert request.', { + fields: ['target', 'offset'], + }); + } + + // Defensive: reject orphaned offset without blockId (safety net for direct adapter callers). + if (hasOffset && !hasBlockId) { + throw new DocumentApiAdapterError('INVALID_TARGET', 'offset requires blockId on insert request.', { + fields: ['offset', 'blockId'], + }); + } + + if (!hasBlockId) return request; + + // Defensive: reject mixed locator modes at adapter boundary (safety net). + 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 }; +} + function resolveWriteTarget(editor: Editor, request: WriteRequest): ResolvedWriteTarget | null { const requestedTarget = request.target; @@ -184,14 +229,18 @@ function toFailureReceipt(failure: ReceiptFailure, resolvedTarget: ResolvedWrite } export function writeAdapter(editor: Editor, request: WriteRequest, options?: MutationOptions): TextMutationReceipt { - const resolvedTarget = resolveWriteTarget(editor, request); + // Normalize friendly locator fields (blockId + offset) into canonical TextAddress + // before resolution. This is the adapter-layer normalization per the contract. + const normalizedRequest = normalizeInsertLocator(request); + + const resolvedTarget = resolveWriteTarget(editor, normalizedRequest); if (!resolvedTarget) { throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'Mutation target could not be resolved.', { - target: request.target, + target: normalizedRequest.target, }); } - const validationFailure = validateWriteRequest(request, resolvedTarget); + const validationFailure = validateWriteRequest(normalizedRequest, resolvedTarget); if (validationFailure) { return toFailureReceipt(validationFailure, resolvedTarget); } @@ -203,8 +252,8 @@ export function writeAdapter(editor: Editor, request: WriteRequest, options?: Mu } if (mode === 'tracked') { - return applyTrackedWrite(editor, request, resolvedTarget); + return applyTrackedWrite(editor, normalizedRequest, resolvedTarget); } - return applyDirectWrite(editor, request, resolvedTarget); + return applyDirectWrite(editor, normalizedRequest, resolvedTarget); }