Conversation
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
|
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (3)
WalkthroughThe PR updates the map analysis streaming text, migrates toast notifications from react-toastify to sonner, includes map images within chat messages, adjusts mobile layout breakpoints, and updates AI model identifiers for Grok and Gemini providers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
URL.createObjectURL(blob)inHeaderSearchButtonis not revoked, causing a likely memory leak during repeated resolution searches.getModel(requireVision)is only partially honoringrequireVision(xAI does; Gemini doesn’t), which can lead to inconsistent behavior when images are attached.- The breakpoint comparison changed to
< 768; confirm this matches the CSS breakpoint semantics to avoid edge-case layout mismatches at exactly768px.
Additional notes (4)
-
Maintainability |
lib/utils/index.ts:52-52
getModel(requireVision)now respectsrequireVisionfor xAI, but the Gemini path ignores it and always returnsgemini-1.5-pro. If resolution search attaches images (as this PR now does), the Gemini fallback should likely choose a vision-capable model whenrequireVision === true, otherwise the behavior differs across providers. -
Maintainability |
lib/utils/index.ts:37-43
The model ID changes for xAI switch togrok-beta/grok-vision-beta. These look like beta identifiers; if this is deliberate, consider how you’ll handle stability and future upgrades. Also, the selected label “Grok 4.2” no longer maps to a “4.x” model name, which can be confusing for users and makes incident debugging harder (UI label and backend model diverge). -
Readability |
components/chat-panel.tsx:56-62
The new mobile breakpoint uses< 768. Most responsive logic uses<=comparisons to avoid a 1px “dead zone” at the exact breakpoint. At exactly768, this code will treat the layout as desktop, which may contradict the rest of the app’s CSS breakpoints depending on howmd:is defined. -
Readability |
components/chat-panel.tsx:292-297
The attachment filename span now combinestruncatewithbreak-all.truncaterelies onoverflow-hidden/text-ellipsis/whitespace-nowrap, whilebreak-allencourages wrapping; together they can produce inconsistent results across browsers and can defeat the goal (either it never wraps, or it wraps but still ellipsizes oddly).
Summary of changes
What changed
-
Resolution search UX
- Initialized the resolution-search summary stream with a loading message (
"Analyzing map view...") inapp/actions.tsx.
- Initialized the resolution-search summary stream with a loading message (
-
Chat panel UI + behavior (
components/chat-panel.tsx)- Standardized the mobile breakpoint to
< 768. - Replaced the attachment oversize
alert()withsonnertoast.error(). - Fixed desktop behavior where the input form could disappear by moving the “New chat” button into the main render instead of returning early.
- Adjusted padding/left-right spacing for mobile vs desktop input.
- Improved filename wrapping by adding
break-all.
- Standardized the mobile breakpoint to
-
Header resolution search (
components/header-search-button.tsx)- Migrated toast provider to
sonner. - Added immediate visual feedback by rendering the captured map image in the
UserMessage(viaURL.createObjectURL(blob)) before submitting the action.
- Migrated toast provider to
-
Model selection logic (
lib/utils/index.ts)- Updated xAI model IDs to respect
requireVision(grok-vision-betavsgrok-beta). - Switched Gemini model ID from
gemini-3-pro-previewtogemini-1.5-proin both selected and fallback paths.
- Updated xAI model IDs to respect
| const imageUrl = URL.createObjectURL(blob); | ||
|
|
||
| setMessages(currentMessages => [ | ||
| ...currentMessages, | ||
| { | ||
| id: nanoid(), | ||
| component: ( | ||
| <UserMessage | ||
| content={[ | ||
| { type: 'text', text: 'Analyze this map view.' }, | ||
| { type: 'image', image: imageUrl } | ||
| ]} | ||
| /> | ||
| ) | ||
| } | ||
| ]) | ||
|
|
There was a problem hiding this comment.
Creating an object URL for the captured image without revoking it can leak memory over time (each search allocates a blob URL that persists until URL.revokeObjectURL). This is especially risky if users perform multiple searches in a session.
Suggestion
Revoke the object URL after it’s no longer needed. If UserMessage only needs the URL for rendering, you can revoke it after the image loads (best), or at least after the request completes.
One pragmatic approach is to track the URL and revoke it in a finally block (may revoke too early if the image is still rendering), or better, pass a callback to revoke on image onLoad.
Example (safer: revoke on image load inside UserMessage):
- Extend
UserMessageimage rendering to accept anonLoadhandler and callURL.revokeObjectURL(imageUrl).
Minimal stopgap (revoke after submission finishes):
const imageUrl = URL.createObjectURL(blob);
try {
setMessages(/* ... */)
// submit
} finally {
URL.revokeObjectURL(imageUrl);
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing URL revocation in the most appropriate place (preferably on image load).
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@components/chat-panel.tsx`:
- Around line 173-193: The fixed "New chat" button wrapper lacks an explicit
z-index and can be rendered behind other stacking contexts; update the wrapper
div (the element rendered when messages.length > 0 && !isMobile) to include a
higher z-index in its className (e.g., add "z-11" or larger) so the Button (the
Button with onClick={() => handleClear()} and the Plus icon) reliably appears
above the sticky input container.
In `@components/header-search-button.tsx`:
- Around line 81-96: The blob URL created by URL.createObjectURL(blob) is never
revoked and can leak memory; update the message creation in the setMessages
callback to use a persistent data URL instead of a blob URL (use the same base64
data URL produced in app/actions.tsx for the AI message) or ensure you call
URL.revokeObjectURL(imageUrl) when the message is removed/cleared; locate the
code that builds the message component (setMessages / UserMessage) and replace
image: imageUrl with the base64 data URL or add cleanup logic tied to message
removal to revoke the blob URL.
In `@lib/utils/index.ts`:
- Line 84: The fallback return uses xai(requireVision ? 'grok-vision-beta' :
'grok-beta') but doesn't apply the same structured-output handling used for the
explicit Grok selection; update the fallback path so when returning via xai(...)
it wraps or configures the chosen model to enforce structured outputs (mirror
the structured-output wrapper/logic used elsewhere for explicit selection),
referencing the same requireVision flag and the model names ('grok-vision-beta'
and 'grok-beta') and the xai call so both paths produce consistent structured
output.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
app/actions.tsxcomponents/chat-panel.tsxcomponents/header-search-button.tsxlib/utils/index.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
lib/utils/index.ts
📚 Learning: 2026-01-17T06:14:51.070Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-17T06:14:51.070Z
Learning: For structured output tasks using ai.generateObject (resolution-search, geojsonParser, map-command-generator, feedback-analyzer), prefer OpenAI gpt-4o. While xAI's grok-4-1-fast-reasoning technically supports structured outputs and vision, OpenAI has proven more reliable integration with the AI SDK's generateObject function and Zod schemas in production.
Applied to files:
lib/utils/index.ts
🧬 Code graph analysis (3)
app/actions.tsx (1)
lib/agents/resolution-search.tsx (2)
resolutionSearch(33-79)f(50-50)
components/header-search-button.tsx (2)
components/user-message.tsx (1)
UserMessage(15-53)components/map/google-map.tsx (2)
GoogleMapComponent(12-86)apiKey(20-29)
components/chat-panel.tsx (3)
lib/utils/index.ts (1)
cn(12-14)components/ui/button.tsx (1)
Button(56-56)components/chat.tsx (1)
Chat(29-229)
🔇 Additional comments (8)
app/actions.tsx (1)
87-87: LGTM!Initializing
summaryStreamwith'Analyzing map view...'provides immediate visual feedback to the user while the resolution search streams its response. This aligns well with the PR objective of improving UI consistency.components/header-search-button.tsx (1)
12-12: LGTM on toast migration.Replacing
react-toastifywithsonneraligns with the PR objective and maintains consistency with the same change inchat-panel.tsx.components/chat-panel.tsx (4)
59-59: LGTM on breakpoint synchronization.Changing the mobile breakpoint from
<= 1024to< 768aligns with theisMobiledetection incomponents/chat.tsx, ensuring consistent responsive behavior across the application. This matches the PR objective of synchronizing the isMobile breakpoint to 768px.
70-70: Good replacement of alert with toast.Using
toast.error()from sonner provides a non-blocking, more polished user experience compared to the nativealert()dialog.
241-246: LGTM on input styling adjustments.The padding changes (
px-4for mobile,pl-14for desktop) appropriately accommodate the attachment button placement on desktop while maintaining clean mobile styling.
294-294: LGTM on filename wrapping.Adding
break-allensures long filenames without natural word breaks (e.g.,map_capture_2026020411570512345.png) wrap properly within the container.lib/utils/index.ts (2)
55-55: Model identifier change requires clarification.The code changed from
gemini-3-pro-previewtogemini-1.5-pro. However, current Gemini API documentation (2025) listsgemini-3-pro-previewas an available model identifier, whilegemini-1.5-prodoes not appear in the current model catalog. This appears to be a regression to an older/obsolete model rather than a correction to a stable identifier. Clarify the reasoning behind this change—if compatibility with an older API version is required, that should be documented.Likely an incorrect or invalid review comment.
40-40:⚠️ Potential issue | 🟠 MajorUse
grok-2-vision-1212(or newer) instead ofgrok-vision-betafor resolution-search structured outputs.
resolution-searchusesstreamObjectwith a Zod schema for structured geospatial analysis output. When satellite images are provided (the primary use case), this callsgetModel(true), which returnsgrok-vision-beta.Per X.AI integration documentation,
grok-vision-betais treated as a legacy model and does not guarantee native JSON schema support for structured outputs—it falls back to a tool-calling workaround. Usegrok-2-vision-1212or newer for reliable JSON schema enforcement with vision capabilities.Alternatively, use OpenAI
gpt-4o(which supports both vision and structured outputs reliably) if X.AI model availability is a constraint.⛔ Skipped due to learnings
Learnt from: ngoiyaeric Repo: QueueLab/QCX PR: 0 File: :0-0 Timestamp: 2026-01-17T06:14:51.070Z Learning: For structured output tasks using ai.generateObject (resolution-search, geojsonParser, map-command-generator, feedback-analyzer), prefer OpenAI gpt-4o. While xAI's grok-4-1-fast-reasoning technically supports structured outputs and vision, OpenAI has proven more reliable integration with the AI SDK's generateObject function and Zod schemas in production.Learnt from: ngoiyaeric Repo: QueueLab/QCX PR: 0 File: :0-0 Timestamp: 2026-01-13T13:26:30.086Z Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| {/* New chat button (appears when there are messages on desktop) */} | ||
| {messages.length > 0 && !isMobile && ( | ||
| <div | ||
| className={cn( | ||
| 'fixed bottom-2 left-2 flex justify-start items-center pointer-events-none md:bottom-8' | ||
| )} | ||
| > | ||
| <Button | ||
| type="button" | ||
| variant={'secondary'} | ||
| className="rounded-full bg-secondary/80 group transition-all hover:scale-105 pointer-events-auto" | ||
| onClick={() => handleClear()} | ||
| data-testid="new-chat-button" | ||
| > | ||
| <span className="text-sm mr-2 group-hover:block hidden animate-in fade-in duration-300"> | ||
| New | ||
| </span> | ||
| <Plus size={18} className="group-hover:rotate-90 transition-all" /> | ||
| </Button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n components/chat-panel.tsx | sed -n '165,200p'Repository: QueueLab/QCX
Length of output: 1559
🏁 Script executed:
#!/bin/bash
# Check for other fixed/sticky positioned elements that might overlap
rg -n "fixed|sticky" --type=tsx -C2 components/Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Search for fixed/sticky positioning in component files
rg "fixed|sticky" components/ -C2Repository: QueueLab/QCX
Length of output: 5523
🏁 Script executed:
# Also check specifically for z-index usage in chat-panel.tsx
rg "z-index|z-\d+" components/chat-panel.tsx -nRepository: QueueLab/QCX
Length of output: 156
Add explicit z-index to the fixed button to prevent stacking order issues.
The fixed button uses bottom-2 left-2 positioning without an explicit z-index, while its parent sticky container has z-10. Since the fixed element is nested as a child in the DOM, it may be rendered behind the sticky parent depending on stacking context. Add z-11 or higher to the fixed wrapper's className to ensure the button appears on top of the sticky input container.
🤖 Prompt for AI Agents
In `@components/chat-panel.tsx` around lines 173 - 193, The fixed "New chat"
button wrapper lacks an explicit z-index and can be rendered behind other
stacking contexts; update the wrapper div (the element rendered when
messages.length > 0 && !isMobile) to include a higher z-index in its className
(e.g., add "z-11" or larger) so the Button (the Button with onClick={() =>
handleClear()} and the Plus icon) reliably appears above the sticky input
container.
| }); | ||
| try { | ||
| return xai('grok-4-fast-non-reasoning'); | ||
| return xai(requireVision ? 'grok-vision-beta' : 'grok-beta'); |
There was a problem hiding this comment.
Same structured output concern applies to the fallback path.
The default fallback path has the same potential issue with Grok models and structured outputs as noted for the explicit selection path above.
🤖 Prompt for AI Agents
In `@lib/utils/index.ts` at line 84, The fallback return uses xai(requireVision ?
'grok-vision-beta' : 'grok-beta') but doesn't apply the same structured-output
handling used for the explicit Grok selection; update the fallback path so when
returning via xai(...) it wraps or configures the chosen model to enforce
structured outputs (mirror the structured-output wrapper/logic used elsewhere
for explicit selection), referencing the same requireVision flag and the model
names ('grok-vision-beta' and 'grok-beta') and the xai call so both paths
produce consistent structured output.
- Fixed model mapping and vision support in getModel. - Improved resolution search UX with loading states and immediate image feedback. - Synchronized mobile breakpoints and layout padding across components. - Fixed suggestion/example submission logic in Chat component. - Implemented robust hydration refresh pattern in Chat component. - Fixed app/actions.tsx type safety and researcher agent call signature. - Enhanced resolution search agent to prioritize user-drawn features. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
2 participants
User description
This PR fixes multiple issues with the resolution search feature and general UI consistency:
PR created automatically by Jules for task 7491228516552462755 started by @ngoiyaeric
PR Type
Bug fix, Enhancement
Description
Fixed model ID mappings in
getModel()to use correct model names and respectrequireVisionflagSynchronized mobile breakpoint from 1024px to 768px across components
Replaced react-toastify with sonner for consistent toast notifications
Fixed chat panel layout bug where input form was hidden on desktop with messages
Improved resolution search UX with loading message and captured image in UserMessage
Enhanced file attachment UI with better text wrapping and error handling
Diagram Walkthrough
File Walkthrough
index.ts
Correct model IDs and add vision supportlib/utils/index.ts
grok-4-fast-non-reasoningtogrok-betawithvision variant support
gemini-3-pro-previewtogemini-1.5-procorrect model names
grok-vision-betawhenrequireVisionistrue
chat-panel.tsx
Fix layout bug and standardize mobile breakpointcomponents/chat-panel.tsx
alert()withtoast.error()for file size validationcontainer to prevent hiding input form on desktop
px-4for mobile, keptpl-14fordesktop
break-allclass to filename display for better text wrappingSuggestionsDropdown, added import forsonnertoast
actions.tsx
Add loading message to resolution searchapp/actions.tsx
map view..."
header-search-button.tsx
Include image in UserMessage and use sonnercomponents/header-search-button.tsx
notifications
converted to URL
immediate visual feedback
displaying message
Summary by CodeRabbit
New Features
Bug Fixes
Chores