Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a SALES tab and Sales view to the Brain UI for curation/rank waves, implements MyStreamWaveSales component with paginated sales fetching, introduces useDebouncedQueryRefetch and useWaveDrops hooks, updates layout/context for sales styling, and expands tests to cover the new flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrainMobileTabs
participant BrainMobile
participant MyStreamWave
participant MyStreamWaveSales
participant useWaveDecisions
participant API
User->>BrainMobileTabs: Click SALES tab (curation wave)
BrainMobileTabs->>BrainMobile: onViewChange(BrainView.SALES)
BrainMobile->>MyStreamWave: Render with activeTab=SALES
MyStreamWave->>MyStreamWaveSales: Render SALES content (waveId)
MyStreamWaveSales->>useWaveDecisions: Fetch decisions for wave
useWaveDecisions->>API: GET /decisions?waveId=...
API-->>useWaveDecisions: Return decisionPoints
useWaveDecisions-->>MyStreamWaveSales: Return decisions
MyStreamWaveSales->>MyStreamWaveSales: Extract sale URLs and render MarketplacePreview cards
MyStreamWaveSales-->>User: Display sales grid
sequenceDiagram
participant useWaveDrops
participant useInfiniteQuery
participant API
participant WebSocket
participant useDebouncedQueryRefetch as Debounce
useWaveDrops->>useInfiniteQuery: Initialize queryKey & queryFn
useInfiniteQuery->>API: Fetch initial drops page
API-->>useInfiniteQuery: Return paginated drops
useInfiniteQuery-->>useWaveDrops: Return data + fetchNextPage
WebSocket->>useWaveDrops: DROP_UPDATE for waveId
useWaveDrops->>Debounce: requestRefetch()
Debounce->>useInfiniteQuery: refetch() (debounced)
useInfiniteQuery->>API: Re-fetch drops
API-->>useInfiniteQuery: Updated drops
useInfiniteQuery-->>useWaveDrops: Updated data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 unit tests (beta)
📝 Coding Plan for PR comments
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 |
![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
Restore enum re-export for activity wrapper consumers
This change removes the module-level re-export of USER_PAGE_ACTIVITY_TAB, but existing in-repo consumers still import that symbol from this file (for example __tests__/components/user/stats/activity/UserPageActivityTab.test.tsx and __tests__/components/user/stats/activity/tabs/UserPageActivityTabs.test.tsx). In environments that run these tests (or any downstream code using that import path), the import now fails with a missing export error, so this commit introduces a breaking API change without migrating all callers.
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/brain/mobile/BrainMobileTabs.tsx (1)
255-287:⚠️ Potential issue | 🟠 MajorHide
OUTCOMEfor curation waves in this mobile branch.This block now adds
SALESfor curation waves, but it still rendersBrainView.OUTCOMEunconditionally a few lines later. That no longer matches the curation tab contract used elsewhere in this PR, where curation waves includeSALESand omitOUTCOME, so mobile will expose an extra tab/view that desktop does not.Suggested fix
- <button - ref={(el) => { - tabRefs.current[BrainView.OUTCOME] = el; - }} - onClick={() => onViewChange(BrainView.OUTCOME)} - className={outcomeButtonClasses} - > - <span className={otucomeButtonTextClasses}>Outcome</span> - </button> + {!isCurationWave && ( + <button + ref={(el) => { + tabRefs.current[BrainView.OUTCOME] = el; + }} + onClick={() => onViewChange(BrainView.OUTCOME)} + className={outcomeButtonClasses} + > + <span className={otucomeButtonTextClasses}>Outcome</span> + </button> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/mobile/BrainMobileTabs.tsx` around lines 255 - 287, The OUTCOME button is rendered unconditionally which exposes BrainView.OUTCOME for curation waves; update the JSX so the button that assigns tabRefs.current[BrainView.OUTCOME] and renders with outcomeButtonClasses/otucomeButtonTextClasses is only rendered when isCurationWave is false (e.g., wrap it in a conditional !isCurationWave) to match the curation wave contract used elsewhere (where curation waves include SALES and omit OUTCOME); ensure tabRefs and onViewChange usage remains identical inside that conditional.
🧹 Nitpick comments (2)
__tests__/hooks/waves/useWaveDecisions.test.ts (2)
13-39: Consider adding edge-case tests for robustness.The current test covers the happy path well. For more confidence, consider adding tests for:
waveIdbeingundefinedornull(query should be disabled)- Empty
dataarray response- Error state handling
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/hooks/waves/useWaveDecisions.test.ts` around lines 13 - 39, Add unit tests in the useWaveDecisions test suite to cover edge cases: 1) call renderHook(() => useWaveDecisions({ waveId: undefined })) and assert useQueryMock was called with enabled: false for QueryKey.WAVE_DECISIONS and that result.current.decisionPoints is empty/handled; 2) mock useQueryMock to return { data: { data: [] }, isError: false, ... } and assert decisionPoints is an empty array; 3) mock useQueryMock to return an error state (isError: true, error: someError) and assert the hook surfaces error state (e.g., result.current.isError or result.current.error) and does not crash; use the existing symbols useWaveDecisions, useQueryMock, fetchMock, QueryKey.WAVE_DECISIONS, and decisionPoints to locate where to change tests.
21-21: Unused mock setup –fetchMock.mockResolvedValuehas no effect.Since
useQueryis fully mocked (lines 22-28), thequeryFninsideuseWaveDecisionsis never executed. This meansfetchMock.mockResolvedValue(data)is dead code and can mislead readers into thinkingcommonApiFetchis being tested.Either remove this line or, if you want to test the integration with
commonApiFetch, mock only the fetch and letuseQuerycall through to the actual implementation.🧹 Proposed fix: Remove unused mock
- fetchMock.mockResolvedValue(data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/hooks/waves/useWaveDecisions.test.ts` at line 21, Remove the dead mock by deleting the fetchMock.mockResolvedValue(data) call in the test for useWaveDecisions; since useQuery is fully mocked (the mock of useQuery that stubs the queryFn on lines ~22-28), the queryFn inside useWaveDecisions never executes and the fetch mock (fetchMock.mockResolvedValue) has no effect—either remove that line, or if you intend to test integration with commonApiFetch instead, stop mocking useQuery so the real queryFn in useWaveDecisions runs and let commonApiFetch be hit by the fetchMock.
🤖 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/MyStreamWaveSales.tsx`:
- Around line 39-45: The intersection callback driving auto-pagination (created
via useIntersectionObserver and stored in intersectionElementRef) calls
fetchNextPage blindly, which causes infinite advancement when pages contain zero
renderable sales (salesUrls filtered by nft_link). Change the guard so
fetchNextPage only runs when there is at least one newly rendered sale on the
current page or the sentinel has been moved below the last rendered item: track
the count/index of rendered sales (e.g., lastRenderableCount) derived from
salesUrls (or detect whether the latest page added any items with nft_link) and
return early if the latest page produced zero new renderable items; otherwise
call fetchNextPage. Keep the existing checks (hasNextPage, isFetching,
isFetchingNextPage) and update the intersection handler used in
useIntersectionObserver accordingly.
---
Outside diff comments:
In `@components/brain/mobile/BrainMobileTabs.tsx`:
- Around line 255-287: The OUTCOME button is rendered unconditionally which
exposes BrainView.OUTCOME for curation waves; update the JSX so the button that
assigns tabRefs.current[BrainView.OUTCOME] and renders with
outcomeButtonClasses/otucomeButtonTextClasses is only rendered when
isCurationWave is false (e.g., wrap it in a conditional !isCurationWave) to
match the curation wave contract used elsewhere (where curation waves include
SALES and omit OUTCOME); ensure tabRefs and onViewChange usage remains identical
inside that conditional.
---
Nitpick comments:
In `@__tests__/hooks/waves/useWaveDecisions.test.ts`:
- Around line 13-39: Add unit tests in the useWaveDecisions test suite to cover
edge cases: 1) call renderHook(() => useWaveDecisions({ waveId: undefined }))
and assert useQueryMock was called with enabled: false for
QueryKey.WAVE_DECISIONS and that result.current.decisionPoints is empty/handled;
2) mock useQueryMock to return { data: { data: [] }, isError: false, ... } and
assert decisionPoints is an empty array; 3) mock useQueryMock to return an error
state (isError: true, error: someError) and assert the hook surfaces error state
(e.g., result.current.isError or result.current.error) and does not crash; use
the existing symbols useWaveDecisions, useQueryMock, fetchMock,
QueryKey.WAVE_DECISIONS, and decisionPoints to locate where to change tests.
- Line 21: Remove the dead mock by deleting the
fetchMock.mockResolvedValue(data) call in the test for useWaveDecisions; since
useQuery is fully mocked (the mock of useQuery that stubs the queryFn on lines
~22-28), the queryFn inside useWaveDecisions never executes and the fetch mock
(fetchMock.mockResolvedValue) has no effect—either remove that line, or if you
intend to test integration with commonApiFetch instead, stop mocking useQuery so
the real queryFn in useWaveDecisions runs and let commonApiFetch be hit by the
fetchMock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c224adb1-74b5-4b9b-b0f8-3cba7d573065
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
__tests__/components/brain/BrainMobile.test.tsx__tests__/components/brain/ContentTabContext.test.tsx__tests__/components/brain/mobile/BrainMobileTabs.test.tsx__tests__/components/brain/my-stream/MyStreamWave.test.tsx__tests__/components/brain/my-stream/MyStreamWaveDesktopTabs.test.tsx__tests__/components/brain/my-stream/MyStreamWaveSales.test.tsx__tests__/components/brain/my-stream/MyStreamWaveTabsLeaderboard.test.tsx__tests__/hooks/useDebouncedQueryRefetch.test.ts__tests__/hooks/useWaveDrops.test.ts__tests__/hooks/useWaveGalleryDrops.test.ts__tests__/hooks/waves/useWaveDecisions.test.tscomponents/brain/BrainMobile.tsxcomponents/brain/ContentTabContext.tsxcomponents/brain/mobile/BrainMobileTabs.tsxcomponents/brain/my-stream/MyStreamWave.tsxcomponents/brain/my-stream/MyStreamWaveDesktopTabs.tsxcomponents/brain/my-stream/MyStreamWaveSales.tsxcomponents/brain/my-stream/MyStreamWaveTabsLeaderboard.tsxcomponents/brain/my-stream/layout/LayoutContext.tsxcomponents/user/stats/activity/UserPageActivityWrapper.tsxcomponents/user/stats/activity/activity.helpers.tshooks/useDebouncedQueryRefetch.tshooks/useDropMessages.tshooks/useWaveDrops.tshooks/useWaveGalleryDrops.tstypes/waves.types.ts
💤 Files with no reviewable changes (2)
- components/user/stats/activity/UserPageActivityWrapper.tsx
- components/user/stats/activity/activity.helpers.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/brain/my-stream/MyStreamWaveSales.tsx (1)
94-101: Consider consolidating duplicate "No sales yet." rendering paths.There are two separate code paths rendering "No sales yet.":
- Lines 94-101: when
salesUrls.length === 0 && !hasNextPage- Lines 118-124: when inside the else branch with
salesUrls.length === 0(buthasNextPageis true)The second path only triggers when there are no renderable sales but more pages exist. Since
handleIntersectionblocks fetching whensalesUrls.length === 0, users would see "No sales yet." even though subsequent pages might contain sales.This is an acceptable trade-off to prevent infinite fetching, but worth documenting with a comment explaining the intentional behavior.
📝 Suggested clarifying comment
} else if (salesUrls.length === 0 && !hasNextPage) { + // No renderable sales and pagination exhausted salesContent = ( ... ); } else { salesContent = ( <div className="tw-p-2 sm:tw-p-4 lg:tw-pr-2"> {salesUrls.length > 0 ? ( ... ) : ( + // No renderable sales yet; we don't auto-paginate with 0 sales + // to prevent infinite fetching on sparse waves <div className="tw-flex tw-h-full tw-items-center tw-justify-center tw-p-6">Also applies to: 118-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveSales.tsx` around lines 94 - 101, Consolidate the duplicate "No sales yet." JSX by extracting it into one reusable constant or small render helper (e.g., create renderNoSales and use it wherever salesContent is set) and replace the two separate JSX blocks with that single reference; also add a short inline comment near handleIntersection and the salesUrls/hasNextPage logic explaining that handleIntersection intentionally blocks further fetches when salesUrls.length === 0 so showing "No sales yet." in that case is expected behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/brain/my-stream/MyStreamWaveSales.tsx`:
- Around line 94-101: Consolidate the duplicate "No sales yet." JSX by
extracting it into one reusable constant or small render helper (e.g., create
renderNoSales and use it wherever salesContent is set) and replace the two
separate JSX blocks with that single reference; also add a short inline comment
near handleIntersection and the salesUrls/hasNextPage logic explaining that
handleIntersection intentionally blocks further fetches when salesUrls.length
=== 0 so showing "No sales yet." in that case is expected behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: abac6ab1-463c-4024-aa03-ee3e80bf64e3
📒 Files selected for processing (2)
__tests__/components/brain/my-stream/MyStreamWaveSales.test.tsxcomponents/brain/my-stream/MyStreamWaveSales.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/brain/my-stream/MyStreamWaveSales.tsx (1)
32-43: Use a stable key instead of${url}-${index}.This list is newest-first, so prepending one new sale shifts every later index and remounts existing
MarketplacePreviewcards. Keep the drop identity alongside the URL and key on that.Suggested refactor
- const salesUrls = useMemo( + const sales = useMemo( () => decisionPoints .slice() .reverse() .flatMap((decisionPoint) => decisionPoint.winners.flatMap((winner) => { const url = getFirstSaleUrl(winner.drop.nft_links); - return url ? [url] : []; + return url ? [{ id: winner.drop.id, url }] : []; }) ), [decisionPoints] ); - const isInitialLoading = isFetching && salesUrls.length === 0; + const isInitialLoading = isFetching && sales.length === 0; @@ - } else if (salesUrls.length === 0) { + } else if (sales.length === 0) { @@ - {salesUrls.map((url, index) => ( - <MarketplacePreview key={`${url}-${index}`} href={url} compact /> + {sales.map(({ id, url }) => ( + <MarketplacePreview key={id} href={url} compact /> ))}Also applies to: 71-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveSales.tsx` around lines 32 - 43, The current stable-key issue: salesUrls is built from decisionPoints and components use `${url}-${index}` which breaks stability when new items prepend; update code to produce and use a stable identity combining the sale URL and the drop's unique id (e.g., use winner.drop.id or another stable drop identifier) rather than the array index — modify the salesUrls construction (where getFirstSaleUrl is called) to return objects like { url, dropId } and then use that dropId when rendering MarketplacePreview (also update the same pattern at the other occurrence around lines 71-73) so keys remain stable across prepends.
🤖 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/MyStreamWaveSales.tsx`:
- Around line 31-43: The sales list is truncated because useWaveDecisions
currently only fetches page 1 with page_size: 100; update the data fetching in
useWaveDecisions (or its caller) to paginate and aggregate all pages (or stream
until no more results) so decisionPoints contains the full history, then keep
the existing useMemo that computes salesUrls from decisionPoints; ensure the
pagination logic deduplicates/merges pages and preserves ordering
(reverse+flatMap behavior relying on decisionPoints) so older rounds are not
silently dropped.
- Around line 31-64: The component currently treats an errored fetch the same as
an empty result because it only checks isFetching and decisionPoints; update the
use of useWaveDecisions to also destructure the error/ isError (or similar) flag
from the hook (e.g., const { decisionPoints, isFetching, error, isError } =
useWaveDecisions({ waveId })) and add an explicit branch before the empty-state
branch that renders an error UI when error/isError is truthy (use the same
styling container used for loading/empty states and include a concise message
like "Failed to load sales" plus error.message). Ensure references to
decisionPoints, salesUrls, isInitialLoading, and salesContent remain correct and
that the error branch runs before the salesUrls.length === 0 branch.
---
Nitpick comments:
In `@components/brain/my-stream/MyStreamWaveSales.tsx`:
- Around line 32-43: The current stable-key issue: salesUrls is built from
decisionPoints and components use `${url}-${index}` which breaks stability when
new items prepend; update code to produce and use a stable identity combining
the sale URL and the drop's unique id (e.g., use winner.drop.id or another
stable drop identifier) rather than the array index — modify the salesUrls
construction (where getFirstSaleUrl is called) to return objects like { url,
dropId } and then use that dropId when rendering MarketplacePreview (also update
the same pattern at the other occurrence around lines 71-73) so keys remain
stable across prepends.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b23ec6f-6030-4979-bed1-cbaa9b1b7f33
📒 Files selected for processing (2)
__tests__/components/brain/my-stream/MyStreamWaveSales.test.tsxcomponents/brain/my-stream/MyStreamWaveSales.tsx
|
GelatoGenesis
approved these changes
Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit