Skip to content

Conversation

@rossmanko
Copy link
Contributor

@rossmanko rossmanko commented Aug 26, 2025

Summary by CodeRabbit

  • New Features

    • Message editing: inline editor with save/cancel and retry for regenerated assistant responses.
    • OpenAI moderation gating to influence assistant replies.
  • Changes

    • Default chat mode changed to "ask"; Ask mode exposes fewer tools.
    • Streamlined markdown rendering for leaner message display.
    • Header/sidebar auth UI updated (sign-out hidden when signed in); mobile sidebar gains internal scrolling.
    • Theme color tweaks; table UI removed.
  • Documentation

    • README and env example updated to include OPENAI_API_KEY.
  • Chores

    • Added OpenAI and streamdown dependencies; removed legacy markdown packages.

@vercel
Copy link

vercel bot commented Aug 26, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hackerai Ready Ready Preview Comment Aug 27, 2025 1:05pm

@coderabbitai
Copy link

coderabbitai bot commented Aug 26, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
Env & Docs
/.env.local.example, README.md
Added OPENAI_API_KEY to env example (removed preceding comment) and appended OpenAI Moderation API entries to README tables.
API Chat Route
/app/api/chat/route.ts
Replaced local truncation/moderation/analytics with processChatMessages(...); executionMode comes from processor; model chosen inline via myProvider.languageModel("agent-model"); adjusted systemPrompt call and imports.
Chat Preprocessing
/lib/chat/chat-processor.ts
New module exporting processChatMessages(...) and addAuthMessage(...) for truncation, moderation gating, executionMode resolution, optional PostHog capture, and auth-message injection.
Moderation
/lib/moderation.ts
New OpenAI moderation helper getModerationResult(messages) using omni-moderation-latest, returning { shouldUncensorResponse } with forbidden-category checks and error handling.
System Prompt
/lib/system-prompt.ts
Signature changed to systemPrompt(mode, executionMode?); prompt content rewritten to reference HackerAI, include mode-specific agent guidance, conditional tool_calling schema, and sandbox-sensitive blocks.
Tools Exposure
/lib/ai/tools/index.ts
For mode "ask", read_file and todo_write are no longer exposed (commented/omitted); allTools definitions remain.
Global State
/app/contexts/GlobalState.tsx
Default mode value changed from "agent" to "ask".
Markdown Rendering
/app/components/MemoizedMarkdown.tsx, .../MessagePartHandler.tsx
Replaced ReactMarkdown stack with Streamdown; MemoizedMarkdown API changed from ({ content, id })({ content }); callers updated to stop passing id.
Messages & Editing UI
/app/components/Messages.tsx, MessageActions.tsx, MessageEditor.tsx, MessageErrorState.tsx, MessagePartHandler.tsx
Added message editing UI and handlers (MessageEditor), editing state/flow in Messages, MessageActions now accepts messageText plus onEdit/isEditing, and new MessageErrorState for error UI.
Chat Hook & Chat UI
/app/hooks/useChatHandlers.ts, app/components/chat.tsx
useChatHandlers extended to accept setMessages and now returns handleEditMessage; Chat wired to pass onEditMessage to Messages and setMessages into the hook.
Message Utilities
/lib/utils/message-utils.ts
New helpers: extractMessageText, hasTextContent, findLastAssistantMessageIndex, and MessagePart type.
Convex: Ownership & Regeneration
/convex/chats.ts, /convex/messages.ts
Added internal verifyChatOwnership and validateServiceKey; centralized ownership checks; added regenerateWithNewContent mutation to update message content and delete subsequent messages.
CSS & Assets
/app/globals.css
Added @source "../node_modules/streamdown/dist/index.js";, removed several custom secondary color CSS variables, and adjusted dark --sidebar color.
UI Cleanup
/components/ui/table-components.tsx
Removed file and its exported Table, Th, Td components.
Dependencies
/package.json
Added openai@^5.15.0 and streamdown@^1.1.3; removed react-markdown, rehype-mathjax, remark-gfm, and remark-math.
Auth Config
/convex/auth.config.ts
Updated issuer and JWKS endpoints from api.workos.comauth.hackerai.co for customJwt providers.

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)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

