Skip to content

Opening a dataset mutates it: the shared dabrowska test corpus lost a session document, and nothing prevents a recurrence #99

Closed
Closed
Opening a dataset mutates it: the shared dabrowska test corpus lost a session document, and nothing prevents a recurrence#99

Description

@stevevanhooser
stevevanhooser
opened on Sep 1, 2026

Part of #90 (PR #61 catch-up). This one is a data-integrity problem in the test corpora rather than a port.

Root cause

Constructing a dataset writes to it. repairDatasetSessionInfo (src/ndi/dataset/_dataset.py:558) writes and deletes documents during construction. Three tutorial verification harnesses open the shared cache in place, so merely running them mutates the corpus.

The observed damage: the shared dabrowska corpus — a symlink into tutorials/_cache/67f723d574f5f79c6062389d — lost exactly one session document (doc_idx=2). Every other document class matches expectations to the unit, which is what makes the single-document loss diagnosable rather than ambiguous.

What it would take

  1. Test fixtures must copy before opening. The corpus fixture should hand each test a private copy, never the shared tree.
  2. Add a mutation canary — a checksum over the shared corpus, asserted before and after, attributed to the construction call itself so the next occurrence names its cause instead of surfacing later as a count mismatch. (PR NDI-python 2026-08 catch-up — hardening bundle + MATLAB parity + CI safety #61 commit 9359af7 exists because the first version of the canary did not attribute the mutation and the report was unusable.)
  3. Un-mask the per-loop type-count assert. It was masked, which is why the loss was not caught when it happened.
  4. Preflight corpus integrity and skip loudly with restore instructions, rather than pinning damaged counts. Explicitly: do not lower the thresholds — that would pin the damage as correct and destroy the signal.
  5. Restore the corpus: re-fetch dataset 67f723d574f5f79c6062389d.

Reference commits: d465ecc, 8c62b9e, 9359af7, 5cddab3.

The larger question this raises

Steps 1–4 protect the corpus. They do not address the underlying fact that opening a dataset read-only is not read-only. That is a design question worth its own decision: whether repairDatasetSessionInfo should run on construction at all, or be an explicit call. Any user opening a dataset to look at it is currently writing to it, and a shared or read-only-mounted dataset will behave differently from a private one.

I would treat the fixture work as the immediate fix and the construction-time mutation as a separate design item.

Related, not the same

PR #61 also carried a prod data-durability flake worth filing upstream as a Cloud API bug: scheduled run 32435192036 (2026-08-21) saw undeleteDataset return a dataset with 0 of 3 documents; the same test passed 08-22 through 08-24. That is a cloud-side issue, not a corpus one.

Activity

  1. stevevanhooser commented on Sep 3, 2026

    @stevevanhooser
    stevevanhooser
    on Sep 3, 2026 – with Claude
    ContributorAuthor

    Scope decision (2026-09-03)

    Discussed with @stevevanhooser. Doing:

    • Step 1: copy-before-open in the test fixture. The dabrowska_dataset fixture in tests/matlab_tests/test_dabrowska.py currently opens ~/Documents/ndi-projects/datasets/dabrowska in place with ndi.dataset.ndi_dataset(DABROWSKA_PATH). Change it to copy the corpus to a tmp_path_factory directory first (the same pattern test_jess_haley.py and test_carbon_fiber.py already use via load_dataset_from_json_dir(target_folder=…)). Also apply the same pattern to the tutorial scripts under tutorials/, so a user running a tutorial in place doesn't mutate their own downloaded dataset.
    • Step 5: restore the corpus — re-fetch dataset 67f723d574f5f79c6062389d (manual step; documented in the PR).

    Deferring:

    • Step 2 (mutation canary), Step 3 (un-mask per-loop type-count assert), Step 4 (preflight skip loudly). Once the fixture can't mutate the shared corpus, these are tripwires guarding a door that's now locked. Not adding them.
    • The larger design question (whether repairDatasetSessionInfo should run on construction at all). Real design item, worth its own decision, not urgent. Both MATLAB (+ndi/+dataset/dir.m constructor + build_session_info) and Python (_dataset.py _discover_correct_session + build_session_info) call the repair on construction — they're in parity, so any future change lands in both.
    • Prod cloud-side flake (scheduled run 32435192036, undeleteDataset returned 0/3 documents). Not corpus-related; belongs upstream.

    MATLAB parity note

    MATLAB has no equivalent test that opens a shared dabrowska corpus in place. tests/+ndi/+unittest/+dataset/OldDatasetTest.m already copies its bundled test corpus to tempname before opening (line 8–13) — the correct pattern. No MATLAB-side test change needed for this issue; the mutation-on-open behavior itself is in parity with Python and would be addressed by the deferred design item.

    PRs to follow on claude/issue-99-dataset-repair-zpw45q in both repos.

    Generated by Claude Code

  2. stevevanhooser
    closed this as completedin e132734on Sep 3, 2026

  3. stevevanhooser commented on Sep 3, 2026

    @stevevanhooser
    stevevanhooser
    on Sep 3, 2026 – with Claude
    ContributorAuthor

    Closing.

    Fix landed as PR #174 (copy-before-open in test_dabrowska.py), with the parity note as VH-Lab/NDI-matlab#924.

    Deferred items are intentionally not follow-ups on this issue:

    • Corpus restore — re-fetch 67f723d574f5f79c6062389d — is a one-off manual step, not a tracked work item.
    • Canary / un-mask / preflight are unnecessary once the fixture can't mutate the shared corpus.
    • The repairDatasetSessionInfo-on-construction design question is a real, separate design item that applies to both MATLAB and Python; file a new issue if/when we decide to change that behavior.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions