Skip to content

Commit 7a9a448

Browse files
authored
fix: bug text edit commands fail on targets returned by find (#2488)
* fix(document-api): unify insert target/ref handling and restore CLI offset compat * fix: preserve paragraph runtime ids and reject empty insert refs * fix: selection insert target validation and alias ref ambiguity * fix: document-api ref-based insert target resolution * fix: insert receipt targets for structured content * fix: behavior tests * chore: fix test import
1 parent d5317ef commit 7a9a448

44 files changed

Lines changed: 1148 additions & 503 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/cli/src/__tests__/cli.test.ts

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,42 @@ describe('superdoc CLI', () => {
10281028
expect(verifyEnvelope.data.result.total).toBeGreaterThan(0);
10291029
});
10301030

1031-
test('insert with --block-id and --offset targets a specific block position', async () => {
1031+
test('insert with --block-id and --offset targets a specific block position (legacy compat)', async () => {
1032+
const insertSource = join(TEST_DIR, 'insert-blockid-legacy-offset-source.docx');
1033+
const insertOut = join(TEST_DIR, 'insert-blockid-legacy-offset-out.docx');
1034+
await copyFile(SAMPLE_DOC, insertSource);
1035+
1036+
const target = await firstTextRange(['find', insertSource, '--type', 'text', '--pattern', 'Wilde']);
1037+
1038+
const insertResult = await runCli([
1039+
'insert',
1040+
insertSource,
1041+
'--block-id',
1042+
target.blockId,
1043+
'--offset',
1044+
'0',
1045+
'--value',
1046+
'CLI_BLOCKID_LEGACY_OFFSET_INSERT',
1047+
'--out',
1048+
insertOut,
1049+
]);
1050+
1051+
expect(insertResult.code).toBe(0);
1052+
1053+
const verifyResult = await runCli([
1054+
'find',
1055+
insertOut,
1056+
'--type',
1057+
'text',
1058+
'--pattern',
1059+
'CLI_BLOCKID_LEGACY_OFFSET_INSERT',
1060+
]);
1061+
expect(verifyResult.code).toBe(0);
1062+
const verifyEnvelope = parseJsonOutput<SuccessEnvelope<{ result: { total: number } }>>(verifyResult);
1063+
expect(verifyEnvelope.data.result.total).toBeGreaterThan(0);
1064+
});
1065+
1066+
test('insert with --block-id and --start/--end targets a specific block position', async () => {
10321067
const insertSource = join(TEST_DIR, 'insert-blockid-offset-source.docx');
10331068
const insertOut = join(TEST_DIR, 'insert-blockid-offset-out.docx');
10341069
await copyFile(SAMPLE_DOC, insertSource);
@@ -1041,7 +1076,9 @@ describe('superdoc CLI', () => {
10411076
insertSource,
10421077
'--block-id',
10431078
target.blockId,
1044-
'--offset',
1079+
'--start',
1080+
'0',
1081+
'--end',
10451082
'0',
10461083
'--value',
10471084
'CLI_BLOCKID_OFFSET_INSERT_1597',
@@ -1064,7 +1101,7 @@ describe('superdoc CLI', () => {
10641101
expect(verifyEnvelope.data.result.total).toBeGreaterThan(0);
10651102
});
10661103

1067-
test('insert with --block-id alone defaults offset to 0', async () => {
1104+
test('insert with --block-id alone defaults start/end to 0', async () => {
10681105
const insertSource = join(TEST_DIR, 'insert-blockid-only-source.docx');
10691106
const insertOut = join(TEST_DIR, 'insert-blockid-only-out.docx');
10701107
await copyFile(SAMPLE_DOC, insertSource);
@@ -1093,15 +1130,19 @@ describe('superdoc CLI', () => {
10931130
expect(resolvedTarget?.range.end).toBe(0);
10941131
});
10951132

1096-
test('insert with --offset but no --block-id returns INVALID_ARGUMENT', async () => {
1133+
test('insert with --start but no --block-id returns validation error', async () => {
10971134
const insertSource = join(TEST_DIR, 'insert-offset-no-blockid-source.docx');
10981135
const insertOut = join(TEST_DIR, 'insert-offset-no-blockid-out.docx');
10991136
await copyFile(SAMPLE_DOC, insertSource);
11001137

1138+
// --start/--end without --block-id are not normalized into a target.
1139+
// They pass through as unknown fields and are rejected by validation.
11011140
const result = await runCli([
11021141
'insert',
11031142
insertSource,
1104-
'--offset',
1143+
'--start',
1144+
'5',
1145+
'--end',
11051146
'5',
11061147
'--value',
11071148
'should-fail',
@@ -1110,9 +1151,6 @@ describe('superdoc CLI', () => {
11101151
]);
11111152

11121153
expect(result.code).toBe(1);
1113-
const envelope = parseJsonOutput<ErrorEnvelope>(result);
1114-
expect(envelope.error.code).toBe('INVALID_ARGUMENT');
1115-
expect(envelope.error.message).toContain('Unknown field');
11161154
});
11171155

11181156
test('insert with --type html inserts HTML content into the document', async () => {

apps/cli/src/__tests__/conformance/harness.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -375,9 +375,10 @@ export class ConformanceHarness {
375375
): Promise<{ docPath: string; changeId: string; target: TextRangeAddress }> {
376376
const sourceDoc = await this.copyFixtureDoc(`${label}-source`);
377377
const target = await this.firstTextRange(sourceDoc, stateDir);
378-
const collapsedTarget: TextRangeAddress = {
379-
...target,
380-
range: { start: target.range.start, end: target.range.start },
378+
const selectionTarget = {
379+
kind: 'selection',
380+
start: { kind: 'text', blockId: target.blockId, offset: target.range.start },
381+
end: { kind: 'text', blockId: target.blockId, offset: target.range.start },
381382
};
382383
const outDoc = this.createOutputPath(`${label}-with-tracked-change`);
383384

@@ -386,7 +387,7 @@ export class ConformanceHarness {
386387
'insert',
387388
sourceDoc,
388389
'--target-json',
389-
JSON.stringify(collapsedTarget),
390+
JSON.stringify(selectionTarget),
390391
'--value',
391392
'TRACKED_CONFORMANCE_TOKEN',
392393
'--change-mode',
@@ -412,7 +413,7 @@ export class ConformanceHarness {
412413
throw new Error(`Tracked-change fixture did not produce a tracked change id for ${label}`);
413414
}
414415

415-
return { docPath: outDoc, changeId, target: collapsedTarget };
416+
return { docPath: outDoc, changeId, target };
416417
}
417418

418419
async firstTwoBlockAddresses(

apps/cli/src/__tests__/conformance/scenarios.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,15 +2084,19 @@ export const SUCCESS_SCENARIOS = {
20842084
'doc.insert': async (harness: ConformanceHarness): Promise<ScenarioInvocation> => {
20852085
const stateDir = await harness.createStateDir('doc-insert-success');
20862086
const docPath = await harness.copyFixtureDoc('doc-insert');
2087-
const target = await harness.firstTextRange(docPath, stateDir);
2088-
const collapsed = { ...target, range: { start: target.range.start, end: target.range.start } };
2087+
const textRange = await harness.firstTextRange(docPath, stateDir);
2088+
const selectionTarget = {
2089+
kind: 'selection',
2090+
start: { kind: 'text', blockId: textRange.blockId, offset: textRange.range.start },
2091+
end: { kind: 'text', blockId: textRange.blockId, offset: textRange.range.start },
2092+
};
20892093
return {
20902094
stateDir,
20912095
args: [
20922096
'insert',
20932097
docPath,
20942098
'--target-json',
2095-
JSON.stringify(collapsed),
2099+
JSON.stringify(selectionTarget),
20962100
'--value',
20972101
'CONFORMANCE_INSERT',
20982102
'--out',

apps/cli/src/cli/operation-params.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,11 +478,6 @@ const TEXT_TARGET_FLAT_PARAMS: CliOperationParamSpec[] = [
478478
{ name: 'end', kind: 'flag', type: 'number', description: 'End offset within the block (character index).' },
479479
];
480480

481-
const INSERT_FLAT_PARAMS: CliOperationParamSpec[] = [
482-
{ name: 'blockId', kind: 'flag', flag: 'block-id', type: 'string', description: 'Block ID of the target paragraph.' },
483-
{ name: 'offset', kind: 'flag', type: 'number', description: 'Character offset within the block for insertion.' },
484-
];
485-
486481
const LIST_TARGET_FLAT_PARAMS: CliOperationParamSpec[] = [
487482
{ name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string', description: 'Node ID of the target list item.' },
488483
];
@@ -536,7 +531,15 @@ const EXTRA_CLI_PARAMS: Partial<Record<string, CliOperationParamSpec[]>> = {
536531
},
537532
],
538533
// Text-range operations: flat flags (--block-id, --start, --end) as shortcuts for --target-json
539-
'doc.insert': [...INSERT_FLAT_PARAMS],
534+
'doc.insert': [
535+
...TEXT_TARGET_FLAT_PARAMS,
536+
{
537+
name: 'offset',
538+
kind: 'flag',
539+
type: 'number',
540+
description: 'Character offset for insertion (alias for --start/--end with same value).',
541+
},
542+
],
540543
'doc.replace': [...TEXT_TARGET_FLAT_PARAMS],
541544
'doc.delete': [...TEXT_TARGET_FLAT_PARAMS],
542545
'doc.styles.apply': [

apps/cli/src/lib/invoke-input.ts

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,12 @@ const FORMAT_TARGET_OPERATIONS = CLI_DOC_OPERATIONS.filter((operationId): operat
9393
* The CLI still supports legacy single-block text range flags/JSON inputs and
9494
* upgrades them to the equivalent SelectionTarget before dispatch.
9595
*/
96-
const SELECTION_TARGET_OPERATIONS = new Set<CliExposedOperationId>(['replace', 'delete', ...FORMAT_TARGET_OPERATIONS]);
96+
const SELECTION_TARGET_OPERATIONS = new Set<CliExposedOperationId>([
97+
'insert',
98+
'replace',
99+
'delete',
100+
...FORMAT_TARGET_OPERATIONS,
101+
]);
97102

98103
/**
99104
* Operations that still accept a text-range target (textAddressSchema):
@@ -104,11 +109,7 @@ const SELECTION_TARGET_OPERATIONS = new Set<CliExposedOperationId>(['replace', '
104109
*/
105110
const TEXT_ADDRESS_TARGET_OPERATIONS = new Set<CliExposedOperationId>(['comments.create', 'comments.patch']);
106111

107-
/**
108-
* Insert is a text-range operation but uses `offset` instead of `start`/`end`
109-
* to specify a zero-width insertion point.
110-
*/
111-
const INSERT_OPERATION: CliExposedOperationId = 'insert';
112+
// INSERT_OPERATION removed — insert now uses SelectionTarget via SELECTION_TARGET_OPERATIONS.
112113

113114
/**
114115
* List operations that accept a list-item target (listItemAddressSchema):
@@ -196,14 +197,16 @@ function normalizeFlatTargetFlags(operationId: CliExposedOperationId, apiInput:
196197
return apiInput;
197198
}
198199

199-
// --- Selection-based text mutations (replace, delete, format.*) ---
200+
// --- Selection-based text mutations (insert, replace, delete, format.*) ---
200201
if (SELECTION_TARGET_OPERATIONS.has(operationId)) {
201202
const blockId = apiInput.blockId;
202203
if (typeof blockId === 'string') {
203-
const start = typeof apiInput.start === 'number' ? apiInput.start : 0;
204-
const end = typeof apiInput.end === 'number' ? apiInput.end : 0;
204+
// Legacy --offset for insert: expand to collapsed start/end
205+
const hasOffset = typeof apiInput.offset === 'number';
206+
const start = typeof apiInput.start === 'number' ? apiInput.start : hasOffset ? (apiInput.offset as number) : 0;
207+
const end = typeof apiInput.end === 'number' ? apiInput.end : hasOffset ? (apiInput.offset as number) : 0;
205208
assertLegacySelectionTargetSupported(operationId, { range: { start, end } });
206-
const { blockId: _, start: _s, end: _e, ...rest } = apiInput;
209+
const { blockId: _, start: _s, end: _e, offset: _o, ...rest } = apiInput;
207210
return {
208211
...rest,
209212
target: textAddressToSelectionTarget({ blockId, range: { start, end } }),
@@ -227,20 +230,6 @@ function normalizeFlatTargetFlags(operationId: CliExposedOperationId, apiInput:
227230
return apiInput;
228231
}
229232

230-
// --- Insert operation (uses offset for zero-width insertion point) ---
231-
if (operationId === INSERT_OPERATION) {
232-
const blockId = apiInput.blockId;
233-
if (typeof blockId === 'string') {
234-
const offset = typeof apiInput.offset === 'number' ? apiInput.offset : 0;
235-
const { blockId: _, offset: _o, ...rest } = apiInput;
236-
return {
237-
...rest,
238-
target: { kind: 'text', blockId, range: { start: offset, end: offset } },
239-
};
240-
}
241-
return apiInput;
242-
}
243-
244233
// --- Block delete (nodeType + nodeId → block target) ---
245234
if (operationId === 'blocks.delete') {
246235
const nodeType = apiInput.nodeType;

apps/cli/src/lib/operation-args.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@ function acceptsLegacyTextAddressTarget(
9797
): boolean {
9898
if (param.name !== 'target' || !isTextAddressLike(value)) return false;
9999
const docApiId = toDocApiId(operationId);
100-
return docApiId === 'replace' || docApiId === 'delete' || docApiId?.startsWith('format.') === true;
100+
return (
101+
docApiId === 'insert' || docApiId === 'replace' || docApiId === 'delete' || docApiId?.startsWith('format.') === true
102+
);
101103
}
102104

103105
/**

apps/docs/document-api/reference/_generated-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -986,5 +986,5 @@
986986
}
987987
],
988988
"marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}",
989-
"sourceHash": "4b45100573fb74d1eb2f006d5064fbcc2e5af3331272207dccbab5f0c97142ef"
989+
"sourceHash": "52013243a974232110399da20b569ed98bfa7ab25ab0c09f314168b748ebc7a8"
990990
}

apps/docs/document-api/reference/index.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ The tables below are grouped by namespace.
7070
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/markdown-to-fragment"><code>markdownToFragment</code></a></span> | <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><code>editor.doc.markdownToFragment(...)</code></span> | Convert a Markdown string into an SDM/1 structural fragment. |
7171
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/info"><code>info</code></a></span> | <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><code>editor.doc.info(...)</code></span> | Return document summary info including word, character, paragraph, heading, table, image, comment, tracked-change, SDT-field, list, and page counts, plus outline and capabilities. |
7272
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/clear-content"><code>clearContent</code></a></span> | <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><code>editor.doc.clearContent(...)</code></span> | Clear all document body content, leaving a single empty paragraph. |
73-
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/insert"><code>insert</code></a></span> | <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><code>editor.doc.insert(...)</code></span> | Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. |
73+
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/insert"><code>insert</code></a></span> | <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><code>editor.doc.insert(...)</code></span> | Insert content into the document. Two input shapes: text-based (value + type) inserts inline content at a SelectionTarget or ref position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target/ref is omitted, content appends at the end of the document. Text mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. |
7474
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/replace"><code>replace</code></a></span> | <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><code>editor.doc.replace(...)</code></span> | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts a BlockNodeAddress (replaces whole block), SelectionTarget (expands to full covered block boundaries), or ref plus SDFragment content. |
7575
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/delete"><code>delete</code></a></span> | <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><code>editor.doc.delete(...)</code></span> | Delete content at a contiguous document selection. Accepts a SelectionTarget or mutation-ready ref. Supports cross-block deletion and optional block-edge expansion via behavior mode. |
7676

0 commit comments

Comments
 (0)