Skip to content

feat: offer inline-script env setup as an unresolved-import quick fix - #1788

Merged
Stella Huang (StellaHuang95) merged 1 commit into
microsoft:mainfrom
StellaHuang95:stellahuang-microsoft-inline-script-code-action
Sep 15, 2026
Merged

Stella Huang (StellaHuang95) merged 1 commit into
microsoft:mainfrom
StellaHuang95:stellahuang-microsoft-inline-script-code-action

Conversation

@StellaHuang95

Copy link
Copy Markdown
Contributor

Problem

The PEP 723 inline-script feature has exactly one discovery surface — a CodeLens — and provideCodeLenses returns [] whenever the document is dirty (src/features/inlineScript/codeLens.ts:51).

So the CodeLens is absent at the one moment a user most needs it: right after typing import requests and seeing the red squiggle. Users who don't already know PEP 723 never discover the feature at all.

Change

Register a CodeActionProvider (src/features/inlineScript/setupCodeAction.ts) that offers "Set up this script's Python environment" as a quick fix when:

  1. the inline-scripts feature flag is on,
  2. an unresolved-import diagnostic is present at the invocation range,
  3. the file is a local .py that could carry an inline-script environment,
  4. it isn't already set up (routing.shouldRoute(uri) is false), and
  5. it declares a valid # /// script block.

Gates are ordered cheapest-first because provideCodeActions can fire on cursor movement — the context.diagnostics scan happens before any parsing, so the common case (no unresolved import) costs nothing but a short array scan.

Metadata is parsed from the in-memory buffer, so unlike the CodeLens this works while the document is dirty.

Honest labeling — wording and mechanics

The title says what the action does, not that the squiggle will clear. It may well not: the unresolved module might be undeclared in the block, or declared under a different distribution name (PIL vs pillow). Correspondingly:

  • action.diagnostics is left unset — populating it tells VS Code the action resolves those diagnostics and opts it into fix-all affordances.
  • action.isPreferred is left unset, so it never pre-empts a genuine import fix such as "add import".

There is deliberately no import-name → package-name mapping. Setup installs the block's declared dependencies verbatim; the import name is never an input.

Diagnostic codes

Matched on code only, never source — Pyrefly-backed Pylance reports its source as the literal string pylance + pyrefly, so any source allow-list would be wrong somewhere. Diagnostic.code is string | number | {value, target}; the union is normalized and lowercased before comparison.

Checker Codes
Pyright / Pylance / basedpyright reportMissingImports, reportMissingModuleSource
Ty unresolved-import, possibly-missing-import
Pyrefly missing-import, missing-source, missing-source-for-stubs
mypy import-not-found, import-untyped

reportMissingModuleSource is included on purpose

Pylance's own isMissingImportDiagnostic() excludes it — and rightly so for their fix, since a stub-resolved module is already spelled correctly and has nothing for a "change spelling" action to suggest. For us the meaning is the opposite kind of useful: a stub was found but the source was not, i.e. the package isn't installed — precisely what setting up the environment fixes.

Reappears after a block edit — intentional

Once the script is set up the action disappears. If the user then edits the # /// script block (e.g. to add the dependency that was missing), it comes back by itself: InlineScriptRoutingRegistry.setMetadata resets validatedAssociation to false when the metadata identity changes (src/common/inlineScript/routingRegistry.ts:74-75). This is desired behavior and is covered by a test.

Save before setup — and a race it closes

setUpInlineScriptEnvironment resolves the block from disk via readInlineScriptMetadataFromFile. Since the quick fix is offered on a dirty buffer, the handler now saves first.

Saving alone wasn't enough. setUpInlineScriptEnvironment compares the metadata identity before and after create, and skips the association if it changed mid-setup. For a block the user had just typed, routing metadata is still undefined — the detector's own save handler would then populate it during create, the identity would go undefined → X, and the association would be silently skipped. The user would wait for an environment and get nothing.

saveScriptBeforeSetup() closes this by re-reading the file it just wrote and seeding routing.setMetadata(). The detector's later write computes the same identity from the same bytes, so the identity stays stable.

Tests

