fix(vscode-ide-companion): use existing editor group for diff instead of forcing a new one#4130
Merged
Merged
Conversation
… of forcing a new one When the chat webview is in the leftmost group, opening a diff previously called ensureLeftGroupOfChatWebview() which forcibly created a new editor group. This was disruptive UX — there is often an existing empty group to the right that could be reused. Change the fallback chain from "left neighbor → force-create → Beside" to "left neighbor → right neighbor → Beside". Also apply the same fix to the readonly file opener in FileMessageHandler.
Collaborator
Author
|
Test: |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the VS Code IDE companion to stop force-creating a new editor group when opening diffs/readonly files, instead reusing an existing neighboring editor group (right side fallback) when the chat webview is in the leftmost group.
Changes:
- Update diff opening logic to prefer left neighbor, then reuse right neighbor before falling back.
- Add
findRightGroupOfChatWebviewand extract sharedfindWebviewGrouphelper. - Apply the same neighbor-group fallback behavior to the readonly file opener in
FileMessageHandler.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/vscode-ide-companion/src/diff-manager.ts | Switches diff target group selection to try left neighbor then right neighbor. |
| packages/vscode-ide-companion/src/utils/editorGroupUtils.ts | Adds right-neighbor lookup and deduplicates chat webview group discovery. |
| packages/vscode-ide-companion/src/webview/handlers/FileMessageHandler.ts | Reuses the same neighbor-group selection approach for readonly file opening. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
…fallback, shared scan helper, comment accuracy - Add ?? vscode.ViewColumn.Beside to targetViewColumn declaration so the fallback is explicit even if the downstream usage is reached without it - Extract findNeighborGroup helper to de-duplicate the near-identical scan loops in findLeftGroupOfChatWebview and findRightGroupOfChatWebview - Update stale comment in FileMessageHandler to reflect that the readonly document may open in the right group, not only the left
…fix param naming, add tests - Delete ensureLeftGroupOfChatWebview and waitForTabGroupsCondition which are no longer called by any code path - Remove now-unused openChatCommand import - Rename _cur → cur in findNeighborGroup callbacks (param was used, the underscore prefix was misleading) - Add editorGroupUtils.test.ts with 12 unit tests for findLeft/findRight - Add createAndOpenTempFile viewColumn tests to FileMessageHandler.test.ts covering left-neighbor, right-neighbor, and Beside fallback cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sageHandler with diff-manager Move vscode.ViewColumn.Beside fallback to the targetViewColumn declaration so both diff-manager and FileMessageHandler follow the same pattern: left ?? right ?? Beside at declaration, plain viewColumn at usage.
…r.test.ts vscodeMock.window.tabGroups.all was initialized as plain [] which TypeScript infers as never[], causing assignment errors in CI. Add explicit type annotation to match the objects assigned in tests.
…issing webview too The fallback chain left ?? right ?? Beside also falls through to Beside when the chat webview group is not found (both helpers return undefined). Update comments in both diff-manager and FileMessageHandler.
tanzhenxin
approved these changes
May 15, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Review
This fixes the diff-editor-forced-new-group bug by replacing the asynchronous left-only fallback (which had a real race between newGroupLeft, onDidChangeTabGroups, and the diff open) with a fully synchronous left → right neighbor → ViewColumn.Beside chain. The same fix lands on the readonly file opener so the diff editor and the temp-file opener no longer drift. The extracted findWebviewGroup and symmetric findNeighborGroup helpers also remove a copy-paste of the webview-group scan, and 17 new tests cover both helpers and the message-handler call site.
Verdict
APPROVE — Correct, removes a real async race, simplifies the helper module, and adds proportional test coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FileMessageHandlerDetails
Before:
left neighbour → force-create new groupAfter:
left neighbour → right neighbour → ViewColumn.BesideThe
ViewColumn.Besidefallback only triggers when neither a left nor right neighbor exists.Changed files
diff-manager.tsensureLeftGroupOfChatWebviewwithfindRightGroupOfChatWebviewin fallback logiceditorGroupUtils.tsfindRightGroupOfChatWebview, extract sharedfindWebviewGrouphelper to deduplicate webview lookupFileMessageHandler.tsTest plan
ViewColumn.Besidewhen no neighbor groups exist