Skip to content

Keep PEP 723 inline script CodeLens live and persistent - #1906

Merged
Stella Huang (StellaHuang95) merged 2 commits into
microsoft:mainfrom
StellaHuang95:stellahuang95-pep-723-architecture
Oct 8, 2026
Merged

Stella Huang (StellaHuang95) merged 2 commits into
microsoft:mainfrom
StellaHuang95:stellahuang95-pep-723-architecture

Conversation

@StellaHuang95

@StellaHuang95 Stella Huang (StellaHuang95) commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Keep the setup CodeLens available whenever a recognizable PEP 723 script block exists, including incomplete or malformed live edits.
  • Keep Script environment ready (Python X.Y.Z) persistent for validated, available script environments.
  • Re-evaluate unsaved metadata edits immediately while preserving Ready and exact-file routing for body-only edits.
  • Prevent pending saved-metadata reads from restoring stale routing after a live metadata edit.
  • Validate malformed metadata when setup is invoked, preserving diagnostics and avoiding invalid environment associations.
  • Harden setup, save, cache-repair, and stale-operation behavior and document the updated user flow.
  • Add focused parser, routing, CodeLens, detector, setup, manager, and real VS Code integration coverage.

Validation

  • npm run lint
  • npm run compile-tests
  • npm run unittest — 2,812 passing, 7 pending after rebasing onto the latest main
  • Production webpack build
  • Inline VS Code integration — 8 passing
  • Real-environment PEP 723 workflows — 21 passing
  • Smoke suite — 31 passing
  • Environment discovery E2E — 4 passing
  • Manual Windows VS Code Insiders testbed matrix covering setup, persistent Ready, live edits, Undo/Revert, malformed metadata, diagnostics, quick fixes, Run File, Pylance routing, cache repair, restart persistence, and feature-off behavior
  • Static core, additional-use-case, quick-fix, and 8-KiB boundary verification

Notes

The tolerant block detector is presentation-only. Semantic parsing remains authoritative for diagnostics, setup eligibility, environment identity, persistence, and exact-file routing.

A separate full integration run completed with 57 passing and 8 pending tests plus one existing environment-creation timeout. The timed-out fixture selects the system manager, which cannot quick-create headlessly; instrumentation confirmed it does not enter the PEP 723 path.

@StellaHuang95 Stella Huang (StellaHuang95) added the feature-request Request for new features or functionality label Oct 7, 2026
@StellaHuang95
Stella Huang (StellaHuang95) force-pushed the stellahuang95-pep-723-architecture branch from 7f582fd to f302aba Compare October 7, 2026 22:26
@rchiodo

Rich Chiodo (rchiodo) commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR.

Comment thread src/features/inlineScript/lazyDetector.ts
@rchiodo

Copy link
Copy Markdown
Contributor

Result: ⚠️ partially-verified

Verification details

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

Summary: Offline dependency installation and `npm run compile-tests` succeeded in the container. Targeted inline-script suites passed: 285 common, 214 feature, and 370 manager tests, with 5 manager tests pending. No executed test failed; discovery's Git probes failed because the container checkout lacked Git metadata. The eight new VS Code integration tests could not run because no downloaded VS Code runtime was available. Unit coverage supports the live CodeLens, persistent Ready, validation, and routing behavior, but editor-host verification remains incomplete.

Test runs: 5 passed, 1 failed, 1 not run

  • ⚠️ Not run | Live inline-script CodeLens VS Code integration tests | npm run integration-test -- --grep 'Integration: Live inline script CodeLens'
  • ❌ Failed | unrelated to this PR | Batched dependency and test discovery | printf 'Sandbox profile: %s\n' "$AUTOMATION_SANDBOX_PROFILE"; node --version; npm --version; git log -1 --oneline; git status --short; node -e 'const fs=require("fs"); const p=require("./package.json"); console.log(JSON.stringify({scripts:p.scripts},null,2)); for(const f of ["node_modules/typescript/bin/tsc","node_modules/mocha/bin/mocha.js","out/test/unittests.js",".vscode-test"]) console.log(f,fs.existsSync(f));'; git ls-files 'src/test/common/inlineScript/' 'src/test/features/inlineScript/' 'src/test/managers/builtin/inlineScript/*' 'src/test/integration/inlineScript';
  • ✅ Passed | Inline-script common unit tests | node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter dot 'out/test/common/inlineScript/*.unit.test.js'
  • ✅ Passed | Inline-script feature unit tests | node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter dot 'out/test/features/inlineScript/*.unit.test.js'
  • ✅ Passed | Inline-script manager unit tests | node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter dot 'out/test/managers/builtin/inlineScript/*.unit.test.js'
  • ✅ Passed | Offline dependency bootstrap | npm ci --offline --no-audit --no-fund
  • ✅ Passed | Test compilation | npm run compile-tests
