From 2b01f272470b552aa47f08c5e351ba7a3107cfaf Mon Sep 17 00:00:00 2001 From: mangomac Date: Fri, 16 Oct 2026 13:56:11 +0900 Subject: [PATCH 1/2] Pull notebook diagnostics after cell reorder --- client-node-tests/src/integration.test.ts | 27 +++++++++++++++++++ .../src/servers/fullNotebookServer.ts | 2 +- client/src/common/diagnostic.ts | 11 ++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/client-node-tests/src/integration.test.ts b/client-node-tests/src/integration.test.ts index afa2afc14..42d332c79 100644 --- a/client-node-tests/src/integration.test.ts +++ b/client-node-tests/src/integration.test.ts @@ -2130,6 +2130,33 @@ suite('Full notebook tests', () => { await revertAllDirty(); }).timeout(5000); + + test('Notebook document: reordering cells pulls diagnostics', async(): Promise => { + const notebook = await vscode.workspace.openNotebookDocument('jupyter-notebook', createNotebookData()); + await vscode.window.showNotebookDocument(notebook); + + const pulled: string[] = []; + client.middleware.provideDiagnostics = async (doc, p, token, next) => { + pulled.push(doc instanceof vscode.Uri ? doc.toString() : doc.uri.toString()); + return next(doc, p, token); + }; + + try { + // Move the first cell down. This only changes the order of the cells. + const firstUri = notebook.cellAt(0).document.uri.toString(); + const editor = vscode.window.activeNotebookEditor; + isDefined(editor); + editor.selection = new vscode.NotebookRange(0, 1); + pulled.length = 0; + await vscode.commands.executeCommand('notebook.cell.moveDown'); + assert.strictEqual(notebook.cellAt(1).document.uri.toString(), firstUri); + await new Promise((resolve) => setTimeout(resolve, 750)); + assert.ok(pulled.length > 0, 'Expected diagnostics to be pulled after the cell order changed'); + } finally { + client.middleware.provideDiagnostics = undefined; + await revertAllDirty(); + } + }).timeout(5000); }); suite('Simple notebook tests', () => { diff --git a/client-node-tests/src/servers/fullNotebookServer.ts b/client-node-tests/src/servers/fullNotebookServer.ts index 963860cd4..b67a0ec88 100644 --- a/client-node-tests/src/servers/fullNotebookServer.ts +++ b/client-node-tests/src/servers/fullNotebookServer.ts @@ -47,7 +47,7 @@ connection.onInitialize((_params: InitializeParams): any => { diagnosticProvider: { identifier: 'diagnostic-provider', documentSelector: null, - interFileDependencies: false, + interFileDependencies: true, workspaceDiagnostics: false } }; diff --git a/client/src/common/diagnostic.ts b/client/src/common/diagnostic.ts index b70e55921..7f786cb21 100644 --- a/client/src/common/diagnostic.ts +++ b/client/src/common/diagnostic.ts @@ -1038,6 +1038,17 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape { this.diagnosticRequestor.pull(cell.document, () => { this.backgroundScheduler.trigger(); }); } } + + // Moving cells changes the order of the notebook without changing the text + // or the URIs of the cells. Cell order can change the diagnostics of other + // cells when the server reports inter file dependencies. + if (options.interFileDependencies && event.cells?.structure !== undefined) { + for (const cell of event.notebook.getCells()) { + if (matchesCell(cell)) { + this.diagnosticRequestor.pull(cell.document, () => { this.backgroundScheduler.trigger(); }); + } + } + } })); } From ff03c8726e20ef47ecc17528100f9ebd01ebf73f Mon Sep 17 00:00:00 2001 From: mangomac <333602023+mangomac@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:42:45 +0900 Subject: [PATCH 2/2] fix: avoid duplicate notebook diagnostic pulls and tighten reorder test Signed-off-by: mangomac <333602023+mangomac@users.noreply.github.com> --- client-node-tests/src/integration.test.ts | 12 +++++++++--- client/src/common/diagnostic.ts | 6 +++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/client-node-tests/src/integration.test.ts b/client-node-tests/src/integration.test.ts index 42d332c79..88459af0d 100644 --- a/client-node-tests/src/integration.test.ts +++ b/client-node-tests/src/integration.test.ts @@ -2147,16 +2147,22 @@ suite('Full notebook tests', () => { const editor = vscode.window.activeNotebookEditor; isDefined(editor); editor.selection = new vscode.NotebookRange(0, 1); + // Let the pulls caused by opening the notebook finish before observing. + await new Promise((resolve) => setTimeout(resolve, 500)); pulled.length = 0; await vscode.commands.executeCommand('notebook.cell.moveDown'); assert.strictEqual(notebook.cellAt(1).document.uri.toString(), firstUri); - await new Promise((resolve) => setTimeout(resolve, 750)); - assert.ok(pulled.length > 0, 'Expected diagnostics to be pulled after the cell order changed'); + // Wait for a pull of the moved cell, with an upper bound. + const deadline = Date.now() + 3000; + while (!pulled.includes(firstUri) && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 50)); + } + assert.ok(pulled.includes(firstUri), 'Expected the moved cell to be pulled after the cell order changed'); } finally { client.middleware.provideDiagnostics = undefined; await revertAllDirty(); } - }).timeout(5000); + }).timeout(8000); }); suite('Simple notebook tests', () => { diff --git a/client/src/common/diagnostic.ts b/client/src/common/diagnostic.ts index bb6617ccf..549fdf3ba 100644 --- a/client/src/common/diagnostic.ts +++ b/client/src/common/diagnostic.ts @@ -1018,8 +1018,10 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape { const textEvents = event.cells?.textContent || []; const changedCells = textEvents.map( (c) => event.notebook.getCells().find(cell => cell.document.uri.toString() === c.document.uri.toString())); + const pulledUris = new Set(); for (const cell of changedCells) { if (cell && matchesCell(cell)) { + pulledUris.add(cell.document.uri.toString()); this.diagnosticRequestor.pull(cell.document, () => { this.backgroundScheduler.trigger(); }); } } @@ -1034,6 +1036,7 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape { const openedCells = event.cells?.structure?.didOpen || []; for (const cell of openedCells) { if (matchesCell(cell)) { + pulledUris.add(cell.document.uri.toString()); this.diagnosticRequestor.pull(cell.document, () => { this.backgroundScheduler.trigger(); }); } } @@ -1043,7 +1046,8 @@ class DiagnosticFeatureProviderImpl implements DiagnosticProviderShape { // cells when the server reports inter file dependencies. if (options.interFileDependencies && event.cells?.structure !== undefined) { for (const cell of event.notebook.getCells()) { - if (matchesCell(cell)) { + // Cells pulled above for this event do not need a second pull. + if (matchesCell(cell) && !pulledUris.has(cell.document.uri.toString())) { this.diagnosticRequestor.pull(cell.document, () => { this.backgroundScheduler.trigger(); }); } }