Conversation
- Implement drag-and-drop file upload functionality in the Copilot component. - Add file upload progress tracking and error handling in the ChatBox. - Introduce new components for file management, including FileList, FileCard, and MessageFileList. - Ensure proper styling with Tailwind CSS and maintain performance optimizations using React.memo. - Update translations for file-related messages and limits. - Refactor existing components to support new file handling features and improve user experience.
…onents - Moved file type configuration, image file checking, file size formatting, and file extension extraction to a new utility module for better code organization. - Updated FileCard and MessageFileList components to utilize the new utility functions, improving readability and maintainability. - Enhanced styling with Tailwind CSS for consistency across components. - Implemented performance optimizations using React.memo and useMemo for efficient rendering.
- Introduced comprehensive unit tests for ToolService, covering the instantiation of copilot toolsets including read_file and list_files tools. - Validated the correct metadata and functionality of the tools within the service. - Ensured that the tests utilize mocking for dependencies to isolate the ToolService behavior. - Enhanced code maintainability and reliability through thorough testing.
- Simplified the CopilotActions component by adjusting the button styles and tooltip logic for better clarity and performance. - Enhanced the FileCard component by improving the upload progress handling and error display, ensuring proper use of optional chaining and nullish coalescing. - Updated file type configurations for better consistency and clarity in file handling. - Improved styling with Tailwind CSS for a more cohesive design across components. - Implemented performance optimizations using React.memo and refined component structures for maintainability.
- Updated FileCard to support a new 'compact' mode for improved display of image files, including upload progress indicators and error handling. - Introduced useMemo for determining if all files are images, optimizing rendering in FileList. - Enhanced styling with Tailwind CSS for better visual consistency. - Implemented performance optimizations using React.memo and refined component structures for maintainability.
…omponents - Implemented real-time upload progress tracking in the ChatBox component using XMLHttpRequest for improved user feedback. - Updated FileCard to differentiate between upload and processing phases based on metadata presence, enhancing UI clarity. - Replaced static progress indicators with dynamic loading icons for better user experience during file processing. - Ensured proper use of optional chaining and nullish coalescing for safer data handling. - Maintained styling consistency with Tailwind CSS across components.
📝 WalkthroughWalkthroughAdds a file upload/preview UX and end-to-end upload hook, integrates uploads into Copilot/chat flows and stores, adds FilePreview components, enriches DriveFile usage in invocation params, exposes Copilot-scoped read/list file tools, and auto-detects/enables vision capability for known models in skill invocation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatBox
participant FileUploadHook as useFileUpload
participant UploadAPI as /v1/misc/upload
participant DriveAPI as Drive API
participant Store
User->>ChatBox: Drop/select files
ChatBox->>FileUploadHook: handleFileUpload(files)
loop per file
FileUploadHook->>FileUploadHook: validate & create pending contextItem
FileUploadHook->>Store: create upload entry (uploadId)
FileUploadHook->>UploadAPI: XHR upload (progress)
UploadAPI-->>FileUploadHook: progress events / storageKey
FileUploadHook->>DriveAPI: batchCreateDriveFiles(storageKey)
DriveAPI-->>FileUploadHook: DriveFile (fileId)
FileUploadHook->>FileUploadHook: update contextItem (entityId -> fileId)
FileUploadHook->>Store: mark upload completed
end
FileUploadHook-->>ChatBox: provide contextItems + relevantUploads
User->>ChatBox: Send message (includes completedFileItems)
ChatBox->>FileUploadHook: clearFiles()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/index.tsx (1)
172-181: Stale closure in cleanup function may leak object URLs.The cleanup function captures
fileContentfrom the initial render. WhenfileContentchanges, the old object URL won't be revoked because the cleanup sees only the current value at cleanup time (after state update). This can cause memory leaks.Proposed fix using a ref to track the URL
+import { memo, useState, useEffect, useCallback, useMemo, useRef } from 'react'; ... + const fileUrlRef = useRef<string | null>(null); + useEffect(() => { fetchFileContent(); - - // Cleanup object URL on unmount - return () => { - if (fileContent?.url) { - URL.revokeObjectURL(fileContent.url); - } - }; }, [fetchFileContent]); + + // Track and cleanup object URLs properly + useEffect(() => { + if (fileContent?.url) { + fileUrlRef.current = fileContent.url; + } + return () => { + if (fileUrlRef.current) { + URL.revokeObjectURL(fileUrlRef.current); + fileUrlRef.current = null; + } + }; + }, [fileContent?.url]);
🤖 Fix all issues with AI agents
In `@apps/api/src/modules/skill/skill.service.ts`:
- Around line 436-442: The code accesses modelProviderMap.agent.config,
modelProviderMap.titleGeneration.config, and
modelProviderMap.queryAnalysis.config without null checks, which can throw when
findDefaultProviderItem returns null; mirror the defensive pattern used for
copilot by checking each provider entry before parsing and passing to
ensureVisionCapability: for agent/titleGeneration/queryAnalysis, if the entry
exists call ensureVisionCapability(safeParseJSON(entry.config) as
LLMModelConfig) else pass undefined. Update the calls that reference agent,
titleGeneration, and queryAnalysis in the block alongside chat and copilot to
use this null-safe conditional access.
In `@apps/api/src/modules/tool/tool.service.spec.ts`:
- Around line 6-9: The test file imports SkillService, ToolExecutionService,
SandboxService, and ResourceService which are unused/incorrect for ToolService;
open the ToolService class to see its actual constructor parameters and replace
these imports with the exact classes/providers it depends on (use the same
symbols named in ToolService's constructor), then update the
Test.createTestingModule providers to provide those real dependencies (or mocks)
so the spec imports match ToolService's actual dependencies.
- Around line 22-36: The test module is missing required providers for
ToolService; add mock providers for EncryptionService, McpServerService,
ComposioService, CreditService, ToolFactory, ToolInventoryService, and
ToolWrapperFactoryService to the Test.createTestingModule providers array
(similar to existing mocks for ConfigService/PrismaService/SkillService/etc.),
ensuring each uses a simple stubbed useValue/mock object so Nest can instantiate
ToolService in beforeEach where service = module.get<ToolService>(ToolService).
In
`@packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/file-preview-modal.tsx`:
- Around line 108-118: The code uses a non-null assertion on language when
rendering CodeRenderer in file-preview-modal.tsx which can pass undefined at
runtime (see extractContentCategory -> category 'code'), so update the case
'code' branch to avoid language! by providing a safe fallback or resolving a
language before render (e.g., derive from file extension or use a default like
'text'/'plaintext'), or change CodeRenderer to accept undefined and handle it
internally; reference the CodeRenderer usage and the language variable when
making the fix.
- Around line 26-50: The extractContentCategory function is duplicated; move it
into a single shared utility and import it where needed. Create a new utility
module containing the extractContentCategory implementation (and its
ContentCategoryResult type) that uses getCodeLanguage, then replace the local
implementations in both extractContentCategory usages (e.g., in
file-preview-modal.tsx and index.tsx) with an import from that new module so
both files call the same shared function.
In `@packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx`:
- Around line 523-545: The inline style object on the chat container and the
hardcoded color classes for the abort modal should be replaced with Tailwind
tokens and static classes: move the boxShadow inline style into a reusable
Tailwind class (or add it to the component stylesheet/utility) and replace
style-based color values in Modal.confirm okButtonProps (currently using
'!bg-[`#0E9F77`]' etc.) with the design-system error token classes (e.g.,
bg-red-600 border-red-600 hover:bg-red-700 hover:border-red-700) for the abort
action; update references around Modal.confirm, onOk/abortAction,
currentExecutingResult.resultId and the surrounding div to use the new
classNames and remove inline object creation.
- Around line 115-141: The selector passed to useCopilotStoreShallow and the
results mapping should use optional chaining and nullish coalescing: update
references to currentSessionId, sessionResultIds, pendingPrompt (e.g.,
state.currentSessionId[canvasId],
state.sessionResultIds[state.currentSessionId?.[canvasId]],
state.pendingPrompt[canvasId]) to use ?. and ?? as appropriate, and change the
results mapping inside useMemo to safely access resultMap (use
resultMap?.[resultId] ?? undefined) so lookups and Record indexings are
null-safe; adjust set/get usages accordingly in the selector and results
computation to avoid runtime undefined errors.
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx`:
- Line 118: Translate the Chinese inline comments in
packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx to
English: locate the JSX/JS comments around the uploading overlay and progress UI
inside the FileCard component (the comments at the positions corresponding to
lines 118, 136, and 143) and replace each Chinese comment with a clear English
equivalent (e.g., "Uploading Stage - Dark overlay + Centered Progress Ring",
"Upload failed state - retry button and error message", "File metadata / actions
area"). Keep comment meaning intact and concise, preserving existing comment
placement and punctuation.
- Around line 107-114: Create a proper TypeScript union type (e.g., FileIconType
= '3d'|'acrobat'|'android'|...|'video') that lists the 17 allowed literal values
and export it from file-utils.ts, then update the FILE_TYPE_CONFIG type to use
this FileIconType instead of string | undefined so fileConfig.type is typed as
FileIconType | undefined; finally remove the `as any` casts in the FileIcon
usages (references: FileIcon component usage in file-card.tsx and the second
occurrence mentioned) so the prop accepts the typed fileConfig.type directly.
In `@packages/canvas-common/src/context.ts`:
- Around line 196-224: The filesFromContextItems mapping currently constructs
SkillContextFileItem/DriveFile even when item.entityId or canvasId are missing
and uses direct property access; update the map callback in
filesFromContextItems to skip items lacking a valid item.entityId (or resolved
detail.fileId from variableToFileIdMap) and to only attach the nested file
object when required fields (fileId and canvasId) exist, using optional chaining
(e.g., item?.entityId, item?.metadata?.canvasId) to guard accesses; ensure
variable resolution via variableToFileIdMap.get(item.metadata.variableId) also
verifies the returned detail contains fileId and canvasId before returning a
SkillContextFileItem, and log/omit items that fail those validations so
downstream consumers receive only valid DriveFile metadata.
In `@packages/i18n/src/zh-Hans/ui.ts`:
- Around line 3601-3617: Replace the hardcoded “10” and “50MB” values in the
translation entries with placeholders and ensure the render layer injects config
values; specifically update the translation keys fileLimit.reached,
fileLimit.someSkipped, fileSizeLimit, maxFilesPerTask, maxUploadSize (and
uploading if showing size) to use placeholders like {{maxFiles}} and {{maxSize}}
(or {{maxSizeLabel}}) and update the calling code that retrieves these
translations to pass the numeric/string values from the app/config (e.g.,
reading maxFiles and maxUploadSize from configuration or constants) so the UI
always reflects the current limits.
🧹 Nitpick comments (23)
packages/ai-workspace-common/src/styles/additional-styles/utility-patterns.css (1)
91-114: Consider respectingprefers-reduced-motionfor accessibility.The shake animation provides good visual feedback for errors, but users with vestibular disorders or motion sensitivity may find it uncomfortable. Adding a reduced-motion media query improves accessibility.
♿ Proposed accessibility enhancement
`@keyframes` shake { 0%, 100% { transform: translateX(0); } 10%, 30%, 50%, 70%, 90% { transform: translateX(-2px); } 20%, 40%, 60%, 80% { transform: translateX(2px); } } .animate-shake { animation: shake 0.5s ease-in-out; } + +@media (prefers-reduced-motion: reduce) { + .animate-shake { + animation: none; + } +}As per coding guidelines: "Use clear visual indicators for focus, hover, and error states in form elements" — ensuring the animation respects user preferences maintains usability while still providing feedback through other means (e.g., color changes, border styles).
packages/skill-template/src/prompts/templates/copilot-agent-system.md (2)
51-51: Consider using proper markdown headings instead of bold emphasis.Lines 51, 59, 89, and 97 use bold emphasis (
**text**) for what appear to be section headings. Using proper markdown headings (e.g.,#### Step 1: Identify Image Type) would improve document structure, enable better navigation/TOC generation, and enhance accessibility.📝 Suggested markdown heading structure
Line 51:
-**Step 1: Identify Image Type** +#### Step 1: Identify Image TypeLine 59:
-**Step 2: Extract Key Information Based on Type** +#### Step 2: Extract Key Information Based on TypeLine 89:
-**Step 3: Design Workflow with Specific Details** +#### Step 3: Design Workflow with Specific DetailsLine 97:
-**Step 4: Quality Check** +#### Step 4: Quality CheckAlso applies to: 59-59, 89-89, 97-97
112-112: Add language specifiers to fenced code blocks for better rendering.The code blocks at lines 112, 246, 255, and 264 are missing language specifiers. Adding appropriate identifiers (e.g.,
```markdownor```text) would enable syntax highlighting and improve rendering in markdown viewers.📝 Suggested language specifiers
Line 112 (analysis example):
-``` +```text Image Type: UI/UX Design - Login page mockupLine 246 (scenario example):
-``` +```text User: "Analyze this sales data and generate a report"Line 255 (scenario example):
-``` +```text User: "Create tests for these API endpoints"Line 264 (scenario example):
-``` +```text User: "Summarize this technical specification"Also applies to: 246-246, 255-255, 264-264
apps/api/src/modules/tool/tool.service.spec.ts (1)
43-151: ExtractmockEngineto reduce duplication.The
mockEngineobject is defined identically in all four tests. Consider extracting it to thedescribeblock level or a shared helper.♻️ Proposed refactor to extract mockEngine
describe('instantiateCopilotToolsets', () => { + const mockEngine = { + service: { + librarySearch: jest.fn(), + getActionResult: jest.fn(), + getFileContent: jest.fn(), + listFiles: jest.fn(), + generateDoc: jest.fn(), + generateCodeArtifact: jest.fn(), + sendEmail: jest.fn(), + getTime: jest.fn(), + executeCode: jest.fn(), + }, + }; + it('should return 5 tools including read_file and list_files', () => { const mockUser = createMock<User>(); - const mockEngine = { - service: { - librarySearch: jest.fn(), - getActionResult: jest.fn(), - getFileContent: jest.fn(), - listFiles: jest.fn(), - generateDoc: jest.fn(), - generateCodeArtifact: jest.fn(), - sendEmail: jest.fn(), - getTime: jest.fn(), - executeCode: jest.fn(), - }, - }; // Access private method via type assertion const tools = (service as any).instantiateCopilotToolsets(mockUser, mockEngine); // ... rest of test }); // Apply same removal to other tests... });apps/api/src/modules/skill/skill-invoker.service.ts (1)
623-633: Consider using debug log level for vision capability diagnostics.This logging runs on every skill invocation with files and logs detailed internal state. While useful during development,
this.logger.infomay be too verbose for production. Consider usingthis.logger.debuginstead.♻️ Suggested change
- this.logger.info( + this.logger.debug( { filesCount: ctx?.files?.length ?? 0, imageFilesCount: imageFiles.length, hasVisionCapability, explicitVisionConfig: modelConfig?.capabilities?.vision, providerItemId: data.providerItem?.itemId, modelId, }, '[Vision Debug] Image processing check', );packages/ai-workspace-common/src/components/canvas/copilot/index.tsx (1)
136-155: Well-designed dropzone overlay.The visual feedback is comprehensive with distinct states for enabled vs. disabled uploads. Good use of semantic colors (
#00A870for success/enabled, gray tones for disabled).One minor consideration: The hardcoded colors (
#00A870,#C7CACD,#1C1F23) could be extracted to design system variables for consistency with the coding guidelines recommending success color#12B76A. However, if these match the Figma design specs, this is acceptable.packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx (1)
34-45: Consider error handling for parallel file uploads.The
Promise.allapproach is efficient for parallel uploads, but if one file fails, the entire batch is rejected and subsequent files won't complete their upload attempt. Also, errors are not caught here.Consider whether
Promise.allSettledwould be more appropriate for a better user experience, or ensure thatonUploadFilehandles errors internally.♻️ Optional: More resilient upload handling
const handleFileChange = useCallback( async (e: React.ChangeEvent<HTMLInputElement>) => { const files = e.target.files; if (!files?.length) return; - // Upload files in parallel - await Promise.all(Array.from(files).map((file) => onUploadFile(file))); + // Upload files in parallel, continue even if some fail + await Promise.allSettled(Array.from(files).map((file) => onUploadFile(file))); // Reset input e.target.value = ''; }, [onUploadFile], );packages/ai-workspace-common/src/components/canvas/copilot/file-utils.ts (2)
41-43: Consider adding explicit return types for utility functions.Per coding guidelines, functions (especially public APIs) should have explicit return types for better documentation and type safety.
♻️ Optional: Add explicit return types
-export const getFileTypeConfig = (ext: string) => { +export const getFileTypeConfig = (ext: string): { color: string; type?: string } => { return FILE_TYPE_CONFIG[ext.toLowerCase()] || FILE_TYPE_CONFIG.default; };-export const formatFileSize = (bytes?: number): string => { +export const formatFileSize = (bytes?: number): string => { // Already has return type - good!-export const getFileExtension = (filename?: string): string => { +export const getFileExtension = (filename?: string): string => { // Already has return type - good!Only
getFileTypeConfigis missing an explicit return type.Also applies to: 67-72, 77-79
50-62: Consider clarifying parameter semantics forisImageFile.The dual-purpose first parameter
extOrMimeTypeworks but the signature could be clearer. The current implementation correctly prioritizes MIME type checking.♻️ Optional: Clearer function signature with JSDoc
/** * Check if file is an image type based on extension or MIME type + * `@param` mimeTypeOrExt - Either a MIME type (e.g., "image/png") or file extension + * `@param` ext - Optional explicit extension to check (takes precedence if mimeTypeOrExt is not a MIME type) */ -export const isImageFile = (extOrMimeType?: string, ext?: string): boolean => { +export const isImageFile = (mimeTypeOrExt?: string, ext?: string): boolean => { // Check MIME type first - if (extOrMimeType?.startsWith('image/')) return true; + if (mimeTypeOrExt?.startsWith('image/')) return true; // Check extension - const extension = ext ?? extOrMimeType; + const extension = ext ?? mimeTypeOrExt; if (extension) { return IMAGE_EXTENSIONS.includes(extension.toLowerCase()); } return false; };packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/index.tsx (1)
207-304: Consider memoizingrenderFilePreviewwithuseCallback.Per coding guidelines, expensive computations and functions should be memoized.
renderFilePreviewis a render function that creates new JSX elements on every render, potentially causing unnecessary re-renders of child components.Wrap with useCallback
- const renderFilePreview = () => { + const renderFilePreview = useCallback(() => { if (loading) { return <Skeleton.Button block active={true} size="large" style={{ height: '140px' }} />; } // ... rest of the function - }; + }, [loading, error, fileContent, file, t, activeTab, handleTabChange, disableTruncation, purePreview, markdownClassName, handleDownload, isDownloading, fetchFileContent]);packages/ai-workspace-common/src/components/canvas/copilot/file-list.tsx (4)
63-64: MemoizeuploadMapto prevent unnecessary re-renders.Creating a new
Mapon every render defeats the purpose of memoization for child components. Sinceuploadsis in the dependency, the map should be memoized.Wrap with useMemo
- // Create a map of uploadId -> UploadProgress for quick lookup - const uploadMap = new Map(uploads.map((u) => [u.id, u])); + // Create a map of uploadId -> UploadProgress for quick lookup + const uploadMap = useMemo(() => new Map(uploads.map((u) => [u.id, u])), [uploads]);
88-90: WraphandleScrollRightwithuseCallback.Per coding guidelines, function props and event handlers should use
useCallbackto maintain referential equality and prevent unnecessary re-renders.Proposed fix
- const handleScrollRight = () => { - scrollRef.current?.scrollBy({ left: 174, behavior: 'smooth' }); - }; + const handleScrollRight = useCallback(() => { + scrollRef.current?.scrollBy({ left: 174, behavior: 'smooth' }); + }, []);
8-26: Consider movingChevronRightIconoutside the module or extracting to a shared location.This icon component is duplicated in
message-file-list.tsx. Per coding guidelines, avoiding code duplication is recommended. Consider extracting to a shared icons file or using a common icon library.
75-84: Debounce the resize event handler to improve performance.Per coding guidelines, handlers for events that might fire rapidly (like resize) should be debounced. The current implementation calls
checkScrollon every resize event.Add debouncing for resize handler
+import { useMemo } from 'react'; + +// Add a simple debounce utility or import from lodash +const debounce = <T extends (...args: any[]) => void>(fn: T, ms: number) => { + let timeoutId: ReturnType<typeof setTimeout>; + return (...args: Parameters<T>) => { + clearTimeout(timeoutId); + timeoutId = setTimeout(() => fn(...args), ms); + }; +}; useEffect(() => { checkScroll(); const el = scrollRef.current; + const debouncedCheckScroll = debounce(checkScroll, 100); el?.addEventListener('scroll', checkScroll); - window.addEventListener('resize', checkScroll); + window.addEventListener('resize', debouncedCheckScroll); return () => { el?.removeEventListener('scroll', checkScroll); - window.removeEventListener('resize', checkScroll); + window.removeEventListener('resize', debouncedCheckScroll); }; }, [fileItems.length, checkScroll]);packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/file-preview-modal.tsx (1)
156-161: SimplifycanDownload- useMemo is unnecessary here.The logic is simply checking if
fileContentis truthy. The comment says "All files can be downloaded" which makes the useMemo overly complex for a simple boolean check.Simplify to a direct boolean
- // Check if the file supports download - const canDownload = useMemo(() => { - if (!fileContent) return false; - // All files can be downloaded - return true; - }, [fileContent]); + // All files with content can be downloaded + const canDownload = !!fileContent;packages/ai-workspace-common/src/components/canvas/copilot/message-file-list.tsx (3)
11-29:ChevronRightIconis duplicated fromfile-list.tsx.This component is identical (except for default size) to the one in
file-list.tsx. Consider extracting to a shared icons file.Extract to shared location
Create a shared icon component:
// packages/ai-workspace-common/src/components/common/icons/chevron-right.tsx export const ChevronRightIcon = ({ size = 16, className }: { size?: number; className?: string }) => ( <svg width={size} height={size} viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg" className={className} > <path d="M9 18L15 12L9 6" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" /> </svg> );Then import in both files instead of duplicating.
197-199: WraphandleScrollRightwithuseCallback.Per coding guidelines, event handlers should use
useCallbackfor referential stability.Proposed fix
- const handleScrollRight = () => { - scrollRef.current?.scrollBy({ left: 200, behavior: 'smooth' }); - }; + const handleScrollRight = useCallback(() => { + scrollRef.current?.scrollBy({ left: 200, behavior: 'smooth' }); + }, []);
184-193: Same debouncing recommendation applies here.The resize event handler should be debounced to prevent performance issues during window resizing, similar to the issue in
file-list.tsx.packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx (2)
67-70: WraphandleRetryClickwithuseCallback.Per coding guidelines, event handlers should use
useCallbackto prevent unnecessary re-renders of child components.Proposed fix
- const handleRetryClick = (e: React.MouseEvent) => { - e.stopPropagation(); - onRetry?.(item.entityId); - }; + const handleRetryClick = useCallback((e: React.MouseEvent) => { + e.stopPropagation(); + onRetry?.(item.entityId); + }, [onRetry, item.entityId]);
40-44: Consider memoizing derived values for consistency.While
extensionandisImageare memoized,fileSize,thumbnailUrl, andfileConfigare computed inline. For consistency and to follow the guidelines about usinguseMemofor object creation, consider memoizing these as well.Proposed fix
const extension = useMemo(() => getFileExtension(item.title), [item.title]); const isImage = useMemo(() => isImageFile(extension), [extension]); - const fileSize = formatFileSize(item.metadata?.size); - const thumbnailUrl = item.metadata?.thumbnailUrl || item.metadata?.previewUrl; - const fileConfig = getFileTypeConfig(extension); + const fileSize = useMemo(() => formatFileSize(item.metadata?.size), [item.metadata?.size]); + const thumbnailUrl = item.metadata?.thumbnailUrl ?? item.metadata?.previewUrl; + const fileConfig = useMemo(() => getFileTypeConfig(extension), [extension]);packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx (3)
163-165: Add JSDoc for new functions (guideline requirement).Please add JSDoc for new handlers like
handleFileUpload,handleRetryFile,handleRemoveFile,handleSendMessage, andhandleAbort.✍️ Example JSDoc (apply similarly to other handlers)
+ /** + * Uploads a file and tracks progress, then creates a drive file reference. + */ const handleFileUpload = useCallback(
186-188: Use nullish coalescing for defaults.
||can override valid falsy values. Replace with??for IDs and MIME type defaults.🛠️ Proposed fix
- const uploadId = existingUploadId || genUniqueId(); - const tempEntityId = existingEntityId || `pending_${uploadId}`; + const uploadId = existingUploadId ?? genUniqueId(); + const tempEntityId = existingEntityId ?? `pending_${uploadId}`; ... - type: file.type || 'application/octet-stream', + type: file.type ?? 'application/octet-stream',Also applies to: 303-307
412-420: Avoid inline handlers in JSX and guard file arrays before.map.Inline functions break referential equality and violate the “no inline handlers/object creation in render” rule. Also guard
filesbefore mapping.🛠️ Proposed fix
+ const handleQueryChange = useCallback((value: string) => { + setQuery(value); + }, [setQuery]); + + const handleSendFromInput = useCallback(() => { + handleSendMessage('input_enter_send'); + }, [handleSendMessage]); + + const handleSendFromButton = useCallback(() => { + handleSendMessage('button_click_send'); + }, [handleSendMessage]); + + const handleUploadMultipleImages = useCallback(async (files: File[]) => { + await Promise.all((files ?? []).map((file) => handleFileUpload(file))); + }, [handleFileUpload]); ... - onRegisterFileUploadHandler(async (files: File[]) => { - // Upload files in parallel - await Promise.all(files.map((file) => handleFileUpload(file))); - }); + onRegisterFileUploadHandler(async (files: File[]) => { + // Upload files in parallel + await Promise.all((files ?? []).map((file) => handleFileUpload(file))); + }); ... - setQuery={(value) => { - setQuery(value); - }} + setQuery={handleQueryChange} ... - handleSendMessage={() => handleSendMessage('input_enter_send')} + handleSendMessage={handleSendFromInput} ... - onUploadMultipleImages={async (files) => { - // Upload files in parallel - await Promise.all(files.map((file) => handleFileUpload(file))); - }} + onUploadMultipleImages={handleUploadMultipleImages} ... - onSendMessage={() => handleSendMessage('button_click_send')} + onSendMessage={handleSendFromButton}Also applies to: 560-584
Comment on lines
+436
to
+442
| chat: ensureVisionCapability(safeParseJSON(modelProviderMap.chat.config) as LLMModelConfig), | ||
| copilot: ensureVisionCapability( | ||
| modelProviderMap.copilot | ||
| ? (safeParseJSON(modelProviderMap.copilot.config) as LLMModelConfig) | ||
| : undefined, | ||
| ), | ||
| agent: ensureVisionCapability(safeParseJSON(modelProviderMap.agent.config) as LLMModelConfig), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and look at the context around lines 436-442
cat -n apps/api/src/modules/skill/skill.service.ts | sed -n '400,470p'Repository: refly-ai/refly
Length of output: 3427
🏁 Script executed:
# Search for modelProviderMap definition and initialization
rg -n -B 5 -A 15 'modelProviderMap\s*=' apps/api/src/modules/skill --type=tsRepository: refly-ai/refly
Length of output: 1918
🏁 Script executed:
# Search for prepareModelProviderMap function
rg -n -A 20 'prepareModelProviderMap' apps/api/src/modules --type=tsRepository: refly-ai/refly
Length of output: 3875
🏁 Script executed:
# Get more of the prepareModelProviderMap function to see all model handling
cat -n apps/api/src/modules/provider/provider.service.ts | sed -n '946,1050p'Repository: refly-ai/refly
Length of output: 4821
🏁 Script executed:
# Also check ModelScene type definition to understand what models are required
rg -n 'type ModelScene\|enum ModelScene' apps/api/src --type=ts -A 10Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for ModelScene type/enum definition
rg -n 'ModelScene' apps/api/src/modules --type=ts | head -20Repository: refly-ai/refly
Length of output: 1544
🏁 Script executed:
# Get the rest of findDefaultProviderItem to see what it returns
cat -n apps/api/src/modules/provider/provider.service.ts | sed -n '1050,1100p'Repository: refly-ai/refly
Length of output: 2671
🏁 Script executed:
# Check if findDefaultProviderItem can return null for agent/titleGeneration/queryAnalysis
cat -n apps/api/src/modules/provider/provider.service.ts | sed -n '1092,1105p'Repository: refly-ai/refly
Length of output: 717
🏁 Script executed:
# Verify the return type is indeed nullable
rg -n 'findDefaultProviderItem' apps/api/src/modules/provider/provider.service.ts -A 5 | head -20Repository: refly-ai/refly
Length of output: 1110
Add null safety for agent, titleGeneration, and queryAnalysis model provider access.
Lines 436, 442-444 access modelProviderMap.chat.config, modelProviderMap.agent.config, modelProviderMap.titleGeneration.config, and modelProviderMap.queryAnalysis.config without null checks, while line 437-441 properly checks modelProviderMap.copilot for existence. The findDefaultProviderItem function can return null for agent, titleGeneration, and queryAnalysis (line 1102 in provider.service.ts), making direct property access unsafe. This is inconsistent with the defensive pattern used for copilot, image, video, and audio.
🛡️ Proposed fix to add null safety
chat: ensureVisionCapability(safeParseJSON(modelProviderMap.chat.config) as LLMModelConfig),
copilot: ensureVisionCapability(
modelProviderMap.copilot
? (safeParseJSON(modelProviderMap.copilot.config) as LLMModelConfig)
: undefined,
),
- agent: ensureVisionCapability(safeParseJSON(modelProviderMap.agent.config) as LLMModelConfig),
- titleGeneration: safeParseJSON(modelProviderMap.titleGeneration.config) as LLMModelConfig,
- queryAnalysis: safeParseJSON(modelProviderMap.queryAnalysis.config) as LLMModelConfig,
+ agent: ensureVisionCapability(
+ modelProviderMap.agent
+ ? (safeParseJSON(modelProviderMap.agent.config) as LLMModelConfig)
+ : undefined,
+ ),
+ titleGeneration: modelProviderMap.titleGeneration
+ ? (safeParseJSON(modelProviderMap.titleGeneration.config) as LLMModelConfig)
+ : undefined,
+ queryAnalysis: modelProviderMap.queryAnalysis
+ ? (safeParseJSON(modelProviderMap.queryAnalysis.config) as LLMModelConfig)
+ : undefined,🤖 Prompt for AI Agents
In `@apps/api/src/modules/skill/skill.service.ts` around lines 436 - 442, The code
accesses modelProviderMap.agent.config, modelProviderMap.titleGeneration.config,
and modelProviderMap.queryAnalysis.config without null checks, which can throw
when findDefaultProviderItem returns null; mirror the defensive pattern used for
copilot by checking each provider entry before parsing and passing to
ensureVisionCapability: for agent/titleGeneration/queryAnalysis, if the entry
exists call ensureVisionCapability(safeParseJSON(entry.config) as
LLMModelConfig) else pass undefined. Update the calls that reference agent,
titleGeneration, and queryAnalysis in the block alongside chat and copilot to
use this null-safe conditional access.
Comment on lines
+26
to
+50
| const extractContentCategory = (contentType: string, fileName: string): ContentCategoryResult => { | ||
| // Media types | ||
| if (contentType === 'image/svg+xml') return { category: 'svg' }; | ||
| if (contentType.startsWith('image/')) return { category: 'image' }; | ||
| if (contentType.startsWith('video/')) return { category: 'video' }; | ||
| if (contentType.startsWith('audio/')) return { category: 'audio' }; | ||
|
|
||
| // Document types | ||
| if (contentType === 'application/pdf') return { category: 'pdf' }; | ||
| if (contentType === 'application/json') return { category: 'json' }; | ||
| if (contentType === 'application/javascript') return { category: 'code' }; | ||
|
|
||
| // Text types - further categorize by file extension | ||
| if (contentType.startsWith('text/')) { | ||
| const language = getCodeLanguage(fileName); | ||
|
|
||
| if (language === 'html') return { category: 'html' }; | ||
| if (language === 'markdown' || language === 'mdx') return { category: 'markdown' }; | ||
| if (language) return { category: 'code', language }; | ||
|
|
||
| return { category: 'text' }; | ||
| } | ||
|
|
||
| return { category: 'unsupported' }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
extractContentCategory is duplicated from index.tsx.
This function is identical to the one in the parent index.tsx file. Per coding guidelines (DRY principle), consider extracting it to a shared utility file to avoid duplication.
Extract to shared utility
Create a new file packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/utils.ts:
import { getCodeLanguage } from '@refly-packages/ai-workspace-common/utils/file-type';
export interface ContentCategoryResult {
category: string;
language?: string;
}
export const extractContentCategory = (contentType: string, fileName: string): ContentCategoryResult => {
// Media types
if (contentType === 'image/svg+xml') return { category: 'svg' };
if (contentType.startsWith('image/')) return { category: 'image' };
if (contentType.startsWith('video/')) return { category: 'video' };
if (contentType.startsWith('audio/')) return { category: 'audio' };
// Document types
if (contentType === 'application/pdf') return { category: 'pdf' };
if (contentType === 'application/json') return { category: 'json' };
if (contentType === 'application/javascript') return { category: 'code' };
// Text types - further categorize by file extension
if (contentType.startsWith('text/')) {
const language = getCodeLanguage(fileName);
if (language === 'html') return { category: 'html' };
if (language === 'markdown' || language === 'mdx') return { category: 'markdown' };
if (language) return { category: 'code', language };
return { category: 'text' };
}
return { category: 'unsupported' };
};Then import from both files:
-const extractContentCategory = (contentType: string, fileName: string): ContentCategoryResult => {
- // ... implementation
-};
+import { extractContentCategory } from './utils';🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/file-preview-modal.tsx`
around lines 26 - 50, The extractContentCategory function is duplicated; move it
into a single shared utility and import it where needed. Create a new utility
module containing the extractContentCategory implementation (and its
ContentCategoryResult type) that uses getCodeLanguage, then replace the local
implementations in both extractContentCategory usages (e.g., in
file-preview-modal.tsx and index.tsx) with an import from that new module so
both files call the same shared function.
Comment on lines
+108
to
+118
| case 'code': | ||
| return ( | ||
| <CodeRenderer | ||
| source="preview" | ||
| fileContent={fileContent} | ||
| file={file} | ||
| language={language!} | ||
| activeTab="code" | ||
| onTabChange={() => {}} | ||
| disableTruncation={false} | ||
| /> |
There was a problem hiding this comment.
Non-null assertion on language may cause runtime issues.
When extractContentCategory returns { category: 'code' } without a language (e.g., for application/javascript), the language! assertion at line 114 will pass undefined to CodeRenderer, potentially causing issues.
Proposed fix
case 'code':
return (
<CodeRenderer
source="preview"
fileContent={fileContent}
file={file}
- language={language!}
+ language={language ?? 'text'}
activeTab="code"
onTabChange={() => {}}
disableTruncation={false}
/>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case 'code': | |
| return ( | |
| <CodeRenderer | |
| source="preview" | |
| fileContent={fileContent} | |
| file={file} | |
| language={language!} | |
| activeTab="code" | |
| onTabChange={() => {}} | |
| disableTruncation={false} | |
| /> | |
| case 'code': | |
| return ( | |
| <CodeRenderer | |
| source="preview" | |
| fileContent={fileContent} | |
| file={file} | |
| language={language ?? 'text'} | |
| activeTab="code" | |
| onTabChange={() => {}} | |
| disableTruncation={false} | |
| /> |
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/file-preview-modal.tsx`
around lines 108 - 118, The code uses a non-null assertion on language when
rendering CodeRenderer in file-preview-modal.tsx which can pass undefined at
runtime (see extractContentCategory -> category 'code'), so update the case
'code' branch to avoid language! by providing a safe fallback or resolving a
language before render (e.g., derive from file extension or use a default like
'text'/'plaintext'), or change CodeRenderer to accept undefined and handle it
internally; reference the CodeRenderer usage and the language variable when
making the fix.
Comment on lines
+523
to
545
| Modal.confirm({ | ||
| title: t('copilot.abortConfirmModal.title'), | ||
| content: t('copilot.abortConfirmModal.content'), | ||
| okText: t('copilot.abortConfirmModal.confirm'), | ||
| cancelText: t('copilot.abortConfirmModal.cancel'), | ||
| icon: null, | ||
| centered: true, | ||
| okButtonProps: { | ||
| className: '!bg-[#0E9F77] !border-[#0E9F77] hover:!bg-[#0C8A66] hover:!border-[#0C8A66]', | ||
| }, | ||
| onOk: async () => { | ||
| await abortAction(currentExecutingResult.resultId); | ||
| message.success(t('copilot.abortSuccess')); | ||
| }, | ||
| }); | ||
| }, [currentExecutingResult, abortAction, t]); | ||
|
|
||
| return ( | ||
| <div | ||
| className="w-full p-3 rounded-xl overflow-hidden border-[0.5px] border-solid border-[rgba(0,0,0,0.1)] bg-[rgba(255,255,255,0.9)]" | ||
| style={{ | ||
| boxShadow: '0px 5px 30px 0px rgba(31,35,41,0.05), 0px 0px 2px 0px rgba(31,35,41,0.04)', | ||
| }} |
There was a problem hiding this comment.
Replace inline styles and non-standard colors with Tailwind tokens.
Inline style violates Tailwind-only styling and “no inline object creation.” Also, abort is destructive — use error color #F04438 per guidelines.
🎨 Proposed fix
+ const containerClassName =
+ 'w-full p-3 rounded-xl overflow-hidden border-[0.5px] border-solid ' +
+ 'border-black/10 bg-white/90 ' +
+ 'shadow-[0px_5px_30px_0px_rgba(31,35,41,0.05),_0px_0px_2px_0px_rgba(31,35,41,0.04)]';
...
- <div
- className="w-full p-3 rounded-xl overflow-hidden border-[0.5px] border-solid border-[rgba(0,0,0,0.1)] bg-[rgba(255,255,255,0.9)]"
- style={{
- boxShadow: '0px 5px 30px 0px rgba(31,35,41,0.05), 0px 0px 2px 0px rgba(31,35,41,0.04)',
- }}
- >
+ <div className={containerClassName}>
...
- okButtonProps: {
- className: '!bg-[`#0E9F77`] !border-[`#0E9F77`] hover:!bg-[`#0C8A66`] hover:!border-[`#0C8A66`]',
- },
+ okButtonProps: {
+ className: '!bg-[`#F04438`] !border-[`#F04438`] hover:!bg-[`#F04438`] hover:!border-[`#F04438`]',
+ },🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx`
around lines 523 - 545, The inline style object on the chat container and the
hardcoded color classes for the abort modal should be replaced with Tailwind
tokens and static classes: move the boxShadow inline style into a reusable
Tailwind class (or add it to the component stylesheet/utility) and replace
style-based color values in Modal.confirm okButtonProps (currently using
'!bg-[`#0E9F77`]' etc.) with the design-system error token classes (e.g.,
bg-red-600 border-red-600 hover:bg-red-700 hover:border-red-700) for the abort
action; update references around Modal.confirm, onOk/abortAction,
currentExecutingResult.resultId and the surrounding div to use the new
classNames and remove inline object creation.
Comment on lines
+107
to
+114
| <FileIcon | ||
| color={fileConfig.color} | ||
| type={fileConfig.type as any} | ||
| fold={false} | ||
| height={32} | ||
| width="24" | ||
| glyphColor="white" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file-card.tsx to understand the context
wc -l packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsxRepository: refly-ai/refly
Length of output: 134
🏁 Script executed:
# Get the relevant code sections around lines 107-114 and 202-209
sed -n '100,120p' packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsxRepository: refly-ai/refly
Length of output: 790
🏁 Script executed:
# Also check lines 202-209
sed -n '195,215p' packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsxRepository: refly-ai/refly
Length of output: 819
🏁 Script executed:
# Find the FileIcon component
find . -name "*icon*" -type f \( -name "*.ts" -o -name "*.tsx" \) | head -20Repository: refly-ai/refly
Length of output: 372
🏁 Script executed:
# Search for FileIcon definition
rg "export.*FileIcon" --type=ts --type=tsx -A 3Repository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Check FileIcon component definition
cat -n packages/ai-workspace-common/src/components/common/resource-icon/file-icon.tsxRepository: refly-ai/refly
Length of output: 7576
🏁 Script executed:
# Also check the imports in file-card.tsx to understand where fileConfig comes from
head -50 packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsxRepository: refly-ai/refly
Length of output: 2024
🏁 Script executed:
# Now let's find the getFileTypeConfig function to understand what type it returns
rg -n "getFileTypeConfig" packages/ai-workspace-common/src/components/canvas/copilot/ -A 10Repository: refly-ai/refly
Length of output: 6442
🏁 Script executed:
# Let's search in file-utils to find the getFileTypeConfig definition
find . -name "*file-utils*" -type fRepository: refly-ai/refly
Length of output: 132
🏁 Script executed:
# Check the file-utils.ts to see FILE_TYPE_CONFIG and its type definition
cat -n packages/ai-workspace-common/src/components/canvas/copilot/file-utils.tsRepository: refly-ai/refly
Length of output: 3225
🏁 Script executed:
# Let's verify the exact types being passed match FileIcon's expectations
grep -n "type:" packages/ai-workspace-common/src/components/canvas/copilot/file-utils.tsRepository: refly-ai/refly
Length of output: 1118
🏁 Script executed:
# Check if there's any TypeScript definition file for FileIcon
find . -name "*file-icon*" -o -name "resource-icon*" | grep -E "\.(d\.ts|ts)$"Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Check if there's a types definition or index file for resource-icon
ls -la packages/ai-workspace-common/src/components/common/resource-icon/Repository: refly-ai/refly
Length of output: 417
🏁 Script executed:
# Check the index.tsx for resource-icon
cat -n packages/ai-workspace-common/src/components/common/resource-icon/index.tsxRepository: refly-ai/refly
Length of output: 182
🏁 Script executed:
# Let's verify the actual FileIcon component has TypeScript types or just PropTypes
head -100 packages/ai-workspace-common/src/components/common/resource-icon/file-icon.tsx | tail -30Repository: refly-ai/refly
Length of output: 518
Replace as any cast with proper type definition for FileIcon's type prop.
The fileConfig.type is loosely typed as string | undefined in the FILE_TYPE_CONFIG, but FileIcon expects one of 17 specific literal string values: '3d' | 'acrobat' | 'android' | 'audio' | 'binary' | 'code' | 'code2' | 'compressed' | 'document' | 'drive' | 'font' | 'image' | 'presentation' | 'settings' | 'spreadsheet' | 'vector' | 'video'.
Create a TypeScript type constant for FileIcon's valid type values and apply it to FILE_TYPE_CONFIG in file-utils.ts to ensure type safety without the as any cast. This applies to both occurrences (lines 107-114 and 202-209).
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx`
around lines 107 - 114, Create a proper TypeScript union type (e.g.,
FileIconType = '3d'|'acrobat'|'android'|...|'video') that lists the 17 allowed
literal values and export it from file-utils.ts, then update the
FILE_TYPE_CONFIG type to use this FileIconType instead of string | undefined so
fileConfig.type is typed as FileIconType | undefined; finally remove the `as
any` casts in the FileIcon usages (references: FileIcon component usage in
file-card.tsx and the second occurrence mentioned) so the prop accepts the typed
fileConfig.type directly.
| )} | ||
| </div> | ||
|
|
||
| {/* Uploading Stage - Dark overlay + Centered Progress Ring (对应截图1: 上传 loading) */} |
There was a problem hiding this comment.
Translate Chinese comments to English.
Per coding guidelines, all code comments MUST be written in English. Lines 118, 136, and 143 contain Chinese comments.
Proposed fix
- {/* Uploading Stage - Dark overlay + Centered Progress Ring (对应截图1: 上传 loading) */}
+ {/* Uploading Stage - Dark overlay + Centered Progress Ring */}
{isUploadPhase && (
- {/* Processing or Success Phase - Bottom right 16x16 indicator (对应截图2: 入库进度/成功) */}
+ {/* Processing or Success Phase - Bottom right 16x16 indicator */}
{isProcessingPhase && (
- {/* Success Phase - Bottom right 16x16 indicator with icon only */}
+ {/* Success Phase - Bottom right 16x16 indicator with icon only */}Also applies to: 136-136, 143-143
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx` at
line 118, Translate the Chinese inline comments in
packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx to
English: locate the JSX/JS comments around the uploading overlay and progress UI
inside the FileCard component (the comments at the positions corresponding to
lines 118, 136, and 143) and replace each Chinese comment with a clear English
equivalent (e.g., "Uploading Stage - Dark overlay + Centered Progress Ring",
"Upload failed state - retry button and error message", "File metadata / actions
area"). Keep comment meaning intact and concise, preserving existing comment
placement and punctuation.
Comment on lines
+196
to
+224
| const filesFromContextItems: SkillContextFileItem[] = | ||
| purgedItems | ||
| ?.filter((item) => item?.type === 'file') | ||
| .map((item): SkillContextFileItem | null => { | ||
| // For resource variables, resolve variableId to fileId | ||
| if (item.metadata?.source === 'variable' && item.metadata?.variableId) { | ||
| const detail = variableToFileIdMap.get(item.metadata.variableId); | ||
| if (detail) { | ||
| // Find the variable to get its name | ||
| return detail; | ||
| } | ||
| // If variableId cannot be resolved, skip this item | ||
| console.warn(`Cannot resolve variableId ${item.metadata.variableId} to fileId, skipping`); | ||
| return null; | ||
| } | ||
| // If variableId cannot be resolved, skip this item | ||
| console.warn(`Cannot resolve variableId ${item.metadata.variableId} to fileId, skipping`); | ||
| return null; | ||
| } | ||
| // For direct file references, use entityId as fileId | ||
| return { | ||
| fileId: item.entityId, | ||
| }; | ||
| }) | ||
| .filter((item): item is SkillContextFileItem => item !== null); | ||
| // For direct file references, use entityId as fileId and preserve metadata | ||
| return { | ||
| fileId: item.entityId, | ||
| // Store partial file metadata for display purposes | ||
| file: { | ||
| fileId: item.entityId, | ||
| name: item.title ?? '', | ||
| type: item.metadata?.mimeType ?? '', | ||
| size: item.metadata?.size, | ||
| canvasId: '', // Required by type but not needed for display | ||
| } as DriveFile, | ||
| }; | ||
| }) | ||
| .filter((item): item is SkillContextFileItem => item !== null) ?? []; |
There was a problem hiding this comment.
Guard missing fileId/canvasId before emitting DriveFile metadata.
If item.entityId is absent or canvasId is empty, this produces invalid DriveFile data that can fail downstream validation or deduplication. Also, new property accesses should use optional chaining per guidelines. Consider skipping invalid items and only attaching file when required fields exist.
✅ Proposed fix
.map((item): SkillContextFileItem | null => {
// For resource variables, resolve variableId to fileId
- if (item.metadata?.source === 'variable' && item.metadata?.variableId) {
- const detail = variableToFileIdMap.get(item.metadata.variableId);
+ if (item.metadata?.source === 'variable') {
+ const variableId = item.metadata?.variableId;
+ if (!variableId) {
+ console.warn('File context item missing variableId, skipping');
+ return null;
+ }
+ const detail = variableToFileIdMap.get(variableId);
if (detail) {
// Find the variable to get its name
return detail;
}
// If variableId cannot be resolved, skip this item
- console.warn(`Cannot resolve variableId ${item.metadata.variableId} to fileId, skipping`);
+ console.warn(
+ 'Cannot resolve variableId ' + variableId + ' to fileId, skipping',
+ );
return null;
}
// For direct file references, use entityId as fileId and preserve metadata
- return {
- fileId: item.entityId,
- // Store partial file metadata for display purposes
- file: {
- fileId: item.entityId,
- name: item.title ?? '',
- type: item.metadata?.mimeType ?? '',
- size: item.metadata?.size,
- canvasId: '', // Required by type but not needed for display
- } as DriveFile,
- };
+ const fileId = item?.entityId;
+ if (!fileId) {
+ console.warn('File context item missing entityId, skipping');
+ return null;
+ }
+ const canvasId = item.metadata?.canvasId;
+ return {
+ fileId,
+ ...(canvasId
+ ? {
+ // Store partial file metadata for display purposes
+ file: {
+ fileId,
+ name: item?.title ?? '',
+ type: item.metadata?.mimeType ?? '',
+ size: item.metadata?.size,
+ canvasId,
+ } as DriveFile,
+ }
+ : {}),
+ };
})🤖 Prompt for AI Agents
In `@packages/canvas-common/src/context.ts` around lines 196 - 224, The
filesFromContextItems mapping currently constructs
SkillContextFileItem/DriveFile even when item.entityId or canvasId are missing
and uses direct property access; update the map callback in
filesFromContextItems to skip items lacking a valid item.entityId (or resolved
detail.fileId from variableToFileIdMap) and to only attach the nested file
object when required fields (fileId and canvasId) exist, using optional chaining
(e.g., item?.entityId, item?.metadata?.canvasId) to guard accesses; ensure
variable resolution via variableToFileIdMap.get(item.metadata.variableId) also
verifies the returned detail contains fileId and canvasId before returning a
SkillContextFileItem, and log/omit items that fail those validations so
downstream consumers receive only valid DriveFile metadata.
Comment on lines
+3601
to
+3617
| fileLimit: { | ||
| reached: '已达到文件数量上限（10个）', | ||
| someSkipped: '因数量限制跳过 {{skipped}} 个文件', | ||
| }, | ||
| uploadFile: '上传文件', | ||
| selectFromLibrary: '从文件库选择', | ||
| addAttachment: '添加附件', | ||
| fileAttachment: '文件附件', | ||
| uploadInProgress: '请等待文件上传完成', | ||
| processing: '处理中...', | ||
| uploading: '上传中 {{progress}}%...', | ||
| uploadFailed: '上传失败', | ||
| addToFileFailed: '入库失败', | ||
| fileSizeLimit: '文件大小超过50MB限制', | ||
| maxFilesPerTask: '每次最多上传10个文件', | ||
| dropFilesToUpload: '拖放文件到此处上传', | ||
| maxUploadSize: '单个文件最大 50MB', |
There was a problem hiding this comment.
Parameterize file limits to avoid stale values.
Hardcoded “10” and “50MB” can drift from actual limits/config. Use placeholders and pass values from config to keep the UI accurate.
🔧 Suggested update
fileLimit: {
- reached: '已达到文件数量上限（10个）',
+ reached: '已达到文件数量上限（{{max}}个）',
someSkipped: '因数量限制跳过 {{skipped}} 个文件',
},
@@
- fileSizeLimit: '文件大小超过50MB限制',
- maxFilesPerTask: '每次最多上传10个文件',
+ fileSizeLimit: '文件大小超过 {{size}} MB 限制',
+ maxFilesPerTask: '每次最多上传 {{max}} 个文件',
dropFilesToUpload: '拖放文件到此处上传',
- maxUploadSize: '单个文件最大 50MB',
+ maxUploadSize: '单个文件最大 {{size}} MB',📝 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.
Suggested change
| fileLimit: { | |
| reached: '已达到文件数量上限（10个）', | |
| someSkipped: '因数量限制跳过 {{skipped}} 个文件', | |
| }, | |
| uploadFile: '上传文件', | |
| selectFromLibrary: '从文件库选择', | |
| addAttachment: '添加附件', | |
| fileAttachment: '文件附件', | |
| uploadInProgress: '请等待文件上传完成', | |
| processing: '处理中...', | |
| uploading: '上传中 {{progress}}%...', | |
| uploadFailed: '上传失败', | |
| addToFileFailed: '入库失败', | |
| fileSizeLimit: '文件大小超过50MB限制', | |
| maxFilesPerTask: '每次最多上传10个文件', | |
| dropFilesToUpload: '拖放文件到此处上传', | |
| maxUploadSize: '单个文件最大 50MB', | |
| fileLimit: { | |
| reached: '已达到文件数量上限（{{max}}个）', | |
| someSkipped: '因数量限制跳过 {{skipped}} 个文件', | |
| }, | |
| uploadFile: '上传文件', | |
| selectFromLibrary: '从文件库选择', | |
| addAttachment: '添加附件', | |
| fileAttachment: '文件附件', | |
| uploadInProgress: '请等待文件上传完成', | |
| processing: '处理中...', | |
| uploading: '上传中 {{progress}}%...', | |
| uploadFailed: '上传失败', | |
| addToFileFailed: '入库失败', | |
| fileSizeLimit: '文件大小超过 {{size}} MB 限制', | |
| maxFilesPerTask: '每次最多上传 {{max}} 个文件', | |
| dropFilesToUpload: '拖放文件到此处上传', | |
| maxUploadSize: '单个文件最大 {{size}} MB', |
🤖 Prompt for AI Agents
In `@packages/i18n/src/zh-Hans/ui.ts` around lines 3601 - 3617, Replace the
hardcoded “10” and “50MB” values in the translation entries with placeholders
and ensure the render layer injects config values; specifically update the
translation keys fileLimit.reached, fileLimit.someSkipped, fileSizeLimit,
maxFilesPerTask, maxUploadSize (and uploading if showing size) to use
placeholders like {{maxFiles}} and {{maxSize}} (or {{maxSizeLabel}}) and update
the calling code that retrieves these translations to pass the numeric/string
values from the app/config (e.g., reading maxFiles and maxUploadSize from
configuration or constants) so the UI always reflects the current limits.
…components - Replaced conditional rendering of file icons with a unified NodeIcon component for improved consistency and maintainability. - Enhanced image handling logic in FileCard and MessageFileList components using optional chaining and nullish coalescing for safer data access. - Removed unused file type configuration logic to simplify the codebase. - Updated styling with Tailwind CSS for better visual consistency across components. - Implemented performance optimizations using React.memo to prevent unnecessary re-renders.
- Increased icon size from 20 to 24 for improved visibility in the FileCard component. - Ensured consistent styling with Tailwind CSS.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In
`@packages/ai-workspace-common/src/components/canvas/canvas-resources/file-list/file-item.tsx`:
- Around line 37-43: Update the direct property accesses in the NodeIcon props
to use optional chaining like the rest of the component: change occurrences of
file.name, file.category and file.url passed into the NodeIcon (the NodeIcon
component instantiation) to use file?.name, file?.category and file?.url so
access is consistent with the existing file?.name usage elsewhere in this
component.
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx`:
- Around line 173-174: Replace the hard-coded red color used for
error/destructive states with the design-system error token (`#F04438`) or the
semantic class token (e.g., text-destructive) so components use the standard
error color; specifically update the Refresh icon usage in file-card.tsx (the
Refresh component with className="text-[`#D52515`]") and the other occurrences
referenced (lines ~224-245) to use the token (text-destructive or
text-[`#F04438`]) instead of text-[`#D52515`].
- Around line 94-103: The clickable card divs (e.g., the element using className
with isSuccess and onClick handled by handleCardClick) must be
keyboard-accessible: add tabIndex={0}, role="button", an appropriate aria-label
(descriptive text), and implement an onKeyDown handler that invokes
handleCardClick when Enter or Space is pressed; apply the same changes for the
other card blocks referenced (around the blocks you noted and functions handling
their clicks). For icon-only buttons, ensure each has a clear aria-label or
aria-labelledby and is focusable (tabIndex if not a native button) or,
preferably, convert them to <button> elements so they inherit keyboard behavior
and ARIA support by default (update any handlers like onClick to work with the
new element). Ensure existing click handlers (e.g., handleCardClick or icon
click handlers) are reused by the key handlers to avoid duplicate logic.
- Around line 46-47: Replace logical-OR fallbacks with nullish coalescing for
optional metadata and add safe fallbacks for possibly undefined titles: change
the thumbnail assignment (thumbnailUrl) to use ?? against
item.metadata?.previewUrl, change mime type fallback (mimeType) to use ??
'text/plain', and update all translation calls that currently use || (lines
referencing t(...)) to use ?? and ensure they pass a fallback string when
item.title may be undefined (e.g., t('key', { defaultValue: item.title ??
'Untitled' }) or similar). Locate and update usages of thumbnailUrl, mimeType,
and the translation invocations to apply these changes.
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-utils.ts`:
- Around line 27-31: Replace the magic numbers in formatFileSize with named
constants (e.g., KILOBYTE = 1024 and MEGABYTE = 1024 * KILOBYTE) and change the
initial null/undefined check from if (!bytes) to if (bytes == null) so that 0 is
handled correctly; then use the constants in the subsequent comparisons and
divisions inside formatFileSize (e.g., compare bytes < KILOBYTE and bytes <
MEGABYTE and divide by KILOBYTE/MEGABYTE when formatting).
In
`@packages/ai-workspace-common/src/components/canvas/copilot/message-file-list.tsx`:
- Around line 69-112: ImageThumbnail and MessageFileCard use clickable divs (and
the scroll arrow) without keyboard or ARIA support; update these components
(ImageThumbnail, MessageFileCard, and the scroll arrow handler) to use semantic
interactive elements or add keyboard handlers and ARIA attributes: replace or
augment the clickable divs with a button or add role="button", tabIndex={0},
onKeyDown that triggers handleClick for Enter/Space, and provide aria-label or
aria-labelledby for the actions; ensure NodeIcon receives an accessible text
alternative (e.g., pass alt or aria-hidden appropriately and add aria-label
describing the thumbnail) and verify setCurrentFile invocation in
MessageFileCard remains unchanged.
- Around line 58-65: The DriveFile construction uses || and raw item.title which
can lead to incorrect defaults or undefined values; update the driveFile
creation and other DriveFile constructions (references to the DriveFile type and
the driveFile variable) to use nullish coalescing (??) for fields like
metadata?.mimeType and metadata?.size, and provide explicit fallback values for
item.title (e.g., an empty string or "Untitled") wherever item.title is assigned
(the three occurrences creating DriveFile objects around the message-file-list
component). Also ensure optional chaining (?.) is used when accessing
item.metadata properties so required DriveFile fields are never left undefined.
🧹 Nitpick comments (5)
packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx (2)
69-72: Memoize event handlers and remove inline callbacks.
This keeps handler references stable and avoids inline function creation in render.♻️ Suggested update
- const handleRetryClick = (e: React.MouseEvent) => { + const handleRetryClick = useCallback((e: React.MouseEvent) => { e.stopPropagation(); onRetry?.(item.entityId); - }; + }, [onRetry, item.entityId]); + + const handleRemoveClick = useCallback((e: React.MouseEvent) => { + e.stopPropagation(); + onRemove(item.entityId); + }, [onRemove, item.entityId]);- {!disabled && !isUploading && ( - <button + {!disabled && !isUploading && ( + <button type="button" - onClick={(e) => { - e.stopPropagation(); - onRemove(item.entityId); - }} + onClick={handleRemoveClick} className={cn( 'absolute top-0.5 right-0.5 w-4 h-4 rounded-full opacity-0 group-hover:opacity-100 transition-opacity', 'bg-black/40 hover:bg-black/60 flex items-center justify-center', 'cursor-pointer border-none outline-none', )} >- {!disabled && !isUploading && ( - <button + {!disabled && !isUploading && ( + <button type="button" - onClick={(e) => { - e.stopPropagation(); - onRemove(item.entityId); - }} + onClick={handleRemoveClick} className={cn( 'absolute -top-1.5 -right-1.5 w-4 h-4 rounded-full opacity-0 group-hover:opacity-100 transition-opacity', 'bg-black/40 hover:bg-black/60 flex items-center justify-center', 'cursor-pointer border-none outline-none', )} >As per coding guidelines, always use useCallback for function props and avoid inline object/array creation in render.
Also applies to: 147-154, 266-271
94-103: Replace inline size styles with Tailwind utilities.
This avoids inline style objects and keeps styling consistent with Tailwind usage.🎨 Suggested update
- style={{ width: '48px', minWidth: '48px', maxWidth: '48px', height: '48px' }} + className={cn( + 'relative group rounded-lg overflow-hidden bg-[`#F6F6F6`]', + 'w-12 h-12 min-w-12 max-w-12', + hasError && 'bg-red-50 ring-1 ring-red-200', + isShaking && 'animate-shake', + isSuccess && 'cursor-pointer', + )}- style={{ width: '166px', minWidth: '166px', maxWidth: '166px', height: '48px' }} + className={cn( + 'relative group flex items-center gap-2 p-1 rounded-lg bg-[`#F6F6F6`]', + 'w-[166px] min-w-[166px] max-w-[166px] h-12', + hasError && 'bg-red-50', + isShaking && 'animate-shake', + isSuccess && 'cursor-pointer hover:bg-gray-200 transition-colors', + )}As per coding guidelines, always use Tailwind CSS for styling and avoid inline object/array creation in render.
Also applies to: 181-190
packages/ai-workspace-common/src/components/canvas/copilot/message-file-list.tsx (3)
10-28: WrapChevronRightIconwithReact.memoto avoid unnecessary re-renders.♻️ Suggested update
-const ChevronRightIcon = ({ size = 14, className }: { size?: number; className?: string }) => ( +const ChevronRightIcon = memo( + ({ size = 14, className }: { size?: number; className?: string }) => ( <svg width={size} height={size} viewBox="0 0 14 14" fill="none" xmlns="http://www.w3.org/2000/svg" className={className} > <path d="M5.25 3.5L8.75 7L5.25 10.5" stroke="currentColor" strokeWidth="1.5" strokeLinecap="round" strokeLinejoin="round" /> </svg> -); + ), +);As per coding guidelines, always wrap pure components with React.memo.
109-111: Replace inline styles with Tailwind utilities (including gradient mask).
This keeps styling consistent and avoids inline style objects.As per coding guidelines, always use Tailwind CSS for styling and avoid inline object/array creation in render.
Also applies to: 199-203
154-177: Extract scroll constants and memoize the scroll handler.
This removes magic numbers and keeps the handler stable for the onClick prop.♻️ Suggested update
+ const SCROLL_END_PADDING_PX = 10; + const SCROLL_OFFSET_PX = 200; + const checkScroll = useCallback(() => { const el = scrollRef.current; if (!el) return; const hasOverflow = el.scrollWidth > el.clientWidth; - const notAtEnd = el.scrollLeft < el.scrollWidth - el.clientWidth - 10; + const notAtEnd = + el.scrollLeft < el.scrollWidth - el.clientWidth - SCROLL_END_PADDING_PX; setShowRightArrow(hasOverflow && notAtEnd); }, []); - const handleScrollRight = () => { - scrollRef.current?.scrollBy({ left: 200, behavior: 'smooth' }); - }; + const handleScrollRight = useCallback(() => { + scrollRef.current?.scrollBy({ left: SCROLL_OFFSET_PX, behavior: 'smooth' }); + }, []);As per coding guidelines, avoid magic numbers and always use useCallback for function props.
Comment on lines
+37
to
+43
| <NodeIcon | ||
| type="file" | ||
| filename={file.name} | ||
| url={file.category === 'image' ? file.url : undefined} | ||
| filled={false} | ||
| small | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/canvas/canvas-resources/file-list/file-item.tsxRepository: refly-ai/refly
Length of output: 2433
Use optional chaining consistently throughout the component.
Line 39 directly accesses file.name, while line 51 uses file?.name. The codebase should use optional chaining uniformly. Update line 39–40 to:
🔧 Suggested update
<NodeIcon
type="file"
- filename={file.name}
- url={file.category === 'image' ? file.url : undefined}
+ filename={file?.name ?? ''}
+ url={file?.category === 'image' ? file?.url : undefined}
filled={false}
small
/>This aligns with the coding guidelines requiring optional chaining when accessing object properties and ensures consistency with the optional chaining pattern already used elsewhere in this component.
📝 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.
Suggested change
| <NodeIcon | |
| type="file" | |
| filename={file.name} | |
| url={file.category === 'image' ? file.url : undefined} | |
| filled={false} | |
| small | |
| /> | |
| <NodeIcon | |
| type="file" | |
| filename={file?.name ?? ''} | |
| url={file?.category === 'image' ? file?.url : undefined} | |
| filled={false} | |
| small | |
| /> |
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/canvas-resources/file-list/file-item.tsx`
around lines 37 - 43, Update the direct property accesses in the NodeIcon props
to use optional chaining like the rest of the component: change occurrences of
file.name, file.category and file.url passed into the NodeIcon (the NodeIcon
component instantiation) to use file?.name, file?.category and file?.url so
access is consistent with the existing file?.name usage elsewhere in this
component.
Comment on lines
+46
to
+47
| const thumbnailUrl = item.metadata?.thumbnailUrl || item.metadata?.previewUrl; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the file and check its existence
find . -name "file-card.tsx" -path "*canvas/copilot*" 2>/dev/nullRepository: refly-ai/refly
Length of output: 132
🏁 Script executed:
#!/bin/bash
# Check file size and read the relevant sections
wc -l "packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx"Repository: refly-ai/refly
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Read lines 40-95 to cover all the sections mentioned in the review
sed -n '40,95p' "packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx" | cat -nRepository: refly-ai/refly
Length of output: 2371
🏁 Script executed:
#!/bin/bash
# Read lines 200-260 to cover the remaining sections
sed -n '200,260p' "packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx" | cat -nRepository: refly-ai/refly
Length of output: 3178
Use nullish coalescing (??) instead of logical OR (||) for optional metadata fallbacks.
As per coding guidelines, all fallback patterns should use nullish coalescing to properly handle undefined values:
- Line 46:
item.metadata?.thumbnailUrl ?? item.metadata?.previewUrl - Line 85:
item.metadata?.mimeType ?? 'text/plain' - Lines 209, 250-252: Replace
||with??in translation calls
Additionally, provide fallback values for potentially undefined item.title in lines 84, 209, and 250.
🔧 Suggested updates
- const thumbnailUrl = item.metadata?.thumbnailUrl || item.metadata?.previewUrl;
+ const thumbnailUrl = item.metadata?.thumbnailUrl ?? item.metadata?.previewUrl; const driveFile: DriveFile = {
fileId: item.entityId,
canvasId,
- name: item.title,
- type: item.metadata?.mimeType || 'text/plain',
+ name: item.title ?? t('common.untitled'),
+ type: item.metadata?.mimeType ?? 'text/plain',
size: item.metadata?.size,
category: isImage ? 'image' : 'document',
};- {item.title}
+ {item.title ?? t('common.untitled')}- {errorType === 'addToFile' ? t('common.sync') || 'Sync' : t('common.retry') || 'Retry'}
+ {errorType === 'addToFile' ? (t('common.sync') ?? 'Sync') : (t('common.retry') ?? 'Retry')}📝 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.
Suggested change
| const thumbnailUrl = item.metadata?.thumbnailUrl || item.metadata?.previewUrl; | |
| const thumbnailUrl = item.metadata?.thumbnailUrl ?? item.metadata?.previewUrl; |
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx`
around lines 46 - 47, Replace logical-OR fallbacks with nullish coalescing for
optional metadata and add safe fallbacks for possibly undefined titles: change
the thumbnail assignment (thumbnailUrl) to use ?? against
item.metadata?.previewUrl, change mime type fallback (mimeType) to use ??
'text/plain', and update all translation calls that currently use || (lines
referencing t(...)) to use ?? and ensure they pass a fallback string when
item.title may be undefined (e.g., t('key', { defaultValue: item.title ??
'Untitled' }) or similar). Locate and update usages of thumbnailUrl, mimeType,
and the translation invocations to apply these changes.
Comment on lines
+94
to
+103
| <div | ||
| className={cn( | ||
| 'relative group rounded-lg overflow-hidden bg-[#F6F6F6]', | ||
| hasError && 'bg-red-50 ring-1 ring-red-200', | ||
| isShaking && 'animate-shake', | ||
| isSuccess && 'cursor-pointer', | ||
| )} | ||
| style={{ width: '48px', minWidth: '48px', maxWidth: '48px', height: '48px' }} | ||
| onClick={handleCardClick} | ||
| > |
There was a problem hiding this comment.
Make clickable containers and icon-only controls keyboard accessible.
The card wrappers and icon-only buttons should be reachable via keyboard and have ARIA labels.
As per coding guidelines, ensure interactive elements are keyboard accessible and include appropriate ARIA attributes / alternative text for icons.
Also applies to: 147-175, 181-191, 265-279
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx`
around lines 94 - 103, The clickable card divs (e.g., the element using
className with isSuccess and onClick handled by handleCardClick) must be
keyboard-accessible: add tabIndex={0}, role="button", an appropriate aria-label
(descriptive text), and implement an onKeyDown handler that invokes
handleCardClick when Enter or Space is pressed; apply the same changes for the
other card blocks referenced (around the blocks you noted and functions handling
their clicks). For icon-only buttons, ensure each has a clear aria-label or
aria-labelledby and is focusable (tabIndex if not a native button) or,
preferably, convert them to <button> elements so they inherit keyboard behavior
and ARIA support by default (update any handlers like onClick to work with the
new element). Ensure existing click handlers (e.g., handleCardClick or icon
click handlers) are reused by the key handlers to avoid duplicate logic.
Comment on lines
+173
to
+174
| <Refresh size={14} className="text-[#D52515]" /> | ||
| </button> |
There was a problem hiding this comment.
Use the standard error color token for error states.
Current hard-coded red does not match the design-system error color.
Based on learnings: Error color (#F04438) should be used for error states and destructive actions.
Also applies to: 224-245
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-card.tsx`
around lines 173 - 174, Replace the hard-coded red color used for
error/destructive states with the design-system error token (`#F04438`) or the
semantic class token (e.g., text-destructive) so components use the standard
error color; specifically update the Refresh icon usage in file-card.tsx (the
Refresh component with className="text-[`#D52515`]") and the other occurrences
referenced (lines ~224-245) to use the token (text-destructive or
text-[`#F04438`]) instead of text-[`#D52515`].
Comment on lines
+27
to
+31
| export const formatFileSize = (bytes?: number): string => { | ||
| if (!bytes) return ''; | ||
| if (bytes < 1024) return `${bytes}B`; | ||
| if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)}KB`; | ||
| return `${(bytes / (1024 * 1024)).toFixed(1)}MB`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine its content
file_path="packages/ai-workspace-common/src/components/canvas/copilot/file-utils.ts"
if [ -f "$file_path" ]; then
echo "File exists. Checking content around lines 27-31:"
cat -n "$file_path" | sed -n '20,40p'
else
echo "File not found at $file_path"
# Try to find the file
find . -name "file-utils.ts" -type f 2>/dev/null | head -5
fiRepository: refly-ai/refly
Length of output: 755
🏁 Script executed:
# Also check the file size to ensure we can read it entirely if needed
if [ -f "packages/ai-workspace-common/src/components/canvas/copilot/file-utils.ts" ]; then
wc -l "packages/ai-workspace-common/src/components/canvas/copilot/file-utils.ts"
fiRepository: refly-ai/refly
Length of output: 133
Replace magic numbers with named constants and fix zero-byte file handling.
The function uses hardcoded magic numbers (1024, 1024 * 1024) that repeat throughout the code. Additionally, if (!bytes) incorrectly treats 0 as falsy, causing zero-byte files to return an empty string instead of '0B'. Define constants for file size units and use bytes == null to properly handle the optional parameter:
const KILOBYTE = 1024;
const MEGABYTE = 1024 * KILOBYTE;
export const formatFileSize = (bytes?: number): string => {
if (bytes == null) return '';
if (bytes < KILOBYTE) return `${bytes}B`;
if (bytes < MEGABYTE) return `${(bytes / KILOBYTE).toFixed(1)}KB`;
return `${(bytes / MEGABYTE).toFixed(1)}MB`;
};🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/canvas/copilot/file-utils.ts`
around lines 27 - 31, Replace the magic numbers in formatFileSize with named
constants (e.g., KILOBYTE = 1024 and MEGABYTE = 1024 * KILOBYTE) and change the
initial null/undefined check from if (!bytes) to if (bytes == null) so that 0 is
handled correctly; then use the constants in the subsequent comparisons and
divisions inside formatFileSize (e.g., compare bytes < KILOBYTE and bytes <
MEGABYTE and divide by KILOBYTE/MEGABYTE when formatting).
Comment on lines
+58
to
+65
| const driveFile: DriveFile = { | ||
| fileId: item.entityId, | ||
| canvasId, | ||
| name: item.title, | ||
| type: item.metadata?.mimeType || 'image/*', | ||
| size: item.metadata?.size, | ||
| category: 'image', | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "message-file-list.tsx" -type fRepository: refly-ai/refly
Length of output: 140
🏁 Script executed:
rg "type DriveFile|interface DriveFile" -A 10Repository: refly-ai/refly
Length of output: 5576
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/canvas/copilot/message-file-list.tsx | head -150Repository: refly-ai/refly
Length of output: 6191
Use nullish coalescing and provide fallbacks for potentially undefined fields in DriveFile objects.
The code creates DriveFile objects with fields that may be undefined but are required by the type definition. Line 62 and 100 should use nullish coalescing (??) instead of the logical OR operator (||), and item.title assignments at lines 61, 99, and 128 require fallback values since they may be undefined.
🔧 Suggested updates
const driveFile: DriveFile = {
fileId: item.entityId,
canvasId,
- name: item.title,
+ name: item.title ?? 'Untitled',
- type: item.metadata?.mimeType || 'image/*',
+ type: item.metadata?.mimeType ?? 'image/*',
size: item.metadata?.size,
category: 'image',
}; const driveFile: DriveFile = {
fileId: item.entityId,
canvasId,
- name: item.title,
+ name: item.title ?? 'Untitled',
- type: item.metadata?.mimeType || 'text/plain',
+ type: item.metadata?.mimeType ?? 'text/plain',
size: item.metadata?.size,
category: 'document',
}; <div className="text-xs font-normal truncate text-gray-900 leading-normal">
- {item.title}
+ {item.title ?? 'Untitled'}
</div>Per coding guidelines, always use nullish coalescing (??) or default values for potentially undefined values, and optional chaining (?.) when accessing object properties.
Also applies to: 96-103, 127-129
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/message-file-list.tsx`
around lines 58 - 65, The DriveFile construction uses || and raw item.title
which can lead to incorrect defaults or undefined values; update the driveFile
creation and other DriveFile constructions (references to the DriveFile type and
the driveFile variable) to use nullish coalescing (??) for fields like
metadata?.mimeType and metadata?.size, and provide explicit fallback values for
item.title (e.g., an empty string or "Untitled") wherever item.title is assigned
(the three occurrences creating DriveFile objects around the message-file-list
component). Also ensure optional chaining (?.) is used when accessing
item.metadata properties so required DriveFile fields are never left undefined.
Comment on lines
+69
to
+112
| return ( | ||
| <div | ||
| className="w-12 h-12 flex-shrink-0 rounded-xl overflow-hidden cursor-pointer hover:opacity-80 transition-opacity" | ||
| onClick={handleClick} | ||
| > | ||
| <NodeIcon | ||
| type="image" | ||
| url={thumbnailUrl ?? undefined} | ||
| small={false} | ||
| className="!w-full !h-full !rounded-xl" | ||
| /> | ||
| </div> | ||
| ); | ||
| }); | ||
|
|
||
| ImageThumbnail.displayName = 'ImageThumbnail'; | ||
|
|
||
| // File card component - matches Figma design | ||
| const MessageFileCard = memo(({ item, canvasId }: { item: IContextItem; canvasId: string }) => { | ||
| const extension = getFileExtension(item.title); | ||
| const fileSize = formatFileSize(item.metadata?.size); | ||
| const { setCurrentFile } = useCanvasResourcesPanelStoreShallow((state) => ({ | ||
| setCurrentFile: state.setCurrentFile, | ||
| })); | ||
|
|
||
| const handleClick = useCallback(() => { | ||
| // Convert IContextItem to DriveFile format | ||
| const driveFile: DriveFile = { | ||
| fileId: item.entityId, | ||
| canvasId, | ||
| name: item.title, | ||
| type: item.metadata?.mimeType || 'text/plain', | ||
| size: item.metadata?.size, | ||
| category: 'document', | ||
| }; | ||
| setCurrentFile(driveFile); | ||
| }, [item, canvasId, setCurrentFile]); | ||
|
|
||
| return ( | ||
| <div | ||
| className="flex items-center gap-2 p-1 rounded-lg bg-[#F6F6F6] flex-shrink-0 cursor-pointer hover:bg-gray-200 transition-colors" | ||
| style={{ width: '166px', height: '48px' }} | ||
| onClick={handleClick} | ||
| > |
There was a problem hiding this comment.
Make clickable UI elements keyboard accessible and add ARIA labels.
The image thumbnail, file card, and scroll arrow are clickable divs without keyboard affordances.
As per coding guidelines, ensure all interactive elements are keyboard accessible and include appropriate ARIA attributes / alternative text for icons.
Also applies to: 195-210
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/message-file-list.tsx`
around lines 69 - 112, ImageThumbnail and MessageFileCard use clickable divs
(and the scroll arrow) without keyboard or ARIA support; update these components
(ImageThumbnail, MessageFileCard, and the scroll arrow handler) to use semantic
interactive elements or add keyboard handlers and ARIA attributes: replace or
augment the clickable divs with a button or add role="button", tabIndex={0},
onKeyDown that triggers handleClick for Enter/Space, and provide aria-label or
aria-labelledby for the actions; ensure NodeIcon receives an accessible text
alternative (e.g., pass alt or aria-hidden appropriately and add aria-label
describing the thumbnail) and verify setCurrentFile invocation in
MessageFileCard remains unchanged.
- Introduced a new custom hook, useFileUpload, to manage file uploads, including progress tracking and error handling. - Updated ChatBox and PureCopilot components to utilize the new file upload logic, improving maintainability and performance. - Enhanced file handling in CopilotMessage, FileCard, and MessageFileList components with improved logic for displaying file types and thumbnails. - Ensured consistent styling with Tailwind CSS across all updated components. - Implemented performance optimizations using React.memo and refined component structures for better maintainability. - Added support for pending files in the Copilot store to manage file states effectively.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/web/public/sitemap-templates.xml`:
- Around line 63-68: Replace the incorrect slug "wa-bum1orola64tamvrr370klc2" in
the <loc> element with the correct "wfa-bum1orola64tamvrr370klc2" so the URL
matches the other workflow-template entries; locate the <url> block whose <loc>
contains "wa-bum1orola64tamvrr370klc2" and update the slug prefix from "wa-" to
"wfa-".
In `@packages/ai-workspace-common/src/components/pure-copilot/index.tsx`:
- Around line 100-110: The polling in onCanvasRequired (which checks
canvasCreationInProgress.current and spins a setInterval waiting for
currentCanvasId) can run forever; change it to enforce a timeout/maximum
attempts by wrapping the interval in a cancellable promise that also starts a
setTimeout (or counts attempts) and on timeout clears the interval and rejects
(or resolves with a safe fallback) so the promise always settles; ensure you
clearInterval and clear the timeout when currentCanvasId becomes available, and
surface the error or fallback to the caller instead of leaving the interval
running.
In `@packages/ai-workspace-common/src/hooks/use-file-upload.ts`:
- Around line 26-345: Add a useEffect cleanup that runs on unmount to revoke any
created object URLs and clear pending upload state: iterate
pendingFilesRef.current to revoke each previewUrl and delete entries, revoke any
previewUrl on contextItems, call removeUpload for any remaining uploadIds, and
finally clear contextItems (reuse clearFiles if appropriate); place this effect
alongside the hook setup (referencing pendingFilesRef, contextItems,
removeUpload, and clearFiles) so active/uploads-orphaned previews are cleaned
when the component unmounts.
🧹 Nitpick comments (3)
apps/web/public/sitemap-templates.xml (1)
3-74: Consider generating this sitemap from the template source-of-truth to prevent drift.
Manual edits to long lists are prone to typos and staleness; a build-time generator could keep URLs and lastmod dates aligned.packages/ai-workspace-common/src/components/pure-copilot/index.tsx (1)
18-19: Consider using the sharedACCEPT_FILE_EXTENSIONSconstant to avoid duplication.This hardcoded string duplicates the accepted file extensions defined in
packages/ai-workspace-common/src/components/canvas/workflow-variables/constants.ts. If the allowed extensions change, this string could drift out of sync.♻️ Proposed refactor
+import { ACCEPT_FILE_EXTENSIONS } from '../canvas/workflow-variables/constants'; + -const ACCEPT_FILE_EXTENSIONS = - '.jpg,.jpeg,.png,.gif,.bmp,.webp,.pdf,.doc,.docx,.xls,.xlsx,.ppt,.pptx,.txt,.md,.csv'; +const ACCEPT_FILE_EXTENSIONS_STRING = ACCEPT_FILE_EXTENSIONS.map((ext) => `.${ext}`).join(',');Then use
ACCEPT_FILE_EXTENSIONS_STRINGat line 354.packages/ai-workspace-common/src/components/canvas/copilot/file-utils.ts (1)
83-84: Consider using nullish coalescing for the fallback.The
|| ''works correctly for strings since empty string is the intended fallback, but for consistency with coding guidelines, consider using?? ''.Suggested fix
export const getFileExtension = (filename?: string): string => { - return filename?.split('.').pop()?.toLowerCase() || ''; + return filename?.split('.').pop()?.toLowerCase() ?? ''; };
Comment on lines
+63
to
+68
| <url> | ||
| <loc>https://refly.ai/workflow-template/wa-bum1orola64tamvrr370klc2</loc> | ||
| <lastmod>2025-11-20</lastmod> | ||
| <changefreq>weekly</changefreq> | ||
| <priority>0.7</priority> | ||
| </url> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the slug patterns in the sitemap file
rg -n "bum1orola64tamvrr370klc2" -S
rg -n "workflow-template/wa-" -S
rg -n "workflow-template/wfa-" -SRepository: refly-ai/refly
Length of output: 1614
Fix typo in workflow-template slug on line 64: wa- should be wfa-.
Line 64 uses wa-bum1orola64tamvrr370klc2 while all 12 other entries in the file use the wfa- prefix. This inconsistency will produce a broken sitemap URL.
🤖 Prompt for AI Agents
In `@apps/web/public/sitemap-templates.xml` around lines 63 - 68, Replace the
incorrect slug "wa-bum1orola64tamvrr370klc2" in the <loc> element with the
correct "wfa-bum1orola64tamvrr370klc2" so the URL matches the other
workflow-template entries; locate the <url> block whose <loc> contains
"wa-bum1orola64tamvrr370klc2" and update the slug prefix from "wa-" to "wfa-".
Comment on lines
+100
to
+110
| onCanvasRequired: async () => { | ||
| if (canvasCreationInProgress.current) { | ||
| return new Promise<string>((resolve) => { | ||
| const checkInterval = setInterval(() => { | ||
| if (currentCanvasId) { | ||
| clearInterval(checkInterval); | ||
| resolve(currentCanvasId); | ||
| } | ||
| }, 100); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Polling mechanism lacks timeout, risking infinite loop.
The setInterval polling waits for currentCanvasId to be set but has no timeout or maximum attempts. If canvas creation fails silently or setCurrentCanvasId is never called, this interval runs indefinitely, causing a memory leak and potentially blocking subsequent uploads.
🛠️ Proposed fix with timeout
if (canvasCreationInProgress.current) {
return new Promise<string>((resolve, reject) => {
+ const maxWaitTime = 30000; // 30 seconds
+ const startTime = Date.now();
const checkInterval = setInterval(() => {
if (currentCanvasId) {
clearInterval(checkInterval);
resolve(currentCanvasId);
+ } else if (Date.now() - startTime > maxWaitTime) {
+ clearInterval(checkInterval);
+ reject(new Error('Canvas creation timed out'));
}
}, 100);
});
}🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/pure-copilot/index.tsx` around
lines 100 - 110, The polling in onCanvasRequired (which checks
canvasCreationInProgress.current and spins a setInterval waiting for
currentCanvasId) can run forever; change it to enforce a timeout/maximum
attempts by wrapping the interval in a cancellable promise that also starts a
setTimeout (or counts attempts) and on timeout clears the interval and rejects
(or resolves with a safe fallback) so the promise always settles; ensure you
clearInterval and clear the timeout when currentCanvasId becomes available, and
surface the error or fallback to the caller instead of leaving the interval
running.
Comment on lines
+26
to
+345
| export const useFileUpload = ({ | ||
| canvasId, | ||
| maxFileCount = MAX_FILE_COUNT, | ||
| maxFileSize = MAX_FILE_SIZE, | ||
| onCanvasRequired, | ||
| }: UseFileUploadOptions) => { | ||
| const { t } = useTranslation(); | ||
| const [contextItems, setContextItems] = useState<IContextItem[]>([]); | ||
| const pendingFilesRef = useRef<Map<string, PendingFileData>>(new Map()); | ||
| const fileCountRef = useRef(0); | ||
| const { refetch: refetchFiles } = useFetchDriveFiles(); | ||
|
|
||
| const { uploads, startUpload, updateProgress, setUploadSuccess, setUploadError, removeUpload } = | ||
| useImageUploadStore(); | ||
|
|
||
| const relevantUploads = useMemo(() => { | ||
| const uploadIds = new Set(contextItems.map((item) => item.metadata?.uploadId).filter(Boolean)); | ||
| return uploads.filter((u) => uploadIds.has(u.id)); | ||
| }, [uploads, contextItems]); | ||
|
|
||
| const fileCount = useMemo( | ||
| () => contextItems.filter((item) => item.type === 'file').length, | ||
| [contextItems], | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| fileCountRef.current = fileCount; | ||
| }, [fileCount]); | ||
|
|
||
| const hasUploadingFiles = useMemo( | ||
| () => relevantUploads.some((u) => u.status === 'uploading'), | ||
| [relevantUploads], | ||
| ); | ||
|
|
||
| const completedFileItems = useMemo( | ||
| () => | ||
| contextItems.filter((item) => { | ||
| if (item.type !== 'file') return false; | ||
| if (item.entityId.startsWith('pending_')) return false; | ||
| if (item.metadata?.errorType) return false; | ||
| return true; | ||
| }), | ||
| [contextItems], | ||
| ); | ||
|
|
||
| const handleFileUpload = useCallback( | ||
| async (file: File, existingUploadId?: string, existingEntityId?: string) => { | ||
| if (!existingUploadId) { | ||
| if (fileCountRef.current >= maxFileCount) { | ||
| message.warning(t('copilot.fileLimit.reached')); | ||
| return; | ||
| } | ||
| fileCountRef.current += 1; | ||
| } | ||
|
|
||
| if (file.size > maxFileSize) { | ||
| if (!existingUploadId) { | ||
| fileCountRef.current -= 1; | ||
| } | ||
| message.error(t('copilot.fileSizeLimit')); | ||
| return; | ||
| } | ||
|
|
||
| let currentCanvasId = canvasId; | ||
| if (!currentCanvasId && onCanvasRequired) { | ||
| try { | ||
| currentCanvasId = await onCanvasRequired(); | ||
| } catch (_error) { | ||
| if (!existingUploadId) { | ||
| fileCountRef.current -= 1; | ||
| } | ||
| message.error(t('copilot.canvasCreationFailed')); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (!currentCanvasId) { | ||
| if (!existingUploadId) { | ||
| fileCountRef.current -= 1; | ||
| } | ||
| message.error(t('copilot.canvasRequired')); | ||
| return; | ||
| } | ||
|
|
||
| const uploadId = existingUploadId || genUniqueId(); | ||
| const tempEntityId = existingEntityId || `pending_${uploadId}`; | ||
|
|
||
| startUpload([ | ||
| { | ||
| id: uploadId, | ||
| fileName: file.name, | ||
| progress: 0, | ||
| status: 'uploading', | ||
| }, | ||
| ]); | ||
|
|
||
| const isImageFile = file.type.startsWith('image/'); | ||
| let previewUrl = pendingFilesRef.current.get(uploadId)?.previewUrl; | ||
| if (!previewUrl && isImageFile) { | ||
| previewUrl = URL.createObjectURL(file); | ||
| } | ||
|
|
||
| pendingFilesRef.current.set(uploadId, { file, previewUrl }); | ||
|
|
||
| if (!existingEntityId) { | ||
| setContextItems((prev) => [ | ||
| ...prev, | ||
| { | ||
| type: 'file', | ||
| entityId: tempEntityId, | ||
| title: file.name, | ||
| metadata: { size: file.size, mimeType: file.type, uploadId, previewUrl }, | ||
| } as IContextItem, | ||
| ]); | ||
| } else { | ||
| setContextItems((prev) => | ||
| prev.map((item) => | ||
| item.entityId === existingEntityId | ||
| ? { | ||
| ...item, | ||
| metadata: { ...item.metadata, errorType: undefined }, | ||
| } | ||
| : item, | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| const uploadResult = await new Promise<{ | ||
| data?: { data?: { storageKey: string }; success: boolean }; | ||
| }>((resolve, reject) => { | ||
| const xhr = new XMLHttpRequest(); | ||
| const formData = new FormData(); | ||
| formData.append('file', file); | ||
| formData.append('entityId', currentCanvasId); | ||
| formData.append('entityType', 'canvas'); | ||
|
|
||
| xhr.upload.addEventListener('progress', (e) => { | ||
| if (e.lengthComputable) { | ||
| const percent = Math.round((e.loaded * 100) / e.total); | ||
| updateProgress(uploadId, Math.min(percent, 99)); | ||
| } | ||
| }); | ||
|
|
||
| xhr.onreadystatechange = () => { | ||
| if (xhr.readyState === 4) { | ||
| if (xhr.status >= 200 && xhr.status < 300) { | ||
| try { | ||
| const res = JSON.parse(xhr.responseText); | ||
| resolve({ data: res }); | ||
| } catch (e) { | ||
| reject(e); | ||
| } | ||
| } else { | ||
| reject(new Error(`Upload failed with status ${xhr.status}`)); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| xhr.onerror = () => reject(new Error('Network error during upload')); | ||
|
|
||
| xhr.open('POST', `${serverOrigin}/v1/misc/upload`); | ||
| xhr.withCredentials = !isDesktop(); | ||
| xhr.send(formData); | ||
| }); | ||
|
|
||
| updateProgress(uploadId, 100); | ||
|
|
||
| const { data, success } = uploadResult?.data ?? {}; | ||
|
|
||
| if (success && data) { | ||
| setContextItems((prev) => | ||
| prev.map((item) => | ||
| item.entityId === tempEntityId | ||
| ? { | ||
| ...item, | ||
| metadata: { ...item.metadata, storageKey: data.storageKey }, | ||
| } | ||
| : item, | ||
| ), | ||
| ); | ||
|
|
||
| try { | ||
| const { data: createResult } = await getClient().batchCreateDriveFiles({ | ||
| body: { | ||
| canvasId: currentCanvasId, | ||
| files: [ | ||
| { | ||
| name: file.name, | ||
| canvasId: currentCanvasId, | ||
| storageKey: data.storageKey, | ||
| type: file.type || 'application/octet-stream', | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| if (createResult?.success && createResult.data?.[0]) { | ||
| const driveFile = createResult.data[0]; | ||
| setUploadSuccess(uploadId); | ||
|
|
||
| setContextItems((prev) => | ||
| prev.map((item) => | ||
| item.entityId === tempEntityId | ||
| ? { | ||
| ...item, | ||
| entityId: driveFile.fileId, | ||
| title: driveFile.name, | ||
| metadata: { ...item.metadata, uploadId, errorType: undefined }, | ||
| } | ||
| : item, | ||
| ), | ||
| ); | ||
|
|
||
| pendingFilesRef.current.delete(uploadId); | ||
| await refetchFiles(); | ||
| } else { | ||
| throw new Error('addToFile'); | ||
| } | ||
| } catch { | ||
| setUploadError(uploadId, t('copilot.addToFileFailed')); | ||
| setContextItems((prev) => | ||
| prev.map((item) => | ||
| item.entityId === tempEntityId | ||
| ? { | ||
| ...item, | ||
| metadata: { ...item.metadata, errorType: 'addToFile' }, | ||
| } | ||
| : item, | ||
| ), | ||
| ); | ||
| } | ||
| } else { | ||
| throw new Error('upload'); | ||
| } | ||
| } catch { | ||
| setUploadError(uploadId, t('copilot.uploadFailed')); | ||
| setContextItems((prev) => | ||
| prev.map((item) => | ||
| item.entityId === tempEntityId | ||
| ? { | ||
| ...item, | ||
| metadata: { ...item.metadata, errorType: 'upload' }, | ||
| } | ||
| : item, | ||
| ), | ||
| ); | ||
| } | ||
| }, | ||
| [ | ||
| canvasId, | ||
| maxFileCount, | ||
| maxFileSize, | ||
| onCanvasRequired, | ||
| t, | ||
| startUpload, | ||
| updateProgress, | ||
| setUploadSuccess, | ||
| setUploadError, | ||
| refetchFiles, | ||
| ], | ||
| ); | ||
|
|
||
| const handleRetryFile = useCallback( | ||
| (entityId: string) => { | ||
| const item = contextItems.find((i) => i.entityId === entityId); | ||
| if (!item?.metadata?.uploadId) return; | ||
|
|
||
| const uploadId = item.metadata.uploadId; | ||
| const pendingData = pendingFilesRef.current.get(uploadId); | ||
|
|
||
| if (pendingData?.file) { | ||
| handleFileUpload(pendingData.file, uploadId, entityId); | ||
| } | ||
| }, | ||
| [contextItems, handleFileUpload], | ||
| ); | ||
|
|
||
| const handleRemoveFile = useCallback( | ||
| (entityId: string) => { | ||
| const item = contextItems.find((i) => i.entityId === entityId); | ||
| if (item?.metadata?.uploadId) { | ||
| removeUpload(item.metadata.uploadId); | ||
| const pendingData = pendingFilesRef.current.get(item.metadata.uploadId); | ||
| if (pendingData?.previewUrl) { | ||
| URL.revokeObjectURL(pendingData.previewUrl); | ||
| } | ||
| pendingFilesRef.current.delete(item.metadata.uploadId); | ||
| } | ||
| if (item?.metadata?.previewUrl) { | ||
| URL.revokeObjectURL(item.metadata.previewUrl); | ||
| } | ||
| setContextItems((prev) => prev.filter((i) => i.entityId !== entityId)); | ||
| }, | ||
| [contextItems, removeUpload], | ||
| ); | ||
|
|
||
| const clearFiles = useCallback(() => { | ||
| for (const item of contextItems) { | ||
| if (item.metadata?.previewUrl) { | ||
| URL.revokeObjectURL(item.metadata.previewUrl); | ||
| } | ||
| if (item.metadata?.uploadId) { | ||
| pendingFilesRef.current.delete(item.metadata.uploadId); | ||
| } | ||
| } | ||
| setContextItems([]); | ||
| }, [contextItems]); | ||
|
|
||
| return { | ||
| contextItems, | ||
| fileCount, | ||
| hasUploadingFiles, | ||
| completedFileItems, | ||
| relevantUploads, | ||
| handleFileUpload, | ||
| handleRetryFile, | ||
| handleRemoveFile, | ||
| clearFiles, | ||
| }; |
There was a problem hiding this comment.
Consider adding cleanup on unmount to prevent memory leaks.
The hook creates object URLs via URL.createObjectURL and tracks pending uploads, but there's no cleanup effect when the component unmounts. If the component unmounts during an active upload, object URLs and pending references won't be cleaned up.
🛠️ Proposed fix
+ // Cleanup on unmount
+ useEffect(() => {
+ return () => {
+ for (const [, data] of pendingFilesRef.current) {
+ if (data.previewUrl) {
+ URL.revokeObjectURL(data.previewUrl);
+ }
+ }
+ pendingFilesRef.current.clear();
+ };
+ }, []);
+
const relevantUploads = useMemo(() => {Add this cleanup effect after line 36 (after useFetchDriveFiles call).
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/hooks/use-file-upload.ts` around lines 26 -
345, Add a useEffect cleanup that runs on unmount to revoke any created object
URLs and clear pending upload state: iterate pendingFilesRef.current to revoke
each previewUrl and delete entries, revoke any previewUrl on contextItems, call
removeUpload for any remaining uploadIds, and finally clear contextItems (reuse
clearFiles if appropriate); place this effect alongside the hook setup
(referencing pendingFilesRef, contextItems, removeUpload, and clearFiles) so
active/uploads-orphaned previews are cleaned when the component unmounts.
- Added "pino-pretty" to the ignoreDependencies in knip.json for improved linting. - Refactored ToolService tests to include additional dependencies such as EncryptionService, McpServerService, ComposioService, CreditService, ToolFactory, ToolInventoryService, and ToolWrapperFactoryService, enhancing test coverage and maintainability. - Ensured consistent use of mocking for new dependencies to isolate ToolService behavior effectively.
…eCopilot components - Implemented drag-and-drop state management in PureCopilot, aligning with Copilot's functionality. - Added event handlers for drag enter, leave, over, and drop to manage file uploads effectively. - Improved user experience by displaying a dropzone overlay during drag operations, indicating upload limits and status. - Ensured consistent styling with Tailwind CSS across the updated components. - Utilized optional chaining and nullish coalescing for safer data handling in drag-and-drop logic.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/ai-workspace-common/src/components/pure-copilot/index.tsx`:
- Around line 440-456: The dropzone "success" green hex is using `#0E9F77`; update
the conditional styles and Attachment color to the palette's `#12B76A`: change the
border color string 'border-[`#0E9F77`]' to 'border-[`#12B76A`]', update the
semi-transparent background rgba('rgba(14,159,119,0.25)') to the matching rgba
for `#12B76A` (e.g. 'rgba(18,183,106,0.25)'), and change the Attachment component
color prop from '#0E9F77' to '#12B76A' wherever the isUploadDisabled ? ... : ...
branch is used (references: isDragging, isUploadDisabled, Attachment).
- Around line 76-80: The effect that syncs currentCanvasId with
pureCopilotCanvas only sets the ID when pureCopilotCanvas is present, so when
pureCopilotCanvas becomes null the stale currentCanvasId remains; update the
useEffect that watches pureCopilotCanvas to also clear currentCanvasId by
calling setCurrentCanvasId(undefined or null) when pureCopilotCanvas is falsy,
ensuring the component (and any upload logic) no longer targets the old canvas;
locate the effect using the useEffect hook and the pureCopilotCanvas and
setCurrentCanvasId identifiers to implement this change.
- Around line 146-257: Add JSDoc blocks above each handler (handleSendMessage,
handleFileInputChange, handleDragEnter, handleDragLeave, handleDragOver,
handleDrop) describing the purpose, parameters and return values; for event
handlers include the event parameter type (e.g.,
React.ChangeEvent<HTMLInputElement>, React.DragEvent) with `@param` tags, mark
async functions with `@async` if appropriate and document side effects (e.g.,
navigation, file uploads, state updates) and return type (usually void or
Promise<void>) using `@returns`; ensure the JSDoc follows the project's style
(one-line summary + brief details) and place each block immediately above its
corresponding function declaration.
- Around line 416-427: The icon-only Attachment and Send Button components lack
accessible names; add accessibility labels by setting a title prop and matching
aria-label on each Button: for the attachment Button (the one using
fileInputRef, fileCount) add title="Attach files" and aria-label="Attach files";
for the send Button (the one referencing query, completedFileItems, isCreating)
add title="Send message" and aria-label="Send message". Use the same visible
strings or i18n keys if you add them to translations (e.g.,
t('copilot.attachments')/t('copilot.sendMessage')) to keep consistency.
🧹 Nitpick comments (2)
packages/ai-workspace-common/src/components/pure-copilot/index.tsx (2)
100-103: Extract upload limits to named constants.
10and50 * 1024 * 1024are duplicated across the flow, making future changes error‑prone.♻️ Proposed refactor
const ACCEPT_FILE_EXTENSIONS = '.jpg,.jpeg,.png,.gif,.bmp,.webp,.pdf,.doc,.docx,.xls,.xlsx,.ppt,.pptx,.txt,.md,.csv'; +const MAX_FILE_COUNT = 10; +const MAX_FILE_SIZE_BYTES = 50 * 1024 * 1024; ... - } = useFileUpload({ + } = useFileUpload({ canvasId: currentCanvasId, - maxFileCount: 10, - maxFileSize: 50 * 1024 * 1024, + maxFileCount: MAX_FILE_COUNT, + maxFileSize: MAX_FILE_SIZE_BYTES, onCanvasRequired: async () => { ... -const isUploadDisabled = fileCount >= 10; +const isUploadDisabled = fileCount >= MAX_FILE_COUNT; ... -const remainingSlots = 10 - fileCount; +const remainingSlots = MAX_FILE_COUNT - fileCount; ... - disabled={fileCount >= 10} + disabled={fileCount >= MAX_FILE_COUNT}Also applies to: 210-253, 420-420
401-405: Avoid inline function props; memoize withuseCallback.Inline handlers in JSX recreate functions every render and violate the “useCallback for function props” rule.
✅ Suggested change
+const handleUploadMultipleImages = useCallback( + async (files: File[]) => { + const safeFiles = files ?? []; + if (safeFiles.length === 0) return; + await Promise.all(safeFiles.map((file) => handleFileUpload(file))); + }, + [handleFileUpload], +); + +const handleOpenFilePicker = useCallback(() => { + fileInputRef.current?.click(); +}, []); ... - onUploadMultipleImages={async (files) => { - await Promise.all(files.map((file) => handleFileUpload(file))); - }} + onUploadMultipleImages={handleUploadMultipleImages} ... - onClick={() => fileInputRef.current?.click()} + onClick={handleOpenFilePicker}Also applies to: 419-419
Comment on lines
+76
to
+80
| useEffect(() => { | ||
| if (pureCopilotCanvas?.canvasId) { | ||
| setCurrentCanvasId(pureCopilotCanvas.canvasId); | ||
| } | ||
| }, [pureCopilotCanvas]); |
There was a problem hiding this comment.
Reset currentCanvasId when the store canvas is cleared.
If pureCopilotCanvas is set to null later, currentCanvasId stays stale and future uploads can target the old canvas. You should clear it when the store value is falsy.
🛠️ Proposed fix
useEffect(() => {
- if (pureCopilotCanvas?.canvasId) {
- setCurrentCanvasId(pureCopilotCanvas.canvasId);
- }
+ setCurrentCanvasId(pureCopilotCanvas?.canvasId ?? null);
}, [pureCopilotCanvas]);📝 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.
Suggested change
| useEffect(() => { | |
| if (pureCopilotCanvas?.canvasId) { | |
| setCurrentCanvasId(pureCopilotCanvas.canvasId); | |
| } | |
| }, [pureCopilotCanvas]); | |
| useEffect(() => { | |
| setCurrentCanvasId(pureCopilotCanvas?.canvasId ?? null); | |
| }, [pureCopilotCanvas]); |
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/pure-copilot/index.tsx` around
lines 76 - 80, The effect that syncs currentCanvasId with pureCopilotCanvas only
sets the ID when pureCopilotCanvas is present, so when pureCopilotCanvas becomes
null the stale currentCanvasId remains; update the useEffect that watches
pureCopilotCanvas to also clear currentCanvasId by calling
setCurrentCanvasId(undefined or null) when pureCopilotCanvas is falsy, ensuring
the component (and any upload logic) no longer targets the old canvas; locate
the effect using the useEffect hook and the pureCopilotCanvas and
setCurrentCanvasId identifiers to implement this change.
Comment on lines
146
to
+257
| const handleSendMessage = useCallback(() => { | ||
| if (!query.trim() || isCreating) return; | ||
| debouncedCreateCanvas(source, { | ||
| initialPrompt: query, | ||
| }); | ||
| if (hasUploadingFiles) { | ||
| message.info(t('copilot.uploadInProgress')); | ||
| return; | ||
| } | ||
|
|
||
| setTimeout(() => { | ||
| if (!query.trim() && completedFileItems.length === 0) return; | ||
|
|
||
| if (currentCanvasId) { | ||
| setPendingPrompt(currentCanvasId, query); | ||
| setPendingFiles(currentCanvasId, completedFileItems); | ||
|
|
||
| const queryParams = new URLSearchParams(); | ||
| if (source) { | ||
| queryParams.append('source', source); | ||
| } | ||
| navigate(`/workflow/${currentCanvasId}?${queryParams.toString()}`); | ||
|
|
||
| if (source === 'onboarding') { | ||
| setTimeout(() => { | ||
| setHidePureCopilotModal(true); | ||
| }, 1000); | ||
| } | ||
|
|
||
| clearFiles(); | ||
| setQuery(''); | ||
| }, 1000); | ||
| }, [query, debouncedCreateCanvas, isCreating, source]); | ||
| setPureCopilotCanvas(source || 'default', null); | ||
| } else { | ||
| debouncedCreateCanvas(source, { | ||
| initialPrompt: query, | ||
| }); | ||
|
|
||
| setTimeout(() => { | ||
| setQuery(''); | ||
| }, 1000); | ||
| } | ||
| }, [ | ||
| query, | ||
| currentCanvasId, | ||
| hasUploadingFiles, | ||
| completedFileItems, | ||
| clearFiles, | ||
| source, | ||
| setPendingPrompt, | ||
| navigate, | ||
| setHidePureCopilotModal, | ||
| setPureCopilotCanvas, | ||
| debouncedCreateCanvas, | ||
| t, | ||
| ]); | ||
|
|
||
| const handleFileInputChange = useCallback( | ||
| async (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const files = Array.from(e.target.files || []); | ||
| if (files.length > 0) { | ||
| await Promise.all(files.map((file) => handleFileUpload(file))); | ||
| } | ||
| if (fileInputRef.current) { | ||
| fileInputRef.current.value = ''; | ||
| } | ||
| }, | ||
| [handleFileUpload], | ||
| ); | ||
|
|
||
| const isUploadDisabled = fileCount >= 10; | ||
|
|
||
| const handleDragEnter = useCallback((e: React.DragEvent) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| dragCounterRef.current += 1; | ||
| if (e.dataTransfer?.types?.includes('Files')) { | ||
| setIsDragging(true); | ||
| } | ||
| }, []); | ||
|
|
||
| const handleDragLeave = useCallback((e: React.DragEvent) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| dragCounterRef.current -= 1; | ||
| if (dragCounterRef.current <= 0) { | ||
| dragCounterRef.current = 0; | ||
| setIsDragging(false); | ||
| } | ||
| }, []); | ||
|
|
||
| const handleDragOver = useCallback((e: React.DragEvent) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| // Ensure the copy cursor is shown | ||
| if (e.dataTransfer) { | ||
| e.dataTransfer.dropEffect = 'copy'; | ||
| } | ||
| }, []); | ||
|
|
||
| const handleDrop = useCallback( | ||
| async (e: React.DragEvent) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| dragCounterRef.current = 0; | ||
| setIsDragging(false); | ||
|
|
||
| if (isUploadDisabled) return; | ||
|
|
||
| const files = Array.from(e.dataTransfer?.files ?? []); | ||
| if (files.length === 0) return; | ||
|
|
||
| const remainingSlots = 10 - fileCount; | ||
| const filesToUpload = files.slice(0, remainingSlots); | ||
| await Promise.all(filesToUpload.map((file) => handleFileUpload(file))); | ||
| }, | ||
| [handleFileUpload, fileCount, isUploadDisabled], | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/pure-copilot/index.tsx | head -300Repository: refly-ai/refly
Length of output: 11410
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/pure-copilot/index.tsx | tail -100Repository: refly-ai/refly
Length of output: 4457
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/pure-copilot/index.tsx | sed -n '259,290p'Repository: refly-ai/refly
Length of output: 1180
Add JSDoc blocks for the handler functions.
Per coding guidelines, all functions in JavaScript/TypeScript files require JSDoc comments. The handlers handleSendMessage, handleFileInputChange, handleDragEnter, handleDragLeave, handleDragOver, and handleDrop lack documentation.
🧩 Example (apply similarly to other handlers)
+/**
+ * Sends the current prompt and/or uploaded files to the active canvas.
+ */
const handleSendMessage = useCallback(() => {
...
}, [🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/pure-copilot/index.tsx` around
lines 146 - 257, Add JSDoc blocks above each handler (handleSendMessage,
handleFileInputChange, handleDragEnter, handleDragLeave, handleDragOver,
handleDrop) describing the purpose, parameters and return values; for event
handlers include the event parameter type (e.g.,
React.ChangeEvent<HTMLInputElement>, React.DragEvent) with `@param` tags, mark
async functions with `@async` if appropriate and document side effects (e.g.,
navigation, file uploads, state updates) and return type (usually void or
Promise<void>) using `@returns`; ensure the JSDoc follows the project's style
(one-line summary + brief details) and place each block immediately above its
corresponding function declaration.
Comment on lines
+416
to
427
| <Button | ||
| type="text" | ||
| icon={<Attachment size={20} />} | ||
| onClick={() => fileInputRef.current?.click()} | ||
| disabled={fileCount >= 10} | ||
| className="!text-refly-text-2 hover:!text-refly-text-0" | ||
| /> | ||
| <Button | ||
| type="primary" | ||
| shape="circle" | ||
| disabled={!query.trim() || isCreating} | ||
| disabled={(!query.trim() && completedFileItems.length === 0) || isCreating} | ||
| icon={<Send size={20} color="var(--refly-bg-canvas)" />} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the file around lines 416-427 to see the full context
wc -l packages/ai-workspace-common/src/components/pure-copilot/index.tsxRepository: refly-ai/refly
Length of output: 128
🏁 Script executed:
# Check the actual buttons at the specified lines with context
sed -n '400,440p' packages/ai-workspace-common/src/components/pure-copilot/index.tsxRepository: refly-ai/refly
Length of output: 1622
🏁 Script executed:
# Search for translation keys related to copilot attachments and send message
rg "copilot\.(attachments|sendMessage)" --type ts --type tsxRepository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Also search for any existing aria-label usage in this file
rg "aria-label" packages/ai-workspace-common/src/components/pure-copilot/index.tsxRepository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for translation keys without file type restriction
rg "copilot\.(attachments|sendMessage)" --type-add "json:*.json" -t json -t ts -t tsxRepository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Search for any copilot translation keys
rg "'copilot\." --type-add "json:*.json" -t json | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Check if there's a translation file for copilot
fd "copilot" --type f -e json -e ts -e tsx | grep -i "i18n\|locale\|translation" | head -10Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Look for how Button component handles aria-label
rg "aria-label|title=" packages/ai-workspace-common/src/components/pure-copilot/index.tsx -A 2 -B 2Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for translation usage pattern in the file
rg "useTranslation|t\(" packages/ai-workspace-common/src/components/pure-copilot/index.tsx | head -5Repository: refly-ai/refly
Length of output: 267
🏁 Script executed:
# Find i18n or translation directories and files
fd -e json -e yml -e yaml | grep -E "i18n|locale|translation|messages|lang" | head -20Repository: refly-ai/refly
Length of output: 154
🏁 Script executed:
# Look for locales directory structure
find . -type d -name "locales" -o -name "i18n" -o -name "translations" 2>/dev/nullRepository: refly-ai/refly
Length of output: 73
🏁 Script executed:
# Search in common locations for translation files
ls -la packages/ai-workspace-common/src/ | head -20Repository: refly-ai/refly
Length of output: 970
🏁 Script executed:
# Check if there's a locales folder at root or in packages
find . -maxdepth 3 -type d -name "locales" 2>/dev/nullRepository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for JSON files that might contain copilot translations
find . -type f -name "*.json" | xargs grep -l "copilot" 2>/dev/null | head -10Repository: refly-ai/refly
Length of output: 83
🏁 Script executed:
# Explore the i18n package structure
ls -la packages/i18n/Repository: refly-ai/refly
Length of output: 346
🏁 Script executed:
# Look for locales or translation files
find packages/i18n -type f \( -name "*.json" -o -name "*.ts" \) | head -20Repository: refly-ai/refly
Length of output: 386
🏁 Script executed:
# Search for copilot in i18n package
rg "copilot" packages/i18n/ | head -20Repository: refly-ai/refly
Length of output: 339
🏁 Script executed:
# Check what's in the i18n package
cd packages/i18n && find . -type f -name "*.json" | head -20Repository: refly-ai/refly
Length of output: 88
🏁 Script executed:
# Read the copilot section from en-US ui.ts
rg -A 50 "copilot: \{" packages/i18n/src/en-US/ui.ts | head -80Repository: refly-ai/refly
Length of output: 3642
🏁 Script executed:
# Read the copilot section from zh-Hans ui.ts
rg -A 50 "copilot: \{" packages/i18n/src/zh-Hans/ui.ts | head -80Repository: refly-ai/refly
Length of output: 1351
🏁 Script executed:
# Check for attachments or sendMessage keys specifically
rg "attachments|sendMessage" packages/i18n/src/Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for Button component definition and check if it supports aria-label
fd -e tsx -e ts Button | grep -E "components|button" | head -10Repository: refly-ai/refly
Length of output: 139
🏁 Script executed:
# Look for Button component in common areas
find . -path "*/components/*Button*" -type f | head -5Repository: refly-ai/refly
Length of output: 141
🏁 Script executed:
# Search for other icon-only buttons with aria-label or title in the codebase
rg "icon=\{" --type ts --type tsx -A 5 | grep -E "aria-label|title=" | head -10Repository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Check if there's a title or aria-label convention in other buttons
rg "Attachment|Send" packages/ai-workspace-common/src/components -B 3 -A 3 | head -40Repository: refly-ai/refly
Length of output: 4263
🏁 Script executed:
# Look for how other buttons in the project handle accessibility
rg "type=\"text\"" packages/ai-workspace-common/src/components -B 2 -A 5 | grep -E "aria|title" | head -10Repository: refly-ai/refly
Length of output: 1452
🏁 Script executed:
# Check Ant Design Button documentation or component usage pattern
rg "Button.*title=" packages/ai-workspace-common/src/components -B 1 -A 1 | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for icon buttons with title or aria-label patterns
rg "icon=\{" packages/ai-workspace-common/src/components -A 3 | grep -E "title|aria" | head -10Repository: refly-ai/refly
Length of output: 1376
🏁 Script executed:
# Look at the exact implementation around the problematic buttons
sed -n '410,435p' packages/ai-workspace-common/src/components/pure-copilot/index.tsxRepository: refly-ai/refly
Length of output: 1134
Icon-only buttons need accessible labels.
Both buttons lack accessible names for screen readers. The suggested translation keys (copilot.attachments and copilot.sendMessage) do not exist in the i18n files.
Add a title prop to both buttons (following the codebase pattern) or create the missing translation keys first, then add aria-label={t('copilot.attachments')} and aria-label={t('copilot.sendMessage')}:
Suggested approaches
Option 1: Using title prop (simpler, follows codebase pattern):
<Button
type="text"
icon={<Attachment size={20} />}
+ title={t('common.attachFile')}
onClick={() => fileInputRef.current?.click()}
disabled={fileCount >= 10}
className="!text-refly-text-2 hover:!text-refly-text-0"
/>Option 2: Create translation keys and use aria-label:
- Add
attachments: 'Attach files'andsendMessage: 'Send message'to the copilot section in i18n - Then add
aria-label={t('copilot.attachments')}to the first button andaria-label={t('copilot.sendMessage')}to the second
📝 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.
Suggested change
| <Button | |
| type="text" | |
| icon={<Attachment size={20} />} | |
| onClick={() => fileInputRef.current?.click()} | |
| disabled={fileCount >= 10} | |
| className="!text-refly-text-2 hover:!text-refly-text-0" | |
| /> | |
| <Button | |
| type="primary" | |
| shape="circle" | |
| disabled={!query.trim() || isCreating} | |
| disabled={(!query.trim() && completedFileItems.length === 0) || isCreating} | |
| icon={<Send size={20} color="var(--refly-bg-canvas)" />} | |
| <Button | |
| type="text" | |
| icon={<Attachment size={20} />} | |
| title={t('common.attachFile')} | |
| onClick={() => fileInputRef.current?.click()} | |
| disabled={fileCount >= 10} | |
| className="!text-refly-text-2 hover:!text-refly-text-0" | |
| /> | |
| <Button | |
| type="primary" | |
| shape="circle" | |
| title={t('copilot.sendMessage')} | |
| disabled={(!query.trim() && completedFileItems.length === 0) || isCreating} | |
| icon={<Send size={20} color="var(--refly-bg-canvas)" />} | |
| /> |
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/pure-copilot/index.tsx` around
lines 416 - 427, The icon-only Attachment and Send Button components lack
accessible names; add accessibility labels by setting a title prop and matching
aria-label on each Button: for the attachment Button (the one using
fileInputRef, fileCount) add title="Attach files" and aria-label="Attach files";
for the send Button (the one referencing query, completedFileItems, isCreating)
add title="Send message" and aria-label="Send message". Use the same visible
strings or i18n keys if you add them to translations (e.g.,
t('copilot.attachments')/t('copilot.sendMessage')) to keep consistency.
Comment on lines
+440
to
+456
| {/* Dropzone overlay: Figma 20776-18504 (active) / 20776-18912 (limit) */} | ||
| {isDragging && ( | ||
| <div | ||
| className={cn( | ||
| 'absolute inset-0 z-50 rounded-[12px] border-2 border-dashed', | ||
| 'flex flex-col items-center justify-center gap-2', | ||
| isUploadDisabled | ||
| ? 'border-[#C7CACD] bg-[rgba(28,31,35,0.1)] backdrop-blur-[25px]' | ||
| : 'border-[#0E9F77] bg-[rgba(14,159,119,0.25)] backdrop-blur-[25px]', | ||
| )} | ||
| > | ||
| <Attachment size={32} color={isUploadDisabled ? '#1C1F23' : '#0E9F77'} /> | ||
| <div | ||
| className={cn( | ||
| 'text-sm font-medium text-center', | ||
| isUploadDisabled ? 'text-[#1C1F23] opacity-50' : 'text-[#1C1F23]', | ||
| )} |
There was a problem hiding this comment.
Align dropzone “success” green with the palette.
The active state uses #0E9F77, but the guideline specifies #12B76A for success.
🎨 Suggested tweak
- : 'border-[`#0E9F77`] bg-[rgba(14,159,119,0.25)] backdrop-blur-[25px]',
+ : 'border-[`#12B76A`] bg-[rgba(18,183,106,0.25)] backdrop-blur-[25px]',
...
-<Attachment size={32} color={isUploadDisabled ? '#1C1F23' : '#0E9F77'} />
+<Attachment size={32} color={isUploadDisabled ? '#1C1F23' : '#12B76A'} />Also applies to: 451-451
🤖 Prompt for AI Agents
In `@packages/ai-workspace-common/src/components/pure-copilot/index.tsx` around
lines 440 - 456, The dropzone "success" green hex is using `#0E9F77`; update the
conditional styles and Attachment color to the palette's `#12B76A`: change the
border color string 'border-[`#0E9F77`]' to 'border-[`#12B76A`]', update the
semi-transparent background rgba('rgba(14,159,119,0.25)') to the matching rgba
for `#12B76A` (e.g. 'rgba(18,183,106,0.25)'), and change the Attachment component
color prop from '#0E9F77' to '#12B76A' wherever the isUploadDisabled ? ... : ...
branch is used (references: isDragging, isUploadDisabled, Attachment).
anthhub
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/skill-template/src/prompts/templates/copilot-agent-system.md`:
- Around line 51-105: Replace the bolded step titles (e.g. "**Step 1: Identify
Image Type**", "**Step 2: Extract Key Information Based on Type**", "**Step 3:
Design Workflow with Specific Details**", "**Step 4: Quality Check**") with
proper Markdown headings (e.g. "## Step 1: Identify Image Type") across the
template so the step lines become real headings; apply the same heading level
and style consistently for all "Step X" lines and update any related checklist
or references if needed so Markdownlint no longer flags heading misuse.
- Around line 112-270: The markdown fences in the prompt section are missing
language identifiers which triggers markdownlint; update every fenced code block
that contains plain text (the UI/UX spec starting with "Image Type: UI/UX Design
- Login page mockup" and the example prose blocks) to use ```text and update the
JSON workflow block (the object with "title": "登录页面组件生成") to use ```json so all
fenced blocks include the appropriate language tag; ensure you modify every
occurrence of triple-backtick fences in this file (including the example
scenarios at the bottom) so linting is satisfied.
🧹 Nitpick comments (1)
packages/canvas-common/src/context.ts (1)
1-8: Consider usingimport typeforDriveFile.
DriveFileappears to be used only as a type assertion (line 224). Per guidelines, types should be imported explicitly usingimport typesyntax to enable better tree-shaking and clarify intent.♻️ Suggested change
import { ActionResult, CanvasNodeType, - DriveFile, SkillContext, SkillContextFileItem, WorkflowVariable, } from '@refly/openapi-schema'; +import type { DriveFile } from '@refly/openapi-schema';
Comment on lines
+51
to
+105
| **Step 1: Identify Image Type** | ||
| - UI/UX Design (mockups, wireframes, screenshots) | ||
| - Flowchart/Diagram (process flows, architecture diagrams) | ||
| - Data Visualization (charts, graphs, dashboards) | ||
| - Code Screenshot (code snippets, terminal output) | ||
| - Document/Form (scanned documents, forms, tables) | ||
| - Other (photos, illustrations, etc.) | ||
|
|
||
| **Step 2: Extract Key Information Based on Type** | ||
|
|
||
| For **UI/UX Design**, extract: | ||
| - **Layout**: Overall structure (grid, flex, centered), main sections | ||
| - **Components**: All interactive elements (buttons, inputs, cards, navigation) | ||
| - **Colors**: Primary, secondary, accent colors (with hex codes if visible) | ||
| - **Typography**: Font sizes, weights, hierarchy | ||
| - **Spacing**: Padding, margins, gaps between elements | ||
| - **States**: Default, hover, active, disabled states if shown | ||
|
|
||
| For **Flowchart/Diagram**, extract: | ||
| - **Steps**: Start point, intermediate steps, end point | ||
| - **Decisions**: Conditions, branches, outcomes | ||
| - **Flow**: Sequential, parallel, or conditional paths | ||
| - **Actors**: Who or what is involved in each step | ||
| - **Data**: What data flows between steps | ||
|
|
||
| For **Data Visualization**, extract: | ||
| - **Chart Type**: Bar, line, pie, scatter, etc. | ||
| - **Axes**: X-axis (time, categories), Y-axis (values, metrics) | ||
| - **Trends**: Increasing, decreasing, stable, seasonal patterns | ||
| - **Values**: Specific data points, ranges, peaks | ||
| - **Insights**: Key findings, anomalies, comparisons | ||
|
|
||
| For **Code Screenshot**, extract: | ||
| - **Language**: Programming language used | ||
| - **Structure**: Functions, classes, imports, exports | ||
| - **Logic**: Key algorithms, API calls, database queries | ||
| - **Dependencies**: Libraries, frameworks, tools used | ||
|
|
||
| **Step 3: Design Workflow with Specific Details** | ||
|
|
||
| Translate extracted information into precise task prompts: | ||
| - Include exact specifications (colors, sizes, component names) | ||
| - Reference specific elements from the image | ||
| - Provide step-by-step instructions based on image content | ||
| - Create resource variables to reference the image for Node Agent execution | ||
|
|
||
| **Step 4: Quality Check** | ||
|
|
||
| Before calling generate_workflow, verify: | ||
| - [ ] Have I identified the image type correctly? | ||
| - [ ] Have I extracted all visible key information? | ||
| - [ ] Have I noted specific values (colors, sizes, text)? | ||
| - [ ] Will the workflow tasks leverage these image details? | ||
| - [ ] Are task prompts specific enough for Node Agent to execute? | ||
|
|
There was a problem hiding this comment.
Use headings instead of bold for step titles.
Markdownlint flags the bold “Step X” lines as heading misuse. Converting them to proper headings improves structure and avoids lint failures.
🔧 Suggested update (apply to all step titles)
-**Step 1: Identify Image Type**
+#### Step 1: Identify Image Type
...
-**Step 2: Extract Key Information Based on Type**
+#### Step 2: Extract Key Information Based on Type
...
-**Step 3: Design Workflow with Specific Details**
+#### Step 3: Design Workflow with Specific Details
...
-**Step 4: Quality Check**
+#### Step 4: Quality Check🧰 Tools
🪛 LanguageTool
[style] ~62-~62: ‘Overall structure’ might be wordy. Consider a shorter alternative.
Context: ...UI/UX Design*, extract: - Layout: Overall structure (grid, flex, centered), main sections -...
(EN_WORDINESS_PREMIUM_OVERALL_STRUCTURE)
[style] ~89-~89: This phrase is redundant. Consider writing “Details”.
Context: ...ls used Step 3: Design Workflow with Specific Details Translate extracted information into...
(SPECIFIC_DETAILS)
🪛 markdownlint-cli2 (0.20.0)
[warning] 51-51: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
[warning] 59-59: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
[warning] 89-89: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
[warning] 97-97: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In `@packages/skill-template/src/prompts/templates/copilot-agent-system.md` around
lines 51 - 105, Replace the bolded step titles (e.g. "**Step 1: Identify Image
Type**", "**Step 2: Extract Key Information Based on Type**", "**Step 3: Design
Workflow with Specific Details**", "**Step 4: Quality Check**") with proper
Markdown headings (e.g. "## Step 1: Identify Image Type") across the template so
the step lines become real headings; apply the same heading level and style
consistently for all "Step X" lines and update any related checklist or
references if needed so Markdownlint no longer flags heading misuse.
Comment on lines
+112
to
+270
| ``` | ||
| Image Type: UI/UX Design - Login page mockup | ||
|
|
||
| Extracted Information: | ||
|
|
||
| Layout: | ||
| - Centered card layout (max-width: 400px) | ||
| - Vertical stack with consistent spacing | ||
| - White background with subtle shadow | ||
|
|
||
| Components: | ||
| 1. Header: "欢迎登录" (text-2xl, font-bold, text-center) | ||
| 2. Username input: | ||
| - Placeholder: "请输入用户名" | ||
| - Left icon: user icon | ||
| - Border: 1px solid #E5E7EB | ||
| - Rounded: rounded-lg | ||
| 3. Password input: | ||
| - Placeholder: "请输入密码" | ||
| - Left icon: lock icon | ||
| - Right icon: eye icon (show/hide toggle) | ||
| - Border: 1px solid #E5E7EB | ||
| 4. Checkbox: "记住我" (text-sm, text-gray-600) | ||
| 5. Submit button: | ||
| - Text: "登录" | ||
| - Background: #155EEF | ||
| - Text: white | ||
| - Full width | ||
| - Rounded: rounded-lg | ||
| - Padding: py-3 | ||
| 6. Footer link: "忘记密码？" (text-sm, text-blue-600, text-center) | ||
|
|
||
| Colors: | ||
| - Primary: #155EEF | ||
| - Text: #1F2937 | ||
| - Secondary text: #6B7280 | ||
| - Border: #E5E7EB | ||
| - Background: #FFFFFF | ||
|
|
||
| Spacing: | ||
| - Card padding: p-8 (32px) | ||
| - Input spacing: space-y-4 (16px) | ||
| - Button margin-top: mt-6 (24px) | ||
| ``` | ||
|
|
||
| **Your Workflow Design**: | ||
|
|
||
| ```json | ||
| { | ||
| "title": "登录页面组件生成", | ||
| "tasks": [ | ||
| { | ||
| "id": "task-1", | ||
| "title": "生成 React 登录组件", | ||
| "prompt": "基于设计稿 @{type=var,id=var-1,name=design_image}，生成 React 登录表单组件。\n\n## 布局要求\n- 居中卡片布局：max-w-md mx-auto\n- 白色背景：bg-white\n- 阴影：shadow-lg\n- 圆角：rounded-xl\n- 内边距：p-8\n\n## 组件结构\n1. 标题：\n - 文本：\"欢迎登录\"\n - 样式：text-2xl font-bold text-gray-900 text-center mb-6\n\n2. 用户名输入框：\n - Placeholder: \"请输入用户名\"\n - 左侧图标：用户图标\n - 样式：border border-gray-300 rounded-lg px-4 py-3 w-full\n - 状态：focus:border-[#155EEF] focus:ring-2 focus:ring-[#155EEF]/20\n\n3. 密码输入框：\n - Placeholder: \"请输入密码\"\n - 左侧图标：锁图标\n - 右侧图标：眼睛图标（点击切换显示/隐藏）\n - 样式：同用户名输入框\n - 功能：useState 管理 showPassword 状态\n\n4. 记住我复选框：\n - 文本：\"记住我\"\n - 样式：text-sm text-gray-600\n - 位置：左对齐，mt-4\n\n5. 登录按钮：\n - 文本：\"登录\"\n - 样式：bg-[#155EEF] text-white w-full py-3 rounded-lg font-medium mt-6\n - 悬停：hover:bg-[#1349CC]\n - 点击：处理表单提交\n\n6. 忘记密码链接：\n - 文本：\"忘记密码？\"\n - 样式：text-sm text-[#155EEF] text-center mt-4 block\n - 悬停：hover:underline\n\n## 技术要求\n- 使用 React Hooks (useState)\n- 使用 Tailwind CSS\n- 表单验证：用户名和密码不能为空\n- 导出为默认组件\n\n## 文件名\nLoginForm.tsx", | ||
| "toolsets": ["generate_code_artifact"], | ||
| "dependentTasks": [] | ||
| } | ||
| ], | ||
| "variables": [ | ||
| { | ||
| "variableId": "var-1", | ||
| "variableType": "resource", | ||
| "name": "design_image", | ||
| "description": "登录页面设计稿（供 Node Agent 执行时参考）", | ||
| "required": true, | ||
| "resourceTypes": ["image"], | ||
| "value": [] | ||
| } | ||
| ] | ||
| } | ||
| ``` | ||
|
|
||
| #### Key Principles for Image-Based Workflow Design | ||
|
|
||
| 1. **Be Specific**: Extract and include exact values (colors, sizes, text) from the image | ||
| 2. **Be Structured**: Organize information hierarchically (layout → components → details) | ||
| 3. **Be Actionable**: Translate visual elements into concrete implementation steps | ||
| 4. **Reference Image**: Always create resource variables for images so Node Agent can access them | ||
| 5. **Verify Quality**: Use the quality checklist before generating workflow | ||
|
|
||
| #### Common Pitfalls to Avoid | ||
|
|
||
| ❌ **Vague Analysis**: | ||
| - "I see a login page" → Too generic | ||
| - "There are some buttons" → Not specific enough | ||
|
|
||
| ✅ **Detailed Analysis**: | ||
| - "I see a login page with centered card layout (max-w-md), containing username input, password input with show/hide toggle, remember me checkbox, and a primary button with #155EEF background" | ||
|
|
||
| ❌ **Generic Task Prompts**: | ||
| - "Generate a login component" → Node Agent will guess | ||
|
|
||
| ✅ **Specific Task Prompts**: | ||
| - "Generate React login component with: centered card (max-w-md), white bg, shadow-lg, username input with user icon, password input with lock icon and eye toggle, remember me checkbox, submit button (bg-[#155EEF], full width, py-3), forgot password link. Use Tailwind CSS." | ||
|
|
||
| #### Automatic Image Processing | ||
|
|
||
| **How It Works:** | ||
| - Images attached to user messages are automatically passed to you via vision capability | ||
| - You can directly observe and analyze image content - no tool call needed | ||
| - Images appear in your conversation context as visual content | ||
| - Use the analysis framework above to extract information systematically | ||
|
|
||
| **DO NOT use read_file for images:** | ||
| - ❌ `read_file` does NOT support images (will return error) | ||
| - ✅ Images are automatically available in your context via vision capability | ||
| - ✅ Just analyze what you see and design workflows with specific details | ||
|
|
||
| ### File Content Access for Workflow Design | ||
|
|
||
| You have access to `list_files` and `read_file` tools to help design better workflows based on actual file content: | ||
|
|
||
| #### When to use these tools | ||
|
|
||
| **Use `list_files` when:** | ||
| - User mentions "files" but doesn't specify which ones | ||
| - You need to see what files are available in the canvas | ||
| - User says "analyze all my files" or "process these files" | ||
|
|
||
| **Use `read_file` when:** | ||
| - You need to understand file structure to design appropriate tasks | ||
| - User uploads data files (CSV, JSON) and asks to "analyze" or "process" | ||
| - User uploads documents (PDF, Word, text files) and asks to "create workflow based on this spec" | ||
| - You need to see actual content to design accurate workflow steps | ||
|
|
||
| **DO NOT use `read_file` for:** | ||
| - ❌ Image files - images are automatically passed via vision capability (see "Image Understanding" section above) | ||
| - ❌ Audio/Video files - not supported by read_file | ||
| - ❌ Files when content is not needed for workflow design | ||
|
|
||
| **Examples:** | ||
|
|
||
| ✅ **Scenario 1: Data Analysis Workflow** | ||
| ``` | ||
| User: "Analyze this sales data and generate a report" | ||
| You: | ||
| 1. Use read_file to check CSV structure (columns, data types) | ||
| 2. Design workflow with appropriate analysis tasks based on actual columns | ||
| 3. Create variables for configurable parameters (date range, metrics) | ||
| ``` | ||
|
|
||
| ✅ **Scenario 2: API Testing Workflow** | ||
| ``` | ||
| User: "Create tests for these API endpoints" | ||
| You: | ||
| 1. Use read_file to read API documentation | ||
| 2. Identify endpoints, parameters, expected responses | ||
| 3. Design workflow with test tasks for each endpoint | ||
| ``` | ||
|
|
||
| ✅ **Scenario 3: Document Processing Workflow** | ||
| ``` | ||
| User: "Summarize this technical specification" | ||
| You: | ||
| 1. Use read_file to read the document content | ||
| 2. Understand document structure and key sections | ||
| 3. Design workflow with summarization tasks | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Markdownlint flags the fenced blocks without language tags. Add text/json (or appropriate) for all code fences in this section.
🔧 Suggested update (apply to all fenced blocks)
-```
+```text
Image Type: UI/UX Design - Login page mockup
...
-```
+```
-```json
+```json
{
"title": "登录页面组件生成",
...
}-```
+```text
User: "Analyze this sales data and generate a report"
...
-```
+```🧰 Tools
🪛 LanguageTool
[style] ~219-~219: This phrase is redundant. Consider writing “details”.
Context: ... what you see and design workflows with specific details ### File Content Access for Workflow D...
(SPECIFIC_DETAILS)
🪛 markdownlint-cli2 (0.20.0)
[warning] 112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 246-246: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 255-255: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 264-264: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@packages/skill-template/src/prompts/templates/copilot-agent-system.md` around
lines 112 - 270, The markdown fences in the prompt section are missing language
identifiers which triggers markdownlint; update every fenced code block that
contains plain text (the UI/UX spec starting with "Image Type: UI/UX Design -
Login page mockup" and the example prose blocks) to use ```text and update the
JSON workflow block (the object with "title": "登录页面组件生成") to use ```json so all
fenced blocks include the appropriate language tag; ensure you modify every
occurrence of triple-backtick fences in this file (including the example
scenarios at the bottom) so linting is satisfied.
…or workflow variables. Update related operations in the copilot API to include options and isSingle properties for variable creation and updates. Improve documentation for variable types and their usage.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/canvas-common/src/workflow-plan.ts`:
- Around line 75-80: The zod schema field isSingle is documented as "Defaults to
true" but currently only marked .optional(), so add an explicit default to the
schema (e.g., call .default(true) on the z.boolean() chain for isSingle in
workflow-plan.ts) so omitted values resolve to true; you can keep or remove
.optional() when adding .default(true) but ensure the final chain for isSingle
includes .default(true) and retains the describe text.
Comment on lines
+75
to
+80
| isSingle: z | ||
| .boolean() | ||
| .optional() | ||
| .describe( | ||
| 'Whether only single selection is allowed (only for option type variables). Defaults to true.', | ||
| ), |
There was a problem hiding this comment.
isSingle described as defaulting to true, but schema lacks .default(true).
The description states "Defaults to true" but the field is only marked .optional(). When isSingle is omitted, consumers will receive undefined, not true. This inconsistency may cause unexpected behavior in downstream code that relies on the documented default.
🔧 Proposed fix to add explicit default
isSingle: z
.boolean()
- .optional()
+ .default(true)
.describe(
'Whether only single selection is allowed (only for option type variables). Defaults to true.',
),📝 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.
Suggested change
| isSingle: z | |
| .boolean() | |
| .optional() | |
| .describe( | |
| 'Whether only single selection is allowed (only for option type variables). Defaults to true.', | |
| ), | |
| isSingle: z | |
| .boolean() | |
| .default(true) | |
| .describe( | |
| 'Whether only single selection is allowed (only for option type variables). Defaults to true.', | |
| ), |
🤖 Prompt for AI Agents
In `@packages/canvas-common/src/workflow-plan.ts` around lines 75 - 80, The zod
schema field isSingle is documented as "Defaults to true" but currently only
marked .optional(), so add an explicit default to the schema (e.g., call
.default(true) on the z.boolean() chain for isSingle in workflow-plan.ts) so
omitted values resolve to true; you can keep or remove .optional() when adding
.default(true) but ensure the final chain for isSingle includes .default(true)
and retains the describe text.
…roved visual consistency. Update Tailwind CSS classes accordingly.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx`:
- Around line 18-27: Add a JSDoc block above the exported component and give the
exported constant an explicit type: change the declaration of CopilotActions to
include the React memo return type, e.g. export const CopilotActions:
React.MemoExoticComponent<React.FC<CopilotActionsProps>> = memo((props:
CopilotActionsProps) => { ... }); and add a brief /** ... */ JSDoc describing
the component and its props (referencing CopilotActions and CopilotActionsProps)
to keep the public API documented and typed.
- Around line 101-115: Replace the hardcoded hex background colors for the
destructive Stop button and the primary Send button with the project's design
tokens: update the Stop button (the button rendering when onAbort is used and
contains the Stop icon) to use the error/destructive token (red, `#F04438`) and
update the Send button (the branch using canSend true) to use the primary CTA
token (blue, `#155EEF`); use the theme/token utility or CSS variables your
codebase provides (instead of literal hexes) so the classes for the Stop button
and the Send button reference the error and primary tokens respectively for both
normal and hover states.
- Around line 56-119: Add an "isUploading" local state and use it inside
handleFileChange and the attachmentButton to show a loading UI: in
handleFileChange (the function that awaits the async onUploadFile prop) set
isUploading = true before calling await onUploadFile(files) and ensure
isUploading = false in a finally block so it resets on success/error; update
attachmentButton to render a spinner or alternate visual when isUploading is
true and disable clicks (or prevent handleAttachmentClick from opening
uploadRef) while isUploading is true; keep existing uploadDisabled logic intact
so the button remains visually disabled when limits are reached and combine it
with isUploading to provide proper feedback.
- Around line 31-35: The canSend variable currently may be a string when
query?.trim() yields text; change its initialization to ensure a boolean by
nullish-coalescing the trimmed query to an empty string and then coercing to
boolean, e.g., replace the expression using query?.trim() with a boolean
coercion of (query?.trim() ?? '') combined with fileCount > 0; update the const
canSend declaration accordingly (refer to symbol canSend and the use of query
and fileCount).
- Around line 37-46: The handler handleFileChange currently uses Promise.all and
may skip resetting the input when an upload fails; change it to use
Promise.allSettled for parallel uploads and wrap the await in a try-finally so
e.target.value is always cleared in the finally block; also use optional
chaining when reading files (e.target.files?.length) to follow guidelines and
avoid runtime errors, keeping the call to onUploadFile for each file unchanged.
🧹 Nitpick comments (3)
packages/ai-workspace-common/src/components/canvas/common/toolset-icon/index.tsx (1)
87-93: Pre-existing: Potential null reference when accessingtoolset.type.This is not introduced by this PR, but worth noting: if
toolsetKeyis provided but not inbuiltinToolsetIconMap,toolsetis undefined, anddisableInventoryLookupis true, line 87 will throw a runtime error.Per coding guidelines, consider using optional chaining:
🛡️ Suggested defensive fix
- if (toolset.type === 'mcp') { + if (toolset?.type === 'mcp') {And add an early return or fallback for when
toolsetis undefined but inventory lookup is disabled:if (!toolset && toolsetKey && !disableInventoryLookup) { return <ToolsetIconWithInventory toolsetKey={toolsetKey} size={size} className={className} />; } + + // Fallback when toolset is missing and inventory lookup is disabled + if (!toolset) { + return null; + }packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx (2)
76-81: Avoid inline tooltip style objects; use Tailwind-friendly styling instead.
This prevents re-creating objects on each render and aligns with the Tailwind-only styling rule.Suggested diff
+ const tooltipOverlayClass = 'rounded-lg'; ... <Tooltip title={uploadDisabled ? t('copilot.maxFilesPerTask') : t('copilot.uploadFile')} placement="top" - overlayInnerStyle={{ borderRadius: '8px' }} + overlayClassName={tooltipOverlayClass} color="#000" >As per coding guidelines: Always use Tailwind CSS for styling components; Always avoid inline object/array creation in render to prevent unnecessary re-renders.
42-96: Update comments to explain “why,” not “what.”
These comments restate code behavior; either remove or reword to capture rationale.As per coding guidelines: Comments should be concise and explain 'why' rather than 'what'.
Comment on lines
+18
to
+27
| export const CopilotActions = memo( | ||
| ({ | ||
| query, | ||
| fileCount, | ||
| maxFileCount, | ||
| isExecuting, | ||
| onUploadFile, | ||
| onSendMessage, | ||
| onAbort, | ||
| }: CopilotActionsProps) => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd copilot-actions.tsxRepository: refly-ai/refly
Length of output: 136
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx | head -150Repository: refly-ai/refly
Length of output: 4916
Add JSDoc and an explicit return type for the exported component.
Keeps the public API self-documented and consistent with TypeScript guidelines.
Suggested diff
+/**
+ * Renders Copilot action controls (attachments and send/stop).
+ */
export const CopilotActions = memo(
({
query,
fileCount,
maxFileCount,
isExecuting,
onUploadFile,
onSendMessage,
onAbort,
- }: CopilotActionsProps) => {
+ }: CopilotActionsProps): JSX.Element => {📝 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.
Suggested change
| export const CopilotActions = memo( | |
| ({ | |
| query, | |
| fileCount, | |
| maxFileCount, | |
| isExecuting, | |
| onUploadFile, | |
| onSendMessage, | |
| onAbort, | |
| }: CopilotActionsProps) => { | |
| /** | |
| * Renders Copilot action controls (attachments and send/stop). | |
| */ | |
| export const CopilotActions = memo( | |
| ({ | |
| query, | |
| fileCount, | |
| maxFileCount, | |
| isExecuting, | |
| onUploadFile, | |
| onSendMessage, | |
| onAbort, | |
| }: CopilotActionsProps): JSX.Element => { |
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx`
around lines 18 - 27, Add a JSDoc block above the exported component and give
the exported constant an explicit type: change the declaration of CopilotActions
to include the React memo return type, e.g. export const CopilotActions:
React.MemoExoticComponent<React.FC<CopilotActionsProps>> = memo((props:
CopilotActionsProps) => { ... }); and add a brief /** ... */ JSDoc describing
the component and its props (referencing CopilotActions and CopilotActionsProps)
to keep the public API documented and typed.
Comment on lines
+31
to
+35
| const uploadDisabled = fileCount >= maxFileCount; | ||
| const canSend = query?.trim() || fileCount > 0; | ||
| const acceptExtensions = ACCEPT_FILE_EXTENSIONS ?? []; | ||
| const acceptValue = | ||
| acceptExtensions?.length > 0 ? acceptExtensions.map((ext) => `.${ext}`).join(',') : undefined; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant section
file_path="packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx"
if [ -f "$file_path" ]; then
echo "=== File content (lines 25-40) ==="
sed -n '25,40p' "$file_path" | cat -n
else
echo "File not found"
fiRepository: refly-ai/refly
Length of output: 823
🏁 Script executed:
# Search for usage of canSend in the file to understand how it's used
rg "canSend" packages/ai-workspace-common/src/components/canvas/copilot/ -A 2 -B 2Repository: refly-ai/refly
Length of output: 1650
Make canSend always a boolean by using nullish coalescing.
Currently canSend can be a non-empty string (when query?.trim() returns a value) or a boolean. While this works due to JavaScript's type coercion, it violates the coding guideline requiring nullish coalescing for potentially undefined values and reduces type safety.
Suggested diff
- const canSend = query?.trim() || fileCount > 0;
+ const canSend =
+ (query?.trim()?.length ?? 0) > 0 || fileCount > 0;🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx`
around lines 31 - 35, The canSend variable currently may be a string when
query?.trim() yields text; change its initialization to ensure a boolean by
nullish-coalescing the trimmed query to an empty string and then coercing to
boolean, e.g., replace the expression using query?.trim() with a boolean
coercion of (query?.trim() ?? '') combined with fileCount > 0; update the const
canSend declaration accordingly (refer to symbol canSend and the use of query
and fileCount).
Comment on lines
+37
to
+46
| const handleFileChange = useCallback( | ||
| async (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const files = e.target.files; | ||
| if (!files?.length) return; | ||
|
|
||
| // Upload files in parallel | ||
| await Promise.all(Array.from(files).map((file) => onUploadFile(file))); | ||
| // Reset input | ||
| e.target.value = ''; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/ai-workspace-common && find . -name "copilot-actions.tsx" -type fRepository: refly-ai/refly
Length of output: 109
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx | head -60Repository: refly-ai/refly
Length of output: 2252
🏁 Script executed:
grep -n "onUploadFile" packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx -B 5 -A 5Repository: refly-ai/refly
Length of output: 1122
🏁 Script executed:
grep -n "try\|catch\|Promise\|\.all\|\.allSettled" packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsxRepository: refly-ai/refly
Length of output: 190
Use Promise.allSettled with try-finally to ensure input reset even on upload failures.
Promise.all rejects if any upload fails, skipping the reset statement and leaving the input field populated—this prevents users from retrying and risks an unhandled rejection.
Suggested fix
const handleFileChange = useCallback(
async (e: React.ChangeEvent<HTMLInputElement>) => {
- const files = e.target.files;
+ const input = e.target;
+ const files = input?.files;
if (!files?.length) return;
- // Upload files in parallel
- await Promise.all(Array.from(files).map((file) => onUploadFile(file)));
- // Reset input
- e.target.value = '';
+ try {
+ await Promise.allSettled(Array.from(files).map((file) => onUploadFile(file)));
+ } finally {
+ if (input) input.value = '';
+ }
},
[onUploadFile],
);Per coding guidelines: Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript, and always use try-finally for cleanup operations that must execute regardless of errors.
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx`
around lines 37 - 46, The handler handleFileChange currently uses Promise.all
and may skip resetting the input when an upload fails; change it to use
Promise.allSettled for parallel uploads and wrap the await in a try-finally so
e.target.value is always cleared in the finally block; also use optional
chaining when reading files (e.target.files?.length) to follow guidelines and
avoid runtime errors, keeping the call to onUploadFile for each file unchanged.
Comment on lines
+56
to
+119
| const attachmentButton = ( | ||
| <button | ||
| type="button" | ||
| className={cn( | ||
| 'w-7 h-7 rounded-lg flex items-center justify-center', | ||
| 'border-none outline-none focus:outline-none focus-visible:outline-none', | ||
| '[&:focus]:ring-0 [&:focus]:ring-offset-0', | ||
| 'cursor-pointer', | ||
| uploadDisabled && 'opacity-50', | ||
| )} | ||
| onClick={handleAttachmentClick} | ||
| > | ||
| <Attachment size={18} color="#1C1F23" /> | ||
| </button> | ||
| ); | ||
|
|
||
| return ( | ||
| <div className="flex items-center justify-between mt-2"> | ||
| {/* Left side: Attachment button - direct upload */} | ||
| <div className="flex items-center"> | ||
| <Tooltip | ||
| title={uploadDisabled ? t('copilot.maxFilesPerTask') : t('copilot.uploadFile')} | ||
| placement="top" | ||
| overlayInnerStyle={{ borderRadius: '8px' }} | ||
| color="#000" | ||
| > | ||
| <div className="flex items-center justify-center">{attachmentButton}</div> | ||
| </Tooltip> | ||
|
|
||
| {/* Hidden file upload input */} | ||
| <input | ||
| ref={uploadRef} | ||
| type="file" | ||
| multiple | ||
| accept={acceptValue} | ||
| onChange={handleFileChange} | ||
| className="hidden" | ||
| /> | ||
| </div> | ||
|
|
||
| {/* Right side: Send/Stop button */} | ||
| <div className="flex items-center"> | ||
| {isExecuting ? ( | ||
| <button | ||
| type="button" | ||
| className="w-9 h-9 rounded-full bg-[#1C1F23] hover:bg-[#3D4043] flex items-center justify-center border-none cursor-pointer transition-colors" | ||
| onClick={onAbort} | ||
| > | ||
| <Stop size={20} color="white" /> | ||
| </button> | ||
| ) : ( | ||
| <button | ||
| type="button" | ||
| disabled={!canSend} | ||
| className={cn( | ||
| 'w-9 h-9 rounded-full flex items-center justify-center border-none transition-colors', | ||
| canSend | ||
| ? 'bg-[#1C1F23] hover:bg-[#3D4043] cursor-pointer' | ||
| : 'bg-[rgba(28,31,35,0.1)] cursor-not-allowed', | ||
| )} | ||
| onClick={onSendMessage} | ||
| > | ||
| <Send size={20} color="white" /> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsxRepository: refly-ai/refly
Length of output: 140
🏁 Script executed:
cat -n packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsxRepository: refly-ai/refly
Length of output: 4916
Add a loading state for the attachment button during file uploads.
The onUploadFile function is async (as seen in the prop definition), and handleFileChange awaits the upload operation. However, the attachment button provides no visual feedback while files are being uploaded—it only shows a disabled state based on file count limits. Consider adding a loading indicator (spinner or visual change) to reflect the in-flight upload state, similar to how the send button uses isExecuting for async feedback.
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx`
around lines 56 - 119, Add an "isUploading" local state and use it inside
handleFileChange and the attachmentButton to show a loading UI: in
handleFileChange (the function that awaits the async onUploadFile prop) set
isUploading = true before calling await onUploadFile(files) and ensure
isUploading = false in a finally block so it resets on success/error; update
attachmentButton to render a spinner or alternate visual when isUploading is
true and disable clicks (or prevent handleAttachmentClick from opening
uploadRef) while isUploading is true; keep existing uploadDisabled logic intact
so the button remains visually disabled when limits are reached and combine it
with isUploading to provide proper feedback.
Comment on lines
+101
to
+115
| className="w-9 h-9 rounded-full bg-[#1C1F23] hover:bg-[#3D4043] flex items-center justify-center border-none cursor-pointer transition-colors" | ||
| onClick={onAbort} | ||
| > | ||
| <Stop size={20} color="white" /> | ||
| </button> | ||
| ) : ( | ||
| <button | ||
| type="button" | ||
| disabled={!canSend} | ||
| className={cn( | ||
| 'w-9 h-9 rounded-full flex items-center justify-center border-none transition-colors', | ||
| canSend | ||
| ? 'bg-[#1C1F23] hover:bg-[#3D4043] cursor-pointer' | ||
| : 'bg-[rgba(28,31,35,0.1)] cursor-not-allowed', | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "copilot-actions.tsx"Repository: refly-ai/refly
Length of output: 136
🏁 Script executed:
sed -n '95,120p' packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsxRepository: refly-ai/refly
Length of output: 1034
Align CTA and destructive button colors with design tokens.
The Stop button is a destructive action (abort) and should use the error red (#F04438). The Send button is the primary CTA and should use the primary blue (#155EEF).
Suggested diff
- className="w-9 h-9 rounded-full bg-[`#1C1F23`] hover:bg-[`#3D4043`] flex items-center justify-center border-none cursor-pointer transition-colors"
+ className="w-9 h-9 rounded-full bg-[`#F04438`] hover:bg-[`#F04438`]/90 flex items-center justify-center border-none cursor-pointer transition-colors"
onClick={onAbort}
>
<Stop size={20} color="white" />
</button>
) : (
<button
type="button"
disabled={!canSend}
className={cn(
'w-9 h-9 rounded-full flex items-center justify-center border-none transition-colors',
canSend
- ? 'bg-[`#1C1F23`] hover:bg-[`#3D4043`] cursor-pointer'
- : 'bg-[rgba(28,31,35,0.1)] cursor-not-allowed',
+ ? 'bg-[`#155EEF`] hover:bg-[`#155EEF`]/90 cursor-pointer'
+ : 'bg-[`#155EEF`]/40 cursor-not-allowed',
)}
onClick={onSendMessage}🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-actions.tsx`
around lines 101 - 115, Replace the hardcoded hex background colors for the
destructive Stop button and the primary Send button with the project's design
tokens: update the Stop button (the button rendering when onAbort is used and
contains the Stop icon) to use the error/destructive token (red, `#F04438`) and
update the Send button (the branch using canSend true) to use the primary CTA
token (blue, `#155EEF`); use the theme/token utility or CSS variables your
codebase provides (instead of literal hexes) so the classes for the Stop button
and the Send button reference the error and primary tokens respectively for both
normal and hover states.
…ying internal tools, enhancing performance and maintainability. Update InternalToolRenderer import and add key handling for copilot file tools.
…plified descriptions of the two-level agent architecture and tool selection guide, enhancing readability and understanding of workflow design processes.
…uctured tables for agent architecture and tool selection. Improved readability and understanding of workflow design processes.
mrcfps
approved these changes
Feb 3, 2026
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
2 participants
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
New Features
Documentation
Tests
Localization