chore(deps-dev): update mypy requirement from >=1.8.0 to >=2.3.1 - #71
chore(deps-dev): update mypy requirement from >=1.8.0 to >=2.3.1#71dependabot[bot] wants to merge 1 commit into
Conversation
Updates the requirements on [mypy](https://github.com/python/mypy) to permit the latest version. - [Changelog](https://github.com/python/mypy/blob/master/CHANGELOG.md) - [Commits](python/mypy@v1.8.0...v2.3.1) --- updated-dependencies: - dependency-name: mypy dependency-version: 2.3.1 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
srpatcha
left a comment
There was a problem hiding this comment.
Review — eDB#71 "chore(deps-dev): update mypy requirement from >=1.8.0 to >=2.3.1"
head: 599b21e author: app/dependabot ci: pass (CodeQL only — the test matrix did not run)
Verdict: The bump itself is safe — I installed both versions and measured it: mypy
2.3.1 reports fewer errors than 1.8.0 on this tree, not more. What makes the PR
pointless as it stands is that nothing in this repo consumes the number it changes, and
the mypy step it exists to improve is configured so it cannot fail.
What was measured
Cloned origin/master (0457c0a), installed each version, ran CI's exact command:
| Version | mypy . --ignore-missing-imports --no-strict-optional |
|---|---|
| 1.8.0 (current floor) | 343 errors in 43 files (77 source files checked) |
| 2.3.1 (proposed floor) | 338 errors in 41 files (77 source files checked) |
So the upgrade introduces no new failures. Under the repo's own [tool.mypy] block
(pyproject.toml:76-80, disallow_untyped_defs = true) against src/ it reports
68 errors in 25 files.
Findings
| # | Severity | File:line | Finding | Recommended fix |
|---|---|---|---|---|
| 1 | High | .github/workflows/ci.yml:42 |
continue-on-error: true on the Type check (mypy) step. mypy cannot fail this build, so raising its floor changes nothing observable. .ai/reviewer.md is explicit that a verification whose result is discarded is a finding regardless of the reason given. The 338 errors above are being emitted into a green tick today. |
Remove continue-on-error: true from ci.yml:41-42. If 338 errors cannot be fixed at once, gate on a subset — mypy src/edb/security src/edb/api --strict — and grow it, rather than running everything and ignoring all of it. |
| 2 | High | .github/workflows/ci.yml:33 |
The test job installs from requirements.txt, a file that does not exist in this repository. git show origin/master:requirements.txt fails; there is no such file at any path. The most recent CI — eDB push run on master concluded failure with ERROR: Could not open requirements file: [Errno 2] No such file or directory: 'requirements.txt' in its log. The job as written can never pass. |
Replace ci.yml:33-34 with pip install -e ".[dev]". That installs the project and the dev extra — which is precisely where mypy and ruff live (pyproject.toml:38-45) — and is the only change that makes this PR's floor take effect. |
| 3 | High | .github/workflows/ci.yml:34 |
Even setting aside finding 2, line 34 is pip install pytest pytest-cov pytest-benchmark mypy ruff — unconstrained, and never installing the package or its [project.optional-dependencies].dev extra. No check in this repository reads pyproject.toml:44, the line this PR edits. CI resolves whatever mypy is newest on the day it runs; the declared floor is documentation, not a constraint. That is why the PR's checks are green while proving nothing. |
Same fix as finding 2. Until then, note in the PR that the change is declarative only. |
| 4 | Medium | src/edb/config.py:39 and :62 |
The upgraded mypy flags a real defect the current CI throws away: src/edb/api/app.py:43: error: "list[str]" has no attribute "split" [attr-defined]. Root cause — EDBConfig declares cors_origins twice, at config.py:39 as list[str] defaulting to ["http://localhost:3000"] and again at config.py:62 as str defaulting to "http://localhost:3000". I confirmed at runtime that the second wins (EDBConfig.model_fields['cors_origins'].annotation is str, and app.py:43's .split(",") works), so this is not a crash — but it means the CORS-origins field has a silently dead second declaration. Anyone editing line 39 to widen or narrow allowed origins would change nothing at all. |
Delete config.py:39-42, keeping the str form at :62-65 that the code actually uses. Then finding 1's fix will keep this class of defect from returning. |
| 5 | Medium | .github/workflows/ci.yml:8 (at base 5e436bd6) |
The Test (Python …) matrix did not run on this PR; the only checks are Analyze (Python)/CodeQL and assign. Root cause, verified: at base 5e436bd6, ci.yml's trigger was pull_request: branches: [main] while this PR targets master. master fixed that in #70 (6d53641) with branches: [master, main]; the PR is 3 commits behind. The green tick on this PR reflects CodeQL, not tests. |
Rebase onto origin/master, then apply findings 1–3 so the rebased run is meaningful. |
| 6 | Medium | repo setting: branches/master/protection |
GET /repos/embeddedos-org/eDB/branches/master/protection returns required_status_checks: null. Reviews are required; no check is. CI — eDB is currently failure on master and that blocks nothing, which is how finding 2 has survived. |
Set required status checks on master once finding 2 is fixed and the job can actually pass. |
| 7 | Low | pyproject.toml:11 vs .github/workflows/ci.yml:17 |
requires-python = ">=3.11" but the CI matrix is ["3.10", "3.11", "3.12"]. The package declares it does not support 3.10 while CI claims to test it. Not caused by this PR, and it happens not to bite here — mypy 2.3.1's own requires_python is >=3.10, so the new floor installs on every matrix entry. |
Drop "3.10" from the matrix, or lower requires-python if 3.10 really is supported. |
| 8 | Low | (repo-wide) | .github/dependabot.yml is absent from master — removed by eaf4e1c (PR #68), whose message reads "Dependabot is disabled org-wide: config removed here, and alerts plus automated security fixes turned off via the API. 90 open Dependabot PRs". This PR is an orphan: nothing will rebase or supersede it, and closing it will not cause the bump to be re-proposed. |
Merge it or close it deliberately. Leaving it open is the one option that produces nothing. |
Architecture conformance
Conforms, with one caveat that is about §21 rather than §5.1.
- §5.1 architectural law. One line in
[project.optional-dependencies].dev. mypy is a
development-time tool, not a runtime dependency, so nothing points up a tier. eDB does
not gain a dependency on any higher-level product. - §21 tier placement. eDB is Tier 3 — Advanced ("AI, neural interfaces and embedded
data"). Its own dev tooling belongs in its ownpyproject.toml. §21.1 untouched. - Caveat.
.ai/platform.mdrequires that compatibility claims be checked against
§23.2 rather than asserted. A version floor that no check installs (finding 3) is an
asserted claim. That is a property of this repo's CI, not of this PR, but this PR is
the thing being asked to carry the claim.
Proposed changes
Smallest sequence that keeps things working:
- Fix
ci.yml:33-34→pip install -e ".[dev]"(findings 2 and 3). Do this first; it is
the change that makes everything else observable. It also fixes a job that currently
cannot pass at all. - Delete the dead
cors_originsdeclaration atconfig.py:39-42(finding 4). - Merge this PR. With step 1 in place the floor is now real, and I have measured that
2.3.1 does not regress the error count. - Remove
continue-on-error: truefromci.yml:42, gating on a subset first
(finding 1). Do this last — it is the step that turns 338 pre-existing errors into a
red build, and it needs its own plan. - Set required status checks on
master(finding 6).
Not checked
- The 338 mypy errors themselves. I counted them under both versions and read only the
one that led to finding 4. I have not established that the other 337 are benign, and I
am not claiming they are. - Which errors are new in 2.3.1 versus resolved. The totals moved 343 → 338 across 43 →
41 files; I did not diff the two error sets, so it is possible some errors disappeared
and others appeared while the net moved down by five. - mypy 1.x → 2.x release notes. I measured the outcome on this tree rather than reading
the changelog. A behaviour change that this codebase happens not to exercise would not
show up in my numbers. - The test suite.
pytestwas not run —ci.yml:33prevents CI from doing so and I
did not work around it locally. I do not know whether eDB's tests currently pass. - Python 3.10/3.11 specifically. All local runs used Python 3.12.14 on Linux. The
matrix's other interpreters andmacos-13/windows-2022were not exercised.
Automated architecture review of 599b21eedd5e — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.
Updates the requirements on mypy to permit the latest version.
Changelog
Sourced from mypy's changelog.
... (truncated)
Commits
d642c44Bump version to 2.3.1a392429[mypyc] Fix crash on double yielding Iterators (#21826)4843e77[mypyc] Fixdefault_factoryfor inherited dataclass (#21785)14f5df9[mypyc] Clear coroutine env on coroutine completion (#21734)6dfa06dFix crash when unpacking return value from overload (#21830)a385746Bump version to 2.3.1+dev8aabf84Drop +dev from version4d8ad2aUpdate changelog for 2.3 release (#21728)2c21546[mypyc] Update documentation of race conditions under free threading (#21726)a9f62a3[mypyc] Make attribute access memory safe on free-threaded builds (#21705)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)