sessions: fix: guard chat grid neighbor lookup before layout - #331617
Conversation
Apply the existing outer layout to a newly created session grid before synchronous reconciliation can query adjacent views. Add coverage for opening and focusing a session whose active chat is a child chat. Fixes microsoft#331616 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b53c514c-a525-410f-94bc-213bc6bdaaf1
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Benjamin Christopher Simmonds (@benibenj)Matched files:
|
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR fixes a session-opening race where chat reconciliation ran before the session grid’s first layout, causing adjacent-view queries to throw and leaving the chat UI partially initialized when the active chat is a child chat.
Changes:
- Call
_applyLayout()before installing the synchronous reconciliationautorunto ensure the grid has performed its first layout. - Add a regression test that opens and focuses a session whose active chat is a child chat with a
parentChat.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/browser/parts/chatGroupsView.ts | Reorders layout vs. reconciliation setup to ensure grid layout invariants before adjacent-view queries. |
| src/vs/sessions/test/browser/chatGroupsView.test.ts | Adds a regression test covering an already-laid-out view opening a session with an active child chat. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
Assert that reconciliation lays out the replacement chat view and scope the focus check to the view owner document. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b53c514c-a525-410f-94bc-213bc6bdaaf1
|
Manual E2E verification re-run on macOS against the performance-safe fixed source build (
This exercises the same session-switch path and active-child prerequisite as the original failure. The focused regression additionally records layout calls: the resolved chat is laid out once and the transient parallel-composer view is laid out zero times. |
Track layout for the current session grid so initial reconciliation uses logical group order until geometric neighbor queries are safe. Preserve reconcile-before-layout ordering and cover that the transient composer receives no layout calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b53c514c-a525-410f-94bc-213bc6bdaaf1
2c79ffb
into
microsoft:main
* sessions: fix: lay out chat grid before reconciliation Apply the existing outer layout to a newly created session grid before synchronous reconciliation can query adjacent views. Add coverage for opening and focusing a session whose active chat is a child chat. Fixes #331616 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b53c514c-a525-410f-94bc-213bc6bdaaf1 * sessions: test: verify active child layout and focus Assert that reconciliation lays out the replacement chat view and scope the focus check to the view owner document. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b53c514c-a525-410f-94bc-213bc6bdaaf1 * sessions: fix: guard chat grid neighbor lookup before layout Track layout for the current session grid so initial reconciliation uses logical group order until geometric neighbor queries are safe. Preserve reconcile-before-layout ordering and cover that the transient composer receives no layout calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b53c514c-a525-410f-94bc-213bc6bdaaf1 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b53c514c-a525-410f-94bc-213bc6bdaaf1
Fixes #331616
What changed
This change tracks whether the current Agents window session grid has completed its first layout. Reconciliation uses logical group order until geometric neighbor queries are safe.
It also adds a regression test for opening and focusing a session whose active chat is a child chat with a
parentChat.User-visible failure
Opening an affected session could leave the chat area partially initialized:
Clicking the visible main tab repaired the view and caused the remaining chat tab to appear.
Root cause
ChatGroupsView.setSessionpreviously created and attached a newSerializableGrid, installed anautorunthat synchronously invoked_reconcile, and only then called_applyLayout.When the outer
ChatGroupsViewhad already been laid out and the session's active chat had aparentChat, reconciliation attempted to place the child beside its parent._findAdjacentGroupcalledSerializableGrid.getNeighborViews, but the new grid had not received its firstlayout()call and threw:The main chat had already been assigned before the exception. Since those observable writes were not rolled back, the UI retained the main tab while failing to assign or activate the child chat, producing the partial state above.
Why this fix
The current grid resets a
_gridDidLayoutflag when the session changes and sets it afterlayout()completes._findAdjacentGrouponly callsgetNeighborViewsonce that flag is true; before then it uses its existing logical group-order fallback.This retains the original reconcile-before-layout ordering, so the resolved chat view is laid out once and the transient parallel-conversation composer is not laid out before being replaced.
The change does not add exception handling or a focus-specific workaround.
Validation
ChatGroupsViewunit tests pass.git diff --checkpasses.Can't call getNeighborViews before first layouterror.