I twitch my whiskers, nibble keys,
A moderation hop and streaming breeze.
OPENAI tucked neat in env’s new line,
Edits, regenerations, prompts redesigned.
Hop, hack, and hum — this rabbit ships the shine. 🐇✨

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch daily-branch-2025-08-26

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 choice

Good 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_KEY and 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 risks

Right 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 allTools for 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 allTools object 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_here

Optionally:

+# OpenAI (Moderation)
+OPENAI_API_KEY=your_openai_api_key_here
lib/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: prefer UIMessage[] over any[].

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 against omni-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.

📥 Commits

Reviewing files that changed from the base of the PR and between f9ffdb8 and f6dc574.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is 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 sites

All usages of systemPrompt have 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 in app/api/chat/route.ts now correctly invokes systemPrompt(mode, executionMode)
• No three-argument usages found
• No references in any .test.ts[x] or .spec.ts[x] files

lib/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

Copy link

@coderabbitai coderabbitai bot left a 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: key on child component is likely ineffective here.

Unless this element participates in an array at this level, React won’t use the key for reconciliation. Prefer placing the key on 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.

📥 Commits

Reviewing files that changed from the base of the PR and between f6dc574 and e5c234d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is 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: Confirm CodeHighlight receives an inline/block signal from Streamdown.

Your CodeHighlight infers inline-ness from a node prop. Ensure Streamdown passes the same shape as react-markdown; otherwise inline code may render as block. If needed, adapt CodeHighlight to also accept an inline boolean 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");

  ...
};

Copy link

@coderabbitai coderabbitai bot left a 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 when serviceKey is omitted

Right now, anyone can call saveMessage without a valid serviceKey and without user auth. This is a security issue: it enables arbitrary writes to any chat_id. If this endpoint is intended for server-to-server use, require a valid serviceKey. If it must be callable by clients, enforce user auth + chat ownership like saveMessageFromClient.

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 check

Right 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 textarea

Screen 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 saves

Optionally 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 text

Using ?? 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 mismatch

hasTextContent 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 links

Pass 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 accessibility

Mark 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: set type="button" on non-submit controls

