Skip to content

Epoch reviewer Phase 2: bad-channel flagging, auto-flag suggestions, condition labels#225

Open
jdpigeon wants to merge 19 commits into
epoch-reviewer-phase1from
epoch-reviewer-phase2
Open

Epoch reviewer Phase 2: bad-channel flagging, auto-flag suggestions, condition labels#225
jdpigeon wants to merge 19 commits into
epoch-reviewer-phase1from
epoch-reviewer-phase2

Conversation

@jdpigeon

@jdpigeon jdpigeon commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What & why

Phase 2 of the Interactive Epoch Review UI (docs/epoch-review-ui-plan.md §0/§9) — full parity with (and beyond) MNE's cleaning GUI. Builds on Phase 1 (PR #224). Locked decisions: OQ4 (dev-gated auto-flag button + settings), OQ5 (bad channels enabled everywhere + 4-ch warning), New-A (auto-flag in Python), New-B (codeToLabel legends).

Three features

1. Bad-channel flagging (OQ5)
Click a channel label → toggles it bad (struck-through red label + red lane wash across all epochs). Bad channels ride the Phase-1 apply path: CleanEpochs({dropIndices, badChannels})apply_rejection sets info['bads']. A shadcn Dialog warns when you mark a 2nd bad channel on a 4-channel (Muse) dataset, nudging re-collection — informational, you can proceed. (8-ch+ devices don't warn.)

2. Auto-flag artifacts (OQ4 + New-A, Python/MNE)
A dev-gated "Auto-flag artifacts" button (visibility via AUTO_FLAG_EXPERIMENTS, a constant set keyed by the EXPERIMENTS enum — devs opt experiments in/out) with an expandable settings panel exposing the peak-to-peak threshold (µV).

click → GetSuggestedRejections(threshold) → suggest_rejections(epochs, threshold)  [Python, peak-to-peak]
   → 'suggestedRejections' dataKey (fire-and-forget, NO runPython RPC)
   → SetSuggestedRejections → componentDidUpdate PRE-MARKS suggested epochs (additive, overridable)

Suggestions are advisory — they pre-mark epochs (with reasons shown) but you can unclick any of them; the actual drop still goes through apply_rejection, so the saved .fif stays MNE-exact.

3. Condition legend labels (New-B)
EpochReviewer + LiveErpPane legends now show human-readable condition labels via buildMarkerRegistry(stimuli).codeToLabel (fallback "Condition {code}").

Changes

  • utils.pysuggest_rejections(epochs, threshold_uv) (peak-to-peak, Marker excluded, drops nothing), native-tested.
  • backboneSuggestedRejection type, Get/SetSuggestedRejections, reducer suggestedRejections (reset on new arrays + cleanup), getSuggestedRejectionsEpic + message branch, requestSuggestRejections, AUTO_FLAG_EXPERIMENTS + DEFAULT_PTP_THRESHOLD_UV.
  • UIEpochReviewer (clickable bad-channel labels + lane wash + codeToLabel legend), LiveErpPane (codeToLabel legend), CleanComponent (badChannels state, warning Dialog, auto-flag button/settings, suggestion merge), CleanContainer (passes params).

Out of scope

Phase 3 (onboarding/guided mode). apply_rejection reused from Phase 1, untouched.

Verification

  • npm run typecheck0 errors · eslint0 errors (2 benign warnings)
  • npx vitest run44/44 · native pytest tests/analysis/17/17 (3 new suggest_rejections)
  • npm run buildgreen
  • Independent adversarial review traced all three features end-to-end; verdict PASS, no CRITICAL/WARNING.

Known behaviors flagged in review (non-blocking, candidate follow-ups)

  1. get_epochs_arrays/suggest_rejections use pick_types(eeg=True) (MNE default exclude='bads') — after a Clean that flags a bad channel, the re-fetched reviewer omits that channel from the display (saved .fif unaffected). Defensible; confirm the UX you want.
  2. Re-running Auto-flag with the same threshold re-adds suggestions the user had manually unclicked (additive merge).
  3. Threshold input has no min guard (0 flags everything).

Stacked on epoch-reviewer-phase1 (PR #224). 🤖 Generated with Claude Code

Dano Morrison added 3 commits July 7, 2026 10:33
Auto-flag = dev-gated one-click button + expandable threshold settings;
suggestions computed in Python (peak-to-peak), pre-marked but overridable.
Bad-channel flagging enabled on all devices with a warning Dialog on the 2nd
bad channel of a 4-ch dataset. Condition legends use codeToLabel.
…bels

Full-parity phase (docs/epoch-review-ui-plan.md §0/§9).

Bad-channel flagging (OQ5):
- Click a channel label to toggle it bad (struck-through red label + red lane
  wash across all epochs). Flows into the Phase-1 apply path:
  CleanEpochs({dropIndices, badChannels}) → apply_rejection sets info['bads'].
- shadcn Dialog warns when marking a 2nd bad channel on a 4-channel (Muse)
  dataset — informational, user can proceed.

Auto-flag (OQ4 + New-A, Python/MNE):
- Dev-gated "Auto-flag artifacts" button (AUTO_FLAG_EXPERIMENTS set, keyed by
  the EXPERIMENTS enum) + expandable peak-to-peak threshold settings.
- suggest_rejections(epochs, threshold_uv) in utils.py (peak-to-peak per epoch,
  Marker excluded; advisory — drops nothing), native-tested. New
  'suggestedRejections' dataKey (fire-and-forget, no runPython RPC) →
  SetSuggestedRejections → CleanComponent PRE-MARKS suggested epochs (additive
  union, fully overridable) and shows reasons. Actual drop still via
  apply_rejection, so the saved .fif stays MNE-exact.

Condition legend (New-B):
- EpochReviewer + LiveErpPane legends use human-readable labels via
  buildMarkerRegistry(stimuli).codeToLabel (fallback to "Condition {code}").

typecheck 0 · eslint 0 errors · vitest 44/44 · native pytest 17/17 · build green
Phase 3 (onboarding layer, OQ3 open — route via office-hours/ceo-review) and
Phase 4 (N-channel/WebGL, a11y, keyboard, perf; OQ7 open) added under Deferred.
Three non-blocking Phase 2 review notes (bad-channel display exclude='bads',
additive auto-flag re-merge, threshold min guard) added under tech debt.

@jdpigeon jdpigeon left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too many unnecessary comments. Add this repo's preference for avoiding comments outside of function, prop, or data structure definitions to learnings.md

Comment thread src/renderer/actions/pyodideActions.ts Outdated
// eslint-disable-next-line @typescript-eslint/no-explicit-any
ReceiveError: createAction<any, 'RECEIVE_ERROR'>('RECEIVE_ERROR'), // Worker error event — shape is dynamic
SetWorkerReady: createAction('SET_WORKER_READY'),
// Auto-flag: request suggestions (payload = peak-to-peak threshold in µV).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this comment

// all epochs; amp +/- scale trace amplitude. Rendering stays Canvas 2D + a DOM
// overlay for labels/hit-targets (no canvas hit-testing).
//
// Interactive (Phase 2): channel labels in the left gutter are click-to-flag

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these process-specific comments. Leaveo only a high level comment that concisely describes the intent and bejavoir of the component.


const REJECTED_TRACE_COLOR = 'rgba(120, 120, 120, 0.5)';
const REJECTED_FILL_COLOR = 'rgba(120, 120, 120, 0.15)';
// Translucent wash over a flagged bad channel's lane (drawn across all epochs).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

const firstShown = clampedStart + 1;
const lastShown = clampedStart + visibleCount;

// Unique condition codes (sorted) for the legend — mirrors the canvas draw's

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

return this.state.selectedSubject === subjectFromFilepath;
});

