ADR-339: ML pipeline core data model (Sample hierarchy, Manifest, ManifestStore)

[jodavis-claude]

Work item

ADR-339 — build the unified Sample/Manifest[S]/ManifestStore data model in ml/pipeline/core/: the typed sample hierarchy, generic manifest collection, and JSON (schema v1) round-trip store that every later ML pipeline stage (ADR-340 onward) will serialise through.

Changes

• ml/pipeline/core/__init__.py — new, empty package marker for the core/ subpackage.
• ml/pipeline/core/sample.py — new. Sample base dataclass (name, content_hash, applied_values, parent_name, parent_content_hash, all defaulted); SampleWithPath(Sample) adds path; TextSample(Sample) adds content/label, with __post_init__ setting name = content_hash; AudioSample(SampleWithPath) adds transcript/label/seed; SampleSpectrogram/SampleTokens (both SampleWithPath, path-based, no seed).
• ml/pipeline/core/manifest.py — new. Manifest[S] (classic Generic[S]/TypeVar style; by_name/by_content_hash dict-backed lookups; add() raises ValueError on duplicate names; __len__/__iter__ for enumeration). ManifestStore (write/read; JSON schema v1 — schema_version, sample_type, samples; write raises ValueError on empty manifest or mixed sample types; read type-dispatches off the persisted sample_type field via a _SAMPLE_TYPES_BY_NAME registry).
• ml/test/pipeline/__init__.py, ml/test/pipeline/core/__init__.py — new, empty test package markers mirroring the source layout.
• ml/test/pipeline/core/test_sample.py, test_manifest.py, test_manifest_store.py — new, test-first unit tests covering all of the above.

Design decisions

• Manifest.__len__/__iter__ were added beyond the Component Breakdown's literal text — needed by ManifestStore.write to detect an empty manifest and inspect sample types, judged as within Manifest[S]'s own collection responsibility.
• Fixed a gap found during self-review: the TDD trio's first ManifestStore.read implementation wrote a sample_type field but still required the caller to pass the type explicitly rather than dispatching off it. Replaced with a _SAMPLE_TYPES_BY_NAME registry so deserialization dispatches purely from the persisted JSON (separate commit).
• SampleSpectrogram/SampleTokens are path-based (extend SampleWithPath, no extra fields), consistent with AudioSample and the ModifierStage output-directory pattern — resolves a known ambiguity per the task brief's own recommendation.
• All six dataclasses are plain (non-frozen), every field defaulted, per the task brief's recommendation (no spec requirement for immutability).
• Open ambiguity, not resolved in this PR: no Python test runner (pytest) is wired into scripts/validate.sh yet, so this PR's unit tests cannot be verified by the project's standard build/test quality gate script. Recommend a small follow-up task adding pytest to ml/requirements.txt and a scripts/validate-ml-tests.sh/.cmd.
• No E2E/Gherkin scenarios — this repo's only E2E harness targets the C# WPF/Blazor UI; not applicable to this Python ML pipeline code.

Testing completed

• 15 unit tests, all passing (python3 -m pytest test/ -v from ml/).
• mypy --strict pipeline test passes with zero issues.

[jodavis-claude]

Reviewed against ADR-339's exit criteria (Sample hierarchy, Manifest[S], ManifestStore) and ml/_spec_OopPipeline.md's Key Design Decisions / Component Breakdown. All four exit-criteria bullets are met, mypy --strict pipeline test and python3 -m pytest test/ -v both pass locally (15/15). Tests follow AAA structure with clear Arrange/Act/Assert comments and one behavior per test.

Two correctness/fault-tolerance issues in manifest.py worth addressing before merge (see inline comments): a silent duplicate-content_hash overwrite in Manifest.add, and a raw KeyError (instead of a descriptive ValueError, consistent with the rest of ManifestStore) when read() encounters an unrecognized sample_type.

Note (non-blocking, already flagged by the developer in the PR description): pytest still isn't wired into scripts/validate.sh, so this PR's tests can't be verified by the project's standard quality-gate script yet. Worth a quick follow-up task, but out of scope for ADR-339's own exit criteria.

[jodavis-claude]

Sign-off review

Both prior review threads verified as correctly addressed:

1. Manifest.add() silent content_hash overwrite — fixed with a ValueError check on duplicate content_hash (mirroring the existing duplicate-name check), placed before either backing dict is mutated. Regression test test_add_duplicate_content_hash_different_names_raises_value_error uses AudioSample to correctly isolate the content_hash collision from the name-uniqueness path. Verified.
2. ManifestStore.read() bare KeyError on unknown sample_type — fixed with a try/except KeyError that re-raises a descriptive ValueError, consistent with the class's other failure modes. Regression test test_read_unrecognized_sample_type_raises_value_error covers it. Verified.

Ran the full suite locally: 17/17 pytest tests pass, mypy --strict pipeline test clean.

No new issues found in the files touched by this fix pass (ml/pipeline/core/manifest.py, ml/test/pipeline/core/test_manifest.py, ml/test/pipeline/core/test_manifest_store.py).

One pre-existing note carried forward for awareness, not blocking this PR: Known Ambiguity #1 (no pytest wiring into scripts/validate.sh/ml/requirements.txt) remains unresolved, as flagged in the original task brief and implementation summary — Python unit tests still cannot be verified through the project's standard quality-gate script. Worth a small follow-up task before the ML test suite grows further.

Approving for hand-off.