feat: implement matchmaking queue and stockfish wasm integration #695
Conversation
|
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 a client matchmaking modal and updates hook signatures; integrates Stockfish WASM bot analysis into the main page; implements a new backend tournament bracket module with services and tests; and applies small backend test/import/model/error mapping tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as frontend/app/page.tsx
participant Modal as MatchmakingModal
participant Hook as useMatchmaking
participant Server as Backend API
participant WS as WebSocket
User->>Page: open matchmaking modal
Page->>Modal: render(isOpen=true)
User->>Modal: clicks "Rated" or "Casual"
Modal-->>Page: onConfirm(matchType)
Page->>Hook: joinMatchmaking(matchType)
Hook->>Server: POST /v1/matchmaking/join { match_type, ... }
Server-->>Hook: { request_id, sessionId?, ... }
Hook-->>Page: store request/session id
Page->>WS: connect websocket for updates
WS-->>Page: match found / updates
sequenceDiagram
participant User
participant Page as frontend/app/page.tsx
participant Stockfish as Stockfish WASM
participant Chess as chess.js
participant UI as Board
User->>Page: start bot mode / confirm personality
Page->>Chess: game.reset() / setPosition("start")
loop when it's bot's turn and Stockfish ready
Page->>Stockfish: analyzePosition(fen, depth)
Stockfish-->>Page: { bestMove, eval, depth }
Page->>Chess: makeMove(bestMove)
Page->>UI: render position and display eval/depth
end
User->>Chess: make player move
Page->>Page: clear botAnalysis, update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@Spagero763 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/hook/useMatchmaking.ts (1)
152-163:⚠️ Potential issue | 🔴 CriticalCall the
/cancelendpoint instead of the non-existent/leaveendpoint, and sendrequest_idinstead ofsessionId.The backend has no
/leaveendpoint. The cancel endpoint is at/v1/matchmaking/canceland expects{ request_id: Uuid }, not{ sessionId }. Currently, the fetch request returns 404, which is silently ignored, leaving the request in Redis indefinitely. Change the call to:fetch(`${API_BASE}/v1/matchmaking/cancel`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ request_id: sessionIdRef.current }), credentials: "include", }).catch(() => {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/hook/useMatchmaking.ts` around lines 152 - 163, The cancelMatchmaking function currently calls the non-existent /v1/matchmaking/leave and sends { sessionId }, causing 404s; update the fetch inside cancelMatchmaking to call /v1/matchmaking/cancel and send { request_id: sessionIdRef.current } (keep method POST, headers and credentials as-is) so the backend receives the expected payload and the Redis request is cleared.
🧹 Nitpick comments (3)
frontend/hook/useMatchmaking.ts (1)
21-21: Return type ofjoinMatchmakingis misdeclared.The implementation at line 87 is
async, so it actually returnsPromise<void>, but theUseMatchmakingReturninterface types it as returningvoid. Callers awaiting the result will see the wrong type. Either declare it asPromise<void>(preferred — letspage.tsxawait joinMatchmaking(type)if needed) or wrap the body so it truly returnsvoid.Proposed type fix
- joinMatchmaking: (matchType?: "Rated" | "Casual") => void; + joinMatchmaking: (matchType?: "Rated" | "Casual") => Promise<void>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/hook/useMatchmaking.ts` at line 21, The UseMatchmakingReturn interface incorrectly types joinMatchmaking as returning void while the implemented function joinMatchmaking is async; change the declaration in UseMatchmakingReturn so joinMatchmaking: (matchType?: "Rated" | "Casual") => Promise<void> (or update the joinMatchmaking implementation to not be async) so callers can correctly await joinMatchmaking and types align with the async function.frontend/app/page.tsx (1)
48-48: AvoidanyforbotAnalysis.A typed shape (e.g.
{ bestMove: string; evaluation?: number; depth?: number }) keeps the consumers at lines 152–156 and 290–296 honest — particularly theevaluation?.toFixed(2)access, which currently silently renders empty if the engine omits an evaluation. Ideally the type is exported fromuseStockfishWASMso the hook's contract drives the consumer.Suggested typing
- const [botAnalysis, setBotAnalysis] = useState<any>(null); + type BotAnalysis = { bestMove: string; evaluation?: number; depth?: number }; + const [botAnalysis, setBotAnalysis] = useState<BotAnalysis | null>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` at line 48, Replace the loose any for botAnalysis with a concrete exported type from the useStockfishWASM hook (for example export an interface like BotAnalysis { bestMove: string; evaluation?: number; depth?: number; } from useStockfishWASM) and update the state declaration to use that type (i.e., const [botAnalysis, setBotAnalysis] = useState<BotAnalysis | null>(null)); ensure consumers that call evaluation?.toFixed(2) handle evaluation as number | undefined per the new type so the hook’s contract enforces the shape used by the rendering code and other callers.backend/modules/matchmaking/models.rs (1)
102-108: Consider broadening round-trip assertions.The test only verifies
id,match_type, andmax_elo_diffafter deserialization, leavingplayer.wallet_address,player.elo,player.join_time, andinvite_addressuncovered. A full structural equality check would catch silent regressions (e.g., a future rename of a serde field onPlayer). DerivingPartialEqonPlayer/MatchRequestand assertingreq == deserializedwould simplify and strengthen the test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/matchmaking/models.rs` around lines 102 - 108, The test only checks a few fields after round-tripping; derive PartialEq (and Debug if helpful) for Player and MatchRequest, update the test to assert full structural equality by replacing the individual field asserts with a single assert_eq!(req, deserialized), and ensure the serde-derived structs still compile with the new traits so the round-trip comparison covers player.wallet_address, player.elo, player.join_time, invite_address, etc.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/components/matchmaking/MatchmakingModal.tsx`:
- Around line 17-82: Replace the current modal root and markup in the
MatchmakingModal component with Radix Dialog primitives to get ARIA, Escape
handling, focus trap, backdrop-click-to-close and scroll-lock automatically: use
Dialog.Root (controlled by isOpen), Dialog.Portal, Dialog.Overlay (for the
backdrop), Dialog.Content (for the centered panel) and inside it replace the
heading and description with Dialog.Title and Dialog.Description, and use
Dialog.Close for the Cancel button; wire Dialog.Root's onOpenChange to call the
existing onClose so backdrop/Escape/close buttons close the modal; keep existing
handlers on the Rated/Casual buttons (onConfirm) and preserve all styling
classes when migrating elements.
In `@frontend/app/page.tsx`:
- Around line 290-296: The UI renders an empty "Eval:" row when botAnalysis has
partial data; ensure the whole info row only renders when the values exist by
checking botAnalysis.evaluation and botAnalysis.depth (or both) before
rendering, or supply explicit fallbacks (e.g., "N/A") so evaluation is formatted
safely (use evaluation only after confirming it's a number) and depth is shown
or replaced; update the conditional around the div that uses gameMode,
botAnalysis.evaluation, and botAnalysis.depth (or add null-coalescing fallback
for evaluation and depth) to avoid showing "Eval: " with no value.
- Around line 140-165: The effect can apply a stale/broken bot move because
analyzePosition is awaited without re-validating state; update playBotMove to
snapshot the current FEN and other guards before the await (e.g., const
fenBefore = game.fen(), const turnBefore = game.turn()), then after
analyzePosition resolves re-check active, gameMode === "bot", game.turn() ===
turnBefore (or === "b"), !game.isGameOver(), and game.fen() === fenBefore before
calling setBotAnalysis and applying the move; also wrap game.move(...) in a
try/catch and verify the returned move is non-null (or otherwise legal) before
calling setPosition to avoid silently failing when the underlying game changed.
- Around line 70-72: The bot move effect currently calls analyzePosition from
useStockfishWASM but doesn't gate on the readiness flag; add stockfishReady to
the effect's dependency array and ensure the effect early-returns when
stockfishReady is false so the bot only triggers when the engine is explicitly
ready; update the useEffect that references analyzePosition (the bot move
effect) to include stockfishReady and a guard that exits if !stockfishReady.
In `@frontend/hook/useMatchmaking.ts`:
- Around line 92-102: The hook useMatchmaking currently sends a hardcoded test
wallet ("wallet_address") and fixed elo (1200) in the POST to
`${API_BASE}/v1/matchmaking/join`; replace that with the authenticated wallet
and real player rating: obtain the connected wallet address from the same
provider/context used by Web3StatusBar (e.g., call the wallet context hook used
elsewhere in the app inside useMatchmaking) and fetch the player's current ELO
from the backend (or from user profile state) before composing the join request
body; if this stub is intentional, wrap the test behavior behind a clear feature
flag or dev-only conditional (and add a TODO comment) so production builds
always use the real wallet and ELO.
In `@frontend/package.json`:
- Line 32: Remove the dead npm dependency "stockfish.js" from package.json and
verify the actual runtime engine in public assets: open package.json and delete
the "stockfish.js": "^10.0.2" entry, then confirm that
frontend/public/assets/stockfish.js and frontend/public/assets/stockfish.wasm
contain Stockfish 16.1 (as required by issue `#655`) because StockfishWASM.tsx
loads the engine from /assets/stockfish.js (see the load call in
StockfishWASM.tsx), not from the npm package; if the assets are not 16.1,
replace them with the correct Stockfish 16.1 builds, then run the app build to
ensure the static asset is used and remove any unused imports referencing the
npm package.
---
Outside diff comments:
In `@frontend/hook/useMatchmaking.ts`:
- Around line 152-163: The cancelMatchmaking function currently calls the
non-existent /v1/matchmaking/leave and sends { sessionId }, causing 404s; update
the fetch inside cancelMatchmaking to call /v1/matchmaking/cancel and send {
request_id: sessionIdRef.current } (keep method POST, headers and credentials
as-is) so the backend receives the expected payload and the Redis request is
cleared.
---
Nitpick comments:
In `@backend/modules/matchmaking/models.rs`:
- Around line 102-108: The test only checks a few fields after round-tripping;
derive PartialEq (and Debug if helpful) for Player and MatchRequest, update the
test to assert full structural equality by replacing the individual field
asserts with a single assert_eq!(req, deserialized), and ensure the
serde-derived structs still compile with the new traits so the round-trip
comparison covers player.wallet_address, player.elo, player.join_time,
invite_address, etc.
In `@frontend/app/page.tsx`:
- Line 48: Replace the loose any for botAnalysis with a concrete exported type
from the useStockfishWASM hook (for example export an interface like BotAnalysis
{ bestMove: string; evaluation?: number; depth?: number; } from
useStockfishWASM) and update the state declaration to use that type (i.e., const
[botAnalysis, setBotAnalysis] = useState<BotAnalysis | null>(null)); ensure
consumers that call evaluation?.toFixed(2) handle evaluation as number |
undefined per the new type so the hook’s contract enforces the shape used by the
rendering code and other callers.
In `@frontend/hook/useMatchmaking.ts`:
- Line 21: The UseMatchmakingReturn interface incorrectly types joinMatchmaking
as returning void while the implemented function joinMatchmaking is async;
change the declaration in UseMatchmakingReturn so joinMatchmaking: (matchType?:
"Rated" | "Casual") => Promise<void> (or update the joinMatchmaking
implementation to not be async) so callers can correctly await joinMatchmaking
and types align with the async function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b997fba1-6d94-470d-a354-adb7950b6978
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/public/assets/stockfish.wasmis excluded by!**/*.wasm
📒 Files selected for processing (8)
backend/modules/matchmaking/elo.rsbackend/modules/matchmaking/models.rsbackend/modules/matchmaking/service.rsfrontend/app/components/matchmaking/MatchmakingModal.tsxfrontend/app/page.tsxfrontend/hook/useMatchmaking.tsfrontend/package.jsonfrontend/public/assets/stockfish.js
💤 Files with no reviewable changes (1)
- backend/modules/matchmaking/service.rs
| if (!isOpen) return null; | ||
|
|
||
| return ( | ||
| <div className="fixed inset-0 bg-black/80 backdrop-blur-md z-50 flex items-center justify-center p-4 animate-overlay-in"> | ||
| <div className="bg-gray-900/90 border border-teal-500/30 rounded-3xl p-8 max-w-md w-full shadow-[0_0_50px_rgba(20,184,166,0.15)] animate-modal-in"> | ||
| <h2 className="text-3xl font-extrabold text-white mb-2 tracking-tight text-center bg-gradient-to-r from-teal-400 to-blue-500 bg-clip-text text-transparent"> | ||
| Select Match Type | ||
| </h2> | ||
| <p className="text-gray-400 text-center mb-8 text-sm uppercase tracking-widest font-medium"> | ||
| Choose your competitive level | ||
| </p> | ||
|
|
||
| <div className="grid gap-4"> | ||
| <button | ||
| onClick={() => onConfirm("Rated")} | ||
| className="group relative flex items-center gap-6 p-5 rounded-2xl bg-gradient-to-br from-teal-500/10 to-blue-500/10 border border-teal-500/20 hover:border-teal-400/50 hover:bg-teal-500/20 transition-all duration-300 text-left" | ||
| > | ||
| <div className="bg-teal-500/20 p-4 rounded-xl group-hover:scale-110 transition-transform duration-300"> | ||
| <FaTrophy className="text-2xl text-teal-400" /> | ||
| </div> | ||
| <div> | ||
| <h3 className="text-xl font-bold text-white group-hover:text-teal-300 transition-colors"> | ||
| Rated Match | ||
| </h3> | ||
| <p className="text-gray-400 text-sm">Win or lose ELO points</p> | ||
| </div> | ||
| </button> | ||
|
|
||
| <button | ||
| onClick={() => onConfirm("Casual")} | ||
| className="group relative flex items-center gap-6 p-5 rounded-2xl bg-gradient-to-br from-gray-800/50 to-gray-700/50 border border-gray-700/50 hover:border-blue-400/50 hover:bg-blue-500/10 transition-all duration-300 text-left" | ||
| > | ||
| <div className="bg-blue-500/20 p-4 rounded-xl group-hover:scale-110 transition-transform duration-300"> | ||
| <FaGamepad className="text-2xl text-blue-400" /> | ||
| </div> | ||
| <div> | ||
| <h3 className="text-xl font-bold text-white group-hover:text-blue-300 transition-colors"> | ||
| Casual Match | ||
| </h3> | ||
| <p className="text-gray-400 text-sm">Practice without ELO stakes</p> | ||
| </div> | ||
| </button> | ||
| </div> | ||
|
|
||
| <button | ||
| onClick={onClose} | ||
| className="mt-8 w-full py-3 rounded-xl text-gray-500 hover:text-white transition-colors font-medium text-sm" | ||
| > | ||
| Cancel | ||
| </button> | ||
| </div> | ||
|
|
||
| <style jsx>{` | ||
| @keyframes overlay-in { | ||
| from { opacity: 0; } | ||
| to { opacity: 1; } | ||
| } | ||
| @keyframes modal-in { | ||
| from { opacity: 0; transform: scale(0.9) translateY(20px); } | ||
| to { opacity: 1; transform: scale(1) translateY(0); } | ||
| } | ||
| .animate-overlay-in { animation: overlay-in 0.3s ease-out; } | ||
| .animate-modal-in { animation: modal-in 0.4s cubic-bezier(0.16, 1, 0.3, 1); } | ||
| `}</style> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the recommended way to implement an accessible modal dialog using @radix-ui/react-dialog?
💡 Result:
The recommended way to implement an accessible modal dialog using @radix-ui/react-dialog is to compose all required parts from the official Radix Primitives Dialog component: Root (with modal={true} default), Trigger, Portal, Overlay, Content, Title, Description, and Close. This structure ensures WAI-ARIA compliance, automatic focus trapping, ESC key closure, click-outside handling, screen reader announcements, and keyboard navigation. Key accessibility features are built-in: - Focus is trapped within the modal - Proper ARIA roles and attributes (dialog role on Content) - Title and Description are announced by screen readers (use VisuallyHidden if visually hiding) - Overlay prevents interaction with background content - Body scrolling is blocked when open Installation: npm install @radix-ui/react-dialog Basic implementation: jsx import * as React from 'react'; import * as Dialog from '@radix-ui/react-dialog'; // Optional: import { VisuallyHidden } from '@radix-ui/react-visually-hidden'; export default function AccessibleModal { const [open, setOpen] = React.useState(false); return ( <Dialog.Root open={open} onOpenChange={setOpen}> <Dialog.Trigger asChild> Open Modal </Dialog.Trigger> <Dialog.Portal> <Dialog.Overlay className="overlay" /> <Dialog.Content className="content"> <Dialog.Title className="title"> Modal Title (or wrap in VisuallyHidden to hide visually) </Dialog.Title> <Dialog.Description className="description"> Modal description for screen readers. </Dialog.Description> {/* Your modal content /}
@keyframes overlayShow { from { opacity: 0; } to { opacity: 1; } } @keyframes contentShow { from { opacity: 0; transform: translate(-50%, -48%) scale(0.96); } to { opacity: 1; transform: translate(-50%, -50%) scale(1); } } For controlled state, use open and onOpenChange. Always include Title (required for a11y) and Description (optional, set aria-describedby={undefined} on Content if omitted). Test with screen readers like VoiceOver/NVDA. This follows the official anatomy and adheres to WAI-ARIA Dialog pattern with full keyboard support (Tab, Shift+Tab, Esc).
Citations:
- 1: https://www.radix-ui.com/primitives/docs/components/dialog
- 2: https://www.radix-ui.com/docs/primitives/components/dialog
- 3: https://ilovejs.net/npm/radix-ui-react-dialog
- 4: https://www.radix-ui.com/primitives/docs/overview/introduction
- 5: https://www.radix-ui.com/primitives/docs/overview/accessibility
- 6: https://mintlify.com/radix-ui/primitives/components/dialog
🏁 Script executed:
# Find package.json in frontend directory
find . -name "package.json" -path "*/frontend/*" -type fRepository: NOVUS-X/XLMate
Length of output: 82
🏁 Script executed:
# Check MatchmakingModal.tsx file
find . -name "MatchmakingModal.tsx" -type fRepository: NOVUS-X/XLMate
Length of output: 117
🏁 Script executed:
# Search for page.tsx references to modal/dialog
find . -name "page.tsx" -path "*/frontend/app/*" -type fRepository: NOVUS-X/XLMate
Length of output: 214
🏁 Script executed:
# Check if `@radix-ui/react-dialog` is in frontend/package.json
grep -i "radix\|@radix-ui/react-dialog" ./frontend/package.jsonRepository: NOVUS-X/XLMate
Length of output: 137
🏁 Script executed:
# Read the MatchmakingModal.tsx file to verify content and line numbers
cat -n ./frontend/app/components/matchmaking/MatchmakingModal.tsx | head -100Repository: NOVUS-X/XLMate
Length of output: 4014
🏁 Script executed:
# Check page.tsx for dialog/modal implementations with accessibility attributes
rg -n "role.*dialog|aria-modal" ./frontend/app/page.tsx -A 3 -B 3Repository: NOVUS-X/XLMate
Length of output: 1312
Add missing accessibility primitives to modal dialog.
The modal lacks several critical accessibility and keyboard behaviors already implemented in frontend/app/page.tsx on its inline overlays:
- No
role="dialog"/aria-modal="true"— screen readers will not announce this as a modal - No
aria-labelledby/aria-describedby— title and description are not linked for assistive technologies - No
Escapekey handler to dismiss - No focus trap — Tab navigation can leak to elements behind the overlay; focus is not managed on open/close
- Backdrop click does not close the modal
Recommended: Use @radix-ui/react-dialog (already in frontend/package.json as ^1.1.11), which provides focus trap, Escape handling, scroll lock, and proper ARIA wiring automatically. Alternatively, manually add the missing handlers and ARIA attributes.
Minimal manual hardening (if not adopting Radix)
-export function MatchmakingModal({
- isOpen,
- onClose,
- onConfirm,
-}: MatchmakingModalProps) {
- if (!isOpen) return null;
-
- return (
- <div className="fixed inset-0 bg-black/80 backdrop-blur-md z-50 flex items-center justify-center p-4 animate-overlay-in">
- <div className="bg-gray-900/90 ...">
- <h2 className="...">
+export function MatchmakingModal({
+ isOpen,
+ onClose,
+ onConfirm,
+}: MatchmakingModalProps) {
+ React.useEffect(() => {
+ if (!isOpen) return;
+ const onKey = (e: KeyboardEvent) => {
+ if (e.key === "Escape") onClose();
+ };
+ window.addEventListener("keydown", onKey);
+ return () => window.removeEventListener("keydown", onKey);
+ }, [isOpen, onClose]);
+
+ if (!isOpen) return null;
+
+ return (
+ <div
+ className="fixed inset-0 bg-black/80 backdrop-blur-md z-50 flex items-center justify-center p-4 animate-overlay-in"
+ onClick={onClose}
+ >
+ <div
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="mm-title"
+ aria-describedby="mm-desc"
+ onClick={(e) => e.stopPropagation()}
+ className="bg-gray-900/90 ..."
+ >
+ <h2 id="mm-title" className="...">
Select Match Type
</h2>
- <p className="text-gray-400 ...">
+ <p id="mm-desc" className="text-gray-400 ...">
Choose your competitive level
</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/components/matchmaking/MatchmakingModal.tsx` around lines 17 -
82, Replace the current modal root and markup in the MatchmakingModal component
with Radix Dialog primitives to get ARIA, Escape handling, focus trap,
backdrop-click-to-close and scroll-lock automatically: use Dialog.Root
(controlled by isOpen), Dialog.Portal, Dialog.Overlay (for the backdrop),
Dialog.Content (for the centered panel) and inside it replace the heading and
description with Dialog.Title and Dialog.Description, and use Dialog.Close for
the Cancel button; wire Dialog.Root's onOpenChange to call the existing onClose
so backdrop/Escape/close buttons close the modal; keep existing handlers on the
Rated/Casual buttons (onConfirm) and preserve all styling classes when migrating
elements.
| {gameMode === "bot" && botAnalysis && ( | ||
| <div className="flex items-center gap-4 text-xs font-mono text-teal-400"> | ||
| <span>Eval: {botAnalysis.evaluation > 0 ? "+" : ""}{botAnalysis.evaluation?.toFixed(2)}</span> | ||
| <span className="opacity-50">|</span> | ||
| <span>Depth: {botAnalysis.depth}</span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Defensive rendering for partial bot analysis.
botAnalysis.evaluation > 0 ? "+" : "" followed by botAnalysis.evaluation?.toFixed(2) is inconsistent: if evaluation is undefined the comparison is false (no sign) and ?.toFixed(2) renders nothing, so the user sees "Eval: " with no value. Same issue for depth. Either short-circuit the whole row or provide explicit fallbacks.
Suggested fix
- {gameMode === "bot" && botAnalysis && (
+ {gameMode === "bot" && botAnalysis && typeof botAnalysis.evaluation === "number" && (
<div className="flex items-center gap-4 text-xs font-mono text-teal-400">
- <span>Eval: {botAnalysis.evaluation > 0 ? "+" : ""}{botAnalysis.evaluation?.toFixed(2)}</span>
+ <span>Eval: {botAnalysis.evaluation > 0 ? "+" : ""}{botAnalysis.evaluation.toFixed(2)}</span>
<span className="opacity-50">|</span>
- <span>Depth: {botAnalysis.depth}</span>
+ <span>Depth: {botAnalysis.depth ?? "—"}</span>
</div>
)}📝 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.
| {gameMode === "bot" && botAnalysis && ( | |
| <div className="flex items-center gap-4 text-xs font-mono text-teal-400"> | |
| <span>Eval: {botAnalysis.evaluation > 0 ? "+" : ""}{botAnalysis.evaluation?.toFixed(2)}</span> | |
| <span className="opacity-50">|</span> | |
| <span>Depth: {botAnalysis.depth}</span> | |
| </div> | |
| )} | |
| {gameMode === "bot" && botAnalysis && typeof botAnalysis.evaluation === "number" && ( | |
| <div className="flex items-center gap-4 text-xs font-mono text-teal-400"> | |
| <span>Eval: {botAnalysis.evaluation > 0 ? "+" : ""}{botAnalysis.evaluation.toFixed(2)}</span> | |
| <span className="opacity-50">|</span> | |
| <span>Depth: {botAnalysis.depth ?? "—"}</span> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/page.tsx` around lines 290 - 296, The UI renders an empty
"Eval:" row when botAnalysis has partial data; ensure the whole info row only
renders when the values exist by checking botAnalysis.evaluation and
botAnalysis.depth (or both) before rendering, or supply explicit fallbacks
(e.g., "N/A") so evaluation is formatted safely (use evaluation only after
confirming it's a number) and depth is shown or replaced; update the conditional
around the div that uses gameMode, botAnalysis.evaluation, and botAnalysis.depth
(or add null-coalescing fallback for evaluation and depth) to avoid showing
"Eval: " with no value.
…ervice #543 Fixes from CodeRabbit review on PR #695: - fix(matchmaking): correct cancel endpoint from /leave to /v1/matchmaking/cancel with proper request_id payload matching backend MatchmakingService::cancel_request - fix(matchmaking): remove hardcoded TEST_WALLET_ address and fixed elo 1200; wallet and elo are now resolved server-side from the authenticated JWT session - fix(matchmaking): change joinMatchmaking return type from void to Promise<void> to match the async implementation - fix(matchmaking): remove dead stockfish.js@^10.0.2 npm dependency; engine loads from /assets/stockfish.js at runtime (already bundled in public/) - fix(page): guard bot analyzePosition call with stockfishReady flag and capture FEN before await to discard stale results if position changed (race condition) - fix(page): guard opponent move application with gameMode check to prevent stale moves being applied after game reset (race condition) - fix(page): show N/A instead of empty string when bot evaluation is undefined - fix(modal): add role=dialog, aria-modal, aria-labelledby, Escape key handler, and focus trap to MatchmakingModal for full accessibility compliance - fix(models): derive PartialEq on Player and MatchRequest; expand serialization tests to full structural equality across Rated, Casual, and Private variants feat: implement tournament bracket management service (closes #543) - feat(backend): add bracket.rs to tournament module with BracketService supporting SingleElimination, DoubleElimination, and RoundRobin formats; ELO-based seeding; automatic byes for non-power-of-two brackets; winner advancement; standings - test(backend): 12 unit tests covering creation, seeding, bye slots, round progression, completion, and all error paths (BracketError variants) - feat(frontend): add BracketView component with visual elimination grid and round-robin standings table, champion banner, and participant list - feat(frontend): add /tournament page with tournament list sidebar, create form with format selection, and integrated BracketView Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/hook/useMatchmaking.ts (1)
103-110:⚠️ Potential issue | 🟡 MinorValidate
sessionIdbefore opening the WebSocket.If the backend ever returns a response that has neither
request_idnorsessionId, line 104 yieldsundefined, line 105 storesundefinedin the ref, and line 109 builds a URL containing literally?session=undefined. The WS will connect, get rejected/timed out, and the user lands in theerrorstatus with no useful diagnostic. Worth failing fast here.🛡️ Suggested guard
const json = await res.json(); - const sessionId = json.request_id || json.sessionId; + const sessionId: string | undefined = json.request_id ?? json.sessionId; + if (!sessionId) { + throw new Error("Matchmaking response missing request_id"); + } sessionIdRef.current = sessionId;Also note:
sessionIdRef.currentis later sent asrequest_idto/v1/matchmaking/cancel, which deserializes into aUuidon the backend (seebackend/modules/matchmaking/routes.rsCancelRequest). The legacysessionIdfallback may not be aUuid, so a cancel using that fallback would fail backend deserialization. If the legacy field is truly gone server-side, dropping the fallback altogether would be cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/hook/useMatchmaking.ts` around lines 103 - 110, Validate the parsed sessionId before opening the WebSocket: after reading const json = await res.json() and computing const sessionId = json.request_id || json.sessionId, check that sessionId is a non-empty, valid value (and preferably a UUID if your backend expects one); if it's falsy or malformed, set the hook's error state (or return/fail fast), log the raw response for diagnostics, and do not assign sessionIdRef.current or instantiate new WebSocket(`${WS_BASE}/v1/matchmaking/ws?session=${sessionId}`). Also consider removing the legacy sessionId fallback (json.sessionId) so cancel requests that use sessionIdRef.current always send request_id (a UUID) to /v1/matchmaking/cancel to avoid backend deserialization errors.
♻️ Duplicate comments (1)
frontend/app/page.tsx (1)
291-297:⚠️ Potential issue | 🟡 MinorDepth still missing a fallback.
Evaluation now correctly handles null/undefined via the
!= nullcheck, butbotAnalysis.depthon line 295 is rendered raw. Ifdepthisundefined(partial result, or any future engine path that emitsbestmovebefore a depth update), the user seesDepth:with an empty value — same shape of bug the evaluation fix addressed.💚 Suggested fix
- <span>Depth: {botAnalysis.depth}</span> + <span>Depth: {botAnalysis.depth ?? "—"}</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` around lines 291 - 297, The Depth display can be empty when botAnalysis.depth is undefined; update the rendering where gameMode === "bot" && botAnalysis is used to render Depth (the span that currently shows "Depth: {botAnalysis.depth}") to include a fallback similar to the evaluation check — e.g., render botAnalysis.depth when not null/undefined otherwise a placeholder like "N/A" or "—". Ensure you use the same null check style (botAnalysis.depth != null) and update the span that references botAnalysis.depth so partial results don't show an empty value.
🧹 Nitpick comments (8)
frontend/app/page.tsx (4)
285-288:aiPersonalitymay render asundefined.If
aiPersonalityis unset when bot mode is opened (before the personality modal is confirmed), the header readsBot: undefined. A small fallback (aiPersonality ?? "balanced"or similar) keeps the UI clean during the brief window before personality selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` around lines 285 - 288, The Bot header can render "undefined" because aiPersonality may be unset; update the JSX that renders the bot label (the ternary using gameMode and onlineStatusLabel) to use a safe fallback for aiPersonality (e.g., aiPersonality ?? "balanced" or a short default string) so the else branch becomes `Bot: ${aiPersonality ?? "balanced"}` and avoids displaying undefined before the personality modal is confirmed.
168-173:isMyTurndefaults totruewhengameModeisnull.Before any mode is picked,
gameMode === "bot"is false → first clause istrue, second clause isgameMode !== "online"→true, soisMyTurn === true.handleMoveis only callable when the board is rendered with agameMode(the chessboard panel is gated ongameModefor game state UI), but the value beingtruewhile no game exists is still a small footgun if this flag is ever consumed elsewhere.♻️ Suggested tightening
- const isMyTurn = - (gameMode === "bot" ? game.turn() === "w" : true) && - (gameMode !== "online" || - (socketStatus === "connected" && - ((playerColor === "white" && game.turn() === "w") || - (playerColor === "black" && game.turn() === "b")))); + const isMyTurn = + gameMode === "bot" + ? game.turn() === "w" + : gameMode === "online" + ? socketStatus === "connected" && + ((playerColor === "white" && game.turn() === "w") || + (playerColor === "black" && game.turn() === "b")) + : false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` around lines 168 - 173, The isMyTurn computation can be true when gameMode is null; tighten it by short-circuiting on a valid gameMode before evaluating the existing logic: ensure gameMode is not null/undefined (e.g., gameMode && ...) or explicitly check it equals one of the valid modes before using game.turn(), playerColor, or socketStatus. Update the isMyTurn expression (the constant named isMyTurn) so it returns false when no gameMode is selected, leaving the existing bot/online branch logic (game.turn(), playerColor, socketStatus) intact; this prevents handleMove or other consumers from seeing a spurious true when no game exists.
48-48: AvoidanyforbotAnalysis.The
AnalysisResultshape returned byanalyzePositionis well-defined inStockfishWASM.tsx. Usinganyhere forfeits the typing benefits and is what allowed the partial-render bug below (line 295) to slip through previously.♻️ Suggested fix
-import useStockfishWASM from "@/components/chess/StockfishWASM"; +import useStockfishWASM, { type AnalysisResult } from "@/components/chess/StockfishWASM"; ... - const [botAnalysis, setBotAnalysis] = useState<any>(null); + const [botAnalysis, setBotAnalysis] = useState<AnalysisResult | null>(null);(adjust the import path/name if
AnalysisResultisn't exported from the module — exporting it is a small, worthwhile change.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` at line 48, Replace the loose any for botAnalysis with the concrete AnalysisResult type: import (or export) AnalysisResult from StockfishWASM.tsx and change the useState declaration for botAnalysis to useState<AnalysisResult | null>(null); update any places that read or write botAnalysis (e.g., where analyzePosition is called and where the partial-render bug occurred) to respect the typed shape so fields are accessed safely rather than assumed.
228-236: Matchmaking flow handlers look right;joinMatchmakingpromise floats.The open/close/confirm wiring is clean and
handleMatchmakingClosecorrectly resetsgameModeso a cancel from the modal returns the user to the mode picker. Minor:joinMatchmaking(type)returnsPromise<void>and is invoked unawaited on line 230. The hook handles its own errors internally (setserror/status), so this is functionally fine, but avoid joinMatchmaking(type);(or.catch(() => {})) keeps@typescript-eslint/no-floating-promiseshappy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` around lines 228 - 236, handleMatchmakingConfirm currently calls joinMatchmaking(type) without handling the returned Promise which triggers the no-floating-promises lint rule; update handleMatchmakingConfirm to explicitly acknowledge the promise by either prefixing the call with void (e.g., void joinMatchmaking(type)) or appending .catch(() => {}) so the Promise returned by joinMatchmaking is not left unhandled while preserving existing behavior; locate the handler function named handleMatchmakingConfirm and apply one of these two minimal changes.backend/modules/matchmaking/models.rs (1)
76-149: Tests look solid; consider one resilience tweak.Coverage spans all three
MatchTypevariants and bothNone/Someshapes for the optional fields, which is good. One small concern: the round-trip tests useUtc::now()directly. Round-trip equality ofDateTime<Utc>throughserde_jsondoes work because chrono serializes as an RFC3339 string with nanoseconds, but if you ever swap the serializer (e.g., to a format that drops sub-second precision or normalizes to seconds), these tests will start flaking unpredictably. A fixed timestamp viaUtc.with_ymd_and_hms(...)(or aDateTime::parse_from_rfc3339literal) would make the assertion intent explicit and the tests deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/matchmaking/models.rs` around lines 76 - 149, The tests use Utc::now() for Player.join_time which can make equality flaky if serialization precision changes; replace the dynamic timestamps in test_match_request_round_trip, test_match_request_with_invite_address_round_trip, and test_casual_match_request_round_trip with a fixed DateTime (e.g., construct via Utc.with_ymd_and_hms(...) or parse a literal with DateTime::parse_from_rfc3339 and convert to Utc) and set Player.join_time to that constant so the serde round-trip assertions remain deterministic.frontend/app/tournament/page.tsx (1)
32-50: Optional: silencereact-hooks/exhaustive-depsand avoid staleselectedcapture.
fetchBracketscloses overselected, which is currently safe because it's only invoked on mount, but the function-in-render pattern will trigger lint warnings and is fragile if it's ever called from another effect/handler. Consider lifting it intouseCallback(and updating the deps) or inlining the body in the effect.♻️ Suggested refactor
- useEffect(() => { - fetchBrackets(); - }, []); - - async function fetchBrackets() { - setLoading(true); - setError(null); - try { - const res = await fetch(`${API_BASE}/v1/tournaments`, { credentials: "include" }); - if (!res.ok) throw new Error(`Server error: ${res.status}`); - const data: TournamentBracket[] = await res.json(); - setBrackets(data); - if (data.length > 0 && !selected) setSelected(data[0]); - } catch (e) { - setError(e instanceof Error ? e.message : "Failed to load tournaments"); - } finally { - setLoading(false); - } - } + const fetchBrackets = useCallback(async () => { + setLoading(true); + setError(null); + try { + const res = await fetch(`${API_BASE}/v1/tournaments`, { credentials: "include" }); + if (!res.ok) throw new Error(`Server error: ${res.status}`); + const data: TournamentBracket[] = await res.json(); + setBrackets(data); + setSelected((prev) => prev ?? data[0] ?? null); + } catch (e) { + setError(e instanceof Error ? e.message : "Failed to load tournaments"); + } finally { + setLoading(false); + } + }, []); + + useEffect(() => { + fetchBrackets(); + }, [fetchBrackets]);(Don't forget
import { useCallback } from "react".)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/tournament/page.tsx` around lines 32 - 50, The fetchBrackets function is defined inside the component and closes over selected, which triggers react-hooks/exhaustive-deps warnings and can lead to stale captures; move fetchBrackets out of the render by wrapping it with useCallback (e.g., const fetchBrackets = useCallback(async () => { ... }, [setBrackets, setLoading, setError, setSelected, API_BASE, selected?]), or alternatively inline its body directly inside the useEffect, and then update the useEffect deps to include fetchBrackets (or the values used) so selected is read from the latest state and the linter is satisfied; ensure you import useCallback if you choose that approach and keep the same error/loading/setSelected behavior.frontend/components/tournament/BracketView.tsx (1)
120-124: Avoid mutating thematchesprop during render.
Array.prototype.sortsorts in place. Today the parent passes a freshly-filtered array so this is safe, but it's a fragile invariant — any caller that passes a memoized or shared array would silently be reordered on every render. Prefer[...matches].sort(...)ormatches.toSorted(...).♻️ Proposed change
- {matches - .sort((a, b) => a.match_number - b.match_number) + {[...matches] + .sort((a, b) => a.match_number - b.match_number) .map((m) => ( <MatchCard key={m.id} match={m} bracket={bracket} /> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/tournament/BracketView.tsx` around lines 120 - 124, The render currently calls matches.sort(...) which mutates the incoming matches array; change this to sort a copy instead (e.g., use [...matches].sort(...) or matches.toSorted(...)) so the prop is not mutated during render; update the map block that creates <MatchCard key={m.id} match={m} bracket={bracket} /> to iterate over the non-mutating sorted copy and keep the same key/props.backend/modules/tournament/src/bracket.rs (1)
291-320: Minor: round-robin "round" assignment doesn't represent a real round-robin schedule.The pairing loop emits matches in lexicographic order
(i, j)and bumpsroundwhenevermatch_number > matches_per_round. This packs matches by count but ignores the constraint that a player can only play once per round. For example with 4 players you get round 1 =(0,1)and(0,2)— player 0 plays twice in round 1.If "round" is purely cosmetic for the UI this is harmless; if any scheduling logic (or the planned UI) treats a round as "matches that can be played simultaneously", you'll need a proper circle/round-robin schedule (Berger tables / circle method). Worth at minimum a doc comment so future readers don't rely on the round numbers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/tournament/src/bracket.rs` around lines 291 - 320, The current generate_round_robin_matches emits lexicographic pairs and increments round by count, allowing a player to appear multiple times in the same round; replace that with a proper circle/Berger round-robin generator: if participants.len() is odd, add a dummy/bye, then perform n-1 rounds (or n rounds when using a bye) where in each round you pair the first half with the reversed second half after rotating the array (so each player appears at most once per round), create BracketMatch entries with round set to the current round and match_number starting at 1 for each round, and preserve other fields (id, player1_id, player2_id, status, etc.); alternatively, if you decide round is only cosmetic, add a clear doc comment on generate_round_robin_matches and leave the current pairing but document that rounds are not simultaneous-play rounds so callers must not assume mutual-exclusivity of players within a round.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/modules/tournament/src/bracket.rs`:
- Around line 322-330: determine_round_robin_winner currently picks a winner
non-deterministically via HashMap iteration; change it to compute the maximum
win count, collect all candidate Uuids with that count, and if more than one
apply a deterministic tie-breaker (prefer a seeded/eloed participant if
available by looking up the participant in TournamentBracket.participants or
participant metadata, otherwise pick the smallest/lexicographic Uuid) so
repeated runs yield the same winner; update the function
determine_round_robin_winner to implement this candidate collection +
deterministic comparator and return the chosen Uuid (or None).
- Around line 237-289: generate_single_elimination_matches currently marks bye
matches as Completed but never writes their winners into the next round, causing
deadlocks; after constructing round 1 matches (the loop that pushes BracketMatch
with round == 1), iterate the completed bye matches and for each compute the
destination match in round 2 as dest_round = 2 and dest_match_number =
(match.match_number + 1) / 2, then write the winner id into dest.player1_id if
match.match_number is odd else dest.player2_id (use BracketMatch.round,
BracketMatch.match_number, player1_id/player2_id, winner_id), creating or
updating the corresponding BracketMatch in the matches vector before building
subsequent rounds; also add a unit test that records the non-bye semifinal and
asserts the final match has both players populated.
- Around line 111-124: The DoubleElimination branch is unimplemented and
currently falls through to SingleElimination; update the backend to either fully
implement double-elimination or remove the enum/option. If implementing: add
distinct logic where BracketFormat::DoubleElimination computes winners and
losers bracket sizes (total_rounds for winners and losers ≈ winners_rounds - 1),
replace generate_single_elimination_matches call with a new
generate_double_elimination_matches that returns both winners and losers bracket
matches, and update record_result to route losers from each winners round into
the corresponding losers round and handle grand-final + possible reset; ensure
all references to BracketFormat::DoubleElimination (including total_rounds,
matches generation, and record_result) are updated and add tests exercising
two-loss elimination. If removing: delete DoubleElimination from BracketFormat
and remove it from FORMAT_OPTIONS and any UI-facing code so only
SingleElimination/RoundRobin remain, and update tests accordingly.
In `@frontend/app/page.tsx`:
- Around line 70-72: The inline config object passed to useStockfishWASM in
page.tsx causes a new reference every render and forces StockfishWASM (see
useStockfishWASM and the useEffect in StockfishWASM.tsx that depends on config)
to reinitialize; fix by providing a stable config reference—either hoist a
constant (e.g., STOCKFISH_CONFIG = { jsBridgePath: "/assets/stockfish.js" })
outside the component or wrap the object in useMemo(() => ({ jsBridgePath:
"/assets/stockfish.js" }), []) before calling useStockfishWASM—so
analyzePosition/stockfishReady are not repeatedly torn down and recreated.
In `@frontend/app/tournament/page.tsx`:
- Around line 57-64: Add an HTTP handler that exposes POST /v1/tournaments and
forwards requests to BracketService::create_bracket; accept a JSON payload with
at minimum { name: string, participants: string[] } (or implement a two-step
flow where an authenticated user registers and then a separate endpoint triggers
bracket creation once >=2 participants exist), validate participants length
(>=2) before calling create_bracket, authenticate the caller using the same
middleware used by other routes, return appropriate 400/401/500 errors for bad
input/auth/failures, and wire the new handler into the server router so the
frontend POST to /v1/tournaments no longer returns 404.
---
Outside diff comments:
In `@frontend/hook/useMatchmaking.ts`:
- Around line 103-110: Validate the parsed sessionId before opening the
WebSocket: after reading const json = await res.json() and computing const
sessionId = json.request_id || json.sessionId, check that sessionId is a
non-empty, valid value (and preferably a UUID if your backend expects one); if
it's falsy or malformed, set the hook's error state (or return/fail fast), log
the raw response for diagnostics, and do not assign sessionIdRef.current or
instantiate new WebSocket(`${WS_BASE}/v1/matchmaking/ws?session=${sessionId}`).
Also consider removing the legacy sessionId fallback (json.sessionId) so cancel
requests that use sessionIdRef.current always send request_id (a UUID) to
/v1/matchmaking/cancel to avoid backend deserialization errors.
---
Duplicate comments:
In `@frontend/app/page.tsx`:
- Around line 291-297: The Depth display can be empty when botAnalysis.depth is
undefined; update the rendering where gameMode === "bot" && botAnalysis is used
to render Depth (the span that currently shows "Depth: {botAnalysis.depth}") to
include a fallback similar to the evaluation check — e.g., render
botAnalysis.depth when not null/undefined otherwise a placeholder like "N/A" or
"—". Ensure you use the same null check style (botAnalysis.depth != null) and
update the span that references botAnalysis.depth so partial results don't show
an empty value.
---
Nitpick comments:
In `@backend/modules/matchmaking/models.rs`:
- Around line 76-149: The tests use Utc::now() for Player.join_time which can
make equality flaky if serialization precision changes; replace the dynamic
timestamps in test_match_request_round_trip,
test_match_request_with_invite_address_round_trip, and
test_casual_match_request_round_trip with a fixed DateTime (e.g., construct via
Utc.with_ymd_and_hms(...) or parse a literal with DateTime::parse_from_rfc3339
and convert to Utc) and set Player.join_time to that constant so the serde
round-trip assertions remain deterministic.
In `@backend/modules/tournament/src/bracket.rs`:
- Around line 291-320: The current generate_round_robin_matches emits
lexicographic pairs and increments round by count, allowing a player to appear
multiple times in the same round; replace that with a proper circle/Berger
round-robin generator: if participants.len() is odd, add a dummy/bye, then
perform n-1 rounds (or n rounds when using a bye) where in each round you pair
the first half with the reversed second half after rotating the array (so each
player appears at most once per round), create BracketMatch entries with round
set to the current round and match_number starting at 1 for each round, and
preserve other fields (id, player1_id, player2_id, status, etc.); alternatively,
if you decide round is only cosmetic, add a clear doc comment on
generate_round_robin_matches and leave the current pairing but document that
rounds are not simultaneous-play rounds so callers must not assume
mutual-exclusivity of players within a round.
In `@frontend/app/page.tsx`:
- Around line 285-288: The Bot header can render "undefined" because
aiPersonality may be unset; update the JSX that renders the bot label (the
ternary using gameMode and onlineStatusLabel) to use a safe fallback for
aiPersonality (e.g., aiPersonality ?? "balanced" or a short default string) so
the else branch becomes `Bot: ${aiPersonality ?? "balanced"}` and avoids
displaying undefined before the personality modal is confirmed.
- Around line 168-173: The isMyTurn computation can be true when gameMode is
null; tighten it by short-circuiting on a valid gameMode before evaluating the
existing logic: ensure gameMode is not null/undefined (e.g., gameMode && ...) or
explicitly check it equals one of the valid modes before using game.turn(),
playerColor, or socketStatus. Update the isMyTurn expression (the constant named
isMyTurn) so it returns false when no gameMode is selected, leaving the existing
bot/online branch logic (game.turn(), playerColor, socketStatus) intact; this
prevents handleMove or other consumers from seeing a spurious true when no game
exists.
- Line 48: Replace the loose any for botAnalysis with the concrete
AnalysisResult type: import (or export) AnalysisResult from StockfishWASM.tsx
and change the useState declaration for botAnalysis to useState<AnalysisResult |
null>(null); update any places that read or write botAnalysis (e.g., where
analyzePosition is called and where the partial-render bug occurred) to respect
the typed shape so fields are accessed safely rather than assumed.
- Around line 228-236: handleMatchmakingConfirm currently calls
joinMatchmaking(type) without handling the returned Promise which triggers the
no-floating-promises lint rule; update handleMatchmakingConfirm to explicitly
acknowledge the promise by either prefixing the call with void (e.g., void
joinMatchmaking(type)) or appending .catch(() => {}) so the Promise returned by
joinMatchmaking is not left unhandled while preserving existing behavior; locate
the handler function named handleMatchmakingConfirm and apply one of these two
minimal changes.
In `@frontend/app/tournament/page.tsx`:
- Around line 32-50: The fetchBrackets function is defined inside the component
and closes over selected, which triggers react-hooks/exhaustive-deps warnings
and can lead to stale captures; move fetchBrackets out of the render by wrapping
it with useCallback (e.g., const fetchBrackets = useCallback(async () => { ...
}, [setBrackets, setLoading, setError, setSelected, API_BASE, selected?]), or
alternatively inline its body directly inside the useEffect, and then update the
useEffect deps to include fetchBrackets (or the values used) so selected is read
from the latest state and the linter is satisfied; ensure you import useCallback
if you choose that approach and keep the same error/loading/setSelected
behavior.
In `@frontend/components/tournament/BracketView.tsx`:
- Around line 120-124: The render currently calls matches.sort(...) which
mutates the incoming matches array; change this to sort a copy instead (e.g.,
use [...matches].sort(...) or matches.toSorted(...)) so the prop is not mutated
during render; update the map block that creates <MatchCard key={m.id} match={m}
bracket={bracket} /> to iterate over the non-mutating sorted copy and keep the
same key/props.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2fe0050c-f7df-418f-a613-6dcdefba4a06
📒 Files selected for processing (9)
backend/modules/matchmaking/models.rsbackend/modules/tournament/src/bracket.rsbackend/modules/tournament/src/lib.rsfrontend/app/components/matchmaking/MatchmakingModal.tsxfrontend/app/page.tsxfrontend/app/tournament/page.tsxfrontend/components/tournament/BracketView.tsxfrontend/hook/useMatchmaking.tsfrontend/package.json
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/app/components/matchmaking/MatchmakingModal.tsx
| let total_rounds = match &format { | ||
| BracketFormat::SingleElimination | BracketFormat::DoubleElimination => { | ||
| let size = next_power_of_two(participants.len() as u32); | ||
| (size as f64).log2() as u32 | ||
| } | ||
| BracketFormat::RoundRobin => (participants.len() as u32).saturating_sub(1), | ||
| }; | ||
|
|
||
| let matches = match &format { | ||
| BracketFormat::SingleElimination | BracketFormat::DoubleElimination => { | ||
| generate_single_elimination_matches(&participants, total_rounds) | ||
| } | ||
| BracketFormat::RoundRobin => generate_round_robin_matches(&participants), | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm there is no losers-bracket logic anywhere in the module.
rg -nP -C2 '\b(loser|losers_bracket|grand_final|bracket_reset)\b'Repository: NOVUS-X/XLMate
Length of output: 3597
🏁 Script executed:
# First, locate and examine the bracket.rs file
fd -n "bracket.rs" --type fRepository: NOVUS-X/XLMate
Length of output: 289
🏁 Script executed:
# Read the bracket.rs file to verify the code and check for DoubleElimination handling
wc -l backend/modules/tournament/src/bracket.rsRepository: NOVUS-X/XLMate
Length of output: 104
🏁 Script executed:
# Look at the BracketFormat enum definition
rg -n "enum BracketFormat" -A 10 backend/modules/tournament/src/bracket.rsRepository: NOVUS-X/XLMate
Length of output: 304
🏁 Script executed:
# Examine the record_result function around line 185
sed -n '180,220p' backend/modules/tournament/src/bracket.rsRepository: NOVUS-X/XLMate
Length of output: 1760
🏁 Script executed:
# Check lines 111-124 to verify the snippet
sed -n '111,124p' backend/modules/tournament/src/bracket.rsRepository: NOVUS-X/XLMate
Length of output: 732
🏁 Script executed:
# Search for tests related to DoubleElimination
rg -n "DoubleElimination" backend/modules/tournament/src/bracket.rsRepository: NOVUS-X/XLMate
Length of output: 350
🏁 Script executed:
# Search for test module and coverage
rg -n "#\[test\]|mod tests|DoubleElimination" backend/modules/tournament/src/Repository: NOVUS-X/XLMate
Length of output: 2342
🏁 Script executed:
# Look for frontend FORMAT_OPTIONS or similar references
fd -e tsx -e ts -e jsx -e js | xargs rg "FORMAT_OPTIONS|DoubleElimination" 2>/dev/null | head -20Repository: NOVUS-X/XLMate
Length of output: 731
🏁 Script executed:
# Check tests in bracket.rs to see if any test DoubleElimination
rg -n "DoubleElimination" backend/modules/tournament/src/bracket.rs | grep -i testRepository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Look at the tests section to see what formats are tested
sed -n '333,585p' backend/modules/tournament/src/bracket.rs | head -100Repository: NOVUS-X/XLMate
Length of output: 3474
🏁 Script executed:
# Search for any test that mentions double or elimination
rg -n "double|elimination" -i backend/modules/tournament/src/bracket.rs | grep -i testRepository: NOVUS-X/XLMate
Length of output: 40
Major: DoubleElimination has no distinct implementation — it is identical to SingleElimination.
Both total_rounds (line 112) and matches generation (line 120) fall through the same arm as SingleElimination, and record_result (line 185) advances winners with no losers-bracket handling. A double-elimination tournament must:
- generate a losers bracket (size ≈ winners bracket − 1),
- route losers from each winners-bracket round into the corresponding losers round,
- handle the grand final (and potential reset).
As shipped, selecting "Double Elimination" in the UI yields a single-elimination bracket — the frontend even promises "Two losses to be eliminated" (FORMAT_OPTIONS), but the backend silently implements single-elimination semantics. Either:
- implement double elimination properly, or
- remove
DoubleEliminationfromBracketFormat(and the frontendFORMAT_OPTIONS) until it is implemented.
There is no test exercising double-elimination semantics; please add coverage when implementing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/tournament/src/bracket.rs` around lines 111 - 124, The
DoubleElimination branch is unimplemented and currently falls through to
SingleElimination; update the backend to either fully implement
double-elimination or remove the enum/option. If implementing: add distinct
logic where BracketFormat::DoubleElimination computes winners and losers bracket
sizes (total_rounds for winners and losers ≈ winners_rounds - 1), replace
generate_single_elimination_matches call with a new
generate_double_elimination_matches that returns both winners and losers bracket
matches, and update record_result to route losers from each winners round into
the corresponding losers round and handle grand-final + possible reset; ensure
all references to BracketFormat::DoubleElimination (including total_rounds,
matches generation, and record_result) are updated and add tests exercising
two-loss elimination. If removing: delete DoubleElimination from BracketFormat
and remove it from FORMAT_OPTIONS and any UI-facing code so only
SingleElimination/RoundRobin remain, and update tests accordingly.
| fn generate_single_elimination_matches( | ||
| participants: &[TournamentParticipant], | ||
| total_rounds: u32, | ||
| ) -> Vec<BracketMatch> { | ||
| let bracket_size = next_power_of_two(participants.len() as u32) as usize; | ||
| let mut matches = Vec::new(); | ||
|
|
||
| // Round 1: standard seeding (1 vs N, 2 vs N-1, ...) | ||
| let r1_count = bracket_size / 2; | ||
| for i in 0..r1_count { | ||
| let p1 = participants.get(i).map(|p| p.id); | ||
| let p2 = participants.get(bracket_size - 1 - i).map(|p| p.id); | ||
|
|
||
| let (p1_id, p2_id, status, winner_id, completed_at) = match (p1, p2) { | ||
| // Bye: top seed advances automatically | ||
| (Some(id), None) => (Some(id), None, MatchStatus::Completed, Some(id), Some(Utc::now())), | ||
| (Some(id1), Some(id2)) => (Some(id1), Some(id2), MatchStatus::Pending, None, None), | ||
| _ => (None, None, MatchStatus::Pending, None, None), | ||
| }; | ||
|
|
||
| matches.push(BracketMatch { | ||
| id: Uuid::new_v4(), | ||
| round: 1, | ||
| match_number: (i + 1) as u32, | ||
| player1_id: p1_id, | ||
| player2_id: p2_id, | ||
| winner_id, | ||
| status, | ||
| scheduled_at: None, | ||
| completed_at, | ||
| }); | ||
| } | ||
|
|
||
| // Subsequent rounds: slots are filled as winners advance | ||
| for round in 2..=total_rounds { | ||
| let count = bracket_size / 2_usize.pow(round); | ||
| for i in 0..count { | ||
| matches.push(BracketMatch { | ||
| id: Uuid::new_v4(), | ||
| round, | ||
| match_number: (i + 1) as u32, | ||
| player1_id: None, | ||
| player2_id: None, | ||
| winner_id: None, | ||
| status: MatchStatus::Pending, | ||
| scheduled_at: None, | ||
| completed_at: None, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| matches | ||
| } |
There was a problem hiding this comment.
Critical: bye winners are never advanced — brackets with non-power-of-two player counts deadlock.
Round 1 byes are auto-completed in generate_single_elimination_matches (lines 250-255), but subsequent rounds are created with player1_id: None, player2_id: None and the only path that propagates a winner to the next round is inside record_result. Since record_result is never called for a bye, the bye winner is never written into round 2.
Trace for a 3-player bracket:
- Created: r1m1 = bye(seed 1, Completed, winner=seed1), r1m2 = seed2 vs seed3 (Pending), r2m1 = (None, None, Pending).
record_result(r1m2, X)⇒ evenmatch_number⇒ X is placed inr2m1.player2_id. r2m1 is now(None, X, Pending).record_result(r2m1, seed1)fails the membership checkplayer1_id != Some(seed1) && player2_id != Some(seed1)⇒ returnsPlayerNotInMatch.- The tournament is stuck — there is no way to record the final.
The existing test bye_slots_auto_advance_top_seed_in_odd_bracket (lines 504-522) only asserts that the bye match itself is Completed; it does not verify advancement. Please propagate bye winners during bracket creation (or whenever a Completed match is added) and add a test that records the non-bye result and asserts the final match has both players.
🐛 Sketch of fix in generate_single_elimination_matches
After pushing round 1 with byes, walk completed byes and pre-fill round 2 slots before subsequent rounds are filled in:
// Round 1: standard seeding (1 vs N, 2 vs N-1, ...)
@@
}
+
+ // Pre-advance bye winners into round 2 so they don't get lost.
+ let r1_byes: Vec<_> = matches
+ .iter()
+ .filter(|m| m.round == 1 && m.status == MatchStatus::Completed)
+ .map(|m| (m.match_number, m.winner_id))
+ .collect();
// Subsequent rounds: slots are filled as winners advance
for round in 2..=total_rounds {
let count = bracket_size / 2_usize.pow(round);
for i in 0..count {
matches.push(BracketMatch {
id: Uuid::new_v4(),
round,
match_number: (i + 1) as u32,
player1_id: None,
player2_id: None,
winner_id: None,
status: MatchStatus::Pending,
scheduled_at: None,
completed_at: None,
});
}
}
+
+ for (mn, winner) in r1_byes {
+ let next_match_number = (mn + 1) / 2;
+ if let Some(next) = matches
+ .iter_mut()
+ .find(|m| m.round == 2 && m.match_number == next_match_number)
+ {
+ if mn % 2 == 1 {
+ next.player1_id = winner;
+ } else {
+ next.player2_id = winner;
+ }
+ }
+ }
matches
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/tournament/src/bracket.rs` around lines 237 - 289,
generate_single_elimination_matches currently marks bye matches as Completed but
never writes their winners into the next round, causing deadlocks; after
constructing round 1 matches (the loop that pushes BracketMatch with round ==
1), iterate the completed bye matches and for each compute the destination match
in round 2 as dest_round = 2 and dest_match_number = (match.match_number + 1) /
2, then write the winner id into dest.player1_id if match.match_number is odd
else dest.player2_id (use BracketMatch.round, BracketMatch.match_number,
player1_id/player2_id, winner_id), creating or updating the corresponding
BracketMatch in the matches vector before building subsequent rounds; also add a
unit test that records the non-bye semifinal and asserts the final match has
both players populated.
| fn determine_round_robin_winner(bracket: &TournamentBracket) -> Option<Uuid> { | ||
| let mut wins: HashMap<Uuid, u32> = HashMap::new(); | ||
| for m in &bracket.matches { | ||
| if let Some(w) = m.winner_id { | ||
| *wins.entry(w).or_insert(0) += 1; | ||
| } | ||
| } | ||
| wins.into_iter().max_by_key(|(_, w)| *w).map(|(id, _)| id) | ||
| } |
There was a problem hiding this comment.
Minor: tie-breaking in determine_round_robin_winner is non-deterministic.
HashMap::into_iter() yields entries in unspecified order, so when two players are tied on wins, max_by_key returns whichever happens to come first in the hash iteration — different runs of the same tournament can declare different champions. This is observable from the persisted bracket.winner_id.
Use a deterministic tie-breaker (e.g., higher seed/ELO, or earliest participant id) and ideally fall back to a runoff/marker rather than an arbitrary winner.
♻️ Proposed deterministic fallback
-fn determine_round_robin_winner(bracket: &TournamentBracket) -> Option<Uuid> {
- let mut wins: HashMap<Uuid, u32> = HashMap::new();
- for m in &bracket.matches {
- if let Some(w) = m.winner_id {
- *wins.entry(w).or_insert(0) += 1;
- }
- }
- wins.into_iter().max_by_key(|(_, w)| *w).map(|(id, _)| id)
-}
+fn determine_round_robin_winner(bracket: &TournamentBracket) -> Option<Uuid> {
+ let mut wins: HashMap<Uuid, u32> = HashMap::new();
+ for m in &bracket.matches {
+ if let Some(w) = m.winner_id {
+ *wins.entry(w).or_insert(0) += 1;
+ }
+ }
+ // Tie-break by participant seed (lower seed = higher ELO) for determinism.
+ bracket
+ .participants
+ .iter()
+ .filter_map(|p| wins.get(&p.id).map(|w| (p.id, *w, p.seed)))
+ .max_by(|a, b| a.1.cmp(&b.1).then(b.2.cmp(&a.2)))
+ .map(|(id, _, _)| id)
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/modules/tournament/src/bracket.rs` around lines 322 - 330,
determine_round_robin_winner currently picks a winner non-deterministically via
HashMap iteration; change it to compute the maximum win count, collect all
candidate Uuids with that count, and if more than one apply a deterministic
tie-breaker (prefer a seeded/eloed participant if available by looking up the
participant in TournamentBracket.participants or participant metadata, otherwise
pick the smallest/lexicographic Uuid) so repeated runs yield the same winner;
update the function determine_round_robin_winner to implement this candidate
collection + deterministic comparator and return the chosen Uuid (or None).
| const { analyzePosition, isReady: stockfishReady } = useStockfishWASM({ | ||
| jsBridgePath: "/assets/stockfish.js", | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm that useStockfishWASM's init effect depends on the config reference identity
fd -t f 'StockfishWASM' frontend
rg -nP -C5 'useEffect\b' frontend/components/chess/StockfishWASM.tsxRepository: NOVUS-X/XLMate
Length of output: 546
🏁 Script executed:
#!/bin/bash
# Read the full useEffect hook including its dependency array
sed -n '105,125p' frontend/components/chess/StockfishWASM.tsxRepository: NOVUS-X/XLMate
Length of output: 687
🏁 Script executed:
#!/bin/bash
# Read the current code in page.tsx at the specified lines
sed -n '65,75p' frontend/app/page.tsxRepository: NOVUS-X/XLMate
Length of output: 394
🏁 Script executed:
#!/bin/bash
# Read from line 150 onwards to find the useEffect closing and dependency array
sed -n '150,175p' frontend/components/chess/StockfishWASM.tsxRepository: NOVUS-X/XLMate
Length of output: 620
🏁 Script executed:
#!/bin/bash
# Find the cleanup function definition
sed -n '90,105p' frontend/components/chess/StockfishWASM.tsxRepository: NOVUS-X/XLMate
Length of output: 405
Stockfish WASM config object is recreated every render — engine re-initializes repeatedly.
The inline config object { jsBridgePath: "/assets/stockfish.js" } creates a new reference on every render. The useEffect hook at line 105 of StockfishWASM.tsx includes config in its dependency array (line 172), so each render triggers full re-initialization: Worker.terminate(), spawn a new worker, post the init message, and wait for ready. Every state change in Home (position update, modal toggle, online player tick) tears down and respawns the engine—both expensive and racy with analyzePosition calls.
♻️ Suggested fix — stable config reference
+const STOCKFISH_CONFIG = { jsBridgePath: "/assets/stockfish.js" } as const;
+
export default function Home() {
...
- const { analyzePosition, isReady: stockfishReady } = useStockfishWASM({
- jsBridgePath: "/assets/stockfish.js",
- });
+ const { analyzePosition, isReady: stockfishReady } = useStockfishWASM(STOCKFISH_CONFIG);(or useMemo(() => ({ jsBridgePath: "/assets/stockfish.js" }), []) if the config ever needs to be derived from props/state.)
📝 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 { analyzePosition, isReady: stockfishReady } = useStockfishWASM({ | |
| jsBridgePath: "/assets/stockfish.js", | |
| }); | |
| const STOCKFISH_CONFIG = { jsBridgePath: "/assets/stockfish.js" } as const; | |
| export default function Home() { | |
| const { analyzePosition, isReady: stockfishReady } = useStockfishWASM(STOCKFISH_CONFIG); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/page.tsx` around lines 70 - 72, The inline config object passed
to useStockfishWASM in page.tsx causes a new reference every render and forces
StockfishWASM (see useStockfishWASM and the useEffect in StockfishWASM.tsx that
depends on config) to reinitialize; fix by providing a stable config
reference—either hoist a constant (e.g., STOCKFISH_CONFIG = { jsBridgePath:
"/assets/stockfish.js" }) outside the component or wrap the object in useMemo(()
=> ({ jsBridgePath: "/assets/stockfish.js" }), []) before calling
useStockfishWASM—so analyzePosition/stockfishReady are not repeatedly torn down
and recreated.
| const res = await fetch(`${API_BASE}/v1/tournaments`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| credentials: "include", | ||
| body: JSON.stringify({ name: name.trim(), format }), | ||
| }); | ||
| if (!res.ok) throw new Error(`Failed to create tournament: ${res.status}`); | ||
| const created: TournamentBracket = await res.json(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the tournaments POST handler to see how it constructs participants.
fd -t f -e rs | xargs rg -nP -C3 '"/v1/tournaments"|tournaments.*post|post.*tournaments|create_bracket\s*\('Repository: NOVUS-X/XLMate
Length of output: 7681
🏁 Script executed:
fd -t f -e rs backend/modules/tournament | xargs rg -l "POST|post|route|handler" | head -20Repository: NOVUS-X/XLMate
Length of output: 1154
🏁 Script executed:
# Search for the tournaments route/handler definition
rg -A 10 "tournaments.*POST|POST.*tournaments|/v1/tournaments" backend/modules/tournament/srcRepository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Look for main.rs or lib.rs to understand the route structure
fd -t f \( -name "main.rs" -o -name "lib.rs" \) backend/modules/tournament/src | xargs catRepository: NOVUS-X/XLMate
Length of output: 289
🏁 Script executed:
# Find all Rust files in the tournament module
fd . backend/modules/tournament -t f -e rsRepository: NOVUS-X/XLMate
Length of output: 357
🏁 Script executed:
# Search for tournament POST route handling across the backend
rg "POST|post" backend --type rust | grep -i tournamentRepository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Look for route definitions that might include "/v1/tournaments"
rg "/tournaments|create_tournament" backend --type rust -A 5 | head -100Repository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Find the main service module structure
ls -la backend/modules/service/src/ 2>/dev/null || ls -la backend/src/ 2>/dev/null || find backend -maxdepth 2 -type d -name srcRepository: NOVUS-X/XLMate
Length of output: 532
🏁 Script executed:
# Search for Actix-web POST route definitions or similar HTTP handler patterns
rg "post\(|POST|create.*tournament" backend -t rust -B 2 -A 8 | head -150Repository: NOVUS-X/XLMate
Length of output: 10172
🏁 Script executed:
# Look for the tournament module being used and how tournaments are created
rg "BracketService::create_bracket|create_bracket" backend -t rust -B 5 -A 5Repository: NOVUS-X/XLMate
Length of output: 11282
🏁 Script executed:
# Search for tournament routes in the API/routes structure
find backend -name "*.rs" -type f | xargs rg "tournament|/v1" | grep -i "route\|post\|tournament"Repository: NOVUS-X/XLMate
Length of output: 11261
🏁 Script executed:
# Look at the main API module setup to understand route structure
cat backend/modules/api/src/lib.rs 2>/dev/null || cat backend/modules/src/lib.rs 2>/dev/null || find backend -name "lib.rs" -exec grep -l "tournament\|routes" {} \;Repository: NOVUS-X/XLMate
Length of output: 348
🏁 Script executed:
# Search specifically for tournament POST handlers
rg "tournament" backend -t rust -B 3 -A 10 | grep -E "(post|POST|create.*tournament|fn.*tournament)" -A 10 -B 3Repository: NOVUS-X/XLMate
Length of output: 13847
🏁 Script executed:
# Search for /v1/tournaments or tournaments endpoint
rg "/tournaments|v1/tournaments" backend -t rust -B 5 -A 10Repository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Check games.rs for tournament-related endpoints
cat backend/modules/api/src/games.rs | head -200Repository: NOVUS-X/XLMate
Length of output: 6992
🏁 Script executed:
# Look for all POST routes in the API
rg "#\[post\]|web::post()" backend/modules/api -A 3Repository: NOVUS-X/XLMate
Length of output: 596
🏁 Script executed:
# Look for server.rs to see all registered routes
cat backend/modules/api/src/server.rs | head -300Repository: NOVUS-X/XLMate
Length of output: 9418
🏁 Script executed:
# Search for tournament in the entire backend
rg "tournament" backend --type rust -l | head -20Repository: NOVUS-X/XLMate
Length of output: 193
🏁 Script executed:
# Check if there's a tournament route handler anywhere
find backend -name "*.rs" -exec grep -l "tournament.*route\|/tournament\|v1.*tournament" {} \;Repository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Final check: search for any tournament-related API handlers or routes
rg "tournament" backend/modules/api -t rustRepository: NOVUS-X/XLMate
Length of output: 40
🏁 Script executed:
# Verify there's no tournament module imported or configured anywhere in api/src
grep -r "tournament" backend/modules/api/src/ || echo "No tournament references found in API module"Repository: NOVUS-X/XLMate
Length of output: 103
POST /v1/tournaments endpoint does not exist in the backend.
The frontend code calls ${API_BASE}/v1/tournaments to create a tournament, but this endpoint is not registered in backend/modules/api/src/server.rs. The tournament module exists internally at backend/modules/tournament/src/ with BracketService::create_bracket(), which requires at least 2 participants, but it is never exposed as an HTTP API route.
Server routes are configured for /v1/players, /v1/games, /v1/auth, /v1/ai, /v1/ws, and NFT endpoints—but tournament management is missing entirely. This form submission will always fail with a 404 error.
Create a tournament API handler module in the backend (e.g., backend/modules/api/src/tournaments.rs) that exposes a POST /v1/tournaments route. The handler must either accept a participants list in the request body or implement a registration phase where the authenticated user can join before the bracket is created (since create_bracket requires ≥2 participants).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/tournament/page.tsx` around lines 57 - 64, Add an HTTP handler
that exposes POST /v1/tournaments and forwards requests to
BracketService::create_bracket; accept a JSON payload with at minimum { name:
string, participants: string[] } (or implement a two-step flow where an
authenticated user registers and then a separate endpoint triggers bracket
creation once >=2 participants exist), validate participants length (>=2) before
calling create_bracket, authenticate the caller using the same middleware used
by other routes, return appropriate 400/401/500 errors for bad
input/auth/failures, and wire the new handler into the server router so the
frontend POST to /v1/tournaments no longer returns 404.
Frontend (ESLint errors): - app/page.tsx: replace useState<any> with useState<AnalysisResult | null>; export AnalysisResult interface from StockfishWASM.tsx and import it - app/tournament/page.tsx: convert fetchBrackets to useCallback and add it to the useEffect dependency array to satisfy react-hooks/exhaustive-deps Backend (cargo check failure): - .github/workflows/backend.yml: remove --features api from 'cargo check -p api' — the api crate has no such feature; the api feature belongs to st_core and is already activated via the dependency declaration in api/Cargo.toml
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
frontend/app/page.tsx (1)
70-72:⚠️ Potential issue | 🟠 MajorInline Stockfish config still re-initializes the engine on every render.
This duplicates a previously raised concern that does not appear to have been addressed. The object literal
{ jsBridgePath: "/assets/stockfish.js" }produces a new reference on every render ofHome. Infrontend/components/chess/StockfishWASM.tsx, the inituseEffectlistsconfigin its deps (andanalyzePositionis memoized overconfigtoo), so each parent re-render — driven byposition,onlinePlayerCount, modal toggles, matchmaking status — tears the worker down (terminate()), respawns it, re-postsinit, and recreatesanalyzePosition. That in turn triggers the bot-move effect at Line 166 (which depends onanalyzePosition), creating extra churn and races against in‑flightanalyzePositioncalls.Hoist a stable reference (or memoize), e.g.:
♻️ Suggested fix
+const STOCKFISH_CONFIG = { jsBridgePath: "/assets/stockfish.js" } as const; + export default function Home() { ... - const { analyzePosition, isReady: stockfishReady } = useStockfishWASM({ - jsBridgePath: "/assets/stockfish.js", - }); + const { analyzePosition, isReady: stockfishReady } = useStockfishWASM(STOCKFISH_CONFIG);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` around lines 70 - 72, The inline config object passed to useStockfishWASM in Home (the { jsBridgePath: "/assets/stockfish.js" } literal) creates a new reference each render which forces the init effect in frontend/components/chess/StockfishWASM.tsx to teardown and re-init the worker (affecting analyzePosition and downstream effects); fix by hoisting or memoizing the config so it is stable across renders (e.g., create a constant or use useMemo for the config used when calling useStockfishWASM in Home) so that useStockfishWASM, analyzePosition, and the worker init effect no longer retrigger on unrelated renders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/tournament/page.tsx`:
- Line 77: The outer container div with className "min-h-screen p-4 md:p-8
text-white" sets role="main" and aria-label="Tournaments", which duplicates the
real <main> rendered later; remove role="main" and the redundant aria-label from
that outer div, or alternatively change the inner <main> element to a <section>
so there is only one main landmark; locate the outer div and the inner <main> in
page.tsx (the elements shown in the diff) and apply one of these two fixes
consistently.
- Around line 52-74: handleCreate leaves stale error state and doesn't reset
format on success; clear any previous error at the start of handleCreate (call
setError("") before or immediately after e.preventDefault()) so a succeeding
request won't show an old banner, and after a successful create reset the form
symmetry by also clearing format (call setFormat(defaultValue) alongside
setName("") and setShowCreate(false)); refer to the handleCreate function and
the setError, setFormat, setName, setShowCreate, and setCreating state setters
when making these changes.
- Around line 32-50: fetchBrackets currently lists selected in its dependency
array causing it to be re-created whenever setSelected is called; remove
selected from fetchBrackets dependencies and change the “select first” logic so
it doesn't rely on the outer selected variable — either (A) after parsing data
in fetchBrackets call setSelected(prev => prev ?? data[0]) to read selection via
functional setter, or (B) move the “if (data.length > 0 && !selected)
setSelected(data[0])” check into the useEffect that calls fetchBrackets (reading
selected there once), so fetchBrackets has a stable identity and useEffect no
longer refetches on selection changes.
---
Duplicate comments:
In `@frontend/app/page.tsx`:
- Around line 70-72: The inline config object passed to useStockfishWASM in Home
(the { jsBridgePath: "/assets/stockfish.js" } literal) creates a new reference
each render which forces the init effect in
frontend/components/chess/StockfishWASM.tsx to teardown and re-init the worker
(affecting analyzePosition and downstream effects); fix by hoisting or memoizing
the config so it is stable across renders (e.g., create a constant or use
useMemo for the config used when calling useStockfishWASM in Home) so that
useStockfishWASM, analyzePosition, and the worker init effect no longer
retrigger on unrelated renders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03435803-54bd-40a2-b02d-c1d42eb7f99d
📒 Files selected for processing (4)
.github/workflows/backend.ymlfrontend/app/page.tsxfrontend/app/tournament/page.tsxfrontend/components/chess/StockfishWASM.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/components/chess/StockfishWASM.tsx
| const fetchBrackets = useCallback(async () => { | ||
| setLoading(true); | ||
| setError(null); | ||
| try { | ||
| const res = await fetch(`${API_BASE}/v1/tournaments`, { credentials: "include" }); | ||
| if (!res.ok) throw new Error(`Server error: ${res.status}`); | ||
| const data: TournamentBracket[] = await res.json(); | ||
| setBrackets(data); | ||
| if (data.length > 0 && !selected) setSelected(data[0]); | ||
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : "Failed to load tournaments"); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }, [selected]); | ||
|
|
||
| useEffect(() => { | ||
| void fetchBrackets(); | ||
| }, [fetchBrackets]); |
There was a problem hiding this comment.
Selecting a tournament triggers a full refetch (effect/callback dependency loop).
Because fetchBrackets lists selected in its dependency array, every call to setSelected(...) (e.g., when the user clicks a tournament in the sidebar on line 167, or after creating one on line 66) re-creates fetchBrackets, which in turn re-fires the useEffect on line 48–50 and re-hits GET /v1/tournaments. This is wasteful, can clobber freshly created local state with stale server data, and causes a brief loading flash on every selection.
The dependency only exists to avoid overwriting the user's selection when refetching. Read selected via the functional state setter (or move the "select first" logic into the effect) so fetchBrackets has a stable identity.
♻️ Suggested fix
- const fetchBrackets = useCallback(async () => {
+ const fetchBrackets = useCallback(async () => {
setLoading(true);
setError(null);
try {
const res = await fetch(`${API_BASE}/v1/tournaments`, { credentials: "include" });
if (!res.ok) throw new Error(`Server error: ${res.status}`);
const data: TournamentBracket[] = await res.json();
setBrackets(data);
- if (data.length > 0 && !selected) setSelected(data[0]);
+ setSelected((prev) => prev ?? (data.length > 0 ? data[0] : null));
} catch (e) {
setError(e instanceof Error ? e.message : "Failed to load tournaments");
} finally {
setLoading(false);
}
- }, [selected]);
+ }, []);📝 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 fetchBrackets = useCallback(async () => { | |
| setLoading(true); | |
| setError(null); | |
| try { | |
| const res = await fetch(`${API_BASE}/v1/tournaments`, { credentials: "include" }); | |
| if (!res.ok) throw new Error(`Server error: ${res.status}`); | |
| const data: TournamentBracket[] = await res.json(); | |
| setBrackets(data); | |
| if (data.length > 0 && !selected) setSelected(data[0]); | |
| } catch (e) { | |
| setError(e instanceof Error ? e.message : "Failed to load tournaments"); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }, [selected]); | |
| useEffect(() => { | |
| void fetchBrackets(); | |
| }, [fetchBrackets]); | |
| const fetchBrackets = useCallback(async () => { | |
| setLoading(true); | |
| setError(null); | |
| try { | |
| const res = await fetch(`${API_BASE}/v1/tournaments`, { credentials: "include" }); | |
| if (!res.ok) throw new Error(`Server error: ${res.status}`); | |
| const data: TournamentBracket[] = await res.json(); | |
| setBrackets(data); | |
| setSelected((prev) => prev ?? (data.length > 0 ? data[0] : null)); | |
| } catch (e) { | |
| setError(e instanceof Error ? e.message : "Failed to load tournaments"); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| void fetchBrackets(); | |
| }, [fetchBrackets]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/tournament/page.tsx` around lines 32 - 50, fetchBrackets
currently lists selected in its dependency array causing it to be re-created
whenever setSelected is called; remove selected from fetchBrackets dependencies
and change the “select first” logic so it doesn't rely on the outer selected
variable — either (A) after parsing data in fetchBrackets call setSelected(prev
=> prev ?? data[0]) to read selection via functional setter, or (B) move the “if
(data.length > 0 && !selected) setSelected(data[0])” check into the useEffect
that calls fetchBrackets (reading selected there once), so fetchBrackets has a
stable identity and useEffect no longer refetches on selection changes.
| async function handleCreate(e: React.FormEvent) { | ||
| e.preventDefault(); | ||
| if (!name.trim()) return; | ||
| setCreating(true); | ||
| try { | ||
| const res = await fetch(`${API_BASE}/v1/tournaments`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| credentials: "include", | ||
| body: JSON.stringify({ name: name.trim(), format }), | ||
| }); | ||
| if (!res.ok) throw new Error(`Failed to create tournament: ${res.status}`); | ||
| const created: TournamentBracket = await res.json(); | ||
| setBrackets((prev) => [created, ...prev]); | ||
| setSelected(created); | ||
| setShowCreate(false); | ||
| setName(""); | ||
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : "Failed to create tournament"); | ||
| } finally { | ||
| setCreating(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor: stale form/error state after create.
Two small UX nits in handleCreate:
errorfrom a previous failed attempt isn't cleared on a new submit, so a successful create still shows the old red banner (line 139–143) until the next list refetch (which now won't happen because of the dependency fix above).formatis not reset after a successful create, so the next "New Tournament" form opens pre‑populated with the last choice (probably fine, butnameis reset — be intentional about the asymmetry).
♻️ Suggested fix
async function handleCreate(e: React.FormEvent) {
e.preventDefault();
if (!name.trim()) return;
setCreating(true);
+ setError(null);
try {📝 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.
| async function handleCreate(e: React.FormEvent) { | |
| e.preventDefault(); | |
| if (!name.trim()) return; | |
| setCreating(true); | |
| try { | |
| const res = await fetch(`${API_BASE}/v1/tournaments`, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| credentials: "include", | |
| body: JSON.stringify({ name: name.trim(), format }), | |
| }); | |
| if (!res.ok) throw new Error(`Failed to create tournament: ${res.status}`); | |
| const created: TournamentBracket = await res.json(); | |
| setBrackets((prev) => [created, ...prev]); | |
| setSelected(created); | |
| setShowCreate(false); | |
| setName(""); | |
| } catch (e) { | |
| setError(e instanceof Error ? e.message : "Failed to create tournament"); | |
| } finally { | |
| setCreating(false); | |
| } | |
| } | |
| async function handleCreate(e: React.FormEvent) { | |
| e.preventDefault(); | |
| if (!name.trim()) return; | |
| setCreating(true); | |
| setError(null); | |
| try { | |
| const res = await fetch(`${API_BASE}/v1/tournaments`, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| credentials: "include", | |
| body: JSON.stringify({ name: name.trim(), format }), | |
| }); | |
| if (!res.ok) throw new Error(`Failed to create tournament: ${res.status}`); | |
| const created: TournamentBracket = await res.json(); | |
| setBrackets((prev) => [created, ...prev]); | |
| setSelected(created); | |
| setShowCreate(false); | |
| setName(""); | |
| } catch (e) { | |
| setError(e instanceof Error ? e.message : "Failed to create tournament"); | |
| } finally { | |
| setCreating(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/tournament/page.tsx` around lines 52 - 74, handleCreate leaves
stale error state and doesn't reset format on success; clear any previous error
at the start of handleCreate (call setError("") before or immediately after
e.preventDefault()) so a succeeding request won't show an old banner, and after
a successful create reset the form symmetry by also clearing format (call
setFormat(defaultValue) alongside setName("") and setShowCreate(false)); refer
to the handleCreate function and the setError, setFormat, setName,
setShowCreate, and setCreating state setters when making these changes.
| } | ||
|
|
||
| return ( | ||
| <div className="min-h-screen p-4 md:p-8 text-white" role="main" aria-label="Tournaments"> |
There was a problem hiding this comment.
Duplicate main landmark.
The outer <div> on line 77 declares role="main" while line 186 also renders a <main> element, producing two main landmarks on the page. Screen readers expect exactly one. Drop role="main" (and likely the redundant aria-label="Tournaments" since <h1> already names the region) from the outer container, or convert the inner <main> to a plain <section>.
♻️ Suggested fix
- <div className="min-h-screen p-4 md:p-8 text-white" role="main" aria-label="Tournaments">
+ <div className="min-h-screen p-4 md:p-8 text-white">Also applies to: 186-186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/tournament/page.tsx` at line 77, The outer container div with
className "min-h-screen p-4 md:p-8 text-white" sets role="main" and
aria-label="Tournaments", which duplicates the real <main> rendered later;
remove role="main" and the redundant aria-label from that outer div, or
alternatively change the inner <main> element to a <section> so there is only
one main landmark; locate the outer div and the inner <main> in page.tsx (the
elements shown in the diff) and apply one of these two fixes consistently.
…seError ApiError::DatabaseError expects DbErr, not String. All 9 map_err calls in rating.rs were passing format!() strings directly. Fix by importing DbErr and wrapping each message with DbErr::Custom(format!(...)).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/app/page.tsx (1)
73-75:⚠️ Potential issue | 🔴 CriticalMissing import for
useStockfishWASMhook and config object recreated every render.Two issues:
Missing import:
useStockfishWASMis used but not imported. Combined with theAnalysisResulttype, add the import as shown in the previous comment.Config object recreation: The inline config object creates a new reference on every render. Per the
useEffectinStockfishWASM.tsxthat depends onconfig, this causes the Stockfish worker to be torn down and respawned on every state change (position update, modal toggle, etc.)—both expensive and racy with pendinganalyzePositioncalls.
,♻️ Proposed fix — stable config reference
+import { useStockfishWASM, type AnalysisResult } from "@/components/chess/StockfishWASM"; + +const STOCKFISH_CONFIG = { jsBridgePath: "/assets/stockfish.js" } as const; + export default function Home() { ... - const { analyzePosition, isReady: stockfishReady } = useStockfishWASM({ - jsBridgePath: "/assets/stockfish.js", - }); + const { analyzePosition, isReady: stockfishReady } = useStockfishWASM(STOCKFISH_CONFIG);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` around lines 73 - 75, The component is missing the import for useStockfishWASM (and the AnalysisResult type) and also recreates the config object each render which causes StockfishWASM (see StockfishWASM.tsx and its useEffect on config) to teardown/restart; fix by adding the missing import for useStockfishWASM and AnalysisResult, and create a stable config reference (e.g., memoize the config passed to useStockfishWASM with useMemo or hoist it outside the component) so analyzePosition and the worker are not repeatedly recreated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/page.tsx`:
- Around line 155-161: The current flow may call setBotAnalysis(result) even if
game.move(...) returns null (illegal move); change the sequence to first compute
from/to/promotion from result.bestMove, call game.move({ from, to, promotion })
and capture its return value, then only call setBotAnalysis(result) and
setPosition(game.fen()) if the move result is non-null; if game.move returns
null, avoid updating state (optionally log or handle the rejection) and ensure
the checks around active, result.bestMove, fenBeforeAnalysis and game.isGameOver
remain in place.
- Around line 48-49: The file uses the AnalysisResult type and useStockfishWASM
hook but neither is imported; add imports for AnalysisResult and
useStockfishWASM near the other component imports at the top of page.tsx
(reference the AnalysisResult type used in the state declaration:
setBotAnalysis/useState<AnalysisResult | null> and the useStockfishWASM hook
call where it's invoked around line 73) so TypeScript can resolve those symbols
and the component will compile.
---
Duplicate comments:
In `@frontend/app/page.tsx`:
- Around line 73-75: The component is missing the import for useStockfishWASM
(and the AnalysisResult type) and also recreates the config object each render
which causes StockfishWASM (see StockfishWASM.tsx and its useEffect on config)
to teardown/restart; fix by adding the missing import for useStockfishWASM and
AnalysisResult, and create a stable config reference (e.g., memoize the config
passed to useStockfishWASM with useMemo or hoist it outside the component) so
analyzePosition and the worker are not repeatedly recreated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a45ff8ea-bd2f-4162-98d3-3da4dff4790f
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
frontend/app/page.tsxfrontend/package.json
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json
| if (active && result.bestMove && game.fen() === fenBeforeAnalysis && !game.isGameOver()) { | ||
| setBotAnalysis(result); | ||
| const from = result.bestMove.substring(0, 2); | ||
| const to = result.bestMove.substring(2, 4); | ||
| const promotion = result.bestMove.length > 4 ? result.bestMove.substring(4, 5) : undefined; | ||
| game.move({ from, to, promotion }); | ||
| setPosition(game.fen()); |
There was a problem hiding this comment.
Consider checking game.move() result before updating state.
If game.move() returns null (move rejected by chess.js), setBotAnalysis(result) has already been called, leaving the UI showing analysis for a move that wasn't applied. While rare (Stockfish returning an illegal move), this could cause UI inconsistency.
🛡️ Suggested defensive fix
if (active && result.bestMove && game.fen() === fenBeforeAnalysis && !game.isGameOver()) {
- setBotAnalysis(result);
const from = result.bestMove.substring(0, 2);
const to = result.bestMove.substring(2, 4);
const promotion = result.bestMove.length > 4 ? result.bestMove.substring(4, 5) : undefined;
- game.move({ from, to, promotion });
- setPosition(game.fen());
+ const applied = game.move({ from, to, promotion });
+ if (applied) {
+ setBotAnalysis(result);
+ setPosition(game.fen());
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/page.tsx` around lines 155 - 161, The current flow may call
setBotAnalysis(result) even if game.move(...) returns null (illegal move);
change the sequence to first compute from/to/promotion from result.bestMove,
call game.move({ from, to, promotion }) and capture its return value, then only
call setBotAnalysis(result) and setPosition(game.fen()) if the move result is
non-null; if game.move returns null, avoid updating state (optionally log or
handle the rejection) and ensure the checks around active, result.bestMove,
fenBeforeAnalysis and game.isGameOver remain in place.
- Add missing useStockfishWASM and AnalysisResult imports in page.tsx - Implement std::error::Error trait for BracketError in bracket.rs - Fixes CI/build issues for PR #695
- Add missing TransactionTrait import from sea_orm - Fix ApiError::DatabaseError type mismatches using .map_err(ApiError::from) - Resolves CI failures in PR #695
- Add TransactionTrait import in games.rs - Fix ApiError::DatabaseError type mismatches using .map_err(ApiError::from) - Remove invalid summary attribute from utoipa::path in st_core/endpoint.rs - Fix unused variable/import warnings in chess, security modules - Resolves CI failures in PR #695
…api tests pass locally
…ates - Updated bytes, idna, rustls-webpki, and time to patched versions - Regenerated Cargo.lock with latest compatible secure versions - Reduced vulnerabilities from 8 to 1 (1 allowed unmaintained warning) - All critical security issues resolved
Closes #545
Closes #655
Closes #543
Summary
Resolves all CodeRabbit review issues from the initial PR and implements the tournament bracket management service (#543).
Fixes (PR #695 review)
/v1/matchmaking/leave→/v1/matchmaking/cancelwith correctrequest_idpayload matchingMatchmakingService::cancel_requestTEST_WALLET_+ random address and fixed ELO 1200; wallet and ELO are now resolved server-side from the authenticated JWT session (credentials: "include")stockfish.js@^10.0.2; engine loads from/assets/stockfish.jsat runtime (already bundled inpublic/)analyzePositionbehindstockfishReadyflag; capture FEN beforeawaitand discard stale results if position changed before the response arriveslastOpponentMoveapplication with agameMode === "online"check to prevent stale moves applying after game resetMatchmakingModalnow hasrole="dialog",aria-modal,aria-labelledby, Escape key close, and a full focus trapjoinMatchmakinginterface updated fromvoidtoPromise<void>to match theasyncimplementationPlayerandMatchRequestnow derivePartialEq; round-trip tests use full structural equality across all threeMatchTypevariantsFeature: Tournament Bracket Management Service (#543)
Backend (
backend/modules/tournament/src/bracket.rs):BracketServicewithSingleElimination,DoubleElimination, andRoundRobinformatsrecord_resultadvances winners to the next round and marks the champion on completionget_standingsreturns participants sorted by win countNotEnoughPlayers,TournamentAlreadyStarted,TournamentNotStarted,MatchNotFound,PlayerNotInMatch,MatchAlreadyCompleted), bye-slot logic, seeding, and full bracket completionFrontend:
frontend/components/tournament/BracketView.tsx— visual bracket grid for elimination formats, standings table for round-robin, champion banner, participant listfrontend/app/tournament/page.tsx— tournament list sidebar, create form with format picker, integratedBracketViewSummary by CodeRabbit
New Features
Tests
Chores