Preserve cited versions and add annotation review across document updates

[JSv4]

Summary

Implements the design in #2389. Document updates now preserve the exact version a citation refers to while exposing the current version separately. Human annotations on the preceding version receive an explicit, audited review workflow in the document viewer.

Changes

• Preserve citation targets and stamp permanent mention URLs with ?v=N; expose permission-checked cited/current version information.
• Default reference and document-relationship reads to active versions, with an explicit historical-reference option. Reconcile derived graph edges and carry user-authored document relationships forward while retaining their originals.
• Add annotation review decisions with Approve, Place, and Drop actions. Validate text/PDF placement on the server, enforce corpus/document permissions, and serialize decisions to prevent duplicate successors.
• Add desktop/mobile review panels, stale counts and annotation state indicators, cited/current reference links, and version-specific navigation. Failed or late placement responses cannot create false local annotations or affect a different document.
• Add the decision-table migration and backfill existing resolved mention URLs; update the GraphQL schema, documentation, screenshots, and changelog.

Annotation-to-annotation relationships remain attached to their historical evidence and are recreated manually after reviewing the successor annotations, as specified by the design.

Test plan

• Backend: targeted pytest suites cover versioning, review, enrichment, governance, permissions, relationship APIs, schema parity, and service-layer/frontend GraphQL architecture rules. The fresh-database run passed 191 tests, including migrations; the final corpus-reference/current-source regression run passed all 35 tests.
• Frontend: vitest run for annotation hooks/types and navigation — 182 tests passed.
• Browser: playwright test -c playwright-ct.config.ts for AnnotationVersionReview, DocumentVersionSelector, DocumentReferencesPanel, and VersionBadge — 33 tests passed. Review tests also passed with desktop/mobile layouts; screenshots inspected.
• tsc --noEmit -p frontend/tsconfig.json — passed.
• python manage.py makemigrations annotations --check --dry-run — no changes detected.
• pre-commit run --files <changed files> — passed, including full-project mypy. Commit-time frontend formatting/lint checks passed.
• git diff --check and merge simulation against current main — clean.

Checklist

• Tests pass locally for affected code
• Pre-commit checks pass for all changed files, including full-project mypy
• TypeScript compiles cleanly
• Changelog fragment added under changelog.d/
• Dependency review — no new dependencies

Contributor License Agreement

By submitting this pull request, you agree to license your contribution
under the project's Contributor License Agreement. First-time
contributors: a bot will comment on this PR asking you to confirm by
replying with a short sign phrase — no separate signup required.

[claude]

Review

This is a large, well-organized feature (versioning-aware annotation review + citation pinning). I traced the permission model, the review-state machine, and several frontend consumers in depth. Found one clear permission-bypass, a correctness gap in the review workflow across multiple version hops, and several smaller issues worth a look before merge.

🔴 Security: historical-document reads skip the corpus-linkage check, reintroducing a cross-corpus annotation leak

config/graphql/document_types.py:309

return AnnotationService.get_document_annotations(
    document_id=root.id,
    user=user,
    corpus_id=corpus_pk,
    ...
    check_current_version=root.is_current,   # <-- new
    context=info.context,
)

check_current_version gates a DocumentPath.objects.filter(document_id, corpus_id, is_current=True, is_deleted=False).exists() check inside AnnotationService.get_document_annotations (opencontractserver/annotations/services/annotation_service.py:294-306). That check exists precisely because _compute_effective_permissions (same file, ~line 49) only verifies MIN(document_permission, corpus_permission) — it never verifies the document actually has/had a path into that corpus. Two independent object-level grants (doc READ + corpus READ) would otherwise be sufficient to read annotations for any corpus_id.

Wiring check_current_version=root.is_current skips the guard whenever the document is a historical version (Document.is_current=False — the versioning-tree flag, distinct from DocumentPath.is_current). Since the top-level document(id:) query (config/graphql/document_queries.py:256) resolves any document by ID with no corpus scoping, a user with independent READ on a historical Document X and READ on an unrelated Corpus Y can query:

document(id: X) { allAnnotations(corpusId: Y) { edges { node { rawText } } } }

and get back annotations scoped to (X, Y) even though X was never (or is no longer) part of Y.

The same bypass propagates to relationships: opencontractserver/annotations/services/relationship_service.py:215-226 reuses get_document_annotations(..., check_current_version=False, ...) for the source/target-annotation prefetch whenever not document.is_current, with no corpus re-validation.

