48bc271afc
- compose-test-sync: fix 3 failing tests (NOTE_TO_MIDI, DrumLoopAnalyzer mock, section name) - generate-song: CLI wrapper + RPP validator (6 structural checks) + 4 e2e tests - reascript-hybrid: ReaScriptGenerator + command protocol + CLI + 16 unit tests - 110/110 tests passing - Full SDD cycle (propose→spec→design→tasks→apply→verify) for all 3 changes
91 lines
3.7 KiB
Markdown
91 lines
3.7 KiB
Markdown
# Design: compose-test-sync
|
|
|
|
## Technical Approach
|
|
|
|
Fix 3 failing tests by replacing calls to removed/changed compose.py APIs with their current equivalents. Each fix is surgical — only the broken import or call signature changes, no test logic rewrites.
|
|
|
|
## File Changes
|
|
|
|
| File | Action | Description |
|
|
|------|--------|-------------|
|
|
| `tests/test_section_builder.py` | Modify | Replace `root_to_midi` with `NOTE_TO_MIDI` dict lookup |
|
|
| `tests/test_render_cli.py` | Modify | Remove obsolete `DrumLoopAnalyzer` patch, retain `SampleSelector` if still needed |
|
|
| `tests/test_compose_integration.py` | Modify | Remove `analysis` arg, use section name `"chorus"` instead of `"verse"` |
|
|
|
|
## Fix Details
|
|
|
|
### Fix 1: `test_root_to_midi` (test_section_builder.py:121)
|
|
|
|
**Problem**: Test imports `root_to_midi` which no longer exists in compose.py.
|
|
**Fix**: Replace the import and the two assertions.
|
|
|
|
```python
|
|
# Before
|
|
from scripts.compose import root_to_midi
|
|
assert root_to_midi("A", 4) == 69
|
|
assert root_to_midi("C", 4) == 60
|
|
|
|
# After
|
|
from scripts.compose import NOTE_TO_MIDI
|
|
assert NOTE_TO_MIDI["A"] + (4 + 1) * 12 == 69
|
|
assert NOTE_TO_MIDI["C"] + (4 + 1) * 12 == 60
|
|
```
|
|
|
|
`NOTE_TO_MIDI` is at compose.py:37 and maps `{"A": 69, "C": 60, ...}`. The formula `NOTE_TO_MIDI[root] + (octave + 1) * 12` reconstructs the old `root_to_midi` behavior.
|
|
|
|
---
|
|
|
|
### Fix 2: `test_melody_uses_pentatonic` (test_compose_integration.py:200)
|
|
|
|
**Problem 1**: `build_melody_track` no longer accepts `analysis` as 5th arg.
|
|
**Problem 2**: `build_lead_track` (called inside `build_melody_track`) skips any section not named `"chorus"`, `"chorus2"`, or `"final"`. Test passes `"verse"` → zero clips generated.
|
|
|
|
**Fix**: Remove `analysis` arg from call, change section name to `"chorus"`.
|
|
|
|
```python
|
|
# Before
|
|
sections = [SectionDef(name="verse", bars=4, energy=1.0)]
|
|
offsets = [0.0]
|
|
track = build_melody_track(sections, offsets, "A", True, seed=42)
|
|
|
|
# After
|
|
sections = [SectionDef(name="chorus", bars=4, energy=1.0)]
|
|
offsets = [0.0]
|
|
track = build_melody_track(sections, offsets, "A", True, seed=42)
|
|
```
|
|
|
|
Note: `analysis = _fake_analysis()` at line 204 becomes unused — leave it in place (no harm) or remove if preferred.
|
|
|
|
---
|
|
|
|
### Fix 3: `test_main_without_render_produces_rpp` (test_render_cli.py:44)
|
|
|
|
**Problem**: Test patches `scripts.compose.DrumLoopAnalyzer` which no longer exists in compose.py.
|
|
**Fix**: Remove `patch("scripts.compose.DrumLoopAnalyzer")` from both the mock setup and the with statement.
|
|
|
|
```python
|
|
# Before
|
|
with patch("scripts.compose.SampleSelector") as mock_cls, \
|
|
patch("scripts.compose.DrumLoopAnalyzer") as mock_a_cls:
|
|
|
|
# After
|
|
with patch("scripts.compose.SampleSelector") as mock_cls:
|
|
```
|
|
|
|
Lines 48-55 (fake `DrumLoopAnalysis` object) and lines 68-70 (mock analyzer setup) should be removed since `fake_analysis` and `mock_a` are no longer used.
|
|
|
|
Verify `SampleSelector` patch is still functional — if `SampleSelector` was also removed, remove that patch too and simplify the mock block.
|
|
|
|
## Verification Strategy
|
|
|
|
1. Run `pytest tests/ -q` — expect 90/90 pass
|
|
2. Run each fixed test individually to confirm:
|
|
- `pytest tests/test_section_builder.py::TestHelpers::test_root_to_midi -v`
|
|
- `pytest tests/test_compose_integration.py::TestComposeIntegration::test_melody_uses_pentatonic -v`
|
|
- `pytest tests/test_render_cli.py::TestComposeNoRender::test_main_without_render_produces_rpp -v`
|
|
3. Confirm no compose.py files modified: `git diff --name-only scripts/`
|
|
|
|
## Open Questions
|
|
|
|
- **SampleSelector still used?**: Verify `SampleSelector` is still imported/patched in compose.py before leaving that patch in place. If also removed, strip it.
|
|
- **None**: All three fixes are clear from the proposal and code inspection. |