docs(review): 📝 add cycle 5 multi-agent review and implementation plan

Author: hletrd

.context/plans/cycle5-implementation-2026-05-04.md

@@ -0,0 +1,76 @@
+# Cycle 5 Implementation Plan — 2026-05-04
+
+Based on cycle 5 aggregate review at `.context/reviews/_aggregate.md`.
+New findings this cycle: 1 LOW actionable (dead code), 2 LOW style. All quality gates pass clean.
+Cycle 4 plan items (P10, P11) all completed.
+
+---
+
+## Phase 1 — Code quality fix
+
+### P12 — Remove dead `isMapRenderExportError` function (C5-F1)
+
+- **Severity**: Low (code quality / fragility) | **Confidence**: High
+- **File**: `src/lib/useExportController.ts:24-27, 262-272`
+- **Issue**: `isMapRenderExportError` uses fragile substring matching on error messages. `ExportError` instances from `waitForStableMap` already carry codes `'EXPORT_MAP_RENDER'` and `'EXPORT_MAP_IDLE'` that are mapped in `EXPORT_ERROR_I18N`. The catch block checks `error instanceof ExportError && EXPORT_ERROR_I18N[error.code]` first, making the substring check dead code.
+- **Fix**:
+  1. Remove the `isMapRenderExportError` function.
+  2. Simplify the catch block error classification to only use `ExportError.code`.
+  3. Add a final fallback for non-ExportError errors that checks for map render messages in the catch-all else branch.
+- **Effort**: Small
+- **Status**: TODO
+
+## Phase 2 — Style fixes
+
+### P13 — Fix indentation in MapView progress effect (C5-F2, carried from C4-F1)
+
+- **Severity**: Low (style) | **Confidence**: High
+- **File**: `src/components/MapView.tsx:1064-1067`
+- **Issue**: 6-space indentation instead of 4-space.
+- **Fix**: Re-indent to 4 spaces.
+- **Effort**: Trivial
+- **Status**: TODO
+
+### P14 — Fix indentation in SceneEditor scenes list (C5-F3)
+
+- **Severity**: Low (style) | **Confidence**: High
+- **File**: `src/components/SceneEditor.tsx:568`
+- **Issue**: 8-space indentation at scenes list rendering.
+- **Fix**: Dedent by 4 spaces.
+- **Effort**: Trivial
+- **Status**: TODO
+
+---
+
+## Deferred findings (carried forward with exit criteria)
+
+All items from cycles 1-4 carry forward unchanged:
+- DEF-01 MapView.tsx monolith (Low — requires large refactor)
+- DEF-02 No tests for MapView pure utilities (Low — blocked by DEF-01)
+- DEF-03 No tests for export controller (Low — complex async testing)
+- DEF-04 No tests for parseCoordinateQuery (Low — easy but low priority)
+- DEF-05 mediabunny no explicit cleanup API (Info — library limitation)
+- DEF-06 waitForIdle type mismatch (Info — no runtime impact)
+- All N-series deferred items from cycles 1-2
+
+---
+
+## Quality gates
+
+After each commit:
+- `npm run lint` — must pass
+- `npm run typecheck` — must pass
+- `npm run build` — must pass
+- `npm run test` — must pass
+- `npm audit --audit-level=high` — must pass
+- `git commit -S` — GPG-signed with conventional commit + gitmoji
+
+---
+
+## Completion Status (updated after implementation)
+
+| Item | Status | Commit | Notes |
+|------|--------|--------|-------|
+| P12 — Remove dead error function | DONE | | Removed isMapRenderExportError, simplified catch block |
+| P13 — Fix MapView indentation | ALREADY DONE | (cycle 4, 829daa2) | Was fixed in cycle 4 |
+| P14 — Fix SceneEditor indentation | DONE | | Fixed {scenes.map indentation from 10 to 8 spaces |

.context/reviews/_aggregate.md

@@ -1,58 +1,83 @@
-# Aggregate Review — Travelback (2026-05-04, Cycle 4)
+# Aggregate Review — Travelback (2026-05-04, Cycle 5)
 
 ## Overview
 
-Deep review completed. All quality gates pass clean (lint=0, typecheck=clean, test=219/219, audit=0 vulns, build=clean). The codebase is in excellent condition after cycles 1-3 fixes. Previous cycles' MEDIUM findings (C3-F1 MapView monolith, C3-C1 stale deferred findings) were addressed or carried. This cycle found only minor/cosmetic issues.
+Deep review completed across all 11 agent perspectives. All quality gates pass clean (lint=0, typecheck=clean, test=219/219, audit=0 vulns, build=clean). The codebase remains in excellent condition after cycles 1-4. This cycle found one actionable code quality issue and minor style inconsistencies.
 
 ## Deduplicated Findings (ordered by severity/confidence)
 
-### LOW PRIORITY
+### LOW PRIORITY — ACTIONABLE
 
-#### C4-F1. Inconsistent indentation in MapView progress effect
+#### C5-F1. Dead code: `isMapRenderExportError` uses fragile substring matching
+**Severity**: Low (code quality / fragility) | **Confidence**: High
+**File**: `src/lib/useExportController.ts:24-27`
+**Issue**: `isMapRenderExportError` checks `error.message.includes('Map did not finish rendering')` to classify export errors. However, `ExportError` instances from `waitForStableMap` (lines 177, 189) already carry codes `'EXPORT_MAP_RENDER'` and `'EXPORT_MAP_IDLE'`, which are mapped in `EXPORT_ERROR_I18N` (lines 17-22). The catch block at line 267 checks `error instanceof ExportError && EXPORT_ERROR_I18N[error.code]` before the substring fallback, making `isMapRenderExportError` dead code. If someone changes the error message text, the function would silently break.
+**Fix**: Remove `isMapRenderExportError` function and the substring check in the catch block. Rely solely on `ExportError.code` for classification.
+**Agent agreement**: code-reviewer (C5-F1), critic (C5-C2), verifier (V6), debugger (C5-DB1).
+
+### LOW PRIORITY — STYLE
+
+#### C5-F2. Inconsistent indentation in MapView progress effect
 **Severity**: Low (style) | **Confidence**: High
 **File**: `src/components/MapView.tsx:1064-1067`
-**Issue**: Lines 1064-1067 (marker position update inside the progress useEffect) use 6-space indentation instead of the surrounding 4-space indentation. This is a formatting inconsistency introduced during cycle 2 refactoring.
+**Issue**: 6-space indentation instead of surrounding 4-space. Carried from C4-F1.
 **Fix**: Re-indent to 4 spaces.
 
-#### C4-F2. TimelineSelector `hasTime` not memoized
-**Severity**: Low (perf) | **Confidence**: Medium
-**File**: `src/components/TimelineSelector.tsx:369`
-**Issue**: `hasTime` is computed via `points.some((p) => p.time)` on every render. For tracks with up to 250K points, this iterates the full array each time the component re-renders. Since `points` reference only changes on track load/trim, this should be memoized.
-**Fix**: Wrap in `useMemo` keyed on `points`.
-
-### INFORMATIONAL (no action required)
-
-#### C4-I1. exportVideo waitForIdle signature uses `Promise<void>` but callers return `Promise<boolean>`
-**Severity**: Info | **Confidence**: High
-**File**: `src/lib/videoEncoder.ts:83` vs `src/components/MapView.tsx:689`
-**Issue**: `exportVideo` declares `waitForIdle: () => Promise<void>` but the actual MapView implementation returns `Promise<boolean>`. TypeScript allows this (boolean return is discarded) but the API contract is slightly misleading.
-**Fix**: None required — this is standard TypeScript variance behavior. Document for awareness.
-
-#### C4-I2. `smoothCameraState` wrapper is now a trivial delegate to `lerpCamera`
-**Severity**: Info | **Confidence**: High
-**File**: `src/components/MapView.tsx:77-79`
-**Issue**: After cycle 3's P06 consolidation, `smoothCameraState` is now a one-line wrapper calling `lerpCamera(previous, target, factor, linear, bearingFactor)`. The wrapper exists for readability (naming the concept) and is used in exactly one call site. No action needed.
-**Fix**: None required.
-
-#### C4-I3. mediabunny Output has no explicit cleanup API on abort
+#### C5-F3. Inconsistent indentation in SceneEditor scenes list
+**Severity**: Low (style) | **Confidence**: High
+**File**: `src/components/SceneEditor.tsx:568`
+**Issue**: 8-space indentation (extra 4 spaces) at the scenes list rendering block.
+**Fix**: Dedent by 4 spaces.
+
+### CARRIED DEFERRED ITEMS (from cycles 1-4)
+
+#### DEF-01. MapView.tsx monolith (1200 lines, 7+ concerns)
+**Severity**: Low (architecture) | **Confidence**: High
+**File**: `src/components/MapView.tsx`
+**Original**: C3-F1. Extract pure functions to src/lib/mapUtils.ts.
+**Status**: Deferred. Requires significant refactoring effort. Not blocking.
+
+#### DEF-02. No tests for MapView pure utility functions
+**Severity**: Low (test gap) | **Confidence**: High
+**Original**: C3-TE1, C5-TE1.
+**Status**: Deferred. Blocked by DEF-01 (extraction).
+
+#### DEF-03. No tests for export controller state machine
+**Severity**: Low (test gap) | **Confidence**: High
+**Original**: C5-TE2.
+**Status**: Deferred. Complex async testing setup required.
+
+#### DEF-04. No tests for JourneyCreator parseCoordinateQuery
+**Severity**: Low (test gap) | **Confidence**: High
+**Original**: C5-TE3.
+**Status**: Deferred. Easy to implement, low priority.
+
+#### DEF-05. mediabunny Output has no explicit cleanup API on abort
 **Severity**: Info | **Confidence**: Medium
 **File**: `src/lib/videoEncoder.ts:168-173`
-**Issue**: When export is aborted, `output.finalize()` is correctly skipped (to avoid corrupt MP4), but the Output/CanvasSource/BufferTarget objects rely on GC for cleanup. mediabunny does not expose a `dispose()` or `close()` method. This was flagged in C3-F4 but is a library limitation, not a code defect.
-**Fix**: None — deferred until mediabunny adds explicit cleanup API.
+**Original**: C3-F4, C4-I3.
+**Status**: Deferred. Library limitation, not a code defect.
+
+#### DEF-06. `waitForIdle` type mismatch (Promise<boolean> vs Promise<void>)
+**Severity**: Info | **Confidence**: High
+**Files**: `src/components/MapView.tsx:35` vs `src/lib/videoEncoder.ts:83`
+**Original**: C4-I1.
+**Status**: Deferred. TypeScript variance allows this. No runtime impact.
 
-## VERIFIED FIXES FROM CYCLES 1-3
+### VERIFIED FIXES FROM CYCLES 1-4 (all intact)
 
-All prior fixes verified:
-- C3-F3 referenceGridData dependency: VERIFIED (line 866 includes `referenceGridData`)
-- C3-F2 camera smoothing consolidation: VERIFIED (`smoothCameraState` delegates to `lerpCamera`)
-- C3-P1 fallback timer optimization: VERIFIED (line 117: `document.visibilityState === 'hidden'` guard)
-- C3-C1 stale deferred findings: VERIFIED (N01 archived, N10 corrected)
-- All cycle 1-2 fixes: VERIFIED (no regressions)
+- C4-F2 `hasTime` memoization: VERIFIED (TimelineSelector.tsx:363)
+- C3-F2 camera smoothing consolidation: VERIFIED (MapView.tsx:77-79)
+- C3-F3 referenceGridData dependency: VERIFIED (MapView.tsx:866)
+- C3-P1 fallback timer optimization: VERIFIED (usePlaybackController.ts:117)
+- C2-DB-01 export progress restoration: VERIFIED (useExportController.ts:311)
+- C2-DB-04 resetSize cleanup: VERIFIED (MapView.tsx:685-702)
+- All cycle 1 fixes: VERIFIED (no regressions)
 
 ## AGENT FAILURES
 
-None. Single-agent review completed.
+None. All 11 agent perspectives completed successfully.
 
 ## Cross-Agent Agreement Summary
 
-N/A — single-agent cycle. All findings are low-informational; no prior-agency disagreement.
+All agents agree the codebase is in excellent condition. The only actionable finding (C5-F1) is agreed upon by 4 agents (code-reviewer, critic, verifier, debugger). All other findings are style/informational.

.context/reviews/code-reviewer.md

@@ -1,42 +1,37 @@
-# Code Reviewer — Cycle 3 (2026-05-04)
+# Code Reviewer — Cycle 5 (2026-05-04)
 
 ## Scope
-Full codebase review. Cycle 2 aggregate findings (C2-F1 through C2-F6) verified as resolved.
+Full codebase review. Focus on deeper analysis of deferred items, recently modified files, and remaining substantive issues.
 
 ## Findings
 
-### C3-F1. MapView.tsx is a 1214-line monolith violating Single Responsibility
-**Severity**: Medium | **Confidence**: High
-**File**: `src/components/MapView.tsx` (all 1214 lines)
-**Issue**: MapView.tsx combines at least 7 distinct concerns: map initialization/cleanup, track geometry building, camera interpolation and smoothing, reference grid computation, marker management, export frame rendering, and debug state exposure. This makes the component hard to test, hard to review, and a high-conflict zone for concurrent changes.
-**Fix**: Extract pure functions (geometry builders, grid computation, camera smoothing) into a `src/lib/mapUtils.ts` module. Consider extracting the imperative handle implementation into a custom hook.
-**Effort**: Large
-
-### C3-F2. Duplicated camera smoothing logic between MapView and camera.ts
-**Severity**: Low | **Confidence**: High
-**File**: `src/components/MapView.tsx:66-93` vs `src/lib/camera.ts:120-138`
-**Issue**: `smoothCameraState()` in MapView duplicates the camera interpolation logic in `lerpCamera()` from camera.ts. Both use `shortestLngDelta` for antimeridian-safe longitude interpolation and linear lerp for zoom/pitch. The MapView version lacks smoothstep easing (uses raw factor), creating inconsistent smoothing between export and playback.
-**Fix**: Replace `smoothCameraState` with a call to a shared interpolation function from camera.ts.
-**Effort**: Small
-
-### C3-F3. useEffect missing referenceGridData dependency in style-change effect
-**Severity**: Low | **Confidence**: Medium
-**File**: `src/components/MapView.tsx:857-880`
-**Issue**: The style-change effect reads `referenceGridData` from the closure but its dependency array only contains `[mapStyleKey]`. While `referenceGridData` is memoized on `track`, if it changed while the style was also changing, stale grid data would be rendered.
-**Fix**: Add `referenceGridData` to the dependency array.
-**Effort**: Trivial
-
-### C3-F4. exportVideo does not explicitly close Output on abort
-**Severity**: Low | **Confidence**: Medium
-**File**: `src/lib/videoEncoder.ts:130-173`
-**Issue**: When export is aborted, `output.finalize()` is skipped but the Output object may hold WebCodecs encoder resources. If mediabunny doesn't clean up on GC, this could leak.
-**Fix**: Add explicit cleanup in the finally block when !completed, or document the assumption.
-**Effort**: Small
-
-### C3-F5. Verified: C2-F1 export progress restoration is FIXED
+### C5-F1. `isMapRenderExportError` uses fragile substring matching instead of error codes
+**Severity**: Low (fragility) | **Confidence**: High
+**File**: `src/lib/useExportController.ts:24-27`
+**Issue**: `isMapRenderExportError` checks `error.message.includes('Map did not finish rendering')` to classify export errors. However, `ExportError` instances at lines 177 and 189 in `waitForStableMap` already carry codes `'EXPORT_MAP_RENDER'` and `'EXPORT_MAP_IDLE'`, which are mapped in `EXPORT_ERROR_I18N` at lines 17-22. The substring check is either dead code (if the ExportError code path always matches first) or a fragile fallback. If someone changes the error message text, classification breaks silently.
+**Fix**: Remove `isMapRenderExportError` and rely solely on `error instanceof ExportError && EXPORT_ERROR_I18N[error.code]` in the catch block.
+
+### C5-F2. MapView progress effect indentation inconsistency (carried from C4-F1)
+**Severity**: Low (style) | **Confidence**: High
+**File**: `src/components/MapView.tsx:1064-1067`
+**Issue**: Lines inside the progress useEffect use 6-space indentation instead of the surrounding 4-space.
+**Fix**: Re-indent to 4 spaces.
+
+### C5-F3. SceneEditor scenes list indentation inconsistency
+**Severity**: Low (style) | **Confidence**: High
+**File**: `src/components/SceneEditor.tsx:568`
+**Issue**: The scenes list rendering uses 8-space indentation (extra 4 spaces) compared to surrounding JSX at the same nesting level.
+**Fix**: Dedent by 4 spaces.
+
+### C5-F4. Verified: C4-F2 `hasTime` memoization is FIXED
 **Severity**: N/A | **Confidence**: High
-**File**: `src/lib/useExportController.ts:148,252,311`
-**Status**: Verified fixed.
+**File**: `src/components/TimelineSelector.tsx:363`
+**Status**: `useMemo` wrapping confirmed. No action needed.
+
+### C5-F5. Verified: C3-F2 camera smoothing consolidation is FIXED
+**Severity**: N/A | **Confidence**: High
+**File**: `src/components/MapView.tsx:77-79`
+**Status**: `smoothCameraState` delegates to `lerpCamera` from camera.ts. No duplication.
 
 ## Summary
-Codebase in excellent shape. Main actionable finding is MapView.tsx monolith (C3-F1). Duplicated camera smoothing (C3-F2) is a small consistency issue.
+Codebase remains in excellent condition. The only actionable finding is removing the fragile substring error classification (C5-F1). Two minor indentation issues carried from prior cycles.

.context/reviews/critic.md

@@ -1,31 +1,25 @@
-# Critic — Cycle 3 (2026-05-04)
+# Critic — Cycle 5 (2026-05-04)
 
 ## Scope
-Multi-perspective critique of the whole change surface.
+Multi-perspective critique of the full change surface and deferred items.
 
 ## Findings
 
-### C3-C1. Deferred items have grown stale — no re-evaluation mechanism
-**Severity**: Medium | **Confidence**: High
-**Files**: `.context/plans/deferred-findings-*.md`
-**Issue**: 14+ deferred findings carried forward across multiple cycles. Some (e.g., N01 "Per-frame trail rebuild during playback") may no longer be accurate. Without re-validation, the deferred list becomes misleading noise.
-**Fix**: Periodically re-validate deferred findings against current code.
-**Effort**: Small
+### C5-C1. Deferred items need re-evaluation (carried from C3-C1)
+**Severity**: Low (process) | **Confidence**: High
+**Files**: `.context/plans/` deferred items
+**Issue**: 14 deferred items carried forward from cycles 1-4. Some (like MapView monolith, test coverage gaps) remain valid. Others may have been implicitly resolved. The deferred list needs pruning.
+**Fix**: Re-evaluate each deferred item against current code state.
 
-### C3-C2. MapView complexity creates review blind spots
-**Severity**: Medium | **Confidence**: High
-**File**: `src/components/MapView.tsx`
-**Issue**: At 1214 lines with 7+ concerns, reviewers tend to skim or focus only on recently-changed regions. Same as C3-F1.
-**Fix**: Extract pure functions from MapView.
-**Effort**: Large
+### C5-C2. Dead code in export error handling
+**Severity**: Low (code quality) | **Confidence**: High
+**File**: `src/lib/useExportController.ts:24-27`
+**Issue**: `isMapRenderExportError` is dead code. The ExportError code path handles all cases. Confirmed by verifier (V6).
+**Fix**: Remove the function and the substring check.
 
-### C3-C3. No regressions from cycle 1-2 fixes
+### C5-C3. No regressions from cycles 1-4 fixes
 **Severity**: N/A | **Confidence**: High
-**Issue**: Reviewed all cycle-1 and cycle-2 commits. The exportSucceeded guard, scene tests, and other fixes are correct. No regressions.
-
-### C3-C4. Quality gates clean
-**Severity**: N/A | **Confidence**: High
-**Issue**: lint=0, typecheck=clean, test=219/219, audit=0 vulns.
+**Status**: All prior fixes verified intact. Quality gates clean.
 
 ## Summary
-Codebase in excellent condition. Main critique: deferred findings need cleanup, MapView needs decomposition. No regressions.
+Codebase in excellent condition. One dead code finding. Deferred items need periodic re-evaluation. No regressions.