Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
There was a problem hiding this comment.
Pull request overview
This PR expands the game/competition experience by adding best-of-3 series support to multiplayer sessions, introducing a client-side “competition” layer (streaks, rivals, season framing), and enhancing UI/UX across Home/Configure/Leaderboard/Session pages (including Pixelz replays).
Changes:
- Add
seriesLength(1 or 3) to session creation and propagate series state across “next session” creation on the API. - Introduce
competitionclient module (streaks, rivals, seasons) and wire it into Home/Profile/Leaderboard plus in-game result insights. - Add Pixelz replay viewer and richer SessionRoom/Leaderboard UI presentation.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/sessions.ts | Adds seriesLength validation to shared create-session schema. |
| packages/shared/src/sessions.test.ts | Tests acceptance/rejection of supported series lengths. |
| apps/web/src/pages/SessionRoom.tsx | Displays series metadata, improved results/lobby UI, and Pixelz winner replay. |
| apps/web/src/pages/SessionRoom.test.tsx | Updates assertions to match the revised SessionRoom UI/call patterns. |
| apps/web/src/pages/Profile.tsx | New Profile page pulling local competition stats + season leaderboard snapshot. |
| apps/web/src/pages/Leaderboard.tsx | Adds window/scope controls, rivals UX, richer summary cards, and board labeling. |
| apps/web/src/pages/JoinSession.tsx | Shows stake/format labels for invites using competition formatting helpers. |
| apps/web/src/pages/Home.tsx | Rebuilds Home into a competition dashboard with daily reset/streak/quick actions. |
| apps/web/src/pages/Configure.tsx | Adds solo/multi mode tabs, series-length toggle, presets, and improved flows. |
| apps/web/src/lib/competition.ts | New competition state engine (localStorage) + formatting/season/rival helpers. |
| apps/web/src/lib/competition.test.ts | Adds unit tests for UTC windows, streak logic, rivals summary, seasons, tiers. |
| apps/web/src/lib/api.ts | Extends CreateSessionRequest with seriesLength; adds leaderboard query params. |
| apps/web/src/index.css | Adds extensive styling for new dashboard/leaderboard/profile/session components. |
| apps/web/src/games/reflex/ReflexGame.tsx | Adds local competition recording + “ghost” pacing + richer result insights. |
| apps/web/src/games/pixelz/PixelzGame.tsx | Adds local competition recording, cell pulse feedback, and replay viewing. |
| apps/web/src/components/PixelzReplayViewer.tsx | New component to replay Pixelz move sequences on a board. |
| apps/web/src/App.tsx | Adds Profile route + navigation link. |
| apps/api/src/sessions.ts | Persists/propagates series state and blocks next-session creation when decided. |
| apps/api/src/sessions.test.ts | Updates session-next tests and adds coverage for best-of-3 series behavior. |
| apps/api/src/leaderboard.ts | Adds UTC-window/since filtering for leaderboard queries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web/src/pages/Home.tsx
Outdated
| const dailyDateKey = useMemo(() => toDateKey(now), [now]); | ||
| const daily = useMemo(() => getDailyChallenges(now), [now]); | ||
| const dailyChallenges = useMemo(() => daily.challenges, [daily]); | ||
| const overview = useMemo(() => getCompetitionOverview(), [dailyDateKey]); |
There was a problem hiding this comment.
overview is memoized only on dailyDateKey, but getCompetitionOverview() reads from localStorage (and uses current time by default). After completing a run, this component re-renders every second but overview won’t update until UTC day changes, so streak/completed-today badges can stay stale. Consider either removing this useMemo, or pass now into getCompetitionOverview(now) and include now in the memo deps (or subscribe to storage updates).
| const overview = useMemo(() => getCompetitionOverview(), [dailyDateKey]); | |
| const overview = useMemo(() => getCompetitionOverview(now), [now, dailyDateKey]); |
There was a problem hiding this comment.
Addressed in 0f833fb. Home now recomputes the competition overview from now, so the reset/streak/daily-completion state stays fresh within the same UTC day.
apps/web/src/pages/Profile.tsx
Outdated
|
|
||
| const dailyDateKey = useMemo(() => toDateKey(now), [now]); | ||
| const currentSeason = useMemo(() => getCurrentSeason(now), [now]); | ||
| const profile = useMemo(() => getCompetitionProfile(now), [dailyDateKey]); |
There was a problem hiding this comment.
profile is computed from getCompetitionProfile(now) but the memo deps are [dailyDateKey]. Within the same day, local progress updates (total plays, PBs, rivals, etc.) won’t be reflected even though the component re-renders every second. Consider removing the memo or including now (and/or a storage-change signal) in the dependencies so the profile refreshes after runs.
| const profile = useMemo(() => getCompetitionProfile(now), [dailyDateKey]); | |
| const profile = useMemo(() => getCompetitionProfile(now), [now]); |
There was a problem hiding this comment.
Addressed in 0f833fb. Profile now recomputes getCompetitionProfile(now) from now, so local progress changes during the same day show up without waiting for the UTC date key to roll over.
apps/api/src/sessions.ts
Outdated
| ? Object.fromEntries( | ||
| Object.entries(rawWins as Record<string, unknown>) | ||
| .filter((entry): entry is [string, number] => typeof entry[0] === "string" && typeof entry[1] === "number") | ||
| .map(([userId, wins]) => [userId, Math.max(0, Math.trunc(wins))]) |
There was a problem hiding this comment.
parseSeriesState treats any typeof wins === "number" as valid; this includes NaN/Infinity, and Math.max(0, Math.trunc(wins)) will yield NaN, which can break decided detection in nextSeriesState. Consider filtering with Number.isFinite(wins) (and defaulting invalid values to 0) when normalizing seriesWins.
| .map(([userId, wins]) => [userId, Math.max(0, Math.trunc(wins))]) | |
| .map(([userId, wins]) => { | |
| const safeWins = Number.isFinite(wins) ? wins : 0; | |
| return [userId, Math.max(0, Math.trunc(safeWins))]; | |
| }) |
There was a problem hiding this comment.
Addressed in 0f833fb. parseSeriesState now guards seriesWins with Number.isFinite before truncation so invalid numeric values normalize safely to 0 instead of leaking NaN into later series-state checks.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76292d59b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [levelId, personalBest]); |
There was a problem hiding this comment.
Using personalBest (an object literal rebuilt on each render) in this dependency array makes the effect fire continuously once a user has any PB. In that state, the effect repeatedly calls fetchLeaderboard and then setGhostTarget(...), which causes more renders and another fetch cycle. This can hammer the API and keep the component in unnecessary render/network churn during normal play.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 0f833fb. The leaderboard ghost fetch now bails out immediately when a PB exists and depends on personalBest?.timeMs instead of the rebuilt object, which stops the fetch/render loop.
apps/web/src/lib/competition.ts
Outdated
| wins[winnerUserId] = (wins[winnerUserId] ?? 0) + 1; | ||
| } | ||
| const targetWins = length === 3 ? 2 : 1; | ||
| const decided = Object.values(wins).some((value) => value >= targetWins) || Boolean(winnerUserId && round >= length); |
There was a problem hiding this comment.
This decided check only becomes true when someone reaches target wins (or when there is a winner in the last round). If a best-of-3 reaches round 3 with no winner (e.g., tied first place), decided stays false even though the backend blocks additional rounds after the max round count. The UI then incorrectly offers another round and the host hits a 409 error when trying to continue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 0f833fb. getSeriesMeta now treats hitting the round limit as decided even without a last-round winner, and I added a regression test for the round-3 exhaustion case in competition.test.ts.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| row.map((colorIndex, x) => ( | ||
| <div | ||
| key={`${y}-${x}`} | ||
| className={`pixelz-cell ${recentlyChanged.includes(`${y}-${x}`) ? "pixelz-cell--pulse" : ""}`} |
There was a problem hiding this comment.
recentlyChanged.includes(...) is called for every cell during board render. Since recentlyChanged can be large (worst case: most of the grid changes on a move), this becomes an O(cells × changed) hot path and can cause jank on larger boards. Consider storing recentlyChanged as a Set<string> (or a boolean grid keyed by index) so the per-cell lookup is O(1).
apps/web/src/pages/Leaderboard.tsx
Outdated
| Promise.all( | ||
| myBoardIds.map(async (boardId) => { | ||
| const board = await fetchBoard(boardId).catch(() => null); | ||
| if (!board) return null; | ||
| return [boardId, { boardId, width: board.width, height: board.height, numColors: board.numColors }] as const; |
There was a problem hiding this comment.
This code triggers one fetchBoard request per entry in myBoardIds. If a user has many played boards, this becomes an N+1 network pattern that can noticeably slow the page and add load to the API. Consider lazy-loading metadata only for boards currently shown/selected (or fetching board metadata in batch / caching across mounts).
apps/web/src/pages/Configure.tsx
Outdated
| const sessionResult = await Promise.race([ | ||
| supabase.auth.getSession(), | ||
| new Promise<never>((_, reject) => { | ||
| setTimeout(() => reject(new Error("Auth session lookup timed out. Please refresh and try again.")), 8000); | ||
| }), | ||
| ]); | ||
| const { data: { session } } = sessionResult; | ||
| if (!session?.access_token) { | ||
| setInviteError("Sign in to create a multiplayer invite."); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
withSession uses Promise.race([... , new Promise((_, reject) => setTimeout(() => reject(...), 8000))]) without clearing the timeout or otherwise handling the losing promise. If supabase.auth.getSession() resolves quickly, the timeout promise will still reject ~8s later and can trigger an unhandled rejection in the browser. Consider clearing the timeout once the session resolves, or converting the timeout branch to a resolved sentinel value (or using an abortable API) so there’s no unobserved rejection.
| const sessionResult = await Promise.race([ | |
| supabase.auth.getSession(), | |
| new Promise<never>((_, reject) => { | |
| setTimeout(() => reject(new Error("Auth session lookup timed out. Please refresh and try again.")), 8000); | |
| }), | |
| ]); | |
| const { data: { session } } = sessionResult; | |
| if (!session?.access_token) { | |
| setInviteError("Sign in to create a multiplayer invite."); | |
| return null; | |
| } | |
| const sessionResult = await new Promise((resolve, reject) => { | |
| const timeoutId = setTimeout(() => { | |
| reject(new Error("Auth session lookup timed out. Please refresh and try again.")); | |
| }, 8000); | |
| supabase.auth | |
| .getSession() | |
| .then((result) => { | |
| clearTimeout(timeoutId); | |
| resolve(result); | |
| }) | |
| .catch((error) => { | |
| clearTimeout(timeoutId); | |
| reject(error); | |
| }); | |
| }); | |
| const { | |
| data: { session }, | |
| } = sessionResult as { data: { session: { access_token?: string } | null } }; | |
| if (!session?.access_token) { | |
| setInviteError("Sign in to create a multiplayer invite."); | |
| return null; | |
| } |
apps/web/src/lib/competition.ts
Outdated
| export function describeSessionFormat(gameId: GameId, levelId: string | null, settings?: BoardSettings): string { | ||
| const seriesLabel = settings?.seriesLength === 3 ? " · best of 3" : ""; | ||
| if (gameId === "reflex") { | ||
| const rounds = levelId ? REFLEX_LEVELS[levelId as ReflexLevelId] : Number(settings?.height ?? 10); |
There was a problem hiding this comment.
describeSessionFormat falls back to Number(settings?.height ?? 10) for Reflex when levelId is null/unknown. Reflex settings use rounds, not height, so this can produce incorrect format strings for non-predefined Reflex sessions (or any future custom Reflex formats). Consider extending BoardSettings to include rounds?: number and using settings?.rounds (or a safer default) for the fallback.
| const rounds = levelId ? REFLEX_LEVELS[levelId as ReflexLevelId] : Number(settings?.height ?? 10); | |
| const rounds = | |
| levelId | |
| ? REFLEX_LEVELS[levelId as ReflexLevelId] | |
| : Number((settings as any)?.rounds ?? 10); |
apps/web/src/pages/Profile.tsx
Outdated
| (async () => { | ||
| try { | ||
| const token = (await supabase.auth.getSession()).data.session?.access_token ?? undefined; | ||
| const sinceIso = getSeasonWindowStart().toISOString(); |
There was a problem hiding this comment.
sinceIso is derived from getSeasonWindowStart() (which uses the current system time) even though this effect is keyed off currentSeason.id computed from the component’s now. To avoid any mismatch (especially around quarter boundaries) and to make the dependency relationship explicit, use currentSeason.start.toISOString() (or getSeasonWindowStart(now)), so the fetched window always matches the displayed season.
| const sinceIso = getSeasonWindowStart().toISOString(); | |
| const sinceIso = getSeasonWindowStart(now).toISOString(); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
The rank calculation inside this loop is based on score/entry.score, but Pixelz leaderboard ordering is by moves then timeMs (Pixelz score is obsolete/constant). This can mis-rank a run and incorrectly trigger or skip the sign-in prompt. Update the comparison to use (finalMoves < entry.moves) || (finalMoves === entry.moves && elapsed < entry.timeMs) (and drop score from the ordering) so it matches the backend leaderboard sort.
There was a problem hiding this comment.
Addressed in 83c51df. The Pixelz sign-in prompt rank calculation now compares runs using the same ordering as the backend leaderboard: moves first, then time.
| }); | ||
|
|
||
| expect(leaveSession).toHaveBeenCalledWith("session-1"); | ||
| expect(leaveSession).toHaveBeenCalled(); |
There was a problem hiding this comment.
This test assertion was loosened to toHaveBeenCalled(), but the component calls leaveSession(data.session.id). Keeping toHaveBeenCalledWith("session-1") here (as done in the following test) better verifies the contract and prevents regressions where the wrong session id is passed.
| expect(leaveSession).toHaveBeenCalled(); | |
| expect(leaveSession).toHaveBeenCalledWith("session-1"); |
There was a problem hiding this comment.
Addressed in 83c51df. Restored the stricter test assertion so it verifies leaveSession is called with the expected session id.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web/src/pages/Leaderboard.tsx
Outdated
| levelParam !== effectiveLevel || | ||
| (game === "reflex" && !levelParam) || | ||
| (game === "pixelz" && !levelParam) || | ||
| windowParam !== windowKey || |
There was a problem hiding this comment.
The window/search-param normalization treats an omitted window query param as different from the default windowKey === "all" (windowParam !== windowKey). Because you delete the param when windowKey === "all", this condition stays true (null !== "all") and can cause redundant setSearchParams calls (and potentially an update loop) on the default view. Treat windowParam == null as equivalent to "all" in the condition, similar to the scopeParam handling, before calling setSearchParams.
| windowParam !== windowKey || | |
| (windowParam !== windowKey && !(windowParam == null && windowKey === "all")) || |
apps/api/src/sessions.ts
Outdated
| function parseSeriesState(settings: Record<string, unknown> | null | undefined): SeriesState { | ||
| const rawLength = Number(settings?.seriesLength); | ||
| const seriesLength: 1 | 3 = rawLength === 3 ? 3 : 1; | ||
| const currentRound = Math.max(1, Math.trunc(Number(settings?.currentRound ?? 1))); |
There was a problem hiding this comment.
parseSeriesState can produce currentRound = NaN when settings.currentRound is present but non-numeric (e.g., corrupted DB JSON), because Math.max(1, Math.trunc(Number(...))) returns NaN if the inner value is NaN. That will break nextSeriesState (nextRound becomes NaN, comparisons fail, and series completion checks become unreliable). Parse currentRound with a Number.isFinite guard and fall back to 1 when invalid.
| const currentRound = Math.max(1, Math.trunc(Number(settings?.currentRound ?? 1))); | |
| const rawCurrentRound = Number(settings?.currentRound); | |
| const currentRound = Number.isFinite(rawCurrentRound) | |
| ? Math.max(1, Math.trunc(rawCurrentRound)) | |
| : 1; |
apps/web/src/lib/competition.ts
Outdated
| decided: false, | ||
| }; | ||
| } | ||
| const round = Math.max(1, Math.trunc(settings?.currentRound ?? 1)); |
There was a problem hiding this comment.
getSeriesMeta computes round via Math.max(1, Math.trunc(settings?.currentRound ?? 1)), which can yield NaN if currentRound is present but non-numeric (e.g., string/NaN from backend or older clients). A NaN round breaks the UI logic (decided calculation and next-round labeling). Add a Number.isFinite/Number(...) normalization so invalid values fall back to round 1.
| const round = Math.max(1, Math.trunc(settings?.currentRound ?? 1)); | |
| const currentRoundRaw = Number(settings?.currentRound); | |
| const currentRoundSafe = Number.isFinite(currentRoundRaw) ? currentRoundRaw : 1; | |
| const round = Math.max(1, Math.trunc(currentRoundSafe)); |
|
@codex review pr |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83c51df84e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (delayTimerRef.current) clearTimeout(delayTimerRef.current); | ||
| }; | ||
| }, [levelId]); | ||
| }, [levelId, personalBest?.timeMs]); |
There was a problem hiding this comment.
Including personalBest?.timeMs in this initialization effect causes it to run again right after a run sets a new personal best (setLevelProgress(...)). On that rerun, the effect clears completion state (setLastResultWasBest, setResultInsight, setRivalInsight, etc.), so PB/improvement insights are wiped on the very runs that should display them. This effect is level-reset logic and should not be retriggered by post-run PB updates.
Useful? React with 👍 / 👎.
apps/web/src/pages/Leaderboard.tsx
Outdated
| const highlightedLatestKey = latestMine ? `${latestMine.userId}-${latestMine.createdAt}` : null; | ||
| const highlightedBestKey = bestMine ? `${bestMine.userId}-${bestMine.createdAt}` : null; | ||
| const podium = visibleEntries.slice(0, 3); | ||
| const percentile = bestMine ? Math.max(1, Math.round((1 - (bestMine.rank - 1) / Math.max(data.entries.length, 1)) * 100)) : null; |
There was a problem hiding this comment.
This percentile formula is inverted for the displayed label: rank 1 yields Top 100% and worse ranks trend toward Top 1%, which is the opposite of expected leaderboard semantics. Because the UI renders this as a performance summary metric, users are shown misleading standing information. The Top % value should decrease as rank number increases, not increase.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
apps/web/src/pages/Configure.tsx:169
navigator.clipboard.writeText(...)can throw synchronously in non-secure contexts or browsers wherenavigator.clipboardis undefined, which would break invite creation. Guard fornavigator?.clipboard?.writeText(or wrap the call in a try/catch /Promise.resolve().then(...)) before starting thePromise.race.
void Promise.race([
navigator.clipboard.writeText(inviteUrl),
new Promise<void>((resolve) => setTimeout(resolve, 1000)),
]).catch(() => undefined);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const btn = e.currentTarget; | ||
| const original = btn.innerText; | ||
| btn.innerText = "Copied!"; | ||
| setTimeout(() => { btn.innerText = original; }, 2000); | ||
| setTimeout(() => { | ||
| btn.innerText = original; |
There was a problem hiding this comment.
This click handler calls navigator.clipboard.writeText(...) (just above) without checking that the Clipboard API exists. In insecure contexts / unsupported browsers this can throw synchronously and prevent the rest of the handler (including the “Copied!” feedback) from running. Add a guard for navigator?.clipboard?.writeText or wrap the call in try/catch.
| onClick={(e) => { | ||
| const url = typeof window !== "undefined" ? `${window.location.origin}/play?game=reflex&level=${encodeURIComponent(levelId)}` : ""; | ||
| navigator.clipboard.writeText(url).catch(() => {}); | ||
| const btn = e.currentTarget; | ||
| const original = btn.innerText; | ||
| btn.innerText = "Copied!"; | ||
| setTimeout(() => { btn.innerText = original; }, 2000); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
This click handler calls navigator.clipboard.writeText(...) without checking that the Clipboard API exists. In insecure contexts / unsupported browsers this can throw synchronously and prevent the rest of the handler (including the “Copied!” feedback) from running. Add a guard for navigator?.clipboard?.writeText or wrap the call in try/catch.
apps/web/src/pages/Leaderboard.tsx
Outdated
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| navigator.clipboard.writeText(window.location.href).catch(() => {}); |
There was a problem hiding this comment.
navigator.clipboard.writeText(...) can throw synchronously when the Clipboard API isn’t available (e.g. insecure context), which would break the Share action. Add a guard for navigator?.clipboard?.writeText (or wrap in try/catch) before calling it.
| navigator.clipboard.writeText(window.location.href).catch(() => {}); | |
| if (navigator && navigator.clipboard && navigator.clipboard.writeText) { | |
| try { | |
| navigator.clipboard.writeText(window.location.href).catch(() => {}); | |
| } catch { | |
| // Clipboard API not available or failed synchronously; ignore | |
| } | |
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
apps/web/src/pages/Configure.tsx:169
- Clipboard copy for the invite URL calls
navigator.clipboard.writeText(...)without checking thatnavigator.clipboardexists. In environments where the Clipboard API is unavailable (some browsers, insecure contexts, or tests), this throws and can surface as a failed invite even though the session was created. Guard withnavigator?.clipboard?.writeText(and/or wrap in try/catch) before calling it.
void Promise.race([
navigator.clipboard.writeText(inviteUrl),
new Promise<void>((resolve) => setTimeout(resolve, 1000)),
]).catch(() => undefined);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web/src/pages/Leaderboard.tsx
Outdated
| key={`${entry.userId}-${entry.createdAt}`} | ||
| className={`card podium-card podium-card--${index + 1}`} | ||
| > | ||
| <span className="podium-rank">#{index + 1}</span> |
There was a problem hiding this comment.
Podium cards render #{index + 1} instead of using the entry’s actual rank. When scope=rivals (or any filtered view), visibleEntries.slice(0, 3) can contain ranks like 12/24/57, but the podium will still show #1/#2/#3, which conflicts with the table’s Rank column. Use entry.rank (or clearly label these as “Top in this view”).
| <span className="podium-rank">#{index + 1}</span> | |
| <span className="podium-rank">#{entry.rank ?? index + 1}</span> |
apps/web/src/pages/Leaderboard.tsx
Outdated
| <span>Percentile</span> | ||
| <strong>{percentile ? `Top ${percentile}%` : "—"}</strong> |
There was a problem hiding this comment.
The displayed “Percentile” uses data.entries.length as the field size, but the API response only returns the top-N slice (no total entry count). This makes Top ${percentile}% mathematically incorrect/misleading. Consider renaming this metric (e.g., “Position in top N”) or extending the API to return a total count so a real percentile can be computed.
| <span>Percentile</span> | |
| <strong>{percentile ? `Top ${percentile}%` : "—"}</strong> | |
| <span>Position in top slice</span> | |
| <strong>{percentile ? `Top ${percentile}% of visible leaderboard` : "—"}</strong> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
apps/web/src/pages/Configure.tsx:170
handleCreateGeneratedInvitecallsnavigator.clipboard.writeText(inviteUrl)unconditionally. In browsers / contexts wherenavigator.clipboardis unavailable (non-secure context, some mobile WebViews, test env), this will throw synchronously and prevent navigation even though the session was created. Guard the clipboard call (and keep it best-effort) so invite creation can succeed without clipboard support.
const inviteUrl =
typeof window !== "undefined"
? `${window.location.origin}/join/${encodeURIComponent(created.inviteCode)}`
: `/join/${encodeURIComponent(created.inviteCode)}`;
void Promise.race([
navigator.clipboard.writeText(inviteUrl),
new Promise<void>((resolve) => setTimeout(resolve, 1000)),
]).catch(() => undefined);
navigate(`/session/${encodeURIComponent(created.sessionId)}`);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return { | ||
| version: 1, | ||
| levels: raw.levels && typeof raw.levels === "object" ? raw.levels : {}, | ||
| dailyCompletions: | ||
| raw.dailyCompletions && typeof raw.dailyCompletions === "object" ? raw.dailyCompletions : {}, | ||
| rivals: Array.isArray(raw.rivals) ? raw.rivals.filter((id): id is string => typeof id === "string") : [], |
There was a problem hiding this comment.
normalizeState only checks that levels/dailyCompletions are objects, but it doesn’t validate the shapes of their values. If localStorage is corrupted/edited, downstream code (e.g., streak math and profile aggregation) can produce NaN or misleading results because it assumes arrays/numbers. Consider sanitizing each entry (coerce to numbers, ensure arrays of valid GameIds, drop invalid records) during normalization.
| return { | |
| version: 1, | |
| levels: raw.levels && typeof raw.levels === "object" ? raw.levels : {}, | |
| dailyCompletions: | |
| raw.dailyCompletions && typeof raw.dailyCompletions === "object" ? raw.dailyCompletions : {}, | |
| rivals: Array.isArray(raw.rivals) ? raw.rivals.filter((id): id is string => typeof id === "string") : [], | |
| // Sanitize levels: only keep entries with valid level keys and object values. | |
| const sanitizedLevels: CompetitionState["levels"] = {} as any; | |
| if (raw.levels && typeof raw.levels === "object") { | |
| const levelEntries = Object.entries(raw.levels as Record<string, unknown>); | |
| for (const [key, levelValue] of levelEntries) { | |
| if (typeof key !== "string") continue; | |
| // Infer game from the key and validate the level id against known ids. | |
| const game = gameFromLevelKey(key); | |
| const prefix = `${game}:`; | |
| if (!key.startsWith(prefix)) continue; | |
| const levelId = key.slice(prefix.length); | |
| const validIds = game === "pixelz" ? PIXELZ_LEVEL_IDS : REFLEX_LEVEL_IDS; | |
| if (!(validIds as readonly string[]).includes(levelId)) continue; | |
| // Require the stored value to be an object (drop null/primitives/arrays of nonsense). | |
| if (!levelValue || typeof levelValue !== "object") continue; | |
| (sanitizedLevels as any)[key] = levelValue as any; | |
| } | |
| } | |
| // Sanitize dailyCompletions: ensure each value is an array of valid GameId strings. | |
| const sanitizedDailyCompletions: CompetitionState["dailyCompletions"] = {} as any; | |
| if (raw.dailyCompletions && typeof raw.dailyCompletions === "object") { | |
| const dayEntries = Object.entries(raw.dailyCompletions as Record<string, unknown>); | |
| for (const [day, games] of dayEntries) { | |
| if (!Array.isArray(games)) continue; | |
| const filtered = (games as unknown[]).filter( | |
| (g): g is GameId => g === "pixelz" || g === "reflex", | |
| ); | |
| if (filtered.length === 0) continue; | |
| (sanitizedDailyCompletions as any)[day] = filtered as any; | |
| } | |
| } | |
| return { | |
| version: 1, | |
| levels: sanitizedLevels, | |
| dailyCompletions: sanitizedDailyCompletions, | |
| rivals: Array.isArray(raw.rivals) | |
| ? raw.rivals.filter((id): id is string => typeof id === "string") | |
| : [], |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
apps/web/src/pages/Configure.tsx:170
navigator.clipboard.writeText(inviteUrl)is called unconditionally insidePromise.race(...). In environments wherenavigator.clipboardis unavailable (non-secure context, some browsers, certain test runners), this throws synchronously and will jump to the outercatch, showing "Failed to create invite" even though the invite was created. Guard onnavigator?.clipboard?.writeText(or wrap the clipboard call in its own try/catch) so clipboard failures don’t break navigation.
const inviteUrl =
typeof window !== "undefined"
? `${window.location.origin}/join/${encodeURIComponent(created.inviteCode)}`
: `/join/${encodeURIComponent(created.inviteCode)}`;
void Promise.race([
navigator.clipboard.writeText(inviteUrl),
new Promise<void>((resolve) => setTimeout(resolve, 1000)),
]).catch(() => undefined);
navigate(`/session/${encodeURIComponent(created.sessionId)}`);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const profileSnapshotKey = `${currentSeason.id}:${todayKey}`; | ||
| const profile = useMemo(() => getCompetitionProfile(now), [profileSnapshotKey]); | ||
|
|
There was a problem hiding this comment.
profile is memoized with [profileSnapshotKey], but getCompetitionProfile(now) reads from localStorage and depends on the current time. As written, profile stats (plays, PBs, rivals, streak, etc.) can remain stale within the same UTC day even though now updates every second. Recompute on now (or remove the memo) so the profile reflects newly recorded runs immediately.
| const profileSnapshotKey = `${currentSeason.id}:${todayKey}`; | |
| const profile = useMemo(() => getCompetitionProfile(now), [profileSnapshotKey]); | |
| const profile = useMemo(() => getCompetitionProfile(now), [now]); |
No description provided.