Daily branch 2025 09 24 #38
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds persistent chat mode (chatMode) stored in localStorage, threads default model slug through UI, API, and DB, introduces document drag-and-drop and latest-ref hooks, refactors chat initialization to distinguish existing vs new chats and suppress premature "not found" UI, and updates schema/Convex to persist default_model_slug. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Chat UI
participant Global as GlobalState
participant Store as client-storage
participant API as /api/chat (finish)
participant Actions as lib/db/actions
participant Convex as convex.chats
participant DB as Chats Table
Note over UI,Global: App start / route change
UI->>Global: read chatMode
Global->>Store: readChatMode()
Store-->>Global: "ask" | "agent" | null
alt Existing chatId in route
UI->>API: getChatById(chatId)
API->>Convex: getChatById
Convex-->>API: { title, todos, default_model_slug }
API-->>UI: data
alt default_model_slug present and not initialized
UI->>Global: setChatMode("ask"|"agent")
Global->>Store: writeChatMode(mode)
end
else New chat flow
UI->>UI: awaitingServerChat = true (suppress not-found)
UI->>API: create/init chat (async elsewhere)
API-->>UI: chat id created
UI->>UI: awaitingServerChat = false
end
sequenceDiagram
participant UI as Chat UI
participant API as /api/chat (finish)
participant Actions as lib/db/actions.updateChat
participant Convex as convex.chats.updateChat
participant DB as Chats Table
UI->>API: Finish (title, finishReason, todos, defaultModelSlug)
API->>Actions: updateChat(chatId, ..., defaultModelSlug)
Actions->>Convex: updateChat({..., default_model_slug: defaultModelSlug})
Convex->>DB: update row
DB-->>Convex: ok
Convex-->>Actions: ok
Actions-->>API: ok
API-->>UI: ok
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 2
🧹 Nitpick comments (9)
app/hooks/useLatestRef.ts (1)
3-9: Avoid one-render staleness by updating the ref synchronouslyUpdate
ref.currentduring render instead of in an effect so async callbacks always see the latest value without a one-render lag.export const useLatestRef = <T>(value: T) => { - const ref = useRef<T>(value); - useEffect(() => { - ref.current = value; - }, [value]); - return ref; + const ref = useRef<T>(value); + // Keep current value in sync without waiting for effects + ref.current = value; + return ref; };app/hooks/useDocumentDragAndDrop.ts (2)
1-1: Use latest handlers without re-binding; import useLatestRefBind once and read the latest handlers from a ref to avoid frequent add/remove cycles.
-import { useEffect } from "react"; +import { useEffect } from "react"; +import { useLatestRef } from "./useLatestRef";
14-31: Make dragover/drop non-passive and avoid re-binding handlers
- Use
useLatestRefso listeners attach once and always read fresh handlers.- Mark
dragoveranddroplisteners{ passive: false }sopreventDefault()works reliably.-export const useDocumentDragAndDrop = (handlers: { +export const useDocumentDragAndDrop = (handlers: { handleDragEnter: DragHandler; handleDragLeave: DragHandler; handleDragOver: DragHandler; handleDrop: DragHandler; }) => { - const { handleDragEnter, handleDragLeave, handleDragOver, handleDrop } = - handlers; + const handlersRef = useLatestRef(handlers); - useEffect(() => { - const onEnter = (e: DragEvent) => handleDragEnter(e); - const onLeave = (e: DragEvent) => handleDragLeave(e); - const onOver = (e: DragEvent) => handleDragOver(e); - const onDrop = (e: DragEvent) => handleDrop(e); + useEffect(() => { + const onEnter = (e: DragEvent) => handlersRef.current.handleDragEnter(e); + const onLeave = (e: DragEvent) => handlersRef.current.handleDragLeave(e); + const onOver = (e: DragEvent) => handlersRef.current.handleDragOver(e); + const onDrop = (e: DragEvent) => handlersRef.current.handleDrop(e); - document.addEventListener("dragenter", onEnter); - document.addEventListener("dragleave", onLeave); - document.addEventListener("dragover", onOver); - document.addEventListener("drop", onDrop); + document.addEventListener("dragenter", onEnter); + document.addEventListener("dragleave", onLeave); + document.addEventListener("dragover", onOver, { passive: false }); + document.addEventListener("drop", onDrop, { passive: false }); return () => { document.removeEventListener("dragenter", onEnter); document.removeEventListener("dragleave", onLeave); - document.removeEventListener("dragover", onOver); - document.removeEventListener("drop", onDrop); + document.removeEventListener("dragover", onOver); + document.removeEventListener("drop", onDrop); }; - }, [handleDragEnter, handleDragLeave, handleDragOver, handleDrop]); + }, []); // read latest handlers via ref };lib/utils/client-storage.ts (1)
56-63: Add a clearChatMode helper for logout and account switchingProvide a dedicated API to remove the persisted chat mode so consumers (e.g., logout) can reset local state.
export const writeChatMode = (mode: "ask" | "agent"): void => { if (!isBrowser()) return; try { window.localStorage.setItem(CHAT_MODE_STORAGE_KEY, mode); } catch { // ignore } }; + +export const clearChatMode = (): void => { + if (!isBrowser()) return; + try { + window.localStorage.removeItem(CHAT_MODE_STORAGE_KEY); + } catch { + // ignore + } +};convex/schema.ts (1)
11-13: Confirm field naming vs. value semantics
default_model_slugholds"ask" | "agent", which are modes, not model slugs. If intentional, considerdefault_modefor clarity; if not, widen the validator to true model slugs.Would you like me to generate a codemod plan if we decide to rename the field across API, DB, and UI?
lib/utils/logout.ts (2)
3-3: Also clear persisted chat mode on logoutReset chat mode so users don’t carry over state after logout.
-import { clearAllDrafts } from "@/lib/utils/client-storage"; +import { clearAllDrafts, clearChatMode } from "@/lib/utils/client-storage";
7-17: Invoke chat mode clearing with draftsCall
clearChatMode()alongside drafts purge.try { clearAllDrafts(); + clearChatMode(); } catch { // ignore } finally {app/hooks/useChatHandlers.ts (1)
93-98: Consider extracting chat activation logic to reduce duplication.This block handles chat initialization for new non-temporary chats. Similar logic appears to be handled by
activateChatLocally(lines 362-365 in chat.tsx). Consider consolidating this logic.- if (!isExistingChat && !temporaryChatsEnabledRef.current) { - setChatTitle(null); - setCurrentChatId(chatId); - window.history.replaceState({}, "", `/c/${chatId}`); - activateChatLocally(); - } + if (!isExistingChat && !temporaryChatsEnabledRef.current) { + // Delegate all activation logic to the parent component + activateChatLocally(); + }The parent component could then handle all the state updates including
setChatTitle,setCurrentChatId, and URL updates in one place.app/contexts/GlobalState.tsx (1)
139-141: Consider debouncing localStorage writes for chat mode.Writing to localStorage on every
chatModechange could potentially cause performance issues if the mode is changed rapidly. Consider debouncing these writes.+ const chatModeTimeoutRef = useRef<NodeJS.Timeout | null>(null); + // Persist chat mode preference to localStorage on change useEffect(() => { - writeChatMode(chatMode); + // Clear any pending write + if (chatModeTimeoutRef.current) { + clearTimeout(chatModeTimeoutRef.current); + } + + // Debounce the write by 500ms + chatModeTimeoutRef.current = setTimeout(() => { + writeChatMode(chatMode); + chatModeTimeoutRef.current = null; + }, 500); + + return () => { + if (chatModeTimeoutRef.current) { + clearTimeout(chatModeTimeoutRef.current); + } + }; }, [chatMode]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/api/chat/route.ts(1 hunks)app/components/ChatInput.tsx(7 hunks)app/components/chat.tsx(12 hunks)app/contexts/GlobalState.tsx(5 hunks)app/hooks/useChatHandlers.ts(8 hunks)app/hooks/useDocumentDragAndDrop.ts(1 hunks)app/hooks/useLatestRef.ts(1 hunks)app/hooks/useMessageScroll.ts(0 hunks)convex/chats.ts(5 hunks)convex/schema.ts(1 hunks)lib/db/actions.ts(3 hunks)lib/utils/client-storage.ts(2 hunks)lib/utils/logout.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/hooks/useMessageScroll.ts
🧰 Additional context used
📓 Path-based instructions (3)
convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.ts: Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
When a function returns null, include returns: v.null() and return null explicitly
Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Use query/mutation/action only for public API functions
Do not try to register functions via the api or internal objects
Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
In JS implementations, functions without an explicit return value implicitly return null
Use ctx.runQuery from queries/mutations/actions to call a query
Use ctx.runMutation from mutations/actions to call a mutation
Use ctx.runAction from actions to call an action
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise extract shared helper code
Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
When calling a function in the same file via ctx.run*, add an explicit return type annotation at the call site to avoid TS circularity
Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Respect file-based routing for function references: e.g., convex/example.ts export f -> api.example.f; nested paths map to dot-separated namespaces
For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
v.bigint() is deprecated; use v.int64() for signed 64-bit integers
Use v.record(keys, values) for record-like data; v.map() and v.set() are not supported
For full-text search, use withSearchIndex("ind...
Files:
convex/schema.tsconvex/chats.ts
convex/schema.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/schema.ts: Define the Convex schema in convex/schema.ts
Import schema definition functions (defineSchema, defineTable) from convex/server
Understand system fields: _id uses v.id(tableName) and _creationTime uses v.number(); they’re auto-added to all documents
Include all indexed fields in the index name (e.g., by_field1_and_field2 for ["field1","field2"])
Query indexes in the same column order as defined; create separate indexes for alternate orders
Files:
convex/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
convex/schema.tsapp/hooks/useChatHandlers.tsapp/hooks/useDocumentDragAndDrop.tslib/db/actions.tslib/utils/client-storage.tsconvex/chats.tsapp/hooks/useLatestRef.tsapp/api/chat/route.tsapp/contexts/GlobalState.tsxlib/utils/logout.tsapp/components/chat.tsxapp/components/ChatInput.tsx
🧬 Code graph analysis (3)
app/hooks/useChatHandlers.ts (1)
app/components/DataStreamProvider.tsx (1)
useDataStream(40-46)
app/contexts/GlobalState.tsx (2)
types/chat.ts (1)
ChatMode(4-4)lib/utils/client-storage.ts (2)
readChatMode(45-54)writeChatMode(56-63)
app/components/chat.tsx (2)
app/hooks/useLatestRef.ts (1)
useLatestRef(3-9)app/hooks/useDocumentDragAndDrop.ts (1)
useDocumentDragAndDrop(5-32)
🔇 Additional comments (15)
lib/utils/client-storage.ts (2)
14-14: LGTM: dedicated storage key for chat modeConsistent key name; pairs well with read/write helpers below.
45-54: Validate and fail safe on bad values — goodReturning
nullfor unknown or missing values avoids unexpected states.lib/db/actions.ts (2)
145-157: Type surface extension for updateChat — goodOptional
"ask" | "agent"aligns with route payload.
159-166: ConfirmdefaultModelSlugmapping — theupdateChatmutation’s args includedefaultModelSlugand it’s correctly mapped todefault_model_slugon the server.app/components/ChatInput.tsx (4)
42-47: Switched drafts import to client-storage — good consolidationCentralizes storage-related helpers behind one module.
76-78: Renamed state to chatMode/setChatMode — consistent with PRMatches the new persistence API and backend field.
95-99: Free plan fallback to 'ask' — solid guardrailPrevents accidental agent usage for free users post-hydration or refresh.
243-246: UI wiring for mode selector and placeholder — correctConditionals, labels, and click handlers reflect chatMode accurately.
Also applies to: 271-283, 287-301
app/api/chat/route.ts (1)
241-242: Approve defaultModelSlug persistence
Confirmeddefault_model_slugin the Convex schema and its mapping inupdateChat.convex/chats.ts (1)
25-27: LGTM! Consistent propagation of chat mode preferences.The addition of
default_model_slugfield is properly implemented across all relevant query and mutation handlers with appropriate validation usingv.union(v.literal("ask"), v.literal("agent")).Also applies to: 91-93, 171-171, 208-208, 228-230
app/hooks/useChatHandlers.ts (1)
25-27: LGTM! Well-structured extension of chat handler interface.The addition of
isExistingChatandactivateChatLocallyprops provides clean separation between new and existing chat flows.Also applies to: 37-38
app/contexts/GlobalState.tsx (2)
123-123: LGTM! Proper migration from mode to chatMode with persistence.The renaming from
modetochatModeis consistently applied, and the persistence logic usingreadChatMode/writeChatModeensures user preferences are maintained across sessions.Also applies to: 126-129
164-168: Validate temporary chat parameter to prevent injection.The temporary chat state is initialized directly from URL parameters without validation. Consider validating the parameter value more strictly.
// Initialize temporary chats from URL parameter const [temporaryChatsEnabled, setTemporaryChatsEnabled] = useState(() => { if (typeof window === "undefined") return false; const urlParams = new URLSearchParams(window.location.search); - return urlParams.get("temporary-chat") === "true"; + const param = urlParams.get("temporary-chat"); + // Only accept exact string "true", treat everything else as false + return param === "true"; });Likely an incorrect or invalid review comment.
app/components/chat.tsx (2)
69-69: Good use of useLatestRef to prevent stale closures.Using
useLatestRefforisExistingChatandtemporaryChatsEnabledensures callbacks always have access to the current values, preventing race conditions in async flows.Also applies to: 74-74
339-344: LGTM! Clean encapsulation of drag-and-drop handlers.The migration from inline document event listeners to the
useDocumentDragAndDrophook improves code organization and reusability.
…try/catch to swallow benign races
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)
app/hooks/useChatHandlers.ts (1)
15-27: UseId<"chats">forchatIdinUseChatHandlersProps.Change
chatId: stringtochatId: Id<"chats">in the interface (and in any other handler signatures, e.g. lines 37–39) to align with your Convex schema and generated types.
🧹 Nitpick comments (4)
lib/chat/chat-processor.ts (1)
108-111: Gate authorization text to agent mode (avoid adding to simple ask flows).If intended only for pentesting/agent scenarios, also check mode to prevent injecting this text into regular Q&A.
- // If moderation allows, add authorization message - if (moderationResult.shouldUncensorResponse) { + // If moderation allows and in agent mode, add authorization message + if (moderationResult.shouldUncensorResponse && mode === "agent") { addAuthMessage(messagesWithUrls); }convex/messages.ts (2)
329-333: Avoid .filter in queries; use an index instead (per repo Convex guidelines).Filtering after an index scan is discouraged. Use a composite index on (chat_id, role) to fetch the last assistant message efficiently.
Apply this diff after adding an index like "by_chat_id_role" (chat_id, role):
- const lastAssistantMessage = await ctx.db - .query("messages") - .withIndex("by_chat_id", (q) => q.eq("chat_id", args.chatId)) - .filter((q) => q.eq(q.field("role"), "assistant")) - .order("desc") - .first(); + // Requires composite index: "by_chat_id_role" on (chat_id, role) + const lastAssistantMessage = await ctx.db + .query("messages") + .withIndex("by_chat_id_role", (q) => + q.eq("chat_id", args.chatId).eq("role", "assistant"), + ) + .order("desc") + .first();
440-446: Brittle string match for “Unauthorized”; prefer ConvexError code checks.Align with your other handlers: check ConvexError.data.code instead of error.message.includes("Unauthorized").
Example tweak:
- if (error instanceof Error && error.message.includes("Unauthorized")) { - throw error; - } + if (error instanceof ConvexError) { + // Re-throw service/chat auth errors; return [] for benign cases + throw error; + }If you need to distinguish between CHAT_* codes and service-key auth, branch on error.data?.code.
app/hooks/useChatHandlers.ts (1)
93-98: Verify activation branch: existing vs new chat logic likely inverted.
activateChatLocally()is called only when!isExistingChat, which reads counterintuitive. If the intent is to activate an already-existing chat locally and update history for it, split the conditions.Proposed refactor:
- if (!isExistingChat && !temporaryChatsEnabledRef.current) { - setChatTitle(null); - setCurrentChatId(chatId); - window.history.replaceState({}, "", `/c/${chatId}`); - activateChatLocally(); - } + if (!temporaryChatsEnabledRef.current) { + if (isExistingChat) { + window.history.replaceState({}, "", `/c/${chatId}`); + activateChatLocally(); + } else { + setChatTitle(null); + setCurrentChatId(chatId); + window.history.replaceState({}, "", `/c/${chatId}`); + } + }Confirm desired behavior for existing vs new chats before applying.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/hooks/useChatHandlers.ts(9 hunks)convex/messages.ts(2 hunks)lib/chat/chat-processor.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.ts: Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
When a function returns null, include returns: v.null() and return null explicitly
Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Use query/mutation/action only for public API functions
Do not try to register functions via the api or internal objects
Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
In JS implementations, functions without an explicit return value implicitly return null
Use ctx.runQuery from queries/mutations/actions to call a query
Use ctx.runMutation from mutations/actions to call a mutation
Use ctx.runAction from actions to call an action
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise extract shared helper code
Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
When calling a function in the same file via ctx.run*, add an explicit return type annotation at the call site to avoid TS circularity
Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Respect file-based routing for function references: e.g., convex/example.ts export f -> api.example.f; nested paths map to dot-separated namespaces
For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
v.bigint() is deprecated; use v.int64() for signed 64-bit integers
Use v.record(keys, values) for record-like data; v.map() and v.set() are not supported
For full-text search, use withSearchIndex("ind...
Files:
convex/messages.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
convex/messages.tsapp/hooks/useChatHandlers.tslib/chat/chat-processor.ts
🧬 Code graph analysis (2)
app/hooks/useChatHandlers.ts (1)
app/components/DataStreamProvider.tsx (1)
useDataStream(40-46)
lib/chat/chat-processor.ts (1)
lib/moderation.ts (1)
getModerationResult(5-62)
🔇 Additional comments (4)
convex/messages.ts (1)
646-649: Benign no-op on missing message looks good; aligns with returns: v.null().This prevents spurious client errors when the target message was removed/edited. No further action needed.
Confirm the client treats a null return here as a benign outcome (no toast/error). If not, I can help adjust the client handler.
app/hooks/useChatHandlers.ts (3)
44-44: LGTM: chatMode rename and usage is consistent.Consistently sourcing
chatModefrom global state improves clarity and avoids stale literals.
126-126: LGTM: mode is consistently passed as chatMode across submit/regenerate/retry/edit flows.Ensures server receives a uniform
modesignal everywhere.Also applies to: 139-139, 194-194, 216-216, 290-290
249-257: StrengthenmessageIdtyping toId<"messages">
- Change the handler signature to
const handleEditMessage = async (messageId: Id<"messages">, newContent: string) => { … }- Remove the cast and pass directly:
- await regenerateWithNewContent({ messageId: messageId as Id<"messages">, newContent });
- await regenerateWithNewContent({ messageId, newContent });
- Verify
ChatMessage.idis alreadyId<"messages">(update upstream if it’s stillstring)- (Optional) Migrate any
string[]ID arrays (e.g.idsToClean) toId<"messages">[]- Confirm your Convex table is named
"messages"
| // Only log unexpected errors. "Message not found" is treated as a benign no-op above. | ||
| if ( | ||
| !( | ||
| error instanceof Error && | ||
| (error.message.includes("Message not found") || | ||
| error.message.includes("CHAT_NOT_FOUND") || | ||
| error.message.includes("CHAT_UNAUTHORIZED")) | ||
| ) | ||
| ) { | ||
| console.error("Failed to regenerate with new content:", error); | ||
| } | ||
| // Do not surface benign errors to the client | ||
| if ( | ||
| error instanceof Error && | ||
| error.message.includes("Message not found") | ||
| ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Don’t detect Convex access errors via error.message; use ConvexError.data.code.
String matching won’t catch ConvexError codes (CHAT_NOT_FOUND/CHAT_UNAUTHORIZED). This will log expected access errors noisily and defeats the intended silencing.
Apply this diff to use ConvexError codes and drop the unreachable “Message not found” fallback:
- // Only log unexpected errors. "Message not found" is treated as a benign no-op above.
- if (
- !(
- error instanceof Error &&
- (error.message.includes("Message not found") ||
- error.message.includes("CHAT_NOT_FOUND") ||
- error.message.includes("CHAT_UNAUTHORIZED"))
- )
- ) {
- console.error("Failed to regenerate with new content:", error);
- }
- // Do not surface benign errors to the client
- if (
- error instanceof Error &&
- error.message.includes("Message not found")
- ) {
- return null;
- }
+ // Only log unexpected errors. Silences Convex access errors.
+ if (
+ !(
+ error instanceof ConvexError &&
+ (error.data?.code === "CHAT_NOT_FOUND" ||
+ error.data?.code === "CHAT_UNAUTHORIZED")
+ )
+ ) {
+ console.error("Failed to regenerate with new content:", error);
+ }
+ // "Message not found" is already handled above as a no-op.📝 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.
| // Only log unexpected errors. "Message not found" is treated as a benign no-op above. | |
| if ( | |
| !( | |
| error instanceof Error && | |
| (error.message.includes("Message not found") || | |
| error.message.includes("CHAT_NOT_FOUND") || | |
| error.message.includes("CHAT_UNAUTHORIZED")) | |
| ) | |
| ) { | |
| console.error("Failed to regenerate with new content:", error); | |
| } | |
| // Do not surface benign errors to the client | |
| if ( | |
| error instanceof Error && | |
| error.message.includes("Message not found") | |
| ) { | |
| return null; | |
| } | |
| // Only log unexpected errors. Silences Convex access errors. | |
| if ( | |
| !( | |
| error instanceof ConvexError && | |
| (error.data?.code === "CHAT_NOT_FOUND" || | |
| error.data?.code === "CHAT_UNAUTHORIZED") | |
| ) | |
| ) { | |
| console.error("Failed to regenerate with new content:", error); | |
| } | |
| // "Message not found" is already handled above as a no-op. |
🤖 Prompt for AI Agents
In convex/messages.ts around lines 705 to 722, the review points out we should
not detect Convex access errors by matching error.message; instead check for
ConvexError and its data.code values (e.g. "CHAT_NOT_FOUND" and
"CHAT_UNAUTHORIZED") and remove the unreachable "Message not found" string
fallback. Update the error handling to: 1) import/recognize ConvexError and test
error instanceof ConvexError, 2) check error.data?.code for the specific access
codes to suppress logging and to return null for the benign case, and 3) only
console.error for truly unexpected errors; drop the message-string matching
branch entirely.
| // Check moderation for the last user message | ||
| const moderationResult = await getModerationResult( | ||
| messagesWithUrls, | ||
| subscription === "pro" || subscription === "ultra", | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Unconditional moderation now sends content for free tier—gate behind feature flag/consent.
This change will send all user content to OpenAI for moderation regardless of tier. Verify privacy/legal posture and add a runtime kill‑switch and explicit intent.
Apply this diff to add a flag and avoid moderation when disabled:
- // Check moderation for the last user message
- const moderationResult = await getModerationResult(
- messagesWithUrls,
- subscription === "pro" || subscription === "ultra",
- );
+ // Check moderation for the last user message
+ const isPaidTier = subscription === "pro" || subscription === "ultra";
+ const shouldModerate =
+ isPaidTier || process.env.ENABLE_MODERATION_ALL_TIERS === "true";
+ const moderationResult = shouldModerate
+ ? await getModerationResult(messagesWithUrls, isPaidTier)
+ : { shouldUncensorResponse: false };🏁 Script executed:
#!/bin/bash
# Search for definitions and usages of getModerationResult in the codebase
rg -n "getModerationResult" -C3 --glob "*.ts"Length of output: 1404
Gate moderation behind a paid-tier check or explicit flag
lib/chat/chat-processor.ts:102-106 always sends free-tier content to OpenAI. Add a runtime kill-switch and opt-in requirement:
- // Check moderation for the last user message
- const moderationResult = await getModerationResult(
- messagesWithUrls,
- subscription === "pro" || subscription === "ultra",
- );
+ // Check moderation for the last user message
+ const isPaidTier = subscription === "pro" || subscription === "ultra";
+ const shouldModerate =
+ isPaidTier || process.env.ENABLE_MODERATION_ALL_TIERS === "true";
+ const moderationResult = shouldModerate
+ ? await getModerationResult(messagesWithUrls, isPaidTier)
+ : { shouldUncensorResponse: false };📝 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.
| // Check moderation for the last user message | |
| const moderationResult = await getModerationResult( | |
| messagesWithUrls, | |
| subscription === "pro" || subscription === "ultra", | |
| ); | |
| // Check moderation for the last user message | |
| const isPaidTier = subscription === "pro" || subscription === "ultra"; | |
| const shouldModerate = | |
| isPaidTier || process.env.ENABLE_MODERATION_ALL_TIERS === "true"; | |
| const moderationResult = shouldModerate | |
| ? await getModerationResult(messagesWithUrls, isPaidTier) | |
| : { shouldUncensorResponse: false }; |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes