feat(python-setup): once-per-session env adoption telemetry - #2122
Open
rugpanov wants to merge 6 commits into
Open
feat(python-setup): once-per-session env adoption telemetry#2122rugpanov wants to merge 6 commits into
rugpanov wants to merge 6 commits into
Conversation
*Why* The extension persists a Python-setup state per project but has no measure of whether the managed environment actually sticks. Compute-drift detection (python_env.drift) compares the selected compute's env key against the recorded one, but it never checks that the .venv still exists — a user who deletes the environment while compute is unchanged is not "drifted", yet has plainly stopped using it. Drift is also event-driven, so it has no per-session denominator to turn into an adoption rate. *What* Add python_env.adoption: a once-per-session, categorical gauge emitted only for a project that has a uv-native setup on record (databricks.pythonSetup.setupState present). Its mere presence is the adoption-rate denominator; it carries venvPresent (does the managed .venv interpreter still exist on disk) and currentTargetType (cluster | serverless | none, read straight from the connection — no env key derived, so no second source of truth vs the CLI). - constants.ts: new Events.PYTHON_ENV_ADOPTION + EventType with per-field comments. - pythonSetupExtensions.ts: recordPythonSetupAdoption, enumerating both fields explicitly (allowlist discipline) so the schema stays compiler-enforced. - PythonSetupAdoptionReporter: thin, dependency-injected controller that dedupes once per project root per session, gates on VPEX-active, and is best-effort (a throwing seam never propagates into the observed flow, and does not latch the dedup so a transient failure retries). - extension.ts: wire real seams and fire on the first CONNECTED transition (so the compute kind is known); dedup makes repeats safe. - telemetry/README.md: rationale for the event (VPEX gate, distinction from drift, why it derives no env key). Additive and measurement-only: no persisted-state change, no migration, and no behavior change to the flow it observes. *Verification* - npx tsc --noEmit -p tsconfig.json — clean. - yarn test:unit — 831 passing, 10 pending, 0 failing (incl. 8 new controller tests and the new recordPythonSetupAdoption emitter tests). - yarn fix && yarn test:lint — eslint + prettier clean. Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 14, 2026 14:06 — with 
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
*Why* Multi-source review of the adoption gauge surfaced three items. *What* - Rename the controller PythonSetupAdoptionReporter -> PythonSetupAdoptionManager: "Reporter" is not a documented class-role suffix (CODE_CONVENTIONS §2); the class reacts to triggers and owns the per-session dedup, which is the XManager role, and it now matches its sibling PythonSetupDriftManager. - Also fire the gauge on setup completion, not only on CONNECTED. A first setup runs while already connected, so a project that becomes VPEX-active mid-session emitted nothing until the next reload; mirroring the drift manager's setupCompleted trigger closes that gap. The manager dedupes per root, so the added trigger never double-emits. - Document the multi-root shared-baseline limitation in telemetry/README.md: with a single workspace-scoped setupState key, a never-set-up sibling root can emit a spurious venvPresent=false. Same limitation the drift detector carries; the fix is the deferred per-project storage schema. *Verification* - npx tsc --noEmit -p tsconfig.json — clean. - yarn test:unit — 840 passing, 10 pending, 0 failing. - yarn fix && eslint/prettier on changed files — clean. Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests triggered for |
rugpanov
requested a deployment
to
test-trigger-is
August 14, 2026 14:15 — with
GitHub Actions
Queued
GitHub Actions
Queued
*Why* Re-review showed the setup-completed trigger added in the previous commit does not work: the persisted setupState write is fire-and-forget and lands on a later microtask, but the setup controller's state event fires synchronously, so a reading taken there reads the project as not-yet-VPEX-active and emits nothing. It also read the *active* project at fire time, which a mid-setup root switch could make the wrong one. *What* - Remove the `pythonSetupEnvironment.onDidChangeState` trigger; adoption is now a clean connect-time reading on `CONNECTED` (plus the activation-already-connected case). A project set up for the first time this session is measured from its next connect; the venv it just provisioned is already implied by python_env.setup.result = ok. - Update the inline and README docs to describe the connect-time semantics and why setup completion is deliberately not a trigger, and soften the "compute is known" wording (CONNECTED can carry currentTargetType = none). *Verification* - npx tsc --noEmit -p tsconfig.json — clean. - yarn test:unit — 840 passing, 10 pending, 0 failing. - yarn fix && eslint/prettier — clean. Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 14, 2026 14:22 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
*Why* setupState is a single workspace-scoped key with no per-project namespacing, so in a multi-root workspace it can't be attributed to the active root: a never-set-up sibling root would emit a spurious venvPresent=false and inflate the adoption denominator. Documenting the skew (as done for the drift detector) still leaves the adoption metric — whose whole purpose is an accurate rate — contaminated in that case. *What* Add an isAttributable seam to PythonSetupAdoptionManager; report() suppresses the reading (without latching, so a later single-root state still reports) when attribution is ambiguous. Wire it to `workspace.workspaceFolders.length <= 1`, so the gauge is emitted only when the single setupState key unambiguously describes the one root. Update the README to say multi-root is skipped rather than emitting spurious data. The drift detector still fires under the same single-key limitation; the real fix for both is the deferred per-project storage schema. *Verification* - npx tsc --noEmit -p tsconfig.json — clean. - yarn test:unit — 841 passing, 10 pending, 0 failing (adds a multi-root skip test). - yarn fix && eslint/prettier — clean. Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 14, 2026 14:29 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
*Why* The adoption README claimed multi-root workspaces were "suppressed entirely", which over-claims: the one-root guard is a heuristic, not proof of provenance, since the single workspace-scoped setupState key records no root. *What* Reword the limitation note to say the gauge is skipped for multi-root and that the guard reduces but does not eliminate mis-attribution (a mid-session reduction to one root can still orphan a sibling's key); the complete fix is the deferred per-project storage schema. Docs only, no behavior change. *Verification* - prettier -c — clean. No code changed. Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 14, 2026 14:32 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
*Why* The README explained "why it derives no env key" by referring to an earlier design plan, which a reader of the code has no context for. *What* Reword the section to state the rationale directly (reports the compute kind only; the CLI is the env-key authority and drift already emits off it) without referencing any prior plan. Docs only. *Verification* - prettier -c — clean. No code changed. Co-authored-by: Isaac
Contributor
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
rugpanov
requested a deployment
to
test-trigger-is
August 14, 2026 14:54 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests ✅ all 35 test jobs passed for |
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
1 participant
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.
Why
The extension persists a Python-setup state per project but has no measure of whether the managed environment actually sticks. The compute-drift detection (
python_env.drift) compares the selected compute's env key against the recorded one — but it never checks that the.venvstill exists. A user who deletes the environment while compute is unchanged is not "drifted", yet has plainly stopped using the managed env. Drift is also event-driven, so it carries no per-session denominator to turn into an adoption rate.What
Adds
python_env.adoption: a once-per-session, categorical gauge emitted only for a project that has a uv-native setup on record (databricks.pythonSetup.setupStatepresent). Its mere presence is the adoption-rate denominator; it carries:venvPresent— whether the project's managed.venvinterpreter still exists on disk.currentTargetType—cluster | serverless | none, read straight from the connection. No env key is derived here, so there is no second source of truth competing with the CLI / drift detector.Changes:
constants.ts: newEvents.PYTHON_ENV_ADOPTION+EventTypewith per-field comments (the schema's single source of truth).pythonSetupExtensions.ts:recordPythonSetupAdoption, enumerating both fields explicitly (allowlist discipline) so the emitted schema stays compiler-enforced.PythonSetupAdoptionManager: thin, dependency-injected controller — dedupes once per project root per session, gates on VPEX-active, skips when attribution is ambiguous (multi-root), and is best-effort (a throwing seam never propagates into the observed flow, and does not latch the dedup, so a transient failure retries).extension.ts: wire real seams and fire on the firstCONNECTEDtransition (so a compute is attached); dedup makes repeats safe.telemetry/README.md: rationale for the event — the VPEX gate, its distinction frompython_env.drift, why it derives no env key, the connect-time-only trigger, and the multi-root limitation.Additive and measurement-only: no persisted-state change, no migration, and no behavior change to the flow it observes.
Multi-root note
setupStateis a single workspace-scoped key with no per-project namespacing, so in a multi-root workspace it can't be pinned to the active root; the gauge is skipped there rather than emit a spuriousvenvPresent: false. Fully resolving it needs a per-project storage schema (a shared limitation the drift detector also carries), deferred.Verification
npx tsc --noEmit -p tsconfig.json— clean.yarn test:unit— 841 passing, 10 pending, 0 failing (10 new controller tests + emitter tests).yarn fix && yarn test:lint— eslint + prettier clean.This pull request and its description were written by Isaac.