Search a project's priority documents first in Navigate To

[xperiandri]

Roslyn passes SearchProjectAsync a set of priorityDocuments — the documents the user already has open — but the F# implementation never looked at it, searching project.Documents in whatever order the project enumerates them. On a large project, the throttled parse queue could spend its first slots on files the user has never opened, so declarations from their own open files only showed up once that first batch finished.

Partition the project's documents by priority before searching, and search the priority batch first. The open documents are parsed and their results returned before the rest of the project is even queued.

Stacked on #20483 — only the last commit here is new; the diff will shrink to just that once #20483 merges.

🤖 Generated with Claude Code

[github-actions]

✅ Release notes checked

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
`vsintegration/src`	docs/release-notes/.VisualStudio/18.vNext.md

[T-Gro]

🤖 🕵️ AI review — verify independently.

[T-Gro]

🤖 🕵️ [P2] Cold linked projects show declarations from the wrong #if branch — the fallback COMPILED;EDITING cache key reuses the FOO project's parse in the non-FOO project.

// Shared path and text version; parsing options not yet cached.
// Search the FOO project, then the project without FOO.
module Shared
#if FOO
let fooOnly = 1
#else
let plainOnly = 1
#endif
// Non-FOO project: fooOnly is returned; plainOnly is missing.

[xperiandri]

The sharing is real, and it is the approximate path rather than a leak into the exact one.

A project whose options have not arrived has no defines to parse with, so the document is parsed with the editing defaults and the entry is stored under them. Two instances that are both cold therefore read one entry, and for a file with #if that entry can hold the wrong branch. What keeps it out of the exact search is the flag on the entry: GetNavigableItems takes a cached entry only when not entry.Approximate (line 139), and every entry written from a quick parse carries Approximate = true. The only caller that reads them is GetNavigableItemsWhileLoading, whose contract is explicitly "results from a previously computed cache, even if that cache is out of date", and whose results the search window marks as incomplete.

So the declarations from the wrong branch are visible exactly where stale declarations are allowed to be, and the first exact search reparses with the instance's real defines and overwrites the entry.

What is worth tightening is the one case where the two are indistinguishable: a project whose real defines happen to equal the editing defaults would read a cold instance's approximate entry as its own. The flag still refuses it for the exact search, so the effect is a redundant reparse rather than a wrong result — but the key deserves to say "defines unknown" rather than spell out a define set the instance never asked for, the same way AnyDefines says "no defines matter here". I will make that key distinct.

[T-Gro]

🤖 🕵️ [P2] Priority batching delays all Navigate To results — two 700 ms document loads on 16 workers took about 1.4 s instead of 0.8 s; this API publishes nothing until both batches finish.

let! results = search (Seq.append priority rest)
return results |> Array.concat |> Array.toImmutableArray

[xperiandri]

This one I do not think holds, because of which method it is on.

IFSharpNavigateToSearchService.SearchProjectAsync returns Task<ImmutableArray<FSharpNavigateToSearchResult>>: it has no callback and cannot publish anything before it returns, so the user sees nothing from this project until the whole project is searched, with or without the priority split. Ordering the documents inside it cannot delay that — the same documents are searched on the same throttle, only in a different order — and the measured 1.4 s against 0.8 s is what two batches of one document each cost when the batches are run one after another, which is the shape the repro builds rather than the shape here: priority documents and the rest go through one whenAllThrottled over one sequence, not two awaited halves.

Where publishing early does exist is the loading path, SearchCachedDocumentsAsync, which takes onResultsFound and reports per document as each finishes — and that one already searches priority documents first, for exactly the reason you are pointing at.

If the ordering inside a single throttled pass is measurably worse than none, I would like to see it as a run of the editor tests rather than a synthetic two-document case, since the whole point of the parameter is that Roslyn hands us the documents the user is looking at.

[Copilot]

Copilot review overview

🟡 Changes recommended

Address the parsing-options cache key, clarify the non-streaming behavior, and correct duplicate or overstated release notes.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Include parsing options in the cache key · New
• Remove duplicate release-note entry · New

What changed in this PR

Updates F# Navigate To to prioritize open documents and improve multi-target parsing efficiency.

Changes:

• Adds priority-document batching, throttling, and parse-result caching.
• Adds multi-target tests and Roslyn test helpers.
• Registers tests and updates release notes.

File	Summary
vsintegration/​tests/​FSharp.Editor.Tests/​NavigateToSearchServiceTests.fs	Tests priority document ordering.
vsintegration/​tests/​FSharp.Editor.Tests/​MultiTargetNavigateToSearchTests.fs	Adds multi-target caching coverage.
vsintegration/​tests/​FSharp.Editor.Tests/​Helpers/​RoslynHelpers.fs	Adds multi-target solution helpers.
vsintegration/​tests/​FSharp.Editor.Tests/​FSharp.Editor.Tests.fsproj	Registers the new tests.
vsintegration/​src/​FSharp.Editor/​Navigation/​NavigateToSearchService.fs	Implements priority ordering, throttling, and caching.
docs/​release-notes/​.VisualStudio/​18.vNext.md	Documents the Navigate To changes.

[xperiandri]

You are right, and it was worse than the language version: fixed in 6e78b9dccf.

The key now carries the language version beside the defines, and both are read only where the project has produced its options — TryGetCompilationDefinesAndLangVersionForEditingDocument, which answers nothing instead of the editing defaults. A project whose options have not arrived parses for itself. What made that necessary is that the editing defaults are not a neutral placeholder: they are exactly the define set a project that defines nothing of its own really has, so a document keyed under them read that project's parse — with the branch of a #if that project chose, not its own.

The remaining FSharpParsingOptions cannot differ for one path: ApplyLineDirectives is false for every document the editor parses, and IsInteractive follows from the file's own extension.

New test, a project whose options have not arrived does not answer from another project's parse: two projects hold the same file behind one TextLoader — a linked file open in the editor, which is the only way the two documents report the same text version and one can read the other's entry at all — one of them with options that define nothing, the other with none. Before the fix the second answered from the first's parse; now it asks for the parse it cannot have and is cancelled, as it was before any entry existed. Thirteen *NavigateTo* tests pass.

[xperiandri]

Fixed in 810adcd2e2, the second way you suggest. The entry describing the old approach — the first instance searches every file, the others only what they alone compile — was superseded by the one describing the parse sharing that replaced it, and stayed behind. It is now the note for this change, the priority documents, and the one for #20483 remains.

[github-actions]

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: Compiler-service or IDE design-time behavior changes.

Generated by PR Tooling Safety Check · gpt56 1.7M · ◷