Skip to content

[Parity] Add core matrix contracts - #140

Merged
suraj-ranganath merged 2 commits into
feature/eeglab-core-parity-completionfrom
phase/1-parity-matrix-contracts
Jun 5, 2026
Merged

[Parity] Add core matrix contracts#140
suraj-ranganath merged 2 commits into
feature/eeglab-core-parity-completionfrom
phase/1-parity-matrix-contracts

Conversation

@suraj-ranganath

Copy link
Copy Markdown
Member

Add the Phase 1 EEGLAB core parity matrix, validator tooling, focused tests, and development documentation for later parity phases. This establishes the status taxonomy, stale-skip contract, and standalone runtime rule without porting feature behavior.

Fixes #132
Part of #131

@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown

Claude finished @suraj-ranganath's task in 3m 22s —— View job


Code review

  • Overall assessment: Needs changes (one stray file); the tooling itself is solid.
  • Highest-risk area: Accidentally-committed .notes/implementation-notes.html
  • Merge recommendation: Needs changes

This is a documentation/tooling-only PR (parity matrix + validator + tests + docs). No runtime behavior is ported, and the runtime-boundary rule is correctly upheld — only the validator/tests read src/eegprep/eeglab, never package code. The validator is clean, the test set is well-targeted, and CI checks out the EEGLAB submodule (submodules: true), so test_committed_eeglab_parity_matrix_validates will pass.

Blocking

None.

Important

.notes/implementation-notes.html looks accidentally committed and is out of scope.
.gitignore excludes .notes/*, allowing only .gitkeep and eeglab-migration-gap-audit.md (lines 4–6), so this file had to be force-added. Its first section — "Session History Progress Bugfix Notes" about pop_resample/ICA store behavior — is leftover scratch from an unrelated task, not Phase 1 parity content. Agent scratch notes shouldn't be tracked in the repo.
Fix: git rm .notes/implementation-notes.html and drop it from the PR (move the Phase 1 design rationale into the PR description or docs/ if it's worth keeping).
Fix this →

Nits

  • tools/validate_eeglab_parity_matrix.py is an unused duplicate entry point. It only re-exports and calls main from eeglab_parity_matrix.py; it's referenced by no docs, test, CI, or pre-commit hook (the docs tell users to run python -m tools.eeglab_parity_matrix, and the test imports main from the module directly). Either remove it or wire it into the documented workflow so it isn't dead code.

  • Tests silently depend on an initialized submodule. When src/eegprep/eeglab isn't checked out, discover_in_scope_eeglab_paths (tools/eeglab_parity_matrix.py:105) returns an empty set, so every matrix row trips the "classifies out-of-scope EEGLAB path" branch and test_committed_eeglab_parity_matrix_validates fails with thousands of misleading errors rather than a clear skip. CI is fine, but a local uv run pytest tests/test_eeglab_parity_matrix.py without submodules is very confusing. Consider pytest.skip(...) when the functions tree is absent.

Test gaps

The suite is solid (validation pass, full-taxonomy presence, missing-classification failure, stale-policy failure, CLI JSON, docs assertions). One validator branch is untested: the seen_paths - expected_paths "classifies out-of-scope EEGLAB path" error. A small test that injects a bogus in-matrix path would lock in that behavior. Low priority.

EEGLAB parity notes

No parity risk in this PR — it adds no EEG-data behavior. Classification accuracy of the 8378-line matrix can't be independently verified without the pinned EEGLAB submodule, but the validator's coverage check (expected-vs-seen .m files) is a sound drift guard and matlab_runtime_skip/external_dependency_skip are used correctly to keep MATLAB-path/GUI-shim/external-toolbox functions out of standalone runtime scope.
· branch phase/1-parity-matrix-contracts

@suraj-ranganath

Copy link
Copy Markdown
Member Author

🤖

Problem — .notes/eeglab-migration-gap-audit.md identified the first seven EEGLAB core parity gaps, but there was no committed function-level contract. Without one, later phase agents could silently miss public helpers, overuse stale skips, or introduce runtime dependence on the vendored EEGLAB tree.

Approach — docs/parity/eeglab_core_parity_matrix.json now classifies 608 in-scope EEGLAB functions, including the explicit clean_asr audit row. tools/eeglab_parity_matrix.py:130 validates schema, status taxonomy, stale-skip evidence, duplicates, and full coverage. docs/source/development.rst:204 documents update rules, stale policy, and the standalone runtime boundary.

Key code:

expected_paths = discover_in_scope_eeglab_paths(repo_root)
seen_paths: set[str] = set()
status_counts: Counter[str] = Counter()
category_counts: Counter[str] = Counter()

for index, row in enumerate(rows):
    location = f"rows[{index}]"
    if not isinstance(row, dict):
        errors.append(MatrixValidationError(location, "must be an object"))
        continue
    _validate_row(row, location, errors)
    eeglab_path = row.get("eeglab_path")
    if isinstance(eeglab_path, str):
        if eeglab_path in seen_paths:
            errors.append(MatrixValidationError(location, f"duplicates eeglab_path {eeglab_path!r}"))
        seen_paths.add(eeglab_path)

Tests — tests/test_eeglab_parity_matrix.py covers committed matrix validation, complete status taxonomy, missing-row failure, stale-policy failure, JSON CLI output, and docs contract text. Verified with focused pytest, full ruff check/format check, ty, and ./pre-commit.py --changed-from origin/feature/eeglab-core-parity-completion.

@suraj-ranganath
suraj-ranganath merged commit 3f931f7 into feature/eeglab-core-parity-completion Jun 5, 2026
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