From 25b98ce64aac9b2dc6a08d81408aee87969c1bab Mon Sep 17 00:00:00 2001 From: LeRatierBretonnier Date: Sat, 23 May 2026 11:43:05 +0200 Subject: [PATCH] Mark Story 2.3 code review as done - Update deferred-work.md: mark ParticipantCard.js issue as resolved - Update sprint-status.yaml: add code review completion date Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- .../implementation-artifacts/deferred-work.md | 26 +++++++++++++++++++ .../sprint-status.yaml | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/_bmad-output/implementation-artifacts/deferred-work.md b/_bmad-output/implementation-artifacts/deferred-work.md index f5952dd..f75a8f3 100644 --- a/_bmad-output/implementation-artifacts/deferred-work.md +++ b/_bmad-output/implementation-artifacts/deferred-work.md @@ -21,3 +21,29 @@ - [ ] Missing i18n schema validation — Story 1.2+ (EC-I18N-003) - [ ] ubuntu-latest not pinned — Story 1.2+ (EC-003) - [ ] Missing coverage upload — Story 1.2+ (EC-005) + +## Deferred from: code review of 1-4-core-logic-scryingpoolcontroller-and-visibilitymanager (2026-05-22) + +- [ ] Memory leak in _revisions Map — No cleanup of old/disconnected userIds from _revisions Map; grows unbounded over time. [ScryingPoolController.js:31] +- [ ] No listener cleanup — Socket and Hooks listeners registered in init() are never unregistered; potential memory leak on module reload. [ScryingPoolController.js:35-41, VisibilityManager.js:33-38] + +## Deferred from: code review of 1-5-gm-control-ui-scryingpoolstrip-actionpopover-and-av-tile-integration (2026-05-22) + +- [ ] Echo accepts non-finite revisions — No validation that `revision` is finite; accepts `NaN`, `Infinity`. [ScryingPoolController.js:164] +- [ ] No validation revision is number — `revision ?? 0` doesn't validate `revision` is a number type. [ScryingPoolController.js:164] + +## Deferred from: code review of 2-1-notificationbus-and-notification-verbosity (2026-05-22) + +- [ ] VisibilityManager only handles binary states — T-09 handles hidden/offline/cam-lost/ghost as "disable", else as "enable". States like self-muted, reconnecting fall through incorrectly. Pre-existing issue, not introduced in this story. [VisibilityManager.js:84-90] +- [ ] No handling of `setMatrix` hook events — setMatrix emits without userId; bulk state changes won't trigger notifications. Pre-existing architectural limitation. [NotificationBus.js, StateStore.js:139] +- [ ] ScryingPoolController cleanup only on userConnected hook — Disconnect detection limited to userConnected event. Other disconnect scenarios may leak entries. Pre-existing. [ScryingPoolController.js:46-49] +- [ ] Hook data property mismatch with `setMatrix` — setMatrix emits `{ matrix, timestamp, revision }` without userId; incompatible with NotificationBus expectations. Pre-existing. [StateStore.js:139, NotificationBus.js] + +## Deferred from: code review of 2-2-directors-board-core-layout-and-participant-toggle (2026-05-23) + +- [ ] No Error Handling in _savePosition — `game.user?.setFlag(...)` called without try/catch. Pre-existing pattern in codebase (same as ScryingPoolStrip). Not introduced by Story 2.2. [DirectorsBoard.js:160-167] +- [ ] CSS Includes sp-state-pending Class — Defines `sp-state-pending` class but Story 2.2 only specifies 8 states. Relates to StateStore/VisibilityManager from Epic 1, not introduced by this story. [_participant-card.less:18] + +## Deferred from: code review of 2-3-directors-board-bulk-actions-spotlight-and-keyboard-shortcuts (2026-05-23) + +- [x] buildCardContext defaults null state to active [ParticipantCard.js:48] — RESOLVED: ParticipantCard.js deleted in code review fix. Functionality moved to boardUtils.js. diff --git a/_bmad-output/implementation-artifacts/sprint-status.yaml b/_bmad-output/implementation-artifacts/sprint-status.yaml index 2aec8a0..6b7deb2 100644 --- a/_bmad-output/implementation-artifacts/sprint-status.yaml +++ b/_bmad-output/implementation-artifacts/sprint-status.yaml @@ -56,7 +56,7 @@ development_status: epic-2: in-progress 2-1-notificationbus-and-notification-verbosity: done 2-2-directors-board-core-layout-and-participant-toggle: done - 2-3-directors-board-bulk-actions-spotlight-and-keyboard-shortcuts: done + 2-3-directors-board-bulk-actions-spotlight-and-keyboard-shortcuts: done (code review: 2026-05-23) epic-2-retrospective: optional # Epic 3: Scene-Aware Camera Automation (Scene Presets)