From d05df5f2bb3381d6eb0e8b6a439172f69cc119a2 Mon Sep 17 00:00:00 2001 From: jdpigeon Date: Mon, 6 Jul 2026 14:18:58 -0400 Subject: [PATCH 1/4] Fix Pyodide analysis pipeline: dataKey routing, MEMFS epochs, MNE 1.x PSD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harvests the still-valid fixes from the stale PR #194 (pyodide-fix) and re-applies them onto current main. #194 itself isn't mergeable — it was built on a superseded asset-serving base (HTTP server) that main replaced with the pyodide:// protocol, and it predates the buildMarkerRegistry marker-code fix (its loadEpochsEpic reverted to the broken 0-based event map). So this takes the logic, not the commits, and keeps buildMarkerRegistry intact. The pipeline crash (getEpochsInfoEpic: "Cannot read properties of undefined (reading 'map')") was because requestEpochsInfo/requestChannelInfo relied on worker.postMessage returning data — it returns undefined; results come back on the message channel. Fixes: - dataKey routing (parallel to plotKey): worker echoes dataKey + converts the PyProxy result to plain JS (structuredClone can't clone a PyProxy); pyodideMessageEpic routes epochsInfo -> SetEpochInfo, channelInfo -> SetChannelInfo. The two info epics + loadEpochsEpic are now fire-and-forget. - loadCleanedEpochs: stage .fif files in Pyodide MEMFS via new fs:readFileAsBytes IPC + writeEpochsToMemfs (WASM FS can't read host OS paths). - plotPSD: raw.plot_psd() -> raw.compute_psd().plot() (MNE 1.2+ removed plot_psd). - loadTopoEpic: call plotTopoMap, not plotTestPlot. - renderAnalyzeButton: show when epochsInfo.length > 0 (not only when drop >= 2). - eslint: ignore leftover src/renderer/utils/pyodide/** (pyodide.asm.js pegged the linter). Drops now-unused DEVICES/plotTestPlot/parseSingleQuoteJSON imports. Skipped from #194: the channel-index-0 fix (current loadERPEpic already handles 0 correctly post-Emotiv-removal), saveEpochs paren (already closed on main), and Prettier-only churn. Typecheck clean, lint completes, 29/29 tests pass. Needs in-app Pyodide verification (can't run WASM in CI/locally here). Also documents the lab.js 23.x datastore gotcha in learnings. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01SNEh5EFg4dSpRrfJ2ae4vL --- .llms/learnings.md | 15 +++ TODOS.md | 2 +- eslint.config.mjs | 1 + src/main/index.ts | 6 + src/preload/index.ts | 3 + .../components/CleanComponent/index.tsx | 20 ++- src/renderer/epics/pyodideEpics.ts | 115 +++++++++--------- src/renderer/types/electron.d.ts | 1 + src/renderer/utils/webworker/index.ts | 48 ++++++-- src/renderer/utils/webworker/webworker.js | 36 ++++-- 10 files changed, 152 insertions(+), 95 deletions(-) diff --git a/.llms/learnings.md b/.llms/learnings.md index 368a6a8a..9443e174 100644 --- a/.llms/learnings.md +++ b/.llms/learnings.md @@ -122,6 +122,21 @@ Affected files: `src/renderer/experiments/*/experiment.ts` (and `custom/experime // New (lab.js 23.x): hooks: { 'before:prepare': initLoopWithStimuli } ``` +## Lab.js 23.x: datastore moved to `global.datastore` (not `options.datastore`) + +Reading the experiment's recorded data at flow end must use +`study.global.datastore` (a getter → `controller.global.datastore`, used +throughout lab.js internals: `base/component.ts` set/commit/update). Lab.js +23.x removed `study.options.datastore` — it's `undefined`. The old path in +`ExperimentWindow.tsx`'s `on('end')` handler (`options.datastore.exportCsv()`) +threw *inside* lab.js's end sequence: `stopOutgoing` (`flipIterable.ts`) logs +`console.error('Error ending', c)` and **re-throws**, so a throw in our end +handler aborts the whole end/commit — the surfaced error is misleadingly the +component dump, not the real TypeError. Side effect: the aborted end left +`appState.json` half-written, which then read back as a "corrupted workspace". +Same 23.x major-bump breakage class as `hooks`/`this.id` — audit any other +`study.options.*` access. + ## Lab.js stimulus `filepath` must be a browser URL, not a filesystem path `balanceStimuliByCondition` (in `src/renderer/utils/labjs/functions.ts`) generates a `filepath` field used by lab.js HTML templates (``). This must be a browser-loadable URL, not a raw filesystem path like `/Users/.../Face1.jpg`. diff --git a/TODOS.md b/TODOS.md index 54625ee6..80fad40f 100644 --- a/TODOS.md +++ b/TODOS.md @@ -18,7 +18,7 @@ Deferred and in-flight work. Keep this current — when something ships, delete ## Known issues / tech debt -- [ ] **Restore the Pyodide worker RPC (request/response correlation)** — the analysis/Clean pipeline is broken and it's pre-existing (identical on `main`, not LSL). Root cause: a past "simplify the epics" refactor deleted the id/OpenPromise correlation layer but left ~10 call sites (`loadCSV`, `filterIIR`, `epochEvents`, `requestEpochsInfo`, `requestChannelInfo`, plots…) still doing `await worker.postMessage(...)`. Native `worker.postMessage` returns `undefined` immediately (resolves on *post*, not on Python completion), so **every `await` in `webworker/index.ts` is a no-op** — sequencing currently works only by luck/timing, and `requestEpochsInfo` returns `undefined` → `getEpochsInfoEpic` crashes on `.map` (cryptic `Cannot read properties of undefined (reading 'map')` at `pyodideEpics.ts:~206`). Fix: reinstate a small `runPython(worker, code, ctx?)` RPC — module-level `Map` + one `message` listener; worker echoes the `id` back with `{results}`/`{error}`. Then epics `await` real results and map to actions; `pyodideMessageEpic` + the `plotKey` switch go away (plots are just requests whose result is an SVG string). Caveats: (1) structured Python returns must cross as JSON strings (`json.dumps` / `JSON.parse`) since `postMessage` can't clone a PyProxy; (2) awaiting naturally keeps one call in flight — add a worker-side one-at-a-time queue only if concurrent chains ever bite. Needs in-app testing (Pyodide/WASM) — not a drive-by. See `.llms/learnings.md` plot-routing note for the current (to-be-replaced) pattern. +- [ ] **(Optional) Full Pyodide worker RPC** — the analysis/Clean pipeline crash is now **fixed** (harvested from PR #194): a `dataKey` routing pattern parallel to `plotKey` — the worker echoes `dataKey` + PyProxy-converted results, and `pyodideMessageEpic` routes `epochsInfo`→`SetEpochInfo` / `channelInfo`→`SetChannelInfo`; the info epics are fire-and-forget. This unblocks the pipeline without the bigger refactor. The deeper latent issue remains, though: `worker.postMessage` returns `undefined` on *post*, so the `await`s in `webworker/index.ts` are no-ops and cross-message sequencing still relies on worker FIFO. A true `runPython(worker, code, ctx?)` RPC — `Map` + one `message` listener, worker echoes `id` — would let epics `await` real results and delete the `plotKey`/`dataKey` switch entirely. Only worth doing if the FIFO sequencing ever actually bites; not urgent now. - [ ] Pyodide-fidelity smoke test — analysis pipeline is tested against native MNE, not yet under Pyodide/WASM (see `.llms/learnings.md`). - [ ] Pre-existing TypeScript errors (not regressions): `experimentEpics.ts` (RxJS operator types), `routes.tsx` (Redux container prop types). diff --git a/eslint.config.mjs b/eslint.config.mjs index 5f625f95..663e2742 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -24,6 +24,7 @@ export default [ 'coverage/**', '.worktrees/**', 'src/renderer/utils/webworker/src/**', + 'src/renderer/utils/pyodide/**', '**/*.css.d.ts', '**/*.scss.d.ts', ], diff --git a/src/main/index.ts b/src/main/index.ts index dfbb68a1..c4313dde 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -407,6 +407,12 @@ ipcMain.handle('fs:readFiles', (_event, filePathsArray: string[]) => { }); }); +ipcMain.handle('fs:readFileAsBytes', (_event, filePath: string) => { + // Returns a Uint8Array (Buffer extends Uint8Array) for binary files like .fif. + // Crosses IPC via structured clone — arrives as a Uint8Array in the renderer. + return fs.readFileSync(filePath); +}); + // EEG streaming — main process holds write streams for performance ipcMain.handle( 'eeg:createWriteStream', diff --git a/src/preload/index.ts b/src/preload/index.ts index 026f12e4..2330df60 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -142,6 +142,9 @@ contextBridge.exposeInMainWorld('electronAPI', { readFiles: (filePathsArray: string[]): Promise => ipcRenderer.invoke('fs:readFiles', filePathsArray), + readFileAsBytes: (filePath: string): Promise => + ipcRenderer.invoke('fs:readFileAsBytes', filePath), + // ------------------------------------------------------------------ // EEG streaming — main process holds the write stream for performance // ------------------------------------------------------------------ diff --git a/src/renderer/components/CleanComponent/index.tsx b/src/renderer/components/CleanComponent/index.tsx index 9c83d8c8..6e2e8f9d 100644 --- a/src/renderer/components/CleanComponent/index.tsx +++ b/src/renderer/components/CleanComponent/index.tsx @@ -123,18 +123,14 @@ export default class Clean extends Component { renderAnalyzeButton() { const { epochsInfo } = this.props; - if (!isNil(epochsInfo)) { - const drop = epochsInfo.find( - (infoObj) => infoObj.name === 'Drop Percentage' - )?.value; - - if (drop && typeof drop === 'number' && drop >= 2) { - return ( - - - - ); - } + // Show whenever epoch stats exist — let the user decide from the numbers, + // instead of only surfacing the button when the data looked bad (drop >= 2). + if (!isNil(epochsInfo) && epochsInfo.length > 0) { + return ( + + + + ); } return null; } diff --git a/src/renderer/epics/pyodideEpics.ts b/src/renderer/epics/pyodideEpics.ts index 89a53644..5d5df451 100644 --- a/src/renderer/epics/pyodideEpics.ts +++ b/src/renderer/epics/pyodideEpics.ts @@ -1,5 +1,5 @@ import { combineEpics, Epic } from 'redux-observable'; -import { EMPTY, fromEvent, Observable, ObservableInput, of } from 'rxjs'; +import { EMPTY, fromEvent, Observable, of } from 'rxjs'; import { map, mergeMap, tap, pluck, filter } from 'rxjs/operators'; import { toast } from 'react-toastify'; import { isActionOf } from '../utils/redux'; @@ -10,6 +10,7 @@ import { buildMarkerRegistry } from '../utils/eeg/markerRegistry'; import { loadCSV, loadCleanedEpochs, + writeEpochsToMemfs, filterIIR, epochEvents, requestEpochsInfo, @@ -18,19 +19,13 @@ import { plotPSD, plotERP, plotTopoMap, - plotTestPlot, saveEpochs, loadPyodide, loadPatches, applyPatches, loadUtils, } from '../utils/webworker'; -import { - DEVICES, - MUSE_CHANNELS, - PYODIDE_VARIABLE_NAMES, -} from '../constants/constants'; -import { parseSingleQuoteJSON } from '../utils/webworker/functions'; +import { MUSE_CHANNELS, PYODIDE_VARIABLE_NAMES } from '../constants/constants'; import { readFiles } from '../utils/filesystem/read'; @@ -90,13 +85,31 @@ const pyodideMessageEpic: Epic< mergeMap>((worker) => fromEvent(worker, 'message')), // eslint-disable-next-line @typescript-eslint/no-explicit-any mergeMap>((e) => { - const { results, error, plotKey } = e.data; + const { results, error, plotKey, dataKey } = e.data; if (error) { toast.error(`Pyodide: ${error}`); return of(PyodideActions.ReceiveError(error)); } + + // Route data results (tagged with dataKey, not plotKey). These come back + // as plain JS (the worker converts the PyProxy before postMessage). + if (dataKey === 'epochsInfo') { + // results is an array of single-key objects, e.g. [{Condition1: 10}, {'Drop Percentage': 5}] + const epochInfoArray = ( + results as Array> + ).map((infoObj) => ({ + name: Object.keys(infoObj)[0], + value: infoObj[Object.keys(infoObj)[0]], + })); + return of(PyodideActions.SetEpochInfo(epochInfoArray)); + } + if (dataKey === 'channelInfo') { + // results is an array of channel-name strings + return of(PyodideActions.SetChannelInfo(results as string[])); + } + // Route plot results to the appropriate Redux state slot. - // results is a base64-encoded PNG string returned from Python. + // results is an SVG string returned from Python. const mimeBundle = results ? { 'image/svg+xml': results } : null; switch (plotKey) { case 'ready': @@ -121,31 +134,27 @@ const loadEpochsEpic: Epic = ( filter(isActionOf(PyodideActions.LoadEpochs)), pluck('payload'), filter((filePathsArray: string[]) => filePathsArray.length >= 1), - // readFiles is async — mergeMap (not map) so the resolved CSV strings flow - // downstream. With map, the unresolved Promise reached worker.postMessage - // and threw DataCloneError ("Promise could not be cloned"). - mergeMap((filePathsArray) => readFiles(filePathsArray) as Promise), - mergeMap((csvArray) => loadCSV(state$.value.pyodide.worker!, csvArray)), - mergeMap(() => filterIIR(state$.value.pyodide.worker!, 1, 30)), - map(() => { - if (!state$.value.experiment.params?.stimuli) { - return {}; + mergeMap(async (filePathsArray) => { + const worker = state$.value.pyodide.worker!; + // readFiles is async — await before posting csvArray to the worker. + // (An unresolved Promise into postMessage throws DataCloneError.) + const csvArray = await readFiles(filePathsArray); + // Queue processing messages in order; the worker runs them sequentially. + loadCSV(worker, csvArray); + filterIIR(worker, 1, 30); + if (state$.value.experiment.params?.stimuli) { + // event_id VALUES must equal the numeric codes written to the CSV Marker + // column (stimulus.type). buildMarkerRegistry keeps this in lockstep with + // collection — array indices silently dropped codes that didn't match. + const { eventId } = buildMarkerRegistry( + state$.value.experiment.params.stimuli + ); + epochEvents(worker, eventId, -0.1, 0.8); } - - // event_id VALUES must equal the numeric codes written to the CSV Marker - // column (stimulus.type). Deriving the map from the shared MarkerRegistry - // keeps it in lockstep with collection — using array indices here silently - // dropped any epoch whose code didn't happen to match an index. - const { eventId } = buildMarkerRegistry( - state$.value.experiment.params.stimuli - ); - - return epochEvents(state$.value.pyodide.worker!, eventId, -0.1, 0.8); - }), - tap((e) => { - console.log('epoched events: ', e); + // Result returns asynchronously via pyodideMessageEpic → SetEpochInfo. + requestEpochsInfo(worker, PYODIDE_VARIABLE_NAMES.RAW_EPOCHS); }), - map(() => PyodideActions.GetEpochsInfo(PYODIDE_VARIABLE_NAMES.RAW_EPOCHS)) + mergeMap(() => EMPTY) ); const loadCleanedEpochsEpic: Epic< @@ -157,9 +166,12 @@ const loadCleanedEpochsEpic: Epic< filter(isActionOf(PyodideActions.LoadCleanedEpochs)), pluck('payload'), filter((filePathsArray) => filePathsArray.length >= 1), - map((epochsArray) => - loadCleanedEpochs(state$.value.pyodide.worker!, epochsArray) - ), + mergeMap(async (epochsArray) => { + // .fif epochs live on the host OS; stage them in Pyodide's MEMFS first + // (the WASM filesystem can't reach host paths). + const { memfsPaths, fsFiles } = await writeEpochsToMemfs(epochsArray); + loadCleanedEpochs(state$.value.pyodide.worker!, memfsPaths, fsFiles); + }), mergeMap(() => of( PyodideActions.GetEpochsInfo(PYODIDE_VARIABLE_NAMES.CLEAN_EPOCHS), @@ -195,20 +207,9 @@ const getEpochsInfoEpic: Epic< action$.pipe( filter(isActionOf(PyodideActions.GetEpochsInfo)), pluck('payload'), - mergeMap( - (varName) => - requestEpochsInfo( - state$.value.pyodide.worker!, - varName - ) as unknown as Promise[]> - ), - map((epochInfoArray) => - epochInfoArray.map((infoObj) => ({ - name: Object.keys(infoObj)[0], - value: infoObj[Object.keys(infoObj)[0]], - })) - ), - map(PyodideActions.SetEpochInfo) + // Fire-and-forget: result returns via pyodideMessageEpic → SetEpochInfo. + tap((varName) => requestEpochsInfo(state$.value.pyodide.worker!, varName)), + mergeMap(() => EMPTY) ); const getChannelInfoEpic: Epic< @@ -218,15 +219,9 @@ const getChannelInfoEpic: Epic< > = (action$, state$) => action$.pipe( filter(isActionOf(PyodideActions.GetChannelInfo)), - mergeMap( - () => - requestChannelInfo( - state$.value.pyodide.worker! - ) as unknown as Promise - ), - map((channelInfoString) => - PyodideActions.SetChannelInfo(parseSingleQuoteJSON(channelInfoString)) - ) + // Fire-and-forget: result returns via pyodideMessageEpic → SetChannelInfo. + tap(() => requestChannelInfo(state$.value.pyodide.worker!)), + mergeMap(() => EMPTY) ); const loadPSDEpic: Epic = ( @@ -245,7 +240,7 @@ const loadTopoEpic: Epic = ( ) => action$.pipe( filter(isActionOf(PyodideActions.LoadTopo)), - tap(() => plotTestPlot(state$.value.pyodide.worker!)), + tap(() => plotTopoMap(state$.value.pyodide.worker!)), mergeMap(() => EMPTY) ); diff --git a/src/renderer/types/electron.d.ts b/src/renderer/types/electron.d.ts index 51baac6c..c97e4e76 100644 --- a/src/renderer/types/electron.d.ts +++ b/src/renderer/types/electron.d.ts @@ -79,6 +79,7 @@ declare global { filename: string ) => Promise; readFiles: (filePathsArray: string[]) => Promise; + readFileAsBytes: (filePath: string) => Promise; // EEG streaming createEEGWriteStream: ( diff --git a/src/renderer/utils/webworker/index.ts b/src/renderer/utils/webworker/index.ts index dfa286a0..0d071bfc 100644 --- a/src/renderer/utils/webworker/index.ts +++ b/src/renderer/utils/webworker/index.ts @@ -43,18 +43,41 @@ export const loadCSV = async (worker: Worker, csvArray: Array) => { // --------------------------- // MNE-Related Data Processing -export const loadCleanedEpochs = async ( +export const loadCleanedEpochs = ( worker: Worker, - epochsArray: string[] + memfsPaths: string[], + fsFiles: Array<{ path: string; bytes: Uint8Array }> ) => { - await worker.postMessage({ + worker.postMessage({ + fsFiles, data: [ - `clean_epochs = concatenate_epochs([read_epochs(file) for file in ${epochsArray}])`, + `clean_epochs = concatenate_epochs([read_epochs(file) for file in ${JSON.stringify(memfsPaths)}])`, `conditions = OrderedDict({key: [value] for (key, value) in clean_epochs.event_id.items()})`, ].join('\n'), }); }; +// .fif epochs live on the host OS filesystem, which Pyodide's WASM FS can't +// reach. Read the bytes via IPC and stage them at /tmp/ in MEMFS; the +// worker writes `fsFiles` into MEMFS before running the read_epochs Python. +export const writeEpochsToMemfs = async ( + filePaths: string[] +): Promise<{ + memfsPaths: string[]; + fsFiles: Array<{ path: string; bytes: Uint8Array }>; +}> => { + const memfsPaths: string[] = []; + const fsFiles: Array<{ path: string; bytes: Uint8Array }> = []; + for (const filePath of filePaths) { + const bytes: Uint8Array = + await window.electronAPI.readFileAsBytes(filePath); + const memfsPath = `/tmp/${path.basename(filePath)}`; + memfsPaths.push(memfsPath); + fsFiles.push({ path: memfsPath, bytes }); + } + return { memfsPaths, fsFiles }; +}; + // NOTE: this command includes a ';' to prevent returning data export const filterIIR = async ( worker: Worker, @@ -88,20 +111,21 @@ export const epochEvents = async ( ].join('\n'), }); -export const requestEpochsInfo = async ( - worker: Worker, - variableName: string -) => { - const pyodideReturn = await worker.postMessage({ +export const requestEpochsInfo = (worker: Worker, variableName: string) => { + // Fire-and-forget: the result comes back on the worker message channel, + // tagged with dataKey, and pyodideMessageEpic routes it to SetEpochInfo. + worker.postMessage({ data: `get_epochs_info(${variableName})`, + dataKey: 'epochsInfo', }); - return pyodideReturn; }; -export const requestChannelInfo = async (worker: Worker) => +export const requestChannelInfo = (worker: Worker) => { worker.postMessage({ data: `[ch for ch in clean_epochs.ch_names if ch != 'Marker']`, + dataKey: 'channelInfo', }); +}; // ----------------------------- // Plot functions @@ -117,7 +141,7 @@ export const plotPSD = async (worker: Worker) => { plotKey: 'psd', data: [ 'import io', - '_fig = raw.plot_psd(fmin=1, fmax=30, show=False)', + '_fig = raw.compute_psd(fmin=1, fmax=30).plot(show=False)', '_buf = io.BytesIO()', '_fig.savefig(_buf, format="svg", bbox_inches="tight")', 'plt.close(_fig)', diff --git a/src/renderer/utils/webworker/webworker.js b/src/renderer/utils/webworker/webworker.js index 2c6b1d95..77d3e7d6 100644 --- a/src/renderer/utils/webworker/webworker.js +++ b/src/renderer/utils/webworker/webworker.js @@ -62,9 +62,7 @@ const pyodideReadyPromise = (async () => { // Set matplotlib backend before any imports so it takes effect on first import. // Must be 'agg' (non-interactive, buffer-based) — web workers have no DOM, // so WebAgg fails with "cannot import name 'document' from 'js'". - await pyodide.runPythonAsync( - 'import os; os.environ["MPLBACKEND"] = "agg"' - ); + await pyodide.runPythonAsync('import os; os.environ["MPLBACKEND"] = "agg"'); // Load micropip so we can install MNE and its pure-Python deps. await pyodide.loadPackage('micropip', { checkIntegrity: false }); @@ -74,12 +72,14 @@ const pyodideReadyPromise = (async () => { // and relative paths — it rejects the pyodide:// custom scheme. // Workaround: JS-fetch each .whl via the protocol handler (which supports it), // write the bytes into Pyodide's emscripten virtual FS, then install via emfs://. - const manifest = await fetch(`${PYODIDE_ASSET_BASE}/packages/manifest.json`) - .then((r) => r.json()); + const manifest = await fetch( + `${PYODIDE_ASSET_BASE}/packages/manifest.json` + ).then((r) => r.json()); for (const { filename } of Object.values(manifest)) { - const buffer = await fetch(`${PYODIDE_ASSET_BASE}/packages/${filename}`) - .then((r) => r.arrayBuffer()); + const buffer = await fetch( + `${PYODIDE_ASSET_BASE}/packages/${filename}` + ).then((r) => r.arrayBuffer()); pyodide.FS.writeFile(`/tmp/${filename}`, new Uint8Array(buffer)); } @@ -100,7 +100,15 @@ self.onmessage = async (event) => { return; } - const { data, plotKey, ...context } = event.data; + const { data, plotKey, dataKey, fsFiles, ...context } = event.data; + + // Write any files to Pyodide's MEMFS before running Python code, so host OS + // paths (e.g. .fif epoch files) can be staged in the WASM virtual filesystem. + if (fsFiles && Array.isArray(fsFiles)) { + for (const { path: filePath, bytes } of fsFiles) { + pyodide.FS.writeFile(filePath, bytes); + } + } // Expose context values as globals so Python can access them via the js module. for (const [key, value] of Object.entries(context)) { @@ -108,8 +116,16 @@ self.onmessage = async (event) => { } try { - self.postMessage({ results: await pyodide.runPythonAsync(data), plotKey }); + let results = await pyodide.runPythonAsync(data); + // Convert PyProxy objects (Python lists/dicts) to plain JS before postMessage, + // which uses structuredClone — a PyProxy is not serializable and would throw. + if (results && typeof results.toJs === 'function') { + const proxy = results; + results = results.toJs({ dict_converter: Object.fromEntries }); + proxy.destroy(); + } + self.postMessage({ results, plotKey, dataKey }); } catch (error) { - self.postMessage({ error: error.message, plotKey }); + self.postMessage({ error: error.message, plotKey, dataKey }); } }; From 7ed3071bd18ce7ae74a5a095458682f660db7196 Mon Sep 17 00:00:00 2001 From: jdpigeon Date: Mon, 6 Jul 2026 19:25:40 -0400 Subject: [PATCH 2/4] =?UTF-8?q?Fix=20NameError:=20name=20'js'=20in=20load?= =?UTF-8?q?=5Fdata=20=E2=80=94=20import=20the=20Pyodide=20js=20module=20lo?= =?UTF-8?q?cally?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit load_data() reads the recorded CSVs from `js.csvArray` (the worker sets self.csvArray before running it), but the `js` module was never imported in that scope — a prior worker refactor dropped the global import that used to make it available. In Pyodide, `js` must be explicitly imported. Import it locally inside the `csv_strings is None` guard so the native-MNE tests, which pass csv_strings, never touch the Pyodide-only module. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01SNEh5EFg4dSpRrfJ2ae4vL --- src/renderer/utils/webworker/utils.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/renderer/utils/webworker/utils.py b/src/renderer/utils/webworker/utils.py index 1c6cba75..023053f8 100644 --- a/src/renderer/utils/webworker/utils.py +++ b/src/renderer/utils/webworker/utils.py @@ -36,9 +36,14 @@ def load_data(sfreq=128., replace_ch_names=None, csv_strings=None): raw : an instance of mne.io.RawArray The loaded data. """ - ## js is loaded in loadPackages - ## TODO: Received attached variable name + ## TODO: Receive attached variable name instead of the fixed js.csvArray if csv_strings is None: + # `js` is Pyodide's proxy for the worker's global scope; webworker.js sets + # self.csvArray before running load_data(). Import it locally (not at module + # top) so the native-MNE tests, which pass csv_strings, never touch the + # Pyodide-only `js` module. A prior worker refactor dropped the global import + # that used to make `js` available here, causing NameError: name 'js'. + import js csv_strings = js.csvArray raw = [] for csv in csv_strings: From 2d0d5a3bed1ab52b6cfcf13241eaa4a185eabd7e Mon Sep 17 00:00:00 2001 From: jdpigeon Date: Mon, 6 Jul 2026 19:32:52 -0400 Subject: [PATCH 3/4] Fix Clean Data: events=False for MNE 1.x + close the returned Figure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit raw_epochs.plot(..., events=None) raised "events must be an instance of bool or ndarray, got None" — MNE 1.x validates the events arg; False is the no-events-overlaid default. Also close the returned Figure so the worker's PyProxy→JS conversion doesn't try to structuredClone it back (DataCloneError); this clean plot isn't routed to the UI. Same MNE-1.x-API-tightening class as the plot_psd → compute_psd fix. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01SNEh5EFg4dSpRrfJ2ae4vL --- src/renderer/utils/webworker/index.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/renderer/utils/webworker/index.ts b/src/renderer/utils/webworker/index.ts index 0d071bfc..bb1a212b 100644 --- a/src/renderer/utils/webworker/index.ts +++ b/src/renderer/utils/webworker/index.ts @@ -132,7 +132,14 @@ export const requestChannelInfo = (worker: Worker) => { export const cleanEpochsPlot = async (worker: Worker) => { await worker.postMessage({ - data: `raw_epochs.plot(scalings='auto', n_epochs=6, title="Clean Data", events=None)`, + // MNE 1.x validates `events` as bool|ndarray — events=None raises TypeError; + // False is the "no events overlaid" default. Also close the returned Figure + // so the worker doesn't try to structuredClone a PyProxy back (this plot is + // not routed to the UI; wiring it to a plotKey would be a separate feature). + data: [ + `_fig = raw_epochs.plot(scalings='auto', n_epochs=6, title="Clean Data", events=False)`, + `plt.close(_fig)`, + ].join('\n'), }); }; From b109fa5b103f34e84c346c1df917ba9d7074c12e Mon Sep 17 00:00:00 2001 From: jdpigeon Date: Mon, 6 Jul 2026 20:02:14 -0400 Subject: [PATCH 4/4] added epoch review ui plan --- docs/epoch-review-ui-plan.md | 251 +++++++++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 docs/epoch-review-ui-plan.md diff --git a/docs/epoch-review-ui-plan.md b/docs/epoch-review-ui-plan.md new file mode 100644 index 00000000..20a0ee6e --- /dev/null +++ b/docs/epoch-review-ui-plan.md @@ -0,0 +1,251 @@ +# Interactive Epoch Review UI — Preliminary Plan + +**Status:** Preliminary / discussion. Written to seed a planning loop — vision and +context first, deep technical decisions deliberately left open (flagged as Open +Questions). Not yet an implementation spec. + +**Owner:** Dano · **Drafted:** 2026-07-06 + +--- + +## 1. Vision + +Replace MNE's native Matplotlib "clean epochs" GUI — which **cannot run in the +Pyodide/WASM port** — with a first-class, fully interactive React experience for +reviewing and cleaning epoched EEG. + +This is not just a feature port. It is a chance to make epoch review **an +onboarding experience to EEG signal analysis** as much as a tool. The target +user is a student who may be seeing epoched EEG for the first time. The UI should +teach *what an epoch is, what artifacts look like, and why we reject them* while +they do the work — in the app's lighthearted, student-friendly voice (per +`.llms/CLAUDE.md`). It must also be a genuinely capable tool an experienced user +trusts. Both, not either. + +It should be **fully featured and interactive** — at least at parity with what +MNE's GUI offered, and ideally beyond it (guided artifact detection, live ERP +feedback, explanations) because we control the whole surface. + +--- + +## 2. Why this exists — the WASM gap + +BrainWaves runs its analysis (MNE-Python) **in-browser via Pyodide/WASM inside a +web worker** (see the `pyodide-mne` skill and `docs/pyodide-in-electron-vite.md`). +That environment has hard constraints: + +- The worker uses Matplotlib's **`agg` backend** — headless, buffer-based. There + is **no GUI event loop and no window**. WebAgg (Matplotlib's browser backend) + does not work in a worker context at all. +- Every *static* plot we render today (PSD, topo map, ERP) works by having Python + `savefig()` an **SVG string**, tagging it with a `plotKey`, and shipping that + string back over the worker message channel to render in `PyodidePlotWidget`. +- `epochs.plot()` is the **only** analysis call that tried to be *interactive*. + It returns a live Matplotlib Figure object, which (a) has no GUI to display in, + and (b) can't even cross the worker boundary — a `PyProxy`/Figure isn't + `structuredClone`-able, so `postMessage` throws `DataCloneError`. + +**This is not an MNE-version problem.** We are already on **MNE 1.12.1** (current). +The recent errors (`plot_psd` → `compute_psd`, `events=None` → `events=False`) are +just the code catching up to the modern MNE API; bumping MNE won't remove them. +The interactive-GUI problem is architectural: **native Matplotlib GUIs don't exist +in WASM, and no MNE version changes that.** The only path to interactive epoch +review is to build the UI ourselves — which is what this plan is for. + +--- + +## 3. What MNE's Matplotlib GUI did (the experience to replicate) + +`epochs.plot()` opened an interactive browser. Reference behavior: + +**Layout** +- Epochs laid out **side by side horizontally**; channels **stacked vertically**. +- Vertical divider lines between epochs; epoch index labels along the bottom. +- A configurable window of `n_epochs` visible at once (our call used 6). +- Epochs colored by **event/condition** (our numeric marker codes → conditions), + with a legend. +- Event markers drawn as vertical lines within each epoch. + +**Navigation** +- Scroll through epochs (←/→ / Page keys) and channels (↑/↓). +- Amplitude **scaling** up/down (`scalings='auto'` set the initial scale). +- Butterfly mode, DC-removal toggle, annotation toggle, help overlay. + +**The core action — rejection** +- **Click an epoch** → toggle it "bad" (visually greyed). On window close, the + marked epochs are **dropped** (`epochs.drop(indices)`). +- **Click a channel label** → mark the channel bad (`epochs.info['bads']`), + excluding it from downstream analysis. + +**Result** +- On close, the Epochs object is mutated: bad epochs dropped, bad channels + flagged. That cleaned object is what gets saved (`-cleaned-epo.fif`) and fed to + topo/PSD/ERP. + +**Adjacent MNE capability we should exploit** +- MNE can **auto-flag** epochs via peak-to-peak `reject` thresholds, recording + *why* each was dropped in the **`drop_log`**. This is gold for onboarding: + "we flagged these as likely blinks — here's the trace, do you agree?" + +--- + +## 4. What "cleaning" actually means (and a caveat) + +Cleaning = **removing artifact-contaminated epochs and bad channels** so the +average (ERP) reflects brain activity, not blinks/muscle/movement. Common +artifacts a student should learn to recognize: + +- **Eye blinks** — large, slow frontal deflections (AF7/AF8 on Muse). +- **Muscle/EMG** — high-frequency bursts. +- **Movement/drift** — slow baseline wander. +- **Electrode pop** — sudden step in one channel. + +**Caveat that raises the stakes:** filtering already happens at load (`filterIIR` +1–30 Hz). So *without* interactive rejection, today's "Clean Data" does no real +cleaning — it just re-saves every epoch. The interactive review **is** the +feature; there's no meaningful non-interactive fallback. (A read-only static SVG +of the epochs is possible as a stopgap, but it can't clean anything.) + +--- + +## 5. Design goals + +1. **Onboarding-first.** A newcomer should leave understanding epochs and + artifacts. Explanations, a guided mode, plain-language cues — not jargon walls. +2. **Fully featured & interactive.** Parity-plus with MNE's GUI: reject epochs, + flag channels, scroll, scale, zoom, condition coloring. +3. **A tool experts trust.** Auto-suggestions must be overridable; nothing hidden + or magic; the resulting cleaned data must be exactly what MNE would produce. +4. **Live feedback.** Ideally, show the **ERP updating as you reject** — the + single most powerful teaching moment (see the signal get cleaner). +5. **Hackable & extensible.** Fits the CLAUDE.md horizon (block-based programming, + embedded notebooks). Don't leak MNE/Pyodide abstractions into the UI. +6. **Device-agnostic.** Works for 4-ch Muse today, N-ch (Neurosity, external LSL) + tomorrow. + +--- + +## 6. Replicating it in React — the pieces + +Four concerns. Each has real design choices (deferred to the planning loop). + +### 6a. Data path: Python → renderer +We stop shipping a *GUI object* and instead ship the **raw numbers**, then render +them ourselves. Needed from Python per "clean" request: +- Epoch data array: `epochs.get_data()` → shape `(n_epochs, n_channels, n_times)`. +- Metadata: `ch_names`, `sfreq`, `times`, per-epoch condition (from the marker + registry), and the **auto-reject flags + `drop_log`** for suggestions. + +**Transport is the crux** (and the lesson from this whole debugging session): +don't JSON-serialize float arrays, and don't return a `PyProxy`. Get the numpy +buffer as a **`Float32Array` / `ArrayBuffer`** and send it as a **transferable** +over `postMessage` (zero-copy). Small metadata rides as JSON alongside. This +sidesteps the serialization wall entirely — buffers cross cleanly; Figures don't. + +### 6b. Rendering +Muse is tiny (4 ch × ~256 samples × dozens of epochs), but we must not design for +4 channels — Neurosity (8) and external LSL devices (32–64) are on the roadmap. +- **Canvas 2D** is the pragmatic default (smooth for thousands of points). +- **WebGL** if channel/epoch counts get large. +- **SVG/DOM overlay** on top for interaction targets (epoch columns, channel + labels, tooltips, selection highlights) — cheap hit-testing and accessibility. +- Likely a **hybrid**: canvas traces + DOM/SVG interaction layer. + +### 6c. Interaction model (parity-plus with MNE) +- Click/tap an epoch → toggle reject (clear visual state). +- Click a channel label → toggle bad channel. +- Scroll/scrub epochs and channels; amplitude scaling; horizontal zoom. +- Condition coloring + legend. +- **Beyond MNE:** hover tooltips ("this looks like a blink"), a "reject all + flagged" / "review flagged only" mode, keyboard-first flow, undo. + +### 6d. Apply path: renderer → Python +- Collect rejected epoch indices + bad channels → dispatch an action → epic posts + to the worker → Python `epochs.drop(indices)` + set `info['bads']` → save + `-cleaned-epo.fif` via the existing MEMFS/`saveEpochs` path. +- The result must be **bit-identical to what MNE's GUI would have produced** — the + UI changes, the science does not. + +### 6e. The onboarding layer (the differentiator) +- Plain-language explanations of epochs and each artifact type. +- **Guided mode** (default for newcomers): step through auto-flagged epochs with + "why we flagged this," student confirms/overrides → teaches artifact spotting. +- Channel legend tied to head position (Muse 10-20: TP9/AF7/AF8/TP10). +- **Live ERP preview** pane that updates as epochs are rejected. +- Tone: encouraging, lighthearted, student-facing. + +--- + +## 7. Architecture fit & constraints + +- **Worker protocol** (`pyodide-mne` skill): today's plots use fire-and-forget + `postMessage` + `plotKey`/`dataKey` reply routing through `pyodideMessageEpic`. + A data-heavy request/response (fetch epoch arrays, get result back, then let the + user act) pushes toward finally doing the **`runPython` RPC** already tracked in + `TODOS.md` ("Optional Full Pyodide worker RPC"). This feature is the strongest + reason yet to build it — worth deciding early. +- **New Python helpers** (`webworker/utils.py`): `get_epochs_arrays(epochs)` → + buffer + metadata; `apply_rejection(epochs, drop_indices, bad_channels)`. Keep + them native-testable (the `tests/analysis/` pattern) so cleaning logic is + verified against real MNE in CI. +- **New epic(s)** in `pyodideEpics.ts` + **new actions** (fetch/set epoch data, + apply rejection). Reuse `buildMarkerRegistry` for condition labels/colors. +- **New React component** (`EpochReviewer` or similar) replacing the plot area in + `CleanComponent`. Styling: shadcn/ui + Tailwind, brand teal, student tone. +- **Constraints to respect:** headless `agg` in the worker; no large-array JSON; + transferables for buffers; keep main/renderer/worker separation clean; don't + leak Pyodide/MNE types into React. + +--- + +## 8. Open questions (for the planning loop) + +1. **Rendering tech** — Canvas 2D now, or WebGL up front for future high-channel + devices? +2. **RPC first?** — Do we build the `runPython` request/response RPC as a + prerequisite (cleaner data fetch), or bolt this onto the current fire-and-forget + `dataKey` pattern? +3. **Onboarding depth** — Is guided mode the default? How much curriculum + (tooltips only vs. a real walkthrough)? +4. **Auto-rejection** — Expose peak-to-peak thresholds to the user, or keep them + as invisible "suggestions"? What defaults? +5. **Bad channels on Muse** — dropping 1 of 4 channels is drastic; do we support + channel rejection for low-channel devices, or epochs-only there? +6. **Live ERP preview** — in-scope for v1 (big teaching win, more compute) or a + fast-follow? +7. **Static fallback** — keep a read-only SVG epochs view for environments where + the interactive UI can't run, or all-in on the React UI? + +--- + +## 9. Rough phases (to be firmed up in planning) + +- **Phase 0 — Transport & read-only render.** Ship epoch arrays + metadata across + the worker boundary (transferable buffers); render static traces in React. + Proves the data path and rendering choice. +- **Phase 1 — Core interaction.** Click-to-reject epochs, scroll/scale/zoom, apply + → `epochs.drop` → save `-cleaned-epo.fif`. Reaches functional parity with the + *essential* MNE workflow. +- **Phase 2 — Full parity.** Bad-channel flagging, condition coloring/legend, + auto-flag suggestions from `drop_log`/peak-to-peak. +- **Phase 3 — Onboarding layer.** Explanations, guided mode, artifact tutorials, + live ERP preview. +- **Phase 4 — Polish & generalize.** N-channel devices (Neurosity/LSL), + accessibility, keyboard flow, performance. + +--- + +## 10. Context references + +- **Skills:** `pyodide-mne` (worker↔Python protocol, plot routing, `pyodide://`), + `redux-observable-epochs` (epic anatomy, numeric marker-code contract). +- **Docs:** `docs/pyodide-in-electron-vite.md`, `docs/user-flow.md`. +- **Learnings** (`.llms/learnings.md`): agg backend / WebAgg-in-worker limits; + plot-result routing pattern; PyProxy serialization; marker registry / numeric + event codes; the analysis pipeline testability pattern. +- **Code today:** `src/renderer/utils/webworker/{index.ts,webworker.js,utils.py}` + (worker + Python), `src/renderer/epics/pyodideEpics.ts` (epics), + `src/renderer/components/CleanComponent/`, `PyodidePlotWidget.tsx` (existing + static-plot render path), `src/renderer/utils/eeg/markerRegistry.ts`. +- **Related TODO:** `TODOS.md` → "Optional Full Pyodide worker RPC" (this feature + is the strongest motivation to build it).