The new test suites (test_annotation_version_review.py, test_reference_versioning.py) only query historical versions against the corpus they actually belong to, so this gap isn't caught. Suggest requiring some DocumentPath linkage for historical documents too (drop is_current=True from the filter rather than skipping the whole check).

🟠 Correctness: review state becomes permanently stuck once a document is re-imported twice

state_for_annotation (opencontractserver/annotations/services/version_review.py:203-212) resolves the "next version" via:

child = Document.objects.visible_to_user(user, lightweight=True).filter(
    parent_id=annotation.document_id,
    path_records__corpus_id=annotation.corpus_id,
    path_records__is_deleted=False,
).order_by("id").first()

— no is_current filter on either the document or the path. _lock_review (:250-263), which actually gates carryForwardAnnotation/dropStaleAnnotation, requires the target document to be is_current=True and have an is_current=True, is_deleted=False path, and requires target_document.parent_id == annotation.document_id (i.e. an immediate child).

Concretely: doc v1 (corpus C) has human annotation X, unreviewed. Re-import → v2 (X still unreviewed). Re-import again → v3.

• versionState for X still resolves child = v2 (lowest-id match, no currency check) → renders STALE forever, since v2 is no longer current and no decision can ever be recorded against it.
• _previous() (:87-97) filters strictly by document.parent_id, so viewing v3's review panel lists only v2's annotations — X (which lives on v1) never appears there either.
• carryForwardAnnotation(annotationId: X, targetDocumentId: v2 or v3) both fail _lock_review's checks (v2 isn't current; v3's parent isn't v1).

X is stuck as "STALE" with no reachable UI path to review or dismiss it. Worth deciding whether multi-hop review is in scope for this PR (chain the review through each hop) or whether state_for_annotation/_scope at least need currency filters so the state reported matches what _lock_review will actually accept.

Related: _scope (:70-84) has the same gap — it accepts any non-deleted DocumentPath, including purely historical ones, so review()/stale_count() can operate on a document that's been removed from the corpus. Combined with staleAnnotationCount being declared non-null (Int! in config/graphql/document_types.py:1781), a mismatch here raises PermissionDenied and nulls out the entire document object in the GraphQL response (not just the count) rather than returning 0.

🟡 Frontend: citation link bypasses the app's URL safety gate

frontend/src/components/knowledge_base/document/DocumentReferencesPanel.tsx:278

<Link to={reference.sourceAnnotation.linkUrl}>cited v{reference.targetVersionNumber}</Link>

Every other link in this panel routes through openSafeUrl (line 516), which validates the scheme and opens external targets in a new tab with noopener,noreferrer. Annotation.link_url is allowed to be a full external http(s):// URL (see validate_link_url in opencontractserver/annotations/models.py:105, used for e.g. LAW citations to external sites), so this Link (react-router) can receive an absolute external URL. Clicking it navigates the current tab away — tearing down the whole SPA session — instead of getting the same-tab-preserving, new-tab-for-external treatment the rest of the app uses. Not an XSS vector (server-side validate_link_url still blocks non-http(s) schemes), but it's an inconsistency that should route through openSafeUrl like its neighbor.

🟡 Performance: manual placement re-parses the PAWLS file up to 3x per review action

carry_forward() (opencontractserver/annotations/services/version_review.py:349) always calls propose(annotation, document) — which loads+parses the PAWLS file and builds a PlasmaPDF translation layer — even when a manual placement is supplied and proposal is only used for the selected == proposal REAPPROVED/CORRECTED comparison. When placement is supplied, _manual_placement() (:273) then calls _load_placement_source again (:280, second full load; the resulting layer is unused for PDFs) and, for token-layer docs, does a third raw file open + json.load (:298) to get pages. This all happens while holding SELECT ... FOR UPDATE locks from _lock_review on the annotation and target document, so on a large PDF this also extends how long a review decision blocks other concurrent reviews on the same document version.

Other things worth a look (not independently deep-verified, but concrete enough to flag)

