User Interface Updates#741
Conversation
…2218-00c8db21" This reverts commit 696bff5.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Manus AI seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe pull request adds runtime guards and fallbacks, persists sanitized resolution history, adjusts vision-model selection and timezone handling, refreshes mobile and settings interfaces, and updates service-worker, ignore, and sidebar test support files. ChangesRuntime behavior
Interface updates
Support artifacts
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/actions.tsx`:
- Line 212: Update the history sanitization flow producing sanitizedHistory so
IMAGE_PROCESSED is not persisted as an image part or URL. Convert it to a text
placeholder such as “[Image processed]”, omit that image part, or add matching
sentinel handling in components/user-message.tsx before it reaches Image
rendering; preserve normal image parts unchanged.
In `@components/chat-panel.tsx`:
- Around line 130-133: Update the upload flow around the optimistic message
creation and Supabase initialization guard to roll back the optimistic message
when initialization fails. In the catch path, remove the fake attachment message
and clear the selected file state before returning, while preserving the
existing optimistic behavior when upload initialization succeeds.
In `@components/empty-screen.tsx`:
- Around line 40-44: Update the button rendered in the example-message loop by
adding an explicit type="button" and simplify its onClick handler to a non-async
callback since submitMessage is not awaited; preserve the existing
submitMessage(item.message) behavior.
In `@components/map/mapbox-map.tsx`:
- Around line 445-453: Remove the unused initializedRef declaration and all
assignments to initializedRef.current in the Mapbox initialization success and
failure paths. Keep the existing !map.current initialization guard unchanged; do
not add initializedRef to it, preserving React 18 Strict Mode re-initialization
behavior.
In `@lib/agents/resolution-search.tsx`:
- Around line 69-93: Update the timezone formatting flow around the localTime
calculation to track an effectiveTimezone initialized from timezone, set it to
UTC in the catch fallback, and use effectiveTimezone when constructing the
prompt so the displayed label matches the formatted timestamp.
In `@tests/sidebar.spec.ts`:
- Around line 6-8: Update the Later-dismissal try/catch around
page.locator('text=Later').click so it ignores only the expected timeout when
the control is absent. Re-throw any other click or actionability error instead
of allowing setup to continue with the dialog open.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5005f2f8-d49f-4e03-9606-14eaa832b949
📒 Files selected for processing (17)
.gitignoreapp/actions.tsxapp/globals.csscomponents/chat-panel.tsxcomponents/chat.tsxcomponents/empty-screen.tsxcomponents/map-loading-context.tsxcomponents/map/mapbox-map.tsxcomponents/settings/components/settings-skeleton.tsxcomponents/settings/components/settings.tsxcomponents/settings/components/system-prompt-form.tsxcomponents/settings/components/user-management-form.tsxlib/agents/resolution-search.tsxlib/supabase/browser-client.tslib/utils/index.tspublic/sw.jstests/sidebar.spec.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
public/sw.js
[warning] Avoid using the initial state variable in setState
Context: setTimeout(t,e)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🪛 React Doctor (0.7.6)
components/empty-screen.tsx
[warning] 40-40: Your users can submit the form by accident because a <button> with no type defaults to submit.
Set an explicit button type so plain buttons do not submit forms by accident: type="button", "submit", or "reset".
(button-has-type)
🔇 Additional comments (11)
app/globals.css (1)
121-136: LGTM!Also applies to: 152-155, 183-194
lib/supabase/browser-client.ts (1)
5-8: LGTM!components/chat.tsx (1)
98-99: LGTM!components/settings/components/settings-skeleton.tsx (1)
9-13: LGTM!components/settings/components/settings.tsx (1)
197-203: LGTM!Also applies to: 214-216, 230-230, 242-242, 288-288, 299-299
components/settings/components/system-prompt-form.tsx (1)
148-148: LGTM!components/settings/components/user-management-form.tsx (1)
76-76: LGTM!.gitignore (1)
62-64: LGTM!public/sw.js (1)
2-2: LGTM!components/map-loading-context.tsx (1)
14-20: LGTM!lib/utils/index.ts (1)
44-45: 🩺 Stability & AvailabilityDrop this warning.
grok-vision-betais a valid xAI vision model identifier, so these branches don’t need to change.> Likely an incorrect or invalid review comment.
| const supabase = getSupabaseBrowserClient() | ||
| if (!supabase) { | ||
| throw new Error('Supabase client is not initialized.') | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Rollback the optimistic message when upload initialization fails.
The optimistic message is added before this guard. When the guard throws, the catch returns while the fake attachment remains in chat history and the selected file remains attached, so retrying can duplicate the message. Either defer setMessages until the upload succeeds or remove the optimistic entry in the catch path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/chat-panel.tsx` around lines 130 - 133, Update the upload flow
around the optimistic message creation and Supabase initialization guard to roll
back the optimistic message when initialization fails. In the catch path, remove
the fake attachment message and clear the selected file state before returning,
while preserving the existing optimistic behavior when upload initialization
succeeds.
| <button | ||
| key={item.message} | ||
| onClick={async () => { | ||
| submitMessage(item.message); | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add an explicit button type to prevent unintended form submissions.
A plain <button> element defaults to type="submit". If this component is placed within a <form> context in the future, clicking the example message will trigger a form submission. Explicitly set type="button".
Also, the onClick handler is currently marked as async but doesn't await anything, which can be safely simplified.
💡 Proposed fix
- <button
- key={item.message}
- onClick={async () => {
- submitMessage(item.message);
- }}
+ <button
+ key={item.message}
+ type="button"
+ onClick={() => {
+ submitMessage(item.message);
+ }}📝 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 | |
| key={item.message} | |
| onClick={async () => { | |
| submitMessage(item.message); | |
| }} | |
| <button | |
| key={item.message} | |
| type="button" | |
| onClick={() => { | |
| submitMessage(item.message); | |
| }} |
🧰 Tools
🪛 React Doctor (0.7.6)
[warning] 40-40: Your users can submit the form by accident because a <button> with no type defaults to submit.
Set an explicit button type so plain buttons do not submit forms by accident: type="button", "submit", or "reset".
(button-has-type)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/empty-screen.tsx` around lines 40 - 44, Update the button rendered
in the example-message loop by adding an explicit type="button" and simplify its
onClick handler to a non-async callback since submitMessage is not awaited;
preserve the existing submitMessage(item.message) behavior.
Source: Linters/SAST tools
| initializedRef.current = true | ||
| setIsMapReady(true) | ||
| setIsMapLoaded(true) // Set map loaded state to true | ||
| }) | ||
| } catch (error) { | ||
| console.error('Failed to initialize Mapbox Map:', error) | ||
| initializedRef.current = true | ||
| setIsMapReady(true) | ||
| setIsMapLoaded(true) // Set map loaded state to true | ||
| }) | ||
| setIsMapLoaded(true) // Set loaded/ready so overlay disappears and app remains functional |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove the unused initializedRef.
The initializedRef is assigned here but never read, making it dead code.
While it might seem tempting to add this ref to the initialization guard (if (!map.current && !initializedRef.current)) to prevent multiple failure logs, doing so would actually break React 18 Strict Mode. In Strict Mode, refs persist across the simulated unmount and remount cycle, so checking the ref would permanently prevent the map from re-initializing after the simulated unmount. The current !map.current guard works correctly on its own, so it is best to remove this ref entirely.
🧹 Proposed cleanup
- initializedRef.current = true
setIsMapReady(true)
setIsMapLoaded(true) // Set map loaded state to true
})
} catch (error) {
console.error('Failed to initialize Mapbox Map:', error)
- initializedRef.current = true
setIsMapReady(true)
setIsMapLoaded(true) // Set loaded/ready so overlay disappears and app remains functional
}(You can also safely remove its declaration const initializedRef = useRef<boolean>(false) around line 33).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/map/mapbox-map.tsx` around lines 445 - 453, Remove the unused
initializedRef declaration and all assignments to initializedRef.current in the
Mapbox initialization success and failure paths. Keep the existing !map.current
initialization guard unchanged; do not add initializedRef to it, preserving
React 18 Strict Mode re-initialization behavior.
| let localTime = ''; | ||
| try { | ||
| localTime = now.toLocaleString('en-US', { | ||
| timeZone: timezone, | ||
| hour: '2-digit', | ||
| minute: '2-digit', | ||
| hour12: true, | ||
| weekday: 'long', | ||
| year: 'numeric', | ||
| month: 'long', | ||
| day: 'numeric' | ||
| }); | ||
| } catch (e) { | ||
| console.error(`Invalid timezone specified: ${timezone}, falling back to UTC`, e); | ||
| localTime = now.toLocaleString('en-US', { | ||
| timeZone: 'UTC', | ||
| hour: '2-digit', | ||
| minute: '2-digit', | ||
| hour12: true, | ||
| weekday: 'long', | ||
| year: 'numeric', | ||
| month: 'long', | ||
| day: 'numeric' | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -C 5 '\bfetchLocationNews\s*\(' libRepository: QueueLab/QCX
Length of output: 1799
🏁 Script executed:
sed -n '1,180p' lib/agents/resolution-search.tsxRepository: QueueLab/QCX
Length of output: 6636
Use the fallback timezone in the prompt too. When toLocaleString falls back to UTC, the prompt still labels the time with the original invalid timezone, so the timestamp and timezone can disagree. Store an effectiveTimezone and use it in the prompt.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/agents/resolution-search.tsx` around lines 69 - 93, Update the timezone
formatting flow around the localTime calculation to track an effectiveTimezone
initialized from timezone, set it to UTC in the catch fallback, and use
effectiveTimezone when constructing the prompt so the displayed label matches
the formatted timestamp.
| try { | ||
| await page.locator('text=Later').click({ timeout: 3000 }); | ||
| } catch (e) {} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tests/sidebar.spec.ts ==\n'
nl -ba tests/sidebar.spec.ts | sed -n '1,80p'
printf '\n== components/purchase-credits-popup.tsx ==\n'
nl -ba components/purchase-credits-popup.tsx | sed -n '1,220p'Repository: QueueLab/QCX
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tests/sidebar.spec.ts ==\n'
awk 'NR>=1 && NR<=120 { printf "%4d %s\n", NR, $0 }' tests/sidebar.spec.ts
printf '\n== components/purchase-credits-popup.tsx ==\n'
awk 'NR>=1 && NR<=260 { printf "%4d %s\n", NR, $0 }' components/purchase-credits-popup.tsxRepository: QueueLab/QCX
Length of output: 4811
Do not swallow all dismissal errors.
Catch only the expected timeout when Later is absent; rethrow other click/actionability errors so setup fails instead of continuing with the dialog still open.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/sidebar.spec.ts` around lines 6 - 8, Update the Later-dismissal
try/catch around page.locator('text=Later').click so it ignores only the
expected timeout when the control is absent. Re-throw any other click or
actionability error instead of allowing setup to continue with the dialog open.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: Request changes
I found two blocking regressions.
-
lib/utils/index.ts:44and:90— invalid xAI image model selection.getModel(true)now hard-codes the legacygrok-vision-betamodel ID for both the selected Grok and default xAI paths.resolutionSearchandresearchercallgetModel(hasImage), so every xAI-backed image request can fail before analysis; the surroundingtry/catchonly wraps provider/model construction, not the request. Please use an image-capable model ID actually supported by the configured xAI API and preserve a request-level fallback if xAI rejects it. -
app/actions.tsx:194— invalid persisted image URL. This keeps the part astype: "image"but replaces its URL with the literalIMAGE_PROCESSED.aiState.donepersists this history, andgetUIStateFromAIStatelater passes the array toUserMessage, which renders every image part throughnext/image; revisiting a chat after a resolution search can therefore break rendering with an invalid imagesrc. Drop the image part or render a valid explicit placeholder instead of persisting an invalid image URL.
Validation: inspected the complete 17-file diff, relevant call sites and persistence/rendering paths, PR discussion and prior formal reviews, and the existing checks for head b7c8d653e5931a9f4ab77a34f6fda1fcdf43e4f3. No tests, lint, build, or other CI-style commands were run.
ngoiyaeric
changed the title
…tadata and filtering sanitized images
2 participants
…2218-00c8db21"
This reverts commit 696bff5.
Summary by CodeRabbit
Bug Fixes
UI Improvements