const codeToLabel = buildMarkerRegistry(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This smells wrong. buildMarkerRegistry sounds like something that should be executed once rather then to get a label on every render

Comment thread src/renderer/constants/constants.ts Outdated

// Experiments where the "Auto-flag artifacts" button is offered on the Clean
// screen. Dev-configurable — add/remove experiments here to opt them in/out.
export const AUTO_FLAG_EXPERIMENTS = new Set<EXPERIMENTS>([

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this way of registering auto flag. WHy don't we do it the inverse way: add an optional hideAutoFlagEpochs field to an experiment definition data structure -- maybe experiment params? does that not already exist?

Comment thread src/renderer/epics/pyodideEpics.ts Outdated
action$.pipe(
filter(isActionOf(PyodideActions.GetSuggestedRejections)),
pluck('payload'),
// Fire-and-forget: result returns via pyodideMessageEpic →

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this comment. This knowledge should exist in the pyodide skill if it does not already

Comment thread src/renderer/utils/webworker/index.ts Outdated
});
};

// Auto-flag: ask Python which epochs look like artifacts (peak-to-peak >

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

jdpigeon and others added 16 commits July 7, 2026 17:19
- Strip process-narrating / Phase-history comments (EpochReviewer top block,
  pyodideActions, pyodideEpics, webworker index, EpochReviewer color/legend);
  keep only definition-level intent comments.
- Invert auto-flag registration: replace the central AUTO_FLAG_EXPERIMENTS set
  with an opt-out hideAutoFlagEpochs field on ExperimentParameters (default on),
  colocated with the experiment definition.
- Memoize buildMarkerRegistry by stimuli reference so the code->label map isn't
  rebuilt on every CleanComponent render.
- Record the repo's comment-style preference in .llms/learnings.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019uiGzUcGN7j6qDWrfH5MCr
Lets any contributor drive the live BrainWaves Electron app over CDP
(agent-browser connect 9333) for QA/dogfooding, instead of a headless
browser that whitescreens without the preload bridge. SKILL.md self-
documents install (npm i -g agent-browser); skills-lock.json pins the
source + content hash. See .llms/learnings.md for the launch/attach loop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GaZP9vefeQV4CvNAm5Vv2Y
…very epoch

load_data built the MNE RawArray straight from the µV CSV values without the
µV→V scale MNE expects, so peak-to-peak came back ~1e6× inflated (tens of
millions of "µV") and suggest_rejections flagged all epochs → blank ERPs.

- Scale eeg rows ×1e-6 in load_data, keyed on ch_type=='eeg' (stim/Marker row
  untouched; marker codes stay numeric).
- Emit µV (×1e6) at the two display boundaries — the epoch-viewer buffer
  (get_epochs_arrays) and the ERP plot (already labeled uV) — so the viewer and
  plot show the same numbers as before while MNE-internal units are now correct.
- Regression guard: assert normal ptp lands 1–1000µV (not ~1e7).
- Fix test_detects_injected_artifact: it added a DC offset (ptp-neutral) and
  only "passed" because the units bug flagged everything; now injects a real
  single-sample spike.

QA plan 6e (T1).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
openWorkspaceDir passed a relative "BrainWaves_Workspaces/<title>" to
shell.showItemInFolder, which silently no-ops on non-absolute paths — the Home
screen "Go to Folder" button did nothing. Add a shell:openWorkspaceDir handler
that resolves the absolute path with getWorkspaceDir and opens it via
shell.openPath; point the renderer at it.

QA plan 9a (T4).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
isEEGEnabled defaulted to false — nonsensical for an EEG app, and a run would
silently record behavior only with no warning. Flip the default to true, and
guard handleStartExperiment: if EEG is disabled, or enabled but no device is
connected, confirm ("...records responses but no brain data. Continue anyway?")
before starting, reusing the existing showMessageBox pattern.

QA plan 7 (T3).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
Muse is Bluetooth — there is no USB dongle. Drop the COMPUTER_CONNECTABILITY
"Insert the USB Receiver" step so "Turn your headset on" → Next goes straight to
searching for devices.

QA plan 8 (T6).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
…TIMULUS_n

buildMarkerRegistry now derives display labels from the experiment's own
stimulus.condition, so Clean counts/legend and the ERP read "Face"/"House"
instead of the generic STIMULUS_1/2. Numeric codes are untouched (they still
drive the CSV Marker column and MNE event_id — labels are display-only). Falls
back to STIMULUS_n when a stimulus has no condition, and refuses to collapse two
codes under one label (would silently merge conditions in analysis). The Clean
legend and Python ERP legend pick the names up automatically (both read off the
registry / epochs.event_id).

QA plan 6b (T7).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
The savedEpochs branch wrote whatever buffer arrived and only caught write
errors. An empty/dropped buffer wrote nothing and errored nothing, leaving the
Analyze picker mysteriously empty. Guard: if the buffer is missing or
zero-length, surface an error toast instead of failing silently.

QA plan 9b (T5).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
Cleaning with all epochs flagged produced an empty, unanalyzable dataset (and
previously wrote a degenerate .fif with no error) — the source of the empty
Analyze picker after auto-flag over-rejected. handleCleanData now confirms
("This will reject all N epochs, leaving nothing to analyze") before dropping
when the reject set covers every epoch.

Note: this file also carries in-progress epoch-reviewer Phase 2 Clean-screen
changes that were already staged in the working tree; they are committed here
because they could not be separated non-interactively. The T2 guard is the
handleCleanData method + its wiring.

QA plan 6e/9b (T2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
- 6a: the ⚙ auto-flag settings button now uses Button size="icon" (h-9 w-9),
  matching the height of the adjacent Clean Data / Auto-flag buttons.
- 6g: replace the raw µV number input with a range slider labeled by effect
  ("More flags ← → Fewer flags"), showing the current µV inline and via
  aria-valuetext. Bounds come from getPtpThresholdPreset(device) — a single
  validated range today, with a per-device seam for later (no fabricated
  per-hardware ceilings). Only meaningful now that units are fixed (T1).

QA plan 6a/6g.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
When a run finished, RunComponent silently re-rendered the identical pre-run
landing — no confirmation it recorded, and the only way forward (top nav) was
undiscoverable. Track hasFinished and, once a run ends, show a completion panel
("Recording complete") with a primary "Clean your data →" link to the Clean
screen and a "Run again" secondary. Closes the Collect dead-end.

QA plan 5.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
- 3: replace the Emotiv saline-soak help tip with device-agnostic guidance
  (clean bare skin, sweep hair) that also explains sensor settling; rename the
  HELP_STEP.SIGNAL_SALINE step to SIGNAL_SETTLING. Mount the HelpButton +
  HelpSidebar on the Explore screen (previously Collect-only).
- 1 (lightweight): show an amber role="status" settling banner for the first
  ~45s after connecting ("a red, jumpy signal is normal while sensors make
  contact… watch it turn green"), auto-dismissed by a timer and manually
  closeable. No change to the signal-quality math — the honest red state still
  surfaces after the window. Full amber per-dot settling state (DF2) deferred to
  a live design-review pass.

QA plan 3 + 1 (lightweight).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
…dedupe wrapper

Concrete markup fixes for the pre-run landing (QA item 4), short of the full
two-column DF1 redesign (deferred to a live design-review):
- The "mystery empty bar" was a `w-full` button holding only a pencil emoji;
  now a small ghost icon button beside the title.
- Collapse the redundant nested h-screen/gradient wrapper (the outer container
  already provides it) into a centered flex.
- Wrap the subject/group/session summary in a compact shadcn Card, right-size
  the Run Experiment button (no longer w-full), and drop the <h1>{title}</h1>
  that duplicated the nav breadcrumb.

QA plan 4 (markup subset; DF1 live-signal column deferred).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
When no cleaned data exists, the "Select Clean Datasets" picker rendered an
empty box with no guidance — the confusing blank Analyze screen from the QA
walkthrough. Show a warm message ("No cleaned data yet — clean a recording
first…") with a "Go to Clean →" link instead.

QA plan 9b / T11 (Analyze empty state).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
Checks off T1–T7, 6a/6g, #3/#4-markup/#5, lightweight #1, and the Analyze empty
state (commits beb1620..376740f), and documents why #2/#6c (profile-first),
#4-DF1 + DF2-full (visual/design-review), and #6d (Phase-3 product) are deferred.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTrEip1cj1QLTU48qr9jLw
- Refactored the web worker's message handling to ensure asynchronous commands are processed in sequence, preventing overlap and ensuring Python globals are available before subsequent commands.
- Introduced a new function `load_clean_epochs` in `utils.py` to load and concatenate cleaned .fif epoch files from MEMFS paths, improving data handling.
- Updated the `loadCleanedEpochs` function in `index.ts` to utilize the new `load_clean_epochs` function for better clarity and functionality.
- Adjusted message posting logic to prevent sending non-structured-cloneable MNE objects back to the main thread.

QA plan updates included for the new functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant