Conversation
…value to be reset
Fix `Add to integration` for /`typecreator` cmd
…refresh requests and improve error logging
Re-enable auto-reveal in tree view based on the active webview
…of the expression
fix completions not displayed for field access
Fix expression cursor on popup
Fix when removing all the values in the expression editor causes the …
…able fix invalid form status update in new variable creation of addConnect…
fix create config not saving the values
…s are always available, including notify and refresh commands for the Ballerina extension
…proper project type before creating test profiles
fix helperpane search fields are not focusable
Remove 'Show Examples' command and related functionality
Add force replace option for language server download
Fix incomplete source generation of union types in initial render
…packages using tree view
Update Record Configuration toggle name from Guided to Record
Enable visualizing constructs using code lenses in multi-package directory setups
…d listeners, ensuring proper positioning of unconnected listeners below other nodes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix listener overlap and component menu button click issue in Architecture Diagram
kanushka
requested review from
madushajg
and removed request for
gigara and
hevayo
WalkthroughAdds a new BI command NOTIFY_PROJECT_EXPLORER and tree-reveal plumbing; removes the BBE show-examples activation; refactors expression editor modes from GUIDED→RECORD and async completion handling; introduces workspace-aware datamapper paths, position-aware project-explorer reveals, menu mouse-event guards, and small UI/utility tweaks across multiple packages. Changes
Sequence Diagram(s)sequenceDiagram
participant Ext as Extension
participant SM as StateMachine
participant ProjExp as ProjectExplorer
participant Tree as TreeView
Note over Ext,Tree: Project-root + projectInfo notify flow
Ext->>SM: updateProjectRootAndInfo(path, projectInfo)
SM->>SM: update context with projectInfo
SM->>ProjExp: execute BI_COMMANDS.NOTIFY_PROJECT_EXPLORER(payload)
ProjExp->>ProjExp: revealInTreeView(documentUri, projectPath, position, view)
ProjExp->>Tree: treeView.reveal(matchedItem)
Tree->>Tree: expand & focus item
sequenceDiagram
participant Editor as ExpressionEditor
participant Chip as ChipExpressionEditor
participant CodeUtils as CodeUtils
participant Source as CompletionsProvider
Note over Editor,Source: Async completion lifecycle
Editor->>Chip: onChange() -> mark scheduledRef = true
Chip->>CodeUtils: request completion via async supplier
CodeUtils->>CodeUtils: wait for scheduledRef to clear (poll/RAF)
CodeUtils->>Source: await getCompletions()
Source-->>CodeUtils: CompletionItem[]
CodeUtils-->>Chip: return CompletionResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus during review:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-11-10T15:04:50.474ZApplied to files:
📚 Learning: 2025-11-10T15:05:11.309ZApplied to files:
🔇 Additional comments (1)
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: 6
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-extension/test/ai/integration_tests/libs/setup.ts (1)
61-65: Adopt polling pattern for extension activation verification to match codebase standards.The code currently relies on fixed 20-second waits without verifying the extension actually activated. However, the same codebase already implements a superior pattern in
code.test.tsthat polls for command availability rather than using fixed delays.Adopt the polling approach from
code.test.ts:
- Poll for a known extension command registration every 2 seconds
- Set maximum attempts to 30 (same constants already defined in setup.ts)
- Throw a clear error if activation times out instead of silently proceeding
Example from existing codebase (
code.test.ts):let attempts = 0; while (attempts < TIMING.MAX_ACTIVATION_ATTEMPTS) { const availableCommands = await vscode.commands.getCommands(); if (availableCommands.includes(VSCODE_COMMANDS.YOUR_EXTENSION_COMMAND)) { break; } await new Promise(resolve => setTimeout(resolve, TIMING.EXTENSION_ACTIVATION_RETRY_INTERVAL)); attempts++; } if (attempts >= TIMING.MAX_ACTIVATION_ATTEMPTS) { throw new Error("Extension command never registered - extension failed to activate"); }This ensures reliable test setup and catches activation failures early with clear error messages rather than confusing downstream test failures.
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
394-428: Potential crash when typing.with no preceding wordWhen the cursor is after a leading dot (e.g., the document is
"."),context.matchBefore(/\w*/)can returnnull. BecauselastNonSpaceChar === '.', the guard that checks!wordis skipped, but laterword.textandword.fromare accessed unconditionally, which will throw at runtime.Suggest making
wordhandling null‑safe and derivingfrom/prefixaccordingly, for example:- const completions = await getCompletions(); - const prefix = word.text; + const completions = await getCompletions(); + const from = word ? word.from : context.pos; + const prefix = word ? word.text : ""; @@ - return { - from: word.from, + return { + from, options: filteredCompletions.map(item => ({Optionally, tighten the earlier guard too:
- const word = context.matchBefore(/\w*/); - if (lastNonSpaceChar !== '.' && ( - !word || (word.from === word.to && !context.explicit) - )) { + const word = context.matchBefore(/\w*/); + if (lastNonSpaceChar !== '.' && ( + !word || (word.from === word.to && !context.explicit) + )) { return null; }This keeps the special “dot” behavior while avoiding null dereferences.
♻️ Duplicate comments (4)
workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GraphQLServiceWidget.tsx (1)
194-200: Code duplication - see AIServiceWidget.tsx comment.This is the same pattern flagged in AIServiceWidget.tsx. Consider using the suggested custom hook to eliminate duplication across all widget files.
workspaces/ballerina/component-diagram/src/components/nodes/ListenerNode/ListenerNodeWidget.tsx (1)
177-183: Code duplication - see AIServiceWidget.tsx comment.This is the same pattern flagged in AIServiceWidget.tsx. Consider using the suggested custom hook.
workspaces/ballerina/component-diagram/src/components/nodes/ConnectionNode/ConnectionNodeWidget.tsx (1)
167-173: Code duplication - see AIServiceWidget.tsx comment.This is the same pattern flagged in AIServiceWidget.tsx. Consider extracting to a reusable hook to maintain consistency across all node widgets.
workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GeneralWidget.tsx (1)
200-206: Code duplication - see AIServiceWidget.tsx comment.This is the same pattern flagged in AIServiceWidget.tsx. The suggested custom hook would eliminate this duplication across all 5 widget files.
🧹 Nitpick comments (8)
workspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsx (1)
160-161: LGTM—breadcrumb skip list updated correctly.The additions of "bi add project skip" and "welcome" to the skip list are consistent with the existing pattern and will properly filter these views from the breadcrumb navigation.
Consider adding a trailing comma after "welcome" for consistency with modern JavaScript/TypeScript style and easier future modifications:
"bi add project skip", - "welcome" + "welcome",workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/AIServiceWidget.tsx (1)
84-90: Consider extracting this pattern into a reusable hook.The
handleMenuMouseDownandhandleMenuMouseUppattern is duplicated identically across 5 widget files (AIServiceWidget, GraphQLServiceWidget, ListenerNodeWidget, ConnectionNodeWidget, and GeneralWidget). Consider extracting this into a custom hook to reduce duplication.Example:
// hooks/useMenuEventHandlers.ts export function useMenuEventHandlers() { const handleMenuMouseDown = (event: React.MouseEvent) => { event.stopPropagation(); }; const handleMenuMouseUp = (event: React.MouseEvent) => { event.stopPropagation(); }; return { handleMenuMouseDown, handleMenuMouseUp }; }Then in each widget:
- const handleMenuMouseDown = (event: React.MouseEvent) => { - event.stopPropagation(); - }; - - const handleMenuMouseUp = (event: React.MouseEvent) => { - event.stopPropagation(); - }; + const { handleMenuMouseDown, handleMenuMouseUp } = useMenuEventHandlers();workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
90-115: Async completion gating is reasonable but relies on completions always arrivingThe new flow (
completionsRef,completionsFetchScheduledRef,waitForStateChange, andcompletionSource) is coherent: a change schedules a fetch,retrieveCompletionsupdatesprops.completions, and the effect syncs the ref and clears the flag before CodeMirror asks for completions.Two things to be aware of:
- If
props.completionsdoes not change identity (e.g., the same array instance is reused) or a fetch fails without updating completions,completionsFetchScheduledRef.currentmay remaintrue, andwaitForStateChangewill spin viarequestAnimationFrameindefinitely.completionSourceis memoized onprops.completions, but the editor extensions are only installed once in the empty-depsuseEffect, so re-memoization doesn’t affect the active completion source; the design correctly relies on the mutablecompletionsRefinstead.Consider:
- Ensuring
retrieveCompletionsalways produces a new array instance and always clears the “fetch scheduled” condition (even on error), or- Adding a simple timeout / max-frame safeguard in
waitForStateChangeto fall back to the latestcompletionsRef.currentinstead of spinning forever.Also applies to: 128-144, 299-304
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (1)
638-673: More eager diagnostics + completions: ensure backends can handle the call volumeThe updated
onChangehandlers for bothExpressionFieldandExpandedEditornow:
- Always clear diagnostics and recompute them (when
inputMode === InputMode.EXP) on every change, and- Always call
retrieveCompletions, passing atriggerCharacteronly when applicable.This behavior is consistent with the new async completion pipeline and should help keep completions and diagnostics in sync with the editor state. It does, however, rely on
retrieveCompletionsand diagnostics backends being efficient and safe to call on every keystroke.If the underlying services are remote or heavy, you may want to:
- Confirm they already debounce/throttle or cache under the hood, or
- Introduce lightweight debouncing around
retrieveCompletions/ diagnostics here.Also applies to: 705-741
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
77-103: Union/enum member auto‑selection looks good; minor redundancy in fallback.The new branch that auto‑selects a union/enum member and recursively selects required nested fields is consistent with the rest of the helper behavior.
One small cleanup: inside the
!hasSelectedMemberblock,getSelectedUnionMember(field)already falls back to the first member when there is no explicit selection or value, so the extra|| field.members[0]is redundant:- const hasSelectedMember = field.members.some(member => member.selected === true); + const hasSelectedMember = field.members.some((member) => member.selected === true); @@ - // Select the first member (or use getSelectedUnionMember logic) - const memberToSelect = getSelectedUnionMember(field) || field.members[0]; + // Select the best matching member (selected/value/selectedDataType, then first) + const memberToSelect = getSelectedUnionMember(field);This keeps the intent clear and avoids duplicating fallback logic.
Also applies to: 107-111
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx (1)
65-71: Avoid double-runningupdateFieldsSelectionon the same member fieldsRight now
updateFieldsSelectionis invoked twice for the selected member’s nested fields:
- In
initializeUnionMember, right afterupdateFormFieldMemberSelection(selectedMember)(Lines 65–71).- Inside
updateFormFieldMemberSelection, for the sameselectedMember.fields(Lines 116–123).- Similarly,
handleMemberTypecallsupdateFormFieldMemberSelection(selectedMember)and then again callsupdateFieldsSelection(selectedMember.fields, true)(Lines 146–148).Because
updateFormFieldMemberSelectionalready handles both selecting the member and recursively updating its fields (required vs optional), the extra calls are redundant and just add work and complexity.You can safely remove them and rely on
updateFormFieldMemberSelectionas the single source of truth for nested-field selection:- if (selectedMember) { - updateFormFieldMemberSelection(selectedMember); - - if (selectedMember.fields && selectedMember.fields.length > 0) { - updateFieldsSelection(selectedMember.fields, true); - } - - setSelectedMemberType(memberTypeName); - setParameter(selectedMember); + if (selectedMember) { + updateFormFieldMemberSelection(selectedMember); + + setSelectedMemberType(memberTypeName); + setParameter(selectedMember); @@ - updateFormFieldMemberSelection(selectedMember); - - if (selectedMember.fields && selectedMember.fields.length > 0) { - updateFieldsSelection(selectedMember.fields, true); - } + updateFormFieldMemberSelection(selectedMember);This keeps behavior the same while reducing redundant recursion and making the control flow easier to follow.
Also applies to: 116-123, 146-148
workspaces/bi/bi-extension/src/project-explorer/activate.ts (1)
42-46: Core command wiring is good; consider disposing and loosening event typingThe core commands +
setTreeViewintegration look right and align with the newNOTIFY_PROJECT_EXPLORERflow. Two minor points:
- The
registerCommanddisposables aren’t pushed intocontext.subscriptions, so ifactivateProjectExplorercan ever run more than once, you’ll accumulate handlers. PassingcontextintoregisterCoreCommandsand doingcontext.subscriptions.push(commands.registerCommand(...))would make cleanup explicit.- The
eventparameter inNOTIFY_PROJECT_EXPLORERis typed with all fields required, but callers (e.g.,notifyTreeView) may legitimately omit some. Typing it as a partial (e.g.event: { projectPath?: string; documentUri?: string; position?: NodePosition; view?: MACHINE_VIEW }) would better reflect runtime usage.Also applies to: 69-85
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
470-479: Avoid calling NOTIFY_PROJECT_EXPLORER when BI extension isn’t installed
notifyTreeViewcurrently does:const biExtension = extensions.getExtension('wso2.ballerina-integrator'); if (biExtension && !biExtension.isActive) { return; } commands.executeCommand(BI_COMMANDS.NOTIFY_PROJECT_EXPLORER, { ... });If the BI extension isn’t installed (
biExtension === undefined), this still issues the command, which will fail with “command not found” and may spam the console.You can make this more defensive by bailing out when the extension is missing or inactive:
const biExtension = extensions.getExtension('wso2.ballerina-integrator'); if (!biExtension || !biExtension.isActive) { return; } void commands.executeCommand(BI_COMMANDS.NOTIFY_PROJECT_EXPLORER, { projectPath, documentUri, position, view, });Also marking the promise as
void(or explicitlyawaiting it) avoids unhandled rejection warnings if the command itself throws.Also applies to: 1028-1049
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts(1 hunks)workspaces/ballerina/ballerina-extension/package.json(1 hunks)workspaces/ballerina/ballerina-extension/scripts/download-ls.js(2 hunks)workspaces/ballerina/ballerina-extension/src/extension.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts(2 hunks)workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/features/project/cmds/cmd-runner.ts(0 hunks)workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/stateMachine.ts(9 hunks)workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts(0 hunks)workspaces/ballerina/ballerina-extension/src/views/bbe/activator.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/views/visualizer/activate.ts(2 hunks)workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/batch-processing.ts(0 hunks)workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/constants.ts(0 hunks)workspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.ts(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/ModeSwitcher/index.tsx(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx(3 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx(5 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/HelperPane.tsx(0 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/types.ts(1 hunks)workspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsx(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/Configurables/AddConfigurableVariables/index.tsx(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionWizard/index.tsx(0 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx(4 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/Configurables.tsx(1 hunks)workspaces/ballerina/component-diagram/src/components/nodes/ConnectionNode/ConnectionNodeWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/AIServiceWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GeneralWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GraphQLServiceWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/ListenerNode/ListenerNodeWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/utils/diagram.ts(3 hunks)workspaces/bi/bi-extension/src/project-explorer/activate.ts(3 hunks)workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts(5 hunks)workspaces/common-libs/ui-toolkit/src/components/Button/Button.tsx(1 hunks)
💤 Files with no reviewable changes (6)
- workspaces/ballerina/ballerina-extension/src/features/project/cmds/cmd-runner.ts
- workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts
- workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionWizard/index.tsx
- workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/batch-processing.ts
- workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/HelperPane.tsx
- workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/constants.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/ballerina/ballerina-extension/src/views/bbe/activator.tsworkspaces/ballerina/ballerina-extension/src/views/visualizer/activate.tsworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-extension/src/extension.tsworkspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.tsworkspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsxworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/ballerina/ballerina-extension/src/views/bbe/activator.tsworkspaces/ballerina/ballerina-extension/src/extension.tsworkspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.tsworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
📚 Learning: 2025-11-05T10:31:47.583Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 830
File: workspaces/ballerina/ballerina-extension/test/ai/post_proccess/post.test.ts:0-0
Timestamp: 2025-11-05T10:31:47.583Z
Learning: In the workspaces/ballerina/ballerina-extension project, the tsconfig.json uses "rootDirs": ["src", "test"], which allows test files to import from src using shorter relative paths. For example, from test/ai/post_proccess/, the import '../../stateMachine' correctly resolves to src/stateMachine.ts due to this configuration.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.tsworkspaces/ballerina/ballerina-extension/src/views/visualizer/activate.tsworkspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.tsworkspaces/ballerina/ballerina-extension/src/features/bi/activator.tsworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
🧬 Code graph analysis (7)
workspaces/ballerina/ballerina-side-panel/src/components/ModeSwitcher/index.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
getDefaultExpressionMode(49-52)
workspaces/ballerina/ballerina-extension/src/views/visualizer/activate.ts (2)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (2)
openView(821-833)StateMachine(793-819)workspaces/ballerina/ballerina-extension/src/rpc-managers/visualizer/rpc-manager.ts (1)
openView(46-59)
workspaces/ballerina/component-diagram/src/utils/diagram.ts (1)
workspaces/ballerina/bi-diagram/src/resources/constants.ts (1)
NODE_GAP_Y(53-53)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
updateFieldsSelection(67-112)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (4)
workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts (1)
buildProjectsStructure(26-59)workspaces/ballerina/ballerina-extension/src/RPCLayer.ts (1)
notifyCurrentWebview(175-177)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
ProjectInfo(1926-1935)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (2)
workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (1)
ProjectExplorerEntryProvider(72-274)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
buildCompletionSource(394-429)
🔇 Additional comments (27)
workspaces/common-libs/ui-toolkit/src/components/Button/Button.tsx (1)
35-36: LGTM! Clean extension of ButtonProps interface.The new mouse event handlers are correctly typed and will be passed through to the underlying VSCodeButton component via the spread operator. This enables fine-grained control over event propagation in consuming components.
workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/AIServiceWidget.tsx (1)
112-119: LGTM! Correct wiring of event handlers.The MenuButton correctly wires the mouse event handlers to prevent propagation, ensuring menu interactions don't trigger parent node drag/select behavior.
workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GraphQLServiceWidget.tsx (1)
256-263: LGTM! Event handlers correctly wired.workspaces/ballerina/component-diagram/src/components/nodes/ListenerNode/ListenerNodeWidget.tsx (1)
207-214: LGTM! Propagation control correctly implemented.workspaces/ballerina/component-diagram/src/components/nodes/ConnectionNode/ConnectionNodeWidget.tsx (1)
210-217: LGTM! Menu interaction isolation working as intended.workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GeneralWidget.tsx (1)
262-269: LGTM! Event propagation correctly controlled.workspaces/ballerina/component-diagram/src/utils/diagram.ts (2)
27-27: LGTM!The new constants
NODE_GAP_YandLISTENER_NODE_HEIGHTare correctly imported and used in the updated positioning logic.
67-86: LGTM!The listener separation logic correctly distinguishes between listeners with and without attached services. Positioning connected listeners at the average Y of their service nodes is a sensible approach for visual alignment.
workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts (1)
20-20: LGTM! Imports support the new workspace detection logic.The added imports (Uri and workspace/project utility functions) are appropriate for implementing the runtime guards that determine whether to activate the test explorer.
Also applies to: 25-27
workspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.ts (1)
36-39: LGTM: Removal of SHOW_EXAMPLES command aligns with PR objectives.The removal of the
SHOW_EXAMPLEScommand from theVSCODE_COMMANDSobject is consistent with the PR's goal of removing the Ballerina by Examples (BBE) feature.workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (1)
128-150: RECORD mode sharing TextModeEditor path looks consistentRouting both
InputMode.TEXTandInputMode.RECORDthroughTextModeEditoraligns with the new record-mode semantics and keeps the chip editor strictly for expression mode. No issues spotted here.workspaces/ballerina/ballerina-side-panel/src/components/ModeSwitcher/index.tsx (1)
34-37: Default mode change to RECORD for record fields is consistentUsing
InputMode.RECORDas the primary/default mode whenisRecordTypeFieldis true keeps the UI aligned withExpressionEditor’s record handling. Dependency array[valueTypeConstraint, isRecordTypeField]is also appropriate.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/types.ts (1)
19-28: Disregard the enum rename concern – InputMode.GUIDED was never used in the codebase.The review conflated two separate enums. InputMode (in ChipExpressionEditor/types.ts) and TypeInputMode (in TypeModeSwitcher/index.tsx) are distinct. Verification found zero references to
InputMode.GUIDEDanywhere—onlyTypeInputMode.GUIDEDexists in the unrelated type editor. All currentInputModeusages in ExpressionField.tsx, ExpressionEditor.tsx, and ModeSwitcher/index.tsx referenceTEXT,EXP, orRECORDvalues. The change poses no breaking change risk.Likely an incorrect or invalid review comment.
workspaces/ballerina/ballerina-visualizer/src/views/BI/Configurables/AddConfigurableVariables/index.tsx (1)
55-55: LGTM!The modification flag is correctly set before persisting the config variable, signaling that the default value has been modified. This aligns with the broader configurables workflow changes in the PR.
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
56-56: LGTM!The new command constant
NOTIFY_PROJECT_EXPLORERis well-named and follows the existing naming conventions for BI commands.workspaces/ballerina/ballerina-extension/src/extension.ts (1)
157-159: BBE activation properly disabled with future work tracked.The BBE activation is correctly commented out, consistent with changes in
workspaces/ballerina/ballerina-extension/src/views/bbe/activator.ts. The TODO comment ensures this isn't forgotten for future implementation.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/Configurables.tsx (1)
149-149: LGTM!The modification flag is correctly set before the RPC call, consistent with the pattern in
workspaces/ballerina/ballerina-visualizer/src/views/BI/Configurables/AddConfigurableVariables/index.tsx.workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts (1)
199-203: LGTM!The early return guard prevents potential runtime errors when
projectPathis undefined, avoiding downstream path construction failures. This is a good defensive programming practice.workspaces/ballerina/ballerina-extension/package.json (1)
1183-1183: LGTM!The new
download-ls:prereleasescript correctly integrates the--replaceflag introduced inscripts/download-ls.js, enabling forced re-download of prerelease language server versions during the build process.workspaces/ballerina/ballerina-extension/scripts/download-ls.js (2)
13-13: LGTM!The
forceReplaceflag is correctly parsed from command-line arguments, enabling the new forced re-download behavior.
181-190: The review comment can be dismissed; the code is compatible with the documented Node.js requirement.The project's README explicitly requires Node.js version 22.x or later. Since
fs.rmSync()was introduced in Node.js 14.14.0, the enforced minimum version (22.x) far exceeds the API requirement. No compatibility issue exists, and no changes are needed.Likely an incorrect or invalid review comment.
workspaces/ballerina/ballerina-extension/src/views/bbe/activator.ts (1)
106-109: GitHub issue is valid and actively tracked—no changes needed.The verification confirms that issue #1967 ("Implement BI sample onboarding flow") exists in the wso2/product-ballerina-integrator repository, is in OPEN state, and is actively being tracked. The TODO comment in the code appropriately documents the dependency on this external issue and includes a valid reference link. The implementation is correctly deferred pending issue resolution.
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (2)
720-727: Guard against undefinedprojectRoot/context.documentUriin inline mapping path calculation
path.relative(projectRoot, context.documentUri)and the subsequentpath.relative(workspacePath, context.documentUri)will throw if either side isundefined. It looks likeprojectRoot/documentUrishould be set by the state machine, but this function currently assumes that invariant.To make this more robust, consider deriving a
currentFilePathfrom the most reliable source (e.g.inlineMappingRequest.metadata.codeData.lineRange.fileName), and only callingpath.relativeonce both it andprojectRoot(orworkspacePath) are non-empty:const currentFilePath = context.documentUri || inlineMappingRequest.metadata.codeData.lineRange.fileName; if (!currentFilePath || !projectRoot) { throw new Error("Unable to determine current file path or project root for inline mappings"); } let targetFilePath = path.relative(projectRoot, currentFilePath); const workspacePath = context.workspacePath; if (workspacePath) { targetFilePath = path.relative(workspacePath, currentFilePath); }
800-829: Workspace-relative filename handling for context types looks correctUsing
StateMachine.context()plusworkspacePath/projectPathto normalizefilePathto a workspace-relativetargetFilePath(with an absolute-path guard) is consistent with the rest of the AI mapping flows and should produce stable filenames in the response payload.workspaces/ballerina/ballerina-extension/src/views/visualizer/activate.ts (1)
79-82: VerifylangClient/projectRootavailability in SHOW_VISUALIZERThe new flow (workspace-aware handling +
getProjectInfo→updateProjectRootAndInfo) is directionally good, but it assumes a few invariants:
StateMachine.langClient()is non-null whenSHARED_COMMANDS.SHOW_VISUALIZERis invoked andprojectRootis truthy.documentPath(and thusprojectRoot = await findBallerinaPackageRoot(documentPath)) is available when the command is triggered from contexts without an active editor (e.g., palette).It’s worth double-checking these call sites; if there’s any chance
langClientorprojectRootcan be unset here, adding simple guards (or short-circuiting back to the previous behavior) would avoid runtime errors.Also applies to: 83-106, 108-114
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
111-125: Project-root+info update and tree synchronization look coherentUsing
UPDATE_PROJECT_ROOT_AND_INFOplus theStateMachine.updateProjectRootAndInfohelper to atomically:
- update
projectPathandprojectInfoin context,- rebuild project structure via
buildProjectsStructure(..., true),- notify the webview and project explorer (
notifyCurrentWebview+notifyTreeView),gives you a clean single entry point for retargeting the current project. The additional
notifyTreeViewcalls onUPDATE_PROJECT_LOCATION,OPEN_VIEW, andVIEW_UPDATEtransitions are consistent with that model and should keep the BI explorer in sync with whatever the visualizer is focused on.Also applies to: 169-176, 188-223, 300-327, 385-433, 807-812
workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (1)
72-123: Refresh gating, project dedupe, and position wiring look good
- The
_isRefreshing/_pendingRefreshpattern inrefreshavoids overlapping refreshes and correctly queues an extra refresh when needed.- Deduplicating
projectStructure.projectsbyprojectPathbefore generating tree data prevents duplicate roots in multi-project scenarios.getComponentsnow threadscomp.iconandcomp.positionintoProjectExplorerEntryand usescomp.positionwhen invokingSHARED_COMMANDS.SHOW_VISUALIZER, which aligns with the new position-aware visualizer and state machine wiring.Also applies to: 314-324, 514-537
| testController = tests.createTestController('ballerina-integrator-tests', 'WSO2 Integrator: BI Tests'); | ||
|
|
||
| const workspaceRoot = getWorkspaceRoot(); | ||
|
|
||
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | ||
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | ||
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | ||
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | ||
|
|
||
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Fix resource leak: dispose testController on early exit.
The testController is created at line 32 before the workspace validation checks. If the early return executes (lines 41-43), the controller is never disposed or added to subscriptions, causing a resource leak.
Apply this diff to move controller creation after validation:
export async function activate(ballerinaExtInstance: BallerinaExtension) {
- testController = tests.createTestController('ballerina-integrator-tests', 'WSO2 Integrator: BI Tests');
-
const workspaceRoot = getWorkspaceRoot();
const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot));
const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot));
const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject();
const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE;
if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) {
return;
}
+
+ testController = tests.createTestController('ballerina-integrator-tests', 'WSO2 Integrator: BI Tests');
// Create test profiles to display.
testController.createRunProfile('Run Tests', TestRunProfileKind.Run, runHandler, true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| testController = tests.createTestController('ballerina-integrator-tests', 'WSO2 Integrator: BI Tests'); | |
| const workspaceRoot = getWorkspaceRoot(); | |
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | |
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | |
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | |
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | |
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | |
| return; | |
| } | |
| const workspaceRoot = getWorkspaceRoot(); | |
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | |
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | |
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | |
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | |
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | |
| return; | |
| } | |
| testController = tests.createTestController('ballerina-integrator-tests', 'WSO2 Integrator: BI Tests'); |
🤖 Prompt for AI Agents
workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts
around lines 32-43: testController is instantiated before workspace/project
validation and never disposed on the early return, causing a leak; move the
testController creation to after the validation branch (i.e., only create/add to
subscriptions once isBallerinaWorkspace || isBallerinaProject || isSingleFile is
true) OR, alternatively, dispose testController immediately before returning
from the early-exit branch so the controller is not leaked.
| const workspaceRoot = getWorkspaceRoot(); | ||
|
|
||
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | ||
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | ||
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | ||
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | ||
|
|
||
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Add error handling for workspace detection logic.
The async checks for workspace type (lines 36-38) have no error handling. If any of these utility functions throw due to file I/O errors, permission issues, or invalid paths, activation will fail silently without cleanup or user feedback.
Consider wrapping the detection logic in a try-catch block:
+ try {
const workspaceRoot = getWorkspaceRoot();
const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot));
const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot));
const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject();
const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE;
if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) {
return;
}
+ } catch (error) {
+ console.error('Failed to determine Ballerina workspace type:', error);
+ return;
+ }📝 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 workspaceRoot = getWorkspaceRoot(); | |
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | |
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | |
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | |
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | |
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | |
| return; | |
| } | |
| try { | |
| const workspaceRoot = getWorkspaceRoot(); | |
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | |
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | |
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | |
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | |
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | |
| return; | |
| } | |
| } catch (error) { | |
| console.error('Failed to determine Ballerina workspace type:', error); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts
around lines 34 to 43, the async workspace detection calls
(checkIsBallerinaWorkspace, checkIsBallerinaPackage, getCurrentBallerinaProject)
are unguarded and can throw causing silent activation failure; wrap these calls
in a try-catch, log the caught error via the extension logger, show a
user-facing VS Code notification (e.g., window.showErrorMessage) about the
detection failure, and return early from activation to avoid leaving the
extension in a broken state; ensure any resources are cleaned up before
returning.
| const [inputMode, setInputMode] = useState<InputMode>(recordTypeField ? InputMode.RECORD : InputMode.EXP); | ||
| const [isExpressionEditorHovered, setIsExpressionEditorHovered] = useState<boolean>(false); |
There was a problem hiding this comment.
Record fields now force RECORD mode, but comment still mentions GUIDED
Using InputMode.RECORD as the initial and enforced mode when recordTypeField is present, and opening the record config page directly on focus in that mode, all align well with the new RECORD semantics.
Minor nit: the comment above the effect still says “always use GUIDED mode” while the code now uses InputMode.RECORD, which can be confusing for future readers. Consider updating it, e.g.:
- useEffect(() => {
- // If recordTypeField is present, always use GUIDED mode
+ useEffect(() => {
+ // If recordTypeField is present, always use RECORD mode
if (recordTypeField) {
setInputMode(InputMode.RECORD);
return;
}Otherwise, the mode handling and focus behavior look correct.
Also applies to: 403-431, 433-449
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx
around lines 346-347 (and also update the comment occurrences near 403-431 and
433-449), the inline comment still says “always use GUIDED mode” even though the
code now initializes and enforces InputMode.RECORD when recordTypeField is
present; update those comments to accurately state that RECORD mode is
used/forced for recordTypeField (and that focus opens the record config page),
keeping the intent and wording concise so future readers aren’t misled by the
old GUIDED reference.
| const isInitialized = useRef(false); | ||
|
|
||
| // Initialize: If the union is selected, ensure the selected member and its required fields are also selected | ||
| // Initialize union member selection when param becomes selected |
There was a problem hiding this comment.
🧩 Analysis chain
Initialization guard can skip union member setup when param.selected changes later
The isInitialized flag is set in the first useEffect before we know whether initializeUnionMember() actually did anything, and the second effect is guarded by !isInitialized.current. This combination means:
- If the union param is initially unselected (and not required), the first effect sets
isInitialized.current = truebut doesn’t callinitializeUnionMember(). - Later, if
param.selectedbecomestruefrom outside this component, the second effect won’t run the initializer because!isInitialized.currentis alreadyfalse.
That can leave param.selected === true while no member (and its nested required fields) is initialized.
A more robust pattern is to let initializeUnionMember set isInitialized.current when it actually succeeds, and have callers just check the flag:
- const isInitialized = useRef(false);
+ const isInitialized = useRef(false);
@@
- const initializeUnionMember = () => {
+ const initializeUnionMember = () => {
if (!param.members || param.members.length === 0) {
return false;
}
// Check if a member is already selected
const hasSelectedMember = param.members.some(member => member.selected === true);
if (hasSelectedMember) {
- return true; // Already initialized
+ isInitialized.current = true;
+ return true; // Already initialized
}
@@
- if (memberToSelect) {
+ if (memberToSelect) {
const memberTypeName = getUnionParamName(memberToSelect);
if (memberTypeName) {
const selectedMember = param.members.find((field) => getUnionParamName(field) === memberTypeName);
if (selectedMember) {
updateFormFieldMemberSelection(selectedMember);
@@
- setSelectedMemberType(memberTypeName);
- setParameter(selectedMember);
-
- return true;
+ setSelectedMemberType(memberTypeName);
+ setParameter(selectedMember);
+
+ isInitialized.current = true;
+ return true;
}
}
}
return false; // Failed to initialize
@@
- useEffect(() => {
- // Only run initialization once
- if (isInitialized.current) {
- return;
- }
- isInitialized.current = true;
-
- // If union param is selected (or required), ensure a member is selected
- if (paramSelected && param.members && param.members.length > 0) {
- initializeUnionMember();
- }
- }, []);
+ useEffect(() => {
+ // If union param is selected (or required), ensure a member is selected
+ if (paramSelected && param.members && param.members.length > 0 && !isInitialized.current) {
+ initializeUnionMember();
+ }
+ }, []);
@@
- useEffect(() => {
- // If param becomes selected but we haven't initialized the member yet
- if (param.selected && !isInitialized.current && param.members && param.members.length > 0) {
- const initialized = initializeUnionMember();
- if (initialized) {
- isInitialized.current = true;
- }
- }
- }, [param.selected]);
+ useEffect(() => {
+ // If param becomes selected but we haven't initialized the member yet
+ if (param.selected && param.members && param.members.length > 0 && !isInitialized.current) {
+ initializeUnionMember();
+ }
+ }, [param.selected]);
@@
- if (newSelectedState) {
- // Ensure member is initialized before proceeding
- if (!isInitialized.current) {
- initializeUnionMember();
- isInitialized.current = true;
- }
+ if (newSelectedState) {
+ // Ensure member is initialized before proceeding
+ if (!isInitialized.current) {
+ initializeUnionMember();
+ }This keeps the “run once” semantics, but only after real initialization has happened, and preserves the safety net for cases where param.selected flips outside this component.
Also applies to: 47-55, 58-81, 83-95, 97-105, 165-176
---
🏁 Script executed:
```shell
#!/bin/bash
# Extract the UnionType component to examine implementation
cat workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx
Length of output: 10745
Initialization guard can skip union member setup when param.selected changes after mount—fix confirmed
The identified issue is real. When the component mounts with param.selected = false and is not required:
- The first
useEffect(line 82–93) setsisInitialized.current = trueat line 90 before checking whetherinitializeUnionMember()actually ran. - Later, if
param.selectedbecomestrueexternally, the seconduseEffect(line 95–104) will not execute because!isInitialized.currentis alreadyfalse(line 97). - The union member initialization is skipped.
The proposed fix is sound: move isInitialized.current = true into the initializeUnionMember() function at successful initialization points (after member selection logic at lines 58, 71–73, and similarly in toggleParamCheck at line 174). This ensures the flag reflects actual initialization state, not just whether the effects have run.
Additionally, updateFieldsSelection is called redundantly:
- Line 69–70 in
initializeUnionMemberduplicates the call already made byupdateFormFieldMemberSelectionat line 118–119. - Line 138–139 in
handleMemberTypesimilarly duplicates the call fromupdateFormFieldMemberSelection.
Remove the redundant calls at lines 69–70 and 138–139 for clarity.
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx
(context around lines 43–45 and the referenced effects/handlers), the
isInitialized guard is set too early which can prevent union member
initialization when param.selected changes after mount; move setting
isInitialized.current = true into initializeUnionMember() at the points where
initialization actually completes (after the member selection logic around lines
58 and 71–73) and also set it in toggleParamCheck once initialization succeeds
(around line 174), and remove the redundant updateFieldsSelection calls at lines
69–70 and 138–139 so updateFormFieldMemberSelection remains the single caller
for field selection updates.
| // Position unconnected listeners below all other nodes | ||
| if (unconnectedListeners.length > 0) { | ||
| // Find the maximum Y position among all nodes | ||
| const allNodes = [...connectedListeners, ...entryNodes, ...connectionNodes]; | ||
| let maxY = 100; // Default starting position if no other nodes | ||
|
|
||
| if (allNodes.length > 0) { | ||
| maxY = Math.max(...allNodes.map(node => { | ||
| const nodeHeight = node.height || LISTENER_NODE_HEIGHT; | ||
| return node.getY() + nodeHeight; | ||
| })); | ||
| } | ||
|
|
||
| // Position unconnected listeners below, with spacing | ||
| unconnectedListeners.forEach((listenerNode, index) => { | ||
| const yPosition = maxY + NODE_GAP_Y/2 + (index * (LISTENER_NODE_HEIGHT + NODE_GAP_Y/2)); | ||
| listenerNode.setPosition(listenerX, yPosition); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Incorrect height fallback for heterogeneous node types.
Line 109 uses LISTENER_NODE_HEIGHT as the fallback for all node types (connected listeners, entry nodes, and connection nodes), but entry nodes have dynamic heights calculated by calculateEntryNodeHeight() (lines 249-264), and connection nodes may also have different dimensions. Using LISTENER_NODE_HEIGHT as a universal fallback can result in an incorrect maxY calculation, potentially causing unconnected listeners to overlap with taller entry or connection nodes.
Consider using type-specific height constants or calculating heights based on node type:
if (allNodes.length > 0) {
maxY = Math.max(...allNodes.map(node => {
- const nodeHeight = node.height || LISTENER_NODE_HEIGHT;
+ let nodeHeight = LISTENER_NODE_HEIGHT; // default
+ if (node.getType() === NodeTypes.ENTRY_NODE) {
+ // Use actual entry node height or a safe default
+ nodeHeight = node.height || 200; // entry nodes are typically taller
+ } else if (node.getType() === NodeTypes.CONNECTION_NODE) {
+ nodeHeight = node.height || 100; // connection node height
+ } else {
+ nodeHeight = node.height || LISTENER_NODE_HEIGHT;
+ }
return node.getY() + nodeHeight;
}));
}Alternatively, verify that all node models consistently populate the height property so the fallback is rarely needed.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In workspaces/ballerina/component-diagram/src/utils/diagram.ts around lines 101
to 119, the maxY fallback uses LISTENER_NODE_HEIGHT for all node types which is
wrong for entry and connection nodes; change the height fallback to use a
type-aware height (e.g. if node is an entry node call
calculateEntryNodeHeight(node) or use node.type to select the correct
constant/height calculator, and for connection nodes use their specific height
or a CONNECTION_NODE_HEIGHT constant) so maxY is computed from the actual
heights; alternatively ensure every node model sets node.height earlier and use
that consistently instead of defaulting to LISTENER_NODE_HEIGHT.
| private matchesPathAndPosition(item: ProjectExplorerEntry, targetPath: string, targetPosition: NodePosition | undefined): boolean { | ||
| // Path must match | ||
| if (item.info !== targetPath) { | ||
| return false; | ||
| } | ||
|
|
||
| // If no target position is provided, match by path only | ||
| if (!targetPosition) { | ||
| return true; | ||
| } | ||
|
|
||
| // If target position is provided but item has no position, don't match | ||
| if (!item.position) { | ||
| return true; // Fall back to path-only matching for items without position | ||
| } | ||
|
|
||
| // Compare positions | ||
| return this.positionsMatch(item.position, targetPosition); | ||
| } |
There was a problem hiding this comment.
Fix path+position matching fallback for items without position
In matchesPathAndPosition:
// If target position is provided but item has no position, don't match
if (!item.position) {
return true; // Fall back to path-only matching for items without position
}The comment and code disagree. As written, when a targetPosition is provided but the tree item has no position, this returns true, so any item with the right path but no position will match a position-specific request.
If the intent is to only fall back to path-only matching when no target position is provided (which the earlier branch already handles), this branch should return false:
- // If target position is provided but item has no position, don't match
- if (!item.position) {
- return true; // Fall back to path-only matching for items without position
- }
+ // If target position is provided but item has no position, don't match
+ if (!item.position) {
+ return false;
+ }This keeps path-only matches in the !targetPosition case, and ensures position-aware reveals only select items that actually carry positional metadata.
🤖 Prompt for AI Agents
In workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
around lines 198 to 216, the branch that handles a missing item.position
incorrectly returns true when a targetPosition is provided; change the behavior
so that if a targetPosition exists but the item lacks position metadata the
function returns false (i.e., only fall back to path-only matching when
targetPosition is undefined), leaving the earlier checks and the final
positionsMatch call intact.
Merge "ballerina-integrator-1.5.2" into "release-ballerina"
Sync bi-1.5.x with release-ballerina branch changes
|
|
|
Closing this, all change are merge to |
8 participants
Purpose
This pull request Syncs the
maincodebase with features and fixes developed for the BI extension'sv1.5.xrelease cycle.Summary by CodeRabbit
New Features
Bug Fixes
Removed Features
✏️ Tip: You can customize this high-level summary in your review settings.