Skip to content

Commit 91ffae8

Browse files
adewaleclaude
andcommitted
feat(phase-23): Extend MAX_STEPS to 128, fix mid-playback preloading, audit docs
## Extended Step Sequencer (64 → 128 steps) - MAX_STEPS now 128 (8 bars at 16th note resolution) - STEP_COUNT_OPTIONS: [4, 8, 12, 16, 24, 32, 64, 96, 128] - Added 128-step test coverage for polyrhythms and edge cases ## Bug Fix #8: Mid-playback sampled instrument preloading - SamplePicker now preloads sampled instruments on selection - Fixes piano not playing when added during playback - Added 5 tests for preloading behavior - Documented in DEBUGGING-LESSONS-LEARNED.md ## Demo Sessions (showcasing extended patterns) - progressive-house-build.json (128-step evolving build) - polyrhythmic-evolution.json (odd-length: 5, 7, 11, 13, 17, 19, 23 steps) - edm-drop-section.json (8-bar pre-drop + drop) - extended-afrobeat.json (96-step triplet groove) ## Documentation Audit (internal consistency) - Updated all references from 64 to 128 steps - Fixed CHANGELOG, ROADMAP, SPEC, STATUS, TUNING-CONSTANTS - Updated validation in session-api.ts and worker/validation.ts - Aligned types.ts comments with actual MAX_STEPS value 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 14cc36c commit 91ffae8

20 files changed

+1538
-99
lines changed

CHANGELOG.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
9696
### Added
9797

9898
#### Audio Engine
99-
- Step sequencer with 16 tracks × 64 steps
99+
- Step sequencer with 16 tracks × 128 steps
100100
- 16 synthesized samples (drums, bass, synth, FX)
101101
- 5 real-time synth presets (bass, lead, pad, pluck, acid)
102102
- Lookahead scheduling (25ms timer, 100ms ahead)
@@ -146,10 +146,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
146146
- SPA routing support
147147

148148
### Technical Details
149-
- `MAX_STEPS = 64` (4 bars of 16th notes)
149+
- `MAX_STEPS = 128` (8 bars of 16th notes)
150150
- `STEPS_PER_PAGE = 16` (1 bar)
151151
- `MAX_TRACKS = 16`
152-
- Global step counter (0-63) with per-track modulo for polyrhythms
152+
- Global step counter (0-127) with per-track modulo for polyrhythms
153153

154154
---
155155

