Reference web UI: Governance Graph panel, one-click bootstrap, References side panel

[JSv4]

What this is

The user-facing surface of the reference web — items 2, 3 and 4 from #1976's integration callout, stacked on feature/corpus-reference-enrichment (the governanceGraph query + CorpusReference substrate it consumes).

1. Governance Graph panel — the reference web, rendered

A bipartite composition that makes the system's pitch literal: the filings above, the law below.

• Filing documents settle into an upper band under a deterministic d3-force layout (seeded LCG, fixed ticks, no animation loop — same testability contract as DocumentGraphGlimpse). Connected components over DOCUMENT edges drive per-cluster hues, so each primary + its exhibits reads as one constellation; swarm-sized clusters spread across the full band instead of packing into a disc.
• Statute sections pin to a law shelf: authorities in canonical order, sections in numeric order, staggered citation-head labels (DGCL § 145), authority captions beneath. Citations without an in-system target render as dashed ghost nodes.
• Mention-weighted amber arcs sweep from filings to the exact section of the governing law; dashed slate for unresolved. Hover focuses a node's incident edges; clicking any document or statute node navigates to its canonical page — across corpora (statutes land in their authority corpus).
• Scale-aware density handling (degree-gated shelf labels, thinned edges, abbreviated captions) keeps the 200-node backend cap legible — tuned live against a 134-document S-1 corpus (168 docs / 38 sections / 794 references on screen).
• Ships as the governance-graph CAML embed and in the CorpusIntelligenceOverview fallback, so both landing paths get it.

2. One-click bootstrap (CorpusAction auto-wiring)

The panel's empty state is the affordance: "Map the reference web" discovers the enrichment analyzer by task name, starts an immediate corpus analysis, installs an add_document CorpusAction so the web grows as documents arrive, then polls ("Weaving the reference web…") until the first nodes land. Proven end-to-end live: click → Celery corpus run → woven DGCL self-citation graph + installed action.

3. References side panel — citation → statute reading experience

A new References tab in the document viewer: Cites (outbound, grouped by canonical key with mention counts — "SEC Rule § 144 ×26" — ghosts annotated cited, not yet ingested) and Cited by (inbound, grouped by source document). Resolved rows navigate to the cited statute/exhibit text; inbound rows resolve the source document by id via a shared useNavigateToDocumentById hook. Proven live: Fervo S-1 → 71 grouped citations → click → Rule 144's actual section text.

Backend

