Add workflow support to BI extension#1498
Conversation
WalkthroughThis change introduces comprehensive workflow support across the Ballerina integration platform. It adds new node kinds (WORKFLOW, ACTIVITY, ACTIVITY_CALL, SEND_EVENT, WAIT_EVENT, WORKFLOW_START), extends RPC infrastructure for workflow event retrieval, enhances form handling for workflow input types, and updates UI components throughout the visualizer and side panel to support workflow and activity management. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes This change spans 33 file groups across multiple architectural layers (types, RPC infrastructure, implementation, UI, forms, diagrams) with significant logic additions. Concerns include duplicate method definitions in extended-language-client.ts and rpc-manager.ts, substantial new form handling and state management in FlowDiagram/index.tsx (+411/-21), complex node rendering logic in BaseNodeWidget.tsx, and extensive side panel UI reorganization. While many changes follow established patterns, the heterogeneous nature and density of new logic across different domains demand careful cross-layer verification. Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGeneratorNew/index.tsx (1)
762-762:⚠️ Potential issue | 🟡 MinorRemove debug
console.log.
console.log("#STACK", stack)on every render of the type helper is noisy. This should be removed before merge.Proposed fix
- console.log("#STACK", stack);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGeneratorNew/index.tsx` at line 762, Remove the debug console.log call that prints the stack on every render — locate the console.log("#STACK", stack) invocation (inside the type helper in the FormGeneratorNew component) and delete it (or replace it with a conditional debug/logging utility if persistent debug output is required), ensuring no stray console logging remains in the render path.workspaces/ballerina/bi-diagram/src/utils/node.ts (1)
87-139:⚠️ Potential issue | 🟡 MinorAvoid “undefined : …” titles when module is missing.
If
node.codedata.moduleis undefined, the current logic will return"undefined : <label>", which is a user-facing glitch. Consider falling back to the label when module is absent.🔧 Proposed fix
- const module = node.codedata.module?.includes(".") + const module = node.codedata.module?.includes(".") ? node.codedata.module.split(".").pop() : node.codedata.module; - return `${module} : ${label}`; + return module ? `${module} : ${label}` : label;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/bi-diagram/src/utils/node.ts` around lines 87 - 139, getNodeTitle can produce titles like "undefined : <label>" when node.codedata.module is missing; change the final return so that if node.codedata.module is falsy you return just label (or `${label}`) instead of `"undefined : ${label}"`. Locate the code using the symbols getNodeTitle, node.codedata.module, and label and update the branch that builds module with module = node.codedata.module?... to ensure you check for a non-empty module (and after extracting the last segment) and only return `${module} : ${label}` when module is defined, otherwise return label.
🧹 Nitpick comments (16)
workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionFormStatic/index.tsx (2)
86-91: Move pure helper to module scope and avoid in-place mutationTwo separate concerns:
Scope:
hideTypeDescriptionFieldcaptures nothing from the component closure (no props, state, or refs). Defining it inside the component body causes it to be re-created on every render. Move it to module scope.Mutation: The function modifies
flowNode.properties.typeDescriptionin-place on the RPC response object. If the same response object is referenced elsewhere (e.g. cached by the RPC layer), the silent mutation could cause unexpected behaviour. Prefer a shallow copy of the affected node.♻️ Proposed refactor
- const hideTypeDescriptionField = (flowNode: FunctionNode): FunctionNode => { - if (flowNode?.properties?.typeDescription) { - flowNode.properties.typeDescription.hidden = true; - } - return flowNode; - };Add at module scope (outside the component):
+function hideTypeDescriptionField(flowNode: FunctionNode): FunctionNode { + if (!flowNode?.properties?.typeDescription) { + return flowNode; + } + return { + ...flowNode, + properties: { + ...flowNode.properties, + typeDescription: { + ...flowNode.properties.typeDescription, + hidden: true, + }, + }, + }; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionFormStatic/index.tsx` around lines 86 - 91, The helper hideTypeDescriptionField currently lives inside the component and mutates the RPC response in-place; move hideTypeDescriptionField to module scope (outside the component) and change its implementation to avoid in-place mutation by returning a new FunctionNode with a shallow copy of properties and a copied typeDescription where hidden is set to true (e.g., clone flowNode and flowNode.properties, and clone/replace flowNode.properties.typeDescription with hidden: true) so callers use the new object without altering the original RPC response.
151-163:hideTypeDescriptionFieldcall ingetFunctionNodeis a no-opAt line 151,
hideTypeDescriptionFieldsetstypeDescription.hidden = trueon the object. However, lines 158–163 immediately rebuildpropertiesviaObject.keys().reduce()that explicitly excludes every key whose lowercase form includes'typedescription'. Line 164 then replacesflowNode.propertieswith this rebuilt object, discarding the mutatedtypeDescriptionentry entirely. Thehidden = trueflag is never observed; the call at line 151 is dead code in this path.In
getExistingFunctionNode(line 205) the helper is effective because no such deletion follows.Either remove the
hideTypeDescriptionFieldcall fromgetFunctionNode(since the deletion already handles hiding), or align both paths to use only the hidden-flag approach and drop thereduceblock.♻️ Suggested cleanup for `getFunctionNode`
- let flowNode = hideTypeDescriptionField(res.flowNode); + let flowNode = res.flowNode; let properties = flowNode.properties as NodeProperties; console.log("FLOWNODE", flowNode) // Remove the description fields from properties properties = Object.keys(properties).reduce((acc, key) => { if (!key.toLowerCase().includes('functionnamedescription') && !key.toLowerCase().includes('typedescription')) { acc[key] = properties[key]; } return acc; }, {} as NodeProperties); flowNode.properties = properties;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionFormStatic/index.tsx` around lines 151 - 163, The call to hideTypeDescriptionField in getFunctionNode is dead because the subsequent Object.keys(...).reduce(...) rebuilds flowNode.properties and drops any key containing 'typedescription', so remove the hideTypeDescriptionField(flowNode) invocation from getFunctionNode and rely on the existing reduce-based exclusion; ensure flowNode.properties remains set from the reduced object and leave getExistingFunctionNode unchanged (or conversely, if you prefer the hidden-flag approach, remove the reduce block and instead set typeDescription.hidden = true in getFunctionNode to match getExistingFunctionNode).workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/EnumEditor/EnumEditor.tsx (1)
34-44: LGTM — the length-check correctly extends fallback coverage to empty arrays.The switch from
??to.length > 0is intentional: the old nullish coalescing only triggered whenprops.itemswasnull/undefined, whereas the new check also falls back tofield.itemOptionswhenitemsis passed as an explicit empty array[]. This is the right semantics here.One low-risk caveat:
.lengthwill throw at runtime ifprops.itemsisundefined(e.g., from a JavaScript caller not enforcing the TypeScript interface). The previous??form was implicitly resilient to that. Since the type already declaresitemsas required (OptionProps[], non-optional), this is not a concern for typed callers, but it's worth keeping in mind if the component is ever called from an untyped context.🛡️ Optional: defensively handle undefined `items` at runtime
- const baseItems = props.items.length > 0 ? props.items : (props.field.itemOptions ?? []); + const baseItems = (props.items?.length > 0 ? props.items : null) ?? (props.field.itemOptions ?? []);Or more readably:
- const baseItems = props.items.length > 0 ? props.items : (props.field.itemOptions ?? []); + const baseItems = props.items?.length > 0 ? props.items : (props.field.itemOptions ?? []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/EnumEditor/EnumEditor.tsx` around lines 34 - 44, The current itemsList computation uses props.items.length which will throw if props.items is undefined; update the useMemo to defensively treat props.items as an array before checking length (e.g., coerce to an empty array or check Array.isArray(props.items)) so it falls back to props.field.itemOptions safely; ensure you still append the default object with id "default-option" and value DEFAULT_NONE_SELECTED_VALUE and keep the dependency array referencing props.items and props.field.itemOptions.workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsx (1)
948-948:data: propslogs the entire component props object — use a targeted log payload.
propsis the fullFormPropsobject (callbacks, all form field definitions, expression editor config, etc.). The key namedataalso conflicts with the localdataparameter (form values), making the log confusing.🪵 Suggested fix
- console.error(">>> Form validation failed, not saving", { data: props, formData: data }); + console.error(">>> Form validation failed, not saving", { node: selectedNode, formData: data });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsx` at line 948, The console.error call is logging the entire props object and using the key name "data" which conflicts with the local data parameter; update the log in the Form component (the console.error call) to emit a targeted payload—rename the conflicting key to formData (or values) and log only relevant props fields (e.g., props.formId, props.fieldDefinitions, or props.onSubmit presence) or a small propsSnapshot object instead of full props so the output is concise and non-sensitive.workspaces/ballerina/ballerina-side-panel/src/components/NodeList/categoryConfig.ts (1)
68-87: LGTM — New entries follow the established pattern precisely.One minor note: the "Create Function" exclusion condition is now a four-clause chain (
!== "Data Mappers" && !==...). As more categories are added over time, this will keep growing. A set-based check would be easier to maintain:♻️ Optional simplification
+const GENERIC_FUNCTION_EXCLUDED = new Set(["Data Mappers", "Natural Functions", "Workflows", "Activities"]); { type: 'function', tooltip: "Create Function", emptyStateLabel: "Create Function", handlerKey: 'onAddFunction', - condition: (title) => title !== "Data Mappers" && title !== "Natural Functions" && title !== "Workflows" && title !== "Activities" + condition: (title) => !GENERIC_FUNCTION_EXCLUDED.has(title) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/NodeList/categoryConfig.ts` around lines 68 - 87, Replace the long chained exclusion in the "Create Function" item's condition with a set-based membership check: locate the object whose properties include type: 'function', tooltip: "Create Function", emptyStateLabel: "Create Function", and handlerKey: 'onAddFunction', then change its condition from a && chain of title !== "Data Mappers" && title !== "Natural Functions" && title !== "Workflows" && title !== "Activities" to using a Set/array of excluded titles (e.g., const excluded = new Set([...])) and return !excluded.has(title) so adding future categories is a single change.workspaces/ballerina/ballerina-visualizer/src/components/EntryPointTypeCreator/index.tsx (1)
32-54: Duplicated styled components — consider extracting to a shared module.
BreadcrumbContainer,BreadcrumbItem, andBreadcrumbSeparatorare defined identically inworkspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx(lines 177–199), and are even exported there. Duplicating them in this file creates a maintenance burden if the design tokens ever change.The cleanest fix is to extract these into a shared breadcrumb styles module (e.g.,
src/components/Breadcrumb/styles.ts) and import from there in both files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/components/EntryPointTypeCreator/index.tsx` around lines 32 - 54, BreadcrumbContainer, BreadcrumbItem, and BreadcrumbSeparator are duplicated here and in FormGenerator; extract these styled components into a single shared module (e.g., create a styles file like src/components/Breadcrumb/styles.ts) and replace the local definitions in EntryPointTypeCreator (and the duplicates in workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx) with imports from that shared module so both files import BreadcrumbContainer, BreadcrumbItem, and BreadcrumbSeparator rather than redefining them; ensure the new module exports those three symbols and update any existing exports/imports accordingly.workspaces/ballerina/ballerina-side-panel/src/components/NodeList/index.tsx (2)
692-720: Duplicated handler-resolution logic.The
handlersmap and thepropsHandler/handlerresolution pattern are copy-pasted in the header actions (lines 692-720) and the empty-state actions (lines 746-769). Extract a small helper to avoid drift between the two copies.Example extraction
+ const resolveActionHandler = (handlerKey: string) => { + const handlers: Record<string, () => void> = { + onAddConnection: handleAddConnection, + onAddFunction: handleAddFunction, + onAdd: handleAdd, + }; + const handler = handlers[handlerKey]; + const propsHandler = props[handlerKey]; + return propsHandler && handler ? handler : null; + };Then replace both inline blocks with a call to
resolveActionHandler(action.handlerKey).Also applies to: 746-769
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/NodeList/index.tsx` around lines 692 - 720, Extract the duplicated handler-resolution logic into a small helper like resolveActionHandler(handlerKey) that encapsulates the handlers map (mapping keys to handleAddConnection, handleAddFunction, handleAdd) and the props lookup (props[handlerKey]) and returns the resolved function or null; then replace both inline blocks in the header actions and empty-state actions with calls to resolveActionHandler(action.handlerKey) and use the returned function (if non-null) for the Button onClick. Ensure the helper is referenced where action.handlerKey is used and preserves the existing e.stopPropagation() and tooltip/addButtonLabel behavior.
644-795: High cognitive complexity ingetCategoryContainerrender block.The IIFE + map block (lines 644-795) handles visibility, separators, headers (fixed vs non-fixed), actions, empty-state, and container selection all in one deeply nested block (~150 lines, ~8 nesting levels). Consider splitting into a
renderCategory(group, index, lastVisibleCategoryTitle)helper to improve readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/NodeList/index.tsx` around lines 644 - 795, The render block inside reorderedGroups.map is too large and complex; extract it into a helper function (e.g., renderCategory(group, index, lastVisibleCategoryTitle)) and move all logic that computes visibility, separators, header selection (isCategoryFixed, HeaderComponent, headerProps, toggleCategory), action handlers (getCategoryActions, handleAddConnection, handleAddFunction, handleAdd, props[action.handlerKey]), empty-state rendering (shouldShowEmptyCategory, categoryActions empty-state mapping), and container selection (shouldUseConnectionContainer, getConnectionContainer, getCategoryContainer, getNodesContainer) into that helper; ensure the helper returns the same JSX and updates lastVisibleCategoryTitle/reads expandedCategories and shouldExpandAll, and then replace the IIFE + map body with a simple call to renderCategory for each group to reduce nesting and cognitive complexity.workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/SendEventForm/index.tsx (2)
352-371: Dual-trigger paths forfetchWorkflowEvents.Both
handleFormChange(line 364) and theuseEffect(line 349) can callfetchWorkflowEventswhen the workflow name changes. TheworkflowRefguard prevents duplicate fetches, but the intent could be clearer. If you adopt the refactoreduseEffectabove (watching only the derived workflow name), you could remove the explicit fetch fromhandleFormChangeentirely and let the state change propagate naturally through the effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/SendEventForm/index.tsx` around lines 352 - 371, The handleFormChange currently duplicates logic by calling fetchWorkflowEvents when the workflow name changes; remove the branch in handleFormChange that checks if (fieldKey === fieldKeys.workflowName) and the internal fetchWorkflowEvents call (and its early return), leaving onChange?.(...) intact and keeping the eventName branch; rely on the existing useEffect that watches the derived/normalized workflow name (and the workflowRef guard) to trigger fetchWorkflowEvents, so the state change from handleFormChange propagates and the effect handles the fetch.
338-350:useEffectdepends onformFields, causing it to re-fire on every field update.This effect re-runs on every
formFieldsstate change (e.g., whenapplySendEventFieldsupdates fields after fetching events). TheworkflowRef.currentguard prevents infinite loops, but the effect still executes its body unnecessarily on every unrelated field change. Consider deriving the workflow name value outside the effect and watching only that derived value.Example: watch only the workflow name value
+ const currentWorkflowName = useMemo(() => { + if (!fieldKeys.workflowName) return ""; + return normalizeStringValue(formFields.find((f) => f.key === fieldKeys.workflowName)?.value); + }, [fieldKeys.workflowName, formFields]); + useEffect(() => { - if (!fieldKeys.workflowName) { + if (!currentWorkflowName || currentWorkflowName === workflowRef.current) { return; } - const workflowName = normalizeStringValue(formFields.find((field) => field.key === fieldKeys.workflowName)?.value); - if (!workflowName || workflowName === workflowRef.current) { - return; - } const preferredEventName = fieldKeys.eventName ? normalizeStringValue(formFields.find((field) => field.key === fieldKeys.eventName)?.value) : ""; - void fetchWorkflowEvents(workflowName, preferredEventName); - }, [fieldKeys, formFields, fetchWorkflowEvents]); + void fetchWorkflowEvents(currentWorkflowName, preferredEventName); + }, [currentWorkflowName, fetchWorkflowEvents]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/SendEventForm/index.tsx` around lines 338 - 350, The useEffect currently lists formFields in its dependency list causing it to run on every field update; compute the normalized workflowName (using normalizeStringValue on formFields.find(... fieldKeys.workflowName)) outside the useEffect and use that derived workflowName in the effect's dependency array instead of formFields, keep the existing workflowRef.current guard, and likewise derive preferredEventName (from fieldKeys.eventName) outside the effect so you can pass only the necessary values and fetchWorkflowEvents into the dependency array to avoid unnecessary re-runs.workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx (3)
284-292: Search withlimit: 1000could be expensive.The
searchcall is a fallback for type resolution, but requesting up to 1000 items for a single type lookup is disproportionate. A smaller limit (e.g., 50-100) with the specific type name inqshould suffice.Proposed fix
const searchResponse = await rpcClient.getBIDiagramRpcClient().search({ filePath, queryMap: { q: workflowInputType, offset: 0, - limit: 1000 + limit: 100 }, searchKind: "TYPE" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx` around lines 284 - 292, The search call using rpcClient.getBIDiagramRpcClient().search with queryMap.q = workflowInputType is using a very large limit (1000) which is expensive; reduce the limit to a smaller reasonable value (e.g., 50 or 100) and ensure q is the specific type name (workflowInputType) so the fallback type lookup still finds matches without fetching excessive items; update the queryMap.limit from 1000 to the chosen smaller number and keep the rest of the search call (filePath, offset, searchKind) unchanged.
72-77:hideTypeDescriptionFieldmutates the input object in place.This mutates
flowNode.properties.typeDescription.hiddendirectly. Compare with the immutable version inFormGenerator/index.tsx(lines 445-455) that returns new objects via spread. The in-place mutation works here since it's called on fresh RPC response objects, but the inconsistency between the two files is a maintenance hazard.Immutable alternative
const hideTypeDescriptionField = (flowNode: FunctionNode): FunctionNode => { - if (flowNode?.properties?.typeDescription) { - flowNode.properties.typeDescription.hidden = true; - } - return flowNode; + if (!flowNode?.properties?.typeDescription) { + return flowNode; + } + return { + ...flowNode, + properties: { + ...flowNode.properties, + typeDescription: { + ...flowNode.properties.typeDescription, + hidden: true, + }, + }, + }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx` around lines 72 - 77, hideTypeDescriptionField currently mutates flowNode.properties.typeDescription.hidden in place; change it to return a new FunctionNode object instead: create a new typeDescription object by spreading flowNode.properties.typeDescription and setting hidden:true, create a new properties object by spreading flowNode.properties and replacing typeDescription with that new object, and finally return a new flowNode by spreading the original flowNode and replacing properties with the new properties object (mirror the immutable pattern used in FormGenerator/index.tsx).
233-342:enrichWorkflowInputTypeModeluses unsafe type casting to mutate properties not defined in the type system.The function directly assigns to
(primaryInputType as any).typeModelat lines 261 and 328, bypassing TypeScript's type checker. ThetypeModelproperty is not defined in theInputTypeinterface or any of its constituent types (BaseType,DropdownType,TemplateType,IdentifierType), making these mutations invisible to static analysis.Consider:
- Extending
BaseTypeto include an optionaltypeModelproperty so mutations are properly typed.- Extracting the two resolution strategies (lines 255–276 via types API and lines 285–325 via search API) into separate helper functions to reduce the function's complexity and improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx` around lines 233 - 342, The function enrichWorkflowInputTypeModel mutates a non‑declared property by assigning (primaryInputType as any).typeModel; add an optional typeModel?: TypeModel (or appropriate interface) to the common base type (e.g., extend BaseType or the shared InputType union) so these assignments are typed, and refactor the two lookup paths into helpers (e.g., resolveTypeFromTypesApi(rpcClient, filePath, workflowInputType, primaryInputType, resolveTypeByLinePosition) and resolveTypeFromSearchApi(rpcClient, filePath, workflowInputType, primaryInputType, resolveTypeByLinePosition)) to encapsulate the getTypes/getType and search logic and call them from enrichWorkflowInputTypeModel (keeping resolveTypeByLinePosition as the small helper it currently is).workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx (1)
1567-1575:EntryPointTypeCreatoris duplicated across two render branches with identical props.The
EntryPointTypeCreatorblock at lines 1567-1575 (insideVARIABLEbranch) and lines 1731-1739 (inside default branch) are identical. Since the component is controlled byisTypeEditorOpenand renders as a modal, it could be hoisted outside both branches (e.g., at the end of theEditorContext.Provider, similar to howFormGeneratorNewhandles it).Also applies to: 1731-1739
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx` around lines 1567 - 1575, The EntryPointTypeCreator modal is duplicated in two render branches; hoist a single instance out of the VARIABLE/default branches so it renders once under the same provider scope (e.g., after EditorContext.Provider similar to FormGeneratorNew). Remove the two occurrences and add one EntryPointTypeCreator that uses the same props: isOpen={isTypeEditorOpen}, onClose={handleTypeEditorClose}, onTypeCreate={handleTypeCreated}, initialTypeName={editingTypeName || "WorkflowInput"}, modalTitle="Define Workflow Input Type", modalWidth={650}, modalHeight={600}, ensuring it remains within the same component so those handlers and state are in scope.workspaces/ballerina/ballerina-visualizer/src/views/BI/DiagramWrapper/index.tsx (1)
255-277: Consider replacing the string-prefix hack with metadata/LS node kind when available.The
@workflow:*string match is brittle to formatting/annotation ordering and could silently mislabel titles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/DiagramWrapper/index.tsx` around lines 255 - 277, Replace the brittle prefix-check in getWorkflowTitleFromSource by first checking for explicit metadata/AST/Language Server properties on parentMetadata (e.g., a workflowKind, annotations array, or nodeKind field) and use those to determine "Workflow Activity" vs "Workflow"; only fall back to examining parentMetadata.sourceCode for `@workflow`:* markers if those structured fields are absent. Update getWorkflowTitleFromSource to read those properties on parentMetadata (and normalize annotation values if needed), and keep getTitle unchanged so it returns the structured workflow title when available.workspaces/ballerina/bi-diagram/src/components/nodes/BaseNode/BaseNodeWidget.tsx (1)
198-234: Consider extracting workflow detection helpers for readability.The deeply nested ternary for
workflowTargetFunctionName(lines 209–218) and the chained fallback forfunctionViewRange(lines 219–226) are correct but dense. Consider extracting a small helper or using early assignments to reduce nesting.♻️ Suggested simplification for workflowTargetFunctionName
- const workflowTargetFunctionName = - isWorkflowActivityCall - ? (typeof model.node.properties?.["activityFunction"]?.value === "string" - ? model.node.properties?.["activityFunction"]?.value.trim() - : undefined) - : (isWorkflowInitCall - ? (typeof model.node.properties?.["processFunction"]?.value === "string" - ? model.node.properties?.["processFunction"]?.value.trim() - : undefined) - : undefined); + const workflowTargetFunctionName = (() => { + const propKey = isWorkflowActivityCall ? "activityFunction" + : isWorkflowInitCall ? "processFunction" + : undefined; + if (!propKey) return undefined; + const val = model.node.properties?.[propKey]?.value; + return typeof val === "string" ? val.trim() : undefined; + })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/bi-diagram/src/components/nodes/BaseNode/BaseNodeWidget.tsx` around lines 198 - 234, The complex nested ternaries for workflowTargetFunctionName and the chained fallbacks for functionViewRange make the logic hard to read; extract them into small helper functions (e.g., getWorkflowTargetFunctionName(model) and getWorkflowFunctionViewRange(model)) or use early-return/early-assignment locals that reference the existing predicates isWorkflowActivityCall and isWorkflowInitCall, then replace the ternaries with calls to those helpers; ensure the helpers use the same property lookups (model.node.properties?.["activityFunction"]?.value, ["processFunction"]?.value, and their codedata.lineRange) and keep hasViewRange/canViewFunction logic unchanged but referencing the new helper results (workflowTargetFunctionName and functionViewRange).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-manager.ts`:
- Around line 329-331: getArtifactDataFromNodeKind() currently returns
DIRECTORY_MAP.WORKFLOW for the 'WORKFLOW' case which causes updateSourceCode()'s
artifact notification filter to miss workflow artifacts; change the 'WORKFLOW'
case in getArtifactDataFromNodeKind to return DIRECTORY_MAP.FUNCTION so
workflows are treated the same as functions (matching the special-case in
getComponents() and allowing the notification filter in updateSourceCode to find
the artifact). Ensure you update the case in the function handling node kinds
(getArtifactDataFromNodeKind) to return DIRECTORY_MAP.FUNCTION.
In `@workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts`:
- Around line 138-141: The current dedupeArtifactsById call drops distinct
artifacts that share the same id across modules; update deduplication to use a
module-scoped key (module + id) instead of id alone: modify dedupeArtifactsById
(or add a helper) to compute a unique key using artifact.module and artifact.id
(e.g., `${artifact.module}::${artifact.id}`) and use that when building the seen
set, then replace the existing call in the block that pushes functions and
workflows into response.directoryMap[DIRECTORY_MAP.FUNCTION] so it preserves
artifacts from different modules; references: response.directoryMap,
dedupeArtifactsById, getComponents, ARTIFACT_TYPE.Functions/Workflows,
DIRECTORY_MAP.FUNCTION, and ProjectStructureArtifactResponse.artifact.module/id.
In `@workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsx`:
- Around line 941-945: The HACK bypass in the onValid callback (selectedNode /
handleOnSave) is unreachable because isValid (the useMemo that sets validation
via setError) unconditionally injects errors for ACTIVITY_CALL and WAIT_EVENT,
which disables the Save button and causes handleSubmit to route to the onError
path; fix by either exempting these node types inside the isValid logic (skip
calling setError for keys when selectedNode === "ACTIVITY_CALL" || selectedNode
=== "WAIT_EVENT") or by moving the bypass for selectedNode out of the onValid
handler so it runs before react-hook-form validation/handleSubmit routing
(ensure handleOnSave or handleOnSaveClick can execute without relying on
formState validity). Also add a TODO/GH issue reference next to the exemption
and replace the broad debug object ({ data: props, formData: data }) with a
targeted log of only needed fields.
In `@workspaces/ballerina/ballerina-side-panel/src/components/NodeList/index.tsx`:
- Around line 43-56: The CATEGORY_DISPLAY_ORDER map contains a likely typo key
"loggin" that duplicates "logging"; update the CATEGORY_DISPLAY_ORDER constant
in index.tsx to either remove the "loggin" entry (keep only "logging": 7) or, if
this is intentionally defending against a backend misspelling, keep both but add
a one-line comment above CATEGORY_DISPLAY_ORDER explaining the defensive reason;
reference the CATEGORY_DISPLAY_ORDER symbol to locate and modify the keys
accordingly.
In
`@workspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsx`:
- Around line 244-245: The breadcrumb short-name for MACHINE_VIEW.BIWorkflowForm
("Workflow") was not included in hackToSkipForms, causing the workflow form to
remain visible while other "* SKIP" forms are hidden; if this was unintentional,
add the lowercase "workflow" string to the hackToSkipForms array so the
BIWorkflowForm breadcrumb is excluded from the nav bar, otherwise add a comment
near hackToSkipForms explaining the intentional visibility difference for
MACHINE_VIEW.BIWorkflowForm.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/FlowDiagram/index.tsx`:
- Around line 560-566: The code assumes eventStartNode.metadata exists and will
crash if it's undefined; update the extraction to guard with optional chaining
(use eventStartNode?.metadata?.data when assigning eventStartMetadata) and then
build parentMetadata from that safe value (keep the cast to ParentMetadata and
preserve sourceCode assignment from eventStartNode?.codedata?.sourceCode) so
missing metadata won't throw at runtime.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/SendEventForm/index.tsx`:
- Around line 312-317: Remove the two debug console.log statements around the
RPC call to getAllEvents (the logs that print ">>> requesting
workflowManager/getAllEvents" and ">>> received workflowManager/getAllEvents
response") since they may expose sensitive workflow data; either delete them or
replace them with a conditional debug/logger call (e.g., use a debug flag or a
logger.debug) tied to environment/dev mode and keep the RPC invocation via
rpcClient.getBIDiagramRpcClient().getAllEvents with the same
normalizedWorkflowName and fileName variables.
---
Outside diff comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGeneratorNew/index.tsx`:
- Line 762: Remove the debug console.log call that prints the stack on every
render — locate the console.log("#STACK", stack) invocation (inside the type
helper in the FormGeneratorNew component) and delete it (or replace it with a
conditional debug/logging utility if persistent debug output is required),
ensuring no stray console logging remains in the render path.
In `@workspaces/ballerina/bi-diagram/src/utils/node.ts`:
- Around line 87-139: getNodeTitle can produce titles like "undefined : <label>"
when node.codedata.module is missing; change the final return so that if
node.codedata.module is falsy you return just label (or `${label}`) instead of
`"undefined : ${label}"`. Locate the code using the symbols getNodeTitle,
node.codedata.module, and label and update the branch that builds module with
module = node.codedata.module?... to ensure you check for a non-empty module
(and after extracting the last segment) and only return `${module} : ${label}`
when module is defined, otherwise return label.
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/EnumEditor/EnumEditor.tsx`:
- Around line 34-44: The current itemsList computation uses props.items.length
which will throw if props.items is undefined; update the useMemo to defensively
treat props.items as an array before checking length (e.g., coerce to an empty
array or check Array.isArray(props.items)) so it falls back to
props.field.itemOptions safely; ensure you still append the default object with
id "default-option" and value DEFAULT_NONE_SELECTED_VALUE and keep the
dependency array referencing props.items and props.field.itemOptions.
In `@workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsx`:
- Line 948: The console.error call is logging the entire props object and using
the key name "data" which conflicts with the local data parameter; update the
log in the Form component (the console.error call) to emit a targeted
payload—rename the conflicting key to formData (or values) and log only relevant
props fields (e.g., props.formId, props.fieldDefinitions, or props.onSubmit
presence) or a small propsSnapshot object instead of full props so the output is
concise and non-sensitive.
In
`@workspaces/ballerina/ballerina-side-panel/src/components/NodeList/categoryConfig.ts`:
- Around line 68-87: Replace the long chained exclusion in the "Create Function"
item's condition with a set-based membership check: locate the object whose
properties include type: 'function', tooltip: "Create Function",
emptyStateLabel: "Create Function", and handlerKey: 'onAddFunction', then change
its condition from a && chain of title !== "Data Mappers" && title !== "Natural
Functions" && title !== "Workflows" && title !== "Activities" to using a
Set/array of excluded titles (e.g., const excluded = new Set([...])) and return
!excluded.has(title) so adding future categories is a single change.
In `@workspaces/ballerina/ballerina-side-panel/src/components/NodeList/index.tsx`:
- Around line 692-720: Extract the duplicated handler-resolution logic into a
small helper like resolveActionHandler(handlerKey) that encapsulates the
handlers map (mapping keys to handleAddConnection, handleAddFunction, handleAdd)
and the props lookup (props[handlerKey]) and returns the resolved function or
null; then replace both inline blocks in the header actions and empty-state
actions with calls to resolveActionHandler(action.handlerKey) and use the
returned function (if non-null) for the Button onClick. Ensure the helper is
referenced where action.handlerKey is used and preserves the existing
e.stopPropagation() and tooltip/addButtonLabel behavior.
- Around line 644-795: The render block inside reorderedGroups.map is too large
and complex; extract it into a helper function (e.g., renderCategory(group,
index, lastVisibleCategoryTitle)) and move all logic that computes visibility,
separators, header selection (isCategoryFixed, HeaderComponent, headerProps,
toggleCategory), action handlers (getCategoryActions, handleAddConnection,
handleAddFunction, handleAdd, props[action.handlerKey]), empty-state rendering
(shouldShowEmptyCategory, categoryActions empty-state mapping), and container
selection (shouldUseConnectionContainer, getConnectionContainer,
getCategoryContainer, getNodesContainer) into that helper; ensure the helper
returns the same JSX and updates lastVisibleCategoryTitle/reads
expandedCategories and shouldExpandAll, and then replace the IIFE + map body
with a simple call to renderCategory for each group to reduce nesting and
cognitive complexity.
In
`@workspaces/ballerina/ballerina-visualizer/src/components/EntryPointTypeCreator/index.tsx`:
- Around line 32-54: BreadcrumbContainer, BreadcrumbItem, and
BreadcrumbSeparator are duplicated here and in FormGenerator; extract these
styled components into a single shared module (e.g., create a styles file like
src/components/Breadcrumb/styles.ts) and replace the local definitions in
EntryPointTypeCreator (and the duplicates in
workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx)
with imports from that shared module so both files import BreadcrumbContainer,
BreadcrumbItem, and BreadcrumbSeparator rather than redefining them; ensure the
new module exports those three symbols and update any existing exports/imports
accordingly.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/DiagramWrapper/index.tsx`:
- Around line 255-277: Replace the brittle prefix-check in
getWorkflowTitleFromSource by first checking for explicit metadata/AST/Language
Server properties on parentMetadata (e.g., a workflowKind, annotations array, or
nodeKind field) and use those to determine "Workflow Activity" vs "Workflow";
only fall back to examining parentMetadata.sourceCode for `@workflow`:* markers if
those structured fields are absent. Update getWorkflowTitleFromSource to read
those properties on parentMetadata (and normalize annotation values if needed),
and keep getTitle unchanged so it returns the structured workflow title when
available.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/FormGenerator/index.tsx`:
- Around line 1567-1575: The EntryPointTypeCreator modal is duplicated in two
render branches; hoist a single instance out of the VARIABLE/default branches so
it renders once under the same provider scope (e.g., after
EditorContext.Provider similar to FormGeneratorNew). Remove the two occurrences
and add one EntryPointTypeCreator that uses the same props:
isOpen={isTypeEditorOpen}, onClose={handleTypeEditorClose},
onTypeCreate={handleTypeCreated}, initialTypeName={editingTypeName ||
"WorkflowInput"}, modalTitle="Define Workflow Input Type", modalWidth={650},
modalHeight={600}, ensuring it remains within the same component so those
handlers and state are in scope.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/SendEventForm/index.tsx`:
- Around line 352-371: The handleFormChange currently duplicates logic by
calling fetchWorkflowEvents when the workflow name changes; remove the branch in
handleFormChange that checks if (fieldKey === fieldKeys.workflowName) and the
internal fetchWorkflowEvents call (and its early return), leaving
onChange?.(...) intact and keeping the eventName branch; rely on the existing
useEffect that watches the derived/normalized workflow name (and the workflowRef
guard) to trigger fetchWorkflowEvents, so the state change from handleFormChange
propagates and the effect handles the fetch.
- Around line 338-350: The useEffect currently lists formFields in its
dependency list causing it to run on every field update; compute the normalized
workflowName (using normalizeStringValue on formFields.find(...
fieldKeys.workflowName)) outside the useEffect and use that derived workflowName
in the effect's dependency array instead of formFields, keep the existing
workflowRef.current guard, and likewise derive preferredEventName (from
fieldKeys.eventName) outside the effect so you can pass only the necessary
values and fetchWorkflowEvents into the dependency array to avoid unnecessary
re-runs.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionForm/index.tsx`:
- Around line 284-292: The search call using
rpcClient.getBIDiagramRpcClient().search with queryMap.q = workflowInputType is
using a very large limit (1000) which is expensive; reduce the limit to a
smaller reasonable value (e.g., 50 or 100) and ensure q is the specific type
name (workflowInputType) so the fallback type lookup still finds matches without
fetching excessive items; update the queryMap.limit from 1000 to the chosen
smaller number and keep the rest of the search call (filePath, offset,
searchKind) unchanged.
- Around line 72-77: hideTypeDescriptionField currently mutates
flowNode.properties.typeDescription.hidden in place; change it to return a new
FunctionNode object instead: create a new typeDescription object by spreading
flowNode.properties.typeDescription and setting hidden:true, create a new
properties object by spreading flowNode.properties and replacing typeDescription
with that new object, and finally return a new flowNode by spreading the
original flowNode and replacing properties with the new properties object
(mirror the immutable pattern used in FormGenerator/index.tsx).
- Around line 233-342: The function enrichWorkflowInputTypeModel mutates a
non‑declared property by assigning (primaryInputType as any).typeModel; add an
optional typeModel?: TypeModel (or appropriate interface) to the common base
type (e.g., extend BaseType or the shared InputType union) so these assignments
are typed, and refactor the two lookup paths into helpers (e.g.,
resolveTypeFromTypesApi(rpcClient, filePath, workflowInputType,
primaryInputType, resolveTypeByLinePosition) and
resolveTypeFromSearchApi(rpcClient, filePath, workflowInputType,
primaryInputType, resolveTypeByLinePosition)) to encapsulate the
getTypes/getType and search logic and call them from
enrichWorkflowInputTypeModel (keeping resolveTypeByLinePosition as the small
helper it currently is).
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/FunctionFormStatic/index.tsx`:
- Around line 86-91: The helper hideTypeDescriptionField currently lives inside
the component and mutates the RPC response in-place; move
hideTypeDescriptionField to module scope (outside the component) and change its
implementation to avoid in-place mutation by returning a new FunctionNode with a
shallow copy of properties and a copied typeDescription where hidden is set to
true (e.g., clone flowNode and flowNode.properties, and clone/replace
flowNode.properties.typeDescription with hidden: true) so callers use the new
object without altering the original RPC response.
- Around line 151-163: The call to hideTypeDescriptionField in getFunctionNode
is dead because the subsequent Object.keys(...).reduce(...) rebuilds
flowNode.properties and drops any key containing 'typedescription', so remove
the hideTypeDescriptionField(flowNode) invocation from getFunctionNode and rely
on the existing reduce-based exclusion; ensure flowNode.properties remains set
from the reduced object and leave getExistingFunctionNode unchanged (or
conversely, if you prefer the hidden-flag approach, remove the reduce block and
instead set typeDescription.hidden = true in getFunctionNode to match
getExistingFunctionNode).
In
`@workspaces/ballerina/bi-diagram/src/components/nodes/BaseNode/BaseNodeWidget.tsx`:
- Around line 198-234: The complex nested ternaries for
workflowTargetFunctionName and the chained fallbacks for functionViewRange make
the logic hard to read; extract them into small helper functions (e.g.,
getWorkflowTargetFunctionName(model) and getWorkflowFunctionViewRange(model)) or
use early-return/early-assignment locals that reference the existing predicates
isWorkflowActivityCall and isWorkflowInitCall, then replace the ternaries with
calls to those helpers; ensure the helpers use the same property lookups
(model.node.properties?.["activityFunction"]?.value, ["processFunction"]?.value,
and their codedata.lineRange) and keep hasViewRange/canViewFunction logic
unchanged but referencing the new helper results (workflowTargetFunctionName and
functionViewRange).
| case 'WORKFLOW': | ||
| return { artifactType: DIRECTORY_MAP.WORKFLOW }; | ||
| // Add other cases as needed |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd workspaces/ballerina/ballerina-extension && find . -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: wso2/vscode-extensions
Length of output: 788
🏁 Script executed:
# Get the actual code at the mentioned lines and surrounding context
cat -n workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-manager.ts | sed -n '320,340p'Repository: wso2/vscode-extensions
Length of output: 1095
🏁 Script executed:
# Search for DIRECTORY_MAP definitions and WORKFLOW usage
rg -n "DIRECTORY_MAP\s*=" workspaces/ballerina/ballerina-extension/src -A 10Repository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
# Check how artifactType is used in tree building and updates
rg -n "artifactType" workspaces/ballerina/ballerina-extension/src --context 3Repository: wso2/vscode-extensions
Length of output: 37246
🏁 Script executed:
# Search for tree builder logic and workflow grouping
rg -n "updateSourceCode|refreshTreeStructure|projectStructure" workspaces/ballerina/ballerina-extension/src -lRepository: wso2/vscode-extensions
Length of output: 1248
🏁 Script executed:
# Find DIRECTORY_MAP definition and check WORKFLOW usage
rg -n "DIRECTORY_MAP\s*[=:{]" workspaces/ballerina/ballerina-extension/src -A 20 | head -100Repository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
# Check if DIRECTORY_MAP.WORKFLOW is ever used besides the mapping in rpc-manager.ts
rg "DIRECTORY_MAP\.WORKFLOW" workspaces/ballerina/ballerina-extension/srcRepository: wso2/vscode-extensions
Length of output: 537
🏁 Script executed:
# Examine the exact context of how artifactType filters work in updateSourceCode
cat -n workspaces/ballerina/ballerina-extension/src/utils/project-artifacts-handler.ts | sed -n '60,80p'Repository: wso2/vscode-extensions
Length of output: 966
🏁 Script executed:
# Check project-artifacts.ts to see the full artifact filtering logic
cat -n workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts | sed -n '165,180p'Repository: wso2/vscode-extensions
Length of output: 1061
🏁 Script executed:
# Check the state-machine-utils WORKFLOW case to see if it compensates for this mismatch
cat -n workspaces/ballerina/ballerina-extension/src/utils/state-machine-utils.ts | grep -B 5 -A 15 "case DIRECTORY_MAP.WORKFLOW:"Repository: wso2/vscode-extensions
Length of output: 1070
🏁 Script executed:
# Check how updateSourceCode processes the artifactData and calls the handler
cat -n workspaces/ballerina/ballerina-extension/src/utils/source-utils.ts | sed -n '130,170p'Repository: wso2/vscode-extensions
Length of output: 2740
🏁 Script executed:
# Verify the complete flow: search for where updateSourceCode is called from bi-diagram rpc-manager
rg -n "updateSourceCode" workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-manager.ts -B 5 -A 2 | head -60Repository: wso2/vscode-extensions
Length of output: 4277
🏁 Script executed:
# Check the complete context of how updateSourceCode uses the filtered artifacts
cat -n workspaces/ballerina/ballerina-extension/src/utils/source-utils.ts | sed -n '50,100p'Repository: wso2/vscode-extensions
Length of output: 2833
🏁 Script executed:
# Check what happens after the filter and artifact notification
cat -n workspaces/ballerina/ballerina-extension/src/utils/project-artifacts-handler.ts | sed -n '55,85p'Repository: wso2/vscode-extensions
Length of output: 1428
🏁 Script executed:
# Check what updateSourceCode returns and how artifacts are obtained
cat -n workspaces/ballerina/ballerina-extension/src/utils/source-utils.ts | sed -n '175,220p'Repository: wso2/vscode-extensions
Length of output: 2620
🏁 Script executed:
# Check if there's a notification payload structure or if artifacts come from a specific notification
rg -n "projectStructureUpdate|onDidChange|ArtifactNotification" workspaces/ballerina/ballerina-extension/src -A 3Repository: wso2/vscode-extensions
Length of output: 26916
🏁 Script executed:
# Check how workflow artifacts are stored/created - are they stored as WORKFLOW or FUNCTION?
rg -n "artifact\.type\s*=|type:\s*DIRECTORY_MAP" workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts -B 3 -A 1 | head -80Repository: wso2/vscode-extensions
Length of output: 1260
🏁 Script executed:
# Check if there's any handling in buildProjectsStructure that processes WORKFLOW artifacts
cat -n workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts | sed -n '25,100p'Repository: wso2/vscode-extensions
Length of output: 3313
🏁 Script executed:
# Check traverseComponents to see how artifacts are categorized
cat -n workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts | sed -n '200,270p'Repository: wso2/vscode-extensions
Length of output: 4573
🏁 Script executed:
# Check if DIRECTORY_MAP.WORKFLOW is even initialized in the directoryMap
rg -n "DIRECTORY_MAP\.WORKFLOW" workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.tsRepository: wso2/vscode-extensions
Length of output: 205
🏁 Script executed:
# Final check: verify the entire directoryMap initialization to confirm no WORKFLOW bucket
cat -n workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts | sed -n '61,95p'Repository: wso2/vscode-extensions
Length of output: 1713
🏁 Script executed:
# Check what happens when the filter returns empty in updateSourceCode
cat -n workspaces/ballerina/ballerina-extension/src/utils/source-utils.ts | sed -n '185,195p'Repository: wso2/vscode-extensions
Length of output: 945
Map WORKFLOW artifactType to DIRECTORY_MAP.FUNCTION to prevent artifact notification timeout.
getArtifactDataFromNodeKind() maps WORKFLOW to DIRECTORY_MAP.WORKFLOW, but the project tree structure does not include a WORKFLOW bucket—workflows are grouped under DIRECTORY_MAP.FUNCTION. When updateSourceCode uses this artifactType to filter artifact notifications, the filter will find no matching artifacts (line 67 in project-artifacts-handler.ts searches for data.type === artifactData.artifactType), causing the update to timeout after 10 seconds and fail.
The special case logic in getComponents() (line 169 in project-artifacts.ts) acknowledges this by explicitly treating WORKFLOW artifacts as FUNCTION, but this does not apply to the notification subscription filter path used by updateSourceCode.
🛠️ Fix
case 'WORKFLOW':
- return { artifactType: DIRECTORY_MAP.WORKFLOW };
+ return { artifactType: DIRECTORY_MAP.FUNCTION };📝 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.
| case 'WORKFLOW': | |
| return { artifactType: DIRECTORY_MAP.WORKFLOW }; | |
| // Add other cases as needed | |
| case 'WORKFLOW': | |
| return { artifactType: DIRECTORY_MAP.FUNCTION }; | |
| // Add other cases as needed |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-manager.ts`
around lines 329 - 331, getArtifactDataFromNodeKind() currently returns
DIRECTORY_MAP.WORKFLOW for the 'WORKFLOW' case which causes updateSourceCode()'s
artifact notification filter to miss workflow artifacts; change the 'WORKFLOW'
case in getArtifactDataFromNodeKind to return DIRECTORY_MAP.FUNCTION so
workflows are treated the same as functions (matching the special-case in
getComponents() and allowing the notification filter in updateSourceCode to find
the artifact). Ensure you update the case in the function handling node kinds
(getArtifactDataFromNodeKind) to return DIRECTORY_MAP.FUNCTION.
| response.directoryMap[DIRECTORY_MAP.FUNCTION].push(...await getComponents(artifacts[ARTIFACT_TYPE.Functions], projectPath, DIRECTORY_MAP.FUNCTION, "function")); | ||
| response.directoryMap[DIRECTORY_MAP.FUNCTION].push(...await getComponents(artifacts[ARTIFACT_TYPE.Workflows], projectPath, DIRECTORY_MAP.FUNCTION, "function")); | ||
| response.directoryMap[DIRECTORY_MAP.FUNCTION] = dedupeArtifactsById(response.directoryMap[DIRECTORY_MAP.FUNCTION]); | ||
| response.directoryMap[DIRECTORY_MAP.DATA_MAPPER].push(...await getComponents(artifacts[ARTIFACT_TYPE.DataMappers], projectPath, DIRECTORY_MAP.DATA_MAPPER, "dataMapper")); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for BaseArtifact interface definition and id field
rg -n "interface BaseArtifact|type BaseArtifact" workspaces/ballerina -SRepository: wso2/vscode-extensions
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Find where id is defined in artifact interfaces
find workspaces/ballerina -name "*.ts" -type f | xargs rg -l "interface.*Artifact|type.*Artifact" | head -10Repository: wso2/vscode-extensions
Length of output: 399
🏁 Script executed:
#!/bin/bash
# Look at the dedupeArtifactsById function implementation
cat -n workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts | sed -n '1,200p'Repository: wso2/vscode-extensions
Length of output: 11279
🏁 Script executed:
#!/bin/bash
# View BaseArtifact interface definition
sed -n '1900,1950p' workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.tsRepository: wso2/vscode-extensions
Length of output: 1622
🏁 Script executed:
#!/bin/bash
# Search for any module-scoped id patterns or documentation
rg -n "id.*module|module.*id" workspaces/ballerina/ballerina-core/src/interfaces -B2 -A2Repository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check if there are any tests showing potential duplicate ids across modules
rg -n "dedupeArtifactsById" workspaces/ballerina -B5 -A5Repository: wso2/vscode-extensions
Length of output: 3646
🏁 Script executed:
#!/bin/bash
# Search for how artifact.id is generated/assigned
rg -n "artifact\.id\s*=" workspaces/ballerina/ballerina-core/src -B2 -A2 | head -50Repository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for other locations that might have the same issue
# Check if there are other places where artifacts from different sources are merged
rg -n "push.*getComponents.*ARTIFACT_TYPE" workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.tsRepository: wso2/vscode-extensions
Length of output: 1791
🏁 Script executed:
#!/bin/bash
# Check the full traverseComponents function to see if other directories merge multiple sources
sed -n '134,152p' workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.tsRepository: wso2/vscode-extensions
Length of output: 2308
🏁 Script executed:
#!/bin/bash
# Search for any tests or examples that might show duplicate ids
rg -n "dedupeArtifacts|duplicate.*id|id.*duplicate" workspaces/ballerina -B3 -A3Repository: wso2/vscode-extensions
Length of output: 6810
🏁 Script executed:
#!/bin/bash
# Check all places where ProjectStructureArtifactResponse is created to ensure moduleName is always populated
grep -n "moduleName:" workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.tsRepository: wso2/vscode-extensions
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Verify if artifact.id is scoped by module or globally unique
# by checking the language server implementation or comments
rg -n "artifact.*id.*unique|uniqueness|global.*id" workspaces/ballerina/ballerina-core -B2 -A2Repository: wso2/vscode-extensions
Length of output: 48
Use module-scoped keys to deduplicate artifacts correctly.
When functions and workflows are merged into the same directory, deduping by id alone can incorrectly drop valid artifacts if the same function or workflow name exists across multiple modules. Since ProjectStructureArtifactResponse includes moduleName (from artifact.module), update the deduplication key to use both module and id:
function dedupeArtifactsById(artifacts: ProjectStructureArtifactResponse[]): ProjectStructureArtifactResponse[] {
const uniqueArtifacts = new Map<string, ProjectStructureArtifactResponse>();
- artifacts.forEach((artifact) => uniqueArtifacts.set(artifact.id, artifact));
+ artifacts.forEach((artifact) => {
+ const key = `${artifact.moduleName ?? ""}:${artifact.id}`;
+ uniqueArtifacts.set(key, artifact);
+ });
return Array.from(uniqueArtifacts.values());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts`
around lines 138 - 141, The current dedupeArtifactsById call drops distinct
artifacts that share the same id across modules; update deduplication to use a
module-scoped key (module + id) instead of id alone: modify dedupeArtifactsById
(or add a helper) to compute a unique key using artifact.module and artifact.id
(e.g., `${artifact.module}::${artifact.id}`) and use that when building the seen
set, then replace the existing call in the block that pushes functions and
workflows into response.directoryMap[DIRECTORY_MAP.FUNCTION] so it preserves
artifacts from different modules; references: response.directoryMap,
dedupeArtifactsById, getComponents, ARTIFACT_TYPE.Functions/Workflows,
DIRECTORY_MAP.FUNCTION, and ProjectStructureArtifactResponse.artifact.module/id.
| //HACK: skip form validation for "ACTIVITY_CALL" and "WAIT_EVENT" nodes until fixed diagnostic issue from LS. | ||
| if (selectedNode === "ACTIVITY_CALL" || selectedNode === "WAIT_EVENT") { | ||
| handleOnSave(data); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the broader context of the Form component to understand error handling
cd workspaces/ballerina/ballerina-side-panel && find . -type f \( -name "*.tsx" -o -name "*.ts" \) | head -20Repository: wso2/vscode-extensions
Length of output: 853
🏁 Script executed:
# Search for ACTIVITY_CALL and WAIT_EVENT references in the workspace
rg -n "ACTIVITY_CALL|WAIT_EVENT" workspaces/ballerina/ballerina-side-panel --type tsx --type ts -B2 -A2Repository: wso2/vscode-extensions
Length of output: 93
🏁 Script executed:
# Check for setError calls in the Form component and surrounding context
rg -n "setError" workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsx -B3 -A3Repository: wso2/vscode-extensions
Length of output: 868
🏁 Script executed:
# Search for onFormValidation provider and how it's used
rg -n "onFormValidation" workspaces/ballerina/ballerina-side-panel --type tsx --type ts -B2 -A2Repository: wso2/vscode-extensions
Length of output: 93
🏁 Script executed:
# Check for isValid and error handling flow
rg -n "isValid|diagnostics" workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsx -B2 -A2 | head -60Repository: wso2/vscode-extensions
Length of output: 2741
🏁 Script executed:
# Search for where handleSetDiagnosticsInfo is called - this will show when LS diagnostics are processed
rg -n "handleSetDiagnosticsInfo" workspaces/ballerina/ballerina-side-panel/src --type ts --type tsx -B2 -A2Repository: wso2/vscode-extensions
Length of output: 93
🏁 Script executed:
# Get the disableSaveButton logic and surrounding context
sed -n '825,840p' workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsxRepository: wso2/vscode-extensions
Length of output: 759
🏁 Script executed:
# Look at the isValid useMemo to understand when errors are set
sed -n '770,810p' workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsxRepository: wso2/vscode-extensions
Length of output: 1666
🏁 Script executed:
# Get context around line 791 where setError is called - what's the condition?
sed -n '775,800p' workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsxRepository: wso2/vscode-extensions
Length of output: 1214
🏁 Script executed:
# Look at the handleOnSaveClick function to understand the full flow
sed -n '930,965p' workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsxRepository: wso2/vscode-extensions
Length of output: 1282
HACK bypass is structurally unreachable — LS diagnostic errors block both the button and react-hook-form routing.
The bypass at lines 941-945 is dead code when LS sends diagnostics for ACTIVITY_CALL and WAIT_EVENT nodes. The issue occurs in two gates simultaneously:
-
Button remains disabled: When LS diagnostics arrive for these nodes, the
isValiduseMemo (lines 775-810) callssetError(key, ...)unconditionally (line 791) — there is no node-type check to exempt these nodes. This populates theerrorsobject. Line 833 then disables the Save button viaObject.keys(errors).length > 0, sohandleOnSaveClickis never invoked. -
Even if clicked, react-hook-form routes to error handler: The
handleSubmitcallback routes to the onError handler (line 958) whenformState.errorsis non-empty (which includes manually-set errors fromsetError), bypassing the success callback where the HACK lives.
Fix strategy: Either skip calling setError for these node types in the isValid useMemo (lines 775-810), or move the bypass logic outside the handleSubmit onValid callback before validation occurs. Additionally, add a reference to the LS diagnostic issue (e.g., GitHub issue number) so the debt is tracked.
The secondary concern about confusing logging at line 948 ({ data: props, formData: data }) remains valid — should use more targeted logging instead of the full props object.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workspaces/ballerina/ballerina-side-panel/src/components/Form/index.tsx`
around lines 941 - 945, The HACK bypass in the onValid callback (selectedNode /
handleOnSave) is unreachable because isValid (the useMemo that sets validation
via setError) unconditionally injects errors for ACTIVITY_CALL and WAIT_EVENT,
which disables the Save button and causes handleSubmit to route to the onError
path; fix by either exempting these node types inside the isValid logic (skip
calling setError for keys when selectedNode === "ACTIVITY_CALL" || selectedNode
=== "WAIT_EVENT") or by moving the bypass for selectedNode out of the onValid
handler so it runs before react-hook-form validation/handleSubmit routing
(ensure handleOnSave or handleOnSaveClick can execute without relying on
formState validity). Also add a TODO/GH issue reference next to the exemption
and replace the broad debug object ({ data: props, formData: data }) with a
targeted log of only needed fields.
| const CATEGORY_DISPLAY_ORDER: Record<string, number> = { | ||
| connections: 0, | ||
| statement: 1, | ||
| statements: 1, | ||
| control: 2, | ||
| controls: 2, | ||
| ai: 3, | ||
| workflow: 4, | ||
| workflows: 4, | ||
| "error handling": 5, | ||
| concurrency: 6, | ||
| logging: 7, | ||
| loggin: 7, | ||
| }; |
There was a problem hiding this comment.
Typo: "loggin" should likely be "logging".
Line 55 maps "loggin" → 7, but "logging" is already mapped at line 54. If this is a defensive guard against a known backend misspelling, a comment would clarify intent. Otherwise, it reads as a copy-paste typo with no effect.
Proposed fix
"error handling": 5,
concurrency: 6,
logging: 7,
- loggin: 7,
+ // "loggin" removed — duplicate of "logging" above
};📝 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 CATEGORY_DISPLAY_ORDER: Record<string, number> = { | |
| connections: 0, | |
| statement: 1, | |
| statements: 1, | |
| control: 2, | |
| controls: 2, | |
| ai: 3, | |
| workflow: 4, | |
| workflows: 4, | |
| "error handling": 5, | |
| concurrency: 6, | |
| logging: 7, | |
| loggin: 7, | |
| }; | |
| const CATEGORY_DISPLAY_ORDER: Record<string, number> = { | |
| connections: 0, | |
| statement: 1, | |
| statements: 1, | |
| control: 2, | |
| controls: 2, | |
| ai: 3, | |
| workflow: 4, | |
| workflows: 4, | |
| "error handling": 5, | |
| concurrency: 6, | |
| logging: 7, | |
| // "loggin" removed — duplicate of "logging" above | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workspaces/ballerina/ballerina-side-panel/src/components/NodeList/index.tsx`
around lines 43 - 56, The CATEGORY_DISPLAY_ORDER map contains a likely typo key
"loggin" that duplicates "logging"; update the CATEGORY_DISPLAY_ORDER constant
in index.tsx to either remove the "loggin" entry (keep only "logging": 7) or, if
this is intentionally defending against a backend misspelling, keep both but add
a one-line comment above CATEGORY_DISPLAY_ORDER explaining the defensive reason;
reference the CATEGORY_DISPLAY_ORDER symbol to locate and modify the keys
accordingly.
| case MACHINE_VIEW.BIWorkflowForm: | ||
| return "Workflow"; |
There was a problem hiding this comment.
Confirm: "Workflow" intentionally omitted from hackToSkipForms?
Every other MACHINE_VIEW value with a "SKIP" suffix has its breadcrumb short-name added to hackToSkipForms so it is hidden from the nav bar:
| Enum value | Short-name | In hackToSkipForms? |
|---|---|---|
BIFunctionForm = "Add Function SKIP" |
"Function" |
✅ "function" |
BIMainFunctionForm = "Add Automation SKIP" |
"Automation" |
✅ "automation" |
BINPFunctionForm = "Add Natural Function SKIP" |
"Natural Function" |
✅ "add natural function" |
BIWorkflowForm = "Add Workflow SKIP" |
"Workflow" |
❌ missing |
If the intent is for the workflow form breadcrumb to be visible (different UX from the function/automation forms), this is fine. Otherwise "workflow" needs to be added to hackToSkipForms (line 151).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsx`
around lines 244 - 245, The breadcrumb short-name for
MACHINE_VIEW.BIWorkflowForm ("Workflow") was not included in hackToSkipForms,
causing the workflow form to remain visible while other "* SKIP" forms are
hidden; if this was unintentional, add the lowercase "workflow" string to the
hackToSkipForms array so the BIWorkflowForm breadcrumb is excluded from the nav
bar, otherwise add a comment near hackToSkipForms explaining the intentional
visibility difference for MACHINE_VIEW.BIWorkflowForm.
| const eventStartNode = model.flowModel.nodes.find( | ||
| (node) => node.codedata.node === "EVENT_START" | ||
| )?.metadata.data as ParentMetadata | undefined; | ||
| ); | ||
| const eventStartMetadata = eventStartNode?.metadata.data as ParentMetadata | undefined; | ||
| const parentMetadata = eventStartMetadata | ||
| ? { ...eventStartMetadata, sourceCode: eventStartNode?.codedata?.sourceCode } | ||
| : undefined; |
There was a problem hiding this comment.
Guard against missing event metadata.
eventStartNode?.metadata.data will throw if metadata is undefined; use optional chaining to avoid a runtime crash.
🛠️ Suggested fix
-const eventStartMetadata = eventStartNode?.metadata.data as ParentMetadata | undefined;
+const eventStartMetadata = eventStartNode?.metadata?.data as ParentMetadata | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/FlowDiagram/index.tsx`
around lines 560 - 566, The code assumes eventStartNode.metadata exists and will
crash if it's undefined; update the extraction to guard with optional chaining
(use eventStartNode?.metadata?.data when assigning eventStartMetadata) and then
build parentMetadata from that safe value (keep the cast to ParentMetadata and
preserve sourceCode assignment from eventStartNode?.codedata?.sourceCode) so
missing metadata won't throw at runtime.
| console.log(">>> requesting workflowManager/getAllEvents", { workflowName: normalizedWorkflowName, filePath: fileName }); | ||
| const response = await rpcClient.getBIDiagramRpcClient().getAllEvents({ | ||
| workflowName: normalizedWorkflowName, | ||
| filePath: fileName, | ||
| }); | ||
| console.log(">>> received workflowManager/getAllEvents response", response); |
There was a problem hiding this comment.
Remove debug console.log statements.
Lines 312 and 317 log RPC request/response payloads that may contain sensitive workflow data. These should be removed or guarded behind a debug flag before merging.
Proposed fix
- console.log(">>> requesting workflowManager/getAllEvents", { workflowName: normalizedWorkflowName, filePath: fileName });
const response = await rpcClient.getBIDiagramRpcClient().getAllEvents({
workflowName: normalizedWorkflowName,
filePath: fileName,
});
- console.log(">>> received workflowManager/getAllEvents response", response);📝 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(">>> requesting workflowManager/getAllEvents", { workflowName: normalizedWorkflowName, filePath: fileName }); | |
| const response = await rpcClient.getBIDiagramRpcClient().getAllEvents({ | |
| workflowName: normalizedWorkflowName, | |
| filePath: fileName, | |
| }); | |
| console.log(">>> received workflowManager/getAllEvents response", response); | |
| const response = await rpcClient.getBIDiagramRpcClient().getAllEvents({ | |
| workflowName: normalizedWorkflowName, | |
| filePath: fileName, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Forms/SendEventForm/index.tsx`
around lines 312 - 317, Remove the two debug console.log statements around the
RPC call to getAllEvents (the logs that print ">>> requesting
workflowManager/getAllEvents" and ">>> received workflowManager/getAllEvents
response") since they may expose sensitive workflow data; either delete them or
replace them with a conditional debug/logger call (e.g., use a debug flag or a
logger.debug) tied to environment/dev mode and keep the RPC invocation via
rpcClient.getBIDiagramRpcClient().getAllEvents with the same
normalizedWorkflowName and fileName variables.
2 participants
Purpose
This PR introduces core workflow capabilities to the BI extension, enabling the definition and execution of structured processes.
Key Changes
Technical Debt / TODOs
Related Tasks
Summary by CodeRabbit
Release Notes