fix: guard notebook match selection against unmaterialized tree element (fixes #328427)
- Dominant language
- TypeScript
- Stars
- 193k
- Forks
- 42.4k
- PR merge metrics
- PR metrics pending
Description
### Summary
`TreeError [SearchView] Tree element not found: [object Object]` is thrown when opening a Search view result that lives inside a notebook. In `SearchView.open()`, after `elemParent.updateMatchesForEditorWidget()` rebuilds the notebook file match's child matches, the code calls `this.tree.setSelection([match])` on the `WorkbenchCompressibleAsyncDataTree`. Because that tree lazy-loads its nodes, the freshly-rebuilt `match` object may not yet be materialized in the tree, so `setSelection` throws an unhandled error surfaced to telemetry (856 users on 1.131.0).
Fixes microsoft/vscode\#328427
Recommended reviewer: `@osortega`
### Culprit Commit
| Field | Value |
|-------|-------|
| Commit | [`88056f2`](https://github.com/microsoft/vscode/commit/88056f2c97d6bde7cf151bfff35b4c3e2c65b407) |
| Author | `@andreamah` |
| PR | #228886 |
| Message | Use `WorkbenchCompressibleAsyncDataTree` for lazy loading AI results (#228886) |
| Why | This commit converted the Search view tree from a fully-materialized sync tree to a lazy-loading async data tree. `open()` kept calling `setSelection`/`setFocus` with a `match` reference that a sync tree always contained but the async tree only contains once its parent chain has been loaded, so the pre-existing notebook-branch call became able to throw `Tree element not found`. This is effectively a pre-existing mechanism (the async-tree conversion predates the shipped build by well over a year); the recent telemetry spike is a re-surfacing variant of #326646, whose prior fix (#326652) guarded a different code path (`textSearchQuickAccess.moveToSearchViewlet`) but not this notebook `open()` path. |
### Code Flow
```mermaid
sequenceDiagram
participant User as Search result (notebook)
participant Open as SearchView.open
participant Parent as NotebookFileInstanceMatch
participant Tree as CompressibleAsyncDataTree
participant Model as CompressedObjectTreeModel
User->>Open: open notebook match
Open->>Parent: updateMatchesForEditorWidget()
Note over Parent: rebuilds child matches;
new match objects created
Open->>Tree: setSelection([match])
Tree->>Model: map match -> node (getNode/getCompressedNode)
Note over Model: ⚠️ match not yet in nodes map
Note over Model: 💥 TreeError [SearchView] Tree element not found
```
### Affected Files
| File | Role | Evidence |
|------|------|----------|
| `src/vs/base/browser/ui/tree/compressedObjectTreeModel.ts` | crash site | L347 `throw new TreeError(this.user, 'Tree element not found: ...')` (from stack) |
| `src/vs/workbench/contrib/search/browser/searchView.ts` | root cause | L2289-L2294: `setSelection([match])`/`setFocus([match])` called on the lazy async tree with a `match` rebuilt by `updateMatchesForEditorWidget()` (L2283) that may not be materialized |
| `src/vs/base/browser/ui/tree/asyncDataTree.ts` | precondition | `hasNode()` reports whether an element has been materialized in the async tree |
### Repro Steps
1. Open a workspace containing notebooks with many text matches for a common term.
2. Search for that term in the Search view so notebook file matches appear.
3. Open a notebook match whose parent notebook file match is not yet expanded/materialized in the tree.
4. `updateMatchesForEditorWidget()` rebuilds the child matches; intermittently the rebuilt `match` has not yet been materialized in the async tree, so `setSelection` throws `TreeError [SearchView] Tree element not found`. Timing-dependent: larger result sets and collapsed notebook files increase the likelihood.
### How the Fix Works
**Chosen approach** — `searchView.ts` → `open()` notebook branch: guard the `showMatch`/`setSelection`/`setFocus` block with `this.tree.hasNode(match)`. When the rebuilt `match` has not been materialized in the async tree, the block is skipped — the editor has already been opened and highlighted earlier in `open()`, so no functional behavior is lost. This checks a documented precondition of the async-data-tree API before invoking `setSelection`, at the site that produces the invalid call (fix at the caller that makes the unsupported call, not at the shared tree crash site — which keeps the `TreeError`/telemetry pipeline intact for genuinely invalid input elsewhere). It reuses the exact `hasNode` materialization check already used throughout this same view (`searchView.ts` L417, L2769), so it introduces no new contract, no `try/catch`, and removes no `logService.error` call.
**Why this fix**: after this change, `searchView.ts:2292` only calls `setSelection`/`setFocus` when `hasNode` confirms the `match` exists in the tree, so `CompressedObjectTreeModel.getCompressedNode` can no longer receive an absent element from this notebook `open()` path.
**Alternatives considered**: awaiting a full recursive tree materialization/`expandTo` before selecting — rejected because it would force-expand collapsed notebook file matches (changing UX) and broaden the change well beyond the failing path, when graceful degradation (skip the redundant re-selection) already suffices. Wrapping `setSelection` in `try/catch` — rejected because it hides the symptom from telemetry instead of preventing the invalid call at the producer.
### Recommended Owner
`@osortega` — current Search View owner and reviewer/assignee of the prior accepted fix for this same signature (#326652 / #326646). The culprit-commit author's change is effectively pre-existing; the area owner is the live, authoritative owner of Search View UI.
> Generated by [errors-fix](https://github.com/microsoft/vscode-engineering/actions/runs/30644629761) · opus48 · 335.1 AIC · ⌖ 11.4 AIC · ⊞ 18.1K · [◷](https://github.com/search?q=repo%3Amicrosoft%2Fvscode+%22gh-aw-workflow-id%3A+errors-fix%22&type=pullrequests)
---
> [!NOTE]
> This was originally intended as a pull request, but the git push operation failed.
>
> **Original error:** The process '/usr/bin/git' failed with exit code 128
>
> **Workflow Run:** [View run details and download bundle artifact](https://github.com/microsoft/vscode-engineering/actions/runs/30644629761)
>
> The bundle file is available in the `agent` artifact in the workflow run linked above.
To create a pull request with the changes:
```sh
# Download the artifact from the workflow run
gh run download 30644629761 -n agent -D /tmp/agent-30644629761
# Fetch the bundle into a temporary ref, then update the local branch
git fetch /tmp/agent-30644629761/aw-microsoft-vscode-fix-search-tree-element-not-found-328427.bundle refs/heads/fix/search-tree-element-not-found-328427:refs/bundles/create-pr-fix-search-tree-element-not-found-328427-42a1139c8f064b08-d870d4c6
git update-ref refs/heads/fix/search-tree-element-not-found-328427-42a1139c8f064b08 refs/bundles/create-pr-fix-search-tree-element-not-found-328427-42a1139c8f064b08-d870d4c6
git checkout fix/search-tree-element-not-found-328427-42a1139c8f064b08
# Ensure the working tree matches the updated branch
git reset --hard
# Remove the temporary bundle ref
git update-ref -d refs/bundles/create-pr-fix-search-tree-element-not-found-328427-42a1139c8f064b08-d870d4c6
# Push the branch to origin
git push https://github.com/bryanchen-d/vscode.git fix/search-tree-element-not-found-328427-42a1139c8f064b08
# Create the pull request
gh pr create --title 'fix: guard notebook match selection against unmaterialized tree element (fixes #328427)' --base main --head bryanchen-d:fix/search-tree-element-not-found-328427-42a1139c8f064b08 --repo microsoft/vscode
```
Contributor guide
Assessment
This issue has not been assessed yet.