Fix debounce cancellation gap and two redundant reparse paths (issue #471) - #478
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implement GetSemanticTokensForRangeAsync() with range-based filtering to reduce encoding cost for textDocument/semanticTokens/range requests. The method filters tags to the requested line range BEFORE encoding, avoiding the previous no-op optimization that computed the entire document. - Add ISemanticTokenService.GetSemanticTokensForRangeAsync interface method - Implement FilterToLineRange() helper to exclude out-of-range tags early - Modify Encode() to accept optional line-range parameters - Handle LSP range semantics (end position at column 0 is exclusive) Test: GetSemanticTokensForRangeAsync_excludes_tags_outside_the_requested_line_range Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ng (issue #471) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#471) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pass the requested range's line bounds to IGherkinInlayHintService.Build instead of building hints for the whole document and filtering afterward. This reduces the candidate set processed by the handler, improving performance on large solutions. The existing output filter remains in place for correctness at the range's edges, since Build's line-range check is coarser than the position check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add CodeLensResolveHandler, routing a resolving lens to whichever CodeLens handler produced it based on the "kind" discriminator in CodeLens.Data (stepUsage -> StepCodeLensHandler, hookMatchCount -> HookMatchCountCodeLensHandler, anything else returned unchanged). Register it for DI and wire the codeLens/resolve route right after the existing textDocument/codeLens registration. Also teach the LSP.Server.Specs harness to follow up with codeLens/resolve when a lens comes back with Command unset (the deferred-resolve path landed in the two prior tasks, but no spec exercised it end-to-end until this dispatcher existed) -- without this, the default (non-VS) harness profile now used by StepCodeLens.feature's Background left three scenarios asserting directly on the placeholder lens's null Command. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Final-review fix wave for the issue #471 range/resolve branch. The deferred codeLens path shipped gated on `!IsVisualStudio`, i.e. active for exactly the two clients that cannot handle it. VS Code's stepCodeLens.ts has no resolveCodeLens and discards lens.data; Rider's StepUsagesCodeVisionProvider.kt filters out command == null lenses. Confirmed live in VS Code during Task 9: step-usage lenses stopped rendering and hook-match lenses degraded. Replace it with ClientIdeContext.SupportsCodeLensResolve, backed by a deliberately empty opt-in allowlist, so every shipped client gets today's eager behaviour. ResolveAsync, CodeLensResolveHandler, the codeLens/resolve route and resolveProvider = true are all kept intact and tested, ready to switch on. Also in this wave: - SemanticTokenService: FilterToLineRange now resolves each tag position once and hands it to CollectLeafTokens, instead of both resolving independently (the filter had been a net add of ResolvePosition calls). Token mapping moved ahead of resolution and the end position short-circuits, so the range path is now strictly cheaper than before the filter existed. - HookMatchCountCodeLensHandler: malformed-Data/hook-not-found resolve fallback no longer emits a clickable goToMatchingScenarios command built from a fabricated file:///unknown URI. - StepCodeLensHandler.WithZeroUsages: drop three unused parameters. - Range semanticTokens responses get a ResultId distinguishable from the full-document one. - InlayHintHandler applies the same exclusive-end adjustment as SemanticTokenService before scoping Build. - Comment/doc corrections in Program.cs, ISemanticTokenService and the Specs resolve helper; field alignment and hoisted-gate style consistency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Declaring codeLensProvider statically (90a9701) woke up vscode-languageclient's default CodeLensFeature alongside the extension's own hand-rolled providers (registerStepCodeLens, registerHookCodeLens), which register against the same document selectors. Both then independently request and render lenses, doubling every hook-match and step-usage lens in the editor. Add middleware that swallows provideCodeLenses/resolveCodeLens before they reach the built-in feature, restoring the hand-rolled providers as the sole source of lenses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Visual Studio's built-in LSP client can't map Reqnroll's custom token types to a classifier of its own -- tokens for it flow entirely through the separate reqnroll/semanticTokens push mechanism (SemanticTokensPushHandler) plus the VS extension's SemanticTokensClassificationInterceptor. Advertising full/range pull support to VS anyway doesn't help it and isn't free: its built-in client has been observed issuing its own pull requests, duplicating the expensive full-document encode the push path already paid for, for a response VS then discards. Withhold Full/Range for VS while still advertising the Legend, since SemanticTokensClassificationInterceptor reads it from the same initialize response to decode pushed tokens -- the push notification itself carries no legend of its own. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI's eslint flagged the `as never` casts on the `next` callback in both tests -- TypeScript already accepts a zero-arg function where a multi-arg one is expected, so the casts were redundant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ex (issue #471) FindUsages was an unindexed full scan over every cached feature document's every step, called once per binding by StepCodeLensHandler -- the dominant cost behind the CodeLens/refresh stalls reported in #471 on large solutions. Adds BindingId, a stable identity hashed from a binding's declaring type, method signature, step block and expression (SHA-256, truncated to 64 bits) rather than its source location -- borrowing clangd's SymbolID shape per the design comment on #471. BindingMatchService now maintains a BindingId -> steps reverse index (O(1) lookup) plus a per-file, binary-searched location index used only to translate a raw SourceLocation into a BindingId for callers that don't already hold a binding object. Shard-precise eviction on invalidate falls out of FeatureBindingMatchSet.Steps already being the shard. At the real repro's scale (~1,300 bindings/file, Reqnroll.VeryLargeFeature) binary search measures ~40x faster than a linear per-file scan, so the location index's write path batches per-file resorts once per Store() call rather than once per step to avoid reintroducing quadratic write cost on a large feature file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sue #471) FindUnusedStepDefinitionsService, CompletionHandler and StepCodeLensHandler's eager textDocument/codeLens path all already had a ProjectStepDefinitionBinding in hand and only extracted its SourceLocation to call FindUsages -- switch them to the new BindingId overload to skip location math and the per-location cache/ post-hoc expression filter FindUnusedStepDefinitionsService needed under the old location-keyed lookup (BindingId is already per-expression-specific). StepCodeLensHandler.ResolveAsync now prefers a BindingId stashed in the lens's Data payload at creation time, falling back to the SourceLocation-based path only for a payload that predates this field. StepRenameHandler, StepReferencesHandler and FindStepUsagesHandler are unchanged -- they only have a raw cursor position, not a binding object, and get the index's speedup transparently through the unchanged SourceLocation overload. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… (issue #471) RefreshDebouncer.Schedule cancels a pending, not-yet-started run when a newer MatchCacheChangedNotification supersedes it, but SemanticTokensRefreshHandler, InlayHintRefreshHandler, and CodeLensRefreshHandler (via CodeLensRefreshRequester) all sent their actual SendRequest with CancellationToken.None instead of the token they were given -- so once a refresh request was dispatched, no later notification could cancel or collapse it. Confirmed live: three workspace/semanticTokens/refresh requests dispatched 5-10s apart, all still pending simultaneously, resolving together in the same ~2.5s window at the end. Pass the debouncer-supplied token through in all three handlers so a superseding trigger can actually abort a refresh already in flight, not just skip one that hadn't started yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…iliation (issue #471) BindingRegistryChangedHandler.Handle already sequences a full replacement correctly in one method -- RediscoverCsFilesAsync (Roslyn overlay) runs before ReparseOpenFilesAsync (feature-file reparse + MatchCacheChanged publish), so the feature-file reparse already waits for the C# overlay. But RediscoverCsFilesAsync reconciles each .cs file via the same UpdateFromSourceForProjectAsync path used for a live single-file edit, whose ApplyRoslynFileUpdateAsync unconditionally raised its own independent BindingRegistryChanged(false) whenever the Roslyn-parsed result differed from the connector's compiled-DLL result -- which it essentially always does, since reflection-based and Roslyn-source-based extraction of the same file aren't guaranteed byte-identical even with zero real edits. That fired a second, fully independent Handle() invocation that reparsed the same feature files again, redundant with the reparse the original invocation was already about to do moments later. Add a notify parameter (default true, preserving existing behaviour for live edits and file deletions) threaded from ApplyRoslynFileUpdateAsync through UpdateFromSourceForProjectAsync, and pass notify: false from RediscoverCsFilesAsync specifically -- its caller already reparses and notifies unconditionally right after it returns, so a second independent notification only duplicates that work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ed (issue #471) textDocument/didOpen on a .cs file unconditionally reparsed and patched the registry from source, with no check for whether the project's connector discovery had already run. Confirmed live: a step-definitions file opened at startup got a full Roslyn parse, then the exact same unedited file was reconciled again by RediscoverCsFilesAsync 7 seconds later -- parsed twice for zero new information. This can't be removed outright: RunDiscoveryAsync skips firing BindingRegistryChanged when the connector's assembly hash doesn't change (e.g. no compiled DLL exists yet -- an unbuilt project), so RediscoverCsFilesAsync never runs for such a project either, leaving didOpen's parse as the only mechanism that ever populates bindings from source. VS Code hits this state routinely by design -- its extension only ever runs a design-time MSBuild evaluation, never an actual build. Add ConnectorBindingRegistryProvider.HasSuccessfulConnectorRun, tracking specifically whether the out-of-process connector has ever loaded real bindings from a compiled DLL (deliberately narrower than "Current is populated", since a Roslyn per-file patch can populate Current too). Gate the didOpen registry-update in CSharpBindingDiscoveryService.UpdateFromSourceAsync on this flag when isOpen is true; didChange (an edit) always still applies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 25, 2026
…registry-fixes # Conflicts: # tests/LSP/Reqnroll.IdeSupport.LSP.Server.Tests/Features/CodeLens/StepCodeLensHandlerTests.cs
6 tasks
clrudolphi
added a commit
that referenced
this pull request
Aug 25, 2026
…he attribute (#484) * Fix step-usage CodeLens rendering below the method instead of above the attribute StepCodeLensHandler positioned its lens using SourceLocation.SourceFileLine, which is the method identifier's line for Roslyn-discovered bindings, or a PDB sequence-point line (often a line or more into the method body) for connector-discovered ones -- never the attribute's own line. A separate, AST-backfilled AttributeSourceLine already exists and is used by BindingLocationMatcher/RenameBindingResolver/CSharpAttributeLiteralResolver, but this handler never adopted it. This was masked as long as every .cs didOpen always re-ran the Roslyn parser (whose method-identifier line reads close enough to "right"). Once item 7 (#478) started skipping that redundant reparse when the connector had already succeeded, the registry kept the connector's less precise PDB-derived location instead, and the lens visibly moved to a line at or after the method declaration. Fix: prefer binding.AttributeSourceLine when known, falling back to the existing SourceFileLine/Column for bindings where the backfill itself failed. Also corrects a stale doc comment claiming AttributeSourceLine is always null for connector-discovered bindings -- it's been backfilled via a Roslyn re-parse in ConnectorDiscoveryService for a while. * Anchor step-usage CodeLens on the method-identifier line, not the attribute The previous commit anchored the lens on binding.AttributeSourceLine, which rendered correctly in Visual Studio but one line too high in VS Code: VS Code's CodeLens always renders as a floating row above its anchor line rather than overlaid on it, so anchoring on the attribute's own line pushed the visual position a line further up than intended. The conventional CodeLens anchor for a "N references"-style lens -- the one every client (VS Code, VS, Rider) already uses for the built-in C# references lens -- is the method declaration's own line, not the attribute's. That's exactly what SourceLocation.SourceFileLine already means for Roslyn-discovered bindings; it was only ever imprecise for connector-discovered ones, whose wire-format location is a raw PDB sequence point that can land a line or more into the method body. Fix the actual imprecision instead of routing around it: ConnectorDiscoveryService now backfills the exact AST-based method-identifier location for connector-discovered bindings too (BindingImporter.TryGetMethodIdentifierLocation), mirroring what StepDefinitionFileParser already does for Roslyn discovery. StepCodeLensHandler goes back to anchoring on SourceLocation.SourceFileLine directly, which is now precise for both discovery paths. * Add regression coverage for the method-identifier location backfill Unit-level (BindingImporter.ImportStepDefinition applies the override) and integration-level (ConnectorDiscoveryService.RunDiscovery replaces a deliberately-wrong PDB line with the AST-derived method-identifier line) tests confirming the fix from the previous commit actually takes effect end-to-end, not just that the AST lookup itself returns the right answer in isolation.
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
1 participant
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
Stacked on #475. Implements items 5, 6, and 7 from the tracking comment on #471 — three fixes found while root-causing the VS semantic-tokens/inlay-hint "push loop" (which turned out not to be a repeating trigger at all, just MediatR's default sequential dispatch compounded by these two redundant reparse paths).
Item 5 —
RefreshDebouncercancellation gap:SemanticTokensRefreshHandler,InlayHintRefreshHandler, andCodeLensRefreshHandler(viaCodeLensRefreshRequester) all sent their actual refresh request withCancellationToken.Noneinstead of the token the debouncer gave them, so once a refresh was dispatched, no later trigger could cancel or collapse it — only one still waiting out the debounce delay. Confirmed live: threeworkspace/semanticTokens/refreshrequests dispatched 5-10s apart, all still pending simultaneously, resolving together in the same ~2.5s window at the end. Now threads the debouncer's token through in all three.Item 6 — redundant
BindingRegistryChanged(false)from startup reconciliation:BindingRegistryChangedHandler.Handle's full-replacement flow already sequences correctly (Roslyn overlay → feature-file reparse → notify, all in one method), but the overlay step (RediscoverCsFilesAsync) reuses the same notifying path as a live single-file edit — and Roslyn-parsed source vs. the connector's compiled-DLL extraction of the same unedited file essentially always differ enough to trip that notification, firing a second, fully independent reparse of the same feature files moments later. Added anotifyparameter (defaulttrue, unchanged for live edits/deletions) so the startup reconciliation can patch the registry quietly, since its caller already reparses and notifies unconditionally right after it returns.Item 7 — redundant
.csreparse ondidOpen: everytextDocument/didOpenon a.csfile unconditionally reparsed and patched the registry from source, regardless of whether the project's connector discovery had already run. Confirmed live: the same unedited file parsed twice within 7 seconds. Can't be removed outright — an unbuilt project (no compiled DLL) never gets a connector-driven reconciliation at all, sodidOpen's parse is the only mechanism populating bindings from source in that state, and VS Code hits it routinely (its extension only ever runs a design-time MSBuild evaluation, never an actual build). AddedConnectorBindingRegistryProvider.HasSuccessfulConnectorRun(narrower than "registry is populated" — a Roslyn per-file patch can populate it too) and gated thedidOpenregistry-update on it;didChangealways still applies.Test plan
dotnet test tests/LSP/Reqnroll.IdeSupport.LSP.Server.Tests— 822 passing (new: token-passthrough regression tests for all three refresh handlers +CodeLensRefreshRequester;notify: falsecoverage forApplyRoslynFileUpdateAsync/UpdateFromSourceForProjectAsync;HasSuccessfulConnectorRungating coverage forUpdateFromSourceAsync, including the VS-Code-unbuilt-project case staying un-gated)dotnet test tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs— 147 passing, 18 skipped (pre-existing)🤖 Generated with Claude Code