Re-enable auto-reveal in tree view based on the active webview #971
Conversation
…refresh requests and improve error logging
WalkthroughThe changes implement a cross-extension communication mechanism that enables the Ballerina state machine to notify the BI project explorer of view and position changes. This is accomplished through a new command Changes
Sequence Diagram(s)sequenceDiagram
participant SM as State Machine
participant CMD as BI Command Bus
participant ACT as Project Explorer<br/>Activate
participant PROV as Project Explorer<br/>Provider
participant TV as Tree View
SM->>CMD: notifyTreeView()<br/>(NOTIFY_PROJECT_EXPLORER)
CMD->>ACT: Command Handler
ACT->>PROV: revealInTreeView(documentUri,<br/>projectPath,<br/>position, view)
activate PROV
PROV->>PROV: findItemByPathAndPosition()
PROV->>PROV: matchesPathAndPosition()
PROV->>TV: treeView.reveal(item)
deactivate PROV
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts(2 hunks)workspaces/ballerina/ballerina-extension/src/stateMachine.ts(8 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)
🧰 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.
📚 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/stateMachine.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
📚 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/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/src/stateMachine.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/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
🧬 Code graph analysis (3)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (2)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)workspaces/ballerina/ballerina-core/src/interfaces/bi.ts (1)
NodePosition(23-23)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (2)
workspaces/ballerina/ballerina-core/src/interfaces/bi.ts (1)
NodePosition(23-23)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (2)
workspaces/ballerina/ballerina-core/src/interfaces/bi.ts (1)
NodePosition(23-23)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
🔇 Additional comments (23)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (2)
828-828: LGTM! Correctly uses workspace-relative path.The change to use
targetFilePathensures that generated code blocks reference the correct workspace-relative path, which aligns with the PR's objective to improve navigation and auto-reveal behavior in workspace projects.
803-822: The review comment's analysis is incorrect—the code patterns are not identical at all three locations.Lines 354-361 and 675-682 are genuinely duplicated and could be refactored, but lines 803-822 implement fundamentally different logic:
- Lines 354-361 & 675-682: Assume
absoluteCustomFunctionsPathis already absolute; simply choose between workspace-relative or project-relative paths based on whetherworkspacePathexists.- Lines 803-822: Handle the case where
filePathmay be relative or absolute (usingpath.isAbsoluteandpath.join), requiring additional processing before computing the relative path.The suggested refactoring conflates these different implementations and would lose functionality. While extracting logic from lines 354-361 and 675-682 is reasonable, including 803-822 in the same helper would be incorrect.
Likely an incorrect or invalid review comment.
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
31-57: New BI command constant is consistent and well-scoped
NOTIFY_PROJECT_EXPLORERfollows the existing BI command naming and namespace; no issues from a stability or API standpoint.workspaces/ballerina/ballerina-extension/src/stateMachine.ts (8)
5-24: Core import changes look correctBringing in
BI_COMMANDS,NodePositionand the state-machine utility helpers from@wso2/ballerina-core/ local utils is consistent with how they’re used later in the file; no issues here.
111-121: UPDATE_PROJECT_ROOT now syncs tree view with project rootThe added
notifyTreeViewcall afterbuildProjectsStructureandnotifyCurrentWebviewensures the explorer is updated when the project root changes, while still resolving the pending root-update promise. This is a sensible place to hook the notification and should be safe given the existing async flow.
161-175: UPDATE_PROJECT_LOCATION tree notification is aligned with context updatesYou first
assignthe newdocumentUri/position(with sensible fallbacks) and then notify the tree with the effective values. This keeps the explorer in sync with location-only changes without altering the active view, which matches the intent of this transition.
178-221: Initial BI / non-BI flows now notify the tree correctlyThe added
assignactions plusnotifyTreeViewin bothinitialize.onDonebranches correctly:
- Capture
isBI,projectPath,workspacePath,scope,org,package.- Emit a tree notification keyed by
event.data.projectPathand the current view.This should give the project explorer enough information to highlight the appropriate project root right after discovery.
296-325: extensionReady.OPEN_VIEW: good sequencing of context update and tree notificationUsing an action array to:
- Assign all
viewLocationfields into context, includingprojectPath.- Then call
notifyTreeViewwith the just-updated values,keeps the tree, webview, and machine context aligned. This wiring looks correct.
381-432: viewReady transitions correctly drive tree auto‑revealBoth
OPEN_VIEWandVIEW_UPDATEnow:
- Rehydrate context from
event.viewLocation(with appropriate fallbacks).- Immediately call
notifyTreeViewwith the current project path, document URI, position, and view.This ensures the explorer tracks navigation within an already-active view. The separation between
OPEN_VIEW(full re-init) andVIEW_UPDATE(position-only change) reads clean and matches the state machine’s design.
471-473: Project info fetch now prefers workspacePath, avoiding multi-project ambiguitySwitching
getProjectInfoto usecontext.workspacePath || context.projectPathshould remove ambiguity in multi-project workspaces while still working for single-project cases. This is a straightforward and correct improvement.
1023-1035: notifyTreeView helper centralizes BI explorer notificationsWrapping
commands.executeCommand(BI_COMMANDS.NOTIFY_PROJECT_EXPLORER, …)innotifyTreeViewkeeps the various call sites concise and consistent; having the payload shape defined in one place makes future changes easier.workspaces/bi/bi-extension/src/project-explorer/activate.ts (3)
18-18: Extended imports are consistent with new reveal behaviorImporting
MACHINE_VIEWandNodePositionalongside the command constants matches their usage in the new command handler; no issues here.
39-43: Tree view reference is correctly wired into the data providerCalling
projectExplorerDataProvider.setTreeView(projectTree)right after creation gives the provider the tree handle needed forrevealInTreeViewwithout changing existing activation flow.
66-95: NOTIFY_PROJECT_EXPLORER command registration is correctly forwardedRegistering
BI_COMMANDS.NOTIFY_PROJECT_EXPLORERand delegating todataProvider.revealInTreeView(documentUri, projectPath, position, view)matches the payload emitted from the Ballerina state machine and keeps the reveal logic encapsulated in the provider. This wiring looks sound.workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (9)
22-32: Core imports align with new tree and position featuresPulling in
DIRECTORY_MAP, project-structure types,VisualizerLocation,MACHINE_VIEW, andNodePositionis consistent with the new provider responsibilities and type usage; no concerns here.
44-61: Storing NodePosition on ProjectExplorerEntry is appropriateAdding the
positionfield and threading it through the constructor allows precise mapping from tree items back to source locations, which is exactly what auto-reveal needs. The defaulting and assignment are straightforward.
72-85: TreeView reference and refresh state flags are well-placedThe new
_treeView,_isRefreshing,_pendingRefreshfields andsetTreeViewmethod give the provider enough internal state to:
- Support reveal operations.
- Manage concurrent refresh requests.
This is a good encapsulation of tree view concerns inside the provider.
86-122: Refresh re-entrancy and error handling improvements look solidThe updated
refresh:
- Coalesces overlapping refresh calls via
_isRefreshingand_pendingRefresh.- Clears
_databefore repopulating and fires a change notification once.- Logs errors with a clear
[ProjectExplorer]prefix and still restores a consistent empty state.- Executes any pending refresh after the current one finishes.
This should reduce flicker and redundant work without introducing recursion issues.
124-153: revealInTreeView entrypoint correctly coordinates path/position‑based revealThe public
revealInTreeViewmethod:
- Safely no-ops when the tree view hasn’t been set.
- Prefers
documentUriwhen present, otherwise falls back toprojectPathwhen the view is notWorkspaceOverview.- Only reveals when a matching item is found, with select/focus/expand flags set.
This is a clean abstraction for external callers.
155-193: Recursive search helpers are structured clearly
findItemByPathAndPositionandsearchChildrenByPathAndPositionwalk the tree depth‑first, checking each node via a centralized matcher. The recursion and early returns are straightforward and should perform adequately for typical project sizes.
221-226: Position equality check is precise and appropriateComparing all four fields (
startLine,startColumn,endLine,endColumn) for equality is a reasonable definition of “same node” in this context. If you later need fuzzy matching (e.g., line-only), it can be relaxed here without touching callers.
276-336: Project deduplication in getProjectStructureData avoids duplicate rootsUsing a
Mapkeyed byproject.projectPathto buildfilteredProjectsensures that:
- Each physical project path appears once in the tree.
isSingleProjectis computed on the de‑duplicated list.This should prevent duplicated roots when the state context includes repeated projects.
514-531: File entry construction now correctly carries positions into the treePassing
comp.positioninto theProjectExplorerEntryconstructor wires AST positions fromProjectStructureArtifactResponseall the way into the tree, enabling accurate auto-reveal. The extraisCodiconargument (false) maintains previous icon behavior.
workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
Show resolved
Hide resolved
2 participants
Purpose
Improve the auto-reveal behavior to precisely highlight the corresponding item in the tree view based on the user’s navigation in the active webview.
Addresses: wso2/product-ballerina-integrator#1947
Screen.Recording.2025-11-19.at.20.48.26.mov
Summary by CodeRabbit