docs(adr): T2 implementation retrospective

This commit is contained in:
Joey Yakimowich-Payne 2026-04-19 09:29:07 -06:00
commit a27cb29a5b
No known key found for this signature in database

View file

@ -456,3 +456,26 @@ Capped at 50 snapshots to bound memory. Cleared on Save because the saved state
- Infinite history — memory unbounded. 50 is generous for a single editing session.
- Per-modifier undo (like per-field undo in some editors) — over-granular for structured data edits.
- Persist history to localStorage — adds complexity for marginal benefit; users expect editor state to reset between sessions.
---
## T2 Implementation Retrospective
### T1 simplifications resolved
- **Immediate-apply hot-swap → turn-boundary queue (T2-ADR-1)**: T1's `handleModifierProfileUpdate` applied profile changes immediately on receipt; T2's implementation enqueues into `Room.pendingProfile` and applies via `applyPendingProfileIfAny` after the next successful `applyMove`. NACKs at apply time (re-validation against post-move session) route to the original proposer's socket via `findSocketByToken`.
- **Host-only authority → two-player consent (T2-ADR-2)**: T1 broadcast modifier swaps from any host message; T2 requires the opponent to send `modifier-profile.consent` with `approve` before the swap is enqueued. Solo-mode preserves the host-shortcut path (`modifier-profile.update`) since the sole participant is trivially the sole consenter.
### Discovered patterns
- **Reused `modifier-profile.queued` ack for proposer** rather than introducing a `modifier-profile.proposal-sent`. Client receipt handlers stay uniform; the observable difference is the opponent's wire traffic (`proposal-pending` vs. silence). Documented in protocol.ts.
- **Capture-phase `stopImmediatePropagation` for nested modal Esc handling**: ModifierProfileEditor's Esc handler now uses capture phase with `stopImmediatePropagation` so the RulesDrawer's own Esc handler does NOT also fire. Without this, both would close on a single keystroke, leaving the drawer's pointer-events backdrop briefly lingering. Discovered post-T1 via the solo-smoke regression test.
- **Token-keyed pending-proposer tracking**: `Room.pendingProposerToken` (and `proposalState.proposedByToken`) survive socket reconnects. NACK routing on apply-time validation failure finds the proposer by token, not socket id, so a brief disconnect doesn't lose the rejection notification.
- **Timeout stale-handle guard**: 60s proposal timeouts use `setTimeout` whose handler checks `room.proposalState !== currentProposalRef` before firing. Prevents firing rejection broadcasts on stale (already-consented or already-superseded) proposals.
- **Client-side undo/redo via `pushSnapshot`**: ModifierProfileEditor maintains its history-stack purely client-side. The 50-snapshot cap is generous for a single editing session. History clears on Save/Cancel (the saved state becomes a new baseline).
- **Component-local clipboard for copy/paste**: No OS clipboard interaction. Cross-panel sharing goes through a `clipboard` state lifted to the editor root. Dedup by `(pieceType, color, kind)` for type modifiers and `(square, kind)` for instance modifiers prevents pastes from creating additive duplicates.
- **Inline conflict resolution via `applyFix` dispatcher**: Each error/warning code routes to a specific resolution. `E_PROFILE_NO_KING` and `E_PROFILE_ATTR_LIMIT` are advisory-only (require user judgement); the rest auto-resolve.
### Deviations
- None — implementation matched ADRs as written.