19 tests in setupCodeAction.unit.test.ts plus a 6-test setupInlineScriptEnvironmentHandler suite in setupEnvironment.unit.test.ts. Coverage includes every gating branch, all four diagnostic dialects, the {value, target} object form, case-insensitivity, the pylance + pyrefly source, diagnostics/isPreferred left unset, reappear-after-block-edit, in-memory parse while dirty, and the header byte bound.

Guard tests verified to fail without their guards

Per the convention established in #1772, each guard was removed one at a time and the covering test confirmed to fail — not merely to pass today. All 11 were detected:

Guard Failures when removed
feature-flag gate 1
unresolved-import diagnostic gate 2
routing-key (local .py) gate 1
already-set-up (shouldRoute) gate 2
PEP 723 metadata gate 3
header byte budget bound 1
action.diagnostics left unset 1
action.isPreferred left unset 1
save-before-setup 1
seed routing metadata after save 1
companion-prompt failure isolation 1

Two findings this produced, both worth calling out because they were caught by the mutation run rather than by the tests passing:

  • A real bug in this PR. promptUpdateExtensionsForInlineScripts() originally sat inside the setup try, so a prompt failure showed the user a "setup failed" message even though the environment had been created successfully. It's now outside the try with its own .catch, mirroring what the bulk path already did.
  • A test that passed for the wrong reason. does not run setup when the document could not be saved originally still passed with the guard removed: manager.create was unconfigured, so setEnvironment was never called either way, and the error message it asserted was being emitted by the notify path instead. It now configures a would-succeed setup and asserts manager.create was never called plus the specific save-failure message.

Notes

  • python-envs.setupInlineScriptEnv remains out of package.json by design; it's invoked only by the two UI surfaces.
  • No changes to ms-python.python or Pylance are needed.
  • One nuance worth a reviewer's eye: isInlineScriptsFeatureEnabled() is re-read per call in the provider, whereas registration is latched at activation. Flipping the setting mid-session without a reload makes the code action go inert while the CodeLens keeps showing. That's the safer direction, and the cheapest-first gate ordering requires the per-call check.

Validation

npm run lint ✓ · npm run compile-tests ✓ · npm run unittest ✓ (2323 passing, 6 pending, 0 failing)

Nothing here is user-visible: the whole surface stays behind the undeclared internal flag python-envs.inlineScripts.enabled, which defaults to false.

@StellaHuang95 Stella Huang (StellaHuang95) added the feature-request Request for new features or functionality label Sep 15, 2026
@bschnurr

Copy link
Copy Markdown
Member

🔒 Automated review in progress — Bill Schnurr (@bschnurr) is auto-reviewing this PR.

@StellaHuang95
Stella Huang (StellaHuang95) force-pushed the stellahuang-microsoft-inline-script-code-action branch from 53e524c to 06a02df Compare September 15, 2026 21:03
The only discovery surface for PEP 723 inline-script environment setup was
a CodeLens, and codeLens.ts hides it whenever the document is dirty. At the
moment a user types `import requests` and sees a red squiggle, the CodeLens
is gone, so users who do not already know about PEP 723 never find the
feature.

Register a CodeActionProvider that offers "Set up this script's Python
environment" when an unresolved-import diagnostic sits in a `.py` file that
declares a `# /// script` block and has no inline-script environment yet.
The provider parses the in-memory buffer, so it works while dirty, and the
command handler saves the document before setup because the environment
manager reads metadata from disk.

Gates run cheapest-first, before any parsing, because provideCodeActions can
fire on cursor movement: feature flag, then a matching diagnostic, then the
routing key, then shouldRoute, and only then the header parse.

Diagnostics are matched on `code` only, never `source` (Pyrefly-backed
Pylance reports its source as the literal "pylance + pyrefly"). Pyright, Ty,
Pyrefly and mypy dialects are all covered, including the {value, target}
object form of Diagnostic.code. reportMissingModuleSource is included
deliberately, unlike in Pylance's own isMissingImportDiagnostic: a stub
without source means the package is not installed, which setup fixes.

The action deliberately leaves `diagnostics` and `isPreferred` unset. Setup
installs the block's declared dependencies verbatim and may not resolve the
import at all, so the action must not claim to fix the diagnostic or
pre-empt a real import fix. The title promises environment setup, nothing
more.

