Separate out images from the rest of the context, don't clear it as user clicks in canvas #2977
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds explicit local vs external image handling across chat input, prompt provider, AI tool, and UI. Introduces ImageMessageContext fields (source/path/branchId), base64 data URL conversion, internal drag-and-drop parsing, updated pills/components, and UploadImageTool validation rejecting local images; prompt now emits local-images and available-images blocks. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CI as ChatInput
participant FS as File System / Clipboard
participant Ctx as Chat Context Store
rect rgba(200,230,255,0.25)
note over U,CI: Internal drag (application/json)
U->>CI: Drag/drop internal image payload (application/json)
CI->>CI: Parse JSON via Zod, load branch file content
CI->>CI: convertToBase64DataUrl(content, mime)
CI->>Ctx: add ImageMessageContext {source: local, path, branchId, mimeType, content}
end
rect rgba(230,255,200,0.25)
note over U,CI: External file drop/paste
U->>CI: Drop/paste image file
CI->>FS: Read file bytes & mime
FS-->>CI: Bytes + mime
CI->>CI: convertToBase64DataUrl(bytes, mime)
CI->>Ctx: add ImageMessageContext {source: external, displayName, mimeType, content}
end
sequenceDiagram
participant LLM as LLM
participant PP as PromptProvider
participant Tool as upload_image
participant Proj as Project FS
PP->>PP: Split IMAGE contexts -> localImages & externalImages
PP-->>LLM: Prompt with <local-images> then <available-images>
alt LLM requests external upload
LLM->>Tool: upload_image(image_id, destination_path?, filename?)
Tool->>Tool: Validate sandbox & conversation
Tool->>Tool: Find image by ID in image contexts
Tool->>Tool: If source == local -> throw error
Tool->>Proj: Write file to public/(path)
Proj-->>Tool: OK -> Tool returns path
else LLM references local image
note right of LLM: Use provided path/branchId (no upload)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(4 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx(2 hunks)apps/web/client/src/components/store/editor/chat/context.ts(4 hunks)packages/ai/src/tools/classes/upload-image.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/components/store/editor/chat/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/components/store/editor/chat/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxpackages/ai/src/tools/classes/upload-image.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/components/store/editor/chat/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxpackages/ai/src/tools/classes/upload-image.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx
🧬 Code graph analysis (5)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx (1)
AttachedImages(10-49)
apps/web/client/src/components/store/editor/chat/context.ts (3)
packages/models/src/chat/message/context.ts (3)
ImageMessageContext(38-42)MessageContext(54-60)HighlightMessageContext(29-36)packages/models/src/element/element.ts (1)
DomElement(12-16)apps/web/client/src/components/store/editor/frames/manager.ts (1)
FrameData(13-17)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (3)
apps/web/client/src/components/store/editor/chat/context.ts (2)
context(38-40)context(42-44)packages/models/src/chat/message/context.ts (1)
MessageContext(54-60)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-context-pill.tsx (1)
DraftContextPill(8-52)
packages/ai/src/tools/classes/upload-image.ts (4)
packages/ai/src/tools/shared/type.ts (1)
BRANCH_ID_SCHEMA(3-7)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(15-209)apps/web/client/src/components/store/editor/chat/context.ts (2)
imageContext(46-48)imageContext(50-52)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx (3)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-42)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/draft-image-pill.tsx (1)
DraftImagePill(7-64)
| <div className="flex flex-row items-center gap-1.5 text-xs text-foreground-tertiary pt-1 px-1"> | ||
| <span>Attached Images</span> | ||
| </div> | ||
| <div className="flex flex-row flex-wrap items-center gap-1.5 px-1"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Localize “Attached Images”.
This label is user-facing and should come from next-intl instead of being hardcoded. Please wire it through the translations hook to stay compliant with our localization guideline. As per coding guidelines
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/attached-images.tsx
around lines 28 to 31, the user-facing label "Attached Images" is hardcoded;
replace it with a next-intl translation. Import and call useTranslations with
the appropriate namespace (e.g., const t = useTranslations('ChatInput') or the
component's namespace), replace the literal span content with
t('attachedImages') (or the agreed key), and ensure the corresponding key
"attachedImages" is added to the relevant locale JSON files; keep the JSX
structure unchanged other than swapping the hardcoded string for the t(...)
call.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/ai/src/tools/classes/upload-image.ts (1)
16-17: Update description to explicitly state "public/" as default.The description still says "public directory" but should explicitly state "public/" to match the implementation default at line 105.
Apply this diff:
- "Uploads an image from the chat context to the project's file system. Use this tool when the user asks you to save, add, or upload an image to their project. The image will be stored in public directory by default and can be referenced in code. After uploading, you can use the file path in your code changes."; + "Uploads an image from the chat context to the project's file system. Use this tool when the user asks you to save, add, or upload an image to their project. The image will be stored in the public/ directory by default and can be referenced in code. After uploading, you can use the file path in your code changes.";
🧹 Nitpick comments (1)
packages/ai/src/tools/classes/upload-image.ts (1)
105-106: Consider normalizing destinationPath to handle trailing slashes.If a user provides
destination_pathwith a trailing slash (e.g.,"public/"), the path construction at line 106 would result in double slashes:"public//filename.ext". While this typically works in most file systems, it's not ideal.Apply this diff to normalize the path:
- const destinationPath = args.destination_path?.trim() ?? 'public'; + const destinationPath = (args.destination_path?.trim() ?? 'public').replace(/\/+$/, ''); const fullPath = `${destinationPath}/${filename}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ai/src/tools/classes/upload-image.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/ai/src/tools/classes/upload-image.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/ai/src/tools/classes/upload-image.ts
🧬 Code graph analysis (1)
packages/ai/src/tools/classes/upload-image.ts (4)
packages/ai/src/tools/shared/type.ts (1)
BRANCH_ID_SCHEMA(3-7)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(15-209)apps/web/client/src/components/store/editor/chat/context.ts (2)
imageContext(46-48)imageContext(50-52)
🔇 Additional comments (2)
packages/ai/src/tools/classes/upload-image.ts (2)
42-44: LGTM: Explicit sandbox validation.The explicit check for sandbox existence with a descriptive error message prevents potential null reference errors downstream.
56-73: LGTM: Robust image search by ID.The search logic correctly:
- Iterates from most recent to oldest message
- Filters by MessageContextType.IMAGE
- Matches by unique image ID
- Handles cases where metadata or context might be missing
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/ai/src/tools/classes/upload-image.ts (1)
102-104: Consider stripping existing extensions from custom filenames.The description specifies filenames should be "without extension", but the code doesn't validate or strip extensions if provided. If a user supplies
myimage.jpg, the result would bemyimage.jpg.png. While the parameter description sets the expectation, you could make the implementation more forgiving:- const filename = args.filename - ? `${args.filename}.${extension}` - : `${uuidv4()}.${extension}`; + const baseFilename = args.filename + ? args.filename.replace(/\.[^.]+$/, '') + : uuidv4(); + const filename = `${baseFilename}.${extension}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(5 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsx(1 hunks)apps/web/client/src/components/store/editor/chat/context.ts(4 hunks)packages/ai/src/contexts/classes/local-image.ts(1 hunks)packages/ai/src/contexts/helpers.ts(2 hunks)packages/ai/src/prompt/provider.ts(3 hunks)packages/ai/src/tools/classes/upload-image.ts(3 hunks)packages/models/src/chat/message/context.ts(3 hunks)packages/utility/src/file.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/components/store/editor/chat/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxpackages/ai/src/contexts/helpers.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxpackages/utility/src/file.tspackages/ai/src/contexts/classes/local-image.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxpackages/models/src/chat/message/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/components/store/editor/chat/context.tspackages/ai/src/prompt/provider.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxpackages/ai/src/tools/classes/upload-image.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsxpackages/ai/src/contexts/helpers.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsxpackages/utility/src/file.tspackages/ai/src/contexts/classes/local-image.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxpackages/models/src/chat/message/context.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/components/store/editor/chat/context.tspackages/ai/src/prompt/provider.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxpackages/ai/src/tools/classes/upload-image.ts
🧬 Code graph analysis (9)
packages/ai/src/contexts/helpers.ts (1)
packages/ai/src/contexts/classes/local-image.ts (1)
LocalImageContext(5-18)
packages/ai/src/contexts/classes/local-image.ts (1)
packages/models/src/chat/message/context.ts (1)
LocalImageMessageContext(45-50)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (2)
packages/utility/src/file.ts (2)
getMimeType(85-99)convertToBase64DataUrl(120-129)packages/models/src/chat/message/context.ts (1)
LocalImageMessageContext(45-50)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (3)
apps/web/client/src/components/store/editor/chat/context.ts (2)
context(44-46)context(48-50)packages/models/src/chat/message/context.ts (2)
ImageMessageContext(39-43)LocalImageMessageContext(45-50)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsx (1)
LocalImagePill(9-108)
apps/web/client/src/components/store/editor/chat/context.ts (3)
packages/models/src/element/element.ts (1)
DomElement(12-16)apps/web/client/src/components/store/editor/frames/manager.ts (1)
FrameData(13-17)packages/models/src/chat/message/context.ts (2)
MessageContext(62-69)HighlightMessageContext(30-37)
packages/ai/src/prompt/provider.ts (1)
packages/ai/src/prompt/helpers.ts (1)
wrapXml(1-3)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsx (5)
packages/models/src/chat/message/context.ts (1)
LocalImageMessageContext(45-50)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)packages/file-system/src/hooks/index.ts (1)
useFile(1-1)packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
getTruncatedName(7-17)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (3)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
validateImageLimit(35-48)packages/utility/src/file.ts (1)
convertToBase64DataUrl(120-129)packages/models/src/chat/message/context.ts (1)
LocalImageMessageContext(45-50)
packages/ai/src/tools/classes/upload-image.ts (3)
packages/ai/src/tools/shared/type.ts (1)
BRANCH_ID_SCHEMA(3-7)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(15-209)
🪛 Biome (2.1.2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsx
[error] 27-27: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (5)
packages/ai/src/tools/classes/upload-image.ts (5)
16-17: Description now correctly reflects the implementation.The updated description properly documents the default upload path as
public/(matching line 105) and clearly distinguishes between images in<available-images>(which this tool uploads) vs.<local-images>(which already exist). This resolves the inconsistency flagged in previous reviews.
19-31: Parameter descriptions are clear and accurate.The parameter schema updates correctly document:
image_idas the unique ID from available imagesdestination_pathdefaulting to"public"(consistent with line 105)filenameas optional and without extension
42-50: Robust defensive checks for required resources.The explicit validation for sandbox existence and active conversation provides clear error messages and prevents downstream issues. Well done.
59-59: Optional chaining correctly handles missing metadata.The check safely skips messages without metadata or context.
62-77: Confirm IMAGE-only filtering is intentional. The upload-image tool should only targetMessageContextType.IMAGE; local images (LOCAL_IMAGE) are handled separately.
| if (context.type !== MessageContextType.LOCAL_IMAGE) { | ||
| console.warn('LocalImagePill received non-local-image context'); | ||
| return null; | ||
| } | ||
|
|
||
| // Convert content to data URL for display | ||
| useEffect(() => { | ||
| if (!content) { | ||
| setImageUrl(null); | ||
| return; | ||
| } | ||
|
|
||
| // Handle SVG files (text content) | ||
| if (typeof content === 'string' && context.displayName.toLowerCase().endsWith('.svg')) { | ||
| const svgDataUrl = `data:image/svg+xml;base64,${btoa(content)}`; | ||
| setImageUrl(svgDataUrl); | ||
| return; | ||
| } | ||
|
|
||
| // Handle other text files (shouldn't happen for images, but just in case) | ||
| if (typeof content === 'string') { | ||
| setImageUrl(null); | ||
| return; | ||
| } | ||
|
|
||
| // Handle binary content (PNG, JPG, etc.) | ||
| const blob = new Blob([content as BlobPart], { type: context.mimeType || 'image/*' }); | ||
| const url = URL.createObjectURL(blob); | ||
| setImageUrl(url); | ||
|
|
||
| // Clean up function to revoke object URL (only for blob URLs) | ||
| return () => { | ||
| URL.revokeObjectURL(url); | ||
| }; | ||
| }, [content, context.mimeType, context.displayName]); |
There was a problem hiding this comment.
Fix conditional hook usage in LocalImagePill
Returning before useEffect runs means this component renders fewer hooks on some passes, which React will throw for (Biome already flags it). Move the guard after the effect (and short-circuit inside the effect) so the hook order stays stable.
@@
- if (context.type !== MessageContextType.LOCAL_IMAGE) {
- console.warn('LocalImagePill received non-local-image context');
- return null;
- }
-
- // Convert content to data URL for display
- useEffect(() => {
+ // Convert content to data URL for display
+ useEffect(() => {
+ if (context.type !== MessageContextType.LOCAL_IMAGE) {
+ setImageUrl(null);
+ return;
+ }
if (!content) {
setImageUrl(null);
return;
}
// Handle SVG files (text content)
- if (typeof content === 'string' && context.displayName.toLowerCase().endsWith('.svg')) {
+ const displayName = context.displayName?.toLowerCase() ?? '';
+ if (typeof content === 'string' && displayName.endsWith('.svg')) {
const svgDataUrl = `data:image/svg+xml;base64,${btoa(content)}`;
setImageUrl(svgDataUrl);
return;
}
@@
return () => {
URL.revokeObjectURL(url);
};
- }, [content, context.mimeType, context.displayName]);
+ }, [content, context.mimeType, context.displayName, context.type]);
+
+ if (context.type !== MessageContextType.LOCAL_IMAGE) {
+ console.warn('LocalImagePill received non-local-image context');
+ return null;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (context.type !== MessageContextType.LOCAL_IMAGE) { | |
| console.warn('LocalImagePill received non-local-image context'); | |
| return null; | |
| } | |
| // Convert content to data URL for display | |
| useEffect(() => { | |
| if (!content) { | |
| setImageUrl(null); | |
| return; | |
| } | |
| // Handle SVG files (text content) | |
| if (typeof content === 'string' && context.displayName.toLowerCase().endsWith('.svg')) { | |
| const svgDataUrl = `data:image/svg+xml;base64,${btoa(content)}`; | |
| setImageUrl(svgDataUrl); | |
| return; | |
| } | |
| // Handle other text files (shouldn't happen for images, but just in case) | |
| if (typeof content === 'string') { | |
| setImageUrl(null); | |
| return; | |
| } | |
| // Handle binary content (PNG, JPG, etc.) | |
| const blob = new Blob([content as BlobPart], { type: context.mimeType || 'image/*' }); | |
| const url = URL.createObjectURL(blob); | |
| setImageUrl(url); | |
| // Clean up function to revoke object URL (only for blob URLs) | |
| return () => { | |
| URL.revokeObjectURL(url); | |
| }; | |
| }, [content, context.mimeType, context.displayName]); | |
| // Convert content to data URL for display | |
| useEffect(() => { | |
| if (context.type !== MessageContextType.LOCAL_IMAGE) { | |
| setImageUrl(null); | |
| return; | |
| } | |
| if (!content) { | |
| setImageUrl(null); | |
| return; | |
| } | |
| // Handle SVG files (text content) | |
| const displayName = context.displayName?.toLowerCase() ?? ''; | |
| if (typeof content === 'string' && displayName.endsWith('.svg')) { | |
| const svgDataUrl = `data:image/svg+xml;base64,${btoa(content)}`; | |
| setImageUrl(svgDataUrl); | |
| return; | |
| } | |
| // Handle other text files (shouldn't happen for images, but just in case) | |
| if (typeof content === 'string') { | |
| setImageUrl(null); | |
| return; | |
| } | |
| // Handle binary content (PNG, JPG, etc.) | |
| const blob = new Blob([content as BlobPart], { type: context.mimeType || 'image/*' }); | |
| const url = URL.createObjectURL(blob); | |
| setImageUrl(url); | |
| // Clean up function to revoke object URL (only for blob URLs) | |
| return () => { | |
| URL.revokeObjectURL(url); | |
| }; | |
| }, [content, context.mimeType, context.displayName, context.type]); | |
| if (context.type !== MessageContextType.LOCAL_IMAGE) { | |
| console.warn('LocalImagePill received non-local-image context'); | |
| return null; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 27-27: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/local-image-pill.tsx
around lines 21–55, the component currently returns early before calling
useEffect when context.type !== MessageContextType.LOCAL_IMAGE which changes
hook order; move the type guard so the component always runs hooks: keep the
early-return logic but relocate it to after the useEffect call (and inside the
effect, check context.type and short-circuit to setImageUrl(null) and return),
ensuring useEffect always runs with stable hook order while still not processing
non-local-image contexts and revoking blob URLs on cleanup.
| { elements, frames }, | ||
| ) => this.generateContextFromReaction({ elements, frames }).then((context) => (this.context = context)), | ||
| ) => this.generateContextFromReaction({ elements, frames }).then((context) => { | ||
| // Preserve existing images when updating context from canvas selection |
There was a problem hiding this comment.
@saddlepaddle why do we need the local image context at all if we just do this?
There was a problem hiding this comment.
hmm well we need it so the LLM knows it's a local file basically - that way it knows how to grab it / what to add to the dom / that it shouldn't try uploading it.
There was a problem hiding this comment.
I did it so the agent is aware of local vs. not local files! We have to upload non-local files first before we can use them, but for local files we don't want the llm to re-upload them each time it tries to use it 👍🏼
| id?: string; | ||
| }; | ||
|
|
||
| export type LocalImageMessageContext = BaseMessageContext & { |
There was a problem hiding this comment.
Should we actually update the file context type to handle binary/image instead of making a type specifically for local image?
There was a problem hiding this comment.
ooo good call! yeah we can merge 👍🏼. It's still useful to differentiate the two though since we want the agent to know to not try to re-upload local assets.
saddlepaddle
force-pushed
the
separate-out-image-parts-in-context
branch
from
7ed4ffd to
0a7b468
Compare
saddlepaddle
force-pushed
the
separate-out-image-parts-in-context
branch
from
0a7b468 to
bebcc2c
Compare
| displayName: displayName ?? file.name, | ||
| }; | ||
| editorEngine.chat.context.addContexts([contextImage]); | ||
| }; |
There was a problem hiding this comment.
Add a reader.onerror callback in the handleImageEvent function to gracefully handle file read errors.
saddlepaddle
force-pushed
the
separate-out-image-parts-in-context
branch
from
bebcc2c to
7efac6a
Compare
| {selectedImages.map((imageContext) => ( | ||
| <DraftImagePill | ||
| <ImagePill | ||
| key={imageContext.content} |
There was a problem hiding this comment.
Consider using a more unique key for the ImagePill component rather than relying solely on imageContext.content. Using a unique identifier (e.g. imageContext.id or combining with an index) can help avoid potential key collisions.
| key={imageContext.content} | |
| key={imageContext.id} |
|
|
||
| useEffect(() => { | ||
| const handleGlobalKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === 'Enter' && suggestionRef.current?.handleEnterSelection()) { |
There was a problem hiding this comment.
The global keydown handler calls suggestionRef.current?.handleEnterSelection() twice. Consider storing the result in a variable to ensure idempotency and avoid unintended side effects.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai/src/tools/classes/upload-image.ts (1)
66-73: Potential issue with undefined image ID matching.The
find()operation matches onctx.id === args.image_id. SinceImageMessageContext.idis optional (id?: string), this could incorrectly match when both areundefined. If multiple images lack IDs, the first one would be selected regardless of which was intended.Consider adding validation to ensure the image ID is present:
- // Find image by ID - const match = imageContexts.find((ctx) => ctx.id === args.image_id); + // Find image by ID - ensure both IDs are defined + const match = imageContexts.find((ctx) => ctx.id && ctx.id === args.image_id); if (match) {Additionally, validate that external images have IDs when they're added to the context, or generate IDs server-side if missing.
♻️ Duplicate comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
265-290: Add error handler for FileReader.The
FileReaderusage at lines 275-289 lacks anonerrorcallback. If the file read fails (e.g., due to permissions, corruption, or unsupported format), the user won't receive feedback.Apply this diff to add error handling:
const reader = new FileReader(); reader.onload = async (event) => { const compressedImage = await compressImageInBrowser(file); const base64URL = compressedImage ?? (event.target?.result as string); const contextImage: ImageMessageContext = { id: uuidv4(), type: MessageContextType.IMAGE, source: 'external', content: base64URL, mimeType: file.type, displayName: displayName ?? file.name, }; editorEngine.chat.context.addContexts([contextImage]); }; + reader.onerror = () => { + toast.error('Failed to read image file'); + }; reader.readAsDataURL(file);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/client/src/app/_components/hero/create.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx(7 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx(3 hunks)apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx(1 hunks)apps/web/client/src/components/store/editor/chat/context.ts(3 hunks)packages/ai/src/prompt/provider.ts(3 hunks)packages/ai/src/tools/classes/upload-image.ts(4 hunks)packages/models/src/chat/message/context.ts(1 hunks)packages/utility/src/file.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/web/client/src/components/store/editor/chat/context.ts
- apps/web/client/src/app/_components/hero/create.tsx
- packages/utility/src/file.ts
- apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx
- apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx
- packages/models/src/chat/message/context.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxpackages/ai/src/prompt/provider.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxpackages/ai/src/tools/classes/upload-image.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxpackages/ai/src/prompt/provider.tsapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxpackages/ai/src/tools/classes/upload-image.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx
🧬 Code graph analysis (6)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx (2)
packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)apps/web/client/src/components/store/editor/chat/context.ts (2)
context(43-45)context(47-49)
packages/ai/src/prompt/provider.ts (2)
packages/ai/src/prompt/helpers.ts (1)
wrapXml(1-3)packages/ai/src/contexts/classes/image.ts (1)
ImageContext(5-29)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (2)
packages/utility/src/file.ts (2)
getMimeType(85-99)convertToBase64DataUrl(120-129)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)
packages/ai/src/tools/classes/upload-image.ts (3)
packages/ai/src/tools/shared/type.ts (1)
BRANCH_ID_SCHEMA(3-7)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(15-209)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (10)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/components/store/editor/chat/index.ts (2)
FOCUS_CHAT_INPUT_EVENT(8-8)sendMessage(43-49)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
validateImageLimit(35-48)packages/utility/src/file.ts (1)
convertToBase64DataUrl(120-129)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)packages/utility/src/image.ts (1)
compressImageInBrowser(7-29)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/suggestions.tsx (1)
SuggestionsRef(9-12)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsx (1)
QueueItems(10-50)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (1)
InputContextPills(19-60)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/action-buttons.tsx (1)
ActionButtons(7-78)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (3)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx (1)
ImagePill(7-64)apps/web/client/src/components/store/editor/chat/context.ts (2)
context(43-45)context(47-49)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)
🔇 Additional comments (8)
packages/ai/src/tools/classes/upload-image.ts (2)
79-86: LGTM! Good defensive check for local images.This validation correctly prevents re-uploading images that already exist in the project. The error message is clear and actionable.
113-113: LGTM! Correct empty string handling.The
||operator properly treats empty or whitespace-only strings as falsy, falling back to the default 'public' path as intended.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (1)
40-47: LGTM! Type-safe rendering of image contexts.The type guard at line 40 ensures the type cast at line 44 is safe. The updated component name and type align with the new ImageMessageContext structure.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx (1)
7-17: LGTM! Clean component rename with updated types.The rename from
DraftImagePilltoImagePilland the updated type signature (ImageMessageContext) align with the broader PR objectives of separating image context handling.packages/ai/src/prompt/provider.ts (1)
114-121: LGTM! Good fallback for missing image IDs.Line 116 correctly uses
img.id || 'unknown'to handle optional IDs on external images.apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (1)
82-111: LGTM! Well-implemented local image handling.The function correctly:
- Loads actual image content from the filesystem with error handling (lines 88-91)
- Converts to base64 data URL using the utility function (line 93)
- Creates a properly structured
ImageMessageContextwithsource: 'local'and all required fields (lines 95-103)This aligns well with the PR's objective of differentiating local vs external images.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (2)
192-263: LGTM! Comprehensive drag-and-drop implementation.The drop handler correctly:
- Validates drag data using Zod schema (lines 201-202)
- Distinguishes internal (image panel) vs external (filesystem) sources
- Performs image limit validation (lines 204-213)
- Loads actual file content for internal drops with error handling (lines 215-228)
- Creates properly structured
ImageMessageContextwithsource: 'local'for internal images (lines 233-241)- Falls back to external file handling when internal drag data is absent (lines 251-262)
This implementation aligns well with the PR objectives of preserving images during canvas interaction and handling local project images.
41-46: LGTM! Good validation schema for drag data.The Zod schema provides type-safe parsing of internal drag-and-drop payloads from the image panel, ensuring only valid image metadata is processed.
saddlepaddle
force-pushed
the
separate-out-image-parts-in-context
branch
from
7efac6a to
72a240c
Compare
saddlepaddle
force-pushed
the
separate-out-image-parts-in-context
branch
from
72a240c to
f6ae439
Compare
|
|
||
| if (suggestionRef.current?.handleEnterSelection()) { | ||
| setTimeout(() => textareaRef.current?.focus(), 0); | ||
| async function sendMessage() { |
There was a problem hiding this comment.
After sending a message (in sendMessage), the previous version scrolled to the bottom via onScrollToBottom. If that behavior is desired, consider re-adding a scroll callback.
There was a problem hiding this comment.
@saddlepaddle this is a valid regression which makes me worried about what else is removed.
There was a problem hiding this comment.
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)
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx (1)
82-111: CheckbranchDatabefore reading files.At line 88,
branchData?.codeEditor.readFile(imagePath)can fail ifbranchDatais undefined. The optional chaining onbranchDatareturnsundefined, but attempting to callreadFileonundefinedwould throw a runtime error. While the try-catch will catch this, the error message "Failed to add image to chat" (line 109) doesn't clearly indicate that branch data was unavailable.Apply this diff to add an explicit check:
const handleAddToChat = async (imagePath: string) => { try { const fileName = imagePath.split('/').pop() || imagePath; const mimeType = getMimeType(fileName); + if (!branchData) { + throw new Error('Branch data not available'); + } + // Load the actual image file content - const fileContent = await branchData?.codeEditor.readFile(imagePath); + const fileContent = await branchData.codeEditor.readFile(imagePath); if (!fileContent) { throw new Error('Failed to load image file'); }
♻️ Duplicate comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (2)
101-116: Fix duplicatehandleEnterSelection()call.Lines 103 and 109 both call
suggestionRef.current?.handleEnterSelection(). If this method has side effects (e.g., consuming a suggestion, updating state), calling it twice can cause incorrect behavior or state corruption.Apply this diff to call the method only once:
useEffect(() => { const handleGlobalKeyDown = (e: KeyboardEvent) => { - if (e.key === 'Enter' && suggestionRef.current?.handleEnterSelection()) { + const handled = suggestionRef.current?.handleEnterSelection(); + if (e.key === 'Enter' && handled) { e.preventDefault(); e.stopPropagation(); // Stop the event from bubbling to the canvas e.stopImmediatePropagation(); - // Handle the suggestion selection - suggestionRef.current.handleEnterSelection(); } };
267-292: Add error handler for FileReader.The
FileReaderat line 277 lacks anonerrorcallback. If file reading fails (e.g., permissions issue, corrupt file), the user receives no feedback, and the operation silently fails.Apply this diff to add error handling:
const reader = new FileReader(); reader.onload = async (event) => { const compressedImage = await compressImageInBrowser(file); const base64URL = compressedImage ?? (event.target?.result as string); const contextImage: ImageMessageContext = { id: uuidv4(), type: MessageContextType.IMAGE, source: 'external', content: base64URL, mimeType: file.type, displayName: displayName ?? file.name, }; editorEngine.chat.context.addContexts([contextImage]); }; +reader.onerror = () => { + console.error('Failed to read image file'); + toast.error('Failed to read image file'); +}; reader.readAsDataURL(file);
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsx (1)
210-274: Optional: Consider i18n for user-facing text.The file contains hardcoded user-facing strings throughout (e.g., "Add to Chat", "Rename", "Delete", dialog messages). As per coding guidelines, consider using next-intl messages/hooks for internationalization.
Note: This is a pre-existing pattern in the file and not introduced by this PR.
As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/client/src/app/_components/hero/create.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx(7 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx(3 hunks)apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx(1 hunks)apps/web/client/src/components/store/editor/chat/context.ts(4 hunks)packages/ai/src/prompt/provider.ts(3 hunks)packages/ai/src/tools/classes/upload-image.ts(4 hunks)packages/models/src/chat/message/context.ts(1 hunks)packages/utility/src/file.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/utility/src/file.ts
- packages/models/src/chat/message/context.ts
- apps/web/client/src/app/_components/hero/create.tsx
- apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx
- packages/ai/src/prompt/provider.ts
- apps/web/client/src/components/store/editor/chat/context.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsxpackages/ai/src/tools/classes/upload-image.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsxpackages/ai/src/tools/classes/upload-image.ts
🧬 Code graph analysis (5)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (3)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx (1)
ImagePill(7-64)apps/web/client/src/components/store/editor/chat/context.ts (2)
context(83-85)context(87-89)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx (1)
packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (8)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/components/store/editor/chat/index.ts (1)
FOCUS_CHAT_INPUT_EVENT(8-8)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
validateImageLimit(35-48)packages/utility/src/file.ts (1)
convertToBase64DataUrl(120-125)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/suggestions.tsx (1)
SuggestionsRef(9-12)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsx (1)
QueueItems(10-50)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/action-buttons.tsx (1)
ActionButtons(7-78)
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx (3)
packages/utility/src/file.ts (2)
getMimeType(85-99)convertToBase64DataUrl(120-125)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/folder-list.tsx (1)
FolderList(12-38)
packages/ai/src/tools/classes/upload-image.ts (3)
packages/ai/src/tools/shared/type.ts (1)
BRANCH_ID_SCHEMA(3-7)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(15-209)
🔇 Additional comments (15)
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/image-item.tsx (1)
153-156: LGTM: Dropdown closes correctly after adding to chat.The change improves UX by explicitly closing the dropdown after the "Add to Chat" action. This behavior is consistent with the existing pattern where the dropdown closes for rename and delete actions (lines 87-91).
apps/web/client/src/app/project/[id]/_components/left-panel/design-panel/image-tab/index.tsx (2)
3-9: LGTM!The imports correctly reflect the new dependencies for handling image context with base64 data URLs. All imported symbols are utilized in the implementation.
62-64: LGTM!Including the error message in the toast provides better debugging information for users. The fallback to 'Unknown error' is appropriate.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/input-context-pills.tsx (2)
2-2: LGTM!The addition of
ImageMessageContexttype import aligns with the type safety improvements for handling image contexts.
62-69: LGTM!The type guard at line 62 ensures the type cast to
ImageMessageContextis safe. The component correctly passes the typed context toImagePill.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/image-pill.tsx (2)
1-17: LGTM!The component correctly enforces type safety with
ImageMessageContext. The type guard and warning message are appropriate.
66-66: LGTM!The display name correctly reflects the renamed component for better debugging in React DevTools.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (6)
3-30: LGTM!The imports correctly reflect the expanded functionality for handling internal and external image drag-and-drop with source distinction and base64 data URL conversion.
42-47: LGTM!The Zod schema correctly validates internal drag-and-drop payloads for images from the image panel.
194-265: LGTM!The internal drag-and-drop flow correctly validates JSON payloads with Zod, checks for
branchDataavailability, loads file content, and converts to base64 data URL. The fallback to external file drops is properly structured with appropriate error handling.
294-348: LGTM!Screenshot capture correctly validates image limits, attempts capture from available frames, and handles errors appropriately. The source is correctly set to 'external' for screenshots.
350-367: LGTM!The drag state handlers correctly detect both external image files and internal JSON payloads from the image panel, providing appropriate visual feedback.
375-466: LGTM!The component structure correctly wires drag-and-drop handlers, maintains proper state management, and renders all child components with appropriate props.
packages/ai/src/tools/classes/upload-image.ts (2)
79-85: Excellent addition to prevent misuse of local images.The check correctly rejects attempts to upload images that already exist in the project, with a clear and actionable error message guiding the AI to reference the existing path instead.
110-114: Validate and sanitize upload paths
NeitherfilenamenordestinationPathare checked for..segments. Ensure the sandbox’swriteFile(e.g. in SandboxManager or CodeFileSystem) normalizes and rejects paths escaping the intended directory, or explicitly strip/deny..inargs.filenameandargs.destination_pathbefore composingfullPath.
|
@saddlepaddle this rebase seems to have override changes from main under |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (3)
149-163: Verify if scroll-to-bottom behavior should be restored.The
onScrollToBottomprop is received but never invoked after sending a message. Previous versions called it aftersendMessage. If automatic scrolling to show the newly sent message is desired, consider addingonScrollToBottom()after a successful send.
95-110: Remove duplicate handleEnterSelection call.Line 97 calls
suggestionRef.current?.handleEnterSelection()in the condition, then Line 103 calls it again inside the block. If the method has side effects, this double invocation could cause unexpected behavior.Apply this diff to fix the duplicate call:
useEffect(() => { const handleGlobalKeyDown = (e: KeyboardEvent) => { - if (e.key === 'Enter' && suggestionRef.current?.handleEnterSelection()) { + const handled = suggestionRef.current?.handleEnterSelection(); + if (e.key === 'Enter' && handled) { e.preventDefault(); e.stopPropagation(); - // Stop the event from bubbling to the canvas e.stopImmediatePropagation(); - // Handle the suggestion selection - suggestionRef.current.handleEnterSelection(); } };
261-286: Add error handling for FileReader.Missing
reader.onerrorcallback. If file reading fails, the user receives no feedback and the operation silently fails.Apply this diff to add error handling:
const reader = new FileReader(); reader.onload = async (event) => { const compressedImage = await compressImageInBrowser(file); const base64URL = compressedImage ?? (event.target?.result as string); const contextImage: ImageMessageContext = { id: uuidv4(), type: MessageContextType.IMAGE, source: 'external', content: base64URL, mimeType: file.type, displayName: displayName ?? file.name, }; editorEngine.chat.context.addContexts([contextImage]); }; +reader.onerror = () => { + toast.error('Failed to read image file'); +}; reader.readAsDataURL(file);
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
288-342: LGTM with minor note on hardcoded text.Screenshot capture logic is robust with proper error handling. However, toast messages (lines 338, 340) use hardcoded English text instead of i18n keys per coding guidelines.
Consider replacing hardcoded toast messages with i18n:
-toast.success('Screenshot added to chat'); +toast.success(t(transKeys.editor.panels.edit.tabs.chat.screenshot.success));-toast.error('Failed to capture screenshot. Error: ' + error); +toast.error(t(transKeys.editor.panels.edit.tabs.chat.screenshot.error, { error }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(2 hunks)apps/web/client/src/components/store/editor/chat/context.ts(3 hunks)apps/web/client/src/components/store/editor/image/index.ts(2 hunks)packages/code-provider/src/providers/codesandbox/utils/read-file.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/code-provider/src/providers/codesandbox/utils/read-file.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/components/store/editor/chat/context.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/store/editor/image/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/components/store/editor/image/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/components/store/editor/image/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx
🧬 Code graph analysis (3)
apps/web/client/src/components/store/editor/image/index.ts (1)
packages/utility/src/file.ts (1)
convertToBase64DataUrl(120-125)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (9)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/components/store/editor/chat/index.ts (2)
FOCUS_CHAT_INPUT_EVENT(8-8)sendMessage(43-49)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
validateImageLimit(35-48)packages/utility/src/file.ts (1)
convertToBase64DataUrl(120-125)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)packages/utility/src/image.ts (1)
compressImageInBrowser(7-29)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/suggestions.tsx (1)
SuggestionsRef(9-12)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsx (1)
QueueItems(10-50)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/chat-context.tsx (1)
ChatContextWindow(19-49)
apps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx (1)
packages/utility/src/file.ts (1)
convertToBase64DataUrl(120-125)
🔇 Additional comments (13)
apps/web/client/src/components/store/editor/image/index.ts (1)
2-2: LGTM! Good refactor to use centralized utility.Extracting base64 data URL creation into
convertToBase64DataUrlreduces duplication and improves maintainability. The utility correctly handlesUint8Arrayinput (guaranteed by theinstanceofcheck on line 164), and SVG text handling remains appropriately separate.Also applies to: 166-166
apps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/file-content/code-editor.tsx (3)
1-3: LGTM! Import changes support the new features.The imports are correctly updated to support:
- The new CMD+L keyboard shortcut via
keymap- Proper data URL formatting with
convertToBase64DataUrlthat handles Unicode safely
38-41: LGTM! Improved data URL formatting.The switch to
convertToBase64DataUrlproperly formats the data URL with the correct MIME type and safely handles Unicode content. This is a clear improvement over the previous approach.
82-103: LGTM! Well-implemented keyboard shortcut.The CMD+L shortcut (Cmd on Mac, Ctrl elsewhere) is properly implemented:
- Validates selection exists before proceeding
- Correctly calls callbacks to add selection to chat and focus input
- Follows CodeMirror keymap conventions by returning true when handled, false otherwise
This enhances the UX for adding code selections to chat.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (9)
3-30: LGTM!Import statements properly use path aliases, include necessary dependencies for the new local/external image handling, and follow coding guidelines.
42-47: LGTM!The schema correctly validates internal drag-and-drop payloads and enables safe parsing with type checking.
49-76: LGTM!Component properly wrapped with
observerand state/refs initialized correctly per MobX and React guidelines.
78-93: LGTM!Focus management effects are implemented correctly with appropriate dependencies.
122-147: LGTM!Keyboard event handling correctly manages Tab navigation and Enter key submission with proper IME composition support.
172-186: LGTM!Paste handling correctly detects and processes image clipboard items.
188-259: LGTM!Drop handling elegantly distinguishes internal (local branch images) from external (file system) sources, with proper validation, error handling, and source attribution.
344-361: LGTM!Drag state management correctly identifies both internal (application/json) and external image sources for visual feedback.
363-458: LGTM!Chat mode toggle handler and JSX structure correctly integrate all components, event handlers, and drag-and-drop functionality with proper styling and conditional rendering.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (2)
97-104: Avoid double-invoking suggestion selection
handleEnterSelection()is executed twice on the same keydown path, so the suggestion logic runs twice and can insert duplicate content or trigger side effects twice. Store the result once and reuse it.- if (e.key === 'Enter' && suggestionRef.current?.handleEnterSelection()) { + if (e.key === 'Enter') { + const suggestionHandled = + suggestionRef.current?.handleEnterSelection() ?? false; + if (!suggestionHandled) { + return; + } e.preventDefault(); e.stopPropagation(); // Stop the event from bubbling to the canvas e.stopImmediatePropagation(); - // Handle the suggestion selection - suggestionRef.current.handleEnterSelection(); + requestAnimationFrame(() => textareaRef.current?.focus()); }
168-173: Localize placeholder text
getPlaceholderTextstill returns a literal string for the ASK mode. Please move it intonext-intlusing the existingthelper and translation keys so all user-facing copy is localized. Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (8)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/components/store/editor/chat/index.ts (2)
FOCUS_CHAT_INPUT_EVENT(8-8)sendMessage(43-49)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/context-pills/helpers.tsx (1)
validateImageLimit(35-48)packages/utility/src/file.ts (1)
convertToBase64DataUrl(120-125)packages/models/src/chat/message/context.ts (1)
ImageMessageContext(38-45)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/suggestions.tsx (1)
SuggestionsRef(9-12)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/queue-items/index.tsx (1)
QueueItems(10-50)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/action-buttons.tsx (1)
ActionButtons(7-78)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
Show resolved
Hide resolved
2 participants
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Important
This PR enhances image handling by separating image contexts, improving drag-and-drop functionality, and refining image upload processes across the application.
ChatContextincontext.ts.context.ts.public/by default inupload-image.ts.DraftImagePilltoImagePilland updates references increate.tsxandinput-context-pills.tsx.chat-input/index.tsxandimage-item.tsx.ImagePillto handle image removal inimage-pill.tsx.convertToBase64DataUrlinfile.tsfor consistent image encoding.read-file.tsto useconvertToBase64for binary files.sourceattribute toImageMessageContextincontext.tsto distinguish between local and external images.This description was created by
for 66e76b7. You can customize this summary. It will automatically update as commits are pushed.