feat: SDD workflow — test sync, song generation + validation, ReaScript hybrid pipeline
- 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
This commit is contained in:
262
.sdd/changes/archive/2026-05-03-generate-song/change/design.md
Normal file
262
.sdd/changes/archive/2026-05-03-generate-song/change/design.md
Normal file
@@ -0,0 +1,262 @@
|
||||
# Design: generate-song CLI
|
||||
|
||||
## Technical Approach
|
||||
|
||||
A thin CLI wrapper (`scripts/generate.py`) that delegates to the existing `compose.py` `main()`, plus a text-based `.rpp` validator (`src/validator/rpp_validator.py`). No new architecture layers — reuse compose pipeline, add thin orchestration on top.
|
||||
|
||||
## Architecture Decisions
|
||||
|
||||
### Decision: Thin CLI wrapper (not a fork)
|
||||
|
||||
**Choice**: `generate.py` calls `compose.main()` directly as a function import, not as a subprocess.
|
||||
**Alternatives considered**: Subprocess spawn (`subprocess.run([sys.executable, "scripts/compose.py", ...])`), or duplicating compose logic in generate.py.
|
||||
**Rationale**: Subprocess adds process overhead and requires serializing args. Duplication violates DRY. Direct function call is clean and allows `--validate` to call `validate_rpp_output()` on the in-memory result before writing to disk.
|
||||
|
||||
### Decision: Text-based RPP validation (not full parser)
|
||||
|
||||
**Choice**: Validate `.rpp` output using regex/string search on the raw file text.
|
||||
**Alternatives considered**: Full RPP parser (complex, out of scope), or relying on `SongDefinition.validate()` only.
|
||||
**Rationale**: RPP files are text-format. For structural validation (track count, audio paths, MIDI notes, send routing, plugin chains), regex over the raw text is sufficient and avoids building a complete parser. The validator is not a schema validator — it checks output invariants only.
|
||||
|
||||
### Decision: `validate_rpp_output()` as reusable module (not inline in generate.py)
|
||||
|
||||
**Choice**: `src/validator/rpp_validator.py` with a single exported function `validate_rpp_output(rpp_path: str) -> list[str]`.
|
||||
**Alternatives considered**: Inline validation in `generate.py`, or a class-based validator.
|
||||
**Rationale**: Reusability — other tools (tests, scripts, future CLI variants) can call the validator without running generation. Single function is simplest API.
|
||||
|
||||
### Decision: Perc loop fallback in compose.py (no change to DRUMLOOP_FILES)
|
||||
|
||||
**Choice**: `build_perc_track()` in `compose.py` already falls back when `91bpm bellako percloop.wav` is absent — it silently skips missing files (see compose.py line 282: `if perc_path.exists()`).
|
||||
**Alternatives considered**: Add a fallback dict lookup in generate.py, or modify DRUMLOOP_FILES.
|
||||
**Rationale**: The spec requires "skip silently rather than crash" — this behavior already exists in `build_perc_track()`. No changes needed to compose.py.
|
||||
|
||||
## Data Flow
|
||||
|
||||
```
|
||||
generate.py main()
|
||||
├── argparse (--bpm, --key, --output, --seed, --validate)
|
||||
├── import compose.main
|
||||
│ └── compose.main(args) → writes output/rpp
|
||||
└── if --validate:
|
||||
└── validate_rpp_output(output_path) → list[str]
|
||||
├── count <TRACK> blocks
|
||||
├── regex <SOURCE WAVE> for audio paths → verify exist
|
||||
├── regex <NOTE for MIDI clip notes
|
||||
├── compute arrangement end beat
|
||||
├── regex AUXRECV for send routing
|
||||
└── count VST/FXCHAIN for plugin chains
|
||||
```
|
||||
|
||||
## File Changes
|
||||
|
||||
| File | Action | Description |
|
||||
|------|--------|-------------|
|
||||
| `scripts/generate.py` | Create | CLI entry point, imports and calls `compose.main()` |
|
||||
| `src/validator/rpp_validator.py` | Create | `validate_rpp_output()` — text-based RPP structural validator |
|
||||
| `tests/test_generate_song.py` | Create | E2E smoke test + validation + reproducibility |
|
||||
|
||||
## Interfaces / Contracts
|
||||
|
||||
### `scripts/generate.py`
|
||||
|
||||
```python
|
||||
# scripts/generate.py
|
||||
import argparse, sys
|
||||
from pathlib import Path
|
||||
|
||||
_ROOT = Path(__file__).parent.parent
|
||||
sys.path.insert(0, str(_ROOT))
|
||||
import compose
|
||||
|
||||
def main():
|
||||
parser = argparse.ArgumentParser(description="Generate a REAPER .rpp song.")
|
||||
parser.add_argument("--bpm", type=float, default=95)
|
||||
parser.add_argument("--key", default="Am")
|
||||
parser.add_argument("--output", default="output/song.rpp")
|
||||
parser.add_argument("--seed", type=int, default=42)
|
||||
parser.add_argument("--validate", action="store_true")
|
||||
args = parser.parse_args()
|
||||
|
||||
if args.bpm <= 0:
|
||||
raise ValueError("bpm must be > 0")
|
||||
|
||||
output_path = Path(args.output)
|
||||
output_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Delegate to compose.main
|
||||
sys.argv = [
|
||||
sys.argv[0],
|
||||
"--bpm", str(args.bpm),
|
||||
"--key", args.key,
|
||||
"--output", str(output_path),
|
||||
"--seed", str(args.seed),
|
||||
]
|
||||
compose.main()
|
||||
|
||||
if args.validate:
|
||||
from src.validator.rpp_validator import validate_rpp_output
|
||||
errors = validate_rpp_output(str(output_path))
|
||||
if errors:
|
||||
print("Validation errors:", file=sys.stderr)
|
||||
for e in errors:
|
||||
print(f" - {e}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
```
|
||||
|
||||
### `src/validator/rpp_validator.py`
|
||||
|
||||
```python
|
||||
# src/validator/rpp_validator.py
|
||||
"""Text-based .rpp structural validator.
|
||||
|
||||
Validates: track count, audio clip paths, MIDI note presence,
|
||||
arrangement duration, send routing, and plugin chain presence.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
def validate_rpp_output(rpp_path: str) -> list[str]:
|
||||
"""Validate a generated .rpp file. Returns list of error strings."""
|
||||
errors = []
|
||||
content = Path(rpp_path).read_text(encoding="utf-8")
|
||||
lines = content.split("\n")
|
||||
|
||||
# 1. Track count — count <TRACK> opening blocks
|
||||
track_count = sum(1 for line in lines if line.strip() == "<TRACK")
|
||||
if track_count < 9:
|
||||
errors.append(f"Expected 9 tracks, got {track_count}")
|
||||
elif track_count > 9:
|
||||
errors.append(f"Expected 9 tracks, got {track_count} (possible duplicate)")
|
||||
|
||||
# 2. Audio clip paths — verify <SOURCE WAVE "..."> paths exist
|
||||
import re
|
||||
wave_pattern = re.compile(r'<SOURCE WAVE "([^"]+)"')
|
||||
for match in wave_pattern.finditer(content):
|
||||
path = match.group(1)
|
||||
if not Path(path).exists():
|
||||
errors.append(f"Audio clip path does not exist: {path}")
|
||||
|
||||
# 3. MIDI clips have notes — look for NOTE events in MIDI items
|
||||
midi_items = re.findall(r'<ITEM[^>]*>[^]*?</ITEM>', content, re.DOTALL)
|
||||
for item in midi_items:
|
||||
if "<SOURCE MIDI" in item:
|
||||
# Count NOTE entries in this MIDI item
|
||||
note_count = len(re.findall(r'NOTE \d+ \d+ \d+', item))
|
||||
if note_count == 0:
|
||||
errors.append("MIDI clip has no notes")
|
||||
|
||||
# 4. Arrangement duration — find last item position + length
|
||||
item_positions = re.findall(r'<ITEM[^>]*>\s*POSITION (\d+\.?\d*)', content)
|
||||
if item_positions:
|
||||
last_pos = max(float(p) for p in item_positions)
|
||||
# Also get lengths to find actual end
|
||||
item_lengths = re.findall(r'<ITEM[^>]*>[^]*?LENGTH (\d+\.?\d*)', content, re.DOTALL)
|
||||
if item_lengths:
|
||||
ends = [float(p) + float(l) for p, l in zip(item_positions, item_lengths)]
|
||||
max_end = max(ends)
|
||||
expected_beats = 52 * 4 # 52 bars at 4 beats/bar
|
||||
if max_end < expected_beats:
|
||||
errors.append(f"Arrangement ends at beat {max_end}, expected at least {expected_beats}")
|
||||
|
||||
# 5. Send routing — each non-return track should have AUXRECV
|
||||
# Identify return tracks (have RECVFX but no MAINSEND)
|
||||
track_blocks = re.findall(r'<TRACK>(?:[^<]|<(?!TRACK>))*?(?=<TRACK>|$)', content, re.DOTALL)
|
||||
for track in track_blocks:
|
||||
track_name_match = re.search(r'<NAME "([^"]+)"', track)
|
||||
if not track_name_match:
|
||||
continue
|
||||
name = track_name_match.group(1)
|
||||
# Skip return tracks
|
||||
if name in ("Reverb", "Delay"):
|
||||
continue
|
||||
# Non-return tracks need AUXRECV
|
||||
if "AUXRECV" not in track:
|
||||
errors.append(f"Track '{name}' missing send to return track")
|
||||
|
||||
# 6. Plugin chains — each track should have FXCHAIN with VST entries
|
||||
for track in track_blocks:
|
||||
name_match = re.search(r'<NAME "([^"]+)"', track)
|
||||
if not name_match:
|
||||
continue
|
||||
name = name_match.group(1)
|
||||
if name in ("Reverb", "Delay"):
|
||||
continue # Return tracks have different structure
|
||||
if "<FXCHAIN" not in track:
|
||||
errors.append(f"Track '{name}' missing FXCHAIN")
|
||||
elif not re.search(r'VST\d?', track):
|
||||
errors.append(f"Track '{name}' missing VST plugin in FXCHAIN")
|
||||
|
||||
return errors
|
||||
```
|
||||
|
||||
### `tests/test_generate_song.py`
|
||||
|
||||
```python
|
||||
# tests/test_generate_song.py
|
||||
import subprocess, sys, tempfile
|
||||
from pathlib import Path
|
||||
import pytest
|
||||
|
||||
def test_generate_cli_smoke(tmp_path):
|
||||
output = tmp_path / "song.rpp"
|
||||
result = subprocess.run(
|
||||
[sys.executable, "scripts/generate.py",
|
||||
"--bpm", "95", "--key", "Am",
|
||||
"--output", str(output), "--seed", "42"],
|
||||
capture_output=True, text=True
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert output.exists()
|
||||
assert output.stat().st_size > 0
|
||||
|
||||
def test_validate_passes_for_valid_output(tmp_path):
|
||||
output = tmp_path / "song.rpp"
|
||||
subprocess.run(
|
||||
[sys.executable, "scripts/generate.py",
|
||||
"--bpm", "95", "--key", "Am",
|
||||
"--output", str(output), "--seed", "42", "--validate"],
|
||||
check=True
|
||||
)
|
||||
from src.validator.rpp_validator import validate_rpp_output
|
||||
errors = validate_rpp_output(str(output))
|
||||
assert errors == [], f"Unexpected errors: {errors}"
|
||||
|
||||
def test_validate_detects_track_count_violation(tmp_path, monkeypatch):
|
||||
# Write a malformed rpp with only 5 tracks
|
||||
rpp_path = tmp_path / "bad.rpp"
|
||||
rpp_path.write_text("<TRACK>\n" * 5, encoding="utf-8")
|
||||
from src.validator.rpp_validator import validate_rpp_output
|
||||
errors = validate_rpp_output(str(rpp_path))
|
||||
assert any("Expected 9 tracks" in e for e in errors)
|
||||
|
||||
def test_reproducibility_same_seed(tmp_path):
|
||||
output_a = tmp_path / "song_a.rpp"
|
||||
output_b = tmp_path / "song_b.rpp"
|
||||
for out in (output_a, output_b):
|
||||
subprocess.run(
|
||||
[sys.executable, "scripts/generate.py",
|
||||
"--bpm", "95", "--key", "Am",
|
||||
"--output", str(out), "--seed", "42"],
|
||||
check=True
|
||||
)
|
||||
assert output_a.read_bytes() == output_b.read_bytes()
|
||||
```
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
| Layer | What to Test | Approach |
|
||||
|-------|-------------|----------|
|
||||
| Unit | `validate_rpp_output()` correctness | Synthesize malformed .rpp strings, check error detection |
|
||||
| Integration | CLI + compose end-to-end | `subprocess.run` with `--seed 42`, assert file written |
|
||||
| E2E | Reproducibility + validation | Two runs with same seed → byte-identical output |
|
||||
|
||||
## Migration / Rollout
|
||||
|
||||
No migration required — this is a net-new feature (new files only). No breaking changes to existing compose.py or reaper_builder.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- None — all decisions resolved in spec or above.
|
||||
Reference in New Issue
Block a user