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

.llms/learnings.md

@@ -9,6 +9,16 @@ Format: brief heading + explanation + (optional) relevant file paths.
 
 <!-- Add entries below this line -->
 
+## Comment style: keep them on definitions, not inside logic
+
+This repo prefers minimal comments. Write them on **function, prop, or data-structure
+definitions** (docstrings / JSDoc, a one-liner over a type field, an interface member) —
+they describe intent and behavior that outlives the implementation. Avoid comments
+*inside* function bodies that narrate the process step-by-step, restate the code, or
+tag "Phase N"/PR history; that knowledge belongs in the definition's summary, the
+relevant skill (e.g. the pyodide skill), or git history. A component's top-of-file
+comment should concisely state what it is and does — not a diagram of every branch.
+
 ## Markers: device-agnostic injection via the EEGDriver interface
 
 Marker injection used to be Muse-only and lived in the UI (`RunComponent.eventCallback`

TODOS.md

@@ -15,12 +15,15 @@ Deferred and in-flight work. Keep this current — when something ships, delete
 - [ ] External / generic LSL stream support (any EEG device).
 - [ ] Lab.js cleanup — remove jspsych, type lab.js data. Not on the content critical path; users never see it.
 - [ ] Lesson surface beyond markdown — block-based programming, embedded notebooks (the CLAUDE.md extensibility horizon).
+- [ ] **Epoch reviewer Phase 3 — onboarding layer.** Plan: `docs/epoch-review-ui-plan.md` §9. Plain-language explanations of epochs + each artifact type, a **guided mode** (step through auto-flagged epochs with "why we flagged this," student confirms/overrides), channel legend tied to head position (Muse 10-20), student-facing tone. Builds on the Phase 0-2 reviewer (PRs #223/#224/#225). **Open question OQ3 (onboarding depth) is still unresolved** — how much curriculum (tooltips only vs. a real walkthrough), guided-mode-as-default? This is product-shaped, not architecture — take it through `/office-hours` or `/plan-ceo-review` before forging. Note it overlaps the "in-app lesson surface" critical-path work above; sequence deliberately.
+- [ ] **Epoch reviewer Phase 4 — polish & generalize.** Plan: `docs/epoch-review-ui-plan.md` §9. N-channel devices (Neurosity 8-ch, external LSL 32-64-ch) — the renderer already windows/downsamples but needs real testing at scale and possibly the WebGL swap behind `drawEpochs` (OQ1 left that a later swap). Accessibility, keyboard-first flow, performance. Also OQ7 (keep a read-only static-SVG fallback for environments where the interactive UI can't run?) is still open.
 
 ## Known issues / tech debt
 
 - [ ] **(Optional) Full Pyodide worker RPC** — the analysis/Clean pipeline crash is now **fixed** (harvested from PR #194): a `dataKey` routing pattern parallel to `plotKey` — the worker echoes `dataKey` + PyProxy-converted results, and `pyodideMessageEpic` routes `epochsInfo`→`SetEpochInfo` / `channelInfo`→`SetChannelInfo`; the info epics are fire-and-forget. This unblocks the pipeline without the bigger refactor. The deeper latent issue remains, though: `worker.postMessage` returns `undefined` on *post*, so the `await`s in `webworker/index.ts` are no-ops and cross-message sequencing still relies on worker FIFO. A true `runPython(worker, code, ctx?)` RPC — `Map<id,{resolve,reject}>` + one `message` listener, worker echoes `id` — would let epics `await` real results and delete the `plotKey`/`dataKey` switch entirely. Only worth doing if the FIFO sequencing ever actually bites; not urgent now.
 - [ ] Pyodide-fidelity smoke test — analysis pipeline is tested against native MNE, not yet under Pyodide/WASM (see `.llms/learnings.md`). **In progress:** the epoch-review Phase 0 (see `docs/epoch-review-ui-plan.md` §0) adds a *narrow* Pyodide test for the `get_epochs_arrays` float32 buffer path (byteLength, decode-vs-native, transfer detaches source); the full-pipeline Pyodide job remains deferred.
 - [ ] Pre-existing TypeScript errors (not regressions): `experimentEpics.ts` (RxJS operator types), `routes.tsx` (Redux container prop types).
+- [ ] **Epoch reviewer Phase 2 polish (from PR #225 review).** Three non-blocking behaviors flagged during the Phase 2 forge: (1) `get_epochs_arrays`/`suggest_rejections` use `pick_types(eeg=True)` whose MNE default is `exclude='bads'`, so after a Clean that flags a bad channel the re-fetched reviewer omits that channel from the display (saved `.fif` is unaffected) — decide whether to keep bad channels visible-but-greyed (`exclude=[]`) instead of vanishing; (2) re-running "Auto-flag" with the same threshold re-adds suggestions the user had manually unclicked (additive union merge in `CleanComponent.componentDidUpdate`); (3) the auto-flag threshold `<input>` has no min guard (0 µV flags everything). All in `src/renderer/components/CleanComponent/` + `webworker/utils.py`.
 
 ## Done recently
 

docs/epoch-review-ui-plan.md

@@ -45,9 +45,32 @@ temper→forge; Phases 2–4 remain roadmap.
 - **Delivery → two sequential PRs:** Phase 0 (transport + static render), then
   Phase 1 (interaction + apply + live ERP).
 
+### Phase 2 decisions (2026-07-07)
+
+- **OQ4 (auto-flag UX) → one-click button + expandable settings, dev-gated.** An
+  "Auto-flag artifacts" button a newcomer can just press; an expandable settings
+  panel exposes the peak-to-peak threshold(s) for users who want to control/learn
+  more. The button is **dev-configurable per experiment** (a constant set keyed by
+  the `EXPERIMENTS` enum, gated on `CleanComponent`'s `type` prop) so devs opt
+  experiments in/out.
+- **New-A (where auto-flag runs) → Python/MNE.** A worker round-trip
+  `suggest_rejections(epochs, threshold)` computes peak-to-peak per epoch in Python
+  and returns suggested indices + reasons (new `suggestedRejections` dataKey,
+  extends the OQ2 pattern). Suggestions **pre-mark** epochs in the reviewer and are
+  fully **overridable**; the actual drop still goes through the Phase-1
+  `apply_rejection` path so the saved `.fif` stays MNE-exact.
+- **OQ5 (bad channels on low-channel devices) → enabled everywhere, with a guard.**
+  Click a channel label to toggle it bad; bad channels flow to `apply_rejection`'s
+  `bad_channels` param (already built in Phase 1). On a 4-channel dataset, a
+  **warning Dialog** appears when the user marks a 2nd bad channel, nudging them to
+  re-collect if signal quality is that poor (informational — they can proceed).
+- **New-B (condition legend) → human-readable labels via `codeToLabel`.** Plumb the
+  marker registry's `codeToLabel` (built from `state.experiment.params.stimuli`)
+  into the reviewer + Live ERP legends instead of raw numeric codes.
+- **Delivery → one PR (Phase 2)**, stacked on Phase 1.
+
 Remaining open (not forge-blocking; decide when their phase comes): OQ3 (onboarding
-depth), OQ4 (auto-reject UX/thresholds), OQ5 (bad channels on 4-ch Muse), OQ7
-(static fallback).
+depth, Phase 3), OQ7 (static fallback).
 
 ---
 
@@ -342,10 +365,11 @@ Traced end to end against `webworker/`, `src/main/index.ts`, `src/preload/index.
    `runPython` RPC (reinforced by OQ6's client-side live ERP needing no round-trip).
 3. **Onboarding depth** — Is guided mode the default? How much curriculum
    (tooltips only vs. a real walkthrough)?
-4. **Auto-rejection** — Expose peak-to-peak thresholds to the user, or keep them
-   as invisible "suggestions"? What defaults?
-5. **Bad channels on Muse** — dropping 1 of 4 channels is drastic; do we support
-   channel rejection for low-channel devices, or epochs-only there?
+4. **Auto-rejection — RESOLVED (§0, Phase 2): one-click "Auto-flag" button +
+   expandable threshold settings, dev-gated per experiment; suggestions computed in
+   Python (peak-to-peak) and pre-marked but overridable.**
+5. **Bad channels on Muse — RESOLVED (§0, Phase 2): enabled on all devices; a
+   warning Dialog fires when marking a 2nd bad channel on a 4-channel dataset.**
 6. **Live ERP preview — RESOLVED (§0): in scope for v1 (Phase 1), computed
    client-side** over the Phase-0 buffer (Flavor 1 only; real-time-during-experiment
    is a separate future non-Python effort).
@@ -396,8 +420,15 @@ Traced end to end against `webworker/`, `src/main/index.ts`, `src/preload/index.
   - Reaches functional parity-plus with the *essential* MNE workflow.
   - **Note:** the write-back bridge (§6d) already landed standalone in PR #222, so
     Phase 1 just points the apply action at a bridge that works.
-- **Phase 2 — Full parity.** Bad-channel flagging, condition coloring/legend,
-  auto-flag suggestions from `drop_log`/peak-to-peak.
+- **Phase 2 — Full parity (PR 4). LOCKED (§0 Phase 2 decisions).**
+  - **Bad-channel flagging:** click a channel label → toggle bad; bad channels flow
+    to the existing `apply_rejection` `bad_channels` param. Warning Dialog on the 2nd
+    bad channel of a 4-ch dataset (OQ5).
+  - **Auto-flag (OQ4 + New-A):** dev-gated "Auto-flag artifacts" button + expandable
+    threshold settings → `suggest_rejections(epochs, threshold)` in Python (peak-to-
+    peak, native-testable) → `suggestedRejections` dataKey → pre-marks epochs in the
+    reviewer (overridable) with reasons shown.
+  - **Condition legend (New-B):** real labels via `codeToLabel`.
 - **Phase 3 — Onboarding layer.** Explanations, guided mode, artifact tutorials,
   live ERP preview.
 - **Phase 4 — Polish & generalize.** N-channel devices (Neurosity/LSL),

src/renderer/actions/pyodideActions.ts

@@ -14,6 +14,15 @@ export interface EpochArraysMeta {
   drop_log: string[][];
 }
 
+// Auto-flag: a single artifact suggestion returned by Python's
+// suggest_rejections(epochs, threshold_uv) — one epoch/channel over threshold.
+export interface SuggestedRejection {
+  index: number;
+  channel: string;
+  peak_uv: number;
+  reason: string;
+}
+
 // -------------------------------------------------------------------------
 // Actions
 
@@ -56,6 +65,13 @@ export const PyodideActions = {
   // 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'),
+  GetSuggestedRejections: createAction<number, 'GET_SUGGESTED_REJECTIONS'>(
+    'GET_SUGGESTED_REJECTIONS'
+  ),
+  SetSuggestedRejections: createAction<
+    SuggestedRejection[],
+    'SET_SUGGESTED_REJECTIONS'
+  >('SET_SUGGESTED_REJECTIONS'),
 } as const;
 
 export type PyodideActionType = ActionType<

src/renderer/components/CleanComponent/EpochReviewer.tsx

@@ -8,33 +8,23 @@ import {
   epochChannelSeries,
 } from './epochArrays';
 
-// ---------------------------------------------------------------------------
-// Canvas layout (matches MNE's epochs.plot): epochs run ACROSS (x), channels
-// are STACKED vertically (y). One trace per (epoch, channel) cell.
-//
-//   [◀ Prev] [Next ▶]              [amp －] [amp ＋]
-//            epoch 0        epoch 1        epoch 2      ...
-//          ┌────────────┬────────────┬────────────┐
-//   ch 0   │  ~~~~~~~~   │ ░░grey░░✕░░ │  ~~~~~~~~   │  channel lane
-//          ├────────────┼────────────┼────────────┤    (rejected column
-//   ch 1   │  ~~~~~~~~   │ ░░░░░░░░░░ │  ~~~~~~~~   │     is greyed out)
-//          ├────────────┼────────────┼────────────┤
-//   ch 2   │  ~~~~~~~~   │ ░░░░░░░░░░ │  ~~~~~~~~   │
-//          └────────────┴────────────┴────────────┘
-//             epoch 0       epoch 1       epoch 2      (bottom index labels)
-//
-// Interactive (Phase 1): a transparent DOM overlay div per visible column makes
-// each epoch click-to-reject (rejected epochs grey out); Prev/Next page through
-// all epochs; amp ＋/－ scale trace amplitude. Rendering stays Canvas 2D + a DOM
-// overlay for labels/hit-targets (no canvas hit-testing).
-// ---------------------------------------------------------------------------
+// Interactive epoch reviewer: epochs run across (x), channels stacked (y).
+// Click an epoch column to reject it; click a channel label to flag it bad
+// (its lane washes red across all epochs). Prev/Next paginate, amp +/- scales
+// traces. Canvas 2D for traces, a DOM overlay for labels/click targets.
 
 interface Props {
   epochArrays: { buffer: ArrayBuffer; meta: EpochArraysMeta } | null;
   // ABSOLUTE epoch indices the student has marked for rejection.
   rejected: Set<number>;
   // Toggle a single ABSOLUTE epoch index in/out of the rejected set.
   onToggleEpoch: (index: number) => void;
+  // Channel names the student has flagged as "bad" (controlled by the parent).
+  badChannels: Set<string>;
+  // Toggle a single channel name in/out of the bad-channel set.
+  onToggleChannel: (name: string) => void;
+  // Optional map from numeric event code to a human-readable condition label.
+  codeToLabel?: Record<number, string>;
 }
 
 // Logical canvas size (scaled up for devicePixelRatio at draw time).
@@ -56,11 +46,15 @@ const GAIN_MAX = 20;
 
 const REJECTED_TRACE_COLOR = 'rgba(120, 120, 120, 0.5)';
 const REJECTED_FILL_COLOR = 'rgba(120, 120, 120, 0.15)';
+const BAD_CHANNEL_FILL_COLOR = 'rgba(200, 60, 60, 0.10)';
 
 export default function EpochReviewer({
   epochArrays,
   rejected,
   onToggleEpoch,
+  badChannels,
+  onToggleChannel,
+  codeToLabel,
 }: Props): JSX.Element {
   const canvasRef = useRef<HTMLCanvasElement | null>(null);
   // First epoch of the current page (absolute index).
@@ -100,7 +94,7 @@ export default function EpochReviewer({
     ctx.clearRect(0, 0, CANVAS_WIDTH, CANVAS_HEIGHT);
 
     const { buffer } = epochArrays;
-    const { n_epochs, n_channels, n_times, event_codes } = meta;
+    const { n_epochs, n_channels, n_times, event_codes, ch_names } = meta;
 
     const plotWidth = CANVAS_WIDTH - LABEL_GUTTER;
     const plotHeight = CANVAS_HEIGHT - BOTTOM_GUTTER;
@@ -120,6 +114,20 @@ export default function EpochReviewer({
       }
     }
 
+    // Translucent red wash over flagged bad-channel lanes — spans every epoch
+    // column (drawn under traces) so bad channels read at a glance.
+    for (let ch = 0; ch < n_channels; ch += 1) {
+      if (badChannels.has(ch_names[ch])) {
+        ctx.fillStyle = BAD_CHANNEL_FILL_COLOR;
+        ctx.fillRect(
+          LABEL_GUTTER,
+          ch * laneHeight,
+          CANVAS_WIDTH - LABEL_GUTTER,
+          laneHeight
+        );
+      }
+    }
+
     // Faint lane dividers (channels).
     ctx.strokeStyle = 'rgba(0, 0, 0, 0.08)';
     ctx.lineWidth = 1;
@@ -235,7 +243,7 @@ export default function EpochReviewer({
         ctx.stroke();
       }
     }
-  }, [epochArrays, meta, rejected, clampedStart, perPage, gain]);
+  }, [epochArrays, meta, rejected, clampedStart, perPage, gain, badChannels]);
 
   // Empty state — friendly, brand-styled, student-facing.
   if (!epochArrays || !meta || meta.n_epochs === 0) {
@@ -252,6 +260,10 @@ export default function EpochReviewer({
   const firstShown = clampedStart + 1;
   const lastShown = clampedStart + visibleCount;
 
+  const uniqueSortedCodes = [...new Set(meta.event_codes)].sort(
+    (a, b) => a - b
+  );
+
   return (
     <div className="text-left">
       <div className="mb-2 flex items-center justify-between">
@@ -306,22 +318,41 @@ export default function EpochReviewer({
           style={{ width: CANVAS_WIDTH, height: CANVAS_HEIGHT }}
         />
 
-        {/* Channel labels (left gutter). */}
-        {meta.ch_names.map((name, ch) => (
-          <div
-            key={name}
-            className="absolute truncate pr-1 text-right text-[10px] text-gray-500"
-            style={{
-              left: 0,
-              width: LABEL_GUTTER,
-              top: ch * laneHeight,
-              height: laneHeight,
-              lineHeight: `${laneHeight}px`,
-            }}
-          >
-            {name}
-          </div>
-        ))}
+        {/* Channel labels (left gutter) double as click-to-flag bad-channel
+            toggles. A flagged channel renders struck-through + red and its lane
+            is washed red across every epoch column (see canvas draw). */}
+        {meta.ch_names.map((name, ch) => {
+          const isBad = badChannels.has(name);
+          return (
+            <div
+              key={name}
+              role="button"
+              tabIndex={0}
+              aria-pressed={isBad}
+              aria-label={`${isBad ? 'Unflag' : 'Flag'} channel ${name} as bad`}
+              title={`${isBad ? 'Unflag' : 'Flag'} channel ${name} as bad`}
+              className={`absolute cursor-pointer truncate pr-1 text-right text-[10px] ${
+                isBad ? 'text-red-500 line-through' : 'text-gray-500'
+              }`}
+              style={{
+                left: 0,
+                width: LABEL_GUTTER,
+                top: ch * laneHeight,
+                height: laneHeight,
+                lineHeight: `${laneHeight}px`,
+              }}
+              onClick={() => onToggleChannel(name)}
+              onKeyDown={(e) => {
+                if (e.key === 'Enter' || e.key === ' ') {
+                  e.preventDefault();
+                  onToggleChannel(name);
+                }
+              }}
+            >
+              {name}
+            </div>
+          );
+        })}
 
         {/* Transparent click targets — one per visible epoch column. Clicking
             toggles that ABSOLUTE epoch index in/out of the rejected set. */}
@@ -375,6 +406,28 @@ export default function EpochReviewer({
         })}
       </div>
 
+      {/* Condition legend: one swatch + human-readable label per unique code. */}
+      {uniqueSortedCodes.length > 0 && (
+        <div className="mt-2 flex flex-wrap gap-3">
+          {uniqueSortedCodes.map((code) => (
+            <span
+              key={code}
+              className="flex items-center gap-1 text-xs text-gray-600"
+            >
+              <span
+                className="inline-block h-3 w-3 rounded-sm"
+                style={{
+                  backgroundColor: cssColorForIndex(
+                    conditionIndexForCode(code, uniqueSortedCodes)
+                  ),
+                }}
+              />
+              {codeToLabel?.[code] ?? `Condition ${code}`}
+            </span>
+          ))}
+        </div>
+      )}
+
       <p className="mt-1 text-xs text-gray-500">
         showing {firstShown}–{lastShown} of {meta.n_epochs} epochs
         {rejected.size > 0 && ` · ${rejected.size} marked for rejection`}

src/renderer/components/CleanComponent/LiveErpPane.tsx

@@ -24,6 +24,8 @@ interface Props {
   epochArrays: { buffer: ArrayBuffer; meta: EpochArraysMeta } | null;
   // ABSOLUTE epoch indices marked for rejection (excluded from the average).
   rejected: Set<number>;
+  // Optional map from numeric event code to a human-readable condition label.
+  codeToLabel?: Record<number, string>;
 }
 
 const CANVAS_WIDTH = 640;
@@ -36,6 +38,7 @@ const PAD_BOTTOM = 12;
 export default function LiveErpPane({
   epochArrays,
   rejected,
+  codeToLabel,
 }: Props): JSX.Element {
   const canvasRef = useRef<HTMLCanvasElement | null>(null);
   const [channel, setChannel] = useState(0);
@@ -249,7 +252,7 @@ export default function LiveErpPane({
                   ),
                 }}
               />
-              Condition {code} ({count})
+              {codeToLabel?.[code] ?? `Condition ${code}`} ({count})
             </span>
           );
         })}

src/renderer/components/CleanComponent/__tests__/EpochReviewer.test.tsx

@@ -43,6 +43,8 @@ describe('EpochReviewer', () => {
         epochArrays={makeEpochArrays()}
         rejected={new Set()}
         onToggleEpoch={onToggleEpoch}
+        badChannels={new Set()}
+        onToggleChannel={vi.fn()}
       />
     );
 
@@ -61,6 +63,8 @@ describe('EpochReviewer', () => {
         epochArrays={makeEpochArrays()}
         rejected={new Set([0])}
         onToggleEpoch={onToggleEpoch}
+        badChannels={new Set()}
+        onToggleChannel={vi.fn()}
       />
     );