Setup also now seeds routing metadata after saving a dirty document. Without
it a just-typed block goes from no metadata to an identity while `create`
runs, which setUpInlineScriptEnvironment reads as a concurrent edit and
silently skips the association. The companion-extension prompt moves out of
the setup try block so a failure there is no longer reported to the user as
a setup failure.

The feature stays behind the internal python-envs.inlineScripts.enabled
flag, so none of this is user-visible yet.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@StellaHuang95
Stella Huang (StellaHuang95) force-pushed the stellahuang-microsoft-inline-script-code-action branch from 06a02df to 85625cb Compare September 15, 2026 21:14
@bschnurr

Copy link
Copy Markdown
Member

Result: 🔴 could-not-verify

Verification details

Verification: Isolated verification observed failures that were not classified as caused by this PR: Dependency and test discovery, Compile test sources. The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: Verification could not proceed because offline dependency restoration failed with `ENOTCACHED` for `brace-expansion-2.1.4`. Consequently, `npm run compile-tests` failed because `tsc` was unavailable, and the targeted Mocha suites could not run. I identified 34 generated test cases added by the PR, covering the code-action gates and setup handler behavior. Confidence is low because no executable tests completed.

Test runs: 2 failed, 2 not run

  • ⚠️ Not run | Inline-script code-action and setup-handler unit suites | node ./node_modules/mocha/bin/mocha.js --config=./build/.mocha.unittests.json --grep 'Inline script setup code action|setupInlineScriptEnvironmentHandler'
  • Failed | unrelated to this PR | Dependency and test discovery | printf 'sandbox_profile=%s\n' "${AUTOMATION_SANDBOX_PROFILE:-}"; printf 'node='; node --version; printf 'npm='; npm --version; if [ -d node_modules ]; then echo 'node_modules=present'; else echo 'node_modules=missing'; fi; node -e "const p=require('./package.json'); console.log('compile-tests='+p.scripts['compile-tests']); console.log('unittest='+p.scripts.unittest);"; printf '%s\n' '--- mocha config ---'; cat build/.mocha.unittests.json; printf '%s\n' '--- changed paths vs HEAD1 ---'; git diff --name-status HEAD1..HEAD
  • ⚠️ Not run | Offline dependency bootstrap | npm ci --offline
  • Failed | unrelated to this PR | Compile test sources | npm run compile-tests
⚠️ Inline-script code-action and setup-handler unit suites diagnostic output
Test compilation was unavailable because offline dependency restoration failed and `tsc` was not installed.
Dependency and test discovery diagnostic output
sandbox_profile=typescript
node=v22.21.1
npm=10.9.4
node_modules=missing
compile-tests=tsc -p . --outDir out
unittest=mocha --config=./build/.mocha.unittests.json
warning: Not a git repository. Use --no-index to compare two paths outside a working tree
⚠️ Offline dependency bootstrap diagnostic output
npm error code ENOTCACHED
npm error request to https://packagefeedproxy.microsoft.io/npm/brace-expansion/-/brace-expansion-2.1.4.tgz failed: cache mode is 'only-if-cached' but no cached response is available.
Compile test sources diagnostic output
> vscode-python-envs@1.37.0 compile-tests
> tsc -p . --outDir out

sh: 1: tsc: not found

@bschnurr Bill Schnurr (bschnurr) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@bschnurr Bill Schnurr (bschnurr) added the review-auto:approved Automated review: no blocking findings (approval posted). label Sep 15, 2026
@StellaHuang95
Stella Huang (StellaHuang95) merged commit 3742822 into microsoft:main Sep 15, 2026
46 checks passed
Stella Huang (StellaHuang95) added a commit to StellaHuang95/vscode-python-environments that referenced this pull request Sep 15, 2026
Condenses the comments added in microsoft#1788 down to what is not obvious from the
code: the deliberate inclusion of reportMissingModuleSource, the reason the
routing metadata is reseeded after a save, and the reason the companion
extension version check sits outside the try block.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Stella Huang (StellaHuang95) added a commit that referenced this pull request Sep 16, 2026
)

