Skip to content

Commit accd36a

Browse files
authored
Add isArray and isMap to tree-agent context (#25725)
This gives the LLM an alternative to `Array.isArray` and `instanceof Map` that will work on our node types, since `Array.isArray` does not work with our non-pojo array classes. The provided functions also work for JS arrays and Maps, so the LLM doesn't have to distinguish between when to use the built-in Array.isArray vs. when to use our provided `isArray` helper - it can always use the latter. This also gives us the option in the future to force it to use ours by making `Array.isArray` error within the boundaries of the eval. This also adds some explicit tests in SharedTree for `Array.isArray` support.
1 parent edfd9fa commit accd36a

File tree

6 files changed

+142
-17
lines changed

6 files changed

+142
-17
lines changed

packages/dds/tree/src/test/simple-tree/node-kinds/arrayNode.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,18 @@ describe("ArrayNode", () => {
7272
assert.equal(n.y, 3);
7373
assert.deepEqual(thisList, [n, n]);
7474
});
75+
76+
it("does not pass Array.isArray", () => {
77+
const array = init(CustomizableNumberArray, [1, 2, 3]);
78+
assert.equal(Array.isArray(array), false);
79+
});
80+
});
81+
82+
describeHydration("pojo-emulation", (init) => {
83+
it("passes Array.isArray", () => {
84+
const array = init(PojoEmulationNumberArray, [1, 2, 3]);
85+
assert.equal(Array.isArray(array), true);
86+
});
7587
});
7688

