ci(python): add advisory ty type checking with SARIF reporting - #3752
Draft
mcosgriff wants to merge 13 commits into
Draft
ci(python): add advisory ty type checking with SARIF reporting#3752mcosgriff wants to merge 13 commits into
mcosgriff wants to merge 13 commits into
Conversation
- Add python_type_check.yml alongside python_lint, running ty with
--exit-zero so it annotates but never fails the build yet
- Add tools/ty_report.py to convert ty's GitLab JSON into SARIF for
code scanning, since ty cannot emit SARIF and its paths need
rewriting to be repo-root relative
- Add tools/generate_singleton_stubs.py and the five .pyi it emits, so
metaclass-forwarded calls like Logger.info("msg") check correctly;
drops diagnostics from 816 to 675
- Pass --no-build and --no-install-project to every uv step, so no
build backend executes in CI
- Add ty dev dependency plus just stubs and typecheck recipes
Co-Authored-By: Claude noreply@anthropic.com
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3752 +/- ##
==========================================
- Coverage 80.01% 79.10% -0.91%
==========================================
Files 885 894 +9
Lines 65382 66857 +1475
Branches 2543 2543
==========================================
+ Hits 52313 52885 +572
- Misses 12407 13310 +903
Partials 662 662
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Lock the PEP 723 dependencies of both tools scripts and pass --locked everywhere, so a new mypy or ruff release cannot silently change what runs or make stubs-check report spurious drift - Validate --input, --sarif and --markdown in ty_report.py before touching the file system, confining them to the repo, cwd, temp dir and the paths the runner declares via GITHUB_STEP_SUMMARY and friends - Reject traversing arguments, symlinks, directories and missing parents with a clear message and exit 1 instead of writing Co-Authored-By: Claude noreply@anthropic.com
- Constrain every field ty_report.py parses from ty's JSON: rule names must be kebab-case or become unknown-rule, helpUri is emitted only for names that matched, and line/column are clamped to SARIF positions - Drop diagnostics whose path escapes the repository, which previously leaked the runner's absolute filesystem paths into the SARIF - Escape values interpolated into the markdown summary, so a crafted rule name cannot close the link and inject its own into the summary - Reject non-list input and malformed entries with a specific message instead of raising TypeError partway through the report - Add tools to the ty check paths and group _typeshed with the standard library, so the generated stubs sort the way a reader expects Co-Authored-By: Claude noreply@anthropic.com
- Replace the ruff and ty version ranges with exact pins: ruff's formatter decides whether the generated stubs still match, and any ty 0.0.x release can change which diagnostics are reported - Pin the stub generator's script dependencies the same way, which bumps mypy 1.20.2 to 2.3.1 now that a <2 cap no longer holds it back - Hold ruff at 0.16.4 in both uv.lock and the script lockfile, so just format and just stubs cannot disagree about formatting Co-Authored-By: Claude noreply@anthropic.com
- Add .github/actions/setup-uv-python and route all five uv workflows through it, so one file holds the uv version instead of the three that were in effect: 0.10.4, 0.12.5, and unpinned - Give the action a hardcoded --frozen and boolean flags rather than a caller-supplied argument string, so no call site can skip the lockfile - Add tool_version_check to watch the pins dependabot cannot see, filing one tracking issue for versions in Actions inputs, Dockerfile ARGs and PEP 723 script headers - Split github-actions majors into their own dependabot group, since one group is one pull request and a single breaking action upgrade would otherwise block every routine bump - Set setup-uv's working-directory everywhere, so cache discovery finds openc3/python/uv.lock instead of a repository root without one Co-Authored-By: Claude noreply@anthropic.com
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ring sanitization' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Accumulating it into a bash array left the lockfile guarantee invisible to both readers and scanners, though the flag was always present. Only the optional flags build up in a variable now; behavior is unchanged. Co-Authored-By: Claude noreply@anthropic.com
v9 stopped pruning the cache by default and v10 disables caching for pull_request_target, workflow_run and release when enable-cache is auto. Neither applies here: every call site sets enable-cache explicitly and no workflow triggers on those events. Co-Authored-By: Claude noreply@anthropic.com
The job syncs with openc3 installed as an editable project, and uv revalidates that install on every uv run, so --no-build fails with "can't be installed because it is marked as --no-build but has no binary distribution" and exits 2. It only works where the job also passes --no-install-project.
ruff parses files and never imports them, so the lint job has no use for the editable openc3 install. Skipping it is what allows --no-build, which keeps any third-party build backend from executing in that job.
Code scanning renders the rule id in its Rule filter, so a bare unresolved-attribute is indistinguishable from another tool's rules. CodeQL namespaces the same way with rb/ and go/ prefixes. The docs anchor still uses the bare name, so helpUri is unaffected.
- Fix the ruff job, which exited 2: it skips the editable openc3 install, and uv run --frozen re-evaluates whether that skipped project could be installed, rejecting it under --no-build - Apply the same pair to the unit test uv run calls, so no third-party build backend executes there either; that job keeps the project installed, which pytest needs to import openc3
Both test signatures fit on one line at the configured 120 columns. The file landed unformatted because python_lint only format-checks openc3 and the script-runner scripts, never test/.
|
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
2 participants
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.
Summary
Phase 1 of #3651 — bring-up only.
tyruns on every Python PR with--exit-zero, so it reports but cannot block a merge.tycheck run beside CodeQL. Rule ids are namespacedty/so they're distinguishable in the Rule filter, and each links to its rule docs..pyiit generates take the count 820 → 679, by teaching ty that the singleton metaclasses forward tocls.instance().just stubs-checkfails CI if a stub goes stale.--frozenis enforced rather than caller-supplied, and the static-analysis jobs build no source distributions.tool_version_checkfiles a tracking issue for the pins dependabot can't see — Actions inputs, Dockerfile ARGs, PEP 723 headers.The remaining 679 are deliberately untouched; roughly half trace to ~30 declarations rather than 679 distinct problems. Phase 2 in #3651 covers the fixes and removing
--exit-zero.Test plan
Refs #3651
Associated https://github.com/OpenC3/cosmos-enterprise/pull/716