Staging to Prod.#41
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds penalty-mode gameplay and UI/timing, raises bar-battle cap to 20, introduces a correlation-aware lobby command machine and invite-code utilities, implements pending-deletion restore and richer refresh semantics in auth flows, updates UI components and tests, and includes supporting infra (i18n, fonts, sounds, preload hook, test storage shim). ChangesPenalties, Lobby, and Auth Restore
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/geo/countries.ts (1)
534-540:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLos Angeles coordinates look closer to San Bernardino than downtown LA.
In
src/lib/geo/countries.ts, the"los angeles"point (lon: -117.65, lat: 34.15) is ~55.7 km from downtown LA (34.0522, -118.2437) and ~33.5 km from San Bernardino (34.1083, -117.2898). If the intent is to mark Los Angeles’s city center, update these coordinates. https://www.google.com/maps?q=34.15,-117.65🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/geo/countries.ts` around lines 534 - 540, The "los angeles" entry in the countries.ts dataset uses coordinates (lon: -117.65, lat: 34.15) which are nearer San Bernardino; update the "los angeles" object's lon/lat to downtown LA coordinates (approx lon: -118.2437, lat: 34.0522) in src/lib/geo/countries.ts for the "los angeles" key and keep the city/country/flag fields unchanged; verify the updated point is now close to LA city center and adjust slightly if you prefer an alternate official center.src/types/api.generated.ts (1)
901-908:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
secretquery param in callback type contract.Line 897-898 documents callback auth via
secretquery param, and Line 925 defines a 401 for invalid secret, but Line 901-908 omitssecretfrom typed query params. This creates a client/server contract mismatch for/api/v1/auth/sms/smsoffice-callback.Suggested type fix
parameters: { query: { + secret: string; reference: string; status: string; reason?: string; destination: string; timestamp?: string; operator?: string; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types/api.generated.ts` around lines 901 - 908, The generated callback query type for /api/v1/auth/sms/smsoffice-callback is missing the required `secret` param, causing a client/server contract mismatch; update the `query` shape in the type (the object containing `reference`, `status`, `reason?`, `destination`, `timestamp?`, `operator?`) to include `secret: string` (required, not optional) so the typed contract matches the documented/authenticated callback and the 401 handling.
🧹 Nitpick comments (8)
src/features/welcome/WelcomeScreen.tsx (1)
185-193: 💤 Low valueConsider removing the unnecessary fallback.
The
variantprop usesauthNoticeModal ?? 'check-email', but the modal only renders whenauthNoticeModal !== null. The fallback will never be used.♻️ Proposed cleanup
<WelcomeAuthNoticeModal open={authNoticeModal !== null} - variant={authNoticeModal ?? 'check-email'} + variant={authNoticeModal!} onClose={handleCloseAuthNoticeModal}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/welcome/WelcomeScreen.tsx` around lines 185 - 193, The variant prop fallback is redundant because WelcomeAuthNoticeModal is only rendered when authNoticeModal !== null; remove the nullish coalescing fallback and pass authNoticeModal directly to variant (i.e., use variant={authNoticeModal}) and keep existing handlers (handleCloseAuthNoticeModal, handleNoticeModalGoToSignIn, handleRestorePendingDeletion) and restore props (restoreSubmitting, restoreError) unchanged so behavior is preserved.src/lib/config.ts (1)
4-12: ⚖️ Poor tradeoffConsider environment-specific configuration strategy.
Hardcoding
PHONE_AUTH_ENABLED = trueremoves the ability to toggle this feature via environment variables. While the comment indicates this is intentional for staging testing, consider whether you want to:
- Keep the env var for production control, or
- Use a different approach to gate the feature per environment
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/config.ts` around lines 4 - 12, Replace the hardcoded export const PHONE_AUTH_ENABLED = true with an environment-driven boolean: read a new env var (e.g., process.env.PHONE_AUTH_ENABLED) and parse it to a boolean with a sensible default (true for staging or configurable via another env like NODE_ENV); update the PHONE_AUTH_ENABLED constant to use that parsed value so the feature can be toggled per environment while preserving current default behavior. Ensure parsing handles "1"/"true"/"false"/"0" case-insensitively and falls back to the existing default when unset.src/components/auth/__tests__/WelcomeScreen.test.tsx (1)
328-328: 💤 Low valueConsider relaxing the assertion to avoid testing implementation details.
The test explicitly expects
undefinedas the 4th argument, but this tests how the function is called rather than what it does. If the implementation changes to omit the argument entirely (which is semantically identical), this test would break.Consider using
expect.anything()or omitting the check for optional parameters:- expect(socialLoginWithIdTokenMock).toHaveBeenCalledWith('google', 'tok', 'nonce', undefined); + expect(socialLoginWithIdTokenMock).toHaveBeenCalledWith('google', 'tok', 'nonce', expect.anything());Or verify just the required parameters and use
toHaveBeenCalledTimesfor call count.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/auth/__tests__/WelcomeScreen.test.tsx` at line 328, The assertion in WelcomeScreen.test.tsx is too strict by expecting an explicit fourth argument of undefined when calling socialLoginWithIdTokenMock; update the test to only assert the meaningful parameters (e.g., 'google', 'tok', 'nonce') and relax or omit the check for the optional fourth argument—either drop the fourth-arg check and rely on toHaveBeenCalledTimes or use a matcher like expect.anything()/a wildcard for the fourth parameter when calling socialLoginWithIdTokenMock so changes that omit the trailing undefined won't break the test.src/lib/api/__tests__/client.test.ts (1)
32-33: 💤 Low valueRemove the redundant/no-op setup line.
Line 32 sets
mockRejectedValue(undefined)via optional chaining (the method always exists on avi.fn) and is immediately overwritten by line 33. It has no effect and is misleading in the test.♻️ Proposed cleanup
- mockedRequest.mockRejectedValue?.(undefined); mockedRequest.mockRejectedValue(new ApiError("Request failed", 401, null));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/api/__tests__/client.test.ts` around lines 32 - 33, Remove the redundant no-op mock setup: delete the line calling mockedRequest.mockRejectedValue?.(undefined) since mockRejectedValue always exists on the vi.fn and the next line mockedRequest.mockRejectedValue(new ApiError("Request failed", 401, null)) immediately overwrites it; update the test in client.test.ts by keeping only the meaningful mockedRequest.mockRejectedValue(new ApiError(...)) call (refer to mockedRequest.mockRejectedValue and ApiError in the test).src/features/friend/hooks/useLobbyCommandMachine.ts (2)
208-208: 💤 Low valueClarify magic numbers in backoff formula.
The backoff delay
220 + attempt * 140uses unexplained constants. Consider extracting these as named constants with comments describing the rationale (e.g., minimum backoff, increment per retry).♻️ Suggested refactor
+const RETRY_BASE_DELAY_MS = 220; +const RETRY_INCREMENT_MS = 140; + for (let attempt = 0; attempt <= maxRetries; attempt += 1) { const result = await send(correlationId); if (sequenceRef.current !== sequence) return null; // ... logging ... if (result.ok) { // ... success handling ... } if (result.retryable && attempt < maxRetries) { - await wait(220 + attempt * 140); + await wait(RETRY_BASE_DELAY_MS + attempt * RETRY_INCREMENT_MS); continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/friend/hooks/useLobbyCommandMachine.ts` at line 208, Extract the magic numbers in the backoff formula into named constants (e.g., MIN_BACKOFF_MS and BACKOFF_INCREMENT_MS) used in the wait call inside useLobbyCommandMachine (the location with await wait(220 + attempt * 140)); replace the inline expression with a computed value using those constants, and add a brief comment explaining the rationale (minimum base backoff and incremental backoff per retry) and units (ms) so the intent is clear.
231-231: 💤 Low valueVerify asymmetric retry counts across lobby commands.
createLobbyretries up to 2 times,joinByCodeup to 5, andleaveLobby3. The higher retry count for join may reflect its time-sensitivity (joining an active lobby before it fills/closes), but the difference is notable. Confirm this trade-off aligns with the intended UX and failure recovery strategy.Also applies to: 268-268, 283-283
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/friend/hooks/useLobbyCommandMachine.ts` at line 231, createLobby, joinByCode, and leaveLobby use different maxRetries (2, 5, 3 respectively); confirm this asymmetry is intentional and either normalize retry counts or document the rationale. Locate the retry configs in useLobbyCommandMachine (symbols: createLobby, joinByCode, leaveLobby) and: if they should be uniform, replace hard-coded values with a shared constant (e.g., LOBBY_COMMAND_MAX_RETRIES) and use it for all three; if the differences are intentional, add a short comment explaining why joinByCode needs more retries (e.g., time-sensitivity) and ensure those counts are defined as named constants for clarity and future tuning.src/stores/realtime-match/match.slice.ts (1)
158-160: ⚡ Quick winDocument the
matchPaused/resume countdown state flow
setMatchCountdownforcesmatchPaused: truewhenpayload.reason === 'resume'(and clearspauseUntil), somatchPausedis used as a gameplay/UI gate during the resume countdown. The pause state is then cleared only on the subsequentmatch:resumesocket event (store.clearMatchPaused()), not automatically when the countdown reaches 0. Add a brief comment nearsetMatchCountdown(or in the state docs) explaining thatmatchPausedis expected to remain true untilmatch:resume, to avoid confusion about the “resume sets paused=true” logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/realtime-match/match.slice.ts` around lines 158 - 160, Add a short comment near setMatchCountdown (or in the match slice state docs) explaining that when payload.reason === 'resume' the reducer intentionally sets state.matchPaused = true and clears pauseUntil to gate gameplay/UI during the resume countdown, and that this paused flag is NOT cleared when the countdown reaches 0 but is cleared only when the external socket event match:resume calls store.clearMatchPaused(); reference state.matchPaused, pauseUntil, pausedAt, setMatchCountdown, and clearMatchPaused in the comment so future readers understand the flow.src/features/possession/hooks/usePossessionFieldState.ts (1)
125-135: 💤 Low valueReplace the inline
MatchStatePayloadcast with a typedpenalty.kicksTakenshape
- A repo-wide TypeScript search finds
kicksTakenonly in the inline assertion insrc/features/possession/hooks/usePossessionFieldState.ts(around lines 125-135), so the shared realtime/socket typing forMatchStatePayloaddoesn’t declarepenalty?.kicksTaken.- If the server sends
penalty.kicksTaken, extend the shared socket types; otherwise avoid depending on this untyped payload shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/possession/hooks/usePossessionFieldState.ts` around lines 125 - 135, The inline cast to MatchStatePayload for accessing penalty.kicksTaken is unsafe; update types instead of casting: either extend the shared realtime/socket MatchStatePayload to declare penalty?: { kicksTaken?: { seat1?: number; seat2?: number } } so usePossessionFieldState can access stateWithPenalty.penalty?.kicksTaken with correct typing, or introduce a local typed shape (e.g., PenaltyKicksTaken) and a small type guard/nullable extraction in usePossessionFieldState before calling reconstructPenaltyAttempts (referencing stateWithPenalty, possessionState.penaltyGoals, reconstructPenaltyAttempts) so you no longer rely on an inline assertion. Ensure the chosen change is applied in the shared types if the server actually sends kicksTaken; otherwise keep it local and remove the cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/layout/Sidebar.tsx`:
- Line 85: The badge typography in the Sidebar component was changed from
font-black to font-semibold which lightens the visual weight; locate the badge
span(s) in Sidebar.tsx (the span with className containing "rounded-full
bg-red-500 ... font-poppins ... text-white") and either revert the class from
font-semibold back to font-black if the original heavy weight is required by
design, or confirm and propagate the new weight consistently across the other
badge occurrences mentioned (the other span(s) referenced as "99-99"); update
the className(s) accordingly in the Sidebar component and any duplicate badge
elements so the typography matches the approved design decision.
- Around line 72-81: The inline style in Sidebar (in the JSX that builds the nav
button using the isActive variable) lowered font weight to 700 and may have
dropped the Tailwind tracking utility; restore the original heavier weight by
changing the inline style to fontWeight: 900 or, better, remove the inline
fontWeight and re-add the Tailwind class "font-black" to the className for the
button (target the same JSX block that currently sets style={{ fontFamily:
"'Poppins', sans-serif", fontWeight: 700, ... }} and the class list with
"relative inline-flex ... tracking-wide"), and ensure "tracking-wide" remains
present in the class list.
In `@src/features/friend/FriendMatchHubPage.tsx`:
- Around line 78-86: The effect returns immediately after
router.push(`/friend/room/${lobby.inviteCode}`) and never clears the navigating
flag; ensure you reset the navigation state after navigation by calling the
appropriate setter (e.g. setIsNavigatingToRoom(false)) or a shared reset
function rather than returning early. Concretely, in the block that checks
lobby?.inviteCode && isNavigatingToRoom, call resetLobbyCommand() (if it doesn't
already) and then reset the isNavigatingToRoom state (via
setIsNavigatingToRoom(false) or the existing resetNavigation helper) after
router.push, remove the early return so the state-clearing runs, and keep
dependencies including isNavigatingToRoom, resetLobbyCommand and router
unchanged.
In `@src/features/welcome/useWelcomeAuthController.ts`:
- Around line 449-458: The early return inside the verifyGeorgianPhoneOtp catch
block leaves authSubmitting true; fix by ensuring authSubmitting is reset before
returning when isPendingDeletionAuthError(error) is true — specifically, call
the state setter that clears loading (authSubmitting -> false) before calling
setPendingRestoreAction and setAuthNoticeModal or immediately after those but
prior to returning in the catch branch that handles isPendingDeletionAuthError;
locate this logic around verifyGeorgianPhoneOtp, isPendingDeletionAuthError,
setPendingRestoreAction and setAuthNoticeModal and ensure the loading state is
explicitly cleared on that early-return path.
---
Outside diff comments:
In `@src/lib/geo/countries.ts`:
- Around line 534-540: The "los angeles" entry in the countries.ts dataset uses
coordinates (lon: -117.65, lat: 34.15) which are nearer San Bernardino; update
the "los angeles" object's lon/lat to downtown LA coordinates (approx lon:
-118.2437, lat: 34.0522) in src/lib/geo/countries.ts for the "los angeles" key
and keep the city/country/flag fields unchanged; verify the updated point is now
close to LA city center and adjust slightly if you prefer an alternate official
center.
In `@src/types/api.generated.ts`:
- Around line 901-908: The generated callback query type for
/api/v1/auth/sms/smsoffice-callback is missing the required `secret` param,
causing a client/server contract mismatch; update the `query` shape in the type
(the object containing `reference`, `status`, `reason?`, `destination`,
`timestamp?`, `operator?`) to include `secret: string` (required, not optional)
so the typed contract matches the documented/authenticated callback and the 401
handling.
---
Nitpick comments:
In `@src/components/auth/__tests__/WelcomeScreen.test.tsx`:
- Line 328: The assertion in WelcomeScreen.test.tsx is too strict by expecting
an explicit fourth argument of undefined when calling
socialLoginWithIdTokenMock; update the test to only assert the meaningful
parameters (e.g., 'google', 'tok', 'nonce') and relax or omit the check for the
optional fourth argument—either drop the fourth-arg check and rely on
toHaveBeenCalledTimes or use a matcher like expect.anything()/a wildcard for the
fourth parameter when calling socialLoginWithIdTokenMock so changes that omit
the trailing undefined won't break the test.
In `@src/features/friend/hooks/useLobbyCommandMachine.ts`:
- Line 208: Extract the magic numbers in the backoff formula into named
constants (e.g., MIN_BACKOFF_MS and BACKOFF_INCREMENT_MS) used in the wait call
inside useLobbyCommandMachine (the location with await wait(220 + attempt *
140)); replace the inline expression with a computed value using those
constants, and add a brief comment explaining the rationale (minimum base
backoff and incremental backoff per retry) and units (ms) so the intent is
clear.
- Line 231: createLobby, joinByCode, and leaveLobby use different maxRetries (2,
5, 3 respectively); confirm this asymmetry is intentional and either normalize
retry counts or document the rationale. Locate the retry configs in
useLobbyCommandMachine (symbols: createLobby, joinByCode, leaveLobby) and: if
they should be uniform, replace hard-coded values with a shared constant (e.g.,
LOBBY_COMMAND_MAX_RETRIES) and use it for all three; if the differences are
intentional, add a short comment explaining why joinByCode needs more retries
(e.g., time-sensitivity) and ensure those counts are defined as named constants
for clarity and future tuning.
In `@src/features/possession/hooks/usePossessionFieldState.ts`:
- Around line 125-135: The inline cast to MatchStatePayload for accessing
penalty.kicksTaken is unsafe; update types instead of casting: either extend the
shared realtime/socket MatchStatePayload to declare penalty?: { kicksTaken?: {
seat1?: number; seat2?: number } } so usePossessionFieldState can access
stateWithPenalty.penalty?.kicksTaken with correct typing, or introduce a local
typed shape (e.g., PenaltyKicksTaken) and a small type guard/nullable extraction
in usePossessionFieldState before calling reconstructPenaltyAttempts
(referencing stateWithPenalty, possessionState.penaltyGoals,
reconstructPenaltyAttempts) so you no longer rely on an inline assertion. Ensure
the chosen change is applied in the shared types if the server actually sends
kicksTaken; otherwise keep it local and remove the cast.
In `@src/features/welcome/WelcomeScreen.tsx`:
- Around line 185-193: The variant prop fallback is redundant because
WelcomeAuthNoticeModal is only rendered when authNoticeModal !== null; remove
the nullish coalescing fallback and pass authNoticeModal directly to variant
(i.e., use variant={authNoticeModal}) and keep existing handlers
(handleCloseAuthNoticeModal, handleNoticeModalGoToSignIn,
handleRestorePendingDeletion) and restore props (restoreSubmitting,
restoreError) unchanged so behavior is preserved.
In `@src/lib/api/__tests__/client.test.ts`:
- Around line 32-33: Remove the redundant no-op mock setup: delete the line
calling mockedRequest.mockRejectedValue?.(undefined) since mockRejectedValue
always exists on the vi.fn and the next line mockedRequest.mockRejectedValue(new
ApiError("Request failed", 401, null)) immediately overwrites it; update the
test in client.test.ts by keeping only the meaningful
mockedRequest.mockRejectedValue(new ApiError(...)) call (refer to
mockedRequest.mockRejectedValue and ApiError in the test).
In `@src/lib/config.ts`:
- Around line 4-12: Replace the hardcoded export const PHONE_AUTH_ENABLED = true
with an environment-driven boolean: read a new env var (e.g.,
process.env.PHONE_AUTH_ENABLED) and parse it to a boolean with a sensible
default (true for staging or configurable via another env like NODE_ENV); update
the PHONE_AUTH_ENABLED constant to use that parsed value so the feature can be
toggled per environment while preserving current default behavior. Ensure
parsing handles "1"/"true"/"false"/"0" case-insensitively and falls back to the
existing default when unset.
In `@src/stores/realtime-match/match.slice.ts`:
- Around line 158-160: Add a short comment near setMatchCountdown (or in the
match slice state docs) explaining that when payload.reason === 'resume' the
reducer intentionally sets state.matchPaused = true and clears pauseUntil to
gate gameplay/UI during the resume countdown, and that this paused flag is NOT
cleared when the countdown reaches 0 but is cleared only when the external
socket event match:resume calls store.clearMatchPaused(); reference
state.matchPaused, pauseUntil, pausedAt, setMatchCountdown, and clearMatchPaused
in the comment so future readers understand the flow.
🪄 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 Plus
Run ID: ccfadd63-4b8f-4cc0-9dc4-bb46371a5106
📒 Files selected for processing (103)
.gitignoreCLAUDE.mdsrc/app/(app)/friend/room/[code]/page.tsxsrc/app/(app)/settings/__tests__/page.test.tsxsrc/app/(app)/settings/page.tsxsrc/app/(fullscreen)/dev/animations/page.tsxsrc/app/(fullscreen)/dev/bar-battle/page.tsxsrc/app/(fullscreen)/dev/penalties/page.tsxsrc/app/layout.tsxsrc/components/auth/OAuthCallbackScreen.tsxsrc/components/auth/__tests__/WelcomeScreen.test.tsxsrc/components/game/PossessionQuestionPanel.tsxsrc/components/layout/Sidebar.tsxsrc/components/layout/__tests__/AppShell.test.tsxsrc/components/layout/app-shell/useAppShellViewModel.tssrc/components/shared/BanCategoryCard.tsxsrc/components/shared/FriendPlayModal.tsxsrc/contexts/LocaleContext.tsxsrc/contexts/__tests__/LocaleContext.test.tsxsrc/features/daily/CareerPathGame.tsxsrc/features/daily/FootballLogicGame.tsxsrc/features/daily/daily-challenge.utils.tssrc/features/friend/FriendMatchHubPage.tsxsrc/features/friend/components/AlreadyInLobbyModal.tsxsrc/features/friend/components/CreateJoinPanel.tsxsrc/features/friend/components/FriendLobbyScreen.tsxsrc/features/friend/components/LobbyBrowsePanel.tsxsrc/features/friend/components/LobbyCard.tsxsrc/features/friend/components/LobbyHeader.tsxsrc/features/friend/components/LobbySettings.tsxsrc/features/friend/components/__tests__/CreateJoinPanel.test.tsxsrc/features/friend/hooks/__tests__/useFriendLobbyLogic.test.tsxsrc/features/friend/hooks/useFriendLobbyLogic.tssrc/features/friend/hooks/useLobbyCommandMachine.tssrc/features/play/ModeSelectionScreen.tsxsrc/features/possession/components/BarBattleOverlay.tsxsrc/features/possession/components/GoalProgressBar.tsxsrc/features/possession/components/HalftimeScreen.tsxsrc/features/possession/components/LiveSpecialQuestionPanel.tsxsrc/features/possession/components/PenaltyHUD.tsxsrc/features/possession/components/PenaltyStartCountdownOverlay.tsxsrc/features/possession/components/PitchVisualization.tsxsrc/features/possession/components/PossessionHUD.tsxsrc/features/possession/components/PossessionMatchViewport.tsxsrc/features/possession/components/__tests__/BarBattleOverlay.test.tsxsrc/features/possession/components/__tests__/HalftimeScreen.test.tsxsrc/features/possession/components/__tests__/PenaltyHUD.test.tsxsrc/features/possession/components/__tests__/PitchVisualization.test.tsxsrc/features/possession/components/__tests__/barBattle.helpers.test.tssrc/features/possession/components/bar-battle/BarBattleBar.tsxsrc/features/possession/components/bar-battle/BarBattleBars.tsxsrc/features/possession/components/bar-battle/BarBattleStackedBar.tsxsrc/features/possession/components/bar-battle/barBattle.helpers.tssrc/features/possession/components/bar-battle/barBattle.types.tssrc/features/possession/components/bar-battle/useBarBattleViewModel.tssrc/features/possession/components/live-special/shared.tsxsrc/features/possession/components/pitch/PitchHtmlActors.tsxsrc/features/possession/components/pitch/usePitchSceneModel.tssrc/features/possession/hooks/__tests__/useBarBattle.test.tssrc/features/possession/hooks/__tests__/usePossessionBarBattleFlights.test.tssrc/features/possession/hooks/useBarBattle.tssrc/features/possession/hooks/usePossessionAnimationOrchestrator.tssrc/features/possession/hooks/usePossessionBarBattleFlights.tssrc/features/possession/hooks/usePossessionFieldState.tssrc/features/possession/hooks/usePossessionGoalCelebration.tssrc/features/possession/hooks/usePossessionMatchSounds.tssrc/features/possession/hooks/usePossessionRoundTransition.tssrc/features/possession/hooks/useRealtimePossessionMatchController.tssrc/features/settings/SettingsScreen.tsxsrc/features/settings/components/SettingsSection.tsxsrc/features/training/TrainingMatchProvider.tsxsrc/features/welcome/WelcomeAuthNoticeModal.tsxsrc/features/welcome/WelcomeScreen.tsxsrc/features/welcome/useWelcomeAuthController.tssrc/lib/analytics/game-events.tssrc/lib/api/__tests__/client.test.tssrc/lib/api/client.tssrc/lib/auth/__tests__/auth.refresh.test.tssrc/lib/auth/__tests__/postAuthRedirect.test.tssrc/lib/auth/auth.service.tssrc/lib/auth/postAuthRedirect.tssrc/lib/config.tssrc/lib/friend/inviteCode.tssrc/lib/geo/__tests__/countryCode.test.tssrc/lib/geo/countries.tssrc/lib/geo/resolveLocation.tssrc/lib/match/__tests__/useRealtimeGameLogic.test.tssrc/lib/match/useRealtimeGameLogic.tssrc/lib/queries/__tests__/leaderboard.queries.test.tsxsrc/lib/queries/__tests__/social.queries.test.tsxsrc/lib/queries/leaderboard.queries.tssrc/lib/queries/social.queries.tssrc/lib/realtime/socket-client.tssrc/lib/realtime/socket.types.tssrc/lib/sounds/__tests__/gameSounds.test.tssrc/lib/sounds/gameSounds.tssrc/lib/usePreloadImages.tssrc/messages/en.jsonsrc/messages/ka.jsonsrc/stores/__tests__/realtimeMatch.store.test.tssrc/stores/realtime-match/match.slice.tssrc/test/setup.tssrc/types/api.generated.ts
💤 Files with no reviewable changes (2)
- CLAUDE.md
- src/features/daily/daily-challenge.utils.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/messages/ka.json (1)
48-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVersion string mismatch between locales.
The Georgian locale shows version
0.9.15 (ბეტა)while the English locale (line 114 inen.json) showsVersion 0.9.5 (Beta). Version strings should be identical across all locales to maintain consistency in user-facing displays and simplify release tracking.🔧 Suggested fix to align with English version
- "version": "ვერსია 0.9.15 (ბეტა)", + "version": "ვერსია 0.9.5 (ბეტა)",Alternatively, if 0.9.15 is correct, update
en.jsonline 114 to match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/messages/ka.json` at line 48, The "version" value in src/messages/ka.json ("ვერსია 0.9.15 (ბეტა)") is inconsistent with the English locale; update the "version" key in either src/messages/ka.json or src/messages/en.json so both locales use the identical version string (e.g., change the "version" value in src/messages/ka.json to match the English "version" value from src/messages/en.json or vice versa if 0.9.15 is the intended release).
🧹 Nitpick comments (2)
src/features/settings/SettingsScreen.tsx (1)
507-514: ⚡ Quick winThe
last-childclose-button hack is fragile in both dialogs.Both dialogs rely on
[&>button:last-child]:hiddento suppress the defaultDialogContentclose button. That only works while the wrapper keeps rendering its internal close button as the last child. Please switch this to a wrapper-level API instead of reaching into its DOM structure from each caller.Also applies to: 601-608
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/settings/SettingsScreen.tsx` around lines 507 - 514, The DialogContent consumer is using a fragile CSS selector "[&>button:last-child]:hidden" to hide the internal close button; instead add a wrapper-level prop (e.g., hideDefaultCloseButton: boolean) to the DialogContent component and use that prop inside DialogContent to conditionally render or omit its default close button. Update callers (the SettingsScreen usages around the ModalCloseButton and the other dialog at lines ~601-608) to remove the CSS hack and pass hideDefaultCloseButton={true}, retaining the external ModalCloseButton and the onClose logic that checks isUpdatingPhone and calls setPhoneDialogOpen(false).src/features/profile/components/AvatarPicker.tsx (1)
437-438: ⚡ Quick winAvoid hiding the default dialog close button via
last-child.
[&>button:last-child]:hiddencouples this screen toDialogContent’s current internal DOM order. Ifsrc/components/ui/dialog.tsxever adds another trailing button or reorders children, this will hide the wrong element and the duplicate close button comes back. A first-classhideDefaultCloseButtonprop on the dialog wrapper would make this safe.♻️ Safer direction
<DialogContent hideDefaultCloseButton className="sm:max-w-3xl max-h-[90dvh] overflow-y-auto rounded-[24px] border-0 bg-brand-blue p-6 sm:p-8"> <ModalCloseButton onClose={() => onOpenChange(false)} /> ... </DialogContent>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/profile/components/AvatarPicker.tsx` around lines 437 - 438, Remove the fragile CSS selector [&>button:last-child]:hidden from the DialogContent usage and instead pass a first-class prop to explicitly hide the built-in close button (e.g., <DialogContent hideDefaultCloseButton ...>); keep the custom ModalCloseButton(onClose={() => onOpenChange(false)}) inside the DialogContent. Update the DialogContent usage here (reference symbol: DialogContent) to include hideDefaultCloseButton and remove the selector, and if needed add support for that prop in the dialog wrapper implementation so it suppresses the default close button rather than relying on DOM order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/shared/ModalCloseButton.tsx`:
- Around line 40-44: In ModalCloseButton (component ModalCloseButton), replace
the Tailwind classes that hide the browser focus outline: change "outline-none
focus:outline-none" to "outline-hidden focus:outline-hidden" while keeping the
existing focus-visible:ring-* classes intact; update the class string used in
the close button element (where outline-none appears) so
forced-colors/high-contrast users retain an accessible focus indicator.
In `@src/features/welcome/WelcomeGoogleButton.tsx`:
- Around line 70-72: The Button in WelcomeGoogleButton.tsx currently strips
keyboard focus styles (it contains focus-visible:ring-0 and
focus-visible:outline-none); restore a visible focus indicator by removing those
neutralizing classes and replacing them with an accessible focus-visible style
(for example use focus-visible:ring-2 or focus-visible:outline-2 plus a
high-contrast color like focus-visible:ring-black or
focus-visible:ring-brand-yellow-deep) on the same Button element (the one using
onClick and the long className) so keyboard users can see focus.
In `@src/stores/auth.store.ts`:
- Around line 129-143: Both terminal branches that clear tokens and set
anonymous state (the branch under isAuthFailure(retryError) and the branch
checking refreshed.terminal) must also call resetUser() to reset analytics
identity; add a call to resetUser() immediately after clearLocalSession() and
before set({ status: "anonymous", user: null, hasBootstrapped: true }) in both
places so the PostHog identity is cleared when bootstrap forces an anonymous
session.
- Around line 146-154: The retry catch currently logs bootstrapUser() failures
and leaves the store in "loading"; if the error is an HTTP 401 we must treat it
as terminal: detect a 401 from retryError (e.g. retryError.status === 401 or
retryError.response?.status === 401), clear any local session/auth tokens (call
the existing session clear function or remove stored credentials), set({ status:
"anonymous", user: null, hasBootstrapped: true }) and log the terminal outcome
instead of leaving "loading"; for non-401 errors keep the existing warn/log and
set({ status: "loading" }) behavior. Ensure you reference bootstrapUser,
refreshSession, set, and logger.warn/logger.info when making the change.
---
Outside diff comments:
In `@src/messages/ka.json`:
- Line 48: The "version" value in src/messages/ka.json ("ვერსია 0.9.15 (ბეტა)")
is inconsistent with the English locale; update the "version" key in either
src/messages/ka.json or src/messages/en.json so both locales use the identical
version string (e.g., change the "version" value in src/messages/ka.json to
match the English "version" value from src/messages/en.json or vice versa if
0.9.15 is the intended release).
---
Nitpick comments:
In `@src/features/profile/components/AvatarPicker.tsx`:
- Around line 437-438: Remove the fragile CSS selector
[&>button:last-child]:hidden from the DialogContent usage and instead pass a
first-class prop to explicitly hide the built-in close button (e.g.,
<DialogContent hideDefaultCloseButton ...>); keep the custom
ModalCloseButton(onClose={() => onOpenChange(false)}) inside the DialogContent.
Update the DialogContent usage here (reference symbol: DialogContent) to include
hideDefaultCloseButton and remove the selector, and if needed add support for
that prop in the dialog wrapper implementation so it suppresses the default
close button rather than relying on DOM order.
In `@src/features/settings/SettingsScreen.tsx`:
- Around line 507-514: The DialogContent consumer is using a fragile CSS
selector "[&>button:last-child]:hidden" to hide the internal close button;
instead add a wrapper-level prop (e.g., hideDefaultCloseButton: boolean) to the
DialogContent component and use that prop inside DialogContent to conditionally
render or omit its default close button. Update callers (the SettingsScreen
usages around the ModalCloseButton and the other dialog at lines ~601-608) to
remove the CSS hack and pass hideDefaultCloseButton={true}, retaining the
external ModalCloseButton and the onClose logic that checks isUpdatingPhone and
calls setPhoneDialogOpen(false).
🪄 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 Plus
Run ID: 3c523b5c-d3a2-4fa3-8e06-4238f2a72734
📒 Files selected for processing (23)
src/components/auth/OAuthCallbackScreen.tsxsrc/components/auth/__tests__/OAuthCallbackScreen.test.tsxsrc/components/auth/__tests__/WelcomeScreen.test.tsxsrc/components/shared/ModalCloseButton.tsxsrc/features/friend/components/CreateJoinPanel.tsxsrc/features/friend/components/LobbyBrowsePanel.tsxsrc/features/profile/components/AvatarPicker.tsxsrc/features/settings/SettingsScreen.tsxsrc/features/welcome/InAppBrowserInstructions.tsxsrc/features/welcome/WelcomeGoogleButton.tsxsrc/features/welcome/WelcomeLoginDialog.tsxsrc/features/welcome/WelcomePhoneAuthForm.tsxsrc/features/welcome/WelcomeScreen.tsxsrc/features/welcome/useWelcomeAuthController.tssrc/lib/auth/auth.service.tssrc/lib/auth/google-identity.tssrc/lib/auth/in-app-browser.tssrc/lib/auth/useGeorgianPhoneAuthAvailability.tssrc/lib/i18n/copy.tssrc/messages/en.jsonsrc/messages/ka.jsonsrc/stores/__tests__/auth.store.test.tssrc/stores/auth.store.ts
💤 Files with no reviewable changes (2)
- src/lib/auth/in-app-browser.ts
- src/features/welcome/InAppBrowserInstructions.tsx
✅ Files skipped from review due to trivial changes (1)
- src/lib/i18n/copy.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/features/friend/components/LobbyBrowsePanel.tsx
- src/features/welcome/WelcomeScreen.tsx
- src/components/auth/OAuthCallbackScreen.tsx
- src/features/friend/components/CreateJoinPanel.tsx
- src/lib/auth/auth.service.ts
- src/components/auth/tests/WelcomeScreen.test.tsx
- src/features/welcome/useWelcomeAuthController.ts
- Google/Facebook buttons: show a loading spinner + disable on press so a tap always gives immediate feedback while the token exchange / OAuth redirect runs. - Google: GIS-everywhere — the overlaid Google button now covers the whole button so any tap runs the in-page token flow (works in Safari/Chrome and in-app browsers); classic redirect is kept only as a fallback when GIS can't load. - OAuth callback: a cancelled/denied consent (error in the query OR hash) is no longer treated as a failure — it returns to the landing page instead of logging an error and flashing the failure screen. - Possession bar: the 100 goal-threshold label is green instead of red. - Tweak the OAuth 'finalizing' loading copy.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/messages/en.json (1)
686-686:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the product-name casing in this restore message.
Quizballis inconsistent with the rest of the UI branding and should stayQuizBall.✏️ Suggested copy fix
- "restoreAccountDescription": "This account is scheduled for deletion. Restoring it will cancel deletion and let you continue using Quizball.", + "restoreAccountDescription": "This account is scheduled for deletion. Restoring it will cancel deletion and let you continue using QuizBall.",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/messages/en.json` at line 686, The string value for the localization key "restoreAccountDescription" uses incorrect product-name casing ("Quizball"); update the value to use the correct branding "QuizBall" so the message reads: This account is scheduled for deletion. Restoring it will cancel deletion and let you continue using QuizBall. Ensure you only change the product name in the "restoreAccountDescription" entry in src/messages/en.json.src/features/daily/__tests__/CountdownGame.test.tsx (1)
86-100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWait past the debounce before asserting the answer stays hidden.
Line 96 only schedules the debounced auto-check path, so the assertion on Line 99 runs before the changed behavior can fire. A delayed reveal regression would still pass this test.
🧪 Suggested test fix
- it('does not leak answers as suggestions while typing a partial', () => { + it('does not leak answers as suggestions while typing a partial', async () => { + vi.useFakeTimers(); render( <CountdownGame session={session as never} onBack={vi.fn()} onComplete={vi.fn()} /> ); const input = screen.getByPlaceholderText('Press Enter to submit...'); fireEvent.change(input, { target: { value: 'rona' } }); + await vi.advanceTimersByTimeAsync(200); // The full answer must NOT be revealed before it's accepted. expect(screen.queryByText('Cristiano Ronaldo')).not.toBeInTheDocument(); + vi.useRealTimers(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/daily/__tests__/CountdownGame.test.tsx` around lines 86 - 100, The test for CountdownGame is asserting too soon—after fireEvent.change the component schedules a debounced auto-check, so the assertion that "Cristiano Ronaldo" is not revealed must wait until the debounce window has elapsed; update the test around the input interaction in CountdownGame.test.tsx to advance timers or await the component (e.g., use act + jest.advanceTimersByTime(debounceMs) or await waitFor with a timeout) after calling fireEvent.change so the debounced path (the component's auto-check logic) has a chance to run before asserting that the full answer is not in the document.src/features/play/ModeSelectionScreen.tsx (1)
103-145:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix entrance animation gating to avoid SSR permanently disabling it
shouldPlayEntranceAnimation()returnsfalsewhenwindowis undefined, souseState(shouldPlayEntranceAnimation)initializesplayEntranceAnimationtofalseduring SSR. That keeps themotion.divin the non-animated path (initial=false,transition.duration=0) and theuseEffectnever setsPLAY_ENTRANCE_SESSION_KEY, so a hard refresh won’t trigger the “once per browser session” animation when the key is missing. Move the sessionStorage read and derived animation state into a mounted effect (or gate rendering on mount) so the client can decide after hydration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/play/ModeSelectionScreen.tsx` around lines 103 - 145, The entrance animation flag is being initialized during SSR because useState(shouldPlayEntranceAnimation) calls window-dependent logic; instead initialize playEntranceAnimation to a safe server value (e.g. false/undefined) and determine the true client-side value inside a mounting effect: in an effect run on mount call shouldPlayEntranceAnimation() and read PLAY_ENTRANCE_SESSION_KEY from sessionStorage (catching errors) and then setPlayEntranceAnimation according to those results (and set the session key when animation should play). Update any logic that reads playEntranceAnimation (e.g. motion.div props and the existing useEffect that writes PLAY_ENTRANCE_SESSION_KEY) to rely on the client-set state so SSR does not permanently disable the animation.src/features/welcome/useWelcomeAuthController.ts (1)
193-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSplit GIS acquisition from the in-place Google login path.
This
try/catchis too broad. OncesignInWithGoogleIdentity()succeeds, a successful in-place login returns without clearingsocialSubmittingor closing the dialog, and a laterbootstrap()failure is treated as “GIS unavailable” and falls back into a second redirect-based OAuth attempt. Keep the GIS acquisition fallback separate from the token-exchange/bootstrap path.Proposed fix
if (googleClientId) { let googleIdentity: { idToken: string; nonce?: string } | null = null; try { googleIdentity = await signInWithGoogleIdentity(googleClientId); - await socialLoginWithIdToken('google', googleIdentity.idToken, googleIdentity.nonce); - await bootstrap({ force: true }); - return; } catch (gisError) { - if (googleIdentity && isPendingDeletionAuthError(gisError)) { - setPendingRestoreAction({ - kind: 'social-token', - provider: 'google', - idToken: googleIdentity.idToken, - nonce: googleIdentity.nonce, - }); - setAuthNoticeModal('pending-deletion'); - setLoginOpen(true); - setSocialSubmitting(null); - return; - } console.warn('GIS sign-in unavailable, falling back to redirect', gisError); } + + if (googleIdentity) { + try { + await socialLoginWithIdToken('google', googleIdentity.idToken, googleIdentity.nonce); + await bootstrap({ force: true }); + setLoginOpen(false); + setSocialSubmitting(null); + return; + } catch (error) { + if (isPendingDeletionAuthError(error)) { + setPendingRestoreAction({ + kind: 'social-token', + provider: 'google', + idToken: googleIdentity.idToken, + nonce: googleIdentity.nonce, + }); + setAuthNoticeModal('pending-deletion'); + setLoginOpen(true); + setSocialSubmitting(null); + return; + } + console.error('Google login failed', error); + setSocialSubmitting(null); + return; + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/welcome/useWelcomeAuthController.ts` around lines 193 - 225, Split the GIS acquisition and the token-exchange/bootstrap flows: first wrap only signInWithGoogleIdentity(googleClientId) in its own try/catch so failures there trigger the redirect fallback via socialLogin('google', redirectTo); once signInWithGoogleIdentity returns a googleIdentity, perform socialLoginWithIdToken('google', googleIdentity.idToken, googleIdentity.nonce) and await bootstrap({ force: true }) inside a separate try/catch so bootstrap failures are handled locally (setSocialSubmitting(null), show an error modal or keep dialog open) and do not fall through to the redirect path; ensure on successful in-place login you clear socialSubmitting and close the dialog (call setSocialSubmitting(null) and setLoginOpen(false)) and return to prevent executing the redirect block; keep existing pending-deletion handling (isPendingDeletionAuthError) after socialLoginWithIdToken as-is using setPendingRestoreAction and setAuthNoticeModal.
🧹 Nitpick comments (2)
src/app/(app)/daily/challenges/[challengeId]/page.tsx (1)
54-61: ⚡ Quick winAvoid invalidating the daily-challenge list twice.
useCompleteDailyChallenge()already invalidatesqueryKeys.dailyChallenges.allinonSettled, so this helper guarantees a second refetch on every completion. Keeping the wallet invalidation here is fine, but the daily-challenge invalidation should live in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/daily/challenges/[challengeId]/page.tsx around lines 54 - 61, The helper invalidateAfterComplete is redundantly invalidating the daily-challenge list; remove the queryClient.invalidateQueries({ queryKey: queryKeys.dailyChallenges.all }) call from invalidateAfterComplete and only keep the wallet invalidation (queryClient.invalidateQueries({ queryKey: queryKeys.store.wallet() })); rely on useCompleteDailyChallenge's onSettled handler to perform the daily-challenge list invalidation so you don't trigger a duplicate refetch. Ensure Promise.all (or the surrounding async logic) is updated accordingly to only include the wallet invalidation.src/components/shared/BanCategoryCard.tsx (1)
103-110: ⚡ Quick winDon't mark every card image as eager/high-priority.
These cards render in groups, so
loading="eager"plusfetchPriority="high"on all of them makes every artwork compete with more important above-the-fold assets. The browser defaults are a better fit here unless you're intentionally boosting a single hero image.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/shared/BanCategoryCard.tsx` around lines 103 - 110, The card image in BanCategoryCard.tsx is marked with loading="eager" and fetchPriority="high", causing every card to compete with above-the-fold assets; remove those attributes (or set loading="lazy" if you want deferred loading) from the <img> in the BanCategoryCard component so only intentional hero images are prioritized, or add a prop to selectively enable eager/high priority for a single hero image if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/auth/OAuthCallbackScreen.tsx`:
- Around line 54-64: The current OAuthCallbackScreen branch treats any provider
"error" as a benign cancellation; update the oauth error handling to only
silently handle known user-cancel codes (e.g., "access_denied",
"consent_required", "user_denied" — use whatever provider-specific cancellation
codes your app expects) by checking oauthError against a small whitelist before
logging the benign message, replacing history, and calling router.replace("/");
if oauthError is not in that whitelist, do not return here so the existing
failure handling runs and surfaces real OAuth/server errors. Locate this logic
around the variables hashParams, oauthError, logger, and router.replace in
OAuthCallbackScreen and implement the whitelist check.
In `@src/features/daily/CountdownGame.tsx`:
- Around line 71-88: handleRoundEnd can run twice and both calls will read the
same captured currentRound/foundAnswers, causing duplicate appends and
overshooting currentRound; make the transition atomic by using a functional
updater on setCurrentRound that examines the previous round and only
advances/marks finished once, and perform the append to setAllRoundAnswers and
setFinished based on that previous value (e.g., inside the updater or
immediately after checking prev in a single synchronous branch), ensuring you
still use setAllRoundAnswers(prev => [...prev, foundAnswers]) and clear
foundAnswers/input only when the update actually happened; update handleRoundEnd
to rely on prev values (not captured currentRound) to be idempotent.
In `@src/features/daily/dailyChallengeSessionPrefetch.ts`:
- Around line 39-45: The cache currently treats an in-flight entry stale by
comparing now - existing.createdAt against TTL_MS and can trigger duplicate
POSTs; change the logic so pending promises are never expired: when reading
cache (in prefetchDailyChallengeSession and consumeDailyChallengeSession) always
return existing.promise if existing.promise is still pending, and only apply TTL
eviction based on a resolvedAt timestamp set when the promise resolves (or set
createdAt on resolution inside the promise chain that wraps
createDailyChallengeSession(...).then(toDailyChallengeSession)). Update
references to existing.createdAt, entry.promise and TTL_MS and ensure
createDailyChallengeSession and toDailyChallengeSession are wrapped so
resolution sets the freshness timestamp instead of using the original insertion
time.
In `@src/features/daily/MoneyDropGame.tsx`:
- Around line 251-255: The auto-advance timeout set in the block using
autoAdvanceRef.current and setTimeout can still fire after a manual advance;
modify the logic to store the timeout id (e.g., in a new ref like
autoAdvanceTimeoutRef), clearTimeout(autoAdvanceTimeoutRef.current) when the
player manually advances (in the handler that calls
handleNextQuestionRef.current), and set autoAdvanceTimeoutRef.current =
setTimeout(...) when scheduling the auto-advance; also reset/clear the ref after
the timeout callback runs to avoid leaking stale ids.
In `@src/features/possession/components/HalftimeScreen.tsx`:
- Around line 269-272: The hardcoded title "Ban 1 Category Each" in the
HalftimeScreen component should be localized: update the div that currently
renders that literal to use the app's i18n translation function (e.g. call
t('possession.banTitle') or the project's equivalent) and ensure useTranslation
or the project's translation hook is imported and used in HalftimeScreen; also
add the corresponding translation key/value to the locale files for all
supported languages.
In `@src/features/profile/components/AvatarPicker.tsx`:
- Around line 259-264: In AvatarPicker, the empty-option label is hardcoded as
"None" inside the span and must be localized; update the AvatarPicker component
to use the app's localization helper (e.g., useTranslation() / t or i18n.t) and
replace the literal "None" in the span with the localized key (add a suitable
key like "avatar.none" to the translation files if missing) so the picker is
fully translated.
In `@src/features/welcome/WelcomeFacebookButton.tsx`:
- Around line 21-34: The button loses its accessible name when submitting
because the loading branch only renders <Loader2>; preserve the label by adding
aria-label={t('welcome.continueWithFacebook')} to the Button (or include an
offscreen span with the same t('welcome.continueWithFacebook') text inside the
submitting branch). Update the component where Button is rendered (the
submitting conditional that shows Loader2) so the Button keeps an accessible
name while submitting; use the existing i18n key
t('welcome.continueWithFacebook') to ensure consistency.
---
Outside diff comments:
In `@src/features/daily/__tests__/CountdownGame.test.tsx`:
- Around line 86-100: The test for CountdownGame is asserting too soon—after
fireEvent.change the component schedules a debounced auto-check, so the
assertion that "Cristiano Ronaldo" is not revealed must wait until the debounce
window has elapsed; update the test around the input interaction in
CountdownGame.test.tsx to advance timers or await the component (e.g., use act +
jest.advanceTimersByTime(debounceMs) or await waitFor with a timeout) after
calling fireEvent.change so the debounced path (the component's auto-check
logic) has a chance to run before asserting that the full answer is not in the
document.
In `@src/features/play/ModeSelectionScreen.tsx`:
- Around line 103-145: The entrance animation flag is being initialized during
SSR because useState(shouldPlayEntranceAnimation) calls window-dependent logic;
instead initialize playEntranceAnimation to a safe server value (e.g.
false/undefined) and determine the true client-side value inside a mounting
effect: in an effect run on mount call shouldPlayEntranceAnimation() and read
PLAY_ENTRANCE_SESSION_KEY from sessionStorage (catching errors) and then
setPlayEntranceAnimation according to those results (and set the session key
when animation should play). Update any logic that reads playEntranceAnimation
(e.g. motion.div props and the existing useEffect that writes
PLAY_ENTRANCE_SESSION_KEY) to rely on the client-set state so SSR does not
permanently disable the animation.
In `@src/features/welcome/useWelcomeAuthController.ts`:
- Around line 193-225: Split the GIS acquisition and the
token-exchange/bootstrap flows: first wrap only
signInWithGoogleIdentity(googleClientId) in its own try/catch so failures there
trigger the redirect fallback via socialLogin('google', redirectTo); once
signInWithGoogleIdentity returns a googleIdentity, perform
socialLoginWithIdToken('google', googleIdentity.idToken, googleIdentity.nonce)
and await bootstrap({ force: true }) inside a separate try/catch so bootstrap
failures are handled locally (setSocialSubmitting(null), show an error modal or
keep dialog open) and do not fall through to the redirect path; ensure on
successful in-place login you clear socialSubmitting and close the dialog (call
setSocialSubmitting(null) and setLoginOpen(false)) and return to prevent
executing the redirect block; keep existing pending-deletion handling
(isPendingDeletionAuthError) after socialLoginWithIdToken as-is using
setPendingRestoreAction and setAuthNoticeModal.
In `@src/messages/en.json`:
- Line 686: The string value for the localization key
"restoreAccountDescription" uses incorrect product-name casing ("Quizball");
update the value to use the correct branding "QuizBall" so the message reads:
This account is scheduled for deletion. Restoring it will cancel deletion and
let you continue using QuizBall. Ensure you only change the product name in the
"restoreAccountDescription" entry in src/messages/en.json.
---
Nitpick comments:
In `@src/app/`(app)/daily/challenges/[challengeId]/page.tsx:
- Around line 54-61: The helper invalidateAfterComplete is redundantly
invalidating the daily-challenge list; remove the
queryClient.invalidateQueries({ queryKey: queryKeys.dailyChallenges.all }) call
from invalidateAfterComplete and only keep the wallet invalidation
(queryClient.invalidateQueries({ queryKey: queryKeys.store.wallet() })); rely on
useCompleteDailyChallenge's onSettled handler to perform the daily-challenge
list invalidation so you don't trigger a duplicate refetch. Ensure Promise.all
(or the surrounding async logic) is updated accordingly to only include the
wallet invalidation.
In `@src/components/shared/BanCategoryCard.tsx`:
- Around line 103-110: The card image in BanCategoryCard.tsx is marked with
loading="eager" and fetchPriority="high", causing every card to compete with
above-the-fold assets; remove those attributes (or set loading="lazy" if you
want deferred loading) from the <img> in the BanCategoryCard component so only
intentional hero images are prioritized, or add a prop to selectively enable
eager/high priority for a single hero image if needed.
🪄 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 Plus
Run ID: b736bb0b-c10f-491a-9d84-77347cd4ce3e
⛔ Files ignored due to path filters (2)
public/sounds/correct_answer.mp3is excluded by!**/*.mp3public/sounds/wrong_answer.mp3is excluded by!**/*.mp3
📒 Files selected for processing (38)
src/app/(app)/daily/challenges/[challengeId]/page.tsxsrc/app/(app)/daily/challenges/page.tsxsrc/components/auth/OAuthCallbackScreen.tsxsrc/components/layout/app-shell/useAppShellViewModel.tssrc/components/shared/BanCategoryCard.tsxsrc/components/shared/FriendPlayModal.tsxsrc/features/daily/CareerPathGame.tsxsrc/features/daily/ClueGame.tsxsrc/features/daily/CountdownGame.tsxsrc/features/daily/FootballLogicGame.tsxsrc/features/daily/ImposterGame.tsxsrc/features/daily/MoneyDropGame.tsxsrc/features/daily/PutInOrderGame.tsxsrc/features/daily/__tests__/CountdownGame.test.tsxsrc/features/daily/components/DailyChallengeIntro.tsxsrc/features/daily/components/useResultSplash.tssrc/features/daily/dailyChallengeSessionPrefetch.tssrc/features/friend/components/CreateJoinPanel.tsxsrc/features/leaderboard/LeaderboardScreen.tsxsrc/features/play/ModeSelectionScreen.tsxsrc/features/play/RankedCategoryBlockingScreen.tsxsrc/features/possession/components/GoalProgressBar.tsxsrc/features/possession/components/HalftimeScreen.tsxsrc/features/possession/hooks/usePossessionMatchSounds.tssrc/features/profile/components/AvatarPicker.tsxsrc/features/store/StoreScreen.tsxsrc/features/welcome/WelcomeFacebookButton.tsxsrc/features/welcome/WelcomeGoogleButton.tsxsrc/features/welcome/WelcomeLoginDialog.tsxsrc/features/welcome/WelcomeScreen.tsxsrc/features/welcome/useWelcomeAuthController.tssrc/lib/avatars/partNames.tssrc/lib/avatars/parts.tssrc/lib/queries/dailyChallenges.queries.tssrc/lib/sounds/gameSounds.tssrc/messages/en.jsonsrc/messages/ka.jsonsrc/styles/globals.css
✅ Files skipped from review due to trivial changes (2)
- src/features/leaderboard/LeaderboardScreen.tsx
- src/lib/avatars/parts.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/lib/sounds/gameSounds.ts
- src/features/daily/CareerPathGame.tsx
- src/features/daily/FootballLogicGame.tsx
- src/features/welcome/WelcomeScreen.tsx
- src/features/welcome/WelcomeGoogleButton.tsx
- src/features/welcome/WelcomeLoginDialog.tsx
- src/components/shared/FriendPlayModal.tsx
- src/messages/ka.json
Contributor
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 (3)
src/messages/en.json (1)
686-686:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent brand name capitalization.
"Quizball" should be "QuizBall" to match the branding used elsewhere in the file (e.g., copyright on line 643).
Suggested fix
- "restoreAccountDescription": "This account is scheduled for deletion. Restoring it will cancel deletion and let you continue using Quizball.", + "restoreAccountDescription": "This account is scheduled for deletion. Restoring it will cancel deletion and let you continue using QuizBall.",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/messages/en.json` at line 686, The value for the key "restoreAccountDescription" uses inconsistent branding ("Quizball"); update the string value to use the correct capitalization "QuizBall" so it matches other entries (e.g., the copyright) — locate the "restoreAccountDescription" entry and replace "Quizball" with "QuizBall".src/messages/ka.json (2)
48-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVersion mismatch between locale files.
The Georgian locale shows version
0.9.15while the English locale (en.json line 114) shows0.9.5. These should be consistent across all locale files to avoid user confusion.Suggested fix (assuming 0.9.5 is correct)
- "version": "ვერსია 0.9.15 (ბეტა)", + "version": "ვერსია 0.9.5 (ბეტა)",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/messages/ka.json` at line 48, Update the Georgian locale's "version" value to match the canonical version used in other locales (change the "version" key value in src/messages/ka.json from "ვერსია 0.9.15 (ბეტა)" to the correct canonical string used in en.json, e.g., "ვერსია 0.9.5 (ბეტა)") so the "version" key is consistent across locale files; ensure you update the exact "version" JSON key in ka.json to the same text used in en.json.
284-284:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent brand name capitalization.
Same issue as in en.json — "Quizball" should be "QuizBall" to match the product branding.
Suggested fix
- "restoreAccountDescription": "ეს ანგარიში წაშლის რიგშია. აღდგენა გააუქმებს წაშლას და Quizball-ის გამოყენებას გააგრძელებთ.", + "restoreAccountDescription": "ეს ანგარიში წაშლის რიგშია. აღდგენა გააუქმებს წაშლას და QuizBall-ის გამოყენებას გააგრძელებთ.",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/messages/ka.json` at line 284, The string value for the key restoreAccountDescription contains the incorrectly capitalized product name "Quizball"; update the value so the brand matches the canonical "QuizBall" (i.e., change "Quizball-ის" to "QuizBall-ის") in the restoreAccountDescription entry to ensure consistent branding across locales.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/messages/en.json`:
- Line 686: The value for the key "restoreAccountDescription" uses inconsistent
branding ("Quizball"); update the string value to use the correct capitalization
"QuizBall" so it matches other entries (e.g., the copyright) — locate the
"restoreAccountDescription" entry and replace "Quizball" with "QuizBall".
In `@src/messages/ka.json`:
- Line 48: Update the Georgian locale's "version" value to match the canonical
version used in other locales (change the "version" key value in
src/messages/ka.json from "ვერსია 0.9.15 (ბეტა)" to the correct canonical string
used in en.json, e.g., "ვერსია 0.9.5 (ბეტა)") so the "version" key is consistent
across locale files; ensure you update the exact "version" JSON key in ka.json
to the same text used in en.json.
- Line 284: The string value for the key restoreAccountDescription contains the
incorrectly capitalized product name "Quizball"; update the value so the brand
matches the canonical "QuizBall" (i.e., change "Quizball-ის" to "QuizBall-ის")
in the restoreAccountDescription entry to ensure consistent branding across
locales.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 456cb91c-c186-4dfe-a9ef-0c38350b22f2
📒 Files selected for processing (19)
src/components/auth/OAuthCallbackScreen.tsxsrc/components/auth/__tests__/WelcomeScreen.test.tsxsrc/components/shared/ModalCloseButton.tsxsrc/features/daily/CountdownGame.tsxsrc/features/daily/MoneyDropGame.tsxsrc/features/friend/FriendMatchHubPage.tsxsrc/features/play/RankedCategoryBlockingScreen.tsxsrc/features/possession/components/HalftimeScreen.tsxsrc/features/possession/hooks/__tests__/usePossessionMatchSounds.test.tssrc/features/profile/components/AvatarPicker.tsxsrc/features/welcome/WelcomeFacebookButton.tsxsrc/features/welcome/WelcomeGoogleButton.tsxsrc/features/welcome/useWelcomeAuthController.tssrc/lib/api/__tests__/client.test.tssrc/lib/api/client.tssrc/lib/auth/google-identity.tssrc/messages/en.jsonsrc/messages/ka.jsonsrc/stores/auth.store.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- src/features/welcome/WelcomeFacebookButton.tsx
- src/components/shared/ModalCloseButton.tsx
- src/features/possession/hooks/tests/usePossessionMatchSounds.test.ts
- src/lib/auth/google-identity.ts
- src/lib/api/client.ts
- src/features/possession/components/HalftimeScreen.tsx
- src/stores/auth.store.ts
- src/features/friend/FriendMatchHubPage.tsx
- src/features/profile/components/AvatarPicker.tsx
- src/components/auth/tests/WelcomeScreen.test.tsx
- src/features/daily/CountdownGame.tsx
- src/features/welcome/useWelcomeAuthController.ts
- src/components/auth/OAuthCallbackScreen.tsx
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
Improvements
Tests