diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6e55feaf8..8b933cdff 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 @@ -85,8 +99,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/flake8-isort-pin.fixed.md b/changelog.d/flake8-isort-pin.fixed.md new file mode 100644 index 000000000..1d358f31b --- /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`. 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 000000000..80cee381f --- /dev/null +++ b/changelog.d/mypy-hook-stub-parity.changed.md @@ -0,0 +1,23 @@ +- 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:157`: + `apps.get_model("corpuses", "CorpusDescriptionRevision")` is deliberately + unresolvable (the test asserts the model was dropped), but 6.1.0's plugin + 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/shared/QuerySets.py b/opencontractserver/shared/QuerySets.py index 93eff56c6..ed8eeb93d 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 b4774ec12..5f0d4639e 100644 --- a/opencontractserver/tests/test_corpus_canonical_caml_migration.py +++ b/opencontractserver/tests/test_corpus_canonical_caml_migration.py @@ -157,8 +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): - apps.get_model("corpuses", "CorpusDescriptionRevision") + 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