Skip to content

Commit cb8fedd

Browse files
harbournickharbournick
authored
feat(document-api): improve cross block selection and deleting (#2391)
* feat(document-api): improve cross block selection and deleting * feat(document-api): add ranges.resolve operation for deterministic range construction * fix(document-api): fix selection-target mutation handling * chore: type fix * fix(plan-engine): repair selection mutation execution and receipt resolution * chore: test fixes * chore: type check * chore: fix behavior tests * chore: docs tweaks
1 parent 382ffa6 commit cb8fedd

119 files changed

Lines changed: 9740 additions & 3084 deletions

File tree

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__/conformance/scenarios.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,8 @@ function sampleInlineAliasValue(key: InlineAliasKey): unknown {
408408
case 'fontSize':
409409
case 'fontSizeCs':
410410
return 14;
411+
case 'fontFamily':
412+
return 'Courier New';
411413
case 'letterSpacing':
412414
return 0.5;
413415
case 'position':

‎apps/cli/src/__tests__/lib/invoke-input.test.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, expect, test } from 'bun:test';
2+
import { extractInvokeInput } from '../../lib/invoke-input';
3+
import { CliError } from '../../lib/errors';
4+
5+
describe('extractInvokeInput', () => {
6+
test('converts replace flat range flags into a single-block SelectionTarget', () => {
7+
const input = extractInvokeInput('replace', {
8+
doc: 'fixture.docx',
9+
blockId: 'p1',
10+
start: 2,
11+
end: 5,
12+
text: 'Updated',
13+
}) as Record<string, unknown>;
14+
15+
expect(input).toEqual({
16+
text: 'Updated',
17+
target: {
18+
kind: 'selection',
19+
start: { kind: 'text', blockId: 'p1', offset: 2 },
20+
end: { kind: 'text', blockId: 'p1', offset: 5 },
21+
},
22+
});
23+
});
24+
25+
test('upgrades legacy TextAddress target-json input for format.apply', () => {
26+
const input = extractInvokeInput('format.apply', {
27+
target: {
28+
kind: 'text',
29+
blockId: 'p1',
30+
range: { start: 0, end: 4 },
31+
},
32+
inline: { bold: true },
33+
}) as Record<string, unknown>;
34+
35+
expect(input).toEqual({
36+
target: {
37+
kind: 'selection',
38+
start: { kind: 'text', blockId: 'p1', offset: 0 },
39+
end: { kind: 'text', blockId: 'p1', offset: 4 },
40+
},
41+
inline: { bold: true },
42+
});
43+
});
44+
45+
test('preserves text-address targets for comments.create', () => {
46+
const input = extractInvokeInput('comments.create', {
47+
blockId: 'p1',
48+
start: 1,
49+
end: 3,
50+
text: 'Review this',
51+
}) as Record<string, unknown>;
52+
53+
expect(input).toEqual({
54+
text: 'Review this',
55+
target: {
56+
kind: 'text',
57+
blockId: 'p1',
58+
range: { start: 1, end: 3 },
59+
},
60+
});
61+
});
62+
63+
test('rejects collapsed legacy text ranges for format operations', () => {
64+
expect(() =>
65+
extractInvokeInput('format.bold', {
66+
target: {
67+
kind: 'text',
68+
blockId: 'p1',
69+
range: { start: 2, end: 2 },
70+
},
71+
}),
72+
).toThrow(CliError);
73+
});
74+
});

‎apps/cli/src/cli/__tests__/schema-ref-resolution.test.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,80 @@ describe('operation-params deriveParamsFromInputSchema with $ref', () => {
168168
expect(modeParam).toBeDefined();
169169
expect(modeParam!.type).toBe('string'); // enum → string
170170
});
171+
172+
test('derives params from top-level allOf by merging object members', () => {
173+
const inputSchema = {
174+
allOf: [
175+
{
176+
oneOf: [
177+
{
178+
type: 'object',
179+
properties: {
180+
target: { $ref: '#/$defs/TextAddress' },
181+
},
182+
required: ['target'],
183+
},
184+
{
185+
type: 'object',
186+
properties: {
187+
ref: { type: 'string' },
188+
},
189+
required: ['ref'],
190+
},
191+
],
192+
},
193+
{
194+
type: 'object',
195+
properties: {
196+
text: { type: 'string' },
197+
},
198+
required: ['text'],
199+
},
200+
],
201+
};
202+
const { params } = deriveParamsFromInputSchema(inputSchema, $defs);
203+
const paramNames = params.map((param) => param.name);
204+
205+
expect(paramNames).toContain('target');
206+
expect(paramNames).toContain('ref');
207+
expect(paramNames).toContain('text');
208+
expect(params.find((param) => param.name === 'text')?.required).toBe(true);
209+
expect(params.find((param) => param.name === 'target')?.required).toBe(false);
210+
expect(params.find((param) => param.name === 'ref')?.required).toBe(false);
211+
});
212+
213+
test('merges duplicate properties across oneOf branches into a single param schema', () => {
214+
const inputSchema = {
215+
oneOf: [
216+
{
217+
type: 'object',
218+
properties: {
219+
target: { $ref: '#/$defs/TextAddress' },
220+
text: { type: 'string' },
221+
},
222+
required: ['target', 'text'],
223+
},
224+
{
225+
type: 'object',
226+
properties: {
227+
target: {
228+
oneOf: [{ $ref: '#/$defs/TextAddress' }, { type: 'string' }],
229+
},
230+
content: { type: 'object' },
231+
},
232+
required: ['target', 'content'],
233+
},
234+
],
235+
};
236+
const { params } = deriveParamsFromInputSchema(inputSchema, $defs);
237+
const targetParam = params.find((param) => param.name === 'target');
238+
239+
expect(targetParam).toBeDefined();
240+
expect(targetParam!.type).toBe('json');
241+
expect((targetParam!.schema as { oneOf: CliTypeSpec[] }).oneOf.length).toBeGreaterThan(1);
242+
expect(params.find((param) => param.name === 'text')).toBeDefined();
243+
expect(params.find((param) => param.name === 'content')).toBeDefined();
244+
});
171245
});
172246

