feat(deepnote): one notebook per .deepnote file#429
Merged
Conversation
…d) + add project-id resolver
Chunk 1 of single-notebook migration (§4 partial, §5). No behaviour change.
- Manager caches originals in a nested Map<projectId, Map<notebookId, project>>
so sibling files sharing a project.id no longer clobber each other.
- New API: getOriginalProject(projectId, notebookId) exact/no-fallback,
getAnyProjectEntry(projectId), storeOriginalProject/updateOriginalProject
(3-arg), updateProjectIntegrations iterates all entries.
- Update IDeepnoteNotebookManager and IPlatformDeepnoteNotebookManager; repoint
all project-level read-only callers to getAnyProjectEntry.
- Add canonical readDeepnoteProjectFile and resolveProjectIdFor{File,Notebook}.
- Selection state and init-run tracking intentionally kept (removed in later chunks).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…rop selection machinery Chunk 2 of single-notebook migration (§1 + Cleanup). - deserializeNotebook renders the first non-init notebook (findDefaultNotebook), falling back to the only/init notebook; never composes init. - serializeNotebook resolves the target from document metadata alone (projectId + notebookId required) and looks it up with the exact getOriginalProject, throwing clear errors instead of falling back to a wrong sibling. - detectContentChanges collapses to a single-notebook comparison. - Remove the ?notebook=<id> selection machinery: findCurrentNotebookId, the manager's selection state + interface methods, the explorer's query-param opens and selectNotebookForProject calls, and the tree item's custom resourceUri. - Explorer no longer depends on IDeepnoteNotebookManager. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…k siblings Chunk 3 of single-notebook migration (§0, §2, §3). - Add allocateSiblingUri: the single filesystem-aware, collision-safe sibling filename allocator (bumps -2/-3 before .deepnote, honors an in-batch reserved set, bounded retries). - Add a notebook file factory (buildSingleNotebookFile / buildSiblingNotebookFileUri) for creating sibling single-notebook files (wired into the explorer in a later chunk). - Add DeepnoteMultiNotebookSplitter: on opening a multi-notebook .deepnote file, offer to split it into one new single-notebook file per notebook. The action flushes the editor if dirty, writes all children, migrates the environment selection, then closes the tab and deletes the original to trash. A child-write failure leaves the original intact (write-before-delete). - Wire the splitter into activation with an optional (desktop-only) environment mapper; add a refresh() passthrough on the explorer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
Chunk 4 of single-notebook migration (§6). - Add DeepnoteProjectMetadataPropagator (desktop): given a project id and a project-level mutator, enumerate every sibling .deepnote file on disk (open or closed), apply the change, and write it back. Skips no-op writes, refreshes the manager cache for open siblings, and collects per-file failures instead of aborting. Fires an onFileWritten hook so the file watcher treats each write as a self-write (no reload/save storm). - Route integration updates and project rename through the propagator so closed siblings stay consistent; web falls back to the cache-only / single-file paths. - Expose getOriginalProject/updateOriginalProject on the platform manager interface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…us bar Chunk 5 of single-notebook migration (§7). - Tree is grouped: ProjectGroup (by project id) -> ProjectFile -> Notebook. A single-notebook file is a leaf labelled with its notebook; legacy multi-notebook files stay collapsible. The init notebook is excluded from counts everywhere. - Refresh is grouping-safe: refreshNotebook evicts every sibling cache entry for a project id and all refreshes fire a full-tree change (no per-item fires). - Commands are project-scoped vs notebook-scoped; new/duplicate/add-notebook create sibling files via the factory (never appended), delete removes the file for a single-notebook file, and notebook names are unique within a project group. - Add a status bar item showing the active Deepnote notebook with a "Copy Active Deepnote Notebook Details" command. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
Chunk 6 of single-notebook migration (§8). - Key the server starter maps, the config handle, and the kernel auto-selector by notebook.uri.toString() - the same identity the kernel and controller use - so a notebook's server is 1:1 with its kernel. Sibling notebooks of one project no longer share a server; the working directory and SQL env are taken from each notebook's own file. - Fix environment deletion: stop every server using the environment (including closed notebooks whose server is still running) before removing the mappings, driven from the notebook->environment mapper. Drop the dead environmentServers map that was never populated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…le reader Chunk 7a of single-notebook migration (§9). - Write snapshots with notebook-scoped filenames via @deepnote/convert (generateSnapshotFilename / parseSnapshotFilename), replacing the local slug and filename regex. - readSnapshot resolves snapshots path-free (it runs at deserialize, which has no URI): glob by project id, rank the notebook-scoped match first and keep legacy project-scoped snapshots as a fallback, and skip an empty-output "latest" (save race) or a corrupt file while walking candidates. Legacy snapshots are read, never migrated or deleted. - Defer the execution snapshot save until outputs settle (quiet window with a max wait) and cancel it on re-execute / close. - Use convert's computeSnapshotHash on the save path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
Chunk 7b of single-notebook migration (§10). - The init runner now subscribes to kernel start and restart events and runs the init notebook found in its own sibling .deepnote file (matched by project id + initNotebookId via isValidSiblingInitCandidate), instead of looking it up in the main file's notebooks. - Track "init has run" per kernel in a WeakSet<IKernel>: a fresh kernel runs init once, and an in-place restart (which fires onDidRestartKernel) re-runs it so the kernel is re-initialized before the next user cell. A missing sibling is logged and skipped without permanently marking the project. - Remove the manager's persistent init-run tracking and the selector's init staging; the runner owns init triggering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #429 +/- ##
===========================
===========================
🚀 New features to boost your workflow:
|
- void the fire-and-forget onExecutionComplete call (no-floating-promises), matching the existing void performSnapshotSave pattern. - Use American "behavior" in a comment. - Add test-only technical words (basenames, initmain, Résumé, unparseable) to cspell. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…of duplicating it The mocha ESM loader wholesale-mocked @deepnote/convert and reimplemented its pure helpers (resolveSnapshotNotebookId, splitByNotebooks, isValidSiblingInitCandidate, snapshot filename generate/parse, hashing, etc.). That duplicated upstream logic with no drift detection: if convert changed, the mock silently kept the old behavior and tests stayed green against a fiction. - Remove the @deepnote/convert interception from build/mocha-esm-loader.js so unit tests exercise the real package's pure functions (and now track its actual API). - Mock only the one genuinely side-effecting export, convertIpynbFilesToDeepnoteFile (real node:fs I/O), via esmock in the explorer import suites where it is used. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…r paths From the Codex review of the PR (F1/F3/F4/F5), all independently verified: - F1 (P1): snapshot save fetched the cached project with getAnyProjectEntry(projectId), which can return the wrong sibling when multiple single-notebook siblings of one project are open, silently skipping the snapshot write. Use the exact getOriginalProject(projectId, notebookId) lookup instead. - F3: collectNotebookNamesForProject globbed **/*.deepnote without skipping snapshot sidecars, so stale snapshot notebook names polluted the name-uniqueness set. Filter snapshot files (matching the tree provider and propagator). - F4: detectContentChanges compared notebooks[0]; for a legacy [init, main] file the edited notebook is not at index 0, so edits were missed and modifiedAt preserved. Match the notebook by id. - F5: the deferred-save timer fired performSnapshotSave as a floating promise; wrap the save body in try/catch/finally so a build/write failure is logged (not an unhandled rejection) and execution state is always cleared. Adds regression tests for F1 (exact lookup), F3 (snapshot exclusion), and F4 (match by id). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…el init runs Addresses round-2 code-review findings G2 and G3 (both verified P2). - G2: deepnoteFileChangeWatcher's snapshot block-id recovery used the project-only getAnyProjectEntry(projectId), which can return a different open sibling's cached project (siblings share project.id), leaving originalBlocks undefined and silently skipping recovered outputs. Use the exact getOriginalProject(projectId, notebookId) — the same fix already applied to snapshotService (F1), here in the watcher path that was missed. - G3: moving init execution to the event-driven runner dropped the notebook-close cancellation that the kernel auto-selector used to provide, so closing a notebook mid-init left the remaining init blocks executing against a closed notebook. Tie the init run to a CancellationTokenSource cancelled on notebook close and dispose it in a finally. Adds regression tests for both (each fails on the pre-fix code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URccsVKXeNKZqqPi89L4ro
…ort/delete Removes two pieces of functionality; also folds in the branch's in-progress updates this work was layered on top of (they could not be isolated, as the removals are interleaved with and built on top of that WIP). Removed - project-metadata propagator: - Delete DeepnoteProjectMetadataPropagator and its types, drop the DI binding, and unwire it everywhere (activation, file-change watcher self-write hook, integration webview, explorer rename). Project-level fields are no longer fanned out across sibling files: each notebook owns its own integrations, and project-name drift is accepted for now. Drop the now-dead updateOriginalProject manager method and two stale comments. Removed - project-level explorer commands: - Delete the exportProject and deleteProject commands (constants, command-arg type, registrations, package.json command defs + sidebar menus, nls titles, and their unit tests). Per-notebook export remains via the existing exportNotebook command (first non-init notebook of the file). Also includes the branch's pending updates the above was built on: dependency bumps (incl. @deepnote/convert 4.0), the getOriginalProject -> getProjectForNotebook manager rename and getAnyProjectEntry removal, and assorted snapshot/serializer/kernel adjustments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Brings in #432 (Cloud SQL integration support). Resolved the package.json and package-lock.json conflicts by keeping this branch's newer @deepnote/* versions (blocks 4.6.0, convert 4.0.0, runtime-core 0.4.0); @deepnote/database-integrations is 1.5.0 on both sides, and the Cloud SQL source from #432 merged cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
@deepnote/blocks@4.6.0+ renders `text-cell-bullet` blocks with `indent_level >= 1` using leading spaces (two per level) before the bullet marker. stripMarkdown's bullet regex only matches at column 0, so the leading indentation must be trimmed first for the plain-text cell value to round-trip correctly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
…es re-exports snapshotFiles.ts re-exported six snapshot-filename helpers from @deepnote/convert. Remove the re-export block and import the helpers directly from @deepnote/convert at each use site (snapshotService.ts and the snapshotFiles unit test). snapshotFiles.ts now keeps only its local helpers (SNAPSHOT_FILE_SUFFIX, isSnapshotFile, extractProjectIdFromSnapshotUri) plus the single internal use of parseSnapshotFilename. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Refactor the buildSnapshotPath method to accept an object as an argument, improving readability and maintainability. Update all relevant calls to this method throughout the snapshotService and its unit tests to match the new signature. This change enhances the clarity of parameter usage and reduces the risk of errors when passing arguments.
Trim the single-notebook test suites by removing duplicate and tautological tests and collapsing/merging several others, shrinking the PR's test additions by ~640 lines with no loss of real coverage. Cuts target only tests this branch added: - exact-(projectId, notebookId)-lookup restatements duplicated across the watcher, serializer, snapshot, and manager suites - wrapper tests already covered by the delegate's own tests (addNotebookToProject, sibling-file allocation, project-id resolution) - tautologies over trivial template/getter functions (serverUtils) - framework-registration smoke tests (status bar) Merges keep the one meaningful assertion and drop the duplicate scaffolding (e.g. legacy-delete no-op folded into the existing delete test; two init builders parametrized into one). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
When splitting a legacy multi-notebook .deepnote file into single-notebook
siblings, rename the original to `<name>.deepnote.legacy` instead of moving it
to the OS trash. The `.legacy` suffix takes it out of the extension's view (it
no longer matches `*.deepnote`) while keeping it on disk next to the split
results, so the user can restore it by removing the suffix.
Unlike `workspace.fs.delete({ useTrash: true })`, this is deterministic and does
not depend on an OS trash backend (which can be absent on headless Linux).
Collisions bump the name to `.legacy-2`, `.legacy-3`, … and the rename still
happens only after every child is durably written (write-before-retire).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Add an ExTester end-to-end test that drives the real VS Code UI through the on-open split of a legacy multi-notebook .deepnote file: it asserts the split prompt, the one-file-per-notebook result, the retained `.legacy` backup, that each sibling opens without re-prompting, and that content plus the project integration fan out into every split file. Add a `createScreenshotter(this)` helper that captures step screenshots into a per-spec directory derived from the running test file (`test/e2e/screenshots/<spec>/`), plus the `sales-analytics.deepnote` fixture. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
…ites Add ExTester end-to-end suites covering: - opening a plain single-notebook file (opens directly, no split prompt, the status bar shows the notebook name); - splitting a multi-notebook file that declares an init notebook (the init notebook becomes its own single-notebook sibling; each main sibling still references it via initNotebookId); - the init-notebook runner: the sibling init notebook runs hidden in a main notebook's kernel so its definitions are available, and re-runs after a kernel restart. Add the quick-notes and etl-pipeline fixtures (including the pre-split extract/init siblings), and disable the kernel-restart confirmation in the E2E settings so the restart test can drive it non-interactively. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Add an ExTester end-to-end test asserting the Deepnote Explorer groups sibling
.deepnote files by project: three files sharing one project.id collapse into a
single "Marketing" group ("3 files") whose leaves are the three notebooks, while
a file from a different project appears as its own group. Reads the tree by
diffing visible leaves before/after expanding (avoids the page-object library's
flaky CustomTreeItem.getChildItems). Adds the three marketing fixtures.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
When several E2E suites run in one ExTester session (as in CI, via the `*.e2e.test.js` glob), every workspace-folder open after the first failed with "Failed to open folder after 5 attempts" in the suite's `before all` hook — only the alphabetically-first suite passed. Root cause: the simple "Open Folder" dialog (files.simpleDialog.enable) navigates one directory level *toward* the typed path per OK click and only accepts the folder once the browser is AT it. The helper clicked OK once then re-opened the dialog each attempt, which reset navigation back to the default directory — for the 2nd+ open that default is the previous, now-deleted workspace, so the dialog fell back to "/" and never converged on the target. Fix: click OK repeatedly within a single dialog until the pre-open workbench element detaches (reload = folder accepted), instead of re-opening per attempt; and set `window.openFoldersInNewWindow: "off"` so "Open Folder" reuses the current window, keeping that reload detectable. Verified with four suites (16 tests) opening four folders in one session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Add an ExTester end-to-end test for the Deepnote status-bar item: it shows the active notebook's name (with the "Copy Active Deepnote Notebook Details" tooltip), hides when a non-notebook editor is focused, and — on click — copies the notebook details to the clipboard with a confirmation toast. The clipboard is verified by pasting into a scratch text file and reading it back. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Add an ExTester end-to-end test for the notebook-management commands that create and rename sibling .deepnote files from the Deepnote explorer: New Notebook, Add Notebook (project-group context menu), Duplicate Notebook, and Rename Notebook — each verified by the resulting notebook name inside the sibling files plus the confirmation toast. Delete Notebook is included as a pending test: its context-menu -> native confirmation-modal interaction is unreliable to drive under ExTester (documented inline), so it is left as a manual check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Add an ExTester end-to-end test for the Deepnote integrations UI: opening "Manage Integrations" for a notebook whose project declares an integration lists it (the "Sales BigQuery" integration on the sales-analytics-revenue fixture), while a plain notebook (quick-notes) shows no such integration. Adds the sales-analytics-revenue fixture (a single-notebook split of the Sales Analytics project carrying the BigQuery integration + its SQL cell). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
Add an ExTester end-to-end test for renaming a Deepnote project from the Explorer: none of the three "Marketing" siblings is opened, then the project group is renamed to "Growth" via its context menu; the new name is asserted to fan out to every sibling .deepnote file on disk (and the old name gone), with the Explorer group relabelled and a confirmation toast. Extract the shared Deepnote tree helpers (getDeepnoteExplorerSection, readDeepnoteTreeRows, findDeepnoteGroup/Leaf, selectDeepnoteContextMenu) into test/e2e/helpers/deepnoteTree.ts for reuse across the tree-driven suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2CLu8UmD8ceGNyv8u96pQ
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/platform/deepnote/deepnoteDocumentFlush.unit.test.ts (1)
14-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the "not dirty" and "no matching document" branches.
Both tests exercise the dirty-document path only; the early-return branches (
!openDocument?.isDirtywhen nothing matches, or when the match isn't dirty) are untouched.✅ Suggested additional cases
+ test('returns true immediately when no open document matches the uri', async () => { + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + + const result = await flushNotebookDocumentIfDirty(Uri.file('/workspace/target.deepnote')); + + assert.isTrue(result); + }); + + test('returns true without saving when the matching document is not dirty', async () => { + const targetUri = Uri.file('/workspace/target.deepnote'); + let saveCalls = 0; + const doc = fakeNotebookDocument(targetUri, false, async () => { + saveCalls++; + return true; + }); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([doc]); + + const result = await flushNotebookDocumentIfDirty(targetUri); + + assert.isTrue(result); + assert.strictEqual(saveCalls, 0, 'a non-dirty document must not be saved'); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/deepnote/deepnoteDocumentFlush.unit.test.ts` around lines 14 - 46, Add tests to the flushNotebookDocumentIfDirty suite covering an open matching document with isDirty false and no matching document in workspace.notebookDocuments. Assert both cases return false and verify the save callback is not invoked where applicable, while preserving the existing dirty-document tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/platform/deepnote/deepnoteDocumentFlush.unit.test.ts`:
- Around line 14-46: Add tests to the flushNotebookDocumentIfDirty suite
covering an open matching document with isDirty false and no matching document
in workspace.notebookDocuments. Assert both cases return false and verify the
save callback is not invoked where applicable, while preserving the existing
dirty-document tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b53be18b-e3be-4bb3-80e0-2f449442f882
📒 Files selected for processing (26)
src/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteExplorerView.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/deepnoteInitNotebookRunner.node.unit.test.tssrc/notebooks/deepnote/deepnoteMultiNotebookSplitter.tssrc/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookFileFactory.tssrc/notebooks/deepnote/deepnoteNotebookFileFactory.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/notebooks/deepnote/deepnoteTestHelpers.tssrc/notebooks/deepnote/deepnoteTreeDataProvider.unit.test.tssrc/notebooks/deepnote/integrations/integrationWebview.tssrc/notebooks/deepnote/integrations/projectIntegrationsWriter.tssrc/notebooks/deepnote/integrations/projectIntegrationsWriter.unit.test.tssrc/notebooks/deepnote/snapshots/snapshotService.tssrc/notebooks/deepnote/snapshots/snapshotService.unit.test.tssrc/platform/deepnote/deepnoteDocumentFlush.tssrc/platform/deepnote/deepnoteDocumentFlush.unit.test.tstest/e2e/suite/environment.e2e.test.tstest/e2e/suite/initOnlyFile.e2e.test.tstest/e2e/suite/notebookCommands.e2e.test.tstest/e2e/suite/openSingleNotebook.e2e.test.tstest/e2e/suite/snapshots.e2e.test.tstest/e2e/suite/splitInitNotebook.e2e.test.tstest/e2e/suite/splitMultiNotebook.e2e.test.tstest/e2e/suite/splitSafety.e2e.test.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- test/e2e/suite/snapshots.e2e.test.ts
- test/e2e/suite/openSingleNotebook.e2e.test.ts
- test/e2e/suite/environment.e2e.test.ts
- src/notebooks/deepnote/deepnoteNotebookFileFactory.unit.test.ts
- src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts
- test/e2e/suite/splitMultiNotebook.e2e.test.ts
- src/notebooks/deepnote/deepnoteInitNotebookRunner.node.unit.test.ts
- test/e2e/suite/splitInitNotebook.e2e.test.ts
- test/e2e/suite/initOnlyFile.e2e.test.ts
- test/e2e/suite/notebookCommands.e2e.test.ts
- test/e2e/suite/splitSafety.e2e.test.ts
- src/notebooks/deepnote/deepnoteActivationService.ts
- src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
- src/notebooks/deepnote/deepnoteTreeDataProvider.unit.test.ts
- src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
- src/notebooks/deepnote/snapshots/snapshotService.unit.test.ts
- src/notebooks/deepnote/snapshots/snapshotService.ts
- src/notebooks/deepnote/deepnoteExplorerView.ts
Use "Filenames" instead of "Basenames" so the comment passes cspell. Co-authored-by: Cursor <cursoragent@cursor.com>
The project-rename e2e only exercised closed siblings, so the guard that saves open, dirty notebooks before the rewrite (otherwise the file-watcher reload clobbers the live cell edits) was unprotected. Extend the existing suite: open one sibling, type an unsaved marker into its code cell, then Rename Project, and assert the marker survived on disk alongside the new project name. Verified as a real regression guard — with the pre-rewrite flush removed, this new case fails (marker lost) while the existing three still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/suite/projectRename.e2e.test.ts (1)
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the editor-focus delay.
Extract
400to a module-levelEDITOR_FOCUS_SETTLE_DELAYconstant.Proposed change
const TREE_LOAD_TIMEOUT = 30_000; +const EDITOR_FOCUS_SETTLE_DELAY = 400; - await driver.sleep(400); + await driver.sleep(EDITOR_FOCUS_SETTLE_DELAY);As per coding guidelines, “Extract magic numbers (retry counts, delays, timeouts) as named constants near the top of the module.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/suite/projectRename.e2e.test.ts` at line 51, Extract the 400 millisecond delay used by the editor-focus flow in projectRename.e2e.test.ts into a module-level constant named EDITOR_FOCUS_SETTLE_DELAY near the top of the module, then use that constant in the driver.sleep call.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/e2e/suite/projectRename.e2e.test.ts`:
- Line 51: Extract the 400 millisecond delay used by the editor-focus flow in
projectRename.e2e.test.ts into a module-level constant named
EDITOR_FOCUS_SETTLE_DELAY near the top of the module, then use that constant in
the driver.sleep call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58b71a30-0d25-4305-ab74-8b6030cf99f8
📒 Files selected for processing (1)
test/e2e/suite/projectRename.e2e.test.ts
… files The `notebook` codicon renders as a clock face at tree size (verified against the codicon set in VS Code 1.111 — it's a real glyph, `\ebaf`, not a fallback), so every `.deepnote` file showed a clock in the Explorer. Replace it: - Single-notebook files now use `file-code`, matching the notebook children they represent — every "notebook you open" looks the same. - Legacy multi-notebook files use `book` (a container of notebooks), and their tooltip gains a "(legacy)" suffix to mark the retired one-file-many-notebooks layout. Icons rendered and confirmed in a headless VS Code 1.111 (folder / book / file-code, no clock); tree-item and provider unit-test icon and tooltip assertions updated accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
Contributor
Author
|
@dinohamzic thanks for the findings .. I've addressed all of them |
dinohamzic
reviewed
Jul 16, 2026
dinohamzic
left a comment
dinohamzic
left a comment
Contributor
There was a problem hiding this comment.
I didn't spot any major issue while manual testing, and the reported ones are fixed, nice one.
Another AI review in case you spot anything relevant:
-
[P1] Integration updates can report success without reaching disk. projectIntegrationsWriter.ts:84
cacheUpdatedmakes the operation succeed even when no files were written—for example, when a.deepnotefile is opened outside a workspace folder—or when some sibling writes fail. The webview then posts “Configuration saved successfully,” but integration membership can disappear on reopen or differ across siblings. Persistence should target the active file explicitly and report/throw on incomplete fan-out. -
[P2] Snapshot selection is still capped before ranking. snapshotService.ts:909
The scoped search still returns an arbitrary 200 files before sorting, so after 200 executions the notebook’s
latestor newest snapshot can be omitted. The legacy pass is broader and filters sibling-scoped files only after the same cap, so noisy siblings can still hide the legacy fallback. Query exactlatestpatterns first, then apply retention or a cap after filtering and ordering. -
[P2] A failed child write can still leave an orphan. deepnoteMultiNotebookSplitter.ts:155
Cleanup is registered only after
writeFileresolves. Filesystem providers may create or partially write the destination before rejecting, leaving an untracked sibling that causes suffixed duplicates on retry. Register safe cleanup before the write or explicitly probe/delete the current target on failure. -
[P2] Splitting proceeds when the original tab failed to close. deepnoteMultiNotebookSplitter.ts:261
tabGroups.close()returns whether the close succeeded, but the result is ignored. If it returns false, the extension renames the file while its editor remains open; a later save can recreate the legacy.deepnotefile alongside the generated siblings. Return/check the boolean and abort before rename.
Saving integrations reported success from the in-memory cache update alone, so a
folderless / out-of-workspace notebook (findFiles covers only open folders) or a
partial sibling fan-out could post "Configuration saved successfully" having
written nothing to the file being edited — integration membership then vanished
on reopen.
persistProjectIntegrations now writes the ACTIVE file explicitly first, then fans
out, and returns { activePersisted, siblingsFailed } judged from disk (not the
cache); the webview only claims success when the active file reached disk and
warns on an incomplete fan-out. persistProjectIntegrations / writeIntegrationsToFile
also take a single params object instead of positional arguments.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
findSnapshotOutputs passed maxResults to findFiles, which truncates an UNORDERED set to 200 BEFORE ranking — so a notebook with >200 of its own snapshots could have its `latest` dropped (stale outputs on open), and a sibling flood could push a notebook's legacy fallback out of the window (no outputs on open). The glob is now uncapped and the 200 bound is applied to the ranked candidate list, so the best candidate is always considered and only the read loop is bounded. Tests honor findFiles' maxResults so a pre-ranking cap is observable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
…en tabs Two failure paths after the child-write phase of a split: - A provider that created a destination then rejected mid-write left an untracked orphan, because the delete-compensation was registered AFTER the write; a retry then bumped the name to a "-2" duplicate. Register the cleanup BEFORE the write, guarded by an exists() probe so a write that created nothing stays a no-op. - closeNotebookTab discarded the tabGroups.close() result, so a re-dirtied tab whose close was cancelled left the file renamed out from under an open editor — a later save recreated the legacy .deepnote. It now returns the boolean (and closes every matching tab); the split throws before rename if it can't close. Adds a TabInputNotebook mock (verified field/constructor-exact against the real vscode 1.95.0 source) so the close-cancelled path can be driven in a unit test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
"folderless" is flagged by cspell; reword to "no open folder" instead of adding a dictionary entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/notebooks/deepnote/integrations/projectIntegrationsWriter.ts`:
- Line 69: Update the project integration discovery flow around the findFiles
call and its returned activePersisted/siblingsFailed result to track discovery
failures separately. Propagate that failure through the result so the webview
can warn that sibling propagation may be incomplete, rather than reporting full
success when a workspace folder scan fails.
- Around line 94-97: After re-reading the project in the save flow, validate
that projectData.project.id still matches projectId before assigning
integrations in projectIntegrationsWriter. If it does not match, stop without
writing; preserve the existing integrations-only update for the matching
project.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a76c43a-f082-41dd-8d9b-261de2f5f984
📒 Files selected for processing (12)
src/notebooks/deepnote/deepnoteMultiNotebookSplitter.tssrc/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.tssrc/notebooks/deepnote/integrations/integrationManager.tssrc/notebooks/deepnote/integrations/integrationWebview.tssrc/notebooks/deepnote/integrations/integrationWebview.unit.test.tssrc/notebooks/deepnote/integrations/projectIntegrationsWriter.tssrc/notebooks/deepnote/integrations/projectIntegrationsWriter.unit.test.tssrc/notebooks/deepnote/integrations/types.tssrc/notebooks/deepnote/snapshots/snapshotService.tssrc/notebooks/deepnote/snapshots/snapshotService.unit.test.tssrc/test/mocks/vsc/index.tssrc/test/vscode-mock.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/notebooks/deepnote/integrations/integrationManager.ts
- src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts
- src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts
- src/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.ts
- src/notebooks/deepnote/snapshots/snapshotService.unit.test.ts
- src/notebooks/deepnote/snapshots/snapshotService.ts
The integration writer checked project.id before flushing the open document, but the flush can save a stale dirty tab whose on-disk file was swapped to a different project. The post-flush re-read was then written without re-checking identity, so integrations could land in the wrong project's file. Re-validate after the re-read and skip on mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
dinohamzic
approved these changes
Jul 17, 2026
tkislan
added a commit
that referenced
this pull request
Jul 17, 2026
Resolves conflicts from the single-notebook refactor (#429) against the telemetry PR: - Explorer/environments/integration/activation constructors: keep both sides' injected params (added `analytics: ITelemetryService`). - DeepnoteExplorerView adopts main's refactored command handlers and re-applies telemetry: methods return `boolean` so the command wrappers record `{ completed }` (open/create/import/delete/duplicate/export). - Dropped `delete_project` and project-level `export_notebook` telemetry: #429 removed the deleteProject/exportProject commands, methods, and package.json menus entirely. Removed the now-unused `delete_project` from the TelemetryEventName union. - deepnoteCellExecutionAnalytics: use main's `getProjectForNotebook(projectId, notebookId)` (was `getOriginalProject`). - package-lock.json: main's lockfile + posthog-node@4.18.0. typecheck 0, lint 0, compile-tsc 0, unit tests 2502 passing / 0 failing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
6 tasks
tkislan
added a commit
that referenced
this pull request
Jul 17, 2026
Brings in the squash-merged PR #429 ("one notebook per .deepnote file"). This branch forked from an earlier tk/single-notebook, so #429's later work surfaced as conflicts in shared files. Resolved by taking #429's newer versions of the shared single-notebook code (persistProjectIntegrations refactor with activeFileUri, snapshot candidate ranking, abortable tab-close) while preserving this PR's env-file integration additions and unioning divergent imports. Compiles clean; full unit suite green (2543 passing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CPTs6CHNncauUuTGkwpNtH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the extension to a single notebook per
.deepnotefile model, removing the multi-notebook-per-file machinery (the?notebook=<id>URI selection and its timers/manager state).What changes
(projectId, notebookId)lookup..deepnoteof the project on disk (open or closed), with file-watcher self-write suppression.notebook.uri.toString(), 1:1 with the kernel/controller). Environment deletion now actually stops every server using it (including closed-but-running ones), fixing a real bug.WeakSet<IKernel>, not a persistent project flag).Review hardening & polish
Since the initial build, the branch has been through review and a correctness / UX pass:
latestout of the window.{project}-{notebook}.deepnotefrom the project name instead of accumulating sibling names.file-code; legacy multi-notebook files usebookand carry a "(legacy)" tooltip (the oldnotebookcodicon rendered as a clock at tree size).DeepnoteTreeDataProvideris constructor-injected for testability, and the split prompt now flags the multi-notebook layout as legacy.How it was built
95 commits: the feature first landed in 7 sequential, independently-reviewed chunks, followed by the review-driven hardening pass above, a full ExTester E2E suite, and a comment / style cleanup.
Verification
tscclean (0 errors),eslintclean,prettier --checkclean, both esbuild bundles build.🤖 Generated with Claude Code
https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ
Summary by CodeRabbit
.deepnotefiles by project and shifts key actions to notebook-scoped workflows (create/duplicate/rename/delete/import/export), with project renaming staying group-level.