Improvements for the AI Assistant#403
Merged
Merged
Conversation
… that the ai reads an entity before editing
Contributor
|
Warning Review limit reached
More reviews will be available in 51 minutes and 7 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR refactors the AI assistant chat by extracting the large inline ChangesAI Assistant Chat Refactor
Sequence Diagram(s)sequenceDiagram
participant User
participant AIAssistantChat as chat.tsx
participant useChat
participant useFrontendToolHandler
participant useAIAssistantChats
participant EditorState
User->>AIAssistantChat: open chat panel
AIAssistantChat->>useAIAssistantChats: getChat(crateId)
useAIAssistantChats-->>AIAssistantChat: persisted messages
AIAssistantChat->>useChat: initialize with messages
User->>AIAssistantChat: submit message
useChat->>useFrontendToolHandler: onToolCall → handleToolCall(toolCall, addToolOutput)
useFrontendToolHandler->>EditorState: read/edit/create/delete entity
EditorState-->>useFrontendToolHandler: result
useFrontendToolHandler-->>useChat: addToolOutput (success or output-error)
useChat-->>AIAssistantChat: status = ready, updated messages
AIAssistantChat->>useAIAssistantChats: updateChat(crateId, messages)
User->>AIAssistantChat: edit user message
AIAssistantChat->>useChat: setMessages + regenerate()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
components/ai/message.tsx (1)
1-4: ⚡ Quick winDeclare this component as client-side explicitly.
Add
"use client"at the top to keep client boundary explicit and consistent with repo rules for TSX components.Suggested change
+"use client" + import Markdown from "react-markdown"As per coding guidelines,
**/*.{tsx,jsx}should “Mark client components with"use client"directive at the top of the file”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/ai/message.tsx` around lines 1 - 4, The TSX file components/ai/message.tsx is missing the "use client" directive at the top. Add "use client" as the very first line in the file, before all imports including Markdown, remarkGfm, memo, and PropsWithChildren. This explicitly marks the component as client-side and keeps it consistent with the repository's coding guidelines for TSX components.Source: Coding guidelines
components/ai/user-message.tsx (1)
1-7: ⚡ Quick winAdd an explicit client boundary for this hook-based component.
This component uses client-only hooks and event handlers; add
"use client"at file top for consistent boundary declaration.Suggested change
+"use client" + import { CheckIcon, CopyIcon, PencilIcon, XIcon } from "lucide-react"As per coding guidelines,
**/*.{tsx,jsx}should “Mark client components with"use client"directive at the top of the file”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/ai/user-message.tsx` around lines 1 - 7, The user-message.tsx component uses client-only React hooks (useCallback, useState) and the useCopyToClipboard hook, but is missing the "use client" directive required for client components in Next.js. Add "use client" as the very first line at the top of the file before any import statements to properly mark this as a client component and comply with the coding guidelines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/ai/message.tsx`:
- Around line 90-93: The anchor element in the markdown link renderer (the `a`
component with `target={"_blank"}`) is missing the `rel` attribute which is
needed to prevent opener attacks when opening links in new tabs. Add
`rel="noopener noreferrer"` to the anchor tag alongside the existing `target`,
`href`, and `className` attributes to harden the link against security
vulnerabilities.
In `@components/ai/user-message.tsx`:
- Around line 67-75: The icon-only buttons in the user message component lack
accessible names for screen readers. Add aria-label attributes to both Button
components: one containing the PencilIcon with onClick={startEditing} should
have an aria-label like "Edit message", and the button containing the
CopyIcon/CheckIcon with onClick={copyMessage} should have an aria-label like
"Copy message". These labels will be announced by screen readers to provide
context for visually impaired users.
In `@lib/ai/tools.ts`:
- Around line 33-42: The `$set` field in the inputSchema currently allows `@id`
to be set, which causes silent failures downstream when `editEntity({...initial,
...$set})` is called because changing `@id` gets rejected by
`editorState.editEntity`. Add validation to the `$set` record schema using Zod's
`.refine()` method to disallow the `@id` key and reject it with a clear error
message stating that `@id` changes must be handled through `moveEntity` instead
of `$set`.
In `@lib/ai/use-frontend-tool-handler.ts`:
- Line 11: The module-level readEntities map persists across all hook instances
and crate switches, causing stale data and unbounded memory growth. Refactor the
caching mechanism to be scoped per crateId by accepting crateId as a parameter
to the hook and implementing a caching function (such as getReadEntitiesCache)
that returns or manages a cache specific to each crateId. Replace all direct
references to the readEntities map with calls to this new crate-aware caching
function to ensure each crate maintains its own isolated cache that is properly
cleaned up when switching between crates.
- Around line 225-253: The promises array contains nested arrays instead of flat
promises, preventing proper settlement. The Array.from(entities.values()).map()
call returns an array of promises that should be spread into the outer promises
array, and similarly the Object.keys(entity).map() result containing property
validation promises should be flattened before passing to the inner
Promise.allSettled call. Use the spread operator to flatten both the entity
validation promises (after the map call) and the property validation promises
(after the Object.keys map call) so that Promise.allSettled receives a
single-level array of promises to await.
In `@lib/state/editor-state.ts`:
- Around line 452-460: The removeProperty method allows deletion of reserved
entity properties like `@id` and `@type`, which can break entity invariants and
identity-dependent flows. Add validation at the start of the removeProperty
method to check if the propertyName being deleted is a reserved key (`@id` or
`@type`) and prevent deletion of these critical properties by returning early or
throwing an error before the setState call.
---
Nitpick comments:
In `@components/ai/message.tsx`:
- Around line 1-4: The TSX file components/ai/message.tsx is missing the "use
client" directive at the top. Add "use client" as the very first line in the
file, before all imports including Markdown, remarkGfm, memo, and
PropsWithChildren. This explicitly marks the component as client-side and keeps
it consistent with the repository's coding guidelines for TSX components.
In `@components/ai/user-message.tsx`:
- Around line 1-7: The user-message.tsx component uses client-only React hooks
(useCallback, useState) and the useCopyToClipboard hook, but is missing the "use
client" directive required for client components in Next.js. Add "use client" as
the very first line at the top of the file before any import statements to
properly mark this as a client component and comply with the coding guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7425c099-54f9-4e79-aeca-4a7c2cb2b3ef
📒 Files selected for processing (10)
components/ai/chat.tsxcomponents/ai/message.tsxcomponents/ai/model-selection.tsxcomponents/ai/user-message.tsxlib/ai/tools.tslib/ai/types.tslib/ai/use-frontend-tool-handler.tslib/state/ai-assistant-chats.tslib/state/editor-state.tslib/utils.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/editor/type-field.tsx`:
- Around line 67-71: The onClick handler onRemoveEntry is attached to
DropdownMenuContent instead of DropdownMenuItem, which causes the remove handler
to execute even when the menu item is disabled on line 70. Remove the onClick
prop from DropdownMenuContent and move it to the DropdownMenuItem component so
that the disabled state properly prevents the click handler from executing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f661b3d5-fc3f-49f2-b68a-180a3b2f80ce
📒 Files selected for processing (6)
components/ai/message.tsxcomponents/ai/user-message.tsxcomponents/editor/single-property-editor.tsxcomponents/editor/type-field.tsxlib/ai/use-frontend-tool-handler.tslib/state/editor-state.ts
✅ Files skipped from review due to trivial changes (1)
- components/editor/single-property-editor.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- components/ai/message.tsx
- components/ai/user-message.tsx
- lib/state/editor-state.ts
- lib/ai/use-frontend-tool-handler.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
@id/@typeproperties during edits.