Minigame Overhaul, Leaderboard System, and Onboarding/UI Improvements - #527
Conversation
|
Caution Review failedThe pull request is closed. Note
|
| Cohort / File(s) | Summary |
|---|---|
Backend: Minigame Controller backend/src/controllers/minigameController.js |
New/extended game types; controllers derive walletAddress from auth/params/queries, accept limit for leaderboards, and forward wallet info to service calls. |
Backend: Minigame Service backend/src/services/minigameService.js |
Sessions now store player_id (resolved from wallet), endGameSession(sessionId, finalScore, gameType) computes XP, added aggregated per-player game & global leaderboards, getGameSession, getGameTypes, getGameLeaderboard, and responses now surface player_wallet. |
Backend: CORS backend/src/index.js |
Production CORS origins expanded to include https://localhost:5173. |
Frontend: App routing & Component exports client/src/App.tsx, client/src/components/index.ts, client/src/components/leaderboard/index.ts |
New /leaderboard route; leaderboard module re-exported in component barrels. |
Frontend: Leaderboard UI & Page client/src/components/leaderboard/leaderboard.tsx, client/src/pages/leaderboard.tsx |
New Leaderboard component and LeaderboardPage supporting per-game/global views, top-3 styling, truncated wallet display, and configurable limits/titles. |
Frontend: API config client/src/config/api.ts |
Added API_CONFIG.ENDPOINTS.MINIGAMES endpoints (sessions, session end, player stats, leaderboards, types, bonus XP). |
Frontend: Hooks & API surface client/src/hooks/use-minigame-api.ts, client/src/hooks/use-game-score-submission.ts, client/src/hooks/index.ts |
New useMinigameApi (session lifecycle, leaderboards, stats, game types) and useGameScoreSubmission hooks; both exported from hooks barrel. |
Frontend: Game integrations client/src/components/mini-games/floppy-fish/floppy-fish-game-canvas.tsx, client/src/components/mini-games/fish-dodge-game.tsx, client/src/pages/bubble-jumper.tsx |
Wired score-submission via useGameScoreSubmission into floppy-fish, fish-dodge, and bubble-jumper flows; prevented duplicate submissions. |
Frontend: Landing & Onboarding client/src/components/landing/hero-section.tsx, client/src/pages/onboarding/onboarding.tsx, client/src/pages/onboarding/start.tsx |
Stronger wallet validation, pre-checks for existing aquariums, sync-to-backend flows (validatePlayer/syncPlayerToBackend), processing/loading states, improved toast/error handling, and adjusted redirect timing. |
Frontend: Policies & Config client/src/config/policies.ts |
Replaced single WORLD_ADDRESS policy with multiple per-contract address entries and updated DEV_POLICIES mapping and method lists. |
Frontend: Misc & Tooling client/vite.config.ts, client/src/config/policies.ts, client/src/config/api.ts, client/package.json |
Enabled mkcert plugin in Vite; bumped prettier dev dependency; minor type/formatting tweaks in UI components and types. |
Sequence Diagram(s)
sequenceDiagram
participant Player as Frontend (Player UI)
participant Hook as useMinigameApi / useGameScoreSubmission
participant Server as Backend API (MinigameController)
participant DB as Database
Note over Player,Hook: Game ends -> submit score
Player->>Hook: handleGameOver(finalScore, gameType)
Hook->>Hook: map frontend gameType -> backend gameType
alt no active session
Hook->>Server: POST /v1/minigames/sessions { wallet, gameType }
Server->>DB: find_or_create player_id by wallet_address
DB-->>Server: player_id
Server-->>Hook: 201 { sessionId, player_wallet }
end
Hook->>Server: POST /v1/minigames/sessions/:sessionId/end { finalScore, gameType }
Server->>Server: calculateXP(gameType, finalScore)
Server->>DB: update session with finalScore, xp, player_id
DB-->>Server: updated session
Server->>DB: aggregate leaderboards / best scores (on demand)
DB-->>Server: leaderboard rows
Server-->>Hook: success + leaderboard/ack
Hook-->>Player: show result / updated leaderboard
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
- Areas to focus:
backend/src/services/minigameService.js— player_id resolution, XP calculation logic, aggregation queries and return shapes.client/src/hooks/use-minigame-api.ts&use-game-score-submission.ts— mapping game types, session lifecycle, race conditions and wallet-state error handling.- Onboarding/landing components — async flows, delayed redirects, and sync-to-backend error paths.
client/src/config/policies.ts— correctness of contract address keys and method lists.
Possibly related PRs
- Add Bubble Jumper Mini-Game and Redesign Mini-Games Page #505 — Adds Bubble Jumper frontend/game pieces; overlaps with new game types and leaderboard integration.
- Add Fish Dodge Mini-Game with Mobile Support #522 — Adds Fish Dodge game; overlaps with score-submission wiring and game components.
- Ux refactor #485 — Modifies floppy-fish frontend; overlaps with score-submission integration and canvas changes.
Suggested reviewers
- Josue19-08
- BrayanMQ
Poem
🐰 I hopped to count each tiny score,
Wallets clapped as leaderboards soared.
Fish and bubbles danced in line,
I tallied hops and called them mine.
Hooray — small wins in code, divine!
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The pull request title accurately summarizes the primary changes across backend and frontend: minigame system overhaul, new leaderboard feature, and onboarding/UI improvements. |
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
backend/src/middleware/auth.js(1 hunks)backend/src/services/minigameService.js(8 hunks)client/package.json(1 hunks)client/src/components/mini-games/fish-dodge-game.tsx(2 hunks)client/src/components/ui/badge.tsx(1 hunks)client/src/components/ui/button.tsx(1 hunks)client/src/hooks/use-minigame-api.ts(1 hunks)client/src/pages/bubble-jumper.tsx(3 hunks)client/src/pages/onboarding/start.tsx(5 hunks)client/src/types/market.ts(1 hunks)
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
client/src/components/landing/hero-section.tsx (1)
1-10: Fix Prettier formatting issues.The CI pipeline is failing due to Prettier formatting violations. Run the formatter to resolve:
npx prettier --write client/src/components/landing/hero-section.tsxclient/src/pages/onboarding/start.tsx (1)
1-1: Fix Prettier formatting issues.The CI pipeline reports Prettier formatting errors. Run
npx prettier --write client/src/pages/onboarding/start.tsxto fix code style issues before merging.backend/src/services/minigameService.js (1)
1-1: Fix Prettier formatting issues.The pipeline detected code style issues. Run the formatter before merging.
#!/bin/bash # Fix formatting issues npx prettier --write backend/src/services/minigameService.js
🧹 Nitpick comments (9)
client/src/components/landing/hero-section.tsx (3)
78-107: Use consistent language for comments.Lines 79 and 102 contain Spanish comments while the rest of the codebase uses English. Consider translating for consistency.
- // Para jugadores existentes, obtener su último acuario desde backend + // For existing players, fetch their last aquarium from backend console.log('🏠 Fetching player aquariums from backend...');- // Sin acuarios, tratar como jugador nuevo o redirigir a onboarding para crear uno + // No aquariums found, redirect to onboarding to create one console.log('⚠️ No aquariums found, redirecting to /onboarding');
69-75: Inconsistent notification API usage.The component mixes
toast.error()(from sonner) withsuccess()andinfo()(fromuseNotifications). This creates inconsistent UX patterns and makes the code harder to maintain.Consider standardizing on one notification system throughout the component:
-import { toast } from 'sonner'; // ... in the component - toast.error("Error loading aquarium data. Please contact support."); + // Use the notification hook consistently, or add error() to useNotificationsIf
useNotificationsdoesn't expose anerror()method, either extend it or usetoastconsistently throughout.Also applies to: 91-92, 104-104, 112-112
108-115: Error recovery path could be improved.When aquarium fetch fails, showing a toast and staying on the page is safer than redirecting, but the user has no clear recovery action. Consider adding a retry mechanism or clearer guidance.
The current approach is acceptable for now, but a future enhancement could be:
} catch (aqError) { console.error("Failed to fetch aquariums", aqError); - toast.error("Could not load your aquariums. Please try again."); + toast.error("Could not load your aquariums. Please try again.", { + action: { + label: "Retry", + onClick: () => handleStartGame(), + }, + }); }client/vite.config.ts (1)
35-35: Consider English comments for codebase consistency.The new comments are in Spanish while the rest of the codebase uses English. For consistency and broader team accessibility, consider using English.
Apply this diff:
- // https se habilita automáticamente con vite-plugin-mkcert + // HTTPS is automatically enabled with vite-plugin-mkcert port: 5173, host: true, fs: { allow: ['..'], }, proxy: { '/api': { target: 'http://localhost:3001', changeOrigin: true, - secure: false, // Permitir conexión a backend HTTP desde HTTPS + secure: false, // Allow connection to HTTP backend from HTTPS rewrite: path => path.replace(/^\/api/, '/api'), }, },Also applies to: 45-45
client/src/config/policies.ts (2)
198-205: Empty methods array for Shop Catalog.The
SHOP_CATALOG_ADDRESScontract block has an empty methods array with a TODO-style comment. If no player-facing methods exist, consider removing this entry entirely to avoid confusion, or add the actual methods if they're known.- // ===== Shop Catalog System ===== - [SHOP_CATALOG_ADDRESS]: { - name: 'Aqua Stark Shop', - description: 'Catálogo de la tienda', - methods: [ - // Add methods if players interact with it directly (e.g. buying items not via Game) - ], - },
3-21: Contract addresses in the PR accurately match manifest_sepolia.json; however, they are hardcoded rather than loaded dynamically from the manifest.The review comment's observation is verified: all 9 contract addresses in the PR (lines 3-21) exactly match those in
client/manifest_sepolia.json. The suggestion to import or validate these from the manifest is valid best practice, though the PR currently hardcodes them. Since the client only maintains a Sepolia manifest (no dev variant), the current approach works for single-network deployments. For future multi-environment support, importing addresses from the manifest at runtime would be beneficial.client/src/pages/onboarding/onboarding.tsx (1)
339-339: Consider making the indexing delay configurable.The hardcoded 5-second delay for fish indexing works but is fragile. Consider extracting this to a constant or configuration value for easier tuning.
+const FISH_INDEXING_DELAY_MS = 5000; + // ... in handleCreateFish - await delay(5000); // 5 seconds for both fish to be indexed + await delay(FISH_INDEXING_DELAY_MS); // Wait for fish to be indexedclient/src/pages/leaderboard.tsx (1)
59-59: Consider making the limit configurable.The hardcoded limit of 50 works but could be made dynamic based on screen size or user preference for better UX.
backend/src/services/minigameService.js (1)
256-311: Consider performance optimization for leaderboard aggregation.The leaderboard methods load all sessions for a game type (or all sessions globally) into memory, then aggregate in JavaScript. For large datasets (thousands of sessions), this approach may cause:
- High memory usage
- Slow response times
- Database load from full table scans
Consider these optimizations:
- Database-side aggregation: Use SQL window functions or aggregations to compute best scores and rankings in the database.
- Caching: Cache leaderboard results with a short TTL (e.g., 1-5 minutes) to reduce load.
- Indexing: Ensure
game_type,player_id, andscorecolumns are indexed for faster queries.Example SQL for game leaderboard (pseudo-code):
SELECT player_id, MAX(score) as best_score, RANK() OVER (ORDER BY MAX(score) DESC) as rank FROM minigame_sessions WHERE game_type = $1 AND score > 0 GROUP BY player_id ORDER BY best_score DESC LIMIT $2Would you like me to help draft a database-optimized version?
Also applies to: 341-406
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
backend/src/controllers/minigameController.js(5 hunks)backend/src/index.js(1 hunks)backend/src/services/minigameService.js(8 hunks)client/src/App.tsx(2 hunks)client/src/components/index.ts(1 hunks)client/src/components/landing/hero-section.tsx(4 hunks)client/src/components/leaderboard/index.ts(1 hunks)client/src/components/leaderboard/leaderboard.tsx(1 hunks)client/src/components/mini-games/floppy-fish/floppy-fish-game-canvas.tsx(3 hunks)client/src/config/api.ts(1 hunks)client/src/config/policies.ts(8 hunks)client/src/hooks/index.ts(1 hunks)client/src/hooks/use-game-score-submission.ts(1 hunks)client/src/hooks/use-minigame-api.ts(1 hunks)client/src/pages/leaderboard.tsx(1 hunks)client/src/pages/onboarding/onboarding.tsx(6 hunks)client/src/pages/onboarding/start.tsx(5 hunks)client/vite.config.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
client/src/hooks/use-minigame-api.ts (2)
backend/scripts/test-store-api.js (1)
url(32-32)client/src/config/api.ts (3)
post(118-134)put(136-152)get(105-116)
client/src/App.tsx (1)
client/src/pages/leaderboard.tsx (1)
LeaderboardPage(7-72)
client/src/components/leaderboard/leaderboard.tsx (3)
client/src/components/leaderboard/index.ts (1)
Leaderboard(1-1)client/src/hooks/use-minigame-api.ts (1)
LeaderboardEntry(29-35)client/src/lib/utils.ts (1)
cn(4-6)
backend/src/services/minigameService.js (2)
backend/src/config/supabase.js (6)
supabase(7-10)supabase(7-10)TABLES(19-30)TABLES(19-30)supabaseAdmin(13-16)supabaseAdmin(13-16)backend/scripts/create-tables.js (5)
supabaseAdmin(32-49)supabaseAdmin(63-77)supabaseAdmin(91-106)supabaseAdmin(120-133)supabaseAdmin(147-160)
🪛 GitHub Actions: Backend CI/CD
backend/src/services/minigameService.js
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
🪛 GitHub Actions: Frontend CI
client/src/components/leaderboard/index.ts
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
client/src/hooks/use-game-score-submission.ts
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
client/src/hooks/use-minigame-api.ts
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
client/src/pages/onboarding/onboarding.tsx
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
client/src/pages/leaderboard.tsx
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
client/src/components/landing/hero-section.tsx
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
client/src/pages/onboarding/start.tsx
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
client/src/components/leaderboard/leaderboard.tsx
[error] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix code style issues in this file.
🪛 GitHub Actions: General CI
client/src/components/leaderboard/index.ts
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
client/src/hooks/use-game-score-submission.ts
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
client/src/hooks/use-minigame-api.ts
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
client/src/pages/onboarding/onboarding.tsx
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
client/src/pages/leaderboard.tsx
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
client/src/components/landing/hero-section.tsx
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
client/src/pages/onboarding/start.tsx
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
client/src/components/leaderboard/leaderboard.tsx
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
backend/src/services/minigameService.js
[error] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix formatting in this file.
🔇 Additional comments (24)
client/src/components/landing/hero-section.tsx (3)
30-39: Improved address validation.Checking
account?.addressinstead of justaccountis a safer approach since an account object could exist without a valid address property. The early return pattern here is clean.
56-59: Good fallback logic for player existence.Treating a player as existing if they're on-chain even when backend check fails (
validation.exists || validation.isOnChain) is a sensible resilience pattern that prevents re-registration of existing on-chain players.
89-93: Defensive check for missing aquarium ID is good.Validating the presence of
aquariumIdbefore proceeding prevents downstream errors. The early return with user feedback is appropriate.client/vite.config.ts (1)
7-7: The review comment is incorrect—mkcert import remains commented out and inactive.The code review assumes the import statement on line 7 is active and that mkcert has been added to the plugins array (line 13), but the current repository state shows:
- Line 7:
// import mkcert from 'vite-plugin-mkcert';(commented out)- Line 13: plugins array contains only
[react(), wasm(), topLevelAwait()](mkcert not included)- package.json: vite-plugin-mkcert is not installed (null in both dependencies and devDependencies)
The review comment describes a desired final state that was never applied to the codebase. The verification request about production builds is therefore moot since mkcert is not actively configured.
Likely an incorrect or invalid review comment.
client/src/config/policies.ts (4)
98-125: Duplicate fish-related methods across FishSystem and Game System.Both contracts define
add_fish_to_aquarium,breed_fishes,move_fish_to_aquarium, andnew_fish. The comment on line 123 also expresses uncertainty aboutpurchase_fishplacement.This duplication may be intentional if both contracts expose these entrypoints, but the uncertainty comment suggests the contract boundaries aren't fully clear. Consider documenting which contract is the canonical source for each action, or consolidate if one contract delegates to another.
Also applies to: 127-168
298-329: LGTM!The Cartridge configuration is well-structured with sensible defaults for session duration, network configuration, and UX options.
331-346: LGTM!Updated DEV_POLICIES to use the actual
AQUA_STARK_ADDRESSinstead of a zero-address, which is a more accurate representation for development testing. The minimal method set keeps the dev policy focused.
50-96: The review comment is based on incorrect assumptions about the file structure.The current
policies.tsfile does not contain separate contract addresses (AQUA_STARK_ADDRESS,TRANSACTION_ADDRESS,FISH_SYSTEM_ADDRESS,GAME_ADDRESS) as claimed. All methods are unified under a singleWORLD_ADDRESScontract. The methods that appear to overlap (e.g.,add_fish_to_aquarium,breed_fishes) belong to different game systems but are configured in one policy set with clarifying display names (e.g., "Add Fish to Aquarium (Game)"). This is not a duplicate permissions issue since there is only one contract address and one policy configuration.Likely an incorrect or invalid review comment.
client/src/pages/onboarding/start.tsx (2)
68-80: LGTM!The redirect logic correctly waits for account loading to complete before starting the redirect timer, and properly cleans up the timeout.
267-287: LGTM!The conditional rendering provides clear feedback during account loading, and the button correctly disables when registering or when no account address is available.
client/src/pages/onboarding/onboarding.tsx (3)
150-153: LGTM!The enhanced wallet validation checking
account?.addressinstead of justaccountis more robust, and the error message is user-friendly.
182-195: LGTM!The sync operation is correctly wrapped in try-catch to prevent blocking the main onboarding flow if backend sync fails. This is appropriate since the on-chain creation is the critical path.
109-132: The review comment references code that does not exist in the current codebase.The code snippet showing a
useEffectwithcheckExistingAquariumsand the dependency array[account, navigate, setActiveAquariumId]at lines 109-132 cannot be found inclient/src/pages/onboarding/onboarding.tsx.In the current file:
- Lines 109-132 contain state declarations and the
handleFishSelectfunction, not auseEffectgetPlayerAquariumsis only called at lines 194 and 203 withinhandleCreateAquarium, not in a separateuseEffectchecking for existing aquariums- There is no
checkExistingAquariumsfunction in the fileSince the code being reviewed does not exist in the codebase, the dependency issue described in the review comment cannot be verified or addressed.
Likely an incorrect or invalid review comment.
client/src/config/api.ts (1)
60-70: Minigame endpoint block looks consistent with existing API configPaths follow the established
/v1/...pattern and placeholder style; this should plug cleanly into the new minigame flows (sessions, leaderboards, stats, game types, bonus XP) as long as minigameRoutes expose the same paths. Just keep this list in sync with any future backend renames (especiallyPLAYER_STATSandBONUS_XP).backend/src/index.js (1)
60-67: Dev CORS origins update looks goodAllowing both
http://localhost:5173andhttps://localhost:5173for non‑production CORS is reasonable and keeps local dev flexible without affecting production origins.client/src/App.tsx (1)
34-35: Leaderboard route wiring is straightforward and consistentImporting
LeaderboardPageand exposing it at/leaderboardfits cleanly alongside the existing information/help routes, and the page itself already links back to/mini-games, so the navigation story looks coherent.Also applies to: 85-85
client/src/components/index.ts (1)
51-53: Leaderboard components are correctly added to the central barrelThe new
export * from './leaderboard';entry follows the existing grouping pattern (comment + export) and will make leaderboard UI available via the main components barrel without disrupting anything else.client/src/components/mini-games/floppy-fish/floppy-fish-game-canvas.tsx (1)
3-4: Score submission hook is cleanly integrated into Floppy FishUsing
useGameScoreSubmission('floppy-fish')and passinghandleGameOverintouseGameLogicis a tidy way to keep game logic and score submission concerns separated. The hook’s internal single‑flight guard should also protect against multiple game‑over events.Just ensure
useGameLogic’s signature has been updated everywhere it’s used to accept the callback parameter so TypeScript and runtime stay in sync.Also applies to: 22-24, 40-40
client/src/hooks/index.ts (1)
86-87: LGTM!The new hook exports follow the existing barrel pattern and integrate cleanly with the minigame feature set.
backend/src/controllers/minigameController.js (2)
55-58: LGTM!Adding support for the new game types ('floppy_fish', 'bubble_jumper', 'fish_dodge') aligns with the minigame expansion described in the PR objectives.
172-178: Review comment is incorrect — the code snippet referenced does not exist in the repository.The actual code at lines 172-178 in
backend/src/controllers/minigameController.jsis:const { playerId } = req.params; const stats = await MinigameService.getPlayerStats(playerId); res.json({ success: true, data: stats });This does not contain the wallet address fallback chain (
req.user?.walletAddress || req.params?.playerId || req.query?.wallet) mentioned in the review comment. Additionally, authentication middleware (AuthMiddleware.verifyToken) is globally applied to all routes inminigameRoutes.jsat line 9, before any route definitions, ensuring all endpoints—including/player/stats—require authentication. No security issue exists as described.Likely an incorrect or invalid review comment.
backend/src/services/minigameService.js (2)
143-145: LGTM!Adding game type aliases ('floppy_fish', 'bubble_jumper', 'fish_dodge') for XP calculation aligns with the new game types and provides good frontend flexibility.
602-661: LGTM!The new
getGameTypes()method provides a clean, static metadata source for available games. This is well-structured and supports the frontend game selection UI.client/src/components/leaderboard/leaderboard.tsx (1)
39-39: I'll help you verify the review comment about the potential infinite loop from unstable function references in the useEffect dependency array. Let me start by examining the relevant code.
<function_calls>
#!/bin/bashFirst, find the useMinigameApi hook file
fd -t f 'use-minigame-api' --type ts --type tsx
</function_calls>
<function_calls>
#!/bin/bashSearch for useMinigameApi definition
rg -nP 'useMinigameApi' -t ts -t tsx -A 2
</function_calls>
<function_calls>
#!/bin/bashLook at the leaderboard.tsx file around line 39
cat -n client/src/components/leaderboard/leaderboard.tsx | head -60
</function_calls>
<function_calls>
#!/bin/bashSearch for all hook files
fd -t f -e ts -e tsx 'hook' client/src/
</function_calls>
| static async getGameLeaderboard(req, res) { | ||
| try { | ||
| const { gameType } = req.params; | ||
| const limit = parseInt(req.query.limit) || 10; |
There was a problem hiding this comment.
Fix NaN bug from unvalidated parseInt.
parseInt(req.query.limit) returns NaN when limit is missing, non-numeric, or invalid. While the fallback || 10 handles NaN, it would be clearer and safer to validate explicitly.
Apply this diff to add validation:
- const limit = parseInt(req.query.limit) || 10;
+ const limit = parseInt(req.query.limit, 10) || 10;
+ if (limit < 1 || limit > 100) {
+ return res.status(400).json({ error: 'Limit must be between 1 and 100' });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const limit = parseInt(req.query.limit) || 10; | |
| const limit = parseInt(req.query.limit, 10) || 10; | |
| if (limit < 1 || limit > 100) { | |
| return res.status(400).json({ error: 'Limit must be between 1 and 100' }); | |
| } |
🤖 Prompt for AI Agents
In backend/src/controllers/minigameController.js around line 218, the current
parseInt(req.query.limit) can yield NaN for missing or invalid input; update
this line to explicitly validate the query param: parse with
parseInt(req.query.limit, 10) (or Number), check that the result is a finite
positive integer (e.g., Number.isInteger(value) && value > 0), and only use that
value; otherwise set limit = 10 as the safe default.
| static async getGlobalLeaderboard(req, res) { | ||
| try { | ||
| const leaderboard = await MinigameService.getGlobalLeaderboard(); | ||
| const limit = parseInt(req.query.limit) || 20; |
There was a problem hiding this comment.
Fix NaN bug from unvalidated parseInt.
Same issue as Line 218: parseInt(req.query.limit) can return NaN. Add explicit validation and consider enforcing a maximum limit to prevent performance issues.
Apply this diff to add validation:
- const limit = parseInt(req.query.limit) || 20;
+ const limit = parseInt(req.query.limit, 10) || 20;
+ if (limit < 1 || limit > 100) {
+ return res.status(400).json({ error: 'Limit must be between 1 and 100' });
+ }🤖 Prompt for AI Agents
In backend/src/controllers/minigameController.js around line 258,
parseInt(req.query.limit) can produce NaN and be used directly; validate the
parsed value, fall back to a safe default (e.g. 20) when invalid, coerce to an
integer, enforce a positive minimum and a configurable maximum cap (e.g. 100) to
prevent excessive limits, and use the validated value for limit. Ensure you
check isNaN or Number.isInteger after parsing and clamp the result between
allowed bounds before use.
| const { data: player, error: playerError } = await supabase | ||
| .from(TABLES.PLAYERS) | ||
| .select('player_id') | ||
| .eq('wallet_address', playerWallet) | ||
| .single(); | ||
|
|
||
| if (playerError || !player) { | ||
| throw new Error(`Player not found for wallet: ${playerWallet}`); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract player lookup into a reusable helper method.
The player_id lookup pattern from wallet_address is duplicated in createGameSession (lines 40-48), getPlayerStats (lines 180-189), and awardBonusXP (lines 504-513). This violates DRY and makes updates harder.
Apply this diff to extract a helper:
+ /**
+ * Get player_id from wallet address
+ * @private
+ */
+ static async _getPlayerIdFromWallet(walletAddress) {
+ const { data: player, error } = await supabase
+ .from(TABLES.PLAYERS)
+ .select('player_id')
+ .eq('wallet_address', walletAddress)
+ .single();
+
+ if (error || !player) {
+ throw new Error(`Player not found for wallet: ${walletAddress}`);
+ }
+
+ return player.player_id;
+ }
+
static async createGameSession(playerWallet, gameType) {
try {
- // Get player_id from wallet_address
- const { data: player, error: playerError } = await supabase
- .from(TABLES.PLAYERS)
- .select('player_id')
- .eq('wallet_address', playerWallet)
- .single();
-
- if (playerError || !player) {
- throw new Error(`Player not found for wallet: ${playerWallet}`);
- }
+ const playerId = await this._getPlayerIdFromWallet(playerWallet);Then apply similar changes to getPlayerStats and awardBonusXP.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/src/services/minigameService.js around lines 40 to 48, extract the
repeated player lookup into a reusable helper (e.g., getPlayerIdByWallet) that
accepts supabase client and walletAddress, performs the
.from(TABLES.PLAYERS).select('player_id').eq('wallet_address', wallet).single()
call, throws a clear Error if not found or on query error, and returns the
player_id; then replace the inline query in createGameSession (lines 40-48),
getPlayerStats (lines ~180-189), and awardBonusXP (lines ~504-513) to call this
helper and use its returned player_id, preserving existing error behavior and
logging.
| // Sum best scores from all games for each player | ||
| const playerTotalScores = {}; | ||
| Object.entries(playerGameScores).forEach(([key, score]) => { | ||
| const playerId = key.split('_')[0]; // Extract player_id from key |
There was a problem hiding this comment.
Fragile player_id extraction from composite key.
The expression key.split('_')[0] assumes player_id values never contain underscores. If a player_id contains an underscore (e.g., UUID format or user-generated), this will extract only the first segment and cause data corruption.
Use a more robust key format or parsing strategy:
const playerGameScores = {};
sessions.forEach(session => {
const playerId = session.player_id;
- const key = `${playerId}_${session.game_type}`;
+ const key = JSON.stringify({ playerId, gameType: session.game_type });
if (!playerGameScores[key] || session.score > playerGameScores[key]) {
playerGameScores[key] = session.score;
}
});
// Sum best scores from all games for each player
const playerTotalScores = {};
- Object.entries(playerGameScores).forEach(([key, score]) => {
- const playerId = key.split('_')[0]; // Extract player_id from key
+ Object.entries(playerGameScores).forEach(([keyStr, score]) => {
+ const { playerId } = JSON.parse(keyStr);
if (!playerTotalScores[playerId]) {
playerTotalScores[playerId] = 0;
}Or use a Map with tuple keys for cleaner code.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/src/services/minigameService.js around line 365, the code uses
key.split('_')[0] which breaks when player_id contains underscores; replace this
with a robust parse: find the separator position with key.lastIndexOf('_') and
extract playerId as key.substring(0, sep) (and if sep === -1 fall back to the
whole key or throw), so you correctly handle player_ids containing underscores;
additionally, update any code that builds these composite keys (or migrate to a
structured format like JSON-encoded keys, a unique delimiter, or a Map with
tuple keys) so creation and parsing remain consistent.
| if (response && response.success && response.data && response.data.length > 0) { | ||
| console.log('✅ Found existing aquarium, redirecting to game...'); | ||
| const primaryAquarium = response.data[0]; | ||
| const existingId = primaryAquarium.on_chain_id; | ||
|
|
||
| setActiveAquariumId(existingId, account.address); | ||
| toast.success('Existing aquarium found! resuming game...'); | ||
| navigate(`/loading?aquarium=${existingId}`); |
There was a problem hiding this comment.
Add defensive check for on_chain_id property.
The code assumes primaryAquarium.on_chain_id exists, but if the API response doesn't include this field, existingId will be undefined, causing issues when passed to setActiveAquariumId and the navigation URL.
if (response && response.success && response.data && response.data.length > 0) {
console.log('✅ Found existing aquarium, redirecting to game...');
const primaryAquarium = response.data[0];
const existingId = primaryAquarium.on_chain_id;
+ if (!existingId) {
+ console.warn('⚠️ Aquarium found but missing on_chain_id');
+ return;
+ }
+
setActiveAquariumId(existingId, account.address);
toast.success('Existing aquarium found! resuming game...');
navigate(`/loading?aquarium=${existingId}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response && response.success && response.data && response.data.length > 0) { | |
| console.log('✅ Found existing aquarium, redirecting to game...'); | |
| const primaryAquarium = response.data[0]; | |
| const existingId = primaryAquarium.on_chain_id; | |
| setActiveAquariumId(existingId, account.address); | |
| toast.success('Existing aquarium found! resuming game...'); | |
| navigate(`/loading?aquarium=${existingId}`); | |
| if (response && response.success && response.data && response.data.length > 0) { | |
| console.log('✅ Found existing aquarium, redirecting to game...'); | |
| const primaryAquarium = response.data[0]; | |
| const existingId = primaryAquarium.on_chain_id; | |
| if (!existingId) { | |
| console.warn('⚠️ Aquarium found but missing on_chain_id'); | |
| return; | |
| } | |
| setActiveAquariumId(existingId, account.address); | |
| toast.success('Existing aquarium found! resuming game...'); | |
| navigate(`/loading?aquarium=${existingId}`); |
🤖 Prompt for AI Agents
In client/src/pages/onboarding/onboarding.tsx around lines 117 to 124, the code
assumes primaryAquarium.on_chain_id exists; add a defensive check that the
property is present and a non-empty string (or number as expected) before
assigning existingId, calling setActiveAquariumId, and navigating; if
on_chain_id is missing, log or toast an error/warning, do not call
setActiveAquariumId or navigate, and gracefully return or use a documented
fallback (e.g., primaryAquarium.id) only if appropriate.
| // Wait for account to load after login | ||
| useEffect(() => { | ||
| // Only redirect if no wallet connected (Starknet) OR no Cartridge | ||
| // Give time for account to load after Cartridge login | ||
| const checkAccount = () => { | ||
| if (account?.address) { | ||
| console.log('✅ Account loaded:', account.address); | ||
| setIsLoadingAccount(false); | ||
| } else { | ||
| console.log('⏳ Waiting for account to load...'); | ||
| // Keep checking for a bit longer | ||
| setTimeout(() => { | ||
| if (!account?.address) { | ||
| console.log('⚠️ Account still not loaded after timeout'); | ||
| setIsLoadingAccount(false); | ||
| // Don't redirect immediately, let user see the page | ||
| } | ||
| }, 3000); | ||
| } | ||
| }; | ||
|
|
||
| // Initial check | ||
| checkAccount(); | ||
|
|
||
| // Also check when account changes | ||
| if (account?.address) { | ||
| setIsLoadingAccount(false); | ||
| } | ||
| }, [account]); |
There was a problem hiding this comment.
Race condition and cleanup issue in account loading effect.
The useEffect has multiple issues:
- The
setTimeoutat line 49 creates a closure over staleaccountvalue and runs after the effect may have re-executed or unmounted. - No cleanup for the inner timeout, causing potential state updates on unmounted components.
- The redundant check at lines 63-65 always runs after
checkAccount(), potentially causing double state updates.
Apply this diff to fix the race condition and cleanup:
// Wait for account to load after login
useEffect(() => {
- // Give time for account to load after Cartridge login
- const checkAccount = () => {
- if (account?.address) {
- console.log('✅ Account loaded:', account.address);
- setIsLoadingAccount(false);
- } else {
- console.log('⏳ Waiting for account to load...');
- // Keep checking for a bit longer
- setTimeout(() => {
- if (!account?.address) {
- console.log('⚠️ Account still not loaded after timeout');
- setIsLoadingAccount(false);
- // Don't redirect immediately, let user see the page
- }
- }, 3000);
- }
- };
-
- // Initial check
- checkAccount();
-
- // Also check when account changes
if (account?.address) {
+ console.log('✅ Account loaded:', account.address);
setIsLoadingAccount(false);
+ return;
}
+
+ console.log('⏳ Waiting for account to load...');
+ const timeoutId = setTimeout(() => {
+ console.log('⚠️ Account still not loaded after timeout');
+ setIsLoadingAccount(false);
+ }, 3000);
+
+ return () => clearTimeout(timeoutId);
}, [account]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Wait for account to load after login | |
| useEffect(() => { | |
| // Only redirect if no wallet connected (Starknet) OR no Cartridge | |
| // Give time for account to load after Cartridge login | |
| const checkAccount = () => { | |
| if (account?.address) { | |
| console.log('✅ Account loaded:', account.address); | |
| setIsLoadingAccount(false); | |
| } else { | |
| console.log('⏳ Waiting for account to load...'); | |
| // Keep checking for a bit longer | |
| setTimeout(() => { | |
| if (!account?.address) { | |
| console.log('⚠️ Account still not loaded after timeout'); | |
| setIsLoadingAccount(false); | |
| // Don't redirect immediately, let user see the page | |
| } | |
| }, 3000); | |
| } | |
| }; | |
| // Initial check | |
| checkAccount(); | |
| // Also check when account changes | |
| if (account?.address) { | |
| setIsLoadingAccount(false); | |
| } | |
| }, [account]); | |
| // Wait for account to load after login | |
| useEffect(() => { | |
| if (account?.address) { | |
| console.log('✅ Account loaded:', account.address); | |
| setIsLoadingAccount(false); | |
| return; | |
| } | |
| console.log('⏳ Waiting for account to load...'); | |
| const timeoutId = setTimeout(() => { | |
| console.log('⚠️ Account still not loaded after timeout'); | |
| setIsLoadingAccount(false); | |
| }, 3000); | |
| return () => clearTimeout(timeoutId); | |
| }, [account]); |
🤖 Prompt for AI Agents
client/src/pages/onboarding/start.tsx around lines 39 to 66: the effect that
waits for the account to load creates a setTimeout that closes over a
potentially stale account value, doesn't clear the timer on re-renders/unmounts,
and contains a redundant immediate check after calling checkAccount; fix by
removing the redundant post-check (lines 63-65), capture the timeout id in a ref
(e.g. timeoutRef) so you can clearTimeout on cleanup and before scheduling a new
one, and in the timeout callback reference the latest account via the account
variable from the effect scope (or via a ref) so it checks current state before
calling setIsLoadingAccount; finally return a cleanup function that clears any
pending timeout to prevent state updates after unmount.
| if (validation.exists) { | ||
| console.log('✅ Player already exists, redirecting...'); | ||
| if (validation.isOnChain && !validation.isInBackend) { | ||
| setRegistrationStep('Syncing existing account...'); | ||
| await syncPlayerToBackend(validation.playerData!, account.address); | ||
| } | ||
| toast.success('Welcome back!'); | ||
| // Determine redirection based on existing data? For now, onboarding handles aquarium check | ||
| navigate('/onboarding'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Unsafe non-null assertion on validation.playerData.
At line 120, validation.playerData! assumes playerData is defined when validation.isOnChain is true. If validatePlayer returns { exists: true, isOnChain: true, isInBackend: false, playerData: undefined } due to an edge case, this will pass undefined to syncPlayerToBackend, potentially causing runtime errors.
Apply this diff to add a defensive check:
if (validation.exists) {
console.log('✅ Player already exists, redirecting...');
- if (validation.isOnChain && !validation.isInBackend) {
+ if (validation.isOnChain && !validation.isInBackend && validation.playerData) {
setRegistrationStep('Syncing existing account...');
- await syncPlayerToBackend(validation.playerData!, account.address);
+ await syncPlayerToBackend(validation.playerData, account.address);
}
toast.success('Welcome back!');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (validation.exists) { | |
| console.log('✅ Player already exists, redirecting...'); | |
| if (validation.isOnChain && !validation.isInBackend) { | |
| setRegistrationStep('Syncing existing account...'); | |
| await syncPlayerToBackend(validation.playerData!, account.address); | |
| } | |
| toast.success('Welcome back!'); | |
| // Determine redirection based on existing data? For now, onboarding handles aquarium check | |
| navigate('/onboarding'); | |
| return; | |
| } | |
| if (validation.exists) { | |
| console.log('✅ Player already exists, redirecting...'); | |
| if (validation.isOnChain && !validation.isInBackend && validation.playerData) { | |
| setRegistrationStep('Syncing existing account...'); | |
| await syncPlayerToBackend(validation.playerData, account.address); | |
| } | |
| toast.success('Welcome back!'); | |
| // Determine redirection based on existing data? For now, onboarding handles aquarium check | |
| navigate('/onboarding'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In client/src/pages/onboarding/start.tsx around lines 116 to 126, the code uses
a non-null assertion validation.playerData! when calling syncPlayerToBackend
which may be undefined in edge cases; add a defensive check that ensures
validation.playerData is defined before calling syncPlayerToBackend — if it's
undefined, log an error, show a user-facing toast (or setRegistrationStep to an
error state), and return/abort the sync flow instead of calling the function
with undefined; do not use the '!' operator and only call syncPlayerToBackend
when validation.playerData is truthy (or attempt to re-fetch the player data
first), ensuring a clear early-return path for the error case.
| if (errorMessage.includes('USERNAME ALREADY TAKEN') || errorMessage.includes('ALREADY REGISTERED')) { | ||
| toast.success('Account found! Redirecting...'); | ||
| // Try to sync backend just in case and redirect | ||
| try { | ||
| const reValidation = await validatePlayer(account.address); | ||
| if (reValidation.isOnChain && !reValidation.isInBackend) { | ||
| await syncPlayerToBackend(reValidation.playerData!, account.address); | ||
| } | ||
| } catch (e) { console.warn("Sync failed on fallback", e); } | ||
|
|
||
| navigate('/onboarding'); |
There was a problem hiding this comment.
Same non-null assertion risk in error recovery path.
Line 170 uses reValidation.playerData! with the same risk as the earlier occurrence. Additionally, consider using toast.info instead of toast.success since this is an error recovery path, not a true success.
if (errorMessage.includes('USERNAME ALREADY TAKEN') || errorMessage.includes('ALREADY REGISTERED')) {
- toast.success('Account found! Redirecting...');
+ toast.info('Account found! Redirecting...');
// Try to sync backend just in case and redirect
try {
const reValidation = await validatePlayer(account.address);
- if (reValidation.isOnChain && !reValidation.isInBackend) {
- await syncPlayerToBackend(reValidation.playerData!, account.address);
+ if (reValidation.isOnChain && !reValidation.isInBackend && reValidation.playerData) {
+ await syncPlayerToBackend(reValidation.playerData, account.address);
}
} catch (e) { console.warn("Sync failed on fallback", e); }🤖 Prompt for AI Agents
In client/src/pages/onboarding/start.tsx around lines 164 to 174, the
error-recovery path uses a non-null assertion on reValidation.playerData! and
shows a success toast; change this to safely check that reValidation.playerData
is defined before calling syncPlayerToBackend (e.g., if (reValidation.isOnChain
&& !reValidation.isInBackend && reValidation.playerData) { await
syncPlayerToBackend(reValidation.playerData, account.address); }) to avoid
possible runtime null/undefined access, and replace toast.success('Account
found! Redirecting...') with toast.info('Account found — redirecting...') to
reflect an informational recovery state. Ensure the existing catch remains to
log sync failures.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
client/src/hooks/use-minigame-api.ts (2)
74-100: Standardize URL construction across all API calls.This is already flagged in a past review. The code mixes raw endpoints (e.g.,
API_CONFIG.ENDPOINTS.MINIGAMES.SESSIONS) withbuildApiUrl()calls. Pick one approach for consistency.Also applies to: 116-118, 165-167, 186-201
13-17: Remove unusedREVERSE_GAME_TYPE_MAPconstant.The static analysis correctly flags this constant as unused. Either remove it now or use it when mapping backend game types back to frontend slugs (e.g., in leaderboard display).
-const REVERSE_GAME_TYPE_MAP: Record<string, string> = { - floppy_fish: 'floppy-fish', - bubble_jumper: 'bubble-jumper', - fish_dodge: 'fish-dodge', -};client/src/hooks/use-game-score-submission.ts (1)
3-3: Remove unusedtoastimport to fix pipeline failure.The static analysis and pipeline correctly flag this as unused. Either remove the import or uncomment the toast calls if user feedback is desired.
import { useCallback, useRef } from 'react'; import { useMinigameApi } from './use-minigame-api'; -import { toast } from 'sonner';client/src/components/leaderboard/leaderboard.tsx (1)
124-135: Fix score display logic to match context (already flagged).The score value at lines 126-128 doesn't match the label logic at lines 130-134. For game-specific leaderboards,
best_scoreshould be displayed, but the current code preferstotal_score.<div className='text-right'> <p className='text-yellow-400 font-bold text-lg'> - {entry.total_score?.toLocaleString() || - entry.best_score?.toLocaleString() || - 0} + {gameType + ? (entry.best_score?.toLocaleString() ?? 0) + : (entry.total_score?.toLocaleString() ?? 0)} </p>client/src/pages/onboarding/onboarding.tsx (1)
123-130: Add defensive check foron_chain_idproperty (already flagged).Line 125 assumes
primaryAquarium.on_chain_idexists. If the API response doesn't include this field,existingIdwill beundefined, causing issues withsetActiveAquariumIdand navigation.const primaryAquarium = response.data[0]; const existingId = primaryAquarium.on_chain_id; + if (!existingId) { + console.warn('⚠️ Aquarium found but missing on_chain_id'); + return; + } + setActiveAquariumId(existingId, account.address);client/src/pages/onboarding/start.tsx (3)
39-66: Race condition and cleanup issues in account loading effect (already flagged).The
setTimeoutat line 49 closes over potentially staleaccountvalue and lacks cleanup. The redundant check at lines 63-65 may cause double state updates.useEffect(() => { - const checkAccount = () => { - if (account?.address) { - console.log('✅ Account loaded:', account.address); - setIsLoadingAccount(false); - } else { - console.log('⏳ Waiting for account to load...'); - setTimeout(() => { - if (!account?.address) { - console.log('⚠️ Account still not loaded after timeout'); - setIsLoadingAccount(false); - } - }, 3000); - } - }; - checkAccount(); - if (account?.address) { - setIsLoadingAccount(false); - } + if (account?.address) { + console.log('✅ Account loaded:', account.address); + setIsLoadingAccount(false); + return; + } + + console.log('⏳ Waiting for account to load...'); + const timeoutId = setTimeout(() => { + console.log('⚠️ Account still not loaded after timeout'); + setIsLoadingAccount(false); + }, 3000); + + return () => clearTimeout(timeoutId); }, [account]);
120-122: Unsafe non-null assertion onvalidation.playerData(already flagged).Line 122 uses
validation.playerData!assuming it's always defined whenisOnChainis true. Add a defensive check to prevent runtime errors.- if (validation.isOnChain && !validation.isInBackend) { + if (validation.isOnChain && !validation.isInBackend && validation.playerData) { setRegistrationStep('Syncing existing account...'); - await syncPlayerToBackend(validation.playerData!, account.address); + await syncPlayerToBackend(validation.playerData, account.address); }
173-179: Same non-null assertion risk in error recovery path (already flagged).Line 176 uses
reValidation.playerData!with the same risk. Also consider usingtoast.infoinstead oftoast.successsince this is an error recovery path.- toast.success('Account found! Redirecting...'); + toast.info('Account found! Redirecting...'); try { const reValidation = await validatePlayer(account.address); - if (reValidation.isOnChain && !reValidation.isInBackend) { + if (reValidation.isOnChain && !reValidation.isInBackend && reValidation.playerData) { await syncPlayerToBackend( - reValidation.playerData!, + reValidation.playerData, account.address ); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
client/src/components/landing/hero-section.tsx(4 hunks)client/src/components/leaderboard/index.ts(1 hunks)client/src/components/leaderboard/leaderboard.tsx(1 hunks)client/src/hooks/use-game-score-submission.ts(1 hunks)client/src/hooks/use-minigame-api.ts(1 hunks)client/src/pages/leaderboard.tsx(1 hunks)client/src/pages/onboarding/onboarding.tsx(6 hunks)client/src/pages/onboarding/start.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- client/src/pages/leaderboard.tsx
- client/src/components/leaderboard/index.ts
🧰 Additional context used
🧬 Code graph analysis (3)
client/src/hooks/use-game-score-submission.ts (1)
client/src/hooks/use-minigame-api.ts (1)
useMinigameApi(57-258)
client/src/hooks/use-minigame-api.ts (1)
client/src/config/api.ts (3)
post(118-134)put(136-152)get(105-116)
client/src/pages/onboarding/start.tsx (1)
client/src/hooks/usePlayerValidation.ts (1)
usePlayerValidation(55-207)
🪛 GitHub Actions: Frontend CI
client/src/hooks/use-game-score-submission.ts
[error] 3-3: 'toast' is defined but never used. Allowed unused vars must match /^_/u @typescript-eslint/no-unused-vars
🪛 GitHub Check: frontend
client/src/hooks/use-game-score-submission.ts
[failure] 3-3:
'toast' is defined but never used. Allowed unused vars must match /^_/u
client/src/hooks/use-minigame-api.ts
[failure] 13-13:
'REVERSE_GAME_TYPE_MAP' is assigned a value but never used. Allowed unused vars must match /^_/u
🔇 Additional comments (8)
client/src/hooks/use-game-score-submission.ts (1)
17-44: Score submission guard logic is well-implemented.The single-flight guard via
isSubmittingRefprevents duplicate submissions, and the non-positive score check is appropriate. The error is logged without disrupting UX.client/src/components/leaderboard/leaderboard.tsx (2)
91-139: Component structure and UI logic are well-organized.The ranking display with icons, wallet formatting, and responsive styling are cleanly implemented. Empty state and error handling provide good UX.
24-39: Based on my verification, I found that the file referenced in the review comment (client/src/components/leaderboard/leaderboard.tsx) does not exist in this repository.I searched extensively through:
- The components directory structure
- All
.tsxfiles in the codebase- Specific searches for "leaderboard" files
The file path and code snippet in the review comment do not match any files in the AquaStark/Aqua-Stark repository. This suggests the review comment may have been copied from a different repository or PR, or the file has been removed/renamed.
File does not exist in repository - review comment references non-existent code.
The review addresses a useEffect dependency concern, but since the file being reviewed is not present in this codebase, the comment cannot be applied or verified.
Likely an incorrect or invalid review comment.
client/src/components/landing/hero-section.tsx (2)
78-120: Aquarium fetch flow has good defensive checks and error handling.The null check for
aquariumId(lines 89-98) and graceful error handling without forcing navigation (lines 113-119) are well-implemented improvements.Minor: Consider translating Spanish comments to English for consistency (lines 79, 107).
56-59: Good defensive handling for on-chain-only players.The
playerExistscheck that considers bothvalidation.existsandvalidation.isOnChainensures players who exist on-chain but failed backend validation are still treated as existing users.client/src/pages/onboarding/onboarding.tsx (1)
154-158: Good wallet validation tightening.The stricter check for
account?.addresswith an explicit error message improves UX when the wallet isn't properly connected.client/src/pages/onboarding/start.tsx (1)
277-297: Loading state UI is a good addition.The loading indicator while waiting for the account to load improves UX by providing clear feedback instead of showing a broken state.
client/src/hooks/use-minigame-api.ts (1)
83-97: The response object structure concern is unfounded. The code is correct.After examining
use-api.ts, I verified that:
useApi().post<T>()returnsPromise<ApiResponse<T>>(line 461), whereApiResponse<T>is defined as{ data: T, status, statusText, headers, success, error? }- In the minigame code,
post<{ success: boolean; data: GameSession }>()meansresponse.datais of type{ success: boolean; data: GameSession }- Therefore,
response.data.successandresponse.data.dataare properly typed and valid accessesThe double-wrapping is intentional by design:
ApiResponsewraps the API response payload. No runtime error will occur.
| checkExistingAquariums(); | ||
| }, [account, navigate, setActiveAquariumId]); |
There was a problem hiding this comment.
Missing getPlayerAquariums in useEffect dependency array.
The effect uses getPlayerAquariums but it's not listed in the dependency array. If the function reference changes, the effect won't re-run as expected. Add it or use useCallback to stabilize the function.
- }, [account, navigate, setActiveAquariumId]);
+ }, [account, navigate, setActiveAquariumId, getPlayerAquariums]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| checkExistingAquariums(); | |
| }, [account, navigate, setActiveAquariumId]); | |
| checkExistingAquariums(); | |
| }, [account, navigate, setActiveAquariumId, getPlayerAquariums]); |
🤖 Prompt for AI Agents
In client/src/pages/onboarding/onboarding.tsx around lines 136 to 137, the
useEffect calls getPlayerAquariums but doesn't include it in the dependency
array; add getPlayerAquariums to the dependency array or wrap getPlayerAquariums
in useCallback (exporting/stabilizing its reference) and then include that
stable reference in the dependency array so the effect re-runs correctly when
the function or its dependencies change.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
client/src/hooks/use-minigame-api.ts (2)
109-112: Standardize URL construction for minigame endpointsYou still mix raw endpoint constants and
buildApiUrl:
- Raw constants:
SESSIONS,GLOBAL_LEADERBOARD,PLAYER_STATS,GAME_TYPESbuildApiUrl:END_SESSION,GAME_LEADERBOARDFor maintainability, it’s better to pick one pattern (the repo’s other hooks generally pass raw endpoint constants into
useApi, which then handles base URL logic) and use it consistently across:
endGameSessiongetGameLeaderboardgetGlobalLeaderboardgetPlayerStatsgetGameTypescreateGameSessionE.g., follow the
use-player-apistyle and build any path/query params directly on the endpoint string before passing it toget/post/put.Also applies to: 153-167, 180-195, 201-232
13-45: Consider adding a small mapping layer to decouple backend field naming
GameSessionandLeaderboardEntrycurrently expose backend‑style snake_case fields (session_id,player_wallet,best_score,total_score), whilePlayerStatsis camelCase. That mixes naming styles in your public types and bakes backend response shapes directly into the hook’s consumer API.Non‑blocking, but consider:
- Keeping exported interfaces fully camelCase (e.g.,
sessionId,playerWallet,bestScore,totalScore).- Adding internal mappers that convert the raw backend JSON into these DTOs before returning from the hook.
This keeps the rest of the UI insulated from backend naming changes and reads more idiomatically in TS/React code.
🧹 Nitpick comments (1)
client/src/hooks/use-minigame-api.ts (1)
6-12: Game type mapping andsubmitScorecomposition look solid (optionally tighten types)The
GAME_TYPE_MAP+mapGameTypepattern and thesubmitScoreflow (create session → immediately end it) are straightforward and correct.If you want to reduce stringly‑typed usage later, you could introduce a
type GameSlug = 'floppy-fish' | 'bubble-jumper' | 'fish-dodge';(plus any future slugs) and use that instead of plainstringforgameTypewhere applicable, but this is purely optional.Also applies to: 58-63, 137-148
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/hooks/use-game-score-submission.ts(1 hunks)client/src/hooks/use-minigame-api.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/hooks/use-game-score-submission.ts
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/hooks/use-minigame-api.ts (2)
backend/scripts/test-store-api.js (1)
url(32-32)client/src/config/api.ts (3)
post(118-134)put(136-152)get(105-116)
🪛 GitHub Actions: Frontend CI
client/src/hooks/use-minigame-api.ts
[error] 82-82: tsc --noEmit failed with TS2559 in src/hooks/use-minigame-api.ts:82: Type 'string' has no properties in common with type 'Partial'.
🔇 Additional comments (1)
client/src/hooks/use-minigame-api.ts (1)
234-251: Hook surface is cohesive and matches minigame/leaderboard use casesThe returned state (
currentSession,loading,error) plus actions andmapGameTypegive consumers a clean, focused API for the minigame flows and leaderboards. No changes needed here.
1 participant
## 🔥 Pull Request: Minigame Overhaul, Leaderboard System, and Onboarding/UI Improvements
📌 Related Issue
Closes #ISSUE_NUMBER
📝 Description
This PR introduces a complete update to the minigame flow, adds the new Leaderboard system, and improves several UI areas including the hero section and onboarding screens.
It also adds new hooks, updates client configuration, and integrates all backend changes required to support the new score submission workflow.
✅ Changes Made
Backend
miniggameControllerwith updated logic and cleaner endpoint structure.minigameServiceto support the new score submission and validation flow.index.jsto register updated minigame routes and server configuration.Frontend
useMinigameApianduseGameScoreSubmission.api.tsandpolicies.tswith new endpoints and constants.onboarding.tsxandstart.tsx).App.tsx.components/index.ts.📷 Evidence
🚀 Additional Notes
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.