Problem
An edit to one notebook cell can invalidate diagnostics in every other cell.
The changed cell is pulled immediately, but other affected cells enter the
generic background document scheduler, which waits 500 ms before each pull.
This means a notebook with 10 dependent cells can take approximately 5 seconds
to show correct diagnostics; 100 dependent cells can take approximately 50
seconds. There is no exponential timer in the implementation: the observed
problem is a fixed per-cell delay that accumulates linearly.
This is also an issue because it delays diagnostic pulls for other documents also (but, arguably, you're not supposed to import notebooks 😆).
Minimal reproduction
First enable the already-supported dependency capability in
client-node-tests/src/servers/fullNotebookServer.ts:
diagnosticProvider: {
identifier: 'diagnostic-provider',
documentSelector: null,
- interFileDependencies: false,
+ interFileDependencies: true,
workspaceDiagnostics: false
}
Then add this test to the existing suite('Full notebook tests', ...) in
client-node-tests/src/integration.test.ts:
test('Notebook dependent cells refresh without a per-cell delay', async () => {
const dependentCellCount = 10;
const notebook = await vscode.workspace.openNotebookDocument(
'jupyter-notebook',
new vscode.NotebookData([
new vscode.NotebookCellData(vscode.NotebookCellKind.Code, 'value = 1', 'python'),
...Array.from({ length: dependentCellCount }, () =>
new vscode.NotebookCellData(vscode.NotebookCellKind.Code, 'use(value)', 'python'),
),
]),
);
const initialPulls = new Set<string>();
const dependentPullTimes = new Map<string, number>();
let editStartedAt: number | undefined;
client.middleware.provideDiagnostics = async (document, previousResultId, token, next) => {
const uri = document instanceof vscode.Uri ? document : document.uri;
const key = uri.toString();
if (editStartedAt === undefined) {
initialPulls.add(key);
} else if (key !== notebook.cellAt(0).document.uri.toString()) {
dependentPullTimes.set(key, Date.now() - editStartedAt);
}
return next(document, previousResultId, token);
};
try {
await vscode.window.showNotebookDocument(notebook);
for (let attempt = 0; attempt < 100; attempt++) {
if (initialPulls.size === notebook.cellCount) {
break;
}
await new Promise<void>((resolve) => setTimeout(resolve, 20));
}
assert.strictEqual(initialPulls.size, notebook.cellCount);
editStartedAt = Date.now();
const edit = new vscode.WorkspaceEdit();
edit.insert(notebook.cellAt(0).document.uri, new vscode.Position(0, 0), '# change\n');
assert.strictEqual(await vscode.workspace.applyEdit(edit), true);
for (let attempt = 0; attempt < 150; attempt++) {
if (dependentPullTimes.size === dependentCellCount) {
break;
}
await new Promise<void>((resolve) => setTimeout(resolve, 50));
}
assert.strictEqual(
dependentPullTimes.size,
dependentCellCount,
'Every dependent notebook cell should be refreshed',
);
const elapsed = Math.max(...dependentPullTimes.values());
assert.ok(
elapsed < 1000,
`Refreshing ${dependentCellCount} dependent cells took ${elapsed} ms: ` +
JSON.stringify([...dependentPullTimes.values()]),
);
} finally {
client.middleware.provideDiagnostics = undefined;
await revertAllDirty();
}
}).timeout(10000);
Expected
After a cell edit, diagnostic pulls for other cells in the same notebook are
coalesced or scheduled promptly as a group.
Problem
An edit to one notebook cell can invalidate diagnostics in every other cell.
The changed cell is pulled immediately, but other affected cells enter the
generic background document scheduler, which waits 500 ms before each pull.
This means a notebook with 10 dependent cells can take approximately 5 seconds
to show correct diagnostics; 100 dependent cells can take approximately 50
seconds. There is no exponential timer in the implementation: the observed
problem is a fixed per-cell delay that accumulates linearly.
This is also an issue because it delays diagnostic pulls for other documents also (but, arguably, you're not supposed to import notebooks 😆).
Minimal reproduction
First enable the already-supported dependency capability in
client-node-tests/src/servers/fullNotebookServer.ts:diagnosticProvider: { identifier: 'diagnostic-provider', documentSelector: null, - interFileDependencies: false, + interFileDependencies: true, workspaceDiagnostics: false }Then add this test to the existing
suite('Full notebook tests', ...)inclient-node-tests/src/integration.test.ts:Expected
After a cell edit, diagnostic pulls for other cells in the same notebook are
coalesced or scheduled promptly as a group.