fix(scripts): run ruff format check and lock-bound ruff in lint:py

[Bill Berry (WilliamBerryiii)]

fix(scripts): run ruff format check and lock-bound ruff in lint:py

Description

npm run lint:py ran ruff check only, while the CI Python lint job ran ruff check and ruff format --check. A locally green Python change could still fail CI. The local runner also picked whichever ruff it found first — the project .venv, otherwise a global install — so local and CI runs could execute different ruff versions and reach different formatter verdicts.

This change made the default local lane execute the same command set as CI and bound ruff selection to each project's committed lockfile.

• fix(scripts): added ruff format --check . after ruff check . in the default branch of Invoke-PythonLint.ps1. Both gates always run, so a lint failure cannot mask a formatting failure, and each result records checkExitCode and formatExitCode separately with its own console diagnostic.

• fix(scripts): added lock-aware ruff resolution to PythonLintHelpers.psm1 through three new exported functions. Get-LockedRuffVersion reads the ruff version pinned by uv.lock, Get-RuffVersionString reports a candidate binary's version, and Resolve-ProjectRuff accepts only an exact match — evaluating the project .venv binary (Linux, then Windows layout) before a global ruff, and retaining every rejected candidate as diagnostic evidence.

  • The runner never synchronizes dependencies. A missing or stale binary fails the project before ruff executes and reports the required version plus uv sync --locked as the setup action.
  • Projects without a uv.lock keep the previous .venv-then-global fallback and report an explicit unlocked-fallback mode, making no version claim.
  • -Fix mode resolves through the same resolver, so checks and fixes cannot run different ruff versions. Its command sequence remains check . --fix followed by format ..

• test(scripts): extended the two owning Pester suites. PythonLintHelpers.Tests.ps1 gained coverage for lock parsing (valid, ruff-free, malformed, and version-bleed locks), exact .venv matching on both layouts, global fallback after a .venv mismatch, global mismatch rejection, and all three unlocked-fallback outcomes. Invoke-PythonLint.Tests.ps1 gained a call ledger asserting exact ruff arguments and ordering, a non-mutation assertion for default mode, separate lint-only and format-only failure contexts, a resolution-failure context asserting ruff is never invoked, an exact -Fix sequence assertion, and serialized per-phase field assertions. The former generic failure context was reclassified as combined failure rather than removed, and the existing missing-ruff, exception, discovery, output-persistence, and real-ruff I001 contexts were left intact.

• docs(architecture): corrected the two lint:py rows in docs/architecture/workflows.md and added a "Python Lint Parity" subsection naming the three remaining execution differences — CI provisions the environment while the local runner only verifies it, local discovery covers projects the per-PR matrix excludes, and the hosted lane defaults to running only when a PR changes .py or .pyi files.

• docs(scripts): rewrote the Invoke-PythonLint.ps1 Features, Parameters, and Usage text in scripts/linting/README.md to describe lock-aware verify-only resolution, the default formatter check, per-phase exit codes, and unlocked fallback.

• fix(devcontainer): changed the per-skill provisioning loop in .devcontainer/scripts/on-create.sh from uv sync to uv sync --locked, so a fresh container installs the exact ruff versions the verify-only lane requires. Every pyproject.toml under .github/skills has a committed uv.lock, so no project loses provisioning.

package.json was not modified. .github/instructions/ci-owned-validation.instructions.md was consulted first, and the existing lint:py wrapper already satisfied the requirement without a script-taxonomy change.

Related Issue(s)

Closes #2694

Type of Change

Select all that apply:

Code & Documentation:

• Bug fix (non-breaking change fixing an issue)
• New feature (non-breaking change adding functionality)
• Breaking change (fix or feature causing existing functionality to change)
• Documentation update

Infrastructure & Configuration:

• GitHub Actions workflow
• Linting configuration (markdown, PowerShell, etc.)
• Security configuration
• DevContainer configuration
• Dependency update

AI Artifacts:

