Skip to content

Commit 456209a

Browse files
authored
Merge branch 'main' into claude/resolve-issue-1246-XUuxZ
Signed-off-by: JSIV <5049984+JSv4@users.noreply.github.com>
2 parents f2eaccf + 3b96d80 commit 456209a

170 files changed

Lines changed: 808 additions & 530 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: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- **Return-type annotations across `config/graphql/` resolvers and mutations** (Issue #1332, follow-up to #1331): The largest, least-typed subtree in the backend (459 function definitions, ~4.4% return-annotation coverage at baseline) is now at 100% return-annotation coverage. Touched files include every `*_mutations.py`, every `*_queries.py`, every `*_types.py`, plus `filters.py`, `base.py`, `base_types.py`, `security.py`, `optimized_file_resolvers.py`, `permissioning/permission_annotator/{middleware,mixins,utils}.py`, and the small utility modules. No behavioral changes — annotations only.
13+
- **`mutate(...)` on `graphene.Mutation` subclasses**: typed as forward references to the enclosing class (`-> "ClassName"`). Discovered and fixed the latent bug in `config/graphql/analysis_mutations.py:179` (`DeleteAnalysisMutation.mutate`) where the success path had no `return` statement; annotation is `-> "DeleteAnalysisMutation | None"` and an explicit `return None` was added to preserve the original implicit-None behavior on success.
14+
- **`resolve_*` methods**: typed as `-> Any` by default, refined where the GraphQL field type makes the runtime return obvious (e.g. `resolve_in_use -> bool`, `resolve_datacell_count -> int`).
15+
- **`AnnotatePermissionsForReadMixin`** (`config/graphql/permissioning/permission_annotator/mixins.py`): per the issue's specific guidance, `resolve_my_permissions -> list[str]`, `resolve_is_published -> bool`, `resolve_object_shared_with -> list[dict[str, Any]]`. The pre-existing wrong annotation `list[PermissionTypes]` (an Enum, while the implementation returns plain strings) was corrected to `list[str]`. The now-unused `PermissionTypes` import was removed.
16+
- **Filter / queryset helpers** (`filter_by_*`, `text_search_method`, `get_node`, `get_queryset`, `_get_*`, etc.) typed as `-> Any` to keep the change conservative; tightening to `QuerySet[Model]` is a follow-up.
17+
- **`config/graphql/permissioning/permission_annotator/utils.py`** had a broken import (`config.graphql.permission_annotator.middleware` instead of `config.graphql.permissioning.permission_annotator.middleware`) — fixed in passing.
18+
- **`config/graphql/conversation_types.py`**: replaced `base64.binascii.Error` with a direct `binascii.Error` import (pre-existing — `base64` re-exports `binascii` at runtime but `mypy` doesn't see the re-export).
19+
- **Var-annotated additions**: `id_to_children: dict[Any, list[Any]]` in `base_types.py`, `read_only_fields: list[str]` in `serializers.py`, `this_model_permission_id_map: dict[int, str]` etc. in middleware.
20+
- **Five modules graduated from the mypy baseline** (`mypy.ini` → no longer `ignore_errors = True`): `config.graphql.base_types`, `config.graphql.conversation_types`, `config.graphql.permissioning.permission_annotator.middleware`, `config.graphql.permissioning.permission_annotator.utils`, `config.graphql.serializers`. Their entries in `docs/typing/mypy_baseline.txt` (11 lines) were also pruned. Future PRs can graduate the remaining baselined files as the structural issues they expose (custom `visible_to_user` manager method not seen by `django-stubs`, `set_permissions_for_obj_to_user` signature mismatch, mixin `_meta` access) are addressed.
21+
- **Tooling**: zero new `# type: ignore` markers; black & isort applied; `flake8 config/graphql/` clean. `mypy --config-file mypy.ini opencontractserver config` passes with the updated baseline.
1222
- **Mypy: graduated `opencontractserver/users/tasks.py` out of the baseline** (Issue #1333 follow-up): `tasks.py` was the last `opencontractserver.users` module still suppressed in `mypy.ini`. PR #1370 left it untyped because the file is only loaded when `settings.USE_AUTH0=True`, so it never failed at runtime under the test settings; the typing gap kept the package short of the issue's "all four packages at ≥80% return-annotation coverage" Done-When criterion. Added return + parameter annotations to all five Auth0 sync tasks (`get_new_auth0_token`, `apply_data_to_user`, `sync_remote_user`, `ensure_valid_auth0_token`, `get_user_details_async`), introduced a module-level docstring documenting the `USE_AUTH0` gating, and removed the `[mypy-opencontractserver.users.tasks] ignore_errors = True` section. Local `data` rebound from request body (`dict[str, str]`) to response payload (`dict[str, Any]`) was split into two distinctly-named variables (`request_data` / `payload`) so the types are unambiguous; behavior is unchanged. No callers needed updating — `config/graphql_auth0_auth/utils.py` still consumes `sync_remote_user.delay(...)` exactly as before.
1323

1424
### Fixed
@@ -23,6 +33,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2333
- **Tests**`opencontractserver/tests/test_extraction_grounding.py`:
2434
- `TestGroundingPipelinePDFIntegration` (new class): builds a synthetic two-page PAWLS payload (no real PDF binary needed), runs grounding through `build_translation_layer`, and verifies (a) annotations land on the correct page, (b) re-running grounding is idempotent, and (c) when PlasmaPDF returns `page=None` the annotation is **skipped** instead of being saved on page 1.
2535
- `test_ground_text_document_is_idempotent`: regression for the duplicate-annotation bug on the SPAN_LABEL path.
36+
37+
- **`CreateCorpusActionModal` opened with the wrong default agent instructions for document triggers** (Issue #1385, `frontend/src/components/corpuses/CreateCorpusActionModal.tsx:136-144,168-171`): the `inlineAgentInstructions` state was initialised with `DEFAULT_MODERATOR_INSTRUCTIONS` even though the default trigger is `add_document` (a document trigger). The trigger-change handler at line 611 swaps to `DEFAULT_DOCUMENT_AGENT_INSTRUCTIONS`, but a user who created an inline agent on the default-selected trigger without first re-selecting the trigger would submit the moderator copy as the new agent's system instructions. Initialised both the `useState` default and `resetForm()` to `DEFAULT_DOCUMENT_AGENT_INSTRUCTIONS` so the pre-interaction value matches the default trigger. Updated `frontend/tests/CreateCorpusActionModal.ct.tsx` "inline-agent create: full happy path" mutation mock to expect `DEFAULT_DOCUMENT_AGENT_INSTRUCTIONS` — the previous mock variable masked this bug because `MockedProvider` was matching the stale moderator default rather than the trigger-appropriate one.
38+
39+
### Changed
40+
41+
- **Test/type cleanup follow-ups from the PR #1383 review** (Issue #1385):
42+
- Pinned the `isProcessing` contract for SYNC_CONTENT in `frontend/tests/CorpusChat.ct.tsx` "SYNC_CONTENT renders a complete message immediately": added an `expect(input).toBeEnabled()` assertion after the reply renders, locking the documented invariant that `setIsProcessing(true)` is owned solely by `ASYNC_START` and that a SYNC_CONTENT-only reply must never disable the input.
43+
- Consolidated the duplicated `::: oc-component` fence dispatcher: extracted `OcComponentBlock` interface and a new `buildOcComponentCustomBlocks(renderMarkdown)` helper into `frontend/src/utils/camlComponents.ts`. Both `frontend/src/hooks/useCamlComponentRenderer.tsx` and `frontend/src/components/corpuses/caml/CamlDirectiveRenderer.tsx` now share the same helper instead of each casting `block` independently.
44+
- Replaced `route: any` and `page: any` escape hatches with the proper `Route` and `Page` types from `@playwright/test` in `frontend/tests/CorpusDescriptionEditor.ct.tsx` (`setupMdRoute` and the abort-route test).
45+
- Migrated `.version-number` CSS-class locators in `frontend/tests/CorpusDescriptionEditor.ct.tsx` to a semantic `data-testid="version-number"` matcher (`page.getByTestId("version-number")`); added the test id to the rendered version-row in `frontend/src/components/corpuses/CorpusDescriptionEditor.tsx`.
46+
2647
- **`test_superuser_sees_all_queryset` miscounts personal corpuses by 1** (Issue #1394, `opencontractserver/tests/test_visibility_managers.py`, `opencontractserver/tests/test_resolvers.py`): Two `VisibleToUserTests.test_superuser_sees_all_queryset` cases asserted that `Corpus.objects.visible_to_user(superuser).count() == 4` (public + private + 2 personal), but the actual count is 5 because the test DB starts with a pre-existing personal corpus owned by django-guardian's `AnonymousUser` (created during fixture setup before/around the username-based skip in `opencontractserver/users/signals.py::user_created_signal`). The assertion is now scoped to corpuses created by the test's two users (`creator__in=[self.user, self.superuser]`), making it resilient to any fixture-level corpuses that exist at test DB init time. Production code is unchanged.
2748
- **Merged `frontend` Codecov flag drops to ~33% on every commit where Frontend CI's CT job fails** (`frontend/package.json` `test:coverage:ct`): the script chained `playwright test ... && mkdir -p ... && nyc report ...`, so a failing CT run short-circuited before `nyc report` could turn the per-test JSON files in `.nyc_output` into an `lcov.info`. The downstream `Upload CT Coverage to Codecov` step (`if: success() || failure()`) then errored with "No coverage reports found" and `frontend-component` did not upload for that SHA. Codecov's server-side aggregation of the `frontend` flag was left with only `frontend-unit` (~23%) and `frontend-e2e` (~24%), pulling the merged number down to ~33% even though the previous commit was at ~67% — observed on six consecutive main commits 2026-04-26T01:02..02:58Z (`2d7033f8`..`be5bcfc8`) before recovering on `30298391`. Mirrored the existing `test:e2e:coverage` pattern (`; CT_EXIT=$?; nyc report ... || echo "No coverage data to report"; exit $CT_EXIT`) so `nyc report` runs regardless of test outcome and the lcov ships even on red CT runs. `frontend-component` will still report a slightly lower number when tests fail (failed tests register fewer hits), but it will report — keeping the merged `frontend` flag's denominator stable.
2849
- **`User.__init__` shared-state mutation re-introduced by branch merge** (`opencontractserver/users/models.py:172-180` removed): PR #1374 (commit `50ed6740`) deleted the `User.__init__` override that mutated `Field.validators[0]` on every instantiation, but a subsequent merge (`b68c1cb4 → 6d2cddbf`) resurrected the override along with its mypy-narrowing changes. The current main on commit `6d2cddbf` therefore reproduced the original `#1358` bug: `User(...)` rebound `username_field.validators[0]` and clobbered any third-party validator prepended to the list. Removed the `__init__` override entirely; the class-body declaration `validators=[UserUnicodeUsernameValidator()]` on the `username` field (still present from PR #1374) is the canonical and only declaration. Also dropped the now-unused `Field` import. Regression coverage from PR #1374 (`opencontractserver/tests/test_user_username_validator.py`) was already on main and is what surfaced the regression in CI.
@@ -37,6 +58,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3758
- Regression coverage: `opencontractserver/tests/test_corpus_isolation_vector_store.py` — six tests covering cross-corpus leak, deletion-aware drop, orphan-set leak, document-scoped retrieval still returns structural rows, viewer-without-doc-permission excluded, creator still sees own row.
3859
- **Test-only**: `opencontractserver/tests/test_pydantic_ai_agents.py`, `opencontractserver/tests/test_structural_annotation_portability.py``Document.objects.create(...)` calls in `TransactionTestCase` setUp now pass `processing_started=timezone.now()` to short-circuit `process_doc_on_create_atomic`, which would otherwise eagerly chain a Celery PDF-ingest task that fails on the (file-less) test document and aborts the whole test class. Pre-existing failure, exposed cleanly when the regression suite was added.
3960

61+
### Fixed
62+
63+
- **`Embedding.embedder_path` could be NULL but was typed `str`** (Issue #1357, `opencontractserver/annotations/models.py:461-465`, `opencontractserver/annotations/models.py:584-585`, `opencontractserver/annotations/migrations/0068_enforce_embedder_path_not_null.py`): The Django field was declared `null=True, blank=True` while the Python annotation claimed `str`, causing a long-standing mypy `assignment` error and — more importantly — silently gutting the partial unique constraints added in migration 0059. Each `unique_embedding_per_{document,annotation,note,conversation,message}_embedder` constraint is conditioned on `<parent>__isnull=False` and keys on `(embedder_path, <parent>)`, so any row with `embedder_path IS NULL` bypassed duplicate prevention for its parent. Every production code path that creates an `Embedding` (`Embedding.objects.store_embedding()`, `HasEmbeddingMixin.add_embedding()`, `worker_uploads._store_embeddings()`) already supplies a concrete `embedder_path` or skips creation when empty, so enforcing non-null at the DB level matches actual behaviour rather than constraining it. New migration 0068 backfills any legacy NULL rows with `settings.DEFAULT_EMBEDDER` (deleting rows that would collide with an existing `(default_embedder_path, parent)` row under the partial unique constraint — they were previously unreachable via any query path since all call sites filter on a concrete embedder path), then `AlterField`s the column to `NOT NULL`. Removed the now-unreachable `or 'Unknown Model'` fallback in `Embedding.__str__`. Migration runs with `atomic = False` so the RunPython backfill commits before `AlterField` takes the `ACCESS EXCLUSIVE` lock to set `NOT NULL`, matching the pattern established by migration 0059.
64+
4065
### Added
4166

4267
- **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:

config/graphql/action_queries.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import logging
6+
from typing import Any
67

78
import graphene
89
from graphene_django.fields import DjangoConnectionField
@@ -33,7 +34,7 @@ class ActionQueryMixin:
3334
)
3435

3536
@login_required
36-
def resolve_corpus_action_templates(self, info, **kwargs):
37+
def resolve_corpus_action_templates(self, info, **kwargs) -> Any:
3738
"""Return available corpus action templates.
3839
3940
Templates are system-level and read-only — any authenticated user
@@ -58,7 +59,7 @@ def resolve_corpus_action_templates(self, info, **kwargs):
5859
)
5960

6061
@login_required
61-
def resolve_corpus_actions(self, info, **kwargs):
62+
def resolve_corpus_actions(self, info, **kwargs) -> Any:
6263
"""
6364
Resolver for corpus_actions that returns actions visible to the current user.
6465
Can be filtered by corpus_id, trigger type, and disabled status.
@@ -93,7 +94,7 @@ def resolve_corpus_actions(self, info, **kwargs):
9394
)
9495

9596
@login_required
96-
def resolve_agent_action_results(self, info, **kwargs):
97+
def resolve_agent_action_results(self, info, **kwargs) -> Any:
9798
"""
9899
Resolver for agent_action_results that returns results visible to the current user.
99100
Can be filtered by corpus_action_id, document_id, and status.
@@ -142,7 +143,7 @@ def resolve_agent_action_results(self, info, **kwargs):
142143
)
143144

144145
@login_required
145-
def resolve_corpus_action_executions(self, info, **kwargs):
146+
def resolve_corpus_action_executions(self, info, **kwargs) -> Any:
146147
"""
147148
Resolver for corpus_action_executions that returns executions visible to
148149
the current user.
@@ -220,7 +221,7 @@ def resolve_corpus_action_executions(self, info, **kwargs):
220221
)
221222

222223
@login_required
223-
def resolve_corpus_action_trail_stats(self, info, corpus_id, since=None):
224+
def resolve_corpus_action_trail_stats(self, info, corpus_id, since=None) -> Any:
224225
"""
225226
Resolver for corpus_action_trail_stats that returns aggregated statistics
226227
for corpus action executions.
@@ -291,7 +292,7 @@ def resolve_corpus_action_trail_stats(self, info, corpus_id, since=None):
291292
corpus_id=graphene.ID(required=False),
292293
)
293294

294-
def resolve_document_corpus_actions(self, info, document_id, corpus_id=None):
295+
def resolve_document_corpus_actions(self, info, document_id, corpus_id=None) -> Any:
295296
"""
296297
Resolve document actions (corpus actions, extracts, analysis rows) with proper
297298
permission filtering.

config/graphql/agent_mutations.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def mutate(
8080
avatar_url=None,
8181
corpus_id=None,
8282
is_public=True,
83-
):
83+
) -> "CreateAgentConfigurationMutation":
8484
user = info.context.user
8585

8686
try:
@@ -204,7 +204,7 @@ def mutate(
204204
avatar_url=None,
205205
is_active=None,
206206
is_public=None,
207-
):
207+
) -> "UpdateAgentConfigurationMutation":
208208
user = info.context.user
209209

210210
try:
@@ -282,7 +282,7 @@ class Arguments:
282282

283283
@login_required
284284
@graphql_ratelimit(rate=RateLimits.WRITE_LIGHT)
285-
def mutate(root, info, agent_id):
285+
def mutate(root, info, agent_id) -> "DeleteAgentConfigurationMutation":
286286
user = info.context.user
287287

288288
try:

config/graphql/agent_types.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""GraphQL type definitions for agent and action types."""
22

3+
from typing import Any
4+
35
import graphene
46
from graphene import relay
57
from graphene_django import DjangoObjectType
@@ -37,7 +39,7 @@ class Meta:
3739
"source_template__id": ["exact"],
3840
}
3941

40-
def resolve_pre_authorized_tools(self, info):
42+
def resolve_pre_authorized_tools(self, info) -> Any:
4143
"""Resolve pre_authorized_tools as a list of strings."""
4244
return self.pre_authorized_tools or []
4345

@@ -61,15 +63,15 @@ class Meta:
6163
"creator__id": ["exact"],
6264
}
6365

64-
def resolve_tools_executed(self, info):
66+
def resolve_tools_executed(self, info) -> Any:
6567
"""Resolve tools_executed as a list of JSON objects."""
6668
return self.tools_executed or []
6769

68-
def resolve_execution_metadata(self, info):
70+
def resolve_execution_metadata(self, info) -> Any:
6971
"""Resolve execution_metadata as JSON dict."""
7072
return self.execution_metadata or {}
7173

72-
def resolve_duration_seconds(self, info):
74+
def resolve_duration_seconds(self, info) -> Any:
7375
"""Resolve duration from the model property."""
7476
return self.duration_seconds
7577

@@ -100,19 +102,19 @@ class Meta:
100102
"creator__id": ["exact"],
101103
}
102104

103-
def resolve_duration_seconds(self, info):
105+
def resolve_duration_seconds(self, info) -> Any:
104106
"""Resolve duration from the model property."""
105107
return self.duration_seconds
106108

107-
def resolve_wait_time_seconds(self, info):
109+
def resolve_wait_time_seconds(self, info) -> Any:
108110
"""Resolve wait time from the model property."""
109111
return self.wait_time_seconds
110112

111-
def resolve_affected_objects(self, info):
113+
def resolve_affected_objects(self, info) -> Any:
112114
"""Resolve affected_objects as a list of JSON objects."""
113115
return self.affected_objects or []
114116

115-
def resolve_execution_metadata(self, info):
117+
def resolve_execution_metadata(self, info) -> Any:
116118
"""Resolve execution_metadata as JSON dict."""
117119
return self.execution_metadata or {}
118120

@@ -180,17 +182,17 @@ class Meta:
180182
"corpus": ["exact"],
181183
}
182184

183-
def resolve_mention_format(self, info):
185+
def resolve_mention_format(self, info) -> Any:
184186
"""Return the @ mention format for this agent."""
185187
if self.slug:
186188
return f"@agent:{self.slug}"
187189
return None
188190

189-
def resolve_available_tools(self, info):
191+
def resolve_available_tools(self, info) -> Any:
190192
"""Resolve available_tools as a list of strings, ensuring proper array type."""
191193
return self.available_tools if self.available_tools else []
192194

193-
def resolve_permission_required_tools(self, info):
195+
def resolve_permission_required_tools(self, info) -> Any:
194196
"""Resolve permission_required_tools as a list of strings, ensuring proper array type."""
195197
return self.permission_required_tools if self.permission_required_tools else []
196198

@@ -261,5 +263,5 @@ class Meta:
261263
"created",
262264
)
263265

264-
def resolve_pre_authorized_tools(self, info):
266+
def resolve_pre_authorized_tools(self, info) -> Any:
265267
return self.pre_authorized_tools or []

0 commit comments

Comments
 (0)