refactor: Update UI components and enhance game logic with improved match handling and user experience features#22
Merged
Merged
Conversation
…atch handling and user experience features
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughRefactors the realtime possession match UI into a controller + presentational components, adds support for non-multiple-choice question kinds (countdown, put-in-order, clues), updates socket types/handlers and store state for multi-kind questions and acks, introduces multiple new possession hooks/tests, and adjusts various auth, dev, and layout UI pieces. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant C as RealtimeMatchController
participant S as Socket (Server)
participant Store as Realtime Store
participant View as Presentational Views
rect rgba(240,248,255,0.5)
UI->>C: mount (props, matchId)
C->>Store: read match / inventory / session
C->>S: subscribe socket events (match:*, acks)
end
rect rgba(224,255,224,0.5)
S->>C: match:question / roundResult / ack events
C->>Store: update state (setQuestion, setAnswerAck, setRoundResult)
C->>C: compute viewportModel & questionAreaModel (hooks)
C->>View: provide models & callbacks (toggleMute, useChanceCard, quit/restart)
end
rect rgba(255,240,245,0.5)
View->>UI: render pitch/hud/question panels
UI->>C: user actions (answer, chance card, quit)
C->>S: emit client events (match:answer, match:chance_card_use, match:leave)
S->>C: ack events (countdown_guess_ack, clues_guess_ack, session:state)
C->>Store: ack handling (setCountdownGuessAck / setCluesGuessAck)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (6)
src/features/play/ModeSelectionScreen.tsx (1)
174-181: Bring dev CTA button styles back to the feature design system.Line 174 and Line 181 use
rounded-xl, omitborder-b-4, and use non-standard palette colors.As per coding guidelines `src/{components,features}/**/*.{ts,tsx}: Use chunky 3D borders with border-b-4 border-[darker shade] on cards, buttons, and badges`, `Use rounded-2xl or rounded-3xl for rounded corners on game elements`, and `src/{components,features,app}/**/*.{ts,tsx}: Use Duolingo-inspired colors: Background `#131F24`, Cards `#1B2F36`, Blue `#1CB0F6`, Green `#58CC02`, Red `#FF4B4B`, Orange `#FF9600`, Purple `#CE82FF`, Muted text `#56707A``.🎨 Suggested class updates
- className="inline-flex items-center justify-center rounded-xl bg-black/90 px-3 py-2 text-[10px] font-black uppercase tracking-[0.18em] text-white transition-colors hover:bg-black" + className="inline-flex items-center justify-center rounded-2xl border-b-4 border-[`#131F24`] bg-[`#1B2F36`] px-3 py-2 text-[10px] font-black uppercase tracking-[0.18em] text-white transition-colors hover:bg-[`#1B2F36`]/90" - className="inline-flex items-center justify-center rounded-xl border border-[`#0F3A00`]/20 bg-[`#E6F8C9`] px-3 py-2 text-[10px] font-black uppercase tracking-[0.18em] text-[`#0F3A00`] transition-colors hover:bg-[`#DCF4B6`]" + className="inline-flex items-center justify-center rounded-2xl border-b-4 border-[`#131F24`] bg-[`#58CC02`] px-3 py-2 text-[10px] font-black uppercase tracking-[0.18em] text-[`#131F24`] transition-colors hover:bg-[`#58CC02`]/90"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/play/ModeSelectionScreen.tsx` around lines 174 - 181, The two dev CTA Link elements (the one rendering "Dev Quick Ranked" and the one with href="/dev/mock-match") use non-standard rounded-xl, missing chunky 3D border and non-approved palette; update their className to use rounded-2xl or rounded-3xl, add a 3D border with border-b-4 plus a darker border color (e.g., border-b-4 border-[`#0F3A00`] -> border-b-4 border-[darker-shade]), and replace custom blacks/greens with the design system colors (Cards `#1B2F36` or Green `#58CC02` for green CTAs, Muted text `#56707A` for text) while preserving existing spacing and hover classes so the buttons match the feature design system.src/components/layout/AppShell.tsx (1)
189-211: Verify navigation behavior when activeMatchBanner changes mid-handler.The handler reads
activeMatchBannerat invocation time, which could theoretically change between the null check and the socket emit if a new event arrives. In practice this is unlikely to cause issues since:
- The handler executes synchronously
- React batches state updates
However, consider capturing the banner reference at the start of the handler to ensure consistency throughout execution.
♻️ Optional: Capture banner reference for consistency
const handleRejoinMatch = () => { - if (!activeMatchBanner) return; + const banner = activeMatchBanner; + if (!banner) return; - const matchId = activeMatchBanner.matchId; + const matchId = banner.matchId; startSession({ - mode: activeMatchBanner.mode === "ranked" ? "ranked" : "quizball", - matchType: activeMatchBanner.mode === "ranked" ? "ranked" : "friendly", + mode: banner.mode === "ranked" ? "ranked" : "quizball", + matchType: banner.mode === "ranked" ? "ranked" : "friendly", questionCount: 10, - opponentId: activeMatchBanner.opponent.id, - opponentUsername: activeMatchBanner.opponent.username, - opponentAvatar: activeMatchBanner.opponent.avatarUrl ?? undefined, + opponentId: banner.opponent.id, + opponentUsername: banner.opponent.username, + opponentAvatar: banner.opponent.avatarUrl ?? undefined, }); setGameStage("playing"); - if (activeMatchBanner.source === "rejoin") { + if (banner.source === "rejoin") { getSocket().emit("match:rejoin", { matchId }); clearRejoinAvailable(); } router.push("/game"); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/AppShell.tsx` around lines 189 - 211, Capture activeMatchBanner into a local constant at the start of handleRejoinMatch and use that constant for all subsequent reads (matchId, mode, matchType, opponent fields, and source) so the handler uses a consistent banner snapshot even if state changes during execution; update references in startSession, the getSocket().emit("match:rejoin", { matchId }) check, clearRejoinAvailable(), and any other usages inside handleRejoinMatch to use the captured local variable instead of reading activeMatchBanner repeatedly.src/features/possession/hooks/usePossessionGoalCelebration.ts (1)
64-68: Consider simplifying the null/undefined check.Since
currentQuestionIndexis typed asnumber | null, theundefinedcheck is redundant:useEffect(() => { - if (currentQuestionIndex !== null && currentQuestionIndex !== undefined) { + if (currentQuestionIndex !== null) { pendingGoalRef.current = null; } }, [currentQuestionIndex]);This is a minor nit - the current code is functionally correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/possession/hooks/usePossessionGoalCelebration.ts` around lines 64 - 68, The conditional in the useEffect inside usePossessionGoalCelebration redundantly checks for undefined even though currentQuestionIndex is typed number | null; simplify the check to just `currentQuestionIndex !== null` (or `currentQuestionIndex == null` inverted as needed) and keep the body setting `pendingGoalRef.current = null` unchanged so the effect still runs only when there is a non-null index.src/features/possession/RealtimePossessionMatchScreen.tsx (1)
41-41: Use the feature background token here.These new wrappers still use
#0f1420instead of the repo’s standard background#131F24, so this screen will look off relative to the rest of the game surfaces.As per coding guidelines, "Use Duolingo-inspired colors: Background
#131F24, Cards#1B2F36, Blue#1CB0F6, Green#58CC02, Red#FF4B4B, Orange#FF9600, Purple#CE82FF, Muted text#56707A".Also applies to: 57-57, 78-78, 105-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/possession/RealtimePossessionMatchScreen.tsx` at line 41, The container div in RealtimePossessionMatchScreen uses a hardcoded background color "#0f1420"; update the className(s) on the wrapper div(s) (the element shown in RealtimePossessionMatchScreen) to use the repo's feature/background token instead of the hex (replace the literal "#0f1420" instances at the shown locations with the standard background token or the canonical value "#131F24"); apply the same replacement to the other occurrences referenced (lines near the other wrapper divs) so all wrappers use the feature background token for consistency with the project's color tokens.src/features/possession/realtimePossession.helpers.ts (1)
140-156: Consider adding a type guard or assertion for non-multipleChoice kinds.The function silently returns
nullfor non-multipleChoice question kinds, which is correct per the function name. However, callers might accidentally pass other kinds expecting a result.Consider adding a runtime warning in development mode when a non-multipleChoice question is passed, to help catch unintended usage:
if (!question || question.question.kind !== 'multipleChoice') { if (process.env.NODE_ENV === 'development' && question?.question.kind) { console.warn(`toMultipleChoiceGameQuestion called with ${question.question.kind} question`); } return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/possession/realtimePossession.helpers.ts` around lines 140 - 156, The function toMultipleChoiceGameQuestion currently returns null for non-multipleChoice kinds without any notice; add a dev-only runtime warning to help catch accidental misuse by checking question?.question.kind when question is present and NODE_ENV === 'development', logging a concise warning (e.g., using console.warn) that includes the actual kind; keep the existing return null behavior and avoid changing production behavior or types—update only the early guard in toMultipleChoiceGameQuestion.src/features/possession/hooks/usePossessionRoundTransition.ts (1)
189-241: Consider memoizing deeply nested dependency values.The useEffect dependency array includes deeply nested properties like
localQuestion?.question.categoryNameandpendingQuestion?.question.categoryName. While these are primitives and the current approach works, accessing nested properties directly in dependencies can lead to subtle bugs if object references change but nested values don't.For robustness, consider extracting these values before the effect:
const localCategoryName = localQuestion?.question.categoryName; const pendingCategoryName = pendingQuestion?.question.categoryName;Then use the extracted variables in both the effect body and dependency array. This makes the dependencies explicit and easier to reason about.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/possession/hooks/usePossessionRoundTransition.ts` around lines 189 - 241, The effect uses deeply nested properties directly in its dependency array which can be brittle; extract the nested primitives before the effect (e.g. const localCategoryName = localQuestion?.question.categoryName and const pendingCategoryName = pendingQuestion?.question.categoryName) and then update useEffect to reference those variables instead of localQuestion?.question.categoryName and pendingQuestion?.question.categoryName; keep the same logic inside the effect (setting transitionVisibleRef and calling setTransitionSnapshot) but use the extracted localCategoryName and pendingCategoryName in building title/categoryName and in the dependency array so dependencies are explicit and stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(fullscreen)/dev/match/page.tsx:
- Around line 89-100: The restartMatch flow currently emits
getSocket().emit('match:leave', ...) and then immediately performs local resets
and flips hasAutoStartedRef.current, which can race with the server; change
restartMatch so it waits for a server confirmation (either via socket
acknowledgement callback or a server event such as
'match:left'/'match:leave:ack') before calling
useRealtimeMatchStore.getState().reset(),
useRankedMatchmakingStore.getState().clearRankedMatchmaking(),
resetGameSession(), resetStarting(), and setting hasAutoStartedRef.current =
false; locate restartMatch and replace the immediate post-emit resets with the
same logic inside the socket ack/event handler (or gate any requeue logic on
that server-driven state transition) so requeue cannot occur until leave is
confirmed.
- Line 198: DevOverlay's onQuit currently calls exitToPlay which does not emit
the match:leave event, causing stale server membership; update exitToPlay (or
provide a new handler passed into DevOverlay) so that when in an active
RealtimePossessionMatchScreen it emits the same match:leave flow as the standard
quit path (invoke the match leave/cleanup logic used by
RealtimePossessionMatchScreen, e.g., call the same match.leave or emit
'match:leave' and run the same cleanup/redirect routine) before navigating to
the play screen; ensure you reference and reuse the existing quit/cleanup
function used by RealtimePossessionMatchScreen so behavior is identical.
In `@src/components/shared/HomeRecentMatches.tsx`:
- Around line 92-95: The row div inside HomeRecentMatches that renders each
match (the element with key={match.id} and className including "cursor-pointer")
currently presents a clickable affordance without interactive behavior; either
remove the "cursor-pointer" class (and any hover styles implying clickability)
to avoid misleading users, or make the row truly interactive by adding an
onClick handler (e.g., navigate or open details), keyboard accessibility
(tabIndex, role="button", and key handlers for Enter/Space), and appropriate
aria-labels so the element is accessible and its intent matches the visual
affordance.
- Around line 73-153: The UI cards/badges in HomeRecentMatches (inside the
visibleMatches.map render) use rounded-xl/rounded-lg and flat borders but should
use the project's 3D border style; update the main match container div (the
element with className starting "flex items-center ... rounded-xl") to use
rounded-2xl or rounded-3xl and add a chunky bottom border like "border-b-4
border-[darker-shade]" (choose a darker variant of the existing border color),
and likewise replace rounded-lg on the RP badge (span rendered when
match.rpDelta !== null) and the scoreBadge span (rendered when match.scoreBadge)
with rounded-2xl/3xl and add the same "border-b-4 border-[darker-shade]" where
appropriate; also update the small loading/empty message containers (the divs
that show "Loading recent matches..." and "No recent matches yet.") from
rounded-xl to rounded-2xl/3xl and add the chunky bottom border so all game
elements rendered by visibleMatches.map, match.rpDelta and match.scoreBadge
follow the new rounded and 3D border treatment.
- Around line 52-67: Replace the native wrapper div, header h3 and button with
shadcn/ui primitives: wrap the block with the Card component (use Card or
CardHeader/CardTitle as appropriate) instead of the outer div, render the title
using CardTitle (preserving Clock and the uppercase/fun font classes) and
replace the native button with the Button primitive while keeping the
onClick={() => router.push('/profile')} and the ArrowRight icon; update imports
to include Card (and CardHeader/CardTitle if used) and Button from your
shadcn/ui exports, move relevant className styles onto the Card/CardTitle/Button
components, and remove the old native div/h3/button elements so the UI matches
the shared shadcn patterns.
In `@src/features/dev/DevOverlay.tsx`:
- Around line 84-104: The Restart/Quit buttons use the SkipBtn component but
SkipBtn currently applies rounded-lg and lacks the chunky 3D bottom border;
update the SkipBtn component implementation to use the project's button style
rules by replacing rounded-lg with rounded-2xl (or rounded-3xl per design) and
adding border-b-4 plus the appropriate border color class so Restart/Quit
(rendered from DevOverlay.tsx via the SkipBtn component) follow the project
chunky 3D button styling.
In `@src/features/play/ModeSelectionScreen.tsx`:
- Around line 171-181: The dev links (Link components for "/dev/match" and
"/dev/mock-match" in ModeSelectionScreen) currently stop mouse click propagation
but not keyboard events, so keyboard activation still bubbles to the parent
onKeyDown handler; add an onKeyDown handler that calls e.stopPropagation() (and
also onKeyUp to be safe) to each Link to prevent keyboard events from reaching
the parent ranked-select logic; update the Link props for the components
identified by href="/dev/match" and href="/dev/mock-match" to include
onKeyDown={(e) => e.stopPropagation()} (and onKeyUp={(e) =>
e.stopPropagation()}).
In `@src/features/possession/components/LiveSpecialQuestionPanel.tsx`:
- Line 295: The submitted flag is currently only checking
answerAck?.questionKind and thus can be triggered by a stale ack from a previous
question of the same kind; update the check in LiveSpecialQuestionPanel so it
also matches the current question index (e.g. answerAck?.questionKind ===
'putInOrder' && answerAck?.qIndex === qIndex or the local current question index
variable) so the panel is scoped to the current question; apply the same change
to the other occurrence in this file that uses answerAck?.questionKind to lock
panels/resolved-state copy.
- Around line 332-339: The submit handler (handleSubmit) emits
'match:put_in_order_answer' but waits for answerAck before disabling the UI,
allowing double-submits; fix by introducing a local latch (e.g., isSubmitting or
hasSubmitted state) that is set to true immediately in handleSubmit (before
calling getSocket().emit) and short-circuit future calls, and ensure the
UI/button checks this latch to disable immediately; keep answerAck handling for
server ack (and clear/reset only on explicit retry if needed). Apply the same
change to the other submit block referenced (lines ~407-413) so both emission
sites (getSocket().emit('match:put_in_order_answer')) use the local latch to
prevent multiple emits.
- Around line 607-620: The component mistakenly renames and drops showOptions
(destructured as showOptions: _showOptions) and doesn't use it to disable
interactive inputs, letting countdown/order/clues panels submit during the
reveal phase; fix by restoring/using the prop (either destructure as showOptions
or use _showOptions consistently) inside LiveSpecialQuestionPanel and guard all
input/UI handlers and controls that call countdownGuessAck, cluesGuessAck,
order-related handlers, or submit actions so they are disabled or no-op when
showOptions is false (return early from submit handlers and set interactive
controls to disabled when showOptions is falsy).
In `@src/features/possession/components/PossessionMatchViewport.tsx`:
- Around line 126-129: The mobile PenaltySplash is rendered in a separate empty
relative wrapper so its absolute positioning doesn’t overlay the field; move the
AnimatePresence + PenaltySplash (currently in the div with className "lg:hidden
relative") into the pitch container element that renders the field inside
PossessionMatchViewport so the splash is a positioned child of the pitch (keep
AnimatePresence around PenaltySplash and retain the "lg:hidden" visibility
behavior). Update the JSX to remove the standalone empty wrapper and render
<AnimatePresence><PenaltySplash model={penaltySplash} /></AnimatePresence> as a
direct child of the pitch container to ensure the absolutely positioned splash
overlays the field correctly on mobile.
In `@src/features/possession/components/PossessionQuestionArea.tsx`:
- Around line 48-58: The question panel is only faded out but remains
interactive; update the motion.div rendering in PossessionQuestionArea to
disable interaction when hideQuestionContent is true by adding
aria-hidden={hideQuestionContent} and a conditional className that applies
pointer-events-none (e.g., className={hideQuestionContent ?
'pointer-events-none' : ''}); alternatively, unmount the panels when hidden by
moving the {content.kind === ... ? ... : ...} JSX behind a conditional (render
null when hideQuestionContent) so PossessionQuestionPanel and
LiveSpecialQuestionPanel are not focusable while the overlay is shown.
In `@src/features/possession/hooks/usePossessionFieldState.ts`:
- Around line 324-325: The code hard-codes mirrored to false in
usePossessionFieldState.ts so half-time orientation never flips; change the
mirrored assignment to derive from the current half/orientation state (e.g., use
the existing half flag or pitch orientation variable used elsewhere in the hook)
instead of a constant, and ensure targetGoal (and any logic using mirrored) uses
that computed mirrored value; also update the other occurrence noted around the
385-389 block to the same pattern so second-half shots and penalties animate
toward the correct goal.
In `@src/features/possession/hooks/useRealtimePossessionMatchController.ts`:
- Line 4: The possession controller useRealtimePossessionMatchController is
directly importing useRealtimeGameLogic which creates an illegal coupling;
refactor by removing the direct import of useRealtimeGameLogic and either (a)
move the shared orchestration logic into a new shared lib/store (e.g.,
src/lib/realtime or src/stores/realtime) and have
useRealtimePossessionMatchController consume that, or (b) create a
possession-local adapter wrapper (e.g., create a usePossessionRealtimeAdapter
that delegates to the shared store) and import the adapter instead; update the
controller to call the new shared store/adapter API (keeping the same method
names used in useRealtimePossessionMatchController) and adjust any call
sites/tests to import the adapter/shared store rather than
'@/features/game/hooks/useRealtimeGameLogic'.
In `@src/features/possession/realtimePossession.helpers.ts`:
- Around line 35-71: The current implementations of toAnswerStates and
toRevealAnswerStates build variable-length arrays and cast them to
AnswerStateArray (a fixed 4-tuple), which can lie if optionsCount !== 4; fix by
enforcing the tuple constraint at runtime: add a guard at the start of
toAnswerStates and toRevealAnswerStates that throws or returns an error when
optionsCount !== 4 (or convert to a runtime-safe return like AnswerState[] if
you intend variable counts), and stop using unchecked "as AnswerStateArray"
casts—either construct and return an actual 4-element tuple when optionsCount
=== 4 or change the function signatures to return AnswerState[]; update the
implementations in toAnswerStates and toRevealAnswerStates accordingly.
---
Nitpick comments:
In `@src/components/layout/AppShell.tsx`:
- Around line 189-211: Capture activeMatchBanner into a local constant at the
start of handleRejoinMatch and use that constant for all subsequent reads
(matchId, mode, matchType, opponent fields, and source) so the handler uses a
consistent banner snapshot even if state changes during execution; update
references in startSession, the getSocket().emit("match:rejoin", { matchId })
check, clearRejoinAvailable(), and any other usages inside handleRejoinMatch to
use the captured local variable instead of reading activeMatchBanner repeatedly.
In `@src/features/play/ModeSelectionScreen.tsx`:
- Around line 174-181: The two dev CTA Link elements (the one rendering "Dev
Quick Ranked" and the one with href="/dev/mock-match") use non-standard
rounded-xl, missing chunky 3D border and non-approved palette; update their
className to use rounded-2xl or rounded-3xl, add a 3D border with border-b-4
plus a darker border color (e.g., border-b-4 border-[`#0F3A00`] -> border-b-4
border-[darker-shade]), and replace custom blacks/greens with the design system
colors (Cards `#1B2F36` or Green `#58CC02` for green CTAs, Muted text `#56707A` for
text) while preserving existing spacing and hover classes so the buttons match
the feature design system.
In `@src/features/possession/hooks/usePossessionGoalCelebration.ts`:
- Around line 64-68: The conditional in the useEffect inside
usePossessionGoalCelebration redundantly checks for undefined even though
currentQuestionIndex is typed number | null; simplify the check to just
`currentQuestionIndex !== null` (or `currentQuestionIndex == null` inverted as
needed) and keep the body setting `pendingGoalRef.current = null` unchanged so
the effect still runs only when there is a non-null index.
In `@src/features/possession/hooks/usePossessionRoundTransition.ts`:
- Around line 189-241: The effect uses deeply nested properties directly in its
dependency array which can be brittle; extract the nested primitives before the
effect (e.g. const localCategoryName = localQuestion?.question.categoryName and
const pendingCategoryName = pendingQuestion?.question.categoryName) and then
update useEffect to reference those variables instead of
localQuestion?.question.categoryName and pendingQuestion?.question.categoryName;
keep the same logic inside the effect (setting transitionVisibleRef and calling
setTransitionSnapshot) but use the extracted localCategoryName and
pendingCategoryName in building title/categoryName and in the dependency array
so dependencies are explicit and stable.
In `@src/features/possession/realtimePossession.helpers.ts`:
- Around line 140-156: The function toMultipleChoiceGameQuestion currently
returns null for non-multipleChoice kinds without any notice; add a dev-only
runtime warning to help catch accidental misuse by checking
question?.question.kind when question is present and NODE_ENV === 'development',
logging a concise warning (e.g., using console.warn) that includes the actual
kind; keep the existing return null behavior and avoid changing production
behavior or types—update only the early guard in toMultipleChoiceGameQuestion.
In `@src/features/possession/RealtimePossessionMatchScreen.tsx`:
- Line 41: The container div in RealtimePossessionMatchScreen uses a hardcoded
background color "#0f1420"; update the className(s) on the wrapper div(s) (the
element shown in RealtimePossessionMatchScreen) to use the repo's
feature/background token instead of the hex (replace the literal "#0f1420"
instances at the shown locations with the standard background token or the
canonical value "#131F24"); apply the same replacement to the other occurrences
referenced (lines near the other wrapper divs) so all wrappers use the feature
background token for consistency with the project's color tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93a20834-3245-4b3f-bf36-e25bccd8180e
📒 Files selected for processing (36)
src/app/(app)/play/page.tsxsrc/app/(fullscreen)/dev/match/page.tsxsrc/components/auth/PrivacyPolicyScreen.tsxsrc/components/auth/TermsOfServiceScreen.tsxsrc/components/auth/WelcomeScreen.tsxsrc/components/layout/AppShell.tsxsrc/components/layout/Sidebar.tsxsrc/components/shared/HomeRecentMatches.tsxsrc/features/dev/DevOverlay.tsxsrc/features/game/GameStageRouter.tsxsrc/features/game/__tests__/GameStageRouter.test.tsxsrc/features/game/components/MatchmakingMapScreen.tsxsrc/features/game/hooks/__tests__/useRealtimeGameLogic.test.tssrc/features/game/hooks/useRealtimeGameLogic.tssrc/features/leaderboard/LeaderboardScreen.tsxsrc/features/party/RealtimePartyQuizScreen.tsxsrc/features/play/ModeSelectionScreen.tsxsrc/features/possession/RealtimePossessionMatchScreen.tsxsrc/features/possession/components/LiveSpecialQuestionPanel.tsxsrc/features/possession/components/PossessionMatchViewport.tsxsrc/features/possession/components/PossessionQuestionArea.tsxsrc/features/possession/components/__tests__/PossessionQuestionArea.test.tsxsrc/features/possession/hooks/__tests__/usePossessionFieldState.test.tssrc/features/possession/hooks/__tests__/usePossessionGoalCelebration.test.tssrc/features/possession/hooks/__tests__/usePossessionRoundTransition.test.tssrc/features/possession/hooks/__tests__/useRealtimePossessionMatchController.test.tssrc/features/possession/hooks/usePossessionFieldState.tssrc/features/possession/hooks/usePossessionGoalCelebration.tssrc/features/possession/hooks/usePossessionRoundTransition.tssrc/features/possession/hooks/usePossessionScoreSplashes.tssrc/features/possession/hooks/useRealtimePossessionMatchController.tssrc/features/possession/realtimePossession.helpers.tssrc/lib/realtime/socket-handlers.tssrc/lib/realtime/socket.types.tssrc/stores/__tests__/realtimeMatch.store.test.tssrc/stores/realtimeMatch.store.ts
💤 Files with no reviewable changes (1)
- src/app/(app)/play/page.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/features/possession/hooks/usePossessionFieldState.ts (1)
121-127: Complex possession calculation — consider extracting to named function.The inline calculation for
initialPossessionPctis correct but dense. A helper function with a descriptive name would improve readability.♻️ Suggested extraction
function computePossessionPct(possessionDiff: number, mySeat: number | null): number { const basePct = Math.max(0, Math.min(100, 50 + (possessionDiff / 2))); const adjustedPct = mySeat === 2 ? 100 - basePct : basePct; return Math.max(10, Math.min(90, adjustedPct)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/possession/hooks/usePossessionFieldState.ts` around lines 121 - 127, The inline complex expression computing initialPossessionPct should be extracted into a small named helper (e.g., computePossessionPct) to improve readability: create a function that takes possessionState?.possessionDiff (or default 0) and mySeat, computes basePct = clamp(50 + possessionDiff/2, 0, 100), flips it when mySeat === 2, then clamps the final value to the 10–90 range; replace the large inline expression in the initialPossessionPct assignment with a call to this helper (referencing initialPossessionPct and the new computePossessionPct) so logic and intent are clear.src/lib/realtime/socket-handlers.ts (1)
226-265: Consider extracting i18n resolution into a helper function.The deeply nested ternary for question kind handling is functional but challenging to read and maintain. A dedicated helper like
resolveQuestionI18n(data.question, locale)would improve clarity.♻️ Example refactor
function resolveQuestionI18n(question: GameQuestionDTO, locale: string): ResolvedGameQuestion { const categoryName = question.categoryName ? getI18nText(question.categoryName, locale) : undefined; switch (question.kind) { case 'multipleChoice': return { ...question, resolvedLocale: locale, prompt: getI18nText(question.prompt, locale), options: question.options.map((opt) => getI18nText(opt, locale)), categoryName, }; case 'countdown': return { ...question, resolvedLocale: locale, prompt: getI18nText(question.prompt, locale), categoryName, }; // ... other cases } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/realtime/socket-handlers.ts` around lines 226 - 265, The nested ternary resolving i18n for data.question is hard to read and should be extracted into a helper; implement a function like resolveQuestionI18n(question, locale) that computes categoryName (using getI18nText if present), sets resolvedLocale, and returns the properly transformed object per question.kind (use a switch on question.kind to handle 'multipleChoice', 'countdown', 'putInOrder', default for clues) mapping options/items/clues and calling getI18nText for prompt/instruction/details/content as in the diff, then replace the current nested ternary expression with a call to resolveQuestionI18n(data.question, locale).src/app/(fullscreen)/dev/match/restartMatch.ts (1)
7-9: Minor edge case inhasLeftMatchlogic.The check
snapshot.activeMatchId !== matchIdreturns true for any different match ID, not justnull. If the user somehow joined a new match before the leave was fully confirmed, this would resolve prematurely.For dev tooling this is acceptable, but a stricter check would be:
function hasLeftMatch(snapshot: SessionStatePayload, matchId: string): boolean { - return snapshot.activeMatchId !== matchId; + return snapshot.activeMatchId === null || snapshot.activeMatchId !== matchId; }The current implementation is functionally equivalent but the intent is clearer with
=== nullas the primary condition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(fullscreen)/dev/match/restartMatch.ts around lines 7 - 9, The current hasLeftMatch function uses snapshot.activeMatchId !== matchId which treats any different matchId (including having joined another match) as "left"; change the condition to explicitly check for null (snapshot.activeMatchId === null) to ensure we only report left when there is no active match, keeping the function signature hasLeftMatch(snapshot: SessionStatePayload, matchId: string): boolean and preserving its return type.src/app/(fullscreen)/dev/match/page.tsx (1)
90-112: Restart race condition addressed, but failed restarts are silently swallowed.The
waitForMatchLeaveConfirmationintegration correctly addresses the previous race condition concern. However, when the leave confirmation fails (timeout,MATCH_LEAVE_ERROR, orTRANSITION_IN_PROGRESS), the restart is silently abandoned with only a log warning.The user receives no feedback that their restart attempt failed, and they remain in the current match state. Consider either:
- Retrying the leave operation
- Showing a toast/notification to the user
- Proceeding with local reset anyway (accepting potential desync for dev tooling)
For dev-only tooling, option 3 might be acceptable:
♻️ Proposed fix to proceed with restart even on failure
void waitForMatchLeaveConfirmation(currentMatchId) .then(() => { completeRestart(); }) .catch((error) => { logger.warn('Dev restart failed waiting for match leave confirmation', { error, matchId: currentMatchId }); + // Proceed anyway for dev tooling - server state may be stale but local state will reset + completeRestart(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(fullscreen)/dev/match/page.tsx around lines 90 - 112, The restart currently aborts on waitForMatchLeaveConfirmation errors and only logs a warning; update restartMatch so the catch block calls completeRestart() to proceed with the local reset even when waitForMatchLeaveConfirmation rejects (preserving the existing logger.warn call), so users get the restart behavior for dev tooling; reference the restartMatch function, the completeRestart helper, and the waitForMatchLeaveConfirmation call and ensure hasAutoStartedRef.current is still set to false as part of the fallback path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/`(fullscreen)/dev/match/page.tsx:
- Around line 90-112: The restart currently aborts on
waitForMatchLeaveConfirmation errors and only logs a warning; update
restartMatch so the catch block calls completeRestart() to proceed with the
local reset even when waitForMatchLeaveConfirmation rejects (preserving the
existing logger.warn call), so users get the restart behavior for dev tooling;
reference the restartMatch function, the completeRestart helper, and the
waitForMatchLeaveConfirmation call and ensure hasAutoStartedRef.current is still
set to false as part of the fallback path.
In `@src/app/`(fullscreen)/dev/match/restartMatch.ts:
- Around line 7-9: The current hasLeftMatch function uses snapshot.activeMatchId
!== matchId which treats any different matchId (including having joined another
match) as "left"; change the condition to explicitly check for null
(snapshot.activeMatchId === null) to ensure we only report left when there is no
active match, keeping the function signature hasLeftMatch(snapshot:
SessionStatePayload, matchId: string): boolean and preserving its return type.
In `@src/features/possession/hooks/usePossessionFieldState.ts`:
- Around line 121-127: The inline complex expression computing
initialPossessionPct should be extracted into a small named helper (e.g.,
computePossessionPct) to improve readability: create a function that takes
possessionState?.possessionDiff (or default 0) and mySeat, computes basePct =
clamp(50 + possessionDiff/2, 0, 100), flips it when mySeat === 2, then clamps
the final value to the 10–90 range; replace the large inline expression in the
initialPossessionPct assignment with a call to this helper (referencing
initialPossessionPct and the new computePossessionPct) so logic and intent are
clear.
In `@src/lib/realtime/socket-handlers.ts`:
- Around line 226-265: The nested ternary resolving i18n for data.question is
hard to read and should be extracted into a helper; implement a function like
resolveQuestionI18n(question, locale) that computes categoryName (using
getI18nText if present), sets resolvedLocale, and returns the properly
transformed object per question.kind (use a switch on question.kind to handle
'multipleChoice', 'countdown', 'putInOrder', default for clues) mapping
options/items/clues and calling getI18nText for
prompt/instruction/details/content as in the diff, then replace the current
nested ternary expression with a call to resolveQuestionI18n(data.question,
locale).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa49916b-1dbd-49c3-9560-761f1f95ed5c
📒 Files selected for processing (22)
notes.mdsrc/app/(fullscreen)/dev/match/__tests__/restartMatch.test.tssrc/app/(fullscreen)/dev/match/page.tsxsrc/app/(fullscreen)/dev/match/restartMatch.tssrc/components/auth/AppAuthGate.tsxsrc/components/auth/OAuthCallbackScreen.tsxsrc/components/auth/PublicOnlyGate.tsxsrc/components/auth/__tests__/auth-routing.test.tsxsrc/components/shared/HomeRecentMatches.tsxsrc/features/play/ModeSelectionScreen.tsxsrc/features/possession/__tests__/realtimePossession.helpers.test.tssrc/features/possession/components/LiveSpecialQuestionPanel.tsxsrc/features/possession/components/PossessionMatchViewport.tsxsrc/features/possession/components/PossessionQuestionArea.tsxsrc/features/possession/hooks/__tests__/usePossessionFieldState.test.tssrc/features/possession/hooks/usePossessionFieldState.tssrc/features/possession/realtimePossession.helpers.tssrc/lib/auth/onboarding.tssrc/lib/realtime/socket-handlers.tssrc/lib/realtime/socket.types.tssrc/stores/__tests__/realtimeMatch.store.test.tssrc/stores/realtimeMatch.store.ts
✅ Files skipped from review due to trivial changes (1)
- notes.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src/features/play/ModeSelectionScreen.tsx
- src/components/shared/HomeRecentMatches.tsx
- src/features/possession/hooks/tests/usePossessionFieldState.test.ts
- src/features/possession/components/PossessionQuestionArea.tsx
- src/features/possession/components/PossessionMatchViewport.tsx
This was referenced May 8, 2026
Merged
Merged
This was referenced May 19, 2026
Merged
Merged
Merged
Merged
This was referenced Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style