Port content mapper inspector extension into bundled extension - #64173
Port content mapper inspector extension into bundled extension#64173Andrew Branch (andrewbranch) wants to merge 12 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Offset encoding and mapper-registration invalidation currently break core inspector behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a bundled VS Code inspector for content-mapper virtual files, mappings, and diagnostic directives, addressing #64055.
Changes:
- Adds LSP requests for mapped-file metadata.
- Adds virtual documents, mapping highlights, hovers, and directive views.
- Adds protocol and helper tests.
File summaries
| File | Description |
|---|---|
tsc/internal/lsp/server.go |
Implements inspector requests. |
tsc/internal/lsp/server_contentmapper_test.go |
Tests new requests. |
tsc/internal/lsp/lsproto/lsp_generated.go |
Adds generated protocol types. |
tsc/internal/lsp/lsproto/_generate/generate.mts |
Defines protocol generation inputs. |
packages/vscode-typescript/test/index.test.ts |
Registers the new test. |
packages/vscode-typescript/test/contentMapperVirtualFiles.test.ts |
Tests output identities. |
packages/vscode-typescript/src/session.ts |
Exposes inspector requests. |
packages/vscode-typescript/src/extension.ts |
Registers the inspector provider. |
packages/vscode-typescript/src/diagnosticDirectivesView.ts |
Implements the directives tree. |
packages/vscode-typescript/src/contentMapperVirtualFiles.ts |
Defines client models and identities. |
packages/vscode-typescript/src/contentMapperVirtualDocuments.ts |
Implements virtual documents and mapping UI. |
packages/vscode-typescript/src/client.ts |
Sends inspector LSP requests. |
packages/vscode-typescript/package.nls.json |
Adds contribution labels. |
packages/vscode-typescript/package.json |
Contributes commands, menus, and view. |
packages/vscode-typescript/l10n/bundle.l10n.json |
Adds localized runtime strings. |
Review details
Files not reviewed (1)
- tsc/internal/lsp/lsproto/lsp_generated.go: Generated file
- Files reviewed: 14/15 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Restart synchronization can leave inspector documents stale, and absent directive ranges are reported as real source locations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- tsc/internal/lsp/lsproto/lsp_generated.go: Generated file
Suppressed comments (1)
packages/vscode-typescript/src/contentMapperVirtualDocuments.ts:123
- Refreshing mapped outputs on server initialization races the contribution synchronization that initialization also triggers. For extension-contributed inferred mappers, this request can return no outputs, causing
refreshto delete the tracked entries; the later synchronization event only updates the context, so already-open inspector documents remain deleted or stale after a restart. Defer refreshing tracked sources until contributions have synchronized.
for (const source of this.sourceToVirtualUris.keys()) {
this.refreshSource(vscode.Uri.parse(source));
}
- Files reviewed: 14/15 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The inspector can show stale directives, retain outputs indefinitely, and incorrectly hide UI for deprecated-setting users.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- tsc/internal/lsp/lsproto/lsp_generated.go: Generated file
Suppressed comments (5)
Previously missed (2) — in code that hasn't changed since the last review.
tsc/internal/lsp/lsproto/_generate/generate.mts:451
- The protocol converts these offsets to UTF-16 code-unit offsets, but the public structure only says “offset.” That is ambiguous for clients handling non-BMP text and can make them interpret values as UTF-8 bytes or Unicode code points. Document UTF-16 units here and on the virtual-span start/length fields, then regenerate
lsp_generated.go.
tsc/internal/lsp/server.go:2467 - The new plural-files branch is only exercised with mappers that return one canonical output, so serialization of
SupplementalSourceFiles()(including filenames, script kinds, mappings, and directives) can regress unnoticed. Extend this endpoint test with a mapper that emits at least one supplemental output and assert both returned files.
packages/vscode-typescript/package.json:284
- The command enablement ignores the supported deprecated
typescript.native-preview.showDebugInfosetting, while the provider itself treats that setting as enabled. Include the deprecated configuration key so those users can invoke the inspector.
"enablement": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped",
packages/vscode-typescript/package.json:361
- This menu predicate ignores the supported deprecated
typescript.native-preview.showDebugInfosetting. Include the same fallback used byreadNativePreviewConfig, otherwise the command stays absent from this menu for migrated-extension users.
"when": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped"
packages/vscode-typescript/package.json:382
- This editor-title predicate also ignores the supported deprecated
typescript.native-preview.showDebugInfosetting. Include that key so the inspector UI consistently matches the runtime fallback.
"when": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped",
- Files reviewed: 14/15 changed files
- Comments generated: 4
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Configuration fallback gaps and stale active-editor state can leave the inspector inaccessible.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- tsc/internal/lsp/lsproto/lsp_generated.go: Generated file
Suppressed comments (6)
Previously missed (3) — in code that hasn't changed since the last review.
packages/vscode-typescript/package.json:234
- The view ignores the supported deprecated
typescript.native-preview.showDebugInfosetting.readNativePreviewConfigfalls back to that setting and the provider explicitly listens for it, but with only that setting enabled thiswhenclause remains false, so the inspector view never appears. Include the fallback here as well.
This issue also appears in the following locations of the same file:
- line 284
- line 361
- line 382
tsc/internal/lsp/lsproto/_generate/generate.mts:451
- These ranges are serialized and consumed as UTF-16 code-unit offsets, but the protocol only says “offset.” Since offsets are otherwise encoding-dependent in LSP, clients cannot implement this contract reliably from the schema. Document UTF-16 explicitly and regenerate the Go protocol types.
tsc/internal/lsp/lsproto/_generate/generate.mts:461 - The generated/original starts and lengths are also UTF-16 code-unit counts, as shown by the server’s endpoint conversions, but this wire contract is undocumented. Add that unit to each mapping-field description and regenerate the protocol output.
packages/vscode-typescript/package.json:284
- This enablement clause bypasses the deprecated-setting fallback used by
readNativePreviewConfig. A user who still has onlytypescript.native-preview.showDebugInfoenabled gets a true runtime debug setting but a disabled command. Include both configuration keys.
"enablement": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped",
packages/vscode-typescript/package.json:361
- The command-palette entry is hidden when debug info is enabled through the still-supported deprecated setting, even though the provider handles that setting. Mirror the fallback used by
readNativePreviewConfig.
"when": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped"
packages/vscode-typescript/package.json:382
- The editor-title entry is hidden when only
typescript.native-preview.showDebugInfois enabled, while the runtime treats that deprecated setting as enabled. Include the same fallback configuration key here.
"when": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped",
- Files reviewed: 14/15 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Inspector visibility mishandles the supported legacy debug setting, and directive positions are incorrect for lone-CR files.
Review details
Files not reviewed (1)
- tsc/internal/lsp/lsproto/lsp_generated.go: Generated file
Suppressed comments (5)
Previously missed (2) — in code that hasn't changed since the last review.
packages/vscode-typescript/package.json:234
- This visibility check ignores the supported deprecated
typescript.native-preview.showDebugInfofallback.readNativePreviewConfigstill uses that value when no explicitjs/ts.showDebugInfovalue exists, so the inspector runs in debug mode but this view remains hidden. Drive thewhenclause from a context key computed with the same precedence logic.
This issue also appears in the following locations of the same file:
- line 284
- line 361
- line 382
packages/vscode-typescript/src/diagnosticDirectivesView.ts:256
positionAtonly recognizes LF, so directive labels and tooltips report incorrect line/column positions for files using lone-CR line endings. Treat CR as a line break too, while consuming CRLF as one delimiter.
packages/vscode-typescript/package.json:284
- The runtime sets
activeEditorIsContentMappedto true only when the effective debug setting fromreadNativePreviewConfigis enabled. Rechecking onlyconfig.js/ts.showDebugInfohere disables the command for users relying on the still-supported deprecated setting; use the computed context key as the source of truth.
"enablement": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped",
packages/vscode-typescript/package.json:361
- This menu guard also bypasses the deprecated-setting fallback used by the runtime, so the command is absent from the Command Palette even when
activeEditorIsContentMappedwas computed as true. Rely on that computed context key instead.
"when": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped"
packages/vscode-typescript/package.json:382
- This duplicate config check hides the editor-title action when debug mode is enabled through the supported deprecated setting. The active-editor context already incorporates the effective setting, so use it directly.
"when": "typescript.native-preview.serverRunning && config.js/ts.showDebugInfo && typescript.native-preview.activeEditorIsContentMapped",
- Files reviewed: 14/15 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Cached virtual documents are not refreshed when content-mapper contributions change, and supplemental-output coverage and protocol offset documentation are incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- tsc/internal/lsp/lsproto/lsp_generated.go: Generated file
- Files reviewed: 14/15 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Source rename/delete handling leaves inspector tabs stale, and the enabling setting remains undocumented.
Review details
Files not reviewed (1)
- tsc/internal/lsp/lsproto/lsp_generated.go: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
packages/vscode-typescript/src/contentMapperVirtualDocuments.ts:109
- Renames and deletions of an inspected source are not observed. Because the cache is keyed by the old source URI and this close listener only handles virtual documents, open virtual tabs remain backed by stale entries after their source is renamed or deleted. Handle
onDidRenameFiles/onDidDeleteFilesby purging the affected cached source and emitting deletion events (or explicitly migrate entries on rename).
packages/vscode-typescript/src/contentMapperVirtualDocuments.ts:286 - The inspector is gated by
js/ts.showDebugInfo, but that setting's user-facing description still says it only shows PID and executable-path information (packages/vscode-typescript/package.nls.json:8). As a result, users have no discoverable way to learn why this new view and command remain hidden/disabled. Please update the setting description (including the deprecated alias) to mention the content-mapper inspector, or introduce a dedicated setting.
- Files reviewed: 14/15 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Fixes #64055
This was originally something I completely vibed as a separate extension, and then ported into here after team feedback was positive on bundling it. I have not yet looked closely at the client side implementation.
Screen.Recording.2026-09-04.at.1.41.23.PM.mov