170 lines
7.9 KiB
Markdown
170 lines
7.9 KiB
Markdown
# Retrospective — Epic 1: Core Camera Visibility Control
|
|
|
|
**Date:** 2026-05-22
|
|
**Facilitator:** Amelia (Developer)
|
|
**Project Lead:** Morr
|
|
**Status:** Complete
|
|
|
|
---
|
|
|
|
## Epic Summary
|
|
|
|
| Metric | Value |
|
|
|--------|-------|
|
|
| Stories Completed | 6 / 6 (100%) |
|
|
| Test Count at Close | 296 tests, all passing |
|
|
| Technical Debt Items | 8 open (deferred-work.md) |
|
|
| Major Blockers | 1 (OQ-1 WebRTC spike — resolved as css-fallback) |
|
|
| Architecture Deviations Corrected | 3 |
|
|
| Previous Retrospective | None (first epic) |
|
|
|
|
**Stories:**
|
|
- 1.1 — Module Scaffold, CI/CD & Design Token System ✅
|
|
- 1.2 — WebRTC Spike — Track Disabling API Validation ✅
|
|
- 1.3 — Data Layer — FoundryAdapter, StateStore & Socket Infrastructure ✅
|
|
- 1.4 — Core Logic — ScryingPoolController & VisibilityManager ✅
|
|
- 1.5 — GM Control UI — ScryingPoolStrip, ActionPopover & AV Tile Integration ✅
|
|
- 1.6 — Player Camera Status Badge ✅
|
|
|
|
---
|
|
|
|
## Successes & Strengths
|
|
|
|
1. **TDD discipline held across all 6 stories.** Started at 49 tests (Story 1.1), ended at 296 — every story delivered a passing pipeline with zero regressions.
|
|
|
|
2. **Import boundary enforcement worked.** ESLint `import/no-restricted-paths` wired in Story 1.1 held for the full epic. Zero cross-layer violations slipped through.
|
|
|
|
3. **Accessibility built into ACs, not added later.** `prefers-reduced-motion` gates, native `<dialog>` for focus trapping, `aria-live="polite"` on the VisibilityBadge — all specified upfront and delivered.
|
|
|
|
4. **UI pattern reuse was excellent.** `ActionPopover` (Story 1.5) was directly reused as the pattern for `FirstEncounterPanel` and `VisibilityDetailsPanel` (Story 1.6) — zero wheel reinvention.
|
|
|
|
5. **Architecture guardrails saved multiple errors.** Three corrections were caught and applied mid-story rather than shipping as bugs.
|
|
|
|
6. **Morr's assessment:** Everything runs fine. Clean green on all tests.
|
|
|
|
---
|
|
|
|
## Challenges & Growth Areas
|
|
|
|
### 1. Epic/Architecture Spec Drift (3 occurrences)
|
|
|
|
Mismatches between the epics file and architecture document caught mid-story:
|
|
- Story 1.1: `.eslintrc.js` spec vs ESLint 9 flat config (`eslint.config.js`)
|
|
- Story 1.2: Epic path `src/adapters/foundry-adapter.js` vs architecture-canonical `src/foundry/FoundryAdapter.js`
|
|
- Story 1.2: Setting namespace `video-view-manager.*` in spec vs correct `scrying-pool.*`
|
|
|
|
**Root cause:** Architecture doc written before epics were finalized; paths/namespaces drifted.
|
|
**Resolution:** Dev agent caught all three. No shipped bugs, but correction cycles add overhead.
|
|
|
|
### 2. Spike Story AC Format
|
|
|
|
Story 1.2's ACs assumed `track-disable` would be the spike outcome. The spike proved it wasn't — `css-fallback` is the FoundryVTT v14 reality. The dev agent correctly updated test expectations, but the story file had wrong expectations going in.
|
|
|
|
**Root cause:** Spike ACs written as implementation expectations, not hypothesis format.
|
|
|
|
### 3. Vitest Fake Timer Nested Pattern Not Documented
|
|
|
|
Story 1.6 discovered that `vi.advanceTimersByTime(N)` requires two separate advance calls when a timer callback schedules a nested timer. The first advance fires the outer timer; the second fires the inner one. This cost debugging time.
|
|
|
|
**Pattern discovered:**
|
|
```js
|
|
vi.advanceTimersByTime(10_001); // fires 10s collapse timeout
|
|
vi.advanceTimersByTime(301); // fires 300ms CSS transition replacement timer
|
|
```
|
|
|
|
### 4. Story 1.1 Review Depth
|
|
|
|
The Story 1.1 code review generated 3 separate review prompt files and 7 deferred CI/CD items. While appropriate for a scaffold story, the depth felt heavy relative to the scope.
|
|
|
|
---
|
|
|
|
## Key Insights & Lessons
|
|
|
|
1. **Spike stories need hypothesis-format ACs.** Write: "Given X, when spike runs, then outcome will be documented as one of [A/B/C]" — not pre-determined implementation expectations.
|
|
|
|
2. **Vitest nested timer testing requires two-step advance.** Always use two `vi.advanceTimersByTime()` calls when the callback under test schedules a subsequent timer. Document this pattern visibly in dev notes for any story with timer-based logic.
|
|
|
|
3. **Architecture doc is canonical over epics.** When epics and architecture disagree, architecture wins. Path inconsistencies should be resolved in epics review before stories are created.
|
|
|
|
4. **Foundation story reviews deserve more scrutiny.** Story 1.1 review depth was appropriate; the overhead is proportional to its cross-cutting nature. Non-foundation stories can use lighter review.
|
|
|
|
5. **Reuse patterns establish themselves within 1-2 stories.** ActionPopover → FirstEncounterPanel/VisibilityDetailsPanel shows that patterns spread naturally when the first implementation is clean.
|
|
|
|
---
|
|
|
|
## Deferred Technical Debt (from deferred-work.md)
|
|
|
|
These items are tracked, not forgotten, and should be addressed in Epic 2:
|
|
|
|
**HIGH PRIORITY (should fold into Story 2.1 or 2.2):**
|
|
- [ ] Memory leak: `_revisions` Map in `ScryingPoolController.js:31` — no cleanup on user disconnect
|
|
- [ ] Listener cleanup: `init()` socket + Hooks listeners never unregistered in `ScryingPoolController.js` and `VisibilityManager.js`
|
|
- [ ] Echo revision validation: No guard against NaN/Infinity in `ScryingPoolController.js:164`
|
|
|
|
**MEDIUM PRIORITY:**
|
|
- [ ] Echo accepts non-finite revisions (same file, revision ?? 0 doesn't validate type)
|
|
- [ ] CVE-2023-43645 in flat-cache-4.0.1 (needs verification — may be transitive dev dep only)
|
|
|
|
**LOW PRIORITY (CI/CD, pre-existing):**
|
|
- [ ] Missing failure notifications, missing build artifact upload, no Node matrix testing
|
|
- [ ] Missing concurrency control, missing i18n schema validation, ubuntu-latest not pinned
|
|
|
|
---
|
|
|
|
## Epic 2 Preview & Readiness
|
|
|
|
**Epic 2: Player Notifications & Director's Board**
|
|
Stories: 2.1 NotificationBus, 2.2 Director's Board Core, 2.3 Director's Board Bulk Actions
|
|
|
|
**Dependencies on Epic 1 work:**
|
|
- `ScryingPoolController.action()` + event system ✅ solid
|
|
- `SocketHandler` echo/broadcast ✅ solid
|
|
- `AVTileAdapter` pattern available for Director's Board card tiles ✅
|
|
- `StateStore` visibility matrix ✅
|
|
|
|
**Readiness: CLEAR TO PROCEED ✅**
|
|
No blockers. Recommended: fold memory leak + listener cleanup into Story 2.1 dev tasks.
|
|
|
|
**Preparation notes for Story 2.1:**
|
|
- NotificationBus introduces a 3s coalescing timer — explicitly include the vitest fake timer two-advance pattern in dev notes
|
|
- Include deferred debt items (_revisions cleanup, listener unregistration) as Story 2.1 technical tasks
|
|
- NotificationBus subscribes to ScryingPoolController events — the listener cleanup debt becomes higher priority once another subscriber is added
|
|
|
|
---
|
|
|
|
## Action Items
|
|
|
|
### Technical (carry into Epic 2)
|
|
|
|
| Item | Owner | Priority | Target Story |
|
|
|------|-------|----------|--------------|
|
|
| Fix `_revisions` Map leak (`ScryingPoolController.js:31`) | Dev | High | 2.1 or 2.2 |
|
|
| Add listener cleanup to ScryingPoolController + VisibilityManager | Dev | High | 2.1 or 2.2 |
|
|
| Add echo revision type validation (reject NaN/Infinity) | Dev | Medium | 2.1 or 2.2 |
|
|
| Verify CVE-2023-43645 flat-cache scope | Dev | Low | Backlog |
|
|
|
|
### Process Improvements
|
|
|
|
| Item | Action |
|
|
|------|--------|
|
|
| Spec drift | Review epic paths/namespaces against architecture doc before story creation |
|
|
| Spike ACs | Write spike ACs as hypothesis format, not expected outcomes |
|
|
| Fake timer pattern | Add vitest nested timer two-advance pattern to story dev notes whenever coalescing timers are involved |
|
|
|
|
---
|
|
|
|
## Next Steps
|
|
|
|
1. ✅ Retrospective saved
|
|
2. ✅ `epic-1-retrospective` marked `done` in sprint-status.yaml
|
|
3. → Create Story 2.1 (NotificationBus & Notification Verbosity) using `bmad-create-story`
|
|
4. → Fold deferred debt items into Story 2.1 dev tasks
|
|
|
|
---
|
|
|
|
*Amelia (Developer): "Great session, Morr. Epic 1 delivered a solid, tested foundation. Epic 2 is clear to go."*
|
|
|
|
*Winston (System Architect): "The architecture held. Import boundaries, constructor rules, token system — all intact."*
|
|
|
|
*Sally (UX Designer): "Accessibility-first delivered. Epic 2 has its own UX challenges with the Director's Board — looking forward to it."*
|