Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/cli/src/__tests__/conformance/scenarios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2955,7 +2955,7 @@ export const SUCCESS_SCENARIOS = {
'--destination-json',
JSON.stringify({ kind: 'documentEnd' }),
]),
'doc.tables.split': tableMutationScenario('tables.split', ['--at-row-index', '1']),
'doc.tables.split': tableMutationScenario('tables.split', ['--row-index', '1']),
'doc.tables.convertToText': tableMutationScenario('tables.convertToText', ['--delimiter', 'tab']),
'doc.tables.setLayout': tableMutationScenario('tables.setLayout', ['--alignment', 'center']),
'doc.tables.insertRow': tableMutationScenario('tables.insertRow', ['--row-index', '0', '--position', 'below']),
Expand Down
24 changes: 22 additions & 2 deletions apps/cli/src/cli/operation-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,15 @@ export const CLI_OPERATION_METADATA: Record<CliOperationId, CliOperationMetadata
// Option specs (derived mechanically from params)
// ---------------------------------------------------------------------------

function deriveOptionSpecs(params: readonly CliOperationParamSpec[]): CliOperationOptionSpec[] {
// Legacy flag aliases — accepted by the parser but not in the canonical schema.
// The pre-validation normalizer in operation-executor.ts maps aliased values
// to their canonical names before schema validation.
const OPTION_FLAG_ALIASES: Partial<Record<string, Record<string, string[]>>> = {
// SD-2132: tables.split renamed atRowIndex → rowIndex.
'doc.tables.split': { 'row-index': ['at-row-index'] },
};

function deriveOptionSpecs(operationId: string, params: readonly CliOperationParamSpec[]): CliOperationOptionSpec[] {
const specs: CliOperationOptionSpec[] = [];

for (const param of params) {
Expand All @@ -802,11 +810,23 @@ function deriveOptionSpecs(params: readonly CliOperationParamSpec[]): CliOperati
});
}

const aliases = OPTION_FLAG_ALIASES[operationId];
if (aliases) {
for (const spec of specs) {
if (aliases[spec.name]) {
spec.aliases = aliases[spec.name];
}
}
}

return specs;
}

export const CLI_OPERATION_OPTION_SPECS: Record<CliOperationId, CliOperationOptionSpec[]> = Object.fromEntries(
CLI_OPERATION_IDS.map((operationId) => [operationId, deriveOptionSpecs(CLI_OPERATION_METADATA[operationId].params)]),
CLI_OPERATION_IDS.map((operationId) => [
operationId,
deriveOptionSpecs(operationId, CLI_OPERATION_METADATA[operationId].params),
]),
) as Record<CliOperationId, CliOperationOptionSpec[]>;

// Exposed for unit testing $ref resolution only
Expand Down
19 changes: 19 additions & 0 deletions apps/cli/src/lib/operation-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,24 @@ async function preflightCallContext(
}
}

// ---------------------------------------------------------------------------
// Legacy input normalization
// ---------------------------------------------------------------------------

/**
* Maps legacy parameter names to their canonical replacements before schema
* validation. Keeps backward compat for both wrapper flags and `call --input-json`.
*/
function normalizeLegacyInput(operationId: string, input: Record<string, unknown>): Record<string, unknown> {
// SD-2132: tables.split renamed atRowIndex → rowIndex.
if (operationId === 'doc.tables.split' && input.atRowIndex !== undefined) {
const { atRowIndex, ...rest } = input;
// When both are present, prefer the canonical rowIndex; otherwise migrate.
return rest.rowIndex !== undefined ? rest : { ...rest, rowIndex: atRowIndex };
}
return input;
}

export async function executeOperation(request: ExecuteOperationRequest): Promise<CommandExecution> {
let input: Record<string, unknown>;
const baseContext = request.context;
Expand Down Expand Up @@ -209,6 +227,7 @@ export async function executeOperation(request: ExecuteOperationRequest): Promis
input = (pruneUndefinedDeep(request.input) ?? {}) as Record<string, unknown>;
}

input = normalizeLegacyInput(request.operationId, input);
validateOperationInputData(request.operationId, input, commandName);
await preflightCallContext(request.operationId, input, baseContext);
const effectiveContext = applySessionInputToContext(baseContext, input);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -982,5 +982,5 @@
}
],
"marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}",
"sourceHash": "31ad98dfc8659ebef07fbe3070add1c26fbbc8e0fbfa284e52d0b17e9fdd6b0f"
"sourceHash": "b1e07574718dbcf5b2b9c21f2d944fb24d5a487eeadd1fc07ea1c117bba8aa83"
}
16 changes: 8 additions & 8 deletions apps/docs/document-api/reference/tables/split.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Returns a TableMutationResult receipt confirming the table was split at the targ

