Skip to content

Commit 0115755

Browse files
committed
Fixes #4956: Add proper Jupyter notebook support for apply_diff and write_to_file tools
- Created jupyter-notebook-handler.ts with utilities for parsing and handling .ipynb files - Modified applyDiffTool.ts to extract cell content for editing and reconstruct notebook JSON - Modified multiApplyDiffTool.ts to handle Jupyter notebooks in batch operations - Modified writeToFileTool.ts to detect and properly handle notebook content vs raw JSON - Added comprehensive test suite for all notebook operations - Prevents notebook corruption by maintaining proper JSON structure - Supports both extracted content editing and direct JSON editing workflows
1 parent 2e2f83b commit 0115755

File tree

7 files changed

+1518
-31
lines changed

7 files changed

+1518
-31
lines changed

roo-code-messages.log

Lines changed: 703 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 370 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,370 @@
1+
import { describe, it, expect, beforeEach, afterEach } from "vitest"
2+
import fs from "fs/promises"
3+
import path from "path"
4+
import {
5+
isJupyterNotebook,
6+
parseJupyterNotebook,
7+
applyChangesToNotebook,
8+
writeJupyterNotebook,
9+
validateJupyterNotebookJson,
10+
} from "../jupyter-notebook-handler"
11+
12+
describe("Jupyter Notebook Handler", () => {
13+
const testDir = path.join(__dirname, "test-notebooks")
14+
const testNotebookPath = path.join(testDir, "test.ipynb")
15+
const testTextPath = path.join(testDir, "test.txt")
16+
17+
beforeEach(async () => {
18+
await fs.mkdir(testDir, { recursive: true })
19+
})
20+
21+
afterEach(async () => {
22+
try {
23+
await fs.rm(testDir, { recursive: true, force: true })
24+
} catch (error) {
25+
// Ignore cleanup errors
26+
}
27+
})
28+
29+
describe("isJupyterNotebook", () => {
30+
it("should return true for .ipynb files", () => {
31+
expect(isJupyterNotebook("test.ipynb")).toBe(true)
32+
expect(isJupyterNotebook("/path/to/notebook.ipynb")).toBe(true)
33+
expect(isJupyterNotebook("NOTEBOOK.IPYNB")).toBe(true)
34+
})
35+
36+
it("should return false for non-.ipynb files", () => {
37+
expect(isJupyterNotebook("test.py")).toBe(false)
38+
expect(isJupyterNotebook("test.txt")).toBe(false)
39+
expect(isJupyterNotebook("test")).toBe(false)
40+
expect(isJupyterNotebook("test.ipynb.backup")).toBe(false)
41+
})
42+
})
43+
44+
describe("validateJupyterNotebookJson", () => {
45+
it("should validate correct notebook JSON", () => {
46+
const validNotebook = JSON.stringify({
47+
cells: [],
48+
metadata: {},
49+
nbformat: 4,
50+
nbformat_minor: 2,
51+
})
52+
53+
const result = validateJupyterNotebookJson(validNotebook)
54+
expect(result.valid).toBe(true)
55+
expect(result.error).toBeUndefined()
56+
})
57+
58+
it("should reject invalid JSON", () => {
59+
const result = validateJupyterNotebookJson("invalid json")
60+
expect(result.valid).toBe(false)
61+
expect(result.error).toContain("Invalid JSON")
62+
})
63+
64+
it("should reject JSON without cells", () => {
65+
const invalidNotebook = JSON.stringify({
66+
metadata: {},
67+
nbformat: 4,
68+
nbformat_minor: 2,
69+
})
70+
71+
const result = validateJupyterNotebookJson(invalidNotebook)
72+
expect(result.valid).toBe(false)
73+
expect(result.error).toContain("Missing or invalid 'cells' array")
74+
})
75+
76+
it("should reject JSON without nbformat", () => {
77+
const invalidNotebook = JSON.stringify({
78+
cells: [],
79+
metadata: {},
80+
nbformat_minor: 2,
81+
})
82+
83+
const result = validateJupyterNotebookJson(invalidNotebook)
84+
expect(result.valid).toBe(false)
85+
expect(result.error).toContain("Missing or invalid 'nbformat'")
86+
})
87+
})
88+
89+
describe("parseJupyterNotebook", () => {
90+
it("should return isNotebook false for non-notebook files", async () => {
91+
await fs.writeFile(testTextPath, "Hello world")
92+
const result = await parseJupyterNotebook(testTextPath)
93+
expect(result.isNotebook).toBe(false)
94+
})
95+
96+
it("should parse a simple notebook with code and markdown cells", async () => {
97+
const notebook = {
98+
cells: [
99+
{
100+
cell_type: "markdown",
101+
source: ["# Hello World\n", "This is a markdown cell."],
102+
},
103+
{
104+
cell_type: "code",
105+
source: ["print('Hello, World!')\n", "x = 42"],
106+
},
107+
{
108+
cell_type: "raw",
109+
source: ["This is raw text"],
110+
},
111+
],
112+
metadata: {},
113+
nbformat: 4,
114+
nbformat_minor: 2,
115+
}
116+
117+
await fs.writeFile(testNotebookPath, JSON.stringify(notebook, null, 2))
118+
const result = await parseJupyterNotebook(testNotebookPath)
119+
120+
expect(result.isNotebook).toBe(true)
121+
expect(result.originalJson).toEqual(notebook)
122+
expect(result.extractedContent).toBe(
123+
"# Hello World\nThis is a markdown cell.\nprint('Hello, World!')\nx = 42",
124+
)
125+
expect(result.cellBoundaries).toHaveLength(2)
126+
expect(result.cellBoundaries![0]).toEqual({
127+
cellIndex: 0,
128+
startLine: 1,
129+
endLine: 2,
130+
cellType: "markdown",
131+
})
132+
expect(result.cellBoundaries![1]).toEqual({
133+
cellIndex: 1,
134+
startLine: 3,
135+
endLine: 4,
136+
cellType: "code",
137+
})
138+
})
139+
140+
it("should handle empty cells", async () => {
141+
const notebook = {
142+
cells: [
143+
{
144+
cell_type: "code",
145+
source: [],
146+
},
147+
{
148+
cell_type: "markdown",
149+
source: ["# Title"],
150+
},
151+
],
152+
metadata: {},
153+
nbformat: 4,
154+
nbformat_minor: 2,
155+
}
156+
157+
await fs.writeFile(testNotebookPath, JSON.stringify(notebook, null, 2))
158+
const result = await parseJupyterNotebook(testNotebookPath)
159+
160+
expect(result.isNotebook).toBe(true)
161+
expect(result.extractedContent).toBe("# Title")
162+
expect(result.cellBoundaries).toHaveLength(1)
163+
expect(result.cellBoundaries![0]).toEqual({
164+
cellIndex: 1,
165+
startLine: 1,
166+
endLine: 1,
167+
cellType: "markdown",
168+
})
169+
})
170+
171+
it("should throw error for invalid JSON", async () => {
172+
await fs.writeFile(testNotebookPath, "invalid json")
173+
await expect(parseJupyterNotebook(testNotebookPath)).rejects.toThrow("Failed to parse Jupyter notebook")
174+
})
175+
})
176+
177+
describe("applyChangesToNotebook", () => {
178+
it("should apply changes to notebook cells", () => {
179+
const originalNotebook = {
180+
cells: [
181+
{
182+
cell_type: "markdown" as const,
183+
source: ["# Old Title\n", "Old content."],
184+
},
185+
{
186+
cell_type: "code" as const,
187+
source: ["print('old')\n", "x = 1"],
188+
},
189+
],
190+
metadata: {},
191+
nbformat: 4,
192+
nbformat_minor: 2,
193+
}
194+
195+
const cellBoundaries = [
196+
{
197+
cellIndex: 0,
198+
startLine: 1,
199+
endLine: 2,
200+
cellType: "markdown",
201+
},
202+
{
203+
cellIndex: 1,
204+
startLine: 3,
205+
endLine: 4,
206+
cellType: "code",
207+
},
208+
]
209+
210+
const newExtractedContent = "# New Title\nNew content.\nprint('new')\nx = 2"
211+
212+
const result = applyChangesToNotebook(originalNotebook, newExtractedContent, cellBoundaries)
213+
214+
expect(result.cells[0].source).toEqual(["# New Title\n", "New content."])
215+
expect(result.cells[1].source).toEqual(["print('new')\n", "x = 2"])
216+
})
217+
218+
it("should handle single-line cells", () => {
219+
const originalNotebook = {
220+
cells: [
221+
{
222+
cell_type: "code" as const,
223+
source: ["print('hello')"],
224+
},
225+
],
226+
metadata: {},
227+
nbformat: 4,
228+
nbformat_minor: 2,
229+
}
230+
231+
const cellBoundaries = [
232+
{
233+
cellIndex: 0,
234+
startLine: 1,
235+
endLine: 1,
236+
cellType: "code",
237+
},
238+
]
239+
240+
const newExtractedContent = "print('world')"
241+
242+
const result = applyChangesToNotebook(originalNotebook, newExtractedContent, cellBoundaries)
243+
244+
expect(result.cells[0].source).toEqual(["print('world')"])
245+
})
246+
247+
it("should preserve cells not in boundaries", () => {
248+
const originalNotebook = {
249+
cells: [
250+
{
251+
cell_type: "markdown" as const,
252+
source: ["# Title"],
253+
},
254+
{
255+
cell_type: "raw" as const,
256+
source: ["Raw content"],
257+
},
258+
{
259+
cell_type: "code" as const,
260+
source: ["print('code')"],
261+
},
262+
],
263+
metadata: {},
264+
nbformat: 4,
265+
nbformat_minor: 2,
266+
}
267+
268+
const cellBoundaries = [
269+
{
270+
cellIndex: 0,
271+
startLine: 1,
272+
endLine: 1,
273+
cellType: "markdown",
274+
},
275+
{
276+
cellIndex: 2,
277+
startLine: 2,
278+
endLine: 2,
279+
cellType: "code",
280+
},
281+
]
282+
283+
const newExtractedContent = "# New Title\nprint('new code')"
284+
285+
const result = applyChangesToNotebook(originalNotebook, newExtractedContent, cellBoundaries)
286+
287+
expect(result.cells[0].source).toEqual(["# New Title"])
288+
expect(result.cells[1].source).toEqual(["Raw content"]) // Unchanged
289+
expect(result.cells[2].source).toEqual(["print('new code')"])
290+
})
291+
})
292+
293+
describe("writeJupyterNotebook", () => {
294+
it("should write notebook with proper formatting", async () => {
295+
const notebook = {
296+
cells: [
297+
{
298+
cell_type: "code" as const,
299+
source: ["print('test')"],
300+
},
301+
],
302+
metadata: {},
303+
nbformat: 4,
304+
nbformat_minor: 2,
305+
}
306+
307+
await writeJupyterNotebook(testNotebookPath, notebook)
308+
309+
const writtenContent = await fs.readFile(testNotebookPath, "utf8")
310+
const parsedContent = JSON.parse(writtenContent)
311+
312+
expect(parsedContent).toEqual(notebook)
313+
// Check that it's properly formatted (indented)
314+
expect(writtenContent).toContain(' "cells":')
315+
})
316+
})
317+
318+
describe("integration test", () => {
319+
it("should handle full parse -> modify -> apply cycle", async () => {
320+
const originalNotebook = {
321+
cells: [
322+
{
323+
cell_type: "markdown" as const,
324+
source: ["# Data Analysis\n", "Let's analyze some data."],
325+
},
326+
{
327+
cell_type: "code" as const,
328+
source: ["import pandas as pd\n", "df = pd.read_csv('data.csv')\n", "print(df.head())"],
329+
},
330+
],
331+
metadata: {},
332+
nbformat: 4,
333+
nbformat_minor: 2,
334+
}
335+
336+
// Write original notebook
337+
await fs.writeFile(testNotebookPath, JSON.stringify(originalNotebook, null, 2))
338+
339+
// Parse it
340+
const parseResult = await parseJupyterNotebook(testNotebookPath)
341+
expect(parseResult.isNotebook).toBe(true)
342+
343+
// Modify the extracted content
344+
const modifiedContent =
345+
"# Advanced Data Analysis\nLet's do advanced analysis.\nimport pandas as pd\nimport numpy as np\ndf = pd.read_csv('data.csv')\nprint(df.describe())"
346+
347+
// Apply changes back
348+
const updatedNotebook = applyChangesToNotebook(
349+
parseResult.originalJson!,
350+
modifiedContent,
351+
parseResult.cellBoundaries!,
352+
)
353+
354+
// Write it back
355+
await writeJupyterNotebook(testNotebookPath, updatedNotebook)
356+
357+
// Verify the result
358+
const finalContent = await fs.readFile(testNotebookPath, "utf8")
359+
const finalNotebook = JSON.parse(finalContent)
360+
361+
expect(finalNotebook.cells[0].source).toEqual(["# Advanced Data Analysis\n", "Let's do advanced analysis."])
362+
expect(finalNotebook.cells[1].source).toEqual([
363+
"import pandas as pd\n",
364+
"import numpy as np\n",
365+
"df = pd.read_csv('data.csv')\n",
366+
"print(df.describe())",
367+
])
368+
})
369+
})
370+
})

0 commit comments

Comments
 (0)