• corpusReferences(documentId:) — either-side filter (source mention's document OR resolved target), the single-fetch shape the panel needs. 3 new tests.
• Fixed a governanceGraph crash on real data: .only() vs the Document manager's baked-in select_related("parent") → values_list (governance_graph_service.py). Unit tests missed it; live smoke caught it.

Test plan

• 6 new Playwright CT tests (GovernanceGraph.ct.tsx, DocumentReferencesPanel.ct.tsx) + docScreenshots; 2,473 unit tests green; tsc + lint green.
• 38 backend tests green across governance-graph/enrichment suites; pre-commit incl. mypy green; E001 clean.
• Live smoke on dev DB: embed render on the S-1 corpus, CTA flow on DGCL (Celery weave verified), References panel + both navigation paths.

Notes / follow-ups

• Mobile: the References tab is desktop-first; the mobile "More" sheet integration is a small follow-up.
• The fuller "Explore the full graph" view (zoom/pan, global cross-corpus variant) remains future work; the onExplore escape hatch is wired.

[claude]

Code Review: Reference Web UI (#1977)

This is a well-scoped, well-documented PR. The three-feature bundle (Governance Graph panel, one-click bootstrap CTA, References side panel) plus the backend bugfix are all clearly motivated and tested. The overall design follows established patterns in the codebase. Below are the issues worth addressing before merge, ordered by severity.

---

Bugs / Correctness

1. primaries is recomputed on every render, silently invalidating labeledIds

GovernanceGraphGlimpse.tsx (~line 680):

// This is computed outside useMemo — new reference every render
const primaries = simNodes.filter((n) => n.kind === KINDS.PRIMARY);

const labeledIds = useMemo(() => {
  ...
}, [simNodes, primaries, dense, shelfMaxDegree]); // primaries is always "new"

primaries is a new array reference on every render, so labeledIds recomputes on every render, undoing the memo. Fix: wrap primaries in its own useMemo or inline the filter inside labeledIds's memo.

2. nodeEmphasis is O(nodes × edges) per rendered frame on hover

GovernanceGraphGlimpse.tsx:

const nodeEmphasis = (n: SimNode): number => {
  if (!hoveredId) return 1;
  if (n.id === hoveredId) return 1;
  const incident = edges.some(   // ← scans ALL edges for EVERY node, EVERY render
    (e) => (e.source === hoveredId && e.target === n.id) || ...
  );
  return incident ? 1 : 0.35;
};

With up to 200 nodes and O(200²) edges this is hot during hover. Precompute the incident set when hoveredId changes:

const incidentNodes = useMemo(() => {
  if (!hoveredId) return null;
  const set = new Set<string>();
  edges.forEach((e) => {
    if (e.source === hoveredId) set.add(e.target);
    if (e.target === hoveredId) set.add(e.source);
  });
  return set;
}, [hoveredId, edges]);

Then nodeEmphasis becomes an O(1) set lookup. Same issue applies to edgeEmphasis.

---

Type Safety

3. as any casts in useNavigateToDocumentById.ts

const path = buildCanonicalPath(doc as any, (doc as any).corpus);

as any silently bypasses TypeScript. The GraphQL query response type for document should have slugs / corpus slugs — if the GET_DOCUMENT_BY_ID_FOR_REDIRECT output type already defines these, use a proper cast or helper. If the fields are missing from the type, add them.

---

Minor / Nits

4. Redundant fallback in DocumentReferencesPanel.tsx

variables: { corpusId: corpusId || "", documentId },
skip: !corpusId,

The || "" is dead code — the query never executes when corpusId is falsy because skip: !corpusId short-circuits it. Remove the fallback for clarity.

5. GraphQL/TypeScript type mismatch for documentId

The GraphQL query declares $documentId: ID (optional), but the TS interface GetCorpusReferencesForDocumentInputType has documentId: string (required). Either make the TS field documentId?: string to match the schema, or mark the field required in the schema if it will always be provided by this caller.

6. Polling not explicitly stopped on unmount

In GovernanceGraphLive.tsx, startPolling is called on bootstrap but there is no explicit stopPolling() in a cleanup effect. Apollo's useQuery should clean up on unmount, but it's worth being explicit given the polling state is tracked in local state (weaving):

useEffect(() => {
  return () => { stopPolling(); };   // belt-and-suspenders
}, [stopPolling]);

7. GovernanceGraphGlimpse.tsx is 1 059 lines

The component is functional and well-commented, but three natural seams exist for extraction:

• computeLayout (pure function, could move to a layoutUtils file alongside similar graph helpers)
• The label-formatting helpers (shortLawLabel, ghostLabel, shortFilingLabel) — these are shared-ish with the display panel
• The empty/loading/error states

Not a blocker, but the surface area makes future diffs harder to reason about.

---

What's Well-Done

• .only() → values_list() fix is the correct fix for the Django manager/defer conflict and is well-documented in the changelog fragment.
• Deterministic layout (seeded LCG, synchronous ticks) follows the existing DocumentGraphGlimpse contract and is good for test stability.
• EMPTY_NODES / EMPTY_EDGES stable references in GovernanceGraphLive correctly prevent the layout memo from invalidating on loading frames — same pattern as DocumentGraphLive.
• Bootstrap error handling: the two-tier try/catch (outer for the run, inner for corpus action install) with differentiated toast messages is the right UX.
• Test coverage: the 3 backend tests for the either-side documentId filter (source, target, unrelated), the 3 GovernanceGraphLive CT tests (populated, empty+CTA, skeleton), and the 3 DocumentReferencesPanel CT tests all hit the meaningful cases cleanly.
• Changelog fragments follow the project convention correctly.
• useNavigateToDocumentById is properly extracted as a shared hook and documented.

[codecov]

Codecov Report

❌ Patch coverage is 84.76190% with 112 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...CorpusHome/intelligence/GovernanceGraphGlimpse.tsx	76.98%	5 Missing and 53 partials ⚠️
..._base/document/document_kb/useReferenceMentions.ts	58.97%	11 Missing and 5 partials ⚠️
...es/CorpusHome/intelligence/GovernanceGraphLive.tsx	81.15%	11 Missing and 2 partials ⚠️
...nowledge_base/document/DocumentReferencesPanel.tsx	85.54%	4 Missing and 8 partials ⚠️
.../CorpusHome/intelligence/WantedAuthoritiesCard.tsx	81.48%	2 Missing and 3 partials ⚠️
...ge_base/document/document_kb/RightPanelContent.tsx	0.00%	1 Missing and 1 partial ⚠️
...tserver/llms/tools/core_tools/corpus_references.py	90.00%	2 Missing ⚠️
...nd/src/components/annotator/utils/urlAnnotation.ts	66.66%	0 Missing and 1 partial ⚠️
frontend/src/hooks/useNavigateToDocumentById.ts	87.50%	0 Missing and 1 partial ⚠️
opencontractserver/annotations/models.py	90.00%	1 Missing ⚠️
... and 1 more

📢 Thoughts on this report? Let us know!

[claude]

test comment - ignore

[claude]

placeholder - will be replaced

[claude]

placeholder2

[claude]

test comment

[claude]

Code Review

Overview

This PR ships the user-facing surface of the reference web: the bipartite d3-force Governance Graph panel, a one-click bootstrap CTA, a References side panel in the document viewer, the WantedAuthorities backlog, and backend plumbing (authority bootstrapper, reactive relink, link-URL restamping). Architecture is sound, the DRY cleanup (shared graphCardChrome.ts, graphLayout.ts) is a genuine improvement, and test coverage is solid. A few issues are worth fixing before merge.

---

Issue 1 — DRY violation: displayKey duplicates formatCanonicalLawKey

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

A local displayKey function is functionally identical to formatCanonicalLawKey added to frontend/src/utils/formatters.ts in this very PR. The local version also has a subtle correctness gap: it checks part === 'sec' separately rather than delegating to LAW_ACRONYM_SET — any future addition to LAW_AUTHORITY_ACRONYMS would be silently missed in this panel.

Fix: delete displayKey and call formatCanonicalLawKey instead.

---

Issue 2 — Security: outbound reference navigation bypasses URL safety check

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

The outbound-row click handler calls navigate(group.linkUrl!) with the raw database value. The annotation viewers use openAnnotationUrl from urlAnnotation.ts, which gates on isSafeUrl (rejects javascript:, data:, etc.) before following any link. React Router will not execute javascript: as code, but the pattern is inconsistent with every other link-following path in the codebase.

Fix: use openAnnotationUrl(group.linkUrl!, navigate) for consistency and defense in depth.

---

Issue 3 — Unhandled async errors in useReferenceMentions

File: frontend/src/components/knowledge_base/document/document_kb/useReferenceMentions.ts

The inner IIFE has no try/catch. If discoverAnalyses or fetchMentions rejects (network error, cancelled Apollo client, etc.), the unhandled rejection propagates to the global error handler and causes visible warnings or crashes in React 18 strict mode.

Fix: wrap the IIFE body in try/catch with a silent-failure strategy — reference mentions are a best-effort UI enhancement, not load-bearing.

---

Issue 4 — 'as any' type casts in useNavigateToDocumentById

File: frontend/src/hooks/useNavigateToDocumentById.ts

Two 'as any' casts erase the type returned by GetDocumentByIdForRedirectOutput. If there is a genuine shape mismatch with buildCanonicalPath's signature, the mismatch should be resolved in the types or in buildCanonicalPath rather than suppressed at the call site.

---

Issue 5 — O(n*m) hover scan in GovernanceGraphGlimpse

File: frontend/src/components/corpuses/CorpusHome/intelligence/GovernanceGraphGlimpse.tsx

nodeEmphasis is called once per node on every render, and each call does a full linear scan of the edge list to find incident edges. At the 200-node cap, every onMouseEnter triggers O(n*m) work. A memoised Set of nodes adjacent to the hovered node (recomputed only when hoveredId changes) would reduce per-render cost to O(1) per node. Not a blocker, but the component already has scale-awareness logic so this fits naturally.

---

Minor Notes

• isUrlAnnotation contract change is well-justified (link_url is the navigability contract, not the label) and the test suite update captures the new semantics correctly. The comment cleanly separates URL-authoring affordances from link-following. Good.
• Bootstrap flow early returns: return inside try correctly allows finally to run, so bootstrapping always resets. Good.
• Service layer: both new GraphQL resolvers route through CorpusReferenceService; E001 should stay clean. Good.
• Changelog fragments: correct format, properly filed under changelog.d/. Good.
• reference-web-versioning.md: good practice documenting the stale-resolved-ref gap as a deliberate deferral rather than hiding it. Good.
• relink_corpora_for_keys runs synchronously inside bootstrap_authority_corpus. Fine for the management command (no HTTP timeout), but will need Celery if this ever gets called from a mutation. Worth tracking.

---

Summary

Three must-fix items before merge: (1) the displayKey duplication (DRY violation with a real correctness gap), (2) the URL safety check bypass in the References panel, and (3) missing error handling in useReferenceMentions. The 'as any' casts are a lower-priority type-safety cleanup. Everything else is either already handled well or a documented future-work item.

[claude]

Code Review

This is a well-architected, well-tested PR. The bipartite governance graph, one-click bootstrap flow, and document References panel are clear improvements. The reactive re-link design (per-corpus creator visibility, per-corpus failure isolation) is exactly right. A few issues worth addressing before merge:

---

Bugs / Correctness

1. _reconcile_document_graph drops get_or_create safety — race window on concurrent runs

The old _ensure_document_relationship used get_or_create to prevent duplicate edges when two concurrent apply() calls race on the same corpus. The new reconciliation loop uses DocumentRelationship.objects.create() directly in writer.py. Two simultaneous runs can both see (src, tgt) not in covered and both issue a CREATE, producing a duplicate edge. Swap for get_or_create (keyed on source_document_id, target_document_id, corpus, annotation_label, relationship_type) and only increment document_relationships_created when created is True.

---

2. useReferenceMentions — failed fetch silently kills retries

mergedForRef.current = mergeKey is set before the async operations complete. If discoverAnalyses throws or enrichmentAnalyses.length === 0 in a transient-empty state (e.g. the analysis is still running), the gate is permanently stuck. On remount for the same (document, corpus) the effect short-circuits and citations never appear. Either (a) reset the gate on failure, or (b) set it only after a successful merge.

---

3. GovernanceGraphLive polling not stopped on unmount

The startPolling call in handleBootstrap has no corresponding stopPolling in a cleanup effect. If the user navigates away while the corpus is weaving, the poll continues until the component re-mounts and the weaving && hasNodes effect fires. Add a cleanup effect: return () => { if (weaving) stopPolling(); }.

---

DRY Violation

4. displayKey in DocumentReferencesPanel.tsx duplicates formatCanonicalLawKey

DocumentReferencesPanel.tsx defines a local displayKey function with a hardcoded acronym set that is nearly identical to formatCanonicalLawKey() added to frontend/src/utils/formatters.ts in this same PR. GovernanceGraphGlimpse and WantedAuthoritiesCard already import the shared function; this one should too. Also note: the local version misses "sec" as a special-cased word (the shared function handles it via LAW_AUTHORITY_ACRONYMS), so the two implementations are subtly inconsistent — "sec-rule:10b-5" renders as "Sec Rule § 10b-5" in the References panel but "SEC Rule § 10b-5" everywhere else.

---

Security / Performance

5. wantedAuthorities query has no rate limit

Other resolvers in annotation_queries.py use graphql_ratelimit_dynamic. wanted_authorities performs a cross-corpus Python-side aggregation that could be expensive for users with many visible corpora. The resolver should apply the same rate-limiting decorator as its neighbours.

6. _require_type_label_agreement reaches into Django internals

self._state.fields_cache is a Django internal, not a public API. It works today but has broken between minor releases. The hot path (enrichment writer already calls select_related("annotation_label")) means the cache hit is common, but the fallback ORM query in save() is benign. Consider replacing the cache inspection with a sentinel attribute set by the writer after its select_related chain, or simply tolerating the extra query on the single-row-save path.

---

Minor

7. Dual navigation hooks in DocumentReferencesPanel — both navigate (from useNavigate) for outbound rows with a stored linkUrl and navigateToDocument (from useNavigateToDocumentById) for inbound rows resolved by id are correctly used. No change needed; just confirming this is not dead code.

8. bootstrap_authority_corpus in authorities.py — corpus = Corpus.objects.get(pk=result["corpus_id"]) raises DoesNotExist with no helpful message if the bootstrapper returns an unexpected id. A get_or_none + explicit error would be cleaner, though in practice this cannot happen since the result comes from the same transaction.

---

What is well done

• The relink_corpora_for_keys per-corpus-creator visibility design is correct and test_relink_runs_as_each_corpus_creator_no_private_leak directly validates the security property.
• Extracting get_or_create_analyzer as a @staticmethod so tests and the service share one converge path is a good DRY improvement.
• The values_list fix in governance_graph_service.py (QuerySet.only() incompatibility with a manager that bakes in select_related("parent")) is the right fix and the comment explains it clearly.
• The partial unique constraint + migration data-dedup for keyless CorpusReference rows is correct Postgres idiom.
• docs/architecture/reference-web-versioning.md is exactly the right place to park the stale-pointer known gap — durable, findable, does not block the PR.

---

Summary: Items 1 (race condition in _reconcile_document_graph) and 2 (silent gate on failed merge in useReferenceMentions) are correctness issues worth fixing before merge. Item 4 (DRY violation with a subtle inconsistency) is a quick cleanup. Items 3, 5, and 6 are lower-priority polish.

[claude]

Code Review — Reference Web UI (#1977)

Reviewed as stacked on feature/corpus-reference-enrichment; diff is against that base.

This is a well-architected PR shipping several interlocked features: a bipartite D3 governance graph, a one-click bootstrap flow, a document References panel, and the backend machinery (wanted-authorities queue, reactive re-link, authority bootstrap entry points). Overall design is solid. Notes below range from real correctness concerns to minor observations.

---

Correctness / Bugs

1. CorpusReference.save() fires an extra DB query per row on the enrichment writer hot path

_require_type_label_agreement in opencontractserver/annotations/models.py always executes a fresh Annotation.objects.filter(pk=...).values_list("annotation_label__text") lookup. The comment explicitly says it does not use the ORM in-memory cache, so every CorpusReference.objects.create() call in the enrichment writer adds a round-trip. For a corpus with hundreds of references this adds a proportional query burst. The integrity guarantee is worth having, but the comment saying the writer's select_related("annotation_label") keeps it "cheap" is a DB-page-cache argument, not an ORM cache hit — the writer's select_related is on the Annotation queryset, not on the CorpusReference save path. Worth either (a) checking self.source_annotation.annotation_label if the relation is already in the ORM cache, or (b) explicitly documenting the per-save cost so future callers do not use bulk_create without understanding the constraint is bypassed.

2. relink_corpora_for_keys summary undercounts slug-only repairs

if out["law_references_linked"]:
    summary["corpora_relinked"] += 1

A corpus whose references were already linked but whose mention link_urls were stale (slug drift) gets links_restamped > 0 but law_references_linked == 0, so it does not increment corpora_relinked. The summary would report 0 corpora affected even when links were repaired. Not a functional problem, but could mislead operators reading the Celery task result. One-line fix: if out["law_references_linked"] or out["links_restamped"].

3. No test for the per-corpus failure path in relink_corpora_for_keys

The sweep catches per-corpus exceptions and continues. No test exercises this path. Consider adding a test that monkeypatches _link_external to raise on the second corpus and asserts corpora_failed == 1 while the first corpus's refs are still linked.

---

Performance

4. wanted_authorities loads all EXTERNAL law ref rows into Python memory

for key, ref_corpus_id in qs.values_list("canonical_key", "corpus_id"):

The PR notes this "stays modest" and the reasoning is fair for current scale. The Python-side aggregation is necessary because candidate_keys is a regex the DB cannot express. For a multi-tenant deployment with many large corpora this could grow. A note in the docstring about the expected ceiling, or a count-based guard before iteration, would make the scaling assumption explicit.

5. _reconcile_document_graph: covered re-evaluates the queryset after deletion

covered is computed from projection_rows.values_list(...) after stale_ids are deleted. Django querysets are lazy so this correctly re-executes and excludes deleted rows — confirming this is intentional and not a future footgun if projection_rows is ever evaluated eagerly before the delete.

---

Design / Conventions

6. GovernanceGraphGlimpse.tsx is 967 lines

This file handles layout math, cluster union-find, SVG rendering, hover state, and the empty-state motif in one place. The layout algorithm is non-trivial enough to justify the cohesion, but the render section (roughly lines 600+) could be split into a GovernanceGraphSvg sub-component without touching layout logic. Not blocking, flagging for future maintainability.

7. handleBootstrap outer catch swallows the error without logging

} catch (err) {
  toast.error("Couldn't start reference enrichment.");
}

The flow is correct — inner guards prevent null-deref before reaching startAnalysis. But silently discarding err makes it harder to diagnose unexpected failures in development. Consider console.error(err) inside the catch.

8. relink_corpora_for_keys_task has no retry policy

The Celery task uses bare @shared_task with no max_retries, autoretry_for, or acks_late. Per-corpus failures are caught internally (by design), but an unhandled exception at the task level — e.g. DB connection lost before the loop starts — would permanently drop the sweep without retry. Other tasks in corpus_tasks.py use retry configurations; worth aligning.

---

Minor / Nits

9. corpusId? in DocumentReferencesPanel — skip: !corpusId is present in the query call. Correctly guarded, no issue.

10. Migration deduplication is Python-side

dedupe_keyless_references in 0079 iterates all keyless rows in Python. Since CorpusReference was introduced in migration 0078 (new in this feature branch), there won't be many rows in production yet. For future reference, raw SQL would be more efficient at scale:

DELETE FROM annotations_corpusreference WHERE id NOT IN (
  SELECT MIN(id) FROM annotations_corpusreference
  WHERE canonical_key IS NULL
  GROUP BY source_annotation_id, reference_type
);

11. Changelog fragments and architecture note are excellent. The reference-web-versioning.md design note documenting the known staleness gap on authority re-ingestion is exactly the right kind of durable record for a deliberate deferral.

---

Summary

Strong PR. The data-integrity additions (save() validation, partial unique constraint, migration deduplication, _reconcile_document_graph reconciliation) all address real correctness gaps. The DRY refactoring (graphCardChrome, graphLayout, formatCanonicalLawKey) is the right call. The values_list fix to the governanceGraph crash is a clean production fix.

Main actionable items:

• Medium: corpora_relinked undercounts slug-only repairs (item 2) — one-line fix.
• Low: No test for the per-corpus failure path in relink_corpora_for_keys (item 3).
• Low: relink_corpora_for_keys_task retry policy alignment (item 8).
• Note: CorpusReference.save() query cost is real; the "cheap" comment reasoning should be clarified or the hot-path per-save cost documented (item 1).

[claude]

---

Code Review

Overall: This is a well-structured PR — the force graph layout is cleanly implemented (pure computeLayout function, synchronous ticks, no animation loop), the service layer is respected throughout config/graphql/, the bootstrap tool is properly gated (requires_approval=True, requires_write_permission=True), and the relink_corpora_for_keys sweep correctly impersonates corpus.creator_id to avoid privilege escalation. E001 is clean. Test coverage is solid.

A few items worth addressing before merge:

Bug — Apollo cache collisions (Frontend)

frontend/src/graphql/cache.ts is not updated, so two new queries land without cache isolation:

• corpusReferences — switching between documents in the same corpus session will collapse all results into one cache slot. Document A's references appear on Document B until a forced refetch.
• governanceGraph — navigating between corpora serves the first corpus's graph until the network response arrives.

Both need keyArgs entries per the project's established pattern:

corpusReferences: relayStylePagination(["corpusId", "documentId"]),
governanceGraph: { keyArgs: ["corpusId"] },

Fragile — direct navigate() bypass in DocumentReferencesPanel

DocumentReferencesPanel.tsx (~line 2977, outbound citation rows) calls navigate(group.linkUrl!) directly via useNavigate(). This is fragile: the enrichment service currently produces site-relative /d/... paths, but the existing openAnnotationUrl utility handles the internal/external URL split correctly and would guard against any future http://-absolute value silently failing to navigate. Suggest routing through openAnnotationUrl for consistency. There is also no CT test covering this click path.

Minor — IDOR-lite in resolve_corpus_references (annotation_queries.py)

The new document_id filter applies to an already corpus-scoped queryset (no cross-corpus data leaks), but a caller with READ on a corpus can probe whether an arbitrary global document ID has references in that corpus — the document itself need not be visible to them. Per project IDOR policy the filter should validate the document against Document.objects.visible_to_user(user) before applying it.

Nit — double queryset evaluation in writer.py

In _reconcile_document_graph (~lines 186-201), projection_rows is evaluated as a lazy queryset twice (stale-edge loop + covered set), with a .delete() between them. Materialising to a list before the delete (list(projection_rows.values_list(...))) makes the intent explicit and saves a round-trip.

Summary

Severity	Location	Issue
Bug	cache.ts (not modified)	corpusReferences / governanceGraph missing keyArgs — cache collisions
Fragile	DocumentReferencesPanel.tsx ~L2977	Direct navigate(linkUrl) bypasses openAnnotationUrl; no CT coverage
Minor	annotation_queries.py	document_id filter does not validate doc visibility — IDOR-lite leak
Nit	writer.py ~L186-201	Lazy queryset evaluated twice around a delete

The first two are worth fixing before merge; the rest are low-risk follow-ups.

[claude]

.

[claude]

Code Review — PR 1977

This is a well-architected PR. The design choices are justified, security considerations are addressed explicitly, and tests are broad. Below are findings roughly ordered by severity.

Bugs / Correctness

1. Dead succeeded variable in useReferenceMentions.ts

succeeded is set to true at the end of the happy path but never read. The one-shot gate (mergedForRef.current) is already set at entry and stays set after success, so succeeded is unreachable logic. It should be deleted before it misleads future readers.

2. _require_type_label_agreement fires a DB query on every CorpusReference.save()

The model-level check is correct and the enrichment writer mitigates it with select_related at the queryset level. But that select-related only helps when the annotation is already in the writer's queryset -- any ORM path that creates a CorpusReference without pre-fetching the label (tests, admin, shell) pays a hidden N+1. A follow-up optimization would check the in-memory relation cache before issuing the query. Not a blocker given the writer already addresses the hot path.

Performance Concerns

3. relink_corpora_for_keys loads all EXTERNAL refs into memory

The pairs queryset fetches every distinct (corpus_id, canonical_key) for every EXTERNAL law ref across the whole system, then Python-side-filters against the just-bootstrapped keys. On a large deployment with thousands of corpora and hundreds of external citations each, this loads the full cross-product into memory. A pre-filter on authority prefix (always root.split(":", 1)[0]) would bound the scan before the Python filter runs. Worth a comment or tracked issue.

4. wanted_authorities Python-side aggregation

Same family of concern: loads every (canonical_key, corpus_id) pair visible to the user and aggregates in Python. The existing comment acknowledges this. At realistic scale this is fine; worth monitoring.

Minor Issues

5. References tab always visible regardless of enrichment data

The "References" tab in SidebarTabs.tsx is unconditionally rendered. Users on documents without enrichment data will click it and see an empty panel with no guidance. Consider adding a helpful empty state that links back to the Corpus Intelligence bootstrap flow.

6. handleBootstrap does not check for a pre-existing CorpusAction

If createCorpusAction errors because an add_document action already exists, the catch block swallows it with a generic toast. A user who clicks the bootstrap button a second time will see the soft toast even though the action is already installed. A backend-side upsert or pre-check would be cleaner, but this is low-priority.

7. GovernanceGraphGlimpse.tsx is 967 lines

The layout math, SVG arc helpers, and shelf-positioning logic could be extracted into a governanceGraphLayout.ts utility alongside the existing graphLayout.ts. Not a blocker given it follows the same pattern as DocumentGraphGlimpse.

8. useNavigateToDocumentById casts through unknown

The comment explains the structural subset issue honestly. A cleaner long-term fix is narrowing buildCanonicalPath's parameter types to the minimal required fields (slug and creator.slug), which would remove the cast and catch future breakage at compile time.

Security

9. IDOR guard in resolve_corpus_references — correct

The guard is sound: returning an empty result for both "invisible" and "does not exist" cases correctly prevents enumeration. The inline comment explains the rationale clearly.

10. relink_corpora_for_keys visibility semantics — correct

Running the relink under each corpus's own creator preserves the permission model correctly. Per-corpus exception isolation (log + count, do not abort) is appropriate for a sweep operation.

Test Coverage

Strong overall. test_corpus_reference_model.py directly tests the partial unique constraint and the type/label agreement check with correct transaction.atomic() wrapping. The backfill/linking test files cover the new bootstrap/relink paths. 6 Playwright CT tests and unit tests for formatCanonicalLawKey / createSeededRandom round out the frontend.

One gap: no test for the corpusReferences(documentId:) resolver's IDOR guard -- specifically the case where a valid corpus reader supplies a document ID they cannot see. Worth adding to test_corpus_reference_model.py or a new graphql-layer test.

Changelog / Docs

The changelog fragments are thorough and clearly link code locations to impact. docs/architecture/reference-web-versioning.md is a good example of documenting a known gap explicitly rather than leaving it for someone to rediscover.

Summary

The implementation is correct and well-thought-out. The main actionable items before merge:

1. Remove the dead succeeded variable in useReferenceMentions.ts (one-line cleanup)
2. Add a helpful empty state in the References panel that guides users toward the bootstrap flow -- the tab is always visible but the content gives no direction when no enrichment data exists
3. Track the unbounded-queryset scaling risk in relink_corpora_for_keys -- a comment or tracked issue is sufficient

Everything else is low priority or follow-up work.

[claude]

Code Review: Reference Web UI (PR #1977)

Large, well-engineered PR. Architecture is solid, tests are thorough, and proven against live data. Key observations below.

Issues Worth Addressing

1. _restamp_mention_links is O(all resolved refs) on every linking call

The new implementation loads every resolved LAW/DOCUMENT reference for the corpus (with select_related) on each _link_external invocation — including from relink_corpora_for_keys sweeps that touch each affected corpus. The old code only stamped newly-resolved refs. For corpora with thousands of resolved references this is a regression on the common calling path. Consider stamping only newly-resolved refs during the first pass and reserving the full restamp for a separate/async call.

2. Race condition risk in _reconcile_document_graph

The comment correctly identifies the concurrent-writer scenario and uses get_or_create. However, get_or_create is only race-safe when there is a DB-level unique constraint backing the lookup key. Without a unique constraint on (source_document, target_document, annotation_label, relationship_type, corpus) on DocumentRelationship, two concurrent Celery workers can both find the row missing and both INSERT. Please confirm (or add) the unique constraint.

3. Extra DB query on every CorpusReference.save()

_require_type_label_agreement fires an indexed Annotation lookup on every save(). The comment says the writer select_related('annotation_label') to keep this cheap — but that select-related covers the queryset read path, not the freshly-created instance from get_or_create. Every _ensure_corpus_reference call (one per mention) pays an extra query. Manageable now but worth profiling at scale.

4. bootstrap_authority_corpus partial state on inline relink failure

When make_public=True + relink=True, relink_async=False: the corpus is published first, then inline relink runs. If the relink raises, the corpus is public but references remain EXTERNAL. Idempotent on retry, but explicit error logging of this partial state would be clearer.

---

Positive Observations

• Governance graph crash fix (values_list instead of .only()) is correct and cheaper — no model instantiation, no guardian prefetch.
• Partial-NULL unique constraint + data migration — dedupe_keyless_references is bounded, uses historical models correctly, keeps the oldest row.
• relink_corpora_for_keys visibility isolation — running each corpus's link pass as its creator is exactly right. test_relink_runs_as_each_corpus_creator_no_private_leak directly verifies the security invariant.
• IDOR guard in resolve_corpus_references — the inline comment documents the security rationale clearly (invisible document yields same empty result as one with no references).
• graphLayout.ts extraction with dedicated unit tests, shared by both graph components.
• GovernanceGraphGlimpse / GovernanceGraphLive split — clean presentational/container separation; glimpse has no mutations or queries.
• Apollo cache keyArgs comment referencing CLAUDE.md Bump django-coverage-plugin from 2.0.2 to 2.0.3 #15 prevents the silent cache-collision class of bug.
• GOVERNANCE_GRAPH_WEAVING_MAX_MS timeout — essential for corpora with no law refs; without it the poll would spin forever.
• test_enrichment_backfill.py security tests — private-authority isolation test directly asserts the key visibility invariant.
• REF_DEFINED_TERM early-continue in writer — the comment explaining why definition sites get no CorpusReference row is exactly the kind of WHY comment that belongs in code.
• get_or_create_analyzer extraction — eliminates duplicated convergence logic.

---

Minor Frontend Notes

• useReferenceMentions retry — gate releases on failure for retries, but no cap on retry count. A counter-based backoff after N failures would prevent a stuck gate from hammering the server on persistent errors.
• DocumentReferencesPanel self-reference — a document referencing itself lands only in outbound (likely correct, but confirm intentional).
• GovernanceGraphLive.handleBootstrap — the outer catch shows a generic toast; fine in practice but a more specific message per error type would improve debuggability.

---

Summary: four items to address before merge

1. Confirm or add a DB unique constraint on DocumentRelationship to back the _reconcile_document_graph race guard — high priority
2. _restamp_mention_links O(all resolved) cost — needs a design decision or tracking note before shipping to large-scale deployments
3. Extra save() query in _require_type_label_agreement — profile at scale
4. bootstrap_authority_corpus partial state — log or handle the publish-then-fail case explicitly