ADR-223: Implement ModifierStage[T_in, T_out] abstract base class - #216
Merged
jodavis merged 3 commits intoJun 10, 2026
Conversation
jodavis
approved these changes
Jun 10, 2026
jodavis
marked this pull request as ready for review
June 10, 2026 03:53
jodavis
force-pushed
the
dev/claude/ADR-223-implement-modifierstage-abstract-base
branch
from
June 10, 2026 04:25
575775b to
121044e
Compare
Adds ModifierStage[T_in, T_out] with the three-case transform() algorithm (skip / regen-with-stored-seed / new sample), _compute_content_hash() static helper, GC of orphaned output files, and manifest read/write integration. Also adds randomization.py (ADR-222 prerequisite) containing PassFilter, MinMaxFilter, NormalFilter, and VariationGenerator — required for modifier_stage.py to compile and for the regen path to construct generators from stored seeds. Key decisions: - ValueError from ManifestStore.read() propagates (hard error); no fallback to "no previous manifest" for unreadable/unsupported manifests. - output_dir creation is caller's responsibility; transform() does not call mkdir (consistent with DVC/entry-point owning directory setup per spec). - GC guards with output_dir.exists() so an empty first run does not raise. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bound T_in TypeVar to Sample so mypy can verify content_hash access at all four call sites in transform(); removes four type: ignore[arg-type] suppressions.
jodavis
force-pushed
the
dev/claude/ADR-223-implement-modifierstage-abstract-base
branch
from
June 10, 2026 13:18
121044e to
aaf03ff
Compare
jodavis
approved these changes
Jun 10, 2026
jodavis
enabled auto-merge (squash)
June 10, 2026 13:21
jodavis
deleted the
dev/claude/ADR-223-implement-modifierstage-abstract-base
branch
June 10, 2026 13:24
jodavis-claude
restored the
dev/claude/ADR-223-implement-modifierstage-abstract-base
branch
June 10, 2026 16:12
jodavis
pushed a commit
that referenced
this pull request
Jun 28, 2026
* Use dev-team plugin from feature branch * ADR-223: Implement ModifierStage abstract base class Adds ModifierStage[T_in, T_out] with the three-case transform() algorithm (skip / regen-with-stored-seed / new sample), _compute_content_hash() static helper, GC of orphaned output files, and manifest read/write integration. Also adds randomization.py (ADR-222 prerequisite) containing PassFilter, MinMaxFilter, NormalFilter, and VariationGenerator — required for modifier_stage.py to compile and for the regen path to construct generators from stored seeds. Key decisions: - ValueError from ManifestStore.read() propagates (hard error); no fallback to "no previous manifest" for unreadable/unsupported manifests. - output_dir creation is caller's responsibility; transform() does not call mkdir (consistent with DVC/entry-point owning directory setup per spec). - GC guards with output_dir.exists() so an empty first run does not raise. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ADR-223: address review feedback Bound T_in TypeVar to Sample so mypy can verify content_hash access at all four call sites in transform(); removes four type: ignore[arg-type] suppressions. --------- Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ModifierStage[T_in, T_out]— the abstract base class for all per-sample file-transformation stages — with thetransform()three-case algorithm (skip / regen-with-stored-seed / new sample), GC, seed persistence, and_compute_content_hashstatic helper.randomization.py(ADR-222 prerequisite):PassFilterABC,MinMaxFilter,NormalFilter,VariationGenerator.Files created
ml/pipeline/core/randomization.py—PassFilterABC,MinMaxFilter,NormalFilter,VariationGenerator(ADR-222)ml/pipeline/core/modifier_stage.py—ModifierStage[T_in, T_out]withtransform(),_compute_content_hash(), and three abstract methodsml/test/pipeline/core/test_modifier_stage.py— 30 unit testsKey decisions
ValueErrorfromManifestStore.read()propagates — hard error for unreadable/unsupported manifests (conservative default).output_dircreation is caller's responsibility. GC guards withoutput_dir.exists()to avoid raising on first run._FakeStagein tests usesav_by_content_hashmap keyed oninput_sample.content_hash— makes skip/regen/split-behavior tests fully deterministic.Test coverage
TestComputeContentHash(5 tests): hash stability, seed/parent/applied-values differentiationTestSkipPath(4 tests): file preservation, no_generate_outputcall, manifest writtenTestRegenPath(5 tests): new id via_derive_id, stored seed preserved, content_hash updated, old file GC'dTestNewSamplePath(6 tests):_derive_idcalled, non-zero seed, no previous manifest → all newTestDeterministicStage(3 tests):_is_deterministic=True→ seed 0 for all new samplesTestGarbageCollection(6 tests): orphan removal, manifest.json preserved, current output preservedTestSplitBehavior(1 test): skip and regen in sametransform()callJira: ADR-223