Skip to content

Make PEP 723 metadata diagnostics actionable - #1769

Merged
Stella Huang (StellaHuang95) merged 2 commits into
microsoft:mainfrom
StellaHuang95:fixInlineScriptIssues
Sep 8, 2026
Merged

Make PEP 723 metadata diagnostics actionable#1769
Stella Huang (StellaHuang95) merged 2 commits into
microsoft:mainfrom
StellaHuang95:fixInlineScriptIssues

Conversation

@StellaHuang95

Copy link
Copy Markdown
Contributor

Summary

Make PEP 723 inline-script metadata failures easier to diagnose in the Python Environments output:

  • Include the script path in metadata parsing and validation messages.
  • Translate TOML payload coordinates to the actual Python script line, including metadata preceded by a shebang or comments.
  • Keep warnings concise while retaining the full parser error in verbose logs.
  • Add regression coverage for source labels, script-relative line numbers, and callers that omit the source label.

Parsing acceptance, environment provisioning, and interpreter routing are unchanged. This PR includes both commits from fixInlineScriptIssues, including the formatting follow-up.

A malformed PEP 723 block logged a warning that named no file, printed ten
frames of minified bundle internals, and reported a row index into the
reconstructed TOML rather than a line in the script. It read like an
extension crash while telling the user neither which file was wrong nor
where.

    inline script metadata: failed to parse TOML in `# /// script` block:
    [TomlError: Unterminated string at row 1, col 27, pos 26:
    1> requires-python = ">=3.11
        at TOMLParser.parseBasicString (dist/extension.js:45477:28)
        ... 9 more frames

    inline script metadata in C:\src\app.py: invalid TOML in the
    `# /// script` block (line 2): Unterminated string

Take an optional source label on readInlineScriptMetadata and thread it
through every diagnostic; readInlineScriptMetadataFromFile passes the
script path. Translate the TOML row into a real script line via the known
block offset, drop the payload-relative coordinates and caret excerpt, and
move the full error (with its stack) to the debug level for anyone
diagnosing the parser itself. Parsing behaviour is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rchiodo

Rich Chiodo (rchiodo) commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

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

@rchiodo

Copy link
Copy Markdown
Contributor

Result: 🔴 could-not-verify

Verification details

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

Summary: Podman setup was declined or unavailable; isolated test execution was skipped.

Test runs: none recorded.

@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 the review-auto:approved Automated review: no blocking findings (approval posted). label Sep 8, 2026
@StellaHuang95
Stella Huang (StellaHuang95) merged commit 6cb40a9 into microsoft:main Sep 8, 2026
46 checks passed
*/
export function readInlineScriptMetadata(scriptText: string): InlineScriptMetadata | undefined {
export function readInlineScriptMetadata(scriptText: string, source?: string): InlineScriptMetadata | undefined {
const where = source ? ` in ${source}` : '';

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.

Maybe add the space in the consuming strings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

like in line 124 and 129? not sure if it's a good idea to put repetitive code in multiple places thorough, the space is only added when there's actual source exist do the check is done before it being consumed.

// full error (with stack and excerpt) goes to the debug level for anyone
// diagnosing the parser itself.
const tomlRow = getTomlErrorRow(err);
const at = tomlRow === undefined ? '' : ` (line ${blockStartLine + 1 + tomlRow})`;

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.

Same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants