feat(playground): keep connections across save/reopen + add a Connect-to-Database picker#758
Merged
Merged
Conversation
Correlate the untitled→file save on onDidSaveTextDocument instead of onDidOpenTextDocument. For a brand-new file the file's document model is still empty when it opens, so content correlation failed and the connection was dropped on close. The save event fires after the text is populated and while the untitled is still open, making the re-key reliable. Also gate correlation by document length via offsetAt (no full-text materialization) and skip documents over a size cap.
Broaden the save-time connection correlation to accept any open playground as the source (untitled from a first save, or a file from "Save As…"), excluding the saved file itself. Previously only an untitled source was considered, so re-saving an already-saved playground under a new name dropped its connection when the original file closed. The "exactly one match" guard still prevents ambiguous re-keys, and a regular save of an already-connected playground remains a no-op (its URI is unchanged). Renamed migrateConnectionFromSavedUntitled → migrateConnectionToSavedFile and added file→file Save As + regular-save coverage.
Drop the confusing 'migration' terminology: rename migrateConnectionToSavedFile → transferConnectionToSavedFile and MAX_MIGRATION_TEXT_LENGTH → MAX_TRANSFER_TEXT_LENGTH, reword comments, and change the trace message to 'transferred connection on save'. Updated test labels accordingly.
Add a generic, telemetry-instrumented quick-pick helper that drills through a tree (folders -> clusters -> databases) by repeatedly calling getChildren, presenting one quick pick per level until a leaf matching a given contextValue is selected. Reuses the live Connections branch data provider so folder nesting and connect/auth-on-expand are inherited for free. Includes unit tests.
Add vscode-documentdb.command.playground.connect which prompts the user (via the reusable pickTreeNode picker, stopping at the database level) and binds the active playground to the selected cluster/database. Uses the stable clusterId for the connection per the dual-ID rule. Registered and contributed to the command palette when a playground editor is active.
The 'Not connected' CodeLens and StatusBar affordances now invoke the new connect command instead of the dead-end info popup, turning the always-visible disconnected indicator into a one-click connect action.
Running an unconnected playground (e.g. a saved file reopened in a fresh session) now launches the connection picker instead of showing a dead-end message; if the user picks a database the run continues, otherwise it bails quietly. Instrumented via a dedicated playground.connectOnRun telemetry event.
Keep a non-persistent, in-memory last-binding-per-URI map that survives document close. When a playground is reopened within the same session, restore its binding silently if the target cluster still has live credentials; otherwise leave it disconnected for the Connect affordance. Never reads the document body, so it is safe against the empty-model open race. Cleared on explicit removeConnection and on dispose.
Regenerate the localization bundle for the new connect/reconnect strings and simplify the pickTreeNode test l10n mock to satisfy no-base-to-string lint.
- Show a busy/loading indicator while a level loads by handing showQuickPick a promise of picks with a loadingPlaceHolder, so the quick pick stays visible while a cluster connection is being established instead of vanishing. - Carry each tree item's own iconPath onto the quick pick so clusters, folders and databases use the same icons as the tree. - Replace the warning toast for an empty level with an in-list 'Empty' entry alongside Back, and a 'No connections found' entry at an empty root.
- Connected connection lens now opens a modal offering to switch the playground to a different database; the switch is deferred so cancelling the picker keeps the existing connection (setConnection only runs once a new db is selected). - Empty-level entry drops its description (label only). - No-connections entry uses detail (second line) instead of description.
Add an optional getDetail callback to pickTreeNode (with matchOnDetail so it's searchable) and supply it from the playground connect flow: cluster picks show their host(s), database picks show 'cluster · host'. This disambiguates items that share a display name. Host comes from the non-secret connection string on the cluster model and is omitted gracefully when unavailable.
Mirror the Azure credentials-management browser: list real items first, then a separator, then the navigation actions (Back when nested, Exit always) pinned to the bottom of the list. Selecting Exit cancels the picker.
Give every selectable pick a detail line so rows have a consistent two-line height: folders show a static 'Folder' tag, local emulators 'Local emulators', clusters their host (fallback 'Cluster'), and databases 'cluster · host' (fallback cluster name). Fixes the ragged mix of one- and two-line rows.
- Add an optional getGroup callback to pickTreeNode; when a level has 2+ groups, items are organized under separator headers (Folders, Connections) in first-seen order; single-group levels stay flat. - Database picks now show a static 'Database' detail instead of cluster·host. - The Empty entry gains a 'No entries' description.
- Remove the Exit entry (Esc still cancels). - Move Back to the top of each nested level, followed by a separator. - Drop the Folders/Connections group headers; rely on the tree's natural folders-before-connections order. - Empty entry now shows 'No entries' as a detail line.
tnaum-ms
changed the title
- Add docs/ai-and-plans/PRs/758-query-playground-connections/summary.md capturing what was built and, crucially, why (the two-sub-problem split, why cross-session needs an actionable picker, dual-ID, UX rationale). - Add a module-level overview to pickTreeNode.ts explaining its purpose, design (live-tree walk), termination/safety invariants, telemetry, and reuse; enrich the function doc.
runAll and runSelected had their own early isConnected guard that showed the dead-end 'no connection' warning and returned before executePlaygroundCode ran, so the connect-on-run picker never triggered. Remove those guards (and the now unused PlaygroundService import); executePlaygroundCode launches the connection picker for unconnected playgrounds and continues the run.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the Query Playground UX by (1) preserving the in-memory cluster/database binding across URI-changing saves (untitled → file, and Save As) and (2) adding a first-class “Connect to Database” flow (CodeLens, StatusBar, and connect-on-run) backed by a reusable Connections-tree-driven picker.
Changes:
- Added a generic
pickTreeNodeutility that browses the live Connections tree viagetChildrenand returns a leaf node bycontextValue, with dedicated telemetry and “Back/Empty/No connections” UX. - Updated playground connection lifecycle to transfer bindings on save (content correlation) and to optionally restore a remembered binding on reopen within the same session (credential-gated).
- Added a new “Query Playground: Connect to Database” command and rewired disconnected affordances + connect-on-run to use the picker.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/pickItem/pickTreeNode.ts | New reusable quick-pick tree browser with telemetry and navigation UX. |
| src/utils/pickItem/pickTreeNode.test.ts | Unit tests for tree browsing, filtering, Back/Empty/No connections, icon/detail passthrough. |
| src/documentdb/playground/PlaygroundService.ts | Save-time connection transfer + in-session reopen restore + StatusBar connect affordance updates. |
| src/documentdb/playground/PlaygroundService.test.ts | Tests for save-transfer and in-session restore behavior. |
| src/documentdb/playground/PlaygroundCodeLensProvider.ts | Disconnected CodeLens now routes to Connect instead of info. |
| src/documentdb/playground/PlaygroundCodeLensProvider.test.ts | Updated expectations for the new Connect CodeLens behavior. |
| src/documentdb/playground/constants.ts | Added new playground command id for connect/reconnect. |
| src/documentdb/ClustersExtension.ts | Registered the new connect command. |
| src/commands/playground/runSelected.ts | Removed dead-end “not connected” warning (execution path now handles connect-on-run). |
| src/commands/playground/runAll.ts | Removed dead-end “not connected” warning (execution path now handles connect-on-run). |
| src/commands/playground/executePlaygroundCode.ts | Connect-on-run: prompt to connect when unbound, then continue execution. |
| src/commands/playground/connectDatabase.ts | New connect command + “switch database” modal + conversion from picked DatabaseItem → PlaygroundConnection. |
| package.json | Contributed the new command and enabled it for documentdb-playground editors. |
| l10n/bundle.l10n.json | Added/updated localized strings for picker UX and connect affordances. |
| docs/ai-and-plans/PRs/758-query-playground-connections/summary.md | Design/decision log for the feature set and rationale. |
Thread a telemetrySource parameter through promptAndConnectPlayground so the documentdb.pickTreeNode event distinguishes the explicit connect affordance (playground.connect) from the connect-on-run path (playground.connectOnRun). Addresses Copilot review feedback on PR #758.
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.
Summary
Query Playgrounds bind each
.documentdb.jsdocument to a cluster/database connection, but that binding previously lived only in an in-memory map keyed by the document URI. It was therefore lost whenever the URI changed (saving an untitled scratch file to disk, or Save As) or when a saved playground was reopened in a fresh VS Code session — leaving the file silently disconnected.This PR makes playground connections survive those transitions and adds a first-class way to (re)connect a playground.
What's included
Keep the connection through saves
Connect / reconnect on demand
Query Playground: Connect to Databasecommand plus always-visible affordances: the "Not connected" CodeLens and StatusBar item now open a connection picker instead of a dead-end info popup.Silent in-session restore
Reusable tree picker (
pickTreeNode)getChildren, so folder nesting and connect/auth-on-expand are inherited for free.Design notes
clusterId(nevertreeId) for cache/credential lookups, per the dual-ID rule.documentdb.pickTreeNode(source / outcome / steps),playground.connect,playground.connectOnRun.Testing
pickTreeNodeandPlaygroundService(save-time transfer, in-session restore, lifecycle, connection management).