refactor(arena): split mechanic events from control messages and genericize socket hook#13
Conversation
…ricize socket hook
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThe changes refactor the arena protocol and client-server messaging architecture. They introduce a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@greptile review |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Additional Comments (2)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 146:148
Comment:
**Stale props cause runtime error**
`ArenaActive` no longer defines/receives `elements`, `cursors`, `sendElements`, `sendCursor`, `codeByParticipant`, `testResultsByParticipant`, or `typingProgress`, but the JSX still references them (e.g. `elements={elements}` / `cursors={cursors}`), which will throw at runtime / fail type-checking. This component needs to be updated to derive these from `state.data` + `handleArenaEvent` + `sendAction`, or restore the removed state/handlers so the props passed to `DrawArena`/`CodeArena`/`TypingArena` are defined.
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 50:52
Comment:
**`onGameOver` signature mismatch**
`useArenaSocket` now calls `onGameOver?.(reason, results)` (`use-arena-socket.ts`), but `ArenaActive` still defines `handleGameOver = (reason: string) => ...`, so the hook is being given a callback with the wrong arity/types. Update `handleGameOver` to `(reason: ArenaEndReason, results: ArenaResults) => void` (or adjust the hook API), otherwise this will fail type-checking and the results payload is silently ignored.
How can I resolve this? If you propose a fix, please make it concise. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/routes/arena/-components/arena-active.tsx`:
- Around line 56-68: Destructure data from the useArenaSocket call (const {
data, connectionState, participants, timeRemaining, error, sendAction } =
useArenaSocket(...)) and import ArenaData, DrawData, CodeData, TypingData from
~/shared/arena-protocol; then use a type guard on data?.type to narrow to
DrawData/CodeData/TypingData and derive the arena-specific props (for draw:
elements, cursors, sendElements/sendCursor wrappers that call sendAction with
the draw action shape; for code: codeByParticipant, testResultsByParticipant and
a sendCode/sendTest wrapper; for typing: typingProgress and sendTyping wrapper).
Keep handleArenaEvent and sendAction usage but wrap sendAction into per-arena
helper functions to translate UI payloads into the server action format and
ensure all referenced variables (elements, cursors, sendElements, sendCursor,
codeByParticipant, testResultsByParticipant, typingProgress) are defined based
on the narrowed data.
🧹 Nitpick comments (1)
src/routes/arena/-hooks/use-arena-socket.ts (1)
79-131: Add a lightweight type guard before switching on parsed messages.
Right nowJSON.parseis asserted asServerMsg<T>without validation. A small guard avoids malformed payloads triggering the default branch.♻️ Suggested guard
+ const isServerMsg = (value: unknown): value is ServerMsg<T> => { + if (!value || typeof value !== "object") return false + return typeof (value as { type?: unknown }).type === "string" + } + onMessage: (event) => { - const message = JSON.parse(event.data) as ServerMsg<T> + const parsed: unknown = JSON.parse(event.data) + if (!isServerMsg(parsed)) { + setState((prev) => ({ ...prev, error: "Invalid server message" })) + return + } + const message = parsedAs per coding guidelines: Make use of function guards whenever possible in TypeScript.
| const handleArenaEvent = useCallback((event: ArenaEvent<ArenaType>) => { | ||
| console.log("Arena event:", event) | ||
| }, []) | ||
|
|
||
| const { connectionState, participants, timeRemaining, error, sendAction } = | ||
| useArenaSocket({ | ||
| arenaId, | ||
| userId: user?._id ?? "", | ||
| username: user?.username ?? "Anonymous", | ||
| config, | ||
| onGameOver: handleGameOver, | ||
| onArenaEvent: handleArenaEvent | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the arena-active.tsx file
find . -name "arena-active.tsx" -type fRepository: muzzlol/spectra
Length of output: 106
🏁 Script executed:
# Check the structure of the arena directory
fd "arena" -type d | head -20Repository: muzzlol/spectra
Length of output: 229
🏁 Script executed:
# Get the full content of arena-active.tsx to see the actual code
cat -n src/routes/arena/-components/arena-active.tsx | head -200Repository: muzzlol/spectra
Length of output: 7110
🏁 Script executed:
# Search for the useArenaSocket hook definition
fd "useArenaSocket" --type f
rg "useArenaSocket" --type tsx --type ts -lRepository: muzzlol/spectra
Length of output: 86
🏁 Script executed:
# Search for child component definitions
fd -e tsx -e ts | xargs grep -l "DrawArena\|CodeArena\|TypingArena" | head -10Repository: muzzlol/spectra
Length of output: 205
🏁 Script executed:
cat -n src/routes/arena/-hooks/use-arena-socket.ts 2>/dev/null || cat -n src/routes/arena/-hooks/use-arena-socket.tsxRepository: muzzlol/spectra
Length of output: 4692
🏁 Script executed:
# Search for useArenaSocket implementation
rg "export.*useArenaSocket|function useArenaSocket|const useArenaSocket" -A 50Repository: muzzlol/spectra
Length of output: 3605
🏁 Script executed:
# Check child component signatures - DrawArena
cat -n src/routes/arena/-draw/arena.tsx | head -50Repository: muzzlol/spectra
Length of output: 1905
🏁 Script executed:
# Check child component signatures - CodeArena
cat -n src/routes/arena/-code/arena.tsx | head -50Repository: muzzlol/spectra
Length of output: 1976
🏁 Script executed:
# Check child component signatures - TypingArena
cat -n src/routes/arena/-typing/arena.tsx | head -50Repository: muzzlol/spectra
Length of output: 2043
🏁 Script executed:
# Search for arena-protocol to understand data types
find . -name "*arena-protocol*" -o -name "*arena*protocol*" | head -5Repository: muzzlol/spectra
Length of output: 85
🏁 Script executed:
cat -n ./shared/arena-protocol.ts | head -200Repository: muzzlol/spectra
Length of output: 6355
Wire data into hook destructuring and derive arena props from it (current code won't compile).
useArenaSocket returns data: ArenaData<T> | null, but line 60–68 doesn't destructure it. The JSX below references undefined variables (elements, cursors, sendElements, sendCursor, codeByParticipant, testResultsByParticipant, typingProgress) that should be derived from data and sendAction. This is a build blocker.
Extract data from the hook, use type guards to narrow the data type based on type, then derive per-arena props and wrap sendAction for each arena's action format.
Fix pattern using type guards (adjust action payloads to match your server protocol)
- const { connectionState, participants, timeRemaining, error, sendAction } =
+ const { connectionState, participants, timeRemaining, error, data, sendAction } =
useArenaSocket({
arenaId,
userId: user?._id ?? "",
username: user?.username ?? "Anonymous",
config,
onGameOver: handleGameOver,
onArenaEvent: handleArenaEvent
})
+ const isArenaData = <K extends ArenaType>(
+ arenaType: ArenaType,
+ arenaData: ArenaData<ArenaType> | null,
+ expected: K
+ ): arenaData is ArenaData<K> => arenaType === expected && arenaData !== null
+
+ const drawElements = isArenaData(type, data, "draw") ? (data as DrawData).playerElements[user?._id ?? ""] ?? [] : []
+ const drawCursors = isArenaData(type, data, "draw") ? (data as DrawData).playerCursors ?? {} : {}
+ const codeByParticipant = isArenaData(type, data, "code") ? (data as CodeData).playerCode ?? {} : {}
+ const testResultsByParticipant = isArenaData(type, data, "code") ? (data as CodeData).testResults ?? {} : {}
+ const codeCursors = isArenaData(type, data, "code") ? (data as CodeData).playerCursors ?? {} : {}
+ const typingProgress = isArenaData(type, data, "typing") ? (data as TypingData).progress ?? {} : {}
+
+ const sendElements = useCallback(
+ (elements: DrawData["playerElements"][string]) => {
+ sendAction({ type: "canvas_update", elements } as ClientAction<"draw">)
+ },
+ [sendAction]
+ )
+
+ const sendCursor = useCallback(
+ (x: number, y: number) => {
+ sendAction({ type: "cursor", x, y } as ClientAction<ArenaType>)
+ },
+ [sendAction]
+ )
return (
<div className="flex h-screen flex-col">
...
{type === "draw" && (
<DrawArena
userId={user._id}
participants={participants}
isSpectator={isSpectator}
- elements={elements}
- cursors={cursors}
+ elements={drawElements}
+ cursors={drawCursors}
onElementsChange={sendElements}
onCursorMove={sendCursor}
prompt={prompt}
/>
)}
{type === "code" && (
<CodeArena
userId={user._id}
participants={participants}
isSpectator={isSpectator}
codeByParticipant={codeByParticipant}
testResultsByParticipant={testResultsByParticipant}
- cursors={cursors}
+ cursors={codeCursors}
onCodeChange={(code) => {
- console.log("Code changed:", code.length, "chars")
+ sendAction({ type: "code_update", code } as ClientAction<"code">)
}}
onCursorMove={(line, col) => {
sendCursor(line, col)
}}
prompt={prompt}
/>
)}
{type === "typing" && (
<TypingArena
userId={user._id}
participants={participants}
isSpectator={isSpectator}
prompt={prompt}
progressByParticipant={typingProgress}
onProgressUpdate={(progress) => {
- console.log("Progress update:", progress)
+ sendAction({ type: "progress", progress } as ClientAction<"typing">)
}}
/>
)}Import types from ~/shared/arena-protocol: ArenaData, DrawData, CodeData, TypingData.
🤖 Prompt for AI Agents
In `@src/routes/arena/-components/arena-active.tsx` around lines 56 - 68,
Destructure data from the useArenaSocket call (const { data, connectionState,
participants, timeRemaining, error, sendAction } = useArenaSocket(...)) and
import ArenaData, DrawData, CodeData, TypingData from ~/shared/arena-protocol;
then use a type guard on data?.type to narrow to DrawData/CodeData/TypingData
and derive the arena-specific props (for draw: elements, cursors,
sendElements/sendCursor wrappers that call sendAction with the draw action
shape; for code: codeByParticipant, testResultsByParticipant and a
sendCode/sendTest wrapper; for typing: typingProgress and sendTyping wrapper).
Keep handleArenaEvent and sendAction usage but wrap sendAction into per-arena
helper functions to translate UI payloads into the server action format and
ensure all referenced variables (elements, cursors, sendElements, sendCursor,
codeByParticipant, testResultsByParticipant, typingProgress) are defined based
on the narrowed data.
|
@greptile it's clearly stated that arena-active is a WIP, can you review the changes we did make? |
Additional Comments (2)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 52:55
Comment:
**Type mismatch for onGameOver**
`useArenaSocket` now calls `onGameOver` with `(reason: ArenaEndReason, results: ArenaResults)`, but `handleGameOver` is still typed as `(reason: string)`. This will be a TS error and will also drop the `results` payload at runtime.
```suggestion
const handleGameOver = useCallback((reason: ArenaEndReason, results: ArenaResults) => {
console.log("Game over:", reason, results)
}, [])
```
How can I resolve this? If you propose a fix, please make it concise.
Fix by either (a) reintroducing these fields in the hook, or (b) update Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 146:156
Comment:
**Broken props after refactor**
This component still references `elements`, `cursors`, `sendElements`, and `sendCursor`, but the new `useArenaSocket` return type no longer provides them (it returns `data` and `sendAction`). As-is, this won’t compile.
Fix by either (a) reintroducing these fields in the hook, or (b) update `ArenaActive` to derive `elements/cursors` from `state.data` and send actions via `sendAction` (e.g. `sendAction({ type: "cursor", x, y })`).
How can I resolve this? If you propose a fix, please make it concise. |
|
@greptile re-review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| default: { | ||
| const _exhaustive: never = message | ||
| throw new Error(`Unknown message type: ${_exhaustive}`) | ||
| } |
There was a problem hiding this comment.
Throwing on unknown message
JSON.parse(event.data) is a network payload, but it’s cast to ServerMsg<T> and the default switch branch throws. If the server ever sends an unexpected/extra message type (e.g., protocol mismatch during rollout), this will throw from the websocket handler and can break the arena UI.
Consider ignoring/logging unknown messages instead of throwing, or explicitly handle all message variants you actually send.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/routes/arena/-hooks/use-arena-socket.ts
Line: 128:131
Comment:
**Throwing on unknown message**
`JSON.parse(event.data)` is a network payload, but it’s cast to `ServerMsg<T>` and the `default` switch branch throws. If the server ever sends an unexpected/extra message type (e.g., protocol mismatch during rollout), this will throw from the websocket handler and can break the arena UI.
Consider ignoring/logging unknown messages instead of throwing, or explicitly handle all message variants you actually send.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
You’ll need to finish the migration here (derive draw/code/typing props from Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 146:156
Comment:
**Stale socket API references**
This component still uses `elements`, `cursors`, `sendElements`, and `sendCursor`, but the updated `useArenaSocket` return value no longer provides these (it returns `data` + `sendAction`). As written, this should fail typecheck/build.
You’ll need to finish the migration here (derive draw/code/typing props from `data` and/or wire `sendAction`), or revert these usages until the new API is fully adopted.
How can I resolve this? If you propose a fix, please make it concise. |
|
@CodeRabbit |
|
Key Changes Review1. arenas/server.ts - Mechanic Event Broadcasting ✅The introduction of
Observation: The method signature uses 2. shared/arena-protocol.ts - Protocol Refactoring ✅The type system improvements are solid:
Question: I notice 3. use-arena-socket.ts - Hook Genericization 🎯The genericization is well-executed:
Suggestions:
Minor: The Overall AssessmentThis is a solid refactoring that improves:
The changes are well-structured and should make adding new arena types easier going forward. ✅ Actions performedReview triggered.
|
Additional Comments (7)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 52:52
Comment:
`reason` param should be `ArenaEndReason`, not `string`
```suggestion
const handleGameOver = useCallback((reason: ArenaEndReason, results: ArenaResults) => {
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 153:153
Comment:
`elements` is not defined - was removed in refactor but still referenced here
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 154:154
Comment:
`cursors` is not defined - was removed in refactor but still referenced here
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 155:156
Comment:
`sendElements` and `sendCursor` no longer exist - need to use `sendAction` instead
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 166:168
Comment:
`codeByParticipant`, `testResultsByParticipant`, and `cursors` are undefined - removed in refactor but still referenced
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 175:176
Comment:
`sendCursor` no longer exists
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/routes/arena/-components/arena-active.tsx
Line: 187:187
Comment:
`typingProgress` is not defined - was removed in refactor but still referenced here
How can I resolve this? If you propose a fix, please make it concise. |
1 participant
add typed mechanic envelope to ServerMsg (ArenaEvent)
broadcast draw updates via broadcastMechanic on the DO server
refactor useArenaSocket to generic arena data/state and typed arena_over handling
replace mode-specific senders with sendAction(action: ClientAction)
begin adapting arena-active to new hook/event API (WIP)
Summary by CodeRabbit
Greptile Overview
Greptile Summary
This PR refactors the arena event system by introducing a typed
ArenaEvent<T>envelope withinServerMsg, replacing the previous flat event union. The server now usesbroadcastMechanic()to wrap mechanic events, and the client hookuseArenaSocketwas genericized to handleArenaData<T>with a unifiedsendAction(action: ClientAction<T>)API.Key Changes:
{ type: "mechanic", event: ArenaEvent<T> }toServerMsg<T>sendElements,sendCursor) with genericsendActiononArenaEventcallbackonGameOversignature to includeArenaResultsCritical Issues:
arena-active.tsxhas multiple runtime errors from incomplete refactoring - references undefined variables (elements,cursors,codeByParticipant,testResultsByParticipant,typingProgress) and removed functions (sendElements,sendCursor)handleGameOvercallback (expectsstringinstead ofArenaEndReason)The protocol and hook refactoring is clean and well-typed, but the integration layer is broken and will cause immediate runtime failures.
Confidence Score: 1/5
arena-active.tsxhas 7+ undefined variable references and incorrect function signatures that will cause immediate runtime failures. The file is marked WIP but these errors prevent the code from running at all.src/routes/arena/-components/arena-active.tsx- it has multiple critical errors that must be fixed before mergingImportant Files Changed
broadcastMechanichelper to wrap mechanic events in typed envelope - clean implementation with proper generic typingArenaEvent<T>fromServerMsg<T>and renamedServerEventtoArenaEventfor clarityArenaData<T>, replaced mode-specific senders with unifiedsendAction, addedonArenaEventcallback - well-typed and cleanelements,cursors,codeByParticipant, etc.) and wrong function signaturesSequence Diagram
sequenceDiagram participant Client as Client (React) participant Hook as useArenaSocket participant WS as WebSocket participant DO as Durable Object participant Storage as DO Storage Note over Client,DO: Connection & Initialization Client->>Hook: useArenaSocket({ config, onArenaEvent }) Hook->>WS: connect(arenaId) WS->>DO: WebSocket Upgrade DO->>Storage: load state Hook->>WS: ClientMsg { type: "init", userId, username, config } DO->>Storage: save config & initialize ArenaData<T> DO->>DO: start alarm (tick loop) DO->>WS: ServerMsg { type: "state", participants, data, timeRemaining } WS->>Hook: state message Hook->>Client: return { participants, data, timeRemaining, sendAction } Note over Client,DO: Game Action Flow Client->>Hook: sendAction({ type: "cursor", x, y }) Hook->>WS: ClientAction<T> DO->>DO: handleDraw/handleCode/handleTyping DO->>Storage: update ArenaData<T> DO->>DO: broadcastMechanic(event, ws) DO->>WS: ServerMsg { type: "mechanic", event: ArenaEvent<T> } WS->>Hook: mechanic message Hook->>Client: onArenaEvent(event) Client->>Client: update UI based on event Note over Client,DO: Tick Updates DO->>DO: alarm() fires every 1s DO->>WS: ServerMsg { type: "tick", timeRemaining } WS->>Hook: tick message Hook->>Client: update timeRemaining Note over Client,DO: Game End DO->>DO: alarm() detects timeRemaining <= 0 DO->>Storage: finalize & save results DO->>WS: ServerMsg { type: "arena_over", reason, results } WS->>Hook: arena_over message Hook->>Client: onGameOver(reason, results) DO->>WS: close all connections