I complete mobile ui.#1
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@techeng322 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request introduces various modifications across multiple files in the project. Key changes include updates to component structures, such as the removal of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatComponent
participant ChatProvider
participant ChatService
User->>ChatComponent: Send Message
ChatComponent->>ChatProvider: Update messages
ChatProvider->>ChatService: Handle message
ChatService-->>ChatProvider: Return response
ChatProvider-->>ChatComponent: Update UI
ChatComponent-->>User: Display response
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (10)
app/layout.tsx (1)
2-2: Simplified font handling with GeistSans.The switch to using
GeistSansfrom the Geist library simplifies font management. This approach is more straightforward and reduces custom font configuration.Consider the trade-offs:
- Simplified setup vs. potential loss of flexibility for custom font weights/styles.
- Ensure that
GeistSansprovides all the necessary font variations for your project's typography needs.components/Chat/Chat.tsx (2)
9-14: LGTM: Responsive layout and improved user guidanceThe conditional styling based on
messages.lengthcreates a responsive layout that adapts well to different states of the chat. The new prompt provides clear guidance to users, enhancing the overall user experience. These changes align well with the PR objective of completing the mobile UI.Consider adding an
aria-labelto the div for better accessibility:<div className={`w-screen overflow-hidden ${messages.length ? "px-4 pt-20 mb-[150px] overflow-y-auto h-[calc(100vh-150px)]" : "flex flex-col items-center justify-center h-screen"}`} + aria-label="Chat interface" >
Line range hint
1-22: Overall: Good progress on mobile UI completionThe changes in this file contribute well to the PR objective of completing the mobile UI. Key improvements include:
- Centralized state management with
useChatProvider- Responsive layout adapting to the presence of messages
- Clear user guidance with the new prompt
These modifications enhance the component's suitability for mobile interfaces by improving responsiveness and user experience. Good job on these updates!
As you continue to develop the mobile UI, consider the following:
- Ensure touch-friendly interaction areas for all interactive elements
- Test the layout on various mobile screen sizes to confirm its responsiveness
- Consider implementing swipe gestures for common actions if applicable to your chat interface
components/Chat/Messages.tsx (2)
12-13: LGTM: Improved message container structure.The new div structure enhances the layout of each message with better spacing and visual separation. The use of flexbox and the
gap-2class provides a consistent layout.Consider adding a
items-startclass to the outer div to ensure proper alignment between the icon and the text, especially for multi-line messages:- <div key={index} className="p-3 rounded-lg flex w-full gap-2"> + <div key={index} className="p-3 rounded-lg flex w-full gap-2 items-start">
14-19: LGTM: Added role-based icons for messages.The addition of icons for different message roles enhances the visual distinction between user and AI messages, improving the overall user experience.
To improve accessibility, consider adding an
aria-labelto the icon containers:<div className="size-fit"> {message.role === "user" ? ( - <UserIcon className="h-6 w-6" /> + <UserIcon className="h-6 w-6" aria-label="User message" /> ) : ( - <TvMinimalPlay className="h-6 w-6" /> + <TvMinimalPlay className="h-6 w-6" aria-label="AI message" /> )} </div>This will provide context for screen readers, enhancing the accessibility of your chat interface.
package.json (2)
9-10: LGTM! Consider extending the format script.The addition of the "format" script is a good practice for maintaining consistent code style across the project. The comma after the "lint" script is correctly added to maintain valid JSON syntax.
Consider extending the format script to include all relevant directories and file types. For example:
- "format": "yarn prettier --write app components providers types hooks" + "format": "yarn prettier --write \"**/*.{js,jsx,ts,tsx,json,md}\""This change will ensure that all JavaScript, TypeScript, JSON, and Markdown files throughout the project are formatted, regardless of their location.
36-36: LGTM! Consider using a more flexible version range for Prettier.The addition of Prettier as a dev dependency is excellent for maintaining consistent code style. This aligns well with the new "format" script added earlier.
Consider using a more flexible version range for Prettier to automatically receive bug fixes and minor improvements:
- "prettier": "3.3.3", + "prettier": "^3.3.3",This change allows for compatible updates within the 3.x.x range, ensuring you receive bug fixes and minor improvements automatically.
lib/getFandata.tsx (1)
Line range hint
1-52: Consider using more modern JavaScript features.While the current changes improve code style, there are opportunities to further enhance the code using modern JavaScript features:
Use optional chaining consistently:
Replacefan.recommendations === "string" ? fan.recommendations : ""withfan.recommendations?.toString() ?? "".Use nullish coalescing:
Replacetypeof fan.playlist === "string" ? fan.playlist : ""withfan.playlist ?? "".Use
Array.prototype.flatMap():
Consider replacing theArray.from().concat()pattern withflatMap()for a more concise implementation.Here's an example of how you could refactor the
artistNamesextraction:const artistNames = [ ...(recommendations?.matchAll(/artistName:([^,}]+)/g) ?? []), ...(recentlyPlayed?.matchAll(/artistName:([^,}]+)/g) ?? []), ].flatMap(match => match?.[1]?.trim() ?? []);This approach combines the extraction from both
recommendationsandrecentlyPlayedin a single operation, potentially simplifying the code.lib/server/chat-llm.service.ts (1)
Inconsistency Found Between StreamResponseSchema and streamResponse Method Parameters
StreamResponseSchemaincludesaccountIdas a required field.- The
streamResponsemethod does not utilizeaccountIdin its parameters or implementation.🔗 Analysis chain
Line range hint
22-25: Verify consistency between StreamResponseSchema and streamResponse methodThere's a potential inconsistency between the
StreamResponseSchemaand thestreamResponsemethod parameters. The schema includesaccountIdas a required field, but the method doesn't seem to use it. Consider updating the method signature to match the schema or adjusting the schema ifaccountIdisn't needed in the method.To verify this, let's check for other usages of
StreamResponseSchemaandstreamResponse:Also applies to: 47-50
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usages of StreamResponseSchema and streamResponse echo "Searching for StreamResponseSchema usages:" rg "StreamResponseSchema" --type typescript echo "\nSearching for streamResponse method calls:" rg "streamResponse" --type typescriptLength of output: 345
Script:
#!/bin/bash # Description: Corrected search for StreamResponseSchema and streamResponse usages with proper TypeScript type echo "Searching for StreamResponseSchema usages:" rg "StreamResponseSchema" --type ts echo "\nSearching for streamResponse method calls:" rg "streamResponse" --type tsLength of output: 716
Script:
#!/bin/bash # Description: Extract the definition of the streamResponse method to verify its parameters echo "Extracting streamResponse method definitions:" rg "async streamResponse" --type ts -C 5Length of output: 833
components/Chat/ChatInput.tsx (1)
30-30: Prevent submissions with only whitespaceCurrently,
canSubmitis determined byBoolean(input), which istrueeven ifinputcontains only whitespace. This could allow users to submit empty messages consisting of spaces or tabs.Consider trimming the
inputbefore checking its length to ensure that only meaningful content enables the submit button:- <SubmitButton canSubmit={Boolean(input)} /> + <SubmitButton canSubmit={input.trim().length > 0} />This prevents submissions of messages that are empty after trimming whitespace.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
- app/api/chat/route.ts (1 hunks)
- app/layout.tsx (2 hunks)
- components/Chat/Chat.tsx (1 hunks)
- components/Chat/ChatInput.tsx (2 hunks)
- components/Chat/Messages.tsx (1 hunks)
- components/Chat/SubmitButton.tsx (1 hunks)
- components/Chat/Suggestions.tsx (0 hunks)
- components/LandingPage/LandingPage.tsx (1 hunks)
- hooks/useChat.tsx (1 hunks)
- lib/getChatContext.tsx (1 hunks)
- lib/getFandata.tsx (3 hunks)
- lib/server/chat-llm.service.ts (1 hunks)
- next.config.mjs (1 hunks)
- package.json (3 hunks)
- providers/ChatProvider.tsx (1 hunks)
- tailwind.config.ts (1 hunks)
- types/fans.ts (1 hunks)
💤 Files with no reviewable changes (1)
- components/Chat/Suggestions.tsx
✅ Files skipped from review due to trivial changes (4)
- app/api/chat/route.ts
- lib/getChatContext.tsx
- providers/ChatProvider.tsx
- types/fans.ts
🧰 Additional context used
🔇 Additional comments (23)
next.config.mjs (1)
2-5: LGTM! Good configuration updates.The changes to
nextConfigare beneficial:
Enabling
reactStrictModeis a good practice. It helps identify potential problems in your React application during development.Adding
transpilePackages: ["geist"]ensures that the "geist" package is properly transpiled. This is important if "geist" contains ES6+ code that needs to be converted for compatibility reasons.These configurations will improve development experience and ensure better compatibility.
tailwind.config.ts (3)
11-13: LGTM: Font family configuration looks good.The addition of the
fontFamilyconfiguration with thesansproperty using a CSS variable is a good practice. It allows for flexible font management across the project.
Line range hint
1-18: Clarify the relationship between font changes and mobile UI completion.The changes in this file focus on font configuration, which doesn't seem directly related to completing the mobile UI as mentioned in the PR title. Could you please clarify how this font change contributes to the mobile UI completion? Are there additional changes in other files that are more specifically related to mobile UI enhancements?
To help understand the full scope of the mobile UI changes, you can run the following script to find recent modifications in mobile-related files:
#!/bin/bash # Description: Find recent changes in mobile-related files # Search for files with mobile-related names that have been modified in the last week git log --since=1.week --name-only --pretty=format: | sort | uniq | grep -iE 'mobile|responsive|screen' # List the differences in these files git diff HEAD~10 HEAD -- $(git log --since=1.week --name-only --pretty=format: | sort | uniq | grep -iE 'mobile|responsive|screen')This will help provide context on other mobile UI-related changes that might be part of this PR.
11-13: Verify the removal of thecolorsobject and its impact.The AI summary mentions that a
colorsobject was removed from thetheme.extendconfiguration. However, this removal is not visible in the provided code snippet.Could you please confirm if the
colorsobject was indeed removed? If so, please ensure that this removal doesn't negatively impact the existing color schemes in the project. Run the following script to check for any remaining references to the removed color configurations:If this script returns any results, it might indicate areas of the codebase that need to be updated due to the removal of the
colorsobject.components/Chat/SubmitButton.tsx (1)
3-3: Improved prop naming enhances component clarity and reusability.The change from
isChatboxFocusedtocanSubmitis a good improvement. It makes the component's purpose clearer and increases its reusability. The prop now directly represents the button's ability to submit rather than the state of another component.app/layout.tsx (2)
Line range hint
1-24: Verify impact of removed font variables.The variables
geistSansandgeistMonohave been removed as part of simplifying the font management. While this is consistent with the new approach, it's important to ensure this change doesn't negatively impact other parts of the application.Please run the following script to check for any remaining usage of the removed variables:
#!/bin/bash # Description: Search for usage of removed font variables # Test: Look for geistSans and geistMono usage rg --type=typescript --type=javascript --type=css '\b(geistSans|geistMono)\b'If the script returns any results, those occurrences will need to be updated to use the new
GeistSansimport or alternative styling for monospace text.
18-18: Consistent application of GeistSans font.The change correctly applies the Geist Sans font to the entire application, which is consistent with the new import strategy.
Consider if there are any components that specifically required the previously used monospace font (
geistMono). If so, you may need to implement a separate solution for those cases.To verify the usage of monospace fonts in the project, run the following script:
✅ Verification successful
No remaining usages of monospace fonts detected.
The search did not find any instances of
monoormonospaceinclassNameorstyleattributes across the project. The removal ofgeistMonoappears safe.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for potential usages of monospace fonts in the project # Test: Look for 'mono' or 'monospace' in className attributes or style definitions rg --type=typescript --type=javascript --type=css -i '(className|style).*\b(mono|monospace)\b'Length of output: 273
Script:
#!/bin/bash # Description: Search for potential usages of monospace fonts in the project # Corrected Test: Look for 'mono' or 'monospace' in className attributes or style definitions rg --type=ts --type=js --type=css -i '(className|style).*\b(mono|monospace)\b'Length of output: 80
components/Chat/Chat.tsx (2)
1-1: LGTM: Import changes align with component updatesThe addition of
useChatProviderimport and removal ofSuggestionsimport are consistent with the changes in the component logic. These modifications appear to support the PR objective of completing the mobile UI by simplifying the component structure.
6-6: LGTM: Improved state management withuseChatProviderThe use of
useChatProviderhook centralizes state management, which is a good practice for maintainability.Could you clarify how this change specifically contributes to completing the mobile UI? It seems like a general improvement rather than a mobile-specific change.
components/Chat/Messages.tsx (2)
3-3: LGTM: New import for icons.The addition of
UserIconandTvMinimalPlayfromlucide-reactis appropriate for the UI enhancements made in this component.
9-9: Verify layout and scrolling behavior after class removals.The
max-w-xlandoverflow-y-autoclasses have been removed from the outer div. While this simplifies the styling, it might affect the layout and scrolling behavior of the component.Please confirm that:
- The removal of
max-w-xldoesn't cause unintended layout issues on larger screens.- The content still scrolls properly without the
overflow-y-autoclass, especially when there are many messages.If these changes cause any issues, consider adding appropriate classes or styles to maintain the desired layout and scrolling behavior.
lib/getFandata.tsx (3)
11-11: LGTM: Trailing comma added to arrow function.The addition of a trailing comma after the arrow function parameter is a good practice. It improves code maintainability by making future additions to the array easier and resulting in cleaner diffs in version control.
21-21: LGTM: Consistent use of trailing comma.The addition of a trailing comma here is consistent with the change made earlier in the file. This maintains a uniform coding style, which is excellent for readability and maintainability.
32-33: LGTM: Improved formatting for better readability.The addition of a trailing comma and moving the closing parenthesis to a new line enhances code readability. This is particularly beneficial for longer function bodies or when additional parameters might be added in the future.
hooks/useChat.tsx (4)
5-5: LGTM: Addition of Address importThe addition of the
Addressimport from 'viem' is appropriate, likely used for type annotations in the code.
Line range hint
9-76: LGTM: useChat hook implementationThe implementation of the
useChathook remains consistent and functional. The removal of thezeroAddressimport doesn't affect the hook's logic, and the usage of theAddresstype is appropriate. The core functionality, including user authentication, message handling, and interaction with the chat API, is preserved.
Line range hint
1-76: Summary: Minor import adjustments with no functional changesThe changes in this file are minimal and focused on import statement cleanup. The removal of the unused
zeroAddressimport and the addition of theAddresstype import improve code cleanliness without altering the functionality of theuseChathook. These changes align with good coding practices and don't introduce any apparent issues.However, to ensure completeness:
- Verify that
zeroAddressis indeed no longer needed in this file or any dependent code.- Confirm that the
Addresstype is being used correctly throughout the file.Overall, these changes appear to be a positive refinement of the code.
To double-check the usage of
Address, let's run the following script:#!/bin/bash # Description: Check the usage of Address type in the file # Test: Search for Address usage in the file rg 'Address' hooks/useChat.tsx
Line range hint
1-7: Verify the impact of removing zeroAddress importThe
zeroAddressimport from 'viem' has been removed. While this suggests it's no longer used in the file, we should verify that its removal doesn't have unintended consequences elsewhere in the codebase.Let's run a script to check for any remaining usage of
zeroAddress:✅ Verification successful
Verified: The removal of
zeroAddressdoes not impact other parts of the codebase. No remaining usage ofzeroAddresswas found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of zeroAddress in the codebase # Test: Search for zeroAddress usage rg 'zeroAddress' --type typescript --type tsxLength of output: 313
Script:
#!/bin/bash # Description: Check for any remaining usage of zeroAddress in the codebase # Test: Search for zeroAddress usage rg 'zeroAddress' --type typescript --type tsxLength of output: 80
Script:
#!/bin/bash # Description: Check for any remaining usage of zeroAddress in the codebase # Test: Search for zeroAddress usage in .ts and .tsx files rg 'zeroAddress' -t ts -t tsxLength of output: 57
Script:
#!/bin/bash # Description: Check for any remaining usage of zeroAddress in .ts files # Test: Search for zeroAddress usage in .ts files rg 'zeroAddress' -t tsLength of output: 22
lib/server/chat-llm.service.ts (2)
18-18: LGTM: Syntax correction in ChatMessagesSchemaThis change correctly closes the object definition within the array of the
ChatMessagesSchema. It's a crucial fix that ensures proper schema structure and validation.
Line range hint
22-25: LGTM: Enhanced StreamResponseSchema with user-specific fieldsThe addition of
accountIdandaddressto theStreamResponseSchemais a good improvement. It allows for better user-specific data handling, which aligns with the PR objective of completing the mobile UI.
accountIdas a required UUID ensures each response is tied to a unique user account.- The optional
addressfield provides flexibility for cases where it might not be available or necessary.These changes enhance the schema's capability to handle user-specific information in stream responses.
components/LandingPage/LandingPage.tsx (1)
5-7: Simplification of LandingPage ComponentThe refactoring effectively simplifies the
LandingPagecomponent by rendering only theChatcomponent. This reduces complexity and improves maintainability.components/Chat/ChatInput.tsx (2)
5-6: LGTM!The addition of
messagesfromuseChatProvideris appropriate and ensures the component has access to message data for conditional rendering.
16-17: Confirm thatmessagesis always definedUsing
messages.lengthfor conditional rendering relies onmessagesbeing an array. Ensure thatmessagesis always defined and is an array to prevent potential runtime errors ifmessagesisundefinedornull.To verify that
messagesis always an array, consider adding a default value in your provider or checking its initialization.
| ? "bg-[#00309A] text-white hover:bg-[#002277]" | ||
| : "bg-white text-[#00309A] border border-[#00309A] hover:bg-gray-100" | ||
| } hover:shadow-md group-hover:scale-105`} | ||
| className={`p-1.5 rounded-md border-[1px] border-gray-700 text-gray-700 ${canSubmit ? "bg-white" : "bg-black"}`} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving button styling for better readability and consistency.
While the conditional styling based on canSubmit is implemented correctly, there are some potential issues with the current design:
- When
canSubmitis true, you have gray text on a white background, which might not provide sufficient contrast. - When
canSubmitis false, you have gray text on a black background, which could be hard to read. - The border color remains constant, which might not provide enough visual distinction between states.
Consider the following improvements:
- Use more contrasting colors for text and background in both states.
- Change the border color based on the
canSubmitstate for better visual feedback. - If available, use color variables from your design system instead of hardcoded color values.
Here's a suggested implementation:
className={`p-1.5 rounded-md border-[1px] ${
canSubmit
? "bg-primary text-white border-primary"
: "bg-gray-200 text-gray-500 border-gray-300"
}`}Replace primary, gray-200, etc., with the appropriate color variables from your design system.
| <p className="text-sm font-sans text-pretty break-all"> | ||
| {message.content} | ||
| </p> |
There was a problem hiding this comment.
Refine text styling for better readability.
The updated styling for the message content improves consistency with font-sans, but there are a couple of points to address:
text-prettyis not a standard Tailwind class. Did you mean to usetext-baseor another text size class?break-allmight cause unnatural line breaks. Consider usingbreak-wordsinstead for better readability.
Here's a suggested improvement:
- <p className="text-sm font-sans text-pretty break-all">
+ <p className="text-sm font-sans break-words">This change will maintain the font size and family while improving text wrapping behavior for better readability.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p className="text-sm font-sans text-pretty break-all"> | |
| {message.content} | |
| </p> | |
| <p className="text-sm font-sans break-words"> | |
| {message.content} | |
| </p> |
| "@supabase/supabase-js": "^2.45.1", | ||
| "@tanstack/react-query": "^5.56.2", | ||
| "ai": "^3.3.17", | ||
| "geist": "^1.3.1", |
There was a problem hiding this comment.
💡 Codebase verification
'geist' package added but not utilized in the codebase
- No imports of 'geist' found in project files.
🔗 Analysis chain
LGTM! Please provide more context on the 'geist' package.
The addition of the 'geist' package (version ^1.3.1) is noted. This aligns with the PR objective of completing the mobile UI.
Could you please provide more information on how the 'geist' package will be used in the project? This will help ensure it's properly integrated and utilized across the codebase.
To verify the usage of the 'geist' package, we can run the following script:
This script will help us understand how and where the 'geist' package is being used in the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for imports of the 'geist' package in the project files
# Search for 'geist' imports in JavaScript and TypeScript files
echo "Searching for 'geist' imports:"
rg --type-add 'web:*.{js,jsx,ts,tsx}' --type web "from ['\"]geist['\"]" -g '!node_modules'
# Check if 'geist' is configured in any config files
echo -e "\nChecking for 'geist' configuration:"
rg --type-add 'config:*.{js,json}' --type config "geist" -g '!node_modules'
Length of output: 378
| <form onSubmit={handleSubmit} className="w-full"> | ||
| <textarea | ||
| value={input} | ||
| onChange={handleInputChange} | ||
| onKeyDown={handleKeyDown} | ||
| placeholder="Ask me anything about the music industry..." | ||
| className="bg-transparent w-full p-2 text-sm !border-none !outline-none rounded-md h-auto" | ||
| aria-label="Chat input" | ||
| /> |
There was a problem hiding this comment.
Avoid casting KeyboardEvent to FormEvent when calling handleSubmit
In handleKeyDown, you are casting a KeyboardEvent to a FormEvent<HTMLFormElement>:
handleSubmit(e as unknown as React.FormEvent<HTMLFormElement>);This casting can lead to type safety issues and potential runtime errors since a KeyboardEvent is not a FormEvent.
Consider modifying handleSubmit to make the event parameter optional, allowing it to be called without an event object from handleKeyDown:
- const handleSubmit = (e: React.FormEvent<HTMLFormElement>) => {
+ const handleSubmit = (e?: React.FormEvent<HTMLFormElement>) => {
+ if (e) e.preventDefault();
// Existing submit logic
};
const handleKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => {
if (e.key === "Enter" && !e.shiftKey) {
e.preventDefault();
handleSubmit();
}
};This change maintains type safety and eliminates the need to cast events.
…-to-accept-uri-parameter Add contractUri parameter to createCollection
… gates the bootstrap E2E found two POSTs on initial mount: one with `artist_id: null`, one with `artist_id: <saved-selection>`. Cause: in react-query v5 `isLoading` is `isPending && isFetching` — it's false when the query is disabled. While `useArtistsRoster` waits for `userData` from `UserProvider` to land, `enabled: false`, `isLoading: false`. The bootstrap effect saw "settled, empty roster" and fired POST #1 with `artistId: undefined`. Once `userData` arrived, the query enabled, artists resolved, selection moved to the saved artist, and the mutation effect re-fired with the real `artistId` (POST #2). Switch to `isPending` (true while disabled OR fetching) so the loading flag stays true through the entire "we don't know the roster yet" window. Surfaced as `isLoading` to keep the consumer interface stable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1759) * feat(chat): stamp sessions with selected artist; refactor useArtists Tags every newly-provisioned session with `selectedArtist.account_id` via `POST /api/sessions { artistId }` (api#628 already accepts it), so the sidebar's artist filter (#1756) groups chats under the right artist. Each artist switch mints a fresh session under the new context. The hard part wasn't passing the field — it was making the bootstrap fire exactly once per (artistId, orgId). Earlier iterations stacked refs on top of `useState` to police a duplicate POST that fired because `useArtists` resolved `selectedArtist` across two renders (commit A: `isLoading` flips false with `selectedArtist=null`; commit B: saved selection restored). Both commits independently satisfied the bootstrap's effect gate, both started a POST, both wrote to `sessions.artist_id`. Refs guarded the symptom; the real fix is to make `selectedArtist` resolve in a single render. ### `useArtists` — react-query + derived selection - Roster comes from `useQuery`, keyed on `(accountId, orgId)`. No imperative `getArtists` effect, no `setIsLoading` lifecycle. - `selectedArtist` is a single `useMemo` deriving from `(artists, savedSelections, orgKey, userOverride)`. Precedence: user override (this session) → saved (localStorage, per-org) → artists[0] auto-pick → null. - The race is gone *by construction*. There is no longer a render where `artists` is populated but `selectedArtist` is still null. - `useInitialArtists` deleted — its restoration effect folds into the memo; the sync-on-org-change effect is implicit (memo recomputes when `orgKey` changes). - `setArtists` mirrors React's `setState` signature (array | updater) so `useArtistPinToggle` and `useDeleteArtist` keep working without changes; under the hood it's `queryClient.setQueryData`. - `getArtists(artistId?)` becomes `queryClient.fetchQuery` + an optional `setSelectedArtist` post-fetch, preserving the existing call sites in `useArtistPinToggle`, `useDeleteArtist`, the `CreateArtistToolResult` / `UpdateArtistInfoSuccess` / `UpdateArtistSocialsSuccess` tool result components, and `saveSetting`. ### `useNewChatBootstrap` — useMutation, no refs - POST is semantically a mutation. `useMutation` owns the in-flight / success / error state instead of a hand-rolled state machine. - Effect gates on `mutation.variables` (react-query's own last-mutated args) plus `isPending` / `isSuccess`. Incidental re-renders re-enter the effect but bail idempotently. No refs. - Artist or org change → mutation.variables no longer matches → fresh `mutate()` fires under the new context. Orphan session from the old context is accepted (rare in practice). ### `Artist.tsx` — narrow the hard-nav - Hard-navigates to `/chat` only when switching artists from a tagged chat URL (`/sessions/{sid}/chats/{cid}` or legacy `/chat/{roomId}`). On bare `/chat` the bootstrap re-fires in place when `artistId` changes — no nav, no remount flicker. - `window.location.href` (not `router.replace`) because `useVercelChat`'s `silentlyUpdateUrl` writes via `history.replaceState`, leaving Next's internal router state out of sync with the URL bar — a client-side replace can no-op. ### `lib/sessions/createSession.ts` - `CreateSessionInput` gains optional `artistId`. Wire-compatible with `api/lib/sessions/validateCreateSessionBody.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chat): extract artist selection + chat-session provisioning per OCP Addresses sweetmantech's review comments on lines `useArtists.tsx:80` and `useNewChatBootstrap.ts:107` — net-new logic was being added inline to existing hooks instead of being given its own module. Two extractions: 1. `hooks/artists/useArtistSelection.ts` — owns the per-org artist selection: `useLocalStorage` for saved picks, `userOverride` state for explicit this-session pick/deselect, and the derived `selectedArtist` memo (precedence: override → saved → first). `useArtists.tsx` now just calls it with `(orgKey, artists)`. 2. `lib/sessions/provisionChatSession.ts` — combines `createSession` + `createSandbox` into one function. `useNewChatBootstrap.ts` now wraps that in `useMutation` and owns the trigger / input-change re-fire logic only. No behavior change — both hooks return the same shape and respond to the same inputs. Just smaller files with clearer single responsibility. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): throw on missing Privy token in useArtists queryFn Addresses CodeRabbit feedback on `hooks/useArtists.tsx:61`. The previous shape silently resolved to `[]` when `getAccessToken()` came back null transiently — `useQuery` marked the query successful, `isLoading` flipped false, and `useNewChatBootstrap` saw a "settled, empty roster" and POSTed a session with `artistId: undefined`. Throw so the query goes to `isError` and the bootstrap stays gated. Same fix on the imperative `getArtists` callback for consistency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(chat): extract roster + provisioner hooks per OCP Two more OCP extractions, addressing the remaining inline-net-new-code comments on `useArtists.tsx` and `useNewChatBootstrap.ts`: 1. `hooks/artists/useArtistsRoster.ts` — owns the react-query fetch keyed on `(userId, orgId)`, the `setArtists` optimistic-update helper, and the `refetchArtists` imperative refetch. `useArtists` now consumes it via `useArtistsRoster({ userId, orgId })`. 2. `hooks/sessions/useProvisionChatSession.ts` — owns the mutation lifecycle: the `useMutation` setup, the `useEffect` with `sameInputs` guard, and the state-mapping from mutation state to the discriminated `ProvisionChatSessionState`. `useNewChatBootstrap` shrinks to a thin provider-wiring shim (~30 LOC). Each existing hook now has a single clear responsibility: - `useArtists` composes roster + selection + per-artist settings - `useNewChatBootstrap` wires Privy/Org/Artist providers into the provisioner No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): gate useArtistsRoster on authenticated, drop the throw Matches `useConversations` / `useCredits` — they gate the query on Privy's `authenticated` rather than throwing inside the queryFn for the transient-null-token case. Now this hook does the same: the query won't fire until Privy is ready, and if the token is still null in some pathological case the network call 401s and the query goes to `isError` (same outcome the throw produced). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): use isPending in useArtistsRoster so disabled-query window gates the bootstrap E2E found two POSTs on initial mount: one with `artist_id: null`, one with `artist_id: <saved-selection>`. Cause: in react-query v5 `isLoading` is `isPending && isFetching` — it's false when the query is disabled. While `useArtistsRoster` waits for `userData` from `UserProvider` to land, `enabled: false`, `isLoading: false`. The bootstrap effect saw "settled, empty roster" and fired POST #1 with `artistId: undefined`. Once `userData` arrived, the query enabled, artists resolved, selection moved to the saved artist, and the mutation effect re-fired with the real `artistId` (POST #2). Switch to `isPending` (true while disabled OR fetching) so the loading flag stays true through the entire "we don't know the roster yet" window. Surfaced as `isLoading` to keep the consumer interface stable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com>
2 participants
Summary by CodeRabbit
Release Notes
New Features
ChatInputfor better usability and responsiveness.Bug Fixes
Documentation
Chores
Refactor
LandingPageto focus solely on the chat component, removing unnecessary complexity.