app/docs/DEBUGGING-LESSONS-LEARNED.md

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ This document captures debugging knowledge from fixing bugs in the Keyboardia co
1818
| No sound from instrument | Silent Instrument | [#003](#003-silent-instrument) |
1919
| "not ready" warnings | Play Before Ready | [#004](#004-play-before-ready) |
2020
| Solo'd tracks not playing | Solo State Bug | [#005](#005-solo-state-not-applied) |
21+
| Works for synth: but not sampled: | Namespace Inconsistency | [#006](#006-namespace-prefix-inconsistency) |
22+
| Bug reappears after fix | Fix Without Tests | [#007](#007-fix-without-tests-anti-pattern) |
23+
| Piano silent when added mid-playback | Mid-Playback Preload | [#008](#008-mid-playback-sampled-instrument-not-preloaded) |
2124

2225
---
2326

@@ -487,6 +490,225 @@ private scheduler(state: GridState): void {
487490

488491
---
489492

493+
## #006: Namespace Prefix Inconsistency
494+
495+
**Date**: 2024-12-17
496+
**Severity**: high
497+
**Category**: consistency
498+
499+
### Symptoms
500+
- Feature works for some track configurations but not others
501+
- "synth:piano works but sampled:piano doesn't"
502+
- Preloading misses certain instrument formats
503+
- Intermittent failures depending on how track was created
504+
505+
### Root Cause
506+
Multiple places in the codebase handle instrument type prefixes (`synth:`, `sampled:`, `tone:`, `advanced:`). When logic is duplicated:
507+
1. One place gets updated, another doesn't
508+
2. New namespaces are added but not to all handlers
509+
3. Edge cases are handled inconsistently
510+
511+
Example from Phase 23:
512+
- `scheduler.ts` handled BOTH `synth:piano` and `sampled:piano`
513+
- `preloadInstrumentsForTracks` only handled `synth:piano`
514+
- Result: Piano wouldn't preload for `sampled:piano` tracks
515+
516+
### Detection Strategy
517+
518+
**Code search:**
519+
```bash
520+
# Count prefix handling occurrences - should be centralized
521+
grep -rn "sampleId.startsWith" src/ | grep -v test | wc -l
522+
# If > 5, you have duplication debt
523+
```
524+
525+
**Code patterns (risky):**
526+
```typescript
527+
// Duplicated prefix handling (BAD)
528+
if (sampleId.startsWith('synth:')) { ... }
529+
// Same check in another file...
530+
if (sampleId.startsWith('synth:')) { ... }
531+
532+
// Should use centralized utility (GOOD)
533+
import { parseInstrumentId } from './instrument-types';
534+
const info = parseInstrumentId(sampleId);
535+
```
536+
537+
### Fix
538+
539+
Use centralized utilities in `src/audio/instrument-types.ts`:
540+
541+
```typescript
542+
// Instead of raw startsWith checks
543+
if (track.sampleId.startsWith('synth:')) {
544+
const preset = track.sampleId.replace('synth:', '');
545+
if (isSampledInstrument(preset)) { ... }
546+
}
547+
548+
// Use centralized utility
549+
import { collectSampledInstruments } from './instrument-types';
550+
const instrumentsToLoad = collectSampledInstruments(tracks);
551+
```
552+
553+
### Prevention
554+
1. **Always use instrument-types.ts** for namespace handling
555+
2. **Never write raw startsWith()** for instrument prefixes
556+
3. **Add tests covering ALL namespace formats** when adding features
557+
4. **Search codebase when adding new namespace** to find all handlers
558+
559+
### Related Files
560+
- `src/audio/instrument-types.ts` (centralized utilities)
561+
- `src/audio/instrument-types.test.ts` (comprehensive tests)
562+
- `src/audio/engine.ts` (preloadInstrumentsForTracks)
563+
- `src/audio/scheduler.ts` (playback routing)
564+
565+
### Post-Fix Analysis
566+
```bash
567+
# Find all raw startsWith calls for instrument prefixes
568+
grep -rn "startsWith.*synth:\|startsWith.*sampled:\|startsWith.*tone:" src/ | grep -v test | grep -v instrument-types
569+
# Result should be minimal - most should use centralized utilities
570+
```
571+
572+
---
573+
574+
## #007: Fix Without Tests Anti-Pattern
575+
576+
**Date**: 2024-12-17
577+
**Severity**: medium
578+
**Category**: process
579+
580+
### Symptoms
581+
- Bug is fixed but similar bugs reappear later
582+
- Regression in functionality that was "already fixed"
583+
- No confidence that fix actually addresses root cause
584+
585+
### Root Cause
586+
Fixing a bug without first writing a test that:
587+
1. Reproduces the bug (fails before fix)
588+
2. Passes after the fix is applied
589+
3. Prevents regression
590+
591+
### Detection Strategy
592+
593+
**Process check:**
594+
- Did you write a test that fails BEFORE the fix?
595+
- Does the test pass AFTER the fix?
596+
- Would the test catch if the bug was reintroduced?
597+
598+
### Fix
599+
600+
**Test-first workflow for bug fixes:**
601+
602+
```typescript
603+
// 1. Write test that exposes the bug (should FAIL)
604+
it('preloads piano from sampled:piano track', () => {
605+
const tracks = [{ sampleId: 'sampled:piano' }];
606+
const result = collectSampledInstruments(tracks);
607+
expect(result.has('piano')).toBe(true); // FAILS with buggy code
608+
});
609+
610+
// 2. Run test, confirm it fails
611+
// 3. Apply the fix
612+
// 4. Run test, confirm it passes
613+
// 5. Commit test AND fix together
614+
```
615+
616+
### Prevention
617+
1. **Write failing test first** before any code changes
618+
2. **Include edge cases** in the test (all namespace formats, boundary conditions)
619+
3. **Commit test with fix** so they're always paired
620+
4. **Code review check**: "Where's the test for this fix?"
621+
622+
### Related Files
623+
- All test files
624+
- CI/CD pipeline configuration
625+
626+
---
627+
628+
## #008: Mid-Playback Sampled Instrument Not Preloaded
629+
630+
**Date**: 2024-12-17
631+
**Severity**: high
632+
**Category**: race-condition
633+
634+
### Symptoms
635+
- "Sampled instrument piano not ready, skipping at step X" warning repeated many times
636+
- Piano track added during playback produces no sound
637+
- Other instruments (synths, drums) work fine
638+
- Issue resolves after stopping and starting playback again
639+
640+
### Root Cause
641+
When a sampled instrument track (e.g., `synth:piano`, `sampled:piano`) is added **during playback**, the instrument samples are never preloaded because:
642+
643+
1. `preloadInstrumentsForTracks()` is only called when play starts (in `handlePlayToggle`)
644+
2. Scheduler runs `collectSampledInstruments()` at play start - doesn't include tracks added later
645+
3. `signalMusicIntent('add_track')` initializes audio engine but doesn't preload the specific instrument
646+
647+
### Detection Strategy
648+
649+
**Log patterns:**
650+
```
651+
[Audio] No sampled instruments to preload // At play start (no piano yet)
652+
[Audio] Sampled instrument piano not ready, skipping at step 1
653+
[Audio] Sampled instrument piano not ready, skipping at step 4
654+
[Audio] Sampled instrument piano not ready, skipping at step 9
655+
// ... repeated until play is restarted
656+
[Audio] Preloading sampled instruments: piano // Only after restart
657+
```
658+
659+
**Runtime detection:**
660+
```javascript
661+
// If seeing "not ready" warnings but no corresponding "Preloading" message
662+
// The instrument was added mid-playback
663+
```
664+
665+
### Fix
666+
667+
**In SamplePicker.tsx, preload sampled instruments immediately on selection:**
668+
669+
```typescript
670+
import { getSampledInstrumentId } from '../audio/instrument-types';
671+
import { getAudioEngine } from '../audio/audioTriggers';
672+
673+
const handleSelect = useCallback(async (instrumentId: string) => {
674+
signalMusicIntent('add_track');
675+
676+
// Phase 23: Immediately preload sampled instruments
677+
const sampledId = getSampledInstrumentId(instrumentId);
678+
if (sampledId) {
679+
// Fire and forget - don't block UI
680+
getAudioEngine().then(engine => {
681+
engine.preloadInstrumentsForTracks([{ sampleId: instrumentId }]);
682+
}).catch(() => {}); // Ignore errors, scheduler will retry
683+
}
684+
685+
const name = getInstrumentName(instrumentId);
686+
onSelectSample(instrumentId, name);
687+
}, [onSelectSample]);
688+
```
689+
690+
### Prevention
691+
1. **Preload at point of selection** - Don't wait for play to start
692+
2. **Test mid-playback scenarios** - Add tests that add tracks during playback
693+
3. **Consider useEffect watcher** - Watch track changes and preload new sampled instruments
694+
695+
### Related Files
696+
- src/components/SamplePicker.tsx - Where tracks are added
697+
- src/components/StepSequencer.tsx - Where preload currently happens (only on play)
698+
- src/audio/scheduler.ts - Where "not ready" warning originates
699+
- src/audio/instrument-types.ts - getSampledInstrumentId helper
700+
701+
### Post-Fix Analysis
702+
```bash
703+
# Find other places that add tracks without preloading
704+
grep -r "ADD_TRACK\|add_track" src/ --include="*.ts" --include="*.tsx"
705+
706+
# Find calls to signalMusicIntent that might need preloading
707+
grep -r "signalMusicIntent" src/ --include="*.ts" --include="*.tsx"
708+
```
709+
710+
---
711+
490712
## Debugging Tools Quick Reference
491713

492714
### Enable Debug Tracing

app/scripts/session-api.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,8 @@ function validateTrack(track: unknown, index: number): ValidationError[] {
187187
if (t.stepCount !== undefined) {
188188
if (typeof t.stepCount !== 'number') {
189189
errors.push({ path: `${path}.stepCount`, message: `Expected number, got ${typeof t.stepCount}` });
190-
} else if (![4, 8, 16, 32, 64].includes(t.stepCount)) {
191-
errors.push({ path: `${path}.stepCount`, message: `stepCount must be 4, 8, 16, 32, or 64, got ${t.stepCount}` });
190+
} else if (![4, 8, 12, 16, 24, 32, 64, 96, 128].includes(t.stepCount)) {
191+
errors.push({ path: `${path}.stepCount`, message: `stepCount must be 4, 8, 12, 16, 24, 32, 64, 96, or 128, got ${t.stepCount}` });
192192
}
193193
}
194194

0 commit comments

Comments
 (0)