Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a phaseChangedAt timestamp to RoundState and updates game logic to set it at phase transitions. Introduces a frontend PhaseTimer UI (component, hook, and CSS) and wires it into Arena/App to display phase progress and indeterminate states. Changes
Sequence Diagram(s)sequenceDiagram
participant GameServer
participant Broadcast
participant Frontend
Note over GameServer,Broadcast: Phase transition occurs
GameServer->>GameServer: set round.phase = "answering"/"voting"/"done"
GameServer->>GameServer: set round.phaseChangedAt = now
GameServer->>Broadcast: broadcast updated RoundState
Broadcast->>Frontend: push RoundState (includes phaseChangedAt)
Frontend->>Frontend: PhaseTimer.useElapsed(phaseChangedAt) -> render progress/label
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 |
Add a phase timer bar across arena views to show a 5,000 ms done-phase countdown and indeterminate progress for prompting, answering, voting, and next-prompt loadingIntroduce 📍Where to StartStart with Macroscope summarized fd53118. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
game.ts (1)
338-349:⚠️ Potential issue | 🟡 MinorMissing
phaseChangedAtupdate on the error path to"done".When prompt generation fails,
round.phaseis set to"done"on Line 344 without updatinground.phaseChangedAt. This leaves the timestamp pointing at the start of the "prompting" phase. If the frontend briefly renders this round in the "done" state, thePhaseTimercountdown would use a stalephaseChangedAt, producing an incorrect (likely already-expired) timer display.In practice, the round is immediately archived and
state.activenulled, so visibility is brief—but for consistency:Proposed fix
round.promptTask.error = "Failed after 3 attempts"; + round.phaseChangedAt = Date.now(); round.phase = "done";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@game.ts` around lines 338 - 349, When handling the failed prompt generation in the catch block you set round.phase = "done" but forget to update round.phaseChangedAt, causing the UI PhaseTimer to read a stale timestamp; fix this by assigning round.phaseChangedAt = Date.now() immediately when you set round.phase = "done" (before adding the round to state.completed and nulling state.active), keeping the existing assignments to round.promptTask.finishedAt, round.promptTask.error, state.completed, state.active, and the rerender() call.
🧹 Nitpick comments (2)
frontend.tsx (2)
113-188:PhaseTimer— two minor observations.
remainingon Line 146 is computed fromDate.now()during render rather than derived from theprogressstate. This works becausesetProgresstriggers frequent re-renders, but it means the label text and bar width can disagree by up to 50 ms. Consider deriving both fromprogress:- const remaining = Math.max(0, Math.ceil((DONE_PHASE_DURATION_MS - (Date.now() - round.phaseChangedAt)) / 1000)); + const remaining = Math.max(0, Math.ceil(((1 - progress) * DONE_PHASE_DURATION_MS) / 1000));
- The 50 ms interval keeps ticking after
progressreaches 1. This is harmless since the round transitions away shortly, but you could short-circuit:if (pct >= 1) { clearInterval(id); }Neither is urgent—more of a polish item.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 113 - 188, PhaseTimer: derive the remaining seconds from the progress state instead of recomputing Date.now() in render and stop the interval once progress reaches 1; specifically, in the useEffect that defines tick (used to setProgress) compute pct, call setProgress(pct), and if pct >= 1 setProgress(1) and clearInterval(id) so the interval stops, then replace the remaining calculation (the remaining constant used in the "done" branch) to compute remaining from progress and DONE_PHASE_DURATION_MS (e.g. remaining = Math.max(0, Math.ceil((1 - progress) * DONE_PHASE_DURATION_MS / 1000))) so the label and bar width always agree.
145-146: Clock-skew consideration with server-generated timestamps.
round.phaseChangedAtis set viaDate.now()on the server, but compared againstDate.now()on the client. If the server and client clocks differ by more than a second or so, the countdown will appear skewed (e.g., starting at 6 s or 4 s instead of 5 s). This is inherent to the current approach and probably fine for a game UI, but worth noting.If precision becomes important later, you could send a relative offset or do a lightweight clock-sync handshake over the WebSocket.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 145 - 146, The client is comparing its Date.now() to the server-set round.phaseChangedAt, causing visible skew when clocks differ; update the calculation in the place that computes remaining (the expression using round.phaseChangedAt and DONE_PHASE_DURATION_MS) to apply a clock offset derived at connection time (or use a server-provided remaining duration). Concretely, compute and store clientServerOffset = Date.now() - serverNowAtHandshake (or accept serverRemainingSeconds on the round object), then change the remaining calculation to use Date.now() - clientServerOffset (or use serverRemainingSeconds) so countdowns align with the server timestamp while keeping DONE_PHASE_DURATION_MS and round.phaseChangedAt as the authoritative values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@game.ts`:
- Around line 338-349: When handling the failed prompt generation in the catch
block you set round.phase = "done" but forget to update round.phaseChangedAt,
causing the UI PhaseTimer to read a stale timestamp; fix this by assigning
round.phaseChangedAt = Date.now() immediately when you set round.phase = "done"
(before adding the round to state.completed and nulling state.active), keeping
the existing assignments to round.promptTask.finishedAt, round.promptTask.error,
state.completed, state.active, and the rerender() call.
---
Nitpick comments:
In `@frontend.tsx`:
- Around line 113-188: PhaseTimer: derive the remaining seconds from the
progress state instead of recomputing Date.now() in render and stop the interval
once progress reaches 1; specifically, in the useEffect that defines tick (used
to setProgress) compute pct, call setProgress(pct), and if pct >= 1
setProgress(1) and clearInterval(id) so the interval stops, then replace the
remaining calculation (the remaining constant used in the "done" branch) to
compute remaining from progress and DONE_PHASE_DURATION_MS (e.g. remaining =
Math.max(0, Math.ceil((1 - progress) * DONE_PHASE_DURATION_MS / 1000))) so the
label and bar width always agree.
- Around line 145-146: The client is comparing its Date.now() to the server-set
round.phaseChangedAt, causing visible skew when clocks differ; update the
calculation in the place that computes remaining (the expression using
round.phaseChangedAt and DONE_PHASE_DURATION_MS) to apply a clock offset derived
at connection time (or use a server-provided remaining duration). Concretely,
compute and store clientServerOffset = Date.now() - serverNowAtHandshake (or
accept serverRemainingSeconds on the round object), then change the remaining
calculation to use Date.now() - clientServerOffset (or use
serverRemainingSeconds) so countdowns align with the server timestamp while
keeping DONE_PHASE_DURATION_MS and round.phaseChangedAt as the authoritative
values.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
game.ts (1)
342-353:⚠️ Potential issue | 🟡 Minor
phaseChangedAtnot updated when prompt fails and transitions directly to"done".The early error exit sets
round.phase = "done"(line 348) but leavesphaseChangedAtat the round-creation timestamp. While the frontend'sisShowingPreviousguard prevents any visible countdown mis-rendering in the current flow, the field is semantically incorrect for thatlastCompletedsnapshot.🛡️ Proposed fix
round.promptTask.finishedAt = Date.now(); round.promptTask.error = "Failed after 3 attempts"; round.phase = "done"; + round.phaseChangedAt = Date.now(); state.completed = [...state.completed, round];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@game.ts` around lines 342 - 353, When the prompt failure branch sets round.phase = "done" it doesn't update the timestamp field that records when the phase changed; update round.phaseChangedAt = Date.now() in the same catch block (alongside setting round.promptTask.finishedAt and round.promptTask.error) so the lastCompleted snapshot has the correct phase change time before pushing the round into state.completed and nulling state.active (affecting the same catch handling that calls rerender()).
🧹 Nitpick comments (1)
frontend.tsx (1)
102-102:DONE_PHASE_DURATION_MSis duplicated with the server-side5000ms delay ingame.ts.
game.tsline 481 hardcodes the same5000ms pause:await new Promise((r) => setTimeout(r, 5000));If either value is changed independently, the countdown bar will run under or over the actual transition. Consider extracting a shared constant (e.g. in a
constants.tsmodule) imported by bothgame.tsandfrontend.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` at line 102, The DONE_PHASE_DURATION_MS constant is duplicated; extract a single shared constant by creating a constants module (e.g. export const DONE_PHASE_DURATION_MS = 5000 in constants.ts) and update both frontend.tsx (replace its local DONE_PHASE_DURATION_MS) and game.ts (replace the hardcoded setTimeout delay 5000) to import and use that exported symbol; ensure both files import { DONE_PHASE_DURATION_MS } from the new module and remove the duplicate local values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend.tsx`:
- Around line 104-114: The useElapsed hook can compute a negative value when
Date.now() is less than startTime; update the tick calculation in useElapsed to
clamp the computed difference to zero before converting to seconds (e.g.,
compute diff = Math.max(0, Date.now() - startTime) and then
setElapsed(Math.floor(diff / 1000))). Ensure the initial setElapsed and the tick
both apply the clamp so the hook never returns negative seconds.
---
Outside diff comments:
In `@game.ts`:
- Around line 342-353: When the prompt failure branch sets round.phase = "done"
it doesn't update the timestamp field that records when the phase changed;
update round.phaseChangedAt = Date.now() in the same catch block (alongside
setting round.promptTask.finishedAt and round.promptTask.error) so the
lastCompleted snapshot has the correct phase change time before pushing the
round into state.completed and nulling state.active (affecting the same catch
handling that calls rerender()).
---
Nitpick comments:
In `@frontend.tsx`:
- Line 102: The DONE_PHASE_DURATION_MS constant is duplicated; extract a single
shared constant by creating a constants module (e.g. export const
DONE_PHASE_DURATION_MS = 5000 in constants.ts) and update both frontend.tsx
(replace its local DONE_PHASE_DURATION_MS) and game.ts (replace the hardcoded
setTimeout delay 5000) to import and use that exported symbol; ensure both files
import { DONE_PHASE_DURATION_MS } from the new module and remove the duplicate
local values.
1 participant
Timer at the top explaining what is happening
Summary by CodeRabbit