Persist leaderboard after wave is over#2257
Conversation
📝 WalkthroughWalkthroughAdds a SUBMISSIONS view and component, wires it into desktop and mobile tab/view logic and defaults, changes content/tab normalization when voting ends, exposes leaderboard error state, refactors time and media-query helpers, and adds/updates tests (one old test file removed). Changes
Sequence DiagramsequenceDiagram
participant User
participant TabCtx as ContentTabContext
participant MyStream as MyStreamWave
participant Submissions as MyStreamWaveSubmissions
participant LeaderboardAPI as useWaveDropsLeaderboard
User->>TabCtx: updateAvailableTabs(votingState = ENDED)
TabCtx->>TabCtx: compute tabs -> include SUBMISSIONS
TabCtx->>MyStream: set activeContentTab = SUBMISSIONS
User->>MyStream: render activeContentTab
MyStream->>Submissions: mount(wave, onDropClick)
Submissions->>Submissions: router.replace(remove curation_id)
Submissions->>LeaderboardAPI: fetch initial pages (wave.id)
LeaderboardAPI-->>Submissions: return drops / isError / hasNextPage
User->>Submissions: click drop
Submissions->>Submissions: router.push(drop=..., remove curation_id)
User->>Submissions: scroll sentinel visible
Submissions->>LeaderboardAPI: fetchNextPage()
LeaderboardAPI-->>Submissions: append more drops
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67bbe11837
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
__tests__/components/brain/mobile/BrainMobileTabs.test.tsx (1)
6-19: Prefer importingBrainViewinstead of redefining it in test.Line 10 is correct, but keeping a local enum copy can silently diverge from
components/brain/mobile/brainMobileViews.ts.♻️ Suggested refactor
+import { BrainView } from "@/components/brain/mobile/brainMobileViews"; - -enum BrainView { - DEFAULT = "DEFAULT", - ABOUT = "ABOUT", - LEADERBOARD = "LEADERBOARD", - SUBMISSIONS = "SUBMISSIONS", - SALES = "SALES", - WINNERS = "WINNERS", - OUTCOME = "OUTCOME", - MY_VOTES = "MY_VOTES", - FAQ = "FAQ", - WAVES = "WAVES", - MESSAGES = "MESSAGES", - NOTIFICATIONS = "NOTIFICATIONS", -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/brain/mobile/BrainMobileTabs.test.tsx` around lines 6 - 19, The test currently redefines the BrainView enum which can drift from the source; remove the local enum declaration and import BrainView from the canonical module (components/brain/mobile/brainMobileViews.ts), then update any references in BrainMobileTabs.test.tsx to use the imported BrainView symbol so the test always reflects the real implementation.components/brain/my-stream/MyStreamWaveSubmissions.tsx (2)
56-64: Avoid pushing the same URL repeatedly.Add a no-op guard when the current
dropalready matchesdropIdandcuration_idis already absent.Proposed refactor
const openDropById = useCallback( (dropId: string) => { const params = new URLSearchParams(searchParamsString || ""); + const currentDropId = params.get("drop"); + const hasCurationFilter = params.has(CURATION_FILTER_PARAM); + if (currentDropId === dropId && !hasCurationFilter) { + return; + } params.delete(CURATION_FILTER_PARAM); params.set("drop", dropId); router.push(`${pathname}?${params.toString()}`, { scroll: false }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveSubmissions.tsx` around lines 56 - 64, openDropById currently always pushes a new URL even when nothing changes; update the function (openDropById) to first build URLSearchParams from searchParamsString, read currentDrop = params.get("drop") and check if currentDrop === dropId and !params.has(CURATION_FILTER_PARAM); if both true return early (no-op), otherwise proceed to params.delete(CURATION_FILTER_PARAM), params.set("drop", dropId) and router.push as before; keep the same dependencies ([pathname, router, searchParamsString]).
112-114: Render the intersection sentinel only when needed.Once
hasNextPageis false, the sentinel can be skipped to reduce unnecessary observer callbacks.Proposed refactor
- <div ref={intersectionElementRef}></div> + {hasNextPage && <div ref={intersectionElementRef} aria-hidden="true" />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveSubmissions.tsx` around lines 112 - 114, The sentinel div is always rendered which triggers the intersection observer even when there are no more pages; update the render to only mount the sentinel when hasNextPage is true by conditionally rendering the <div ref={intersectionElementRef}> (keep the existing isFetchingNextPage check for <WaveLeaderboardLoadingBar /> unchanged) so the observer is only attached while hasNextPage is true; locate the sentinel near the isFetchingNextPage usage and change its rendering to depend on hasNextPage and intersectionElementRef.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/components/brain/ContentTabContext.test.tsx`:
- Around line 188-194: The test expectation for result.current.availableTabs is
incorrect for the case where isCurationWave is false; update the assertion in
the test to match buildDefaultTabs behavior by removing MyStreamWaveTab.MY_VOTES
so the expected array is [MyStreamWaveTab.CHAT, MyStreamWaveTab.SUBMISSIONS,
MyStreamWaveTab.WINNERS, MyStreamWaveTab.OUTCOME]; alternatively, if you
intended to assert the curation-wave behavior, set isCurationWave to true before
calling the provider so that MyStreamWaveTab.MY_VOTES is included.
In `@components/brain/mobile/useBrainMobileActiveView.ts`:
- Around line 105-106: The computed waveDefaultView should not pick
BrainView.SUBMISSIONS until the wave payload exists; update the logic around the
waveDefaultView assignment (and the similar block around lines 177-183) to
require a non-null/loaded wave (e.g., check wave !== null or wave.payload
exists) in addition to isRankWave && isCompleted before returning
BrainView.SUBMISSIONS, so normalizeActiveView() won't preserve SUBMISSIONS when
the wave is still loading and BrainMobileViewContent won't render null.
In `@components/brain/my-stream/MyStreamWaveSubmissions.tsx`:
- Around line 35-40: The component currently treats a failed leaderboard fetch
the same as an empty result; update the render logic where
useWaveDropsLeaderboard is used (the destructure with drops, fetchNextPage,
hasNextPage, isFetching, isFetchingNextPage) to check for an error state before
the empty-state branch: use the hook's error/isError (or equivalent) and refetch
(or retry) function to render an explicit error UI with a retry button that
calls refetch, and only fall back to "No submissions to show" when there is no
error and drops is empty; apply the same change for the other occurrence around
lines 85-95 where the leaderboard hook is consumed.
---
Nitpick comments:
In `@__tests__/components/brain/mobile/BrainMobileTabs.test.tsx`:
- Around line 6-19: The test currently redefines the BrainView enum which can
drift from the source; remove the local enum declaration and import BrainView
from the canonical module (components/brain/mobile/brainMobileViews.ts), then
update any references in BrainMobileTabs.test.tsx to use the imported BrainView
symbol so the test always reflects the real implementation.
In `@components/brain/my-stream/MyStreamWaveSubmissions.tsx`:
- Around line 56-64: openDropById currently always pushes a new URL even when
nothing changes; update the function (openDropById) to first build
URLSearchParams from searchParamsString, read currentDrop = params.get("drop")
and check if currentDrop === dropId and !params.has(CURATION_FILTER_PARAM); if
both true return early (no-op), otherwise proceed to
params.delete(CURATION_FILTER_PARAM), params.set("drop", dropId) and router.push
as before; keep the same dependencies ([pathname, router, searchParamsString]).
- Around line 112-114: The sentinel div is always rendered which triggers the
intersection observer even when there are no more pages; update the render to
only mount the sentinel when hasNextPage is true by conditionally rendering the
<div ref={intersectionElementRef}> (keep the existing isFetchingNextPage check
for <WaveLeaderboardLoadingBar /> unchanged) so the observer is only attached
while hasNextPage is true; locate the sentinel near the isFetchingNextPage usage
and change its rendering to depend on hasNextPage and intersectionElementRef.
🪄 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: d66c0128-094b-4d09-8fc5-96e842f2d784
📒 Files selected for processing (20)
__tests__/components/brain/ContentTabContext.test.tsx__tests__/components/brain/mobile/BrainMobileTabs.test.tsx__tests__/components/brain/mobile/BrainMobileViewContent.test.tsx__tests__/components/brain/my-stream/MyStreamWave.test.tsx__tests__/components/brain/my-stream/MyStreamWaveSubmissions.test.tsx__tests__/components/brain/my-stream/MyStreamWaveTabsLeaderboard.test.tsx__tests__/helpers/waves/time.utils.test.tscomponents/brain/ContentTabContext.tsxcomponents/brain/mobile/BrainMobileViewContent.tsxcomponents/brain/mobile/brainMobileViews.tscomponents/brain/mobile/useBrainMobileActiveView.tscomponents/brain/my-stream/MyStreamWave.tsxcomponents/brain/my-stream/MyStreamWaveDesktopTabs.tsxcomponents/brain/my-stream/MyStreamWaveSubmissions.tsxcomponents/brain/my-stream/MyStreamWaveTabsLeaderboard.tsxcomponents/utils/input/identity/IdentitySearch.tsxcomponents/waves/leaderboard/WaveLeaderboardTime.tsxhelpers/waves/time.utils.tshooks/useMediaQuery.tstypes/waves.types.ts
💤 Files with no reviewable changes (1)
- tests/components/brain/my-stream/MyStreamWave.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/brain/my-stream/MyStreamWaveSubmissions.tsx`:
- Around line 49-51: Replace the uses of the void operator that suppress promise
rejections with explicit no-op catch handlers: in handleRetry, change void
refetch() to refetch().catch(() => undefined) (referencing handleRetry and
refetch), and apply the same pattern to the other promise call that currently
uses void on line ~89 — replace that void <promiseCall>() with
<promiseCall>().catch(() => undefined) so any rejection is explicitly handled.
🪄 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: 65c0b52c-e3e3-4cf2-99f5-43585ac727e8
📒 Files selected for processing (6)
__tests__/components/brain/mobile/useBrainMobileActiveView.test.ts__tests__/components/brain/my-stream/MyStreamWaveSubmissions.test.tsx__tests__/hooks/useWaveDropsLeaderboard.test.tscomponents/brain/mobile/useBrainMobileActiveView.tscomponents/brain/my-stream/MyStreamWaveSubmissions.tsxhooks/useWaveDropsLeaderboard.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/components/brain/my-stream/MyStreamWaveSubmissions.test.tsx
- components/brain/mobile/useBrainMobileActiveView.ts
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/brain/my-stream/MyStreamWaveSubmissions.tsx (1)
49-51:⚠️ Potential issue | 🟡 MinorHandle
refetch/fetchNextPagepromises explicitly.At Line 50 and Line 89, promise rejections are still not explicitly handled (
void refetch()/ barefetchNextPage()). This was previously flagged and can still fail static analysis or mask runtime failures.Suggested fix
const handleRetry = useCallback(() => { - void refetch(); + refetch().catch(() => undefined); }, [refetch]); @@ - fetchNextPage(); + fetchNextPage().catch(() => undefined);#!/bin/bash # Verify unresolved floating/suppressed promise patterns in this file rg -nP '\bvoid\s+refetch\s*\(' components/brain/my-stream/MyStreamWaveSubmissions.tsx -C2 rg -nP '\bfetchNextPage\s*\(\s*\)\s*;' components/brain/my-stream/MyStreamWaveSubmissions.tsx -C2 # Inspect hook source to confirm these methods are promise-returning fd -i useWaveDropsLeaderboard | xargs -I{} rg -n 'fetchNextPage|refetch|useInfiniteQuery' "{}" -C3Also applies to: 89-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveSubmissions.tsx` around lines 49 - 51, The refetch and fetchNextPage calls in handleRetry and the infinite-scroll handler currently drop returned promises (void refetch() and bare fetchNextPage()), so update those call sites to explicitly handle promise rejections: make handleRetry an async callback and await refetch() inside a try/catch (or call refetch().catch(err => /* log or set error state */)) and likewise replace the bare fetchNextPage() with await fetchNextPage() inside a try/catch or fetchNextPage().catch(err => /* handle/log */); refer to handleRetry, refetch, and fetchNextPage to locate the places to change and ensure any error is logged or surfaced to component state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/brain/my-stream/MyStreamWaveSubmissions.tsx`:
- Around line 49-51: The refetch and fetchNextPage calls in handleRetry and the
infinite-scroll handler currently drop returned promises (void refetch() and
bare fetchNextPage()), so update those call sites to explicitly handle promise
rejections: make handleRetry an async callback and await refetch() inside a
try/catch (or call refetch().catch(err => /* log or set error state */)) and
likewise replace the bare fetchNextPage() with await fetchNextPage() inside a
try/catch or fetchNextPage().catch(err => /* handle/log */); refer to
handleRetry, refetch, and fetchNextPage to locate the places to change and
ensure any error is logged or surfaced to component state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ee4d3e4-5427-40e1-810b-64492e896398
📒 Files selected for processing (3)
components/brain/my-stream/MyStreamWaveSubmissions.tsxcomponents/waves/leaderboard/WaveLeaderboardTime.tsxknip.jsonc
✅ Files skipped from review due to trivial changes (1)
- knip.jsonc
🚧 Files skipped from review as they are similar to previous changes (1)
- components/waves/leaderboard/WaveLeaderboardTime.tsx



Summary by CodeRabbit
New Features
Bug Fixes
Tests