Phase 2: clean_rawdata and FIRFilt bundled-plugin completion

[suraj-ranganath]

🤖 Phase 2 completes the in-repo cleaning and filtering plugin depth that researchers use in preprocessing workflows.

Summary

Finish useful standalone parity for bundled clean_rawdata and firfilt workflows. This phase should not port Manopt or every MATLAB private helper blindly; it should deliver user-visible cleaning/filtering behavior, diagnostics, reports, boundary behavior, progress output, GUI parity, and tests.

Scope

• clean_rawdata:
  • Riemannian ASR behavior decisions and implementation where feasible.
  • asr_calibrate_r / asr_process_r equivalents or explicit optional-dependency limitation if not maintainable.
  • vis_artifacts-style diagnostics integrated with EEGPrep/EEGBrowser where useful.
  • EEGLAB-like progress messages for GUI plus eegprep-console users.
  • Nonfinite data handling and sample-data regression coverage.
• firfilt:
  • findboundaries, firfilt, firfiltsplit, firfiltreport, fir_filterdcpadded behavior where useful.
  • Order/window helpers: kaiserbeta, invkaiserbeta, invfirwsord, minphaserceps, windows.
  • Order dialogs/wrappers: pop_firpmord, pop_firwsord, pop_kaiserbeta, pop_xfirws if user-facing in EEGLAB.
  • Frequency-response plotting/reporting through native Python/Qt/matplotlib where appropriate.

Acceptance Criteria

• Cleaning/filtering workflows operate on continuous and epoched data, respect boundary events, and invalidate/reactivate ICA consistently with EEGLAB expectations.
• GUI dialogs and reports match EEGLAB layout, labels, defaults, enabled states, and workflow where applicable.
• eegprep-console receives replayable commands and progress output in the right order.
• MATLAB parity tests cover deterministic filter/order/helper outputs.
• Sample-data integration tests cover realistic cleaning/filtering flows.
• No broad Manopt port is added unless it is directly needed and justified.

Required References

• AGENTS.md
• src/eegprep/eeglab/plugins/clean_rawdata/
• src/eegprep/eeglab/plugins/firfilt/
• src/eegprep/plugins/clean_rawdata/
• src/eegprep/plugins/firfilt/
• .agents/skills/eegprep-feature-development/SKILL.md
• .agents/skills/eeglab-gui-visual-parity/SKILL.md
• .agents/skills/gui-agent-flow-qa/SKILL.md

/goal

/goal Complete standalone EEGPrep parity for useful clean_rawdata and firfilt bundled-plugin workflows. Use EEGLAB plugin source as the oracle, implement user-facing cleaning/filtering/report/diagnostic behavior without stale Manopt bloat or runtime EEGLAB dependency, preserve GUI plus eegprep-console synchronization, add MATLAB parity tests for deterministic helpers, sample_data integration tests, visual parity screenshots for dialogs/reports, progress-output tests, ruff/format/ty, non-slow pytest, and pre-commit. Fix review findings unless they are explicit scope creep and report any accepted limitations.

[claude]

🤖 Plan:

