From 5c24d1ec7e8d39431c13465432c951367e6f5e60 Mon Sep 17 00:00:00 2001 From: jdpigeon Date: Tue, 18 Aug 2026 12:54:09 -0400 Subject: [PATCH 1/6] docs: add frontier-smol-handoff skill design spec Draft spec for the project skill that maps OMP prewalk and @smol/@slow roles onto explore-then-handoff. Skill file not written yet. --- ...2026-08-18-frontier-smol-handoff-design.md | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-18-frontier-smol-handoff-design.md diff --git a/docs/superpowers/specs/2026-08-18-frontier-smol-handoff-design.md b/docs/superpowers/specs/2026-08-18-frontier-smol-handoff-design.md new file mode 100644 index 00000000..27321594 --- /dev/null +++ b/docs/superpowers/specs/2026-08-18-frontier-smol-handoff-design.md @@ -0,0 +1,74 @@ +# frontier-smol-handoff + +Date: 2026-08-18 +Status: draft +Type: project skill (`.claude/skills/frontier-smol-handoff/SKILL.md`) + +## Problem + +Agents one-shot implementation, or they write a plan in plan mode and hand that sterile plan to a cheap model. Both waste frontier tokens or starve the cheap model of live exploration context. + +The intended workflow: frontier explores, writes a todo list, starts only when confident, is stopped after the first code edit, then a cheap executor continues from the **same session transcript**. + +## Decision + +Do not invent a custom two-phase protocol. Encode the workflow as a hard-gate skill that uses OMP built-in prewalk and model roles. + +| Role | OMP alias | Job | +|---|---|---| +| Frontier | the session's active model at invoke (`@default` or `@slow`) | Explore, todo, first directional edit | +| Cheap executor | `@smol` (`--prewalk-into`, default) | Continue from the live transcript | + +Arming (any one is enough): + +- `omp --prewalk` (optional `--prewalk-into=@smol`) +- `prewalk.enabled` in settings +- agent frontmatter `prewalk: true` or `prewalk: "@smol"` +- `task.prewalk` / `task.agentPrewalk` for spawned `task` agents + +Handoff mechanics are harness-owned: the first workspace-mutating `edit`/`write` (source, not `todo` / notes / `local://`) after a committed todo list switches the session model to the prewalk target. The cheap model inherits the transcript. No plan file. + +## Anti-patterns (hard forbid) + +- One-shot the whole task on frontier +- Plan mode or `--plan-yolo` then hand a written plan to `@smol` +- Spawn cheap `task` agents with a plan and no exploration context +- Frontier keeps editing after the first mutating write +- `@smol` starts a new architecture after the switch + +Plan mode clears prewalk. If plan mode is on, this skill does not apply. + +## Hard gates + +1. No workspace-mutating `edit`/`write` until a phased todo list exists (named phases, concrete tasks, not "implement the feature") and exploration is declared complete. +2. First workspace-mutating tool call is the last frontier action. Keep it small and directional. +3. After the switch, `@smol` executes the existing todo. No new research, no new design. +4. If prewalk is not armed when the skill is invoked: tell the user to enable it, or stop after the first edit and ask them to switch to `@smol`. Do not keep going on frontier. + +## Trigger + +Explicit only. User invokes `/skill:frontier-smol-handoff` or names the workflow. Not auto-loaded. + +## Skill shape + +Single file: `.claude/skills/frontier-smol-handoff/SKILL.md`. + +- Frontmatter `name` + `description` (when-to-use only; no workflow summary) +- Role table +- Linear steps + the four hard gates +- Rationalization table and red flags (discipline skill) +- Contrast with `--plan-yolo` +- No extra agent definition, no project `prewalk.enabled` flip unless the user asks + +## Testing (writing-skills TDD) + +Discipline skill. RED: pressure scenarios without the skill. GREEN: same scenarios with the skill. REFACTOR: close loopholes from observed rationalizations. + +Expected baseline failures: one-shot under time pressure; plan-then-smol because it looks cheaper; keep editing on frontier after the first change; hand a written plan to a `task` spawn. + +## Non-goals + +- Changing OMP defaults for this repo +- A custom project agent +- Auto-triggering on every multi-step task +- Replacing `writing-plans` / `executing-plans` for users who want those From 2c2d03a7892847c971d5563a75e3d824fa1a1932 Mon Sep 17 00:00:00 2001 From: jdpigeon Date: Wed, 19 Aug 2026 18:05:40 -0400 Subject: [PATCH 2/6] =?UTF-8?q?refactor:=20class=20components=20=E2=86=92?= =?UTF-8?q?=20function=20components?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converts all 20 remaining class components in src/renderer/components/ to function components with hooks. Conversion-only — no behavior changes, no prop shape changes, no file moves. Components converted: PreviewButton, PreviewExperiment, PyodidePlotWidget, InputCollect, InputModal, CleanSidebar, HelpSidebar, PreTestComponent, SecondaryNav, Design, Home, Analyze, Clean, Collect, EEGExploration, ConnectModal, SignalQualityIndicator, Viewer, StimuliDesignColumn, CustomDesign Key judgement conversions: - Viewer: 7 useEffect hooks matching class lifecycle ordering - CustomDesign: useRef for conditionParams + conditionRevision - ConnectModal: useMemo debounces with leading:true, trailing:false - SignalQualityIndicator: propsRef for live-PLS interval in D3 subscription - StimuliDesignColumn: React.memo with custom compare Also: - Removed @babel/plugin-proposal-decorators and @babel/plugin-proposal-class-properties (zero decorators, class properties native everywhere) - Added 6 test files covering the riskier conversions - Updated vitest.config.ts and vite.config.ts to drop babel plugin blocks --- docs/class-to-function-plan.md | 620 +++++++++++--- package-lock.json | 107 --- package.json | 2 - src/renderer/components/AnalyzeComponent.tsx | 761 +++++++++--------- .../CleanComponent/CleanSidebar.tsx | 101 +-- .../__tests__/CleanRejections.test.tsx | 118 +++ .../components/CleanComponent/index.tsx | 349 ++++---- .../CollectComponent/ConnectModal.tsx | 233 +++--- .../CollectComponent/HelpSidebar.tsx | 107 +-- .../CollectComponent/PreTestComponent.tsx | 154 ++-- .../__tests__/CollectModal.test.tsx | 94 +++ .../components/CollectComponent/index.tsx | 138 ++-- .../DesignComponent/CustomDesignComponent.tsx | 619 +++++++------- .../DesignComponent/StimuliDesignColumn.tsx | 263 +++--- .../__tests__/CustomDesignComponent.test.ts | 58 -- .../__tests__/CustomDesignComponent.test.tsx | 109 +++ .../__tests__/StimuliDesignColumn.test.tsx | 73 +- .../components/DesignComponent/index.tsx | 241 +++--- .../components/EEGExplorationComponent.tsx | 176 ++-- .../components/HomeComponent/index.tsx | 234 +++--- src/renderer/components/InputCollect.tsx | 199 +++-- src/renderer/components/InputModal.tsx | 110 ++- .../components/PreviewButtonComponent.tsx | 24 +- .../components/PreviewExperimentComponent.tsx | 48 +- src/renderer/components/PyodidePlotWidget.tsx | 66 +- .../SecondaryNavComponent/index.tsx | 95 ++- .../SignalQualityIndicatorComponent.test.tsx | 65 ++ .../SignalQualityIndicatorComponent.tsx | 72 +- .../components/ViewerComponent.test.tsx | 29 + src/renderer/components/ViewerComponent.tsx | 216 +++-- vite.config.ts | 9 +- vitest.config.ts | 11 +- 32 files changed, 2953 insertions(+), 2548 deletions(-) create mode 100644 src/renderer/components/CleanComponent/__tests__/CleanRejections.test.tsx create mode 100644 src/renderer/components/CollectComponent/__tests__/CollectModal.test.tsx delete mode 100644 src/renderer/components/DesignComponent/__tests__/CustomDesignComponent.test.ts create mode 100644 src/renderer/components/DesignComponent/__tests__/CustomDesignComponent.test.tsx create mode 100644 src/renderer/components/SignalQualityIndicatorComponent.test.tsx create mode 100644 src/renderer/components/ViewerComponent.test.tsx diff --git a/docs/class-to-function-plan.md b/docs/class-to-function-plan.md index a93e0d1e..6fc76b8e 100644 --- a/docs/class-to-function-plan.md +++ b/docs/class-to-function-plan.md @@ -1,145 +1,505 @@ # Plan: class components → function components -## What's actually broken +Rip sheet. Conversion only. One branch, one PR. -The stated pain is *"wasting too much time reading through every piece of this app."* -That is not caused by `class` — it's caused by file size and prop indirection: +**Non-goals:** no component splitting, no container edits, no file moves, no prop-shape changes, no `useCallback` / `useMemo` (except the two cases named below), no new hooks, no renaming the `Home` class in `EEGExplorationComponent.tsx`. -| Symptom | Cause | Fixed by | -|---|---|---| -| 644-line `CustomDesignComponent.tsx` | `renderSectionContent()` is a 400-line method | splitting render methods into sibling files | -| "where does `deviceType` come from?" | `connect()` container → 15 spread props | `useSelector` at point of use | -| "will this break?" | 20 components, ~4200 lines, ~3 renderer component tests | one characterization test per conversion | +Do not touch `src/renderer/containers/*` (already functions). Do not touch `src/renderer/components/d3Classes/EEGViewer.js` (not a React component). Leave `SettingsDropdown` and `HelpButton` — they are already functions. + +--- + +## Rip rules + +Apply in every file. Same diff shape everywhere. + +| Class | Function | +|---|---| +| `export default class X extends Component` | `export default function X(props: Props)` | +| `class X … export default X` | `export default function X(props: Props)` | +| `export class HelpSidebar` | `export function HelpSidebar` — keep the named export | +| `extends PureComponent` | plain function. Do not add `React.memo` | +| `constructor` state init | **one `useState` per field** | +| `this.state.foo` | `foo` | +| `this.setState({ foo })` | `setFoo(foo)` | +| `this.setState({ foo, bar })` | `setFoo(foo); setBar(bar);` — React 18 batches | +| `this.setState(prev => ({ foo: … }))` | `setFoo(prev => …)` — drop the object wrapper | +| `this.setState(prev => ({ …prev, foo }))` | `setFoo(…)` only; other fields are other hooks | +| `this.props.foo` | `props.foo` — **do not destructure** `props` in the signature | +| `handleX() {}` / `handleX = () => {}` | `function handleX() {}` in the body | +| `renderX() {}` | `function renderX() {}` in the body, same file | +| `static foo()` | module-level `function foo()` above the component | +| `componentDidMount` | `useEffect(() => {…}, [])` | +| async `componentDidMount` | same; `void` the promise or use an inner async fn. Guard `setState` after unmount with a `cancelled` flag | +| `componentWillUnmount` | cleanup return of that same `[]` effect | +| `componentDidUpdate` guarded on one prop | `useEffect(() => {…}, [thatProp])` | +| instance field that is a constant from initial props | `useState(() => …)` and never set it | +| instance field that is mutable across renders (`conditionParams`, subscriptions, DOM nodes) | `useRef` | +| `debounce(this.handleX.bind(this), n)` in the constructor | `useMemo(() => debounce(function handleX() {…}, n, opts), [])` plus `useEffect(() => () => handleX.cancel(), [handleX])` | + +**Do not** keep a single `useState` object. Class `setState` merges; hook `setState` replaces. Per-field setters cannot wipe sibling fields. + +**Do not** add `useCallback` / `useMemo` / `React.memo` except: + +1. Debounced handlers (above) — `useMemo` holds the lodash instance so it is not recreated every render. +2. `StimuliDesignColumn` — `React.memo` with a compare that matches the existing `shouldComponentUpdate`. The file exists for that skip. See its card. + +Keep `Props` / `State` interfaces. `State` documents the `useState` group. + +Keep the default-vs-named export style of the file you are in. Keep the function name equal to the class name (`Analyze`, `Home`, `Clean`, `Collect`, `Design`, `CustomDesign`, `PreviewButton`, …). Default-export local name does not matter to importers; don't rename. + +Import `useState` / `useEffect` / `useRef` / `useMemo` from `'react'` as needed. Drop `Component` / `PureComponent`. + +--- + +## Conversion order + +Do not skip ahead. Tests land first so a silent behavior change fails the file you are about to touch. + +### A. Tests (against the current classes) + +Write these before converting the matching file. If the test needs edits after conversion, behavior changed — stop and fix the component, not the test. + +Style: `src/renderer/components/CleanComponent/__tests__/EpochReviewer.test.tsx`. `render()`, 2–4 behavioral assertions, no snapshots, no `Provider`. + +**A1. Rewrite `CustomDesignComponent.test.ts` first.** + +It currently does `new CustomDesign(makeProps())` and assigns `design.setState`. That dies the moment the class is gone. Replace with RTL before converting the component. + +The contract to keep: an older `handleConditionChange('dir', …)` must not overwrite a newer `handleConditionChange('title', …)`. Drive it through the UI (change the folder, then the title, then resolve the deferred `readImages`). Assert `stimulus1.title` is still the newer title. Same mocks (`finishRead` deferred `readImages`) stay. + +**A2. `SignalQualityIndicatorComponent`** — new test file. + +- `Subject` as `signalQualityObservable`. After mount, `subject.observed === true`. +- `unmount()` → `subject.observed === false`. +- `rerender` with a new `Subject` → old unsubscribed, new observed. +- Do **not** assert `setSignalQuality` — there is no React state. The subscribe body paints D3 on `#${channelId}`. + +**A3. `CollectComponent/index`** — new test file. + +- `isEEGEnabled` + not `CONNECTED` → connect modal opens on mount (`handleStartConnect`). +- `rerender` with `connectionStatus={CONNECTION_STATUS.CONNECTED}` → modal closes. +- Need dummy `DeviceActions` / `ExperimentActions` / device props; look at `Props` in the file. + +**A4. `CleanComponent/index`** — new test file. + +- Render with `suggestedRejections={[]}`. +- `rerender` with `suggestedRejections={[{ index: 2 }, { index: 5 }]}`. +- Those indices appear in whatever the review UI uses to mark rejected epochs (the `rejected` set passed to `EpochReviewer`). Stub `readWorkspaceRawEEGData` so mount does not hit the filesystem. + +**A5. `StimuliDesignColumn.test.tsx`** — extend, do not replace. + +Existing test: mount with `audioDir="/tones"` shows `( 2 sounds )`. +Add: `rerender` with a different `audioDir`; `readAudioFiles` resolves a different list; the label updates. + +**A6. `ViewerComponent`** — new test file. jsdom cannot exercise `` IPC or `dom-ready`. Do not pretend. + +- Mock `window.electronAPI.getViewerUrl` → resolve `'http://viewer.local'`. +- First paint: `container` is empty (`viewerUrl === ''` → `return null`). +- After the promise: a `webview` with that `src`. +- Playtest of channels / domain / autoScale / signal-quality IPC is Electron-only (`npm run dev`, desktop window, not `:5173`). + +Skipped: Storybook, MSW, snapshots, coverage target, tests for the other 14 files. `npm run typecheck` is the net for those. + +--- + +### B. Easy files — table rules only + +Convert top to bottom. After each file: it must typecheck in isolation (no leftover `this.`, no `Component` import). + +| # | File | Lines | State fields | Notes | +|---|---|---|---|---| +| 1 | `PreviewButtonComponent.tsx` | 24 | none | `PureComponent` → function. Drop `Pure`. | +| 2 | `PreviewExperimentComponent.tsx` | 49 | none | `static insertPreviewLabJsCallback` → module-level function. `handleImages` is unused in render except… it is **never called**. Leave the function in the body; do not delete. | +| 3 | `PyodidePlotWidget.tsx` | 87 | none | Constructor only binds. Two handlers + render. | +| 4 | `InputCollect.tsx` | 155 | `subject`, `group`, `session`, `isSubjectError`, `isSessionError` | `this.setState({ [field]: … })` is already a `switch` — each branch calls the matching setter. | +| 5 | `CleanComponent/CleanSidebar.tsx` | 172 | `helpStep` | Menu / next / back. | +| 6 | `CollectComponent/HelpSidebar.tsx` | 191 | `helpStep` | **Named export.** `HelpButton` below it stays. Updater form: `setHelpStep(prev => prev + 1)` / `prev - 1`. | +| 7 | `CollectComponent/PreTestComponent.tsx` | 175 | `isPreviewing`, `isSidebarVisible` | `componentDidMount`/`WillUnmount` bind/unbind Mousetrap `esc`. Updater form on preview + sidebar toggle. | +| 8 | `SecondaryNavComponent/index.tsx` | 116 | none | **Drop `shouldComponentUpdate`.** Do not wrap in `memo`. The SCU only compared `activeStep`, so title / `saveButton` / `enableEEGToggle` already could not update — that is a latent skip, not a behavior we keep. `SettingsDropdown` stays. | +| 9 | `DesignComponent/index.tsx` | 319 | `activeStep`, `isPreviewing`, `isNewExperimentModalOpen`, `recentWorkspaces` | async mount: `setRecentWorkspaces(await readWorkspaces())`. One updater on preview toggle. | +| 10 | `HomeComponent/index.tsx` | 379 | `activeStep`, `recentWorkspaces`, `workspaceStates`, `isNewExperimentModalOpen`, `isOverviewComponentOpen`, `overviewExperimentType` | async mount launches Pyodide then reads workspaces. `loadWorkspaceStates` stays a function in the body. | +| 11 | `AnalyzeComponent.tsx` | 560 | 16 fields — see constructor | Biggest mechanical file. Several handlers set 2–3 fields at once; emit 2–3 setters. async mount reads cleaned EEG + behavior. | +| 12 | `CleanComponent/index.tsx` | 471 | 10 fields + `icons` | `icons` is a constructor-only instance field from `props.type`. `const [icons] = useState(() => props.type === EXPERIMENTS.N170 ? ['😊', '🏠', '✕', '📖'] : ['★', '☆', '✕', '📖']);` — never set. `componentDidUpdate` on `suggestedRejections` → `useEffect` that `setRejectedEpochs(prev => { const next = new Set(prev); for (const s of suggested) next.add(s.index); return next; })`. | + +--- + +### C. Judgement files — do these by hand, in this order + +#### C1. `InputModal.tsx` (84) — debounce + +Constructor: + +```ts +this.handleTextEntry = debounce(this.handleTextEntry, 100).bind(this); +``` + +Target: + +```tsx +const handleTextEntry = useMemo( + () => + debounce((event: React.ChangeEvent) => { + setEnteredText(event.target.value); + }, 100), + [] +); +useEffect(() => () => handleTextEntry.cancel(), [handleTextEntry]); +``` + +Default lodash trailing debounce. Do not change the 100ms. Other handlers are plain functions. -Class→function is the **vehicle** for those three, not the goal. Converting a class to a -function and leaving a 644-line file behind buys nothing. +#### C2. `ConnectModal.tsx` (306) — debounce + `UNSAFE_componentWillUpdate` -## What already exists (don't rebuild it) +Two constructor debounces (preserve timings and `{ leading: true, trailing: false }`): -Testing "fixtures" are already installed and working — **nothing to add here**: +```tsx +const propsRef = useRef(props); +propsRef.current = props; + +const handleSearch = useMemo( + () => + debounce(function handleSearch() { + setInstructionProgress(0); + propsRef.current.DeviceActions.SetDeviceAvailability( + DEVICE_AVAILABILITY.SEARCHING + ); + }, 300, { leading: true, trailing: false }), + [] +); +const handleConnect = useMemo( + () => + debounce(function handleConnect() { + /* body of handleConnect, read props via propsRef.current */ + }, 1000, { leading: true, trailing: false }), + [] +); +useEffect( + () => () => { + handleSearch.cancel(); + handleConnect.cancel(); + }, + [handleSearch, handleConnect] +); +``` + +`static getDeviceName` → module-level `function getDeviceName`. Call sites currently `ConnectModal.getDeviceName(…)` become `getDeviceName(…)`. + +`UNSAFE_componentWillUpdate` runs **before** the render that sees the new `deviceAvailability`, so `instructionProgress` updates in the same paint. `useEffect` runs after; one extra frame. Accept that. + +```tsx +useEffect(() => { + if (props.deviceAvailability === DEVICE_AVAILABILITY.NONE) { + setInstructionProgress(INSTRUCTION_PROGRESS.TURN_ON); // 1 + } +}, [props.deviceAvailability]); +``` + +**Stop. That is not equivalent.** The class only fires on the *transition*: + +- `SEARCHING → NONE` → `instructionProgress = 1` (`TURN_ON`) +- `NONE → AVAILABLE` → `instructionProgress = 0` (`SEARCHING`) + +`handleSearch` also sets progress to `0` independently, so progress is not a pure function of `deviceAvailability`. Use a ref for the previous value: + +```tsx +const prevAvailability = useRef(props.deviceAvailability); +useEffect(() => { + const prev = prevAvailability.current; + const next = props.deviceAvailability; + prevAvailability.current = next; + if (next === DEVICE_AVAILABILITY.NONE && prev === DEVICE_AVAILABILITY.SEARCHING) { + setInstructionProgress(INSTRUCTION_PROGRESS.TURN_ON); + } + if (next === DEVICE_AVAILABILITY.AVAILABLE && prev === DEVICE_AVAILABILITY.NONE) { + setInstructionProgress(INSTRUCTION_PROGRESS.SEARCHING); + } +}, [props.deviceAvailability]); +``` + +`componentDidMount` LSL probe stays a `[]` effect. + +#### C3. `SignalQualityIndicatorComponent.tsx` (70) + +There is **no React state**. The first-draft snippet `subscribe(setSignalQuality)` is wrong. The subscribe body paints D3: + +```ts +d3.select(`#${key}`) + .transition() + .duration(this.props.plottingInterval) // live `this.props` at fire time +``` + +Class also does **not** unsubscribe when the observable becomes `null` — `didUpdate` only resubscribes when the new value is non-null. Preserve that (skip the effect body when null; do not add a cleanup that runs on the null transition unless you also skip registering an effect). Simplest faithful form: + +```tsx +export default function SignalQualityIndicatorComponent(props: Props) { + const propsRef = useRef(props); + propsRef.current = props; + const subRef = useRef(null); + + useEffect(() => { + const observable = props.signalQualityObservable; + if (observable == null) return; + subRef.current?.unsubscribe(); + subRef.current = observable.subscribe( + (epoch) => { + Object.keys(epoch.signalQuality).forEach((key) => { + d3.select(`#${key}`) + .attr('visibility', 'show') + .attr('stroke', '#000') + .transition() + .duration(propsRef.current.plottingInterval) + .ease(d3.easeLinear) + .attr('fill', epoch.signalQuality[key]); + }); + }, + (error) => new Error(`Error in signalQualitySubscription ${error}`) + ); + }, [props.signalQualityObservable]); + + useEffect(() => () => { + subRef.current?.unsubscribe(); + }, []); + + return ( +
+ +
+ ); +} +``` -- `vitest` + `jsdom` + `globals: true` (`vitest.config.ts`) -- `@testing-library/react` v16, `@testing-library/dom`, `@testing-library/jest-dom` -- `src/test-setup.ts` wired as `setupFiles` -- `eslint-plugin-react-hooks` `recommended-latest` already on (`eslint.config.mjs:47`) — - the exhaustive-deps safety net for hooks is live from day one -- Good precedent test to copy: `src/renderer/components/CleanComponent/__tests__/EpochReviewer.test.tsx` +Do **not** extract a shared `useObservable` hook for this and Viewer. -The only new test helper worth writing is **one** file (see step 0). +#### C4. `CollectComponent/index.tsx` (138) and `EEGExplorationComponent.tsx` (144) -## Inventory +Same modal-close pattern. Class compares **previous state**: -20 classes, 4255 lines. Grouped by difficulty: +```ts +if (this.props.connectionStatus === CONNECTION_STATUS.CONNECTED && prevState.isConnectModalOpen) { + this.setState({ isConnectModalOpen: false }); +} +``` -**Tier A — trivial (`setState` + render only, no lifecycle).** Mechanical, ~15 min each. -`PreviewButtonComponent` (24), `PreviewExperimentComponent` (49), `InputModal` (84), -`PyodidePlotWidget` (87), `SecondaryNavComponent` (116), `CollectComponent/index` (138), -`HelpSidebar` (191), `CleanSidebar` (172), `PreTestComponent` (175), `InputCollect` (155) +```tsx +useEffect(() => { + if (props.connectionStatus === CONNECTION_STATUS.CONNECTED) { + setIsConnectModalOpen(false); + } +}, [props.connectionStatus]); +``` -**Tier B — has `componentDidMount`/`componentDidUpdate`, still simple state.** -`ConnectModal` (306), `StimuliDesignColumn` (203), `EEGExplorationComponent` (144), -`DesignComponent/index` (319) +Equivalent: the guard only skipped a redundant `setState(false)`. Setting state to the current value is a React no-op. -**Tier C — RxJS subscription lifecycle.** The only genuinely interesting ones. -`SignalQualityIndicatorComponent` (70), `ViewerComponent` (148) +Collect also auto-opens the modal on mount when EEG is on and not connected — that stays a `[]` effect calling `handleStartConnect`. -**Tier D — big, must be split, not just converted.** -`HomeComponent/index` (379), `CleanComponent/index` (471), `AnalyzeComponent` (560), -`CustomDesignComponent` (644) +EEGExploration's class name is `Home`. Keep `export default function Home`. -## Step 0 — the one new fixture (~30 lines, do this first) +#### C5. `StimuliDesignColumn.tsx` (203) — the memo exception -`src/renderer/test-utils.tsx`: +File comment: extracted so text input is not slow. `shouldComponentUpdate` skips unless `title` / `response` / `dir` / `audioDir` / `numberImages` / `numberSounds` change. **Preserve that skip.** ```tsx -// renderWithStore: mounts a component against a real store built from the app's -// reducers, so tests exercise selectors/actions instead of hand-mocked props. -export function renderWithStore(ui, { preloadedState } = {}) { - const store = configureStore({ reducer: rootReducer, preloadedState }); - return { store, ...render({ui}) }; +function StimuliDesignColumn(props: Props) { + const [numberImages, setNumberImages] = useState(undefined); + const [numberSounds, setNumberSounds] = useState(undefined); + + useEffect(() => { + void refreshSoundCount(props.audioDir); + }, [props.audioDir]); // covers mount + audioDir change + + async function refreshSoundCount(audioDir: string) { + if (!audioDir) return; + const sounds = await readAudioFiles(audioDir); + setNumberSounds(sounds.length); + } + // …handlers, render } + +export default React.memo(StimuliDesignColumn, (prev, next) => { + return ( + prev.title === next.title && + prev.response === next.response && + prev.dir === next.dir && + prev.audioDir === next.audioDir && + prev.num === next.num && + prev.numberImages === next.numberImages + ); +}); ``` -Plus a `MemoryRouter` wrapper for the four components that touch `navigate`. -That's it. **Skipped: MSW, storybook, snapshot testing, a component-props factory -library.** Add MSW when there's an HTTP call to mock (there isn't — it's all IPC). +Note: class SCU also compared **state** `numberImages` / `numberSounds`. `React.memo` only sees props; state changes still re-render the memoized component, which is what we want. `onChange` identity is ignored (same as the class — SCU did not compare `onChange`). Include `num` so a reused column with a new index still updates. + +`refreshSoundCount` on mount and on `audioDir` change collapses into one `[props.audioDir]` effect. -## Step 1 — the per-component loop +#### C6. `CustomDesignComponent.tsx` (651) -For each component, in Tier order (A → B → C → D): +Two instance fields, not state: -1. **Characterization test first, against the class.** 2–4 assertions: renders without - crashing given realistic props, and the one interaction that matters (click → callback - fired with expected args). Do NOT assert on markup details; assert on behavior. -2. **Convert.** `this.state.x` → `useState`; `componentDidMount` → `useEffect(…, [])`; - `componentDidUpdate(prev)` → `useEffect(…, [dep])`; handler methods → plain functions - in the body (no `useCallback` unless a profiler says so). -3. **Test stays green, unmodified.** If the test needed edits, behavior changed — that's - the whole point of writing it first. -4. `npm run typecheck && npm run lint` — `react-hooks/exhaustive-deps` catches the - classic `componentDidUpdate` → `useEffect` dependency mistakes. +```ts +private conditionParams: ExperimentParameters; // latest saved-or-in-flight params +private conditionRevision = 0; // stale-folder-scan guard +``` -One PR per tier, not per component. Tier D gets one PR per component. +```tsx +const conditionParamsRef = useRef(mergeCustomParams(props.params)); +const conditionRevisionRef = useRef(0); +``` -## Step 2 — Tier C: the subscription pattern +Every `this.conditionParams` → `conditionParamsRef.current`. Every `++this.conditionRevision` / `this.conditionRevision` → `conditionRevisionRef`. -`SignalQualityIndicatorComponent` and `ViewerComponent` both do: subscribe in -`componentDidMount`, re-subscribe in `componentDidUpdate` when the observable prop -changes, unsubscribe in `componentWillUnmount`. That is exactly one `useEffect`: +`componentWillUnmount` writes the ref back to Redux: ```tsx useEffect(() => { - if (!observable) return; - const sub = observable.subscribe(setSignalQuality); - return () => sub.unsubscribe(); -}, [observable]); + return () => { + props.ExperimentActions.SetParams(conditionParamsRef.current); + props.ExperimentActions.SaveWorkspace(); + }; +}, [props.ExperimentActions]); ``` -The class version has a latent bug worth checking while you're in there: it unsubscribes -in *both* `componentWillUnmount` and the re-subscribe path with no guard against a null -observable arriving mid-flight. The effect form makes that unrepresentable. +`handleSaveParams` default arg `params = this.conditionParams` becomes `params = conditionParamsRef.current`. -Extract it as `useObservable(observable)` **only after both call sites exist and are -identical** — not before. +This is the largest file. Convert in place. Do not split. -## Step 3 — Tier D: split, don't just convert +The A1 RTL test must already be green before you start this file. -The four big ones. Convert *and* split in the same PR, because converting alone leaves -the file just as unreadable: +#### C7. `ViewerComponent.tsx` (148) — last, alone -- `AnalyzeComponent` (560): `renderEpochLabels`, `renderHelpContent`, `renderHelp`, - `renderSectionContent` → 4 sibling components in `components/AnalyzeComponent/`. -- `CustomDesignComponent` (644): `renderSectionContent` is ~395 lines and is really - three screens (question/hypothesis/methods vs. conditions vs. preview). Split by screen. -- `CleanComponent/index` (471): `renderStats`, `renderAnalyzeButton`, `renderSelect`, - `renderReview` → siblings. `renderReview` (125 lines) probably wants its own test. -- `HomeComponent/index` (379): already has `ExperimentCard`/`OverviewComponent` siblings; - continue that pattern for the workspace list. +Genuinely risky. Playtest in the Electron window after this file. -Target: **no component file over ~250 lines** when done. That's the metric that actually -answers the original complaint. +What the class does: -## Step 4 — delete the container layer +1. Mount: `getViewerUrl()` then `setState({ viewerUrl })`. `` is **not** in the DOM yet. +2. `didUpdate` when `viewerUrl` goes `'' → non-empty`: `querySelector('webview')`, attach `dom-ready`. That handler reads `this.props.plottingInterval` and `this.state.channels/domain` **at fire time**, then `setKeyListeners`, then maybe subscribe. +3. `props.channels` change → `setState({ channels })`. +4. `props.signalQualityObservable` identity change + non-null → resubscribe (same null-skip as SignalQuality). +5. If `this.graphView` is still null, return. Else IPC: `channels` / `domain` / `autoScale` state changes. +6. Unmount: unsubscribe + `Mousetrap.unbind('up'|'down')`. -`src/renderer/containers/*Container.ts` exist only to wire `connect()` + inject -`navigate`. Once the component is a function, that indirection is pure cost — it's why -reading `HomeComponent` means opening `HomeContainer` to find out what `deviceType` is. +`componentDidUpdate` does not run on mount; `useEffect` does. The `if (!graphViewRef.current) return` early-out on the IPC effects is what prevents a mount-time `send` into a missing webview. -Per component: replace the `connect()` HOC with `useSelector`/`useDispatch` inside the -component, drop `navigate` prop for `useNavigate()`, delete the container, point the route -at the component directly. Deletes 5 files (`Home`, `Analyze`, `Clean`, `Collect`, -`ExperimentDesign`, `TopNavBar` containers) and shrinks every `Props` interface. +Target shape: -Do this **after** the conversion, per component, not as a big bang — a converted component -still taking props works fine; that's the safe intermediate state. +```tsx +export default function ViewerComponent(props: Props) { + const [channels, setChannels] = useState(() => props.channels ?? MUSE_CHANNELS); + const [domain] = useState(VIEWER_DEFAULTS.domain); + const [autoScale] = useState(VIEWER_DEFAULTS.autoScale); + const [viewerUrl, setViewerUrl] = useState(''); + + const graphViewRef = useRef(null); + const subRef = useRef(null); + const propsRef = useRef(props); + propsRef.current = props; + const channelsRef = useRef(channels); + channelsRef.current = channels; + + function subscribeToObservable(observable: Observable) { + subRef.current?.unsubscribe(); + subRef.current = observable.subscribe({ + next: (chunk) => { + graphViewRef.current?.send('newData', chunk); + }, + error: (error) => + console.error('[viewer] signal quality observable error:', error), + }); + } + + useEffect(() => { + let cancelled = false; + window.electronAPI.getViewerUrl().then((url) => { + if (!cancelled) setViewerUrl(url); + }); + return () => { + cancelled = true; + }; + }, []); + + // Attach once viewerUrl makes exist. Class did this in didUpdate + // keyed on prevState.viewerUrl being empty — equivalent to [viewerUrl] plus + // "only when non-empty". + useEffect(() => { + if (!viewerUrl) return; + const el = document.querySelector('webview') as WebviewTag | null; + graphViewRef.current = el; + const onDomReady = () => { + const p = propsRef.current; + el?.send('initGraph', { + plottingInterval: p.plottingInterval, + channels: channelsRef.current, + domain, + channelColours: channelsRef.current.map(() => '#66B0A9'), + }); + Mousetrap.bind('up', () => graphViewRef.current?.send('zoomIn')); + Mousetrap.bind('down', () => graphViewRef.current?.send('zoomOut')); + if (p.signalQualityObservable != null) { + subscribeToObservable(p.signalQualityObservable); + } + }; + el?.addEventListener('dom-ready', onDomReady); + // Class never removed this listener. No StrictMode in the tree, so do not + // invent a removeEventListener unless you also need it for correctness. + }, [viewerUrl, domain]); + + useEffect(() => { + if (props.channels) setChannels(props.channels); + }, [props.channels]); + + useEffect(() => { + if (props.signalQualityObservable == null) return; + subscribeToObservable(props.signalQualityObservable); + }, [props.signalQualityObservable]); + + useEffect(() => { + if (!graphViewRef.current) return; + graphViewRef.current.send('updateChannels', channels); + }, [channels]); + + useEffect(() => { + if (!graphViewRef.current) return; + graphViewRef.current.send('updateDomain', domain); + }, [domain]); + + useEffect(() => { + if (!graphViewRef.current) return; + graphViewRef.current.send('autoScale'); + }, [autoScale]); + + useEffect(() => { + return () => { + subRef.current?.unsubscribe(); + Mousetrap.unbind('up'); + Mousetrap.unbind('down'); + }; + }, []); + + if (!viewerUrl) return null; + const trueAsString = 'true' as any; + return ( + + ); +} +``` + +`domain` / `autoScale` are never written after init (`VIEWER_DEFAULTS`). Keep the IPC effects anyway — they match the class. They no-op on mount because `graphViewRef` is still null when those effects first run (webview is not in the tree until `viewerUrl` is set, which is a later paint). + +--- + +## Cleanup (after every class is gone) -## Step 5 — cleanup (5 minutes, satisfying) +Babel plugins are in **two** configs, not one. First draft missed `vite.config.ts`. -`vitest.config.ts` loads `@babel/plugin-proposal-decorators` and -`@babel/plugin-proposal-class-properties`. There are **zero decorators** in the codebase -and class properties are native in every supported target. Once the classes are gone, -delete both plugins from the config and both devDependencies. Faster test startup, two -fewer deps. +1. Delete the `babel.plugins` array from `vitest.config.ts` and from `vite.config.ts` (the comment "Legacy decorator support (used throughout the codebase)" is false — zero decorators in `src/`). +2. Remove `@babel/plugin-proposal-decorators` and `@babel/plugin-proposal-class-properties` from `package.json` `devDependencies`. +3. `npm install` to refresh the lockfile. + +--- ## What I'm deliberately not doing @@ -155,17 +515,55 @@ fewer deps. ## Sequencing / effort -| Step | Scope | Est. | +`react-hooks/exhaustive-deps` (`eslint.config.mjs:47`, `recommended-latest`) is the signal to check by hand, not to silence. + +Playtest `npm run dev` — the **desktop window**, not `localhost:5173` — Home → Design → Collect → Clean → Analyze. Device connected for Viewer + SignalQuality. Type in a StimuliDesignColumn title field and confirm it is not janky (the memo). Open ConnectModal and watch the SEARCHING → none-found → TURN_ON copy. + +--- + +## Inventory (20 classes, 4442 lines) + +| File | Lines | Bucket | |---|---|---| -| 0 | `test-utils.tsx` | 0.5h | -| 1 | Tier A (10 components) | 1 day | -| 1 | Tier B (4 components) | 0.5 day | -| 2 | Tier C (2 components) | 0.5 day | -| 3 | Tier D (4 components, split + test) | 2–3 days | -| 4 | delete containers | 0.5 day | -| 5 | babel plugin cleanup | 0.1h | - -Tiers A–C are safe to do in any order and are individually shippable. Step 3 is where the -actual reading-time win lands — if time is short, **do step 3 first on -`CustomDesignComponent` and `AnalyzeComponent`** and leave Tier A as classes indefinitely. -A 24-line class component costs nobody anything. +| `PreviewButtonComponent.tsx` | 24 | easy | +| `PreviewExperimentComponent.tsx` | 49 | easy | +| `PyodidePlotWidget.tsx` | 87 | easy | +| `InputCollect.tsx` | 155 | easy | +| `CleanComponent/CleanSidebar.tsx` | 172 | easy | +| `CollectComponent/HelpSidebar.tsx` | 191 | easy (named export) | +| `CollectComponent/PreTestComponent.tsx` | 175 | easy | +| `SecondaryNavComponent/index.tsx` | 116 | easy (drop SCU) | +| `DesignComponent/index.tsx` | 319 | easy | +| `HomeComponent/index.tsx` | 379 | easy | +| `AnalyzeComponent.tsx` | 560 | easy, large | +| `CleanComponent/index.tsx` | 471 | easy + `icons` lazy state | +| `InputModal.tsx` | 84 | judgement — debounce | +| `CollectComponent/ConnectModal.tsx` | 306 | judgement — debounce + willUpdate | +| `SignalQualityIndicatorComponent.tsx` | 70 | judgement — D3 + live props | +| `CollectComponent/index.tsx` | 138 | judgement — prevState modal | +| `EEGExplorationComponent.tsx` | 144 | judgement — same modal | +| `DesignComponent/StimuliDesignColumn.tsx` | 203 | judgement — `React.memo` | +| `DesignComponent/CustomDesignComponent.tsx` | 651 | judgement — refs + test rewrite | +| `ViewerComponent.tsx` | 148 | judgement — last | + +--- + +## Audit of the first draft + +Keep these corrections; do not re-introduce the original claims. + +- **Not true:** "no `shouldComponentUpdate` / legacy `componentWill*`." Actual: `SecondaryNavComponent` and `StimuliDesignColumn` have `shouldComponentUpdate`. `ConnectModal` has `UNSAFE_componentWillUpdate`. +- **Not true:** SignalQuality "state" + `subscribe(setSignalQuality)`. It has zero React state; it mutates D3. `plottingInterval` is read live inside the subscriber — needs `propsRef`, same trap as Viewer. +- **Not true:** only Viewer needs a ref for live props. SignalQuality does too. +- **Not true:** cleanup is only `vitest.config.ts`. Plugins also live in `vite.config.ts`. +- **Not true:** 4255 lines. `wc -l` is 4442. +- **Missed:** `InputModal` constructor debounce (100ms). Two debounce files, not one. +- **Missed:** `ConnectModal` constructor debounces (300 / 1000, leading). Recreating them every render resets the timer and breaks leading-edge. +- **Missed:** `CustomDesign.conditionParams` + `conditionRevision` instance fields. These are the stale-scan guard the existing test covers. +- **Missed:** `CustomDesignComponent.test.ts` instantiates the class. Rewrite to RTL before converting that file or CI goes red mid-pass. +- **Missed:** `Clean.icons` constructor field derived from `props.type`. +- **Missed:** `ConnectModal.getDeviceName` and `PreviewExperimentComponent.insertPreviewLabJsCallback` statics → module functions. +- **Missed:** HelpSidebar is a named export. +- **Wrong judgement count:** four files. Real judgement list is C1–C7 above. +- **Still true:** no `defaultProps`, no `createRef` / `this.refs`, no `forceUpdate`, no `setState(partial, callback)`, no `getDerivedStateFromProps`. Nine updater-form `setState` sites (Clean ×3, HelpSidebar ×2, PreTest ×2, CustomDesign ×1, Design ×1). No maintained TS class-component codemod worth adding. +- **Still true:** existing function components (`RunComponent`, `ExperimentWindow`) destructure props. We still keep `props.` on converted files so the diff is `this.props.` → `props.`. Do not "fix" that to match `RunComponent` in this PR. diff --git a/package-lock.json b/package-lock.json index 928cd5ee..11dc5604 100644 --- a/package-lock.json +++ b/package-lock.json @@ -51,8 +51,6 @@ "typesafe-actions": "^5.1.0" }, "devDependencies": { - "@babel/plugin-proposal-class-properties": "^7.10.4", - "@babel/plugin-proposal-decorators": "^7.10.5", "@testing-library/dom": "^10.4.1", "@testing-library/jest-dom": "^6.4.0", "@testing-library/react": "^16.0.0", @@ -219,40 +217,6 @@ "node": ">=6.9.0" } }, - "node_modules/@babel/helper-create-class-features-plugin": { - "version": "7.10.5", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/helper-function-name": "^7.10.4", - "@babel/helper-member-expression-to-functions": "^7.10.5", - "@babel/helper-optimise-call-expression": "^7.10.4", - "@babel/helper-plugin-utils": "^7.10.4", - "@babel/helper-replace-supers": "^7.10.4", - "@babel/helper-split-export-declaration": "^7.10.4" - }, - "peerDependencies": { - "@babel/core": "^7.0.0" - } - }, - "node_modules/@babel/helper-function-name": { - "version": "7.10.4", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/helper-get-function-arity": "^7.10.4", - "@babel/template": "^7.10.4", - "@babel/types": "^7.10.4" - } - }, - "node_modules/@babel/helper-get-function-arity": { - "version": "7.10.4", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/types": "^7.10.4" - } - }, "node_modules/@babel/helper-globals": { "version": "7.29.7", "resolved": "https://registry.npmjs.org/@babel/helper-globals/-/helper-globals-7.29.7.tgz", @@ -263,14 +227,6 @@ "node": ">=6.9.0" } }, - "node_modules/@babel/helper-member-expression-to-functions": { - "version": "7.11.0", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/types": "^7.11.0" - } - }, "node_modules/@babel/helper-module-imports": { "version": "7.29.7", "resolved": "https://registry.npmjs.org/@babel/helper-module-imports/-/helper-module-imports-7.29.7.tgz", @@ -303,14 +259,6 @@ "@babel/core": "^7.0.0" } }, - "node_modules/@babel/helper-optimise-call-expression": { - "version": "7.10.4", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/types": "^7.10.4" - } - }, "node_modules/@babel/helper-plugin-utils": { "version": "7.28.6", "dev": true, @@ -319,25 +267,6 @@ "node": ">=6.9.0" } }, - "node_modules/@babel/helper-replace-supers": { - "version": "7.10.4", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/helper-member-expression-to-functions": "^7.10.4", - "@babel/helper-optimise-call-expression": "^7.10.4", - "@babel/traverse": "^7.10.4", - "@babel/types": "^7.10.4" - } - }, - "node_modules/@babel/helper-split-export-declaration": { - "version": "7.11.0", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/types": "^7.11.0" - } - }, "node_modules/@babel/helper-string-parser": { "version": "7.29.7", "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.29.7.tgz", @@ -398,42 +327,6 @@ "node": ">=6.0.0" } }, - "node_modules/@babel/plugin-proposal-class-properties": { - "version": "7.10.4", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/helper-create-class-features-plugin": "^7.10.4", - "@babel/helper-plugin-utils": "^7.10.4" - }, - "peerDependencies": { - "@babel/core": "^7.0.0-0" - } - }, - "node_modules/@babel/plugin-proposal-decorators": { - "version": "7.10.5", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/helper-create-class-features-plugin": "^7.10.5", - "@babel/helper-plugin-utils": "^7.10.4", - "@babel/plugin-syntax-decorators": "^7.10.4" - }, - "peerDependencies": { - "@babel/core": "^7.0.0-0" - } - }, - "node_modules/@babel/plugin-syntax-decorators": { - "version": "7.10.4", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/helper-plugin-utils": "^7.10.4" - }, - "peerDependencies": { - "@babel/core": "^7.0.0-0" - } - }, "node_modules/@babel/plugin-transform-arrow-functions": { "version": "7.27.1", "dev": true, diff --git a/package.json b/package.json index 56a79777..ba708925 100644 --- a/package.json +++ b/package.json @@ -135,8 +135,6 @@ ], "homepage": "https://github.com/makebrainwaves/BrainWaves/", "devDependencies": { - "@babel/plugin-proposal-class-properties": "^7.10.4", - "@babel/plugin-proposal-decorators": "^7.10.5", "@testing-library/dom": "^10.4.1", "@testing-library/jest-dom": "^6.4.0", "@testing-library/react": "^16.0.0", diff --git a/src/renderer/components/AnalyzeComponent.tsx b/src/renderer/components/AnalyzeComponent.tsx index 69ea8e96..960ff07d 100644 --- a/src/renderer/components/AnalyzeComponent.tsx +++ b/src/renderer/components/AnalyzeComponent.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { useEffect, useState } from 'react'; import { Link } from 'react-router-dom'; import { Button } from './ui/button'; import { isNil } from 'lodash'; @@ -53,236 +53,199 @@ interface Props { PyodideActions: typeof PyodideActions; } -interface State { - activeStep: string; - selectedChannel: string; - eegFilePaths: Array<{ - key: string; - text: string; - value: { name: string; dir: string }; - }>; - behaviorFilePaths: Array<{ key: string; text: string; value: string }>; - selectedFilePaths: Array; - selectedBehaviorFilePaths: Array; - selectedSubjects: Array; - selectedDependentVariable: string; - removeOutliers: boolean; - showDataPoints: boolean; - isSidebarVisible: boolean; - displayMode: string; - dataToPlot: PlotlyData[]; - layout: Record; - helpMode: string; - dependentVariables: Array<{ key: string; text: string; value: string }>; -} - -export default class Analyze extends Component { - constructor(props: Props) { - super(props); - this.state = { - activeStep: - this.props.isEEGEnabled === true - ? ANALYZE_STEPS.OVERVIEW - : ANALYZE_STEPS.BEHAVIOR, - eegFilePaths: [{ key: '', text: '', value: { name: '', dir: '' } }], - behaviorFilePaths: [{ key: '', text: '', value: '' }], - dependentVariables: [{ key: '', text: '', value: '' }], - dataToPlot: [] as PlotlyData[], - layout: {}, - selectedDependentVariable: '', - removeOutliers: true, - showDataPoints: false, - isSidebarVisible: false, - displayMode: 'errorbars', - helpMode: 'errorbars', - selectedFilePaths: [], - selectedBehaviorFilePaths: [], - selectedSubjects: [], - selectedChannel: - MUSE_CHANNELS[0], - }; - this.handleChannelSelect = this.handleChannelSelect.bind(this); - this.handleDatasetChange = this.handleDatasetChange.bind(this); - this.handleBehaviorDatasetChange = - this.handleBehaviorDatasetChange.bind(this); - this.handleDependentVariableChange = - this.handleDependentVariableChange.bind(this); - this.handleRemoveOutliers = this.handleRemoveOutliers.bind(this); - this.handleDisplayModeChange = this.handleDisplayModeChange.bind(this); - this.handleDataPoints = this.handleDataPoints.bind(this); - this.saveSelectedDatasets = this.saveSelectedDatasets.bind(this); - this.handleStepClick = this.handleStepClick.bind(this); - this.handleDropdownClick = this.handleDropdownClick.bind(this); - this.toggleDisplayInfoVisibility = - this.toggleDisplayInfoVisibility.bind(this); - } +export default function Analyze(props: Props) { + const [activeStep, setActiveStep] = useState( + props.isEEGEnabled === true + ? ANALYZE_STEPS.OVERVIEW + : ANALYZE_STEPS.BEHAVIOR + ); + const [eegFilePaths, setEegFilePaths] = useState< + Array<{ key: string; text: string; value: { name: string; dir: string } }> + >([{ key: '', text: '', value: { name: '', dir: '' } }]); + const [behaviorFilePaths, setBehaviorFilePaths] = useState< + Array<{ key: string; text: string; value: string }> + >([{ key: '', text: '', value: '' }]); + const [dependentVariables, setDependentVariables] = useState< + Array<{ key: string; text: string; value: string }> + >([{ key: '', text: '', value: '' }]); + const [dataToPlot, setDataToPlot] = useState([]); + const [layout, setLayout] = useState>({}); + const [selectedDependentVariable, setSelectedDependentVariable] = useState(''); + const [removeOutliers, setRemoveOutliers] = useState(true); + const [showDataPoints, setShowDataPoints] = useState(false); + const [isSidebarVisible, setIsSidebarVisible] = useState(false); + const [displayMode, setDisplayMode] = useState('errorbars'); + const [helpMode, setHelpMode] = useState('errorbars'); + const [selectedFilePaths, setSelectedFilePaths] = useState>([]); + const [selectedBehaviorFilePaths, setSelectedBehaviorFilePaths] = useState>([]); + const [selectedSubjects, setSelectedSubjects] = useState>([]); + const [selectedChannel, setSelectedChannel] = useState(MUSE_CHANNELS[0]); - async componentDidMount() { - const workspaceCleanData = await readWorkspaceCleanedEEGData( - this.props.title - ); - const behavioralData = await readWorkspaceBehaviorData(this.props.title); - this.setState({ - eegFilePaths: workspaceCleanData.map((filepath) => ({ - key: filepath.name, - text: filepath.name, - value: filepath.path, - })), - behaviorFilePaths: behavioralData.map((filepath) => ({ - key: filepath.name, - text: filepath.name, - value: filepath.path, - })), - dependentVariables: ['Response Time', 'Accuracy'].map((dv) => ({ + useEffect(() => { + let cancelled = false; + (async () => { + const workspaceCleanData = await readWorkspaceCleanedEEGData(props.title); + const behavioralData = await readWorkspaceBehaviorData(props.title); + if (cancelled) return; + setEegFilePaths( + workspaceCleanData.map((filepath) => ({ + key: filepath.name, + text: filepath.name, + value: filepath.path, + })) + ); + setBehaviorFilePaths( + behavioralData.map((filepath) => ({ + key: filepath.name, + text: filepath.name, + value: filepath.path, + })) + ); + const dvs = ['Response Time', 'Accuracy'].map((dv) => ({ key: dv, text: dv, value: dv, - })), - selectedDependentVariable: 'Response Time', - }); - } + })); + setDependentVariables(dvs); + setSelectedDependentVariable('Response Time'); + })(); + return () => { + cancelled = true; + }; + }, [props.title]); - concatSubjectNames = (subjects: Array) => { + function concatSubjectNames(subjects: Array) { if (subjects.length < 1) return ''; return subjects.reduce((acc, curr) => `${acc}-${curr}`); - }; + } - handleDatasetChange(e: React.ChangeEvent) { + function handleDatasetChange(e: React.ChangeEvent) { const values = Array.from(e.target.selectedOptions, (o) => o.value); - this.setState({ - selectedFilePaths: values, - selectedSubjects: getSubjectNamesFromFiles(values), - }); - this.props.PyodideActions.LoadCleanedEpochs(values); + setSelectedFilePaths(values); + setSelectedSubjects(getSubjectNamesFromFiles(values)); + props.PyodideActions.LoadCleanedEpochs(values); } - handleBehaviorDatasetChange(e: React.ChangeEvent) { + function handleBehaviorDatasetChange(e: React.ChangeEvent) { const values = Array.from(e.target.selectedOptions, (o) => o.value); const aggregatedData = aggregateDataForPlot( readBehaviorData(values), - this.state.selectedDependentVariable, - this.state.removeOutliers, - this.state.showDataPoints, - this.state.displayMode + selectedDependentVariable, + removeOutliers, + showDataPoints, + displayMode ); if (!aggregatedData) return; - const { dataToPlot, layout } = aggregatedData; - this.setState({ - selectedBehaviorFilePaths: values, - selectedSubjects: getSubjectNamesFromFiles(values), - dataToPlot, - layout, - }); + const { dataToPlot: data, layout: lay } = aggregatedData; + setSelectedBehaviorFilePaths(values); + setSelectedSubjects(getSubjectNamesFromFiles(values)); + setDataToPlot(data); + setLayout(lay); } - async handleDropdownClick() { - const behavioralData = await readWorkspaceBehaviorData(this.props.title); - if (behavioralData.length !== this.state.behaviorFilePaths.length) { - this.setState({ - behaviorFilePaths: behavioralData.map((filepath) => ({ + async function handleDropdownClick() { + const behavioralData = await readWorkspaceBehaviorData(props.title); + if (behavioralData.length !== behaviorFilePaths.length) { + setBehaviorFilePaths( + behavioralData.map((filepath) => ({ key: filepath.name, text: filepath.name, value: filepath.path, - })), - }); + })) + ); } } - handleDependentVariableChange(e: React.ChangeEvent) { + function handleDependentVariableChange(e: React.ChangeEvent) { const { value } = e.target; const aggregatedData = aggregateDataForPlot( - readBehaviorData(this.state.selectedBehaviorFilePaths), + readBehaviorData(selectedBehaviorFilePaths), value, - this.state.removeOutliers, - this.state.showDataPoints, - this.state.displayMode + removeOutliers, + showDataPoints, + displayMode ); if (!aggregatedData) return; - const { dataToPlot, layout } = aggregatedData; - this.setState({ selectedDependentVariable: value, dataToPlot, layout }); + const { dataToPlot: data, layout: lay } = aggregatedData; + setSelectedDependentVariable(value); + setDataToPlot(data); + setLayout(lay); } - handleRemoveOutliers() { + function handleRemoveOutliers() { const aggregatedData = aggregateDataForPlot( - readBehaviorData(this.state.selectedBehaviorFilePaths), - this.state.selectedDependentVariable, - !this.state.removeOutliers, - this.state.showDataPoints, - this.state.displayMode + readBehaviorData(selectedBehaviorFilePaths), + selectedDependentVariable, + !removeOutliers, + showDataPoints, + displayMode ); if (!aggregatedData) return; - const { dataToPlot, layout } = aggregatedData; - this.setState({ - removeOutliers: !this.state.removeOutliers, - dataToPlot, - layout, - helpMode: 'outliers', - }); + const { dataToPlot: data, layout: lay } = aggregatedData; + setRemoveOutliers(!removeOutliers); + setDataToPlot(data); + setLayout(lay); } - handleDataPoints() { + function handleDisplayModeChange(value: string) { const aggregatedData = aggregateDataForPlot( - readBehaviorData(this.state.selectedBehaviorFilePaths), - this.state.selectedDependentVariable, - this.state.removeOutliers, - !this.state.showDataPoints, - this.state.displayMode + readBehaviorData(selectedBehaviorFilePaths), + selectedDependentVariable, + removeOutliers, + showDataPoints, + value ); if (!aggregatedData) return; - const { dataToPlot, layout } = aggregatedData; - this.setState({ - showDataPoints: !this.state.showDataPoints, - dataToPlot, - layout, - }); + const { dataToPlot: data, layout: lay } = aggregatedData; + setDisplayMode(value); + setDataToPlot(data); + setLayout(lay); } - handleDisplayModeChange(displayMode) { - if ( - this.state.selectedBehaviorFilePaths && - this.state.selectedBehaviorFilePaths.length > 0 - ) { - const aggregatedData = aggregateDataForPlot( - readBehaviorData(this.state.selectedBehaviorFilePaths), - this.state.selectedDependentVariable, - this.state.removeOutliers, - this.state.showDataPoints, - displayMode - ); - if (!aggregatedData) return; - const { dataToPlot, layout } = aggregatedData; - this.setState({ dataToPlot, layout, displayMode, helpMode: displayMode }); - } + function handleDataPoints() { + const aggregatedData = aggregateDataForPlot( + readBehaviorData(selectedBehaviorFilePaths), + selectedDependentVariable, + removeOutliers, + !showDataPoints, + displayMode + ); + if (!aggregatedData) return; + const { dataToPlot: data, layout: lay } = aggregatedData; + setShowDataPoints(!showDataPoints); + setDataToPlot(data); + setLayout(lay); } - toggleDisplayInfoVisibility() { - this.setState({ isSidebarVisible: !this.state.isSidebarVisible }); + function toggleDisplayInfoVisibility() { + setIsSidebarVisible((prev) => !prev); } - saveSelectedDatasets() { - const data = readBehaviorData(this.state.selectedBehaviorFilePaths); + function saveSelectedDatasets() { + const data = readBehaviorData(selectedBehaviorFilePaths); const aggregatedData = aggregateBehaviorDataToSave( data, - this.state.removeOutliers + removeOutliers ); - storeAggregatedBehaviorData(aggregatedData, this.props.title); + storeAggregatedBehaviorData(aggregatedData as Parameters[0], props.title); } - handleChannelSelect(channelName: string) { - this.setState({ selectedChannel: channelName }); - this.props.PyodideActions.LoadERP(channelName); + function handleChannelSelect(channelName: string) { + setSelectedChannel(channelName); + props.PyodideActions.LoadERP(channelName); } - handleStepClick(step: string) { - this.setState({ activeStep: step }); + function handleStepClick(step: string) { + setActiveStep(step); } - renderEpochLabels() { + function renderEpochLabels() { + const { epochsInfo } = props; if ( - !isNil(this.props.epochsInfo) && - this.state.selectedFilePaths.length >= 1 + !isNil(epochsInfo) && + selectedFilePaths.length >= 1 ) { - const numberConditions = this.props.epochsInfo.filter( + const numberConditions = epochsInfo.filter( (infoObj) => infoObj.name !== 'Drop Percentage' && infoObj.name !== 'Total Epochs' ).length; @@ -292,7 +255,7 @@ export default class Analyze extends Component { : ['red', 'green', 'teal', 'orange']; return (
- {this.props.epochsInfo + {epochsInfo .filter( (infoObj) => infoObj.name !== 'Drop Percentage' && @@ -310,51 +273,38 @@ export default class Analyze extends Component { return
; } - renderHelpContent() { - switch (this.state.helpMode) { + function renderHelpContent() { + switch (helpMode) { case 'datapoints': - return this.renderHelp( + return renderHelp( 'Data Points', - `In this graph, each dot refers to one data point, clustered by group (e.g., conditions). - It's the most "neutral" way of presenting the data, of course, but it may be difficult to see any patterns. - Why is it always a good idea to look at all your datapoints before interpreting any trends in the data?` + 'In this graph, each dot refers to one data point, clustered by group (e.g., conditions).' ); case 'errorbars': - return this.renderHelp( + return renderHelp( 'Bar Graph', - `Bar graphs are the most common way to summarize data. - It allows you to compare mean values between groups of datapoints (e.g., conditions), - and the error bars give some indication of the variance (here: the standard error of the mean). - Importantly, this way of summarizing data assumes that the mean is in fact representative of the data. - Many researchers have veered away from bar graphs because they can be deceptive, especially without error bars. - Can you think of any such cases?` + 'Bar graphs are the most common way to summarize data.' ); case 'whiskers': - return this.renderHelp( + return renderHelp( 'Box Plot', - `Box plots summarize the data in a more informative way: - they actually tell you something about the distribution of datapoints within a group, - by taking the median as its reference point instead of the mean. - The boxes represent so-called "quartiles". - The lines ("whiskers") show how much variability there is in the data outside of those quartiles; - any outliers are shown as individual points.` + 'Box plots summarize the data in a more informative way.' ); case 'outliers': default: - return this.renderHelp( + return renderHelp( 'Outliers', - `A datapoint is tagged as an "outlier" if its value exceeds 2 standard deviations below or above the mean of all data in the group. - Removing such outliers can help unskew the data.` + 'A datapoint is tagged as an "outlier" if its value exceeds 2 standard deviations.' ); } } - renderHelp(header: string, content: string) { + function renderHelp(header: string, content: string) { return (
-
- )} -
-
+ function renderOverview() { + const { child: psdChild } = props.psdPlot; + const { child: topoChild } = props.topoPlot; + return ( +
+

Overview

+ {renderEpochLabels()} +
+
+ + +
+
+

PSD Plot

+ {psdChild ? ( -
- - ); - case ANALYZE_STEPS.ERP: - return ( - <> -
-

ERP

-

- The event-related potential represents EEG activity elicited by - a particular sensory event -

- -
- {this.renderEpochLabels()} -
-
+ ) : ( +

No PSD data available. Clean some data first.

+ )} +
+
+

Topography

+ {topoChild ? ( -
- - ); - case ANALYZE_STEPS.BEHAVIOR: - return ( - <> -
-

Overview

-

- Load datasets from different subjects and view behavioral - results -

-
- Datasets - -
- -
-

Dependent Variable

- -
-
-
- -
- -
-
- {(['datapoints', 'errorbars', 'whiskers'] as const).map( - (mode) => ( - - ) - )} -
- - {this.state.isSidebarVisible && ( -
{this.renderHelpContent()}
- )} + {channel}
-
- - ); - } + ))} +
+
+
+ ); } - render() { + function renderBehavior() { return ( -
- -
- {this.renderSectionContent()} +
+

Behavioral Data

+
+ + +
+
+
+ + +
+
+ + +
+
+
+ + +
+ + +
+
+ + + +
+
+ {dataToPlot.length > 0 ? ( + + ) : ( +

Select datasets to see plots.

+ )} +
); } -} + + const steps = props.isEEGEnabled ? ANALYZE_STEPS : ANALYZE_STEPS_BEHAVIOR; + + return ( +
+ + {isSidebarVisible ? 'Hide' : 'Show'} help + + } + /> + {isSidebarVisible && renderHelpContent()} + {activeStep === ANALYZE_STEPS.OVERVIEW && renderOverview()} + {activeStep === ANALYZE_STEPS.ERP && renderERP()} + {activeStep === ANALYZE_STEPS.BEHAVIOR && renderBehavior()} +
+ ); +} \ No newline at end of file diff --git a/src/renderer/components/CleanComponent/CleanSidebar.tsx b/src/renderer/components/CleanComponent/CleanSidebar.tsx index cf8b14b3..8a11bf9a 100644 --- a/src/renderer/components/CleanComponent/CleanSidebar.tsx +++ b/src/renderer/components/CleanComponent/CleanSidebar.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { useState } from 'react'; import { Button } from '../ui/button'; enum HELP_STEP { @@ -17,46 +17,33 @@ interface Props { handleClose: () => void; } -interface State { - helpStep: HELP_STEP; -} - -export default class CleanSidebar extends Component { - constructor(props) { - super(props); - this.state = { - helpStep: HELP_STEP.MENU, - }; - this.handleStartLearn = this.handleStartLearn.bind(this); - this.handleStartSignal = this.handleStartSignal.bind(this); - this.handleNext = this.handleNext.bind(this); - this.handleBack = this.handleBack.bind(this); - } +export default function CleanSidebar(props: Props) { + const [helpStep, setHelpStep] = useState(HELP_STEP.MENU); - handleStartSignal() { - this.setState({ helpStep: HELP_STEP.SIGNAL_EXPLANATION }); + function handleStartSignal() { + setHelpStep(HELP_STEP.SIGNAL_EXPLANATION); } - handleStartLearn() { - this.setState({ helpStep: HELP_STEP.LEARN_BRAIN }); + function handleStartLearn() { + setHelpStep(HELP_STEP.LEARN_BRAIN); } - handleNext() { + function handleNext() { if ( - this.state.helpStep === HELP_STEP.SIGNAL_MOVEMENT || - this.state.helpStep === HELP_STEP.LEARN_ALPHA + helpStep === HELP_STEP.SIGNAL_MOVEMENT || + helpStep === HELP_STEP.LEARN_ALPHA ) { - this.setState({ helpStep: HELP_STEP.MENU }); + setHelpStep(HELP_STEP.MENU); } else { - this.setState({ helpStep: this.state.helpStep + 1 }); + setHelpStep((prev) => prev + 1); } } - handleBack() { - this.setState({ helpStep: this.state.helpStep - 1 }); + function handleBack() { + setHelpStep((prev) => prev - 1); } - renderMenu() { + function renderMenu() { return (

What would you like to do?

@@ -64,8 +51,8 @@ export default class CleanSidebar extends Component { role="button" tabIndex={0} className="text-lg p-1 cursor-pointer hover:bg-gray-100" - onClick={this.handleStartSignal} - onKeyDown={(e) => e.key === 'Enter' && this.handleStartSignal()} + onClick={handleStartSignal} + onKeyDown={(e) => e.key === 'Enter' && handleStartSignal()} > ★ Improve the signal quality of your sensors
@@ -73,8 +60,8 @@ export default class CleanSidebar extends Component { role="button" tabIndex={0} className="text-lg p-1 cursor-pointer hover:bg-gray-100" - onClick={this.handleStartLearn} - onKeyDown={(e) => e.key === 'Enter' && this.handleStartLearn()} + onClick={handleStartLearn} + onKeyDown={(e) => e.key === 'Enter' && handleStartLearn()} > ⚠ Learn about how the subjects movements create noise
@@ -82,7 +69,7 @@ export default class CleanSidebar extends Component { ); } - renderHelp(header: string, content: string) { + function renderHelp(header: string, content: string) { return ( <>
@@ -93,14 +80,14 @@ export default class CleanSidebar extends Component { @@ -109,64 +96,62 @@ export default class CleanSidebar extends Component { ); } - renderHelpContent() { - switch (this.state.helpStep) { + function renderHelpContent() { + switch (helpStep) { case HELP_STEP.SIGNAL_EXPLANATION: - return this.renderHelp( + return renderHelp( 'Improve the signal quality', 'In order to collect quality data, you want to make sure that all electrodes have a strong connection' ); case HELP_STEP.SIGNAL_SALINE: - return this.renderHelp( + return renderHelp( 'Tip #1: Saturate the sensors in saline', 'Make sure the sensors are thoroughly soaked with saline solution. They should be wet to the touch' ); case HELP_STEP.SIGNAL_CONTACT: - return this.renderHelp( + return renderHelp( 'Tip #2: Ensure the sensors are making firm contact', 'Re-seat the headset to make sure that all sensors contact the head with some tension. You may need to sweep hair out of the way to accomplish this' ); case HELP_STEP.SIGNAL_MOVEMENT: - return this.renderHelp( + return renderHelp( 'Tip #3: Stay still', 'To reduce noise during your experiment, ensure your subject is relaxed and has both feet on the floor. Sometimes, focusing on relaxing the jaw and the tongue can improve the EEG signal' ); case HELP_STEP.LEARN_BRAIN: - return this.renderHelp( + return renderHelp( 'Your brain produces electricity', 'Using the device that you are wearing, we can detect the electrical activity of your brain.' ); case HELP_STEP.LEARN_BLINK: - return this.renderHelp( + return renderHelp( 'Try blinking your eyes', 'Does the signal change? Eye movements create noise in the EEG signal' ); case HELP_STEP.LEARN_THOUGHT: - return this.renderHelp( + return renderHelp( 'Try thinking of a cat', "Does the signal change? Although EEG can measure overall brain activity, it's not capable of reading minds" ); case HELP_STEP.LEARN_ALPHA: - return this.renderHelp( + return renderHelp( 'Try closing your eyes for 10 seconds', 'You may notice a change in your signal due to an increase in alpha waves' ); case HELP_STEP.MENU: default: - return this.renderMenu(); + return renderMenu(); } } - render() { - return ( -
-
- -
- {this.renderHelpContent()} + return ( +
+
+
- ); - } -} + {renderHelpContent()} +
+ ); +} \ No newline at end of file diff --git a/src/renderer/components/CleanComponent/__tests__/CleanRejections.test.tsx b/src/renderer/components/CleanComponent/__tests__/CleanRejections.test.tsx new file mode 100644 index 00000000..d48a1e5b --- /dev/null +++ b/src/renderer/components/CleanComponent/__tests__/CleanRejections.test.tsx @@ -0,0 +1,118 @@ +import React from 'react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { EXPERIMENTS, DEVICES } from '../../../constants/constants'; +import type { SuggestedRejection } from '../../../actions'; +import Clean, { Props as CleanProps } from '../index'; + +// We mock the children to capture the `rejected` prop sent to EpochReviewer. +let mockRejected: Set = new Set(); + +vi.mock('../EpochReviewer', () => ({ + default: (props: { + rejected: Set; + }) => { + mockRejected = props.rejected; + return
; + }, +})); + +vi.mock('../CleanSidebar', () => ({ + default: () =>
, +})); + +vi.mock('../LiveErpPane', () => ({ + default: () =>
, +})); + +vi.mock('lab.js', () => ({})); + +vi.mock('../../../utils/filesystem/storage', () => ({ + readWorkspaceRawEEGData: vi.fn(async () => [ + { name: 'session_1.fif', path: '/sub-01/session_1.fif' }, + ]), +})); + +const fakeEpochArrays = { + buffer: new ArrayBuffer(8), + meta: { + n_epochs: 3, + n_channels: 2, + n_times: 4, + ch_names: ['Fp1', 'Fp2'], + times: [-0.1, 0, 0.1, 0.2], + event_codes: [1, 2, 1], + }, +}; + +const baseProps: Record = { + type: EXPERIMENTS.N170, + title: 'Test_Experiment', + deviceType: DEVICES.MUSE, + epochsInfo: [{ name: 'N170', value: 100 }], + epochArrays: fakeEpochArrays, + PyodideActions: { LoadEpochs: vi.fn() }, + ExperimentActions: { SetSubject: vi.fn() }, + subject: '', + session: 0, + params: null, + suggestedRejections: [] as SuggestedRejection[], +}; + +describe('Clean suggestedRejections merge', () => { + beforeEach(() => { + mockRejected = new Set(); + }); + + it('merges suggestedRejections indices into rejectedEpochs', async () => { + const { rerender } = render( + + + + ); + + // The mount effect reads workspace data and sets subjects/file paths. + // After that, the user selects a file and loads the dataset. + // The file-path multi-select fires onChange via handleRecordingChange. + // Wait for the mount effect to populate the select options. + + await waitFor(() => { + expect(screen.getByText('Load Dataset →')).toBeInTheDocument(); + }); + + // Select the first (and only) file path. + const select = screen.getByRole('listbox') as HTMLSelectElement; + const option = screen.getByRole('option', { name: 'session_1.fif' }) as HTMLOptionElement; + option.selected = true; + await act(async () => { + fireEvent.change(select); + }); + + // Click "Load Dataset" to switch to review view. + await act(async () => { + fireEvent.click(screen.getByText('Load Dataset →')); + }); + + // Now EpochReviewer should be rendered. + expect(screen.getByTestId('epoch-reviewer')).toBeInTheDocument(); + + // Rerender with suggestedRejections. + const suggestions: SuggestedRejection[] = [ + { index: 2, reason: 'High peak-to-peak' }, + { index: 5, reason: 'Muscle artifact' }, + ]; + rerender( + + + + ); + + // The componentDidUpdate in the class (and later the useEffect in the + // function component) merges the new indices into rejectedEpochs. + expect(mockRejected.has(2)).toBe(true); + expect(mockRejected.has(5)).toBe(true); + // The set should not contain an index that was never suggested (1). + expect(mockRejected.size).toBe(2); + }); +}); \ No newline at end of file diff --git a/src/renderer/components/CleanComponent/index.tsx b/src/renderer/components/CleanComponent/index.tsx index a38c1b92..78baeab2 100644 --- a/src/renderer/components/CleanComponent/index.tsx +++ b/src/renderer/components/CleanComponent/index.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import path from 'pathe'; import { Link } from 'react-router-dom'; import { isNil, isString, memoize } from 'lodash'; @@ -49,142 +49,114 @@ interface DropdownOption { value: string; } -interface State { - // Which screen is showing: dataset picker vs. the interactive editor. - view: 'select' | 'review'; - subjects: Array; - eegFilePaths: Array; - selectedSubject: string; - selectedFilePaths: Array; - isSidebarVisible: boolean; - // ABSOLUTE epoch indices the student has marked for rejection. - rejectedEpochs: Set; - // Channel names the student has flagged as bad (dropped across all epochs). - badChannels: Set; - // Peak-to-peak threshold (µV) used by the auto-flag request. - autoFlagThreshold: number; - // Whether the auto-flag threshold settings panel is open. - showAutoFlagSettings: boolean; -} - -export default class Clean extends Component { - icons: string[]; +export default function Clean(props: Props) { + const [view, setView] = useState<'select' | 'review'>('select'); + const [subjects, setSubjects] = useState>([]); + const [eegFilePaths, setEegFilePaths] = useState>([ + { key: '', text: '', value: '' }, + ]); + const [selectedSubject, setSelectedSubject] = useState(props.subject); + const [selectedFilePaths, setSelectedFilePaths] = useState>([]); + const [isSidebarVisible, setIsSidebarVisible] = useState(false); + const [rejectedEpochs, setRejectedEpochs] = useState>(new Set()); + const [badChannels, setBadChannels] = useState>(new Set()); + const [autoFlagThreshold, setAutoFlagThreshold] = useState( + PTP_THRESHOLD.default + ); + const [showAutoFlagSettings, setShowAutoFlagSettings] = useState(false); + const [icons] = useState(() => + props.type === EXPERIMENTS.N170 + ? ['😊', '🏠', '✕', '📖'] + : ['★', '☆', '✕', '📖'] + ); - constructor(props: Props) { - super(props); - this.state = { - view: 'select', - subjects: [], - eegFilePaths: [{ key: '', text: '', value: '' }], - selectedFilePaths: [], - selectedSubject: props.subject, - isSidebarVisible: false, - rejectedEpochs: new Set(), - badChannels: new Set(), - autoFlagThreshold: PTP_THRESHOLD.default, - showAutoFlagSettings: false, + useEffect(() => { + let cancelled = false; + (async () => { + const workspaceRawData = await readWorkspaceRawEEGData(props.title); + if (cancelled) return; + setSubjects( + workspaceRawData + .map( + (filepath) => + filepath.path.split(path.sep)[ + filepath.path.split(path.sep).length - 3 + ] + ) + .reduce((acc, curr) => { + if (acc.find((subject) => subject.key === curr)) { + return acc; + } + return acc.concat({ key: curr, text: curr, value: curr }); + }, [] as DropdownOption[]) + ); + setEegFilePaths( + workspaceRawData.map((filepath) => ({ + key: filepath.name, + text: filepath.name, + value: filepath.path, + })) + ); + })(); + return () => { + cancelled = true; }; - this.handleRecordingChange = this.handleRecordingChange.bind(this); - this.handleLoadData = this.handleLoadData.bind(this); - this.handleSidebarToggle = this.handleSidebarToggle.bind(this); - this.handleSubjectChange = this.handleSubjectChange.bind(this); - this.handleToggleEpoch = this.handleToggleEpoch.bind(this); - this.handleToggleChannel = this.handleToggleChannel.bind(this); - this.handleAutoFlag = this.handleAutoFlag.bind(this); - this.handleCleanData = this.handleCleanData.bind(this); - this.handleThresholdChange = this.handleThresholdChange.bind(this); - this.icons = - props.type === EXPERIMENTS.N170 - ? ['😊', '🏠', '✕', '📖'] - : ['★', '☆', '✕', '📖']; - } + }, [props.title]); - async componentDidMount() { - const workspaceRawData = await readWorkspaceRawEEGData(this.props.title); - this.setState({ - subjects: workspaceRawData - .map( - (filepath) => - filepath.path.split(path.sep)[ - filepath.path.split(path.sep).length - 3 - ] - ) - .reduce((acc, curr) => { - if (acc.find((subject) => subject.key === curr)) { - return acc; - } - return acc.concat({ key: curr, text: curr, value: curr }); - }, []), - eegFilePaths: workspaceRawData.map((filepath) => ({ - key: filepath.name, - text: filepath.name, - value: filepath.path, - })), - }); - } + useEffect(() => { + if (props.suggestedRejections.length > 0) { + setRejectedEpochs((prev) => { + const next = new Set(prev); + for (const s of props.suggestedRejections) next.add(s.index); + return next; + }); + } + }, [props.suggestedRejections]); - handleRecordingChange(e: React.ChangeEvent) { + function handleRecordingChange(e: React.ChangeEvent) { const filePaths = Array.from(e.target.selectedOptions, (o) => o.value); - this.setState({ selectedFilePaths: filePaths }); + setSelectedFilePaths(filePaths); } - handleSubjectChange(e: React.ChangeEvent) { + function handleSubjectChange(e: React.ChangeEvent) { const { value } = e.target; if (!isNil(value) && isString(value)) { - this.setState({ selectedSubject: value, selectedFilePaths: [] }); + setSelectedSubject(value); + setSelectedFilePaths([]); } } - handleLoadData() { - this.props.ExperimentActions.SetSubject(this.state.selectedSubject); - this.props.PyodideActions.LoadEpochs(this.state.selectedFilePaths); - // Launch the editor; a fresh dataset invalidates any previously selected - // epoch indices and bad-channel selections. - this.setState({ - view: 'review', - rejectedEpochs: new Set(), - badChannels: new Set(), - }); - } - - componentDidUpdate(prevProps: Props) { - if (prevProps.suggestedRejections !== this.props.suggestedRejections) { - const suggested = this.props.suggestedRejections; - if (suggested.length > 0) { - this.setState((prev) => { - const next = new Set(prev.rejectedEpochs); - for (const s of suggested) next.add(s.index); - return { rejectedEpochs: next }; - }); - } - } + function handleLoadData() { + props.ExperimentActions.SetSubject(selectedSubject); + props.PyodideActions.LoadEpochs(selectedFilePaths); + setView('review'); + setRejectedEpochs(new Set()); + setBadChannels(new Set()); } - handleToggleEpoch(index: number) { - this.setState((prev) => { - const next = new Set(prev.rejectedEpochs); + function handleToggleEpoch(index: number) { + setRejectedEpochs((prev) => { + const next = new Set(prev); if (next.has(index)) { next.delete(index); } else { next.add(index); } - return { rejectedEpochs: next }; + return next; }); } - handleToggleChannel(name: string) { - const next = new Set(this.state.badChannels); + function handleToggleChannel(name: string) { + const next = new Set(badChannels); const adding = !next.has(name); if (adding) { next.add(name); } else { next.delete(name); } - this.setState({ badChannels: next }); + setBadChannels(next); - // Dropping >1 of a 4-channel (Muse) recording loses a lot of signal — - // informational only; they can still proceed. - if (adding && next.size > 1 && this.props.epochArrays?.meta.n_channels === 4) { + if (adding && next.size > 1 && props.epochArrays?.meta.n_channels === 4) { window.electronAPI.showMessageBox({ buttons: ['Got it'], message: @@ -195,17 +167,13 @@ export default class Clean extends Component { } } - handleAutoFlag() { - this.props.PyodideActions.GetSuggestedRejections( - this.state.autoFlagThreshold - ); + function handleAutoFlag() { + props.PyodideActions.GetSuggestedRejections(autoFlagThreshold); } - async handleCleanData() { - const total = this.props.epochArrays?.meta.n_epochs ?? 0; - const nDropped = this.state.rejectedEpochs.size; - // Rejecting every epoch produces an empty dataset that can't be analyzed - // (and previously wrote a degenerate .fif with no error). Warn first. + async function handleCleanData() { + const total = props.epochArrays?.meta.n_epochs ?? 0; + const nDropped = rejectedEpochs.size; if (total > 0 && nDropped >= total) { const response = await window.electronAPI.showMessageBox({ buttons: ['Cancel', 'Reject all anyway'], @@ -215,31 +183,24 @@ export default class Clean extends Component { return; } } - this.props.PyodideActions.CleanEpochs({ - dropIndices: Array.from(this.state.rejectedEpochs), - badChannels: Array.from(this.state.badChannels), - }); - // After Clean, raw_epochs is re-fetched with fewer epochs, so the old - // absolute indices no longer apply. - this.setState({ - rejectedEpochs: new Set(), - badChannels: new Set(), - }); + props.PyodideActions.CleanEpochs({ dropIndices: [...rejectedEpochs], badChannels: [...badChannels] }); + setRejectedEpochs(new Set()); + setBadChannels(new Set()); } - handleThresholdChange(e: React.ChangeEvent) { + function handleThresholdChange(e: React.ChangeEvent) { const parsed = parseFloat(e.target.value); if (!Number.isNaN(parsed)) { - this.setState({ autoFlagThreshold: parsed }); + setAutoFlagThreshold(parsed); } } - handleSidebarToggle() { - this.setState({ isSidebarVisible: !this.state.isSidebarVisible }); + function handleSidebarToggle() { + setIsSidebarVisible((prev) => !prev); } - renderStats() { - const { epochsInfo } = this.props; + function renderStats() { + const { epochsInfo } = props; if (isNil(epochsInfo) || epochsInfo.length === 0) { return null; } @@ -247,7 +208,7 @@ export default class Clean extends Component {
{epochsInfo.map((infoObj, index) => ( - {this.icons[index]} + {icons[index]} {infoObj.name}:{' '} {infoObj.value} @@ -256,10 +217,8 @@ export default class Clean extends Component { ); } - renderAnalyzeButton() { - const { epochsInfo } = this.props; - // 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). + function renderAnalyzeButton() { + const { epochsInfo } = props; if (!isNil(epochsInfo) && epochsInfo.length > 0) { return ( @@ -270,7 +229,7 @@ export default class Clean extends Component { return null; } - renderSelect(filteredFilePaths: DropdownOption[]) { + function renderSelect(filteredFilePaths: DropdownOption[]) { return (

Clean

@@ -282,10 +241,10 @@ export default class Clean extends Component {

Select Subject

{filteredFilePaths.map((fp) => (