Finish the canvas/headless parity punch list - #49
Merged
Conversation
Close out the three remaining small divergences from the architecture review (following #47 and #48): Select: - SelectionState.apply_select_params now accepts patch=None and builds the display-only state itself; the widget's hand-built copy of that branch (basis derivation + field-by-field state assembly) is gone. Filter: - The mode whitelists moved to nodes/filter.py as FilterMode / SavgolFilterMode Literals; the params models validate them, so a headless SavgolFilterParams(mode='reflect') fails at validation with a clear message instead of inside SciPy. The widget's combo options are derived from the same Literals via get_args, so UI and validation cannot drift. - The legacy Gaussian-settings migration stays widget-side on purpose: it handles the pre-row-list flat-settings shape, which cannot reach the typed params layer (pre-_state workflows load as defaults). PlayAudio: - All Qt-free DSP moved to nodes/playaudio.py: patch validation, rate inference, PCM normalization (prepare_pcm_audio), default time scale, volume-to-dB mapping, output-rate clamping, and playback resampling. Only the QAudioSink/QAudioFormat half remains in the widget. - The node's time_scale/volume_percent params are now consumed: the new render_audition(patch, params) returns device-ready PCM headlessly. New tests: tests/test_nodes/test_playaudio.py (params-driven audition), mode-validation tests in test_filter_params.py; the DSP unit tests now target the node functions.
d-chambers
force-pushed
the
fix/parity-punchlist
branch
from
August 4, 2026 14:00
027a70a to
d5022d5
Compare
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
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.
Description
Closes out the canvas/headless parity theme from the architecture review (following #47 and #48) — the three remaining small divergences:
Select —
SelectionState.apply_select_paramsnow acceptspatch=Noneand builds the display-only state itself, replacing the widget's hand-built copy of that branch (duplicate basis derivation and field-by-field state assembly). Note: the review's original "payload writer lives only in the widget" finding had already been resolved in the meantime —patch_settings_payloadlives on the model — so this closes the last remaining duplication.Filter — the boundary-mode whitelists moved to
nodes/filter.pyasFilterMode/SavgolFilterModeLiterals, and the params models now validate them: a headlessSavgolFilterParams(mode="reflect")(which SciPy rejects) fails at validation with a clear message instead of surviving until the SciPy call. The widget's combo options are derived from the same Literals viaget_args, so UI and validation cannot drift. Deliberately not moved: the legacy Gaussian-settings migration — it handles the pre-row-list flat-settings shape, which cannot reach the typed params layer at all (pre-_stateworkflows load as defaults per the documented storage break), so there is no headless path to diverge.PlayAudio — the node was hollow: it declared
time_scale/volume_percentparams that nothing consumed while all the Qt-free DSP sat in the widget. Everything between a patch and device-ready PCM (shape validation, rate inference, robust-gain PCM normalization, default time scale, volume→dB mapping, output-rate clamping, block-wise resampling) now lives innodes/playaudio.py; only theQAudioSink/QAudioFormathalf remains in the widget. The params are now actually consumed: the newrender_audition(patch, params)returns(prepared, output_rate_hz, pcm)headlessly.CHANGELOG updated under Unreleased → Fixed.
Counterpart review: the Codex CLI is still rate-limited (verified again before this PR; resets Aug 8), so no counterpart review was run; the adversarial self-review record is in the untracked
.scratch/review_parity_punchlist.md(covers theapply_select_paramsbehavior-preservation check, why legacy blobs don't gain a new failure mode from the Literals, the deliberate non-move of the Gaussian migration, and a repo-wide stale-reference sweep after the PlayAudio move).Checklist
I have (if applicable):
tests/test_nodes/test_playaudio.py; mode-validation tests intest_filter_params.py; the PlayAudio DSP unit tests now target the node functions).prek run --all-filesandpytest tests(2114 passed, 44 skipped; all 10 hooks pass).python scripts/bench_compare.py --baseline main. (not run — no hot path is touched: the moves are logic-identical relocations, the Literal validation replaces same-coststrvalidation, and PlayAudio DSP is not benchmark-covered)