⚠️ Live inline-script CodeLens VS Code integration tests diagnostic output
Discovery reported .vscode-test false. These eight tests require a real VS Code extension host; downloading VS Code or extensions is unsupported in the network-isolated verification environment. Command was not executed.
❌ Batched dependency and test discovery diagnostic output
Sandbox profile: typescript
v22.21.1
10.9.4
fatal: not a git repository (or any parent up to mount point /)
node_modules/typescript/bin/tsc false
node_modules/mocha/bin/mocha.js false
out/test/unittests.js false
.vscode-test false
[container exit=128]

@rchiodo Rich Chiodo (rchiodo) added the review-auto:changes-requested Automated review: posted blocking findings to address. label Oct 8, 2026
Keep setup available for recognizable inline metadata while preserving diagnostics and validating on invocation. Persist validated ready state, react to live metadata edits, and retain exact-file routing for body-only edits.

Add focused unit and VS Code integration coverage plus user-facing documentation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Invalidate pending saved-metadata reads when the live PEP 723 block diverges, preventing an older read from restoring an obsolete exact-file route.

Add a regression test for the read/edit completion race.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@StellaHuang95
Stella Huang (StellaHuang95) force-pushed the stellahuang95-pep-723-architecture branch from f302aba to a8e1679 Compare October 8, 2026 00:42
@rchiodo

Copy link
Copy Markdown
Contributor

Result: ⚠️ partially-verified

Verification details

Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: Offline dependency installation and test compilation passed in the disposable container. Targeted inline-script suites reported **870 passing tests**, no failures, and five pending Windows-specific tests. Coverage included persistent Ready labels, live metadata edits, Undo, stale reads, setup validation, routing, and cache recovery. The eight new VS Code integration tests were not run because the sandbox lacks the required Electron/extension-host environment. Confidence is strong for unit-tested behavior, but end-to-end editor behavior remains unverified.

Test runs: 6 passed, 1 not run

  • ⚠️ Not run | Live inline-script CodeLens VS Code integration tests
  • ✅ Passed | Inline-script common unit tests | node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter dot 'out/test/common/inlineScript/*.unit.test.js'
  • ✅ Passed | Inline-script feature unit tests | node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter dot 'out/test/features/inlineScript/*.unit.test.js'
  • ✅ Passed | Inline-script manager unit tests | node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter dot 'out/test/managers/builtin/inlineScript/*.unit.test.js'
  • ✅ Passed | Dependency and runner discovery | printf 'Sandbox profile: %s\n' "$AUTOMATION_SANDBOX_PROFILE"; node --version; npm --version; git status --short; node -e "const p=require('./package.json'); console.log(JSON.stringify({scripts:p.scripts},null,2)); const fs=require('fs'); for (const p of ['package-lock.json','node_modules','node_modules/mocha/bin/mocha.js','node_modules/typescript/bin/tsc','out/test','build/.mocha.unittests.json','.vscode-test']) console.log(p+': '+fs.existsSync(p)); console.log(fs.readFileSync('build/.mocha.unittests.json','utf8'));"
  • ✅ Passed | Offline dependency preparation | npm ci --offline
  • ✅ Passed | Test compilation | npm run compile-tests
⚠️ Live inline-script CodeLens VS Code integration tests diagnostic output
Eight new integration tests require a real VS Code extension host. Preflight found no .vscode-test installation; downloaded-VS-Code Electron tests are unsupported in this offline sandbox. No integration command was executed.

@rchiodo Rich Chiodo (rchiodo) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved via Review Center.

@rchiodo Rich Chiodo (rchiodo) added review-auto:approved Automated review: no blocking findings (approval posted). and removed review-auto:changes-requested Automated review: posted blocking findings to address. labels Oct 8, 2026
@StellaHuang95
Stella Huang (StellaHuang95) merged commit e4f9939 into microsoft:main Oct 8, 2026
111 of 113 checks passed
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