Offer F# declarations to the Copilot chat "#" mention picker - #20409
Offer F# declarations to the Copilot chat "#" mention picker#20409xperiandri wants to merge 5 commits into
Conversation
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
|
github-actions
Bot
added
⚠️ Affects-Build-Infra
|
🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Restore
|
db075ee to
80d555b
Compare
T-Gro
left a comment
T-Gro
left a comment
There was a problem hiding this comment.
Really nice piece of work. The design is clean, the comments explain the why behind every non-obvious choice, and it fills a real gap — F# projects have no Roslyn Compilation, so Copilot's built-in provider never saw F# symbols.
What stood out as excellent
- Separation of concerns.
CopilotSymbolQueryholds all the lookup logic and takes aSolutiondirectly, so it's unit-tested with no VS workspace, whileFSharpCopilotContextProviderstays a thin brokered-service adapter. That split is exactly why the tests read so well. - No project-wide typecheck. Reusing the NavigateTo parse-tree cache (now cleanly extracted as
FSharpNavigableItemsCache, MEF-Sharedso both consumers share one instance) keeps the picker responsive per keystroke. - Throttling via
whenAllThrottled ProcessorCountmirrorsFindReferencesAsync, so a query doesn't launch a parse-per-document storm. - Exhaustive mappings.
symbolContextType/imageIdcover all 11NavigableItemKindcases — no partial-match surprises. - The cache extraction preserves the backtick/operator substring-match fallback verbatim; the
struct-tuple change on that per-keystroke path and thenull→matchconversion are tidy. - Release note added; tests cover search, dedup, snippet extent, doc-comment inclusion, kind mapping, and the snippet-location round-trip.
Suggestions (none blocking)
-
Exception safety at package load (
LanguageService.fs). The registration task handles anullproxy (Copilot absent), but only that. IfGetProxyAsync/RegisterContextProviderAsyncthrows — e.g. a Copilot contract-version mismatch, given you're compile-pinned to18.9.918but bind-redirect to whatever VS ships — the exception escapes theafterPackageLoadedTaskstask. Worth confirmingAddTask(false, …)isolates a faulting task, or wrapping the body intry/withso a Copilot hiccup can't perturb F# package load. -
Batch query is sequential, O(all docs) per query (
QueryMentionBatchAsync).for query in queries do let! … = queryMentions queryruns one full-solution scan per query, serially. Batches are usually tiny so it's fine in practice, but if Copilot ever sends several, they could be de-duplicated or run through the same throttle rather than back-to-back. -
One-line declarations drop their doc comment. In
definitionLines, a construct with no body scope (/// doc+let x = 1) falls through todeclarationLine, item.Range.EndLine, so its doc comment isn't captured — unlike the multi-line path, which deliberately reaches back over the doc comment. Minor; a follow-up could widen the one-line case to include an immediately-preceding///block. -
Nit: the test's hardcoded
"C:\\test.fs"is fine for Windows-only VS tests but couples toRoslynTestHelpers' internal path.
I reviewed statically and confirmed the in-tree helpers (whenAllThrottled, chooseV/tryHeadV/toImmutableArray, ValueOption.ofNullable) and the NavigableItemKind shape; I didn't run a VS-hosted build, so the Microsoft.VisualStudio.Copilot contract surface is taken on faith from the package reference.
Only item 1 feels worth a second look before merge. Thanks for this — it's going to be a delightful quality-of-life win for F# users in the Copilot picker. 🎉
Copilot's built-in symbol provider reads symbols off the Roslyn compilation, which F# projects do not have, so F# declarations never appeared in the picker shown for "#". Proffer a brokered service from FSharp.Editor implementing Copilot's context-provider and mention-queryable contracts. Declarations come from the NavigateTo parse-tree cache, so the picker answers without waiting for a project check; that cache moves into a shared FSharpNavigableItemsCache used by both features. A picked mention resolves by fully qualified name against the current solution, so it survives a file moving, and carries the whole declaration - doc comment included - as its snippet. FSharpPackage now registers the provider moniker with Copilot after package load. The override is no longer DEBUG-only, so it calls its base implementation, which registers the editor factories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e path Sequential per-document scanning made "search" and "declarationsOf" as slow as the slowest single file; run them across documents concurrently instead, throttled the same way FindReferencesAsync throttles its per-document typechecks, so a solution-wide scan does not launch a parse per document all at once. FSharpNavigableItemsCache's version-stamp entries move to struct tuples and its null workspace check to a match, matching this repo's allocation and null-narrowing conventions on a path every keystroke in the mention picker hits. CopilotSymbolMapping collapses its wrapping module into a single qualified top-level module declaration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
80d555b to
28f13e7
Compare
Package load runs its tasks back to back on a single loop, so an exception from the Copilot registration task escaped into F# package load. A Copilot contract version the installed build does not serve would have taken the whole package down; catch and log instead, leaving cancellation alone. A doc comment is only reported as an outlining scope once it spans several lines, so a one-line "///" in front of a declaration was invisible to the scope search and dropped from the snippet. Walk back over the preceding "///" lines directly. Batch mention queries scanned the solution once per query, serially. Distinct search texts now scan concurrently and repeated ones share a single scan. The snippet-location test asserted a hardcoded "C:\test.fs" rather than asking the solution where its document lives. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Resolving a picked mention walks every declaration in every document of the solution, and asked each one for its dotted path as a fresh string purely to compare it. Compare against the container and name in place instead, so the scan allocates nothing per declaration. The doc-comment probe trimmed each candidate line into a new string for the same reason. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 participants
Description
GitHub Copilot Chat's
#mention picker in Visual Studio lets you attach a symbol as context. Copilot's built-in provider reads symbols straight off the RoslynCompilation, which F# projects do not have, so F# types, modules, members and values never showed up there.This adds
FSharpCopilotContextProvider, a brokered service proffered fromFSharp.Editorthat implements Copilot'sICopilotContextProvider/ICopilotMentionQueryable/ICopilotMentionBatchQueryablecontracts directly, backed by the existing NavigateTo parse-tree cache (no project-wide typecheck needed, so the picker answers as fast as you type). A picked mention resolves by fully qualified name against the current solution and attaches the whole declaration — doc comment included — as its snippet.Solution-wide document scanning in
search/declarationsOfruns across documents concurrently, throttled the same wayFindReferencesAsyncthrottles its per-document typechecks, so a query does not launch a parse per document all at once.FSharpNavigableItemsCache's per-document cache entries move to struct tuples on this hot, per-keystroke path, and its null-workspace check moves to amatchper this repo's conventions.Checklist