| Field | Type | Required | Description |
| --- | --- | --- | --- |
| `atRowIndex` | integer | yes | |
| `rowIndex` | integer | yes | |
| `target` | TableAddress | yes | TableAddress |
| `target.kind` | `"block"` | yes | Constant: `"block"` |
| `target.nodeId` | string | yes | |
Expand All @@ -40,14 +40,14 @@ Returns a TableMutationResult receipt confirming the table was split at the targ

| Field | Type | Required | Description |
| --- | --- | --- | --- |
| `atRowIndex` | integer | yes | |
| `nodeId` | string | yes | |
| `rowIndex` | integer | yes | |

### Example request

```json
{
"atRowIndex": 1,
"rowIndex": 1,
"target": {
"kind": "block",
"nodeId": "node-def456",
Expand Down Expand Up @@ -132,19 +132,19 @@ When present, `result.table` is the follow-up address to reuse after this call.
}
],
"properties": {
"atRowIndex": {
"minimum": 1,
"type": "integer"
},
"nodeId": {
"type": "string"
},
"rowIndex": {
"minimum": 1,
"type": "integer"
},
"target": {
"$ref": "#/$defs/TableAddress"
}
},
"required": [
"atRowIndex"
"rowIndex"
],
"type": "object"
}
Expand Down
4 changes: 2 additions & 2 deletions packages/document-api/src/contract/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@
const trackedChangeAddressSchema = ref('TrackedChangeAddress');
const entityAddressSchema = ref('EntityAddress');
const selectionTargetSchema = ref('SelectionTarget');
const targetLocatorSchema = ref('TargetLocator');

Check warning on line 509 in packages/document-api/src/contract/schemas.ts

View workflow job for this annotation

GitHub Actions / validate

'targetLocatorSchema' is assigned a value but never used. Allowed unused vars must match /^_/u
const deleteBehaviorSchema = ref('DeleteBehavior');
const resolvedHandleSchema = ref('ResolvedHandle');
const pageInfoSchema = ref('PageInfo');
Expand Down Expand Up @@ -770,7 +770,7 @@
text: { type: 'string' },
});

const nodeInfoSchema: JsonSchema = {

Check warning on line 773 in packages/document-api/src/contract/schemas.ts

View workflow job for this annotation

GitHub Actions / validate

'nodeInfoSchema' is assigned a value but never used. Allowed unused vars must match /^_/u
type: 'object',
required: ['nodeType', 'kind'],
properties: {
Expand All @@ -786,7 +786,7 @@
additionalProperties: false,
};

const matchContextSchema = objectSchema(

Check warning on line 789 in packages/document-api/src/contract/schemas.ts

View workflow job for this annotation

GitHub Actions / validate

'matchContextSchema' is assigned a value but never used. Allowed unused vars must match /^_/u
{
address: nodeAddressSchema,
snippet: { type: 'string' },
Expand All @@ -797,7 +797,7 @@
['address', 'snippet', 'highlightRange'],
);

const unknownNodeDiagnosticSchema = objectSchema(

Check warning on line 800 in packages/document-api/src/contract/schemas.ts

View workflow job for this annotation

GitHub Actions / validate

'unknownNodeDiagnosticSchema' is assigned a value but never used. Allowed unused vars must match /^_/u
{
message: { type: 'string' },
address: nodeAddressSchema,
Expand Down Expand Up @@ -4766,9 +4766,9 @@
{
target: tableAddressSchema,
nodeId: { type: 'string' },
atRowIndex: { type: 'integer', minimum: 1 },
rowIndex: { type: 'integer', minimum: 1 },
},
['atRowIndex'],
['rowIndex'],
),
oneOf: [{ required: ['target'] }, { required: ['nodeId'] }],
},
Expand Down
4 changes: 3 additions & 1 deletion packages/document-api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ import {
executeRowLocatorOp,
executeCellOrTableScopedCellLocatorOp,
executeDocumentLevelTableOp,
normalizeTablesSplitInput,
executeTablesApplyStyle,
executeTablesSetBorders,
executeTablesSetTableOptions,
Expand Down Expand Up @@ -2230,7 +2231,8 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi {
return executeTableLocatorOp('tables.move', adapters.tables.move.bind(adapters.tables), input, options);
},
split(input, options?) {
return executeTableLocatorOp('tables.split', adapters.tables.split.bind(adapters.tables), input, options);
const normalized = normalizeTablesSplitInput(input);
return executeRowLocatorOp('tables.split', adapters.tables.split.bind(adapters.tables), normalized, options);
},
convertToText(input, options?) {
return executeTableLocatorOp(
Expand Down
42 changes: 41 additions & 1 deletion packages/document-api/src/tables/tables.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,50 @@
import { describe, expect, it, vi } from 'vitest';
import { executeTablesApplyStyle, executeTablesSetBorders, executeTablesSetTableOptions } from './tables.js';
import {
executeTablesApplyStyle,
executeTablesSetBorders,
executeTablesSetTableOptions,
normalizeTablesSplitInput,
} from './tables.js';
import { DocumentApiValidationError } from '../errors.js';

const MOCK_ADAPTER = vi.fn(() => ({ success: true }));
const nodeId = 'table-1';

describe('normalizeTablesSplitInput', () => {
it('passes through canonical rowIndex unchanged', () => {
const input = { nodeId: 'table-1', rowIndex: 2 };
expect(normalizeTablesSplitInput(input)).toEqual(input);
});

it('maps legacy atRowIndex to rowIndex', () => {
const input = { nodeId: 'table-1', atRowIndex: 3 };
const result = normalizeTablesSplitInput(input);
expect(result).toEqual({ nodeId: 'table-1', rowIndex: 3 });
expect(result).not.toHaveProperty('atRowIndex');
});

it('accepts both when values match (prefers rowIndex)', () => {
const input = { nodeId: 'table-1', rowIndex: 1, atRowIndex: 1 };
const result = normalizeTablesSplitInput(input);
expect(result).toEqual({ nodeId: 'table-1', rowIndex: 1 });
expect(result).not.toHaveProperty('atRowIndex');
});

it('rejects conflicting rowIndex and atRowIndex', () => {
const input = { nodeId: 'table-1', rowIndex: 1, atRowIndex: 2 };
expect(() => normalizeTablesSplitInput(input)).toThrow(
'tables.split: cannot provide both rowIndex and atRowIndex with different values.',
);
});

it('preserves all other input fields', () => {
const input = { nodeId: 'table-1', atRowIndex: 1, target: undefined };
const result = normalizeTablesSplitInput(input);
expect(result.nodeId).toBe('table-1');
expect(result.rowIndex).toBe(1);
});
});

describe('executeTablesApplyStyle validation', () => {
it('rejects when neither styleId nor styleOptions is provided', () => {
expect(() => executeTablesApplyStyle('tables.applyStyle', MOCK_ADAPTER, { nodeId } as any)).toThrow(
Expand Down
27 changes: 27 additions & 0 deletions packages/document-api/src/tables/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,33 @@ function validateCellOrTableScopedCellLocator(input: CellOrTableScopedCellLocato
}
}

// ---------------------------------------------------------------------------
// tables.split input normalization
// ---------------------------------------------------------------------------

/**
* Normalizes legacy `atRowIndex` to canonical `rowIndex` for tables.split.
*
* Accepts either name, prefers `rowIndex` when both match, and rejects
* conflicting dual-name input. Returns the input unchanged when only
* `rowIndex` is present.
*/
export function normalizeTablesSplitInput<T extends { rowIndex?: number }>(input: T): T {
const legacy = (input as Record<string, unknown>).atRowIndex;
if (legacy === undefined) return input;

if (input.rowIndex !== undefined && input.rowIndex !== legacy) {
throw new DocumentApiValidationError(
'INVALID_TARGET',
'tables.split: cannot provide both rowIndex and atRowIndex with different values.',
{ fields: ['rowIndex', 'atRowIndex'] },
);
}

const { atRowIndex: _legacy, ...rest } = input as Record<string, unknown>;
return { ...rest, rowIndex: legacy } as T;
}

// ---------------------------------------------------------------------------
// Typed execute helpers
// ---------------------------------------------------------------------------
Expand Down
4 changes: 1 addition & 3 deletions packages/document-api/src/types/table-operations.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ export interface TablesMoveInput extends TableLocator {
// tables.split
// ---------------------------------------------------------------------------

export interface TablesSplitInput extends TableLocator {
atRowIndex: number;
}
export type TablesSplitInput = TableScopedRowLocator;

// ---------------------------------------------------------------------------
// tables.convertToText
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,74 @@ describe('buildOperationArgv', () => {
});
});

describe('legacy atRowIndex normalization for tables.split', () => {
test('maps legacy atRowIndex to canonical rowIndex', () => {
const op = makeOp({
operationId: 'doc.tables.split',
commandTokens: ['doc', 'tables', 'split'],
params: [
{ name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string' },
{ name: 'rowIndex', kind: 'flag', flag: 'row-index', type: 'number' },
],
});
const argv = buildOperationArgv(op, { nodeId: 'table-1', atRowIndex: 2 }, {}, undefined);
expect(argv).toContain('--row-index');
expect(argv[argv.indexOf('--row-index') + 1]).toBe('2');
});

test('does not overwrite explicit rowIndex with legacy atRowIndex', () => {
const op = makeOp({
operationId: 'doc.tables.split',
commandTokens: ['doc', 'tables', 'split'],
params: [
{ name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string' },
{ name: 'rowIndex', kind: 'flag', flag: 'row-index', type: 'number' },
],
});
const argv = buildOperationArgv(op, { nodeId: 'table-1', rowIndex: 1 }, {}, undefined);
expect(argv).toContain('--row-index');
expect(argv[argv.indexOf('--row-index') + 1]).toBe('1');
});

test('accepts both when values match', () => {
const op = makeOp({
operationId: 'doc.tables.split',
commandTokens: ['doc', 'tables', 'split'],
params: [
{ name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string' },
{ name: 'rowIndex', kind: 'flag', flag: 'row-index', type: 'number' },
],
});
const argv = buildOperationArgv(op, { nodeId: 'table-1', rowIndex: 1, atRowIndex: 1 }, {}, undefined);
expect(argv).toContain('--row-index');
expect(argv[argv.indexOf('--row-index') + 1]).toBe('1');
});

test('rejects conflicting rowIndex and atRowIndex', () => {
const op = makeOp({
operationId: 'doc.tables.split',
commandTokens: ['doc', 'tables', 'split'],
params: [
{ name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string' },
{ name: 'rowIndex', kind: 'flag', flag: 'row-index', type: 'number' },
],
});
expect(() => buildOperationArgv(op, { nodeId: 'table-1', rowIndex: 1, atRowIndex: 2 }, {}, undefined)).toThrow(
'tables.split: cannot provide both rowIndex and atRowIndex with different values.',
);
});

test('does not apply normalization to other operations', () => {
const op = makeOp({
operationId: 'doc.tables.delete',
commandTokens: ['doc', 'tables', 'delete'],
params: [{ name: 'nodeId', kind: 'flag', flag: 'node-id', type: 'string' }],
});
const argv = buildOperationArgv(op, { nodeId: 'table-1', atRowIndex: 2 } as any, {}, undefined);
expect(argv).not.toContain('--row-index');
});
});

describe('buildOperationArgv with real generated contract', () => {
const realOpenOp = CONTRACT.operations['doc.open'] as OperationSpec;

Expand Down
9 changes: 9 additions & 0 deletions packages/sdk/langs/node/src/runtime/transport-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ export function buildOperationArgv(
}
}

// Legacy alias: tables.split renamed atRowIndex → rowIndex (SD-2132).
if (operation.operationId === 'doc.tables.split' && normalizedParams.atRowIndex !== undefined) {
if (normalizedParams.rowIndex !== undefined && normalizedParams.rowIndex !== normalizedParams.atRowIndex) {
throw new Error('tables.split: cannot provide both rowIndex and atRowIndex with different values.');
}
const { atRowIndex, ...rest } = normalizedParams;
normalizedParams = { ...rest, rowIndex: atRowIndex };
}

const argv: string[] = [...operation.commandTokens];

for (const spec of operation.params) {
Expand Down
9 changes: 9 additions & 0 deletions packages/sdk/langs/python/superdoc/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ def build_operation_argv(
"""Build the CLI argument vector for an operation invocation."""
payload = apply_default_change_mode(operation, params, default_change_mode)
payload = apply_default_user(operation, payload, user)
# Legacy alias: tables.split renamed atRowIndex → rowIndex (SD-2132).
if operation.get('operationId') == 'doc.tables.split' and 'atRowIndex' in payload:
if 'rowIndex' in payload and payload['rowIndex'] != payload['atRowIndex']:
raise SuperDocError(
'tables.split: cannot provide both rowIndex and atRowIndex with different values.',
code='INVALID_ARGUMENT',
)
payload = {k: v for k, v in payload.items() if k != 'atRowIndex'}
payload['rowIndex'] = params['atRowIndex']
argv: List[str] = list(operation['commandTokens'])
for spec in operation['params']:
_encode_param(argv, spec, payload.get(spec['name']))
Expand Down
Loading
Loading