Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) --------
Expand Down
12 changes: 12 additions & 0 deletions changelog.d/flake8-isort-pin.fixed.md
Original file line number Diff line number Diff line change
@@ -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`.
23 changes: 23 additions & 0 deletions changelog.d/mypy-hook-stub-parity.changed.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions opencontractserver/shared/QuerySets.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import hashlib
from collections.abc import Iterable
from datetime import timedelta
from typing import TYPE_CHECKING, Any, Optional, TypeVar

Expand Down Expand Up @@ -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"
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
11 changes: 10 additions & 1 deletion opencontractserver/tests/test_corpus_canonical_caml_migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading