Merge main into feature/atlas-discovery; confirm webview-ext-webview migration already satisfied#782
Merged
tnaum-ms merged 71 commits intoJul 6, 2026
Conversation
…Sink, BaseRouterContext)
…ain openAppWebview
…red, testing, and webview directories
…tions in README and migration manual
…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
…ry-webview-migration
Copilot
AI
changed the title
Merge main into feature/atlas-discovery (webview migration verified)
Merge main into feature/atlas-discovery; confirm webview-ext-webview migration already satisfied
Jul 6, 2026
Copilot created this pull request from a session on behalf of
tnaum-ms
July 6, 2026 11:33
View session
tnaum-ms
approved these changes
Jul 6, 2026
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. |
languy
approved these changes
Jul 6, 2026
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.
feature/atlas-discoverywas 70 commits behindmainand needed to pick up the@microsoft/vscode-ext-react-webview→@microsoft/vscode-ext-webviewmigration (PR #766) that landed onmainin the meantime.Merge
mainintofeature/atlas-discovery— resolved with no conflicts.Webview migration
feature/atlas-discovery's two commits (Atlas discovery provider, tree items, auth flows, API client) and confirmed they contain no webview UI code — noattachTrpc,openWebview,WebviewController,useTrpcClient, oruseRpcEventsusage.mainbrings in the already-completed@microsoft/vscode-ext-webviewpackage wholesale, so no additional migration edits were required on this branch.@microsoft/vscode-ext-react-webviewremain (source,package.json, lockfile).