If these buttons ever render inside a <form>, their default type is "submit". Set type="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 control

               trigger={
                 <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 lines

There are a couple of redundant blank lines after the useEffect block. 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e5c234d and 3e3a8af.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is 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 --sidebar to #0A0A0A reduces 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.css

The @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 affordances

Auto-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 correct

Pulling 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 content

error.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 errors

Differentiation 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 correct

Passing setMessages into useChatHandlers and exposing handleEditMessage is consistent with the new editing flow.


256-266: Potential state sync race after local edit

After 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 intent

Wrapping verifyChatOwnership in 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 requires verifyChatOwnership to emit distinguishable errors.


129-134: Centralized ownership check here is solid

Using internal.chats.verifyChatOwnership before inserts is the right move and keeps the enforcement consistent across mutations.


167-171: Ownership verification before destructive action looks good

Good 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 flow

Hooking api.messages.regenerateWithNewContent here 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-friendly

Hiding 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 gating

Using extractMessageText/hasTextContent avoids duplicating parsing logic and makes the streaming loader condition precise.


113-141: Editing UI is gated to user messages and preserves structure

Conditionally rendering MessageEditor only for user messages is correct, and the non-edit path keeps MessagePartHandler unchanged.


152-161: Props wiring to MessageActions looks consistent with the new API

Passing messageText, onEdit, and isEditing keeps actions in sync with the editor state.

Copy link

@coderabbitai coderabbitai bot left a 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 propagation

The 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 failure

Currently 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 only

If 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 lists

The 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-2 and ChatSidebarHeader itself uses p-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 scrolling

In flex layouts, children with overflow-y-auto can fail to scroll unless an ancestor sets min-height: 0 (or has overflow clipped). You already have overflow-hidden on the parent; adding min-h-0 makes the behavior more robust across browsers. Optional overscroll-contain prevents 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 accessibility

Use 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-16

You’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 consistent

Same 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 consistent

Mirrors 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 ChatHeader

Both 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 3e3a8af and c02cdb4.

📒 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 index

Using useMemo with findLastAssistantMessageIndex avoids redundant scans on every render.


104-107: Good: centralized text extraction and content checks

extractMessageText and hasTextContent keep the render logic clean and consistent with utilities.


109-111: Good: loader gating matches streaming semantics

Showing the loader only for the last assistant message while streaming and without text content is correct.


175-177: Good: dedicated error component usage

Replacing 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 and ChatItem props, then update the comment accordingly. If the necessary fields line up, I'll recommend normalizing on id for both the React key and the prop; otherwise I’ll flag any discrepancies.

app/components/Header.tsx (1)

31-49: Sign-out pathway confirmed

The Log out entry is still available in the user dropdown menu for authenticated users, invoking the /logout route:

  • In app/components/UserDropdownMenu.tsx (lines 21–23 & 90–92), handleSignOut redirects to /logout, and the <DropdownMenuItem> renders “Log out”.
  • The /logout API route is implemented in app/logout/route.ts, which calls signOut().
  • The /logout path is also whitelisted in middleware.ts (line 10).

No further changes required.

app/components/ChatHeader.tsx (1)

64-81: Sign-out flow confirmed via UserDropdownMenu

The 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
Copy link

@coderabbitai coderabbitai bot left a 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 misconfiguration

If 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 config

Small 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.

📥 Commits

Reviewing files that changed from the base of the PR and between c02cdb4 and bd4431b.

📒 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 Format

The JWKS sanity-check script failed because the WORKOS_CLIENT_ID environment 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 the issuer value exactly matches the iss claim in your user_management tokens (i.e., with or without a trailing slash as your tokens emit).
  • JWKS endpoint functionality:
    1. Set the environment variable:
      export WORKOS_CLIENT_ID="<your client ID>"
    2. Rerun the script to ensure it returns at least one key:
      BASE="${AUTH_BASE_URL:-https://auth.hackerai.co}"
      CID="${WORKOS_CLIENT_ID}"
      curl -sS "${BASE}/sso/jwks/${CID}" | jq '.keys | length'
      – Expect a numeric output ≥ 1.

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 strict iss matching

The first provider’s issuer includes 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 the iss claim 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:

  1. Decode a valid token from each provider and confirm the "iss" claim matches your configured issuer exactly.
  2. 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))
PY

Ensure 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.
Copy link

@coderabbitai coderabbitai bot left a 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 chatId

Slight 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 noisy

State 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 switch

As 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 harmless

Including 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.

📥 Commits

Reviewing files that changed from the base of the PR and between bd4431b and 1155b5a.

📒 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)

Copy link

@coderabbitai coderabbitai bot left a 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 type

Using 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 run

Handle 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 delete

The 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—good

Matches the async usage downstream.


68-82: Good: await edit and keep editor open on failure

Prevents UI drift and handles errors gracefully.

🧹 Nitpick comments (6)
lib/utils/message-utils.ts (1)

35-42: Minor: avoid map+reverse for last assistant index

Iterating 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 configurable

Allow 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 normalizing

Category 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 path

Use 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 ordering

If 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 data

If 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 1155b5a and 41b7d21.

📒 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 guard

getUserChats correctly uses ctx.auth, index, and safe fallbacks.

convex/messages.ts (2)

21-22: Enforce presence of service key for S2S saveMessage

With 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 solid

Using internal.chats.verifyChatOwnership and returning [] for non-existent chats avoids leaks and keeps the UI stable.

@rossmanko rossmanko merged commit 8510a87 into main Aug 27, 2025
3 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Sep 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants