Repository navigation
fix(vscode): resync the document after a context switch - #522
Conversation
The server is pull-based by design: clice/switchContext only re-targets the session, and every language feature refreshes on the client's next request. The extension previously fired a lone documentSymbol request, which refreshed diagnostics but left semantic tokens, links and hints on the old configuration. Re-sync the document instead (a language-id round-trip closes and reopens it on the server): the reopened session resolves under the persisted choice and the editor re-requests every feature. Buffer content and unsaved edits survive the round-trip. The new server-side integration test pins the contract the client relies on — a switched context survives close/reopen and the reopened compile surfaces the new configuration's diagnostics — with zero server changes. The extension e2e asserts the resync round-trip keeps the switched context and restores the language id.
📝 WalkthroughWalkthroughAdds a VS Code document resynchronization helper, invokes it after successful compilation-context switches, and tests diagnostics refresh, language restoration, and context persistence after reopening documents. ChangesDocument context resynchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant VSCode
participant ContextFlow
participant LanguageServer
VSCode->>ContextFlow: switch compilation context
ContextFlow->>VSCode: round-trip document language
VSCode->>LanguageServer: resynchronize document
LanguageServer-->>VSCode: publish diagnostics
VSCode->>ContextFlow: confirm current context
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9da69bd475
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review feedback: the resync's transient plaintext hop must not be mistaken by detectCxxFragment for a fragment awaiting detection (the detector would race the restore and pin a c/cuda-cpp file to cpp), and editors with automatic feature pulls disabled need one explicit pull after the reopen or diagnostics stay cleared.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
editors/vscode/src/feature/context.ts (1)
157-165: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winRace condition corrupts document language.
If
resyncDocumentis invoked concurrently for the same URI (e.g., via rapid context switching in the UI), the second invocation may read"plaintext"as the originallanguageIdwhile the first is in flight. This causes the second invocation to restore the document to"plaintext", permanently breaking language features for that file until manually fixed.Add an early return if a resync is already in progress to prevent this data race.
🔒️ Proposed fix to prevent concurrent resyncs
export async function resyncDocument(uri: string) { + if (resyncing.has(uri)) { + return; + } const doc = vscode.workspace.textDocuments.find( (candidate) => candidate.uri.toString() === uri, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@editors/vscode/src/feature/context.ts` around lines 157 - 165, Add an early return at the start of resyncDocument when the URI is already present in resyncing, before reading the document or languageId; preserve the existing behavior for URIs without an active resync.
🧹 Nitpick comments (1)
editors/vscode/src/feature/context.ts (1)
244-247: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle potential unhandled promise rejections.
If the document was closed mid-resync or if no symbol provider is currently registered,
vscode.executeDocumentSymbolProviderwill reject the returned promise. Thevoidoperator suppresses the return value but does not catch these rejections, which can lead to unhandled promise rejection warnings in the extension host.Consider adding a catch handler to swallow expected rejections cleanly while still allowing the pull to run in the background.
🛠 Proposed fix
- void vscode.commands.executeCommand( + vscode.commands.executeCommand( "vscode.executeDocumentSymbolProvider", vscode.Uri.parse(uri), - ); + ).then(undefined, () => {});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@editors/vscode/src/feature/context.ts` around lines 244 - 247, Update the background vscode.commands.executeCommand call in the document resync flow to attach a rejection handler that swallows expected failures, while preserving fire-and-forget execution and the existing vscode.executeDocumentSymbolProvider invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@editors/vscode/src/feature/context.ts`:
- Around line 157-165: Add an early return at the start of resyncDocument when
the URI is already present in resyncing, before reading the document or
languageId; preserve the existing behavior for URIs without an active resync.
---
Nitpick comments:
In `@editors/vscode/src/feature/context.ts`:
- Around line 244-247: Update the background vscode.commands.executeCommand call
in the document resync flow to attach a rejection handler that swallows expected
failures, while preserving fire-and-forget execution and the existing
vscode.executeDocumentSymbolProvider invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87c29f6c-ad1f-4b46-88cc-90d8a397c796
📒 Files selected for processing (1)
editors/vscode/src/feature/context.ts
1 participant
Background
clice/switchContextis pull-based by design: a successful switch persists the choice and re-targets the session, and every language feature refreshes on the client's next request. The VS Code extension previously fired a single documentSymbol request after switching — that refreshed diagnostics, but semantic tokens, document links and inlay hints kept rendering the old configuration until the user happened to touch the file.Server-side alternatives were considered and deliberately rejected: recompiling in the request handler stalls the response behind a possible PCH rebuild, and any broader push-on-invalidate pipeline recompiles every open tab on unrelated events — too expensive on many-tab workspaces. The refresh is the client's job.
Changes
resyncDocumentperforms a language-id round-trip (to plaintext and back), which closes and reopens the document on the server. The reopened session resolves under the persisted choice, the editor re-requests every feature, and the recompile publishes fresh diagnostics. Buffer content and unsaved edits survive the round-trip; a document closed mid-round-trip is tolerated (the UI refresh still runs).Testing
test_switched_context_survives_reopen): switch to the second CDB entry, close, reopen — the reopened compile surfaces the switched configuration's diagnostics andcurrentContextreports the persisted choice. This pins the server half of the contract the client relies on.resyncDocumentmust produce a fresh diagnostics publish for the file (the direct proof the round-trip reached the server —currentContextalone would pass without any reopen), restore the language id, and keep the switched context. All three e2e scenarios pass locally under WSLg.Summary by CodeRabbit