feat: daisyUI sidebar #15
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis update removes a comprehensive set of custom UI component files, including advanced sidebar, dropdown, sheet, tooltip, and highlight systems, and replaces the chat sidebar with a minimal version. Layout components are refactored to remove dependencies on the deleted UI modules. Additional changes include dependency updates, a new Next.js bundle analysis workflow, and minor configuration tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navbar/Drawer
participant ChatSidebar (Minimal)
participant MainContent
User->>Navbar/Drawer: Clicks hamburger menu
Navbar/Drawer->>ChatSidebar (Minimal): Opens sidebar drawer
ChatSidebar (Minimal)-->>User: Displays chat links
User->>ChatSidebar (Minimal): Selects chat
ChatSidebar (Minimal)->>MainContent: Loads selected chat
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @maamokun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented a new feature that overhauls the application's primary navigation and layout by integrating the daisyUI component library. This involved a substantial refactoring effort, replacing a suite of custom-built UI components with daisyUI's pre-styled and functional alternatives, particularly for the sidebar and main application shell. The goal is to simplify UI development, improve consistency, and reduce the overall maintenance burden of custom UI elements. Additionally, I've updated several core dependencies to their latest versions, ensuring the project benefits from the newest features and stability improvements.
Highlights
- UI Framework Migration: I've undertaken a significant refactor of the application's UI, specifically migrating the custom sidebar and related components to utilize daisyUI's
drawercomponent. This change simplifies the UI codebase by leveraging an existing, robust component library. - Removal of Custom UI Components: Several custom UI components, including
MotionHighlight,DropdownMenu,Sheet,Sidebar,Tooltip,Avatar,Button,Input,Label,Separator, andSkeleton, have been removed. This streamlines the project by reducing custom component maintenance and relying more on external libraries or simpler native elements. - Chat Layout and Sidebar Redesign: The chat layout (
src/app/(chat)/template.tsx) has been completely revamped to integrate the new daisyUI drawer, including a new navigation bar with integrated links and user information. TheChatSidebarcomponent itself has been simplified to only render the chat list. - Dependency Updates: I've updated several
@ai-sdk/*dependencies to their latest versions, including major version bumps for@ai-sdk/anthropicand@ai-sdk/groq. This ensures we are using the most recent features and bug fixes from the AI SDK. - Build Analysis Configuration: I've added
nextBundleAnalysisconfiguration topackage.json, which will help monitor and analyze the size of the Next.js application bundles, aiding in performance optimization.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request is a significant refactoring that replaces a complex, custom sidebar and UI component library with a simpler implementation using daisyUI. This greatly reduces the amount of custom code and dependencies, which is a positive change. However, the new implementation introduces some issues related to semantic HTML, accessibility, and Next.js best practices, such as using <a> tags for internal navigation instead of <Link>, which will cause unnecessary page reloads. I've provided suggestions to address these points.
src/app/(chat)/template.tsx
Outdated
| <ul className="menu menu-horizontal"> | ||
| <Link href={'/settings/models'}> | ||
| <button className={'btn btn-ghost'}> | ||
| <LuBot className="inline-block h-6 w-6" /> | ||
| Models | ||
| </button> | ||
| </Link> | ||
| <Link href={'/settings/files'}> | ||
| <button className={'btn btn-ghost'}> | ||
| <LuFile className="inline-block h-6 w-6" /> | ||
| Files | ||
| </button> | ||
| </Link> | ||
| <Link href={'/settings/shared'}> | ||
| <button className={'btn btn-ghost'}> | ||
| <LuShare className="inline-block h-6 w-6" /> | ||
| Shared chats | ||
| </button> | ||
| </Link> | ||
| <Link href={'/logout'} prefetch={false}> | ||
| <button className={'btn btn-ghost'}> | ||
| <LuLogOut className="inline-block h-6 w-6" /> | ||
| Logout | ||
| </button> | ||
| </Link> | ||
| </ul> |
There was a problem hiding this comment.
The <ul> element has <Link> components as its direct children, which is invalid HTML. <ul> elements should only contain <li> elements as direct children. This can cause issues with accessibility and rendering. Additionally, wrapping a <button> inside a <Link> is an anti-pattern. The <Link> component can be styled directly to look like a button.
<ul className="menu menu-horizontal">
<li>
<Link href={'/settings/models'} className={'btn btn-ghost'}>
<LuBot className="inline-block h-6 w-6" />
Models
</Link>
</li>
<li>
<Link href={'/settings/files'} className={'btn btn-ghost'}>
<LuFile className="inline-block h-6 w-6" />
Files
</Link>
</li>
<li>
<Link href={'/settings/shared'} className={'btn btn-ghost'}>
<LuShare className="inline-block h-6 w-6" />
Shared chats
</Link>
</li>
<li>
<Link href={'/logout'} prefetch={false} className={'btn btn-ghost'}>
<LuLogOut className="inline-block h-6 w-6" />
Logout
</Link>
</li>
</ul>
src/components/sidebar.tsx
Outdated
| <ul> | ||
| {data.chats?.map((chat) => ( | ||
| <li key={chat.id}> | ||
| <a href={`/chat/${chat.id}`}>{chat.name}</a> |
There was a problem hiding this comment.
You are using a standard <a> tag for navigation within your Next.js application. This will trigger a full page reload every time a user clicks on a chat, which is inefficient and provides a poor user experience in a single-page application. You should use the Link component from next/link for client-side navigation. Remember to add import Link from 'next/link'; at the top of the file.
<Link href={`/chat/${chat.id}`}>{chat.name}</Link>
src/app/(chat)/template.tsx
Outdated
| <a className="flex space-x-3 font-bold text-xl normal-case"> | ||
| <img | ||
| alt="MikanDev Logo" | ||
| className="mr-3 h-8 w-auto" | ||
| src={KawaiiLogo.src} | ||
| /> | ||
| Chat | ||
| </a> |
There was a problem hiding this comment.
The <a> tag is used here but doesn't have an href attribute, making it non-functional as a link and semantically incorrect. Since this element appears to be a title/branding element and not a link, using a <div> would be more appropriate for semantic HTML.
<div className="flex space-x-3 font-bold text-xl normal-case">
<img
alt="MikanDev Logo"
className="mr-3 h-8 w-auto"
src={KawaiiLogo.src}
/>
Chat
</div>
src/app/(chat)/template.tsx
Outdated
| <img | ||
| alt={data.user.name} | ||
| src={data.user.avatar || '/default-avatar.png'} | ||
| /> |
There was a problem hiding this comment.
The <img> tag for the user avatar is missing width and height attributes. This can lead to Cumulative Layout Shift (CLS) as the browser has to reflow the page once the image is loaded. It's a best practice to always include these attributes to improve performance and user experience.
<img
alt={data.user.name}
src={data.user.avatar || '/default-avatar.png'}
width={48}
height={48}
/>![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/app/(chat)/template.tsx (1)
33-43: Race condition on initial user creation — use upsertTwo concurrent requests can both pass
findUniqueand race oncreate, causing a unique constraint error. Useupsertorcreatewith conflict handling.Example:
await prisma.user.upsert({ where: { id: claims!.sub }, create: { id: claims!.sub }, update: {}, });
♻️ Duplicate comments (4)
src/app/(chat)/template.tsx (3)
80-88: Brand element should not be an anchor without hrefThis is a non-link brand/title. Use a non-interactive element.
- <a className="flex space-x-3 font-bold text-xl normal-case"> + <div className="flex space-x-3 font-bold text-xl normal-case"> <img alt="MikanDev Logo" className="mr-3 h-8 w-auto" src={KawaiiLogo.src} /> Chat - </a> + </div>
90-115: Fix invalidstructure and button-in-link anti-pattern
must contain
- as direct children; avoid wrapping inside . Style the directly.
- <ul className="menu menu-horizontal"> - <Link href={'/settings/models'}> - <button className={'btn btn-ghost'}> - <LuBot className="inline-block h-6 w-6" /> - Models - </button> - </Link> - <Link href={'/settings/files'}> - <button className={'btn btn-ghost'}> - <LuFile className="inline-block h-6 w-6" /> - Files - </button> - </Link> - <Link href={'/settings/shared'}> - <button className={'btn btn-ghost'}> - <LuShare className="inline-block h-6 w-6" /> - Shared chats - </button> - </Link> - <Link href={'/logout'} prefetch={false}> - <button className={'btn btn-ghost'}> - <LuLogOut className="inline-block h-6 w-6" /> - Logout - </button> - </Link> - </ul> + <ul className="menu menu-horizontal"> + <li> + <Link href="/settings/models" className="btn btn-ghost"> + <LuBot className="inline-block h-6 w-6" /> + <span className="ml-2">Models</span> + </Link> + </li> + <li> + <Link href="/settings/files" className="btn btn-ghost"> + <LuFile className="inline-block h-6 w-6" /> + <span className="ml-2">Files</span> + </Link> + </li> + <li> + <Link href="/settings/shared" className="btn btn-ghost"> + <LuShare className="inline-block h-6 w-6" /> + <span className="ml-2">Shared chats</span> + </Link> + </li> + <li> + <Link href="/logout" prefetch={false} className="btn btn-ghost"> + <LuLogOut className="inline-block h-6 w-6" /> + <span className="ml-2">Logout</span> + </Link> + </li> + </ul>
133-136: Add image dimensions (or use next/image) to prevent CLSThe avatar
<img>lacks width/height. Add them or switch tonext/imagefor optimisation. Also ensure remote domains are configured if using external avatars.- <img + <img alt={data.user.name} src={data.user.avatar || '/default-avatar.png'} + width={48} + height={48} />Alternatively:
import Image from 'next/image'; <Image alt={data.user.name} src={data.user.avatar || '/default-avatar.png'} width={48} height={48} className="rounded-full" />src/components/sidebar.tsx (1)
25-26: Use Next.js Link for client-side navigationUsing
<a>will trigger full page reloads. Usenext/linkand style it directly.- <li key={chat.id}> - <a href={`/chat/${chat.id}`}>{chat.name}</a> + <li key={chat.id}> + <Link href={`/chat/${chat.id}`}>{chat.name}</Link> </li>Add the import:
import Link from 'next/link';
🧹 Nitpick comments (6)
renovate.json (1)
3-3: Cosmetic reformat; no functional impact — LGTMSingle-line array is equivalent to the previous multi-line form. If the repo enforces JSON formatting via a formatter, ensure this aligns to avoid churn in future diffs.
package.json (1)
13-20: Surface and sanity-check all imports from the bumped @AI-SDK packagesI’ve listed every import and core API call for the upgraded packages—major bumps landed on:
- @ai-sdk/[email protected] (createAnthropic)
- @ai-sdk/[email protected] (createGroq)
- @ai-sdk/[email protected] (useChat)
All import paths and function names remain unchanged in:
- src/components/chatContainer.tsx (useChat)
- src/components/chatInput.tsx (startChat)
- src/actions/image.ts (openai, xai, generateImage)
- src/app/api/chat/route.ts (createAnthropic, createGoogleGenerativeAI, createGroq, createOpenAI, createXai, streamText)
No missing or renamed exports detected, but because these are new major releases, please manually confirm:
- The signature and options for useChat still match v2.0.9
- Provider-factory calls (e.g. createAnthropic, createGroq) conform to their updated APIs
- Any breaking changes in React hooks or client factory args
.github/workflows/nextjs_bundle_analysis.yml (2)
1-2: Licence header introduces MPL-2.0 into the repo – verify compatibility
This file carries a HashiCorp copyright and MPL-2.0 SPDX tag. Ensure your project’s licence allows inclusion of MPL-2.0 files, and that attribution/notice requirements are met.If incompatible, replace with your project’s licence header or add proper NOTICE/ATTRIBUTION per MPL guidelines. I can prepare an updated header if you confirm the repo’s licence.
62-69: Handle missing base branch artifact more gracefullyIf the base artifact doesn’t exist (for example in a new repo or after expiry), the download step will error and the compare step will be skipped or fail. To avoid this:
- Explicitly target your artifact by name
- Use the
if_no_artifact_found: warn(orif-no-artifact-found: warn) input to prevent failure when it’s absent- Wrap the compare step in a guard that checks for the presence of the base stats file
Proposed diff in
.github/workflows/nextjs_bundle_analysis.yml(around lines 62–69):- name: Download base branch bundle stats - uses: dawidd6/action-download-artifact@v2 + uses: dawidd6/action-download-artifact@v2 # or bump to a valid later tag if: success() && github.event.number with: workflow: nextjs_bundle_analysis.yml branch: ${{ github.event.pull_request.base.ref }} path: .next/analyze/base + name: bundle # adjust to your artifact’s name + if_no_artifact_found: warn # or if-no-artifact-found: warnAnd wrap the compare step (around lines 84–86):
- name: Compare with base branch bundle - if: success() && github.event.number - run: ls -laR .next/analyze/base && bunx -p nextjs-bundle-analysis compare + if: success() && github.event.number + run: | + ls -laR .next/analyze/base || true + if [ -f ".next/analyze/base/__bundle_analysis.json" ]; then + bunx -p nextjs-bundle-analysis compare + else + echo "No base bundle stats found; skipping compare." + fiPlease confirm:
- the correct action tag (v2 vs. any newer release)
- the exact input key syntax (
if_no_artifact_foundorif-no-artifact-found)- the JSON stats filename and path you’re using
src/app/(chat)/template.tsx (1)
45-50: Query only required fields and consider a capYou only use
idandname, but fetch entire rows. Reduce payload and optionally cap the list.const chats = await prisma.chat.findMany({ where: { userId: claims!.sub }, orderBy: { createdAt: 'desc' }, select: { id: true, name: true }, // take: 100, // optional });src/components/sidebar.tsx (1)
22-28: Align with DaisyUI menu semantics and handle empty stateAdd
menuclasses to the list and show a small fallback when there are no chats.- <ul> - {data.chats?.map((chat) => ( + <ul className="menu menu-vertical"> + {data.chats?.length ? data.chats.map((chat) => ( <li key={chat.id}> - <a href={`/chat/${chat.id}`}>{chat.name}</a> + <Link href={`/chat/${chat.id}`} className="truncate">{chat.name}</Link> </li> - ))} + )) : ( + <li className="opacity-60">No chats yet</li> + )} </ul>Note: ensure
import Link from 'next/link'is added at the top.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.github/workflows/nextjs_bundle_analysis.yml(1 hunks)package.json(4 hunks)renovate.json(1 hunks)src/app/(chat)/template.tsx(2 hunks)src/app/(public)/template.tsx(1 hunks)src/app/(settings)/template.tsx(0 hunks)src/components/animate-ui/effects/motion-highlight.tsx(0 hunks)src/components/animate-ui/radix/dropdown-menu.tsx(0 hunks)src/components/animate-ui/radix/sheet.tsx(0 hunks)src/components/animate-ui/radix/sidebar.tsx(0 hunks)src/components/animate-ui/radix/tooltip.tsx(0 hunks)src/components/sidebar.tsx(1 hunks)src/components/ui/avatar.tsx(0 hunks)src/components/ui/button.tsx(0 hunks)src/components/ui/input.tsx(0 hunks)src/components/ui/label.tsx(0 hunks)src/components/ui/separator.tsx(0 hunks)src/components/ui/skeleton.tsx(0 hunks)
💤 Files with no reviewable changes (12)
- src/components/ui/skeleton.tsx
- src/components/ui/avatar.tsx
- src/components/ui/label.tsx
- src/components/ui/input.tsx
- src/components/ui/button.tsx
- src/components/ui/separator.tsx
- src/components/animate-ui/radix/tooltip.tsx
- src/components/animate-ui/radix/sheet.tsx
- src/app/(settings)/template.tsx
- src/components/animate-ui/effects/motion-highlight.tsx
- src/components/animate-ui/radix/dropdown-menu.tsx
- src/components/animate-ui/radix/sidebar.tsx
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/nextjs_bundle_analysis.yml
27-27: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
36-36: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
63-63: the runner of "dawidd6/action-download-artifact@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
src/app/(public)/template.tsx (1)
8-8: Sidebar provider removal validated – no context consumers remainI’ve searched for any
SidebarProvider,SidebarInset,SidebarTrigger, oruseSidebarusages and found none. The only remaining “sidebar” reference is theChatSidebarcomponent (src/components/sidebar.tsx), which purely renders passed-in data and does not consume any context or hooks. Removal of the old sidebar providers will not break any descendants.
| analyze: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 |
There was a problem hiding this comment.
Update GitHub Actions to supported major versions (actionlint)
The workflow uses older action majors flagged by actionlint. Upgrade to current majors compatible with hosted runners.
Apply:
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4
...
- - name: Restore next build
- uses: actions/cache@v3
+ - name: Restore next build
+ uses: actions/cache@v4
...
- - name: Download base branch bundle stats
- uses: dawidd6/action-download-artifact@v2
+ - name: Download base branch bundle stats
+ uses: dawidd6/action-download-artifact@v6Also applies to: 36-36, 63-63
🧰 Tools
🪛 actionlint (1.7.7)
27-27: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/nextjs_bundle_analysis.yml at lines 27, 36, and 63, the
GitHub Actions use older major versions flagged by actionlint. Update the action
versions to their latest supported major versions compatible with hosted runners
by changing the version tags (e.g., from v2 to v3 or the latest stable major)
for all actions used at these lines.
| - name: Install dependencies | ||
| run: bun install | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make dependency installation reproducible in CI
Use Bun’s lockfile-enforced mode to avoid drift across runs.
- - name: Install dependencies
- run: bun install
+ - name: Install dependencies
+ run: bun install --frozen-lockfile --no-progressOptional: cache Bun’s global package data to speed up installs. I can add that if desired.
📝 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.
| - name: Install dependencies | |
| run: bun install | |
| - name: Install dependencies | |
| run: bun install --frozen-lockfile --no-progress |
🤖 Prompt for AI Agents
In .github/workflows/nextjs_bundle_analysis.yml at lines 32 to 34, the
dependency installation step uses "bun install" without enforcing the lockfile,
which can cause non-reproducible installs. Modify the command to "bun install
--frozen-lockfile" to ensure the lockfile is strictly respected during
installation. Optionally, consider adding caching for Bun’s global package data
to speed up subsequent installs.
| - name: Restore next build | ||
| uses: actions/cache@v3 | ||
| id: restore-build-cache | ||
| env: | ||
| cache-name: cache-next-build | ||
| with: | ||
| # if you use a custom build directory, replace all instances of `.next` in this file with your build directory | ||
| # ex: if your app builds to `dist`, replace `.next` with `dist` | ||
| path: .next/cache | ||
| # change this if you prefer a more strict cache | ||
| key: ${{ runner.os }}-build-${{ env.cache-name }} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden Next.js build cache key to prevent stale caches
Current key is constant, risking incorrect cache hits. Include lockfile hash and os.
- name: Restore next build
uses: actions/cache@v4
id: restore-build-cache
env:
cache-name: cache-next-build
with:
# if you use a custom build directory, replace all instances of `.next` in this file with your build directory
# ex: if your app builds to `dist`, replace `.next` with `dist`
path: .next/cache
# change this if you prefer a more strict cache
- key: ${{ runner.os }}-build-${{ env.cache-name }}
+ key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/bun.lockb', '**/package-lock.json', '**/pnpm-lock.yaml', '**/yarn.lock') }}
+ restore-keys: |
+ ${{ runner.os }}-${{ env.cache-name }}-📝 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.
| - name: Restore next build | |
| uses: actions/cache@v3 | |
| id: restore-build-cache | |
| env: | |
| cache-name: cache-next-build | |
| with: | |
| # if you use a custom build directory, replace all instances of `.next` in this file with your build directory | |
| # ex: if your app builds to `dist`, replace `.next` with `dist` | |
| path: .next/cache | |
| # change this if you prefer a more strict cache | |
| key: ${{ runner.os }}-build-${{ env.cache-name }} | |
| - name: Restore next build | |
| uses: actions/cache@v4 | |
| id: restore-build-cache | |
| env: | |
| cache-name: cache-next-build | |
| with: | |
| # if you use a custom build directory, replace all instances of `.next` in this file with your build directory | |
| # ex: if your app builds to `dist`, replace `.next` with `dist` | |
| path: .next/cache | |
| # change this if you prefer a more strict cache | |
| key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/bun.lockb', '**/package-lock.json', '**/pnpm-lock.yaml', '**/yarn.lock') }} | |
| restore-keys: | | |
| ${{ runner.os }}-${{ env.cache-name }}- |
🧰 Tools
🪛 actionlint (1.7.7)
36-36: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/nextjs_bundle_analysis.yml around lines 35 to 46, the cache
key for restoring the Next.js build is too generic and can cause stale cache
hits. Update the cache key to include a hash of the lockfile (e.g.,
package-lock.json or yarn.lock) and the runner OS to ensure the cache is
invalidated when dependencies change. This will make the cache key unique per
dependency state and OS, preventing incorrect cache usage.
| run: bunx -p nextjs-bundle-analysis report | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin nextjs-bundle-analysis tool version for deterministic results
Ephemeral bunx pulls latest, which can change behaviour. Pin the version (or add as devDependency).
- run: bunx -p nextjs-bundle-analysis report
+ run: bunx -p [email protected] reportSame for compare below.
📝 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.
| run: bunx -p nextjs-bundle-analysis report | |
| run: bunx -p nextjs-bundle-analysis@0.7.0 report |
🤖 Prompt for AI Agents
In .github/workflows/nextjs_bundle_analysis.yml at lines 54-55, the command uses
bunx to run nextjs-bundle-analysis without specifying a version, causing
non-deterministic results due to ephemeral latest pulls. Fix this by pinning the
nextjs-bundle-analysis tool version explicitly in the command or by adding it as
a devDependency in package.json and referencing that fixed version in the
workflow. Apply the same version pinning approach to the compare command
mentioned below in the file.
| "nextBundleAnalysis": { | ||
| "budget": null, | ||
| "budgetPercentIncreaseRed": 20, | ||
| "minimumChangeThreshold": 0, | ||
| "showDetails": true | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm nextjs-bundle-analysis config keys; avoid budget: null if unsupported
Ensure the tool reads these exact keys from package.json. If budget is optional, omit it rather than setting to null to prevent parsing issues.
Please check the tool’s expected package.json schema:
If budget should be omitted, apply:
"nextBundleAnalysis": {
- "budget": null,
"budgetPercentIncreaseRed": 20,
"minimumChangeThreshold": 0,
"showDetails": true
}🌐 Web query:
What configuration keys does "nextjs-bundle-analysis" read from package.json, and are "budgetPercentIncreaseRed", "minimumChangeThreshold", and "showDetails" valid? Should "budget" be omitted or set to null?
💡 Result:
The "nextjs-bundle-analysis" GitHub Action reads configuration from the "nextBundleAnalysis" key in your project's package.json[1]. It accepts only specific configuration keys, and their validity is determined by the documented options. Based on available documentation, here are the answers to your key questions:
Supported package.json Configuration Keys
The action specifically mentions the following keys under "nextBundleAnalysis"[1]:
- showDetails (boolean): Valid; controls whether detailed sections are rendered in the bundle analysis comment (default:
true)[1]. - buildOutputDirectory (string): Valid; for specifying the custom Next.js build output directory. Default is
.next[1].
Validity of Specific Keys
| Key | Valid | Notes |
|---|---|---|
| showDetails | Yes | Directly documented as an option controlling UI detail[1]. |
| budgetPercentIncreaseRed | No | Not mentioned or documented as a supported key[1]. |
| minimumChangeThreshold | No | Not mentioned or documented as a supported key[1]. |
No current documentation supports budgetPercentIncreaseRed or minimumChangeThreshold as valid configuration keys for "nextjs-bundle-analysis" in package.json[1]. If these are present, they may be ignored or unsupported by the tool.
Handling the budget Key
The documentation refers only to performance budgets in general but does not specify a budget property in the package.json configuration section[1]. The workflow might contain details relating to budgets, but there is no mention that setting "budget": null in package.json is required or supported. If a budget key is present without clear documentation, it's safest to omit it rather than set it to null[1].
Summary
- You should only include well-documented keys (
showDetails,buildOutputDirectory) in"nextBundleAnalysis"withinpackage.json[1]. - The keys
budgetPercentIncreaseRed,minimumChangeThreshold, and a plainbudgetkey are not valid according to current GitHub and npm documentation[1]. They should be omitted unless updated documentation specifically adds support. - Use
"showDetails"for controlling display detail, and"buildOutputDirectory"to set your build output location[1].
If you need advanced budgeting features, you may need to configure these within the GitHub workflow file itself, not in the package.json[1].
Citations:
- 1: https://github.com/hashicorp/nextjs-bundle-analysis
- 2: https://www.npmjs.com/package/@next/bundle-analyzer
- 3: http://catchmetrics.io/blog/reducing-nextjs-bundle-size-with-nextjs-bundle-analyzer
- 4: https://mattyasul.com/blog/nextjs-bundle-management/
- 5: https://nextjs.org/docs/pages/guides/package-bundling
- 6: https://www.coteries.com/en/articles/reduce-size-nextjs-bundle
- 7: https://blog.logrocket.com/how-to-analyze-next-js-app-bundles/
- 8: https://nextjs.org/docs/14/pages/building-your-application/optimizing/bundle-analyzer
- 9: https://www.youtube.com/watch?v=bKifBeIgdL4
- 10: https://flaviocopes.com/nextjs-analyze-app-bundle/
Update unsupported nextBundleAnalysis keys in package.json
Only the documented options (showDetails and buildOutputDirectory) are read from nextBundleAnalysis. Remove the unsupported keys (budget, budgetPercentIncreaseRed, minimumChangeThreshold) to avoid them being ignored or causing parsing issues.
• In package.json under "nextBundleAnalysis":
– Keep: showDetails (boolean)
– Optionally add: buildOutputDirectory (string, default: .next)
– Remove: budget, budgetPercentIncreaseRed, minimumChangeThreshold
Suggested diff:
"nextBundleAnalysis": {
- "budget": null,
- "budgetPercentIncreaseRed": 20,
- "minimumChangeThreshold": 0,
"showDetails": true
}If you require performance budgets, configure them within your GitHub workflow rather than in the package.json.
📝 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.
| "nextBundleAnalysis": { | |
| "budget": null, | |
| "budgetPercentIncreaseRed": 20, | |
| "minimumChangeThreshold": 0, | |
| "showDetails": true | |
| } | |
| "nextBundleAnalysis": { | |
| "showDetails": true | |
| } |
🤖 Prompt for AI Agents
In package.json lines 63 to 68, the nextBundleAnalysis section contains
unsupported keys budget, budgetPercentIncreaseRed, and minimumChangeThreshold
that should be removed because only showDetails and buildOutputDirectory are
valid. Remove these unsupported keys and optionally add buildOutputDirectory if
needed, keeping showDetails as is. This will prevent parsing issues and ensure
only recognized options are present.
src/app/(chat)/template.tsx
Outdated
| </div> | ||
| {children} | ||
| </div> | ||
| <div className="drawer-side z-60"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invalid Tailwind class z-60
Tailwind defaults don’t include z-60. Use z-50 or an arbitrary value z-[60].
- <div className="drawer-side z-60">
+ <div className="drawer-side z-50">📝 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.
| <div className="drawer-side z-60"> | |
| <div className="drawer-side z-50"> |
🤖 Prompt for AI Agents
In src/app/(chat)/template.tsx at line 120, the Tailwind class `z-60` is invalid
because Tailwind's default scale does not include `z-60`. Replace `z-60` with
either the closest valid default class `z-50` or use an arbitrary value syntax
like `z-[60]` to apply the desired z-index.
src/app/(chat)/template.tsx
Outdated
| <ul className="menu flex min-h-screen w-80 flex-col bg-base-200 p-4"> | ||
| <ChatSidebar data={data} /> | ||
| <div className="card mt-auto shadow-xl"> | ||
| <div className="card-body"> | ||
| <div className="flex items-center"> | ||
| <div className="avatar"> | ||
| <div className="w-12 rounded-full"> | ||
| <img | ||
| alt={data.user.name} | ||
| src={data.user.avatar || '/default-avatar.png'} | ||
| /> | ||
| </div> | ||
| </div> | ||
| <div className="ml-3"> | ||
| <h2 className="card-title">{data.user.name}</h2> | ||
| <p className="text-gray-500 text-sm">UID {data.user.id}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </ul> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invalid
- children: ChatSidebar renders a
-
Your outer
<ul>has a direct child<ChatSidebar/>which renders a<div>. That’s invalid HTML and breaks accessibility. Wrap with a non-list container and let ChatSidebar own its list.- <ul className="menu flex min-h-screen w-80 flex-col bg-base-200 p-4"> - <ChatSidebar data={data} /> - <div className="card mt-auto shadow-xl"> + <div className="flex min-h-screen w-80 flex-col bg-base-200 p-4"> + <ChatSidebar data={data} /> + <div className="card mt-auto shadow-xl"> <div className="card-body"> <div className="flex items-center"> <div className="avatar"> <div className="w-12 rounded-full"> <img alt={data.user.name} src={data.user.avatar || '/default-avatar.png'} /> </div> </div> <div className="ml-3"> <h2 className="card-title">{data.user.name}</h2> <p className="text-gray-500 text-sm">UID {data.user.id}</p> </div> </div> </div> </div> - </ul> + </div>
📝 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.Suggested change<ul className="menu flex min-h-screen w-80 flex-col bg-base-200 p-4"> <ChatSidebar data={data} /> <div className="card mt-auto shadow-xl"> <div className="card-body"> <div className="flex items-center"> <div className="avatar"> <div className="w-12 rounded-full"> <img alt={data.user.name} src={data.user.avatar || '/default-avatar.png'} /> </div> </div> <div className="ml-3"> <h2 className="card-title">{data.user.name}</h2> <p className="text-gray-500 text-sm">UID {data.user.id}</p> </div> </div> </div> </div> </ul> </div> <div className="flex min-h-screen w-80 flex-col bg-base-200 p-4"> <ChatSidebar data={data} /> <div className="card mt-auto shadow-xl"> <div className="card-body"> <div className="flex items-center"> <div className="avatar"> <div className="w-12 rounded-full"> <img alt={data.user.name} src={data.user.avatar || '/default-avatar.png'} /> </div> </div> <div className="ml-3"> <h2 className="card-title">{data.user.name}</h2> <p className="text-gray-500 text-sm">UID {data.user.id}</p> </div> </div> </div> </div> </div> </div> 🤖 Prompt for AI Agents
In src/app/(chat)/template.tsx around lines 126 to 147, the <ul> element has a direct child <ChatSidebar> component that renders a <div>, which is invalid HTML because <ul> should only have <li> children. To fix this, replace the outer <ul> with a non-list container element like a <div> or <nav> that can contain <ChatSidebar> and other div elements, and ensure that ChatSidebar manages its own list structure internally.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (12)
src/components/header.tsx (3)
373-401: Avoid rendering “0” when there are no buttonsUsing
buttons?.length && (…)will render0when the array is empty. Guard with an explicit length check.- {buttons?.length && ( + {(buttons?.length ?? 0) > 0 && ( <motion.div className="mt-auto flex flex-col gap-2" variants={mobileMenuButtonsVariants} > {buttons?.map((buttonProps) => { return ( - <button - key={buttonProps.title} - {...buttonProps} - className={'btn btn-neutral'} - > - {buttonProps.href ? ( - <Link - href={buttonProps.href} - target={buttonProps.target ?? '_self'} - > - {buttonProps.title} - </Link> - ) : ( - buttonProps.title - )} - </button> + // see next comment for improved link/button semantics + <></> ); })} </motion.div> )}Follow-up: see the next comment for fixing the actual button/link rendering.
380-395: Fix invalid button/link semantics and stray propsCurrently a
<button>receiveshref/targetvia spread and wraps a<Link>, which is invalid HTML and can cause confusing behaviour. Render a Link (anchor) whenhrefexists; otherwise render a button.- <button - key={buttonProps.title} - {...buttonProps} - className={'btn btn-neutral'} - > - {buttonProps.href ? ( - <Link - href={buttonProps.href} - target={buttonProps.target ?? '_self'} - > - {buttonProps.title} - </Link> - ) : ( - buttonProps.title - )} - </button> + <> + {buttonProps.href ? ( + <Link + key={buttonProps.title} + href={buttonProps.href} + target={buttonProps.target ?? '_self'} + rel={ + (buttonProps.target ?? '_self') === '_blank' + ? 'noopener noreferrer' + : undefined + } + className="btn btn-neutral" + > + {buttonProps.title} + </Link> + ) : ( + <button key={buttonProps.title} className="btn btn-neutral"> + {buttonProps.title} + </button> + )} + </>
450-466: Apply the same link/button semantics fix on desktop buttonsMirror the mobile fix: render a Link (anchor) with button styling when
hrefexists; otherwise a real button. Also addrel="noopener noreferrer"for_blank.- <button - key={buttonProps.title} - {...buttonProps} - className={'btn btn-secondary'} - > - {buttonProps.href ? ( - <Link - href={buttonProps.href} - target={buttonProps.target ?? '_self'} - > - {buttonProps.title} - </Link> - ) : ( - buttonProps.title - )} - </button> + <> + {buttonProps.href ? ( + <Link + key={buttonProps.title} + href={buttonProps.href} + target={buttonProps.target ?? '_self'} + rel={ + (buttonProps.target ?? '_self') === '_blank' + ? 'noopener noreferrer' + : undefined + } + className="btn btn-secondary" + > + {buttonProps.title} + </Link> + ) : ( + <button key={buttonProps.title} className="btn btn-secondary"> + {buttonProps.title} + </button> + )} + </>src/actions/ollama.ts (1)
14-21: SSRF risk and missing timeout when validating user-provided host
fetch(${host}/v1/models)uses a user-supplied URL without validation and with no timeout. This enables server-side request forgery (e.g. hitting internal services like 169.254.169.254) and can hang threads.Add protocol/host validation, reject localhost/private networks, and apply a timeout:
try { - const response = await fetch(`${host}/v1/models`); + const url = new URL(host); + if (!['http:', 'https:'].includes(url.protocol)) { + throw new Error('Invalid protocol for Ollama host'); + } + // Basic guard against local/private targets + const hostname = url.hostname.toLowerCase(); + const privateV4 = + /^(10\.\d{1,3}\.\d{1,3}\.\d{1,3}|192\.168\.\d{1,3}\.\d{1,3}|172\.(1[6-9]|2\d|3[0-1])\.\d{1,3}\.\d{1,3})$/.test( + hostname + ) || ['localhost', '127.0.0.1', '0.0.0.0'].includes(hostname); + const loopbackV6 = hostname === '::1'; + if (privateV4 || loopbackV6) { + throw new Error('Forbidden Ollama host'); + } + const ac = new AbortController(); + const t = setTimeout(() => ac.abort(), 7000); + const response = await fetch(`${url.origin}/v1/models`, { signal: ac.signal }); + clearTimeout(t); if (!response.ok) { throw new Error('Invalid Ollama host'); }Optionally, consider DNS resolution checks against private ranges for a stronger guarantee.
src/app/api/chat/route.ts (3)
89-93: Guard against malformed Ollama modelIdSplitting by
' - 'without validation can yieldundefinedparts and 500s later. Validate and return 400 on bad input.- const endpoint = modelId.split(' - ')[0]; - const modelName = modelId.split(' - ')[1]; - const provider = createOllama({ baseURL: `${endpoint}/api` }); + const [endpoint, modelName] = modelId.split(' - '); + if (!endpoint || !modelName) { + return new Response( + 'Invalid Ollama model id format. Expected "<endpoint> - <modelName>".', + { status: 400 } + ); + } + const provider = createOllama({ baseURL: `${endpoint}/api` });
103-115: Ensure stream record cleanup even if addMessage fails
prisma.stream.deleteManyis only called afteraddMessage. If that throws, stream rows linger. Usetry/finally.- onFinish: async (message) => { - await addMessage({ - message: { - content: message.text, - role: 'assistant', - }, - id, - }); - await prisma.stream.deleteMany({ - where: { streamId }, - }); - }, + onFinish: async (message) => { + try { + await addMessage({ + message: { + content: message.text, + role: 'assistant', + }, + id, + }); + } finally { + await prisma.stream.deleteMany({ + where: { streamId }, + }); + } + },Apply similarly in the other two blocks above.
Also applies to: 144-156, 189-200
206-209: Avoid leaking internal error details to clientsReturning
error.messagecan expose internals. Prefer a generic error response in production.- JSON.stringify({ - error: 'An error occurred while processing your request.', - details: error instanceof Error ? error.message : 'Unknown error', - }), + JSON.stringify({ + error: 'An error occurred while processing your request.', + }),src/components/chat.tsx (1)
31-39: Regression: chat no longer auto-scrolls on new messagesChanging deps to
[]runs the scroll only on mount. Users won’t see later messages without manual scrolling.- useEffect(() => { + useEffect(() => { if (bottomRef.current) { bottomRef.current.scrollIntoView({ behavior: 'smooth', block: 'end', }); } - }, []); + }, [msg.length, status]);This triggers on new messages and status transitions, without over-firing.
src/components/chatContainer.tsx (1)
83-88: Align addMessage call to its defined signatureThe
addMessageaction expects an object with:
message: { content: string; role: 'user' | 'assistant' | 'system' | 'data'; attachment?: string }id: string- optional top-level
attachment?: stringInstead of suppressing the error, reshape your payload:
// src/components/chatContainer.tsx ~83–88 await addMessage({ message: { content: messageData.content, role: messageData.role, // only include if defined ...(messageData.attachment && { attachment: messageData.attachment }), }, id: id!, // ensure id is non-empty attachment: messageData.attachment, // optional top-level field });This removes the need for
// @ts-expect-errorand aligns exactly with theaddMessagetype.src/components/chatInput.tsx (3)
171-181: Invalid Tailwind classjustify-leftTailwind uses
justify-start, notjustify-left. This will be ignored by the compiler.- <div className={'card-body justify-left w-full p-3'}> + <div className="card-body justify-start w-full p-3"> … - <div - className={'card-body justify-left w-full p-3'} - > + <div className="card-body justify-start w-full p-3"> … - <div className="card-body justify-left w-full p-3"> + <div className="card-body justify-start w-full p-3">Also applies to: 179-181, 232-236, 304-306
191-196: Invalid Tailwind classtext-md
text-mdisn’t part of Tailwind’s default scale. Usetext-base/text-smor an arbitrary value liketext-[15px].- 'font-semibold text-base-content text-md' + 'font-semibold text-base-content text-base' … - 'font-semibold text-base-content text-md' + 'font-semibold text-base-content text-base' … - <span className="font-semibold text-base-content text-md"> + <span className="font-semibold text-base-content text-base">Also applies to: 240-245, 306-310
364-377: Make the close “X” accessibleWrap the icon in a button with an accessible name and size the hit area.
- <X onClick={() => setAttachment(null)} /> + <button + type="button" + className="btn btn-ghost btn-sm" + aria-label="Remove attachment" + onClick={() => setAttachment(null)} + > + <X /> + </button>
♻️ Duplicate comments (6)
src/components/sidebar.tsx (1)
20-23: Use Next.js Link for client-side navigationAnchor tags trigger full reloads; use
next/linkfor SPA navigation.- {data.chats?.map((chat) => ( - <li key={chat.id}> - <a href={`/chat/${chat.id}`}>{chat.name}</a> - </li> - ))} + {data.chats?.map((chat) => ( + <li key={chat.id}> + <Link href={`/chat/${chat.id}`}>{chat.name}</Link> + </li> + ))}Add outside this hunk:
import Link from 'next/link';src/app/(chat)/template.tsx (5)
68-76: Branding element should not be an anchor without hrefUse a semantic container or link to home. Prior comment noted this.
- <div className="mx-2 flex-1 px-2"> - <a className="flex space-x-3 font-bold text-xl normal-case"> + <div className="mx-2 flex-1 px-2"> + <Link href="/" className="flex space-x-3 font-bold text-xl normal-case"> <img alt="MikanDev Logo" className="mr-3 h-8 w-auto" src={KawaiiLogo.src} /> Chat - </a> + </Link> </div>
78-104: Invalidchildren and nested button inside Link
<ul>must contain<li>. Avoid wrapping<button>inside<Link>. Apply button classes to<Link>directly. This was raised before.- <div className="hidden flex-none lg:block"> - <ul className="menu menu-horizontal"> - <Link href={'/settings/models'}> - <button className={'btn btn-ghost'}> + <div className="hidden flex-none lg:block"> + <ul className="menu menu-horizontal"> + <li> + <Link href="/settings/models" className="btn btn-ghost"> <LuBot className="inline-block h-6 w-6" /> Models - </button> - </Link> - <Link href={'/settings/files'}> - <button className={'btn btn-ghost'}> + </Link> + </li> + <li> + <Link href="/settings/files" className="btn btn-ghost"> <LuFile className="inline-block h-6 w-6" /> Files - </button> - </Link> - <Link href={'/settings/shared'}> - <button className={'btn btn-ghost'}> + </Link> + </li> + <li> + <Link href="/settings/shared" className="btn btn-ghost"> <LuShare className="inline-block h-6 w-6" /> Shared chats - </button> - </Link> - <Link href={'/logout'} prefetch={false}> - <button className={'btn btn-ghost'}> + </Link> + </li> + <li> + <Link href="/logout" prefetch={false} className="btn btn-ghost"> <LuLogOut className="inline-block h-6 w-6" /> Logout - </button> - </Link> + </Link> + </li> </ul> </div>
108-108: Invalid Tailwind classz-60Tailwind default scale excludes
z-60. Usez-50orz-[60]. Previously raised.- <div className="drawer-side z-60"> + <div className="drawer-side z-50">
114-135: Invalidchildren:
<ChatSidebar/>renders a div and there’s a sibling divThe outer container should be a non-list element. Prior comment covered this.
- <ul className="menu flex min-h-screen w-80 flex-col bg-base-200 p-4"> - <ChatSidebar data={data} /> - <div className="card mt-auto shadow-xl"> + <div className="flex min-h-screen w-80 flex-col bg-base-200 p-4"> + <ChatSidebar data={data} /> + <div className="card mt-auto shadow-xl"> … - </div> - </ul> + </div> + </div>
121-124: Add width/height to avatar image to prevent CLSSpecify intrinsic dimensions or switch to
next/image.- <img - alt={data.user.name} - src={data.user.avatar || '/default-avatar.png'} - /> + <img + alt={data.user.name} + src={data.user.avatar || '/default-avatar.png'} + width={48} + height={48} + loading="eager" + decoding="async" + />
🧹 Nitpick comments (19)
src/components/imageGenerator.tsx (2)
67-71: Disable Generate when no model is selected (and minor a11y)Prevent pointless requests and improve UX by disabling the button unless both prompt and model are set; also surface aria-disabled.
- <button - className={'btn btn-secondary'} - disabled={loading || !prompt} - onClick={handleGenerate} - > + <button + className="btn btn-secondary" + disabled={loading || !(prompt && selectedModel)} + aria-disabled={loading} + onClick={handleGenerate} + >
57-64: Clamp the image count on input to enforce min/maxHTML min/max can be bypassed. Clamp the value in onChange and add step/inputMode for better mobile UX.
- <input - className={'input w-24'} - max={9} - min={1} - onChange={(e) => setCount(Number(e.target.value))} - placeholder="Count" - type="number" - value={count} - /> + <input + className="input w-24" + max={9} + min={1} + step={1} + inputMode="numeric" + aria-label="Image count" + onChange={(e) => { + const v = Number(e.target.value); + const next = Number.isFinite(v) ? Math.min(9, Math.max(1, v)) : 1; + setCount(next); + }} + placeholder="Count" + type="number" + value={count} + />src/actions/image.ts (1)
43-60: Action: confirm XAI client factory name before per-request instantiationI’ve verified that
@ai-sdk/openaiexposes acreateOpenAIfactory for per-request clients—but there’s no definitive documentation for a similarcreateXAIexport in@ai-sdk/xai. Please check the Node.js package’s public API (e.g. innode_modules/@ai-sdk/xai/index.d.tsor its README) for the correct factory function name.Once confirmed, refactor
src/actions/image.ts(lines 43–60) to remove theprocess.env.*_API_KEYmutations and instantiate both providers per-request. For example:case 'openai': { - process.env.OPENAI_API_KEY = apiKey; - return await generateImage({ model: openai.image(model), prompt, n: count }); + const { createOpenAI } = await import('@ai-sdk/openai'); + const openaiClient = createOpenAI({ apiKey }); + return await generateImage({ + model: openaiClient.image(model), + prompt, + n: count, + }); } case 'xai': { - process.env.XAI_API_KEY = apiKey; - return await generateImage({ model: xai.image(model), prompt, n: count }); + // replace with the actual factory export from @ai-sdk/xai + const { createXAI } = await import('@ai-sdk/xai'); + const xaiClient = createXAI({ apiKey }); + return await generateImage({ + model: xaiClient.image(model), + prompt, + n: count, + }); }Please confirm the exact name of the XAI factory (e.g.
createXAIor otherwise) before applying this refactor.src/actions/ollama.ts (1)
28-34: Silent catch: consider routing to your error pipelineIf you’re standardising on no console output, consider forwarding errors to your monitoring sink (e.g. Sentry) to retain observability.
src/app/api/chat/route.ts (3)
64-65: Unnecessary await on headers()
headers()is synchronous in Next.js. Drop theawaitfor clarity.- const headersList = await headers(); + const headersList = headers();
117-121: Consider structured logging instead of silent catchCatching as
_errorand returning 500 without any server-side logging makes incidents hard to debug. Route logs to your observability tool (e.g. Sentry) or at least a server logger. Avoid console logs if they’re intentionally removed elsewhere.
92-99: DRY: factor out stream record creationYou create a
streamIdand insert astreamrow in three places. Consider a helper to reduce duplication and centralise error handling.Example helper (outside this file):
- createStream(prisma, chatId): returns streamId
- deleteStream(prisma, streamId)
This also makes it easier to add metrics/retries later.Also applies to: 134-140, 178-184
src/app/(public)/share/[id]/page.tsx (2)
15-16: Good: underscore unused destructured varRenaming to
_providerKeyclarifies intent. Alternatively, avoid destructuring the key if it’s unused by iteratingObject.values(Models). Both are fine.
152-152: Tailwind class conflict: remove duplicate text sizeFirst
<p>already usestext-xl. The second<p>includes bothtext-smandtext-xl; the latter wins. Droptext-smfor consistency.- <p - className={'ml-2 font-bold text-base-content/70 text-sm text-xl'} - > + <p className={'ml-2 font-bold text-base-content/70 text-xl'}>Also applies to: 163-165
src/components/chatMeta.tsx (3)
112-120: Add aria-labels to icon-only buttonsIcon-only buttons aren’t keyboard reader friendly. Provide labels.
- <button + <button aria-label="Rename chat" className="btn btn-ghost btn-sm ml-2" onClick={() => { setOpen(true); }} > <Edit className="h-4 w-4" /> </button> - <button + <button aria-label="Delete chat" className="btn btn-ghost btn-sm ml-2" onClick={() => { handleDelete(); }} > <Trash className="h-4 w-4" /> </button> - {isShared && ( + {isShared && ( <button + aria-label="Copy share link" className="btn btn-ghost btn-sm ml-2" onClick={() => { navigator.clipboard.writeText( `${window.location.origin}/share/${id}` ); toast.success('Share link copied to clipboard!'); }} > <Link className="h-4 w-4" /> </button> )}Also applies to: 121-127, 129-139
68-78: Prefer router navigation over window.location + timeoutUse Next’s
useRouter().replace('/chat')after delete. Avoid arbitrary delays and full reloads.const handleDelete = async () => { try { await deleteChat(id); toast.success('Chat deleted successfully!'); - setTimeout(() => { - window.location.href = '/chat'; - }, 1000); + // Navigate immediately after showing the toast + router.replace('/chat'); } catch (_error) { toast.error('Failed to delete chat. Please try again.'); } };Additions outside this hunk:
// at top imports import { useRouter } from 'next/navigation'; // inside component (near other state) const router = useRouter();
45-47: Silent catches: consider structured loggingKeeping UI clean is fine, but route errors to your logging/monitoring stack so you have breadcrumbs when things go wrong.
Also applies to: 61-63, 75-77
src/components/sidebar.tsx (1)
17-27: Optional: empty stateConsider a simple empty state when there are no chats to guide users.
Example:
- Render “No chats yet” with a CTA to create a new chat when
data.chats?.length === 0.src/components/chatContainer.tsx (4)
62-66: Call preventDefault before early return to avoid accidental form submitsIf this handler is ever bound while
isPublicis true, default browser submit could fire. Movee.preventDefault()above the guard.- if (isPublic) { - return; - } - e.preventDefault(); + e.preventDefault(); + if (isPublic) { + return; + }
68-71: Avoid shadowinginputfromuseChatRename the local variable to improve readability and prevent confusion with
useChat’sinputprop.- const input = form.querySelector('input') as HTMLInputElement; - const message = input.value.trim(); + const inputEl = form.querySelector('input') as HTMLInputElement; + const message = inputEl.value.trim(); … - if (input) { - input.value = ''; + if (inputEl) { + inputEl.value = '';Also applies to: 97-102
170-187: Use the incremented retry count when constructing the ID
setRetryCountis async; using the staleretryCountcan duplicate IDs. Compute the next count first.- setRetryCount((prev) => prev + 1); - setInternalStatus('ready'); + const next = retryCount + 1; + setRetryCount(next); + setInternalStatus('ready'); … - append({ - id: `retry-${retryCount}-${userMsg.id || Date.now()}`, + append({ + id: `retry-${next}-${userMsg.id || Date.now()}`,
103-105: Consider logging errors to an observer (Sentry, console in dev)Swallowing errors hampers diagnosis. Keep the toast, but also log in dev or send to telemetry.
src/app/(chat)/template.tsx (1)
18-20: Unnecessaryawaitonredirect
redirect()throws and never resolves in Next.js. Dropawait.- if (!claims) { - await redirect('/login'); - } + if (!claims) { + redirect('/login'); + }src/components/chatInput.tsx (1)
137-141: Check Tailwind config for custom fractionsI didn’t find any
tailwind.config.*file in the repo to confirm that a10/13fraction has been added. Please verify whether you’ve extended the theme with that custom width. If not, replace the invalid utility with an arbitrary value or a standard class. For example:- <div className="fixed bottom-5 flex w-full max-w-10/13 items-center justify-center rounded-lg bg-neutral p-4 shadow-lg"> + <div className="fixed bottom-5 flex w-full max-w-[77%] items-center justify-center rounded-lg bg-neutral p-4 shadow-lg">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
next.config.ts(1 hunks)package.json(3 hunks)src/actions/chat.ts(1 hunks)src/actions/image.ts(1 hunks)src/actions/ollama.ts(1 hunks)src/app/(chat)/chat/[id]/page.tsx(1 hunks)src/app/(chat)/chat/page.tsx(1 hunks)src/app/(chat)/image/page.tsx(0 hunks)src/app/(chat)/settings/models/page.tsx(1 hunks)src/app/(chat)/template.tsx(2 hunks)src/app/(public)/share/[id]/page.tsx(4 hunks)src/app/api/chat/route.ts(5 hunks)src/components/OllamaModelList.tsx(5 hunks)src/components/OpenRouterConfigButton.tsx(2 hunks)src/components/OpenRouterModelList.tsx(6 hunks)src/components/chat.tsx(4 hunks)src/components/chatContainer.tsx(3 hunks)src/components/chatInput.tsx(4 hunks)src/components/chatMeta.tsx(4 hunks)src/components/header.tsx(1 hunks)src/components/imageGenerator.tsx(1 hunks)src/components/modelCard.tsx(2 hunks)src/components/sidebar.tsx(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- src/app/(chat)/image/page.tsx
✅ Files skipped from review due to trivial changes (8)
- src/app/(chat)/chat/[id]/page.tsx
- src/app/(chat)/settings/models/page.tsx
- src/components/modelCard.tsx
- tsconfig.json
- src/components/OpenRouterConfigButton.tsx
- src/app/(chat)/chat/page.tsx
- src/components/OpenRouterModelList.tsx
- src/components/OllamaModelList.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/app/api/chat/route.ts (1)
src/lib/prisma.ts (1)
prisma(3-3)
src/app/(chat)/template.tsx (1)
src/components/sidebar.tsx (1)
ChatSidebar(15-29)
🔇 Additional comments (10)
src/components/imageGenerator.tsx (1)
27-29: Catch variable rename is fineRenaming to
_errorclearly signals it’s intentionally unused. No functional change.src/components/header.tsx (1)
88-90: Style-only early-return change looks goodBlock form improves readability; logic unchanged.
src/actions/chat.ts (1)
9-12: LGTM – clearer intent for unused variableRenaming the unused destructured key to
_providerKeyimproves clarity with no functional change.src/actions/image.ts (1)
64-71: LGTM – clearer intent for unused variableThe rename to
_providerKeyis sound; behaviour unchanged.src/actions/ollama.ts (1)
20-22: Style-only guard improvement is OKOptional chaining is cleaner; behaviour unchanged.
src/app/(public)/share/[id]/page.tsx (2)
48-53: Good: simpler public check
!chat?.publicis clearer and correctly handlesnullchat.
89-91: Good: strict equality for userIdUsing
===avoids coercion pitfalls.src/components/chat.tsx (1)
40-41: Readability improvements look goodUsing
.at(-1)and optional chaining keep the code concise and safe.Also applies to: 82-83, 92-94, 263-265
src/components/chatContainer.tsx (1)
149-150: Ensure Array.prototype.at(–1) Is Supported at RuntimeWhile TypeScript’s
"lib": ["dom", "dom.iterable", "esnext"]gives you compile-time typings for.at(), yourtarget: "ES2017"build will emit calls toArray.prototype.atun-downleveled. You should therefore:• Confirm your browsers/environments actually support ES2022’s
Array.prototype.at.
• Add or update a browserslist config (e.g. in package.json or a.browserslistrc) so downstream tooling can warn or polyfill as needed.
• Alternatively, bump your TStargetto"ES2022"(or higher) or explicitly include a polyfill (e.g. via core-js) to cover older runtimes.This applies at lines 149–150, 156–157 and 168–169 in
src/components/chatContainer.tsx.src/components/chatInput.tsx (1)
59-61: LGTM – clearer guard clauseThe explicit block improves readability without changing behaviour.
| eslint: { | ||
| ignoreDuringBuilds: true, | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Scope ESLint suppression; don’t disable it globally
Hard-coding ignoreDuringBuilds: true disables lint checks for all builds, reducing code-quality guardrails. Make it conditional via an environment flag and keep a separate lint job gating PRs.
Apply this diff to only disable ESLint where explicitly intended (e.g., bundle analysis job):
- eslint: {
- ignoreDuringBuilds: true,
- },
+ eslint: {
+ ignoreDuringBuilds: process.env.NEXT_IGNORE_ESLINT === 'true',
+ },Example: enable only in the bundle analysis workflow step/job:
# .github/workflows/nextjs_bundle_analysis.yml (snippet)
jobs:
bundle-analysis:
steps:
- name: Build for analysis
env:
NEXT_IGNORE_ESLINT: 'true'
run: npm run buildOptionally, enforce lint in a dedicated job:
jobs:
lint:
steps:
- run: npm ci
- run: npm run lintVerification script to confirm a separate lint gate exists:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for ESLint usage in workflows..."
fd -t f -H .github/workflows || true
rg -n -S -A2 -B2 "(next lint|eslint)" .github/workflows || echo "No explicit ESLint steps found in workflows."
echo "Checking for a lint script in package.json..."
rg -n -S '"lint"\s*:' package.json || echo "No 'lint' script found in package.json."
echo "If neither a workflow lint step nor a package.json lint script exists, consider adding them to keep linting enforced."Length of output: 1093
Ensure ESLint is only skipped when explicitly requested and enforce linting in CI
We’ve confirmed there’s a "lint": "next lint" script in package.json but no dedicated lint job in your workflows. To keep build guardrails:
- Make
ignoreDuringBuildsconditional on an env var:// next.config.ts eslint: { - ignoreDuringBuilds: true, + ignoreDuringBuilds: process.env.NEXT_IGNORE_ESLINT === 'true', }, - In your bundle‐analysis workflow (or wherever you need to skip ESLint), set the flag:
# .github/workflows/nextjs_bundle_analysis.yml jobs: bundle-analysis: steps: - name: Build for analysis env: NEXT_IGNORE_ESLINT: 'true' run: npm run build
- Add a dedicated lint job to your CI workflows to run
npm run lint, for example:# .github/workflows/lint.yml name: Lint on: [pull_request, push] jobs: eslint: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - run: npm ci - run: npm run lint
This ensures linting remains enforced on every PR while still allowing you to bypass it when explicitly needed.
🤖 Prompt for AI Agents
In next.config.ts around lines 5 to 7, the ESLint configuration currently always
ignores linting during builds. Modify the ignoreDuringBuilds setting to be
conditional based on an environment variable, such as NEXT_IGNORE_ESLINT, so
ESLint is only skipped when explicitly requested. Then, update your GitHub
workflows by adding the NEXT_IGNORE_ESLINT environment variable set to 'true' in
jobs where you want to skip linting, and create a dedicated lint job that runs
npm run lint on every pull request and push to enforce linting in CI.
| const _streamContext = createResumableStreamContext({ | ||
| waitUntil: after, | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Unused resumable stream context — remove or wire it up
_streamContext is created but never used. Either:
- remove it (and the
after/createResumableStreamContextimports) to avoid overhead, or - pass it into your streaming response to enable resume support.
At the moment it’s dead code.
Do you actually use resumable streams anywhere else? Run this to check:
🏁 Script executed:
#!/bin/bash
rg -n "createResumableStreamContext|toDataStreamResponse\\(" -A 2 -B 2Length of output: 1507
Remove unused resumable‐stream code
_streamContext is created but never used in any toDataStreamResponse() call. Either wire it into your streaming responses to enable resume support or strip it out entirely to avoid dead code.
Locations to address:
- src/app/api/chat/route.ts:10 – remove the
afterimport fromnext/server - src/app/api/chat/route.ts:12 – remove the
createResumableStreamContextimport - src/app/api/chat/route.ts:20–22 – remove the
_streamContextinstantiation
Suggested diff:
-import { after, type NextRequest } from 'next/server';
+import { type NextRequest } from 'next/server';
-import { createResumableStreamContext } from 'resumable-stream';
-const _streamContext = createResumableStreamContext({
- waitUntil: after,
-});If you do intend to support resumable streams, pass _streamContext into your response calls, e.g.
return result.toDataStreamResponse(_streamContext);🤖 Prompt for AI Agents
In src/app/api/chat/route.ts around lines 10, 12, and 20 to 22, the variable
_streamContext is created but never used, and the imports 'after' from
'next/server' and 'createResumableStreamContext' are also unused. To fix this,
either remove the import of 'after' at line 10, remove the import of
'createResumableStreamContext' at line 12, and delete the _streamContext
instantiation at lines 20 to 22, or if resumable streams are intended, pass
_streamContext into the toDataStreamResponse() call where the streaming response
is returned.
| messages.at(-1).role === 'user' && | ||
| internalStatus !== 'streaming' && | ||
| internalStatus !== 'submitted' | ||
| ) { | ||
| const timer = setTimeout(() => { | ||
| if ( | ||
| messages.length > 0 && | ||
| messages[messages.length - 1].role === 'user' && | ||
| messages.at(-1).role === 'user' && | ||
| //@ts-expect-error |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Safer last-message checks; drop TypeScript suppressions
Use a local last with optional chaining to avoid //@ts-expect-error and potential undefined access. This also reads cleaner.
- if (
- messages.length > 0 &&
- messages.at(-1).role === 'user' &&
- internalStatus !== 'streaming' &&
- internalStatus !== 'submitted'
- ) {
+ const last = messages.length > 0 ? messages.at(-1) : undefined;
+ if (
+ last?.role === 'user' &&
+ internalStatus !== 'streaming' &&
+ internalStatus !== 'submitted'
+ ) {
…
- if (
- messages.length > 0 &&
- //@ts-expect-error
- messages.at(-1).role === 'user' &&
- //@ts-expect-error
- internalStatus !== 'streaming' &&
- //@ts-expect-error
- internalStatus !== 'submitted'
- ) {
+ const last2 = messages.length > 0 ? messages.at(-1) : undefined;
+ if (
+ last2?.role === 'user' &&
+ internalStatus !== 'streaming' &&
+ internalStatus !== 'submitted'
+ ) {
setInternalStatus('error');
}Also applies to: 153-164
🤖 Prompt for AI Agents
In src/components/chatContainer.tsx around lines 149 to 157 and 153 to 164,
replace direct access to the last message using messages.at(-1) with a local
variable assigned via optional chaining (e.g., const last = messages.at(-1)) to
safely check its properties. Remove the //@ts-expect-error comment by ensuring
the code handles the possibility that last may be undefined before accessing its
role, improving type safety and code clarity.
| openRouterModels && openRouterModels.length > 0 ? ( | ||
| <div className="grid w-full grid-cols-2 gap-2"> | ||
| {openRouterModels.map((model) => ( | ||
| <div | ||
| className={`card flex w-full items-center justify-start gap-3 bg-primary ${selectedModel === model.name ? 'btn-active' : ''}`} | ||
| key={model.id} | ||
| onClick={() => { | ||
| setSelectedModel(model.name); | ||
| setModelType('openrouter'); | ||
| setOpen(false); | ||
| }} | ||
| > | ||
| <div |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clickable cards should be interactive elements
Using <div onClick> harms accessibility and keyboard navigation. Use <button type="button"> with proper semantics.
- <div
- className={`card flex w-full items-center justify-start gap-3 bg-primary ${selectedModel === model.name ? 'btn-active' : ''}`}
- key={model.id}
- onClick={() => {
+ <button
+ type="button"
+ className={`card flex w-full items-center justify-start gap-3 bg-primary ${selectedModel === model.name ? 'ring-2 ring-offset-2 ring-primary' : ''}`}
+ key={model.id}
+ onClick={() => {
setSelectedModel(model.name);
setModelType('openrouter');
setOpen(false);
}}
- >
+ >
…
- </div>
+ </button>Apply similarly to the “providers” and “ollama” cards.
Also applies to: 239-245
🤖 Prompt for AI Agents
In src/components/chatInput.tsx around lines 219 to 231 and also lines 239 to
245, the clickable cards are implemented using <div> elements with onClick
handlers, which is not accessible for keyboard users. Replace these <div>
elements with <button type="button"> elements to provide proper semantics and
keyboard interactivity. Ensure the styling and event handlers are preserved on
the <button> elements to maintain the same visual appearance and behavior.
| className="btn btn-ghost btn-sm ml-2" | ||
| onClick={() => { | ||
| handleDelete(); | ||
| }} | ||
| > | ||
| <Trash className="h-4 w-4" /> | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Protect destructive action with a confirm
Deleting immediately is risky. Add a confirmation to prevent accidental loss.
- <button
+ <button
className="btn btn-ghost btn-sm ml-2"
- onClick={() => {
- handleDelete();
- }}
+ onClick={() => {
+ if (confirm('Delete this chat permanently?')) {
+ handleDelete();
+ }
+ }}
>
<Trash className="h-4 w-4" />
</button>🤖 Prompt for AI Agents
In src/components/chatMeta.tsx around lines 121 to 127, the delete button
triggers handleDelete immediately without user confirmation. Wrap the
handleDelete call inside a confirmation dialog by using window.confirm or a
similar method. Only call handleDelete if the user confirms the action,
preventing accidental deletions.
| <button | ||
| className="btn btn-ghost btn-sm ml-2" | ||
| onClick={() => { | ||
| handleDelete(); | ||
| navigator.clipboard.writeText( | ||
| `${window.location.origin}/share/${id}` | ||
| ); | ||
| toast.success('Share link copied to clipboard!'); | ||
| }} | ||
| > | ||
| <Trash className="h-4 w-4" /> | ||
| <Link className="h-4 w-4" /> | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clipboard operation should handle failures
Clipboard API can fail (permissions, HTTP context). Add try/catch to surface errors.
- onClick={() => {
- navigator.clipboard.writeText(
- `${window.location.origin}/share/${id}`
- );
- toast.success('Share link copied to clipboard!');
- }}
+ onClick={async () => {
+ try {
+ await navigator.clipboard.writeText(
+ `${window.location.origin}/share/${id}`
+ );
+ toast.success('Share link copied to clipboard!');
+ } catch (_e) {
+ toast.error('Failed to copy link. Please copy it manually.');
+ }
+ }}📝 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.
| <button | |
| className="btn btn-ghost btn-sm ml-2" | |
| onClick={() => { | |
| handleDelete(); | |
| navigator.clipboard.writeText( | |
| `${window.location.origin}/share/${id}` | |
| ); | |
| toast.success('Share link copied to clipboard!'); | |
| }} | |
| > | |
| <Trash className="h-4 w-4" /> | |
| <Link className="h-4 w-4" /> | |
| </button> | |
| <button | |
| className="btn btn-ghost btn-sm ml-2" | |
| onClick={async () => { | |
| try { | |
| await navigator.clipboard.writeText( | |
| `${window.location.origin}/share/${id}` | |
| ); | |
| toast.success('Share link copied to clipboard!'); | |
| } catch (_e) { | |
| toast.error('Failed to copy link. Please copy it manually.'); | |
| } | |
| }} | |
| > | |
| <Link className="h-4 w-4" /> | |
| </button> |
🤖 Prompt for AI Agents
In src/components/chatMeta.tsx around lines 129 to 139, the clipboard write
operation does not handle potential failures such as permission issues or
unsupported contexts. Wrap the navigator.clipboard.writeText call inside a
try/catch block, and in the catch block, show an error toast or log the error to
inform the user that copying failed.
Summary by CodeRabbit
New Features
Refactor
Chores
Removals