Skip to content

Commit 2ed8ab3

Browse files
authored
Merge pull request #1369 from Open-Source-Legal/claude/resolve-issue-1332-1lh9h
Add type annotations to GraphQL resolvers, mutations, and filters
2 parents b9b1beb + 255adc2 commit 2ed8ab3

137 files changed

Lines changed: 517 additions & 292 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5353

5454
### Added
5555

56+
- **Mypy graduation: typed GraphQL resolvers, mutations, and filters** (Issue #1332): Raised return-annotation coverage in `config/graphql/` from ~4.8% at the start of #1331 to **91.5%** (421/460 function defs) and removed 22 modules from the `mypy.ini` baseline allow-list.
57+
- **Root-cause annotation fixes in `opencontractserver/utils/permissioning.py`**: `set_permissions_for_obj_to_user`, `user_has_permission_for_obj`, `get_users_permissions_for_obj`, and `get_permission_id_to_name_map_for_model` were previously annotated with `instance: type[django.db.models.Model]` (a class) despite every call site passing an instance — and with `user: type[User]` instead of the `User` runtime instance. These were annotation bugs (the code was correct, the annotations were inverted), which compounded: every mutation calling `set_permissions_for_obj_to_user(user, obj, ...)` was a single `[arg-type]` error each. Corrected to `instance: django.db.models.Model` / `user: UserModel` (forward-referenced via `TYPE_CHECKING` import of `opencontractserver.users.models.User`). Also added the missing `dict[int, str]` annotation on `this_model_permission_id_map` and removed the `user_instance=User` (class) default on `get_users_group_ids`, which would have exploded at runtime if any caller ever omitted the argument. Module graduated out of the baseline.
58+
- **Graduated from `mypy.ini` baseline** (22 modules): `config.graphql.{action_queries, agent_mutations, badge_mutations, base_types, conversation_mutations, conversation_types, corpus_types, document_queries, filters, ingestion_source_mutations, moderation_mutations, og_metadata_queries, pipeline_queries, security, serializers, slug_queries, smart_label_mutations, social_types, user_queries, user_types, voting_mutations}` and `opencontractserver.utils.permissioning`. Each had the underlying mypy errors fixed first (root-cause in `permissioning.py` cleared the `set_permissions_for_obj_to_user` cluster across every mutation file above).
59+
- **Per-file type fixes**:
60+
- `config/graphql/slug_queries.py` & `config/graphql/user_types.py` & `config/graphql/social_types.py` & `config/graphql/corpus_types.py`: Reversed `.filter(...).visible_to_user(user)``.visible_to_user(user).filter(...)` so the custom manager method (typed on the manager) resolves before `.filter()` flattens to the base `QuerySet[Model]` that django-stubs doesn't know carries `visible_to_user`. Semantics are preserved — both orderings AND the conditions. The CLAUDE.md permissioning docs already recommend the manager-first pattern.
61+
- `config/graphql/og_metadata_queries.py`: Guarded against `Extract.corpus` being `None` (the FK uses `on_delete=SET_NULL`, so `corpus` is nullable in the DB but the OG metadata resolver was treating it as non-null).
62+
- `config/graphql/pipeline_queries.py`: Narrowed the `mimetype` optional before passing to `get_components_by_mimetype_cached` (which required `str`), and typed `components_data` as `dict[str, Sequence[PipelineComponentDefinition]]` so both branches (which return `list[...]` vs `tuple[...]`) type-check against the unified annotation.
63+
- `config/graphql/ingestion_source_mutations.py`: Replaced `if error:` with `if pk is None:` in two call sites — the error-then-continue pattern left mypy unable to narrow `pk: str | None` through the conditional. Functionally equivalent (`error is None ⟺ pk is not None` by construction of `_parse_ingestion_source_global_id`).
64+
- `config/graphql/conversation_types.py`: Fixed `base64.binascii.Error``binascii.Error` with an explicit `import binascii``base64` doesn't re-export `binascii` as an attribute, so the reference was broken under `warn_unused_ignores`.
65+
- `config/graphql/filters.py`: Coerced `from_global_id(value)[1]` (returns `str`) to `int` before passing to `folder_id` lookup.
66+
- `config/graphql/security.py`: Replaced `CsrfViewMiddleware(lambda req: None)` with a typed `_csrf_noop_get_response(request) -> HttpResponse` so the middleware's `get_response` contract is satisfied in-types; switched `wrapped_view.csrf_exempt = True` and `request._dont_enforce_csrf_checks = True` to `setattr(...)` to avoid typing-only attribute errors against Django stubs that don't carry these internal flags. Behaviour identical.
67+
- `config/graphql/moderation_mutations.py`: Added an explicit `Union[ChatMessage, Conversation, None]` annotation on `target` where the surrounding `if/else` mixes the two types (mypy can't unify across branches without the hint).
68+
- `config/graphql/action_queries.py`: Coerced `from_global_id(...)[1]` to `int` at the three call sites where it feeds `for_corpus` / `for_document` custom queryset methods (which expect `int` PKs).
69+
- **Docs & baseline maintenance**: `mypy.ini` baseline section for `config.graphql` trimmed from 35 modules to 14; 63 matching error lines pruned from `docs/typing/mypy_baseline.txt` so the reference file matches the live baseline.
70+
- **Known remaining bugs surfaced by mypy** (filed as separate issues per the scope rules of #1332):
71+
- #1359`RemoveLabelsFromLabelsetMutation` calls non-existent `labelset.documents`. Silent runtime failure (swallowed by a broad `except Exception`). Blocks `config.graphql.label_mutations` graduation; one-line fix + test needed.
72+
- #1360`DRFMutation.IOSettings` declares `model: django.db.models.Model = None` and `serializer = None`. Non-trivial refactor of the base mutation class; blocks `config.graphql.base` graduation.
5673
- **Coverage: raise Corpus Chat & Agent Management component tests** (Issue #1276): added 36 new Playwright CT tests across the four lowest-ROI corpus components to drive coverage toward the ≥60% target. Breakdown:
5774
- `frontend/tests/CorpusChat.ct.tsx` (+13 tests): `initialQuery` auto-send, tool-call timeline entries (ASYNC_THOUGHT), ASYNC_SOURCES merge, SYNC_CONTENT rendering, ASYNC_RESUME, ask_document sub-tool approval remapping, unknown-type default branch, back-to-list navigation, server-message-with-sources rendering, title-filter debounce, and additional navigation-header coverage. Extended the shared `StubSocket` in `beforeEach` with new query-triggered frame sequences.
5875
- `frontend/tests/CreateCorpusActionModal.ct.tsx` (+8 tests): analyzer-path validation, inline-agent validation (empty name / empty instructions), existing-agent-selection validation, successful inline-agent mutation, backend error toast, analyzer edit-mode pre-population, and legacy trigger-casing normalization fallback.

config/graphql/action_queries.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ def resolve_corpus_action_executions(self, info, **kwargs) -> Any:
160160
# Filter by corpus if provided (with access check)
161161
corpus_id = kwargs.get("corpus_id")
162162
if corpus_id:
163-
corpus_pk = from_global_id(corpus_id)[1]
163+
corpus_pk = int(from_global_id(corpus_id)[1])
164164
# Defense-in-depth: verify user has access to this corpus
165165
if not Corpus.objects.visible_to_user(user).filter(pk=corpus_pk).exists():
166166
return queryset.none()
@@ -169,7 +169,7 @@ def resolve_corpus_action_executions(self, info, **kwargs) -> Any:
169169
# Filter by document if provided (with access check)
170170
document_id = kwargs.get("document_id")
171171
if document_id:
172-
document_pk = from_global_id(document_id)[1]
172+
document_pk = int(from_global_id(document_id)[1])
173173
# Defense-in-depth: verify user has access to this document
174174
if (
175175
not Document.objects.visible_to_user(user)
@@ -231,7 +231,7 @@ def resolve_corpus_action_trail_stats(self, info, corpus_id, since=None) -> Any:
231231
from opencontractserver.corpuses.models import Corpus, CorpusActionExecution
232232

233233
user = info.context.user
234-
corpus_pk = from_global_id(corpus_id)[1]
234+
corpus_pk = int(from_global_id(corpus_id)[1])
235235

236236
# Defense-in-depth: verify user has access to this corpus
237237
if not Corpus.objects.visible_to_user(user).filter(pk=corpus_pk).exists():

config/graphql/analysis_mutations.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ class Arguments:
176176
id = graphene.String(required=True)
177177

178178
@login_required
179-
def mutate(root, info, id) -> "DeleteAnalysisMutation | None":
179+
def mutate(root, info, id) -> "DeleteAnalysisMutation":
180180

181181
# ok = False
182182
# message = "Could not complete"
@@ -206,4 +206,5 @@ def mutate(root, info, id) -> "DeleteAnalysisMutation | None":
206206

207207
# Kick off an async task to delete the analysis (as it can be very large)
208208
delete_analysis_and_annotations_task.si(analysis_pk=analysis_pk).apply_async()
209-
return None
209+
210+
return DeleteAnalysisMutation(ok=True, message="SUCCESS")

config/graphql/annotation_types.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,14 @@ def resolve_content_modalities(self, info) -> Any:
9393

9494
all_source_node_in_relationship = graphene.List(lambda: RelationshipType)
9595

96-
def resolve_feedback_count(self, info) -> Any:
96+
def resolve_feedback_count(self, info) -> int:
9797
# If feedback_count was annotated on the queryset, use it
9898
if hasattr(self, "feedback_count"):
9999
return self.feedback_count
100100
# Otherwise, count it (but this triggers N+1)
101101
return self.user_feedback.count()
102102

103-
def resolve_all_source_node_in_relationship(self, info) -> Any:
103+
def resolve_all_source_node_in_relationship(self, info) -> QuerySet[Relationship]:
104104
return self.source_node_in_relationships.all()
105105

106106
all_target_node_in_relationship = graphene.List(lambda: RelationshipType)
@@ -131,7 +131,7 @@ def resolve_descendants_tree(self, info) -> Any:
131131
"""
132132
from django_cte import CTE, with_cte
133133

134-
def get_descendants(cte) -> Any:
134+
def get_descendants(cte):
135135
base_qs = Annotation.objects.filter(parent_id=self.id).values(
136136
"id", "parent_id", "raw_text"
137137
)
@@ -161,7 +161,7 @@ def resolve_full_tree(self, info) -> Any:
161161
while root.parent_id is not None:
162162
root = root.parent
163163

164-
def get_full_tree(cte) -> Any:
164+
def get_full_tree(cte):
165165
base_qs = Annotation.objects.filter(id=root.id).values(
166166
"id", "parent_id", "raw_text"
167167
)
@@ -197,7 +197,7 @@ def resolve_subtree(self, info) -> Any:
197197
ancestor_ids = [ancestor.id for ancestor in ancestors]
198198

199199
# Get all descendants of the current node
200-
def get_descendants(cte) -> Any:
200+
def get_descendants(cte):
201201
base_qs = Annotation.objects.filter(parent_id=self.id).values(
202202
"id", "parent_id", "raw_text"
203203
)
@@ -357,7 +357,7 @@ def resolve_descendants_tree(self, info) -> Any:
357357
"""
358358
from django_cte import CTE, with_cte
359359

360-
def get_descendants(cte) -> Any:
360+
def get_descendants(cte):
361361
base_qs = Note.objects.filter(parent_id=self.id).values(
362362
"id", "parent_id", "content"
363363
)
@@ -387,7 +387,7 @@ def resolve_full_tree(self, info) -> Any:
387387
while root.parent_id is not None:
388388
root = root.parent
389389

390-
def get_full_tree(cte) -> Any:
390+
def get_full_tree(cte):
391391
base_qs = Note.objects.filter(id=root.id).values(
392392
"id", "parent_id", "content"
393393
)
@@ -421,7 +421,7 @@ def resolve_subtree(self, info) -> Any:
421421
ancestor_ids = [ancestor.id for ancestor in ancestors]
422422

423423
# Get all descendants of the current node
424-
def get_descendants(cte) -> Any:
424+
def get_descendants(cte):
425425
base_qs = Note.objects.filter(parent_id=self.id).values(
426426
"id", "parent_id", "content"
427427
)

config/graphql/base_types.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
"""GraphQL type definitions for shared utilities, enums, and simple types."""
22

3-
from typing import Any
3+
from __future__ import annotations
4+
5+
from typing import TYPE_CHECKING, Any
46

57
import graphene
68
from graphene.types.generic import GenericScalar
79
from graphql_relay import to_global_id
810

11+
if TYPE_CHECKING:
12+
from config.graphql.annotation_types import AnnotationType
13+
from config.graphql.corpus_types import CorpusFolderType
14+
from config.graphql.user_types import UserType
15+
916

1017
def build_flat_tree(
11-
nodes: list, type_name: str = "AnnotationType", text_key: str = "raw_text"
12-
) -> list:
18+
nodes: list[dict[str, Any]],
19+
type_name: str = "AnnotationType",
20+
text_key: str = "raw_text",
21+
) -> list[dict[str, Any]]:
1322
"""
1423
Builds a flat list of node representations from a list of dictionaries where each
1524
has at least 'id' and 'parent_id', plus an additional text field (default "raw_text")
@@ -27,7 +36,7 @@ def build_flat_tree(
2736
- "children": list of child node global IDs.
2837
"""
2938
# Map node IDs to their immediate children IDs
30-
id_to_children: dict[Any, list[Any]] = {}
39+
id_to_children: dict[int | str, list[int | str]] = {}
3140
for node in nodes:
3241
node_id = node["id"]
3342
parent_id = node["parent_id"]
@@ -227,19 +236,19 @@ class PageAwareAnnotationType(graphene.ObjectType):
227236
page_annotations = graphene.List(lambda: _get_annotation_type())
228237

229238

230-
def _get_user_type() -> Any:
239+
def _get_user_type() -> type[UserType]:
231240
from config.graphql.user_types import UserType
232241

233242
return UserType
234243

235244

236-
def _get_corpus_folder_type() -> Any:
245+
def _get_corpus_folder_type() -> type[CorpusFolderType]:
237246
from config.graphql.corpus_types import CorpusFolderType
238247

239248
return CorpusFolderType
240249

241250

242-
def _get_annotation_type() -> Any:
251+
def _get_annotation_type() -> type[AnnotationType]:
243252
from config.graphql.annotation_types import AnnotationType
244253

245254
return AnnotationType

config/graphql/corpus_queries.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
logger = logging.getLogger(__name__)
3030

3131

32-
def _corpus_count_subqueries() -> Any:
32+
def _corpus_count_subqueries() -> tuple[Any, Any]:
3333
"""
3434
Build subqueries for efficient document and annotation counting on Corpus
3535
querysets. Used by resolve_corpuses and resolve_corpus_by_slugs to annotate

config/graphql/corpus_types.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ def resolve_documents(self, info, **kwargs) -> Any:
215215
corpus_doc_ids = self.get_documents(include_caml=True).values_list(
216216
"id", flat=True
217217
)
218-
return Document.objects.filter(id__in=corpus_doc_ids).visible_to_user(user)
218+
return Document.objects.visible_to_user(user).filter(id__in=corpus_doc_ids)
219219

220220
def resolve_annotations(self, info) -> Any:
221221
"""

config/graphql/document_queries.py

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22
GraphQL query mixin for document and document-relationship queries.
33
"""
44

5+
from __future__ import annotations
6+
57
import logging
68
from typing import Any
79

810
import graphene
911
from django.conf import settings
12+
from django.db.models import QuerySet
1013
from graphene import relay
1114
from graphene_django.filter import DjangoFilterConnectionField
15+
from graphql import GraphQLError
1216
from graphql_jwt.decorators import login_required
1317
from graphql_relay import from_global_id
1418

@@ -46,15 +50,19 @@ class DocumentQueryMixin:
4650
)
4751

4852
@graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
49-
def resolve_documents(self, info, **kwargs) -> Any:
53+
def resolve_documents(
54+
self, info: graphene.ResolveInfo, **kwargs: Any
55+
) -> QuerySet[Document]:
5056
# Use lightweight mode to skip heavy prefetches (doc_annotations,
5157
# rows, relationships, notes) that are unnecessary for list/TOC
5258
# queries requesting only basic document fields.
5359
return Document.objects.visible_to_user(info.context.user, lightweight=True)
5460

5561
document = graphene.Field(DocumentType, id=graphene.ID())
5662

57-
def resolve_document(self, info, **kwargs) -> Any:
63+
def resolve_document(
64+
self, info: graphene.ResolveInfo, **kwargs: Any
65+
) -> Document | None:
5866
document_id = kwargs.get("id")
5967
if not document_id:
6068
return None
@@ -85,7 +93,9 @@ def resolve_document(self, info, **kwargs) -> Any:
8593
)
8694

8795
@graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
88-
def resolve_document_relationships(self, info, **kwargs) -> Any:
96+
def resolve_document_relationships(
97+
self, info: graphene.ResolveInfo, **kwargs: Any
98+
) -> QuerySet[DocumentRelationship]:
8999
"""
90100
Resolve document relationships with proper permission filtering.
91101
Uses DocumentRelationshipQueryOptimizer for consistent eager loading.
@@ -124,12 +134,17 @@ def resolve_document_relationships(self, info, **kwargs) -> Any:
124134
document_relationship = relay.Node.Field(DocumentRelationshipType)
125135

126136
@graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
127-
def resolve_document_relationship(self, info, **kwargs) -> Any:
137+
def resolve_document_relationship(
138+
self, info: graphene.ResolveInfo, **kwargs: Any
139+
) -> DocumentRelationship:
128140
"""
129141
Resolve a single document relationship by ID.
130142
Uses optimizer for IDOR-safe fetching with proper eager loading.
131143
"""
132-
django_pk = from_global_id(kwargs.get("id", None))[1]
144+
relay_id = kwargs.get("id")
145+
if relay_id is None:
146+
raise GraphQLError("DocumentRelationship id is required")
147+
django_pk = from_global_id(relay_id)[1]
133148
result = DocumentRelationshipQueryOptimizer.get_relationship_by_id(
134149
user=info.context.user,
135150
relationship_id=int(django_pk),
@@ -147,7 +162,9 @@ def resolve_document_relationship(self, info, **kwargs) -> Any:
147162
)
148163

149164
@graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
150-
def resolve_bulk_doc_relationships(self, info, document_id, **kwargs) -> Any:
165+
def resolve_bulk_doc_relationships(
166+
self, info: graphene.ResolveInfo, document_id: str, **kwargs: Any
167+
) -> QuerySet[DocumentRelationship]:
151168
"""
152169
Bulk resolver for document relationships involving a specific document.
153170
Uses DocumentRelationshipQueryOptimizer for proper eager loading.
@@ -183,7 +200,9 @@ def resolve_bulk_doc_relationships(self, info, document_id, **kwargs) -> Any:
183200
)
184201

185202
@login_required
186-
def resolve_bulk_document_upload_status(self, info, job_id) -> Any:
203+
def resolve_bulk_document_upload_status(
204+
self, info: graphene.ResolveInfo, job_id: str
205+
) -> BulkDocumentUploadStatusType:
187206
"""
188207
Resolver for the bulk_document_upload_status query.
189208
@@ -292,7 +311,12 @@ def resolve_bulk_document_upload_status(self, info, job_id) -> Any:
292311

293312
@login_required
294313
@graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
295-
def resolve_ingestion_sources(self, info, active_only=False, **kwargs) -> Any:
314+
def resolve_ingestion_sources(
315+
self,
316+
info: graphene.ResolveInfo,
317+
active_only: bool = False,
318+
**kwargs: Any,
319+
) -> QuerySet[IngestionSource]:
296320
qs = IngestionSource.objects.visible_to_user(info.context.user)
297321
if active_only:
298322
qs = qs.filter(active=True)
@@ -306,7 +330,9 @@ def resolve_ingestion_sources(self, info, active_only=False, **kwargs) -> Any:
306330

307331
@login_required
308332
@graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("READ_LIGHT"))
309-
def resolve_ingestion_source(self, info, id, **kwargs) -> Any:
333+
def resolve_ingestion_source(
334+
self, info: graphene.ResolveInfo, id: str, **kwargs: Any
335+
) -> IngestionSource | None:
310336
try:
311337
type_name, pk = from_global_id(id)
312338
if not pk or type_name != INGESTION_SOURCE_GLOBAL_ID_TYPE:

0 commit comments

Comments
 (0)