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. |
WalkthroughThe PR updates resolution-search file and message handling, adds runtime fallbacks for Supabase and Mapbox initialization, changes settings and mobile layouts, introduces Playwright authentication bypasses, and refreshes service-worker precache entries. ChangesChat and runtime behavior
Interface updates
Test and build support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatAction
participant AIState
participant ChatStorage
participant ChatUI
User->>ChatAction: submit resolution search with image files
ChatAction->>AIState: create sanitized message history
AIState->>ChatStorage: persist messages and metadata
ChatStorage-->>ChatUI: restore chat state
ChatUI->>ChatUI: filter processed images and invalid payloads
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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
|
|
- Rename `model-selection-form` to `tool-selection-form`. - Replace existing models (Grok, Gemini, GPT) with "SkyFi" and "QCX-Terra". - Fix the layout to use a responsive, column-wrapping flex container on narrow viewports to completely prevent word wrapping and element overlap. - Change tab triggers, values, form validation schemas, and descriptions from "model" to "tool". - Update the default value of the selected tool to "QCX-Terra". Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
- Rename `model-selection-form` to `tool-selection-form`. - Replace existing models (Grok, Gemini, GPT) with "SkyFi" and "QCX-Terra". - Fix layout overflow by applying a responsive, column-wrapping flex container on narrow viewports. - Change tab triggers, values, form validation schemas, and descriptions from "model" to "tool". - Update the default value of the selected tool to "QCX-Terra". - Unconditionally invoke `useUser` hook in `useCurrentUser` to fix React rules-of-hooks violation. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
…2612169477250952 Settings tools select renaming and layout overflow fix
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@lib/actions/chat.ts`:
- Around line 213-241: Use a lossless metadata envelope in lib/actions/chat.ts
lines 213-241: update the message-content transformation to store the original
payload in a dedicated field instead of spreading parsed content, preserving
strings and arrays without reshaping them. In app/search/[id]/page.tsx lines
53-74, update the rehydration logic to restore that field verbatim for original
strings and JSON-stringify it only for originally structured payloads, so
BotMessage receives the original response type and content.
In `@middleware.ts`:
- Around line 4-9: Update the default middleware function to accept NextRequest
and NextFetchEvent instead of any, and initialize clerkMiddleware() once at
module scope. Reuse that module-scoped Clerk handler for non-Playwright requests
while preserving the existing NextResponse.next() test bypass and middleware
contract.
- Around line 5-6: Restrict the Playwright authentication bypass in
middleware.ts to a server-only, non-production test guard rather than the public
NEXT_PUBLIC flag alone. In lib/auth/use-current-user.ts, provide the mock
identity only through a matching test-only guarded provider or equivalent
mechanism, and ensure both paths cannot activate in production or outside
isolated tests.
🪄 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: d5d26ce6-0825-4839-866c-7e7597b9f64a
📒 Files selected for processing (9)
app/actions.tsxapp/search/[id]/page.tsxcomponents/settings/components/settings-skeleton.tsxcomponents/settings/components/settings.tsxcomponents/settings/components/tool-selection-form.tsxlib/actions/chat.tslib/auth/use-current-user.tsmiddleware.tspublic/sw.js
📜 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)
🔇 Additional comments (7)
app/actions.tsx (2)
188-212: LGTM!
698-718: LGTM!middleware.ts (1)
2-2: LGTM!public/sw.js (1)
2-2: 🩺 Stability & AvailabilityValidate the regenerated precache inventory against the build output.
Confirm every
precacheEntriesURL exists in the corresponding.nextorpublicoutput and that the worker is generated from the same build; a stale or missing asset can break installation and offline navigation.components/settings/components/tool-selection-form.tsx (1)
22-122: LGTM!components/settings/components/settings.tsx (1)
17-17: LGTM!Also applies to: 40-40, 60-60, 73-85, 197-203, 214-246, 288-299
components/settings/components/settings-skeleton.tsx (1)
9-13: LGTM!
| const newMessagesData: Omit<DbNewMessage, 'chatId'>[] = chat.messages.map(msg => { | ||
| let content = typeof msg.content === 'object' ? JSON.stringify(msg.content) : msg.content; | ||
|
|
||
| // If the message has a type or name, embed them as metadata in the content | ||
| // since the DB schema doesn't have dedicated columns for them yet. | ||
| if (msg.type || msg.name) { | ||
| try { | ||
| let parsedContent; | ||
| if (typeof msg.content === 'object') { | ||
| parsedContent = msg.content; | ||
| } else { | ||
| try { | ||
| parsedContent = JSON.parse(msg.content); | ||
| } catch (e) { | ||
| // If it's not JSON, we wrap it | ||
| parsedContent = { text: msg.content }; | ||
| } | ||
| } | ||
|
|
||
| content = JSON.stringify({ | ||
| ...parsedContent, | ||
| __metadata: { | ||
| type: msg.type, | ||
| name: msg.name | ||
| } | ||
| }); | ||
| } catch (e) { | ||
| console.error('Failed to embed metadata in message content:', e); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a lossless metadata envelope.
A typed plaintext response such as "Hello" is stored as {"text":"Hello","__metadata":...} and rehydrates as {"text":"Hello"}. BotMessage then renders that JSON instead of the response. Arrays are also converted into numeric-keyed objects by { ...parsedContent }.
lib/actions/chat.ts#L213-L241: store the original payload under a dedicated field instead of spreading it.app/search/[id]/page.tsx#L53-L74: restore that payload verbatim (string) or JSON-stringify it only when it was originally structured.
Proposed protocol
- content = JSON.stringify({
- ...parsedContent,
- __metadata: { type: msg.type, name: msg.name }
- });
+ content = JSON.stringify({
+ __metadata: { type: msg.type, name: msg.name },
+ __content: msg.content
+ });- const { __metadata, ...rest } = parsed;
- content = JSON.stringify(rest);
+ const { __metadata, __content } = parsed;
+ content =
+ typeof __content === 'string'
+ ? __content
+ : JSON.stringify(__content);📝 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.
| const newMessagesData: Omit<DbNewMessage, 'chatId'>[] = chat.messages.map(msg => { | |
| let content = typeof msg.content === 'object' ? JSON.stringify(msg.content) : msg.content; | |
| // If the message has a type or name, embed them as metadata in the content | |
| // since the DB schema doesn't have dedicated columns for them yet. | |
| if (msg.type || msg.name) { | |
| try { | |
| let parsedContent; | |
| if (typeof msg.content === 'object') { | |
| parsedContent = msg.content; | |
| } else { | |
| try { | |
| parsedContent = JSON.parse(msg.content); | |
| } catch (e) { | |
| // If it's not JSON, we wrap it | |
| parsedContent = { text: msg.content }; | |
| } | |
| } | |
| content = JSON.stringify({ | |
| ...parsedContent, | |
| __metadata: { | |
| type: msg.type, | |
| name: msg.name | |
| } | |
| }); | |
| } catch (e) { | |
| console.error('Failed to embed metadata in message content:', e); | |
| } | |
| const newMessagesData: Omit<DbNewMessage, 'chatId'>[] = chat.messages.map(msg => { | |
| let content = typeof msg.content === 'object' ? JSON.stringify(msg.content) : msg.content; | |
| // If the message has a type or name, embed them as metadata in the content | |
| // since the DB schema doesn't have dedicated columns for them yet. | |
| if (msg.type || msg.name) { | |
| try { | |
| content = JSON.stringify({ | |
| __metadata: { | |
| type: msg.type, | |
| name: msg.name | |
| }, | |
| __content: msg.content | |
| }); | |
| } catch (e) { | |
| console.error('Failed to embed metadata in message content:', e); | |
| } |
📍 Affects 2 files
lib/actions/chat.ts#L213-L241(this comment)app/search/[id]/page.tsx#L53-L74
🤖 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/actions/chat.ts` around lines 213 - 241, Use a lossless metadata envelope
in lib/actions/chat.ts lines 213-241: update the message-content transformation
to store the original payload in a dedicated field instead of spreading parsed
content, preserving strings and arrays without reshaping them. In
app/search/[id]/page.tsx lines 53-74, update the rehydration logic to restore
that field verbatim for original strings and JSON-stringify it only for
originally structured payloads, so BotMessage receives the original response
type and content.
| export default function middleware(request: any, event: any) { | ||
| if (process.env.NEXT_PUBLIC_PLAYWRIGHT_TEST === "true") { | ||
| return NextResponse.next(); | ||
| } | ||
| return clerkMiddleware()(request, event); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Preserve the middleware contract and reuse the Clerk handler.
The new wrapper erases Next/Clerk type checking with any and reconstructs clerkMiddleware() for every request. Use NextRequest/NextFetchEvent and initialize the Clerk handler once at module scope.
🤖 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 `@middleware.ts` around lines 4 - 9, Update the default middleware function to
accept NextRequest and NextFetchEvent instead of any, and initialize
clerkMiddleware() once at module scope. Reuse that module-scoped Clerk handler
for non-Playwright requests while preserving the existing NextResponse.next()
test bypass and middleware contract.
| if (process.env.NEXT_PUBLIC_PLAYWRIGHT_TEST === "true") { | ||
| return NextResponse.next(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
The Playwright switch is a shared fail-open authentication control. A public flag changes both server request enforcement and client identity; if enabled outside isolated tests, the application can bypass Clerk and expose a shared mock user.
middleware.ts#L5-L6: require a server-only, non-production guard before skipping Clerk.lib/auth/use-current-user.ts#L6-L18: provide the mock identity through a test-only provider or equivalent guarded mechanism.
📍 Affects 2 files
middleware.ts#L5-L6(this comment)lib/auth/use-current-user.ts#L6-L18
🤖 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 `@middleware.ts` around lines 5 - 6, Restrict the Playwright authentication
bypass in middleware.ts to a server-only, non-production test guard rather than
the public NEXT_PUBLIC flag alone. In lib/auth/use-current-user.ts, provide the
mock identity only through a matching test-only guarded provider or equivalent
mechanism, and ensure both paths cannot activate in production or outside
isolated tests.
…ent 500 errors Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
…okens Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
…2612169477250952-12186543232757801618 Fix Resolution Search unhandled 500 crashes
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 224-235: Update the resolution_search_result persistence in
app/actions.tsx to avoid storing full base64 data URLs in messages.content.
Upload the generated image data through the existing Supabase/object-storage
flow, persist only the resulting storage paths or URLs for image, mapboxImage,
and googleImage, and ensure ResolutionCarousel rehydrates using those references
on reload. Preserve the existing IMAGE_PROCESSED sanitization behavior.
In `@lib/utils/index.ts`:
- Around line 90-91: In the model selection flow returning xai(), remove the
no-op requireVision ternary and assign the model ID directly to 'grok-latest'.
Keep the existing model choice unchanged for both vision-required and non-vision
paths.
🪄 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: 267f9ee5-5bc5-4124-adcd-6e91dc05c53d
📒 Files selected for processing (3)
app/actions.tsxlib/utils/index.tspublic/sw.js
📜 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)
🔇 Additional comments (6)
public/sw.js (1)
2-2: LGTM!app/actions.tsx (4)
43-45: LGTM!
722-727: LGTM!
739-740: LGTM!
762-780: LGTM!Also applies to: 790-816
lib/utils/index.ts (1)
44-45: LGTM!
| { | ||
| id: groupeId, | ||
| role: 'assistant', | ||
| content: JSON.stringify({ | ||
| ...analysisResult, | ||
| geoJson: geoJson, // Use reconstructed GeoJSON for storage/UI | ||
| image: dataUrl, | ||
| mapboxImage: mapboxDataUrl, | ||
| googleImage: googleDataUrl | ||
| }), | ||
| type: 'resolution_search_result' | ||
| }, |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
Persisting full base64 data URLs in messages.content will bloat chat storage.
image, mapboxImage, and googleImage embed complete base64 data URLs into the resolution_search_result content, which is serialized into the messages.content text column (lib/db/schema.ts). This is at odds with the sanitization just above (rewriting the user image to IMAGE_PROCESSED to avoid persisting images), and each row can grow to hundreds of KB–MBs, slowing every rehydration/read and inflating the DB.
Since these are only needed to re-render ResolutionCarousel on reload, consider persisting object-storage paths/URLs (e.g. upload to Supabase storage like the document flow does) and rehydrating from those instead of inlining base64.
🤖 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 `@app/actions.tsx` around lines 224 - 235, Update the resolution_search_result
persistence in app/actions.tsx to avoid storing full base64 data URLs in
messages.content. Upload the generated image data through the existing
Supabase/object-storage flow, persist only the resulting storage paths or URLs
for image, mapboxImage, and googleImage, and ensure ResolutionCarousel
rehydrates using those references on reload. Preserve the existing
IMAGE_PROCESSED sanitization behavior.
| const modelId = requireVision ? 'grok-latest' : 'grok-latest'; | ||
| return xai(modelId); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
No-op ternary — both branches return 'grok-latest'.
requireVision ? 'grok-latest' : 'grok-latest' has no effect and reads as leftover/incomplete logic (contrast with line 44, which differentiates). Collapse it to a direct assignment.
I verified the identifiers against the current xAI catalog: the grok-latest alias resolves to a model with input modalities ["text", "image"], so it is vision-capable and correct for both paths — but note it's a moving alias that silently upgrades, so pin an explicit version if you need reproducibility.
♻️ Proposed cleanup
- const modelId = requireVision ? 'grok-latest' : 'grok-latest';
- return xai(modelId);
+ return xai('grok-latest');📝 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.
| const modelId = requireVision ? 'grok-latest' : 'grok-latest'; | |
| return xai(modelId); | |
| return xai('grok-latest'); |
🤖 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/utils/index.ts` around lines 90 - 91, In the model selection flow
returning xai(), remove the no-op requireVision ternary and assign the model ID
directly to 'grok-latest'. Keep the existing model choice unchanged for both
vision-required and non-vision paths.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/settings/components/settings.tsx (1)
294-304: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent the connected Discord row from overflowing on mobile.
The single-line flex row contains a long provider ID, avatar, and button, but the text container has no
min-w-0or wrapping behavior. Long usernames/IDs can force horizontal overflow and make the Disconnect action difficult to reach on narrow screens.Proposed responsive fix
- <div className="flex items-center gap-4 p-4 border rounded-xl bg-accent/20 border-primary/50"> + <div className="flex flex-col gap-3 p-4 border rounded-xl bg-accent/20 border-primary/50 sm:flex-row sm:items-center"> ... - <div className="flex-1"> + <div className="min-w-0 flex-1"> ... - <p className="text-xs text-muted-foreground">ID: {discordAccount.providerUserId}</p> + <p className="break-all text-xs text-muted-foreground">ID: {discordAccount.providerUserId}</p>🤖 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/settings/components/settings.tsx` around lines 294 - 304, Update the connected Discord row around the flex container holding the avatar, account text, and Disconnect action so the text section can shrink on narrow screens and long usernames/provider IDs wrap or truncate within its bounds. Preserve the existing avatar and action layout while ensuring long values do not cause horizontal overflow or push the Disconnect control off-screen.
🤖 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 `@components/settings/components/settings.tsx`:
- Around line 216-220: Update the className styling on the Tabs.Trigger elements
for System, Tools, Users, Map, and Account to use muted styling by default, then
apply the primary background and foreground styling through the
data-[state=active] variant so the selected tab is visually prominent.
- Line 77: Update the settings component around useUser and the Discord
linked-account UI to await Clerk readiness before rendering Discord controls;
disable the link button while clerkUser is unavailable, and guard the disconnect
handler so destroy/reload only run when an existing external account is present.
---
Outside diff comments:
In `@components/settings/components/settings.tsx`:
- Around line 294-304: Update the connected Discord row around the flex
container holding the avatar, account text, and Disconnect action so the text
section can shrink on narrow screens and long usernames/provider IDs wrap or
truncate within its bounds. Preserve the existing avatar and action layout while
ensuring long values do not cause horizontal overflow or push the Disconnect
control off-screen.
🪄 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 Plus
Run ID: 24c14894-ea62-4e0c-bd2b-64729f97c229
📒 Files selected for processing (1)
components/settings/components/settings.tsx
📜 Review details
🔇 Additional comments (2)
components/settings/components/settings.tsx (2)
61-61: 🎯 Functional CorrectnessVerify legacy
selectedModelvalues are compatible with tool IDs.
ToolSelectionFormemitstool.id, but settings still hydrates the raw persistedselectedModelvalue. Existing model identifiers—or a default that is not present intools—will leave no option selected and can be resubmitted unchanged. Add a migration/normalization or server-side allowlist, and verifyQCX-Terrais an actual tool ID.Also applies to: 242-249
28-28: LGTM!Also applies to: 40-40, 74-75, 78-87, 199-205, 233-233, 305-336, 358-362, 368-368, 376-387
| const [currentTab, setCurrentTab] = useState(initialTab === "model" ? "tool" : initialTab); | ||
| const { mapProvider, setMapProvider } = useSettingsStore(); | ||
| const { user, loading: authLoading } = useCurrentUser(); | ||
| const { user: clerkUser } = useUser(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the settings component around the mentioned lines.
fd -a 'settings.tsx$' . | sed 's#^\./##'
printf '\n--- relevant file sizes ---\n'
wc -l components/settings/components/settings.tsx 2>/dev/null || true
printf '\n--- outlines relevant symbols ---\n'
ast-grep outline components/settings/components/settings.tsx --view compact 2>/dev/null | sed -n '1,220p' || true
printf '\n--- context lines 55-360 ---\n'
cat -n components/settings/components/settings.tsx | sed -n '55,360p'
printf '\n--- Clerk usage summary ---\n'
rg -n "useUser|createExternalAccount|loaded|status|isLoaded|externalAccount|Discord|discord" components/settings/components/settings.tsxRepository: QueueLab/QCX
Length of output: 20037
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- files for useCurrentUser ---\n'
rg -n "function useCurrentUser|const useCurrentUser|export .*useCurrentUser|useCurrentUser" components lib src 2>/dev/null || true
printf '\n--- package deps relevant to Clerk ---\n'
if [ -f package.json ]; then cat package.json | jq '.dependencies, .devDependencies' 2>/dev/null || cat package.json; fi
fd -a 'package.json$' . | xargs -r -n1 sh -c 'echo "--- $0 ---"; jq "{dependencies, devDependencies}" "$0" 2>/dev/null | grep -i clerk || true'Repository: QueueLab/QCX
Length of output: 3873
Prevent Discord linking from silently no-oping while Clerk loads.
clerkUser can be temporarily unavailable while @clerk/nextjs loads: both the linked-account render path and the connect/disconnect handlers will evaluate to no-ops or throw UndefinedValueError, while the auth check is still waiting on useCurrentUser. Await Clerk readiness before rendering the Discord UI, disable the link button until clerkUser exists, and guard the disconnect handler with an existing external account before calling destroy/reload.
🤖 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/settings/components/settings.tsx` at line 77, Update the settings
component around useUser and the Discord linked-account UI to await Clerk
readiness before rendering Discord controls; disable the link button while
clerkUser is unavailable, and guard the disconnect handler so destroy/reload
only run when an existing external account is present.
| <Tabs.Trigger value="system-prompt" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">System</Tabs.Trigger> | ||
| <Tabs.Trigger value="tool" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Tools</Tabs.Trigger> | ||
| <Tabs.Trigger value="user-management" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Users</Tabs.Trigger> | ||
| <Tabs.Trigger value="map" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Map</Tabs.Trigger> | ||
| <Tabs.Trigger value="account" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Account</Tabs.Trigger> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the active tab visually distinct.
Every trigger uses bg-primary, while the active state changes it to bg-primary/80; consequently, inactive tabs are more prominent than the selected tab. Use muted styling by default and apply the primary styling in the active-state variant.
Proposed styling adjustment
- bg-primary text-primary-foreground ... data-[state=active]:bg-primary/80
+ bg-muted text-muted-foreground ... data-[state=active]:bg-primary data-[state=active]:text-primary-foreground📝 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.
| <Tabs.Trigger value="system-prompt" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">System</Tabs.Trigger> | |
| <Tabs.Trigger value="tool" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Tools</Tabs.Trigger> | |
| <Tabs.Trigger value="user-management" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Users</Tabs.Trigger> | |
| <Tabs.Trigger value="map" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Map</Tabs.Trigger> | |
| <Tabs.Trigger value="account" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-primary text-primary-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary/80">Account</Tabs.Trigger> | |
| <Tabs.Trigger value="system-prompt" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-muted text-muted-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary data-[state=active]:text-primary-foreground">System</Tabs.Trigger> | |
| <Tabs.Trigger value="tool" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-muted text-muted-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary data-[state=active]:text-primary-foreground">Tools</Tabs.Trigger> | |
| <Tabs.Trigger value="user-management" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-muted text-muted-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary data-[state=active]:text-primary-foreground">Users</Tabs.Trigger> | |
| <Tabs.Trigger value="map" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-muted text-muted-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary data-[state=active]:text-primary-foreground">Map</Tabs.Trigger> | |
| <Tabs.Trigger value="account" className="inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium ring-offset-background transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 bg-muted text-muted-foreground hover:bg-primary/90 h-10 px-4 py-2 data-[state=active]:bg-primary data-[state=active]:text-primary-foreground">Account</Tabs.Trigger> |
🤖 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/settings/components/settings.tsx` around lines 216 - 220, Update
the className styling on the Tabs.Trigger elements for System, Tools, Users,
Map, and Account to use muted styling by default, then apply the primary
background and foreground styling through the data-[state=active] variant so the
selected tab is visually prominent.
3 participants
…2218-00c8db21"
This reverts commit 696bff5.
Summary by CodeRabbit
New Features
Bug Fixes
UI Improvements