7789
describe("insertable types", () => {

packages/framework/tree-agent/src/agent.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type {
88
TreeFieldFromImplicitField,
99
TreeNodeSchema,
1010
} from "@fluidframework/tree";
11-
import { TreeNode } from "@fluidframework/tree";
11+
import { NodeKind, TreeNode } from "@fluidframework/tree";
1212
import type {
1313
ReadableField,
1414
FactoryContentObject,
@@ -313,6 +313,26 @@ function bindEditorToSubtree<TSchema extends ImplicitFieldSchema>(
313313
},
314314
create,
315315
is,
316+
isArray(node) {
317+
if (Array.isArray(node)) {
318+
return true;
319+
}
320+
if (node instanceof TreeNode) {
321+
const schema = Tree.schema(node);
322+
return schema.kind === NodeKind.Array;
323+
}
324+
return false;
325+
},
326+
isMap(node) {
327+
if (node instanceof Map) {
328+
return true;
329+
}
330+
if (node instanceof TreeNode) {
331+
const schema = Tree.schema(node);
332+
return schema.kind === NodeKind.Map;
333+
}
334+
return false;
335+
},
316336
parent: (child: TreeNode): TreeNode | undefined => Tree.parent(child),
317337
key: (child: TreeNode): string | number => Tree.key(child),
318338
} satisfies Context<TSchema>;

packages/framework/tree-agent/src/api.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ export interface Context<TSchema extends ImplicitFieldSchema> {
4949
* Always use these builder functions when creating new nodes rather than plain JavaScript objects.
5050
*
5151
* Example: Create a new Person node with `context.create.Person({ name: "Alice", age: 30 })`
52-
*
53-
* Example: Create a new Task node with `context.create.Task({ title: "Buy groceries", completed: false })`
5452
*/
5553
create: Record<string, (input: FactoryContentObject) => TreeNode>;
5654

@@ -65,6 +63,16 @@ export interface Context<TSchema extends ImplicitFieldSchema> {
6563
*/
6664
is: Record<string, <T extends TreeNode>(input: T) => input is T>;
6765

66+
/**
67+
* Checks if the given node is an array.
68+
*/
69+
isArray(value: unknown): boolean;
70+
71+
/**
72+
* Checks if the given node is a map.
73+
*/
74+
isMap(value: unknown): boolean;
75+
6876
/**
6977
* Returns the parent node of the given child node.
7078
* @param child - The node whose parent you want to find.

packages/framework/tree-agent/src/prompt.ts

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,15 @@ export function getPrompt<TRoot extends ImplicitFieldSchema>(args: {
7979
const stringified = stringifyTree(field);
8080
const details: SchemaDetails = { hasHelperMethods: false };
8181
const typescriptSchemaTypes = getZodSchemaAsTypeScript(domainTypes, details);
82-
83-
const create =
82+
const exampleTypeName =
83+
nodeTypeUnion === undefined
84+
? undefined
85+
: nodeTypeUnion
86+
.split("|")
87+
.map((part) => part.trim())
88+
.find((part) => part.length > 0);
89+
90+
const createDocs =
8491
exampleObjectName === undefined
8592
? ""
8693
: `\n /**
@@ -102,7 +109,7 @@ export function getPrompt<TRoot extends ImplicitFieldSchema>(args: {
102109
create: Record<string, <T extends TreeData>(input: T) => T>;\n`;
103110

104111
const isDocs =
105-
exampleObjectName === undefined
112+
exampleTypeName === undefined
106113
? ""
107114
: `\n /**
108115
* A collection of type-guard functions for data in the tree.
@@ -111,9 +118,31 @@ export function getPrompt<TRoot extends ImplicitFieldSchema>(args: {
111118
* Call the corresponding function to check if a node is of that specific type.
112119
* This is useful when working with nodes that could be one of multiple types.
113120
*
114-
* ${`Example: Check if a node is a ${exampleObjectName} with \`if (context.is.${exampleObjectName}(node)) {}\``}
121+
* ${`Example: Check if a node is a ${exampleTypeName} with \`if (context.is.${exampleTypeName}(node)) {}\``}
122+
*/
123+
is: Record<string, <T extends TreeData>(data: unknown) => data is T>;
124+
125+
/**
126+
* Checks if the provided data is an array.
127+
* @remarks
128+
* DO NOT use \`Array.isArray\` to check if tree data is an array - use this function instead.
129+
*
130+
* This function will also work for native JavaScript arrays.
131+
*
132+
* ${`Example: \`if (context.isArray(node)) {}\``}
133+
*/
134+
isArray(data: any): boolean;
135+
136+
/**
137+
* Checks if the provided data is a map.
138+
* @remarks
139+
* DO NOT use \`instanceof Map\` to check if tree data is a map - use this function instead.
140+
*
141+
* This function will also work for native JavaScript Map instances.
142+
*
143+
* ${`Example: \`if (context.isMap(node)) {}\``}
115144
*/
116-
is: Record<string, <T extends TreeData>(input: unknown) => input is T>;\n`;
145+
isMap(data: any): boolean;\n`;
117146

118147
const context = `\`\`\`typescript
119148
${nodeTypeUnion === undefined ? "" : `type TreeData = ${nodeTypeUnion};\n\n`} /**
@@ -130,7 +159,7 @@ export function getPrompt<TRoot extends ImplicitFieldSchema>(args: {
130159
* Example: Read the current root with \`const currentRoot = context.root;\`
131160
*${rootTypes.length > 0 ? ` Example: Replace the entire root with \`context.root = context.create.${getFriendlyName(rootTypes[0] ?? oob())}({ });\`\n *` : ""}/
132161
root: ReadableField<TSchema>;
133-
${create}
162+
${createDocs}
134163
${isDocs}
135164
/**
136165
* Returns the parent object/array/map of the given object/array/map, if there is one.

packages/framework/tree-agent/src/test/__snapshots__/prompt.md

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,31 @@ Here is the definition of the `Context` interface:
8888
* Call the corresponding function to check if a node is of that specific type.
8989
* This is useful when working with nodes that could be one of multiple types.
9090
*
91-
* Example: Check if a node is a TestArrayItem with `if (context.is.TestArrayItem(node)) {}`
91+
* Example: Check if a node is a TestMap with `if (context.is.TestMap(node)) {}`
9292
*/
93-
is: Record<string, <T extends TreeData>(input: unknown) => input is T>;
93+
is: Record<string, <T extends TreeData>(data: unknown) => data is T>;
94+
95+
/**
96+
* Checks if the provided data is an array.
97+
* @remarks
98+
* DO NOT use `Array.isArray` to check if tree data is an array - use this function instead.
99+
*
100+
* This function will also work for native JavaScript arrays.
101+
*
102+
* Example: `if (context.isArray(node)) {}`
103+
*/
104+
isArray(data: any): boolean;
105+
106+
/**
107+
* Checks if the provided data is a map.
108+
* @remarks
109+
* DO NOT use `instanceof Map` to check if tree data is a map - use this function instead.
110+
*
111+
* This function will also work for native JavaScript Map instances.
112+
*
113+
* Example: `if (context.isMap(node)) {}`
114+
*/
115+
isMap(data: any): boolean;
94116

95117
/**
96118
* Returns the parent object/array/map of the given object/array/map, if there is one.

packages/framework/tree-agent/src/test/agent.spec.ts

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,6 @@ describe("Semantic Agent", () => {
248248
assert.equal(view.root, "Initial", "Tree should not have changed");
249249
});
250250

251-
it("passes constructors to edit code", async () => {
252-
// Moved to the dedicated context helpers describe block below.
253-
});
254-
255251
it("supplies the system prompt as context", async () => {
256252
const view = independentView(new TreeViewConfiguration({ schema: sf.string }), {});
257253
view.initialize("X");
@@ -365,6 +361,46 @@ describe("Semantic Agent", () => {
365361
assert.equal(await agent.query("Key Edit"), "Done");
366362
assert.equal(view.root.child.value, "KeyWorked");
367363
});
364+
365+
it("provides working isArray helper", async () => {
366+
const sfLocal = new SchemaFactory("TestIsArray");
367+
const NumberArray = sfLocal.array(sfLocal.number);
368+
const view = independentView(new TreeViewConfiguration({ schema: NumberArray }), {});
369+
view.initialize([1, 2, 3]);
370+
const model: SharedTreeChatModel = {
371+
editToolName,
372+
async query({ edit }) {
373+
const result = await edit(
374+
`if (!context.isArray([]) ||!context.isArray(context.root)) { throw new Error('Expected array root'); } context.root.insertAt(0, 99);`,
375+
);
376+
assert.equal(result.type, "success", result.message);
377+
return "ArrayOK";
378+
},
379+
};
380+
const agent = new SharedTreeSemanticAgent(model, view);
381+
assert.equal(await agent.query("Array Edit"), "ArrayOK");
382+
assert.equal(view.root[0], 99);
383+
});
384+
385+
it("provides working isMap helper", async () => {
386+
const sfLocal = new SchemaFactory("TestIsMap");
387+
class NumberMap extends sfLocal.map("NumberMap", sfLocal.number) {}
388+
const view = independentView(new TreeViewConfiguration({ schema: NumberMap }), {});
389+
view.initialize(new NumberMap(new Map([["x", 1]])));
390+
const model: SharedTreeChatModel = {
391+
editToolName,
392+
async query({ edit }) {
393+
const result = await edit(
394+
`if (!context.isMap(new Map()) || !context.isMap(context.root)) { throw new Error('Expected map root'); } context.root.set('y', 2);`,
395+
);
396+
assert.equal(result.type, "success", result.message);
397+
return "MapOK";
398+
},
399+
};
400+
const agent = new SharedTreeSemanticAgent(model, view);
401+
assert.equal(await agent.query("Map Edit"), "MapOK");
402+
assert.equal(view.root.get("y"), 2);
403+
});
368404
});
369405

370406
it("supplies additional context if the tree changes between queries", async () => {
@@ -433,7 +469,6 @@ describe("Semantic Agent", () => {
433469
});
434470

435471
describe("bindEditor", () => {
436-
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call */
437472
interface TestContext {
438473
root: string;
439474
}
@@ -485,6 +520,5 @@ describe("Semantic Agent", () => {
485520
await assert.rejects(promise, /asyncBoom/);
486521
assert.equal(view.root, "Initial", "Tree should not have changed after async error");
487522
});
488-
/* eslint-enable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call */
489523
});
490524
});

0 commit comments

Comments
 (0)