uberwald/scrying-pool
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 5 Wiki Activity
Files
main
scrying-pool/_bmad-output/implementation-artifacts/epic-1-retro-2026-05-22.md
T
uberwald a1e8886fce Story 3.2 done
2026-05-23 18:23:48 +02:00

7.9 KiB
Raw Permalink Blame History

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:

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."

Reference in New Issue View Git Blame Copy Permalink