typing: graduate tests.test_document_* chunk out of the mypy baseline

[JSv4]

Summary

Continues the mypy baseline drain tracked in #1738 (the #1331 → #1335 → #1447 cadence). After the config.* tail (PR #1748), the tests.permissioning.* subpackage (PR #1768), and the annotation leaf-file chunk (PR #1776), this is the next cohesive batch from the issue's tests.* leaf-files batching plan: the document domain chunk (12 files).

Removes all 12 [mypy-opencontractserver.tests.test_document_*] ignore_errors blocks from mypy.ini, prunes the corresponding 331 lines from docs/typing/mypy_baseline.txt (5141 → 4810), and fixes the 243 errors that surface under the currently-pinned mypy==2.0.0 / django-stubs==6.0.5 (per .pre-commit-config.yaml).

After this PR, the remaining baseline is: opencontractserver.mcp.tests.test_mcp, the other tests.* leaf-file domains (corpus / agent / extract / misc), the tests.{websocket,performance_optimizations}.* subpackages, the tests.{base,factories,diagnostic_runner,enhanced_diagnostic_runner} helpers, and the three intentional django-manager-missing model blocks.

The current 243 is below the 331-line baseline snapshot because the newer stubs already resolve several historical errors — notably the whole set_permissions_for_obj_to_user arg-type family (the same observation PR #1776 made for the annotation chunk). Two files (test_document_relationship_permissions, test_document_analysis_row) needed no code change at all — only block removal.

Latent type gap revealed by typing

The issue notes graduation passes are a chance to surface real gaps hidden behind Any. One surfaced here:

• test_document_index_tool.py — FieldFile.name is str | None. Once self.doc was annotated Document (it had been Any, masked behind the attr-defined error), the four setUp methods exposed self.doc.<field>.name (str | None) being passed straight into Storage.exists() / FieldFile.save(), both of which require str. Narrowed via a local name = self.doc.<field>.name + assert name is not None (the file is always saved in setUpClass, so the invariant holds).

Per-file fixes

File	Baseline	Fix
test_document_index_tool.py	132	class-level user/corpus/doc annotations on all 4 classes; User = get_user_model() → direct import; entries: list[IndexEntryItem] (×9) + exported: OpenContractsAnnotationPythonType (×2) literal annotations; FieldFile.name narrowing in the 4 setUps (above).
test_document_versioning_graphql.py	69	self.client → self.graphene_client rename (9 classes / 41 .execute()); 3× assertIsNotNone(x) → assert x is not None.
test_document_admin.py	34	class-level annotations on TestDocumentAdmin (superuser/corpus/document/document2/embedding_*); get_user_model() → direct import. Legit Django self.client = Client() left as-is.
test_document_relationship_mutations.py	34 (2 reproduced)	result.first().relationship_type → result[0].relationship_type after the count()==1 guard; 1× assertIsNotNone → assert. The 32 historical set_permissions_for_obj_to_user arg-types no longer reproduce.
test_document_relationship_permissions.py	23	no code change — historical arg-types no longer reproduce; block removed.
test_document_versioning.py	6	6× assert … is not None (a Document | None import result + FieldFile.name str | None).
test_document_mutations.py	11	self.client → self.graphene_client.
test_document_analysis_row.py	8	no code change — block removed.
test_document_uploads.py	7	self.client → self.graphene_client.
test_document_relationships.py	5	self.client → self.graphene_client.
test_document_queries.py	4	self.client → self.graphene_client.
test_document_path_migration.py	2	declared user/context on the local MockContext mock; get_user_model() → direct import.
Total	331 baseline / 243 current

All changes are behaviour-neutral or behaviour-equivalent: class-level annotations and assert vs assertIsNotNone are runtime no-ops/equivalents; result[0] after a count()==1 guard returns the same row as .first(); the self.client → self.graphene_client rename masks Django's TestCase.client, but the original code already shadowed it by assignment so no test relied on it (the convention matches PR #1776 / permissioning/test_permissioning.py).

Test plan

• mypy --config-file mypy.ini opencontractserver config → Success: no issues found in 1242 source files (the acceptance gate per Continue draining the mypy baseline established in #1331 — test suite + config tail #1738), run in a venv mirroring the .pre-commit-config.yaml pin set (mypy==2.0.0, django-stubs==6.0.5, djangorestframework-stubs==3.17.0, plus the Django runtime stack).
• pyupgrade --py39-plus / black==26.1.0 / isort==6.0.1 / flake8==7.2.0 (+flake8-isort) clean on all changed files; whitespace/EOF pre-commit hooks clean.
• All 12 modules py_compile cleanly.
• Backend CI (backend.yml linter + test jobs) — the authoritative runtime check.

Note on local testing: the Dockerized backend suite could not be exercised in the authoring container (docker compose -f test.yml is unavailable). The changes are behaviour-neutral (see above), so CI's backend job is relied on for the runtime check.

Refs #1738

[claude]

Code review for PR 1851 - typing graduation for tests.test_document.* chunk. See detailed review below.

[claude]

Code Review: typing: graduate tests.test_document_* chunk

Overview

Clean, well-scoped typing graduation PR. Removes 12 ignore_errors blocks from mypy.ini, prunes 331 lines from the baseline (5141 to 4810), and fixes the 243 errors surfacing under mypy==2.0.0 / django-stubs==6.0.5. All changes are behaviour-neutral, consistent with the prior annotation-chunk PR (#1776), and the per-file table in the PR description is a model of clear documentation.

---

What is done well

• self.client → self.graphene_client rename is the right call. Shadowing Django's TestCase.client (the HTTP test client) in graphene test classes is a latent footgun; the rename eliminates the ambiguity and matches the convention established in PR typing: graduate tests.test_annotation_* chunk out of mypy baseline #1776.
• assert name is not None over assertIsNotNone for FieldFile.name narrowing: assertIsNotNone does not narrow the type for mypy; a bare assert x is not None does. Semantically sound too — the file is always saved in setUpClass.
• result[0] after count()==1: Type-correct (avoids the T | None return of .first()) and semantically equivalent.
• Direct from opencontractserver.users.models import User over get_user_model(): Internally consistent with the project's pinned custom user model; fine in test code.
• Class-level attribute annotations (user: User, corpus: Corpus, doc: Document) on test classes using setUpClass/setUpTestData are the correct mypy idiom for class-shared fixtures.
• entries: list[IndexEntryItem] and exported: OpenContractsAnnotationPythonType literal annotations are a nice side-effect — they make the test data shapes self-documenting.

---

One item to double-check

test_document_path_migration.py — MockContext.context annotation

# After
class MockContext:
    user: User
    context: "MockContext"

user: User is clearly needed. The self-referential context: "MockContext" annotation is unusual. Is info.context.context actually accessed anywhere in the code path exercised by this mock? If not, the annotation documents an attribute that is never assigned on the mock object, and a future reader may expect ctx.context to be populated. If it satisfies a protocol or structural subtype check, a brief inline comment would explain the intent. Worth a quick look before merge.

---

Minor: PR body attributes Claude Code

The PR description footer reads Generated by Claude Code. Per CLAUDE.md Baseline Commit Rules: "Never credit Claude or Claude Code in commit messages, PR messages, comments, or any other artifacts." This should be removed from the PR body before merge.

---

Summary

The code changes are correct, minimal, and behaviour-neutral. The MockContext.context annotation and the attribution footer are the only items worth addressing; neither is a blocker for the typing work itself.

[claude]

Code Review

This is a clean, well-scoped typing PR. The approach is consistent with the established cadence (#1748, #1768, #1776), the changes are behaviour-neutral, and the baseline drain progress is real (5141 → 4810).

What's solid

Class-level annotations (user: User, corpus: Corpus, doc: Document) across setUpClass/setUpTestData are the correct Django+mypy pattern. The parallel with prior PRs in this series makes it predictable.

self.client → self.graphene_client rename (9 classes, 41 call sites in test_document_versioning_graphql.py plus the other four files) is the right fix — the shadowing was always latent trouble if any test accidentally fell through to Django's TestCase.client. Consistent with PR #1776.

from opencontractserver.users.models import User direct import is cleaner than User = get_user_model() and is the correct pattern for using User as a type annotation (per CLAUDE.md).

FieldFile.name narrowing (the surfaced latent gap) is the most substantive finding in this batch. The name = self.doc.<field>.name; assert name is not None pattern in the four setUp methods is correct: the invariant holds because the file is always saved in setUpClass, and using assert rather than self.assertIsNotNone is required here — assertIsNotNone does not narrow the type for mypy, assert x is not None does.

assert x is not None replacing assertIsNotNone for the .is_deleted / .title / .version_number / .parent narrowings: same reasoning — the only way to give mypy the narrowing it needs. Runtime failure messages are slightly weaker, but in test fixtures at the point of setup/assertion this is the standard trade-off.

result[0] after count() == 1 guard (test_document_relationship_mutations.py): semantically equivalent to result.first(). The count guard is the pre-existing invariant; the subscript just makes it type-safe.

Minor observations (no blockers)

1. test_document_path_migration.py — MockContext comment (~line 463): The comment explaining the dual-role is justified (a class acting as both info and info.context is genuinely non-obvious), so it earns its place per the project's "WHY is non-obvious" rule. No action needed.

2. Two zero-change files (test_document_relationship_permissions.py, test_document_analysis_row.py): The PR correctly explains that the 23+8 historical set_permissions_for_obj_to_user arg-type errors no longer reproduce under the current stubs — block removal only. Worth the explicit call-out so future readers don't wonder why there's no code diff.

3. 2 residual errors in test_document_relationship_mutations.py: The description acknowledges "2 reproduced" are still present after this PR. These aren't in scope here, but it's worth confirming they're tracked in issue Continue draining the mypy baseline established in #1331 — test suite + config tail #1738 (they appear to be).

4. assert name is not None style consistency: The four setUp methods in test_document_index_tool.py use bare assert; the rest of the test suite generally uses self.assertIsNotNone. This is intentional for mypy narrowing (as noted above), but if the project later adds a mypy plugin or stub that makes assertIsNotNone narrow too, these could be unified. Not worth changing now.

Summary

No bugs, no security issues, no performance concerns. All changes are type-annotation hygiene with correct, behaviour-neutral mechanics. CI's backend job is the appropriate gate for runtime validation given the Docker constraint noted in the test plan. Ready to merge once CI is green.