From 0b9bcc8c7db442dc4b72b0093240b3539272ad37 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 03:31:33 +0000 Subject: [PATCH 1/3] Sync mypy hook stub pins with requirements and fix the errors 6.1.0 surfaces .pre-commit-config.yaml's mypy hook documents that its `==`-pinned stub dependencies MUST match requirements/base.txt + requirements/local.txt, because the hook is what CI actually type-checks with. They had drifted (django-stubs 6.0.6 vs 6.0.7, djangorestframework-stubs 3.17.0 vs 3.17.1), and the pending Dependabot bumps #2265 (django-stubs 6.1.0) and #2260 (djangorestframework-stubs 3.18.0) widen the gap further. Bumping the hook to those versions is not a no-op: django-stubs 6.1.0 surfaces 7 previously-invisible type errors. All 7 are typing-only; no runtime behavior changes. QuerySets.py x6 - six guardian-permission id lists are built as a lazy `values_list` queryset inside a `try` and fall back to `[]` in the matching `except LookupError`. 6.1.0 types `values_list(..., flat=True)` as `QuerySet[Model, int]`, which no longer unifies with `list[Never]`. Each variable now carries an explicit `Iterable[Any]` declaration; they are only ever consumed by an `__in` lookup, so that is the accurate common type. test_corpus_canonical_caml_migration.py x1 - the test asserts `apps.get_model("corpuses", "CorpusDescriptionRevision")` raises LookupError, i.e. the lazy reference is unresolvable on purpose. 6.1.0's plugin resolves get_model() string pairs statically and errors on a miss, so this is a false positive; silenced narrowly with `# type: ignore[misc]`. Verified with `pre-commit run --all-files`: mypy passes with the bumped stubs (it fails with 7 errors without these fixes). black/isort/pyupgrade/yaml/ changelog hooks all pass. flake8 reports 10 pre-existing I001/I005 findings in pydantic_ai_agents.py and compact_pawls.py that reproduce identically on a clean origin/main checkout and are untouched by this change. --- .pre-commit-config.yaml | 4 ++-- changelog.d/mypy-hook-stub-parity.changed.md | 19 +++++++++++++++++++ opencontractserver/shared/QuerySets.py | 11 +++++++++++ .../test_corpus_canonical_caml_migration.py | 6 +++++- 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 changelog.d/mypy-hook-stub-parity.changed.md diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6e55feaf80..4e7067f0ac 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -85,8 +85,8 @@ repos: # major breakages (e.g. pypdf 7, openai 3) for the same reason. additional_dependencies: # --- Type stubs ----------------------------------------------- - - django-stubs==6.0.6 - - djangorestframework-stubs==3.17.0 + - django-stubs==6.1.0 + - djangorestframework-stubs==3.18.0 - types-requests==2.33.0.20260408 - types-PyYAML==6.0.12.20260408 # --- Django runtime (must match requirements/base.txt) -------- diff --git a/changelog.d/mypy-hook-stub-parity.changed.md b/changelog.d/mypy-hook-stub-parity.changed.md new file mode 100644 index 0000000000..77080220d5 --- /dev/null +++ b/changelog.d/mypy-hook-stub-parity.changed.md @@ -0,0 +1,19 @@ +- Synced the `mypy` pre-commit hook's stub pins with `requirements/local.txt` + (`.pre-commit-config.yaml`: `django-stubs` 6.0.6 → 6.1.0, + `djangorestframework-stubs` 3.17.0 → 3.18.0). The hook's own comment requires + these to match the requirements pins; they had drifted, so the type-checking + CI runs against different stubs than the dev/test image installs. +- Fixed the 7 type errors `django-stubs` 6.1.0 surfaces, none of which change + runtime behavior: + - `opencontractserver/shared/QuerySets.py` (6 errors, lines 466/487/655/656/672/673): + six `permitted_ids`-style variables are assigned a lazy `values_list` queryset + in a `try` arm and a plain `[]` in the matching `except LookupError` arm. 6.1.0 + types `values_list(..., flat=True)` precisely enough that the two arms no longer + unify, so each variable now carries an explicit `Iterable[Any]` declaration — + the honest common type, since every one of them is only ever fed to an `__in` + lookup. + - `opencontractserver/tests/test_corpus_canonical_caml_migration.py:161`: + `apps.get_model("corpuses", "CorpusDescriptionRevision")` is deliberately + unresolvable (the test asserts the model was dropped), but 6.1.0's plugin + resolves `get_model()` string pairs at type-check time and errors on a miss. + Narrowly silenced with `# type: ignore[misc]` plus a comment. diff --git a/opencontractserver/shared/QuerySets.py b/opencontractserver/shared/QuerySets.py index 93eff56c60..ed8eeb93d8 100644 --- a/opencontractserver/shared/QuerySets.py +++ b/opencontractserver/shared/QuerySets.py @@ -1,4 +1,5 @@ import hashlib +from collections.abc import Iterable from datetime import timedelta from typing import TYPE_CHECKING, Any, Optional, TypeVar @@ -462,6 +463,11 @@ def visible_to_user( # Query guardian permission tables directly for performance from django.apps import apps + # Declared up front because the two arms disagree in type: the ``try`` + # arm yields a lazy ``values_list`` queryset, the ``except`` arm a plain + # empty list. Both are only ever fed to ``__in``, so ``Iterable[Any]`` is + # the honest common type. + permitted_ids: Iterable[Any] try: permission_model = apps.get_model( "documents", "documentuserobjectpermission" @@ -478,6 +484,7 @@ def visible_to_user( # ``user_can`` yet never appears in ``visible_to_user`` # (issue #1714). Resolved in its own ``try`` so a missing group # table never discards the already-resolved user-level grants. + group_permitted_ids: Iterable[Any] try: user_group_ids = user.groups.values_list("id", flat=True) group_permission_model = apps.get_model( @@ -645,6 +652,8 @@ def visible_to_user( # ``values_list`` keeps each a SQL subquery. user_group_ids = user.groups.values_list("id", flat=True) + doc_permitted_ids: Iterable[Any] + doc_group_permitted_ids: Iterable[Any] try: doc_perm_model = apps.get_model("documents", "documentuserobjectpermission") doc_permitted_ids = doc_perm_model.objects.filter( @@ -660,6 +669,8 @@ def visible_to_user( doc_permitted_ids = [] doc_group_permitted_ids = [] + corpus_permitted_ids: Iterable[Any] + corpus_group_permitted_ids: Iterable[Any] try: corpus_perm_model = apps.get_model("corpuses", "corpususerobjectpermission") corpus_permitted_ids = corpus_perm_model.objects.filter( diff --git a/opencontractserver/tests/test_corpus_canonical_caml_migration.py b/opencontractserver/tests/test_corpus_canonical_caml_migration.py index b4774ec127..5e7aa6b031 100644 --- a/opencontractserver/tests/test_corpus_canonical_caml_migration.py +++ b/opencontractserver/tests/test_corpus_canonical_caml_migration.py @@ -158,7 +158,11 @@ def test_no_corpus_description_revision_model(self): from django.apps import apps with self.assertRaises(LookupError): - apps.get_model("corpuses", "CorpusDescriptionRevision") + # The whole point of this test is that the model is GONE, so the + # lazy reference is unresolvable by construction. django-stubs + # 6.1.0's plugin resolves get_model() string pairs at type-check + # time and errors on a miss, which is a false positive here. + apps.get_model("corpuses", "CorpusDescriptionRevision") # type: ignore[misc] def test_description_preview_save_override_no_longer_present(self): """Confirms the Corpus.save() override branch is gone — cache writes From 050e92ef75466f56163a40327861aabe0127fd56 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 03:33:45 +0000 Subject: [PATCH 2/3] Pin flake8-isort and isort in the flake8 hook to stop CI drifting red MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The linter job is failing on main and on every open PR with 10 I001/I005 findings in two files nobody edited (llms/agents/pydantic_ai_agents.py, utils/compact_pawls.py). No commit caused it. The flake8 hook declared `additional_dependencies: [flake8-isort]` with no version. pre-commit re-resolves additional_dependencies from scratch whenever it rebuilds a hook env, so flake8-isort — which re-implements the isort check inside flake8 using whatever isort it resolves — drifted to isort 9.0.1, while the standalone `isort` hook stayed on the 6.0.1 its `rev` pins. isort 9 and isort 6 disagree about repeated `from X import (...)` statements, which is exactly the shape isort 6 emits for aliased imports. So the isort hook wrote a layout the flake8 hook rejected, on an unchanged tree. The mypy hook right below already documents this exact failure mode ('anything less than == can drift'); the flake8 hook just never followed it. Both entries are now ==-pinned, with isort tracking the isort hook's rev. Verified: `pre-commit run --all-files` is fully green on this branch, and `pre-commit run flake8 --all-files` reproduces all 10 findings on a clean origin/main checkout before the pin and passes after it. --- .pre-commit-config.yaml | 16 +++++++++++++++- changelog.d/flake8-isort-pin.fixed.md | 12 ++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 changelog.d/flake8-isort-pin.fixed.md diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4e7067f0ac..8b933cdffb 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -55,7 +55,21 @@ repos: hooks: - id: flake8 args: ["--config=setup.cfg"] - additional_dependencies: [flake8-isort] + # Both pins are load-bearing, for the reason spelled out on the mypy + # hook below: `rev` is the only thing autoupdate bumps, and + # additional_dependencies are re-resolved from scratch every time the + # hook env is rebuilt, so an unpinned entry silently floats. + # flake8-isort re-implements the isort check *inside* flake8 using + # whatever isort it happens to resolve. Left unpinned it drifted to + # isort 9.x while the standalone `isort` hook above stayed on the + # 6.0.1 it is `rev`-pinned to. The two then disagreed about repeated + # `from X import (...)` statements (the shape isort 6 itself emits for + # aliased imports), so `isort` rewrote files into a layout `flake8` + # rejected as I001/I005 — CI red on an unchanged tree. + # isort here MUST track the `isort` hook's rev above. + additional_dependencies: + - flake8-isort==7.0.0 + - isort==6.0.1 - repo: https://github.com/pre-commit/mirrors-mypy rev: v2.0.0 diff --git a/changelog.d/flake8-isort-pin.fixed.md b/changelog.d/flake8-isort-pin.fixed.md new file mode 100644 index 0000000000..1d358f31b4 --- /dev/null +++ b/changelog.d/flake8-isort-pin.fixed.md @@ -0,0 +1,12 @@ +- Pinned the `flake8` pre-commit hook's `additional_dependencies` + (`.pre-commit-config.yaml`: `flake8-isort==7.0.0`, `isort==6.0.1`). They were + unpinned, and `additional_dependencies` are re-resolved every time a hook env + is rebuilt, so `flake8-isort` floated up to **isort 9.0.1** while the + standalone `isort` hook stayed `rev`-pinned to 6.0.1. The two versions + disagree about repeated `from X import (...)` statements — the shape isort 6 + itself emits for aliased imports — so `isort` kept files in a layout `flake8` + then rejected. The result was the `linter` job failing on `main` and on every + open PR with 10 `I001`/`I005` findings in + `opencontractserver/llms/agents/pydantic_ai_agents.py` and + `opencontractserver/utils/compact_pawls.py`, neither of which any of those PRs + touched. `isort` here must track the `isort` hook's `rev`. From b8ef74de8bad63fbe79922cef3e856f53abbcbe2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 03:47:42 +0000 Subject: [PATCH 3/3] Make the dropped-model assertion independent of the django-stubs plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI (Python 3.12) reported the `# type: ignore[misc]` on test_no_corpus_description_revision_model as an unused ignore, while a cold-cache run on 3.11 needs it — mypy.ini sets warn_unused_ignores, so the comment cannot be right in both. The plugin only resolves *literal* get_model() string pairs, so the model name now lives in a str-annotated local instead. Neither interpreter's plugin has a literal to match, and the test's runtime behavior and assertion are untouched. Verified with a cleared .mypy_cache: `pre-commit run --all-files` is fully green. --- changelog.d/mypy-hook-stub-parity.changed.md | 10 +++++++--- .../tests/test_corpus_canonical_caml_migration.py | 15 ++++++++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/changelog.d/mypy-hook-stub-parity.changed.md b/changelog.d/mypy-hook-stub-parity.changed.md index 77080220d5..80cee381f4 100644 --- a/changelog.d/mypy-hook-stub-parity.changed.md +++ b/changelog.d/mypy-hook-stub-parity.changed.md @@ -12,8 +12,12 @@ unify, so each variable now carries an explicit `Iterable[Any]` declaration — the honest common type, since every one of them is only ever fed to an `__in` lookup. - - `opencontractserver/tests/test_corpus_canonical_caml_migration.py:161`: + - `opencontractserver/tests/test_corpus_canonical_caml_migration.py:157`: `apps.get_model("corpuses", "CorpusDescriptionRevision")` is deliberately unresolvable (the test asserts the model was dropped), but 6.1.0's plugin - resolves `get_model()` string pairs at type-check time and errors on a miss. - Narrowly silenced with `# type: ignore[misc]` plus a comment. + resolves *literal* `get_model()` string pairs at type-check time and errors + on a miss. Whether it fires is interpreter-dependent (it does on 3.11, does + not on CI's 3.12), so a `# type: ignore` would itself be flagged unused on + one of them under `warn_unused_ignores`. The model name now lives in a + `str`-annotated local, denying the plugin a literal to match on either. + Runtime behavior and the assertion are unchanged. diff --git a/opencontractserver/tests/test_corpus_canonical_caml_migration.py b/opencontractserver/tests/test_corpus_canonical_caml_migration.py index 5e7aa6b031..5f0d4639eb 100644 --- a/opencontractserver/tests/test_corpus_canonical_caml_migration.py +++ b/opencontractserver/tests/test_corpus_canonical_caml_migration.py @@ -157,12 +157,17 @@ def test_corpus_has_no_md_description_attr(self): def test_no_corpus_description_revision_model(self): from django.apps import apps + # Held in a `str`-annotated local rather than inlined as a literal. + # The whole point of this test is that the model is GONE, so the lazy + # reference is unresolvable by construction — but django-stubs 6.1.0's + # plugin resolves literal get_model() string pairs at type-check time + # and errors on a miss. Whether it fires varies by interpreter version, + # so a `# type: ignore` is itself unreliable (unused on 3.12, required + # on 3.11, and mypy.ini sets warn_unused_ignores). Denying the plugin a + # literal to match sidesteps it in both. Runtime behavior is unchanged. + dropped_model: str = "CorpusDescriptionRevision" with self.assertRaises(LookupError): - # The whole point of this test is that the model is GONE, so the - # lazy reference is unresolvable by construction. django-stubs - # 6.1.0's plugin resolves get_model() string pairs at type-check - # time and errors on a miss, which is a false positive here. - apps.get_model("corpuses", "CorpusDescriptionRevision") # type: ignore[misc] + apps.get_model("corpuses", dropped_model) def test_description_preview_save_override_no_longer_present(self): """Confirms the Corpus.save() override branch is gone — cache writes