173247
// ---------------------------------------------------------------------------

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

Lines changed: 127 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ const USER_EMAIL_PARAM: CliOperationParamSpec = {
7878
type JsonSchema = Record<string, unknown>;
7979
const AGENT_HIDDEN_PARAM_NAMES = new Set(['out']);
8080

81+
type ObjectSchemaVariant = {
82+
properties: Record<string, JsonSchema>;
83+
required: Set<string>;
84+
};
85+
8186
function resolveRef(schema: JsonSchema, $defs?: Record<string, JsonSchema>): JsonSchema {
8287
if (schema.$ref && $defs) {
8388
const prefix = '#/$defs/';
@@ -90,6 +95,108 @@ function resolveRef(schema: JsonSchema, $defs?: Record<string, JsonSchema>): Jso
9095
return schema;
9196
}
9297

98+
function hasObjectShape(schema: JsonSchema): boolean {
99+
return schema.type === 'object' || schema.properties != null || schema.required != null;
100+
}
101+
102+
function cloneVariant(variant: ObjectSchemaVariant): ObjectSchemaVariant {
103+
return {
104+
properties: { ...variant.properties },
105+
required: new Set(variant.required),
106+
};
107+
}
108+
109+
function directObjectVariant(schema: JsonSchema): ObjectSchemaVariant {
110+
return {
111+
properties: {
112+
...(((schema.properties as Record<string, JsonSchema> | undefined) ?? {}) as Record<string, JsonSchema>),
113+
},
114+
required: new Set<string>(((schema.required as string[] | undefined) ?? []) as string[]),
115+
};
116+
}
117+
118+
function schemasEqual(left: JsonSchema, right: JsonSchema): boolean {
119+
return JSON.stringify(left) === JSON.stringify(right);
120+
}
121+
122+
function mergePropertySchemas(left: JsonSchema, right: JsonSchema): JsonSchema {
123+
if (schemasEqual(left, right)) return left;
124+
125+
const variants: JsonSchema[] = [];
126+
const appendVariant = (schema: JsonSchema) => {
127+
if (variants.some((candidate) => schemasEqual(candidate, schema))) return;
128+
variants.push(schema);
129+
};
130+
131+
if (Array.isArray(left.oneOf)) {
132+
for (const entry of left.oneOf as JsonSchema[]) appendVariant(entry);
133+
} else {
134+
appendVariant(left);
135+
}
136+
137+
if (Array.isArray(right.oneOf)) {
138+
for (const entry of right.oneOf as JsonSchema[]) appendVariant(entry);
139+
} else {
140+
appendVariant(right);
141+
}
142+
143+
return variants.length === 1 ? variants[0]! : { oneOf: variants };
144+
}
145+
146+
function mergeObjectVariants(left: ObjectSchemaVariant, right: ObjectSchemaVariant): ObjectSchemaVariant {
147+
const merged = cloneVariant(left);
148+
for (const [name, schema] of Object.entries(right.properties)) {
149+
const existing = merged.properties[name];
150+
merged.properties[name] = existing ? mergePropertySchemas(existing, schema) : schema;
151+
}
152+
for (const key of right.required) {
153+
merged.required.add(key);
154+
}
155+
return merged;
156+
}
157+
158+
function extractObjectSchemaVariants(rawSchema: JsonSchema, $defs?: Record<string, JsonSchema>): ObjectSchemaVariant[] {
159+
const schema = resolveRef(rawSchema, $defs);
160+
const directVariants = hasObjectShape(schema) ? [directObjectVariant(schema)] : [];
161+
let variants = directVariants.length > 0 ? directVariants.map(cloneVariant) : [];
162+
163+
if (Array.isArray(schema.allOf)) {
164+
variants = variants.length > 0 ? variants : [{ properties: {}, required: new Set<string>() }];
165+
for (const member of schema.allOf as JsonSchema[]) {
166+
const memberVariants = extractObjectSchemaVariants(member, $defs);
167+
if (memberVariants.length === 0) continue;
168+
169+
const nextVariants: ObjectSchemaVariant[] = [];
170+
for (const base of variants) {
171+
for (const part of memberVariants) {
172+
nextVariants.push(mergeObjectVariants(base, part));
173+
}
174+
}
175+
variants = nextVariants;
176+
}
177+
}
178+
179+
const alternativeKeyword = Array.isArray(schema.oneOf) ? 'oneOf' : Array.isArray(schema.anyOf) ? 'anyOf' : null;
180+
if (alternativeKeyword) {
181+
const branches = (schema[alternativeKeyword] as JsonSchema[]).flatMap((member) =>
182+
extractObjectSchemaVariants(member, $defs),
183+
);
184+
if (branches.length > 0) {
185+
const baseVariants = variants.length > 0 ? variants : [{ properties: {}, required: new Set<string>() }];
186+
const nextVariants: ObjectSchemaVariant[] = [];
187+
for (const base of baseVariants) {
188+
for (const branch of branches) {
189+
nextVariants.push(mergeObjectVariants(base, branch));
190+
}
191+
}
192+
variants = nextVariants;
193+
}
194+
}
195+
196+
if (variants.length > 0) return variants;
197+
return hasObjectShape(schema) ? [directObjectVariant(schema)] : [];
198+
}
199+
93200
function schemaToParamType(schema: JsonSchema, $defs?: Record<string, JsonSchema>): CliOperationParamSpec['type'] {
94201
schema = resolveRef(schema, $defs);
95202
if (schema.type === 'string') return 'string';
@@ -167,8 +274,26 @@ function deriveParamsFromInputSchema(
167274
} {
168275
const params: CliOperationParamSpec[] = [];
169276
const positionalParams: string[] = [];
170-
const properties = (inputSchema.properties ?? {}) as Record<string, JsonSchema>;
171-
const required = new Set<string>((inputSchema.required as string[]) ?? []);
277+
const variants = extractObjectSchemaVariants(inputSchema, $defs);
278+
const properties: Record<string, JsonSchema> = {};
279+
const requiredCounts = new Map<string, number>();
280+
281+
for (const variant of variants) {
282+
for (const [name, schema] of Object.entries(variant.properties)) {
283+
const existing = properties[name];
284+
properties[name] = existing ? mergePropertySchemas(existing, schema) : schema;
285+
}
286+
for (const name of variant.required) {
287+
requiredCounts.set(name, (requiredCounts.get(name) ?? 0) + 1);
288+
}
289+
}
290+
291+
const required = new Set<string>();
292+
for (const [name] of Object.entries(properties)) {
293+
if (variants.length > 0 && requiredCounts.get(name) === variants.length) {
294+
required.add(name);
295+
}
296+
}
172297

173298
for (const [name, rawPropSchema] of Object.entries(properties)) {
174299
const propSchema = resolveRef(rawPropSchema, $defs);

0 commit comments

Comments
 (0)