Land Epoch Reviewer stack, remove stale docs/dead code, gate the release pipeline

.github/workflows/release.yml

@@ -6,10 +6,43 @@ on:
       - 'v*'
 
 jobs:
+  # Gate. A tag must not publish anything until the same checks CI runs on every
+  # push have passed on the tagged commit. Runs once rather than per-OS — test.yml
+  # already covers the cross-OS matrix — but nothing is built or uploaded until
+  # this job is green.
+  verify:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Check out Git repository
+        uses: actions/checkout@v4
+
+      - name: Install Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 22
+          cache: 'npm'
+
+      - name: Install dependencies
+        run: npm ci
+
+      - name: Test
+        run: npm test
+
+      - name: Typecheck
+        run: npm run typecheck
+
+      - name: Lint
+        run: npm run lint
+
   release:
+    needs: verify
     runs-on: ${{ matrix.os }}
 
     strategy:
+      # Stop the other platforms as soon as one fails, so a broken tag uploads
+      # as few half-good artifacts as possible.
+      fail-fast: true
       matrix:
         os: [macos-latest, windows-latest, ubuntu-latest]
 
@@ -24,11 +57,20 @@ jobs:
           cache: 'npm'
 
       - name: Install dependencies
-        run: npm install
+        run: npm ci
+
+      - name: Build
+        run: npm run build
+
+      # Between build and publish: refuse to ship a build whose Pyodide payload
+      # is missing. That failure is invisible in dev and breaks analysis entirely
+      # in the packaged app.
+      - name: Verify build artifacts
+        run: npm run test:build
 
-      - name: Build and publish
+      - name: Publish
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           USERNAME: ${{ secrets.USERNAME }}
           PASSWORD: ${{ secrets.PASSWORD }}
-        run: npm run package-ci
+        run: npx electron-builder build --publish always

.github/workflows/test.yml

@@ -24,8 +24,10 @@ jobs:
           node-version: 22
           cache: 'npm'
 
+      # npm ci, not npm install: installs exactly the locked tree, so the commit
+      # CI passes on is the commit that gets packaged.
       - name: Install dependencies
-        run: npm install
+        run: npm ci
 
       - name: Test
         run: npm test
@@ -38,3 +40,8 @@ jobs:
 
       - name: Build
         run: npm run build
+
+      # Must follow Build — asserts out/ and the Pyodide payload are actually
+      # on disk where the packaged app's protocol handler will look for them.
+      - name: Verify build artifacts
+        run: npm run test:build

.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`
@@ -173,8 +183,3 @@ Build note: with `module: ESNext` source but CommonJS main output, a guarded `re
 Experiments emit `this.data.correct_response` as a real boolean (`true`/`false`) and `response_given` as `'yes'`/`'no'` (see `src/renderer/utils/labjs/functions.ts`). But all consumers in `src/renderer/utils/behavior/compute.js` read data **after** it's been written to CSV and re-parsed, so every value is a **string**. That's why existing code gates on `row.correct_response === 'true'` and `row.response_given === 'yes'`, and parses numbers with `parseFloat`.
 
 Trap: a new metric written naively (`row.correct_response === true`, or arithmetic on an unparsed string) will silently return `false`/`0`/`NaN` for post-CSV data — and may *work* on pre-CSV in-memory data, so it passes a quick test and fails in production. Always compare against the string `'true'`/`'yes'` and `parseFloat` before doing math.
-
-## Pre-existing TypeScript errors (do not treat as regressions)
-
-- `src/renderer/epics/experimentEpics.ts` (lines 170, 205) — RxJS operator type mismatch
-- `src/renderer/routes.tsx` (lines 15-17) — Redux container component prop types

TODOS.md

@@ -6,6 +6,8 @@ Deferred and in-flight work. Keep this current — when something ships, delete
 
 - [ ] **Lesson content from Steve Azeka** — owner: Dano. Set a date for the first sample lesson. Fallback: if no lesson by that date, Dano authors one throwaway stub to unblock the lesson surface.
 - [ ] **In-app lesson surface v1** — static markdown render only (no editing, no interactivity). Build against a dummy lesson; tune to real content once a sample lands. Confirm markdown is a rich-enough format with Azeka before building.
+- [ ] **First release dry-run (`v1.0.0-rc.1`)** — the Release workflow has *never executed*; no tag has ever been pushed. It is now gated (verify job: test/typecheck/lint; artifact check between build and publish) and publishes as a **draft**, so a tag no longer auto-updates installed clients until someone publishes the draft by hand. Still unproven end to end: push an RC tag, watch all three OS legs build, download each artifact, launch it, and confirm the app opens and Pyodide reaches `ready`. Do this before any demo depends on a released build.
+- [ ] **Packaged-app launch smoke test** — `tests/build.check.ts` asserts the build output and Pyodide payload are present on disk, but nothing *launches* the packaged app. The gap is a real Electron smoke test (spawn the built app under `xvfb` on Linux, assert the window opens and the worker posts `ready`) — that is what would catch a prod-only Pyodide regression automatically instead of during a demo.
 - [ ] **Cross-platform LSL packaging verification** — `macOS arm64` is done + self-contained (see Done recently). Still needs each build machine: **macOS x64** (`patchDeps` only symlinks on arm64, so an Intel build uses node-labstreaminglayer's original x86_64 dylib — `otool -L` the packaged liblsl to confirm self-contained; the `afterPack` hook bundles whatever external deps it finds, arch-agnostic), **Windows x64** (bundled `lsl_amd64.dll` is a self-contained PE — confirm koffi loads it from the packaged app), **Linux/Ubuntu** (bundled `.so` is x86_64 ELF — `ldd` it + Web-Bluetooth smoke test, `--enable-experimental-web-platform-features` already set).
 
 ## Deferred (post-launch)
@@ -15,12 +17,14 @@ 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.** 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.** 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`).
-- [ ] Pre-existing TypeScript errors (not regressions): `experimentEpics.ts` (RxJS operator types), `routes.tsx` (Redux container prop types).
+- [ ] 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 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.
+- [ ] **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/device-connectivity.md

@@ -1,6 +1,10 @@
 # Device Connectivity
 
-How BrainWaves discovers and connects to EEG devices (currently: Muse only).
+How BrainWaves discovers and connects to EEG devices. Supported devices are Muse
+and Neurosity (first-party drivers) plus external LSL inlet streams; the flow
+diagrams below trace the **Muse** path specifically, since it is the most involved
+(Web Bluetooth device selection). The driver interface itself is device-agnostic —
+see the `redux-observable-epochs` skill for the `EEGDriver` contract.
 
 ---
 
@@ -12,7 +16,7 @@ Device connectivity spans three layers:
 |---|---|---|
 | **UI** | `CollectComponent/`, `EEGExplorationComponent` | Trigger search, display state, handle user selection |
 | **Epics** | `epics/deviceEpics.ts` | Orchestrate async device lifecycle via RxJS |
-| **Driver** | `utils/eeg/muse.ts` | Web Bluetooth API calls via `muse-js` |
+| **Driver** | `utils/eeg/index.ts` (registry) → `muse.ts`, `neurosity.ts`, `lslInlet.ts` | Per-device acquisition behind the shared `EEGDriver` interface (`utils/eeg/types.ts`) |
 
 All device state lives in Redux (`reducers/deviceReducer.ts`). Epics react to dispatched actions and fire new actions as side effects.