Conversation
Signed-off-by: ragnep <ragneinfo@gmail.com>
|
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 new "Brain Sidebar" feature to user pages: client-side components for created and most-active waves, a mobile strip, a hook to fetch favourite waves, minor layout updates to Drops, type updates for WaveCreatorPreview, and comprehensive tests. Changes
Sequence DiagramsequenceDiagram
participant User
participant Page as UserProfilePage
participant Sidebar as UserPageBrainSidebar
participant CreatedHook as useWaves
participant FavHook as useFavouriteWavesOfIdentity
participant API as Backend API
participant Sub as SubComponents
User->>Page: Open profile page
activate Page
Page->>Sidebar: Mount with profile
activate Sidebar
Sidebar->>Sidebar: derive identity (getProfileWaveIdentity)
par fetch created waves
Sidebar->>CreatedHook: request created waves (identity)
CreatedHook->>API: GET /waves?created_by=identity
API-->>CreatedHook: created waves[]
CreatedHook-->>Sidebar: {waves, status}
and fetch favourite waves
Sidebar->>FavHook: request favourites (identity, limit)
FavHook->>API: GET /waves-overview/favourites-of-identity/{id}?limit=N
API-->>FavHook: favourite waves[]
FavHook-->>Sidebar: {waves, status}
end
Sidebar->>Sidebar: compute shouldShowCreated / MostActive
alt any content
Sidebar->>Sub: render Created / MostActive / MobileStrip
Sub-->>User: display sidebar sections
else no content
Sidebar-->>Page: return null (no sidebar)
end
deactivate Sidebar
deactivate Page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Signed-off-by: ragnep <ragneinfo@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
components/user/brain/UserPageBrainSidebarMostActivePlaceholder.tsx (1)
5-12: Consider using a semantic heading element for better accessibility.The
aria-labelledbypattern is good, but using a<span>as the labeling element means screen reader users won't be able to navigate to this section via heading shortcuts. Consider using an<h2>or<h3>(styled appropriately) for better document outline and navigation.♿ Optional: Use semantic heading
<section aria-labelledby="brain-most-active-waves-heading"> - <span + <h3 id="brain-most-active-waves-heading" className="tw-mb-3 tw-text-xs tw-font-semibold tw-uppercase tw-tracking-wider tw-text-iron-500" > Most Active In - </span> + </h3> </section>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/UserPageBrainSidebarMostActivePlaceholder.tsx` around lines 5 - 12, Replace the non-semantic <span id="brain-most-active-waves-heading"> used as the label for the section with a proper heading element (e.g., <h2> or <h3>) in the UserPageBrainSidebarMostActivePlaceholder component so screen readers can navigate via heading shortcuts; keep the id "brain-most-active-waves-heading", retain the existing classes ("tw-mb-3 tw-text-xs tw-font-semibold tw-uppercase tw-tracking-wider tw-text-iron-500") for styling, and ensure the section's aria-labelledby still points to that id.__tests__/components/user/brain/UserPageDrops.test.tsx (1)
22-26: Test description doesn't match the test scenario.The test is named "hides Drops when no profile handle" but actually tests with
profile={null}. This tests the null guard (lines 10-12 in the component) rather than thehaveProfilecheck (line 14).Consider adding a test for the case where profile exists but has no handle:
🧪 Add test for profile without handle
it("hides Drops when no profile handle", () => { - const { queryByTestId } = render(<UserPageDrops profile={null} />); + const { queryByTestId, getByTestId } = render( + <UserPageDrops profile={{ handle: null, primary_wallet: "0x123" } as any} /> + ); expect(queryByTestId("drops")).toBeNull(); - expect(queryByTestId("brain-sidebar")).toBeNull(); + expect(getByTestId("brain-sidebar")).toBeInTheDocument(); + }); + + it("returns null when profile is null", () => { + const { container } = render(<UserPageDrops profile={null} />); + expect(container.firstChild).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/user/brain/UserPageDrops.test.tsx` around lines 22 - 26, Rename the existing test in UserPageDrops.test.tsx to accurately reflect it checks the null profile guard (e.g., "hides Drops when profile is null") and add a new test that renders <UserPageDrops profile={profileWithoutHandle} /> where profileWithoutHandle is an object with no handle property or handle set to falsy; assert that queryByTestId("drops") and queryByTestId("brain-sidebar") are null in that new test. Locate the component under test (UserPageDrops) and ensure the new test exercises the branch that uses the haveProfile/handle check rather than the null guard so both scenarios are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@__tests__/components/user/brain/UserPageDrops.test.tsx`:
- Around line 22-26: Rename the existing test in UserPageDrops.test.tsx to
accurately reflect it checks the null profile guard (e.g., "hides Drops when
profile is null") and add a new test that renders <UserPageDrops
profile={profileWithoutHandle} /> where profileWithoutHandle is an object with
no handle property or handle set to falsy; assert that queryByTestId("drops")
and queryByTestId("brain-sidebar") are null in that new test. Locate the
component under test (UserPageDrops) and ensure the new test exercises the
branch that uses the haveProfile/handle check rather than the null guard so both
scenarios are covered.
In `@components/user/brain/UserPageBrainSidebarMostActivePlaceholder.tsx`:
- Around line 5-12: Replace the non-semantic <span
id="brain-most-active-waves-heading"> used as the label for the section with a
proper heading element (e.g., <h2> or <h3>) in the
UserPageBrainSidebarMostActivePlaceholder component so screen readers can
navigate via heading shortcuts; keep the id "brain-most-active-waves-heading",
retain the existing classes ("tw-mb-3 tw-text-xs tw-font-semibold tw-uppercase
tw-tracking-wider tw-text-iron-500") for styling, and ensure the section's
aria-labelledby still points to that id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 955b2739-3295-4050-aeb7-4142eeb7cb69
📒 Files selected for processing (6)
__tests__/components/user/brain/UserPageBrainSidebar.test.tsx__tests__/components/user/brain/UserPageDrops.test.tsxcomponents/user/brain/UserPageBrainSidebar.tsxcomponents/user/brain/UserPageBrainSidebarCreated.tsxcomponents/user/brain/UserPageBrainSidebarMostActivePlaceholder.tsxcomponents/user/brain/UserPageDrops.tsx
Signed-off-by: ragnep <ragneinfo@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
components/user/brain/userPageBrainSidebar.helpers.ts (1)
3-4: Prefer a canonical fallback overprofile.query.
queryis the lookup input, not the resolved identity, so the same profile can end up with differentuseWaves/useFavouriteWavesOfIdentitycache keys depending on whether the page was opened by ENS, wallet, or id. SinceApiIdentity.primary_walletis always present, using that as the fallback keeps the identity stable.Suggested change
export const getProfileWaveIdentity = (profile: ApiIdentity): string => - profile.handle ?? profile.query ?? profile.primary_wallet; + profile.handle ?? profile.primary_wallet;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/userPageBrainSidebar.helpers.ts` around lines 3 - 4, Change getProfileWaveIdentity to avoid using profile.query as a fallback; use a stable canonical identity order like profile.handle ?? profile.primary_wallet so cache keys remain consistent regardless of lookup method. Update the function (getProfileWaveIdentity) to prefer handle, then primary_wallet, and remove query from the fallback chain.openapi.yaml (2)
4693-4702: Align the endpoint name with the behavior it documents.This contract describes an activity-ranked result, not an explicit “favourite” relationship. Keeping
favourites-of-identity/getFavouriteWavesOfIdentityhere is likely to mislead API consumers and generated clients. Either rename the path/operationId or rewrite the summary/description to describe actual favourites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 4693 - 4702, The path /waves-overview/favourites-of-identity and operationId getFavouriteWavesOfIdentity are misleading because the endpoint returns activity-ranked waves, not an explicit favourites relation; either rename the path and operationId to reflect activity ranking (e.g., /waves-overview/most-active-by-identity and operationId getMostActiveWavesOfIdentity) or update the summary/description of getFavouriteWavesOfIdentity to explicitly state it returns waves ranked by message count authored by the resolved profile and not a favourites list; modify whichever you choose consistently in the OpenAPI doc so path, operationId, summary, and description are aligned.
4698-4728: Document the stable sort order for paginated results.With
limit/offset, “ranked by how many chat messages” is not enough. Ties need a deterministic secondary sort, otherwise clients can see duplicates or skips across pages. Please spell out the effective ordering in the description.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 4698 - 4728, The current description for operationId getFavouriteWavesOfIdentity is ambiguous for paginated results; update the description to declare a deterministic, stable sort order (primary: descending by message count, secondary: deterministic tie-breaker such as descending lastMessageTimestamp or ascending waveId) so limit/offset pagination cannot produce duplicates or skips; modify the OpenAPI description to explicitly state the exact effective ordering used (e.g., "ordered by message_count DESC, then last_message_timestamp DESC, then wave_id ASC") and ensure clients can rely on that stable ordering when paging with the limit and offset parameters.components/user/brain/UserPageBrainSidebarMostActive.tsx (1)
9-24: Extract the shared sidebar loading skeleton.This markup is duplicated almost verbatim in
components/user/brain/UserPageBrainSidebarCreated.tsx. A shared sidebar loading component would keep future styling changes in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/UserPageBrainSidebarMostActive.tsx` around lines 9 - 24, Duplicate JSX for the loading skeleton (the LoadingState component in UserPageBrainSidebarMostActive.tsx) should be extracted into a single reusable component (e.g., BrainSidebarLoading or SidebarLoadingSkeleton) and exported for reuse; create that new component containing the current JSX structure and styles, replace the local LoadingState in UserPageBrainSidebarMostActive and the matching markup in UserPageBrainSidebarCreated with an import and usage of the new BrainSidebarLoading component, and ensure props/aria-labels are preserved (or made configurable) so both places render identically.components/user/brain/UserPageDrops.tsx (1)
13-23: Keep both columns behind the same profile-ready guard.Line 19 already treats
profile.handleas the capability check for the main content, but the sidebar still renders when that check fails. That leaves a half-empty two-column shell for handleless identities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/UserPageDrops.tsx` around lines 13 - 23, The sidebar is rendered even when profile.handle is falsy, leaving a half-empty two-column layout; change the render so both columns are gated by the same profile-ready check: use the existing haveProfile boolean to conditionally render the entire grid (or at least both children) so that Drops and UserPageBrainSidebar only render when haveProfile is true; locate the JSX block that defines the grid and update it so the <div className="tw-min-w-0">{haveProfile && <Drops />}</div> and <UserPageBrainSidebar profile={profile} /> are inside the same haveProfile conditional (or wrap the grid in if (haveProfile) {...}) so the sidebar no longer appears for handleless profiles.
🤖 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/react-query-wrapper/ReactQueryWrapper.tsx`:
- Line 63: The new cache key IDENTITY_FAVOURITE_WAVES is never invalidated by
the existing drop mutation flows; update the mutation handlers so that when
drops are added/updated you also invalidate IDENTITY_FAVOURITE_WAVES.
Concretely, in the addOptimisticDrop flow (function addOptimisticDrop) and in
invalidateDrops (function invalidateDrops) add a call to the
queryClient.invalidateQueries or equivalent targeting the
IDENTITY_FAVOURITE_WAVES key (alongside the existing drop-related invalidations)
so the sidebar "Most Active In" section is refreshed after posting or replying.
In `@components/user/brain/UserPageBrainSidebarCreated.tsx`:
- Around line 48-50: The logic that hides cached waves when a background refetch
errors is in shouldShowWaves and shouldRenderSection: remove the !error check so
shouldShowWaves = waves.length > 0 and adjust shouldRenderSection to use the
updated shouldShowWaves (keep shouldShowLoading logic); then handle/show the
error state separately (e.g., render an inline error banner or icon) so cached
waves remain visible when error is truthy. Apply the same change in
UserPageBrainSidebarMostActive.tsx where the identical !error gating is used.
---
Nitpick comments:
In `@components/user/brain/userPageBrainSidebar.helpers.ts`:
- Around line 3-4: Change getProfileWaveIdentity to avoid using profile.query as
a fallback; use a stable canonical identity order like profile.handle ??
profile.primary_wallet so cache keys remain consistent regardless of lookup
method. Update the function (getProfileWaveIdentity) to prefer handle, then
primary_wallet, and remove query from the fallback chain.
In `@components/user/brain/UserPageBrainSidebarMostActive.tsx`:
- Around line 9-24: Duplicate JSX for the loading skeleton (the LoadingState
component in UserPageBrainSidebarMostActive.tsx) should be extracted into a
single reusable component (e.g., BrainSidebarLoading or SidebarLoadingSkeleton)
and exported for reuse; create that new component containing the current JSX
structure and styles, replace the local LoadingState in
UserPageBrainSidebarMostActive and the matching markup in
UserPageBrainSidebarCreated with an import and usage of the new
BrainSidebarLoading component, and ensure props/aria-labels are preserved (or
made configurable) so both places render identically.
In `@components/user/brain/UserPageDrops.tsx`:
- Around line 13-23: The sidebar is rendered even when profile.handle is falsy,
leaving a half-empty two-column layout; change the render so both columns are
gated by the same profile-ready check: use the existing haveProfile boolean to
conditionally render the entire grid (or at least both children) so that Drops
and UserPageBrainSidebar only render when haveProfile is true; locate the JSX
block that defines the grid and update it so the <div
className="tw-min-w-0">{haveProfile && <Drops />}</div> and
<UserPageBrainSidebar profile={profile} /> are inside the same haveProfile
conditional (or wrap the grid in if (haveProfile) {...}) so the sidebar no
longer appears for handleless profiles.
In `@openapi.yaml`:
- Around line 4693-4702: The path /waves-overview/favourites-of-identity and
operationId getFavouriteWavesOfIdentity are misleading because the endpoint
returns activity-ranked waves, not an explicit favourites relation; either
rename the path and operationId to reflect activity ranking (e.g.,
/waves-overview/most-active-by-identity and operationId
getMostActiveWavesOfIdentity) or update the summary/description of
getFavouriteWavesOfIdentity to explicitly state it returns waves ranked by
message count authored by the resolved profile and not a favourites list; modify
whichever you choose consistently in the OpenAPI doc so path, operationId,
summary, and description are aligned.
- Around line 4698-4728: The current description for operationId
getFavouriteWavesOfIdentity is ambiguous for paginated results; update the
description to declare a deterministic, stable sort order (primary: descending
by message count, secondary: deterministic tie-breaker such as descending
lastMessageTimestamp or ascending waveId) so limit/offset pagination cannot
produce duplicates or skips; modify the OpenAPI description to explicitly state
the exact effective ordering used (e.g., "ordered by message_count DESC, then
last_message_timestamp DESC, then wave_id ASC") and ensure clients can rely on
that stable ordering when paging with the limit and offset parameters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e203f6bc-6b78-47e4-b303-c5379ff41cd7
📒 Files selected for processing (10)
__tests__/components/user/brain/UserPageBrainSidebar.test.tsxcomponents/react-query-wrapper/ReactQueryWrapper.tsxcomponents/user/brain/UserPageBrainSidebar.tsxcomponents/user/brain/UserPageBrainSidebarCreated.tsxcomponents/user/brain/UserPageBrainSidebarMostActive.tsxcomponents/user/brain/UserPageBrainSidebarWaveItem.tsxcomponents/user/brain/UserPageDrops.tsxcomponents/user/brain/userPageBrainSidebar.helpers.tshooks/useFavouriteWavesOfIdentity.tsopenapi.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/components/user/brain/UserPageBrainSidebar.test.tsx
Signed-off-by: ragnep <ragneinfo@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/user/brain/UserPageBrainSidebarCreated.tsx (1)
49-50:⚠️ Potential issue | 🟠 MajorDon’t hide cached waves when refetch errors occur (still present).
Line 49 gates rendering on
!error, which can hide valid cachedwavesduring background refetch failures. This concern was already raised previously and is still unresolved.Suggested adjustment
- const shouldShowWaves = !error && waves.length > 0; + const shouldShowWaves = waves.length > 0;In TanStack React Query v5, can a query expose cached data and a non-null error simultaneously after a background refetch failure?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/UserPageBrainSidebarCreated.tsx` around lines 49 - 50, The current render gate uses "!error && waves.length > 0" which hides cached waves when a background refetch fails (React Query v5 can provide data and an error simultaneously); update the logic so shouldShowWaves only checks waves.length > 0 (remove the "!error" check) and keep shouldRenderSection = shouldShowLoading || shouldShowWaves so cached waves still render despite a non-null error; adjust any downstream UI that relied on the previous error gating if needed (refer to shouldShowWaves, shouldShowLoading, shouldRenderSection, waves, and error).
🤖 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/user/brain/UserPageBrainSidebarCreated.tsx`:
- Around line 49-50: The current render gate uses "!error && waves.length > 0"
which hides cached waves when a background refetch fails (React Query v5 can
provide data and an error simultaneously); update the logic so shouldShowWaves
only checks waves.length > 0 (remove the "!error" check) and keep
shouldRenderSection = shouldShowLoading || shouldShowWaves so cached waves still
render despite a non-null error; adjust any downstream UI that relied on the
previous error gating if needed (refer to shouldShowWaves, shouldShowLoading,
shouldRenderSection, waves, and error).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c536e43-8c8f-4cdf-9d38-54b5d6df0700
📒 Files selected for processing (6)
__tests__/components/user/brain/UserPageBrainSidebar.test.tsx__tests__/components/user/brain/UserPageBrainSidebarWaveItem.test.tsx__tests__/components/waves/drops/WaveCreatorPreviewItem.test.tsxcomponents/user/brain/UserPageBrainSidebarCreated.tsxcomponents/user/brain/UserPageBrainSidebarWaveItem.tsxcomponents/waves/drops/WaveCreatorPreviewItem.tsx
✅ Files skipped from review due to trivial changes (1)
- tests/components/user/brain/UserPageBrainSidebarWaveItem.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- components/user/brain/UserPageBrainSidebarWaveItem.tsx
Signed-off-by: ragnep <ragneinfo@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/user/brain/UserPageBrainSidebarMobileStrip.tsx (1)
65-75: Consider removing the redundantkeyprop from the skeleton element.The
MobileWavePillSkeletoncomponent receiveskeyIdas a prop and also uses it internally on line 68 as akeyattribute on the div. However, when this component is rendered in a.map()(lines 163-168), React'skeyis already provided on the component itself. The internalkey={keyId}on line 68 is redundant.Suggested simplification
function MobileWavePillSkeleton({ keyId }: { readonly keyId: string }) { return ( <div - key={keyId} className="tw-inline-flex tw-h-9 tw-items-center tw-gap-2 tw-rounded-lg tw-border tw-border-solid tw-border-white/[0.06] tw-bg-iron-950/70 tw-p-1 tw-pr-3" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/UserPageBrainSidebarMobileStrip.tsx` around lines 65 - 75, The MobileWavePillSkeleton component is redundantly applying key={keyId} to the root div even though React keys are provided on the component instances when mapped; remove the internal key prop from the div in MobileWavePillSkeleton (keep the keyId prop only for identification if needed) so the component relies on the parent-provided React key and avoid duplicate/unnecessary key attributes.
🤖 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/user/brain/UserPageBrainSidebarMobileStrip.tsx`:
- Around line 87-93: The current visibility guards for cached lists suppress
showing cached waves when a background refetch errors; update the logic in
UserPageBrainSidebarMobileStrip by removing the error checks in
shouldShowCreatedWaves and shouldShowMostActiveWaves so they only depend on the
presence of items (i.e., drop the "(createdError === null || createdError ===
undefined)" and "(mostActiveError === null || mostActiveError === undefined)"
parts) while keeping the existing loading flags (shouldShowMostActiveLoading)
unchanged.
In `@components/user/brain/UserPageBrainSidebarMostActive.tsx`:
- Around line 19-20: The conditional for showing cached waves currently hides
them when a background refetch sets error; update the logic in
UserPageBrainSidebarMostActive (the shouldShowWaves calculation) to remove the
error check and only rely on waves.length > 0 so cached data is rendered even if
error is set, and then surface the error separately (e.g., render an inline
error indicator using the existing error variable) rather than blocking the
waves from being displayed.
---
Nitpick comments:
In `@components/user/brain/UserPageBrainSidebarMobileStrip.tsx`:
- Around line 65-75: The MobileWavePillSkeleton component is redundantly
applying key={keyId} to the root div even though React keys are provided on the
component instances when mapped; remove the internal key prop from the div in
MobileWavePillSkeleton (keep the keyId prop only for identification if needed)
so the component relies on the parent-provided React key and avoid
duplicate/unnecessary key attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdf7cab9-c22e-48a5-8bcc-32c5ff17a1c1
📒 Files selected for processing (14)
__tests__/components/user/brain/UserPageBrainSidebar.test.tsx__tests__/components/user/brain/UserPageBrainSidebarWaveItem.test.tsx__tests__/components/waves/drops/WaveCreatorPreviewItem.test.tsxcomponents/react-query-wrapper/ReactQueryWrapper.tsxcomponents/user/brain/UserPageBrainSidebar.tsxcomponents/user/brain/UserPageBrainSidebarCreated.tsxcomponents/user/brain/UserPageBrainSidebarMobileStrip.tsxcomponents/user/brain/UserPageBrainSidebarMostActive.tsxcomponents/user/brain/UserPageBrainSidebarWaveItem.tsxcomponents/user/brain/UserPageDrops.tsxcomponents/waves/drops/WaveCreatorPreviewItem.tsxcomponents/waves/drops/WaveCreatorPreviewModal.tsxcomponents/waves/drops/WaveCreatorPreviewModalContent.tsxcomponents/waves/drops/waveCreatorPreview.types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- components/react-query-wrapper/ReactQueryWrapper.tsx
- components/user/brain/UserPageDrops.tsx
- components/waves/drops/WaveCreatorPreviewItem.tsx
- components/user/brain/UserPageBrainSidebarWaveItem.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (2)
components/user/brain/UserPageBrainSidebarCreated.tsx (1)
70-70: Minor: Background color inconsistency.The button uses
tw-bg-blackwhile other components in this PR (e.g.,UserPageBrainSidebarMobileStrip.tsx) usetw-bg-iron-950for similar elements. Consider aligning for visual consistency.Suggested change
- className="tw-mt-2 tw-cursor-pointer tw-border-none tw-bg-black tw-px-1 tw-text-xs tw-font-semibold tw-text-iron-500 tw-transition-colors desktop-hover:hover:tw-text-iron-300" + className="tw-mt-2 tw-cursor-pointer tw-border-none tw-bg-iron-950 tw-px-1 tw-text-xs tw-font-semibold tw-text-iron-500 tw-transition-colors desktop-hover:hover:tw-text-iron-300"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/UserPageBrainSidebarCreated.tsx` at line 70, The background color on the created-brain button in UserPageBrainSidebarCreated (the element with className containing "tw-bg-black") should be changed to match the other components' theme; replace "tw-bg-black" with "tw-bg-iron-950" in the className for the JSX element in UserPageBrainSidebarCreated (to align with UserPageBrainSidebarMobileStrip.tsx styling).components/user/brain/UserPageBrainSidebarMobileStrip.tsx (1)
63-73: Redundantkeyprop inside component.The
key={keyId}on Line 66 is inside the component definition, but React'skeyprop must be specified where the component is instantiated (which is already done at Lines 104, 105, 159). Thekeyattribute on the inner<div>serves no reconciliation purpose here. Consider removing it and renamingkeyIdto something more semantic if it's only used for debugging or testing.Suggested change
function MobileWavePillSkeleton({ keyId }: { readonly keyId: string }) { return ( <div - key={keyId} className="tw-inline-flex tw-h-9 tw-items-center tw-gap-2 tw-rounded-lg tw-border tw-border-solid tw-border-white/[0.06] tw-bg-iron-950/70 tw-p-1 tw-pr-3" + data-testid={keyId} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/brain/UserPageBrainSidebarMobileStrip.tsx` around lines 63 - 73, The component MobileWavePillSkeleton currently accepts a prop named keyId and applies it as a React key to the root <div>, which is redundant and ineffective because keys must be set by the parent when instantiating the component; remove the key={keyId} from the component output and rename the prop to a semantic name (e.g., id or testId or pillId) to reflect that it’s for identification/testing only, then update all callers of MobileWavePillSkeleton to pass the new prop name (or drop the prop entirely if unused) so reconciliation keys remain only on the component instances.
🤖 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/user/brain/UserPageBrainSidebarCreated.tsx`:
- Line 70: The background color on the created-brain button in
UserPageBrainSidebarCreated (the element with className containing
"tw-bg-black") should be changed to match the other components' theme; replace
"tw-bg-black" with "tw-bg-iron-950" in the className for the JSX element in
UserPageBrainSidebarCreated (to align with UserPageBrainSidebarMobileStrip.tsx
styling).
In `@components/user/brain/UserPageBrainSidebarMobileStrip.tsx`:
- Around line 63-73: The component MobileWavePillSkeleton currently accepts a
prop named keyId and applies it as a React key to the root <div>, which is
redundant and ineffective because keys must be set by the parent when
instantiating the component; remove the key={keyId} from the component output
and rename the prop to a semantic name (e.g., id or testId or pillId) to reflect
that it’s for identification/testing only, then update all callers of
MobileWavePillSkeleton to pass the new prop name (or drop the prop entirely if
unused) so reconciliation keys remain only on the component instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6c98516-3c5d-4ad4-ad11-e60b4257082d
📒 Files selected for processing (4)
components/user/brain/UserPageBrainSidebar.tsxcomponents/user/brain/UserPageBrainSidebarCreated.tsxcomponents/user/brain/UserPageBrainSidebarMobileStrip.tsxcomponents/user/brain/UserPageBrainSidebarMostActive.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- components/user/brain/UserPageBrainSidebarMostActive.tsx
- components/user/brain/UserPageBrainSidebar.tsx
|
GelatoGenesis
approved these changes
Mar 13, 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
New Features
Improvements
Tests