• Read issue and dependency context (epic Epic: Complete EEGPrep standalone EEGLAB parity and documentation #157, phase 1 Phase 1: Remaining-work audit expansion, runtime contract, and docs architecture #158, base PR [Parity] Complete remaining EEGLAB core gaps #152)
• Survey existing plugins/clean_rawdata/ and plugins/firfilt/ coverage and tests
• Duplicate-work preflight (no overlapping open PR)
• Post analysis (no PR — out of single-agent scope)

Research

This is an epic-phase issue, not a single bug fix. Scope spans plugins/clean_rawdata (Riemannian ASR depth, vis_artifacts diagnostics, progress messages, nonfinite handling), plugins/firfilt (5+ helpers, 4 order/window helpers, 4 pop_* order dialogs, frequency-response plots), plus MATLAB parity tests, sample-data integration tests, GUI visual parity screenshots, and console replay tests. Realistic size is multi-thousand-line and multi-PR, well beyond the <100 line / single-PR threshold for an agent fix.

Two hard gating dependencies block starting Phase 2 now: per epic #157, PR #152 must merge first (it is the base branch for the epic and is still open with MERGEABLE/CI green, awaiting review), and Phase 1 #158 must merge first (it defines the expanded parity matrix, optional_dependency/stale_skip classifications, and phase ownership rows that Phase 2 selects work from). Without #158, there is no committed source of truth telling Phase 2 which Riemannian/Manopt helpers are in-scope vs. classified matlab_runtime_skip.

Current state of the two target plugins (helpful for whoever picks this up):

Relevant code

• src/eegprep/plugins/clean_rawdata/asr_calibrate.py#L428 — useriemannian calibration branch exists; clean_asr.py#L77 still raises NotImplementedError for useriemannian=True
• src/eegprep/plugins/clean_rawdata/pop_clean_rawdata.py#L286 — _show_vis_artifacts already wired through GUI checkbox; full vis_artifacts diagnostic surface (vis_artifacts.m equivalent) is the gap
• src/eegprep/plugins/firfilt/ — has firws, firwsord, pop_eegfiltnew, pop_firma, pop_firpm, pop_firws; missing: findboundaries, firfiltsplit, firfiltreport, kaiserbeta/invkaiserbeta, invfirwsord, minphaserceps, pop_firpmord, pop_firwsord, pop_kaiserbeta, pop_xfirws
• tests/test_clean_rawdata.py, tests/test_clean_asr.py, tests/test_pop_firfilt.py, tests/test_gui_pop_clean_rawdata.py, tests/test_gui_pop_firfilt.py — existing coverage to extend, not replace
• src/eegprep/eeglab/ is empty in this checkout — the MATLAB parity oracle must be re-vendored or cloned locally before parity work can land

Proposed Approach

Do not implement this as one PR. Decompose into independent, mergeable sub-issues that fit the AGENTS.md "smallest change that solves the request" rule, each shipping its own tests + visual parity artifacts where applicable. Suggested split, ordered so later sub-issues depend on earlier ones:

1. clean_rawdata — Riemannian ASR processing closure: implement useriemannian end-to-end through clean_asr (remove the NotImplementedError), add deterministic numerical parity tests against MATLAB asr_calibrate_r/asr_process_r on a small fixture; if Manopt-backed refinement is not feasible standalone, classify it explicitly as optional_dependency in the Phase 1 matrix and document the limitation.
2. clean_rawdata — vis_artifacts diagnostic: expand _show_vis_artifacts into a full EEGBrowser-integrated comparison view matching EEGLAB layout, with visual parity screenshots.
3. clean_rawdata — progress messages + nonfinite handling: align logger output ordering with EEGLAB clean_artifacts console output, add eegprep-console replay test, regression test for NaN/Inf input.
4. firfilt — order/window helpers: port kaiserbeta, invkaiserbeta, invfirwsord, minphaserceps, windows with deterministic MATLAB parity tests (these are small, pure-numerical, easy to verify).
5. firfilt — boundary/split/report helpers: findboundaries, firfiltsplit, firfiltreport, fir_filterdcpadded with continuous + epoched sample-data integration tests.
6. firfilt — order dialog pop_* wrappers: pop_firpmord, pop_firwsord, pop_kaiserbeta, pop_xfirws with GUI visual parity via tools/visual_parity/ and console history coverage.
7. firfilt — frequency-response plotting: native matplotlib/Qt port of EEGLAB plotfresp-style reporting, visual parity screenshot.

Each sub-issue is sized to a single agent PR (~few hundred lines plus tests) and references the Phase 1 matrix row for its classification.

Blockers / asks for a human reviewer before implementation begins:

• Confirm gating order: should Phase 2 wait for PR [Parity] Complete remaining EEGLAB core gaps #152 + Phase 1: Remaining-work audit expansion, runtime contract, and docs architecture #158 to merge, or can sub-issues 4/5 (pure numerical helpers, no matrix dependency) start in parallel on a stacked branch?
• Manopt policy: classify Riemannian-with-Manopt as optional_dependency (pymanopt) or matlab_runtime_skip? Phase 1 should set this; without it, sub-issue 1 cannot pick a definition of done.
• Re-vendoring src/eegprep/eeglab/plugins/clean_rawdata/ and .../firfilt/ for parity oracle access — confirm preferred mechanism (git submodule? checked-in snapshot? developer-only make vendor?).

Will not open a PR. Awaiting human direction on the sub-issue split and the three blocker questions above.

[suraj-ranganath]

🤖 # Research

Phase 1 has merged into the epic branch, so #159 now has committed final-matrix rows to resolve. Current EEGPrep already covers standard clean_rawdata ASR and the four main FIR filter wrappers, but FIRFilt order/window helpers, public split/report/response utilities, and reusable clean_rawdata artifact diagnostics are still matrix gaps.

Relevant code

• src/eegprep/plugins/clean_rawdata/pop_clean_rawdata.py - current dialog and rejected-window browser
• src/eegprep/plugins/clean_rawdata/clean_artifacts.py - cleaning pipeline/progress surface
• src/eegprep/plugins/firfilt/_filtering.py - existing FIR design/apply implementation
• docs/parity/eeglab_final_parity_matrix.json - Phase 2: clean_rawdata and FIRFilt bundled-plugin completion #159 rows and statuses
• tests/test_pop_firfilt.py - FIR boundary/parity coverage to extend

Proposed Fix

Implement the missing useful standalone surfaces in one phase PR against feature/eeglab-full-standalone-completion: FIRFilt order/window/min-phase/boundary/report/response helpers, compact order pop wrappers/dialogs, report/plot hooks in the existing FIR wrappers, and a reusable clean_rawdata vis_artifacts diagnostic API layered on the existing EEGBrowser highlight path. Keep Riemannian ASR as the documented calibration-time path plus explicit optional-dependency limitation for full Manopt-backed processing; no Manopt vendoring or runtime EEGLAB dependency. Add focused unit, sample-data, MATLAB-skippable parity, visual-capture, docs/help, and matrix updates.

[suraj-ranganath]

🤖 Phase PR for this issue has merged into feature/eeglab-full-standalone-completion and integration checks are passing on the epic branch. Closing this phase issue as completed.