Workflow builder UI integration Xyne Dashboard #759
Conversation
avirupsinha12
requested review from
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a Workflow Builder feature: new authenticated /workflow route and Sidebar link, a React Flow-based designer with node/edge editing, configuration panels (AI agent, Email, Form), execution modal, API client, types, utilities, icon set, and a Flow provider with localStorage persistence. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as WorkflowBuilder
participant TemplatesAPI as workflowTemplatesAPI
participant WorkflowsAPI as workflowsAPI
Note over UI: Load template and build graph
User->>UI: Open Workflow Builder
UI->>TemplatesAPI: fetchById(templateId)
TemplatesAPI-->>UI: TemplateFlow
alt Execute node
User->>UI: Click Execute on node
UI->>TemplatesAPI: instantiate(templateId, {name})
TemplatesAPI-->>UI: {workflowId, rootStepId}
UI->>WorkflowsAPI: run(workflowId)
WorkflowsAPI-->>UI: ack
loop Polling
UI->>WorkflowsAPI: fetchById(workflowId)
WorkflowsAPI-->>UI: Flow (step statuses)
UI-->>UI: Update node states
end
else Error
UI-->>UI: show/log error
end
sequenceDiagram
autonumber
actor User
participant AIPanel as AIAgentConfigUI
participant PromptSvc as /agent/generate-prompt (SSE)
User->>AIPanel: Click "Enhance"
AIPanel->>PromptSvc: Open EventSource(model, requirements)
alt Streamed chunks (≤30s)
PromptSvc-->>AIPanel: data: {chunk}
AIPanel-->>AIPanel: aggregate enhancedPrompt
PromptSvc-->>AIPanel: [done]
AIPanel-->>AIPanel: apply enhancedPrompt
else Timeout/Error
AIPanel-->>AIPanel: apply deterministic fallback enhancement
end
User->>AIPanel: Save
AIPanel-->>User: onSave(agentConfig)
sequenceDiagram
autonumber
actor User
participant Card as WorkflowCard
participant Modal as WorkflowExecutionModal
participant UploadAPI as workflowsAPI.uploadFile
participant StatusAPI as workflowsAPI.pollProcessStatus
User->>Card: Click Run
Card->>Modal: Open
User->>Modal: Select/Drop file
Modal->>UploadAPI: POST FormData
UploadAPI-->>Modal: jobId/ack
loop Every 2s
Modal->>StatusAPI: pollProcessStatus
StatusAPI-->>Modal: processing|completed
end
Modal-->>User: Show completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @avirupsinha12, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the Xyne Dashboard by integrating a robust workflow automation capability. It empowers users to construct complex, multi-step processes through an intuitive visual interface, streamlining operations and enabling advanced AI-driven and communication-based tasks. The changes lay the groundwork for future expansions in automated functionalities.
Highlights
- New Workflow Builder UI: Introduces a comprehensive visual workflow builder, allowing users to design and manage automated processes directly within the Xyne Dashboard. This includes a canvas for node-based workflow creation and dedicated configuration panels.
- AI Agent and Email Step Configuration: Adds specialized UI components for configuring AI Agent and Email steps within workflows. Users can define AI agent behaviors, models, and prompts, as well as manage email sending parameters and recipients.
- Workflow Execution and Monitoring: Integrates functionality to execute workflows and monitor their real-time status. This includes an action bar for triggering workflows and a modal for file uploads and progress tracking.
- Core Workflow Infrastructure: Establishes the foundational frontend architecture for workflow management, including new data types, utility functions for flow traversal, and API handlers for interacting with backend workflow services and templates.
- Sidebar Navigation Update: A new 'Workflow Builder' entry has been added to the main application sidebar, providing direct access to the new UI.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: a workflow builder UI integrated into the dashboard. The implementation is comprehensive, covering everything from the visual builder canvas with custom nodes to configuration sidebars and execution modals. The code is well-structured with clear separation of concerns into different components, types, and API handlers. My feedback primarily focuses on addressing several hardcoded values (API URLs, user/template IDs) that are critical for deployment and reusability. Additionally, there are opportunities to improve maintainability by refactoring inline styles into CSS utility classes, which will enhance consistency across the new components.
Resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/workflow/WorkflowBuilder.tsx (1)
1-1732: Add error boundaries and loading statesThe component lacks proper error handling UI and loading states for async operations.
Consider adding:
- Error boundary component to catch React errors
- Loading spinner/skeleton for template fetching
- Error toast/notification for API failures
- Retry mechanisms for failed API calls
🧹 Nitpick comments (40)
frontend/src/components/workflow/Default.ts (1)
3-15: Prefer empty arrays over nulls for list fields (if type allows)Using [] for nextStepIds/toolIds reduces null checks downstream and avoids surprises in array ops.
If Step allows arrays here, update:
description: 'Default step for workflow', parentStepId: null, - nextStepIds: null, - toolIds: null, + nextStepIds: [], + toolIds: [], timeEstimate: 0, - metadata: null, + metadata: null, contents: []frontend/src/components/workflow/ActionBar.tsx (1)
23-31: Minor a11y + value type polishAdd accessible labels and pass a string to the select value to match option values.
- <button - onClick={onExecute} + <button + type="button" + aria-label="Execute workflow" + onClick={onExecute} className="px-4 py-2 bg-slate-800 hover:bg-slate-700 text-white border-none rounded-md cursor-pointer text-sm font-medium flex items-center gap-1.5 transition-all duration-200" > ... - <select - className="border-none bg-transparent text-sm font-medium text-slate-700 cursor-pointer appearance-none pr-4 outline-none" - value={zoomLevel} + <select + aria-label="Zoom level" + className="border-none bg-transparent text-sm font-medium text-slate-700 cursor-pointer appearance-none pr-4 outline-none" + value={String(zoomLevel)} onChange={handleZoomChange} >Also applies to: 38-42
frontend/src/components/workflow/WhatHappensNextUI.tsx (2)
81-83: Prevent interaction when hidden; expose visibility to ATsWhen hidden, panel content can still be focusable via keyboard. Add aria-hidden and block pointer events.
- <div className={`h-full bg-white border-l border-slate-200 flex flex-col overflow-hidden transition-transform duration-300 ease-in-out ${ - isVisible ? 'translate-x-0 w-[380px]' : 'translate-x-full w-0' - }`}> + <div + aria-hidden={!isVisible} + className={`h-full bg-white border-l border-slate-200 flex flex-col overflow-hidden transition-transform duration-300 ease-in-out ${ + isVisible ? 'translate-x-0 w-[380px]' : 'translate-x-full w-0 pointer-events-none' + }`} + >
27-46: Hoist static action lists outside the component (micro perf)availableActions/comingSoonActions are static; define once outside to avoid re-creation on each render.
Also applies to: 48-78
frontend/src/components/workflow/WorkflowExecutionModal.tsx (2)
76-88: File type validation: handle empty MIME by falling back to extensionBrowsers sometimes provide an empty file.type. Consider extension-based fallback to avoid false negatives.
I can draft a small helper to infer type from extension if you want.
222-407: Optional: reduce noisy console logs or gate behind a debug flagVerbose logs can leak details in prod. Consider an env-gated logger.
frontend/src/components/workflow/api/ApiHandlers.ts (3)
62-66: Tightenmetadatatyping to avoidanyleakage.Make instantiate generic over metadata instead of
any.Apply:
- async instantiate(id: string, options: { name: string; metadata?: any }): Promise<ApiResponse<{ workflowId: string; rootStepId: string }>> { + async instantiate<TMeta extends Record<string, unknown> = Record<string, unknown>>(id: string, options: { name: string; metadata?: TMeta }): Promise<ApiResponse<{ workflowId: string; rootStepId: string }>> {
83-96: Consider timeout + idempotency for side-effecting requests.Add optional
AbortControllertimeout and support anIdempotency-Keyheader forrun/completeStepto guard double-submits. Verify backend support before enabling.Example (call-site or here via
options):const c = new AbortController(); const t = setTimeout(() => c.abort(), 10000); try { await workflowsAPI.run(id /*, { signal: c.signal, headers: { 'Idempotency-Key': crypto.randomUUID() } }*/); } finally { clearTimeout(t); }
13-17: Comment mismatch (“ngrok headers”).The helper doesn’t add any ngrok-specific headers. Either remove the note or add them conditionally in dev.
frontend/src/components/workflow/WorkflowIcons.tsx (3)
3-7: Add lightweight a11y support to icons (title/decorative).Expose
titleanddecorativeprops to enable labels or mark purely decorative icons as hidden from AT.Apply:
interface IconProps { className?: string; width?: number; height?: number; + title?: string; + decorative?: boolean; }
10-15: Demonstrate accessible SVG pattern; replicate across icons.Adds role, aria, and focus semantics without changing visuals.
Apply:
-export const DelayIcon: React.FC<IconProps> = ({ className = "w-4 h-4", width = 24, height = 24 }) => ( - <svg className={className} width={width} height={height} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2"> +export const DelayIcon: React.FC<IconProps> = ({ className = "w-4 h-4", width = 24, height = 24, title, decorative = true }) => ( + <svg className={className} width={width} height={height} viewBox="0 0 24 24" role="img" aria-hidden={decorative ? 'true' : undefined} aria-label={decorative ? undefined : title} focusable="false" fill="none" stroke="currentColor" strokeWidth="2"> <circle cx="12" cy="12" r="10"></circle> <polyline points="12 6 12 12 16 14"></polyline> </svg> )
169-172: Prefer theming via currentColor (avoid hard-coded fills).Switch
fill="black"tocurrentColorfor dark-mode/theming. Consider the same for other hex colors unless brand-locked.Apply:
- <svg className={className} xmlns="http://www.w3.org/2000/svg" width={width} height={height} viewBox="0 0 24 24" fill="none"> - <path fillRule="evenodd" clipRule="evenodd" d="M13.4142 12.0002L18.7072 6.70725C19.0982 6.31625 19.0982 5.68425 18.7072 5.29325C18.3162 4.90225 17.6842 4.90225 17.2932 5.29325L12.0002 10.5862L6.70725 5.29325C6.31625 4.90225 5.68425 4.90225 5.29325 5.29325C4.90225 5.68425 4.90225 6.31625 5.29325 6.70725L10.5862 12.0002L5.29325 17.2933C4.90225 17.6842 4.90225 18.3162 5.29325 18.7072C5.48825 18.9022 5.74425 19.0002 6.00025 19.0002C6.25625 19.0002 6.51225 18.9022 6.70725 18.7072L12.0002 13.4143L17.2932 18.7072C17.4882 18.9022 17.7442 19.0002 18.0002 19.0002C18.2562 19.0002 18.5122 18.9022 18.7072 18.7072C19.0982 18.3162 19.0982 17.6842 18.7072 17.2933L13.4142 12.0002Z" fill="black"/> + <svg className={className} xmlns="http://www.w3.org/2000/svg" width={width} height={height} viewBox="0 0 24 24" fill="none"> + <path fillRule="evenodd" clipRule="evenodd" d="M13.4142 12.0002L18.7072 6.70725C19.0982 6.31625 19.0982 5.68425 18.7072 5.29325C18.3162 4.90225 17.6842 4.90225 17.2932 5.29325L12.0002 10.5862L6.70725 5.29325C6.31625 4.90225 5.68425 4.90225 5.29325 5.29325C4.90225 5.68425 4.90225 6.31625 5.29325 6.70725L10.5862 12.0002L5.29325 17.2933C4.90225 17.6842 4.90225 18.3162 5.29325 18.7072C5.48825 18.9022 5.74425 19.0002 6.00025 19.0002C6.25625 19.0002 6.51225 18.9022 6.70725 18.7072L12.0002 13.4143L17.2932 18.7072C17.4882 18.9022 17.7442 19.0002 18.0002 19.0002C18.2562 19.0002 18.5122 18.9022 18.7072 18.7072C19.0982 18.3162 19.0982 17.6842 18.7072 17.2933L13.4142 12.0002Z" fill="currentColor"/>frontend/src/components/workflow/Types.ts (6)
69-69: Narrow Step.status to known values while allowing backend drift.Improves type-safety without breaking.
Apply:
- status?: string + status?: Status | string
166-176: IntroduceAnyFlowunion and reuse in props/context.Reduces repetition across types.
Apply:
export type Flow = { template_id: string step_exe: StepExecution[] tools_exe: ToolExecution[] workflow_info: WorkflowInfo } +export type AnyFlow = Flow | TemplateFlow | LegacyFlow + // Legacy Flow type for backward compatibility export type LegacyFlow = {
179-184: UseAnyFlowin context.Cleaner signatures.
Apply:
export interface FlowContextProps { - flow: Flow | TemplateFlow | LegacyFlow - setFlow: (flow: Flow | TemplateFlow | LegacyFlow) => void + flow: AnyFlow + setFlow: (flow: AnyFlow) => void activeSubStep: Step | null setActiveSubStep: (step: Step | null) => void }
186-191: UseAnyFlowin component props.Avoids duplicating unions.
Apply:
export interface FlowProps { title?: string; - flow?: Flow | TemplateFlow | LegacyFlow; + flow?: AnyFlow; className?: string; user?: UserDetail; }
63-92: Note on mixed snake_case/camelCase fields.Consider adding a small normalizer (server -> UI) to reduce downstream conditional code.
Example utility (outside types):
export function normalizeStep(s: Step) { return { ...s, parentStepId: s.parentStepId ?? s.parent_step_id ?? null, nextStepIds: s.nextStepIds ?? s.child_step_ids ?? [], status: s.status as Status | string, }; }Also applies to: 101-127, 129-148
56-61: Consider discriminated union for Tool values.
val: number | ToolValueis ambiguous at call sites.Example:
export type ScriptTool = { id: string; type: 'script'; config: ToolConfig; val: ToolValue & { script: string } }; export type MessageTool = { id: string; type: 'message'; config: ToolConfig; val: ToolValue & { message: string } }; export type NumericTool = { id: string; type: 'numeric'; config: ToolConfig; val: number }; export type Tool = ScriptTool | MessageTool | NumericTool;frontend/src/components/Sidebar.tsx (1)
174-193: Verify route path: Sidebar links to "/workflow" while the file-based route is "/_authenticated/workflow".If routeTree maps "/workflow" to this file, you're good; otherwise this will 404 or not highlight correctly.
Apply if needed:
- to="/workflow" + to="/_authenticated/workflow"Optional a11y nit: add aria-label for keyboard users.
- <Link + <Link aria-label="Workflow Builder"frontend/src/components/workflow/WorkflowProvider.tsx (3)
56-66: Harden localStorage writes and avoid noisy logs in production.Quota errors can throw; logs can leak data in prod. Wrap setItem/removeItem in try/catch and gate console.log with NODE_ENV.
- console.log("FlowProvider: Setting flow to:", newFlow) + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.log("FlowProvider: Setting flow to:", newFlow) + } setFlow(newFlow) // Save to localStorage - if (newFlow) { - localStorage.setItem("flow_data", JSON.stringify(newFlow)) - } else { - localStorage.removeItem("flow_data") - } + try { + if (newFlow) { + localStorage.setItem("flow_data", JSON.stringify(newFlow)) + } else { + localStorage.removeItem("flow_data") + } + } catch (e) { + // eslint-disable-next-line no-console + console.error("Persisting flow_data failed:", e) + }
68-78: Do the same for activeSubStep persistence and logging.- console.log("FlowProvider: Setting activeSubStep to:", newStep) + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.log("FlowProvider: Setting activeSubStep to:", newStep) + } setActiveSubStep(newStep) // Save to localStorage - if (newStep) { - localStorage.setItem("active_sub_step", JSON.stringify(newStep)) - } else { - localStorage.removeItem("active_sub_step") - } + try { + if (newStep) { + localStorage.setItem("active_sub_step", JSON.stringify(newStep)) + } else { + localStorage.removeItem("active_sub_step") + } + } catch (e) { + // eslint-disable-next-line no-console + console.error("Persisting active_sub_step failed:", e) + }
80-88: Memoize context value to prevent unnecessary re-renders of all consumers.The object literal changes on every render.
+import { useMemo } from "react" ... - return ( - <FlowContext.Provider - value={{ - flow, - setFlow: setFlowWithLog, - activeSubStep, - setActiveSubStep: setActiveSubStepWithLog, - }} - > + const ctxValue = useMemo( + () => ({ + flow, + setFlow: setFlowWithLog, + activeSubStep, + setActiveSubStep: setActiveSubStepWithLog, + }), + [flow, activeSubStep] + ) + + return ( + <FlowContext.Provider value={ctxValue}> {children} </FlowContext.Provider>frontend/src/components/workflow/WorkflowCard.tsx (2)
27-38: Guard against invalid dates and unify formatting.If updatedAt is invalid, show “—”. Also avoid recomputing locale options each render.
- const formatDate = (dateString: string) => { - const date = new Date(dateString) - return date.toLocaleDateString('en-US', { - month: '2-digit', - day: '2-digit', - year: 'numeric' - }) + ' ' + date.toLocaleTimeString('en-US', { - hour: '2-digit', - minute: '2-digit', - hour12: false - }) - } + const formatDate = (dateString: string) => { + const date = new Date(dateString) + if (isNaN(date.getTime())) return "—" + const d = date.toLocaleDateString("en-US", { month: "2-digit", day: "2-digit", year: "numeric" }) + const t = date.toLocaleTimeString("en-US", { hour: "2-digit", minute: "2-digit", hour12: false }) + return `${d} ${t}` + }
66-68: Wire up “View” action or disable it.Add onClick/Link to details/builder to avoid dead UI.
- <Button size="sm" variant="outline" className="border-gray-300 dark:border-gray-600"> - View - </Button> + <Button + size="sm" + variant="outline" + className="border-gray-300 dark:border-gray-600" + onClick={() => {/* navigate to details or open builder preloaded */}} + > + View + </Button>frontend/src/routes/_authenticated/workflow.tsx (2)
57-57: Gate verbose logging to non-production builds.Avoid leaking workflow data in prod logs.
- console.log('API Response:', data) + if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line no-console + console.log('API Response:', data) + }
41-78: Add abort/cancellation to fetch to avoid state updates after unmount.Prevents memory leaks and warnings when navigating away quickly.
- const fetchWorkflows = async () => { + const fetchWorkflows = async (signal?: AbortSignal) => { if (hasFetched.current) return hasFetched.current = true try { setLoading(true) - const response = await fetch('/user/fetch/workflows', { + const response = await fetch('/user/fetch/workflows', { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify({ userId: 2 - }) + }), + signal, })And in the effect:
- useEffect(() => { - fetchWorkflows() - }, []) + useEffect(() => { + const ac = new AbortController() + fetchWorkflows(ac.signal) + return () => ac.abort() + }, [])frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
81-106: Handle SSE events robustly (named events vs JSON types).onmessage only captures default “message” events; if the server emits named events, add listeners or rely solely on data.type.
- eventSource.onmessage = (event) => { + eventSource.onmessage = (event) => { try { const data = JSON.parse(event.data); - - if (event.type === 'ResponseUpdate' || data.type === 'update') { + // Prefer payload type over DOM event.type + if (data.type === 'update') { enhancedPrompt += data.text || data.content || ''; - } else if (event.type === 'End' || data.type === 'end') { + } else if (data.type === 'end') { clearTimeout(timeoutId); eventSource.close();Optionally add explicit listeners if backend uses named events:
+ eventSource.addEventListener('ResponseUpdate', (e: MessageEvent) => { + try { + const d = JSON.parse(e.data) + enhancedPrompt += d.text || d.content || '' + } catch {} + }) + eventSource.addEventListener('End', (e: MessageEvent) => { + clearTimeout(timeoutId) + eventSource.close() + const d = (() => { try { return JSON.parse(e.data) } catch { return {} }})() + const finalPrompt = d.fullPrompt || enhancedPrompt.trim() + if (finalPrompt) { + setAgentConfig(prev => ({ ...prev, systemPrompt: finalPrompt })) + } + setIsEnhancingPrompt(false) + })
45-50: Use a non-blocking UI toast instead of alert() for UX consistency.Alerts are jarring and block the thread.
- if (!agentConfig.systemPrompt.trim()) { - alert('Please enter a system prompt first'); - return; - } + if (!agentConfig.systemPrompt.trim()) { + // showToast({ title: 'Missing system prompt' }) + return; + }Similarly replace the alert on Line 162 with a toast.
37-44: Consider driving model list from config or API.Hard-coding model ids will drift. Read from server or env to reflect availability.
frontend/src/components/workflow/EmailConfigUI.tsx (7)
53-57: Replace deprecated onKeyPress with onKeyDown and type the event.Prevents deprecation warnings and ensures consistent behavior.
Apply these diffs:
- const handleKeyPress = (e: React.KeyboardEvent) => { - if (e.key === 'Enter') { - handleAddEmail(); - } - }; + const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { + if (e.key === 'Enter') { + e.preventDefault(); + handleAddEmail(); + } + };- onKeyPress={handleKeyPress} + onKeyDown={handleKeyDown}Also applies to: 154-161
79-93: Add accessible names and explicit button types.Improve a11y and avoid accidental form submits.
Apply this diff:
- <button + <button + type="button" + aria-label="Back" onClick={onBack} className="flex items-center justify-center"- <button + <button + type="button" + aria-label="Close panel" onClick={onBack} className="flex items-center justify-center"- <button + <button + type="button" + aria-label={`Remove email ${email}`} onClick={() => handleRemoveEmail(email)} className="p-1 hover:bg-gray-200 rounded transition-colors"Also applies to: 111-125, 187-193
135-142: Remove onChange on a disabled input.It never fires and is confusing.
Apply this diff:
<Input id="sending-from" value={emailConfig.sendingFrom} - onChange={(e) => setEmailConfig(prev => ({ ...prev, sendingFrom: e.target.value }))} placeholder="Enter sender email" className="w-full bg-gray-100" disabled />
172-176: Use a stable key for list items.Avoids reconciliation issues.
Apply this diff:
- {emailConfig.emailAddresses.map((email, index) => { + {emailConfig.emailAddresses.map((email) => { const recipient = recipients.find(r => r.email === email); return ( - <div key={index} className="flex items-center justify-between p-3 bg-gray-50 rounded-lg"> + <div key={email} className="flex items-center justify-between p-3 bg-gray-50 rounded-lg">
31-34: Avoid state for static data.Use a const since recipients never change.
Apply this diff:
- const [recipients] = useState<EmailRecipient[]>([ + const recipients: EmailRecipient[] = [ { id: '1', email: 'user.one@example.com', name: 'User One' }, { id: '2', email: 'user.two@example.com', name: 'User Two' } - ]); + ];
63-67: Hide panel from assistive tech when closed.Expose visibility to SRs via aria-hidden.
Apply this diff:
- <div className={`h-full bg-white border-l border-slate-200 flex flex-col overflow-hidden transition-transform duration-300 ease-in-out ${ + <div aria-hidden={!isVisible} className={`h-full bg-white border-l border-slate-200 flex flex-col overflow-hidden transition-transform duration-300 ease-in-out ${ isVisible ? 'translate-x-0 w-[400px]' : 'translate-x-full w-0' }`}>
154-166: Minor UX copy and input type.Better hint and mobile-friendly keyboard.
Apply this diff:
<Input id="add-email" + type="email" value={newEmailAddress} onChange={(e) => setNewEmailAddress(e.target.value)} - onKeyDown={handleKeyDown} + onKeyDown={handleKeyDown} placeholder="type email address" className="w-full pr-16" /> <div className="absolute right-3 top-1/2 transform -translate-y-1/2"> <span className="text-xs text-slate-400 bg-slate-100 px-2 py-1 rounded"> - click "enter" to add + Press Enter to add </span> </div>frontend/src/components/workflow/WorkflowUtils.ts (1)
4-10: Simplify zero-padding.Use padStart for readability.
Apply this diff:
export function padWithZero(value: number): string { - if (value < 10) { - return "0" + value.toString() - } else { - return value.toString() - } + return value.toString().padStart(2, '0') }frontend/src/components/workflow/WorkflowBuilder.tsx (3)
1077-1078: Remove void variable assignmentsThe
voidassignments are unusual and suggest these functions might be unused.- // Prevent unused variable warning - void addNewNode; - // Prevent unused variable warning - void deleteSelectedNodes;If these functions are truly unused, remove them entirely. If they're needed for future use, consider adding a TODO comment instead.
Also applies to: 1090-1091
1263-1269: Extract magic strings to constantsHardcoded strings for workflow metadata should be externalized.
+const DEFAULT_WORKFLOW_NAME = "Test Webhook Flow"; +const DEFAULT_WORKFLOW_DESCRIPTION = "Testing webhook workflow"; +const DEFAULT_ENVIRONMENT = "test"; const instantiateResponse = await workflowTemplatesAPI.instantiate(templateWorkflow.template_id, { - name: "Test Webhook Flow", + name: DEFAULT_WORKFLOW_NAME, metadata: { - description: "Testing webhook workflow", - environment: "test" + description: DEFAULT_WORKFLOW_DESCRIPTION, + environment: DEFAULT_ENVIRONMENT } });
1-1732: Missing PropTypes or TypeScript interfaces for component propsSeveral callback props and configuration objects lack proper type definitions.
Add comprehensive TypeScript interfaces for all props, configs, and API responses to improve type safety and developer experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (10)
frontend/src/assets/arrow-left.svgis excluded by!**/*.svgfrontend/src/assets/bot-logo.svgis excluded by!**/*.svgfrontend/src/assets/check-circle.svgis excluded by!**/*.svgfrontend/src/assets/file-up.svgis excluded by!**/*.svgfrontend/src/assets/grid-dashboard-01.svgis excluded by!**/*.svgfrontend/src/assets/import-dsl.svgis excluded by!**/*.svgfrontend/src/assets/play.svgis excluded by!**/*.svgfrontend/src/assets/plus.svgis excluded by!**/*.svgfrontend/src/assets/sitemap.svgis excluded by!**/*.svgfrontend/src/assets/vector.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
frontend/package.json(2 hunks)frontend/src/components/Sidebar.tsx(2 hunks)frontend/src/components/workflow/AIAgentConfigUI.tsx(1 hunks)frontend/src/components/workflow/ActionBar.tsx(1 hunks)frontend/src/components/workflow/Default.ts(1 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(1 hunks)frontend/src/components/workflow/Types.ts(1 hunks)frontend/src/components/workflow/WhatHappensNextUI.tsx(1 hunks)frontend/src/components/workflow/WorkflowBuilder.tsx(1 hunks)frontend/src/components/workflow/WorkflowCard.tsx(1 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(1 hunks)frontend/src/components/workflow/WorkflowIcons.tsx(1 hunks)frontend/src/components/workflow/WorkflowProvider.tsx(1 hunks)frontend/src/components/workflow/WorkflowUtils.ts(1 hunks)frontend/src/components/workflow/api/ApiHandlers.ts(1 hunks)frontend/src/routeTree.gen.ts(13 hunks)frontend/src/routes/_authenticated/workflow.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: TypeScript Build Check
frontend/src/components/workflow/WorkflowExecutionModal.tsx
[error] 45-45: bunx tsc -b: TypeScript error TS6133: 'isUploading' is declared but its value is never read.
[error] 46-46: bunx tsc -b: TypeScript error TS6133: 'isUploaded' is declared but its value is never read.
🔇 Additional comments (18)
frontend/package.json (3)
37-37: No-op reorderDependency reorder only; no action needed.
82-82: EOF newline fixTrailing newline addition is fine.
32-32: Peer dependency check passed: @xyflow/react@12.8.4 specifies react >=17, which covers React 19frontend/src/components/workflow/Default.ts (1)
6-7: Confirm literal unions for type/statusVerify Step['type'] includes 'manual' and Step['status'] includes 'pending' to avoid narrowing/type errors elsewhere.
frontend/src/components/workflow/ActionBar.tsx (1)
9-55: Solid, focused componentProps and callback handling look clean; styles consistent with the codebase.
frontend/src/components/workflow/api/ApiHandlers.ts (1)
49-58: API surface looks clean and cohesive.Consistent
ApiResponse<T>and endpoint grouping read well.Also applies to: 70-79
frontend/src/components/workflow/WorkflowIcons.tsx (1)
24-29: Icons look consistent and lightweight.Inline SVGs with shared props keep the UI tidy and composable.
Also applies to: 31-45, 46-97, 99-173
frontend/src/components/workflow/Types.ts (1)
1-248: Types are comprehensive and well-structured.Good coverage for template, runtime, and legacy models.
frontend/src/components/Sidebar.tsx (1)
15-16: LGTM on icon import.The new Workflow icon import is correct and consistent with existing usage.
frontend/src/routes/_authenticated/workflow.tsx (2)
28-31: Confirm route mapping for navigation.This file’s route is "/_authenticated/workflow", while Sidebar links to "/workflow". Ensure routeTree.gen.ts aliases "/workflow" to this route.
33-35: Accessing user via matches[matches.length - 1].context is brittle.If parent provides user in a different match, this may be undefined. Prefer a typed root context hook or select the correct match by id.
frontend/src/components/workflow/WorkflowUtils.ts (3)
119-121: Handle Flow | LegacyFlow via a type guard instead of assertion.Avoids runtime surprises if fields diverge.
Apply this diff:
- const legacyFlow = flow as LegacyFlow; - const rootStep = stepDict[legacyFlow.root_step_id] || defaultStep + const rootStepId = (flow as any).root_step_id ?? (flow as any).rootId; + const lastStepId = (flow as any).last_step_id ?? (flow as any).lastStepId; + const rootStep = (rootStepId && stepDict[rootStepId]) || defaultStepAnd below:
- isLastStep: blockingStepId === legacyFlow.last_step_id, + isLastStep: lastStepId ? blockingStepId === lastStepId : false,Please confirm the exact property names in Flow vs LegacyFlow and adjust accordingly.
Also applies to: 191-194
12-38: Clarify invalid time handling.parseTime silently returns [0,0,0] on invalid input; confirm this is intended (it can mask errors).
If strictness is desired, return undefined and handle upstream, or log a warning. I can send a small patch if you confirm preferred behavior.
143-161: Possible ETA double-count for immediate children.You add step.time_needed and also iterate child_step_ids to add again; verify this is intended (BFS will later traverse blockers separately).
If not intended, drop the child iteration here and compute ETA once per node.
frontend/src/routeTree.gen.ts (1)
18-19: Verified: new ‘/workflow’ route is correctly wired and linked.
Route file exists withexport const Route = createFileRoute("/_authenticated/workflow"),routeTree.gen.tsincludes all mappings for/workflowand/_authenticated/workflow, and the Sidebar links to/workflow.frontend/src/components/workflow/WorkflowBuilder.tsx (3)
1219-1235: Memory leak: Polling interval not properly managedThe polling interval state management could lead to memory leaks as the old interval might not be cleared before setting a new one.
const startPolling = useCallback((workflowId: string) => { setIsPolling(true); // Clear any existing interval if (pollingInterval) { clearInterval(pollingInterval); + setPollingInterval(null); } // Start polling every second const interval = setInterval(() => { fetchWorkflowStatus(workflowId); }, 1000); setPollingInterval(interval); - }, [pollingInterval, fetchWorkflowStatus, setIsPolling, setPollingInterval]); + }, [fetchWorkflowStatus, setIsPolling, setPollingInterval]);Remove
pollingIntervalfrom the dependency array to avoid recreating the callback when the interval changes.Likely an incorrect or invalid review comment.
1134-1146: Memory leak: Event listener not properly cleaned upThe event listener uses
as anytype assertion and the cleanup function uses the same, which could lead to memory leaks if the event types don't match.useEffect(() => { - const handleOpenWhatHappensNext = (event: CustomEvent) => { + const handleOpenWhatHappensNext = (event: Event) => { + const customEvent = event as CustomEvent; - const { nodeId } = event.detail; + const { nodeId } = customEvent.detail; setSelectedNodeForNext(nodeId); setShowWhatHappensNextUI(true); }; - window.addEventListener('openWhatHappensNext' as any, handleOpenWhatHappensNext); + window.addEventListener('openWhatHappensNext', handleOpenWhatHappensNext); return () => { - window.removeEventListener('openWhatHappensNext' as any, handleOpenWhatHappensNext); + window.removeEventListener('openWhatHappensNext', handleOpenWhatHappensNext); }; }, []);Likely an incorrect or invalid review comment.
1099-1131: Performance: MutationObserver without disconnect in all pathsThe MutationObserver might not be properly disconnected if the component unmounts while
reactFlowWrapperis null.useEffect(() => { const handleViewportChange = () => { const viewport = getViewport(); const newZoomLevel = Math.round(viewport.zoom * 100); setZoomLevel(newZoomLevel); }; + let observer: MutationObserver | null = null; + let reactFlowWrapper: Element | null = null; + // Listen for viewport changes (including touchpad zoom) - const reactFlowWrapper = document.querySelector('.react-flow__viewport'); + reactFlowWrapper = document.querySelector('.react-flow__viewport'); if (reactFlowWrapper) { - const observer = new MutationObserver(handleViewportChange); + observer = new MutationObserver(handleViewportChange); observer.observe(reactFlowWrapper, { attributes: true, attributeFilter: ['style'] }); // ... rest of the code } + + return () => { + observer?.disconnect(); + if (reactFlowWrapper) { + reactFlowWrapper.removeEventListener('wheel', handleWheel); + } + }; }, [getViewport]);Likely an incorrect or invalid review comment.
| } | ||
|
|
||
| // Base URL for workflow service | ||
| const WORKFLOW_BASE_URL = 'http://localhost:3000/v1'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hard-coded localhost; read base URL from env with a safe fallback.
Prevents mixed-content/CORS issues in prod and enables environment config.
Apply:
-const WORKFLOW_BASE_URL = 'http://localhost:3000/v1';
+const WORKFLOW_BASE_URL = import.meta.env?.VITE_WORKFLOW_BASE_URL ?? '/v1';📝 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 WORKFLOW_BASE_URL = 'http://localhost:3000/v1'; | |
| const WORKFLOW_BASE_URL = import.meta.env?.VITE_WORKFLOW_BASE_URL ?? '/v1'; |
🤖 Prompt for AI Agents
In frontend/src/components/workflow/api/ApiHandlers.ts around line 11, replace
the hard-coded WORKFLOW_BASE_URL with a value read from an environment variable
(e.g. process.env.REACT_APP_API_BASE_URL or import.meta.env.VITE_API_BASE_URL
depending on the build system) and provide a safe fallback that uses the current
origin (window.location.origin) + '/v1' or a relative '/v1' to avoid hard-coded
http://localhost:3000; ensure you normalize/truncate duplicate slashes when
composing the URL and export the constant for use by handlers.
| type: 'email', | ||
| config: { | ||
| sendingFrom: 'noreply@xyne.com', | ||
| emailAddresses: ['arnabdebnath@juspay.in'] |
There was a problem hiding this comment.
Hardcoded email address exposed in code
Personal/internal email address is hardcoded in the component, which is a security concern and should be externalized.
This email address should be moved to environment variables or configuration:
config: {
sendingFrom: 'noreply@xyne.com',
- emailAddresses: ['arnabdebnath@juspay.in']
+ emailAddresses: [process.env.REACT_APP_DEFAULT_EMAIL || 'default@example.com']
}📝 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.
| emailAddresses: ['arnabdebnath@juspay.in'] | |
| config: { | |
| sendingFrom: 'noreply@xyne.com', | |
| emailAddresses: [process.env.REACT_APP_DEFAULT_EMAIL || 'default@example.com'] | |
| } |
🤖 Prompt for AI Agents
In frontend/src/components/workflow/WorkflowBuilder.tsx at line 1478, a
personal/internal email address is hardcoded (emailAddresses:
['arnabdebnath@juspay.in']); replace this by reading the address from
configuration or an environment variable (e.g.
process.env.REACT_APP_SUPPORT_EMAIL or a config file) and pass that value into
emailAddresses, add a safe fallback or validation if the env var is missing,
remove the hardcoded literal from the code, and update any relevant tests or
docs to reference the new config key.
| interface WorkflowCardProps { | ||
| workflow: WorkflowData; | ||
| uploadApiUrl?: string; | ||
| } | ||
|
|
||
| export function WorkflowCard({ workflow, uploadApiUrl = 'http://localhost:8000/upload' }: WorkflowCardProps) { | ||
| const [showExecutionModal, setShowExecutionModal] = useState(false) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hard-coded localhost for upload API.
This will break in non-local environments. Use a configurable base URL (env/config) and default to a relative path.
-interface WorkflowCardProps {
- workflow: WorkflowData;
- uploadApiUrl?: string;
-}
+interface WorkflowCardProps {
+ workflow: WorkflowData;
+ uploadApiUrl?: string; // e.g., import.meta.env.VITE_UPLOAD_API_URL
+}
...
-export function WorkflowCard({ workflow, uploadApiUrl = 'http://localhost:8000/upload' }: WorkflowCardProps) {
+export function WorkflowCard({
+ workflow,
+ uploadApiUrl = (import.meta as any)?.env?.VITE_UPLOAD_API_URL ?? '/api/upload',
+}: WorkflowCardProps) {📝 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.
| interface WorkflowCardProps { | |
| workflow: WorkflowData; | |
| uploadApiUrl?: string; | |
| } | |
| export function WorkflowCard({ workflow, uploadApiUrl = 'http://localhost:8000/upload' }: WorkflowCardProps) { | |
| const [showExecutionModal, setShowExecutionModal] = useState(false) | |
| interface WorkflowCardProps { | |
| workflow: WorkflowData; | |
| uploadApiUrl?: string; // e.g., import.meta.env.VITE_UPLOAD_API_URL | |
| } | |
| export function WorkflowCard({ | |
| workflow, | |
| uploadApiUrl = (import.meta as any)?.env?.VITE_UPLOAD_API_URL ?? '/api/upload', | |
| }: WorkflowCardProps) { | |
| const [showExecutionModal, setShowExecutionModal] = useState(false) | |
| // … | |
| } |
🤖 Prompt for AI Agents
In frontend/src/components/workflow/WorkflowCard.tsx around lines 19 to 25, the
uploadApiUrl prop is hard-coded to 'http://localhost:8000/upload'; change this
to read from a configurable env/config value and fall back to a relative path to
avoid breaking non-local deployments. Replace the default value with something
like: use process.env.REACT_APP_UPLOAD_API_URL (or your app's config import) and
if that's undefined default to '/upload'; ensure the prop defaulting logic uses
that env/config value and update any type if necessary.
67afdbc to
a9888e7
Compare
avirupsinha12
force-pushed
the
Workflow-Builder-UI-Integration-xyne
branch
from
a9888e7 to
609f1fa
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
frontend/src/routes/_authenticated/workflow.tsx (1)
51-54: Do not hard-code userId; use the authenticated user or session.This is a correctness and security risk; prior reviews flagged the same issue.
- body: JSON.stringify({ - userId: 2 - }) + credentials: 'include', + body: JSON.stringify({ + userId: user?.id, // or omit and rely on server session + })frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
176-232: Reduce inline styles; prefer Tailwind utilities for consistency.Large inline blocks make theming hard; move to Tailwind classes (padding, borders, typography) per design system.
71-79: Fix timeout typing and avoid throwing inside setTimeout (browser-safe).Throwing inside the timer creates an unhandled exception; NodeJS.Timeout is incorrect in browser TS setups.
- let timeoutId: NodeJS.Timeout; + let timeoutId: ReturnType<typeof setTimeout> | undefined; ... - timeoutId = setTimeout(() => { - eventSource.close(); - setIsEnhancingPrompt(false); - throw new Error('Request timeout'); - }, 30000); // 30 second timeout + timeoutId = window.setTimeout(() => { + eventSource.close(); + setIsEnhancingPrompt(false); + console.warn('Enhance system prompt request timed out'); + }, 30_000); // 30s timeout
1-1: Close EventSource on unmount and when leaving the panel (prevent leaks).Store the SSE in a ref; close and null it on unmount, on error, on end, and on back/close.
- import React, { useState } from 'react'; + import React, { useEffect, useRef, useState } from 'react';const [isEnhancingPrompt, setIsEnhancingPrompt] = useState(false); + const eventSourceRef = useRef<EventSource | null>(null); + useEffect(() => { + return () => { + eventSourceRef.current?.close(); + eventSourceRef.current = null; + }; + }, []);- const eventSource = new EventSource(url); + const eventSource = new EventSource(url.toString()); + eventSourceRef.current = eventSource;- clearTimeout(timeoutId); - eventSource.close(); + clearTimeout(timeoutId); + eventSource.close(); + eventSourceRef.current = null; const finalPrompt = data.fullPrompt || enhancedPrompt.trim();- eventSource.onerror = (error) => { + eventSource.onerror = (error) => { console.error('EventSource error:', error); clearTimeout(timeoutId); - eventSource.close(); + eventSource.close(); + eventSourceRef.current = null; setIsEnhancingPrompt(false);- <button - onClick={onBack} + <button + onClick={() => { eventSourceRef.current?.close(); eventSourceRef.current = null; onBack(); }} className="flex items-center justify-center"- <button - onClick={onBack} + <button + onClick={() => { eventSourceRef.current?.close(); eventSourceRef.current = null; onBack(); }} className="flex items-center justify-center"Also applies to: 33-36, 69-69, 89-90, 108-139, 186-197, 218-229
frontend/src/components/workflow/WorkflowBuilder.tsx (4)
151-152: Remove unsafe any-casts when reading step.config.Guard config shape instead of (step as any).
- AI agent to analyze and summarize documents using {(step as any).config?.model || 'gpt-oss-120b'}. + AI agent to analyze and summarize documents using {((step.config as { model?: string } | undefined)?.model) || 'gpt-oss-120b'}.- Send emails to {(step as any).config?.emailAddresses?.[0] || 'specified recipients'} via automated workflow. + Send emails to {Array.isArray((step.config as { emailAddresses?: string[] } | undefined)?.emailAddresses) ? (step.config as { emailAddresses?: string[] })!.emailAddresses![0] : 'specified recipients'} via automated workflow.Also applies to: 266-267
95-208: DRY up duplicated node renderers and inline styles.Extract a reusable NodeCard and per-type components (AI, Email, Form) with shared structure; replace inline styles with Tailwind.
Also applies to: 211-323, 326-438
799-805: Make templateId configurable and breadcrumb title dynamic (remove hardcoding).Avoid hardcoded IDs and strings; pass via props/env and render dynamically.
interface WorkflowBuilderProps { flow?: Flow | TemplateFlow; activeStepId?: string; onStepClick?: (step: Step) => void; user?: UserDetail; onBackToWorkflows?: () => void; + templateId?: string; + workflowName?: string; }-const WorkflowBuilderInternal: React.FC<WorkflowBuilderProps> = ({ - onStepClick, - onBackToWorkflows, -}) => { +const WorkflowBuilderInternal: React.FC<WorkflowBuilderProps> = ({ + onStepClick, + onBackToWorkflows, + templateId, +}) => {- const templateId = 'a50e8400-e29b-41d4-a716-446655440010'; + const id = templateId || import.meta.env.VITE_DEFAULT_WORKFLOW_TEMPLATE_ID; + if (!id) return; ... - const response = await workflowTemplatesAPI.fetchById(templateId); + const response = await workflowTemplatesAPI.fetchById(id);-const Header = ({ onBackToWorkflows }: { onBackToWorkflows?: () => void }) => { +const Header = ({ onBackToWorkflows, workflowName }: { onBackToWorkflows?: () => void; workflowName?: string }) => {- <span className='text-[#3B4145] text-sm font-medium leading-5'> / Untitled Workflow</span> + <span className='text-[#3B4145] text-sm font-medium leading-5'> / {workflowName || 'Untitled Workflow'}</span>- <Header onBackToWorkflows={onBackToWorkflows} /> + <Header onBackToWorkflows={onBackToWorkflows} workflowName={props.workflowName} />Also applies to: 808-812, 845-845, 555-556, 568-568, 1609-1610
1477-1479: Remove hardcoded personal email (PII leakage).Do not commit real addresses; externalize to config/env with a safe fallback.
- emailAddresses: ['arnabdebnath@juspay.in'] + emailAddresses: [import.meta.env.VITE_DEFAULT_EMAIL ?? 'default@example.com']
🧹 Nitpick comments (10)
frontend/src/components/workflow/WorkflowIcons.tsx (3)
164-165: Use currentColor instead of hard-coded fills for theming and dark mode compatibility.Replace fixed fills (#181B1D, black, #395A0C) with currentColor so icons inherit text color.
- <path fillRule="evenodd" clipRule="evenodd" d="M20.958 10.9995H7.38002L12.422 5.97852L11.011 4.56152L3.54102 12.0005L11.011 19.4385L12.422 18.0215L7.37802 12.9995H20.958V10.9995Z" fill="#181B1D"/> + <path fillRule="evenodd" clipRule="evenodd" d="M20.958 10.9995H7.38002L12.422 5.97852L11.011 4.56152L3.54102 12.0005L11.011 19.4385L12.422 18.0215L7.37802 12.9995H20.958V10.9995Z" fill="currentColor"/>- <path fillRule="evenodd" clipRule="evenodd" d="M13.4142 12.0002L18.7072 6.70725C19.0982 6.31625 19.0982 5.68425 18.7072 5.29325C18.3162 4.90225 17.6842 4.90225 17.2932 5.29325L12.0002 10.5862L6.70725 5.29325C6.31625 4.90225 5.68425 4.90225 5.29325 5.29325C4.90225 5.68425 4.90225 6.31625 5.29325 6.70725L10.5862 12.0002L5.29325 17.2933C4.90225 17.6842 4.90225 18.3162 5.29325 18.7072C5.48825 18.9022 5.74425 19.0002 6.00025 19.0002C6.25625 19.0002 6.51225 18.9022 6.70725 18.7072L12.0002 13.4143L17.2932 18.7072C17.4882 18.9022 17.7442 19.0002 18.0002 19.0002C18.2562 19.0002 18.5122 18.9022 18.7072 18.7072C19.0982 18.3162 19.0982 17.6842 18.7072 17.2933L13.4142 12.0002Z" fill="black"/> + <path fillRule="evenodd" clipRule="evenodd" d="M13.4142 12.0002L18.7072 6.70725C19.0982 6.31625 19.0982 5.68425 18.7072 5.29325C18.3162 4.90225 17.6842 4.90225 17.2932 5.29325L12.0002 10.5862L6.70725 5.29325C6.31625 4.90225 5.68425 4.90225 5.29325 5.29325C4.90225 5.68425 4.90225 6.31625 5.29325 6.70725L10.5862 12.0002L5.29325 17.2933C4.90225 17.6842 4.90225 18.3162 5.29325 18.7072C5.48825 18.9022 5.74425 19.0002 6.00025 19.0002C6.25625 19.0002 6.51225 18.9022 6.70725 18.7072L12.0002 13.4143L17.2932 18.7072C17.4882 18.9022 17.7442 19.0002 18.0002 19.0002C18.2562 19.0002 18.5122 18.9022 18.7072 18.7072C19.0982 18.3162 19.0982 17.6842 18.7072 17.2933L13.4142 12.0002Z" fill="currentColor"/>- <path d="M10.794 1.33301C12.8533 1.33301 14 2.51967 14 4.55301V11.4397C14 13.5063 12.8533 14.6663 10.794 14.6663H5.20667C3.18 14.6663 2 13.5063 2 11.4397V4.55301C2 2.51967 3.18 1.33301 5.20667 1.33301H10.794ZM5.38667 10.493C5.18667 10.473 4.99333 10.5663 4.88667 10.7397C4.78 10.9063 4.78 11.1263 4.88667 11.2997C4.99333 11.4663 5.18667 11.5663 5.38667 11.5397H10.6133C10.8793 11.513 11.08 11.2857 11.08 11.0197C11.08 10.7463 10.8793 10.5197 10.6133 10.493H5.38667ZM10.6133 7.45234H5.38667C5.09933 7.45234 4.86667 7.68634 4.86667 7.97301C4.86667 8.25967 5.09933 8.49301 5.38667 8.49301H10.6133C10.9 8.49301 11.1333 8.25967 11.1333 7.97301C11.1333 7.68634 10.9 7.45234 10.6133 7.45234ZM7.37933 4.43301H5.38667V4.43967C5.09933 4.43967 4.86667 4.67301 4.86667 4.95967C4.86667 5.24634 5.09933 5.47967 5.38667 5.47967H7.37933C7.66667 5.47967 7.9 5.24634 7.9 4.95234C7.9 4.66634 7.66667 4.43301 7.37933 4.43301Z" fill="#395A0C"/> + <path d="M10.794 1.33301C12.8533 1.33301 14 2.51967 14 4.55301V11.4397C14 13.5063 12.8533 14.6663 10.794 14.6663H5.20667C3.18 14.6663 2 13.5063 2 11.4397V4.55301C2 2.51967 3.18 1.33301 5.20667 1.33301H10.794ZM5.38667 10.493C5.18667 10.473 4.99333 10.5663 4.88667 10.7397C4.78 10.9063 4.78 11.1263 4.88667 11.2997C4.99333 11.4663 5.18667 11.5663 5.38667 11.5397H10.6133C10.8793 11.513 11.08 11.2857 11.08 11.0197C11.08 10.7463 10.8793 10.5197 10.6133 10.493H5.38667ZM10.6133 7.45234H5.38667C5.09933 7.45234 4.86667 7.68634 4.86667 7.97301C4.86667 8.25967 5.09933 8.49301 5.38667 8.49301H10.6133C10.9 8.49301 11.1333 8.25967 11.1333 7.97301C11.1333 7.68634 10.9 7.45234 10.6133 7.45234ZM7.37933 4.43301H5.38667V4.43967C5.09933 4.43967 4.86667 4.67301 4.86667 4.95967C4.86667 5.24634 5.09933 5.47967 5.38667 5.47967H7.37933C7.66667 5.47967 7.9 5.24634 7.9 4.95234C7.9 4.66634 7.66667 4.43301 7.37933 4.43301Z" fill="currentColor"/>Also applies to: 171-171, 136-136
3-7: Add a11y defaults and pass-through SVG props.Decorative icons should be aria-hidden by default; also accept standard SVG props and forward them.
-interface IconProps { - className?: string; - width?: number; - height?: number; -} +type IconProps = React.SVGProps<SVGSVGElement> & { + className?: string + width?: number + height?: number + title?: string +}Example update (apply pattern across icons):
-<svg className={className} width={width} height={height} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2"> +<svg + className={className} + width={width} + height={height} + viewBox="0 0 24 24" + fill="none" + stroke="currentColor" + strokeWidth="2" + aria-hidden={!title} + focusable="false" + {...(title ? {} : { role: 'img' })} +> + {title ? <title>{title}</title> : null}Also applies to: 10-15, 32-37, 39-45, 47-53, 55-61, 63-69, 71-77, 79-85, 87-93, 100-107, 108-116, 118-124, 126-139, 141-146, 148-153, 155-160, 162-167, 169-173
10-15: Avoid conflicting sizing: prefer CSS (className) over width/height.Setting both width/height and Tailwind w-/h- classes can conflict. Standardize on className-based sizing and drop width/height props (or vice versa).
Also applies to: 46-53, 63-69, 71-77, 79-85, 87-93
frontend/src/components/workflow/Types.ts (3)
68-70: Type Step.status as Status for consistency.Narrowing prevents invalid states and aligns with the Status union.
- status?: string + status?: Status
74-77: Replace any with safer shapes (Record<string, unknown>/unknown).Improves type-safety without breaking flexibility.
- metadata?: any | null - config?: any | null + metadata?: Record<string, unknown> | null + config?: Record<string, unknown> | null- metadata: { + metadata: { step_order?: number description?: string user_action?: string tool_type?: string value_type?: string - [key: string]: any + [key: string]: unknown }-export type ToolConfig = { +export type ToolConfig = { unit?: string description?: string url?: string webhook_type?: string message?: string webhook_url?: string - [key: string]: any + [key: string]: unknown }-export type ToolValue = { +export type ToolValue = { script?: string message?: string webhook_url?: string - [key: string]: any + [key: string]: unknown }- config?: { + config?: { complexity?: string estimatedDuration?: string - [key: string]: any + [key: string]: unknown }Also applies to: 94-100, 151-157, 160-166, 168-177, 227-248
118-127: Duplicate “next step” fields can desync. Confirm both are required.Both next_step_ids and nextStepIds exist; if only one is needed, remove the other or map at the boundary.
frontend/src/routes/_authenticated/workflow.tsx (2)
80-82: Hook deps: refetch when user changes; hasFetched may mask updates.Track user.id in the effect (or remove hasFetched and memoize fetchWorkflows).
- useEffect(() => { - fetchWorkflows() - }, []) + useEffect(() => { + fetchWorkflows() + }, [user?.id])
283-299: Minor UX: reduce skeleton layout shift and gate console logs in prod.Consider fixed card heights for skeletons and wrap console.log behind NODE_ENV checks.
Also applies to: 300-305, 306-309
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
47-49: Avoid alert(); surface errors via toast or inline state.Replace window.alert with your app’s toast/toaster or an inline error message.
Also applies to: 162-163
frontend/src/components/workflow/WorkflowBuilder.tsx (1)
1141-1146: Type the CustomEvent instead of casting to any.Declare a typed event detail to avoid any-casts.
// e.g., globally type OpenWhatHappensNextDetail = { nodeId: string }; declare global { interface WindowEventMap { openWhatHappensNext: CustomEvent<OpenWhatHappensNextDetail>; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (10)
frontend/src/assets/arrow-left.svgis excluded by!**/*.svgfrontend/src/assets/bot-logo.svgis excluded by!**/*.svgfrontend/src/assets/check-circle.svgis excluded by!**/*.svgfrontend/src/assets/file-up.svgis excluded by!**/*.svgfrontend/src/assets/grid-dashboard-01.svgis excluded by!**/*.svgfrontend/src/assets/import-dsl.svgis excluded by!**/*.svgfrontend/src/assets/play.svgis excluded by!**/*.svgfrontend/src/assets/plus.svgis excluded by!**/*.svgfrontend/src/assets/sitemap.svgis excluded by!**/*.svgfrontend/src/assets/vector.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
frontend/package.json(3 hunks)frontend/src/components/Sidebar.tsx(2 hunks)frontend/src/components/workflow/AIAgentConfigUI.tsx(1 hunks)frontend/src/components/workflow/ActionBar.tsx(1 hunks)frontend/src/components/workflow/Default.ts(1 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(1 hunks)frontend/src/components/workflow/Types.ts(1 hunks)frontend/src/components/workflow/WhatHappensNextUI.tsx(1 hunks)frontend/src/components/workflow/WorkflowBuilder.tsx(1 hunks)frontend/src/components/workflow/WorkflowCard.tsx(1 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(1 hunks)frontend/src/components/workflow/WorkflowIcons.tsx(1 hunks)frontend/src/components/workflow/WorkflowProvider.tsx(1 hunks)frontend/src/components/workflow/WorkflowUtils.ts(1 hunks)frontend/src/components/workflow/api/ApiHandlers.ts(1 hunks)frontend/src/routeTree.gen.ts(13 hunks)frontend/src/routes/_authenticated/workflow.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- frontend/src/components/workflow/Default.ts
- frontend/src/components/workflow/WorkflowProvider.tsx
- frontend/src/components/workflow/api/ApiHandlers.ts
- frontend/src/components/workflow/WorkflowExecutionModal.tsx
- frontend/src/components/workflow/WorkflowUtils.ts
- frontend/package.json
- frontend/src/components/workflow/ActionBar.tsx
- frontend/src/components/workflow/EmailConfigUI.tsx
- frontend/src/components/workflow/WorkflowCard.tsx
- frontend/src/components/workflow/WhatHappensNextUI.tsx
- frontend/src/components/Sidebar.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/workflow/WorkflowBuilder.tsx (5)
frontend/src/components/workflow/Types.ts (6)
Tool(56-61)Step(64-92)Flow(160-165)TemplateFlow(95-99)UserDetail(1-6)StepExecution(102-127)frontend/src/components/workflow/WorkflowIcons.tsx (5)
DelayIcon(10-15)PythonScriptIcon(17-22)DefaultToolIcon(24-29)FormDocumentIcon(126-138)FormSubmissionIcon(78-85)frontend/src/components/workflow/api/ApiHandlers.ts (2)
workflowTemplatesAPI(50-68)workflowsAPI(71-97)frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
AIAgentConfig(14-21)frontend/src/components/workflow/EmailConfigUI.tsx (1)
EmailConfig(13-16)
frontend/src/routes/_authenticated/workflow.tsx (2)
frontend/src/components/Sidebar.tsx (1)
Sidebar(38-344)frontend/src/components/workflow/WorkflowCard.tsx (1)
WorkflowCard(24-80)
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
frontend/src/routes/_authenticated/search.tsx (1)
Search(82-601)
🔇 Additional comments (1)
frontend/src/routeTree.gen.ts (1)
64-69: LGTM: workflow route wiring is consistent with existing pattern.Generated file; no manual edits needed.
Also applies to: 263-269, 392-416, 449-449, 475-476, 503-504, 532-533, 557-558, 638-690
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
frontend/src/components/workflow/EmailConfigUI.tsx (2)
26-34: Validate, normalize, and dedupe emails (case-insensitive).Prevents invalid/duplicate entries and downstream send errors.
- const handleAddEmail = () => { - if (newEmailAddress && !emailConfig.emailAddresses.includes(newEmailAddress)) { - setEmailConfig(prev => ({ - ...prev, - emailAddresses: [...prev.emailAddresses, newEmailAddress] - })); - setNewEmailAddress(''); - } - }; + const handleAddEmail = () => { + const email = newEmailAddress.trim().toLowerCase(); + if (!email) return; + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(email)) return; + const existing = emailConfig.emailAddresses.map(e => e.toLowerCase()); + if (existing.includes(email)) return; + setEmailConfig(prev => ({ + ...prev, + emailAddresses: [...prev.emailAddresses, email] + })); + setNewEmailAddress(''); + };
58-115: Replace header inline styles with Tailwind for consistency.- <div - className="flex items-center border-b" - style={{ - display: 'flex', - padding: '20px', - alignItems: 'center', - gap: '10px', - alignSelf: 'stretch', - borderBottom: '1px solid var(--gray-300, #E4E6E7)' - }} - > + <div className="flex items-center border-b px-5 py-5 gap-2"> @@ - <button - onClick={onBack} - className="flex items-center justify-center" - style={{ - width: '24px', - height: '24px', - padding: '0', - border: 'none', - background: 'transparent', - cursor: 'pointer' - }} - > + <button onClick={onBack} className="flex items-center justify-center w-6 h-6 p-0 bg-transparent cursor-pointer"> <ArrowLeft className="w-5 h-5 text-gray-600" /> </button> @@ - <h2 - className="flex-1" - style={{ - alignSelf: 'stretch', - color: 'var(--gray-900, #181B1D)', - fontFamily: 'Inter', - fontSize: '16px', - fontStyle: 'normal', - fontWeight: '600', - lineHeight: 'normal', - letterSpacing: '-0.16px', - textTransform: 'capitalize' - }} - > + <h2 className="flex-1 text-gray-900 text-base font-semibold"> Email </h2> @@ - <button - onClick={onBack} - className="flex items-center justify-center" - style={{ - width: '24px', - height: '24px', - padding: '0', - border: 'none', - background: 'transparent', - cursor: 'pointer' - }} - > + <button onClick={onBack} className="flex items-center justify-center w-6 h-6 p-0 bg-transparent cursor-pointer"> <X className="w-5 h-5 text-gray-600" /> </button>frontend/src/components/workflow/AIAgentConfigUI.tsx (2)
175-232: Replace header inline styles with Tailwind; avoid accidental “AI” → “Ai”.- <div - className="flex items-center border-b" - style={{ - display: 'flex', - padding: '20px', - alignItems: 'center', - gap: '10px', - alignSelf: 'stretch', - borderBottom: '1px solid var(--gray-300, #E4E6E7)' - }} - > + <div className="flex items-center border-b px-5 py-5 gap-2"> @@ - <h2 - className="flex-1" - style={{ - alignSelf: 'stretch', - color: 'var(--gray-900, #181B1D)', - fontFamily: 'Inter', - fontSize: '16px', - fontStyle: 'normal', - fontWeight: '600', - lineHeight: 'normal', - letterSpacing: '-0.16px', - textTransform: 'capitalize' - }} - > + <h2 className="flex-1 text-gray-900 text-base font-semibold"> AI Agent </h2>
1-1: Close SSE on unmount and when leaving the panel.-import React, { useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; @@ const [isEnhancingPrompt, setIsEnhancingPrompt] = useState(false); + const eventSourceRef = useRef<EventSource | null>(null); + useEffect(() => { + return () => { + eventSourceRef.current?.close(); + eventSourceRef.current = null; + }; + }, []); @@ - const eventSource = new EventSource(url); + const eventSource = new EventSource(url.toString()); + eventSourceRef.current = eventSource; @@ - eventSource.onerror = (error) => { + eventSource.onerror = (error) => { console.error('EventSource error:', error); clearTimeout(timeoutId); - eventSource.close(); + eventSource.close(); + eventSourceRef.current = null; setIsEnhancingPrompt(false); @@ - <button - onClick={onBack} + <button + onClick={() => { eventSourceRef.current?.close(); eventSourceRef.current = null; onBack(); }} className="flex items-center justify-center" @@ - <button - onClick={onBack} + <button + onClick={() => { eventSourceRef.current?.close(); eventSourceRef.current = null; onBack(); }} className="flex items-center justify-center"Also applies to: 33-36, 108-139, 186-199, 218-231
frontend/src/routes/_authenticated/workflow.tsx (1)
41-78: Add user guard, request timeout/abort, and cookie credentials; allow retry on failurePrevents hung requests, avoids API calls without a user, and supports session cookies. Also reset hasFetched on failures so users can retry.
const fetchWorkflows = async () => { - if (hasFetched.current) return - hasFetched.current = true + if (hasFetched.current) return + if (!user?.id) { + console.warn('No authenticated user; skipping workflows fetch') + return + } + hasFetched.current = true try { setLoading(true) - const response = await fetch('/user/fetch/workflows', { + const ac = new AbortController() + const timeout = setTimeout(() => ac.abort(), 10000) + const response = await fetch('/user/fetch/workflows', { method: 'POST', headers: { 'Content-Type': 'application/json', }, + credentials: 'include', + signal: ac.signal, body: JSON.stringify({ userId: user?.id, // or omit and rely on session on the server }) }) if (response.ok) { const data = await response.json() - console.log('API Response:', data) // Handle different possible response structures if (data.workflows && Array.isArray(data.workflows)) { setWorkflows(data.workflows) } else if (Array.isArray(data)) { setWorkflows(data) } else { console.error('Unexpected response structure:', data) setWorkflows([]) + hasFetched.current = false } } else { console.error('API response not ok:', response.status, response.statusText) setWorkflows([]) + hasFetched.current = false } } catch (error) { console.error('Failed to fetch workflows:', error) setWorkflows([]) + hasFetched.current = false } finally { setLoading(false) + try { clearTimeout(timeout) } catch {} } }
🧹 Nitpick comments (26)
frontend/src/components/workflow/EmailConfigUI.tsx (3)
43-47: Prevent default on Enter to avoid unintended form submits.- if (e.key === 'Enter') { - handleAddEmail(); - } + if (e.key === 'Enter') { + e.preventDefault(); + handleAddEmail(); + }
125-132: Disabled input shouldn’t wire onChange; fix placeholder copy.- <Input + <Input id="sending-from" value={emailConfig.sendingFrom} - onChange={(e) => setEmailConfig(prev => ({ ...prev, sendingFrom: e.target.value }))} - placeholder="Enter sender email" + placeholder="Sender email" className="w-full bg-gray-100" disabled />
175-176: Use stable key; avoid index.- <div key={index} className="flex items-center justify-between p-3 bg-gray-50 rounded-lg"> + <div key={email} className="flex items-center justify-between p-3 bg-gray-50 rounded-lg">frontend/src/components/workflow/WorkflowCard.tsx (3)
40-40: Avoid inline width; use Tailwind for consistency/responsiveness.- <div className="bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 hover:shadow-md transition-shadow rounded-2xl p-6 flex flex-col justify-between h-52" style={{ width: '327px' }}> + <div className="bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 hover:shadow-md transition-shadow rounded-2xl p-6 flex flex-col justify-between h-52 w-[327px]">
65-67: Wire “View” action or remove until implemented.If ready, navigate to a workflow details route (e.g., /workflow/:id) or accept an onView prop to decouple. I can draft the integration.
26-37: Locale-dependent date rendering; consider deterministic formatting.Use date-fns/dayjs to render “MM/dd/yyyy HH:mm” consistently across clients.
frontend/src/components/workflow/WorkflowExecutionModal.tsx (3)
44-46: Remove unused upload state; drop setter calls.These flags aren’t read; simplify state and side effects.
- const [, setIsUploading] = useState(false) - const [, setIsUploaded] = useState(false) @@ - setIsUploading(false) - setIsUploaded(false) @@ - setSelectedFile(file) - setIsUploaded(false) + setSelectedFile(file) @@ - setIsUploading(true) + // begin upload @@ - setIsUploaded(true) + // upload succeeded; begin polling @@ - setIsUploaded(false) setIsProcessing(false) @@ - setIsUploaded(false) setIsProcessing(false) @@ - setIsUploading(false) + // done @@ - setSelectedFile(file) - setIsUploaded(false) + setSelectedFile(file) @@ - setSelectedFile(null) - setIsUploaded(false) + setSelectedFile(null)Also applies to: 54-55, 99-101, 103-128, 141-141, 170-171, 122-123, 126-126
1-1: Ensure polling is cleared on unmount.-import { useState, useRef } from "react" +import { useEffect, useState, useRef } from "react" @@ const resetModalState = () => { @@ } + + // Clear polling if component unmounts while active + useEffect(() => { + return () => { + if (stopPolling) stopPolling() + } + }, [stopPolling])Also applies to: 67-67
20-25: Minor: ‘image/jpg’ isn’t a standard MIME; ‘image/jpeg’ covers JPGs.No action needed, but you can drop the non-standard entry.
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
45-49: Avoid alert(); use your app’s toast/snackbar.Keeps UX consistent and non-blocking.
Also applies to: 162-163
frontend/src/routes/_authenticated/workflow.tsx (5)
57-57: Remove debug log or guard behind dev flagAvoid noisy console in production.
- console.log('API Response:', data)
333-335: Refresh list after exiting builderEnsure newly created workflows appear without a full page reload.
- <WorkflowBuilder - user={user} - onBackToWorkflows={() => setViewMode("list")} - /> + <WorkflowBuilder + user={user} + onBackToWorkflows={() => { + setViewMode("list") + hasFetched.current = false + fetchWorkflows() + }} + />
41-55: Prefer shared API wrapper for consistencyRoute uses raw fetch; consider delegating to a typed API client (e.g., apiRequest) for uniform headers, error parsing, and timeouts.
33-35: Safer user extractionDirectly indexing matches can break if route tree changes. Prefer this route’s context hook or a selector to the exact context shape.
Example:
// const { user } = useRouteContext({ from: '/_authenticated/workflow' })
202-269: Disabled placeholders should communicate intentThe “Templates” and “Import DSL file” cards are clickable but do nothing. Either wire actions or make them aria-disabled with a tooltip.
frontend/src/components/workflow/api/ApiHandlers.ts (2)
120-131: Avoid fragile this-binding in startPollingUsing this can break if the method is destructured. Reference the API object directly.
- const response = await this.pollProcessStatus(); + const response = await workflowsAPI.pollProcessStatus();
55-74: Consider centralizing list/fetch endpoints hereExpose a typed list endpoint (e.g., workflowsAPI.listByUser or listMine) used by the route, keeping fetch logic, headers, and timeouts consistent.
Also applies to: 76-112, 149-181
frontend/src/components/workflow/OnFormSubmissionUI.tsx (9)
74-81: Type the icon helper and show distinct icons per type.Improves clarity and future-proofing.
-const getFieldTypeIcon = (type: string) => { +const getFieldTypeIcon = (type: FormField['type']) => { switch (type) { - case 'file': - return <FileText className="w-4 h-4" />; - default: - return <FileText className="w-4 h-4" />; + case 'file': + return <FileText className="w-4 h-4" />; + case 'text': + return <Type className="w-4 h-4" />; + case 'email': + return <Mail className="w-4 h-4" />; + case 'number': + return <Hash className="w-4 h-4" />; + case 'textarea': + return <AlignLeft className="w-4 h-4" />; + default: + return <FileText className="w-4 h-4" />; } };Add imports:
-import { FileText, ChevronDown } from "lucide-react"; +import { FileText, ChevronDown, Mail, Type, Hash, AlignLeft } from "lucide-react";
9-13: Close button should not trigger back; add onClose and aria labels.Wire close to a dedicated optional handler and add accessible labels.
interface OnFormSubmissionUIProps { onBack: () => void; + onClose?: () => void; onSave?: (formConfig: FormConfig) => void; initialConfig?: FormConfig; }- <button - onClick={onBack} + <button + onClick={onClose ?? onBack} + aria-label="Close panel" className="flex items-center justify-center"Also applies to: 129-143
97-111: Add aria-label to the back button.Improves screen-reader usability.
- <button - onClick={onBack} + <button + onClick={onBack} + aria-label="Go back" className="flex items-center justify-center"
189-200: Make the collapsible header a real button.Use a button for keyboard accessibility instead of a clickable div.
- <div - className="flex items-center justify-between p-3 cursor-pointer hover:bg-gray-50" + <button + type="button" + className="flex w-full items-center justify-between p-3 hover:bg-gray-50" onClick={() => setCollapsedFieldIds(prev => { const newSet = new Set(prev); if (newSet.has(field.id)) { newSet.delete(field.id); // Expand (remove from collapsed) } else { newSet.add(field.id); // Collapse (add to collapsed) } return newSet; })} > ... - </div> + </button>
216-226: Link labels to inputs/selects with ids.Small a11y improvement.
- <Label className="text-sm font-medium text-slate-700"> + <Label htmlFor={`${field.id}-name`} className="text-sm font-medium text-slate-700"> Field Name </Label> <Input + id={`${field.id}-name`} value={field.name} onChange={(e) => updateField(field.id, { name: e.target.value })}- <Label className="text-sm font-medium text-slate-700"> + <Label htmlFor={`${field.id}-placeholder`} className="text-sm font-medium text-slate-700"> Placeholder Text </Label> <Input + id={`${field.id}-placeholder`} value={field.placeholder} onChange={(e) => updateField(field.id, { placeholder: e.target.value })}- <Label className="text-sm font-medium text-slate-700"> + <Label htmlFor={`${field.id}-type`} className="text-sm font-medium text-slate-700"> Element Type </Label> ... - <select + <select + id={`${field.id}-type`} value={field.type}Also applies to: 229-239, 243-255, 247-255
286-295: Disable Save when config is invalid.Prevents empty titles or unnamed fields from being saved.
- <Button - onClick={handleSave} - className="w-full bg-blue-600 hover:bg-blue-700 text-white" - > + <Button + onClick={handleSave} + disabled={!isValidConfig} + className="w-full bg-blue-600 hover:bg-blue-700 text-white disabled:opacity-50 disabled:cursor-not-allowed" + >Add this near the top (after collapsedFieldIds):
+ const isValidConfig = + formConfig.title.trim().length > 0 && + formConfig.fields.length > 0 && + formConfig.fields.every(f => f.name.trim().length > 0);
15-20: Export FormField for external typing.Helps parent components type initialConfig or onSave payloads.
-interface FormField { +export interface FormField { id: string; name: string; placeholder: string; type: 'text' | 'email' | 'file' | 'number' | 'textarea'; }
86-125: Style consistency: prefer Tailwind over inline styles.Optional cleanup to align with the rest of the codebase and reduce CSS-in-JS overhead.
179-283: Confirm whether an “Add Field” action is intentionally omitted.If not, a small addField helper and button would complete the UX for multi-field forms. Happy to propose a patch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
frontend/src/components/workflow/AIAgentConfigUI.tsx(1 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(1 hunks)frontend/src/components/workflow/OnFormSubmissionUI.tsx(1 hunks)frontend/src/components/workflow/WorkflowBuilder.tsx(1 hunks)frontend/src/components/workflow/WorkflowCard.tsx(1 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(1 hunks)frontend/src/components/workflow/WorkflowUtils.ts(1 hunks)frontend/src/components/workflow/api/ApiHandlers.ts(1 hunks)frontend/src/routes/_authenticated/workflow.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/workflow/WorkflowBuilder.tsx
- frontend/src/components/workflow/WorkflowUtils.ts
🧰 Additional context used
🧬 Code graph analysis (6)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (1)
frontend/src/components/workflow/WorkflowIcons.tsx (2)
BackArrowIcon(162-166)CloseIcon(169-173)
frontend/src/components/workflow/WorkflowCard.tsx (1)
frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
WorkflowExecutionModal(37-385)
frontend/src/routes/_authenticated/workflow.tsx (2)
frontend/src/components/Sidebar.tsx (1)
Sidebar(38-344)frontend/src/components/workflow/WorkflowCard.tsx (1)
WorkflowCard(23-78)
frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
frontend/src/components/workflow/api/ApiHandlers.ts (3)
uploadFile(152-181)workflowsAPI(77-182)startPolling(120-147)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
frontend/src/components/workflow/Types.ts (2)
TemplateFlow(95-99)Flow(160-165)
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
frontend/src/routes/_authenticated/search.tsx (1)
Search(82-601)
🔇 Additional comments (5)
frontend/src/components/workflow/WorkflowExecutionModal.tsx (2)
35-35: Confirm 40MB limit matches backend.If the server enforces a different cap, surface the exact limit to avoid UX mismatch. I can align constants once confirmed.
199-384: Good separation of Upload → Processing → Completion; API client usage looks solid.frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
1-1: Auth for SSE?Project’s search route uses an authenticated EventSource helper. If this endpoint requires auth, reuse that helper to avoid 401/CORS. I can wire it if you point me to the helper export.
frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
46-46: LGTM: collapsed state with Set is handled immutably.
1-1: No ‘use client’ needed—this isn’t a Next.js App Router module.
I didn’t find any Next.js dependency or app/pages directory (no next.config.js or “app”/“pages” folders under frontend/), so marking this as a Next.js client component doesn’t apply.Likely an incorrect or invalid review comment.
avirupsinha12
force-pushed
the
Workflow-Builder-UI-Integration-xyne
branch
from
46e92ac to
89adaba
Compare
3 participants
Description
Testing
Additional Notes
Summary by CodeRabbit
New Features
Style
Chores