-
Notifications
You must be signed in to change notification settings - Fork 570
WIP: Onboarding #2044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
WIP: Onboarding #2044
Conversation
- Introduced the PureCopilot component for improved user interaction. - Updated routing to include a new path for the PureCopilot page. - Enhanced styling with new Tailwind CSS variables for control borders. - Refactored the ChatBox component to handle pending prompts and improved message handling. - Removed the HoverCard component to streamline the front-page layout. - Updated translations for new UI elements and prompts. - Added functionality to manage pending prompts in the copilot store.
- Integrated `useSearchParams` to manage onboarding state across Copilot components. - Updated `CopilotHeader`, `CopilotMessage`, and `CopilotContainer` to conditionally render elements based on the onboarding status. - Improved styling and layout adjustments for onboarding experience using Tailwind CSS. - Ensured proper handling of search parameters to maintain application state during onboarding.
…oating behavior - Added floating behavior to the PureCopilot component, allowing it to notify the FrontPage when it becomes visible. - Implemented smooth scrolling to the top of the FrontPage when the copilot is floating. - Enhanced styling with Tailwind CSS for better visual integration. - Utilized React.memo for performance optimization and ensured proper handling of component state changes.
… support - Added functionality to retrieve user locale from the database when initializing canvas state. - Updated `initEmptyCanvasState` to accept a locale parameter, allowing for localized queries in the initial canvas state. - Enhanced test cases to validate the correct initialization of canvas state with different locales. - Ensured proper handling of optional properties and array methods to maintain code robustness.
- Enhanced the SkillResponseActionsComponent to include checks for empty prompts in downstream nodes using useMemo for performance optimization. - Integrated the useCheckEmptyPrompts hook in WorkflowRunForm and useSkillResponseActions to validate prompts before executing workflows. - Updated tooltips and button titles to reflect prompt requirements, improving user feedback. - Added new translations for error messages related to missing prompts in both English and Chinese. - Ensured proper handling of optional properties and array methods to maintain code robustness.
…s hook - Enhanced the useCheckEmptyPrompts hook to include a timer for clearing highlighted nodes after displaying a warning message for empty prompts. - Integrated useEffect to clean up the timer on component unmount, ensuring proper resource management. - Utilized optional chaining and nullish coalescing for robust property access and default values. - Updated dependencies in the checkEmptyPrompts function to include new state management functions for improved functionality.
- Exported RESOURCE_TYPE_ICON_MAP for better accessibility in other components. - Enhanced InputParameterRow to utilize optional chaining and nullish coalescing for safer property access. - Updated display logic in InputParameterRow to handle variable types more effectively, including options and resource values. - Improved StartNode component by passing options and value props to InputParameterRow, ensuring robust handling of variable data. - Applied Tailwind CSS for consistent styling across components.
📝 WalkthroughWalkthroughAdds locale-aware canvas initialization, a PureCopilot onboarding page/component with pending-prompt plumbing and floating UI, empty-prompt validation that blocks runs/reruns (with UI focus/toast/highlight), multiple copilot/front-page UI and input/variable refactors, i18n and token additions, and new workflow-execution listing API/hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FrontPage as Front Page
participant PureCopilot
participant API as Canvas API
participant DB as Database
participant Store as Copilot Store
User->>FrontPage: Focus input / Click sample
FrontPage->>PureCopilot: set floating state
User->>PureCopilot: Send (initialPrompt)
PureCopilot->>API: POST /canvas (initialPrompt)
API->>DB: Fetch user uiLocale
DB-->>API: Return uiLocale
API->>API: initEmptyCanvasState(locale)
API->>DB: Persist canvas with localized state
DB-->>API: Return canvasId
API-->>PureCopilot: Canvas created
PureCopilot->>Store: setPendingPrompt(canvasId, initialPrompt)
User->>FrontPage: Navigate to new canvas
sequenceDiagram
participant User
participant RunForm as Workflow Run Form
participant Checker as useCheckEmptyPrompts
participant Graph as Canvas Graph
participant UI as UI (Toast/Focus)
User->>RunForm: Click Run
RunForm->>Checker: checkEmptyPrompts()
Checker->>Graph: Traverse nodes (skillResponse)
Graph-->>Checker: Return emptyNodeIds
alt emptyNodeIds.length > 0
Checker->>UI: Focus view + show warning toast
Checker->>Graph: Highlight nodes briefly
Checker-->>RunForm: Return non-empty list
RunForm-->>User: Abort run
else
Checker-->>RunForm: []
RunForm->>RunForm: Proceed with execution
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
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/front-page/index.tsx (1)
166-169: MissingavatarAlignin dependency array.The
renderUserAvataruseMemo depends onavatarAlignbut it's not included in the dependency array. This could cause stale renders whenavatarAlignchanges.🐛 Proposed fix
const renderUserAvatar = useMemo( () => <UserAvatar showName={showName} userProfile={userProfile} avatarAlign={avatarAlign} />, - [showName, userProfile], + [showName, userProfile, avatarAlign], );
🤖 Fix all issues with AI agents
In
@packages/ai-workspace-common/src/components/canvas/copilot/copilot-header.tsx:
- Around line 22-25: sessionHistory's useMemo and the two useEffect hooks have
missing dependencies and the store map access lacks optional chaining; update
the dependency arrays to include canvasId and removeHistoryTemplateSession in
the useMemo that computes sessionHistory, and include refetch in both useEffect
dependency arrays, and change state.historyTemplateSessions[canvasId] accesses
to optional chaining form state.historyTemplateSessions?.[canvasId] ?? [] (apply
the same dependency and optional-chaining fixes across the other occurrences
noted: lines 26-54, 75-84, 106-167) so closures are not stale and map lookups
are safe.
In @packages/ai-workspace-common/src/components/canvas/nodes/start.tsx:
- Around line 128-129: Remove the debug console.log call logging
workflowVariables; locate the console.log('workflowVariables',
workflowVariables) statement (in the Start node/component around start.tsx) and
delete it so no development-only logging remains in production code.
In @packages/ai-workspace-common/src/components/pure-copilot/index.tsx:
- Around line 67-76: The setTimeout in handleSendMessage can cause a state
update after unmount; change it to store the timer id in a ref (e.g., const
timeoutRef = useRef<number | null>(null)), clear any existing timer before
creating a new one, assign timeoutRef.current = window.setTimeout(() =>
setQuery(''), 1000), and add a useEffect cleanup that clears timeoutRef.current
on unmount; update handleSendMessage's dependency usage accordingly and keep
debouncedCreateCanvas and setQuery behavior unchanged.
In @packages/canvas-common/src/sync.test.ts:
- Around line 74-81: The assertion is brittle and unsafe: replace direct
indexing like state.nodes[1] and chained property access with optional chaining
(e.g., state.nodes?.[1]?....) to avoid runtime throws when nodes are missing,
and stop using selectedToolsets as any; instead assert on a stable,
non-localized field from the toolset objects (for example use
selectedToolsets?.[0]?.id or selectedToolsets?.[0]?.toolset.key with
expect.any(String) / expect.objectContaining) or cast to a concrete type rather
than any so the test doesn’t rely on localized name strings and is type-safe.
🧹 Nitpick comments (13)
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx (1)
62-73: Consider extractingVariableIconas a separate component.Returning JSX from
useMemoworks but is unconventional. Extracting this as a memoized component would be more idiomatic React and align with the coding guidelines to "split nested components with closures into separate components."♻️ Suggested refactor
const VariableTypeIcon = memo(({ variableType, value }: { variableType: 'string' | 'option' | 'resource'; value?: VariableValue[] }) => { if (variableType === 'option') { return <List size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; } if (variableType === 'resource') { const resourceType = value?.[0]?.resource?.fileType; const Icon = RESOURCE_TYPE_ICON_MAP[resourceType as keyof typeof RESOURCE_TYPE_ICON_MAP] ?? Attachment; return <Icon size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; } return <BiText size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; }); VariableTypeIcon.displayName = 'VariableTypeIcon';Then use
<VariableTypeIcon variableType={variableType} value={value} />in the render.packages/ai-workspace-common/src/hooks/canvas/use-create-canvas.ts (1)
29-29: Consider using the shallow selector pattern for consistency.The store access uses a direct selector, which is fine for a single primitive setter. However, other stores in this file use the
*Shallowpattern. For consistency and to prevent potential re-renders if the store grows, consider using the shallow pattern ifuseCopilotStoreShallowis available.♻️ Optional: Use shallow selector for consistency
- const setPendingPrompt = useCopilotStore((state) => state.setPendingPrompt); + const { setPendingPrompt } = useCopilotStoreShallow((state) => ({ + setPendingPrompt: state.setPendingPrompt, + }));Note: This would require importing
useCopilotStoreShallowinstead ofuseCopilotStore.packages/stores/src/stores/copilot.ts (1)
109-109: Consider cleanup strategy for stale pending prompts.Persisting
pendingPromptis appropriate for the onboarding flow, but prompts may accumulate in localStorage if users abandon canvases. Consider adding cleanup logic (e.g., clearing prompts for deleted canvases, or a TTL-based cleanup) in a future iteration if storage bloat becomes a concern.packages/ai-workspace-common/src/components/canvas/nodes/shared/skill-response-actions.tsx (1)
393-402: Consider removing redundanttitleattribute.The Button at line 394-402 has both a
Tooltipwrapper and atitleattribute (line 401) set tosingleButtonTooltip. This redundancy may cause inconsistent tooltip behavior (native browser tooltip vs Ant Design Tooltip).♻️ Suggested fix
<Tooltip title={<div className="text-[13px]">{singleButtonTooltip}</div>} arrow={false}> <Button type="text" size="small" icon={icon} onClick={handleToggleWorkflowRun} disabled={actionDisabled} className={buttonClassName} - title={singleButtonTooltip} /> </Tooltip>apps/api/src/modules/canvas-sync/canvas-sync.service.ts (1)
193-210: Locale lookup works, but consider avoiding an extra DB roundtrip here.This adds a
prisma.user.findUniqueto getuiLocalebefore callinginitEmptyCanvasState({ locale }). Looks correct, but it’s an extra query on the “no version” path; ifuseralready has locale (or can be selected alongside the initial canvas lookup), you could pass it through and skip the query.Optional simplification:
const locale = dbUser?.uiLocale ?? undefined;can just beconst locale = dbUser?.uiLocale;.packages/ai-workspace-common/src/components/pure-copilot/index.scss (1)
19-57: Consider using design tokens for consistency.The hardcoded color values (
#7effdb,#0dcbff,#729cff) don't align with the established design system tokens. Per the coding guidelines, colors should use the defined palette (primary:#155EEF, etc.).If these are intentional accent colors for the glow effect, consider defining them as CSS custom properties in the theme tokens for maintainability.
packages/ai-workspace-common/src/components/canvas/copilot-container/index.tsx (1)
13-15: UnnecessaryuseMemofor simple property access.
searchParams.get('source')is a trivial synchronous operation. UsinguseMemohere adds overhead without benefit. Simply derive the value inline.♻️ Suggested simplification
- const [searchParams] = useSearchParams(); - const source = useMemo(() => searchParams.get('source'), [searchParams]); - const isOnboarding = source === 'onboarding'; + const [searchParams] = useSearchParams(); + const isOnboarding = searchParams.get('source') === 'onboarding';packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx (1)
127-140: RemovelogEventfrom dependency array.
logEventis a static import from@refly/telemetry-web, not a React hook or prop. Including it in the dependency array is unnecessary and could cause confusion during code review. Based on learnings and best practices, only include actual reactive dependencies.♻️ Suggested fix
[ isExecuting, currentSessionId, query, canvasId, invokeAction, setQuery, setCurrentSessionId, appendSessionResultId, setCreatedCopilotSessionId, addHistoryTemplateSession, - logEvent, + onSendMessage, ],packages/ai-workspace-common/src/components/canvas/front-page/recent-workflow.tsx (1)
46-51: RemovelogEventfrom dependency array.Same as in
chat-box.tsx,logEventis a static import and shouldn't be included in the dependency array.♻️ Suggested fix
- }, [debouncedCreateCanvas, setIsManualCollapse, logEvent]); + }, [debouncedCreateCanvas, setIsManualCollapse]);packages/ai-workspace-common/src/components/canvas/front-page/index.tsx (2)
366-368: Add dependency touseEffector document intentional behavior.The
useEffectcallsgetCanvasListwith an empty dependency array, meaning it only runs on mount. If this is intentional (fetch once on mount), consider adding an eslint-disable comment to clarify. Otherwise, includegetCanvasListin the dependency array.♻️ Suggested clarification
useEffect(() => { getCanvasList(); + // eslint-disable-next-line react-hooks/exhaustive-deps -- intentionally run only on mount }, []);
472-495: Simplify redundant conditional branches.Lines 472-478 and 488-494 render identical skeleton content. The condition on line 479 (
templateCategoryId && templateCategoryId.trim().length > 0) is effectively the opposite of line 472's!templateCategoryId, making the final fallback unreachable.Consider consolidating these branches.
♻️ Suggested simplification
- ) : !templateCategoryId ? ( - // Show loading skeleton if categories exist but none is selected yet - <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 lg:grid-cols-4 gap-4"> - {Array.from({ length: 4 }).map((_, index) => ( - <TemplateCardSkeleton key={index} /> - ))} - </div> - ) : templateCategoryId && templateCategoryId.trim().length > 0 ? ( + ) : templateCategoryId?.trim() ? ( // Show template list when category is selected and valid <TemplateList source="front-page" scrollableTargetId="front-page-scrollable-div" language={currentLanguage} categoryId={templateCategoryId} className="!bg-transparent !px-0 !pt-0" /> ) : ( - // Fallback: show loading skeleton if categoryId is invalid + // Show loading skeleton if categories exist but none is selected yet <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 lg:grid-cols-4 gap-4"> {Array.from({ length: 4 }).map((_, index) => ( <TemplateCardSkeleton key={index} /> ))} </div> )}packages/ai-workspace-common/src/components/pure-copilot/index.tsx (2)
19-44: Remove unusedSamplePromptcomponent.The
SamplePromptcomponent is defined but never used. TherenderSamplePromptsfunction (lines 100-127) renders different JSX inline instead of using this component.Either use this component in
renderSamplePromptsor remove it to reduce bundle size and confusion.
82-98: Hardcoded sample prompts should use translations for i18n support.The sample prompt texts are hardcoded in English. Per coding guidelines, all user-facing text should be translatable using the translation hook. Consider moving these to the i18n files.
♻️ Suggested approach
const samplePrompts = useMemo( () => [ { icon: '📚', - text: "Help me track and analyze the changes in Buffett's U.S. stock holdings this quarter", + text: t('copilot.samplePrompts.buffett'), }, { icon: '📮', - text: "Scrape today's Product Hunt Top 10, generate a summary document, and deliver to my email", + text: t('copilot.samplePrompts.productHunt'), }, { icon: '🖼️', - text: 'Generate a sequence of Makoto Shinkai-style animated scenes for a story about growing up', + text: t('copilot.samplePrompts.shinkaiAnimation'), }, ], - [], + [t], );
| const [searchParams] = useSearchParams(); | ||
| const source = useMemo(() => searchParams.get('source'), [searchParams]); | ||
| const isOnboarding = source === 'onboarding'; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix missing hook dependencies (stale closures) + use optional chaining on store map access.
sessionHistory’suseMemousescanvasIdandremoveHistoryTemplateSessionbut they’re not in the dependency array. This can lead to stale behavior if either changes.- Both
useEffectblocks callrefetch()but omit it from deps. - Store access should follow the optional chaining guideline:
state.historyTemplateSessions?.[canvasId] ?? [].
Proposed fix
const { setCurrentSessionId, historyTemplateSessions, removeHistoryTemplateSession } =
useCopilotStoreShallow((state) => ({
setCurrentSessionId: state.setCurrentSessionId,
- historyTemplateSessions: state.historyTemplateSessions[canvasId] ?? [],
+ historyTemplateSessions: state.historyTemplateSessions?.[canvasId] ?? [],
removeHistoryTemplateSession: state.removeHistoryTemplateSession,
}));
const sessionHistory = useMemo(() => {
- let listFromRequest = data?.data ?? [];
+ let listFromRequest = Array.isArray(data?.data) ? data.data : [];
for (const session of historyTemplateSessions) {
if (!listFromRequest.some((s) => s.sessionId === session.sessionId)) {
listFromRequest = [session, ...listFromRequest];
} else {
removeHistoryTemplateSession(canvasId, session.sessionId);
}
}
return listFromRequest;
- }, [data, historyTemplateSessions]);
+ }, [canvasId, data, historyTemplateSessions, removeHistoryTemplateSession]);
useEffect(() => {
if (isHistoryOpen) {
- refetch();
+ refetch();
}
- }, [isHistoryOpen]);
+ }, [isHistoryOpen, refetch]);
useEffect(() => {
- refetch();
- }, [canvasId]);
+ refetch();
+ }, [canvasId, refetch]);Also applies to: 26-54, 75-84, 106-167
🤖 Prompt for AI Agents
In
@packages/ai-workspace-common/src/components/canvas/copilot/copilot-header.tsx
around lines 22 - 25, sessionHistory's useMemo and the two useEffect hooks have
missing dependencies and the store map access lacks optional chaining; update
the dependency arrays to include canvasId and removeHistoryTemplateSession in
the useMemo that computes sessionHistory, and include refetch in both useEffect
dependency arrays, and change state.historyTemplateSessions[canvasId] accesses
to optional chaining form state.historyTemplateSessions?.[canvasId] ?? [] (apply
the same dependency and optional-chaining fixes across the other occurrences
noted: lines 26-54, 75-84, 106-167) so closures are not stale and map lookups
are safe.
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx
Outdated
Show resolved
Hide resolved
| const handleSendMessage = useCallback(() => { | ||
| if (!query.trim() || isCreating) return; | ||
| debouncedCreateCanvas(source, { | ||
| initialPrompt: query, | ||
| }); | ||
|
|
||
| setTimeout(() => { | ||
| setQuery(''); | ||
| }, 1000); | ||
| }, [query, debouncedCreateCanvas, isCreating, source]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add cleanup for setTimeout to prevent memory leak.
The setTimeout inside handleSendMessage could cause a state update on an unmounted component if the user navigates away quickly. Use useRef to track mount status or clear the timeout on unmount.
🐛 Proposed fix using useRef
+ const timeoutRef = useRef<NodeJS.Timeout | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ };
+ }, []);
+
const handleSendMessage = useCallback(() => {
if (!query.trim() || isCreating) return;
debouncedCreateCanvas(source, {
initialPrompt: query,
});
- setTimeout(() => {
+ timeoutRef.current = setTimeout(() => {
setQuery('');
}, 1000);
}, [query, debouncedCreateCanvas, isCreating, source]);📝 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.
| const handleSendMessage = useCallback(() => { | |
| if (!query.trim() || isCreating) return; | |
| debouncedCreateCanvas(source, { | |
| initialPrompt: query, | |
| }); | |
| setTimeout(() => { | |
| setQuery(''); | |
| }, 1000); | |
| }, [query, debouncedCreateCanvas, isCreating, source]); | |
| const timeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| useEffect(() => { | |
| return () => { | |
| if (timeoutRef.current) { | |
| clearTimeout(timeoutRef.current); | |
| } | |
| }; | |
| }, []); | |
| const handleSendMessage = useCallback(() => { | |
| if (!query.trim() || isCreating) return; | |
| debouncedCreateCanvas(source, { | |
| initialPrompt: query, | |
| }); | |
| timeoutRef.current = setTimeout(() => { | |
| setQuery(''); | |
| }, 1000); | |
| }, [query, debouncedCreateCanvas, isCreating, source]); |
🤖 Prompt for AI Agents
In @packages/ai-workspace-common/src/components/pure-copilot/index.tsx around
lines 67 - 76, The setTimeout in handleSendMessage can cause a state update
after unmount; change it to store the timer id in a ref (e.g., const timeoutRef
= useRef<number | null>(null)), clear any existing timer before creating a new
one, assign timeoutRef.current = window.setTimeout(() => setQuery(''), 1000),
and add a useEffect cleanup that clears timeoutRef.current on unmount; update
handleSendMessage's dependency usage accordingly and keep debouncedCreateCanvas
and setQuery behavior unchanged.
| title: '', | ||
| entityId: expect.any(String), | ||
| metadata: { | ||
| status: 'init', | ||
| query: | ||
| 'Generate a product introduction for Refly.ai and send it to me via email using the send email tool', | ||
| selectedToolsets: expect.any(Array), | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid brittle locale assertions + tighten TS safety (optional chaining, no any).
state.nodes[1]...can throw if nodes are unexpectedly missing; preferstate.nodes?.[1]?.....selectedToolsets as anyis avoidable; cast to an appropriate type (or assert on a stable field likeid/toolset.key) instead of a localizednamestring, otherwise tests can break if UI labels change (or if only the query is localized but toolset name isn’t).
Proposed adjustment
it('should return a valid empty canvas state with Chinese query', () => {
const state = initEmptyCanvasState({ locale: 'zh-CN' });
- expect(state.nodes[1].data?.metadata?.query).toBe(
+ expect(state.nodes?.[1]?.data?.metadata?.query).toBe(
'生成一份 Refly.ai 的产品介绍,并使用发送邮件工具通过邮件发送给我',
);
- expect((state.nodes[1].data?.metadata?.selectedToolsets as any)?.[0]?.name).toBe('发送邮件');
+ const selectedToolsets = state.nodes?.[1]?.data?.metadata?.selectedToolsets;
+ expect(Array.isArray(selectedToolsets)).toBe(true);
+ expect(selectedToolsets?.[0]?.id).toBe('send_email');
});Also applies to: 93-99
🤖 Prompt for AI Agents
In @packages/canvas-common/src/sync.test.ts around lines 74 - 81, The assertion
is brittle and unsafe: replace direct indexing like state.nodes[1] and chained
property access with optional chaining (e.g., state.nodes?.[1]?....) to avoid
runtime throws when nodes are missing, and stop using selectedToolsets as any;
instead assert on a stable, non-localized field from the toolset objects (for
example use selectedToolsets?.[0]?.id or selectedToolsets?.[0]?.toolset.key with
expect.any(String) / expect.objectContaining) or cast to a concrete type rather
than any so the test doesn’t rely on localized name strings and is type-safe.
- Removed unnecessary imports and optimized the VariableItem component by eliminating the image and divider for improved performance and cleaner UI. - Enhanced InputParameterRow to utilize a Popover for displaying variable details, improving user interaction and accessibility. - Applied Tailwind CSS for consistent styling across components. - Ensured robust handling of optional properties and array methods to maintain code robustness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx:
- Line 24: Remove the debug console.log in the VariableHoverCard component:
delete the line printing 'resource' (the console.log('resource', resource) call)
from
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
so no debug output remains in production; if structured runtime logging is
required instead, replace it with the project logger (e.g., processLogger or
existing logging utility) and ensure it follows the component's logging
conventions.
- Around line 39-41: The JSX hardcoded string 'Empty value' in
variable-hover-card.tsx (inside the div rendering {value?.[0]?.text || 'Empty
value'}) must be wrapped with the translation helper (i18n translation wrapper)
and replaced by a translation key (e.g., variableHoverCard.emptyValue) so the UI
text is localizable; update the JSX to use the translation function instead of
the literal and add the corresponding key/value to the i18n resource files for
supported locales.
In @packages/ai-workspace-common/src/components/canvas/nodes/start.tsx:
- Line 93: The JSX div in the Start node component currently has both
"cursor-default" and "cursor-pointer" in its className, causing a conflict; edit
the class list in the div (the line with className="flex gap-2 items-center
justify-between py-1.5 px-3 bg-refly-bg-control-z0 rounded-lg cursor-default
hover:bg-refly-bg-control-z1 transition-colors cursor-pointer") and remove the
redundant "cursor-default" so only "cursor-pointer" remains.
🧹 Nitpick comments (4)
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx (3)
33-37: Unused constantVARIABLE_TYPE_ICON_MAP.This mapping is defined but never used in the file. The
InputParameterRowcomponent uses inline logic in theVariableIconuseMemo instead of referencing this map.Consider either removing this unused constant or refactoring
VariableIconto use it:Option 1: Remove unused constant
-export const VARIABLE_TYPE_ICON_MAP = { - string: BiText, - option: List, - resource: Attachment, -};Option 2: Use the constant in VariableIcon
const VariableIcon = useMemo(() => { + if (variableType === 'resource') { + const resourceType = value?.[0]?.resource?.fileType; + const Icon = + RESOURCE_TYPE_ICON_MAP[resourceType as keyof typeof RESOURCE_TYPE_ICON_MAP] ?? Attachment; + return <Icon size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; + } + const Icon = VARIABLE_TYPE_ICON_MAP[variableType]; + return <Icon size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; - if (variableType === 'option') { - return <List size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; - } - if (variableType === 'resource') { - const resourceType = value?.[0]?.resource?.fileType; - const Icon = - RESOURCE_TYPE_ICON_MAP[resourceType as keyof typeof RESOURCE_TYPE_ICON_MAP] ?? Attachment; - return <Icon size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; - } - return <BiText size={14} color="var(--refly-text-3)" className="flex-shrink-0" />; }, [variableType, value]);
77-111: Inline style object in Popover creates new reference on each render.The
overlayInnerStyleprop receives an inline object, which creates a new reference on every render. This could cause unnecessary re-renders of the Popover component.Extract to a constant or useMemo
+const POPOVER_INNER_STYLE = { padding: 0, backgroundColor: 'transparent', boxShadow: 'none' }; + export const InputParameterRow = memo( ({ variableType, label, options, value, isRequired = false, }: { // ... }) => { // ... return ( <Popover content={/* ... */} placement="right" overlayClassName="variable-hover-card-popover" trigger="hover" mouseEnterDelay={0.3} - overlayInnerStyle={{ padding: 0, backgroundColor: 'transparent', boxShadow: 'none' }} + overlayInnerStyle={POPOVER_INNER_STYLE} >
163-191: Unnecessaryidin dependency array.The
handleAskAIcallback includesidin its dependency array (line 190), butidis not used within the callback body. Onlydata?.entityId,addNode, andgetConnectionInfoare used.Remove unused dependency
- }, [id, addNode, getConnectionInfo]); + }, [data?.entityId, addNode, getConnectionInfo]);packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx (1)
99-104: Inconsistent icon usage for empty resource state.When there's no resource, the component shows a
BiTexticon, but for the resource type header it uses the resource-specific icon (orAttachmentfallback). UsingAttachmentwould be more semantically consistent for an empty resource placeholder.Use Attachment icon for consistency
<div className="flex flex-col items-center justify-center h-full gap-2 text-refly-text-3 opacity-60 py-4"> - <BiText size={24} strokeWidth={1.5} /> + <Attachment size={24} /> <span className="text-[11px] text-center px-4"> {t('canvas.workflow.variables.noContent')} </span> </div>Based on learnings, icons should have appropriate alternative text. Consider adding an
aria-labelfor accessibility.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/ai-workspace-common/src/components/canvas/node-preview/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ai-workspace-common/src/components/canvas/node-preview/start.tsx
🧰 Additional context used
📓 Path-based instructions (19)
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{jsx,tsx}: Always use tailwind css to style the component
Always wrap pure components with React.memo to prevent unnecessary re-renders
Always use useMemo for expensive computations or complex object creation
Always use useCallback for function props to maintain referential equality
Always specify proper dependency arrays in useEffect to prevent infinite loops
Always avoid inline object/array creation in render to prevent unnecessary re-renders
Always use proper key props when rendering lists
Always split nested components with closures into separate components to avoid performance issues and improve code maintainability
**/*.{jsx,tsx}: Always wrap pure components with React.memo to prevent unnecessary re-renders
Always use useMemo for expensive computations or complex object creation in React
Always use useCallback for function props to maintain referential equality in React
Always specify proper dependency arrays in useEffect to prevent infinite loops in React
Always avoid inline object/array creation in render to prevent unnecessary re-renders in React
Always use proper key props when rendering lists in React (avoid using index when possible)
Always split nested components with closures into separate components in React
Use lazy loading for components that are not immediately needed in React
Debounce handlers for events that might fire rapidly (resize, scroll, input) in React
Implement fallback UI for components that might fail in React
Use error boundaries to catch and handle runtime errors in React
**/*.{jsx,tsx}: Place each attribute on a new line when a component has multiple attributes in JSX
Use self-closing tags for elements without children in JSX
Keep JSX expressions simple, extract complex logic to variables
Put closing brackets for multi-line JSX on a new line
**/*.{jsx,tsx}: Component file names should match the component name
Organize function components in order: imports, type definitions, constants, component function, hook calls, e...
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{jsx,tsx,css}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{jsx,tsx,css}: Use Tailwind CSS for styling components
Follow the utility-first approach with Tailwind CSS
Group related utility classes together in Tailwind CSS
Prefer Tailwind utilities over custom CSS when possible
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{jsx,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Each component file should contain only one main component
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Explicitly type props with interfaces or types in React components
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
Use React best practices for frontend code
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:05:07.580Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/11-ui-design-patterns.mdc:0-0
Timestamp: 2025-11-25T03:05:07.580Z
Learning: Applies to **/*.{tsx,ts,jsx,js,vue} : Provide alternative text for images and icons
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
🧬 Code graph analysis (1)
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx (3)
packages/ai-workspace-common/src/context/canvas.tsx (1)
useCanvasContext(859-865)packages/ai-workspace-common/src/components/canvas/node-preview/start.tsx (1)
RESOURCE_TYPE_ICON_MAP(24-29)packages/ai-workspace-common/src/components/canvas/canvas-resources/file-preview/index.tsx (1)
FilePreview(86-271)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build
🔇 Additional comments (6)
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx (2)
54-75: LGTM on memoization approach.Good use of
useMemofor bothdisplayValueandVariableIconcomputations with proper dependency arrays. This follows the coding guidelines for expensive computations and prevents unnecessary re-renders.
244-252: LGTM on passing options and value to InputParameterRow.The props are correctly passed with nullish coalescing to provide default empty arrays, ensuring safe access downstream.
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx (4)
57-64: Using array index as key for options list.While using index as a key is generally discouraged, it's acceptable here since options are static display values unlikely to be reordered or filtered. If options could be dynamically modified, consider using the option value itself as the key.
19-23:canvasIdis retrieved but only used conditionally.The
canvasIdis obtained fromuseCanvasContext()on every render but only used whenvariableType === 'resource'and a resource exists. This is fine for now, but if context access becomes expensive, you could consider conditionalizing it.
115-119: LGTM on component structure and styling.The component is properly wrapped with
memo, has adisplayName, and uses Tailwind CSS classes consistent with the project's design system.
86-95: The type assertion is safe. The object provides all required properties ofDriveFile(fileId,canvasId,name,type), and the optional properties (category,source,scope,size,summary,storageKey) are not needed.FilePreviewexpects a completeDriveFileobject, which is what's being provided here. Thetypefield derivation is correct—mime.getType()returns a string, matchingDriveFile['type'].Likely an incorrect or invalid review comment.
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx
Outdated
Show resolved
Hide resolved
- Removed unnecessary imports and optimized the StartNode component by integrating InputParameterRow for better variable handling. - Enhanced InputParameterRow to accept variable objects directly, improving code clarity and maintainability. - Updated VariableHoverCard to source RESOURCE_TYPE_ICON_MAP from the correct location, ensuring consistent icon usage. - Applied Tailwind CSS for consistent styling across components. - Ensured robust handling of optional properties and array methods to maintain code robustness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsx:
- Around line 103-109: In the InputParameterRow component
(input-parameter-row.tsx) the className string includes both "cursor-default"
and "cursor-pointer"; remove "cursor-default" and keep "cursor-pointer" so the
element consistently shows the pointer cursor for its hover/clickable
behavior—update the className expression where the group flex ... string is
built to omit "cursor-default".
- Around line 72-84: The success message in handleDeleteVariable is shown before
the potentially async onDelete finishes; change the function to await the
onDelete callback (await onDelete?.(variable)) so the try/catch actually wraps
the async operation, then only call message.success after the await and call
message.error in the catch; keep setIsDeleting(true) at the start and
setIsDeleting(false) in finally so the loading state covers the full async
delete.
In
@packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx:
- Line 24: Remove the debug console.log by deleting the line that logs resource
in the VariableHoverCard component (the console.log('resource', resource); in
variable-hover-card.tsx); simply remove this statement so no debug output is
left in the component code before merging.
- Around line 39-41: Replace the hardcoded fallback string in the
VariableHoverCard component: instead of using the literal in the JSX expression
value?.[0]?.text || 'Empty value', use your i18n translation hook (e.g.,
useTranslation/useTranslations) and return a translated fallback like
t('variableHover.emptyValue') (or t('Empty value') if keys are not yet defined);
add the corresponding import and invoke the hook at the top of the
VariableHoverCard component so the JSX uses the translation function for the
fallback.
🧹 Nitpick comments (4)
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx (2)
57-64: Consider using a stable key instead of array index.Using array index as the key can cause issues if the options list is reordered or modified. If each option is unique, use the option value itself as the key.
Proposed fix
{options?.map((opt, i) => ( <div - key={i} + key={opt} className="px-2 text-xs border-solid border-[1px] border-refly-Card-Border rounded-md" > {opt} </div> ))}
99-100: Potentially incorrect icon for empty resource state.The empty resource placeholder uses
BiTexticon, but a resource-type placeholder might be more appropriate (e.g.,Attachmentor a generic file icon) to maintain visual consistency with the resource type.packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsx (2)
58-70:useMemoreturns JSX elements, consideruseCallbackor extraction.Returning JSX from
useMemoworks but is unconventional. The icon logic could be extracted to a helper function or the icon component reference could be memoized instead, with rendering done inline.Alternative approach
- const VariableIcon = useMemo(() => { - const size = isPreview ? 16 : 14; - if (variableType === 'option') { - return <List size={size} color="var(--refly-text-3)" className="flex-shrink-0" />; - } - if (variableType === 'resource') { - const resourceType = value?.[0]?.resource?.fileType; - const Icon = - RESOURCE_TYPE_ICON_MAP[resourceType as keyof typeof RESOURCE_TYPE_ICON_MAP] ?? Attachment; - return <Icon size={size} color="var(--refly-text-3)" className="flex-shrink-0" />; - } - return <BiText size={size} color="var(--refly-text-3)" className="flex-shrink-0" />; - }, [variableType, value, isPreview]); + const IconComponent = useMemo(() => { + if (variableType === 'option') return List; + if (variableType === 'resource') { + const resourceType = value?.[0]?.resource?.fileType; + return RESOURCE_TYPE_ICON_MAP[resourceType as keyof typeof RESOURCE_TYPE_ICON_MAP] ?? Attachment; + } + return BiText; + }, [variableType, value]); + + const iconSize = isPreview ? 16 : 14;Then render inline:
<IconComponent size={iconSize} color="var(--refly-text-3)" className="flex-shrink-0" />
138-141: Template literal with ternary could usecn()for consistency.For consistency with the rest of the component that uses
cn()for conditional class handling, consider usingcn()here as well.Proposed fix
<div - className={`items-center gap-1 flex-shrink-0 ${ - isPopconfirmOpen ? 'flex' : 'hidden group-hover:flex' - }`} + className={cn( + 'items-center gap-1 flex-shrink-0', + isPopconfirmOpen ? 'flex' : 'hidden group-hover:flex' + )} >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/ai-workspace-common/src/components/canvas/node-preview/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/markdown/plugins/tool-call/copilot-workflow-plan.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ai-workspace-common/src/components/markdown/plugins/tool-call/copilot-workflow-plan.tsx
- packages/ai-workspace-common/src/components/canvas/node-preview/start.tsx
🧰 Additional context used
📓 Path-based instructions (19)
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{jsx,tsx}: Always use tailwind css to style the component
Always wrap pure components with React.memo to prevent unnecessary re-renders
Always use useMemo for expensive computations or complex object creation
Always use useCallback for function props to maintain referential equality
Always specify proper dependency arrays in useEffect to prevent infinite loops
Always avoid inline object/array creation in render to prevent unnecessary re-renders
Always use proper key props when rendering lists
Always split nested components with closures into separate components to avoid performance issues and improve code maintainability
**/*.{jsx,tsx}: Always wrap pure components with React.memo to prevent unnecessary re-renders
Always use useMemo for expensive computations or complex object creation in React
Always use useCallback for function props to maintain referential equality in React
Always specify proper dependency arrays in useEffect to prevent infinite loops in React
Always avoid inline object/array creation in render to prevent unnecessary re-renders in React
Always use proper key props when rendering lists in React (avoid using index when possible)
Always split nested components with closures into separate components in React
Use lazy loading for components that are not immediately needed in React
Debounce handlers for events that might fire rapidly (resize, scroll, input) in React
Implement fallback UI for components that might fail in React
Use error boundaries to catch and handle runtime errors in React
**/*.{jsx,tsx}: Place each attribute on a new line when a component has multiple attributes in JSX
Use self-closing tags for elements without children in JSX
Keep JSX expressions simple, extract complex logic to variables
Put closing brackets for multi-line JSX on a new line
**/*.{jsx,tsx}: Component file names should match the component name
Organize function components in order: imports, type definitions, constants, component function, hook calls, e...
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{jsx,tsx,css}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{jsx,tsx,css}: Use Tailwind CSS for styling components
Follow the utility-first approach with Tailwind CSS
Group related utility classes together in Tailwind CSS
Prefer Tailwind utilities over custom CSS when possible
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{jsx,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Each component file should contain only one main component
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Explicitly type props with interfaces or types in React components
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
Use React best practices for frontend code
Files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsxpackages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
🧠 Learnings (9)
📚 Learning: 2025-11-25T03:05:07.580Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/11-ui-design-patterns.mdc:0-0
Timestamp: 2025-11-25T03:05:07.580Z
Learning: Applies to **/*.{tsx,ts,jsx,js,vue,css,scss,less} : Use consistent padding, border radius, and hover states for all buttons
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx
📚 Learning: 2025-11-25T03:05:07.580Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/11-ui-design-patterns.mdc:0-0
Timestamp: 2025-11-25T03:05:07.580Z
Learning: Applies to **/*.{tsx,ts,jsx,js,vue,css,scss,less} : Use clear visual indicators for focus, hover, and error states in form elements
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx
📚 Learning: 2025-11-25T03:05:07.580Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/11-ui-design-patterns.mdc:0-0
Timestamp: 2025-11-25T03:05:07.580Z
Learning: Applies to **/*.{tsx,ts,jsx,js,vue,css,scss,less} : Use subtle borders for card separation
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx
📚 Learning: 2025-11-25T03:05:07.580Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/11-ui-design-patterns.mdc:0-0
Timestamp: 2025-11-25T03:05:07.580Z
Learning: Applies to **/*.{tsx,ts,jsx,js,vue,css,scss,less} : Secondary buttons should have a border with transparent or light background
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx
📚 Learning: 2025-11-25T03:04:41.334Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-11-25T03:04:41.334Z
Learning: Applies to **/*.{tsx,ts} : Use the translation wrapper component and useTranslation hook in components
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/start.tsxpackages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
📚 Learning: 2025-11-25T03:04:41.334Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-11-25T03:04:41.334Z
Learning: Applies to **/*.{tsx,ts} : Ensure all user-facing text is translatable
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
📚 Learning: 2025-11-25T03:05:07.580Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/11-ui-design-patterns.mdc:0-0
Timestamp: 2025-11-25T03:05:07.580Z
Learning: Applies to **/*.{tsx,ts,jsx,js,vue} : Provide alternative text for images and icons
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
📚 Learning: 2025-11-25T03:04:41.334Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-11-25T03:04:41.334Z
Learning: Applies to **/*.{tsx,ts,json} : Support dynamic content with placeholders in translations
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
📚 Learning: 2025-11-25T03:03:31.945Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/01-code-style.mdc:0-0
Timestamp: 2025-11-25T03:03:31.945Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Applied to files:
packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
🧬 Code graph analysis (1)
packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsx (2)
packages/ai-workspace-common/src/components/canvas/launchpad/variable/getVariableIcon.tsx (1)
VariableIcon(80-99)packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx (1)
VariableHoverCard(19-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build
🔇 Additional comments (2)
packages/ai-workspace-common/src/components/canvas/nodes/start.tsx (1)
156-162: LGTM! Clean refactor to shared component.The migration to the shared
InputParameterRowcomponent is well-implemented. Props are passed correctly withvariable,readonly, andisHighlightedflags.packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsx (1)
25-32: Props interface is well-defined.The interface clearly documents the component's API with appropriate optional markers and types.
| const handleDeleteVariable = async (variable: WorkflowVariable) => { | ||
| try { | ||
| setIsDeleting(true); | ||
| onDelete?.(variable); | ||
| message.success( | ||
| t('canvas.workflow.variables.deleteSuccess') || 'Variable deleted successfully', | ||
| ); | ||
| } catch (error) { | ||
| console.error('Failed to delete variable:', error); | ||
| } finally { | ||
| setIsDeleting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Success message shown before confirming deletion completed.
The onDelete callback may be asynchronous, but the success message is shown immediately without awaiting its completion. If onDelete fails, users will still see a success message.
Proposed fix
const handleDeleteVariable = async (variable: WorkflowVariable) => {
try {
setIsDeleting(true);
- onDelete?.(variable);
- message.success(
- t('canvas.workflow.variables.deleteSuccess') || 'Variable deleted successfully',
- );
+ await onDelete?.(variable);
+ message.success(t('canvas.workflow.variables.deleteSuccess'));
} catch (error) {
console.error('Failed to delete variable:', error);
+ message.error(t('canvas.workflow.variables.deleteFailed'));
} finally {
setIsDeleting(false);
}
};🤖 Prompt for AI Agents
In
@packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsx
around lines 72 - 84, The success message in handleDeleteVariable is shown
before the potentially async onDelete finishes; change the function to await the
onDelete callback (await onDelete?.(variable)) so the try/catch actually wraps
the async operation, then only call message.success after the await and call
message.error in the catch; keep setIsDeleting(true) at the start and
setIsDeleting(false) in finally so the loading state covers the full async
delete.
| className={cn( | ||
| 'group flex gap-2 items-center justify-between py-1.5 px-3 cursor-default hover:bg-refly-tertiary-hover transition-colors cursor-pointer', | ||
| isPreview | ||
| ? 'h-[37px] border-solid border-[1px] border-refly-Card-Border rounded-xl' | ||
| : 'h-[30px] bg-refly-bg-control-z0 rounded-[4px]', | ||
| isHighlighted && 'bg-refly-bg-control-z1', | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conflicting cursor CSS classes.
Line 104 has both cursor-default and cursor-pointer applied, which conflict. Based on the hover behavior and clickable nature, cursor-pointer alone should be used.
Proposed fix
className={cn(
- 'group flex gap-2 items-center justify-between py-1.5 px-3 cursor-default hover:bg-refly-tertiary-hover transition-colors cursor-pointer',
+ 'group flex gap-2 items-center justify-between py-1.5 px-3 hover:bg-refly-tertiary-hover transition-colors cursor-pointer',
isPreview📝 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.
| className={cn( | |
| 'group flex gap-2 items-center justify-between py-1.5 px-3 cursor-default hover:bg-refly-tertiary-hover transition-colors cursor-pointer', | |
| isPreview | |
| ? 'h-[37px] border-solid border-[1px] border-refly-Card-Border rounded-xl' | |
| : 'h-[30px] bg-refly-bg-control-z0 rounded-[4px]', | |
| isHighlighted && 'bg-refly-bg-control-z1', | |
| )} | |
| className={cn( | |
| 'group flex gap-2 items-center justify-between py-1.5 px-3 hover:bg-refly-tertiary-hover transition-colors cursor-pointer', | |
| isPreview | |
| ? 'h-[37px] border-solid border-[1px] border-refly-Card-Border rounded-xl' | |
| : 'h-[30px] bg-refly-bg-control-z0 rounded-[4px]', | |
| isHighlighted && 'bg-refly-bg-control-z1', | |
| )} |
🤖 Prompt for AI Agents
In
@packages/ai-workspace-common/src/components/canvas/nodes/shared/input-parameter-row.tsx
around lines 103 - 109, In the InputParameterRow component
(input-parameter-row.tsx) the className string includes both "cursor-default"
and "cursor-pointer"; remove "cursor-default" and keep "cursor-pointer" so the
element consistently shows the pointer cursor for its hover/clickable
behavior—update the className expression where the group flex ... string is
built to omit "cursor-default".
| const { t } = useTranslation(); | ||
| const { canvasId } = useCanvasContext(); | ||
| const resource = value?.[0]?.resource; | ||
| console.log('resource', resource); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug console.log statement.
This debug statement should be removed before merging to main.
Proposed fix
const resource = value?.[0]?.resource;
- console.log('resource', resource);📝 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.
| console.log('resource', resource); | |
| const resource = value?.[0]?.resource; |
🤖 Prompt for AI Agents
In
@packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
at line 24, Remove the debug console.log by deleting the line that logs resource
in the VariableHoverCard component (the console.log('resource', resource); in
variable-hover-card.tsx); simply remove this statement so no debug output is
left in the component code before merging.
| <div className="flex-1 overflow-y-auto text-xs break-all bg-refly-bg-control-z0 rounded-lg p-2"> | ||
| {value?.[0]?.text || 'Empty value'} | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untranslated fallback string.
The fallback 'Empty value' should use the translation hook for i18n consistency. As per coding guidelines, all user-facing text must be translatable.
Proposed fix
<div className="flex-1 overflow-y-auto text-xs break-all bg-refly-bg-control-z0 rounded-lg p-2">
- {value?.[0]?.text || 'Empty value'}
+ {value?.[0]?.text || t('canvas.workflow.variables.emptyValue')}
</div>🤖 Prompt for AI Agents
In
@packages/ai-workspace-common/src/components/canvas/nodes/shared/variable-hover-card.tsx
around lines 39 - 41, Replace the hardcoded fallback string in the
VariableHoverCard component: instead of using the literal in the JSX expression
value?.[0]?.text || 'Empty value', use your i18n translation hook (e.g.,
useTranslation/useTranslations) and return a translated fallback like
t('variableHover.emptyValue') (or t('Empty value') if keys are not yet defined);
add the corresponding import and invoke the hook at the top of the
VariableHoverCard component so the JSX uses the translation function for the
fallback.
- Changed the route path for the PureCopilot page to '/onboarding'. - Enhanced the ProtectedRoute component to redirect users to the onboarding page if they need onboarding and are not already on that page. - Updated the AppLayout component to handle onboarding redirection based on user login status and onboarding requirements. - Integrated optional chaining and nullish coalescing for safer property access in user profile preferences. - Ensured consistent styling with Tailwind CSS across components.
- Updated ProtectedRoute to include additional checks for onboarding redirection, preventing access to workflow paths. - Enhanced CopilotMessage component to manage user onboarding state, updating user preferences and settings accordingly. - Refactored useCreateCanvas hook to change navigation path from '/canvas' to '/workflow' for better clarity. - Improved AppLayout to handle onboarding checks consistently across different routes. - Ensured robust handling of optional properties and array methods for improved code stability.
- Implemented a new endpoint in WorkflowController to list workflow executions with pagination support. - Enhanced WorkflowService to handle the logic for fetching workflow executions based on user and query parameters. - Created a new EarnedVoucherPopup component to display voucher information after workflow runs. - Updated various components and hooks to integrate the new workflow execution listing and voucher display features. - Ensured robust handling of optional properties and array methods for improved code stability. - Applied Tailwind CSS for consistent styling across components.
- Enhanced the Voucher model to include 'run_workflow' as a new source type. - Refactored the VoucherService to handle voucher creation from various sources, including the new 'run_workflow' source. - Updated WorkflowService to trigger voucher creation upon the first successful workflow execution of the day. - Improved the WorkflowController to support additional query parameters for listing workflow executions. - Ensured robust handling of optional properties and array methods for improved code stability. - Applied Tailwind CSS for consistent styling across components.
- Integrated the `cn` utility for conditional class names in the CopilotContainer component. - Simplified the rendering logic by removing unnecessary conditional checks for the onboarding state. - Ensured consistent styling with Tailwind CSS across the component. - Applied performance optimizations by utilizing React.memo and proper key props for list rendering. - Enhanced code maintainability by restructuring nested components and validating object properties before destructuring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx (1)
216-237: Missing dependencies in useCallback:setUserProfileandupdateUserSettings.The
handleApprovecallback usessetUserProfile(line 190) andupdateUserSettings(line 197), but neither is included in the dependency array. This violates the exhaustive-deps rule and could cause stale closure issues. Per coding guidelines, always specify proper dependency arrays.Proposed fix
}, [ canvasId, workflowPlan, tools?.data, getNodes, setNodes, setEdges, setVariables, t, modal, setShowWorkflowRun, defaultAgentModel, onLayout, - logEvent, canvasTitle, query, updateTitle, source, searchParams, setSearchParams, userProfile, + setUserProfile, + updateUserSettings, ]);Note:
logEventis an imported module function (not from a hook), so it doesn't need to be in the dependency array.packages/web-core/src/components/layout/index.tsx (1)
98-153: Root redirect guard can strand users on/after the first redirect.Because
hasRedirectedRef.currentnever resets, navigating back to/later in the same AppLayout lifetime can skip the redirect entirely. Consider removing the ref guard (navigation off/already prevents loops), or resetting it whenlocation.pathname !== '/'.Proposed fix (drop the sticky ref guard)
- const hasRedirectedRef = useRef(false); - // Handle root path redirection based on login status useEffect(() => { if ( location.pathname === '/' && - !userStore.isCheckingLoginStatus && - !hasRedirectedRef.current + !userStore.isCheckingLoginStatus ) { - hasRedirectedRef.current = true; if (userStore.isLogin && userStore.userProfile) { if (needOnboarding) { navigate('/onboarding', { replace: true }); } else { navigate('/workspace', { replace: true }); } } else { // Preserve query parameters (e.g., invite code) when redirecting to login const searchParams = new URLSearchParams(location.search); const loginPath = searchParams.toString() ? `/login?${searchParams.toString()}` : '/login'; navigate(loginPath, { replace: true }); } } }, [ location.pathname, location.search, userStore.isLogin, userStore.userProfile, userStore.isCheckingLoginStatus, needOnboarding, navigate, ]);apps/api/src/modules/workflow-app/workflow-app.service.ts (2)
390-446: Don’t enqueue template generation if share creation failed (processor dependency).You explicitly note enqueueing after shareRecord creation “to ensure the processor can find it”, but share creation errors are swallowed—meaning
shareId/templateShareIdcan be null while the job is still enqueued. That will likely cause processor failures or noisy retries. Gate the enqueue on successful share creation (and ideally on the specific share id the processor expects).Proposed fix (guard enqueue on share creation)
- if (shouldEnqueueGeneration && this.templateQueue) { + if (shouldEnqueueGeneration && this.templateQueue && (shareId || templateShareId)) { try { await this.templateQueue.add( 'generate', { appId, canvasId, uid: user.uid }, { removeOnComplete: true, removeOnFail: true, }, ); this.logger.log(`Enqueued template generation for workflow app: ${appId}`); } catch (error) { this.logger.error( `Failed to enqueue template generation for workflow app ${appId}: ${error?.stack}`, ); } }
330-388:templateGenerationStatus: 'pending'can be written even when no queue will run.If
this.templateQueueis unavailable, the current control flow logs “enforce async-only” but can still persisttemplateGenerationStatusas'pending'(and clear templateContent), leaving records stuck “pending” indefinitely. Consider only writing'pending'when you will actually enqueue, otherwise keep existing status or mark failed with an error.apps/api/src/modules/workflow/workflow.service.ts (1)
52-67: Approve DI and module wiring; however, address race condition and timezone issues in voucher trigger logic.Module structure is correct:
VoucherModuleis imported andVoucherServiceis properly exported invoucher.module.ts, enabling correct NestJS dependency injection.However, the voucher trigger logic (lines 172–201) has two functional issues:
- Race condition on daily execution check: The
executionsToday === 0condition is racy. Multiple concurrent requests or pods can all see 0 before any count increments, causing multiple voucher creations for a single user.- Timezone inconsistency: The check uses local midnight (
new Date()withsetHours(0, 0, 0, 0)) but database timestamps are likely UTC. This misalignment can cause incorrect daily limit tracking.Also, fix error handling on lines 196 and 200: guard
err.messagewith optional chaining (err?.message) or validate the error type. Currently, the code assumeserris an Error object, which violates optional chaining guidelines.packages/web-core/src/pages/login/index.tsx (1)
150-223: Fix missingneedOnboardingdependency and addreturnUrlvalidation to prevent potential open-redirect.
needOnboardingis derived fromuserProfile(line 48) and read inhandleEmailAuth(lines 188, 217), but missing from the dependency array (line 222). If the user profile updates after initial render, the callback uses a stale value.returnUrlfrom search params is directly decoded and passed towindow.location.replace()without validation (lines 189–190, 218). WhilestorePendingRedirectvalidates paths elsewhere, the email auth flow bypasses this check. Consider validating thatreturnUrlis a relative path or matches your allowed domains.Minimal fixes
- }, [authStore, form, isPublicAccessPage, searchParams]); + }, [authStore, form, isPublicAccessPage, searchParams, userProfile]);For
returnUrlvalidation, ensure it's a relative path before using it:- const redirectUrl = returnUrl - ? decodeURIComponent(returnUrl) - : isPublicAccessPage + const decodedUrl = returnUrl ? decodeURIComponent(returnUrl) : null; + const redirectUrl = decodedUrl && decodedUrl.startsWith('/') ? decodedUrl : (isPublicAccessPage ? window.location.href - : defaultRedirectUrl; + : defaultRedirectUrl);packages/ai-workspace-common/src/hooks/canvas/use-create-canvas.ts (1)
29-46: Prevent “stuck loading” if createCanvas throws (use try/finally).If
getClient().createCanvas()throws (network/runtime),setIsCreating(false)won’t run, leaving the UI in a perpetual creating state.Proposed fix
- const setPendingPrompt = useCopilotStore((state) => state.setPendingPrompt); + // Defensive fallback (and aligns with optional-chaining guideline) + const setPendingPrompt = + useCopilotStore((state) => state?.setPendingPrompt) ?? + (() => { + // no-op + }); const createCanvas = async (canvasTitle: string) => { setIsCreating(true); - const { data, error } = await getClient().createCanvas({ - body: { - projectId, - title: canvasTitle, - }, - }); - setIsCreating(false); + try { + const { data, error } = await getClient().createCanvas({ + body: { + projectId, + title: canvasTitle, + }, + }); - if (!data?.success || error) { - return; - } + if (!data?.success || error) { + return; + } - return data?.data?.canvasId; + return data?.data?.canvasId; + } finally { + setIsCreating(false); + } };packages/openapi-schema/src/schemas.gen.ts (1)
1186-1219: Breaking contract:ListSchedulesResponseSchema.datarestructured from array to pagination object.
Existing clients directly accessingdata[0]will break. Also addrequired: ['total', 'page', 'pageSize', 'items']within the data object to make the pagination shape explicit for validators and consumers.
🤖 Fix all issues with AI agents
In @apps/api/src/modules/workflow/workflow.service.ts:
- Around line 172-202: The current non-atomic check using
prisma.workflowExecution.count can race and allow duplicate vouchers; replace
the read-then-act with an atomic approach: either enforce a DB uniqueness
constraint for "user/date" on the voucher (or a dedicated daily_gate table) and
attempt to create the voucher inside a transaction (using prisma.$transaction)
or perform a conditional upsert so only one creation succeeds; move the call to
voucherService.handleCreateVoucherFromSource into that transactional flow and
handle failures by checking the Prisma unique-constraint error rather than
relying on count to prevent duplicates; also harden logging by using
err?.message or JSON.stringify(err) when logging inside the catch blocks around
handleCreateVoucherFromSource and the outer try so you never dereference
undefined (referencing prisma.workflowExecution.count,
voucherService.handleCreateVoucherFromSource, prisma.$transaction/upsert and the
catch blocks).
- Around line 1046-1093: The function listWorkflowExecutions can throw when
params is undefined and allows invalid pagination values; change its
signature/implementation to default params to an empty object (e.g., params =
{}) before destructuring, and clamp page and pageSize to safe ranges (ensure
page = Math.max(1, Number(page) || 1) and pageSize = Math.min(MAX_PAGE_SIZE,
Math.max(1, Number(pageSize) || DEFAULT_PAGE_SIZE))) so skip is computed from
validated values; also ensure after is parsed safely if present. Reference:
listWorkflowExecutions, params, page, pageSize, skip, after, whereClause.
In @apps/web/src/routes/redirects.tsx:
- Around line 84-85: Normalize needOnboarding to a strict boolean by coercing
the possibly-undefined/non-boolean value from userProfile.preferences into
true/false; replace the current declaration of needOnboarding with a boolean
coercion like using Boolean(userProfile?.preferences?.needOnboarding) or
!!userProfile?.preferences?.needOnboarding so it always yields false by default
and is safe for redirect logic in redirects.tsx.
In
@packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx:
- Line 111: Remove the debug console.log call that prints currentNodes in
copilot-message.tsx (the line with console.log('currentNodes', currentNodes));
either delete that statement entirely or replace it with a
non-production-logging guard (e.g., use console.debug or wrap the log in an
environment check like NODE_ENV !== 'production' or use the app's logger at a
debug level) so internal state is not exposed in production.
In @packages/ai-workspace-common/src/queries/infiniteQueries.ts:
- Around line 386-410: The pagination handler in
useListWorkflowExecutionsInfinite has a type/runtime mismatch and unsafe access:
change initialPageParam from the string '1' to the numeric 1, ensure the queryFn
passes a numeric page by coercing pageParam to a number (e.g., Number(pageParam)
with a fallback) when calling listWorkflowExecutions, and make getNextPageParam
defensive by checking that the response has a numeric nextPage (returning
undefined if missing or not a number) so pagination stops safely; update
types/signatures around
useListWorkflowExecutionsInfinite/queryFn/getNextPageParam to reflect page:
number | undefined where needed.
In @packages/ai-workspace-common/src/queries/suspense.ts:
- Around line 850-866: The queryFn inside useListWorkflowExecutionsSuspense
should be converted to an async function and stop using .then(); await
listWorkflowExecutions({ ...clientOptions }) and return its response.data using
optional chaining (e.g., response?.data) instead of force-casting to TData; if
response?.data can be undefined, explicitly handle that case (throw a
descriptive error or return a suitable default) so you don't hide a missing
value by casting. Ensure changes are applied to the queryFn in
useListWorkflowExecutionsSuspense and reference listWorkflowExecutions and
Common.UseListWorkflowExecutionsKeyFn accordingly.
In @packages/ai-workspace-common/src/requests/types.gen.ts:
- Around line 7916-7922: Update ListWorkflowExecutionsResponse so its data shape
includes pagination metadata instead of just an array: replace data?:
Array<WorkflowExecution> with data?: { items: Array<WorkflowExecution>; total?:
number; page?: number; pageSize?: number } (or at minimum include total and
page), and update any code that consumes this type; also make the same change to
the duplicate ListWorkflowExecutionsResponse declaration later in the file so
both paginated endpoints return consistent metadata.
In @packages/request/src/queries/ensureQueryData.ts:
- Around line 328-343: The ensureUseDownloadExportJobResultData function caches
binary payloads without parseAs:'blob', causing JSON parsing failures; either
remove ensureUseDownloadExportJobResultData entirely if it's unused, or stop
using queryClient.ensureQueryData for this endpoint and instead fetch the binary
via the same pattern as use-export-document (call downloadExportJobResult with
parseAs:'blob' directly or via useSuspenseQuery/useQuery that supports parseAs),
and keep ensureUseGetExportJobStatusData unchanged; locate the symbol
ensureUseDownloadExportJobResultData and replace/remove its usages accordingly
so binary responses are never cached via ensureQueryData.
In @packages/request/src/queries/queries.ts:
- Around line 949-965: The hook useDownloadExportJobResult currently uses
useQuery which is inappropriate for binary file downloads; change it to
useMutation (using downloadExportJobResult as the mutationFn) so callers
explicitly trigger downloads and blobs are not auto-refetched or cached, or if
you must keep a query set default options: enabled: false and disable
refetchOnWindowFocus/refetchOnReconnect and document recommended gcTime to avoid
blob memory retention; update the hook signature to return the mutation object
(or accept options for mutation) and replace the queryFn usage that calls
downloadExportJobResult with a mutationFn wired to the same clientOptions.
In @packages/request/src/requests/services.gen.ts:
- Around line 1593-1642: The downloadExportJobResult wrapper currently uses the
default client which will JSON-parse responses and corrupt binary data; update
downloadExportJobResult (and its call to (options?.client ?? client).get with
Options<DownloadExportJobResultData, ThrowOnError>) to handle binary responses
by either: 1) passing a custom fetch/response handler via options that calls
response.blob() (or response.arrayBuffer()) and returns that result, or 2)
switching this endpoint to use the axios-backed client (e.g., client-axios) and
set responseType: 'blob' for the GET to
'/drive/document/export/job/{jobId}/download'; ensure the handler/option is
documented and used only for downloadExportJobResult so other endpoints remain
JSON-parsed.
In @packages/web-core/src/pages/login/index.tsx:
- Around line 42-49: handleEmailAuth (the useCallback defined around lines
150-222) reads needOnboarding (from userProfile) at runtime but does not list
needOnboarding in its dependency array, causing a stale closure; update the
useCallback dependency array for handleEmailAuth to include needOnboarding so
the callback updates when the user’s onboarding flag changes and ensure the
variable used is the same needOnboarding from the useUserStoreShallow selector.
🧹 Nitpick comments (27)
packages/ai-workspace-common/src/components/canvas/copilot-container/index.tsx (2)
14-16: UnnecessaryuseMemofor simple getter.
searchParams.get('source')is a simple string lookup, not an expensive computation. Per coding guidelines,useMemoshould be reserved for expensive computations or complex object creation. This memo adds overhead without benefit since it recomputes wheneversearchParamschanges anyway.♻️ Suggested simplification
- const [searchParams] = useSearchParams(); - const source = useMemo(() => searchParams.get('source'), [searchParams]); - const isOnboarding = source === 'onboarding'; + const [searchParams] = useSearchParams(); + const isOnboarding = searchParams.get('source') === 'onboarding';
82-84: Consider extracting magic number to a constant.The
max-w-[1000px]value could be extracted to a named constant for better maintainability and self-documentation.♻️ Suggested improvement
+const COPILOT_MAX_CONTENT_WIDTH = 1000; + export const CopilotContainer = memo( ({ copilotWidth, setCopilotWidth, maxPanelWidth }: CopilotContainerProps) => { // ... existing code ... return ( <> <div className={cn(/* ... */)} style={{ width: isOnboarding ? '100%' : `${copilotWidth}px` }} > - <div className="max-w-[1000px] h-full mx-auto"> + <div className={`max-w-[${COPILOT_MAX_CONTENT_WIDTH}px] h-full mx-auto`}> <Copilot copilotWidth={copilotWidth} setCopilotWidth={setCopilotWidth} /> </div> </div>Alternatively, use inline style:
- <div className="max-w-[1000px] h-full mx-auto"> + <div className="h-full mx-auto" style={{ maxWidth: COPILOT_MAX_CONTENT_WIDTH }}>packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx (1)
197-204: Add error handling for the settings update mutation.If
updateUserSettingsfails, the local state will showneedOnboarding: falsewhile the server still has it astrue, causing inconsistent behavior on page refresh. Consider adding anonErrorcallback to revert the local state or notify the user.Proposed fix
updateUserSettings({ body: { preferences: { ...userProfile?.preferences, needOnboarding: false, }, }, + }, { + onError: () => { + // Optionally revert local state or show error toast + console.error('Failed to update onboarding settings'); + }, });apps/web/src/routes/redirects.tsx (2)
76-80: Avoid selecting fulluserProfilefrom the store if you only needneedOnboarding(reduces rerenders).Selecting the whole profile increases the chances this route wrapper rerenders frequently (and thus re-evaluates redirects) due to unrelated profile changes.
Proposed refactor
- const { isLogin, isCheckingLoginStatus, userProfile } = useUserStoreShallow((state) => ({ + const { isLogin, isCheckingLoginStatus, needOnboarding } = useUserStoreShallow((state) => ({ isLogin: state.isLogin, isCheckingLoginStatus: state.isCheckingLoginStatus, - userProfile: state.userProfile, + needOnboarding: state.userProfile?.preferences?.needOnboarding ?? false, }));
111-117: Harden onboarding redirect matching and preserve the original destination.String checks can miss valid onboarding subroutes (e.g.
/onboarding/step-2) or trailing slashes; also, redirecting without preserving “where the user was going” can strand users after onboarding unless something else handles it.Proposed fix
if ( needOnboarding && - location.pathname !== '/onboarding' && + !location.pathname.startsWith('/onboarding') && !location.pathname.startsWith('/workflow') ) { - return <Navigate to="/onboarding" replace />; + return <Navigate to="/onboarding" replace state={{ returnTo: location.pathname + location.search }} />; }packages/ai-workspace-common/src/hooks/use-workflow-execution-polling.ts (1)
3-7:onCompletetype tightening is good; consider tighteningonErrortoo.
onCompleteRef.current?.(currentStatus, data)now aligns the callback with the actual query response object rather thanany. For consistency with TS guidelines, consider changingonError?: (error: any) => voidtounknownand narrowing at the call site.Also applies to: 26-34
packages/ai-workspace-common/src/components/voucher/earned-voucher-popup.tsx (1)
1-27: Wrap this pure component withReact.memo(guideline compliance).This component is a pure projection of store state → UI and is a good fit for memoization.
Proposed fix
+import { memo } from 'react'; import { useSubscriptionStoreShallow } from '@refly/stores'; import { VoucherPopup } from './voucher-popup'; /** * Global component for showing voucher popup after earning via workflow run or publish. * This is controlled by the subscription store and can be triggered from anywhere. */ -export const EarnedVoucherPopup = () => { +export const EarnedVoucherPopup = memo(() => { const { earnedVoucherPopupVisible, earnedVoucherResult, hideEarnedVoucherPopup } = useSubscriptionStoreShallow((state) => ({ earnedVoucherPopupVisible: state.earnedVoucherPopupVisible, earnedVoucherResult: state.earnedVoucherResult, hideEarnedVoucherPopup: state.hideEarnedVoucherPopup, })); if (!earnedVoucherResult) return null; return ( <VoucherPopup visible={earnedVoucherPopupVisible} onClose={hideEarnedVoucherPopup} voucherResult={earnedVoucherResult} /> ); -}; +}); + +EarnedVoucherPopup.displayName = 'EarnedVoucherPopup'; export default EarnedVoucherPopup;packages/ai-workspace-common/src/requests/types.gen.ts (1)
11513-11616: Naming/aliasing from codegen (ListWorkflowExecutionsResponse2,ListWorkflowExecutionsError = unknown).Since this is auto-generated: if
ListWorkflowExecutionsResponse2andListWorkflowExecutionsError = unknownare unintended (or you want typed errors), it’s better to adjust the OpenAPI / codegen config than patch this file manually.packages/ai-workspace-common/src/queries/suspense.ts (1)
79-82: Type-only imports: considerimport typefor new request types (codegen/template-level).
ListWorkflowExecutionsData/ListWorkflowExecutionsErrorare types; if your TS config enforces type-only imports, update the generator/template to emitimport type { ... }consistently (doing it manually here will diverge from regeneration).Also applies to: 212-217
packages/openapi-schema/schema.yml (3)
2948-2994: Clarify pagination contract + tighten query param typing for/workflow/list.You accept
page/pageSizebut returndata: WorkflowExecution[]without any pagination metadata; that’s fine if you rely on “fetch until empty”, but it’s ambiguous for API consumers (and for generated clients) given you also have anaftercursor-like filter. Consider either:
- page-based: return
{ total, page, pageSize, items }, or- cursor-based: replace
page/pageSizewithlimit+ returnnextCursor(and keepafter).Also,
page,pageSize, andaftershould beinteger(with bounds) rather thannumberto avoid float types in generated SDKs.Proposed YAML adjustments (typing/bounds only)
/workflow/list: get: @@ parameters: @@ - name: after in: query description: Creation time after filter (unix timestamp in milliseconds) schema: - type: number + type: integer + format: int64 - name: page in: query description: Page number schema: - type: number + type: integer + minimum: 1 default: 1 - name: pageSize in: query description: Page size schema: - type: number + type: integer + minimum: 1 + maximum: 100 default: 10Please confirm whether
/workflow/listis intended to be page-based or cursor-based, and whether it should declare asecurity:requirement like other authenticated endpoints in this spec.
7476-7479: Add an explicit default (and considerreadOnly) forUserPreferences.needOnboarding.Without a default, clients may see
undefinedvsfalseinconsistently depending on serializer behavior.Proposed YAML tweak
needOnboarding: type: boolean description: Whether this user needs onboarding + default: false + readOnly: trueIf the frontend is expected to update this flag, drop
readOnly: true; otherwise, keep it to reflect server-derived state.
12636-12645:ListWorkflowExecutionsResponsematches the new endpoint, but re-check metadata needs.If
/workflow/liststays page-based, you’ll likely wantdatato be an object (e.g.,{ total, page, pageSize, items }) rather than an array, to avoid “guessing” whether more pages exist.Confirm whether returning only
WorkflowExecution[]is sufficient for your UI/SDK usage, or if you need pagination metadata in the response body.packages/ai-workspace-common/src/hooks/use-initialize-workflow.ts (2)
58-72: Consider adding error handling for voucher polling.The polling loop calls
getAvailableVouchers()without error handling. If the API call fails, the loop will continue silently without logging or recovery.♻️ Suggested improvement
for (let attempts = 0; attempts < 10; attempts++) { - const { data: voucherData } = await getClient().getAvailableVouchers(); + const { data: voucherData, error: voucherError } = await getClient().getAvailableVouchers(); + if (voucherError) { + console.warn('Failed to fetch available vouchers:', voucherError); + break; + } const bestVoucher = voucherData?.data?.bestVoucher; if (bestVoucher) {
91-91: RemovecanvasIdfrom dependency array.
canvasIdis not used within thehandleCompletecallback body. Including it in the dependency array causes unnecessary re-memoization whencanvasIdchanges.♻️ Suggested fix
- [t, canvasId, showEarnedVoucherPopup], + [t, showEarnedVoucherPopup],apps/api/src/modules/workflow/workflow.controller.ts (2)
93-113: Add OpenAPI documentation for the new endpoint.Per coding guidelines, API controllers should document endpoints with OpenAPI specifications. Consider adding decorators like
@ApiOperation,@ApiQuery, and@ApiResponse.♻️ Example OpenAPI decorators
@ApiOperation({ summary: 'List workflow executions' }) @ApiQuery({ name: 'canvasId', required: false, description: 'Filter by canvas ID' }) @ApiQuery({ name: 'status', required: false, enum: WorkflowExecutionStatus }) @ApiQuery({ name: 'after', required: false, type: Number, description: 'Unix timestamp filter' }) @ApiQuery({ name: 'page', required: false, default: 1 }) @ApiQuery({ name: 'pageSize', required: false, default: 10 }) @ApiQuery({ name: 'order', required: false, default: 'creationDesc' }) @ApiResponse({ status: 200, type: ListWorkflowExecutionsResponse })
101-101: Consider adding bounds validation forpageSize.Without validation, clients could request very large page sizes (e.g.,
pageSize=100000), potentially causing performance issues or excessive memory usage.♻️ Suggested approach
Consider using a custom pipe or adding validation in the service layer:
@Query('pageSize', new DefaultValuePipe(10), ParseIntPipe) pageSize = 10, // Then validate in service or add: Math.min(pageSize, 100)Alternatively, use class-validator with a DTO that has
@Max(100)constraint.apps/api/src/modules/voucher/voucher.service.ts (1)
233-233: Naming inconsistency:templateIdvssourceId.The
recordPopupTriggermethod at line 340 still usestemplateIdas its parameter name, but it's now being called withsourceId(line 233). Consider renaming the parameter for consistency with the new terminology.♻️ Suggested fix
- async recordPopupTrigger(uid: string, templateId: string, voucherId?: string): Promise<void> { + async recordPopupTrigger(uid: string, sourceId: string, voucherId?: string): Promise<void> { const today = getYYYYMMDD(new Date()); await this.prisma.voucherPopupLog.create({ data: { uid, - templateId, + sourceId, popupDate: today, voucherId, createdAt: new Date(), }, }); }Note: This would also require a database schema update if
templateIdis a column name.Also applies to: 340-351
packages/ai-workspace-common/src/queries/common.ts (1)
653-664: Consider typingclientOptionsinUseListWorkflowExecutionsKeyFn(instead ofOptions<unknown, true>).This improves type safety at call sites and helps prevent accidentally keying on irrelevant option shapes.
Proposed patch
export const UseListWorkflowExecutionsKeyFn = ( - clientOptions: Options<unknown, true> = {}, + clientOptions: Options<ListWorkflowExecutionsData, true> = {}, queryKey?: Array<unknown>, ) => [useListWorkflowExecutionsKey, ...(queryKey ?? [clientOptions])];packages/request/src/queries/queries.ts (1)
3181-3302: Confirm the “get/list” endpoints are intended as mutations (and considervoidvariables for optional options).Several read-ish hooks are exposed as
useMutation(likely fine if these are POST-with-body endpoints), but please double-checkgetAvailableToolsin particular is not a simple GET that would benefit fromuseQuery.Also, for endpoints whose
optionsparam is optional, consider making the mutation variables optional (so callers canmutate()without passing{}).packages/request/src/queries/prefetch.ts (1)
328-343: Prefetchers are fine; be cautious prefetching download payloads (may be large) unless you really need it.Also applies to: 472-479
packages/openapi-schema/src/types.gen.ts (1)
828-1164: Tighten a few schedule-record typings in the OpenAPI schema (enums + filter parity)
WorkflowScheduleRecord.statusincludes'skipped', butListAllScheduleRecordsRequest.statusexcludes it. If records can be skipped and you want parity, the filter enum likely should include'skipped'too.RetryScheduleRecordResponse.data.status?: stringloses the enum benefit; consider modeling it as the same status union (or a narrower enum) in the OpenAPI spec.
These are schema-level improvements; I wouldn’t hand-edit the generated file.packages/ai-workspace-common/src/hooks/canvas/use-create-canvas.ts (1)
7-14:initialPromptoption addition looks good; consider trimming before use.If this is user-provided, consider
options.initialPrompt.trim()(and treat whitespace-only as empty) to avoid seeding a “blank” pending prompt.packages/openapi-schema/src/schemas.gen.ts (5)
975-1039: Consider addingrequiredfields (and clarifying null-vs-omitted) forWorkflowScheduleSchema.
Right now everything is optional because there’s norequiredarray; if the API always returnsscheduleId,uid,canvasId,createdAt,updatedAt, consumers + validators won’t be able to rely on it. Also, you markednextRunAt/lastRunAt/deletedAtasnullable: true; confirm whether the API returnsnullvs omits these fields, and encode that consistently.Proposed schema tightening (should be applied in the OpenAPI source, since this file is generated)
export const WorkflowScheduleSchema = { type: 'object', + required: ['scheduleId', 'uid', 'canvasId', 'name', 'isEnabled', 'cronExpression', 'scheduleConfig', 'createdAt', 'updatedAt'], properties: { scheduleId: { type: 'string', description: 'Schedule ID', },
1248-1339: Schedule record schema looks reasonable; consider aligning status/filter enums.
WorkflowScheduleRecordSchema.statusincludes'skipped', butListAllScheduleRecordsRequestSchema.status(Line 1409-1413) does not. If “skipped” is a real terminal state, the filter enum should likely include it too.
1341-1478: Pagination request/response: add basic bounds and required fields for stronger contracts.
Forpage/pageSize, considerminimum: 1(and possibly a max forpageSize). For the paginateddataobjects, considerrequiredarrays so SDKs/validators don’t treat everything as optional.
1479-1567:GetAvailableToolsResponseSchemaitems should likely requireidandname.
As written, a tool item can be{}. If the UI depends on these, tighten the schema.Proposed change (apply in OpenAPI source)
type: 'object', + required: ['id', 'name'], properties: { id: { type: 'string', description: 'Tool ID', }, name: { type: 'string', description: 'Tool name', }, },
1568-1652: Manual trigger / retry responses: consider making key fields required (and constrainingstatus).
TriggerScheduleManuallyResponseSchema.dataandRetryScheduleRecordResponseSchema.datadefine important identifiers, but nothing is required andstatusis unconstrained. If these are relied upon by clients, tighten to required + enum.
| // Check if it's the first execution today to trigger voucher | ||
| try { | ||
| const todayStart = new Date(); | ||
| todayStart.setHours(0, 0, 0, 0); | ||
|
|
||
| const executionsToday = await this.prisma.workflowExecution.count({ | ||
| where: { | ||
| uid: user.uid, | ||
| createdAt: { | ||
| gte: todayStart, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| if (executionsToday === 0 && canvasData) { | ||
| this.voucherService | ||
| .handleCreateVoucherFromSource( | ||
| user, | ||
| { title: canvasData?.title, nodes: canvasData?.nodes }, | ||
| finalVariables, | ||
| 'run_workflow', | ||
| canvasId, | ||
| ) | ||
| .catch((err) => { | ||
| this.logger.error(`Failed to handle template publish for voucher: ${err.message}`); | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| this.logger.error(`Failed to check today's executions for voucher: ${err.message}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Voucher “first execution today” gate is not concurrency-safe + error handling should guard unknown errors. Two parallel initializations can both observe executionsToday === 0 and create duplicate vouchers; also err.message is unsafe and violates the “optional chaining” guideline.
Proposed fix (make the daily gate atomic-ish + harden error logging)
- // Check if it's the first execution today to trigger voucher
+ // Check if it's the first execution today to trigger voucher
try {
- const todayStart = new Date();
- todayStart.setHours(0, 0, 0, 0);
+ const todayStart = new Date();
+ todayStart.setUTCHours(0, 0, 0, 0);
+
+ // Best-effort distributed guard to reduce duplicate vouchers under concurrency.
+ const voucherLockKey = `voucher:run_workflow:${user.uid}:${todayStart.toISOString().slice(0, 10)}`;
+ const releaseVoucherLock = await this.redis.acquireLock(voucherLockKey, 10_000);
+ if (!releaseVoucherLock) return;
const executionsToday = await this.prisma.workflowExecution.count({
where: {
uid: user.uid,
createdAt: {
gte: todayStart,
},
},
});
if (executionsToday === 0 && canvasData) {
this.voucherService
.handleCreateVoucherFromSource(
user,
{ title: canvasData?.title, nodes: canvasData?.nodes },
finalVariables,
'run_workflow',
canvasId,
)
- .catch((err) => {
- this.logger.error(`Failed to handle template publish for voucher: ${err.message}`);
+ .catch((err: unknown) => {
+ this.logger.error(
+ `Failed to handle run_workflow voucher: ${(err as Error)?.message ?? String(err)}`,
+ );
});
}
+
+ await releaseVoucherLock();
} catch (err) {
- this.logger.error(`Failed to check today's executions for voucher: ${err.message}`);
+ this.logger.error(
+ `Failed to check today's executions for voucher: ${(err as Error)?.message ?? String(err)}`,
+ );
}🤖 Prompt for AI Agents
In @apps/api/src/modules/workflow/workflow.service.ts around lines 172 - 202,
The current non-atomic check using prisma.workflowExecution.count can race and
allow duplicate vouchers; replace the read-then-act with an atomic approach:
either enforce a DB uniqueness constraint for "user/date" on the voucher (or a
dedicated daily_gate table) and attempt to create the voucher inside a
transaction (using prisma.$transaction) or perform a conditional upsert so only
one creation succeeds; move the call to
voucherService.handleCreateVoucherFromSource into that transactional flow and
handle failures by checking the Prisma unique-constraint error rather than
relying on count to prevent duplicates; also harden logging by using
err?.message or JSON.stringify(err) when logging inside the catch blocks around
handleCreateVoucherFromSource and the outer try so you never dereference
undefined (referencing prisma.workflowExecution.count,
voucherService.handleCreateVoucherFromSource, prisma.$transaction/upsert and the
catch blocks).
| /** | ||
| * List workflow executions with pagination | ||
| * @param user - The user requesting the workflow details | ||
| * @param params - Pagination and filter parameters | ||
| * @returns Promise<WorkflowExecution[]> - Paginated workflow execution details | ||
| */ | ||
| async listWorkflowExecutions(user: User, params: ListWorkflowExecutionsData['query']) { | ||
| const { canvasId, status, after, page = 1, pageSize = 10, order = 'creationDesc' } = params; | ||
| const skip = (page - 1) * pageSize; | ||
|
|
||
| // Build where clause | ||
| const whereClause: Prisma.WorkflowExecutionWhereInput = { uid: user.uid }; | ||
| if (canvasId) { | ||
| whereClause.canvasId = canvasId; | ||
| } | ||
| if (status) { | ||
| whereClause.status = status; | ||
| } | ||
| if (after) { | ||
| // after is unix timestamp in milliseconds | ||
| whereClause.createdAt = { | ||
| gt: new Date(after), | ||
| }; | ||
| } | ||
|
|
||
| // Build order by | ||
| const orderBy: Prisma.WorkflowExecutionOrderByWithRelationInput = {}; | ||
| if (order === 'creationAsc') { | ||
| orderBy.createdAt = 'asc'; | ||
| } else if (order === 'creationDesc') { | ||
| orderBy.createdAt = 'desc'; | ||
| } else if (order === 'updationAsc') { | ||
| orderBy.updatedAt = 'asc'; | ||
| } else if (order === 'updationDesc') { | ||
| orderBy.updatedAt = 'desc'; | ||
| } else { | ||
| orderBy.createdAt = 'desc'; | ||
| } | ||
|
|
||
| // Get workflow executions with pagination | ||
| return this.prisma.workflowExecution.findMany({ | ||
| where: whereClause, | ||
| orderBy, | ||
| skip, | ||
| take: pageSize, | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
listWorkflowExecutions can throw if params is undefined; add a default and clamp pagination. ListWorkflowExecutionsData['query'] can be undefined, so destructuring will crash; also consider guarding negative/zero page/pageSize.
Proposed fix
- async listWorkflowExecutions(user: User, params: ListWorkflowExecutionsData['query']) {
- const { canvasId, status, after, page = 1, pageSize = 10, order = 'creationDesc' } = params;
- const skip = (page - 1) * pageSize;
+ async listWorkflowExecutions(user: User, params: ListWorkflowExecutionsData['query'] = {}) {
+ const { canvasId, status, after, order = 'creationDesc' } = params ?? {};
+ const page = Math.max(1, params?.page ?? 1);
+ const pageSize = Math.min(100, Math.max(1, params?.pageSize ?? 10));
+ const skip = (page - 1) * pageSize;🤖 Prompt for AI Agents
In @apps/api/src/modules/workflow/workflow.service.ts around lines 1046 - 1093,
The function listWorkflowExecutions can throw when params is undefined and
allows invalid pagination values; change its signature/implementation to default
params to an empty object (e.g., params = {}) before destructuring, and clamp
page and pageSize to safe ranges (ensure page = Math.max(1, Number(page) || 1)
and pageSize = Math.min(MAX_PAGE_SIZE, Math.max(1, Number(pageSize) ||
DEFAULT_PAGE_SIZE))) so skip is computed from validated values; also ensure
after is parsed safely if present. Reference: listWorkflowExecutions, params,
page, pageSize, skip, after, whereClause.
| const needOnboarding = userProfile?.preferences?.needOnboarding; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normalize needOnboarding to a strict boolean (and default to false).
Right now it can be undefined (or a non-boolean if backend/store typing drifts), which makes redirect behavior harder to reason about.
Proposed fix
- const needOnboarding = userProfile?.preferences?.needOnboarding;
+ const needOnboarding = userProfile?.preferences?.needOnboarding === 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.
| const needOnboarding = userProfile?.preferences?.needOnboarding; | |
| const needOnboarding = userProfile?.preferences?.needOnboarding === true; |
🤖 Prompt for AI Agents
In @apps/web/src/routes/redirects.tsx around lines 84 - 85, Normalize
needOnboarding to a strict boolean by coercing the
possibly-undefined/non-boolean value from userProfile.preferences into
true/false; replace the current declaration of needOnboarding with a boolean
coercion like using Boolean(userProfile?.preferences?.needOnboarding) or
!!userProfile?.preferences?.needOnboarding so it always yields false by default
and is safe for redirect logic in redirects.tsx.
| const startNodes = currentNodes.filter((node) => node.type === 'start'); | ||
| const skillNodes = currentNodes.filter((node) => node.type === 'skillResponse'); | ||
|
|
||
| console.log('currentNodes', currentNodes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug console.log before merging.
This debug statement should be removed as it exposes internal state to the browser console in production.
Proposed fix
- console.log('currentNodes', currentNodes);📝 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.
| console.log('currentNodes', currentNodes); |
🤖 Prompt for AI Agents
In
@packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx
at line 111, Remove the debug console.log call that prints currentNodes in
copilot-message.tsx (the line with console.log('currentNodes', currentNodes));
either delete that statement entirely or replace it with a
non-production-logging guard (e.g., use console.debug or wrap the log in an
environment check like NODE_ENV !== 'production' or use the app's logger at a
debug level) so internal state is not exposed in production.
| export const useListWorkflowExecutionsInfinite = < | ||
| TData = InfiniteData<Common.ListWorkflowExecutionsDefaultResponse>, | ||
| TError = ListWorkflowExecutionsError, | ||
| TQueryKey extends Array<unknown> = unknown[], | ||
| >( | ||
| clientOptions: Options<ListWorkflowExecutionsData, true> = {}, | ||
| queryKey?: TQueryKey, | ||
| options?: Omit<UseInfiniteQueryOptions<TData, TError>, 'queryKey' | 'queryFn'>, | ||
| ) => | ||
| useInfiniteQuery({ | ||
| queryKey: Common.UseListWorkflowExecutionsKeyFn(clientOptions, queryKey), | ||
| queryFn: ({ pageParam }) => | ||
| listWorkflowExecutions({ | ||
| ...clientOptions, | ||
| query: { ...clientOptions.query, page: pageParam as number }, | ||
| }).then((response) => response.data as TData) as TData, | ||
| initialPageParam: '1', | ||
| getNextPageParam: (response) => | ||
| ( | ||
| response as { | ||
| nextPage: number; | ||
| } | ||
| ).nextPage, | ||
| ...options, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and understand its structure
fd -t f infiniteQueries.tsRepository: refly-ai/refly
Length of output: 165
🏁 Script executed:
# Look for the Common type definitions to understand ListWorkflowExecutionsDefaultResponse
rg "ListWorkflowExecutionsDefaultResponse" --type ts -A 3 -B 3Repository: refly-ai/refly
Length of output: 5416
🏁 Script executed:
# Find where listWorkflowExecutions is defined
rg "listWorkflowExecutions" --type ts -A 5 -B 2Repository: refly-ai/refly
Length of output: 21884
🏁 Script executed:
# Look for the backend endpoint to understand actual response shape
fd -t f "workflow" | grep -E "\.(ts|js|json)$"Repository: refly-ai/refly
Length of output: 1311
🏁 Script executed:
# Check other infinite query implementations for comparison
rg "useInfiniteQuery" packages/ai-workspace-common/src/queries/infiniteQueries.ts -A 20 | head -100Repository: refly-ai/refly
Length of output: 2970
🏁 Script executed:
# Find the response type definitions
rg "ListWorkflowExecutionsResponse2" --type ts -A 10 -B 2Repository: refly-ai/refly
Length of output: 8307
🏁 Script executed:
# Look for the actual API response DTO/schema
fd -t f "workflow.dto.ts"Repository: refly-ai/refly
Length of output: 103
🏁 Script executed:
# Check what type is actually returned from the backend
rg "ListWorkflowExecutionsResponse" packages/ai-workspace-common/src/requests/services.gen.ts -A 5 -B 2Repository: refly-ai/refly
Length of output: 481
🏁 Script executed:
# Find the actual ListWorkflowExecutionsResponse type definition
rg "export type ListWorkflowExecutionsResponse" --type ts -A 15Repository: refly-ai/refly
Length of output: 6512
🏁 Script executed:
# Also check the workflow.dto.ts for the backend response structure
cat -n apps/api/src/modules/workflow/workflow.dto.ts | head -100Repository: refly-ai/refly
Length of output: 2183
🏁 Script executed:
# Find BaseResponse type definition to see if nextPage is included
rg "export type BaseResponse" --type ts -A 10Repository: refly-ai/refly
Length of output: 4981
🏁 Script executed:
# Check the backend service's actual return to see if nextPage is being returned
cat -n apps/api/src/modules/workflow/workflow.service.ts | grep -A 30 "listWorkflowExecutions"Repository: refly-ai/refly
Length of output: 1314
🏁 Script executed:
# Get more of the listWorkflowExecutions method to see what it returns
sed -n '1052,1120p' apps/api/src/modules/workflow/workflow.service.tsRepository: refly-ai/refly
Length of output: 2200
🏁 Script executed:
# Also check if there's a pagination wrapper or metadata response
rg "nextPage|pagination|pageCount" apps/api/src/modules/workflow/workflow.service.ts -B 2 -A 2Repository: refly-ai/refly
Length of output: 359
🏁 Script executed:
# Let's check what the controller returns to confirm the response structure
cat -n apps/api/src/modules/workflow/workflow.controller.ts | grep -A 15 "listWorkflowExecutions"Repository: refly-ai/refly
Length of output: 1173
🏁 Script executed:
# Let's check what buildSuccessResponse does
rg "buildSuccessResponse" apps/api --type ts -A 5 -B 2Repository: refly-ai/refly
Length of output: 50371
Fix pagination type safety and guard nextPage access.
The code has two issues:
initialPageParam: '1'(string) vspageParam as numbercreates a type mismatch;as numberis a TS assertion that doesn't coerce at runtime, so the page parameter sent could be a string.getNextPageParamassumesresponse.nextPageexists without checking. The backend's/workflow/listendpoint returns only{ success: true, data: Array<WorkflowExecution> }with nonextPagefield, which means the assertion will fail. This stops pagination (correct behavior), but should be explicitly guarded.
Apply the proposed patch to ensure consistent page parameter types and safe property access:
Proposed fix
export const useListWorkflowExecutionsInfinite = <
TData = InfiniteData<Common.ListWorkflowExecutionsDefaultResponse>,
TError = ListWorkflowExecutionsError,
TQueryKey extends Array<unknown> = unknown[],
>(
clientOptions: Options<ListWorkflowExecutionsData, true> = {},
queryKey?: TQueryKey,
options?: Omit<UseInfiniteQueryOptions<TData, TError>, 'queryKey' | 'queryFn'>,
) =>
useInfiniteQuery({
queryKey: Common.UseListWorkflowExecutionsKeyFn(clientOptions, queryKey),
- queryFn: ({ pageParam }) =>
- listWorkflowExecutions({
- ...clientOptions,
- query: { ...clientOptions.query, page: pageParam as number },
- }).then((response) => response.data as TData) as TData,
- initialPageParam: '1',
- getNextPageParam: (response) =>
- (
- response as {
- nextPage: number;
- }
- ).nextPage,
+ queryFn: ({ pageParam }) => {
+ const page = typeof pageParam === 'number' ? pageParam : Number(pageParam ?? 1);
+ return listWorkflowExecutions({
+ ...clientOptions,
+ query: { ...(clientOptions?.query ?? {}), page },
+ }).then((response) => response.data as TData) as TData;
+ },
+ initialPageParam: 1,
+ getNextPageParam: (response) => {
+ const nextPage = (response as { nextPage?: number } | undefined)?.nextPage;
+ return typeof nextPage === 'number' ? nextPage : undefined;
+ },
...options,
});🤖 Prompt for AI Agents
In @packages/ai-workspace-common/src/queries/infiniteQueries.ts around lines 386
- 410, The pagination handler in useListWorkflowExecutionsInfinite has a
type/runtime mismatch and unsafe access: change initialPageParam from the string
'1' to the numeric 1, ensure the queryFn passes a numeric page by coercing
pageParam to a number (e.g., Number(pageParam) with a fallback) when calling
listWorkflowExecutions, and make getNextPageParam defensive by checking that the
response has a numeric nextPage (returning undefined if missing or not a number)
so pagination stops safely; update types/signatures around
useListWorkflowExecutionsInfinite/queryFn/getNextPageParam to reflect page:
number | undefined where needed.
| export const ensureUseGetExportJobStatusData = ( | ||
| queryClient: QueryClient, | ||
| clientOptions: Options<GetExportJobStatusData, true>, | ||
| ) => | ||
| queryClient.ensureQueryData({ | ||
| queryKey: Common.UseGetExportJobStatusKeyFn(clientOptions), | ||
| queryFn: () => getExportJobStatus({ ...clientOptions }).then((response) => response.data), | ||
| }); | ||
| export const ensureUseDownloadExportJobResultData = ( | ||
| queryClient: QueryClient, | ||
| clientOptions: Options<DownloadExportJobResultData, true>, | ||
| ) => | ||
| queryClient.ensureQueryData({ | ||
| queryKey: Common.UseDownloadExportJobResultKeyFn(clientOptions), | ||
| queryFn: () => downloadExportJobResult({ ...clientOptions }).then((response) => response.data), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, find the downloadExportJobResult implementation
rg "downloadExportJobResult" -t ts -t tsx -t js -A 5Repository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Check the file structure to understand the context
find . -name "ensureQueryData.ts" -type fRepository: refly-ai/refly
Length of output: 169
🏁 Script executed:
# Look for @hey-api configuration
rg "@hey-api" -t ts -t tsx -t json | head -20Repository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Search for openapi-react-query-codegen configuration
rg "openapi-react-query-codegen" -t json -t ts -A 3Repository: refly-ai/refly
Length of output: 5874
🏁 Script executed:
# Search for downloadExportJobResult definition and usage
rg "downloadExportJobResult" --max-count 50Repository: refly-ai/refly
Length of output: 3018
🏁 Script executed:
# Look at the ensureQueryData.ts file to see the context
head -350 ./packages/request/src/queries/ensureQueryData.ts | tail -50Repository: refly-ai/refly
Length of output: 2190
🏁 Script executed:
# Also check the queries.ts file where downloadExportJobResult is likely defined
rg "downloadExportJobResult" ./packages/request/src/queries/queries.ts -A 10Repository: refly-ai/refly
Length of output: 628
🏁 Script executed:
# View the downloadExportJobResult implementation
rg "export const downloadExportJobResult" ./packages/request/src/requests/services.gen.ts -A 20Repository: refly-ai/refly
Length of output: 654
🏁 Script executed:
# Also check the schema to understand the endpoint definition
rg "downloadExportJobResult:" ./packages/openapi-schema/schema.yml -B 5 -A 15Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Check the use-export-document hook to see actual usage
cat -n ./packages/ai-workspace-common/src/hooks/use-export-document.tsRepository: refly-ai/refly
Length of output: 6110
🏁 Script executed:
# Check if ensureUseDownloadExportJobResultData is actually used anywhere
rg "ensureUseDownloadExportJobResultData" --max-count 20Repository: refly-ai/refly
Length of output: 273
🏁 Script executed:
# Check the Options type definition to see if parseAs can be passed
rg "export.*Options" ./packages/request/src/requests/services.gen.ts -B 2 -A 10 | head -40Repository: refly-ai/refly
Length of output: 473
🏁 Script executed:
# Look at the OpenAPI schema to see the endpoint definition
rg "downloadExportJobResult:" ./packages/openapi-schema/schema.yml -B 3 -A 10Repository: refly-ai/refly
Length of output: 40
🌐 Web query:
@hey-api/client-fetch parseAs blob option Options type parameters binary downloads
💡 Result:
Short answer: use the client method options parseAs: "blob". The client supports parseAs = "json" | "text" | "arrayBuffer" | "blob" | "stream", and when you pass parseAs: "blob" the response body is returned as a Blob (or the platform equivalent). Example:
ts
const res = await client.GET("/file", { parseAs: "blob" });
const blob: Blob = res.data;
References: Hey API / openapi-fetch docs for parseAs options and Fetch client docs. [1][2]
Sources
- OpenAPI TypeScript — Fetch options (parseAs list). [1]
- Hey API Fetch client docs (usage/examples). [2]
Remove ensureUseDownloadExportJobResultData or reconfigure for binary downloads.
This function caches binary file payloads (pdf/docx) in QueryClient without configuring parseAs: 'blob'. Without this option, @hey-api/client-fetch defaults to JSON parsing, which will fail on binary content. The correct pattern (shown in use-export-document.ts) calls the endpoint with parseAs: 'blob' to return a Blob. Since ensureQueryData does not expose parseAs configuration through its Options parameter, binary downloads should not be cached this way. Either remove the function if unused, or use useSuspenseQuery/direct API calls for this endpoint instead.
🤖 Prompt for AI Agents
In @packages/request/src/queries/ensureQueryData.ts around lines 328 - 343, The
ensureUseDownloadExportJobResultData function caches binary payloads without
parseAs:'blob', causing JSON parsing failures; either remove
ensureUseDownloadExportJobResultData entirely if it's unused, or stop using
queryClient.ensureQueryData for this endpoint and instead fetch the binary via
the same pattern as use-export-document (call downloadExportJobResult with
parseAs:'blob' directly or via useSuspenseQuery/useQuery that supports parseAs),
and keep ensureUseGetExportJobStatusData unchanged; locate the symbol
ensureUseDownloadExportJobResultData and replace/remove its usages accordingly
so binary responses are never cached via ensureQueryData.
| export const useDownloadExportJobResult = < | ||
| TData = Common.DownloadExportJobResultDefaultResponse, | ||
| TError = DownloadExportJobResultError, | ||
| TQueryKey extends Array<unknown> = unknown[], | ||
| >( | ||
| clientOptions: Options<DownloadExportJobResultData, true>, | ||
| queryKey?: TQueryKey, | ||
| options?: Omit<UseQueryOptions<TData, TError>, 'queryKey' | 'queryFn'>, | ||
| ) => | ||
| useQuery<TData, TError>({ | ||
| queryKey: Common.UseDownloadExportJobResultKeyFn(clientOptions, queryKey), | ||
| queryFn: () => | ||
| downloadExportJobResult({ ...clientOptions }).then( | ||
| (response) => response.data as TData, | ||
| ) as TData, | ||
| ...options, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the downloadExportJobResult function definition
rg -A 10 "function downloadExportJobResult|const downloadExportJobResult|export.*downloadExportJobResult" --type ts --type jsRepository: refly-ai/refly
Length of output: 2780
🏁 Script executed:
# Look for the response type definition
rg "DownloadExportJobResult" --type ts --type js -B 2 -A 5Repository: refly-ai/refly
Length of output: 28210
🏁 Script executed:
# Check the endpoint configuration
rg "downloadExportJobResult|/export.*result|download.*export" --type ts --type js -i | head -20Repository: refly-ai/refly
Length of output: 1921
🏁 Script executed:
# Verify what @hey-api/client-fetch returns for this endpoint
fd -e ts -e js | xargs grep -l "@hey-api/client-fetch" | head -5Repository: refly-ai/refly
Length of output: 335
🌐 Web query:
TanStack React Query v5 best practices file download useQuery vs useMutation
💡 Result:
Short answer
- Use useMutation for user‑initiated file downloads (button clicks, form submits) because downloads are side‑effects/actions, not cached background queries. [1][2]
- Use useQuery only when the file is a cacheable resource you want automatically fetched/invalidated, preloaded, or used by multiple components (rare for real file downloads). [2]
Why
- TanStack docs: queries = fetching/cached data; mutations = create/update/delete or server side‑effects (user actions). Downloads triggered by a user are side‑effects → useMutation. [1][2]
Practical tips (v5 object API)
- v5 uses the object-signature: useMutation({ mutationFn, ... }). [2]
- Request the file as a Blob (fetch/axios responseType: 'blob'), then create a download link with URL.createObjectURL or stream to disk if using server streaming. Revoke the object URL after use to avoid leaks. Use Response.blob() if using fetch. [3]
Example (React + axios, v5)
- Minimal pattern:
const downloadMutation = useMutation({
mutationFn: async (params) => {
const res = await axios.get('/api/download', {
params,
responseType: 'blob',
})
return { blob: res.data, filename: getFilenameFromHeaders(res.headers) }
},
onSuccess: ({ blob, filename }) => {
const url = URL.createObjectURL(blob)
const a = document.createElement('a')
a.href = url
a.download = filename ?? 'file'
document.body.appendChild(a)
a.click()
a.remove()
URL.revokeObjectURL(url)
},
onError: (err) => { /* show error UI */ },
})
<button onClick={() => downloadMutation.mutate({ id: 123 })}>
Download
- If you use fetch directly: await response.blob() and proceed similarly. Remember opaque responses (CORS) can break blob access — ensure correct CORS headers. [3]
Additional best practices
- Provide progress UI: track mutation.isPending / useMutationState or Axios progress callbacks.
- Scope/serialize concurrent downloads if needed via mutation scope in v5.
- Don’t store large blobs in the query cache; keep blobs transient (download via mutation).
- Extract filename from Content-Disposition header when available.
Sources
- TanStack Query — Mutations guide (useMutation semantics). [1]
- TanStack Query v5 docs — object-signature / migration notes. [2]
- MDN — URL.createObjectURL(), Response.blob() and blob URL usage. [3]
🌐 Web query:
@hey-api/client-fetch binary file download response handling
💡 Result:
Short answer — use the Fetch Response that the generated Fetch client exposes and call arrayBuffer()/blob() (or stream() in Node) on it.
Example (browser):
-
call the generated SDK function, read the original Response, then use arrayBuffer() or blob():
const res = await client.myDownloadEndpoint(); // generated call
const fetchResponse = res.response ?? res.raw ?? res; // generated clients expose the original Response under a field (response/raw) — see docs.(heyapi.dev)
const buffer = await fetchResponse.arrayBuffer(); // ArrayBuffer for bytes
// or:
const blob = await fetchResponse.blob(); // Blob for large media
Example (Node — streaming, efficient for large files):
const res = await client.myDownloadEndpoint();
const fetchResponse = res.response ?? res.raw ?? res;
const stream = fetchResponse.body; // ReadableStream (Web) / use stream() helper if client exposes it.
// convert to Node stream and pipe to file:
import { pipeline } from 'stream/promises';
import { createWriteStream } from 'fs';
const nodeStream = require('stream').Readable.fromWeb(stream);
await pipeline(nodeStream, createWriteStream('file.bin')).(github.com)
Notes and gotchas
- Use arrayBuffer() when you need raw bytes; use blob() for browser-file APIs; use streaming (body/stream()) in Node for large files to avoid high memory use.(cnblogs.com)
- The generated Fetch client explicitly exposes the native fetch Response (so you can call these methods). If you don’t see res.response/res.raw, inspect the object returned by your generated client — it includes the original request/response per the docs.(heyapi.dev)
If you want, tell me which generated client function you’re calling (paste the SDK call) and I’ll give an exact snippet matching its returned shape.
Citations:
- 1: https://heyapi.dev/openapi-ts/clients/fetch.html?utm_source=openai
- 2: https://github.com/anduril/lattice-sdk-javascript?utm_source=openai
- 3: https://www.cnblogs.com/keatkeat/p/16300650.html?utm_source=openai
- 4: https://heyapi.dev/openapi-ts/clients/fetch.html?utm_source=openai
Use useMutation instead of useQuery for file downloads.
This endpoint returns binary data (Blob | File), and TanStack Query best practices dictate that downloads should be mutations, not queries. Queries auto-refetch on window focus/reconnect and cache in memory—both undesirable for file downloads. If useQuery is intentional, disable auto-fetching with enabled: false by default so callers must explicitly trigger the download, and document recommended gcTime settings to prevent unnecessary blob caching.
🤖 Prompt for AI Agents
In @packages/request/src/queries/queries.ts around lines 949 - 965, The hook
useDownloadExportJobResult currently uses useQuery which is inappropriate for
binary file downloads; change it to useMutation (using downloadExportJobResult
as the mutationFn) so callers explicitly trigger downloads and blobs are not
auto-refetched or cached, or if you must keep a query set default options:
enabled: false and disable refetchOnWindowFocus/refetchOnReconnect and document
recommended gcTime to avoid blob memory retention; update the hook signature to
return the mutation object (or accept options for mutation) and replace the
queryFn usage that calls downloadExportJobResult with a mutationFn wired to the
same clientOptions.
| /** | ||
| * Start async document export | ||
| * Start an asynchronous document export job using Lambda. Use this for binary files (docx, pdf) that need format conversion. | ||
| */ | ||
| export const startExportJob = <ThrowOnError extends boolean = false>( | ||
| options: Options<StartExportJobData, ThrowOnError>, | ||
| ) => { | ||
| return (options?.client ?? client).post< | ||
| StartExportJobResponse2, | ||
| StartExportJobError, | ||
| ThrowOnError | ||
| >({ | ||
| ...options, | ||
| url: '/drive/document/export/async', | ||
| }); | ||
| }; | ||
|
|
||
| /** | ||
| * Get export job status | ||
| * Get the status of an async export job | ||
| */ | ||
| export const getExportJobStatus = <ThrowOnError extends boolean = false>( | ||
| options: Options<GetExportJobStatusData, ThrowOnError>, | ||
| ) => { | ||
| return (options?.client ?? client).get< | ||
| GetExportJobStatusResponse2, | ||
| GetExportJobStatusError, | ||
| ThrowOnError | ||
| >({ | ||
| ...options, | ||
| url: '/drive/document/export/job/{jobId}', | ||
| }); | ||
| }; | ||
|
|
||
| /** | ||
| * Download export job result | ||
| * Download the result of a completed export job | ||
| */ | ||
| export const downloadExportJobResult = <ThrowOnError extends boolean = false>( | ||
| options: Options<DownloadExportJobResultData, ThrowOnError>, | ||
| ) => { | ||
| return (options?.client ?? client).get< | ||
| DownloadExportJobResultResponse, | ||
| DownloadExportJobResultError, | ||
| ThrowOnError | ||
| >({ | ||
| ...options, | ||
| url: '/drive/document/export/job/{jobId}/download', | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
@hey-api/client-fetch v0.4.0 blob binary download responseType option
💡 Result:
Short answer: @hey-api/client-fetch (the Fetch-based client) does not use an Axios-style responseType option. For binary downloads use the Fetch response API (response.blob()/arrayBuffer()) or provide a custom fetch/response handler via the client configuration or interceptors.
How to do it:
- If you call the generated SDK method (or client.request) with the Fetch client, take the returned Response (or the SDK's raw response) and call await response.blob() or await response.arrayBuffer() to get binary data. See the Fetch-client docs which explain this client is a thin wrapper over Fetch and supports custom fetch/handlers. [1][2]
- Alternatively, set a custom fetch implementation or use response interceptors (client.interceptors.response) to transform the response into a Blob/ArrayBuffer before it’s returned. The Fetch client docs show how to pass a custom fetch and how to configure interceptors. [2][3]
- If you prefer an axios-style responseType: 'blob' API, use the Axios client plugin (@hey-api/client-axios) which supports axios options such as responseType. [1]
References:
- Hey API — Fetch client docs (describes Fetch-based client, custom fetch, interceptors). [1]
- Hey API — Fetch client page (configuration and setConfig / interceptors examples). [2]
- Hey API clients overview (lists available clients including axios). [3]
Sources:
[1] https://heyapi.dev/openapi-ts/clients/fetch (Fetch client docs).
[2] https://heyapi.dev/openapi-ts/clients/fetch.html (Fetch client configuration & examples).
[3] https://github.com/hey-api/openapi-ts (project / clients list).
🏁 Script executed:
# Search for type definitions and similar patterns in the codebase
rg "DownloadExportJobResponse" packages/request/src/requests/services.gen.ts -A 5 -B 5Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Check for Options type definition and its properties
rg "type Options" packages/request/src/ -A 10Repository: refly-ai/refly
Length of output: 5526
🏁 Script executed:
# Look for other download/binary handling patterns in the codebase
rg "responseType|blob|binary" packages/request/src/requests/ -iRepository: refly-ai/refly
Length of output: 986
Add explicit binary response handling for downloadExportJobResult.
@hey-api/client-fetch does not support Axios-style responseType: 'blob' and defaults to JSON parsing, which will corrupt binary downloads. Configure a response interceptor or custom fetch handler to call response.blob() on the response, or switch to @hey-api/client-axios for responseType: 'blob' support.
🤖 Prompt for AI Agents
In @packages/request/src/requests/services.gen.ts around lines 1593 - 1642, The
downloadExportJobResult wrapper currently uses the default client which will
JSON-parse responses and corrupt binary data; update downloadExportJobResult
(and its call to (options?.client ?? client).get with
Options<DownloadExportJobResultData, ThrowOnError>) to handle binary responses
by either: 1) passing a custom fetch/response handler via options that calls
response.blob() (or response.arrayBuffer()) and returns that result, or 2)
switching this endpoint to use the axios-backed client (e.g., client-axios) and
set responseType: 'blob' for the GET to
'/drive/document/export/job/{jobId}/download'; ensure the handler/option is
documented and used only for downloadExportJobResult so other endpoints remain
JSON-parsed.
| const { isLogin, isCheckingLoginStatus, userProfile } = useUserStoreShallow((state) => ({ | ||
| isLogin: state.isLogin, | ||
| isCheckingLoginStatus: state.isCheckingLoginStatus, | ||
| userProfile: state.userProfile, | ||
| })); | ||
|
|
||
| const needOnboarding = userProfile?.preferences?.needOnboarding; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/web-core/src/pages/login/index.tsx | head -150Repository: refly-ai/refly
Length of output: 6825
🏁 Script executed:
cat -n packages/web-core/src/pages/login/index.tsx | sed -n '150,250p'Repository: refly-ai/refly
Length of output: 4357
🏁 Script executed:
# Check where useUserStoreShallow is defined to understand when userProfile is loaded
fd -t f "store" --type f -e ts -e tsx | grep -i user | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
rg "useUserStoreShallow" --type ts --type tsx -lRepository: refly-ai/refly
Length of output: 85
🏁 Script executed:
rg "useUserStoreShallow" -l | head -20Repository: refly-ai/refly
Length of output: 1215
🏁 Script executed:
cat -n packages/stores/src/stores/user.ts | head -100Repository: refly-ai/refly
Length of output: 3860
🏁 Script executed:
# Search for where setIsLogin and setUserProfile are called together
rg "setIsLogin|setUserProfile" -B 2 -A 2 | head -100Repository: refly-ai/refly
Length of output: 9121
🏁 Script executed:
rg "getLoginStatus|useIsLogin" -A 5 -B 5 | head -150Repository: refly-ai/refly
Length of output: 15397
🏁 Script executed:
cat -n packages/ai-workspace-common/src/hooks/use-is-login.tsRepository: refly-ai/refly
Length of output: 1970
🏁 Script executed:
cat -n packages/ai-workspace-common/src/hooks/use-get-user-settings.ts | head -100Repository: refly-ai/refly
Length of output: 5065
🏁 Script executed:
cat -n packages/ai-workspace-common/src/hooks/use-get-user-settings.ts | sed -n '100,150p'Repository: refly-ai/refly
Length of output: 2368
🏁 Script executed:
cat -n packages/ai-workspace-common/src/hooks/use-get-user-settings.ts | sed -n '150,200p'Repository: refly-ai/refly
Length of output: 2156
🏁 Script executed:
# Check if returnUrl is validated anywhere in the codebase
rg "returnUrl" -B 3 -A 3 | head -100Repository: refly-ai/refly
Length of output: 8431
🏁 Script executed:
# Check if useGetUserSettings is called early in the app lifecycle
rg "useGetUserSettings" -B 2 -A 2Repository: refly-ai/refly
Length of output: 1531
🏁 Script executed:
# Check storePendingRedirect to see if it validates returnUrl
rg "storePendingRedirect\|getPendingRedirect" -B 3 -A 10Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
find . -name "*pending-redirect*" -type fRepository: refly-ai/refly
Length of output: 122
Ensure needOnboarding is included in the handleEmailAuth dependency array.
While the needOnboarding type suggestion is reasonable for defensive coding, a more critical issue exists: handleEmailAuth (line 150-222) uses needOnboarding at lines 188 and 217 but omits it from the dependency array (line 222). This causes the callback to capture a stale needOnboarding value, potentially redirecting users incorrectly if their onboarding status changes.
Additionally, add needOnboarding to the dependency array to prevent stale closures:
Proposed fix
const handleEmailAuth = useCallback(async () => {
// ... code ...
- }, [authStore, form, isPublicAccessPage, searchParams]);
+ }, [authStore, form, isPublicAccessPage, searchParams, needOnboarding]);🤖 Prompt for AI Agents
In @packages/web-core/src/pages/login/index.tsx around lines 42 - 49,
handleEmailAuth (the useCallback defined around lines 150-222) reads
needOnboarding (from userProfile) at runtime but does not list needOnboarding in
its dependency array, causing a stale closure; update the useCallback dependency
array for handleEmailAuth to include needOnboarding so the callback updates when
the user’s onboarding flag changes and ensure the variable used is the same
needOnboarding from the useUserStoreShallow selector.
| // Avoid redirect loop: only use cookie-based fast path when there is no returnUrl | ||
| const hasReturnUrl = !!searchParams.get('returnUrl'); | ||
| if (isLoggedIn || (hasLoginCredentials && !hasReturnUrl)) { | ||
| if (needOnboarding) { | ||
| return <Navigate to="/onboarding" replace />; | ||
| } | ||
| return <Navigate to="/workspace" replace />; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the login/index.tsx file to understand the full context
cat -n packages/web-core/src/pages/login/index.tsxRepository: refly-ai/refly
Length of output: 29353
🏁 Script executed:
# Check the file size first to understand what we're dealing with
wc -l packages/web-core/src/pages/login/index.tsxRepository: refly-ai/refly
Length of output: 105
🏁 Script executed:
wc -l packages/web-core/src/pages/login/index.tsxRepository: refly-ai/refly
Length of output: 105
🏁 Script executed:
# Find the store definition for useUserStoreShallow
fd -t f "store" packages/web-core/src | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for useUserStoreShallow definition
rg "useUserStoreShallow|createUserStore" --type ts --type tsx -lRepository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Search for useUserStoreShallow definition in @refly/stores
rg "useUserStoreShallow" --type ts -B 2 -A 10 -lRepository: refly-ai/refly
Length of output: 5010
🏁 Script executed:
# Find where @refly/stores is defined
fd -t f -p "stores" | grep -E "(index|store)" | head -20Repository: refly-ai/refly
Length of output: 910
🏁 Script executed:
# Examine the user store
cat -n packages/stores/src/stores/user.tsRepository: refly-ai/refly
Length of output: 4784
🏁 Script executed:
# Check the useIsLogin hook
cat -n packages/ai-workspace-common/src/hooks/use-is-login.tsRepository: refly-ai/refly
Length of output: 1970
🏁 Script executed:
# Search for where setIsLogin is called
rg "setIsLogin\(true\)" --type ts -B 2 -A 2Repository: refly-ai/refly
Length of output: 552
🏁 Script executed:
# Search for where setUserProfile is called
rg "setUserProfile" --type ts -B 2 -A 2 -lRepository: refly-ai/refly
Length of output: 901
🏁 Script executed:
# Examine use-get-user-settings hook
cat -n packages/ai-workspace-common/src/hooks/use-get-user-settings.tsRepository: refly-ai/refly
Length of output: 9688
🏁 Script executed:
# Find where useGetUserSettings is used
rg "useGetUserSettings" --type ts -lRepository: refly-ai/refly
Length of output: 171
🏁 Script executed:
# Check the app layout or root component
fd -t f "layout\|app\|root" packages/web-core/src | grep -E "(index|layout|app)" | head -10Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Check the layout component
cat -n packages/web-core/src/components/layout/index.tsx | head -150Repository: refly-ai/refly
Length of output: 7511
🏁 Script executed:
# Check the main app entry point
fd -t f -p "App\.tsx|main\.tsx" packages/web-core/src | head -5Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Look for router setup
fd -t f "router\|route" packages/web-core/src | grep -E "\.(ts|tsx)$" | head -10Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Find the main entry point
find packages/web-core/src -name "*.tsx" -o -name "*.ts" | grep -E "(index|app|main)" | head -20Repository: refly-ai/refly
Length of output: 1120
🏁 Script executed:
# Check how AppLayout is used
rg "AppLayout" packages/web-core/src -B 3 -A 3 -lRepository: refly-ai/refly
Length of output: 138
🏁 Script executed:
# Check the main index file
cat packages/web-core/src/index.tsRepository: refly-ai/refly
Length of output: 1851
🏁 Script executed:
# Let me also search for where AppLayout is used as a wrapper
rg "AppLayout" packages/web-core/src -B 5 -A 5 | head -80Repository: refly-ai/refly
Length of output: 1998
🏁 Script executed:
# Let me check if there's any store hydration on app startup
rg "setIsLogin\|setUserProfile" packages/web-core/src --type ts -B 3 -A 1 | grep -E "(import|set|useEffect)" | head -40Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Check if localStorage is used to restore state on startup
rg "localStorage.*refly-user-profile" packages/web-core/src --type ts -B 2 -A 2Repository: refly-ai/refly
Length of output: 509
🏁 Script executed:
# Find the router configuration
fd -t f "router\|Route" packages/web-core | grep -E "\.(ts|tsx)$"Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search in apps directory
fd -t f "router\|Route\|app" apps/web/src | grep -E "\.(ts|tsx)$" | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Let me verify the exact flow by checking when isLogin gets set to true without userProfile
# Search for all places where setIsLogin(true) is called
rg "setIsLogin\(true\)" --type ts -B 5 -A 5Repository: refly-ai/refly
Length of output: 1123
Guard the onboarding redirect on userProfile readiness, not just isLoggedIn.
When isLogin is true but userProfile is still undefined (e.g., during async fetch or if fetch failed), needOnboarding becomes falsy and the user redirects to /workspace instead of /onboarding. To fix, either:
- Wait for
userProfileto be populated before redirecting, or - Check
userProfile?.preferences?.needOnboarding ?? trueto default to onboarding when uncertain
Summary by CodeRabbit
New Features
Style & Design
Localization
✏️ Tip: You can customize this high-level summary in your review settings.