Skip to content

Commit 3a93094

Browse files
committed
docs: Add original code and missing documentation analysis
- Show original SET_TRACK_STEP_COUNT that we're restoring (just sets stepCount) - Document the root cause: design intent was never explicitly stated - Identify documentation gap: invariant enforced design but didn't explain WHY - Add lesson learned: invariants should include rationale, not just checks - Note that LESSONS-LEARNED.md used <= 128 not === 128, adding to confusion
1 parent a0f97e4 commit 3a93094

File tree

1 file changed

+70
-0
lines changed

1 file changed

+70
-0
lines changed

specs/research/STEP-ARRAY-INVARIANT-BUG.md

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,76 @@ Commit `af466ff` titled "fix: Step count sync and array resizing bug" actually *
263263
3. Also added: `bug-patterns.ts` documenting the new (wrong) pattern
264264
4. Also added: Tests verifying the new (wrong) behavior
265265
266+
### Original Code (CORRECT)
267+
268+
```typescript
269+
// grid.tsx BEFORE af466ff - just sets stepCount, no resizing
270+
case 'SET_TRACK_STEP_COUNT': {
271+
const tracks = state.tracks.map((track) => {
272+
if (track.id !== action.trackId) return track;
273+
return { ...track, stepCount: Math.max(1, Math.min(MAX_STEPS, action.stepCount)) };
274+
});
275+
return { ...state, tracks };
276+
}
277+
```
278+
279+
This is exactly what we're proposing to restore.
280+
281+
---
282+
283+
## Missing Documentation: Root Cause of the Bug
284+
285+
### What Was Documented
286+
287+
| Location | What It Said |
288+
|----------|--------------|
289+
| `invariants.ts:106` | `steps.length !== MAX_STEPS` is a violation |
290+
| `invariants.ts:243-249` | Repair pads/truncates to MAX_STEPS |
291+
| `LESSONS-LEARNED.md:2886` | `assert(t.steps.length <= 128)` (note: uses `<=` not `===`) |
292+
293+
### What Was NOT Documented
294+
295+
**The critical design decision was never explicitly stated:**
296+
297+
> "Arrays are always fixed at MAX_STEPS length. The `stepCount` property indicates how many steps are 'active' or 'visible', but arrays never resize."
298+
299+
This missing documentation allowed a developer to reasonably conclude:
300+
- "Arrays should match stepCount for consistency"
301+
- "Let me add a 'fix' to resize arrays"
302+
- "Let me document this 'pattern' for others"
303+
304+
### The Documentation Gap
305+
306+
The invariant check **enforced** the design but didn't **explain** it:
307+
308+
```typescript
309+
// invariants.ts - WHAT (the check)
310+
if (track.steps.length !== MAX_STEPS) {
311+
violations.push(`Track ${track.id}: steps length ${track.steps.length} !== ${MAX_STEPS}`);
312+
}
313+
314+
// MISSING: WHY (the rationale)
315+
// Arrays are fixed-length because:
316+
// 1. stepCount is a "view window" - users may reduce then expand
317+
// 2. Data beyond stepCount should be preserved (non-destructive editing)
318+
// 3. Playback uses modulo arithmetic: track.steps[globalStep % stepCount]
319+
// 4. Simpler invariant: arrays are always exactly MAX_STEPS
320+
```
321+
322+
### Lesson Learned
323+
324+
**Invariants should include rationale, not just checks.**
325+
326+
When adding an invariant, document:
327+
1. **WHAT** the constraint is
328+
2. **WHY** it exists (the design intent)
329+
3. **CONSEQUENCES** of violating it
330+
331+
This would have prevented af466ff - the developer would have seen:
332+
> "stepCount is a view window, arrays never resize"
333+
334+
...and would not have added resizing code.
335+
266336
---
267337
268338
## Recommended Fix

0 commit comments

Comments
 (0)