• relationship_service.py:215-230 — the historical-document Prefetch for relationship endpoints reuses get_document_annotations with analysis_id=None, which filters analysis__isnull=True. A manually-created relationship between two analyzer-produced annotations will render with empty sourceAnnotations/targetAnnotations once the document becomes historical, even though it renders fine while current.
• documents/versioning.py _carry_document_relationships filters corpus=corpus when carrying relationships forward, while the new current_endpoints gate (documents/services/relationships.py) also matches corpus__isnull=True rows gated on both endpoints being current — if any such corpus-null rows exist, they'd silently stop appearing after either endpoint is superseded, with no successor ever created.
• Frontend, AnnotationHooks.tsx (~line 275-366) — if a carryForwardAnnotation mutation fails validation (e.g., incompatible label chosen mid-flow) or its response is lost, pendingReview is never cleared, and every subsequent annotation drawn on that document is silently routed into the review mutation (and discarded on failure) instead of falling back to a normal local annotation.
• frontend/src/utils/navigationUtils.ts getDocumentVersionUrl (~line 359) rewrites the last path segment as the document slug without checking the route is actually a /d/... document route — if the knowledge-base modal is open while on a corpus route (it's a persisted, route-independent overlay), picking a version can rewrite the corpus slug instead.
• REVIEW_REFETCH_QUERIES includes "GetDocumentAnnotationsOnly" (frontend/src/graphql/annotationVersionReview.ts:131), but the only usage of that query is skip: true, so Apollo treats it as standby and this refetch never fires (plus a console warning on every review decision) — the UI stays correct today only because the mutation response is pushed into local state directly.

Nits

• Migration pin_existing_mention_links looks correct (batched iterator, guards on missing slugs, no-op reverse). Minor: since one annotation can have multiple CorpusReference rows, changed[ref.source_annotation_id] = ... keeps whichever is visited last from an unordered queryset, making the pinned ?v= nondeterministic for multi-reference annotations — probably fine since it mirrors existing behavior, just flagging.

Nice test coverage for the "happy path" versioning/review flows (schema parity, permissions, decision uniqueness under concurrent lock). Given the cross-corpus read bypass, I'd want that addressed before merge; the rest are good follow-ups.

[codecov]

Codecov Report

❌ Patch coverage is 89.13738% with 68 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...tractserver/annotations/services/version_review.py	91.55%	19 Missing ⚠️
...dge_base/document/AnnotationVersionReviewPanel.tsx	83.33%	9 Missing and 4 partials ⚠️
config/graphql/annotation_types.py	75.86%	7 Missing ⚠️
...er/enrichment/services/corpus_reference_service.py	85.10%	7 Missing ⚠️
...src/components/annotator/hooks/AnnotationHooks.tsx	73.91%	1 Missing and 5 partials ⚠️
config/graphql/annotation_version_review.py	92.45%	4 Missing ⚠️
...nowledge_base/document/DocumentReferencesPanel.tsx	91.17%	1 Missing and 2 partials ⚠️
config/graphql/corpus_types.py	50.00%	2 Missing ⚠️
config/graphql/document_types.py	81.81%	2 Missing ⚠️
config/graphql/annotation_queries.py	87.50%	1 Missing ⚠️
... and 4 more

📢 Thoughts on this report? Let us know!

[JSv4]

#2398 targets this branch with fixes for the review findings and the three CI failures (all three were the same graphql_relay import in the two new test modules — the package the strawberry migration removed, which is exactly what the test_graphql_dependencies guard is for). It also merges current main in.

Addressed there: the cross-corpus read on historical versions (the linkage check is now unconditional — structural annotations are shared via structural_set, so skipping it exposed a superseded document's parsed structure under any readable corpus); the stuck STALE state across two version hops (one _successor() definition now backs the badge, the count and both mutations, while recorded decisions stay visible as history); staleAnnotationCount nulling its own document through the non-null field; the triple PAWLS parse under the review locks; the cited-version link bypassing openSafeUrl; the pending-review placement surviving a failed mutation; getDocumentVersionUrl rewriting a corpus slug; and the standby GetDocumentAnnotationsOnly refetch.

Two findings I looked at and left alone:

• Historical relationship endpoints drop analyzer annotations. Not a regression. On main the default prefetch resolves through _get_queryset_AnnotationType → visible_to_user, which hides every annotation on a superseded version; the new Prefetch restores the human ones, and analyzer endpoints were already invisible there.
• pin_existing_mention_links nondeterminism for multi-reference annotations. Mirrors existing behaviour.

Each new test was verified to fail with its fix reverted.

---

Generated by Claude Code