diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a4a2f7..8b35e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) - **On-disk storage — failed writes:** when the browser refused to write an object to disk (storage quota exceeded, write error), the object lost its data: it had already been removed from memory. The object now stays in memory with its data, and the error is reported. An interrupted write no longer replaces the previous on-disk copy either. - **On-disk storage — incomplete transfers:** an object whose data the browser wrote to disk only partly was considered stored and removed from memory, so that it was later read back with missing values; a partial read also gave missing values without error. Incomplete writes now fail like other write errors, so that the object keeps its data in memory, and incomplete reads are reported as errors. - **On-disk storage — several tabs:** with DataLab-Web open in two tabs in on-disk storage mode, switching one tab back to memory storage or replacing its workspace deleted the data stored on disk by the other tab, whose objects could then no longer be displayed or processed. Each tab now keeps its data in its own storage area; the areas of closed tabs are reclaimed when the on-disk mode is next enabled. +- **On-disk storage — enabling the mode:** when **Store data on disk** (in the menu of the memory indicator) could not be enabled (storage refused by the browser, as in the private windows of some browsers, or data that could not be written), DataLab-Web was left halfway: the toggle still showed memory storage, some objects were kept on disk, later operations were reported as failed although they had been applied, and a further attempt reported the mode as enabled while nothing was stored on disk. A failed attempt now leaves every object in memory, as before, and the mode can be enabled again once the cause is fixed. +- **On-disk storage — operations reported as failed:** in on-disk storage mode, when the browser refused to write the result of an operation to disk (storage quota exceeded, write error), the operation was reported as failed although it had been applied, so that running it again applied it twice. The operation now succeeds: the objects that could not be written stay in memory with their data, a warning reports the storage error once, and these objects are stored on disk by a later operation when possible. - **HDF5 import — complex and integer data:** single-precision complex signals (`complex64`) lost their imaginary part, and unsigned 32-bit images were clipped to 2 147 483 647. Complex data is now imported as complex objects and integer data keeps its values exactly (images with large values become 64-bit floating-point images); a warning now reports integers beyond 2⁵³, which cannot be represented exactly. Previews show the real part of complex data. - **HDF5 browser — broken links:** files containing a link to a missing object or to a missing file (e.g. synchrotron master files without their detector files) could not be opened at all. Broken links are now skipped and the rest of the file can be browsed and imported. - **HDF5 browser — link cycles:** files containing a group linked into itself or into one of its subgroups were browsed down to a hundred phantom levels, or took too long to open. Such links back to a parent group are now skipped; other aliases are still shown at each of their paths. @@ -26,7 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) - **Image viewer — pixel positions:** images were drawn half a pixel off their coordinates: the first pixel started at the image origin instead of being centered on it, as in Sigima and DataLab desktop. Regions of interest, analysis results, the profile crosshair and the statistics rectangle therefore appeared shifted by half a pixel relative to the computed pixels, and the value shown when hovering the image could be that of the neighbouring pixel. Each pixel is now centered on its coordinates in the image viewer, the multi-image views and the processing preview, so overlays, read-outs, cross-sections and statistics match the computations exactly. The default regions of interest and areas to erase are centered on the image, and the preview of _Create ROI grid…_ now shows the same whole-pixel cells as the created regions of interest. Cross-sections and statistics also work on images with non-uniform coordinates. - **Image viewer — blurred pixels when zooming in:** since version 0.6.1, zooming in on an image showed blurred pixels, each one fading into its neighbours, so that close values could not be told apart. Zoomed-in pixels are now drawn as sharp squares (nearest pixel, as in DataLab desktop) in the image viewer, the _Spatial_ multi-image view and the processing preview, in every browser and in the images exported from the plot. Zoomed-out images are displayed as before. - **AI Assistant — Save to Macros:** since version 0.10.0, clicking **Save to Macros** under a macro proposed by the AI Assistant showed "Saved ✓", but the macro did not appear in the Macros panel; each click added another hidden macro to the workspace. The saved macro now opens in the Macros panel, which is brought to the front, including from a conversation reopened from the history. -- **AI Assistant — macros written as text:** asked to write a macro, the assistant could answer with the code in its reply, without any **Save to Macros** button, in particular for curves its creation tools cannot produce (super-Gaussian, Lorentzian, polynomial…), which it was told to create without a macro. The assistant now proposes such macros through its macro tool, without running them unless asked (the approval dialog says so, and approving such a macro never auto-approves a macro that runs), and a **Save to Macros** button is also offered under each Python code block of its replies. +- **AI Assistant — macros written as text:** asked to write a macro, the assistant could answer with the code in its reply, without any **Save to Macros** button, in particular for curves its creation tools cannot produce (super-Gaussian, Lorentzian, polynomial…), which it was told to create without a macro. The assistant now proposes such macros through its macro tool, without running them unless asked (the approval dialog says so, and approving such a macro never auto-approves a macro that runs), and a **Save to Macros** button is also offered under each Python code block of its replies; the macros saved from a reply with several code blocks are numbered after the request they answer (e.g. _Plot two curves (2)_), so they can be told apart in the Macros panel. - **AI Assistant — macro output:** the assistant received an empty result when reading back the output of the last macro it ran. ### Changed diff --git a/src/App.tsx b/src/App.tsx index 57db29c..42e206b 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1151,6 +1151,23 @@ export default function App() { return unsubscribe; }, [runtime, markDirty]); + // In on-disk storage mode, objects that could not be stored on disk after + // an operation (storage quota exceeded, write error) stay in memory: the + // operation succeeded, the storage error is reported once. + useEffect(() => { + if (!runtime) return; + return runtime.onStorageError((message) => { + void notify({ + kind: "warning", + title: t("Data storage"), + message: t( + "Some data could not be stored on disk and is kept in memory: {message}", + { message }, + ), + }); + }); + }, [runtime, notify]); + // Sync the "Paste metadata" availability from the Python clipboard // once the runtime is ready. The Python ``_METADATA_CLIPBOARD`` is // preserved across HMR reloads, so the React flag (which resets to @@ -2282,6 +2299,9 @@ export default function App() { : t("On-disk storage disabled. Object data is kept in memory."), }); } catch (err) { + // The runtime normally stays in its previous mode, but reports the + // mode it actually ended in (see ``setStorageMode``). + setStoreOnDisk(runtime.getStorageMode() === "disk"); const message = err instanceof Error ? err.message : String(err); await notify({ kind: "error", diff --git a/src/components/AIAssistant/AIAssistantPanel.tsx b/src/components/AIAssistant/AIAssistantPanel.tsx index 79dec28..c0671fb 100644 --- a/src/components/AIAssistant/AIAssistantPanel.tsx +++ b/src/components/AIAssistant/AIAssistantPanel.tsx @@ -950,7 +950,8 @@ function CodeBlockSaveButtons({ {blocks.map((code, i) => ( 1 ? `${title} (${i + 1})` : title} code={code} onSaveMacro={onSaveMacro} label={ diff --git a/src/locales/fr.json b/src/locales/fr.json index 5571e00..d90d8e2 100644 --- a/src/locales/fr.json +++ b/src/locales/fr.json @@ -1199,6 +1199,7 @@ "Sobel filter (vertical)": "Filtre de Sobel (vertical)", "Software": "Logiciel", "Solid": "Continu", + "Some data could not be stored on disk and is kept in memory: {message}": "Des données n'ont pas pu être stockées sur le disque et sont conservées en mémoire : {message}", "Some integer values could not be represented exactly and were rounded during import.": "Certaines valeurs entières ne pouvaient pas être représentées exactement et ont été arrondies lors de l'import.", "Some selected objects have regions of interest. Do you want to delete them as well?": "Certains objets sélectionnés ont des régions d'intérêt. Souhaitez-vous les supprimer également ?", "Source": "Source", diff --git a/src/runtime/WorkerRuntimeProxy.ts b/src/runtime/WorkerRuntimeProxy.ts index 46b489f..acf4491 100644 --- a/src/runtime/WorkerRuntimeProxy.ts +++ b/src/runtime/WorkerRuntimeProxy.ts @@ -13,9 +13,9 @@ * * * **Synchronous accessors** (``getStorageMode`` etc.) are served from a * locally-cached {@link KernelMirror} that the worker pushes on change. - * * **Callback registrars** (``setDialogHandler``, ``onWorkspaceMutation``) - * keep the JS callback on the main thread and bridge the worker's events - * to it — a function can't cross ``postMessage``. + * * **Callback registrars** (``setDialogHandler``, ``onWorkspaceMutation``, + * ``onStorageError``) keep the JS callback on the main thread and bridge + * the worker's events to it — a function can't cross ``postMessage``. * * Every other method is forwarded generically through a ``Proxy`` trap, * so the ~150-method surface needs no hand-written forwarders and any * new ``RuntimeApi`` method works automatically. @@ -65,6 +65,7 @@ export class WorkerRuntimeProxy { private readonly pending = new Map(); private mirror: KernelMirror = INITIAL_MIRROR; private readonly mutationListeners = new Set<(name: string) => void>(); + private readonly storageErrorListeners = new Set<(message: string) => void>(); private dialogHandler: ((kind: string, payload: unknown) => Promise) | null = null; @@ -173,14 +174,18 @@ export class WorkerRuntimeProxy { } if (prop === "onWorkspaceMutation") { return (listener: (name: string) => void) => - this.addMutationListener(listener); + this.addListener(this.mutationListeners, listener); + } + if (prop === "onStorageError") { + return (listener: (message: string) => void) => + this.addListener(this.storageErrorListeners, listener); } // Generic async forward for every other method. return (...args: unknown[]) => this.call(prop, args); }, }; // The Proxy structurally satisfies RuntimeApi: every access yields a - // function with the right shape (sync accessors, the two callback + // function with the right shape (sync accessors, the callback // registrars, or a generic async forward). return new Proxy({}, handler) as unknown as RuntimeApi; } @@ -219,13 +224,31 @@ export class WorkerRuntimeProxy { }); } - private addMutationListener(listener: (name: string) => void): () => void { - this.mutationListeners.add(listener); + private addListener( + listeners: Set<(value: T) => void>, + listener: (value: T) => void, + ): () => void { + listeners.add(listener); return () => { - this.mutationListeners.delete(listener); + listeners.delete(listener); }; } + /** Call every listener of *listeners* with *value*, logging their errors. */ + private notify( + listeners: Set<(value: T) => void>, + value: T, + kind: string, + ): void { + for (const cb of listeners) { + try { + cb(value); + } catch (err) { + console.warn(`[workerRuntime] ${kind} listener threw`, err); + } + } + } + private handleEvent(event: KernelEvent): void { switch (event.type) { case "progress": @@ -255,13 +278,10 @@ export class WorkerRuntimeProxy { break; } case "mutation": - for (const cb of this.mutationListeners) { - try { - cb(event.name); - } catch (err) { - console.warn("[workerRuntime] mutation listener threw", err); - } - } + this.notify(this.mutationListeners, event.name, "mutation"); + break; + case "storage-error": + this.notify(this.storageErrorListeners, event.message, "storage-error"); break; case "dialog-request": void this.handleDialogRequest(event.id, event.kind, event.payload); diff --git a/src/runtime/dlw_h5browser.py b/src/runtime/dlw_h5browser.py index 237e399..b27343e 100644 --- a/src/runtime/dlw_h5browser.py +++ b/src/runtime/dlw_h5browser.py @@ -426,10 +426,10 @@ def _build_node( ) continue try: + # Path in the opened file: ``child_dset.name`` is the path of an + # object reached through an external link in its own file info.children.append( - _build_node( - file_id, child_dset.name, child_dset, _depth + 1, ancestors - ) + _build_node(file_id, child_path, child_dset, _depth + 1, ancestors) ) except (UnicodeDecodeError, TypeError, ValueError, KeyError) as exc: print(f"[h5browser] skip {child_path!r}: {type(exc).__name__}: {exc}") diff --git a/src/runtime/kernelWorker.ts b/src/runtime/kernelWorker.ts index 7c750d3..6f2d820 100644 --- a/src/runtime/kernelWorker.ts +++ b/src/runtime/kernelWorker.ts @@ -84,6 +84,8 @@ async function boot(lang: string, labels: RuntimeLabels): Promise { ); // Forward workspace mutations so the main thread can flip its dirty flag. rt.onWorkspaceMutation((name) => post({ type: "mutation", name })); + // Forward on-disk storage errors, which the main thread reports. + rt.onStorageError((message) => post({ type: "storage-error", message })); // Route Python dialog requests up to the main thread, which owns the UI. rt.setDialogHandler( (kind, payload) => diff --git a/src/runtime/remoteBridge.ts b/src/runtime/remoteBridge.ts index 8a7ac64..b1348c2 100644 --- a/src/runtime/remoteBridge.ts +++ b/src/runtime/remoteBridge.ts @@ -21,6 +21,7 @@ * * Responses with binary payloads (e.g. ``Float64Array``) attach those * buffers as ``Transferable`` to ``postMessage`` for zero-copy transfer. + * Events, sent to every peer, are cloned instead. */ import { buildProxyBridge, type BridgeMethod } from "./proxyBridge"; @@ -184,14 +185,18 @@ export function activateRemoteBridge( target: MessageEventSource, origin: string, msg: OutboundMessage, + transferBuffers?: boolean, ) => void) | null = null; + // Events are cloned, never transferred: they go to every peer (a buffer + // transferred to the first one would arrive empty at the others), and + // their payload belongs to the caller, whose buffers must stay usable. const flushEvents = () => { if (!post || queuedEvents.length === 0 || peers.size === 0) return; const drained = queuedEvents.splice(0); for (const [peer, origin] of peers) { - for (const evt of drained) post(peer, origin, evt); + for (const evt of drained) post(peer, origin, evt, false); } }; @@ -237,8 +242,13 @@ export function activateRemoteBridge( get_protocol_version: async () => RPC_PROTOCOL_VERSION, }; - post = (target: MessageEventSource, origin: string, msg: OutboundMessage) => { - const transfer = collectTransferables(msg); + post = ( + target: MessageEventSource, + origin: string, + msg: OutboundMessage, + transferBuffers = true, + ) => { + const transfer = transferBuffers ? collectTransferables(msg) : []; const targetOrigin = peerTargetOrigin(origin); try { // ``MessageEventSource`` has a narrower type than ``Window`` but @@ -312,7 +322,7 @@ export function activateRemoteBridge( }, emit(name, payload) { const evt: RpcEvent = { type: "event", name, payload }; - for (const [peer, origin] of peers) post(peer, origin, evt); + for (const [peer, origin] of peers) post(peer, origin, evt, false); }, }; } diff --git a/src/runtime/runtime.ts b/src/runtime/runtime.ts index d42241c..edb24f6 100644 --- a/src/runtime/runtime.ts +++ b/src/runtime/runtime.ts @@ -1759,6 +1759,7 @@ export class DataLabRuntime { ) {} private mutationListeners: Set<(name: string) => void> = new Set(); + private storageErrorListeners: Set<(message: string) => void> = new Set(); /** * Subscribe to durable-workspace mutations. The callback fires *after* @@ -1774,6 +1775,31 @@ export class DataLabRuntime { }; } + /** + * Subscribe to the on-disk storage errors following a successful call. + * + * Such a call is not failed (it would be applied twice if retried): the + * objects that could not be stored on disk stay in memory with their data. + * An error is reported once, until the store works again. Returns an + * unsubscribe function; listeners must not throw. + */ + onStorageError(listener: (message: string) => void): () => void { + this.storageErrorListeners.add(listener); + return () => { + this.storageErrorListeners.delete(listener); + }; + } + + private emitStorageError(message: string): void { + for (const cb of this.storageErrorListeners) { + try { + cb(message); + } catch (err) { + console.warn("[runtime] storage-error listener threw", err); + } + } + } + private emitWorkspaceMutation(name: string): void { if (this.mutationListeners.size === 0) return; for (const cb of this.mutationListeners) { @@ -2160,6 +2186,9 @@ await micropip.install(${JSON.stringify(bootRequirements)}) /** JS mirror of bootstrap's ``_SPILLED`` set — object ids whose heavy * array currently lives on disk rather than in the WASM heap. */ private readonly spilledOids = new Set(); + /** Whether the on-disk store failed after the last call that used it: a + * storage error is reported once, until the store works again. */ + private storageFailing = false; /** Cache of {@link getCapsuleSizeLimit} (undefined until read). */ private capsuleSizeLimit: number | null | undefined; @@ -2422,25 +2451,61 @@ await micropip.install(${JSON.stringify(bootRequirements)}) } /** Return paged-in objects to disk and reconcile the on-disk store with - * the model after a call adding, deleting or replacing objects. */ + * the model after a call adding, deleting or replacing objects. + * + * Never throws: the call has already succeeded, so a storage failure must + * not report it as failed (it would be applied twice if retried). Each step + * is attempted even if a previous one failed. An object whose data cannot + * be written stays resident with its data (see ``persistOrRestore``), and + * a later sweep tries again to store it on disk. Failures are logged, and + * reported to the {@link onStorageError} listeners. */ private async diskGuardAfter( name: string, kwargs: Record, pagedIn: string[], ): Promise { + let used = false; + let failure: unknown; + const attempt = async ( + step: () => Promise, + ): Promise => { + used = true; + try { + return await step(); + } catch (err) { + console.error(`[runtime] on-disk storage after ${name} failed`, err); + if (failure === undefined) failure = err; + return undefined; + } + }; const rewrite = MUTATES_HEAVY.has(name); - for (const oid of pagedIn) await this.settleDirect(oid, rewrite); + for (const oid of pagedIn) { + await attempt(() => this.settleDirect(oid, rewrite)); + } if (REPLACES_MODEL.has(name) && kwargs.replace !== false) { // A replaced model forgets the old store at once; an appended file // keeps the current objects and their on-disk copies. this.spilledOids.clear(); - await this.requireStore().clear(); + await attempt(() => this.requireStore().clear()); } - const swept = DISK_SWEEP_CALLS.has(name) && (await this.diskSweepDirect()); + const swept = + DISK_SWEEP_CALLS.has(name) && + (await attempt(() => this.diskSweepDirect())) === true; // Reclaim heap pages freed by the placeholders so later allocations // reuse them instead of growing the heap. if (pagedIn.length || swept) { - await this.invokePy("collect_garbage"); + await attempt(() => this.invokePy("collect_garbage")); + } + if (!used) return; + if (failure === undefined) { + this.storageFailing = false; + } else if (!this.storageFailing) { + this.storageFailing = true; + // ``DOMException`` (quota exceeded...) is not an ``Error`` everywhere + const message = (failure as { message?: unknown } | null)?.message; + this.emitStorageError( + typeof message === "string" ? message : String(failure), + ); } } @@ -2542,7 +2607,9 @@ await micropip.install(${JSON.stringify(bootRequirements)}) * metadata in the heap); ``"ram"`` pages every spilled array back in * and clears the on-disk store. Both transitions are queued so they * never interleave with in-flight Python calls. No-op when already in - * the requested mode. + * the requested mode. A switch to ``"disk"`` that fails (store + * unavailable, write error) leaves the runtime in ``"ram"`` mode with + * every array resident (see {@link revertSpillAllDirect}). */ async setStorageMode(mode: StorageMode): Promise { if (mode === this.storageMode) return; @@ -2557,13 +2624,22 @@ await micropip.install(${JSON.stringify(bootRequirements)}) // Prefer the synchronous-handle store when the runtime runs in a // worker (4–7× faster spills, see ``opfs_sync_spike``); fall back // to the async ``createWritable`` store on the main thread. - this.opfsStore = OpfsSyncObjectStore.isSupported() + const store = OpfsSyncObjectStore.isSupported() ? new OpfsSyncObjectStore() : new OpfsObjectStore(); - await this.opfsStore.init(); + // Kept only once initialised, so that a failed initialisation + // (e.g. storage denied) may be retried by the next switch. + await store.init(); + this.opfsStore = store; + } + try { + await this.spillAllResidentDirect(); + } catch (err) { + await this.revertSpillAllDirect(); + throw err; } this.storageMode = "disk"; - await this.spillAllResidentDirect(); + this.storageFailing = false; } else { await this.pageInAllDirect(); this.storageMode = "ram"; @@ -2575,6 +2651,22 @@ await micropip.install(${JSON.stringify(bootRequirements)}) return next as Promise; } + /** Undo a switch to the on-disk mode that could not spill every object + * (direct, unqueued): the objects already spilled are paged back in and + * the store is emptied, so that the runtime stays in RAM mode. If they + * cannot all be read back, the runtime switches to the on-disk mode + * anyway: the objects left on disk need it to be paged in. */ + private async revertSpillAllDirect(): Promise { + try { + await this.pageInAllDirect(); + } catch (err) { + console.error("[runtime] cannot page spilled objects back in", err); + this.storageMode = "disk"; + return; + } + await this.opfsStore?.clear().catch(() => undefined); + } + /** Total bytes currently spilled to the OPFS store (0 in RAM mode). */ getDiskStoreBytes(): number { return this.opfsStore?.totalBytes() ?? 0; diff --git a/src/runtime/workerProtocol.ts b/src/runtime/workerProtocol.ts index 666e29c..8105474 100644 --- a/src/runtime/workerProtocol.ts +++ b/src/runtime/workerProtocol.ts @@ -13,9 +13,10 @@ * 1. **Request/reply** — the proxy posts a {@link CallRequest} and the * worker answers with a matching {@link ResultEvent} (``ok``/``error``). * 2. **Worker-initiated events** — workspace mutations - * ({@link MutationEvent}) and Python dialog requests + * ({@link MutationEvent}), on-disk storage errors + * ({@link StorageErrorEvent}) and Python dialog requests * ({@link DialogRequestEvent}) are pushed up so the main thread can - * flip its "dirty" flag and render dialogs. + * flip its "dirty" flag, report storage errors and render dialogs. * 3. **State mirror** — the four synchronous accessors of * {@link RuntimeApi} (``getStorageMode`` / ``getDiskStoreBytes`` / * ``getSpilledCount`` / ``getMemoryUsage``) cannot do a round-trip, so @@ -147,6 +148,13 @@ export interface MutationEvent { name: string; } +/** An on-disk storage error following a successful call. Mirrors + * ``DataLabRuntime.onStorageError``. */ +export interface StorageErrorEvent { + type: "storage-error"; + message: string; +} + /** Python requested a dialog; the main thread renders it and answers with a * {@link DialogResponseRequest}. */ export interface DialogRequestEvent { @@ -168,6 +176,7 @@ export type KernelEvent = | BootErrorEvent | ResultEvent | MutationEvent + | StorageErrorEvent | DialogRequestEvent | MirrorEvent; @@ -189,6 +198,7 @@ export const SYNC_MIRROR_METHODS = [ export const CALLBACK_METHODS = [ "setDialogHandler", "onWorkspaceMutation", + "onStorageError", ] as const; /** diff --git a/src/storage/opfsSyncObjectStore.ts b/src/storage/opfsSyncObjectStore.ts index bb80d42..d25018a 100644 --- a/src/storage/opfsSyncObjectStore.ts +++ b/src/storage/opfsSyncObjectStore.ts @@ -91,10 +91,13 @@ export class OpfsSyncObjectStore implements ObjectByteStore { return this.dir; } - private async handle(oid: string): Promise { + private async handle( + oid: string, + create: boolean, + ): Promise { const dir = this.requireDir(); const fh = (await dir.getFileHandle(fileName(oid), { - create: true, + create, })) as SyncCapableFileHandle; return fh.createSyncAccessHandle(); } @@ -107,7 +110,7 @@ export class OpfsSyncObjectStore implements ObjectByteStore { * caller never takes an incomplete copy for a stored payload. */ async put(oid: string, bytes: Uint8Array): Promise { - const h = await this.handle(oid); + const h = await this.handle(oid, true); try { // In-place write, then shrink the file to the exact payload length so // a smaller re-spill never leaves stale trailing bytes. @@ -130,12 +133,13 @@ export class OpfsSyncObjectStore implements ObjectByteStore { } /** - * Read back the payload for *oid*. Throws if it is absent, or if fewer - * bytes than the file size can be read (a read may transfer fewer bytes - * than requested: the remaining bytes are read in turn). + * Read back the payload for *oid*. Throws if it is absent (without creating + * an empty file), or if fewer bytes than the file size can be read (a read + * may transfer fewer bytes than requested: the remaining bytes are read in + * turn). */ async get(oid: string): Promise { - const h = await this.handle(oid); + const h = await this.handle(oid, false); try { const size = h.getSize(); const out = new Uint8Array(size); diff --git a/tests/python/test_h5browser.py b/tests/python/test_h5browser.py index a79cefb..d1b063d 100644 --- a/tests/python/test_h5browser.py +++ b/tests/python/test_h5browser.py @@ -133,6 +133,34 @@ def test_open_file_skips_broken_links(tmp_path, capsys): assert "broken external link to 'missing_detector.h5:/frames'" in output +def test_tree_keeps_paths_of_external_datasets(tmp_path): + """Datasets reached through an external link get their path in the opened + file, and that path reads their own data. + + Regression: they got their path in their own file, which may designate + another dataset of the opened file (whose data was then read instead). + """ + with h5py.File(tmp_path / "detector.h5", "w") as f: + f["/entry/data/data"] = np.full((4, 5), 7.0) + with h5py.File(tmp_path / "master.h5", "w") as f: + f["/entry/data/data"] = np.zeros(3) + f["/entry/data/data_000001"] = h5py.ExternalLink( + "detector.h5", "/entry/data/data" + ) + with h5py.File(tmp_path / "master.h5", "r") as f: + nodes = {} + h5b._flatten(h5b._build_node("fid", "/", f), nodes) + assert {"/entry/data/data", "/entry/data/data_000001"} <= set(nodes) + external = nodes["/entry/data/data_000001"] + assert (external.name, external.h5path) == ( + "data_000001", + "/entry/data/data_000001", + ) + assert external.kind == "image" + np.testing.assert_array_equal(f[external.h5path][()], np.full((4, 5), 7.0)) + assert nodes["/entry/data/data"].kind == "signal" + + def test_open_file_cleans_up_when_tree_fails(h5_bytes, monkeypatch): """A failure while building the tree closes the file and removes its temporary copy; the next file opens normally.""" diff --git a/tests/ts/aiassistant/panel.test.tsx b/tests/ts/aiassistant/panel.test.tsx index d809988..37a033f 100644 --- a/tests/ts/aiassistant/panel.test.tsx +++ b/tests/ts/aiassistant/panel.test.tsx @@ -397,6 +397,47 @@ describe("AIAssistantPanel — Save to Macros", () => { ); }); + it("gives distinct titles to the macros of a reply with several blocks", async () => { + const SECOND_CODE = "print('second')"; + vi.stubGlobal( + "fetch", + mockOpenAI([ + { + content: [ + "```python", + MACRO_CODE, + "```", + "", + "```python", + SECOND_CODE, + "```", + ].join("\n"), + toolCalls: [], + }, + ]), + ); + const onSaveMacro = vi.fn(async () => undefined); + render( + , + { wrapper: ThemeProvider }, + ); + await send("Plot two curves"); + fireEvent.click( + await screen.findByRole("button", { name: /Save code block 1/i }), + ); + fireEvent.click(screen.getByRole("button", { name: /Save code block 2/i })); + await waitFor(() => { + expect(onSaveMacro).toHaveBeenCalledWith( + "Plot two curves (1)", + MACRO_CODE, + ); + expect(onSaveMacro).toHaveBeenCalledWith( + "Plot two curves (2)", + SECOND_CODE, + ); + }); + }); + it("never auto-approves a macro run after remembering a macro offered without running it", async () => { vi.stubGlobal( "fetch", diff --git a/tests/ts/remoteBridge.test.ts b/tests/ts/remoteBridge.test.ts index 80eb3ad..05d2951 100644 --- a/tests/ts/remoteBridge.test.ts +++ b/tests/ts/remoteBridge.test.ts @@ -338,6 +338,42 @@ describe("activateRemoteBridge", () => { handle?.dispose(); }); + it("clones the binary payloads of events instead of transferring them", async () => { + const handle = activateRemoteBridge(runtime, { + allowedOriginsSource: + "?allowedOrigins=https://a.example.com,https://b.example.com", + win, + }); + const peerA: MockPeer = { postMessage: vi.fn() }; + const peerB: MockPeer = { postMessage: vi.fn() }; + for (const [peer, origin, id] of [ + [peerA, "https://a.example.com", 1], + [peerB, "https://b.example.com", 2], + ] as const) { + win.dispatchEvent( + fakeMessageEvent({ + origin, + source: peer, + data: { id, type: "request", method: "get_version" }, + }), + ); + } + await new Promise((r) => setTimeout(r, 0)); + + const data = new Float64Array([1, 2, 3]); + handle?.emit("frame", { data }); + + // Regression: the buffers were transferred to each peer in turn, so that + // the second peer received detached (empty) buffers. + for (const peer of [peerA, peerB]) { + const [msg, , transfer] = peer.postMessage.mock.calls[1]; + expect(msg).toMatchObject({ type: "event", name: "frame" }); + expect(msg.payload.data).toBe(data); + expect(transfer).toBeUndefined(); + } + handle?.dispose(); + }); + it("targets opaque origins only through the wildcard", async () => { const handle = activateRemoteBridge(runtime, { allowedOriginsSource: "?allowedOrigins=*", diff --git a/tests/ts/runtime/diskStorageModeSwitch.test.ts b/tests/ts/runtime/diskStorageModeSwitch.test.ts new file mode 100644 index 0000000..ed154c5 --- /dev/null +++ b/tests/ts/runtime/diskStorageModeSwitch.test.ts @@ -0,0 +1,311 @@ +/** + * Robustness of the on-disk storage mode against storage failures: + * + * - a switch to the on-disk mode that fails (store initialisation, write + * error) leaves the runtime in RAM mode, with every array resident, and may + * be retried; + * - a successful Python call is never reported as failed because its objects + * could not be stored on disk afterwards (it would be applied twice if the + * user retried it). + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { DataLabRuntime } from "../../../src/runtime/runtime"; + +/** Behaviour of the fake OPFS stores, shared with the mocked modules. */ +const opfs = vi.hoisted(() => ({ + initFailures: 0, + failingPuts: new Set(), + failGets: false, + failClear: false, + instances: [] as Array<{ files: Map }>, +})); + +vi.mock("../../../src/storage/opfsSyncObjectStore", () => ({ + OpfsSyncObjectStore: class { + static isSupported(): boolean { + return false; + } + }, +})); + +vi.mock("../../../src/storage/opfsObjectStore", () => ({ + OpfsObjectStore: class { + readonly files = new Map(); + private initialised = false; + + static isSupported(): boolean { + return true; + } + + constructor() { + opfs.instances.push(this); + } + + async init(): Promise { + if (opfs.initFailures > 0) { + opfs.initFailures -= 1; + throw new DOMException("storage denied", "SecurityError"); + } + this.initialised = true; + } + + async put(oid: string, bytes: Uint8Array): Promise { + if (!this.initialised) throw new Error("init() must be called first"); + if (opfs.failingPuts.has(oid)) { + throw new DOMException("quota exceeded", "QuotaExceededError"); + } + this.files.set(oid, bytes); + } + + async get(oid: string): Promise { + const bytes = this.files.get(oid); + if (opfs.failGets || !bytes) { + throw new DOMException(`cannot read ${oid}`, "NotReadableError"); + } + return bytes; + } + + async delete(oid: string): Promise { + this.files.delete(oid); + } + + async clear(): Promise { + if (opfs.failClear) throw new DOMException("busy", "InvalidStateError"); + this.files.clear(); + } + + totalBytes(): number { + return 0; + } + }, +})); + +type Handler = (kwargs: Record) => unknown; + +interface RuntimeInternals { + py: { + globals: { + get: (name: string) => { + callKwargs: (kwargs: Record) => unknown; + destroy: () => void; + }; + }; + }; + _queue: Promise; + _latestReadGenerations: Map; + mutationListeners: Set<(name: string) => void>; + storageMode: "ram" | "disk"; + opfsStore: unknown; + spilledOids: Set; + storageErrorListeners: Set<(message: string) => void>; + storageFailing: boolean; + onStorageError(listener: (message: string) => void): () => void; + setStorageMode(mode: "ram" | "disk"): Promise; + callPy(name: string, kwargs?: Record): Promise; +} + +const PAYLOAD = new Uint8Array([1, 2, 3]); + +/** + * Runtime whose fake Python side keeps the spill bookkeeping of + * ``bootstrap.py``: *resident* objects hold their array, spilled ones only + * their metadata. *handlers* answer the calls under test. + */ +function makeRuntime( + resident: string[], + handlers: Record = {}, +) { + const runtime = Object.create(DataLabRuntime.prototype) as RuntimeInternals; + const calls: string[] = []; + const py = { resident: new Set(resident), spilled: new Set() }; + const builtins: Record = { + spillable_objects: () => Array.from(py.resident, (oid) => ({ oid })), + detach_object_array: ({ oid }) => { + py.resident.delete(String(oid)); + py.spilled.add(String(oid)); + return PAYLOAD; + }, + attach_object_array: ({ oid }) => { + py.spilled.delete(String(oid)); + py.resident.add(String(oid)); + return true; + }, + settle_object_array: ({ oid }) => { + py.resident.delete(String(oid)); + py.spilled.add(String(oid)); + return { state: "rewritten", payload: PAYLOAD }; + }, + disk_sweep_state: () => ({ + spilled: Array.from(py.spilled), + resident: Array.from(py.resident), + }), + collect_garbage: () => ({}), + }; + runtime.py = { + globals: { + get: (name) => ({ + callKwargs: (kwargs) => { + const key = name.replace(/^bridge_/, ""); + calls.push(key); + const handler = handlers[key] ?? builtins[key]; + const value = handler ? handler(kwargs) : null; + return name.startsWith("bridge_") ? { value } : value; + }, + destroy: () => {}, + }), + }, + }; + runtime._queue = Promise.resolve(); + runtime._latestReadGenerations = new Map(); + runtime.mutationListeners = new Set(); + runtime.storageMode = "ram"; + runtime.opfsStore = null; + runtime.spilledOids = new Set(); + runtime.storageErrorListeners = new Set(); + runtime.storageFailing = false; + const storageErrors: string[] = []; + runtime.onStorageError((message) => storageErrors.push(message)); + return { runtime, py, calls, storageErrors }; +} + +beforeEach(() => { + opfs.initFailures = 0; + opfs.failingPuts.clear(); + opfs.failGets = false; + opfs.failClear = false; + opfs.instances.length = 0; + vi.spyOn(console, "error").mockImplementation(() => {}); +}); + +describe("switch to the on-disk storage mode", () => { + it("may be retried after the store could not be initialised", async () => { + const { runtime, py } = makeRuntime(["A"]); + opfs.initFailures = 1; + + await expect(runtime.setStorageMode("disk")).rejects.toThrow( + "storage denied", + ); + expect(runtime.storageMode).toBe("ram"); + expect(runtime.opfsStore).toBeNull(); + expect(py.resident).toEqual(new Set(["A"])); + + await runtime.setStorageMode("disk"); + + expect(runtime.storageMode).toBe("disk"); + expect(runtime.spilledOids).toEqual(new Set(["A"])); + expect(py.spilled).toEqual(new Set(["A"])); + }); + + it("stays in RAM mode with every array resident when a spill fails", async () => { + const { runtime, py } = makeRuntime(["A", "B", "C"]); + opfs.failingPuts.add("B"); + + await expect(runtime.setStorageMode("disk")).rejects.toThrow( + "quota exceeded", + ); + + expect(runtime.storageMode).toBe("ram"); + expect(runtime.spilledOids).toEqual(new Set()); + expect(py.resident).toEqual(new Set(["A", "B", "C"])); + expect(py.spilled).toEqual(new Set()); + expect(opfs.instances[0].files.size).toBe(0); + + // A later switch, once the store accepts the data, spills everything + opfs.failingPuts.clear(); + await runtime.setStorageMode("disk"); + expect(runtime.storageMode).toBe("disk"); + expect(runtime.spilledOids).toEqual(new Set(["A", "B", "C"])); + }); + + it("stays in on-disk mode when spilled arrays cannot be read back", async () => { + const { runtime, py } = makeRuntime(["A", "B"]); + opfs.failingPuts.add("B"); + opfs.failGets = true; + + await expect(runtime.setStorageMode("disk")).rejects.toThrow( + "quota exceeded", + ); + + // "A" is still on disk only: the guard must keep paging it in + expect(runtime.storageMode).toBe("disk"); + expect(runtime.spilledOids).toEqual(new Set(["A"])); + expect(py.spilled).toEqual(new Set(["A"])); + }); +}); + +describe("a successful call is not failed by on-disk storage errors", () => { + /** Disk-mode runtime with *spilled* objects on disk. */ + async function diskRuntime( + spilled: string[], + handlers: Record = {}, + ) { + const made = makeRuntime(spilled, handlers); + await made.runtime.setStorageMode("disk"); + made.calls.length = 0; + return made; + } + + it("returns the result when a paged-in object cannot be rewritten", async () => { + const { runtime, py, calls, storageErrors } = await diskRuntime( + ["A", "B"], + { set_image_data: () => "done" }, + ); + opfs.failingPuts.add("A"); + + // ``set_image_data`` pages its object in and rewrites it afterwards + await expect(runtime.callPy("set_image_data", { oid: "A" })).resolves.toBe( + "done", + ); + + // "A" stays resident with its data, "B" is still on disk + expect(py.resident).toEqual(new Set(["A"])); + expect(runtime.spilledOids).toEqual(new Set(["B"])); + expect(py.spilled).toEqual(new Set(["B"])); + expect(calls).toContain("disk_sweep_state"); + expect(calls).toContain("collect_garbage"); + // The storage error is reported, without failing the call + expect(storageErrors).toEqual(["quota exceeded"]); + }); + + it("reports a storage error once, until the store works again", async () => { + const { runtime, storageErrors } = await diskRuntime(["A"], { + set_image_data: () => "done", + get_image_data: () => "read", + }); + opfs.failingPuts.add("A"); + await runtime.callPy("set_image_data", { oid: "A" }); + await runtime.callPy("set_image_data", { oid: "A" }); + // A call that does not use the store does not end the failure + await runtime.callPy("get_image_data", { oid: "A" }); + await runtime.callPy("set_image_data", { oid: "A" }); + expect(storageErrors).toHaveLength(1); + + opfs.failingPuts.clear(); + await runtime.callPy("set_image_data", { oid: "A" }); + expect(runtime.spilledOids).toEqual(new Set(["A"])); + opfs.failingPuts.add("A"); + await runtime.callPy("set_image_data", { oid: "A" }); + expect(storageErrors).toHaveLength(2); + }); + + it("returns the result when the old store cannot be cleared", async () => { + const { runtime, py } = await diskRuntime(["old"], { + open_workspace_from_bytes: () => { + py.spilled.clear(); + py.resident.add("new"); + return "opened"; + }, + }); + opfs.failClear = true; + + await expect( + runtime.callPy("open_workspace_from_bytes", { replace: true }), + ).resolves.toBe("opened"); + + // The sweep still spills the objects of the new workspace + expect(runtime.spilledOids).toEqual(new Set(["new"])); + expect(py.spilled).toEqual(new Set(["new"])); + expect(py.resident).toEqual(new Set()); + }); +}); diff --git a/tests/ts/runtime/workerRuntimeProxy.test.ts b/tests/ts/runtime/workerRuntimeProxy.test.ts index 3efc953..a45b7d6 100644 --- a/tests/ts/runtime/workerRuntimeProxy.test.ts +++ b/tests/ts/runtime/workerRuntimeProxy.test.ts @@ -286,6 +286,17 @@ describe("WorkerRuntimeProxy", () => { expect(listener).toHaveBeenCalledTimes(1); }); + it("fans on-disk storage errors out to subscribers", () => { + const { worker, runtime } = setup(); + const listener = vi.fn(); + const unsubscribe = runtime.onStorageError(listener); + worker.emit({ type: "storage-error", message: "quota exceeded" }); + expect(listener).toHaveBeenCalledWith("quota exceeded"); + unsubscribe(); + worker.emit({ type: "storage-error", message: "write error" }); + expect(listener).toHaveBeenCalledTimes(1); + }); + it("bridges a dialog request to the registered handler and answers", async () => { const { worker, runtime } = setup(); const handler = vi.fn(async () => ({ ok: true, value: 42 })); diff --git a/tests/ts/storage/opfsSyncObjectStore.test.ts b/tests/ts/storage/opfsSyncObjectStore.test.ts index c639874..22d00a8 100644 --- a/tests/ts/storage/opfsSyncObjectStore.test.ts +++ b/tests/ts/storage/opfsSyncObjectStore.test.ts @@ -256,6 +256,15 @@ describe("OpfsSyncObjectStore", () => { ); }); + it("fails to read an absent payload without creating it", async () => { + // Regression: reading an absent payload created an empty file and + // returned no bytes, which were then taken for the object data. + const store = new OpfsSyncObjectStore(); + await store.init(); + await expect(store.get("missing")).rejects.toThrow(); + expect(await store.has("missing")).toBe(false); + }); + it("clears every stored payload", async () => { const store = new OpfsSyncObjectStore(); await store.init();