Skip to content

feat(ci): add mypy CI job, upgrade mypy 1.14→2.3, align to Python 3.11 - #297

Open
BenjSz wants to merge 6 commits into
mainfrom
benjams/mypi_checking
Open

feat(ci): add mypy CI job, upgrade mypy 1.14→2.3, align to Python 3.11#297
BenjSz wants to merge 6 commits into
mainfrom
benjams/mypi_checking

Conversation

@BenjSz

@BenjSz BenjSz commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a dedicated mypy CI job in test_pull_request.yaml. The pre-commit mypy hook has always existed for local dev, but CI's pre-commit lint job skips it (SKIP: mypy) because torch/transformers aren't installed there. The new job installs those stubs on its own runner and is wired into the run-spyre-unit-tests gate so a mypy failure blocks merge.
  • Upgrade mypy from 1.14 to 2.3 in the dev optional-dependency group. Enables PEP 695 syntax handling and matches what the ecosystem's numpy/torch stubs now assume.
  • Align the mypy analysis target to Python 3.11 (both [tool.mypy].python_version and the CI job's python-version). Every runtime job in the repo — Spyre pod, test_daily, push-to-clickhouse, the whole _test_matrix fleet — uses 3.11. Pinning mypy to 3.13 (as the earlier iteration of this PR did) would let it green-light 3.13-only syntax or stubs that then fail to import on the pod.
  • Set explicit_package_bases = true so mypy handles the hf_adapters/ package layout without complaint.
  • Add ignore_missing_imports overrides for deepspec.* (optional private DSpark dep) and sentence_transformers.* (real runtime dep, but the mypy job stays lean by not installing it — and the package ships no py.typed marker so installing it wouldn't help). Follows the existing sympy.* precedent.
  • Fix scripts/check_uv_lock.sh to defer to uv lock --check instead of blindly requiring a staged uv.lock whenever pyproject.toml is staged. The naive rule fires on tool-only edits ([tool.mypy], [tool.ruff], …) that uv.lock doesn't track, forcing a spurious lock touch. The hook now only fails when the lockfile is genuinely stale.

Test plan

  • The new mypy CI job runs on this PR and reports Success: no issues found in 35 source files.
  • Locally: python3.11 -m venv /tmp/mypy311 && /tmp/mypy311/bin/pip install --quiet torch transformers 'mypy>=2.3.0,<3' && /tmp/mypy311/bin/mypy --config-file=pyproject.toml — passes.
  • Confirm the run-spyre-unit-tests fan-in gate correctly requires mypy to have passed (test by re-running with a deliberate type error in a scratch commit, then reverting).
  • Confirm scripts/check_uv_lock.sh passes on a tool-only pyproject.toml edit (already verified: this PR's own [tool.mypy] edits pass the hook).
  • Confirm the hook still fires when a real dep changes — bump a [project].dependencies entry, don't stage uv.lock, verify the hook fails.

🤖 Generated with Claude Code

Signed-off-by: Benjamin Sznajder <benjams@il.ibm.com>
@BenjSz
BenjSz requested a review from anubhavjana as a code owner August 6, 2026 09:36
@BenjSz
BenjSz enabled auto-merge August 6, 2026 09:36
@BenjSz
BenjSz removed the request for review from anubhavjana August 6, 2026 09:36
@BenjSz BenjSz linked an issue Aug 6, 2026 that may be closed by this pull request
@assaftibm

Copy link
Copy Markdown
Collaborator

Thanks for fixing this — bumping mypy to 2.3.0 is the right root-cause fix. I'd independently started patching the same thing on feat/curated-models (worked around old mypy by leaving python_version unset instead of upgrading it) and have since reverted that commit, so pyproject.toml on my branch is back to matching main. No conflict from my side; this PR should own it.

One thing worth a look before merge — python_version = "3.13" vs requires-python = ">=3.11,<3.15".

Setting it to the highest supported version means mypy accepts syntax that fails at runtime on the lowest. Concretely:

$ mypy --python-version 3.13 probe.py     # `type Alias = int | str`
Success: no issues found in 1 source file

$ mypy --python-version 3.11 probe.py
probe.py:1: error: Type statement is only supported in Python 3.12 and greater  [syntax]

So if someone later adds a PEP 695 type X = ... alias, or any other 3.12+ syntax, CI stays green while pip install on a 3.11 interpreter (which requires-python permits) breaks at import. I checked and there's no such syntax in hf_adapters/ today, so this is forward-looking rather than a live bug.

Normally the fix is python_version = "3.11" (match the floor, not the ceiling). The complication is numpy: uv.lock pins it per interpreter — 2.3.5 below 3.13, 2.5.1 at 3.13+ — and numpy 2.5.1's bundled stubs use PEP 695 aliases, so a 3.11 setting on a 3.13 venv makes mypy reject those stubs and abort before checking anything. That's exactly what the old "3.10" was doing.

I couldn't test whether mypy 2.3.0 handles that combination, since I'd have to upgrade my venv off the lockfile to try it — could you check mypy --python-version 3.11 on your branch? If 2.3.0 parses the numpy stubs regardless of the target (plausible, given the bump is what unblocked this), "3.11" would be strictly better. If it still aborts, then leaving python_version unset is an alternative: mypy analyzes as the interpreter it runs under, which always matches the numpy resolved for that interpreter, and there's no fixed value to go stale.

Either way, not blocking — "3.13" is a strict improvement over "3.10", which was checking zero files.

Two other notes:

  • The deepspec.* override is a duplicate of what I'd written, same intent. Yours is fine; flagging only so it's clear it's independently confirmed as the right call — deepspec is imported lazily inside functions precisely because it's absent off-pod.
  • Folding explicit_package_bases into config rather than the pre-commit entry is a nice cleanup.

Worth knowing for context: the lint job sets SKIP: mypy (it needs the project venv), so mypy only ever runs locally via pre-commit. That's why a config broken to the point of checking zero files went unnoticed for a while.

Signed-off-by: Benjamin Sznajder <benjams@il.ibm.com>
@arielge
arielge force-pushed the benjams/mypi_checking branch from 239d5ea to de8cf85 Compare August 7, 2026 09:11
@arielge
arielge disabled auto-merge August 7, 2026 09:11
arielge
arielge previously approved these changes Aug 7, 2026
BenjSz and others added 3 commits August 9, 2026 11:45
The check_uv_lock hook previously failed on any staged pyproject.toml
whose uv.lock wasn't also staged, even when the pyproject edit was
purely tool config ([tool.mypy], [tool.ruff], …) that uv.lock does not
track. Defer to `uv lock --check` — the authoritative signal — and only
fail when the lockfile is genuinely out-of-date.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Benjamin Sznajder <benjams@il.ibm.com>
Introduces a dedicated mypy job in test_pull_request.yaml so type checking
runs on every push/PR (the local pre-commit mypy hook already exists, but
CI previously skipped it because torch/transformers weren't installed in
the lint job). The new job installs those stubs and runs mypy on its own
runner, and is wired into the run-spyre-unit-tests gate so its result
blocks merge.

Pin both the tool ([tool.mypy].python_version) and the CI job to 3.11 —
the same interpreter every runtime job and the Spyre pod use — so mypy
analyzes the code as it will actually run, rather than as some hypothetical
future 3.13. Also add an ignore_missing_imports override for
sentence_transformers, matching the existing sympy/deepspec pattern: it's
a real runtime dep but not installed in this cheap mypy env, and it ships
no py.typed marker so installing wouldn't help anyway.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Benjamin Sznajder <benjams@il.ibm.com>
@BenjSz
BenjSz requested a review from kmehant as a code owner August 9, 2026 10:55
@BenjSz BenjSz changed the title fix the mypy errors + dummy commit for checking feat(ci): add mypy CI job, upgrade mypy 1.14→2.3, align to Python 3.11 Aug 9, 2026
@BenjSz
BenjSz requested a review from arielge August 9, 2026 11:19
@arielge
arielge enabled auto-merge August 9, 2026 11:59
@arielge
arielge added this pull request to the merge queue Aug 9, 2026
@spyre-ci

spyre-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

❌ merge-queue-integration: failure

:warning: orch *trigger-pr-validation* — *yellow* · arches amd64 · fp amd64=a29a5a2e
L0 deeptools                amd64 ❌
L1 flex                     amd64 ⛔
L2 aiu-toolbox/ibm-aiu-toolbox-e2e amd64 ⛔
L2 spyre-comms              amd64 ⛔
L3 spyre-backend/spyre-backend-dev amd64 ⛔
L4 torch-spyre/torch-spyre-dev amd64 ⛔
L5 hf-adapters/hf-adapters-dev amd64 ⛔
L6 spyre-inference/spyre-inference-dev amd64 ⛔

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 9, 2026
@arielge
arielge added this pull request to the merge queue Aug 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with mypi in the Commits

3 participants