# Call to Play Code & Architecture Review Report I have conducted a thorough review of the commits (`e141229` through `2c204ac`) on branch `calltoplay`, referencing [`FABLE_5_FINDINGS.md`](file:///pantheon/pfs/git/rust/pfs/lanspread/FABLE_5_FINDINGS.md) and [`CALL_TO_PLAY_FIXES_PLAN.md`](file:///pantheon/pfs/git/rust/pfs/lanspread/CALL_TO_PLAY_FIXES_PLAN.md). --- ## 1. Plan Implementation & Deviation Assessment The plan outlined in [`CALL_TO_PLAY_FIXES_PLAN.md`](file:///pantheon/pfs/git/rust/pfs/lanspread/CALL_TO_PLAY_FIXES_PLAN.md) is **faithfully and elegantly implemented across all 5 code commits**, with zero regression to core invariants. | Commit | Scope | Plan Requirements | Code Verification | Status | |---|---|---|---|---| | [`be7ad2e`](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-peer/src/call_to_play.rs) | Atomic Batch Merge & Store Semantics | - Transactional history merge
- Single-pass $O(N)$ compaction
- Conflicting ID batch rejection
- `NeedHistory` for missing roots
- Rebuild event IDs from retained events
- Capacity cap applies to unresolved history only | - `merge_batch_at` in [call_to_play.rs](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-peer/src/call_to_play.rs#L83)
- `deduplicate_batch`
- `unresolved_event_count`
- Tests cover revival & conflict behavior | **Faithful** | | [`e5d70ae`](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-peer/src/services/stream.rs) | Live Replication & Acknowledgement | - Protocol v7 bump
- `CallToPlayAck` return outcomes
- Remove unreliable source-IP equality check
- Enforce envelope vs actor ID matching
- Async handshake resync on delivery failure | - Protocol updated in [`lanspread-proto`](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-proto/src/lib.rs)
- `handle_call_to_play_events` in [stream.rs](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-peer/src/services/stream.rs#L124)
- `delivery_resync_reason` in [call_to_play.rs](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-peer/src/call_to_play.rs#L472) | **Faithful** | | [`9c34efa`](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-tauri-deno-ts/src/lib/callToPlay.ts) | Terminal Outcome Retention | - 15-minute terminal outcome display (`TERMINAL_RETENTION_MS`) for `Running` and `Cancelled`
- Post-15-min compaction to tombstones
- Frontend status reduction, sorting, and badge exclusion
- Peer CLI scenario S49 | - `compact_history` in [call_to_play.rs](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-peer/src/call_to_play.rs#L340)
- Reducer logic in [callToPlay.ts](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-tauri-deno-ts/src/lib/callToPlay.ts)
- Roster/chat retained | **Faithful** | | [`8d3affe`](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-tauri-deno-ts/src/lib/callToPlay.ts) | Extend from Current Deadline | - `extendDeadline`: $\max(\text{now}, \text{currentDeadline}) + \text{duration}$ | - Implemented in `extendDeadline` in [callToPlay.ts](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-tauri-deno-ts/src/lib/callToPlay.ts#L12) | **Faithful** | | [`2c204ac`](file:///pantheon/pfs/git/rust/pfs/lanspread/crates/lanspread-tauri-deno-ts/src/hooks/useCallToPlay.ts) | Startup & Error Guidance | - Replace missing folder prompt with LAN connecting state
- Map specific store errors (`Obsolete`, `NeedHistory`, `HistoryFull`) to human guidance | - Implemented in `useCallToPlay.ts` and `callToPlayPublishErrorMessage` | **Faithful** | ### Implementation Refinements Over the Initial Plan 1. **Tombstone Representation**: Rather than instantiating a separate tombstone data structure, `compact_history` retains **only** the `Start` or `Cancel` event (`event.id == terminal.event_id`) after the 15-minute terminal retention window expires. `terminal_tombstone_call_ids` uses unrooted terminal events to reject any incoming obsolete history. This is cleaner and more memory-efficient than allocating explicit tombstone markers. 2. **Unresolved History Cap (`unresolved_event_count`)**: The active event cap ($4,096$) is enforced strictly against *unresolved* calls (`Create` without `Start`/`Cancel`). As soon as a creator emits `Start` or `Cancel`, that call's events no longer count against the active cap. This guarantees a user can always settle (Start or Cancel) an open call even when the store is full. --- ## 2. Holistic Architecture Review ``` +------------------------+ | Frontend (TS/Tauri) | | Event Reducer & Hooks | +-----------+------------+ | publish_call_to_play / snapshot v +------------------------+ | CallToPlayStore | | (Atomic Batch Merge) | +-----------+------------+ | +-------------------+-------------------+ | (Local immediate) | (Async broadcast) v v +-------------------+ +-------------------+ | Local UI Emitter | | Peer QUIC Stream | +-------------------+ | (Protocol Ver 7) | +---------+---------+ | CallToPlayEvents v +-------------------+ | CallToPlayAck | | (Applied/NeedHist)| +-------------------+ ``` ### Architectural Soundness 1. **Event-Sourced LAN Replication vs Server-Authoritative State**: Maintaining an event-sourced replication model with deterministic reduction is optimal for LAN Spread. Decentralized peer-to-peer LAN parties lack guaranteed central servers. Using atomic batch merges ($O(N)$ compaction) completely eliminates the $O(N^2)$ quadratic slowdown of the previous per-event insertion model. 2. **Network Identity Model**: Removing source-IP equality comparisons fixes a major real-world bug on multi-homed hardware (Ethernet + Wi-Fi / VPN / virtual bridges). Validating that `envelope peer_id` is present in the mDNS peer roster and verifying `event.actor_id == envelope peer_id` accurately matches the trusted-LAN threat model without making false cryptographic guarantees. 3. **Asynchronous Healing**: Local updates succeed instantly for the local user without blocking on network delivery (`task_tracker.spawn(...)`). If a remote peer rejects an event with `NeedHistory` or `NeedHandshake`, an asynchronous full `Hello`/`HelloAck` resync is scheduled. This isolates local UI responsiveness from network transport delays. 4. **Lifecycle & Memory Management**: The 3-tier lifecycle (`Open` $\rightarrow$ `Time's up` [5-min recovery] $\rightarrow$ `Running`/`Cancelled` [15-min display] $\rightarrow$ `Tombstone`) strikes the right balance between retaining full chat/roster history for late joiners and preventing unbounded memory growth. --- ## 3. User Experience (UX) Analysis ``` UX Flow Comparison (Add Time Action) BEFORE: [10-min Call] -- (Filled at min 2) --> Click "+5 mins" --> Deadline set to (2+5) = 7 mins! (SHORTENED!) AFTER: [10-min Call] -- (Filled at min 2) --> Click "+5 mins" --> Deadline set to max(2, 10)+5 = 15 mins! (EXTENDED!) ``` 1. **Intuitive "+5 minutes" Extension**: - *Previous behavior*: Setting deadline to `now + 5` inadvertently shortened calls that reached capacity early. - *Current behavior*: `Math.max(now, currentDeadline) + 5` preserves existing remaining time when extending early, and correctly grants 5 new minutes to an overdue call. 2. **Startup & Connection Guidance**: - *Previous behavior*: Attempting an action during startup raised misleading errors about missing game folders. - *Current behavior*: Shows `"Call to Play is still connecting to the LAN. Try again in a moment."` while `actorId` is initializing, clearing automatically upon connection. 3. **Clear Terminal Receipts (`Running` and `Cancelled`)**: - *Previous behavior*: Starting or canceling a call caused it to disappear or act erratically, hiding game chat. - *Current behavior*: `Running` displays as a clear green success receipt card, and `Cancelled` displays as a read-only historical card. Roster and chat remain accessible for 15 minutes, sorted below active calls and excluded from badge counts. --- ## Conclusion & Recommendation The commits are **clean, robust, and fully faithful to the findings and plan**. The architectural choices are sound for a LAN environment, and the UX is intuitive and frictionless. No further code changes are needed; the implementation is ready for merge.