## Problem

Setting up a PEP 723 inline-script environment signals success only by
the setup CodeLens **disappearing** — which is indistinguishable from
the lens never having been offered in the first place. It also leaves
the chosen base interpreter invisible, which matters when
`requires-python` matches several installed Pythons, or when one was
installed on demand via `uv python install`.

## Change

For five seconds after setup succeeds, the (now hidden) setup lens is
replaced by a passive confirmation anchored at the `# /// script` block:

> `Script environment ready (Python 3.12.4)`

then it expires on its own. The version comes from
`shortenVersionString(environment.version)` — the same helper
`getPythonInfo` uses for `script env (3.12.4)` — so the lens and the
environment's own name cannot disagree.

An empty command id renders the title as plain, non-clickable text: this
is a statement, not an action.

### No extra reflow

The line is already occupied by the setup lens at that moment, so the
confirmation does not add a shift — it delays the single existing one by
five seconds. This was the main objection to an earlier draft, and it
turned out not to apply.

## Scope

The entire behavioural change is one new row in `provideCodeLenses`:

| `isDirty` | metadata | `shouldRoute` | confirmation live | Before |
After |
|---|---|---|---|---|---|
| yes | — | — | — | `[]` | `[]` |
| no | none | — | — | `[]` | `[]` |
| no | yes | yes | **yes** | `[]` | **confirmation** |
| no | yes | yes | no | `[]` | `[]` |
| no | yes | no | — | setup lens | setup lens |

Everything else is byte-identical: the dirty guard, the metadata check,
`shouldRoute` semantics, and the setup lens title, command, arguments
and anchor. `routingRegistry.ts`, `envManager.ts`, `window.apis.ts`,
settings and `package.json` are untouched.

Both setup surfaces — the CodeLens and the unresolved-import quick fix
from #1788 — show the confirmation, since both route through
`python-envs.setupInlineScriptEnv`. The bulk command deliberately does
not: it already ends with its own `Set up {0} of {1} ...` summary and
can process scripts that are not open.

## Implementation notes

- `noteEnvironmentReady` cancels any existing timer for the script
first, so re-running setup restarts the window rather than inheriting a
nearly-expired one.
- Entries are keyed by `getInlineScriptRoutingKey`, so on Windows
`C:\App.py` and `c:\app.py` share one entry.
- `dispose()` clears every pending timer; a test asserts nothing fires
afterwards.
- Confirmations are in-memory and per-window, deliberately not persisted
— a confirmation is about an action you just took.

## Known limitation

If the file is edited **while** the environment builds,
`getSavedMetadataForPersistence` returns `{}` for the dirty document, so
`updateValidatedStateForSelection` leaves `shouldRoute` false and the
confirmation is unreachable until the next save — and lost entirely if
that takes longer than five seconds. Showing it anyway would mean
confirming an association that is not validated, so the miss is
preferable to the lie.

Expiry also depends on VS Code re-querying after
`onDidChangeCodeLenses`. That is normally immediate, but the lens can
outlast five seconds slightly under load. It always clears.

## Tests

Six new provider tests on `sinon.useFakeTimers()` (no wall-clock
dependency): shows with version, omits version when unresolved, expires
and fires exactly one refresh, routed-but-not-just-set-up shows nothing,
hidden while dirty, and no timer leak past `dispose()`. Three handler
tests cover the callback firing on success and staying silent when
creation returns nothing or throws.

The five pre-existing CodeLens tests are unmodified and still pass —
including *"hides the CodeLens once a validated association makes the
script routeable"*, which is the proof that the default routed path is
unchanged.

`npm run lint` OK, `npm run compile-tests` OK, `npm run unittest` OK
(2331 passing, 6 pending, 0 failing)

> `discovers a build that completes after the short retry window` is a
pre-existing flake unrelated to this change — verified by stashing these
changes and running it on a clean tree, where it failed 2 of 4 runs.

**Nothing here is user-visible by default**: the whole surface stays
behind the undeclared internal flag `python-envs.inlineScripts.enabled`,
which defaults to `false`.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-request Request for new features or functionality review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants