Merge main into dev/feature/local-quickstart and migrate to @microsoft/vscode-ext-webview#783
Merged
tnaum-ms merged 72 commits intoJul 6, 2026
Conversation
…Sink, BaseRouterContext)
…ain openAppWebview
…red, testing, and webview directories
…tions in README and migration manual
All four Copilot findings are implemented; add the Iteration 4 change-protocol table (C01/C02 -> 553bf4e, C03 -> 3301a70, C04 -> 19b99cb) and flip the chapter intro from "nothing implemented yet" to implemented. Records the green validation run (l10n no drift, prettier, eslint, jest 2662/159, tsc build).
The Iteration 4 chapter ended on the "Suggested batching" recommendations. Add a closing "Iteration 4 — done" section that lists the steps performed (triage, Batch 1 C01+C02 553bf4e, Batch 2 C03 3301a70, Batch 3 C04 19b99cb, doc finalize cb9cd3b, green validation) and a clear completion message, and reword the batching close to past tense.
…mics and onboarding
…ot extension mode The redesign chose the bundled-vs-dev source layout from extensionMode === Production, but the dev server (webpack serve) emits the bundled asset name (views.js). A webpack-bundled extension running in Development (the normal F5 dev flow, IS_BUNDLE=true, DEVSERVER=true) therefore requested the tsc dev file (index.js) from the dev server and got a 404, leaving the webview blank. Restore the pre-redesign contract: a required isBundled option drives the layout; extensionMode stays CSP-only. The DocumentDB consumer passes isBundled: git commit -q -F /tmp/done-msg.txt && git push -q origin HEAD && echo pushed && git --no-pager log -1 --format='%h %s'ext.isBundle. Adds regression tests.
…ubfolder Move rpcConcurrencyLogger (host) and reportObserverError (webview), plus their tests, into src/webviews/_integration/observability/ so the reference integration folder separates the router/transport wiring from the cross-cutting observability adapters. Update imports, the two external importers, the folder README, and add observability/README.md. No runtime behavior change.
…tion 7 (_integration observability regrouping)
…5 part 1) The host-side dispatch logger (rpcConcurrencyLogger) emitted a console.log on every completed RPC via the framework's consoleProcedureLogger. That line is useful when debugging the extension but is dead weight (string formatting + host-console I/O) on a shipped, installed build where no one watches that console. Gate it behind extensionMode !== Production so dev/F5 and Test builds still log while packaged builds never execute it. The concurrency telemetry gauge stays unconditional and records in every mode, production included. The mode is read per-call: ext.context is always populated by the time an RPC fires (the webview is open, so the extension has activated) and the comparison is O(1), so there is no module-load ordering risk and no measurable added cost. The framework's consoleProcedureLogger stays the neutral, opt-in sink; the gating decision lives in the DocumentDB adapter, preserving the package boundary. Tests stub ext.context.extensionMode per case and add Test-mode (still logs) and Production (no console line, gauge still recorded) coverage. Documented as Iteration 9 in the PR-766 review; Iteration 10 (accumulating-telemetry accumulate/flush redesign, part 2) staged as not-started.
… flush (R766-P05 part 2) callWithAccumulatingTelemetry batched the telemetry emit but still ran the full callWithTelemetryAndErrorHandling wrapper (IActionContext allocation, error- handling wiring, performance.now, Object.entries copy loops, an await) on EVERY call. On hot paths (per webview RPC, per keystroke) that per-call machinery is the cost that adds up, and it runs in production where the gauge is meant to. Redesign (Option 1 from the PR-766 review): the per-call path now runs the populator synchronously against a plain TelemetrySample bag and folds the values into in-memory batch totals - no IActionContext, no await, no callWithTelemetryAndErrorHandling. The Azure pipeline is entered exactly once, on flush, with the rolled-up event. - Callback contract: (sample: TelemetrySample) => void (was (ctx: IActionContext) => T | PromiseLike<T> returning Promise<T | undefined>). Retires the TelemetryWithDistributions cast; distributions is now a first-class bag field. Every call site already void-ed the result and used a synchronous callback. - Errors never accumulate: a populator throw discards the sample and is reported once through callWithTelemetryAndErrorHandling under the same callbackId (the only remaining heavy path, and rare). - Added a finite guard in accumulate so a stray NaN/Infinity cannot poison a sum or min/max reduction. - Unchanged: auto_duration_ms distribution, dist_* rollup keys, batch/interval throttle, flushAccumulatingTelemetry, flushed event name == callbackId. Migrated all direct callers (ClustersExtension, DocumentDBShellPty x6, collectionViewRouter, rpcConcurrencyLogger) and the meterSilentCatch shorthand; meterSilentCatch callers needed no change. Tests rewritten for the sync sample-bag API, adding finite-guard and 'per-call path never enters the telemetry pipeline, only flush does' coverage. Documented as Iteration 10 in the PR-766 review. Validation: jest 2668/2668, eslint (0 errors), tsc --noEmit clean, npm run build, prettier. No user-facing strings.
…teTelemetry (R766-P06) After the Iteration 10 accumulate/flush split, the callWith... name was misleading: the callWith... convention in @microsoft/vscode-azext-utils promises 'run my work inside a managed scope that hands me a full IActionContext (telemetry + errorHandling + ui + valuesToMask) and auto-records duration/result/ errors for it.' The redesigned helper does none of that on the per-call path - it fills a plain sample bag synchronously and returns void. Rename so the name stops promising a scoped call. Behavior unchanged; rename + file move only. - callWithAccumulatingTelemetry -> accumulateTelemetry (verb 'accumulate' signals 'record a data point into a batch', not 'run my work in a scope'). - flushAccumulatingTelemetry -> flushAccumulatedTelemetry. - AccumulatingTelemetryOptions -> AccumulateTelemetryOptions. - Unchanged: TelemetrySample, AUTO_DURATION_DISTRIBUTION_KEY, meterSilentCatch. - File: src/utils/callWithAccumulatingTelemetry.ts -> accumulatingTelemetry.ts (+ .test.ts) via git mv; all importers updated, including meterSilentCatch-only consumers and the two jest.mock paths. The verb now encodes whether your callback runs: accumulateTelemetry does NOT run your work; the deferred runWithAccumulatingTelemetry (issue #777) would, so a runWith... name is honest there. Reviewed the telemetry-instrumentation skill: it does not mention the accumulating helper by any name, so no skill edit was needed. Deliverable 2 (runWithAccumulatingTelemetry, wrap-an-action variant) filed as enhancement issue #777 for future pickup; not built here. Validation: jest 2668/2668, eslint 0 errors, tsc --noEmit clean, npm run build, prettier. No user-facing strings. Documented as Iteration 11 in the PR-766 review.
…s and new options bag requirements
…s and new options bag requirements
…rosoft/vscode-documentdb into dev/tnaum/webview-migrations # Conflicts: # docs/ai-and-plans/PRs/766-webview-ext-package-redesign/webview-ext-migration-manual.md
Copilot created this pull request from a session on behalf of
tnaum-ms
July 6, 2026 11:34
View session
tnaum-ms
approved these changes
Jul 6, 2026
Contributor
✅ Code Quality Checks
This comment is updated automatically on each push. |
Contributor
📦 Build Size Report
Download artifact · updated automatically on each push. |
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
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.
dev/feature/local-quickstartwas 39 commits ahead / 70 commits behindmain, and still used the now-removed@microsoft/vscode-ext-react-webviewpackage. This PR brings the branch up to date and migrates its webview code to the new@microsoft/vscode-ext-webviewpackage, followingdocs/ai-and-plans/PRs/766-webview-ext-package-redesign/webview-ext-migration-manual.md.Merge conflicts
package.json: reconciled dependency list — kept local-quickstart's additions (@microsoft/vscode-container-client,@microsoft/vscode-processutils, etc.) alongside main's@documentdb-js/*packages and@microsoft/vscode-ext-webview; dropped the obsolete@microsoft/vscode-ext-react-webviewentry.package-lock.json: regenerated vianpm install.src/webviews/_integration/*infra, which local-quickstart hadn't modified.Webview migration
Local Quick Start was the only module still on the old package (everything else came in pre-migrated via the merge). Replaced the
WebviewControllerBase-derived class with theopenAppWebviewfactory pattern:src/commands/localQuickStart/openLocalQuickStart.ts: callsopenLocalQuickStartWebview(...)instead ofnew LocalQuickStartController(...).src/webviews/documentdb/localQuickStart/LocalQuickStart.tsx: updateduseTrpcClient()call site — the hook now returns the client directly instead of{ trpcClient }.