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 <vibe@mistral.ai>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user