From 9f4d9dd0c7e2ff9c67d1f250a9d9578a1be19dbc Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 10 Mar 2026 22:36:05 -0700 Subject: [PATCH 1/4] feat: cli improvements, block deletion --- apps/cli/README.md | 150 +++++++-- .../src/__tests__/command-examples.test.ts | 247 +++++++++++++++ apps/cli/src/__tests__/conformance/harness.ts | 30 ++ .../src/__tests__/conformance/scenarios.ts | 27 ++ apps/cli/src/__tests__/lib/validate.test.ts | 40 +-- .../src/cli/cli-only-operation-definitions.ts | 35 ++- apps/cli/src/cli/command-examples.ts | 143 +++++++++ apps/cli/src/cli/commands.ts | 12 +- apps/cli/src/cli/helper-commands.ts | 8 +- apps/cli/src/cli/operation-hints.ts | 8 + apps/cli/src/cli/operation-params.ts | 13 +- apps/cli/src/commands/open.ts | 2 +- apps/cli/src/lib/create-paragraph-input.ts | 24 +- apps/cli/src/lib/error-mapping.ts | 15 +- apps/cli/src/lib/errors.ts | 4 +- apps/cli/src/lib/introspection-dispatch.ts | 4 + apps/cli/src/lib/operation-wrapper-input.ts | 2 + apps/cli/src/lib/validate.ts | 51 +-- packages/document-api/src/blocks/blocks.ts | 132 +++++++- .../src/contract/operation-definitions.ts | 61 +++- .../src/contract/operation-registry.ts | 11 +- packages/document-api/src/contract/schemas.ts | 86 +++++ .../document-api/src/history/history.types.ts | 5 + packages/document-api/src/index.ts | 21 +- packages/document-api/src/invoke/invoke.ts | 2 + .../document-api/src/types/blocks.types.ts | 58 +++- .../contract-conformance.test.ts | 128 +++++++- .../assemble-adapters.ts | 4 +- .../helpers/adapter-utils.ts | 27 +- .../helpers/node-address-resolver.ts | 2 +- .../history-adapter.test.ts | 41 +++ .../document-api-adapters/history-adapter.ts | 10 + .../plan-engine/blocks-wrappers.test.ts | 287 ++++++++++++++++- .../plan-engine/blocks-wrappers.ts | 293 +++++++++++++++--- .../plan-engine/create-insertion.test.ts | 158 +++++++++- .../plan-engine/create-insertion.ts | 69 ++++- .../plan-engine/create-wrappers.ts | 20 +- .../plan-engine/images-wrappers.ts | 7 +- .../plan-engine/toc-wrappers.ts | 4 +- 39 files changed, 2038 insertions(+), 203 deletions(-) create mode 100644 apps/cli/src/__tests__/command-examples.test.ts create mode 100644 apps/cli/src/cli/command-examples.ts diff --git a/apps/cli/README.md b/apps/cli/README.md index 969fac2b85..e535db767c 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -30,31 +30,112 @@ Stateful editing flow (recommended for multi-step edits): ```bash superdoc open ./contract.docx -superdoc find --type text --pattern "termination" -superdoc replace --target-json '{"blockId":"p1","range":{"start":0,"end":11}}' --text "expiration" + +# Use query match to find a mutation-grade target +superdoc query match --select-json '{"type":"text","pattern":"termination"}' --require exactlyOne + +# Mutate using the returned target +superdoc replace --target-json '{"kind":"text","blockId":"p1","range":{"start":0,"end":11}}' --text "expiration" + superdoc save --in-place superdoc close ``` -Legacy compatibility commands (v0.x behavior): +## Choosing the Right Command + +### Which command should I use? + +| I want to... | Use this command | +|--------------|------------------| +| Find a mutation target (block ID, text range) | `query match` | +| Search/browse document content | `find` | +| Insert inline text within a block | `insert` | +| Create a new standalone paragraph | `create paragraph` | +| Create a new heading | `create heading` | +| Insert a list item before/after another list item | `lists insert` | +| Apply formatting to a text range | `format apply` or format helpers (`format bold`, etc.) | +| Apply multiple changes in one operation | `mutations apply` | + +### Mutation targeting workflow + +Always use `query match` to discover targets before mutating: ```bash -superdoc search "indemnification" ./contracts/*.docx -superdoc replace-legacy "ACME Corp" "Globex Inc" ./merger/*.docx -superdoc read ./proposal.docx +# Step 1: Find the target +superdoc query match --select-json '{"type":"text","pattern":"Introduction"}' --require exactlyOne + +# Step 2: Use the returned address in a mutation +superdoc replace --block-id --start --end --text "Overview" ``` +`find` is for content discovery and inspection. `query match` is for mutation targeting — it returns exact addresses with cardinality guarantees. + +### Block-oriented editing workflow + +Use `blocks list` for ordered inspection, then `blocks delete-range` for contiguous removal: + +```bash +superdoc open ./contract.docx + +# 1. Inspect block order, IDs, and text previews +superdoc blocks list --limit 30 + +# 2. Preview the deletion (no mutation, shows what would be removed) +superdoc blocks delete-range \ + --start-json '{"kind":"block","nodeType":"paragraph","nodeId":"abc123"}' \ + --end-json '{"kind":"block","nodeType":"paragraph","nodeId":"def456"}' \ + --dry-run + +# 3. Apply the deletion +superdoc blocks delete-range \ + --start-json '{"kind":"block","nodeType":"paragraph","nodeId":"abc123"}' \ + --end-json '{"kind":"block","nodeType":"paragraph","nodeId":"def456"}' + +superdoc save --in-place +``` + +This replaces the pattern of calling `blocks delete` once per block. A 17-block removal becomes one command. + +### Preview-before-apply workflow + +Use `--dry-run` and `--expected-revision` for safe, auditable mutations: + +```bash +superdoc open ./contract.docx + +# 1. Check session state +superdoc status + +# 2. Find the mutation target +superdoc query match --select-json '{"type":"text","pattern":"termination"}' --require exactlyOne + +# 3. Preview the change (validates input, shows what would change, no mutation) +superdoc replace --block-id p1 --start 0 --end 11 --text "expiration" --dry-run + +# 4. Apply with revision guard (fails if document changed since preview) +superdoc replace --block-id p1 --start 0 --end 11 --text "expiration" --expected-revision 1 + +superdoc save --in-place +``` + +### Common mistakes + +1. **Do not use `find` output to construct mutation targets.** `find` returns discovery-grade data, not mutation-grade addresses. Use `query match` instead. +2. **Do not use `insert --block-id` for sibling block insertion.** `insert` inserts inline text *within* a block. To create a new block adjacent to another, use `create paragraph`, `create heading`, or `lists insert`. +3. **Do not use `create paragraph` to continue a list.** If you want to add a list item adjacent to existing list items, use `lists insert`. `create paragraph` creates a standalone (non-list) paragraph. + ## Command Index | Category | Commands | |----------|----------| -| query | `find`, `get-node`, `get-node-by-id`, `info` | -| mutation | `insert`, `replace`, `delete` | -| format | `format bold` | -| create | `create paragraph` | -| lists | `lists list`, `lists get`, `lists insert`, `lists set-type`, `lists indent`, `lists outdent`, `lists restart`, `lists exit` | -| comments | `comments add`, `comments edit`, `comments reply`, `comments move`, `comments resolve`, `comments remove`, `comments set-internal`, `comments set-active`, `comments go-to`, `comments get`, `comments list` | +| query | `find`, `query match`, `get-node`, `get-node-by-id`, `get-text`, `info` | +| mutation | `insert`, `replace`, `delete`, `blocks delete`, `blocks delete-range`, `blocks list`, `mutations apply`, `mutations preview` | +| format | `format apply`, `format bold`, `format italic`, `format underline`, `format strikethrough` | +| create | `create paragraph`, `create heading`, `create table-of-contents` | +| lists | `lists list`, `lists get`, `lists insert`, `lists create`, `lists attach`, `lists detach`, `lists join`, `lists separate`, `lists set-level`, `lists indent`, `lists outdent`, `lists set-value`, `lists set-type`, `lists convert-to-text` | +| comments | `comments add`, `comments reply`, `comments delete`, `comments get`, `comments list` | | trackChanges | `track-changes list`, `track-changes get`, `track-changes accept`, `track-changes reject`, `track-changes accept-all`, `track-changes reject-all` | +| history | `history get`, `history undo`, `history redo` | | lifecycle | `open`, `save`, `close` | | session | `session list`, `session save`, `session close`, `session set-default`, `session use` | | introspection | `status`, `describe`, `describe command` | @@ -65,6 +146,7 @@ For full command help and examples, run: ```bash superdoc --help +superdoc describe command ``` ## v1 Breaking Changes @@ -73,7 +155,7 @@ This CLI replaces the previous `@superdoc-dev/cli` package surface with the v1 c | Legacy command | v1 status | Migration | |---------------|-----------|-----------| -| `superdoc replace ` | Renamed to `replace-legacy` | Use `replace-legacy`, or use `find` + `replace --target-json` for the v1 workflow. | +| `superdoc replace ` | Renamed to `replace-legacy` | Use `replace-legacy`, or use `query match` + `replace --target-json` for the v1 workflow. | Legacy compatibility is retained for `search`, `read`, and `replace-legacy`. @@ -116,7 +198,7 @@ superdoc status ```bash superdoc open ./contract.docx superdoc status -superdoc find --type text --pattern "termination" +superdoc query match --select-json '{"type":"text","pattern":"termination"}' --require exactlyOne superdoc replace --target-json '{...}' --text "Updated clause" superdoc save --in-place superdoc close @@ -140,28 +222,45 @@ superdoc session close [--discard] ```bash superdoc info [] -superdoc find [] --type text --pattern "termination" -superdoc find [] --type run +superdoc find [] --type text --pattern "termination" --limit 5 +superdoc query match [] --select-json '{"type":"text","pattern":"termination"}' --require exactlyOne superdoc get-node [] --address-json '{"kind":"block","nodeType":"paragraph","nodeId":"p1"}' superdoc get-node-by-id [] --id p1 --node-type paragraph ``` -- Flat `find` flags are convenience syntax and are normalized into the canonical query object used by `editor.doc.find`. -- Use `--query-json` / `--query-file` for complex or programmatic queries. -- For text queries, use `result.context[*].textRanges[*]` as targets for `replace`, `comments add`, and formatting commands. +- `find` returns discovery-grade results for content search and browsing. +- `query match` returns mutation-grade addresses and text ranges — use this before any mutation. +- For text queries, use the returned `blocks[].range` as targets for `replace`, `comments add`, and formatting commands. ## Mutating commands ```bash -superdoc comments add [] --target-json '{...}' --text "Please revise" [--out ./with-comment.docx] superdoc replace [] --target-json '{...}' --text "Updated text" [--out ./updated.docx] +superdoc insert [] --value "New text" [--out ./inserted.docx] +superdoc blocks delete [] --node-type paragraph --node-id abc123 +superdoc blocks delete-range --start-json '{"kind":"block",...}' --end-json '{"kind":"block",...}' +superdoc create paragraph [] --text "New paragraph" [--at-json '{"kind":"after","target":{"kind":"block","nodeType":"paragraph","nodeId":"p1"}}'] +superdoc lists insert [] --node-id li1 --position after --text "New item" superdoc format bold [] --target-json '{...}' [--out ./bolded.docx] +superdoc comments add [] --block-id p1 --start 0 --end 10 --text "Please revise" [--out ./with-comment.docx] ``` - In stateless mode (`` provided), mutating commands require `--out`. - In stateful mode (after `open`), mutating commands update the active working document and `--out` is optional. +- Use `--dry-run` to preview any mutation without applying it. - Use `--expected-revision ` with stateful mutating commands for optimistic concurrency checks. +## Block inspection + +```bash +superdoc blocks list +superdoc blocks list --limit 20 --offset 10 +superdoc blocks list --node-types-json '["paragraph","heading"]' +``` + +- Returns ordered block metadata: ordinal, nodeId, nodeType, textPreview, isEmpty. +- Use the returned nodeIds as targets for `blocks delete`, `blocks delete-range`, or other block-oriented commands. + ## Low-level invocation ```bash @@ -215,6 +314,7 @@ superdoc info ./contract.docx --pretty - `--query-json`, `--query-file` - `--address-json`, `--address-file` - `--target-json`, `--target-file` +- `--at-json`, `--at-file` (for `create paragraph`) ## Stdin support @@ -249,8 +349,14 @@ Error: { "ok": false, "error": { - "code": "VALIDATION_ERROR", - "message": "..." + "code": "INVALID_TARGET", + "message": "Expected paragraph:abc123 but found listItem:abc123.", + "details": { + "requestedNodeType": "paragraph", + "actualNodeType": "listItem", + "nodeId": "abc123", + "remediation": "Use lists.insert to add an item to a list sequence." + } }, "meta": { "version": "1.0.0", diff --git a/apps/cli/src/__tests__/command-examples.test.ts b/apps/cli/src/__tests__/command-examples.test.ts new file mode 100644 index 0000000000..9db312cd05 --- /dev/null +++ b/apps/cli/src/__tests__/command-examples.test.ts @@ -0,0 +1,247 @@ +/** + * Smoke test: every example in DOC_COMMAND_EXAMPLES and CLI_HELPER_COMMANDS + * must parse cleanly through the real CLI argument parser. + * + * This guards against example drift by running each example through: + * 1. Shell-accurate tokenization (strip quotes, split on spaces) + * 2. Command-token stripping (same as the CLI router) + * 3. `parseCommandArgs` with the operation's full option spec set + * 4. `ensureValidArgs` — rejects unknown flags and type mismatches + * 5. JSON payload validation — every --*-json flag must contain valid JSON + * + * If a flag is renamed, removed, misspelled, or a JSON payload is malformed, + * this test catches it before the example ships in `describe command` output. + */ + +import { describe, expect, test } from 'bun:test'; +import { DOC_COMMAND_EXAMPLES } from '../cli/command-examples'; +import { CLI_HELPER_COMMANDS } from '../cli/helper-commands'; +import { CLI_COMMAND_SPECS } from '../cli/commands'; +import { CLI_OPERATION_OPTION_SPECS } from '../cli/operation-params'; +import { parseCommandArgs, ensureValidArgs } from '../lib/args'; +import type { CliOperationId } from '../cli/operation-set'; +import type { OptionSpec } from '../lib/args'; + +// --------------------------------------------------------------------------- +// Wrapper-added extra option specs not in CLI_OPERATION_OPTION_SPECS. +// These are added at runtime by parseWrapperOperationInput for specific ops. +// Mirror them here so the smoke test covers the full flag surface. +// --------------------------------------------------------------------------- + +const WRAPPER_EXTRA_SPECS: Partial> = { + 'doc.find': [ + { name: 'type', type: 'string' }, + { name: 'node-type', type: 'string' }, + { name: 'kind', type: 'string' }, + { name: 'pattern', type: 'string' }, + { name: 'mode', type: 'string' }, + { name: 'case-sensitive', type: 'boolean' }, + { name: 'query-json', type: 'string' }, + { name: 'query-file', type: 'string' }, + { name: 'within-json', type: 'string' }, + { name: 'within-file', type: 'string' }, + ], + 'doc.getNode': [{ name: 'address-file', type: 'string' }], + 'doc.lists.get': [{ name: 'address-file', type: 'string' }], + 'doc.lists.list': [ + { name: 'kind', type: 'string' }, + { name: 'level', type: 'number' }, + { name: 'ordinal', type: 'number' }, + { name: 'query-json', type: 'string' }, + { name: 'query-file', type: 'string' }, + { name: 'within-json', type: 'string' }, + { name: 'within-file', type: 'string' }, + ], + 'doc.create.paragraph': [ + { name: 'input-file', type: 'string' }, + { name: 'text', type: 'string' }, + { name: 'at', type: 'string' }, + { name: 'at-json', type: 'string' }, + { name: 'at-file', type: 'string' }, + { name: 'before-address-json', type: 'string' }, + { name: 'before-address-file', type: 'string' }, + { name: 'after-address-json', type: 'string' }, + { name: 'after-address-file', type: 'string' }, + { name: 'tracked', type: 'boolean' }, + { name: 'direct', type: 'boolean' }, + ], +}; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Shell-accurate tokenization of a CLI example string. + * Handles single-quoted JSON payloads and double-quoted strings. + * Single quotes are stripped (as a shell would) so the parser sees raw values. + */ +function shellTokenize(example: string): string[] { + const tokens: string[] = []; + let current = ''; + let inSingleQuote = false; + let inDoubleQuote = false; + + for (let i = 0; i < example.length; i++) { + const ch = example[i]!; + + if (ch === '\\' && !inSingleQuote && i + 1 < example.length) { + current += example[i + 1]; + i++; + continue; + } + + if (ch === "'" && !inDoubleQuote) { + inSingleQuote = !inSingleQuote; + // Shell strips single quotes — don't include them in output + continue; + } + + if (ch === '"' && !inSingleQuote) { + inDoubleQuote = !inDoubleQuote; + // Shell strips double quotes + continue; + } + + if (ch === ' ' && !inSingleQuote && !inDoubleQuote) { + if (current.length > 0) { + tokens.push(current); + current = ''; + } + continue; + } + + current += ch; + } + + if (current.length > 0) tokens.push(current); + return tokens; +} + +/** Builds the full option spec array for a CLI operation, combining static + wrapper extras. */ +function buildFullOptionSpecs(operationId: string): OptionSpec[] { + const staticSpecs = CLI_OPERATION_OPTION_SPECS[operationId as CliOperationId] ?? []; + const wrapperSpecs = WRAPPER_EXTRA_SPECS[operationId] ?? []; + + const seen = new Set(); + const merged: OptionSpec[] = []; + for (const spec of [...staticSpecs, ...wrapperSpecs]) { + if (seen.has(spec.name)) continue; + seen.add(spec.name); + merged.push(spec); + } + if (!seen.has('help')) merged.push({ name: 'help', type: 'boolean' }); + return merged; +} + +/** + * Strips `superdoc` prefix and command tokens from a tokenized example, + * returning only the flag tokens that would be passed to `parseCommandArgs`. + */ +function stripCommandPrefix(tokens: string[], commandTokens: readonly string[]): string[] { + const rest = [...tokens]; + if (rest[0] === 'superdoc') rest.shift(); + for (const ct of commandTokens) { + if (rest.length > 0 && rest[0] === ct) rest.shift(); + } + return rest; +} + +/** + * Validates that every --*-json flag value in the parsed options is valid JSON. + * Returns an array of error descriptions (empty = all valid). + */ +function validateJsonPayloads(options: Record): string[] { + const errors: string[] = []; + for (const [flag, value] of Object.entries(options)) { + if (!flag.endsWith('-json') || value == null) continue; + const raw = typeof value === 'string' ? value : String(value); + try { + JSON.parse(raw); + } catch { + errors.push(`--${flag}: invalid JSON`); + } + } + return errors; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('DOC_COMMAND_EXAMPLES parse through real CLI parser', () => { + for (const [docApiId, examples] of Object.entries(DOC_COMMAND_EXAMPLES)) { + const cliOpId = `doc.${docApiId}`; + const commandSpec = CLI_COMMAND_SPECS.find((s) => s.operationId === cliOpId && !s.alias); + const fullSpecs = buildFullOptionSpecs(cliOpId); + + for (const example of examples) { + test(`${docApiId}: ${example.slice(0, 80)}...`, () => { + expect(commandSpec).toBeDefined(); + + // 1. Tokenize like a shell + const allTokens = shellTokenize(example); + + // 2. Strip superdoc + command tokens + const flagTokens = stripCommandPrefix(allTokens, commandSpec!.tokens); + + // 3. Parse through the real CLI parser + const parsed = parseCommandArgs(flagTokens, fullSpecs); + + // 4. Validate: no unknown flags, no type errors + expect(parsed.unknown).toEqual([]); + expect(parsed.errors).toEqual([]); + + // ensureValidArgs would throw on unknown/errors — verify it doesn't + expect(() => ensureValidArgs(parsed)).not.toThrow(); + + // 5. Validate JSON payloads are syntactically valid + const jsonErrors = validateJsonPayloads(parsed.options); + expect(jsonErrors).toEqual([]); + }); + } + } +}); + +describe('CLI_HELPER_COMMANDS examples parse through real CLI parser', () => { + for (const helper of CLI_HELPER_COMMANDS) { + const helperKey = helper.tokens.join(' '); + const helperSpec = CLI_COMMAND_SPECS.find((s) => s.key === helperKey); + + // Build full option specs: canonical op + wrapper extras + helper extras + const canonicalOpId = `doc.${helper.canonicalOperationId}` as CliOperationId; + const canonicalSpecs = CLI_OPERATION_OPTION_SPECS[canonicalOpId] ?? []; + const wrapperSpecs = WRAPPER_EXTRA_SPECS[canonicalOpId] ?? []; + const helperExtras: OptionSpec[] = (helper.extraOptionSpecs ?? []).map((s) => ({ + name: s.name, + type: s.type as 'string' | 'number' | 'boolean', + })); + + const seen = new Set(); + const fullSpecs: OptionSpec[] = []; + for (const spec of [...canonicalSpecs, ...wrapperSpecs, ...helperExtras]) { + if (seen.has(spec.name)) continue; + seen.add(spec.name); + fullSpecs.push(spec); + } + if (!seen.has('help')) fullSpecs.push({ name: 'help', type: 'boolean' }); + + for (const example of helper.examples) { + test(`helper ${helperKey}: ${example.slice(0, 80)}...`, () => { + expect(helperSpec).toBeDefined(); + + const allTokens = shellTokenize(example); + const flagTokens = stripCommandPrefix(allTokens, helper.tokens); + + const parsed = parseCommandArgs(flagTokens, fullSpecs); + + expect(parsed.unknown).toEqual([]); + expect(parsed.errors).toEqual([]); + expect(() => ensureValidArgs(parsed)).not.toThrow(); + + const jsonErrors = validateJsonPayloads(parsed.options); + expect(jsonErrors).toEqual([]); + }); + } + } +}); diff --git a/apps/cli/src/__tests__/conformance/harness.ts b/apps/cli/src/__tests__/conformance/harness.ts index f248b6fdf2..3a0cb384f0 100644 --- a/apps/cli/src/__tests__/conformance/harness.ts +++ b/apps/cli/src/__tests__/conformance/harness.ts @@ -415,6 +415,36 @@ export class ConformanceHarness { return { docPath: outDoc, changeId, target: collapsedTarget }; } + async firstTwoBlockAddresses( + docPath: string, + stateDir: string, + ): Promise<{ first: { nodeId: string; nodeType: string }; second: { nodeId: string; nodeType: string } }> { + const { result, envelope } = await this.runCli(['blocks', 'list', docPath, '--limit', '5'], stateDir); + if (result.code !== 0) { + throw new Error(`Unable to list blocks for ${docPath}`); + } + + assertSuccessEnvelope(envelope); + const data = envelope.data as { + result?: { + blocks?: Array<{ nodeId?: string; nodeType?: string }>; + }; + }; + const blocks = data.result?.blocks ?? []; + if (blocks.length < 2) { + throw new Error(`Need at least 2 blocks in ${docPath}, found ${blocks.length}`); + } + const first = blocks[0]!; + const second = blocks[1]!; + if (!first.nodeId || !first.nodeType || !second.nodeId || !second.nodeType) { + throw new Error(`Block entries missing nodeId or nodeType in ${docPath}`); + } + return { + first: { nodeId: first.nodeId, nodeType: first.nodeType }, + second: { nodeId: second.nodeId, nodeType: second.nodeType }, + }; + } + async openSessionFixture( stateDir: string, label: string, diff --git a/apps/cli/src/__tests__/conformance/scenarios.ts b/apps/cli/src/__tests__/conformance/scenarios.ts index 2d6f044e43..2b89ad836b 100644 --- a/apps/cli/src/__tests__/conformance/scenarios.ts +++ b/apps/cli/src/__tests__/conformance/scenarios.ts @@ -1549,6 +1549,14 @@ export const SUCCESS_SCENARIOS = { ], }; }, + 'doc.blocks.list': async (harness: ConformanceHarness): Promise => { + const stateDir = await harness.createStateDir('doc-blocks-list-success'); + const docPath = await harness.copyFixtureDoc('doc-blocks-list'); + return { + stateDir, + args: ['blocks', 'list', docPath, '--limit', '10'], + }; + }, 'doc.blocks.delete': async (harness: ConformanceHarness): Promise => { const stateDir = await harness.createStateDir('doc-blocks-delete-success'); const docPath = await harness.copyFixtureDoc('doc-blocks-delete'); @@ -1566,6 +1574,25 @@ export const SUCCESS_SCENARIOS = { ], }; }, + 'doc.blocks.deleteRange': async (harness: ConformanceHarness): Promise => { + const stateDir = await harness.createStateDir('doc-blocks-delete-range-success'); + const docPath = await harness.copyFixtureDoc('doc-blocks-delete-range'); + const { first, second } = await harness.firstTwoBlockAddresses(docPath, stateDir); + return { + stateDir, + args: [ + 'blocks', + 'delete-range', + docPath, + '--start-json', + JSON.stringify({ kind: 'block', nodeType: first.nodeType, nodeId: first.nodeId }), + '--end-json', + JSON.stringify({ kind: 'block', nodeType: second.nodeType, nodeId: second.nodeId }), + '--out', + harness.createOutputPath('doc-blocks-delete-range-output'), + ], + }; + }, 'doc.lists.list': async (harness: ConformanceHarness): Promise => { const stateDir = await harness.createStateDir('doc-lists-list-success'); const docPath = await harness.copyListFixtureDoc('doc-lists-list'); diff --git a/apps/cli/src/__tests__/lib/validate.test.ts b/apps/cli/src/__tests__/lib/validate.test.ts index d2ca559f81..9410687968 100644 --- a/apps/cli/src/__tests__/lib/validate.test.ts +++ b/apps/cli/src/__tests__/lib/validate.test.ts @@ -132,35 +132,23 @@ describe('validateCreateParagraphInput', () => { expect(result.at?.kind).toBe('before'); }); - test('validates at: before with nodeId shorthand', () => { - const result = validateCreateParagraphInput({ - at: { - kind: 'before', - nodeId: 'p1', - }, - }); - - expect(result.at).toEqual({ - kind: 'before', - target: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, - }); + test('rejects bare nodeId shorthand in before location', () => { + expect(() => + validateCreateParagraphInput({ + at: { kind: 'before', nodeId: 'p1' }, + }), + ).toThrow(CliError); }); - test('validates at: after with nodeId shorthand', () => { - const result = validateCreateParagraphInput({ - at: { - kind: 'after', - nodeId: 'p2', - }, - }); - - expect(result.at).toEqual({ - kind: 'after', - target: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, - }); + test('rejects bare nodeId shorthand in after location', () => { + expect(() => + validateCreateParagraphInput({ + at: { kind: 'after', nodeId: 'p2' }, + }), + ).toThrow(CliError); }); - test('rejects relative at location when both target and nodeId are provided', () => { + test('rejects relative at location when nodeId is provided alongside target', () => { expect(() => validateCreateParagraphInput({ at: { @@ -172,7 +160,7 @@ describe('validateCreateParagraphInput', () => { ).toThrow(CliError); }); - test('rejects relative at location when neither target nor nodeId is provided', () => { + test('rejects relative at location when target is missing', () => { expect(() => validateCreateParagraphInput({ at: { kind: 'after' } })).toThrow(CliError); }); diff --git a/apps/cli/src/cli/cli-only-operation-definitions.ts b/apps/cli/src/cli/cli-only-operation-definitions.ts index 6e4f4227f4..91ce1f029d 100644 --- a/apps/cli/src/cli/cli-only-operation-definitions.ts +++ b/apps/cli/src/cli/cli-only-operation-definitions.ts @@ -46,10 +46,11 @@ export const CLI_ONLY_OPERATION_DEFINITIONS: Record> = { + // ── Core read/locate ──────────────────────────────────────────────── + find: [ + 'superdoc find --type text --pattern "quarterly revenue" --limit 5', + 'superdoc find --select-json \'{"type":"node","nodeType":"heading"}\'', + ], + 'query.match': [ + 'superdoc query match --select-json \'{"type":"text","pattern":"Introduction"}\' --require exactlyOne', + 'superdoc query match --select-json \'{"type":"node","nodeType":"paragraph"}\' --require any --limit 3', + ], + getNode: ['superdoc get-node --address-json \'{"kind":"block","nodeType":"paragraph","nodeId":"abc123"}\''], + getNodeById: ['superdoc get-node-by-id --id abc123'], + getText: ['superdoc get-text'], + info: ['superdoc info'], + + // ── Core mutations ────────────────────────────────────────────────── + insert: [ + 'superdoc insert --value "Hello, world!"', + 'superdoc insert --block-id abc123 --value "Appended text"', + 'superdoc insert --type markdown --value "## New Section"', + ], + replace: [ + 'superdoc replace --block-id abc123 --start 0 --end 5 --text "Updated"', + 'superdoc replace --block-id abc123 --start 0 --end 5 --text "Updated" --dry-run', + 'superdoc replace --block-id abc123 --start 0 --end 5 --text "Updated" --expected-revision 3', + ], + delete: [ + 'superdoc delete --block-id abc123 --start 0 --end 10', + 'superdoc delete --block-id abc123 --start 0 --end 10 --dry-run', + ], + 'blocks.list': [ + 'superdoc blocks list', + 'superdoc blocks list --limit 20', + 'superdoc blocks list --offset 10 --limit 10', + 'superdoc blocks list --node-types-json \'["paragraph","heading"]\'', + ], + 'blocks.delete': [ + 'superdoc blocks delete --node-type paragraph --node-id abc123', + 'superdoc blocks delete --node-type paragraph --node-id abc123 --dry-run', + ], + 'blocks.deleteRange': [ + 'superdoc blocks delete-range --start-json \'{"kind":"block","nodeType":"paragraph","nodeId":"abc123"}\' --end-json \'{"kind":"block","nodeType":"paragraph","nodeId":"def456"}\'', + 'superdoc blocks delete-range --start-json \'{"kind":"block","nodeType":"paragraph","nodeId":"abc123"}\' --end-json \'{"kind":"block","nodeType":"paragraph","nodeId":"def456"}\' --dry-run', + ], + + // ── Create ────────────────────────────────────────────────────────── + 'create.paragraph': [ + 'superdoc create paragraph --text "A new paragraph."', + 'superdoc create paragraph --at document-end --text "Last paragraph."', + 'superdoc create paragraph --at-json \'{"kind":"after","target":{"kind":"block","nodeType":"paragraph","nodeId":"abc123"}}\'', + ], + 'create.heading': [ + 'superdoc create heading --input-json \'{"level":2,"text":"Section Title"}\'', + 'superdoc create heading --input-json \'{"level":1,"text":"Document Title","at":{"kind":"documentStart"}}\'', + ], + 'create.image': ['superdoc create image --src "https://example.com/photo.png" --alt "Photo caption"'], + + // ── Lists ─────────────────────────────────────────────────────────── + 'lists.list': ['superdoc lists list', 'superdoc lists list --kind ordered'], + 'lists.get': ['superdoc lists get --address-json \'{"kind":"block","nodeType":"listItem","nodeId":"li1"}\''], + 'lists.insert': [ + 'superdoc lists insert --node-id abc123 --position after --text "New list item"', + 'superdoc lists insert --node-id abc123 --position before', + ], + 'lists.indent': ['superdoc lists indent --node-id abc123'], + 'lists.outdent': ['superdoc lists outdent --node-id abc123'], + 'lists.create': [ + 'superdoc lists create --input-json \'{"mode":"empty","at":{"kind":"block","nodeType":"paragraph","nodeId":"abc123"},"kind":"ordered"}\'', + ], + 'lists.attach': [ + 'superdoc lists attach --input-json \'{"target":{"kind":"block","nodeType":"listItem","nodeId":"abc123"},"direction":"above"}\'', + ], + 'lists.detach': ['superdoc lists detach --node-id abc123'], + 'lists.join': [ + 'superdoc lists join --input-json \'{"target":{"kind":"block","nodeType":"listItem","nodeId":"abc123"},"direction":"above"}\'', + ], + 'lists.separate': ['superdoc lists separate --node-id abc123'], + 'lists.setLevel': ['superdoc lists set-level --node-id abc123 --level 2'], + 'lists.setValue': ['superdoc lists set-value --node-id abc123 --value-json 5'], + 'lists.convertToText': ['superdoc lists convert-to-text --node-id abc123'], + 'lists.setType': [ + 'superdoc lists set-type --target-json \'{"kind":"block","nodeType":"listItem","nodeId":"abc123"}\' --kind bullet', + ], + + // ── Format ────────────────────────────────────────────────────────── + 'format.apply': ['superdoc format apply --block-id abc123 --start 0 --end 10 --inline-json \'{"bold":true}\''], + + // ── Comments ──────────────────────────────────────────────────────── + 'comments.create': ['superdoc comments create --block-id abc123 --start 0 --end 5 --text "Review this section"'], + 'comments.list': ['superdoc comments list'], + 'comments.get': ['superdoc comments get --id comment-123'], + 'comments.patch': ['superdoc comments patch --id comment-123 --text "Updated wording."'], + 'comments.delete': ['superdoc comments delete --id comment-123'], + + // ── Track Changes ─────────────────────────────────────────────────── + 'trackChanges.list': ['superdoc track-changes list'], + 'trackChanges.get': ['superdoc track-changes get --id tc-123'], + 'trackChanges.decide': ['superdoc track-changes decide --decision accept --target-json \'{"id":"tc-123"}\''], + + // ── History ───────────────────────────────────────────────────────── + 'history.get': ['superdoc history get'], + 'history.undo': ['superdoc history undo'], + 'history.redo': ['superdoc history redo'], + + // ── Mutations (batch) ─────────────────────────────────────────────── + 'mutations.apply': [ + 'superdoc mutations apply --atomic true --change-mode direct --steps-json \'[{"id":"s1","op":"text.rewrite","where":{"by":"select","select":{"type":"text","pattern":"old"},"require":"first"},"args":{"replacement":{"text":"new"}}}]\'', + ], + 'mutations.preview': [ + 'superdoc mutations preview --atomic true --change-mode direct --steps-json \'[{"id":"s1","op":"text.rewrite","where":{"by":"select","select":{"type":"text","pattern":"old"},"require":"first"},"args":{"replacement":{"text":"new"}}}]\'', + ], + + // ── Table of Contents ─────────────────────────────────────────────── + 'create.tableOfContents': [ + 'superdoc create table-of-contents', + 'superdoc create table-of-contents --at-json \'{"kind":"documentStart"}\'', + ], + + // ── Capabilities ──────────────────────────────────────────────────── + 'capabilities.get': ['superdoc capabilities'], +}; diff --git a/apps/cli/src/cli/commands.ts b/apps/cli/src/cli/commands.ts index 628f52963a..ddec71d44e 100644 --- a/apps/cli/src/cli/commands.ts +++ b/apps/cli/src/cli/commands.ts @@ -18,6 +18,7 @@ import { type CliOperationId, } from './operation-set'; import { buildHelperSpecs } from './helper-commands.js'; +import { DOC_COMMAND_EXAMPLES } from './command-examples.js'; // --------------------------------------------------------------------------- // Build command specs for doc-backed operations @@ -38,7 +39,7 @@ function buildDocBackedSpec(docApiId: string, cliOpId: CliOperationId): CliComma requiresDocumentContext: cliRequiresDocumentContext(cliOpId), alias: false, canonicalKey: key, - examples: [], + examples: DOC_COMMAND_EXAMPLES[docApiId] ?? [], }; } @@ -149,6 +150,15 @@ export const CLI_MAX_COMMAND_TOKENS: number = Math.max(...CLI_COMMAND_SPECS.map( function buildHelpText(): string { const lines: string[] = ['Usage: superdoc [options]', '']; + // Common tasks section — surface the most useful commands upfront + lines.push('Common tasks:'); + lines.push(' Find mutation target → query match'); + lines.push(' Insert between list items → lists insert'); + lines.push(' Create a paragraph → create paragraph'); + lines.push(' Insert inline text → insert'); + lines.push(' Batch formatting changes → mutations apply'); + lines.push(''); + const categories = new Map(); for (const spec of CLI_COMMAND_SPECS) { if (spec.alias) continue; diff --git a/apps/cli/src/cli/helper-commands.ts b/apps/cli/src/cli/helper-commands.ts index e089a31c00..7ef81f49ac 100644 --- a/apps/cli/src/cli/helper-commands.ts +++ b/apps/cli/src/cli/helper-commands.ts @@ -65,7 +65,7 @@ export const CLI_HELPER_COMMANDS: readonly CliHelperCommand[] = [ description: 'Apply strikethrough formatting to a text range.', category: 'format', mutates: true, - examples: ['superdoc format strikethrough --blockId p1 --start 0 --end 5'], + examples: ['superdoc format strikethrough --block-id p1 --start 0 --end 5'], }, // --- Track-changes review helpers (route to trackChanges.decide) --- { @@ -116,9 +116,7 @@ export const CLI_HELPER_COMMANDS: readonly CliHelperCommand[] = [ description: 'Add a new comment thread anchored to a text range.', category: 'comments', mutates: true, - examples: [ - 'superdoc comments add --target \'{"kind":"text","blockId":"p1","range":{"start":0,"end":5}}\' --text "Review this"', - ], + examples: ['superdoc comments add --block-id p1 --start 0 --end 5 --text "Review this"'], }, { tokens: ['comments', 'reply'], @@ -146,7 +144,7 @@ export const CLI_HELPER_COMMANDS: readonly CliHelperCommand[] = [ category: 'comments', mutates: true, examples: [ - 'superdoc comments move --id c1 --target \'{"kind":"text","blockId":"p2","range":{"start":0,"end":5}}\'', + 'superdoc comments move --id c1 --target-json \'{"kind":"text","blockId":"p2","range":{"start":0,"end":5}}\'', ], }, { diff --git a/apps/cli/src/cli/operation-hints.ts b/apps/cli/src/cli/operation-hints.ts index a5e3b4331a..5736a880d8 100644 --- a/apps/cli/src/cli/operation-hints.ts +++ b/apps/cli/src/cli/operation-hints.ts @@ -84,7 +84,9 @@ export const SUCCESS_VERB: Record = { insert: 'inserted text', replace: 'replaced text', delete: 'deleted text', + 'blocks.list': 'listed blocks', 'blocks.delete': 'deleted block', + 'blocks.deleteRange': 'deleted block range', 'format.apply': 'applied style', ...buildFormatInlineAliasRecord('applied style'), ...buildParagraphRecord('updated paragraph formatting'), @@ -249,7 +251,9 @@ export const OUTPUT_FORMAT: Record = { insert: 'mutationReceipt', replace: 'mutationReceipt', delete: 'mutationReceipt', + 'blocks.list': 'plain', 'blocks.delete': 'plain', + 'blocks.deleteRange': 'plain', 'format.apply': 'mutationReceipt', ...buildFormatInlineAliasRecord('mutationReceipt'), ...buildParagraphRecord('plain'), @@ -398,7 +402,9 @@ export const RESPONSE_ENVELOPE_KEY: Record insert: null, replace: null, delete: null, + 'blocks.list': 'result', 'blocks.delete': 'result', + 'blocks.deleteRange': 'result', 'format.apply': null, ...buildFormatInlineAliasRecord(null), ...buildParagraphRecord('result'), @@ -587,7 +593,9 @@ export const OPERATION_FAMILY: Record = insert: 'textMutation', replace: 'textMutation', delete: 'textMutation', + 'blocks.list': 'blocks', 'blocks.delete': 'blocks', + 'blocks.deleteRange': 'blocks', 'format.apply': 'textMutation', ...buildFormatInlineAliasRecord('textMutation'), ...buildParagraphRecord('textMutation'), diff --git a/apps/cli/src/cli/operation-params.ts b/apps/cli/src/cli/operation-params.ts index 650e3d8287..2be473b974 100644 --- a/apps/cli/src/cli/operation-params.ts +++ b/apps/cli/src/cli/operation-params.ts @@ -44,7 +44,6 @@ const DRY_RUN_PARAM: CliOperationParamSpec = { kind: 'flag', flag: 'dry-run', type: 'boolean', - agentVisible: false, }; const CHANGE_MODE_PARAM: CliOperationParamSpec = { name: 'changeMode', @@ -58,7 +57,6 @@ const EXPECTED_REVISION_PARAM: CliOperationParamSpec = { kind: 'flag', flag: 'expected-revision', type: 'number', - agentVisible: false, }; const USER_NAME_PARAM: CliOperationParamSpec = { name: 'userName', @@ -78,7 +76,7 @@ const USER_EMAIL_PARAM: CliOperationParamSpec = { // --------------------------------------------------------------------------- type JsonSchema = Record; -const AGENT_HIDDEN_PARAM_NAMES = new Set(['out', 'expectedRevision', 'dryRun']); +const AGENT_HIDDEN_PARAM_NAMES = new Set(['out']); function resolveRef(schema: JsonSchema, $defs?: Record): JsonSchema { if (schema.$ref && $defs) { @@ -416,10 +414,19 @@ const EXTRA_CLI_PARAMS: Partial> = { { name: 'input', kind: 'jsonFlag', flag: 'input-json', type: 'json' }, ...LIST_TARGET_FLAT_PARAMS, ], + 'doc.blocks.list': [ + { name: 'offset', kind: 'flag', flag: 'offset', type: 'number' }, + { name: 'limit', kind: 'flag', flag: 'limit', type: 'number' }, + { name: 'nodeTypes', kind: 'jsonFlag', flag: 'node-types-json', type: 'json' }, + ], 'doc.blocks.delete': [ { name: 'nodeType', kind: 'flag', flag: 'node-type', type: 'string' }, { name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string' }, ], + 'doc.blocks.deleteRange': [ + { name: 'start', kind: 'jsonFlag', flag: 'start-json', type: 'json' }, + { name: 'end', kind: 'jsonFlag', flag: 'end-json', type: 'json' }, + ], 'doc.create.paragraph': [{ name: 'input', kind: 'jsonFlag', flag: 'input-json', type: 'json' }], 'doc.create.heading': [{ name: 'input', kind: 'jsonFlag', flag: 'input-json', type: 'json' }], }; diff --git a/apps/cli/src/commands/open.ts b/apps/cli/src/commands/open.ts index 2b3a9086b8..47856c4018 100644 --- a/apps/cli/src/commands/open.ts +++ b/apps/cli/src/commands/open.ts @@ -215,7 +215,7 @@ export async function runOpen(tokens: string[], context: CommandContext): Promis document: { path: metadata.sourcePath, source: metadata.source, - byteLength: output.byteLength, + byteLength: opened.meta.byteLength, revision: metadata.revision, }, dirty: metadata.dirty, diff --git a/apps/cli/src/lib/create-paragraph-input.ts b/apps/cli/src/lib/create-paragraph-input.ts index af17f4f61b..ede5b02e86 100644 --- a/apps/cli/src/lib/create-paragraph-input.ts +++ b/apps/cli/src/lib/create-paragraph-input.ts @@ -35,21 +35,29 @@ function ensureBlockTarget(value: unknown, path: string): BlockTarget { async function buildFlatInput(parsed: ParsedArgs, commandName: string): Promise { const text = getStringOption(parsed, 'text'); const at = parseAtFlag(getStringOption(parsed, 'at'), commandName); + const atJson = await resolveJsonInput(parsed, 'at'); const beforePayload = await resolveJsonInput(parsed, 'before-address'); const afterPayload = await resolveJsonInput(parsed, 'after-address'); - if (beforePayload != null && afterPayload != null) { + // Count how many location forms were provided + const locationForms = [at, atJson, beforePayload, afterPayload].filter((v) => v != null); + if (locationForms.length > 1) { throw new CliError( 'INVALID_ARGUMENT', - `${commandName}: use only one of --before-address-json or --after-address-json.`, + `${commandName}: use only one of --at, --at-json, --before-address-json, or --after-address-json.`, ); } - if (at && (beforePayload != null || afterPayload != null)) { - throw new CliError( - 'INVALID_ARGUMENT', - `${commandName}: --at cannot be combined with --before-address-json/--after-address-json.`, - ); + // Canonical --at-json path (preferred) + if (atJson != null) { + const validated = validateCreateParagraphInput({ at: atJson, text }, 'input'); + if (!validated.at) { + throw new CliError( + 'VALIDATION_ERROR', + `${commandName}: --at-json produced an empty location. Provide a valid location object.`, + ); + } + return validated; } if (beforePayload != null) { @@ -87,6 +95,8 @@ export async function resolveCreateParagraphInput( const hasFlatFlags = getStringOption(parsed, 'text') != null || getStringOption(parsed, 'at') != null || + getStringOption(parsed, 'at-json') != null || + getStringOption(parsed, 'at-file') != null || getStringOption(parsed, 'before-address-json') != null || getStringOption(parsed, 'before-address-file') != null || getStringOption(parsed, 'after-address-json') != null || diff --git a/apps/cli/src/lib/error-mapping.ts b/apps/cli/src/lib/error-mapping.ts index a0840710c7..eed01ce903 100644 --- a/apps/cli/src/lib/error-mapping.ts +++ b/apps/cli/src/lib/error-mapping.ts @@ -81,8 +81,9 @@ function mapListsError(operationId: CliExposedOperationId, error: unknown, code: return new CliError('TARGET_NOT_FOUND', message, { operationId, details }); } + // Preserve structured INVALID_TARGET details (requestedNodeType, actualNodeType, remediation) if (code === 'INVALID_TARGET') { - return new CliError('INVALID_ARGUMENT', message, { operationId, details }); + return new CliError('INVALID_TARGET', message, { operationId, details }); } if (code === 'TRACK_CHANGE_COMMAND_UNAVAILABLE' || code === 'CAPABILITY_UNAVAILABLE') { @@ -177,10 +178,16 @@ function mapCreateError(operationId: CliExposedOperationId, error: unknown, code return new CliError('TARGET_NOT_FOUND', message, { operationId, details }); } - if (code === 'AMBIGUOUS_TARGET' || code === 'INVALID_TARGET') { + if (code === 'AMBIGUOUS_TARGET') { return new CliError('INVALID_ARGUMENT', message, { operationId, details }); } + // Preserve structured INVALID_TARGET details (requestedNodeType, actualNodeType, remediation) + // so CLI consumers can programmatically triage mismatch errors. + if (code === 'INVALID_TARGET') { + return new CliError('INVALID_TARGET', message, { operationId, details }); + } + if (code === 'TRACK_CHANGE_COMMAND_UNAVAILABLE') { return new CliError('TRACK_CHANGE_COMMAND_UNAVAILABLE', message, { operationId, details }); } @@ -426,7 +433,7 @@ export function mapFailedReceipt(operationId: CliExposedOperationId, result: unk // Lists family if (family === 'lists') { if (failureCode === 'INVALID_TARGET') { - return new CliError('INVALID_ARGUMENT', failureMessage, { operationId, failure }); + return new CliError('INVALID_TARGET', failureMessage, { operationId, failure }); } if (failureCode === 'CAPABILITY_UNAVAILABLE') { return new CliError('TRACK_CHANGE_COMMAND_UNAVAILABLE', failureMessage, { operationId, failure }); @@ -459,7 +466,7 @@ export function mapFailedReceipt(operationId: CliExposedOperationId, result: unk return new CliError('TRACK_CHANGE_COMMAND_UNAVAILABLE', failureMessage, { operationId, failure }); } if (failureCode === 'INVALID_TARGET') { - return new CliError('INVALID_ARGUMENT', failureMessage, { operationId, failure }); + return new CliError('INVALID_TARGET', failureMessage, { operationId, failure }); } return new CliError('COMMAND_FAILED', failureMessage, { operationId, failure }); } diff --git a/apps/cli/src/lib/errors.ts b/apps/cli/src/lib/errors.ts index 054d4d570d..49d6855c97 100644 --- a/apps/cli/src/lib/errors.ts +++ b/apps/cli/src/lib/errors.ts @@ -40,7 +40,9 @@ export type CliErrorCode = | 'CROSS_BLOCK_MATCH' | 'SPAN_FRAGMENTED' | 'PAGE_NUMBERS_NOT_MATERIALIZED' - | 'CAPABILITY_UNAVAILABLE'; + | 'CAPABILITY_UNAVAILABLE' + | 'INVALID_TARGET' + | 'AMBIGUOUS_TARGET'; /** * Intersection type for errors thrown by document-api adapter operations. diff --git a/apps/cli/src/lib/introspection-dispatch.ts b/apps/cli/src/lib/introspection-dispatch.ts index 78f2a23a33..981eb2ed7d 100644 --- a/apps/cli/src/lib/introspection-dispatch.ts +++ b/apps/cli/src/lib/introspection-dispatch.ts @@ -245,6 +245,7 @@ const INTROSPECTION_INVOKERS: Partial'}`, `Document: ${metadata.sourcePath ?? ''}`, + `Source: ${metadata.source}`, + metadata.sourceSnapshot ? `Source size: ${metadata.sourceSnapshot.size} bytes` : undefined, + `Working size: ${byteLength} bytes`, `Session Type: ${metadata.sessionType}`, metadata.collaboration ? `Collab Doc ID: ${metadata.collaboration.documentId}` : undefined, `Revision: ${metadata.revision}`, diff --git a/apps/cli/src/lib/operation-wrapper-input.ts b/apps/cli/src/lib/operation-wrapper-input.ts index 1017b460ce..3eac83b859 100644 --- a/apps/cli/src/lib/operation-wrapper-input.ts +++ b/apps/cli/src/lib/operation-wrapper-input.ts @@ -157,6 +157,8 @@ export async function parseWrapperOperationInput( { name: 'input-file', type: 'string' }, { name: 'text', type: 'string' }, { name: 'at', type: 'string' }, + { name: 'at-json', type: 'string' }, + { name: 'at-file', type: 'string' }, { name: 'before-address-json', type: 'string' }, { name: 'before-address-file', type: 'string' }, { name: 'after-address-json', type: 'string' }, diff --git a/apps/cli/src/lib/validate.ts b/apps/cli/src/lib/validate.ts index 580b3a468d..e9be728705 100644 --- a/apps/cli/src/lib/validate.ts +++ b/apps/cli/src/lib/validate.ts @@ -252,48 +252,23 @@ function validateCreateParagraphLocation(value: unknown, path: string): NonNulla } if (kind === 'before' || kind === 'after') { - const hasTarget = obj.target != null; - const hasNodeId = obj.nodeId != null; - if (hasTarget === hasNodeId) { - throw new CliError('VALIDATION_ERROR', `${path} must include exactly one of target or nodeId.`); + if (obj.nodeId != null) { + throw new CliError( + 'VALIDATION_ERROR', + `${path}: bare "nodeId" shorthand is not supported. Use "target" with an explicit { kind: "block", nodeType, nodeId }.`, + ); } - if (hasTarget) { - expectOnlyKeys(obj, ['kind', 'target'], path); - const target = validateNodeAddress(obj.target, `${path}.target`); - if (target.kind !== 'block') { - throw new CliError('VALIDATION_ERROR', `${path}.target.kind must be "block".`); - } - - if (kind === 'before') { - return { - kind: 'before', - target, - }; - } - - return { - kind: 'after', - target, - }; + expectOnlyKeys(obj, ['kind', 'target'], path); + if (obj.target == null) { + throw new CliError('VALIDATION_ERROR', `${path} must include a "target" BlockNodeAddress.`); } - - expectOnlyKeys(obj, ['kind', 'nodeId'], path); - const nodeId = expectString(obj.nodeId, `${path}.nodeId`); - // nodeId shorthand: wrap in a BlockNodeAddress with nodeType 'paragraph' - // as a default. The adapter falls back to nodeId-only lookup when the - // full nodeType:nodeId key doesn't match, so this works for any block type. - const target = { kind: 'block' as const, nodeType: 'paragraph' as const, nodeId }; - if (kind === 'before') { - return { - kind: 'before', - target, - }; + const target = validateNodeAddress(obj.target, `${path}.target`); + if (target.kind !== 'block') { + throw new CliError('VALIDATION_ERROR', `${path}.target.kind must be "block".`); } - return { - kind: 'after', - target, - }; + + return { kind, target }; } throw new CliError('VALIDATION_ERROR', `${path}.kind must be one of: documentStart, documentEnd, before, after.`); diff --git a/packages/document-api/src/blocks/blocks.ts b/packages/document-api/src/blocks/blocks.ts index 36249557d4..89e7da5a5e 100644 --- a/packages/document-api/src/blocks/blocks.ts +++ b/packages/document-api/src/blocks/blocks.ts @@ -1,20 +1,79 @@ import type { MutationOptions } from '../write/write.js'; import { normalizeMutationOptions } from '../write/write.js'; -import type { BlocksDeleteInput, BlocksDeleteResult } from '../types/blocks.types.js'; -import { DELETABLE_BLOCK_NODE_TYPES } from '../types/base.js'; +import type { + BlocksDeleteInput, + BlocksDeleteResult, + BlocksListInput, + BlocksListResult, + BlocksDeleteRangeInput, + BlocksDeleteRangeResult, +} from '../types/blocks.types.js'; +import { BLOCK_NODE_TYPES, DELETABLE_BLOCK_NODE_TYPES } from '../types/base.js'; import { DocumentApiValidationError } from '../errors.js'; +// --------------------------------------------------------------------------- +// Public API surface +// --------------------------------------------------------------------------- + export interface BlocksApi { + list(input?: BlocksListInput): BlocksListResult; + delete(input: BlocksDeleteInput, options?: MutationOptions): BlocksDeleteResult; + deleteRange(input: BlocksDeleteRangeInput, options?: MutationOptions): BlocksDeleteRangeResult; +} + +export interface BlocksAdapter { + list(input?: BlocksListInput): BlocksListResult; delete(input: BlocksDeleteInput, options?: MutationOptions): BlocksDeleteResult; + deleteRange(input: BlocksDeleteRangeInput, options?: MutationOptions): BlocksDeleteRangeResult; } -export type BlocksAdapter = BlocksApi; +// --------------------------------------------------------------------------- +// Shared constants +// --------------------------------------------------------------------------- -/** Block node types supported by blocks.delete — derived from the shared constant. */ const SUPPORTED_DELETE_NODE_TYPES = new Set(DELETABLE_BLOCK_NODE_TYPES); - -/** Block node types explicitly rejected (row/column semantics out of scope). */ const REJECTED_DELETE_NODE_TYPES = new Set(['tableRow', 'tableCell']); +const VALID_BLOCK_NODE_TYPES = new Set(BLOCK_NODE_TYPES); + +// --------------------------------------------------------------------------- +// blocks.list validation +// --------------------------------------------------------------------------- + +function validateBlocksListInput(input?: BlocksListInput): void { + if (!input) return; + + if (input.offset != null && (typeof input.offset !== 'number' || input.offset < 0)) { + throw new DocumentApiValidationError('INVALID_INPUT', 'blocks.list offset must be a non-negative number.', { + fields: ['offset'], + }); + } + + if (input.limit != null && (typeof input.limit !== 'number' || input.limit < 1)) { + throw new DocumentApiValidationError('INVALID_INPUT', 'blocks.list limit must be a positive number.', { + fields: ['limit'], + }); + } + + if (input.nodeTypes != null) { + if (!Array.isArray(input.nodeTypes) || input.nodeTypes.length === 0) { + throw new DocumentApiValidationError('INVALID_INPUT', 'blocks.list nodeTypes must be a non-empty array.', { + fields: ['nodeTypes'], + }); + } + for (const nt of input.nodeTypes) { + if (!VALID_BLOCK_NODE_TYPES.has(nt)) { + throw new DocumentApiValidationError('INVALID_INPUT', `blocks.list nodeTypes contains unknown type "${nt}".`, { + fields: ['nodeTypes'], + nodeType: nt, + }); + } + } + } +} + +// --------------------------------------------------------------------------- +// blocks.delete validation +// --------------------------------------------------------------------------- function validateBlocksDeleteInput(input: BlocksDeleteInput): void { if (!input || typeof input !== 'object') { @@ -59,6 +118,58 @@ function validateBlocksDeleteInput(input: BlocksDeleteInput): void { } } +// --------------------------------------------------------------------------- +// blocks.deleteRange validation +// --------------------------------------------------------------------------- + +function validateBlockNodeAddress(address: unknown, label: string): void { + if (!address || typeof address !== 'object') { + throw new DocumentApiValidationError('INVALID_INPUT', `blocks.deleteRange requires a ${label} address.`, { + fields: [label], + }); + } + + const addr = address as Record; + + if (addr.kind !== 'block') { + throw new DocumentApiValidationError('INVALID_INPUT', `blocks.deleteRange ${label} must have kind "block".`, { + fields: [`${label}.kind`], + }); + } + + if (!addr.nodeId || typeof addr.nodeId !== 'string') { + throw new DocumentApiValidationError('INVALID_INPUT', `blocks.deleteRange ${label} requires a nodeId string.`, { + fields: [`${label}.nodeId`], + }); + } + + if (!addr.nodeType || typeof addr.nodeType !== 'string') { + throw new DocumentApiValidationError('INVALID_INPUT', `blocks.deleteRange ${label} requires a nodeType string.`, { + fields: [`${label}.nodeType`], + }); + } +} + +function validateBlocksDeleteRangeInput(input: BlocksDeleteRangeInput): void { + if (!input || typeof input !== 'object') { + throw new DocumentApiValidationError('INVALID_INPUT', 'blocks.deleteRange requires an input object.', { + fields: ['input'], + }); + } + + validateBlockNodeAddress(input.start, 'start'); + validateBlockNodeAddress(input.end, 'end'); +} + +// --------------------------------------------------------------------------- +// Execute functions +// --------------------------------------------------------------------------- + +export function executeBlocksList(adapter: BlocksAdapter, input?: BlocksListInput): BlocksListResult { + validateBlocksListInput(input); + return adapter.list(input); +} + export function executeBlocksDelete( adapter: BlocksAdapter, input: BlocksDeleteInput, @@ -67,3 +178,12 @@ export function executeBlocksDelete( validateBlocksDeleteInput(input); return adapter.delete(input, normalizeMutationOptions(options)); } + +export function executeBlocksDeleteRange( + adapter: BlocksAdapter, + input: BlocksDeleteRangeInput, + options?: MutationOptions, +): BlocksDeleteRangeResult { + validateBlocksDeleteRangeInput(input); + return adapter.deleteRange(input, normalizeMutationOptions(options)); +} diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index 31d0d7fb38..6db6207d09 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -270,7 +270,8 @@ export const OPERATION_DEFINITIONS = { }, find: { memberPath: 'find', - description: 'Search the document for text or node matches using SDM/1 selectors.', + description: + 'Search the document for text or node matches using SDM/1 selectors. Returns discovery-grade results — for mutation targeting, use query.match instead.', expectedResult: 'Returns an SDFindResult envelope ({ total, limit, offset, items }). Each item is an SDNodeResult ({ node, address }).', requiresDocumentContext: true, @@ -374,7 +375,8 @@ export const OPERATION_DEFINITIONS = { insert: { memberPath: 'insert', description: - 'Insert content at a target position, or at the end of the document when target is omitted. ' + + 'Insert inline content at a text position within an existing block, or at the end of the document when target is omitted. ' + + 'This is NOT for creating sibling blocks — use create.paragraph, create.heading, or lists.insert for that. ' + 'Accepts two input shapes: legacy string-based (value + type) or structural SDFragment (content). ' + 'Supports text (default), markdown, and html content types via the `type` field in legacy mode. ' + 'Structural mode accepts an SDFragment with typed nodes (paragraphs, tables, images, etc.).', @@ -473,10 +475,26 @@ export const OPERATION_DEFINITIONS = { referenceGroup: 'core', }, + 'blocks.list': { + memberPath: 'blocks.list', + description: + 'List top-level blocks in document order with IDs, types, and text previews. Supports pagination via offset/limit and optional nodeType filtering.', + expectedResult: + 'Returns a BlocksListResult with total block count, an ordered array of block entries (ordinal, nodeId, nodeType, textPreview, isEmpty), and the current document revision.', + requiresDocumentContext: true, + metadata: readOperation({ + throws: ['INVALID_INPUT'], + }), + referenceDocPath: 'blocks/list.mdx', + referenceGroup: 'blocks', + essential: true, + }, + 'blocks.delete': { memberPath: 'blocks.delete', description: 'Delete an entire block node (paragraph, heading, list item, table, image, or sdt) deterministically.', - expectedResult: 'Returns a BlocksDeleteResult receipt confirming the block was removed from the document.', + expectedResult: + 'Returns a BlocksDeleteResult receipt confirming the block was removed, including a deletedBlock summary with ordinal, nodeType, and textPreview.', requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'conditional', @@ -496,6 +514,31 @@ export const OPERATION_DEFINITIONS = { referenceGroup: 'blocks', }, + 'blocks.deleteRange': { + memberPath: 'blocks.deleteRange', + description: + 'Delete a contiguous range of top-level blocks between two endpoints (inclusive). Both endpoints must be direct children of the document node. Supports dry-run preview.', + expectedResult: + 'Returns a BlocksDeleteRangeResult with deletedCount, deletedBlocks array (each with ordinal, nodeId, nodeType, textPreview), before/after revision, and dryRun flag.', + requiresDocumentContext: true, + metadata: mutationOperation({ + idempotency: 'conditional', + supportsDryRun: true, + supportsTrackedMode: false, + possibleFailureCodes: NONE_FAILURES, + throws: [ + 'TARGET_NOT_FOUND', + 'AMBIGUOUS_TARGET', + 'INVALID_TARGET', + 'INVALID_INPUT', + 'CAPABILITY_UNAVAILABLE', + 'INTERNAL_ERROR', + ], + }), + referenceDocPath: 'blocks/delete-range.mdx', + referenceGroup: 'blocks', + }, + 'format.apply': { memberPath: 'format.apply', description: 'Apply inline run-property patch changes to the target range with explicit set/clear semantics.', @@ -533,7 +576,7 @@ export const OPERATION_DEFINITIONS = { 'create.paragraph': { memberPath: 'create.paragraph', - description: 'Create a new paragraph at the target position.', + description: 'Create a standalone paragraph at the target position. To add a list item, use lists.insert instead.', expectedResult: 'Returns a CreateParagraphResult with the new paragraph block ID and address.', requiresDocumentContext: true, metadata: mutationOperation({ @@ -1186,7 +1229,8 @@ export const OPERATION_DEFINITIONS = { }, 'lists.insert': { memberPath: 'lists.insert', - description: 'Insert a new list at the target position.', + description: + 'Insert a new list item before or after an existing list item. The new item inherits the target list context.', expectedResult: 'Returns a ListsInsertResult with the new list item address and block ID.', requiresDocumentContext: true, metadata: mutationOperation({ @@ -1715,7 +1759,8 @@ export const OPERATION_DEFINITIONS = { 'query.match': { memberPath: 'query.match', - description: 'Deterministic selector-based search with cardinality contracts for mutation targeting.', + description: + 'Deterministic selector-based search returning mutation-grade addresses and text ranges. Use this to discover targets before any mutation.', expectedResult: 'Returns a QueryMatchOutput with the resolved target address and cardinality metadata.', requiresDocumentContext: true, metadata: readOperation({ @@ -2647,7 +2692,7 @@ export const OPERATION_DEFINITIONS = { memberPath: 'history.undo', description: 'Undo the most recent history-safe mutation in the active editor.', expectedResult: - 'Returns a HistoryActionResult with noop flag and revision before/after; noop is true when the undo stack is empty.', + 'Returns a HistoryActionResult with noop flag, reason (EMPTY_UNDO_STACK | NO_EFFECT when noop), and revision before/after.', requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'non-idempotent', @@ -2665,7 +2710,7 @@ export const OPERATION_DEFINITIONS = { memberPath: 'history.redo', description: 'Redo the most recently undone action in the active editor.', expectedResult: - 'Returns a HistoryActionResult with noop flag and revision before/after; noop is true when the redo stack is empty.', + 'Returns a HistoryActionResult with noop flag, reason (EMPTY_REDO_STACK | NO_EFFECT when noop), and revision before/after.', requiresDocumentContext: true, metadata: mutationOperation({ idempotency: 'non-idempotent', diff --git a/packages/document-api/src/contract/operation-registry.ts b/packages/document-api/src/contract/operation-registry.ts index 25ee1fab3d..c4353e9150 100644 --- a/packages/document-api/src/contract/operation-registry.ts +++ b/packages/document-api/src/contract/operation-registry.ts @@ -20,7 +20,14 @@ import type { CreateHeadingInput, CreateHeadingResult, } from '../types/create.types.js'; -import type { BlocksDeleteInput, BlocksDeleteResult } from '../types/blocks.types.js'; +import type { + BlocksDeleteInput, + BlocksDeleteResult, + BlocksListInput, + BlocksListResult, + BlocksDeleteRangeInput, + BlocksDeleteRangeResult, +} from '../types/blocks.types.js'; import type { GetNodeByIdInput } from '../get-node/get-node.js'; import type { GetTextInput } from '../get-text/get-text.js'; @@ -488,7 +495,9 @@ export interface OperationRegistry extends FormatInlineAliasOperationRegistry { delete: { input: DeleteInput; options: MutationOptions; output: TextMutationReceipt }; // --- blocks.* --- + 'blocks.list': { input: BlocksListInput | undefined; options: never; output: BlocksListResult }; 'blocks.delete': { input: BlocksDeleteInput; options: MutationOptions; output: BlocksDeleteResult }; + 'blocks.deleteRange': { input: BlocksDeleteRangeInput; options: MutationOptions; output: BlocksDeleteRangeResult }; // --- format.* --- 'format.apply': { input: StyleApplyInput; options: MutationOptions; output: TextMutationReceipt }; diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index e15f889031..0a026e2e19 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -2550,6 +2550,33 @@ const operationSchemas: Record = { failure: textMutationFailureSchemaFor('format.apply'), }, ...formatInlineAliasOperationSchemas, + 'blocks.list': { + input: objectSchema({ + offset: { type: 'number', minimum: 0 }, + limit: { type: 'number', minimum: 1 }, + nodeTypes: { type: 'array', items: { enum: [...blockNodeTypeValues] } }, + }), + output: objectSchema( + { + total: { type: 'number' }, + blocks: { + type: 'array', + items: objectSchema( + { + ordinal: { type: 'number' }, + nodeId: { type: 'string' }, + nodeType: { enum: [...blockNodeTypeValues] }, + textPreview: { oneOf: [{ type: 'string' }, { type: 'null' }] }, + isEmpty: { type: 'boolean' }, + }, + ['ordinal', 'nodeId', 'nodeType', 'textPreview', 'isEmpty'], + ), + }, + revision: { type: 'string' }, + }, + ['total', 'blocks', 'revision'], + ), + }, 'blocks.delete': { input: objectSchema( { @@ -2561,6 +2588,12 @@ const operationSchemas: Record = { { success: { const: true }, deleted: deletableBlockNodeAddressSchema, + deletedBlock: objectSchema({ + ordinal: { type: 'number' }, + nodeId: { type: 'string' }, + nodeType: { type: 'string' }, + textPreview: { oneOf: [{ type: 'string' }, { type: 'null' }] }, + }), }, ['success', 'deleted'], ), @@ -2568,11 +2601,64 @@ const operationSchemas: Record = { { success: { const: true }, deleted: deletableBlockNodeAddressSchema, + deletedBlock: objectSchema({ + ordinal: { type: 'number' }, + nodeId: { type: 'string' }, + nodeType: { type: 'string' }, + textPreview: { oneOf: [{ type: 'string' }, { type: 'null' }] }, + }), }, ['success', 'deleted'], ), failure: preApplyFailureResultSchemaFor('blocks.delete'), }, + 'blocks.deleteRange': { + input: objectSchema( + { + start: blockNodeAddressSchema, + end: blockNodeAddressSchema, + }, + ['start', 'end'], + ), + output: objectSchema( + { + success: { const: true }, + deletedCount: { type: 'number' }, + deletedBlocks: { + type: 'array', + items: objectSchema( + { + ordinal: { type: 'number' }, + nodeId: { type: 'string' }, + nodeType: { type: 'string' }, + textPreview: { oneOf: [{ type: 'string' }, { type: 'null' }] }, + }, + ['ordinal', 'nodeId', 'nodeType', 'textPreview'], + ), + }, + revision: objectSchema( + { + before: { type: 'string' }, + after: { type: 'string' }, + }, + ['before', 'after'], + ), + dryRun: { type: 'boolean' }, + }, + ['success', 'deletedCount', 'deletedBlocks', 'revision', 'dryRun'], + ), + success: objectSchema( + { + success: { const: true }, + deletedCount: { type: 'number' }, + deletedBlocks: { type: 'array' }, + revision: objectSchema({ before: { type: 'string' }, after: { type: 'string' } }, ['before', 'after']), + dryRun: { type: 'boolean' }, + }, + ['success', 'deletedCount', 'deletedBlocks', 'revision', 'dryRun'], + ), + failure: preApplyFailureResultSchemaFor('blocks.deleteRange'), + }, // --- styles.paragraph.* --- 'styles.paragraph.setStyle': { diff --git a/packages/document-api/src/history/history.types.ts b/packages/document-api/src/history/history.types.ts index 6320242cdb..9d386e826c 100644 --- a/packages/document-api/src/history/history.types.ts +++ b/packages/document-api/src/history/history.types.ts @@ -19,6 +19,9 @@ export interface HistoryState { historyUnsafeOperations: readonly OperationId[]; } +/** Machine-readable reason for a history no-op. */ +export type HistoryNoopReason = 'EMPTY_UNDO_STACK' | 'EMPTY_REDO_STACK' | 'NO_EFFECT'; + /** * Result of a history.undo or history.redo action. * Mirrors PlanReceipt's revision shape for consistency. @@ -26,6 +29,8 @@ export interface HistoryState { export interface HistoryActionResult { /** True if the action had no effect (empty stack). */ noop: boolean; + /** Machine-readable reason when noop is true. */ + reason?: HistoryNoopReason; /** Revision bookends matching PlanReceipt.revision shape. */ revision: { before: string; diff --git a/packages/document-api/src/index.ts b/packages/document-api/src/index.ts index f981e4b901..330b184522 100644 --- a/packages/document-api/src/index.ts +++ b/packages/document-api/src/index.ts @@ -17,7 +17,7 @@ export type { MarkdownToFragmentAdapter, } from './markdown-to-fragment/markdown-to-fragment.js'; export { executeMarkdownToFragment } from './markdown-to-fragment/markdown-to-fragment.js'; -export type { HistoryState, HistoryActionResult } from './history/history.types.js'; +export type { HistoryState, HistoryActionResult, HistoryNoopReason } from './history/history.types.js'; import type { CreateParagraphInput, @@ -183,8 +183,15 @@ import { executeCreateTableOfContents, } from './create/create.js'; import type { BlocksAdapter, BlocksApi } from './blocks/blocks.js'; -import { executeBlocksDelete } from './blocks/blocks.js'; -import type { BlocksDeleteInput, BlocksDeleteResult } from './types/blocks.types.js'; +import { executeBlocksList, executeBlocksDelete, executeBlocksDeleteRange } from './blocks/blocks.js'; +import type { + BlocksDeleteInput, + BlocksDeleteResult, + BlocksListInput, + BlocksListResult, + BlocksDeleteRangeInput, + BlocksDeleteRangeResult, +} from './types/blocks.types.js'; import type { CreateHeadingInput, CreateHeadingResult } from './types/create.types.js'; import type { CreateTableInput, @@ -1419,7 +1426,7 @@ export interface DocumentApi { */ trackChanges: TrackChangesApi; /** - * Block-level structural operations (delete whole blocks). + * Block-level structural operations (list, delete, delete-range). */ blocks: BlocksApi; /** @@ -1787,9 +1794,15 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { }, }, blocks: { + list(input?: BlocksListInput): BlocksListResult { + return executeBlocksList(adapters.blocks, input); + }, delete(input: BlocksDeleteInput, options?: MutationOptions): BlocksDeleteResult { return executeBlocksDelete(adapters.blocks, input, options); }, + deleteRange(input: BlocksDeleteRangeInput, options?: MutationOptions): BlocksDeleteRangeResult { + return executeBlocksDeleteRange(adapters.blocks, input, options); + }, }, create: { paragraph(input: CreateParagraphInput, options?: MutationOptions): CreateParagraphResult { diff --git a/packages/document-api/src/invoke/invoke.ts b/packages/document-api/src/invoke/invoke.ts index 1ae22d7bc0..b4b8f8a57f 100644 --- a/packages/document-api/src/invoke/invoke.ts +++ b/packages/document-api/src/invoke/invoke.ts @@ -75,7 +75,9 @@ export function buildDispatchTable(api: DocumentApi): TypedDispatchTable { delete: (input, options) => api.delete(input, options), // --- blocks.* --- + 'blocks.list': (input) => api.blocks.list(input), 'blocks.delete': (input, options) => api.blocks.delete(input, options), + 'blocks.deleteRange': (input, options) => api.blocks.deleteRange(input, options), // --- format.* --- 'format.apply': (input, options) => api.format.apply(input, options), diff --git a/packages/document-api/src/types/blocks.types.ts b/packages/document-api/src/types/blocks.types.ts index 6ab3aecf85..6a41ed4511 100644 --- a/packages/document-api/src/types/blocks.types.ts +++ b/packages/document-api/src/types/blocks.types.ts @@ -1,4 +1,32 @@ -import type { DeletableBlockNodeAddress } from './base.js'; +import type { BlockNodeType, BlockNodeAddress, DeletableBlockNodeAddress } from './base.js'; + +// --------------------------------------------------------------------------- +// blocks.list +// --------------------------------------------------------------------------- + +export interface BlockListEntry { + ordinal: number; + nodeId: string; + nodeType: BlockNodeType; + textPreview: string | null; + isEmpty: boolean; +} + +export interface BlocksListInput { + offset?: number; + limit?: number; + nodeTypes?: BlockNodeType[]; +} + +export interface BlocksListResult { + total: number; + blocks: BlockListEntry[]; + revision: string; +} + +// --------------------------------------------------------------------------- +// blocks.delete +// --------------------------------------------------------------------------- export interface BlocksDeleteInput { target: DeletableBlockNodeAddress; @@ -7,4 +35,32 @@ export interface BlocksDeleteInput { export interface BlocksDeleteResult { success: true; deleted: DeletableBlockNodeAddress; + deletedBlock?: DeletedBlockSummary; +} + +// --------------------------------------------------------------------------- +// blocks.deleteRange +// --------------------------------------------------------------------------- + +export interface BlocksDeleteRangeInput { + start: BlockNodeAddress; + end: BlockNodeAddress; +} + +export interface DeletedBlockSummary { + ordinal: number; + nodeId: string; + nodeType: string; + textPreview: string | null; +} + +export interface BlocksDeleteRangeResult { + success: true; + deletedCount: number; + deletedBlocks: DeletedBlockSummary[]; + revision: { + before: string; + after: string; + }; + dryRun: boolean; } diff --git a/packages/super-editor/src/document-api-adapters/__conformance__/contract-conformance.test.ts b/packages/super-editor/src/document-api-adapters/__conformance__/contract-conformance.test.ts index 8f7caa1c3d..0199f159bf 100644 --- a/packages/super-editor/src/document-api-adapters/__conformance__/contract-conformance.test.ts +++ b/packages/super-editor/src/document-api-adapters/__conformance__/contract-conformance.test.ts @@ -19,7 +19,7 @@ import { import { ListHelpers } from '../../core/helpers/list-numbering-helpers.js'; import { createCommentsWrapper } from '../plan-engine/comments-wrappers.js'; import { createParagraphWrapper, createHeadingWrapper } from '../plan-engine/create-wrappers.js'; -import { blocksDeleteWrapper } from '../plan-engine/blocks-wrappers.js'; +import { blocksDeleteWrapper, blocksDeleteRangeWrapper } from '../plan-engine/blocks-wrappers.js'; import { clearContentWrapper } from '../plan-engine/clear-content-wrapper.js'; import { styleApplyWrapper } from '../plan-engine/plan-wrappers.js'; import { @@ -897,6 +897,94 @@ function makeBlockDeleteEditor( } as unknown as Editor; } +function makeBlockRangeDeleteEditor(): Editor { + const p1 = createNode('paragraph', [createNode('text', [], { text: 'First' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const p2 = createNode('paragraph', [createNode('text', [], { text: 'Second' })], { + attrs: { paraId: 'p2', sdBlockId: 'p2' }, + isBlock: true, + inlineContent: true, + }); + const children = [p1, p2]; + const doc = createNode('doc', children, { isBlock: false }); + + const dispatch = vi.fn(); + const tr = { + setMeta: vi.fn().mockReturnThis(), + mapping: { map: (pos: number) => pos }, + docChanged: false, + delete: vi.fn().mockImplementation(function (this: { docChanged: boolean }) { + this.docChanged = true; + }), + }; + + return { + state: { doc, tr }, + dispatch, + commands: { + deleteBlockNodeById: vi.fn(() => true), + }, + helpers: { + blockNode: { + getBlockNodeById: vi.fn((id: string) => { + const match = children.find((c) => c.attrs?.sdBlockId === id || c.attrs?.paraId === id); + return match ? [{ node: match, pos: 0 }] : []; + }), + }, + }, + } as unknown as Editor; +} + +function makeBlockRangeDeleteEditorWithSectionBreak(): Editor { + const p1 = createNode('paragraph', [createNode('text', [], { text: 'First' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const sectBreakPara = createNode('paragraph', [createNode('text', [], { text: 'Section end' })], { + attrs: { + paraId: 'sect1', + sdBlockId: 'sect1', + paragraphProperties: { sectPr: { name: 'w:sectPr', elements: [] } }, + }, + isBlock: true, + inlineContent: true, + }); + const p3 = createNode('paragraph', [createNode('text', [], { text: 'Third' })], { + attrs: { paraId: 'p3', sdBlockId: 'p3' }, + isBlock: true, + inlineContent: true, + }); + const children = [p1, sectBreakPara, p3]; + const doc = createNode('doc', children, { isBlock: false }); + + const dispatch = vi.fn(); + const tr = { + setMeta: vi.fn().mockReturnThis(), + mapping: { map: (pos: number) => pos }, + docChanged: false, + }; + + return { + state: { doc, tr }, + dispatch, + commands: { + deleteBlockNodeById: vi.fn(() => true), + }, + helpers: { + blockNode: { + getBlockNodeById: vi.fn((id: string) => { + const match = children.find((c) => c.attrs?.sdBlockId === id || c.attrs?.paraId === id); + return match ? [{ node: match, pos: 0 }] : []; + }), + }, + }, + } as unknown as Editor; +} + function makeCommentRecord( commentId: string, overrides: Record = {}, @@ -3813,6 +3901,30 @@ const mutationVectors: Partial> = { ); }, }, + 'blocks.deleteRange': { + throwCase: () => { + const editor = makeBlockRangeDeleteEditor(); + return blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'missing' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, + }, + { changeMode: 'direct' }, + ); + }, + applyCase: () => { + const editor = makeBlockRangeDeleteEditor(); + return blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, + }, + { changeMode: 'direct' }, + ); + }, + }, clearContent: { throwCase: () => { const { editor } = makeTextEditor('Hello'); @@ -7942,6 +8054,20 @@ const dryRunVectors: Partial unknown>> = { expect(deleteBlockNodeById).not.toHaveBeenCalled(); return result; }, + 'blocks.deleteRange': () => { + const editor = makeBlockRangeDeleteEditor(); + const deleteCmd = editor.commands?.deleteBlockNodeById as ReturnType; + const result = blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, + }, + { changeMode: 'direct', dryRun: true }, + ); + expect(deleteCmd).not.toHaveBeenCalled(); + return result; + }, insert: () => { const { editor, dispatch, tr } = makeTextEditor(); const result = textReceiptToSDReceipt( diff --git a/packages/super-editor/src/document-api-adapters/assemble-adapters.ts b/packages/super-editor/src/document-api-adapters/assemble-adapters.ts index 3af135486c..e9bcd6fab8 100644 --- a/packages/super-editor/src/document-api-adapters/assemble-adapters.ts +++ b/packages/super-editor/src/document-api-adapters/assemble-adapters.ts @@ -48,7 +48,7 @@ import { trackChangesRejectAllWrapper, } from './plan-engine/track-changes-wrappers.js'; import { createParagraphWrapper, createHeadingWrapper } from './plan-engine/create-wrappers.js'; -import { blocksDeleteWrapper } from './plan-engine/blocks-wrappers.js'; +import { blocksListWrapper, blocksDeleteWrapper, blocksDeleteRangeWrapper } from './plan-engine/blocks-wrappers.js'; import { listsListWrapper, listsGetWrapper, @@ -400,7 +400,9 @@ export function assembleDocumentApiAdapters(editor: Editor): DocumentApiAdapters rejectAll: (input, options) => trackChangesRejectAllWrapper(editor, input, options), }, blocks: { + list: (input) => blocksListWrapper(editor, input), delete: (input, options) => blocksDeleteWrapper(editor, input, options), + deleteRange: (input, options) => blocksDeleteRangeWrapper(editor, input, options), }, create: { paragraph: (input, options) => createParagraphWrapper(editor, input, options), 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 e7a6a13888..631b22d0c1 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 @@ -9,7 +9,13 @@ import type { } from '@superdoc/document-api'; import { DocumentApiValidationError } from '@superdoc/document-api'; import { getBlockIndex } from './index-cache.js'; -import { findBlockById, isTextBlockCandidate, type BlockCandidate, type BlockIndex } from './node-address-resolver.js'; +import { + findBlockById, + findBlockByNodeIdOnly, + isTextBlockCandidate, + type BlockCandidate, + type BlockIndex, +} from './node-address-resolver.js'; import { computeTextContentLength, resolveTextRangeInBlock } from './text-offset-resolver.js'; import { buildTextMutationResolution, readTextAtResolvedRange } from './text-mutation-resolution.js'; import type { Transaction } from 'prosemirror-state'; @@ -20,7 +26,24 @@ export type WithinResult = { ok: true; range: { start: number; end: number } | u export type ResolvedTextTarget = { from: number; to: number }; function findTextBlockCandidates(index: BlockIndex, blockId: string): BlockCandidate[] { - return index.candidates.filter((candidate) => candidate.nodeId === blockId && isTextBlockCandidate(candidate)); + // Primary: match by canonical nodeId + const primary = index.candidates.filter((c) => c.nodeId === blockId && isTextBlockCandidate(c)); + if (primary.length > 0) return primary; + + // Fallback: alias-aware lookup via the block index (resolves sdBlockId aliases). + // This ensures IDs returned by create/list mutations remain usable in follow-up + // text-targeted commands even if the canonical nodeId differs from the alias. + // AMBIGUOUS_TARGET is re-thrown so callers get precise diagnostics. + try { + const resolved = findBlockByNodeIdOnly(index, blockId); + if (isTextBlockCandidate(resolved)) return [resolved]; + } catch (e) { + // Propagate ambiguity — callers depend on structured AMBIGUOUS_TARGET diagnostics + if (e instanceof DocumentApiAdapterError && e.code === 'AMBIGUOUS_TARGET') throw e; + // TARGET_NOT_FOUND is expected when the alias doesn't exist — fall through + } + + return []; } function assertUnambiguous(matches: BlockCandidate[], blockId: string): void { 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 b72253571b..ee8a416354 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 @@ -110,7 +110,7 @@ export function mapBlockNodeType(node: ProseMirrorNode): BlockNodeType | undefin } } -function resolveBlockNodeId(node: ProseMirrorNode, pos: number, nodeType: BlockNodeType): string | undefined { +export function resolveBlockNodeId(node: ProseMirrorNode, pos: number, nodeType: BlockNodeType): 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 diff --git a/packages/super-editor/src/document-api-adapters/history-adapter.test.ts b/packages/super-editor/src/document-api-adapters/history-adapter.test.ts index 3ff73d3df5..8247c7e173 100644 --- a/packages/super-editor/src/document-api-adapters/history-adapter.test.ts +++ b/packages/super-editor/src/document-api-adapters/history-adapter.test.ts @@ -118,16 +118,57 @@ describe('createHistoryAdapter', () => { }); it('returns noop=false when undo/redo commands succeed', () => { + undoDepthMock.mockReturnValue(1); + redoDepthMock.mockReturnValue(1); const adapter = createHistoryAdapter(makeEditor()); const undoResult = adapter.undo(); const redoResult = adapter.redo(); expect(undoResult.noop).toBe(false); + expect(undoResult.reason).toBeUndefined(); expect(redoResult.noop).toBe(false); + expect(redoResult.reason).toBeUndefined(); expect(undoResult.revision.before).toBeDefined(); expect(undoResult.revision.after).toBeDefined(); expect(redoResult.revision.before).toBeDefined(); expect(redoResult.revision.after).toBeDefined(); }); + + it('returns EMPTY_UNDO_STACK reason when undo stack is empty', () => { + undoDepthMock.mockReturnValue(0); + const adapter = createHistoryAdapter(makeEditor()); + + const result = adapter.undo(); + + expect(result.noop).toBe(true); + expect(result.reason).toBe('EMPTY_UNDO_STACK'); + }); + + it('returns EMPTY_REDO_STACK reason when redo stack is empty', () => { + redoDepthMock.mockReturnValue(0); + const adapter = createHistoryAdapter(makeEditor()); + + const result = adapter.redo(); + + expect(result.noop).toBe(true); + expect(result.reason).toBe('EMPTY_REDO_STACK'); + }); + + it('returns NO_EFFECT when command returns false with non-empty stack', () => { + undoDepthMock.mockReturnValue(1); + const adapter = createHistoryAdapter( + makeEditor({ + commands: { + undo: vi.fn(() => false), + redo: vi.fn(() => true), + } as unknown as Editor['commands'], + }), + ); + + const result = adapter.undo(); + + expect(result.noop).toBe(true); + expect(result.reason).toBe('NO_EFFECT'); + }); }); diff --git a/packages/super-editor/src/document-api-adapters/history-adapter.ts b/packages/super-editor/src/document-api-adapters/history-adapter.ts index 58d7c2e19f..45efb5e788 100644 --- a/packages/super-editor/src/document-api-adapters/history-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/history-adapter.ts @@ -62,10 +62,15 @@ export function createHistoryAdapter(editor: Editor): HistoryAdapter { }); } const revBefore = getRevision(editor); + const depth = getUndoDepth(editor); + if (depth === 0) { + return { noop: true, reason: 'EMPTY_UNDO_STACK', revision: { before: revBefore, after: revBefore } }; + } const success = Boolean(editor.commands.undo()); const revAfter = getRevision(editor); return { noop: !success, + reason: success ? undefined : 'NO_EFFECT', revision: { before: revBefore, after: revAfter }, }; }, @@ -77,10 +82,15 @@ export function createHistoryAdapter(editor: Editor): HistoryAdapter { }); } const revBefore = getRevision(editor); + const depth = getRedoDepth(editor); + if (depth === 0) { + return { noop: true, reason: 'EMPTY_REDO_STACK', revision: { before: revBefore, after: revBefore } }; + } const success = Boolean(editor.commands.redo()); const revAfter = getRevision(editor); return { noop: !success, + reason: success ? undefined : 'NO_EFFECT', revision: { before: revBefore, after: revAfter }, }; }, diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts index 5d6ab2bae3..a853afa360 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts @@ -2,7 +2,7 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import type { Editor } from '../../core/Editor.js'; import type { BlocksDeleteInput, MutationOptions } from '@superdoc/document-api'; -import { blocksDeleteWrapper } from './blocks-wrappers.js'; +import { blocksDeleteWrapper, blocksDeleteRangeWrapper, blocksListWrapper } from './blocks-wrappers.js'; import { registerBuiltInExecutors } from './register-executors.js'; import { DocumentApiAdapterError } from '../errors.js'; @@ -23,6 +23,11 @@ type NodeOptions = { nodeSize?: number; }; +function computeTextContent(typeName: string, children: ProseMirrorNode[], text: string): string { + if (typeName === 'text') return text; + return children.map((c) => (c as any).textContent ?? c.text ?? '').join(''); +} + function createNode(typeName: string, children: ProseMirrorNode[] = [], options: NodeOptions = {}): ProseMirrorNode { const attrs = options.attrs ?? {}; const text = options.text ?? ''; @@ -35,10 +40,13 @@ function createNode(typeName: string, children: ProseMirrorNode[] = [], options: const contentSize = children.reduce((sum, child) => sum + child.nodeSize, 0); const nodeSize = isText ? text.length : options.nodeSize != null ? options.nodeSize : isLeaf ? 1 : contentSize + 2; + const textContent = computeTextContent(typeName, children, text); + const node = { type: { name: typeName }, attrs, text: isText ? text : undefined, + textContent, content: { size: contentSize }, nodeSize, isText, @@ -160,7 +168,8 @@ describe('blocksDeleteWrapper', () => { it('deletes a paragraph block', () => { const { editor } = makeBlockDeleteEditor(); const result = blocksDeleteWrapper(editor, makeInput('paragraph', 'p1'), { changeMode: 'direct' }); - expect(result).toEqual({ success: true, deleted: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' } }); + expect(result).toMatchObject({ success: true, deleted: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' } }); + expect(result.deletedBlock).toMatchObject({ nodeId: 'p1', nodeType: 'paragraph', textPreview: 'Hello' }); }); it('deletes a heading block', () => { @@ -175,7 +184,8 @@ describe('blocksDeleteWrapper', () => { }); const { editor } = makeBlockDeleteEditor({ children: [heading] }); const result = blocksDeleteWrapper(editor, makeInput('heading', 'h1'), { changeMode: 'direct' }); - expect(result).toEqual({ success: true, deleted: { kind: 'block', nodeType: 'heading', nodeId: 'h1' } }); + expect(result).toMatchObject({ success: true, deleted: { kind: 'block', nodeType: 'heading', nodeId: 'h1' } }); + expect(result.deletedBlock).toMatchObject({ nodeId: 'h1', nodeType: 'heading', textPreview: 'Title' }); }); it('deletes a list item block', () => { @@ -190,7 +200,8 @@ describe('blocksDeleteWrapper', () => { }); const { editor } = makeBlockDeleteEditor({ children: [listItem] }); const result = blocksDeleteWrapper(editor, makeInput('listItem', 'li1'), { changeMode: 'direct' }); - expect(result).toEqual({ success: true, deleted: { kind: 'block', nodeType: 'listItem', nodeId: 'li1' } }); + expect(result).toMatchObject({ success: true, deleted: { kind: 'block', nodeType: 'listItem', nodeId: 'li1' } }); + expect(result.deletedBlock).toMatchObject({ nodeId: 'li1', nodeType: 'listItem' }); }); it('deletes a table block', () => { @@ -201,7 +212,8 @@ describe('blocksDeleteWrapper', () => { }); const { editor } = makeBlockDeleteEditor({ children: [table] }); const result = blocksDeleteWrapper(editor, makeInput('table', 't1'), { changeMode: 'direct' }); - expect(result).toEqual({ success: true, deleted: { kind: 'block', nodeType: 'table', nodeId: 't1' } }); + expect(result).toMatchObject({ success: true, deleted: { kind: 'block', nodeType: 'table', nodeId: 't1' } }); + expect(result.deletedBlock).toMatchObject({ nodeId: 't1', nodeType: 'table', textPreview: null }); }); it('rejects image target (inline-only in ProseMirror schema)', () => { @@ -225,7 +237,7 @@ describe('blocksDeleteWrapper', () => { }); const { editor } = makeBlockDeleteEditor({ children: [sdt] }); const result = blocksDeleteWrapper(editor, makeInput('sdt', 'sdt1'), { changeMode: 'direct' }); - expect(result).toEqual({ success: true, deleted: { kind: 'block', nodeType: 'sdt', nodeId: 'sdt1' } }); + expect(result).toMatchObject({ success: true, deleted: { kind: 'block', nodeType: 'sdt', nodeId: 'sdt1' } }); }); it('deletes an empty paragraph block', () => { @@ -361,7 +373,8 @@ describe('blocksDeleteWrapper', () => { changeMode: 'direct', dryRun: true, }); - expect(result).toEqual({ success: true, deleted: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' } }); + expect(result).toMatchObject({ success: true, deleted: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' } }); + expect(result.deletedBlock).toBeDefined(); expect(deleteBlockNodeById).not.toHaveBeenCalled(); }); @@ -405,3 +418,263 @@ describe('blocksDeleteWrapper', () => { }); }); }); + +// --------------------------------------------------------------------------- +// blocksListWrapper — canonical ID consistency +// --------------------------------------------------------------------------- + +describe('blocksListWrapper', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it('emits canonical blockId (not sdBlockId) for non-paragraph block types', () => { + // SDT nodes: resolveBlockNodeId prefers blockId over sdBlockId. + // This test ensures blocks.list uses the same canonical ID as the block + // index, so IDs from blocks.list work in follow-up delete operations. + const sdt = createNode('sdt', [], { + attrs: { blockId: 'sdt-canonical', sdBlockId: 'sdt-internal' }, + isBlock: true, + inlineContent: false, + }); + const paragraph = createNode('paragraph', [createNode('text', [], { text: 'Hello' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const doc = createNode('doc', [sdt, paragraph], { isBlock: false }); + const editor = { + state: { doc }, + } as unknown as Editor; + + const result = blocksListWrapper(editor); + const sdtEntry = result.blocks.find((b) => b.nodeType === 'sdt'); + expect(sdtEntry).toBeDefined(); + // Must use blockId (the canonical ID), not sdBlockId + expect(sdtEntry!.nodeId).toBe('sdt-canonical'); + }); + + it('emits canonical paraId for paragraph types even when sdBlockId differs', () => { + const paragraph = createNode('paragraph', [createNode('text', [], { text: 'Hello' })], { + attrs: { paraId: 'para-canonical', sdBlockId: 'para-internal' }, + isBlock: true, + inlineContent: true, + }); + const doc = createNode('doc', [paragraph], { isBlock: false }); + const editor = { + state: { doc }, + } as unknown as Editor; + + const result = blocksListWrapper(editor); + expect(result.blocks[0]!.nodeId).toBe('para-canonical'); + }); + + it('applies offset and limit pagination correctly', () => { + const children = Array.from({ length: 5 }, (_, i) => + createNode('paragraph', [createNode('text', [], { text: `P${i}` })], { + attrs: { paraId: `p${i}`, sdBlockId: `p${i}` }, + isBlock: true, + inlineContent: true, + }), + ); + const doc = createNode('doc', children, { isBlock: false }); + const editor = { state: { doc } } as unknown as Editor; + + const result = blocksListWrapper(editor, { offset: 1, limit: 2 }); + expect(result.total).toBe(5); + expect(result.blocks).toHaveLength(2); + expect(result.blocks[0]!.ordinal).toBe(1); + expect(result.blocks[0]!.nodeId).toBe('p1'); + expect(result.blocks[1]!.ordinal).toBe(2); + expect(result.blocks[1]!.nodeId).toBe('p2'); + }); + + it('filters by nodeTypes when specified', () => { + const paragraph = createNode('paragraph', [createNode('text', [], { text: 'Hello' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const table = createNode('table', [], { + attrs: { blockId: 't1', sdBlockId: 't1' }, + isBlock: true, + inlineContent: false, + }); + const doc = createNode('doc', [paragraph, table], { isBlock: false }); + const editor = { state: { doc } } as unknown as Editor; + + const result = blocksListWrapper(editor, { nodeTypes: ['table'] }); + expect(result.total).toBe(1); + expect(result.blocks[0]!.nodeType).toBe('table'); + }); +}); + +// --------------------------------------------------------------------------- +// blocksDeleteRangeWrapper — section-break rejection +// --------------------------------------------------------------------------- + +describe('blocksDeleteRangeWrapper', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + function makeRangeDeleteEditor(children: ProseMirrorNode[]) { + const doc = createNode('doc', children, { isBlock: false }); + const dispatch = vi.fn(); + const tr = { + setMeta: vi.fn().mockReturnThis(), + mapping: { map: (pos: number) => pos }, + docChanged: false, + delete: vi.fn().mockImplementation(function (this: { docChanged: boolean }) { + this.docChanged = true; + }), + }; + return { + state: { doc, tr }, + dispatch, + commands: { + deleteBlockNodeById: vi.fn(() => true), + }, + helpers: { + blockNode: { + getBlockNodeById: vi.fn((id: string) => { + const match = children.find((c) => c.attrs?.sdBlockId === id || c.attrs?.paraId === id); + return match ? [{ node: match, pos: 0 }] : []; + }), + }, + }, + } as unknown as Editor; + } + + it('rejects a range that includes a section-break paragraph', () => { + const p1 = createNode('paragraph', [createNode('text', [], { text: 'Before' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const sectBreak = createNode('paragraph', [createNode('text', [], { text: 'Break' })], { + attrs: { + paraId: 'sect1', + sdBlockId: 'sect1', + paragraphProperties: { sectPr: { name: 'w:sectPr', elements: [] } }, + }, + isBlock: true, + inlineContent: true, + }); + const p3 = createNode('paragraph', [createNode('text', [], { text: 'After' })], { + attrs: { paraId: 'p3', sdBlockId: 'p3' }, + isBlock: true, + inlineContent: true, + }); + + const editor = makeRangeDeleteEditor([p1, sectBreak, p3]); + + try { + blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p3' }, + }, + { changeMode: 'direct' }, + ); + expect.unreachable('should have thrown'); + } catch (error) { + expect(error).toBeInstanceOf(DocumentApiAdapterError); + expect((error as DocumentApiAdapterError).code).toBe('INVALID_TARGET'); + expect((error as DocumentApiAdapterError).message).toContain('section break'); + } + }); + + it('also rejects section breaks during dry-run', () => { + const p1 = createNode('paragraph', [createNode('text', [], { text: 'Before' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const sectBreak = createNode('paragraph', [createNode('text', [], { text: 'Break' })], { + attrs: { + paraId: 'sect1', + sdBlockId: 'sect1', + paragraphProperties: { sectPr: { name: 'w:sectPr', elements: [] } }, + }, + isBlock: true, + inlineContent: true, + }); + + const editor = makeRangeDeleteEditor([p1, sectBreak]); + + expect(() => + blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'sect1' }, + }, + { changeMode: 'direct', dryRun: true }, + ), + ).toThrow(DocumentApiAdapterError); + }); + + it('rejects when start nodeType does not match the resolved block', () => { + const li = createNode('paragraph', [createNode('text', [], { text: 'List item' })], { + attrs: { + paraId: 'li1', + sdBlockId: 'li1', + paragraphProperties: { numberingProperties: { numId: 1, ilvl: 0 } }, + }, + isBlock: true, + inlineContent: true, + }); + const p2 = createNode('paragraph', [createNode('text', [], { text: 'Second' })], { + attrs: { paraId: 'p2', sdBlockId: 'p2' }, + isBlock: true, + inlineContent: true, + }); + + const editor = makeRangeDeleteEditor([li, p2]); + + try { + blocksDeleteRangeWrapper( + editor, + { + // Caller says "paragraph" but li1 resolves to "listItem" + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'li1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, + }, + { changeMode: 'direct' }, + ); + expect.unreachable('should have thrown'); + } catch (error) { + expect(error).toBeInstanceOf(DocumentApiAdapterError); + expect((error as DocumentApiAdapterError).code).toBe('INVALID_TARGET'); + expect((error as DocumentApiAdapterError).message).toContain('start expected paragraph'); + expect((error as DocumentApiAdapterError).message).toContain('resolved to listItem'); + } + }); + + it('allows a range without section breaks', () => { + const p1 = createNode('paragraph', [createNode('text', [], { text: 'First' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const p2 = createNode('paragraph', [createNode('text', [], { text: 'Second' })], { + attrs: { paraId: 'p2', sdBlockId: 'p2' }, + isBlock: true, + inlineContent: true, + }); + + const editor = makeRangeDeleteEditor([p1, p2]); + const result = blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, + }, + { changeMode: 'direct' }, + ); + expect(result.success).toBe(true); + expect(result.deletedCount).toBe(2); + }); +}); diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts index 02afc595bd..0cf8ac5345 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts @@ -1,44 +1,81 @@ /** - * Blocks convenience wrappers — bridge blocks.delete to the plan engine's - * execution path via the deleteBlockNodeById editor command. + * Blocks convenience wrappers — bridge blocks.list, blocks.delete, and + * blocks.deleteRange to the plan engine's execution path. * * Follows the same domain-command wrapper pattern as create-wrappers.ts * and lists-wrappers.ts. */ +import type { Node as ProseMirrorNode } from 'prosemirror-model'; import type { Editor } from '../../core/Editor.js'; import { DELETABLE_BLOCK_NODE_TYPES, type BlocksDeleteInput, type BlocksDeleteResult, + type BlocksListInput, + type BlocksListResult, + type BlocksDeleteRangeInput, + type BlocksDeleteRangeResult, + type BlockListEntry, + type DeletedBlockSummary, type MutationOptions, } from '@superdoc/document-api'; import { clearIndexCache, getBlockIndex } from '../helpers/index-cache.js'; -import { findBlockByIdStrict } from '../helpers/node-address-resolver.js'; +import { + findBlockByIdStrict, + findBlockByNodeIdOnly, + mapBlockNodeType, + resolveBlockNodeId, + type BlockCandidate, +} from '../helpers/node-address-resolver.js'; import { DocumentApiAdapterError } from '../errors.js'; import { requireEditorCommand, rejectTrackedMode } from '../helpers/mutation-helpers.js'; import { executeDomainCommand } from './plan-wrappers.js'; +import { getRevision } from './revision-tracker.js'; // --------------------------------------------------------------------------- -// Command types (internal to the wrapper) +// Constants // --------------------------------------------------------------------------- type DeleteBlockNodeByIdCommand = (id: string) => boolean; +const SUPPORTED_DELETE_NODE_TYPES = new Set(DELETABLE_BLOCK_NODE_TYPES); +const REJECTED_DELETE_NODE_TYPES = new Set(['tableRow', 'tableCell']); +const TEXT_PREVIEW_MAX_LENGTH = 80; + // --------------------------------------------------------------------------- -// Supported block types for deletion +// Shared helpers // --------------------------------------------------------------------------- -const SUPPORTED_NODE_TYPES = new Set(DELETABLE_BLOCK_NODE_TYPES); +function extractTextPreview(node: ProseMirrorNode): string | null { + if (!node.isTextblock) return null; + const text = node.textContent; + if (text.length <= TEXT_PREVIEW_MAX_LENGTH) return text; + return text.slice(0, TEXT_PREVIEW_MAX_LENGTH); +} -const REJECTED_NODE_TYPES = new Set(['tableRow', 'tableCell']); +function toBlockSummary(candidate: BlockCandidate, ordinal: number): DeletedBlockSummary { + return { + ordinal, + nodeId: candidate.nodeId, + nodeType: candidate.nodeType, + textPreview: extractTextPreview(candidate.node), + }; +} -// --------------------------------------------------------------------------- -// Validation helpers -// --------------------------------------------------------------------------- +function resolveSdBlockId(candidate: BlockCandidate): string { + const sdBlockId = (candidate.node.attrs as Record)?.sdBlockId; + if (typeof sdBlockId === 'string' && sdBlockId.length > 0) return sdBlockId; -function validateTargetNodeType(nodeType: string): void { - if (REJECTED_NODE_TYPES.has(nodeType)) { + throw new DocumentApiAdapterError( + 'INTERNAL_ERROR', + 'Resolved block candidate is missing sdBlockId attribute. This indicates a schema/extension invariant violation.', + { attrs: candidate.node.attrs }, + ); +} + +function validateDeleteTargetNodeType(nodeType: string): void { + if (REJECTED_DELETE_NODE_TYPES.has(nodeType)) { throw new DocumentApiAdapterError( 'INVALID_TARGET', `blocks.delete does not support "${nodeType}" targets. Table row/column operations are out of scope.`, @@ -46,24 +83,13 @@ function validateTargetNodeType(nodeType: string): void { ); } - if (!SUPPORTED_NODE_TYPES.has(nodeType)) { + if (!SUPPORTED_DELETE_NODE_TYPES.has(nodeType)) { throw new DocumentApiAdapterError('INVALID_TARGET', `blocks.delete does not support "${nodeType}" targets.`, { nodeType, }); } } -function resolveSdBlockId(candidate: { node: { attrs?: Record } }): string { - const sdBlockId = candidate.node.attrs?.sdBlockId; - if (typeof sdBlockId === 'string' && sdBlockId.length > 0) return sdBlockId; - - throw new DocumentApiAdapterError( - 'INTERNAL_ERROR', - 'Resolved block candidate is missing sdBlockId attribute. This indicates a schema/extension invariant violation.', - { attrs: candidate.node.attrs }, - ); -} - function validateCommandLayerUniqueness(editor: Editor, sdBlockId: string): void { const getBlockNodeById = editor.helpers?.blockNode?.getBlockNodeById; if (typeof getBlockNodeById !== 'function') { @@ -92,7 +118,58 @@ function validateCommandLayerUniqueness(editor: Editor, sdBlockId: string): void } // --------------------------------------------------------------------------- -// blocks.delete wrapper +// blocks.list — ordered block inspection +// --------------------------------------------------------------------------- + +function collectTopLevelBlocks(editor: Editor): BlockCandidate[] { + const doc = editor.state.doc; + const results: BlockCandidate[] = []; + + let offset = 0; + for (let i = 0; i < doc.childCount; i++) { + const child = doc.child(i); + const nodeType = mapBlockNodeType(child); + const pos = offset; // doc is root — no opening token in the PM position model + + if (nodeType) { + // Delegate to the canonical ID resolver so IDs match the block index. + // This ensures blocks.list output is directly usable in blocks.delete + // and blocks.deleteRange without ID mismatches. + const nodeId = resolveBlockNodeId(child, pos, nodeType); + + if (nodeId) { + results.push({ node: child, pos, end: pos + child.nodeSize, nodeType, nodeId }); + } + } + offset += child.nodeSize; + } + return results; +} + +export function blocksListWrapper(editor: Editor, input?: BlocksListInput): BlocksListResult { + const topLevel = collectTopLevelBlocks(editor); + + // Apply nodeTypes filter + const filtered = input?.nodeTypes ? topLevel.filter((b) => input.nodeTypes!.includes(b.nodeType)) : topLevel; + + const total = filtered.length; + const offset = input?.offset ?? 0; + const limit = input?.limit ?? total; + const paged = filtered.slice(offset, offset + limit); + + const blocks: BlockListEntry[] = paged.map((candidate, i) => ({ + ordinal: offset + i, + nodeId: candidate.nodeId, + nodeType: candidate.nodeType, + textPreview: extractTextPreview(candidate.node), + isEmpty: candidate.node.textContent.length === 0, + })); + + return { total, blocks, revision: getRevision(editor) }; +} + +// --------------------------------------------------------------------------- +// blocks.delete — single block deletion with scoped receipt // --------------------------------------------------------------------------- export function blocksDeleteWrapper( @@ -100,45 +177,39 @@ export function blocksDeleteWrapper( input: BlocksDeleteInput, options?: MutationOptions, ): BlocksDeleteResult { - // 1. Reject tracked mode (unsupported for this operation) rejectTrackedMode('blocks.delete', options); - // 2. Resolve and validate the target block from the block index const index = getBlockIndex(editor); const candidate = findBlockByIdStrict(index, input.target); - validateTargetNodeType(candidate.nodeType); + validateDeleteTargetNodeType(candidate.nodeType); + + // Capture block summary before deletion for the receipt + const candidateOrdinal = index.candidates.indexOf(candidate); + const deletedBlock = toBlockSummary(candidate, candidateOrdinal); - // 3. Resolve the command-facing sdBlockId - const sdBlockId = resolveSdBlockId(candidate as { node: { attrs?: Record } }); + const sdBlockId = resolveSdBlockId(candidate); - // 4. Acquire the editor command const deleteBlockNodeById = requireEditorCommand( editor.commands?.deleteBlockNodeById, 'blocks.delete', ) as DeleteBlockNodeByIdCommand; - // 5. Preflight command-layer uniqueness check validateCommandLayerUniqueness(editor, sdBlockId); - // 6. Dry run — full validation without mutation if (options?.dryRun) { - return { success: true, deleted: input.target }; + return { success: true, deleted: input.target, deletedBlock }; } - // 7. Execute through plan engine const receipt = executeDomainCommand( editor, () => { const didApply = deleteBlockNodeById(sdBlockId); - if (didApply) { - clearIndexCache(editor); - } + if (didApply) clearIndexCache(editor); return didApply; }, { expectedRevision: options?.expectedRevision }, ); - // 8. Assert success — all pre-checks passed, so false is an internal bug if (receipt.steps[0]?.effect !== 'changed') { throw new DocumentApiAdapterError( 'INTERNAL_ERROR', @@ -147,5 +218,147 @@ export function blocksDeleteWrapper( ); } - return { success: true, deleted: input.target }; + return { success: true, deleted: input.target, deletedBlock }; +} + +// --------------------------------------------------------------------------- +// blocks.deleteRange — contiguous range deletion +// --------------------------------------------------------------------------- + +/** + * Returns true if the given block's paragraph properties contain a section + * break (`sectPr`). Section-break paragraphs mark OOXML section boundaries + * and must not be deleted via range operations. + */ +function hasSectionBreak(candidate: BlockCandidate): boolean { + const attrs = candidate.node.attrs as Record | undefined; + const pPr = attrs?.paragraphProperties as Record | undefined; + return pPr?.sectPr != null && typeof pPr.sectPr === 'object'; +} + +function resolveTopLevelOrdinal(topLevel: BlockCandidate[], candidate: BlockCandidate, label: string): number { + // Match by nodeId since the top-level list and the full block index may hold + // different BlockCandidate object references for the same node. + const idx = topLevel.findIndex((b) => b.nodeId === candidate.nodeId && b.nodeType === candidate.nodeType); + if (idx !== -1) return idx; + + // Candidate was found in the full index but is not a direct doc child + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `blocks.deleteRange ${label} resolved to a nested block (not a direct document child). Only top-level blocks are supported.`, + { nodeId: candidate.nodeId, nodeType: candidate.nodeType }, + ); +} + +export function blocksDeleteRangeWrapper( + editor: Editor, + input: BlocksDeleteRangeInput, + options?: MutationOptions, +): BlocksDeleteRangeResult { + rejectTrackedMode('blocks.deleteRange', options); + + // 1. Collect top-level blocks (direct children of doc node) + const topLevel = collectTopLevelBlocks(editor); + + // 2. Resolve start and end via nodeId (alias-aware) + const index = getBlockIndex(editor); + const startCandidate = findBlockByNodeIdOnly(index, input.start.nodeId); + const endCandidate = findBlockByNodeIdOnly(index, input.end.nodeId); + + // 2a. Validate that resolved nodeType matches caller-supplied nodeType. + // Without this, a stale address (e.g. nodeType:"paragraph" for a listItem) + // would silently resolve and delete the wrong block type. + if (startCandidate.nodeType !== input.start.nodeType) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `blocks.deleteRange start expected ${input.start.nodeType}:${input.start.nodeId} but resolved to ${startCandidate.nodeType}.`, + { expected: input.start.nodeType, actual: startCandidate.nodeType, nodeId: input.start.nodeId }, + ); + } + if (endCandidate.nodeType !== input.end.nodeType) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `blocks.deleteRange end expected ${input.end.nodeType}:${input.end.nodeId} but resolved to ${endCandidate.nodeType}.`, + { expected: input.end.nodeType, actual: endCandidate.nodeType, nodeId: input.end.nodeId }, + ); + } + + // 3. Confirm both are top-level + const startOrdinal = resolveTopLevelOrdinal(topLevel, startCandidate, 'start'); + const endOrdinal = resolveTopLevelOrdinal(topLevel, endCandidate, 'end'); + + // 4. Validate range direction + if (startOrdinal > endOrdinal) { + throw new DocumentApiAdapterError( + 'INVALID_INPUT', + `blocks.deleteRange start ordinal (${startOrdinal}) is after end ordinal (${endOrdinal}). The start must precede or equal the end.`, + { startOrdinal, endOrdinal }, + ); + } + + // 5. Collect the range and build summaries + const rangeBlocks = topLevel.slice(startOrdinal, endOrdinal + 1); + + // 5a. Reject ranges that include section breaks + for (const block of rangeBlocks) { + if (hasSectionBreak(block)) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `blocks.deleteRange cannot delete a range that includes a section break (block "${block.nodeId}" at ordinal ${topLevel.indexOf(block)}).`, + { nodeId: block.nodeId, nodeType: block.nodeType }, + ); + } + } + + const deletedBlocks: DeletedBlockSummary[] = rangeBlocks.map((c, i) => toBlockSummary(c, startOrdinal + i)); + + const revisionBefore = getRevision(editor); + + // 6. Dry run — full validation, no mutation + if (options?.dryRun) { + return { + success: true, + deletedCount: rangeBlocks.length, + deletedBlocks, + revision: { before: revisionBefore, after: revisionBefore }, + dryRun: true, + }; + } + + // 7. Delete the contiguous range in a single transaction. + // Because the blocks are contiguous top-level children, we can delete the + // entire span [first.pos, last.end) in one PM tr.delete — avoiding the + // mismatched-transaction errors that arise from multiple sequential command calls. + const rangeFrom = rangeBlocks[0]!.pos; + const rangeTo = rangeBlocks[rangeBlocks.length - 1]!.end; + + const receipt = executeDomainCommand( + editor, + () => { + const tr = editor.state.tr; + tr.delete(rangeFrom, rangeTo); + editor.dispatch(tr); + clearIndexCache(editor); + return true; + }, + { expectedRevision: options?.expectedRevision }, + ); + + if (receipt.steps[0]?.effect !== 'changed') { + throw new DocumentApiAdapterError( + 'INTERNAL_ERROR', + 'blocks.deleteRange command returned false despite passing all pre-apply checks.', + { start: input.start, end: input.end }, + ); + } + + const revisionAfter = getRevision(editor); + + return { + success: true, + deletedCount: rangeBlocks.length, + deletedBlocks, + revision: { before: revisionBefore, after: revisionAfter }, + dryRun: false, + }; } diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.test.ts b/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.test.ts index 7d882fb372..a3a839fb24 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.test.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.test.ts @@ -1,7 +1,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { Editor } from '../../core/Editor.js'; -import { resolveBlockInsertionPos } from './create-insertion.js'; +import { resolveBlockInsertionPos, resolveCreateAnchor } from './create-insertion.js'; import { PlanError } from './errors.js'; +import { DocumentApiAdapterError } from '../errors.js'; // --------------------------------------------------------------------------- // Module mocks @@ -9,12 +10,21 @@ import { PlanError } from './errors.js'; const mockedDeps = vi.hoisted(() => ({ getBlockIndex: vi.fn(), + findBlockByNodeIdOnly: vi.fn(), })); vi.mock('../helpers/index-cache.js', () => ({ getBlockIndex: mockedDeps.getBlockIndex, })); +vi.mock('../helpers/node-address-resolver.js', async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + findBlockByNodeIdOnly: mockedDeps.findBlockByNodeIdOnly, + }; +}); + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -23,8 +33,19 @@ function makeEditor(): Editor { return {} as unknown as Editor; } +function makeCandidate(overrides: Partial<{ nodeType: string; nodeId: string; pos: number; end: number }> = {}) { + return { + nodeType: 'paragraph', + nodeId: 'p1', + pos: 10, + end: 25, + node: {}, + ...overrides, + }; +} + // --------------------------------------------------------------------------- -// Tests +// resolveBlockInsertionPos (legacy helper — kept for plan-engine executor) // --------------------------------------------------------------------------- describe('resolveBlockInsertionPos', () => { @@ -83,3 +104,136 @@ describe('resolveBlockInsertionPos', () => { expect(resolveBlockInsertionPos(makeEditor(), 'p2', 'after')).toBe(35); }); }); + +// --------------------------------------------------------------------------- +// resolveCreateAnchor (typed-target resolver with pre-flight validation) +// --------------------------------------------------------------------------- + +describe('resolveCreateAnchor', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockedDeps.getBlockIndex.mockReturnValue({ candidates: [], byId: new Map() }); + }); + + it('returns pos and anchor for exact nodeType match with "before"', () => { + const candidate = makeCandidate({ nodeType: 'paragraph', nodeId: 'p1', pos: 10, end: 25 }); + mockedDeps.findBlockByNodeIdOnly.mockReturnValue(candidate); + + const result = resolveCreateAnchor(makeEditor(), { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, 'before'); + + expect(result.pos).toBe(10); + expect(result.anchor).toBe(candidate); + }); + + it('returns end position for "after"', () => { + const candidate = makeCandidate({ pos: 10, end: 25 }); + mockedDeps.findBlockByNodeIdOnly.mockReturnValue(candidate); + + const result = resolveCreateAnchor(makeEditor(), { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, 'after'); + + expect(result.pos).toBe(25); + }); + + it('succeeds when target has no nodeType (any block type accepted)', () => { + const candidate = makeCandidate({ nodeType: 'heading', nodeId: 'h1', pos: 5, end: 20 }); + mockedDeps.findBlockByNodeIdOnly.mockReturnValue(candidate); + + const result = resolveCreateAnchor( + makeEditor(), + { kind: 'block', nodeId: 'h1' } as { kind: 'block'; nodeType: string; nodeId: string }, + 'before', + ); + + expect(result.pos).toBe(5); + expect(result.anchor.nodeType).toBe('heading'); + }); + + it('throws INVALID_TARGET when nodeType does not match', () => { + const candidate = makeCandidate({ nodeType: 'listItem', nodeId: 'li1' }); + mockedDeps.findBlockByNodeIdOnly.mockReturnValue(candidate); + + try { + resolveCreateAnchor(makeEditor(), { kind: 'block', nodeType: 'paragraph', nodeId: 'li1' }, 'before'); + } catch (error) { + expect(error).toBeInstanceOf(PlanError); + expect((error as PlanError).code).toBe('INVALID_TARGET'); + expect((error as PlanError).details).toMatchObject({ + requestedNodeType: 'paragraph', + actualNodeType: 'listItem', + nodeId: 'li1', + }); + return; + } + + throw new Error('expected resolveCreateAnchor to throw INVALID_TARGET'); + }); + + it('provides listItem-specific remediation when target is a listItem', () => { + const candidate = makeCandidate({ nodeType: 'listItem', nodeId: 'li1' }); + mockedDeps.findBlockByNodeIdOnly.mockReturnValue(candidate); + + try { + resolveCreateAnchor(makeEditor(), { kind: 'block', nodeType: 'paragraph', nodeId: 'li1' }, 'before'); + } catch (error) { + expect((error as PlanError).details).toMatchObject({ + remediation: 'Use lists.insert to add an item to a list sequence.', + }); + return; + } + + throw new Error('expected resolveCreateAnchor to throw'); + }); + + it('propagates TARGET_NOT_FOUND from findBlockByNodeIdOnly', () => { + mockedDeps.findBlockByNodeIdOnly.mockImplementation(() => { + throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'Block with nodeId "gone" was not found.', { + nodeId: 'gone', + }); + }); + + try { + resolveCreateAnchor(makeEditor(), { kind: 'block', nodeType: 'paragraph', nodeId: 'gone' }, 'before', 'step-42'); + } catch (error) { + expect(error).toBeInstanceOf(PlanError); + expect((error as PlanError).code).toBe('TARGET_NOT_FOUND'); + expect((error as PlanError).stepId).toBe('step-42'); + return; + } + + throw new Error('expected resolveCreateAnchor to throw TARGET_NOT_FOUND'); + }); + + it('propagates AMBIGUOUS_TARGET from findBlockByNodeIdOnly', () => { + mockedDeps.findBlockByNodeIdOnly.mockImplementation(() => { + throw new DocumentApiAdapterError('AMBIGUOUS_TARGET', 'Multiple blocks share nodeId "dup".', { + nodeId: 'dup', + count: 2, + }); + }); + + try { + resolveCreateAnchor(makeEditor(), { kind: 'block', nodeType: 'paragraph', nodeId: 'dup' }, 'before'); + } catch (error) { + expect(error).toBeInstanceOf(PlanError); + expect((error as PlanError).code).toBe('AMBIGUOUS_TARGET'); + return; + } + + throw new Error('expected resolveCreateAnchor to throw AMBIGUOUS_TARGET'); + }); + + it('attaches stepId to re-wrapped errors', () => { + mockedDeps.findBlockByNodeIdOnly.mockImplementation(() => { + throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'Not found.', { nodeId: 'x' }); + }); + + try { + resolveCreateAnchor(makeEditor(), { kind: 'block', nodeType: 'paragraph', nodeId: 'x' }, 'after', 'my-step'); + } catch (error) { + expect((error as PlanError).stepId).toBe('my-step'); + return; + } + + throw new Error('expected resolveCreateAnchor to throw'); + }); +}); diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.ts b/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.ts index ae001e977d..4c9b546826 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/create-insertion.ts @@ -6,13 +6,16 @@ * - Plan-engine `executeCreateStep` (executor.ts) * - Standalone `create-wrappers.ts` (for before/after target cases) * - * Scope: This module centralizes **position resolution** only. + * Scope: This module centralizes **position resolution** and **pre-flight + * nodeType validation** for create-like operations. * Node creation, ID generation, and command dispatch remain in their * respective call sites. */ +import type { BlockNodeAddress } from '@superdoc/document-api'; import type { Editor } from '../../core/Editor.js'; import { getBlockIndex } from '../helpers/index-cache.js'; +import { findBlockByNodeIdOnly, type BlockCandidate } from '../helpers/node-address-resolver.js'; import { planError } from './errors.js'; /** @@ -41,3 +44,67 @@ export function resolveBlockInsertionPos( } return position === 'before' ? candidate.pos : candidate.end; } + +// --------------------------------------------------------------------------- +// Pre-flight validated anchor resolution for create-like operations +// --------------------------------------------------------------------------- + +export type ResolvedCreateAnchor = { + pos: number; + anchor: BlockCandidate; +}; + +/** + * Resolves the insertion position for a create-like operation with + * pre-flight nodeType validation. + * + * Uses {@link findBlockByNodeIdOnly} for alias-aware, ambiguity-safe lookup. + * When the caller specifies a `nodeType` on the target, the resolved block's + * actual type must match — otherwise an `INVALID_TARGET` error is thrown with + * structured remediation details. + * + * **Do NOT use this for plan-engine steps** — those use + * {@link resolveBlockInsertionPos} directly because the plan step ID + * attribution requires a different error shape. + */ +export function resolveCreateAnchor( + editor: Editor, + target: BlockNodeAddress, + position: 'before' | 'after', + stepId?: string, +): ResolvedCreateAnchor { + const index = getBlockIndex(editor); + + let candidate: BlockCandidate; + try { + candidate = findBlockByNodeIdOnly(index, target.nodeId); + } catch (e) { + // Re-wrap as a planError so callers get consistent error shapes with stepId + if (e instanceof Error && 'code' in e) { + throw planError((e as { code: string }).code, e.message, stepId, (e as { details?: unknown }).details); + } + throw e; + } + + if (target.nodeType && candidate.nodeType !== target.nodeType) { + const remediation = + candidate.nodeType === 'listItem' + ? 'Use lists.insert to add an item to a list sequence.' + : `The block is a ${candidate.nodeType}, not a ${target.nodeType}.`; + + throw planError( + 'INVALID_TARGET', + `Expected ${target.nodeType}:${target.nodeId} but found ${candidate.nodeType}:${candidate.nodeId}.`, + stepId, + { + requestedNodeType: target.nodeType, + actualNodeType: candidate.nodeType, + nodeId: target.nodeId, + remediation, + }, + ); + } + + const pos = position === 'before' ? candidate.pos : candidate.end; + return { pos, anchor: candidate }; +} diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/create-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/create-wrappers.ts index 033d0f717a..e2e78495ff 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/create-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/create-wrappers.ts @@ -19,7 +19,7 @@ import type { } from '@superdoc/document-api'; import { clearIndexCache, getBlockIndex } from '../helpers/index-cache.js'; import { type BlockCandidate } from '../helpers/node-address-resolver.js'; -import { resolveBlockInsertionPos } from './create-insertion.js'; +import { resolveCreateAnchor } from './create-insertion.js'; import { collectTrackInsertRefsInRange } from '../helpers/tracked-change-refs.js'; import { DocumentApiAdapterError } from '../errors.js'; import { requireEditorCommand, ensureTrackedCapability } from '../helpers/mutation-helpers.js'; @@ -55,15 +55,15 @@ type InsertHeadingAtCommand = (options: InsertHeadingAtCommandOptions) => boolea function resolveCreateInsertPosition( editor: Editor, at: CreateParagraphInput['at'] | CreateHeadingInput['at'], - operationLabel: string, ): number { const location = at ?? { kind: 'documentEnd' }; if (location.kind === 'documentStart') return 0; if (location.kind === 'documentEnd') return editor.state.doc.content.size; - // Delegate before/after resolution to shared helper (§13.15 — single insertion path) - return resolveBlockInsertionPos(editor, location.target.nodeId, location.kind); + // Delegate before/after resolution to shared helper with pre-flight nodeType validation + const { pos } = resolveCreateAnchor(editor, location.target, location.kind); + return pos; } // --------------------------------------------------------------------------- @@ -155,7 +155,7 @@ export function createParagraphWrapper( ensureTrackedCapability(editor, { operation: 'create.paragraph' }); } - const insertAt = resolveCreateInsertPosition(editor, input.at, 'paragraph'); + const insertAt = resolveCreateInsertPosition(editor, input.at); if (options?.dryRun) { const canInsert = editor.can().insertParagraphAt?.({ @@ -190,6 +190,7 @@ export function createParagraphWrapper( } const paragraphId = uuidv4(); + let canonicalId = paragraphId; let trackedChangeRefs: CreateParagraphSuccessResult['trackedChangeRefs'] | undefined; const receipt = executeDomainCommand( @@ -205,6 +206,7 @@ export function createParagraphWrapper( clearIndexCache(editor); try { const paragraph = resolveCreatedBlock(editor, 'paragraph', paragraphId); + canonicalId = paragraph.nodeId; if (mode === 'tracked') { trackedChangeRefs = collectTrackInsertRefsInRange(editor, paragraph.pos, paragraph.end); } @@ -230,7 +232,7 @@ export function createParagraphWrapper( }; } - return buildParagraphCreateSuccess(paragraphId, trackedChangeRefs); + return buildParagraphCreateSuccess(canonicalId, trackedChangeRefs); } // --------------------------------------------------------------------------- @@ -252,7 +254,7 @@ export function createHeadingWrapper( ensureTrackedCapability(editor, { operation: 'create.heading' }); } - const insertAt = resolveCreateInsertPosition(editor, input.at, 'heading'); + const insertAt = resolveCreateInsertPosition(editor, input.at); if (options?.dryRun) { const canInsert = editor.can().insertHeadingAt?.({ @@ -288,6 +290,7 @@ export function createHeadingWrapper( } const headingId = uuidv4(); + let canonicalId = headingId; let trackedChangeRefs: CreateHeadingSuccessResult['trackedChangeRefs'] | undefined; const receipt = executeDomainCommand( @@ -304,6 +307,7 @@ export function createHeadingWrapper( clearIndexCache(editor); try { const heading = resolveCreatedBlock(editor, 'heading', headingId); + canonicalId = heading.nodeId; if (mode === 'tracked') { trackedChangeRefs = collectTrackInsertRefsInRange(editor, heading.pos, heading.end); } @@ -326,5 +330,5 @@ export function createHeadingWrapper( }; } - return buildHeadingCreateSuccess(headingId, trackedChangeRefs); + return buildHeadingCreateSuccess(canonicalId, trackedChangeRefs); } diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/images-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/images-wrappers.ts index c0c56431b5..ee85c88d69 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/images-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/images-wrappers.ts @@ -54,7 +54,7 @@ import { import { DocumentApiAdapterError } from '../errors.js'; import { rejectTrackedMode } from '../helpers/mutation-helpers.js'; import { executeDomainCommand } from './plan-wrappers.js'; -import { resolveBlockInsertionPos } from './create-insertion.js'; +import { resolveCreateAnchor } from './create-insertion.js'; import { readImageDimensionsFromDataUri } from '../../core/super-converter/image-dimensions.js'; import { generateUniqueDocPrId } from '../../extensions/image/imageHelpers/startImageUpload.js'; @@ -172,9 +172,10 @@ function resolveImageInsertPosition(editor: Editor, location: ImageCreateLocatio return editor.state.doc.content.size; case 'before': case 'after': - return resolveBlockInsertionPos(editor, location.target.nodeId, location.kind); + return resolveCreateAnchor(editor, location.target, location.kind).pos; case 'inParagraph': { - const pos = resolveBlockInsertionPos(editor, location.target.nodeId, 'before'); + // Pre-flight nodeType validation via resolveCreateAnchor, then compute inline offset + const { pos } = resolveCreateAnchor(editor, location.target, 'before'); // pos points to the start of the paragraph node; +1 enters the inline content. // Add any caller-supplied character offset within the paragraph text. return pos + 1 + (location.offset ?? 0); diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/toc-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/toc-wrappers.ts index 9f59d760a1..1278e1268e 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/toc-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/toc-wrappers.ts @@ -43,7 +43,7 @@ import { getRevision } from './revision-tracker.js'; import { executeDomainCommand } from './plan-wrappers.js'; import { rejectTrackedMode } from '../helpers/mutation-helpers.js'; import { clearIndexCache } from '../helpers/index-cache.js'; -import { resolveBlockInsertionPos } from './create-insertion.js'; +import { resolveCreateAnchor } from './create-insertion.js'; // --------------------------------------------------------------------------- // Typed patch helper @@ -636,7 +636,7 @@ export function createTableOfContentsWrapper( } else if (at.kind === 'documentEnd') { pos = editor.state.doc.content.size; } else { - pos = resolveBlockInsertionPos(editor, at.target.nodeId, at.kind); + pos = resolveCreateAnchor(editor, at.target, at.kind).pos; } // Build instruction from config patch or use defaults From 16c6b54fa4d883d252caaf9352e6ba98f17a5202 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 10 Mar 2026 22:43:19 -0700 Subject: [PATCH 2/4] chore: fix codegen sdk tests --- packages/sdk/codegen/src/__tests__/contract-integrity.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/codegen/src/__tests__/contract-integrity.test.ts b/packages/sdk/codegen/src/__tests__/contract-integrity.test.ts index 4ac16040ae..91c4150af4 100644 --- a/packages/sdk/codegen/src/__tests__/contract-integrity.test.ts +++ b/packages/sdk/codegen/src/__tests__/contract-integrity.test.ts @@ -348,7 +348,7 @@ describe('Intent name integrity', () => { }); describe('agentVisible param annotation integrity', () => { - const EXPECTED_HIDDEN = new Set(['out', 'expectedRevision', 'dryRun']); + const EXPECTED_HIDDEN = new Set(['out']); test('expected transport-envelope params are agentVisible: false', async () => { const contract = await loadJson(CONTRACT_PATH); From 8ef8ac6154e4a5f13df1c66cd3865ee03838694d Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 10 Mar 2026 23:04:46 -0700 Subject: [PATCH 3/4] fix(document-api): prevent silent data loss and incorrect ordinals in blocks.delete/deleteRange --- .../plan-engine/blocks-wrappers.test.ts | 105 ++++++++++++++++ .../plan-engine/blocks-wrappers.ts | 114 ++++++++++++++---- 2 files changed, 194 insertions(+), 25 deletions(-) diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts index a853afa360..2c5f67b0d6 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts @@ -393,6 +393,39 @@ describe('blocksDeleteWrapper', () => { }); }); + // ------------------------------------------------------------------------- + // Ordinal consistency + // ------------------------------------------------------------------------- + + describe('ordinal consistency with blocks.list', () => { + it('reports top-level ordinal, not descendant-traversal index position', () => { + // A table with a nested tableRow — the full block index (via descendants()) + // includes: table, tableRow, paragraph → indexOf(paragraph) = 2. + // But blocks.list only lists top-level blocks: table=0, paragraph=1. + const tableRow = createNode('tableRow', [], { + attrs: { paraId: 'tr1', sdBlockId: 'tr1' }, + isBlock: true, + inlineContent: false, + }); + const table = createNode('table', [tableRow], { + attrs: { blockId: 't1', sdBlockId: 't1' }, + isBlock: true, + inlineContent: false, + }); + const paragraph = createNode('paragraph', [createNode('text', [], { text: 'Hello' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + + const { editor } = makeBlockDeleteEditor({ children: [table, paragraph] }); + const result = blocksDeleteWrapper(editor, makeInput('paragraph', 'p1'), { changeMode: 'direct' }); + + // Must be 1 (top-level: table=0, paragraph=1), NOT 2 (descendant index position) + expect(result.deletedBlock.ordinal).toBe(1); + }); + }); + // ------------------------------------------------------------------------- // Cache invalidation // ------------------------------------------------------------------------- @@ -653,6 +686,78 @@ describe('blocksDeleteRangeWrapper', () => { } }); + it('rejects a range that would silently delete unrecognized node types', () => { + const p1 = createNode('paragraph', [createNode('text', [], { text: 'First' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + // A node type that mapBlockNodeType does not recognize (e.g., bibliography) + const bibliography = createNode('bibliography', [], { + attrs: { blockId: 'bib1' }, + isBlock: true, + inlineContent: false, + }); + const p2 = createNode('paragraph', [createNode('text', [], { text: 'Last' })], { + attrs: { paraId: 'p2', sdBlockId: 'p2' }, + isBlock: true, + inlineContent: true, + }); + + const editor = makeRangeDeleteEditor([p1, bibliography, p2]); + + try { + blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, + }, + { changeMode: 'direct' }, + ); + expect.unreachable('should have thrown'); + } catch (error) { + expect(error).toBeInstanceOf(DocumentApiAdapterError); + expect((error as DocumentApiAdapterError).code).toBe('INVALID_TARGET'); + expect((error as DocumentApiAdapterError).message).toContain('unrecognized'); + expect((error as DocumentApiAdapterError).message).toContain('bibliography'); + } + }); + + it('resolves correctly when different node types share the same nodeId', () => { + // A paragraph and a listItem both have paraId "shared" — different nodeTypes + // but the same raw nodeId. The old findBlockByNodeIdOnly approach would throw + // AMBIGUOUS_TARGET; composite-key lookup correctly disambiguates. + const para = createNode('paragraph', [createNode('text', [], { text: 'Text' })], { + attrs: { paraId: 'shared', sdBlockId: 'sb-para' }, + isBlock: true, + inlineContent: true, + }); + const listItem = createNode('paragraph', [createNode('text', [], { text: 'List item' })], { + attrs: { + paraId: 'shared', + sdBlockId: 'sb-li', + paragraphProperties: { numberingProperties: { numId: 1, ilvl: 0 } }, + }, + isBlock: true, + inlineContent: true, + }); + + const editor = makeRangeDeleteEditor([para, listItem]); + + // Should NOT throw AMBIGUOUS_TARGET — composite keys (paragraph:shared, listItem:shared) are distinct + const result = blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'shared' }, + end: { kind: 'block', nodeType: 'listItem', nodeId: 'shared' }, + }, + { changeMode: 'direct' }, + ); + expect(result.success).toBe(true); + expect(result.deletedCount).toBe(2); + }); + it('allows a range without section breaks', () => { const p1 = createNode('paragraph', [createNode('text', [], { text: 'First' })], { attrs: { paraId: 'p1', sdBlockId: 'p1' }, diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts index 0cf8ac5345..684a634748 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts @@ -10,6 +10,7 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; import type { Editor } from '../../core/Editor.js'; import { DELETABLE_BLOCK_NODE_TYPES, + type BlockNodeAddress, type BlocksDeleteInput, type BlocksDeleteResult, type BlocksListInput, @@ -23,10 +24,10 @@ import { import { clearIndexCache, getBlockIndex } from '../helpers/index-cache.js'; import { findBlockByIdStrict, - findBlockByNodeIdOnly, mapBlockNodeType, resolveBlockNodeId, type BlockCandidate, + type BlockIndex, } from '../helpers/node-address-resolver.js'; import { DocumentApiAdapterError } from '../errors.js'; import { requireEditorCommand, rejectTrackedMode } from '../helpers/mutation-helpers.js'; @@ -183,8 +184,13 @@ export function blocksDeleteWrapper( const candidate = findBlockByIdStrict(index, input.target); validateDeleteTargetNodeType(candidate.nodeType); - // Capture block summary before deletion for the receipt - const candidateOrdinal = index.candidates.indexOf(candidate); + // Compute ordinal in top-level order, consistent with blocks.list output. + // index.candidates includes nested blocks (tableRow, tableCell via descendants()), + // so using indexOf on it would produce ordinals that don't match blocks.list. + const topLevel = collectTopLevelBlocks(editor); + const candidateOrdinal = topLevel.findIndex( + (b) => b.nodeId === candidate.nodeId && b.nodeType === candidate.nodeType, + ); const deletedBlock = toBlockSummary(candidate, candidateOrdinal); const sdBlockId = resolveSdBlockId(candidate); @@ -250,6 +256,75 @@ function resolveTopLevelOrdinal(topLevel: BlockCandidate[], candidate: BlockCand ); } +/** + * Resolves a deleteRange endpoint using the composite `(nodeType, nodeId)` key. + * + * Using the composite key avoids false AMBIGUOUS_TARGET errors that occur with + * nodeId-only lookup when different node types share the same nodeId (e.g., a + * paragraph and a listItem both having paraId "abc123"). + * + * When the exact key isn't found, checks whether the nodeId exists under a + * different nodeType and throws a specific INVALID_TARGET diagnostic. + */ +function resolveRangeEndpoint(index: BlockIndex, address: BlockNodeAddress, label: string): BlockCandidate { + const key = `${address.nodeType}:${address.nodeId}`; + + if (index.ambiguous.has(key)) { + throw new DocumentApiAdapterError('AMBIGUOUS_TARGET', `Multiple blocks share key "${key}".`, { + target: address, + }); + } + + const candidate = index.byId.get(key); + if (candidate) return candidate; + + // Exact key not found — check if the nodeId exists under a different nodeType + const mismatch = index.candidates.find((c) => c.nodeId === address.nodeId); + if (mismatch) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `blocks.deleteRange ${label} expected ${address.nodeType}:${address.nodeId} but resolved to ${mismatch.nodeType}.`, + { expected: address.nodeType, actual: mismatch.nodeType, nodeId: address.nodeId }, + ); + } + + throw new DocumentApiAdapterError('TARGET_NOT_FOUND', `Block "${key}" was not found.`, { + target: address, + }); +} + +/** + * Rejects deletion ranges that contain unrecognized top-level nodes. + * + * `tr.delete(from, to)` removes ALL nodes in the positional span, including + * node types not recognized by `mapBlockNodeType()` (e.g., bibliography, + * footnotes). Without this check, those nodes would be silently destroyed. + */ +function rejectUnmappedNodesInRange(doc: ProseMirrorNode, rangeBlocks: BlockCandidate[]): void { + if (rangeBlocks.length === 0) return; + + const rangeFrom = rangeBlocks[0]!.pos; + const rangeTo = rangeBlocks[rangeBlocks.length - 1]!.end; + const recognizedPositions = new Set(rangeBlocks.map((b) => b.pos)); + + let offset = 0; + for (let i = 0; i < doc.childCount; i++) { + const child = doc.child(i); + const childEnd = offset + child.nodeSize; + + // Only inspect children that overlap the deletion range + if (childEnd > rangeFrom && offset < rangeTo && !recognizedPositions.has(offset)) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `blocks.deleteRange cannot delete range: unrecognized node "${child.type.name}" at position ${offset} would be silently removed.`, + { pmNodeType: child.type.name, pos: offset }, + ); + } + + offset = childEnd; + } +} + export function blocksDeleteRangeWrapper( editor: Editor, input: BlocksDeleteRangeInput, @@ -260,28 +335,12 @@ export function blocksDeleteRangeWrapper( // 1. Collect top-level blocks (direct children of doc node) const topLevel = collectTopLevelBlocks(editor); - // 2. Resolve start and end via nodeId (alias-aware) + // 2. Resolve start and end using composite (nodeType, nodeId) key. + // This avoids false AMBIGUOUS_TARGET errors when different node types + // share the same nodeId (e.g., a paragraph and a listItem with the same paraId). const index = getBlockIndex(editor); - const startCandidate = findBlockByNodeIdOnly(index, input.start.nodeId); - const endCandidate = findBlockByNodeIdOnly(index, input.end.nodeId); - - // 2a. Validate that resolved nodeType matches caller-supplied nodeType. - // Without this, a stale address (e.g. nodeType:"paragraph" for a listItem) - // would silently resolve and delete the wrong block type. - if (startCandidate.nodeType !== input.start.nodeType) { - throw new DocumentApiAdapterError( - 'INVALID_TARGET', - `blocks.deleteRange start expected ${input.start.nodeType}:${input.start.nodeId} but resolved to ${startCandidate.nodeType}.`, - { expected: input.start.nodeType, actual: startCandidate.nodeType, nodeId: input.start.nodeId }, - ); - } - if (endCandidate.nodeType !== input.end.nodeType) { - throw new DocumentApiAdapterError( - 'INVALID_TARGET', - `blocks.deleteRange end expected ${input.end.nodeType}:${input.end.nodeId} but resolved to ${endCandidate.nodeType}.`, - { expected: input.end.nodeType, actual: endCandidate.nodeType, nodeId: input.end.nodeId }, - ); - } + const startCandidate = resolveRangeEndpoint(index, input.start, 'start'); + const endCandidate = resolveRangeEndpoint(index, input.end, 'end'); // 3. Confirm both are top-level const startOrdinal = resolveTopLevelOrdinal(topLevel, startCandidate, 'start'); @@ -299,7 +358,12 @@ export function blocksDeleteRangeWrapper( // 5. Collect the range and build summaries const rangeBlocks = topLevel.slice(startOrdinal, endOrdinal + 1); - // 5a. Reject ranges that include section breaks + // 5a. Reject ranges that contain unrecognized top-level nodes. + // tr.delete(from, to) removes ALL nodes in the positional span, including + // node types not recognized by mapBlockNodeType (e.g., bibliography). + rejectUnmappedNodesInRange(editor.state.doc, rangeBlocks); + + // 5b. Reject ranges that include section breaks for (const block of rangeBlocks) { if (hasSectionBreak(block)) { throw new DocumentApiAdapterError( From 24607309e8a66d868822f82d660ba8c77f8f6dfa Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 10 Mar 2026 23:18:46 -0700 Subject: [PATCH 4/4] chore: fix cli testS --- .../plan-engine/blocks-wrappers.test.ts | 30 +++++++++++++++++++ .../plan-engine/blocks-wrappers.ts | 21 +++++++++---- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts index 2c5f67b0d6..7aa26794b0 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.test.ts @@ -724,6 +724,36 @@ describe('blocksDeleteRangeWrapper', () => { } }); + it('allows passthrough nodes in a deletion range (opaque OOXML preservation)', () => { + const p1 = createNode('paragraph', [createNode('text', [], { text: 'First' })], { + attrs: { paraId: 'p1', sdBlockId: 'p1' }, + isBlock: true, + inlineContent: true, + }); + const passthrough = createNode('passthroughBlock', [], { + attrs: { originalName: 'w:bookmarkStart', originalXml: '' }, + isBlock: true, + inlineContent: false, + }); + const p2 = createNode('paragraph', [createNode('text', [], { text: 'Last' })], { + attrs: { paraId: 'p2', sdBlockId: 'p2' }, + isBlock: true, + inlineContent: true, + }); + + const editor = makeRangeDeleteEditor([p1, passthrough, p2]); + const result = blocksDeleteRangeWrapper( + editor, + { + start: { kind: 'block', nodeType: 'paragraph', nodeId: 'p1' }, + end: { kind: 'block', nodeType: 'paragraph', nodeId: 'p2' }, + }, + { changeMode: 'direct' }, + ); + expect(result.success).toBe(true); + expect(result.deletedCount).toBe(2); // Only recognized blocks counted + }); + it('resolves correctly when different node types share the same nodeId', () => { // A paragraph and a listItem both have paraId "shared" — different nodeTypes // but the same raw nodeId. The old findBlockByNodeIdOnly approach would throw diff --git a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts index 684a634748..daa247a8cc 100644 --- a/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts +++ b/packages/super-editor/src/document-api-adapters/plan-engine/blocks-wrappers.ts @@ -44,6 +44,13 @@ const SUPPORTED_DELETE_NODE_TYPES = new Set(DELETABLE_BLOCK_NODE_TYPES); const REJECTED_DELETE_NODE_TYPES = new Set(['tableRow', 'tableCell']); const TEXT_PREVIEW_MAX_LENGTH = 80; +/** + * PM node types that `mapBlockNodeType` does not recognize but are safe to + * include in range deletions. Passthrough nodes preserve opaque OOXML XML + * and don't contain user-addressable content. + */ +const RANGE_DELETE_SAFE_NODE_TYPES = new Set(['passthroughBlock', 'passthroughInline']); + // --------------------------------------------------------------------------- // Shared helpers // --------------------------------------------------------------------------- @@ -314,11 +321,15 @@ function rejectUnmappedNodesInRange(doc: ProseMirrorNode, rangeBlocks: BlockCand // Only inspect children that overlap the deletion range if (childEnd > rangeFrom && offset < rangeTo && !recognizedPositions.has(offset)) { - throw new DocumentApiAdapterError( - 'INVALID_TARGET', - `blocks.deleteRange cannot delete range: unrecognized node "${child.type.name}" at position ${offset} would be silently removed.`, - { pmNodeType: child.type.name, pos: offset }, - ); + // Passthrough nodes (opaque OOXML preservation) are safe to delete — + // they contain no user-addressable content. + if (!RANGE_DELETE_SAFE_NODE_TYPES.has(child.type.name)) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `blocks.deleteRange cannot delete range: unrecognized node "${child.type.name}" at position ${offset} would be silently removed.`, + { pmNodeType: child.type.name, pos: offset }, + ); + } } offset = childEnd;