• Reviewed contribution with hve-builder and addressed all actionable findings
• Copilot instructions (.github/instructions/*.instructions.md)
• Copilot prompt (.github/prompts/*.prompt.md)
• Copilot agent (.github/agents/*.agent.md)
• Copilot skill (.github/skills/*/SKILL.md)
• Copilot hook (.github/hooks/*/*.json)
• Eval spec added/updated for changed AI artifacts (evals/)

Note for AI Artifact Contributors:

• Agents: Research, indexing/referencing other project (using standard VS Code GitHub Copilot/MCP tools), planning, and general implementation agents likely already exist. Review .github/agents/ before creating new ones.
• Skills: Must include both bash and PowerShell scripts. See Skills.
• Model Versions: Contributions MUST target models listed in the model catalog (scripts/linting/model-catalog.json) whose provider appears in providerAllowlist and whose status is ga or preview. Run npm run lint:models to validate references.
• See Agents Not Accepted and Model Version Requirements.

Other:

• Script/automation (.ps1, .sh, .py)
• Other (please describe):

Testing

Bounded Pester suites, run through npm run test:ps:

Suite	Result
scripts/tests/linting/PythonLintHelpers.Tests.ps1	28 passed, 0 failed, 0 skipped
scripts/tests/linting/Invoke-PythonLint.Tests.ps1	35 passed, 0 failed, 0 skipped

Repository validation:

Command	Result
npm run lint:py	Passed across all discovered projects, reporting the locked ruff version per project
npm run lint:ps	Passed
npm run lint:md	Passed — 493 files, 0 issues
npm run lint:tables	Passed — no reformatting required
npm run lint:frontmatter	Passed — 837 files, 0 errors, 0 warnings
npm run validate:skills	Passed — 62 skills, 0 errors, 0 warnings
npm run lint:dependency-pinning	Passed — 679 dependencies, 0 unpinned
shellcheck .devcontainer/scripts/on-create.sh	Passed — only a pre-existing SC2016 informational finding on an unrelated line

The verify-only contract was exercised for real during validation. scripts/evals/moderation had ruff 0.16.1 in its .venv against a 0.16.2 lock, so the first lint:py run failed that project before ruff executed and reported uv sync --locked. Running that command as a separate setup step brought the environment to the lock, after which the lane passed. The 935 MB moderation environment was never provisioned implicitly by the lint path.

A scoped diff inspection confirmed that exactly the seven intended files changed, with no .py, .pyi, uv.lock, or pyproject.toml diff — satisfying the requirement that no source is reformatted as an incidental side effect.

Checklist

Required Checks

• Documentation is updated (if applicable)
• Files follow existing naming conventions
• Changes are backwards compatible (if applicable)
• Tests added for new functionality (if applicable)

AI Artifact Contributions

• Used hve-builder review mode to review contribution
• Addressed all actionable findings from the hve-builder review
• Verified contribution follows common standards and type-specific requirements

Required Local Checks

The following local-safe validation commands must pass before merging:

• Local validation aggregate: npm run validate:local
• Documentation validation (if docs changed): npm run validate:docs
• Spell checking: npm run spell-check
• Link validation: npm run lint:md-links

Security Considerations

• This PR does not contain any sensitive or NDA information
• Any new dependencies have been reviewed for security issues
• Security-related scripts follow the principle of least privilege

Additional Notes

Behavior change for contributors: a project that commits uv.lock must now have a matching ruff already installed before npm run lint:py will check it. A stale environment fails that project with the required version and uv sync --locked as the named setup action, rather than silently linting with a different ruff. Lint never installs or synchronizes dependencies.

The same --locked flag now applies to devcontainer skill provisioning. Because on-create.sh runs under set -euo pipefail and find -execdir returns non-zero when an invocation fails, a skill whose committed lock no longer resolves would stop container creation instead of silently relocking. The existing pytest-tests.yml and fuzz-tests.yml lanes already run uv sync --locked against the same projects, so a stale lock fails CI before it can reach a fresh container.

Ruff version convergence across skills was deliberately left out of scope. Lock-bound execution makes the selected version deterministic per project, but the repository still spans several ruff 0.15.x minors. Ruff 0.16 brings Markdown code blocks into formatter scope and at least one existing Markdown file would fail under it, so that belongs to its own change rather than riding along with a parity fix.

✅ - Generated by Copilot

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.64%. Comparing base (3310433) to head (0487615).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2709      +/-   ##
==========================================
- Coverage   82.65%   82.64%   -0.01%     
==========================================
  Files         166      166              
  Lines       22214    22200      -14     
  Branches       30       30              
==========================================
- Hits        18361    18348      -13     
+ Misses       3850     3849       -1     
  Partials        3        3              

Flag	Coverage Δ
docusaurus	90.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
scripts/linting/Invoke-PythonLint.ps1	73.07% <ø> (-2.75%)	⬇️
scripts/linting/Modules/PythonLintHelpers.psm1	100.00% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Eval Execution

⚠️ No eval summary was produced.

[Jamie Kim (jkim323)]

Thanks for addressing the script issues and tightening the local/CI Python lint behavior. Have two things for you to look at.

[Jamie Kim (jkim323)]

Hm could we consider using uv sync --locked followed by uv run ruff here, matching python-lint.yml and the siblingInvoke-PythonTests.ps1 runner, instead of manually parsing uv.lock and comparing ruff --version strings? uv sync --locked would provide the missing fresh-environment bootstrap, while uv run ruff would use the project’s lock-selected executable—the same model CI uses. The current resolver recreates only part of uv’s lock/environment-selection behavior and creates a divergent provisioning contract from test:py for the same discovered projects. Was the verify-only divergence intentional and tested against every discovered project?

[Jamie Kim (jkim323)]

Does this propagate a failed per-project uv sync --locked? With find -execdir ... \;, a nonzero command exit only makes that find expression evaluate false; it does not cause find itself to fail, so set -euo pipefail will not stop container setup. A stale skill lock can therefore leave its .venv uncreated while on-create.sh still reports success, and only fail later in lint:py as a missing/mismatched Ruff environment. Could we replace this with an explicit loop or wrapper that returns nonzero when any project sync fails, matching the direct moderation sync below?

[Katrien De Graeve (katriendg)]

Thanks for taking this on — closing the local/CI gap on ruff format --check and binding ruff selection to uv.lock is a genuine correctness win, and the per-phase exit codes plus the "Python Lint Parity" section make the remaining differences legible instead of folklore. I pulled the branch and ran both Pester suites in isolation (62 passed, 1 skipped), checked PSScriptAnalyzer against the repo settings (clean), and validated Get-LockedRuffVersion against all 16 committed uv.lock files — it extracted the right version every time and matched the installed .venv ruff wherever one exists. The doc claims about python-lint.yml and the per-PR matrix exclusion also check out against the workflows.

I've left three inline comments, all about the blast radius of the new verify-only contract rather than the implementation itself: one project that nothing provisions, the moderation eval becoming a hard blocker for validate:local, and an asymmetry with the sibling Invoke-PythonTests.ps1 lane. None of them are objections to the approach — flagging them so the decisions are explicit rather than implicit.

[Katrien De Graeve (katriendg)]

The --locked change is right, but this loop is now load-bearing for the new lint contract and there's a project it doesn't reach.

Get-PythonSkill scans the whole repo. Outside .github/skills there are two Python projects:

./scripts/evals/moderation        [uv.lock / no .venv]
./.github/hooks/shared/telemetry  [uv.lock, ruff 0.15.16]

.github/hooks/shared/telemetry is provisioned by neither this loop (.github/skills only) nor copilot-setup-steps.yml. Under the new verify-only contract, a fresh devcontainer or coding-agent runner will fail npm run lint:py — and therefore npm run validate:local — on that project with uv.lock requires ruff 0.15.16 … Run 'uv sync --locked'. CI won't catch it, because python-lint.yml syncs each matrix directory itself, and it happens to have a .venv in existing containers, which is why local validation passed here.

Two options: extend provisioning to cover non-skill projects, or narrow local discovery to the roots that are actually provisioned.

Separately, copilot-setup-steps.yml:123 still runs bare uv sync while this moved to --locked; the repo instructions ask that both environments be evaluated together, so it's worth deciding deliberately whether they should diverge.

[Katrien De Graeve (katriendg)]

This bullet accurately describes the scope difference, but it understates the consequence now that resolution is strict.

scripts/evals/moderation is excluded from the per-PR matrix precisely because of the torch/detoxify stack (~935 MB). Local discovery still picks it up, and it now requires an exactly-locked ruff there. npm run lint:py is part of validate:local, so the practical effect is that every contributor must provision that heavyweight environment just to run the local aggregate — including contributors who previously had a green run via a global ruff.

Could this bullet say explicitly that the wider local scope is now fatal rather than merely broader? And is it worth giving the runner an exclusion that mirrors the CI one (or a -SkipProject), so the local aggregate doesn't inherit a cost that CI deliberately declined to pay per-PR?

[Katrien De Graeve (katriendg)]

Worth reconciling this with the sibling lane. scripts/linting/Invoke-PythonTests.ps1 (lines ~89-96) already does lock-aware resolution, but it provisions — uv sync --locked --dev when uv.lock exists, uv sync --dev otherwise — and then executes through uv run.

So after this change, npm run test:py repairs exactly the environment npm run lint:py refuses to repair, and a contributor blocked by the lint failure can unblock it by running the test lane. Going through uv run ruff here would have given exact CI command parity and automatic provisioning for free.

The verify-only choice is defensible — the PR description makes a good case that lint shouldn't implicitly install 935 MB — but right now it's a silent departure from an established in-repo precedent. A short comment here (or a line in the parity section) stating that lint intentionally verifies while the test lane provisions would keep the next person from "fixing" the inconsistency in the wrong direction.