Skip to content

Commit fc27097

Browse files
authored
clearer edit tool schema (#7059)
…use it ## 📝 Summary <!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes #123). --> Makes it easier for LLM's to use this tool ## 🔍 Description of Changes <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> ## 📋 Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [x] I have added tests for the changes made. - [x] I have run the code and verified that it works as expected.
1 parent 8c1c1a0 commit fc27097

File tree

6 files changed

+46
-43
lines changed

6 files changed

+46
-43
lines changed

frontend/src/core/ai/model-registry.ts

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,6 @@ import { once } from "@/utils/once";
1313
import type { ProviderId } from "./ids/ids";
1414
import { AiModelId, type QualifiedModelId, type ShortModelId } from "./ids/ids";
1515

16-
export const PROVIDER_SORT_ORDER: ProviderId[] = [
17-
// Sort by popular ones
18-
"anthropic",
19-
"openai",
20-
"google",
21-
"github",
22-
"openrouter",
23-
"deepseek",
24-
"azure",
25-
"bedrock",
26-
"ollama",
27-
];
28-
2916
export interface AiModel extends AiModelType {
3017
roles: Role[];
3118
model: ShortModelId;

frontend/src/core/ai/tools/__tests__/edit-notebook-tool.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ describe("EditNotebookTool", () => {
249249
{
250250
edit: {
251251
type: "add_cell",
252-
position: "end",
252+
position: { type: "notebook_end" },
253253
code: newCode,
254254
},
255255
},
@@ -280,7 +280,7 @@ describe("EditNotebookTool", () => {
280280
{
281281
edit: {
282282
type: "add_cell",
283-
position: { cellId: cellId2, before: true },
283+
position: { type: "relative", cellId: cellId2, before: true },
284284
code: newCode,
285285
},
286286
},
@@ -310,7 +310,7 @@ describe("EditNotebookTool", () => {
310310
{
311311
edit: {
312312
type: "add_cell",
313-
position: { cellId: cellId2, before: false },
313+
position: { type: "relative", cellId: cellId2, before: false },
314314
code: newCode,
315315
},
316316
},
@@ -340,7 +340,7 @@ describe("EditNotebookTool", () => {
340340
{
341341
edit: {
342342
type: "add_cell",
343-
position: { type: "end", columnIndex: 1 },
343+
position: { type: "column_end", columnIndex: 1 },
344344
code: newCode,
345345
},
346346
},
@@ -367,7 +367,11 @@ describe("EditNotebookTool", () => {
367367
{
368368
edit: {
369369
type: "add_cell",
370-
position: { cellId: "nonexistent" as CellId, before: true },
370+
position: {
371+
type: "relative",
372+
cellId: "nonexistent" as CellId,
373+
before: true,
374+
},
371375
code: "y = 2",
372376
},
373377
},
@@ -390,7 +394,7 @@ describe("EditNotebookTool", () => {
390394
edit: {
391395
type: "add_cell",
392396
position: {
393-
type: "end",
397+
type: "column_end",
394398
columnIndex: -1,
395399
},
396400
code: "y = 2",
@@ -540,7 +544,7 @@ describe("EditNotebookTool", () => {
540544
{
541545
edit: {
542546
type: "add_cell",
543-
position: "end",
547+
position: { type: "notebook_end" },
544548
code: "y = 2",
545549
},
546550
},

frontend/src/core/ai/tools/__tests__/registry.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ describe("FrontendToolRegistry", () => {
5555
expect(typeof response.error).toBe("string");
5656

5757
// Verify error message contains expected prefix
58-
expect(response.error).toContain("Error invoking tool ToolExecutionError:");
59-
expect(response.error).toContain('"code":"TOOL_ERROR"');
60-
expect(response.error).toContain('"is_retryable":false');
58+
expect(response.error).toMatchInlineSnapshot(
59+
`"Error invoking tool ToolExecutionError: {"message":"Tool test_frontend_tool returned invalid input: ✖ Invalid input: expected string, received undefined\\n → at name","code":"INVALID_ARGUMENTS","is_retryable":true,"suggested_fix":"Please check the arguments and try again."}"`,
60+
);
6161
});
6262

6363
it("returns tool schemas with expected shape and memoizes the result", () => {

frontend/src/core/ai/tools/base.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ export class ToolExecutionError extends Error {
5151
message: this.message,
5252
code: this.code,
5353
is_retryable: this.isRetryable,
54-
suggested_fix: this.suggestedFix,
55-
meta: this.meta ?? {},
54+
...(this.suggestedFix && { suggested_fix: this.suggestedFix }),
55+
...(this.meta && { meta: this.meta }),
5656
});
5757
return `Error invoking tool ${this.name}: ${stringError}`;
5858
}

frontend/src/core/ai/tools/edit-notebook-tool.ts

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ const description: ToolDescription = {
3131
additionalInfo: `
3232
Args:
3333
edit (object): The editing operation to perform. Must be one of:
34-
- update_cell: Update the code of an existing cell, pass CellId and the new code.
35-
- add_cell: Add a new cell to the notebook. The position of the new cell is specified by the position argument.
36-
Pass "end" to add the new cell at the end of the notebook.
37-
Pass { cellId: cellId, before: true } to add the new cell before the specified cell. And before: false if after the specified cell.
38-
Pass { type: "end", columnIndex: number } to add the new cell at the end of a specified column index. The column index is 0-based.
39-
- delete_cell: Delete an existing cell, pass CellId. For deleting cells, the user needs to accept the deletion to actually delete the cell, so you may still see the cell in the notebook on subsequent edits which is fine.
34+
- update_cell: Update the code of an existing cell, pass cellId and the new code.
35+
- add_cell: Add a new cell to the notebook. The position is specified by the position object with a "type" field:
36+
{ type: "notebook_end" } - Add at the end of the notebook
37+
{ type: "relative", cellId: "...", before: true } - Add before the specified cell (before: false for after)
38+
{ type: "column_end", columnIndex: 0 } - Add at the end of a specific column (0-based index)
39+
- delete_cell: Delete an existing cell, pass cellId. For deleting cells, the user needs to accept the deletion to actually delete the cell, so you may still see the cell in the notebook on subsequent edits which is fine.
4040
4141
For adding code, use the following guidelines:
4242
- Markdown cells: use mo.md(f"""{content}""") function to insert content.
@@ -47,9 +47,9 @@ const description: ToolDescription = {
4747
};
4848

4949
type CellPosition =
50-
| { cellId: CellId; before: boolean }
51-
| { type: "end"; columnIndex: number }
52-
| "end";
50+
| { type: "relative"; cellId: CellId; before: boolean }
51+
| { type: "column_end"; columnIndex: number }
52+
| { type: "notebook_end" };
5353

5454
const editNotebookSchema = z.object({
5555
edit: z.discriminatedUnion("type", [
@@ -60,16 +60,19 @@ const editNotebookSchema = z.object({
6060
}),
6161
z.object({
6262
type: z.literal("add_cell"),
63-
position: z.union([
63+
position: z.discriminatedUnion("type", [
6464
z.object({
65+
type: z.literal("relative"),
6566
cellId: z.string() as unknown as z.ZodType<CellId>,
6667
before: z.boolean(),
6768
}),
6869
z.object({
69-
type: z.literal("end"),
70+
type: z.literal("column_end"),
7071
columnIndex: z.number(),
7172
}),
72-
z.literal("end"),
73+
z.object({
74+
type: z.literal("notebook_end"),
75+
}),
7376
]) satisfies z.ZodType<CellPosition>,
7477
code: z.string(),
7578
}),
@@ -135,21 +138,25 @@ export class EditNotebookTool
135138
case "add_cell": {
136139
const { position, code } = edit;
137140

138-
// By default, add the new cell to the end of the notebook
139141
let notebookPosition: NotebookCellPosition = "__end__";
140142
let before = false;
141143
const newCellId = CellId.create();
144+
const notebook = store.get(notebookAtom);
142145

143-
if (typeof position === "object") {
144-
const notebook = store.get(notebookAtom);
145-
if ("cellId" in position) {
146+
switch (position.type) {
147+
case "relative":
146148
this.validateCellIdExists(position.cellId, notebook);
147149
notebookPosition = position.cellId;
148150
before = position.before;
149-
} else if ("columnIndex" in position) {
151+
break;
152+
case "column_end": {
150153
const columnId = this.getColumnId(position.columnIndex, notebook);
151154
notebookPosition = { type: "__end__", columnId };
155+
break;
152156
}
157+
case "notebook_end":
158+
// Use default: notebookPosition = "__end__"
159+
break;
153160
}
154161

155162
createNewCell({

frontend/src/core/ai/tools/registry.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,12 @@ export class FrontendToolRegistry {
6767
const inputResponse = await inputSchema.safeParseAsync(rawArgs);
6868
if (inputResponse.error) {
6969
const strError = z.prettifyError(inputResponse.error);
70-
throw new Error(`Tool ${toolName} returned invalid input: ${strError}`);
70+
throw new ToolExecutionError(
71+
`Tool ${toolName} returned invalid input: ${strError}`,
72+
"INVALID_ARGUMENTS",
73+
true,
74+
"Please check the arguments and try again.",
75+
);
7176
}
7277
const args = inputResponse.data;
7378

0 commit comments

Comments
 (0)