From 12954fbf4308e3a49d5203b08a5e75011c4a636b Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Tue, 20 Oct 2026 20:56:03 +0800 Subject: [PATCH 1/5] fix: preserve Interactive plots during incremental updates --- src/interactive/manager.ts | 5 +- src/interactive/notebookOutput.ts | 19 +++++++ src/interactive/plotImage.ts | 62 ++++++++++++++++++++++ src/interactive/renderer.ts | 63 ++++++++++++++--------- src/test/browser/interactiveRenderer.html | 56 +++++++++++++++++--- src/test/node/notebookOutput.test.ts | 53 +++++++++++++++++++ 6 files changed, 223 insertions(+), 35 deletions(-) create mode 100644 src/interactive/notebookOutput.ts create mode 100644 src/interactive/plotImage.ts create mode 100644 src/test/node/notebookOutput.test.ts diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 499c820d9..b1527ba61 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -5,6 +5,7 @@ import { randomUUID } from 'crypto'; import { AgentClient } from './client'; import { AgentConfig, AgentSnapshot, DEFAULT_MAX_ASSET_BYTES, ExecutionRecord, SessionEvent, SessionManifest, SourceLocation, object, sessionLabel } from './protocol'; import { AssetStore, AssetStorageStats, exportedAssetName, readAsset } from './assets'; +import { updateNotebookOutputs } from './notebookOutput'; import { defaultStorage, discoverSessions, hasSessionEndpoint, launchAgent, newIdentity, prepareNodeRuntime } from './launcher'; import { NodeRuntime, resolveNodeRuntime } from './nodeExecutable'; import { backendDescriptor, prepareBackendRuntime } from './backendRegistry'; @@ -1165,7 +1166,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro if (execution) { // A dispatch failure may have diagnostics but no R start event. if (!execution.started) { execution.task.start(data.record.started); execution.started = true; } - await execution.task.replaceOutput(outputs); + await updateNotebookOutputs(execution.task, cell.outputs, outputs); } else { // Updating a late plot must preserve the cell URI, edited code, selection, and diagnostics. @@ -1173,7 +1174,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro const update = view.controller.createNotebookCellExecution(cell); update.executionOrder = summary?.executionOrder; update.start(summary?.timing?.startTime); - try { await update.replaceOutput(outputs); } + try { await updateNotebookOutputs(update, cell.outputs, outputs); } finally { update.end(summary?.success, summary?.timing?.endTime); } } } diff --git a/src/interactive/notebookOutput.ts b/src/interactive/notebookOutput.ts new file mode 100644 index 000000000..8d90c2b1d --- /dev/null +++ b/src/interactive/notebookOutput.ts @@ -0,0 +1,19 @@ +import type { NotebookCellExecution, NotebookCellOutput } from 'vscode'; + +/** Keep existing output identities while an execution appends or updates results. */ +export async function updateNotebookOutputs(execution: Pick, + previous: readonly NotebookCellOutput[], next: readonly NotebookCellOutput[]): Promise { + const compatible = previous.length <= next.length && previous.every((output, index) => + JSON.stringify(output.metadata) === JSON.stringify(next[index].metadata) + && (typeof output.metadata?.rDisplayId === 'string' + || output.items.map(item => item.mime).join('\n') === next[index].items.map(item => item.mime).join('\n'))); + if (!compatible) { await execution.replaceOutput(next); return; } + for (let index = 0; index < previous.length; index++) { + const output = previous[index], items = next[index].items; + if (output.items.length !== items.length || output.items.some((item, itemIndex) => + item.mime !== items[itemIndex].mime || Buffer.compare(item.data, items[itemIndex].data) !== 0)) { + await execution.replaceOutputItems(items, output); + } + } + if (next.length > previous.length) { await execution.appendOutput(next.slice(previous.length)); } +} diff --git a/src/interactive/plotImage.ts b/src/interactive/plotImage.ts new file mode 100644 index 000000000..39b192951 --- /dev/null +++ b/src/interactive/plotImage.ts @@ -0,0 +1,62 @@ +/// + +export function plotImageSource(page: Record): string { + // The notebook already carries small/portable images. Using them also avoids + // a second transfer through the remote asset tunnel and its failure modes. + if (typeof page.imageData === 'string' && page.imageData) { return `data:${String(page.mime)};base64,${page.imageData}`; } + if (typeof page.svgData === 'string' && page.svgData) { return `data:image/svg+xml;base64,${page.svgData}`; } + return typeof page.url === 'string' ? page.url : ''; +} + +/** Load one snapshot at a time, coalescing updates without clearing the visible plot. */ +export class PlotImage { + image?: HTMLImageElement; + private loading?: HTMLImageElement; + private desired?: { source: string; width: number; height: number; alt: string }; + private shown?: string; + private disposed = false; + + constructor(private element: HTMLElement, private status: (error: boolean) => void) {} + + ready(): boolean { + return !!this.image?.complete && this.image.naturalWidth > 0 + && (this.shown === this.desired?.source || this.image.src === this.desired?.source); + } + + update(page: Record, alt: string): void { + this.desired = { source: plotImageSource(page), width: Number(page.width), height: Number(page.height), alt }; + if (this.shown === this.desired.source && this.image) { this.image.alt = alt; } + this.load(); + } + + dispose(): void { + this.disposed = true; + if (this.loading) { this.loading.onload = null; this.loading.onerror = null; this.loading.removeAttribute('src'); } + this.loading = undefined; + } + + private load(): void { + if (this.disposed || this.loading || !this.desired || this.shown === this.desired.source) { return; } + const requested = this.desired; + const image = document.createElement('img'); image.crossOrigin = 'anonymous'; + image.style.cssText = 'display:block;max-width:100%;height:auto'; image.alt = requested.alt; + if (requested.width > 0 && requested.height > 0) { image.width = requested.width; image.height = requested.height; } + this.loading = image; + // Reserve space on the first frame; subsequent frames load off screen. + if (!this.image) { this.image = image; this.element.append(image); } + const complete = (error: boolean): void => { + if (this.disposed || this.loading !== image) { return; } + this.loading = undefined; image.onload = null; image.onerror = null; + if (this.desired?.source !== requested.source) { this.load(); return; } + if (!error) { + image.width = image.naturalWidth; image.height = image.naturalHeight; image.alt = this.desired.alt; + if (this.image !== image) { this.image!.replaceWith(image); this.image = image; } + this.shown = requested.source; + } + this.status(error); + }; + image.onload = () => complete(false); + image.onerror = () => complete(true); + image.src = requested.source; + } +} diff --git a/src/interactive/renderer.ts b/src/interactive/renderer.ts index 3ef312bc3..849870a8e 100644 --- a/src/interactive/renderer.ts +++ b/src/interactive/renderer.ts @@ -3,6 +3,7 @@ import { createInlineTable, InlineTable, InlineTableState, inlineTableStyle } from './inlineTable'; import { createListViewer, ListViewer, ListViewNavigation, ListViewPage, ListViewReply, listViewerStyle } from '../listViewer'; import { toolbarButton as button, toolbarStyle } from './rendererToolbar'; +import { PlotImage } from './plotImage'; interface OutputItem { id: string; json(): Record } interface RendererContext { @@ -15,6 +16,7 @@ export function activate(context: RendererContext): { renderOutputItem(item: Out element: HTMLElement; data: Record; table?: InlineTable; list?: ListViewer; saveRequest?: number; saveButton?: HTMLButtonElement; savePlot?(format: string): void; updatePlotPaging?(): void; + updatePlot?(data: Record): void; dispose?(): void; } const outputs = new Map(); @@ -57,8 +59,14 @@ export function activate(context: RendererContext): { renderOutputItem(item: Out }); return { renderOutputItem(item, element) { + const data = item.json(); + const previous = outputs.get(item.id); + if (previous?.updatePlot && previous.element === element && data.kind === 'plot' + && previous.data.displayId === data.displayId && previous.data.generation === data.generation) { + previous.updatePlot(data); return; + } outputs.get(item.id)?.dispose?.(); - const data = item.json(); element.replaceChildren(); + element.replaceChildren(); const choiceKey = `${typeof data.generation === 'string' ? data.generation : ''}:${typeof data.displayId === 'string' ? data.displayId : item.id}`; const choice = choices.get(choiceKey); const connected = data.connected !== false; @@ -154,20 +162,23 @@ export function activate(context: RendererContext): { renderOutputItem(item: Out toolbar.append(open, toggle, reset); status.textContent = connectionHint(data); showView(); state.dispose = () => { state.list?.dispose(); open.onclick = null; toggle.onclick = null; }; } else if (data.kind === 'plot') { - const pages = (Array.isArray(data.pages) && data.pages.length ? data.pages : [data]) as Record[]; + let pages = (Array.isArray(data.pages) && data.pages.length ? data.pages : [data]) as Record[]; const selected = choice?.selectedPlot ?? data.selectedPlot; let index = pages.findIndex(page => page.displayId === selected); if (index < 0) { index = pages.length - 1; } - const current = (): Record => ({ ...pages[index], generation: data.generation }); - const image = document.createElement('img'); image.crossOrigin = 'anonymous'; - image.style.cssText = 'display:block;max-width:100%;height:auto'; element.append(image); - const pngReady = (): boolean => image.complete && image.naturalWidth > 0; + const current = (): Record => ({ ...pages[index], generation: state.data.generation }); + const viewport = document.createElement('div'); element.append(viewport); + const image = new PlotImage(viewport, error => { + status.textContent = error ? 'Could not load this plot. Reconnect to the session and try again.' : connectionHint(state.data); + }); + const pngReady = (): boolean => image.ready(); const savePng = (): void => { - const canvas = document.createElement('canvas'); canvas.width = image.naturalWidth; canvas.height = image.naturalHeight; + const loaded = image.image!; + const canvas = document.createElement('canvas'); canvas.width = loaded.naturalWidth; canvas.height = loaded.naturalHeight; const ctx = canvas.getContext('2d'); try { if (!ctx) { throw new Error('Canvas unavailable'); } - ctx.drawImage(image, 0, 0); send(item, current(), 'savePng', { image: canvas.toDataURL('image/png') }); + ctx.drawImage(loaded, 0, 0); send(item, current(), 'savePng', { image: canvas.toDataURL('image/png') }); } catch { status.textContent = 'Open the plot to export this image.'; } }; // The host's native picker does not resize or scroll the notebook output. @@ -179,7 +190,7 @@ export function activate(context: RendererContext): { renderOutputItem(item: Out save.disabled = !connected; save.setAttribute('aria-haspopup', 'dialog'); state.saveButton = save; state.savePlot = format => { - if (!connected) { return; } + if (state.data.connected === false) { return; } if (format === 'svg') { send(item, current(), 'save'); } else if (format === 'png' && pngReady()) { savePng(); } }; @@ -194,35 +205,37 @@ export function activate(context: RendererContext): { renderOutputItem(item: Out previous.disabled = state.saveRequest !== undefined || index === 0; next.disabled = state.saveRequest !== undefined || index === pages.length - 1; counter.textContent = `Plot ${index + 1} of ${pages.length}`; - fit.disabled = !live || pages[index].resizable === false; + fit.disabled = state.data.connected === false || state.data.archived === true || state.data.running === false || pages[index].resizable === false; fit.hidden = pages[index].resizable === false; + previous.hidden = counter.hidden = next.hidden = pages.length < 2; }; const showPlot = (): void => { - const page = pages[index]; - // Reserve the previous page's dimensions until the new image - // loads, so equal-sized pages do not collapse between clicks. - if (Number(page.width) > 0 && Number(page.height) > 0) { - image.width = Number(page.width); image.height = Number(page.height); - } - image.alt = pages.length > 1 ? `R plot ${index + 1} of ${pages.length}` : 'R plot'; - image.src = typeof page.url === 'string' && page.url ? page.url : typeof page.imageData === 'string' && page.imageData - ? `data:${String(page.mime)};base64,${page.imageData}` : `data:image/svg+xml;base64,${typeof page.svgData === 'string' ? page.svgData : ''}`; + image.update(pages[index], pages.length > 1 ? `R plot ${index + 1} of ${pages.length}` : 'R plot'); state.updatePlotPaging?.(); }; const select = (page: number): void => { if (state.saveRequest !== undefined || page < 0 || page >= pages.length) { return; } index = page; remember(choiceKey, { selectedPlot: String(pages[index].displayId) }); - send(item, data, 'plotPage', { selectedPlot: pages[index].displayId }); - status.textContent = connectionHint(data); showPlot(); + send(item, state.data, 'plotPage', { selectedPlot: pages[index].displayId }); + status.textContent = connectionHint(state.data); showPlot(); + }; + state.updatePlot = updated => { + // A pending format picker belongs to the preceding snapshot. + state.saveRequest = undefined; + state.data = { ...updated }; + pages = (Array.isArray(updated.pages) && updated.pages.length ? updated.pages : [updated]) as Record[]; + const selected = choices.get(choiceKey)?.selectedPlot ?? updated.selectedPlot; + index = pages.findIndex(page => page.displayId === selected); + if (index < 0) { index = pages.length - 1; } + open.disabled = save.disabled = updated.connected === false; + status.textContent = connectionHint(updated); showPlot(); }; - image.onload = () => { image.width = image.naturalWidth; image.height = image.naturalHeight; }; - image.onerror = () => { status.textContent = 'Could not load this plot. Reconnect to the session and try again.'; }; state.dispose = () => { for (const control of [save, open, fit, previous, next]) { control.onclick = null; } - image.onload = null; image.onerror = null; + image.dispose(); }; - if (pages.length > 1) { toolbar.append(previous, counter, next); } + toolbar.append(previous, counter, next); toolbar.append(open, save, fit); showPlot(); } else if (data.kind === 'html' || data.kind === 'htmlText') { const iframe = document.createElement('iframe'); diff --git a/src/test/browser/interactiveRenderer.html b/src/test/browser/interactiveRenderer.html index f96f7ec13..634539d59 100644 --- a/src/test/browser/interactiveRenderer.html +++ b/src/test/browser/interactiveRenderer.html @@ -47,6 +47,10 @@

R Interactive output

img.addEventListener('load',()=>{ clearTimeout(timeout); resolve(); },{ once:true }); img.addEventListener('error',()=>{ clearTimeout(timeout); reject(new Error('Plot load failed')); },{ once:true }); }); +const until = async predicate => { + const deadline=Date.now()+5000; + while (!predicate()) { if (Date.now()>deadline) throw new Error('Renderer update timed out'); await new Promise(resolve=>setTimeout(resolve,10)); } +}; try { const listHost = document.querySelector('#list'); const listData = {kind:'list',displayId:'list',generation:'r1',navigation:{title:'List',path:[],breadcrumbs:[{label:'List',path:[]}]}, @@ -266,7 +270,7 @@

R Interactive output

const initialReady=plot.querySelector('img').complete && plot.querySelector('img').naturalWidth>0; save.click(); let saveRequest=messages.at(-1); - check(saveRequest.action==='saveAs' && saveRequest.pngReady===initialReady,'Save must request native formats and offer PNG only when the image is loaded'); + check(saveRequest.action==='saveAs' && (!saveRequest.pngReady || initialReady),'Save must request native formats and offer PNG only when the image is loaded'); check(save.disabled && !plot.querySelector('[role=menu]'),'Only one native picker may be pending per output'); check(plot.getBoundingClientRect().height===initialHeight && window.scrollY===initialScroll,'Opening Save must preserve output height and scroll position'); reply(saveRequest,{ result:'svg' }); @@ -314,28 +318,28 @@

R Interactive output

check(messages.at(-1).action==='plotPage' && messages.at(-1).selectedPlot==='page1','Navigation must persist the selected page'); check(control(gallery,'Previous plot').disabled && !control(gallery,'Next plot').disabled,'Plot boundaries must disable the corresponding arrow'); check(gallery.getBoundingClientRect().height===galleryHeight,'Changing same-sized pages must not collapse the output while loading'); - await loaded(gallery.querySelector('img')); + await until(()=>gallery.querySelector('img').src.endsWith(pages[0].svgData)); control(gallery,'Open plot').click(); check(messages.at(-1).displayId==='page1','Open must target the selected page'); control(gallery,'Fit R device to cell width').click(); check(messages.at(-1).displayId==='page1' && messages.at(-1).generation==='gallery-generation','Fit must target the selected page and process'); control(gallery,'Save plot as').click(); const gallerySave=messages.at(-1); check(gallerySave.displayId==='page1' && control(gallery,'Next plot').disabled,'Pending Save must freeze navigation to preserve image identity'); reply(gallerySave,{ result:'png' }); check(messages.at(-1).action==='savePng' && messages.at(-1).displayId==='page1' && !control(gallery,'Next plot').disabled,'Save must export the selected page and restore navigation'); - render('gallery-updated',{ ...galleryData,pages:[...pages,{ ...plotData,displayId:'page3' }] },gallery); + render('gallery',{ ...galleryData,pages:[...pages,{ ...plotData,displayId:'page3' }] },gallery); await loaded(gallery.querySelector('img')); check(gallery.querySelector('[data-plot-page]').textContent==='Plot 1 of 3','New frames must not reset a user-selected page'); - render('gallery-offline',{ ...galleryData,connected:false },gallery); await loaded(gallery.querySelector('img')); + render('gallery',{ ...galleryData,connected:false },gallery); await loaded(gallery.querySelector('img')); control(gallery,'Next plot').click(); await loaded(gallery.querySelector('img')); check(gallery.querySelector('[data-plot-page]').textContent==='Plot 2 of 2' && control(gallery,'Save plot as').disabled,'Saved galleries must remain locally pageable'); - render('gallery-stopped',{ ...galleryData,running:false },gallery); + render('gallery',{ ...galleryData,running:false },gallery); control(gallery,'Previous plot').click(); check(gallery.querySelector('[data-plot-page]').textContent==='Plot 1 of 2' && control(gallery,'Fit R device to cell width').disabled && !control(gallery,'Save plot as').disabled,'Stopped galleries must support navigation and export but not device resize'); - render('static-image',{ kind:'plot',displayId:'static-image',mime:'image/svg+xml',imageData:plotData.svgData,resizable:false },gallery); + render('gallery',{ kind:'plot',displayId:'static-image',mime:'image/svg+xml',imageData:plotData.svgData,resizable:false },gallery); await loaded(gallery.querySelector('img')); check(getComputedStyle(control(gallery,'Fit R device to cell width')).display==='none' && !control(gallery,'Save plot as').disabled,'Standard graphics images must be viewable and exportable without offering resize'); - renderer.disposeOutputItem('static-image'); gallery.remove(); + renderer.disposeOutputItem('gallery'); gallery.remove(); const temporary=document.createElement('div'); document.body.append(temporary); - render('temporary',plotData,temporary); + render('temporary',{...plotData,displayId:'replacement'},temporary); const oldTrigger=control(temporary,'Save plot as'); oldTrigger.click(); const oldRequest=messages.at(-1); render('temporary',plotData,temporary); const beforeStale=messages.length; reply(oldRequest,{ result:'svg' }); oldTrigger.click(); @@ -344,6 +348,42 @@

R Interactive output

renderer.disposeOutputItem('temporary'); reply(disposedRequest,{ result:'svg' }); check(messages.at(-1)===disposedRequest,'Disposing output must discard pending format replies'); temporary.remove(); + // Hold actual browser image loads to model a slow asset tunnel. There should + // be only one outstanding request plus the newest queued snapshot. + const slow=document.createElement('div'); document.body.append(slow); + render('slow',plotData,slow); await loaded(slow.querySelector('img')); await new Promise(resolve=>setTimeout(resolve,0)); + const firstImage=slow.querySelector('img'), firstToolbar=slow.querySelector('.r-interactive-toolbar'); + const firstHeight=slow.getBoundingClientRect().height; + const create=document.createElement.bind(document), source=Object.getOwnPropertyDescriptor(HTMLImageElement.prototype,'src'); + const downloads=[]; + document.createElement=(tag,...args)=>{ + const node=create(tag,...args); + if (tag==='img') Object.defineProperty(node,'src',{get(){return source.get.call(this);},set(value){downloads.push({node,value,finish:()=>source.set.call(node,value)});}}); + return node; + }; + try { + const frame=n=>({...plotData,svgData:btoa(String.fromCharCode(...new TextEncoder().encode(svg.replace('retained SVG',`panel ${n}`))))}); + render('slow',frame(2),slow); + for(let n=3;n<=8;n++) render('slow',frame(n),slow); + check(slow.querySelector('img')===firstImage && firstImage.isConnected,'Incoming frames must retain the displayed plot while loading'); + check(slow.querySelector('.r-interactive-toolbar')===firstToolbar && slow.getBoundingClientRect().height===firstHeight,'Live updates must retain the toolbar and output dimensions'); + check(downloads.length===1,`Slow loading must not start a request for every intermediate frame (received ${downloads.length})`); + control(slow,'Save plot as').click(); check(!messages.at(-1).pngReady,'PNG must not export an older snapshot under the new frame identity'); + reply(messages.at(-1),{result:undefined}); + downloads[0].finish(); await until(()=>downloads.length===2); + check(slow.querySelector('img')===firstImage && downloads[1].value.endsWith(frame(8).svgData),'After a slow load, only the newest pending frame should be requested'); + downloads[1].finish(); await until(()=>slow.querySelector('img')===downloads[1].node); + check(slow.querySelector('img').naturalWidth===640,'The final frame must become visible after loading'); + render('slow',frame(8),slow); check(downloads.length===2,'Unchanged final output must not load again'); + render('slow',{...frame(8),url:'http://127.0.0.1:1/unavailable.svg'},slow); + check(downloads.length===2,'Embedded SVG must avoid a second request through a failing asset tunnel'); + render('slow',{...plotData,svgData:undefined,url:'data:image/svg+xml;base64,invalid'},slow); + const failed=downloads.at(-1); failed.finish(); await until(()=>slow.querySelector('[data-status]').textContent.includes('Could not load')); + check(slow.querySelector('img')===downloads[1].node,'A failed replacement must retain the last displayed image'); + render('slow',frame(9),slow); const disposed=downloads.at(-1); + renderer.disposeOutputItem('slow'); disposed.finish(); await new Promise(resolve=>setTimeout(resolve,30)); + check(slow.querySelector('img')===downloads[1].node,'Disposed image loads must not replace the output'); + } finally { document.createElement=create; renderer.disposeOutputItem('slow'); slow.remove(); } const broken=document.createElement('div'); document.body.append(broken); render('broken',{ kind:'plot',svgData:'invalid' },broken); await new Promise(resolve=>broken.querySelector('img').addEventListener('error',resolve,{ once:true })); diff --git a/src/test/node/notebookOutput.test.ts b/src/test/node/notebookOutput.test.ts new file mode 100644 index 000000000..fff15e30d --- /dev/null +++ b/src/test/node/notebookOutput.test.ts @@ -0,0 +1,53 @@ +import * as assert from 'assert'; +import type { NotebookCellExecution, NotebookCellOutput } from 'vscode'; +import { updateNotebookOutputs } from '../../interactive/notebookOutput'; + +const plot = (svg: string, embedded = true): NotebookCellOutput => ({ metadata: { rDisplayId: 'plot-1' }, items: [ + { mime: 'application/vnd.vscode-r.display+json', data: Buffer.from(JSON.stringify({ kind: 'plot', svg })) }, + ...(embedded ? [{ mime: 'image/svg+xml', data: Buffer.from(svg) }] : []) +] }); +const stream = (text: string): NotebookCellOutput => ({ items: [{ mime: 'application/vnd.code.notebook.stdout', data: Buffer.from(text) }] }); + +suite('Interactive notebook output updates', () => { + function execution(outputs: NotebookCellOutput[]): Pick { + return { + replaceOutput: () => assert.fail('Existing plot outputs must not be replaced'), + replaceOutputItems: (items, output) => { + assert.ok(outputs.includes(output), 'Updates must use the existing VS Code output handle'); + output.items = 'mime' in items ? [items] : [...items]; return Promise.resolve(); + }, + appendOutput: added => { outputs.push(...'items' in added ? [added] : added); return Promise.resolve(); } + }; + } + + test('retains plot identity through partial frames, console output and the final frame', async () => { + const initial = plot('panel 1'), outputs = [initial]; + const task = execution(outputs); + await updateNotebookOutputs(task, [...outputs], [plot('panels 1-4'), stream('drawing')]); + await updateNotebookOutputs(task, [...outputs], [plot('panels 1-8'), stream('drawing\ndone')]); + assert.strictEqual(outputs[0], initial); + assert.strictEqual(Buffer.from(outputs[0].items[1].data).toString(), 'panels 1-8'); + assert.strictEqual(Buffer.from(outputs[1].items[0].data).toString(), 'drawing\ndone'); + }); + + test('preserves a growing plot when it exceeds the embedded asset limit', async () => { + const initial = plot('small'), outputs = [initial]; + await updateNotebookOutputs(execution(outputs), [...outputs], [plot('large', false)]); + assert.strictEqual(outputs[0], initial); + assert.strictEqual(outputs[0].items.length, 1); + }); + + test('does not rerender unchanged plots when an execution finishes', async () => { + const outputs = [plot('final')], task = execution(outputs); + task.replaceOutputItems = () => assert.fail('Unchanged images must not reload'); + await updateNotebookOutputs(task, outputs, [plot('final')]); + }); + + test('replaces outputs when their display identity changes', async () => { + const initial = plot('old'), next = plot('new'); next.metadata = { rDisplayId: 'plot-2' }; + const task = execution([initial]); let replaced = false; + task.replaceOutput = outputs => { assert.deepStrictEqual(outputs, [next]); replaced = true; return Promise.resolve(); }; + await updateNotebookOutputs(task, [initial], [next]); + assert.ok(replaced); + }); +}); From a45d7f41893e3b0c293be1949920552ac10b2029 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Tue, 20 Oct 2026 20:56:29 +0800 Subject: [PATCH 2/5] perf: reuse retained Interactive plot assets on refresh --- src/interactive/manager.ts | 15 ++++++++- src/test/suite/interactiveEditor.test.ts | 41 ++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index b1527ba61..62ddac7a3 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -71,6 +71,11 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro private clientId: string; private savedConnections: Map; private outputViews: Map; + // Display events replace their data object. Weak keys release superseded + // snapshots, while console/status updates can reuse encoded plot assets. + private renderedPlots = new WeakMap, { + context: string; output: vscode.NotebookCellOutput; data: Record; + }>(); private status = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, 20); constructor(private context: vscode.ExtensionContext) { @@ -1070,8 +1075,14 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro private async outputs(view: InteractiveView, cell: TranscriptCell, portable = false): Promise { const outputs: vscode.NotebookCellOutput[] = []; const plots: { output: vscode.NotebookCellOutput; data: Record }[] = []; + const plotContext = JSON.stringify([view.base, cell.generation, view.model.generation, view.client.connected, + !['exited', 'stopping'].includes(view.client.manifest.status)]); for (const output of cell.outputs) { const data = output.data; + const cachedPlot = !portable && this.renderedPlots.get(data); + if (cachedPlot && cachedPlot.context === plotContext) { + plots.push(cachedPlot); outputs.push(cachedPlot.output); continue; + } let items: vscode.NotebookCellOutputItem[]; if (output.type === 'stream') { items = [data.channel === 'stderr' ? vscode.NotebookCellOutputItem.stderr(String(data.text)) : vscode.NotebookCellOutputItem.stdout(String(data.text))]; @@ -1127,7 +1138,9 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro } if (data.kind === 'plot' || data.kind === 'image') { const plotOutput = new vscode.NotebookCellOutput(items, { rDisplayId: data.displayId }); - plots.push({ output: plotOutput, data: display }); outputs.push(plotOutput); continue; + const rendered = { output: plotOutput, data: display, context: plotContext }; + if (!portable) { this.renderedPlots.set(data, rendered); } + plots.push(rendered); outputs.push(plotOutput); continue; } } outputs.push(new vscode.NotebookCellOutput(items, { rDisplayId: data.displayId })); diff --git a/src/test/suite/interactiveEditor.test.ts b/src/test/suite/interactiveEditor.test.ts index 7c95ba973..5c0074ad6 100644 --- a/src/test/suite/interactiveEditor.test.ts +++ b/src/test/suite/interactiveEditor.test.ts @@ -1492,6 +1492,47 @@ cat("\n")`; } }); + test('streams an eight-panel plot in place without rereading unchanged assets', async function () { + if (!manifests[0].capabilities.jgd) { this.skip(); } + await vscode.commands.executeCommand('r.interactive.open', manifests[0]); + const notebook = vscode.workspace.notebookDocuments.find(doc => doc.metadata.rSessionId === manifests[0].id); + assert.ok(notebook); + const manager = interactiveManager(bundleContext()) as unknown as { views: Map }; + const view = [...manager.views.values()].find(view => view.client.manifest.id === manifests[0].id); assert.ok(view); + const requests = sinon.spy(view.client, 'request'); + const index = notebook.cellCount; + try { + await vscode.commands.executeCommand('r.runSelection', `local({ + old <- par(mfrow=c(2,4), mar=c(3,3,2,2)); on.exit(par(old)) + set.seed(123) + for (group in 1:2) for (metric in 1:4) { + values <- apply(matrix(rnorm(900), ncol=3), 2, cumsum) + values <- sweep(values, 1, values[,1], "-") + matplot(values, type="n", main=paste("Metric", (group-1)*4+metric)) + abline(v=seq(30,300,by=30), col="gray92") + matlines(values, lty=1, lwd=1.2) + legend("topleft", c("alpha","beta","gamma"), col=1:3, lty=1, bty="n") + Sys.sleep(0.3) + } + for (i in 1:3) { cat("progress", i, "\\n"); Sys.sleep(0.15) } +})`); + const plot = (): vscode.NotebookCellOutput | undefined => notebook.cellCount > index + ? notebook.cellAt(index).outputs.find(output => typeof output.metadata?.rDisplayId === 'string') : undefined; + await until(() => !!plot()); + // VS Code rematerializes the API object on item changes. Its output + // ID is the identity delivered to the notebook renderer. + const outputId = (): string => (plot() as vscode.NotebookCellOutput & { id: string }).id; + const first = outputId(); assert.strictEqual(typeof first, 'string'); + const finalSvg = (): string => Buffer.from(plot()?.items.find(item => item.mime === 'image/svg+xml')?.data ?? []).toString(); + await until(() => notebook.cellAt(index).executionSummary?.success === true && finalSvg().includes('Metric 8')); + assert.strictEqual(outputId(), first, 'All partial and final frames must retain the renderer output identity'); + for (let panel = 1; panel <= 8; panel++) { assert.match(finalSvg(), new RegExp(`Metric ${panel}`)); } + const assets = requests.getCalls().filter(call => call.args[0] === 'asset').map(call => String(call.args[1]?.id)); + assert.ok(assets.length >= 2, 'Exercise multiple partial snapshots'); + assert.strictEqual(assets.length, new Set(assets).size, 'Console and completion updates must reuse encoded plot assets'); + } finally { requests.restore(); } + }); + test('groups two four-panel plot pages and exports every page without transient URLs', async function () { if (!manifests[0].capabilities.jgd) { this.skip(); } const notebook = vscode.workspace.notebookDocuments.find(doc => doc.metadata.rSessionId === manifests[0].id); From 123669a3b05985cabbce93fff89560c1ed2caa06 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Tue, 20 Oct 2026 21:29:15 +0800 Subject: [PATCH 3/5] fix: include Interactive output delivery in execution timing Process runtime events independently of coalesced notebook writes so slow output delivery cannot queue obsolete snapshots ahead of completion. Finish the visible execution timer after the final output update and retain that timing across reconnects. --- src/interactive/manager.ts | 122 ++++++++++++++--------- src/interactive/outputQueue.ts | 53 ++++++++++ src/test/node/outputQueue.test.ts | 60 +++++++++++ src/test/suite/interactiveEditor.test.ts | 60 +++++++++++ 4 files changed, 247 insertions(+), 48 deletions(-) create mode 100644 src/interactive/outputQueue.ts create mode 100644 src/test/node/outputQueue.test.ts diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 62ddac7a3..500e5740b 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -6,6 +6,7 @@ import { AgentClient } from './client'; import { AgentConfig, AgentSnapshot, DEFAULT_MAX_ASSET_BYTES, ExecutionRecord, SessionEvent, SessionManifest, SourceLocation, object, sessionLabel } from './protocol'; import { AssetStore, AssetStorageStats, exportedAssetName, readAsset } from './assets'; import { updateNotebookOutputs } from './notebookOutput'; +import { OutputQueue } from './outputQueue'; import { defaultStorage, discoverSessions, hasSessionEndpoint, launchAgent, newIdentity, prepareNodeRuntime } from './launcher'; import { NodeRuntime, resolveNodeRuntime } from './nodeExecutable'; import { backendDescriptor, prepareBackendRuntime } from './backendRegistry'; @@ -37,10 +38,9 @@ interface InteractiveView { controller: vscode.NotebookController; controllers: vscode.NotebookController[]; inputUri?: vscode.Uri; - executions: Map; + executions: Map; chain: Promise; - pending: Set; - timer?: NodeJS.Timeout; + outputQueue: OutputQueue; reconnectTimer?: NodeJS.Timeout; disposed: boolean; prompts: Set; @@ -71,6 +71,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro private clientId: string; private savedConnections: Map; private outputViews: Map; + private executionTimings: Map; // Display events replace their data object. Weak keys release superseded // snapshots, while console/status updates can reuse encoded plot assets. private renderedPlots = new WeakMap, { @@ -90,6 +91,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro context.workspaceState.get('r.interactive.sessions', []).map(id => ({ id })); this.savedConnections = new Map(saved.map(connection => [connection.id, connection])); this.outputViews = new Map(context.workspaceState.get('r.interactive.outputViews', [])); + this.executionTimings = new Map(context.workspaceState.get('r.interactive.executionTimings', [])); const command = (name: string, handler: (...args: unknown[]) => unknown): void => { this.disposables.push(vscode.commands.registerCommand(name, (...args: unknown[]) => Promise.resolve().then(() => handler(...args)).catch((error: unknown) => this.report(error)))); @@ -188,9 +190,9 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro vscode.workspace.onDidChangeConfiguration(event => { if (event.affectsConfiguration('r.interactive.tableView')) { for (const view of this.views.values()) { - view.chain = view.chain.then(async () => { + view.chain = view.chain.then(() => { for (const cell of this.transcriptCells(view)) { - if (cell.outputs.some(output => output.data.kind === 'table')) { await this.render(view, cell.record.id); } + if (cell.outputs.some(output => output.data.kind === 'table')) { view.outputQueue.schedule(cell.record.id); } } }).catch(error => this.report(error)); } @@ -375,20 +377,23 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro private async clearCompleted(view: InteractiveView): Promise { view.chain = view.chain.then(async () => { if (view.disposed) { return; } - const edits: vscode.NotebookEdit[] = []; - for (const cell of view.notebook.getCells().slice().reverse()) { - const id = cell.metadata.rExecutionId as string | undefined; - const record = id && this.transcriptCells(view).find(cell => cell.record.id === id)?.record; - if (record && !['queued', 'running', 'unknown'].includes(record.state)) { - view.hidden.add(record.id); - const adjacent = edits.at(-1); - if (adjacent?.range.start === cell.index + 1) { - edits[edits.length - 1] = vscode.NotebookEdit.deleteCells(new vscode.NotebookRange(cell.index, adjacent.range.end)); - } else { edits.push(vscode.NotebookEdit.deleteCells(new vscode.NotebookRange(cell.index, cell.index + 1))); } + await view.outputQueue.pause(); + try { + const edits: vscode.NotebookEdit[] = []; + for (const cell of view.notebook.getCells().slice().reverse()) { + const id = cell.metadata.rExecutionId as string | undefined; + const record = id && this.transcriptCells(view).find(cell => cell.record.id === id)?.record; + if (record && !view.executions.has(record.id) && !['queued', 'running', 'unknown'].includes(record.state)) { + view.hidden.add(record.id); + const adjacent = edits.at(-1); + if (adjacent?.range.start === cell.index + 1) { + edits[edits.length - 1] = vscode.NotebookEdit.deleteCells(new vscode.NotebookRange(cell.index, adjacent.range.end)); + } else { edits.push(vscode.NotebookEdit.deleteCells(new vscode.NotebookRange(cell.index, cell.index + 1))); } + } } - } - const edit = new vscode.WorkspaceEdit(); edit.set(view.notebook.uri, edits); - await vscode.workspace.applyEdit(edit); + const edit = new vscode.WorkspaceEdit(); edit.set(view.notebook.uri, edits); + await vscode.workspace.applyEdit(edit); + } finally { view.outputQueue.resume(); } }); await view.chain; } @@ -743,7 +748,8 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro session.bindSessionDocument(vscode.Uri.from({ scheme: 'vscode-notebook-cell', path: notebook.uri.path }), target); const view: InteractiveView = { client, target, model: new Transcript(manifest.generation), history: this.readHistory(manifest), notebook, controller, controllers: [native, fallback], inputUri, executions: new Map(), chain: Promise.resolve(), - pending: new Set(), disposed: false, prompts: new Set(), base: '', hidden: new Set() }; + outputQueue: new OutputQueue(id => this.render(view, id), error => this.report(error)), + disposed: false, prompts: new Set(), base: '', hidden: new Set() }; this.views.set(key, view); controller.updateNotebookAffinity(notebook, vscode.NotebookControllerAffinity.Preferred); // A restored native tab may still have the previous generation's kernel selected. @@ -831,6 +837,14 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro } private async restore(view: InteractiveView, snapshot: AgentSnapshot, preserveDrafts = true): Promise { + // Wait only for the current notebook write. A reconnected R process may + // keep streaming indefinitely; suspend pending writes during restore. + await view.outputQueue.pause(); + try { await this.restoreSnapshot(view, snapshot, preserveDrafts); } + finally { view.outputQueue.resume(); } + } + + private async restoreSnapshot(view: InteractiveView, snapshot: AgentSnapshot, preserveDrafts: boolean): Promise { const previousCells = preserveDrafts ? view.notebook.getCells() : []; for (const { task } of view.executions.values()) { task.end(undefined); } view.executions.clear(); @@ -973,7 +987,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro if (event.data.status === 'exited' && !view.restarting) { await this.sessionNotice(view, 'stopped'); for (const cell of this.transcriptCells(view)) { - if (cell.outputs.some(output => output.type === 'display')) { await this.render(view, cell.record.id); } + if (cell.outputs.some(output => output.type === 'display')) { view.outputQueue.schedule(cell.record.id); } } } } else if (event.type === 'session' && typeof event.data.label === 'string') { @@ -1036,29 +1050,10 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro } if (execution) { execution.task.executionOrder = Number(event.data.order); - if (!execution.started) { execution.task.start(event.time); execution.started = true; } - } - } - if (event.type === 'finished' || event.type === 'uncertain') { - view.pending.delete(changed); - try { await this.render(view, changed); } - finally { - const execution = view.executions.get(changed); - if (execution) { - execution.task.end(event.data.state === 'success' ? true : event.data.state === 'error' ? false : undefined, event.time); - view.executions.delete(changed); - } - } - } else { - view.pending.add(changed); - if (!view.timer) { - view.timer = setTimeout(() => { - view.timer = undefined; - const ids = [...view.pending]; view.pending.clear(); - view.chain = view.chain.then(async () => { for (const id of ids) { await this.render(view, id); } }).catch(error => this.report(error)); - }, 40); + if (!execution.started) { execution.startTime = event.time; execution.task.start(event.time); execution.started = true; } } } + view.outputQueue.schedule(changed, event.type === 'finished' || event.type === 'uncertain'); } private async cellData(view: InteractiveView, cell: TranscriptCell, portable = false): Promise { @@ -1067,7 +1062,8 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro rSource: cell.record.source, rState: cell.record.state, execution_count: cell.record.order }, 'r'); data.executionSummary = { executionOrder: cell.record.order, success: cell.record.state === 'success' ? true : cell.record.state === 'error' ? false : undefined, - timing: cell.record.started && cell.record.ended ? { startTime: cell.record.started, endTime: cell.record.ended } : undefined }; + timing: this.executionTimings.get(`${view.client.manifest.id}:${cell.generation}:${cell.record.id}`) + ?? (cell.record.started && cell.record.ended ? { startTime: cell.record.started, endTime: cell.record.ended } : undefined) }; data.outputs = await this.outputs(view, cell, portable); return data; } @@ -1164,21 +1160,50 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro } private async render(view: InteractiveView, id: string): Promise { + try { await this.renderOutputs(view, id); } + finally { + const execution = view.executions.get(id), record = view.model.cells.get(id)?.record; + if (!view.disposed && !view.outputQueue.has(id) && execution && record && !['queued', 'running'].includes(record.state)) { + // The live timer includes output delivery. End it when the last + // notebook update completes, rather than backdating it to R's + // earlier completion event. Keep the journal's R timing intact. + const endTime = Math.max(execution.startTime ?? 0, Date.now()); + execution.task.end(record.state === 'success' ? true : record.state === 'error' ? false : undefined, endTime); + view.executions.delete(id); + if (execution.startTime !== undefined) { + const key = `${view.client.manifest.id}:${view.model.generation}:${id}`; + this.executionTimings.delete(key); + this.executionTimings.set(key, { startTime: execution.startTime, endTime }); + while (this.executionTimings.size > 1000) { this.executionTimings.delete(this.executionTimings.keys().next().value as string); } + void this.context.workspaceState.update('r.interactive.executionTimings', [...this.executionTimings]).then(undefined, error => this.report(error)); + } + } + } + } + + private async renderOutputs(view: InteractiveView, id: string): Promise { if (view.disposed || view.notebook.isClosed) { return; } const data = this.transcriptCells(view).find(cell => cell.record.id === id); if (!data || !this.cell(view, id)) { return; } // Admission can precede R's start event by an arbitrary queue/startup delay. // Keep VS Code's execution pending; output mutation requires start(). if (data.record.state === 'queued' || (view.executions.get(id)?.started === false && !data.outputs.length)) { return; } - const outputs = await this.outputs(view, data); + // Rendering may await asset reads while the transcript receives newer + // events. Snapshot mutable streams and defer obsolete output writes. + const snapshot = { ...data, outputs: data.outputs.map(output => ({ ...output, + data: output.type === 'stream' ? { ...output.data } : output.data })) }; + const outputs = await this.outputs(view, snapshot); // Asset reads can outlive detach or reconnection. Re-read the live handle. - if (view.disposed || view.notebook.isClosed) { return; } + if (view.disposed || view.notebook.isClosed || view.outputQueue.has(id)) { return; } const cell = this.cell(view, id); if (!cell) { return; } const execution = view.executions.get(id); if (execution) { // A dispatch failure may have diagnostics but no R start event. - if (!execution.started) { execution.task.start(data.record.started); execution.started = true; } + if (!execution.started) { + execution.startTime = snapshot.record.started ?? Date.now(); + execution.task.start(execution.startTime); execution.started = true; + } await updateNotebookOutputs(execution.task, cell.outputs, outputs); } else { @@ -1501,7 +1526,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro throw new Error('Reconnect to the R session before restarting it.'); } await view.chain; - clearTimeout(view.timer); view.timer = undefined; view.pending.clear(); + await view.outputQueue.flush(); for (const { task } of view.executions.values()) { task.end(undefined); } view.executions.clear(); view.client.close(); @@ -1543,8 +1568,9 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro } // Refresh asset URLs and disable process-specific controls without replacing old cells. for (const cell of this.transcriptCells(view)) { - if (cell.outputs.some(output => output.type === 'display')) { await this.render(view, cell.record.id); } + if (cell.outputs.some(output => output.type === 'display')) { view.outputQueue.schedule(cell.record.id); } } + await view.outputQueue.flush(); if (snapshot.manifest.status === 'exited') { throw new Error('The replacement R process exited during startup'); } if (!await view.client.subscribe(view.model.seq)) { await this.restore(view, await view.client.snapshot()); await view.client.subscribe(view.model.seq); } if (this.active === view) { await this.activate(view); } @@ -1624,7 +1650,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro private disposeView(view: InteractiveView, forget = true): void { if (view.disposed) { return; } - view.disposed = true; clearTimeout(view.timer); clearTimeout(view.reconnectTimer); + view.disposed = true; view.outputQueue.dispose(); clearTimeout(view.reconnectTimer); view.client.close(); for (const { task } of view.executions.values()) { task.end(undefined); } view.executions.clear(); for (const controller of view.controllers) { controller.dispose(); } diff --git a/src/interactive/outputQueue.ts b/src/interactive/outputQueue.ts new file mode 100644 index 000000000..b800165ca --- /dev/null +++ b/src/interactive/outputQueue.ts @@ -0,0 +1,53 @@ +/** Serialize notebook writes without blocking incoming runtime events. */ +export class OutputQueue { + private pending = new Set(); + private timer?: NodeJS.Timeout; + private running?: Promise; + private disposed = false; + private paused = 0; + + constructor(private update: (id: string) => Promise, private report: (error: unknown) => void) {} + + schedule(id: string, immediate = false): void { + if (this.disposed) { return; } + this.pending.add(id); + if (immediate) { clearTimeout(this.timer); this.timer = undefined; void this.start(); } + else if (!this.paused && !this.running && !this.timer) { + this.timer = setTimeout(() => { this.timer = undefined; void this.start(); }, 40); + } + } + + has(id: string): boolean { return this.pending.has(id); } + + async flush(): Promise { + clearTimeout(this.timer); this.timer = undefined; + while (this.running || (!this.paused && this.pending.size)) { await this.start(); } + } + + async pause(): Promise { + this.paused++; clearTimeout(this.timer); this.timer = undefined; + await this.running; + } + + resume(): void { if (this.paused) { this.paused--; } void this.start(); } + + dispose(): void { + this.disposed = true; this.pending.clear(); clearTimeout(this.timer); this.timer = undefined; + } + + private start(): Promise { + if (this.running) { return this.running; } + if (this.disposed || this.paused || !this.pending.size) { return Promise.resolve(); } + this.running = Promise.resolve().then(async () => { + while (!this.disposed && !this.paused && this.pending.size) { + const id = this.pending.values().next().value as string; + this.pending.delete(id); + try { await this.update(id); } catch (error) { this.report(error); } + } + }).finally(() => { + this.running = undefined; + if (!this.paused && this.pending.size) { void this.start(); } + }); + return this.running; + } +} diff --git a/src/test/node/outputQueue.test.ts b/src/test/node/outputQueue.test.ts new file mode 100644 index 000000000..6fc69c3c1 --- /dev/null +++ b/src/test/node/outputQueue.test.ts @@ -0,0 +1,60 @@ +import * as assert from 'assert'; +import { OutputQueue } from '../../interactive/outputQueue'; + +suite('Interactive output queue', () => { + test('coalesces updates received during a slow write and retains other cells', async () => { + let release!: () => void; + const blocked = new Promise(resolve => { release = resolve; }); + const seen: string[] = []; + let frame = 1; + const queue = new OutputQueue(async id => { + seen.push(`${id}:${frame}`); + if (seen.length === 1) { await blocked; } + }, error => { throw error; }); + queue.schedule('plot', true); await Promise.resolve(); + for (frame = 2; frame <= 8; frame++) { queue.schedule('plot'); } + frame = 8; queue.schedule('console'); + assert.deepStrictEqual(seen, ['plot:1'], 'Only one output write may be in flight'); + release(); await queue.flush(); + assert.deepStrictEqual(seen, ['plot:1', 'plot:8', 'console:8']); + queue.dispose(); + }); + + test('one failed output does not prevent the final update or another cell', async () => { + const errors: unknown[] = [], seen: string[] = []; + let attempts = 0; + const queue = new OutputQueue(id => { + if (!attempts++) { queue.schedule(id); return Promise.reject(new Error('Output transfer failed')); } + seen.push(id); return Promise.resolve(); + }, error => errors.push(error)); + queue.schedule('plot'); queue.schedule('console'); await queue.flush(); + assert.strictEqual(errors.length, 1); + assert.deepStrictEqual(seen, ['console', 'plot']); + queue.dispose(); + }); + + test('pauses after the current write so restoration does not wait for a streaming session to become idle', async () => { + let release!: () => void; + const blocked = new Promise(resolve => { release = resolve; }); + const seen: string[] = []; + const queue = new OutputQueue(async id => { seen.push(id); await blocked; }, error => { throw error; }); + queue.schedule('old', true); await Promise.resolve(); + const paused = queue.pause(); queue.schedule('pending', true); + release(); await paused; + assert.deepStrictEqual(seen, ['old']); + queue.resume(); await queue.flush(); + assert.deepStrictEqual(seen, ['old', 'pending']); + queue.dispose(); + }); + + test('disposing during a write discards pending output and ignores new work', async () => { + let release!: () => void; + const blocked = new Promise(resolve => { release = resolve; }); + const seen: string[] = []; + const queue = new OutputQueue(async id => { seen.push(id); await blocked; }, error => { throw error; }); + queue.schedule('old', true); await Promise.resolve(); + queue.schedule('pending'); queue.dispose(); queue.schedule('new'); + release(); await queue.flush(); + assert.deepStrictEqual(seen, ['old']); + }); +}); diff --git a/src/test/suite/interactiveEditor.test.ts b/src/test/suite/interactiveEditor.test.ts index 5c0074ad6..5f2879fc4 100644 --- a/src/test/suite/interactiveEditor.test.ts +++ b/src/test/suite/interactiveEditor.test.ts @@ -13,6 +13,8 @@ import { SessionAgent } from '../../interactive/agentMain'; import { AgentClient } from '../../interactive/client'; import { InteractiveSerializer, DISPLAY_MIME } from '../../interactive/notebook'; import { AgentConfig, SessionManifest } from '../../interactive/protocol'; +import type { SessionEvent } from '../../interactive/protocol'; +import type { Transcript, TranscriptCell } from '../../interactive/transcript'; import { AssetStorageStats, exportedAssetName, readAsset } from '../../interactive/assets'; import { resolveExecutable } from '../../interactive/executable'; import { shellQuote } from '../../interactive/launcher'; @@ -1492,6 +1494,64 @@ cat("\n")`; } }); + test('continues processing runtime events while a plot update is delayed', async function () { + if (!manifests[0].capabilities.jgd) { this.skip(); } + await vscode.commands.executeCommand('r.interactive.open', manifests[0]); + const manager = interactiveManager(bundleContext()) as unknown as { + views: Map; outputQueue: { flush(): Promise } }>; + outputs(view: unknown, cell: TranscriptCell): Promise; + }; + const view = [...manager.views.values()].find(view => view.client.manifest.id === manifests[0].id); assert.ok(view); + let release!: () => void; + const delayed = new Promise(resolve => { release = resolve; }); + let executionId: string | undefined, completed: SessionEvent | undefined; + const original = manager.outputs.bind(manager); + const output = sinon.stub(manager, 'outputs').callsFake(async (current, cell) => { + if (current === view && cell.record.code.includes('delayed_plot_update') && cell.outputs.some(item => item.data.kind === 'plot') && !executionId) { + executionId = cell.record.id; + await delayed; + } + return original(current, cell); + }); + const listener = (event: SessionEvent): void => { + if (event.executionId === executionId && event.type === 'finished') { completed = event; } + }; + view.client.on('event', listener); + let observed: string | undefined, released = 0; + try { + await vscode.commands.executeCommand('r.runSelection', 'delayed_plot_update <- TRUE; plot(1:3); Sys.sleep(0.7); title("final delayed plot"); cat("tail after plot\\n")'); + await until(() => !!completed, 'The R execution must finish while the output update is still pending'); + await new Promise(resolve => setTimeout(resolve, 350)); + observed = view.model.cells.get(executionId!)?.record.state; + } finally { + released = Date.now(); + release(); view.client.off('event', listener); + await view.chain; + await view.outputQueue.flush(); + output.restore(); + } + assert.strictEqual(observed, 'success', 'Output delivery must not block processing the runtime completion event'); + const notebook = vscode.workspace.notebookDocuments.find(doc => doc.metadata.rSessionId === manifests[0].id); assert.ok(notebook); + const cell = notebook.getCells().find(cell => cell.metadata.rExecutionId === executionId); assert.ok(cell); + await until(() => cell.executionSummary?.success === true); + const timing = cell.executionSummary?.timing; assert.ok(timing); + assert.ok(timing.endTime >= released, 'The completed timer must include the delayed output delivery'); + assert.ok(completed && timing.endTime - completed.time >= 300, 'Completion must not backdate the live timer to R finishing'); + const finalSvg = cell.outputs.flatMap(output => output.items).find(item => item.mime === 'image/svg+xml'); assert.ok(finalSvg); + assert.match(Buffer.from(finalSvg.data).toString(), /final delayed plot/); + const text = cell.outputs.flatMap(output => output.items).filter(item => item.mime === 'application/vnd.code.notebook.stdout') + .map(item => Buffer.from(item.data).toString()).join(''); + assert.strictEqual(text, 'tail after plot\n', 'Events arriving during the delayed update must retain their console output'); + const key = `${manifests[0].id}:${view.model.generation}:${executionId!}`; + const saved = new Map(bundleContext().workspaceState.get<[string, typeof timing][]>('r.interactive.executionTimings', [])); + assert.deepStrictEqual(saved.get(key), timing, 'Total timing must be saved for reconnect and restore'); + await vscode.commands.executeCommand('r.interactive.detach', notebook.uri); + await vscode.commands.executeCommand('r.interactive.open', manifests[0]); + const restored = vscode.workspace.notebookDocuments.find(doc => doc.metadata.rSessionId === manifests[0].id)?.getCells() + .find(cell => cell.metadata.rExecutionId === executionId); + assert.deepStrictEqual(restored?.executionSummary?.timing, timing, 'Reconnecting must preserve the displayed duration'); + }); + test('streams an eight-panel plot in place without rereading unchanged assets', async function () { if (!manifests[0].capabilities.jgd) { this.skip(); } await vscode.commands.executeCommand('r.interactive.open', manifests[0]); From f88c771ae3f2c897a3c736e697e90b7683068f1d Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Tue, 20 Oct 2026 21:47:45 +0800 Subject: [PATCH 4/5] fix: keep export-name lint suppression beside its regex --- src/interactive/manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 0b74c389f..3e1969ea3 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -974,9 +974,9 @@ export class InteractiveManager } private defaultExportUri(view: InteractiveView, suffix: string): vscode.Uri { - // eslint-disable-next-line no-control-regex -- export names must exclude control characters const name = view.client.manifest.label + // eslint-disable-next-line no-control-regex -- export names must exclude control characters .replace(/[<>:"/\\|?*\x00-\x1f]/g, '-') .replace(/[. ]+$/, '') || 'R'; return vscode.Uri.file(path.join(view.target.workingDir, `${name}-${suffix}`)); From 993827b3f4cd9a2897a5d55d5abffe13c77554c2 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Tue, 20 Oct 2026 23:51:25 +0800 Subject: [PATCH 5/5] fix: serialize Interactive timing persistence Coalesce timing snapshots while storage is busy without blocking completed cells. Add native regressions for delayed persistence and for reporting final output failures while preserving the R evaluation result and releasing the execution handle. --- src/interactive/manager.ts | 24 +++- src/interactive/outputQueue.ts | 2 +- src/test/suite/interactiveEditor.test.ts | 166 +++++++++++++++++++++++ 3 files changed, 188 insertions(+), 4 deletions(-) diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 3e1969ea3..663b6b252 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -111,6 +111,7 @@ export class InteractiveManager { tableView?: string; listView?: string; selectedPlot?: string } >; private executionTimings: Map; + private timingWrites: OutputQueue; // Display events replace their data object. Weak keys release superseded // snapshots, while console/status updates can reuse encoded plot assets. private renderedPlots = new WeakMap< @@ -149,6 +150,22 @@ export class InteractiveManager this.executionTimings = new Map( context.workspaceState.get('r.interactive.executionTimings', []), ); + this.timingWrites = new OutputQueue( + async () => { + // Snapshot when the write starts, coalescing completions that + // arrived while the preceding storage update was in flight. + await context.workspaceState.update('r.interactive.executionTimings', [ + ...this.executionTimings, + ]); + }, + (error) => { + if (this.closed) { + console.error('R Interactive: could not save execution timings', error); + } else { + this.report(error); + } + }, + ); const command = (name: string, handler: (...args: unknown[]) => unknown): void => { this.disposables.push( vscode.commands.registerCommand(name, (...args: unknown[]) => @@ -2292,6 +2309,8 @@ export class InteractiveManager try { await this.renderOutputs(view, id); } finally { + // OutputQueue reports delivery failures separately. Completion + // still reflects R's result and must release the execution handle. const execution = view.executions.get(id), record = view.model.cells.get(id)?.record; if ( @@ -2323,9 +2342,7 @@ export class InteractiveManager this.executionTimings.keys().next().value as string, ); } - void this.context.workspaceState - .update('r.interactive.executionTimings', [...this.executionTimings]) - .then(undefined, (error) => this.report(error)); + this.timingWrites.schedule('executionTimings', true); } } } @@ -3417,6 +3434,7 @@ export class InteractiveManager dispose(): void { this.closed = true; + // Already scheduled timing writes may drain after the views close. setInteractiveExecutor(undefined); for (const view of this.views.values()) { this.disposeView(view); diff --git a/src/interactive/outputQueue.ts b/src/interactive/outputQueue.ts index 98e696bc5..f3c144f96 100644 --- a/src/interactive/outputQueue.ts +++ b/src/interactive/outputQueue.ts @@ -1,4 +1,4 @@ -/** Serialize notebook writes without blocking incoming runtime events. */ +/** Coalesce output-related work without blocking incoming runtime events. */ export class OutputQueue { private pending = new Set(); private timer?: NodeJS.Timeout; diff --git a/src/test/suite/interactiveEditor.test.ts b/src/test/suite/interactiveEditor.test.ts index ed6e410f4..5e6197d81 100644 --- a/src/test/suite/interactiveEditor.test.ts +++ b/src/test/suite/interactiveEditor.test.ts @@ -2794,6 +2794,172 @@ cat("\n")`; } }); + test('coalesces timing storage writes without blocking completed cells', async () => { + await vscode.commands.executeCommand('r.interactive.open', manifests[0]); + const context = bundleContext(); + const manager = interactiveManager(context) as unknown as { + timingWrites?: { flush(): Promise }; + }; + await manager.timingWrites?.flush(); + const notebook = vscode.workspace.notebookDocuments.find( + (doc) => doc.metadata.rSessionId === manifests[0].id, + ); + assert.ok(notebook); + type Timings = [string, { startTime: number; endTime: number }][]; + const snapshots: Timings[] = [], + keys: string[] = []; + let release!: () => void; + const delayed = new Promise((resolve) => { + release = resolve; + }); + const original = context.workspaceState.update.bind(context.workspaceState); + const storage = sinon + .stub(context.workspaceState, 'update') + .callsFake(async (key, value) => { + if (key === 'r.interactive.executionTimings') { + snapshots.push(value as Timings); + if (snapshots.length === 1) { + await delayed; + } + } + await original(key, value); + }); + try { + for (let index = 0; index < 3; index++) { + const position = notebook.cellCount; + await vscode.commands.executeCommand( + 'r.runSelection', + `timing_storage_cell <- ${index}`, + ); + await until( + () => + notebook.cellCount > position && + notebook.cellAt(position).executionSummary?.success === true, + ); + const cell = notebook.cellAt(position); + keys.push( + `${manifests[0].id}:${manifests[0].generation}:${String(cell.metadata.rExecutionId)}`, + ); + } + assert.strictEqual( + snapshots.length, + 1, + 'Timing writes must not overlap while cells continue completing', + ); + } finally { + release(); + await manager.timingWrites?.flush(); + await Promise.all(storage.returnValues); + storage.restore(); + } + assert.strictEqual( + snapshots.length, + 2, + 'The next write must coalesce intervening completions', + ); + const saved = new Map( + context.workspaceState.get('r.interactive.executionTimings', []), + ); + for (const key of keys) { + assert.ok(saved.has(key), `Missing completed execution ${key}`); + } + }); + + test('reports final output failures while preserving R success and allowing the next cell', async () => { + await vscode.commands.executeCommand('r.interactive.open', manifests[0]); + const manager = interactiveManager() as unknown as { + views: Map< + string, + { + client: AgentClient; + model: Transcript; + executions: Map; + outputQueue: { flush(): Promise }; + } + >; + renderOutputs(view: unknown, id: string): Promise; + }; + const view = [...manager.views.values()].find( + (view) => view.client.manifest.id === manifests[0].id, + ); + assert.ok(view); + const notebook = vscode.workspace.notebookDocuments.find( + (doc) => doc.metadata.rSessionId === manifests[0].id, + ); + assert.ok(notebook); + const original = manager.renderOutputs.bind(manager); + const failure = new Error('Final notebook output could not be delivered'); + const errors = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + let failedId: string | undefined; + const render = sinon.stub(manager, 'renderOutputs').callsFake((current, id) => { + const record = view.model.cells.get(id)?.record; + if ( + current === view && + record?.code.startsWith('final_output_failure_runs <-') && + record.state === 'success' + ) { + failedId = id; + return Promise.reject(failure); + } + return original(current, id); + }); + try { + const position = notebook.cellCount; + await vscode.commands.executeCommand( + 'r.runSelection', + 'final_output_failure_runs <- get0("final_output_failure_runs", ifnotfound=0L) + 1L; cat("before failure\\n"); Sys.sleep(0.4); cat("final output\\n")', + ); + await until( + () => + notebook.cellCount > position && + notebook.cellAt(position).executionSummary?.success === true && + errors.called, + ); + await view.outputQueue.flush(); + const cell = notebook.cellAt(position), + id = String(cell.metadata.rExecutionId); + assert.strictEqual(failedId, id, 'Exercise the final output update'); + assert.strictEqual( + view.model.cells.get(id)?.record.state, + 'success', + 'The evaluation result must remain R success', + ); + assert.ok( + !view.executions.has(id), + 'A failed output update must release the execution handle', + ); + assert.ok( + cell.executionSummary?.timing, + 'Failed delivery must still finish the elapsed timer', + ); + assert.ok( + errors.getCalls().some((call) => String(call.args[0]).includes(failure.message)), + 'Delivery failures must be reported', + ); + const next = notebook.cellCount; + await vscode.commands.executeCommand( + 'r.runSelection', + 'cat("runs=", final_output_failure_runs, sep="")', + ); + await until( + () => + notebook.cellCount > next && + notebook.cellAt(next).executionSummary?.success === true, + ); + const text = notebook + .cellAt(next) + .outputs.flatMap((output) => output.items) + .filter((item) => item.mime === 'application/vnd.code.notebook.stdout') + .map((item) => Buffer.from(item.data).toString()) + .join(''); + assert.strictEqual(text, 'runs=1', 'Recovery must not rerun successful R code'); + } finally { + await view.outputQueue.flush(); + render.restore(); + errors.restore(); + } + }); + test('continues processing runtime events while a plot update is delayed', async function () { if (!manifests[0].capabilities.jgd) { this.skip();