-
Notifications
You must be signed in to change notification settings - Fork 51
Daily branch 2025 08 26 #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds OpenAI moderation and a chat preprocessing pipeline, introduces message editing/regeneration (frontend + Convex), changes systemPrompt signature/content and model selection, narrows tools in "ask" mode, switches default chat mode to "ask", replaces markdown renderer with Streamdown, adds OPENAI_API_KEY and new deps, and updates CSS/docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant R as API Route (/api/chat)
participant P as Chat Processor
participant M as Moderation (OpenAI)
participant A as Analytics (PostHog)
participant L as Model Provider
U->>R: POST /api/chat (messages, mode, userID)
R->>P: processChatMessages({ messages, mode, userID, posthog })
P->>P: truncateMessagesToTokenLimit(...)
P->>M: getModerationResult(messages)
M-->>P: { shouldUncensorResponse }
alt shouldUncensorResponse == true
P->>P: addAuthMessage(messages)
end
opt posthog present
P->>A: capture("hackerai-" + mode, { distinctId: userID })
end
P-->>R: { executionMode, truncatedMessages, shouldUncensorResponse }
R->>R: systemPrompt(mode, executionMode)
R->>L: languageModel("agent-model").streamText(truncatedMessages, ...)
L-->>R: stream of tokens
R-->>U: SSE stream (assistant response)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (10)
app/contexts/GlobalState.tsx (1)
71-71: Default mode switched to "ask": consider persisting the last user choiceGood alignment with the new flow. To avoid surprising resets across sessions, persist and rehydrate the mode from localStorage.
Apply within this line to lazily initialize from storage:
- const [mode, setMode] = useState<ChatMode>("ask"); + const [mode, setMode] = useState<ChatMode>(() => { + if (typeof window !== "undefined") { + const saved = window.localStorage.getItem("mode") as ChatMode | null; + if (saved === "agent" || saved === "ask") return saved; + } + return "ask"; + });Add (outside this hunk) to persist changes:
useEffect(() => { if (typeof window !== "undefined") { window.localStorage.setItem("mode", mode); } }, [mode]);Please confirm there are no downstream assumptions that depended on "agent" being the initial mode (e.g., systemPrompt defaults, tool gating).
README.md (1)
67-67: OpenAI (Moderation) entry added: check for duplication and reference env key
- Looks good. Ensure README lists OpenAI only once under Required Services (the PR summary mentioned two entries).
- Consider adding a short note that it uses
OPENAI_API_KEYand that the app fails soft if the key is missing (e.g., moderation disabled or safe default).If duplication exists elsewhere in README, I can open a small PR to dedupe and add the env hint inline.
lib/ai/tools/index.ts (1)
63-66: Avoid constructing all tools in "ask" mode; lazily instantiate to reduce bundle size and edge runtime risksRight now, all tools are created upfront and then filtered, which can:
- Pull in heavy Node-only dependencies even when not exposed (risk for edge runtimes).
- Add unnecessary init cost in "ask" mode.
Refactor to build only what's needed per mode.
Apply this diff in the selected lines to remove the dependency on
allToolsfor ask-mode and instantiate only the allowed tool(s):- ? { - // read_file: allTools.read_file, - // todo_write: createTodoWrite(context), - ...(process.env.EXA_API_KEY && { web_search: allTools.web_search }), - } + ? { + // If you later re-enable these in "ask", use direct factories for consistency: + // read_file: createReadFile(context), + // todo_write: createTodoWrite(context), + ...(process.env.EXA_API_KEY && { web_search: createWebSearchTool(context) }), + }And (outside this hunk) simplify by removing the
allToolsobject and inlining creation in the else-branch:- // Create all available tools - const allTools = { - run_terminal_cmd: createRunTerminalCmd(context), - read_file: createReadFile(context), - write_file: createWriteFile(context), - delete_file: createDeleteFile(context), - search_replace: createSearchReplace(context), - multi_edit: createMultiEdit(context), - todo_write: createTodoWrite(context), - ...(process.env.EXA_API_KEY && { - web_search: createWebSearchTool(context), - }), - }; + // Lazily build tools per mode to avoid initializing unused heavy deps- : allTools; + : { + run_terminal_cmd: createRunTerminalCmd(context), + read_file: createReadFile(context), + write_file: createWriteFile(context), + delete_file: createDeleteFile(context), + search_replace: createSearchReplace(context), + multi_edit: createMultiEdit(context), + todo_write: createTodoWrite(context), + ...(process.env.EXA_API_KEY && { + web_search: createWebSearchTool(context), + }), + };If the chat route runs on Edge, this refactor becomes important to prevent Node-only modules from being bundled.
.env.local.example (1)
21-21: Add OPENAI_API_KEY: order and docs nits
- dotenv-linter suggests alphabetic ordering; place OPENAI_API_KEY before OPENROUTER_API_KEY.
- Consider adding a brief comment to clarify that OpenAI is used exclusively for moderation.
Apply:
-OPENROUTER_API_KEY=your_openrouter_api_key_here -OPENAI_API_KEY=your_openai_api_key_here +OPENAI_API_KEY=your_openai_api_key_here +OPENROUTER_API_KEY=your_openrouter_api_key_hereOptionally:
+# OpenAI (Moderation) +OPENAI_API_KEY=your_openai_api_key_herelib/system-prompt.ts (1)
31-114: Confirm intent: large block (tool_calling + other guidance) hidden when mode === "ask".Right now, when mode is "ask", the prompt omits not only <tool_calling>, but also:
- <maximize_context_understanding>
- <making_code_changes>
- <inline_line_numbers>
- <task_management>
- the final “Answer the user’s request…” instruction
- optional <sandbox_environment>
If “ask” should still get general comprehension and formatting guidance (but not tool-calling specifics), consider hoisting those sections outside the mode !== "ask" guard, and keep only <tool_calling> conditional. Otherwise “ask” conversations may underperform in code understanding/formatting and never see sandbox details.
lib/moderation.ts (3)
5-7: Type safety: preferUIMessage[]overany[].This improves readability and reduces downstream casting. Import the type once and use it across helpers.
+import type { UIMessage } from "ai"; @@ -export async function getModerationResult( - messages: any[], -): Promise<{ shouldUncensorResponse: boolean }> { +export async function getModerationResult( + messages: UIMessage[], +): Promise<{ shouldUncensorResponse: boolean }> {Also applies to: 1-1
14-15: Perf: reuse OpenAI client between calls.Creating a new client per request adds overhead. Cache it after the first successful construction.
const MODERATION_CHAR_LIMIT = 1000; +let openaiClient: OpenAI | null = null; @@ - const openai = new OpenAI({ apiKey: openaiApiKey }); + if (!openaiClient) { + openaiClient = new OpenAI({ apiKey: openaiApiKey }); + }And later:
- const moderation = await openai.moderations.create({ + const moderation = await openaiClient.moderations.create({Also applies to: 3-3
117-129: Validate category keys againstomni-moderation-latest.Category names can drift between models/versions. If they change, your forbidden filter may silently stop matching. Consider mapping with defensive defaults or verifying at startup.
Option: allow partial matches for “/minors”, “/graphic”, etc., or centralize keys in a constants module with unit tests.
Also applies to: 134-142
lib/chat/chat-processor.ts (1)
52-55: Validate env var value instead of blind cast.Avoid propagating invalid execution modes when TERMINAL_EXECUTION_MODE is misconfigured.
- const executionMode: ExecutionMode = - (process.env.TERMINAL_EXECUTION_MODE as ExecutionMode) || "local"; + const envMode = process.env.TERMINAL_EXECUTION_MODE; + const executionMode: ExecutionMode = + envMode === "sandbox" || envMode === "local" ? (envMode as ExecutionMode) : "local";app/api/chat/route.ts (1)
98-101: Sanity check: model selection string.Confirm that myProvider.languageModel("agent-model") resolves to the intended model for all modes (including “ask”). If it should vary by mode, consider passing mode through and mapping to model id.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.env.local.example(1 hunks)README.md(1 hunks)app/api/chat/route.ts(4 hunks)app/contexts/GlobalState.tsx(1 hunks)lib/ai/tools/index.ts(1 hunks)lib/chat/chat-processor.ts(1 hunks)lib/moderation.ts(1 hunks)lib/system-prompt.ts(4 hunks)package.json(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.local.example
[warning] 21-21: [UnorderedKey] The OPENAI_API_KEY key should go before the OPENROUTER_API_KEY key
(UnorderedKey)
🔇 Additional comments (4)
lib/system-prompt.ts (1)
11-12: API change verified—no remaining outdated call sitesAll usages of
systemPrompthave been updated to the new two-parameter signature(mode, executionMode), and there are no references in test or spec files.• Updated signature in
lib/system-prompt.ts
• Call site inapp/api/chat/route.tsnow correctly invokessystemPrompt(mode, executionMode)
• No three-argument usages found
• No references in any.test.ts[x]or.spec.ts[x]fileslib/chat/chat-processor.ts (1)
65-70: Analytics placement LGTM.Event names use the mode and tool names consistently; null-safe check on posthog is good.
app/api/chat/route.ts (2)
69-77: Migration to centralized processor looks good.
- systemPrompt signature updated at call-site.
- executionMode and truncatedMessages sourced from the processor.
- Analytics moved out except for tool-call events.
Also applies to: 98-106
120-131: Good operational hygiene.Pausing the sandbox on both onError and onFinish prevents resource leaks. Nice.
Also applies to: 145-147
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/components/MessagePartHandler.tsx (1)
36-36:keyon child component is likely ineffective here.Unless this element participates in an array at this level, React won’t use the
keyfor reconciliation. Prefer placing thekeyon the array element in the parent that maps over parts, or drop it here.Apply this minimal change if you aren’t relying on a forced remount:
- return <MemoizedMarkdown key={partId} content={part.text ?? ""} />; + return <MemoizedMarkdown content={part.text ?? ""} />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
app/components/MemoizedMarkdown.tsx(1 hunks)app/components/MessagePartHandler.tsx(1 hunks)app/components/Messages.tsx(0 hunks)app/globals.css(1 hunks)components/ui/table-components.tsx(0 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (2)
- app/components/Messages.tsx
- components/ui/table-components.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
app/components/MemoizedMarkdown.tsx (2)
app/components/CodeHighlight.tsx (1)
CodeHighlight(12-80)components/ui/link-with-tooltip.tsx (1)
LinkWithTooltip(11-43)
app/components/MessagePartHandler.tsx (1)
app/components/MemoizedMarkdown.tsx (1)
MemoizedMarkdown(6-32)
🔇 Additional comments (2)
app/globals.css (1)
3-3: Streamdown stylesheet hook looks correct; verify your CSS pipeline recognizes@source.Adding
@source "../node_modules/streamdown/dist/index.js";matches Streamdown’s setup instructions and is the intended way to pull in its Tailwind styles. If you see build-time warnings about an unknown at-rule, confirm your CSS toolchain (e.g., Next.js with Lightning CSS) preserves custom at-rules or follow Streamdown’s alternative guidance if they publish one. (npmjs.com)app/components/MemoizedMarkdown.tsx (1)
10-11: ConfirmCodeHighlightreceives an inline/block signal from Streamdown.Your
CodeHighlightinfers inline-ness from anodeprop. Ensure Streamdown passes the same shape asreact-markdown; otherwise inline code may render as block. If needed, adaptCodeHighlightto also accept aninlineboolean or infer inline when no language/class is present. (npmjs.com, streamdown.ai)Here’s an example tweak for CodeHighlight (outside this file) to be more defensive:
// In app/components/CodeHighlight.tsx export const CodeHighlight = ({ className, children, node, inline, ...props }: any) => { const match = className?.match(/language-(\w+)/); const language = match ? match[1] : undefined; // Prefer explicit inline flag if provided by the renderer, else fall back to heuristics const isInline = typeof inline === "boolean" ? inline : node ? isInlineCode(node) : !language && !String(children).includes("\n"); ... };
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
convex/messages.ts (1)
19-26: Public mutation allows unauthenticated writes whenserviceKeyis omittedRight now, anyone can call
saveMessagewithout a validserviceKeyand without user auth. This is a security issue: it enables arbitrary writes to anychat_id. If this endpoint is intended for server-to-server use, require a validserviceKey. If it must be callable by clients, enforce user auth + chat ownership likesaveMessageFromClient.Suggested hardening (require service key explicitly):
- // Verify service role key - if ( - args.serviceKey && - args.serviceKey !== process.env.CONVEX_SERVICE_ROLE_KEY - ) { - throw new Error("Unauthorized: Invalid service key"); - } + // Enforce server-to-server usage only + if ( + !args.serviceKey || + args.serviceKey !== process.env.CONVEX_SERVICE_ROLE_KEY + ) { + throw new Error("Unauthorized: Missing or invalid service key"); + }If you need dual-mode behavior (server key OR authenticated user), I can provide a minimal refactor that supports both paths safely.
🧹 Nitpick comments (13)
app/components/MessageEditor.tsx (4)
31-35: Preserve user-intended whitespace; only use trim for emptiness checkRight now you save the trimmed content, which can alter formatting (e.g., code blocks or deliberate leading/trailing spaces). Use trim only for the emptiness guard and pass the original content to onSave.
const handleSave = () => { - if (content.trim()) { - onSave(content.trim()); - } + if (content.trim()) { + onSave(content); + } };
48-56: Add an accessible name to the textareaScreen readers have no label for this editable region. Add an aria-label (or associate a visible label) to improve accessibility.
<TextareaAutosize ref={textareaRef} value={content} onChange={(e) => setContent(e.target.value)} onKeyDown={handleKeyDown} - className="w-full p-3 text-foreground rounded-md resize-none focus:outline-none" + className="w-full p-3 text-foreground rounded-md resize-none focus:outline-none" + aria-label="Edit message" minRows={2} maxRows={10} />
61-63: Avoid no-op savesOptionally disable Save when the content hasn't changed from the initial value to prevent redundant mutations/UI churn.
- <Button size="sm" onClick={handleSave} disabled={!content.trim()}> + <Button + size="sm" + onClick={handleSave} + disabled={!content.trim() || content === initialContent} + > Save </Button>
37-44: Type the keyboard event target to the textarea element (minor)If you want stricter typing, use React.KeyboardEvent. Current typing works but is less specific.
lib/utils/message-utils.ts (4)
13-18: Prefer nullish coalescing over || when mapping textUsing ?? avoids treating valid falsy values differently and is idiomatic for possibly-undefined fields.
return parts .filter((part) => part.type === "text") - .map((part) => part.text || "") + .map((part) => part.text ?? "") .join("");
20-30: Function name vs. behavior mismatchhasTextContent returns true for step-start and tool-* parts even without text. If this is intentional (e.g., “there’s meaningful content or activity”), consider renaming to hasRenderableContent or clarify in the docstring to avoid confusion for future callers.
35-40: Simpler last-assistant lookup (optional)Current map/reverse/find is fine, but a reverse for-loop avoids allocations.
Example alternative:
export const findLastAssistantMessageIndex = ( messages: Array<{ role: string }> ): number | undefined => { for (let i = messages.length - 1; i >= 0; i--) { if (messages[i].role === "assistant") return i; } return undefined; };
5-8: Consider tightening types (optional)MessagePart.type could be a discriminated union like 'text' | 'step-start' |
tool-${string}to catch typos at compile time. Non-blocking but improves safety.app/components/MessageErrorState.tsx (2)
35-41: Prevent reverse tabnabbing when opening external linksPass noopener,noreferrer as the features argument to window.open to prevent the new page from gaining access to window.opener.
- onClick={() => - window.open("https://github.com/hackerai-tech/hackerai", "_blank") - } + onClick={() => + window.open( + "https://github.com/hackerai-tech/hackerai", + "_blank", + "noopener,noreferrer" + ) + }
18-18: Add live-region semantics for accessibilityMark the error container as an alert so screen readers announce it when it appears.
- <div className="bg-destructive/10 border border-destructive/20 rounded-lg p-3"> + <div + className="bg-destructive/10 border border-destructive/20 rounded-lg p-3" + role="alert" + aria-live="polite" + >app/components/MessageActions.tsx (2)
57-71: Prevent accidental form submission: settype="button"on non-submit controlsIf these buttons ever render inside a
<form>, their default type is"submit". Settype="button"explicitly.trigger={ <button - onClick={handleCopy} + type="button" + onClick={handleCopy} className="p-1.5 opacity-70 hover:opacity-100 transition-opacity rounded hover:bg-secondary text-muted-foreground" aria-label={copied ? "Copied!" : "Copy message"} >
73-88: Same here: make Edit a non-submit controltrigger={ <button - onClick={onEdit} + type="button" + onClick={onEdit} className="p-1.5 opacity-70 hover:opacity-100 transition-opacity rounded hover:bg-secondary text-muted-foreground" aria-label="Edit message" >app/components/Messages.tsx (1)
58-59: Minor: remove stray empty linesThere are a couple of redundant blank lines after the
useEffectblock. Not a blocker.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
app/components/MessageActions.tsx(3 hunks)app/components/MessageEditor.tsx(1 hunks)app/components/MessageErrorState.tsx(1 hunks)app/components/Messages.tsx(6 hunks)app/components/chat.tsx(2 hunks)app/globals.css(2 hunks)app/hooks/useChatHandlers.ts(4 hunks)convex/chats.ts(1 hunks)convex/messages.ts(6 hunks)lib/utils/message-utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/components/MessageErrorState.tsx (2)
lib/errors.ts (1)
ChatSDKError(36-74)app/components/MemoizedMarkdown.tsx (1)
MemoizedMarkdown(6-32)
convex/messages.ts (3)
convex/_generated/api.js (2)
internal(22-22)internal(22-22)convex/_generated/server.js (2)
mutation(49-49)mutation(49-49)convex/_generated/dataModel.d.ts (1)
Id(48-49)
convex/chats.ts (1)
convex/_generated/server.js (2)
internalQuery(39-39)internalQuery(39-39)
app/components/MessageActions.tsx (2)
types/chat.ts (1)
ChatStatus(56-56)components/ui/with-tooltip.tsx (1)
WithTooltip(16-46)
app/components/chat.tsx (1)
app/hooks/useChatHandlers.ts (1)
useChatHandlers(23-138)
app/hooks/useChatHandlers.ts (3)
types/chat.ts (1)
ChatMessage(58-58)convex/messages.ts (3)
deleteLastAssistantMessage(154-190)saveMessageFromClient(113-149)regenerateWithNewContent(195-251)convex/_generated/dataModel.d.ts (1)
Id(48-49)
🔇 Additional comments (16)
app/globals.css (2)
120-120: Dark sidebar color tweak looks good.Changing
--sidebarto#0A0A0Areduces the harsh contrast of pure black while maintaining strong contrast with--sidebar-foreground: #ffffff. No issues from a readability standpoint.
3-3: Remove non-standard @source rule from globals.cssThe
@source "../node_modules/streamdown/dist/index.js";at-rule is not valid CSS and will be ignored by browsers (and can break your build). Streamdown is already imported in your React components—no CSS import is provided by the package—so you can safely delete this line.• File
app/globals.css, line 3: remove the invalid rule@@ app/globals.css -@source "../node_modules/streamdown/dist/index.js";app/components/MessageEditor.tsx (1)
46-67: Solid UX and keyboard affordancesAuto-focus, select-all, and Cmd/Ctrl+Enter save are implemented cleanly. Save-button disable logic for empty content is correct.
convex/chats.ts (1)
1-1: Import usage looks correctPulling internalQuery alongside query/mutation is appropriate for the ownership verifier.
app/components/MessageErrorState.tsx (2)
21-26: Verify markdown rendering is sanitized for untrusted error contenterror.cause may originate from external systems. Ensure Streamdown escapes HTML and prevents script injection. If not guaranteed, render as plain text or restrict to a safe subset.
Would you like me to check Streamdown’s sanitization guarantees and propose a safe renderer fallback?
10-33: Clear UX for rate-limit vs generic errorsDifferentiation and actionable buttons look good. The “Try Again” copy for rate limits is apt.
app/components/chat.tsx (2)
198-210: Wiring for message editing looks correctPassing setMessages into useChatHandlers and exposing handleEditMessage is consistent with the new editing flow.
256-266: Potential state sync race after local editAfter handleEditMessage trims local messages, the messagesData sync effect may later overwrite local state if Convex hasn’t reflected the edit yet. This could cause a brief flicker/revert. Consider guarding the sync (e.g., track a local “justEdited” ref and skip one sync cycle) or ensure the mutation updates Convex immediately before regenerate.
I can put together a small change that toggles a ref like justEditedRef.current = true in handleEditMessage and clears it in the messagesData sync effect to avoid one overwrite. Want me to draft it?
Also applies to: 262-263
convex/messages.ts (3)
78-87: Swallowing ownership errors changes semantics; confirm intentWrapping
verifyChatOwnershipin a nested try/catch and returning[]masks both “chat not found” and “unauthorized” as empty results. That’s fine if you want to avoid leaking resource existence, but it also prevents the client from differentiating auth failures in this query.If the intent is to hide non-existence but surface unauthorized, consider rethrowing when the error indicates unauthorized and returning
[]only for not-found. This requiresverifyChatOwnershipto emit distinguishable errors.
129-134: Centralized ownership check here is solidUsing
internal.chats.verifyChatOwnershipbefore inserts is the right move and keeps the enforcement consistent across mutations.
167-171: Ownership verification before destructive action looks goodGood to see the same centralized check applied before deleting the last assistant message.
app/hooks/useChatHandlers.ts (1)
42-43: Good: wires new mutation for the edit/regenerate flowHooking
api.messages.regenerateWithNewContenthere keeps the UI logic thin and consistent with server-side ownership checks.app/components/MessageActions.tsx (1)
47-50: Action visibility logic is clear and user-friendlyHiding actions while streaming and while editing keeps the UI clean. Condition reads well.
app/components/Messages.tsx (3)
99-106: Good: centralized text extraction and loader gatingUsing
extractMessageText/hasTextContentavoids duplicating parsing logic and makes the streaming loader condition precise.
113-141: Editing UI is gated to user messages and preserves structureConditionally rendering
MessageEditoronly for user messages is correct, and the non-edit path keepsMessagePartHandlerunchanged.
152-161: Props wiring to MessageActions looks consistent with the new APIPassing
messageText,onEdit, andisEditingkeeps actions in sync with the editor state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/components/Messages.tsx (2)
19-20: Type onEditMessage as async to reflect real usage and allow error propagationThe edit operation is async downstream. Type it as returning Promise so callers can await and handle failures.
- onEditMessage: (messageId: string, newContent: string) => void; + onEditMessage: (messageId: string, newContent: string) => Promise<void>;
67-75: Await the async edit and keep the editor open on failureCurrently not awaiting the async edit risks unhandled rejections and UI drift (editor closes even if save fails).
- const handleSaveEdit = useCallback( - (newContent: string) => { - if (editingMessageId) { - onEditMessage(editingMessageId, newContent); - } - setEditingMessageId(null); - }, - [editingMessageId, onEditMessage], - ); + const handleSaveEdit = useCallback( + async (newContent: string) => { + try { + if (editingMessageId) { + await onEditMessage(editingMessageId, newContent); + setEditingMessageId(null); + } + } catch (e) { + console.error("Failed to save edit:", e); + // Keep editor open so the user can retry/cancel + } + }, + [editingMessageId, onEditMessage], + );
🧹 Nitpick comments (10)
app/components/Messages.tsx (2)
62-66: Guard editing to user messages onlyIf MessageActions ever exposes “Edit” for assistant/system messages, the handler will set editing state but no editor renders. Add a guard (and pass isUser) to prevent confusing UX.
- const handleStartEdit = useCallback((messageId: string) => { - setEditingMessageId(messageId); - }, []); + const handleStartEdit = useCallback( + (messageId: string, isUser: boolean) => { + if (!isUser) return; // only user messages are editable + setEditingMessageId(messageId); + }, + [], + );- onEdit={() => handleStartEdit(message.id)} + onEdit={() => handleStartEdit(message.id, isUser)}Please also confirm MessageActions only shows the edit action when isUser is true.
Also applies to: 159-166
81-88: Nit: avoid per-item inline arrow handlers in large listsThe inline closures created for onMouseEnter per message can add overhead with very long transcripts. Not urgent, but consider event delegation or prebinding handlers if list sizes get large.
Also applies to: 117-118
app/components/ChatSidebar.tsx (2)
154-160: Mobile overlay header: avoid double padding and consider restoring divider; add a11y label for the icon-only control
- Removing the bottom border may make the header blend into the list. Also, the wrapper has
p-2andChatSidebarHeaderitself usesp-2, causing stacked padding.- Suggestion: let the inner component control spacing and use the wrapper only for the divider.
Apply this minimal diff to the wrapper:
- <div className="p-2"> + <div className="border-b border-sidebar-border">Additionally (outside this hunk), consider labeling the icon-only close/collapse button for screen readers:
// In ChatSidebarHeader (outside this diff) <Button variant="ghost" size="sm" className="h-7 w-7 p-0" onClick={handleCloseSidebar} aria-label={isMobile ? "Close chat sidebar" : "Collapse sidebar"} title={isMobile ? "Close chat sidebar" : "Collapse sidebar"} > <PanelLeft className="size-5 text-muted-foreground" aria-hidden="true" /> </Button>
163-171: Scrollable list in flex container: add min-h-0 (and optional overscroll containment) for reliable inner scrollingIn flex layouts, children with
overflow-y-autocan fail to scroll unless an ancestor setsmin-height: 0(or has overflow clipped). You already haveoverflow-hiddenon the parent; addingmin-h-0makes the behavior more robust across browsers. Optionaloverscroll-containprevents scroll chaining on mobile.Proposed diff:
- <div className="flex-1 overflow-hidden"> + <div className="flex-1 min-h-0 overflow-hidden"> <div className="h-full overflow-y-auto"> + <div className="h-full overflow-y-auto overscroll-contain"> <ChatSidebarList chats={chats} currentChatId={currentChatId} handleNewChat={handleNewChat} /> </div>app/components/Header.tsx (3)
33-49: Prefer client-side navigation via Link over window.location for better UX and accessibilityUse Next.js navigation to avoid full page reloads, preserve history state, and allow middle-click/open-in-new-tab semantics.
Apply this within the selected lines:
- <Button - onClick={handleSignIn} - variant="default" - size="default" - className="min-w-[74px] rounded-[10px]" - > - Sign in - </Button> + <Button + asChild + variant="default" + size="default" + className="min-w-[74px] rounded-[10px]" + > + <Link href="/login">Sign in</Link> + </Button> - <Button - onClick={handleSignUp} - variant="outline" - size="default" - className="min-w-16 rounded-[10px]" - > - Sign up - </Button> + <Button + asChild + variant="outline" + size="default" + className="min-w-16 rounded-[10px]" + > + <Link href="/signup">Sign up</Link> + </Button>Outside these lines, add the import:
import Link from "next/link";Optional follow-up: remove now-unused handleSignIn/handleSignUp.
37-45: Nit: inconsistent min-width utilities; verify Tailwind support for min-w-16You’re mixing min-w-[74px] and min-w-16. If your Tailwind config doesn’t extend minWidth with spacing scale, min-w-16 may compile to nothing. For consistency and predictability, use the same explicit value or an agreed token.
- className="min-w-16 rounded-[10px]" + className="min-w-[74px] rounded-[10px]"If you prefer spacing tokens, ensure tailwind.config.ts maps theme.spacing to minWidth; otherwise stick with bracketed values.
62-79: Repeat: use Link for mobile auth buttons to keep SPA navigation consistentSame rationale as desktop. This also lets users long-press to open in a new tab on mobile.
- <Button - onClick={handleSignIn} - variant="default" - size="sm" - className="rounded-[10px]" - > - Sign in - </Button> + <Button asChild variant="default" size="sm" className="rounded-[10px]"> + <Link href="/login">Sign in</Link> + </Button> - <Button - onClick={handleSignUp} - variant="outline" - size="sm" - className="rounded-[10px]" - > - Sign up - </Button> + <Button asChild variant="outline" size="sm" className="rounded-[10px]"> + <Link href="/signup">Sign up</Link> + </Button>Also add/import Link as noted above if not already present.
app/components/ChatHeader.tsx (3)
64-81: Desktop auth controls: switch to Link navigation and align widths
- Use Link for client-side transitions (avoid full reloads).
- Align min-widths or use tokens consistently (see Header.tsx note).
- <Button - onClick={handleSignIn} - variant="default" - size="default" - className="min-w-[74px] rounded-[10px]" - > - Sign in - </Button> + <Button asChild variant="default" size="default" className="min-w-[74px] rounded-[10px]"> + <Link href="/login">Sign in</Link> + </Button> - <Button - onClick={handleSignUp} - variant="outline" - size="default" - className="min-w-16 rounded-[10px]" - > - Sign up - </Button> + <Button asChild variant="outline" size="default" className="min-w-[74px] rounded-[10px]"> + <Link href="/signup">Sign up</Link> + </Button>Outside these lines, add:
import Link from "next/link";Optional: remove unused handleSignIn/handleSignUp afterwards.
103-120: Mobile auth controls: use Link for navigation and keep styling consistentMirrors the desktop recommendation; improves UX and consistency.
- <Button - onClick={handleSignIn} - variant="default" - size="sm" - className="rounded-[10px]" - > - Sign in - </Button> + <Button asChild variant="default" size="sm" className="rounded-[10px]"> + <Link href="/login">Sign in</Link> + </Button> - <Button - onClick={handleSignUp} - variant="outline" - size="sm" - className="rounded-[10px]" - > - Sign up - </Button> + <Button asChild variant="outline" size="sm" className="rounded-[10px]"> + <Link href="/signup">Sign up</Link> + </Button>
64-81: DRY: extract shared AuthButtons to eliminate duplication across Header and ChatHeaderBoth components render identical Sign in/Sign up button groups. Consider a small component that encapsulates the Link-based buttons and sizing variants; makes future changes (e.g., copy, sizes, A/B tests) one-touch.
I can draft the component and the usages if helpful.
Also applies to: 103-120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
app/components/ChatHeader.tsx(2 hunks)app/components/ChatSidebar.tsx(2 hunks)app/components/Header.tsx(2 hunks)app/components/Messages.tsx(6 hunks)app/globals.css(2 hunks)app/hooks/useChatHandlers.ts(4 hunks)lib/utils/message-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/utils/message-utils.ts
- app/globals.css
- app/hooks/useChatHandlers.ts
🧰 Additional context used
🧬 Code graph analysis (1)
app/components/Messages.tsx (4)
lib/utils/message-utils.ts (3)
findLastAssistantMessageIndex(35-42)extractMessageText(13-18)hasTextContent(23-30)app/components/MessageEditor.tsx (1)
MessageEditor(11-67)app/components/MessagePartHandler.tsx (1)
MessagePartHandler(16-66)app/components/MessageErrorState.tsx (1)
MessageErrorState(10-48)
🔇 Additional comments (7)
app/components/Messages.tsx (4)
37-41: Good: memoized computation of last assistant indexUsing useMemo with findLastAssistantMessageIndex avoids redundant scans on every render.
104-107: Good: centralized text extraction and content checksextractMessageText and hasTextContent keep the render logic clean and consistent with utilities.
109-111: Good: loader gating matches streaming semanticsShowing the loader only for the last assistant message while streaming and without text content is correct.
175-177: Good: dedicated error component usageReplacing inline error markup with MessageErrorState improves cohesion and reuse.
app/components/ChatSidebar.tsx (1)
97-105: The verification script has been queued. I’ll review the results once they’re available to confirm the chat object shape andChatItemprops, then update the comment accordingly. If the necessary fields line up, I'll recommend normalizing onidfor both the React key and the prop; otherwise I’ll flag any discrepancies.app/components/Header.tsx (1)
31-49: Sign-out pathway confirmedThe
Log outentry is still available in the user dropdown menu for authenticated users, invoking the/logoutroute:
- In app/components/UserDropdownMenu.tsx (lines 21–23 & 90–92),
handleSignOutredirects to/logout, and the<DropdownMenuItem>renders “Log out”.- The
/logoutAPI route is implemented in app/logout/route.ts, which callssignOut().- The
/logoutpath is also whitelisted in middleware.ts (line 10).No further changes required.
app/components/ChatHeader.tsx (1)
64-81: Sign-out flow confirmed via UserDropdownMenuThe global “Sign out” button has been removed from ChatHeader.tsx, but users can still sign out via the avatar dropdown in app/components/UserDropdownMenu.tsx (handleSignOut →
/logout→ signOut in app/logout/route.ts), satisfying the UX requirement for a clear sign-out path. No further action needed here.
- Updated JWT issuer from api.workos.com to auth.hackerai.co - Fixed authentication error: 'No auth provider found matching the given token' - JWT tokens were being issued from custom domain but Convex expected standard WorkOS domain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
convex/auth.config.ts (1)
1-1: Guard against missing WORKOS_CLIENT_ID to prevent silent misconfigurationIf WORKOS_CLIENT_ID is unset, the strings will interpolate "undefined" into issuer/JWKS URLs, causing all auth to break at runtime without a clear error. Add an explicit guard.
-const clientId = process.env.WORKOS_CLIENT_ID; +const clientId = process.env.WORKOS_CLIENT_ID; +if (!clientId) { + throw new Error("WORKOS_CLIENT_ID is not set. Auth providers cannot be configured without it."); +}
🧹 Nitpick comments (1)
convex/auth.config.ts (1)
3-20: Minor DX/maintenance: de-duplicate string assembly and consider typing the configSmall improvements to reduce future drift and catch typos early.
- DRY: Extract and normalize a single baseAuthUrl, then build issuer/JWKS from it (example shown in the previous comment).
- Types: If available, annotate authConfig with the library’s AuthConfig type to validate provider fields at compile time (import path depends on your auth package).
- Naming: Optional—if WorkOS is no longer the source-of-truth for tokens, consider renaming WORKOS_CLIENT_ID to AUTH_CLIENT_ID (keep backward compat by supporting both).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
convex/auth.config.ts(1 hunks)
🔇 Additional comments (2)
convex/auth.config.ts (2)
14-18: Manual Verification Required: JWKS Endpoint and Issuer FormatThe JWKS sanity-check script failed because the
WORKOS_CLIENT_IDenvironment variable wasn’t set, so we couldn’t confirm whether the endpoint returns keys or if the issuer URL matches token claims exactly. Please perform the following checks:
- convex/auth.config.ts (lines 14–18):
• Verify that theissuervalue exactly matches theissclaim in your user_management tokens (i.e., with or without a trailing slash as your tokens emit).- JWKS endpoint functionality:
- Set the environment variable:
export WORKOS_CLIENT_ID="<your client ID>"- Rerun the script to ensure it returns at least one key:
– Expect a numeric output ≥ 1.BASE="${AUTH_BASE_URL:-https://auth.hackerai.co}" CID="${WORKOS_CLIENT_ID}" curl -sS "${BASE}/sso/jwks/${CID}" | jq '.keys | length'Once confirmed, you can be confident that token validation won’t fail due to mismatched issuer paths or missing JWKS keys.
7-11: Align JWT issuer URLs for strictissmatchingThe first provider’s
issuerincludes a trailing slash (https://auth.hackerai.co/at line 7), whereas the second omits it (https://auth.hackerai.co/user_management/${clientId}at line 14). Because most JWT libraries compare theissclaim using strict string equality, any mismatch in trailing slash will cause validation failures.Please update the issuer URLs to use a consistent format. For example, remove the trailing slash in provider 1:
- issuer: `https://auth.hackerai.co/`, + issuer: `https://auth.hackerai.co`,Locations to update:
- convex/auth.config.ts, line 7
- convex/auth.config.ts, line 14 (if you prefer adding a trailing slash there instead, apply that change to both providers)
Manual verification required:
- Decode a valid token from each provider and confirm the
"iss"claim matches your configuredissuerexactly.- You can use this snippet (replace
<your_token_here>with a real token):export JWT="<your_token_here>" python - <<'PY' import os, json, base64 tok = os.environ["JWT"] payload = tok.split('.')[1] + '=' * (-len(tok.split('.')[1]) % 4) print(json.dumps(json.loads(base64.urlsafe_b64decode(payload)), indent=2)) PYEnsure the printed
"iss"value matches the issuer string you’ve configured above.
- Remove lazy initializer from useState to prevent conditional hook calls - Add missing dependencies to useEffect hooks to prevent stale closures - Fix chatId initialization and route change handling - Ensure proper dependency arrays for all useEffect hooks This resolves the React error #185 that was occurring when logging in and visiting the main page by ensuring hooks are called consistently and with proper dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/components/chat.tsx (4)
43-43: Prefer lazy initializer for chatIdSlight perf/readability win; avoids calling uuidv4 when id/currentChatId are already set.
- const [chatId, setChatId] = useState(id || currentChatId || uuidv4()); + const [chatId, setChatId] = useState(() => id || currentChatId || uuidv4());
63-63: Effect deps include stable setters; safe but noisyState setters are stable; including them is unnecessary unless your ESLint config requires it. If not required, trim to reduce churn.
Optional UX tweak to prevent stale todos flash on route change (clears immediately, before chatData loads):
// inside the effect body where id is handled setTodos([]); // clear while loading target chat's todos
74-74: Same deps nit; consider clearing todos on sidebar chat switchAs above, setters in deps are optional. Also consider clearing todos immediately when switching via sidebar to avoid flashing previous chat’s todos until chatData arrives.
// inside the effect when currentChatId is applied setTodos([]);
86-86: Deps expansion is harmlessIncluding setters is fine but not required; keep only if satisfying exhaustive-deps. No functional issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/components/chat.tsx(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/components/chat.tsx (1)
app/hooks/useChatHandlers.ts (1)
useChatHandlers(23-145)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
lib/utils/message-utils.ts (1)
23-30: Good fix: drop optional chaining on non-nullable typeUsing part.type.startsWith("tool-") aligns with the declared non-optional string. Matches the prior feedback.
lib/moderation.ts (1)
62-89: Bug: findTargetMessage ignores legacy string content; moderation may never runHandle both parts[] and legacy content when selecting the target message.
function findTargetMessage(messages: any[], minLength: number): any | null { let userMessagesChecked = 0; for (let i = messages.length - 1; i >= 0; i--) { const message = messages[i]; if (message.role === "user") { userMessagesChecked++; - // Handle UIMessage format with parts array - if (message.parts && Array.isArray(message.parts)) { - const textContent = message.parts - .filter((part: any) => part.type === "text") - .map((part: any) => part.text) - .join(" "); - - if (textContent.length > minLength) { - return message; - } - } + // Extract text from parts[] or legacy content + let textContent = ""; + if (Array.isArray(message.parts)) { + textContent = message.parts + .filter((part: any) => part.type === "text") + .map((part: any) => part.text ?? "") + .join(" "); + } else if (typeof message.content === "string") { + textContent = message.content; + } + if (textContent.trim().length > minLength) { + return message; + } if (userMessagesChecked >= 3) { break; // Stop after checking three user messages } } } return null; }convex/messages.ts (1)
188-247: Fix: wrong index/identifier used; add role guard; correct range deleteThe mutation queries a non-existent "by_id" index using Convex _id while the UI uses external id. Also, only user messages should be editable. Finally, range query on _creationTime must be done via filter unless your by_chat_id is composite.
Apply:
export const regenerateWithNewContent = mutation({ args: { - messageId: v.id("messages"), + messageId: v.string(), newContent: v.string(), }, returns: v.null(), handler: async (ctx, args) => { const user = await ctx.auth.getUserIdentity(); @@ - const message = await ctx.db - .query("messages") - .withIndex("by_id", (q) => - q.eq("_id", args.messageId as Id<"messages">), - ) - .first(); + const message = await ctx.db + .query("messages") + .withIndex("by_message_id", (q) => q.eq("id", args.messageId)) + .first(); if (!message) { throw new Error("Message not found"); } // Verify chat ownership await ctx.runQuery(internal.chats.verifyChatOwnership, { chatId: message.chat_id, userId: user.subject, }); + + // Only allow edits of user messages + if (message.role !== "user") { + throw new Error("Unauthorized: Only user messages can be edited"); + } await ctx.db.patch(message._id, { parts: [{ type: "text", text: args.newContent }], update_time: Date.now(), }); // Delete all messages after the given message - const messages = await ctx.db - .query("messages") - .withIndex("by_chat_id", (q) => - q - .eq("chat_id", message.chat_id) - .gt("_creationTime", message._creationTime), - ) - .collect(); + const messages = await ctx.db + .query("messages") + .withIndex("by_chat_id", (q) => q.eq("chat_id", message.chat_id)) + .filter((q) => q.gt(q.field("_creationTime"), message._creationTime)) + .collect(); for (const msg of messages) { await ctx.db.delete(msg._id); }Optionally, add idempotency on patch if concurrent edits are possible.
app/components/Messages.tsx (2)
17-26: Prop type for onEditMessage updated to async—goodMatches the async usage downstream.
68-82: Good: await edit and keep editor open on failurePrevents UI drift and handles errors gracefully.
🧹 Nitpick comments (6)
lib/utils/message-utils.ts (1)
35-42: Minor: avoid map+reverse for last assistant indexIterating from the end is simpler and avoids extra allocations.
export const findLastAssistantMessageIndex = ( messages: Array<{ role: string }>, ): number | undefined => { - return messages - .map((msg, index) => ({ msg, index })) - .reverse() - .find(({ msg }) => msg.role === "assistant")?.index; + for (let i = messages.length - 1; i >= 0; i--) { + if (messages[i].role === "assistant") return i; + } + return undefined; };lib/moderation.ts (3)
27-29: Make moderation model configurableAllow overrides via env with a sane default.
- const moderation = await openai.moderations.create({ - model: "omni-moderation-latest", + const moderation = await openai.moderations.create({ + model: process.env.OPENAI_MODERATION_MODEL ?? "omni-moderation-latest", input: input, });
123-148: Verify category key names against current SDK; consider normalizingCategory keys vary across models (e.g., underscores vs slashes). Normalize keys or broaden the forbidden set to avoid false negatives.
Proposed robust check:
- const forbiddenCategories = [ + const forbiddenCategories = [ "sexual", - "sexual/minors", + "sexual/minors", "sexual_and_minors", "hate", - "hate/threatening", + "hate/threatening", "hate_threatening", "harassment", - "harassment/threatening", + "harassment/threatening", "harassment_threatening", "self-harm", - "self-harm/intent", - "self-harm/instruction", + "self-harm/intent", "self-harm_intent", + "self-harm/instruction", "self-harm_instructions", "violence", - "violence/graphic", + "violence/graphic", "violence_graphic", ]; - const hasForbiddenCategory = hazardCategories.some((category) => - forbiddenCategories.includes(category), - ); + const norm = (s: string) => s.replace(/_/g, "-"); + const forbidden = new Set(forbiddenCategories.map(norm)); + const hasForbiddenCategory = hazardCategories.some((c) => forbidden.has(norm(c)));If you want me to confirm the current keys for "omni-moderation-latest", I can run a quick check.
31-35: Noise: console.log in fallback pathUse a debug logger or remove to avoid noisy logs in production.
- console.log("Moderation API returned no results"); + if (process.env.NODE_ENV !== "production") { + console.debug("Moderation API returned no results"); + }convex/messages.ts (1)
150-179: Query for last assistant is fine, but ensure index supports orderingIf by_chat_id isn’t composite with _creationTime, order("desc") still works but is evaluated post-index. This is acceptable for small result sets; consider filtering or pagination if chats grow large.
app/components/Messages.tsx (1)
144-152: Nit: guard against missing parts to avoid runtime errors from malformed dataIf a message without parts sneaks in, this prevents a crash.
- {message.parts.map((part, partIndex) => ( + {(message.parts ?? []).map((part, partIndex) => ( <MessagePartHandler
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
app/components/Messages.tsx(6 hunks)convex/chats.ts(5 hunks)convex/messages.ts(6 hunks)lib/chat/chat-processor.ts(1 hunks)lib/moderation.ts(1 hunks)lib/utils/message-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/chat/chat-processor.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T12:38:06.615Z
Learnt from: RostyslavManko
PR: hackerai-tech/hackerai#14
File: convex/chats.ts:4-24
Timestamp: 2025-08-27T12:38:06.615Z
Learning: Convex functions with serviceKey parameters are designed for backend service-to-service communication and don't require ctx.auth.getUserIdentity() checks. The serviceKey validation against process.env.CONVEX_SERVICE_ROLE_KEY provides sufficient authentication for these backend operations.
Applied to files:
convex/chats.ts
🧬 Code graph analysis (3)
convex/chats.ts (1)
convex/_generated/server.js (2)
internalQuery(39-39)internalQuery(39-39)
convex/messages.ts (2)
convex/chats.ts (1)
validateServiceKey(4-11)convex/_generated/api.js (2)
internal(22-22)internal(22-22)
app/components/Messages.tsx (4)
lib/utils/message-utils.ts (3)
findLastAssistantMessageIndex(35-42)extractMessageText(13-18)hasTextContent(23-30)app/components/MessageEditor.tsx (1)
MessageEditor(11-67)app/components/MessagePartHandler.tsx (1)
MessagePartHandler(16-66)app/components/MessageErrorState.tsx (1)
MessageErrorState(10-48)
🔇 Additional comments (3)
convex/chats.ts (1)
206-234: LGTM: user-scoped listing with identity guardgetUserChats correctly uses ctx.auth, index, and safe fallbacks.
convex/messages.ts (2)
21-22: Enforce presence of service key for S2S saveMessageWith the strengthened validateServiceKey, this mutation will now correctly reject missing keys. Confirm all service callers pass the key.
If any trusted caller lacks the key, either provide it or split into a client-authenticated path (you already have saveMessageFromClient).
73-104: Ownership verification flow is solidUsing internal.chats.verifyChatOwnership and returning [] for non-existent chats avoids leaks and keeps the UI stable.
Summary by CodeRabbit
New Features
Changes
Documentation
Chores