One-click collection-intelligence setup

[JSv4]

Problem

A freshly created corpus lands with an unreadable document index: titles all share the same truncated prefix and Document.description holds whatever the importer wrote (raw metadata dumps like **filing_type:** S-1 **doc_role:** exhibit…), and summary coverage sits at 0%. All the machinery to fix this already exists — the reference-web analyzer, and the Document Description Updater / Document Summary Generator action templates from the Action Library — but nothing composed it at corpus setup, and nothing batch-ran the LLM actions over documents already in the corpus.

Before (documents drawer)	After (LLM descriptions)
**filing_type:** S-1 **doc_role:** exhibit **exhibit_number:** 10.(2)(…	"This document is the primary S-1 registration statement filed by Fervo Energy Company with the SEC on April 17, 2026…"

What this adds

CorpusIntelligenceSetupService (opencontractserver/corpuses/services/intelligence_setup.py) — one idempotent composite:

1. Deterministic: installs the reference-enrichment add_document CorpusAction (same row the governance-graph CTA creates) and starts the first weave immediately.
2. LLM: clones the description + summary templates (bundle pinned in INTELLIGENCE_SETUP_TEMPLATE_NAMES) and batch-runs each over every existing document.

Re-running converges: existing rows are reused, already-run documents are skipped, an in-flight weave isn't duplicated. Per-template failures (e.g. the BATCH_RUN_MAX_DOCS cap) surface in the summary without failing the whole call.

GraphQL: setupCorpusIntelligence mutation + corpusIntelligenceSetupStatus query; createCorpus now returns objId so follow-ups can chain off creation.

Frontend (both entry points):

• IntelligenceSetupBanner mounted inside IntelligencePanel — appears on the intelligence overview and the insight-panel CAML embed, offers "Set up", reports the queued fan-out in a toast, and hides once the bundle is installed.
• New Corpus modal gains a default-on "Set up collection intelligence" opt-in that chains the mutation after creation.

Verification

• test_intelligence_setup.py: service install/idempotence/permission-gating/status + GraphQL smoke (7 tests).
• IntelligenceSetupBanner.ct.tsx: offer → run → hide, and silent-when-set-up (2 CT tests, doc screenshot).
• Live-proven on the dev stack: a fresh 2-doc corpus flips not-set-up → fully-set-up in one call (weave started, 4 agent runs queued); the 37-doc Fervo demo corpus's index is now human-readable after the same machinery drained.

Stacked on #1977.

[claude]

Code Review — PR #1982: One-click collection-intelligence setup

Overall: Solid feature addition. The service architecture is clean, idempotency is handled correctly at every layer, and permission semantics follow project conventions. A few items worth addressing before merge.

---

What it does (summary)

• CorpusIntelligenceSetupService — an idempotent composite that installs the reference-enrichment add_document action, starts the first weave, and clones plus batch-runs the description/summary templates over existing documents.
• GraphQL: setupCorpusIntelligence mutation + corpusIntelligenceSetupStatus query.
• Frontend: IntelligenceSetupBanner (mounts inside IntelligencePanel, hides once fully set up) and a default-on opt-in in the corpus-creation modal.

---

Issues to Address

1. Backend createCorpus must return objId — confirm this is covered

frontend/src/graphql/mutations.ts adds objId to the CREATE_CORPUS query, and Corpuses.tsx reads data.data.createCorpus.objId to chain the setup call. The diff does not show a corresponding backend change to corpus_mutations.py's createCorpus resolver returning obj_id. If that field is absent from the backend response, newCorpusId is always undefined and the opt-in setup silently never fires.

Please confirm (or point to the base-branch commit) that createCorpus already emits obj_id before this PR lands.

2. Private model method: corpus._get_active_documents()

# intelligence_setup.py — setup() method
total_active_documents=corpus._get_active_documents().count(),

This calls an underscore-prefixed method, which by convention is private API. If it is renamed or removed, this breaks silently at runtime (no static analysis will catch it). Either expose a public counterpart on Corpus or reach for the service layer's own queryset helpers.

3. No rate limit on the setupCorpusIntelligence mutation

The status query correctly applies READ_LIGHT throttling. The mutation kicks off potentially dozens of agent runs, but carries no @graphql_ratelimit_dynamic decorator. A user with a large corpus could call this in a tight loop and fan out a lot of background work. Worth adding a WRITE_HEAVY (or equivalent) gate consistent with other mutations that trigger Celery work.

---

Minor Observations

Race condition in _setup_reference_enrichment (acceptable)
The in-flight check and the analysis start are not atomic — another request could slip between the two. Starting a duplicate analysis is recoverable (just wasted work), but a brief comment acknowledging the TOCTOU window is intentional would help future readers.

IntelligenceSetupBanner silently ignores query errors

const { data, refetch } = useQuery(GET_CORPUS_INTELLIGENCE_SETUP_STATUS, ...);
if (!status || status.isFullySetUp) return null;

A network error also satisfies !status, so the banner disappears on transient failures with no feedback. Returning null is fine for a banner, but not destructuring error from useQuery means you cannot distinguish "loading" from "error" if you ever need to log it. Low priority.

Optimistic hide after clicking "Set up"
After click, the banner stays visible with a spinner until refetch() resolves. Since setup is idempotent server-side, hiding optimistically on a successful mutation response (before the refetch) would feel snappier. Not a blocker.

log_action keyword arguments

cls.log_action(
    "Intelligence setup for",
    corpus,
    user,
    reference_started=summary.reference_analysis_started,
    ...
)

BaseService.log_action needs to accept **kwargs for this not to raise TypeError. Quick sanity-check to confirm the base class signature handles these.

---

What is done well

• Idempotency at every step: action dedup, batch-run skip-if-already-run, in-flight analysis guard — all verified by tests.
• Permission semantics are correct: get_or_none (READ) gates visibility; require_permission(UPDATE) gates mutation — consistent with project conventions.
• IDOR prevention: single ambiguous error message for both "not found" and "no permission".
• Per-template error isolation: a BATCH_RUN_MAX_DOCS cap or similar failure in one template surfaces in outcome.error without failing the whole call.
• Soft failure from the modal: intelligence-setup failure emits toast.info, not toast.error, and does not roll back the corpus creation.
• Tests: service (install / idempotence / permission gating / status), GraphQL smoke, and two CT tests with a doc screenshot. Good coverage for the scope.
• Changelog fragment present and well-written.

---

Verdict: Address items 1 through 3 before merge. Everything else is polish.

[codecov]

Codecov Report

❌ Patch coverage is 91.78082% with 30 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
opencontractserver/enrichment/writer.py	89.88%	9 Missing ⚠️
...orpusHome/intelligence/IntelligenceSetupBanner.tsx	87.50%	0 Missing and 5 partials ⚠️
...ractserver/corpuses/services/intelligence_setup.py	96.99%	4 Missing ⚠️
frontend/src/views/Corpuses.tsx	62.50%	2 Missing and 1 partial ⚠️
opencontractserver/utils/span_projection.py	93.75%	2 Missing ⚠️
frontend/src/components/cookies/CookieConsent.tsx	0.00%	1 Missing ⚠️
...es/CorpusHome/intelligence/GovernanceGraphLive.tsx	85.71%	1 Missing ⚠️
frontend/src/components/corpuses/CorpusMapView.tsx	0.00%	1 Missing ⚠️
frontend/src/components/corpuses/CorpusModal.tsx	75.00%	1 Missing ⚠️
..._base/document/document_kb/useReferenceMentions.ts	66.66%	1 Missing ⚠️
... and 2 more

📢 Thoughts on this report? Let us know!

[claude]

Code Review: One-click collection-intelligence setup

Overview: This PR adds a well-structured orchestration layer composing existing enrichment machinery (reference-web analyzer, description/summary templates) into a single idempotent setupCorpusIntelligence call. No blockers found, but a few items worth addressing.

Strengths

• Service layer compliance: CorpusIntelligenceSetupService extends BaseService, uses ServiceResult, defers model imports to prevent circular imports.
• Idempotency is thorough: action rows deduplicated, already-run documents skipped, in-flight analyses not duplicated. test_setup_is_idempotent validates all three.
• IDOR prevention is correct: the stranger test returns the same opaque message whether the corpus is missing or the caller lacks permission.
• Race condition is acknowledged: the comment in _setup_reference_enrichment correctly explains why non-atomic is acceptable.
• get_corpus_documents corpus-as-gate semantic is the right choice for a pipeline-facing caller.

Issues to address before merge

1. Incomplete TypeScript output type (mutations.ts)

SetupCorpusIntelligenceOutputs.summary is missing referenceActionAlreadyInstalled, which is required=True in the backend CorpusIntelligenceSetupSummaryType. The frontend does not consume it today but the type should mirror the schema.

2. Misleading toast when the batch-run cap is hit (IntelligenceSetupBanner.tsx)

When batch_run_on_corpus hits BATCH_RUN_MAX_DOCS, outcome.queued_count stays 0 but ok=True. The toast is built from sum(t.queuedCount), so when the cap fires on all templates the user sees 'Collection intelligence is set up.' even though nothing was queued. Consider checking payload.summary.templates.some(t => t.error) and showing a toast.warn instead.

Optional cleanup

• INTELLIGENCE_SETUP_TEMPLATE_NAMES (corpus_actions.py): Declared list[str] but never mutated. tuple[str, ...] is more semantically accurate for a module-level constant.
• Duplicate mock in the already-set-up CT test: mocks={[statusMock(true), statusMock(true)]} but only one fetch fires. Using two can silently paper over an unexpected extra query.
• No test for the batch-fail path in _setup_templates: A test patching CorpusActionService.batch_run_on_corpus to return a failure would lock in the error-absorption behaviour.

Summary: Orchestration design, idempotency, permission gating, and service layer usage are all sound. Items 1 and 2 are worth fixing before merge. The rest is optional cleanup.

[claude]

Code Review: One-click collection-intelligence setup

Overview

Solid orchestration layer that composes existing enrichment machinery into a single idempotent call. The design is clean — service layer owns the logic, GraphQL is a thin adapter, frontend stays simple. The idempotency story is well-reasoned and the IDOR-safe messaging convention is followed correctly throughout.

---

Issues to Address

1. _setup_templates — unhandled non-IntegrityError exceptions from clone_to_corpus (bug)

intelligence_setup.py:_setup_templates only catches IntegrityError inside the savepoint:

try:
    with transaction.atomic():
        action = template.clone_to_corpus(corpus, creator=user)
except IntegrityError:
    ...

If clone_to_corpus raises anything else (e.g. OperationalError, ValueError), the exception propagates out of the for name in INTELLIGENCE_SETUP_TEMPLATE_NAMES: loop. All remaining templates are aborted and the caller gets a 500 rather than the graceful partial-success response the design promises. The reference action and any already-committed earlier templates are left installed in a half-done state.

Suggested fix — add a broader except after IntegrityError:

except IntegrityError:
    action = CorpusAction.objects.filter(corpus=corpus, source_template=template).first()
    outcome.already_installed = action is not None
except Exception as exc:
    outcome.error = f"Failed to install template: {exc}"
    logger.exception("Intelligence setup: clone failed for %r on corpus %s", name, corpus.pk)
    continue

---

2. createCorpus.objId — backend availability needs confirmation

mutations.ts adds objId to the CREATE_CORPUS query, and Corpuses.tsx chains trySetupIntelligence off data.data.createCorpus.objId. The diff doesn't show a corresponding backend change to the CreateCorpus graphene mutation. If the field isn't already exposed, it resolves to null, newCorpusId is falsy, and the post-create intelligence setup silently never runs even when the user checked the opt-in.

Please confirm the existing CreateCorpus mutation already returns objId (or note where it was added, e.g. stacked PR #1977).

---

Minor Issues

3. CT test mock missing referenceActionAlreadyInstalled

IntelligenceSetupBanner.ct.tsx:setupMock — the summary object omits referenceActionAlreadyInstalled, which the SETUP_CORPUS_INTELLIGENCE query requests. Apollo's MockedProvider returns undefined for it. The component doesn't read this field so the test passes, but the mock doesn't match the real server contract.

  summary: {
    referenceAvailable: true,
    referenceActionInstalledNow: true,
+   referenceActionAlreadyInstalled: false,
    referenceAnalysisStarted: true,

4. status query — three DB queries per call

resolve_corpus_intelligence_setup_status issues three queries (reference-action exists, installed names, missing names). Currently mounted once per corpus page load, so this is fine. Worth a brief comment so the cost is visible if polling or corpus-list rendering is considered later.

---

What's Well Done

• Idempotency design: every step dedupes — reference action checked before create, IntegrityError savepoint mirrors AddTemplateToCorpus, batch-run skips already-executed docs, in-flight analysis detection. Re-running converges cleanly.
• Permission model: get_or_none (visibility) + require_permission(UPDATE) + IDOR-safe single error message throughout.
• Rate limiting: mutation on WRITE_HEAVY, status query on READ_LIGHT.
• Frontend banner: silent-while-loading, silent-once-set-up, post-create failure surfaces as a toast without tainting the corpus-creation success.
• Service architecture: logic lives entirely in the service; GraphQL is a thin adapter.
• Test coverage: service install/idempotence/permission/status + GraphQL smoke + CT banner offer→run→hide. setUpTestData/patch pattern is correct and the seeder mirrors the migration path.
• Changelog fragment: present and well-structured.

---

Summary

Two items before merge: the unhandled-exception gap in _setup_templates (real user-visible 500) and confirmation that createCorpus.objId is already exposed server-side (silent setup-skip if not). Items 3–4 are polish.

[claude]

test

[claude]

Code Review

Overview

This is a well-scoped PR that adds one-click collection-intelligence setup while also fixing several pre-existing issues: GraphQL spec validation being silently disabled, the AWS presigned URL cache TTL deriving from the wrong constant, a 176s→2.3s query performance regression, and 26 invalid frontend GraphQL documents. The core new service is clean and the idempotency story is well thought-out.

---

Critical / Correctness

SetupCorpusIntelligence.mutate has no error boundary around the service call (config/graphql/corpus_mutations.py):

If setup() raises an unexpected exception (e.g. a DatabaseError not caught internally), Graphene surfaces it as a 500-level error rather than an ok=False envelope. Compare to AddTemplateToCorpus and CreateCorpusAction in the same file, both of which wrap their body in try/except Exception. Recommend wrapping the setup() call and result dispatch in a similar guard.

---

Potential Bugs / Regressions

require_permission usage in DeleteMetadataColumn and UpdateFieldset (config/graphql/extract_mutations.py):

The semantics are correct — require_permission returns "" (falsy) on grant and a non-empty string (truthy) on denial — but this is the opposite of the boolean idiom and easy to misread. The BaseService docstring flags this explicitly: "Truthiness inversion … Engineers cross-reading old and new code should not transcribe if user_can(...) to if require_permission(...) directly — that flips the gate." The code is correct, but since these are two new mutations, a brief inline comment clarifying the inversion would protect the next reader, matching the pattern in intelligence_setup.py where the return value is stored in error and checked as if error:.

IntelligenceSetupBanner uses cache-first query policy (IntelligenceSetupBanner.tsx line ~1114):

No fetchPolicy is set, so the status defaults to cache-first. If a user installs the bundle via the Governance Graph bootstrap and then opens the Intelligence Panel, the banner may show a stale "not set up" state. Clicking "Set up" is a server-side no-op (idempotent), but a spurious "Collection intelligence is set up" toast after re-fetching is confusing. cache-and-network would keep the banner honest without an extra round-trip on mount.

---

Design / Style

canSetup field derivation: the logic for excluding unavailable/inactive bundle pieces from is_fully_set_up is the right call (avoids the undismissable-CTA zombie). Worth confirming that canSetup uses the same CRUD gate as the mutation — if the permission tiers drift, viewers could see a CTA that the mutation rejects.

Corpus creation opt-in defaults to true (CorpusModal.tsx): a freshly created empty corpus will still install the reference action (no-op for the document agents, but a write nonetheless). Idempotent so harmless, just worth noting.

GET_MY_CORPUSES drops the myPermissions: ["UPDATE"] filter: the right fix — the filter was an invalid argument causing spec validation failures, and the backend enforces write permission on actual write mutations anyway. No IDOR risk.

---

Positive Highlights

• GraphQL spec validation fix (config/graphql/schema.py): restoring [*specified_rules, DepthLimitValidationRule] closes a real silent-failure surface; the architectural test pinning it is the right long-term guard.
• install_template shared method (corpus_actions.py): eliminating the duplicated clone+IntegrityError-recovery+CRUD-grant pattern between AddTemplateToCorpus and the intelligence bundle is clean; the two paths can no longer drift.
• batch_run_action with allow_partial=True: the silent-skip-on-cap bug was a real user-facing failure; the deterministic-id-order slice and remaining_count field make the partial semantics explicit and observable.
• canSetup on the status payload: prevents the guaranteed-to-fail setup CTA from appearing for read-only viewers — exactly the right place to gate this.
• useReferenceMentions lean query + analyzedCorpusId filter: the 176s→~0s improvement speaks for itself. The switch from useLazyQuery to client.query for imperative await-in-a-loop flows is the correct pattern (lazy query handles can leave promises unsettled on variable changes).
• AWS presigned URL TTL fix (config/settings/base.py): _AWS_EXPIRY (7-day CacheControl max-age) and AWS_QUERYSTRING_EXPIRE (1-hour presign lifetime) were silently conflated; the clamping utility and the explicit AWS_QUERYSTRING_EXPIRE constant seal this class of future confusion.
• Test coverage: service idempotency/permission/status tests, CT tests covering offer→run→hide, error paths, partial-cap messaging, and the GovernanceGraph skip-if-installed test give good confidence across happy and sad paths.

---

Summary

The PR is well-structured and addresses real user-facing pain points. The two items most worth addressing before merge are the missing error boundary in SetupCorpusIntelligence.mutate and the cache-first policy on the status query in IntelligenceSetupBanner. The require_permission usage is semantically correct but would benefit from a short clarifying comment given the inverted truthiness convention.

[claude]

Code Review: One-click collection-intelligence setup (#1982)

Overview

This PR adds CorpusIntelligenceSetupService — a single idempotent call that installs the reference-enrichment action and batch-runs the description/summary agent templates over a corpus's existing documents. It also includes several important bug fixes bundled alongside the feature.

---

Critical Bug Fixes

1. GraphQL spec validation was silently disabled (config/graphql/schema.py)

graphql-core's validate() replaces the default rule set when rules= is provided, so the prior validation_rules = [DepthLimitValidationRule] had been silently skipping all standard spec checks (unknown arguments, unknown fields, type mismatch, ...). Queries with invalid arguments executed with bogus parts silently ignored -- the PR notes ~26 invalid frontend documents shipped as a result.

The fix ([*specified_rules, DepthLimitValidationRule]) and the pinning tests in TestServedValidationRulesIncludeSpecRules are correct. The architectural invariant test (test_frontend_graphql_documents.py) prevents regression cleanly.

2. AWS presigned URL TTL was derived from the wrong variable (config/settings/base.py)

_AWS_EXPIRY is the HTTP CacheControl max-age for stored objects (7 days), not the presigned URL signature lifetime (1 hour default). Using it as _signed_url_lifetime_seconds let the shared URL cache serve dead 403 links for up to 5 hours (half of 7 days). Fixed by introducing AWS_QUERYSTRING_EXPIRE and clamping via the new clamp_shared_url_cache_ttl utility. Clean.

3. N+1 / performance regression in UserFeedbackQuerySet.visible_to_user (opencontractserver/shared/QuerySets.py)

The uncorrelated IN subquery forced the planner to materialise the entire annotations visibility table before probing it. The correlated Exists(...pk=OuterRef(...)) replacement has identical semantics and is measurably faster. The comment quantifying the regression (0.8s/evaluation, 3-min response on a 108-mention doc) is excellent context.

---

Core Feature

Architecture and permissions: Well-structured. Permission gating at CRUD (the same tier AddTemplateToCorpus and CreateCorpusAction require) is correct and the IDOR-collapse via _NOT_FOUND_MESSAGE matches the project pattern.

Idempotency design: The combination of fast-path dedup -> savepoint-wrapped clone -> IntegrityError recovery -> already_installed sentinel is solid. The _already_run_document_ids skip logic prevents re-queueing already-executed documents on repeat calls. Tests confirm both directions.

install_template extraction (corpus_actions.py): Good DRY move -- the install recipe now lives once and both AddTemplateToCorpus and the setup service use it. The refactored AddTemplateToCorpus mutation is simpler.

allow_partial=True in batch_run_action: Cleanly implemented and tested.

---

Issues / Concerns

1. DeleteMetadataColumn -- require_permission truthiness is non-obvious

config/graphql/extract_mutations.py:

if column is None or BaseService.require_permission(
    column, user, PermissionTypes.DELETE, request=info.context
):
    return DeleteMetadataColumn(ok=False, message=not_found_msg)

require_permission returns "" (falsy) on grant and a non-empty string (truthy) on denial -- the result is inverted relative to what if ... : looks like at first read. This is correct but subtly different from the documented idiom in base.py and from the same PR's intelligence_setup.py:setup(). Consider aligning:

error = BaseService.require_permission(column, user, PermissionTypes.DELETE, request=info.context)
if column is None or error:
    return DeleteMetadataColumn(ok=False, message=not_found_msg)

2. resolve_corpus_intelligence_setup_status skips @login_required

config/graphql/corpus_queries.py: Anonymous users can query setup status for publicly-visible corpora. The service correctly returns can_setup=False and None for invisible corpora, so there is no privilege escalation. But this is the only stats-type resolver in CorpusQueries that skips @login_required. If the intent is to show the banner to anonymous viewers of public corpora, a comment confirming that would clarify it is deliberate.

3. install_template race recovery (None, False) path lacks explanation

corpus_actions.py:

except IntegrityError:
    return (
        CorpusAction.objects.filter(corpus=corpus, source_template=template).first(),
        False,
    )

Practically impossible (the racing insert would have to be rolled back between the IntegrityError and the re-fetch), but the return type is tuple[CorpusAction | None, bool] and intelligence_setup.py silently skips on action is None. A brief comment on why None is theoretically possible but not expected here would help the next reader.

4. No test for canSetup: false variant in IntelligenceSetupBanner CT tests

The tests cover offer->run->hide and silent-when-fully-set-up. Missing: isFullySetUp: false but canSetup: false (user has READ but not CRUD). The banner presumably should hide the action button or render non-interactively. Without a test this case could regress silently.

5. remaining_count is not surfaced to the user

IntelligenceSetupBanner.tsx fires the mutation and shows a queued-document count, but remainingCount (documents deferred past the batch cap) does not appear anywhere in the UI. For a large corpus, users will not know setup is only partially complete. Consider a follow-up issue for a toast like "12 documents queued; 88 will be processed as they arrive."

---

Minor Nits

• VersionHistoryPanel.tsx: ID! -> String! for documentId/corpusId is a valid spec fix.
• GovernanceGraph.ct.tsx: removal of corpus fields from redirectMock is consistent with the query change removing corpus context from the document-by-ID redirect query.
• All changelog fragments are present and follow the fragment pattern correctly.

---

Test Coverage

• test_intelligence_setup.py (511 lines, 7+ tests): install/idempotence/permission gating/status/GraphQL smoke -- solid.
• test_batch_run_corpus_action.py: two new tests cover allow_partial in both directions. Good.
• IntelligenceSetupBanner.ct.tsx: offer->run->hide + silent-when-set-up. Missing canSetup: false (see above).
• TestServedValidationRulesIncludeSpecRules: pins the spec-rules fix with three cases.
• test_frontend_graphql_documents.py: validates all literal GQL documents against the schema at CI time -- excellent architectural guard.

---

Summary

Well-crafted PR. The three bundled bug fixes (spec validation disabled, AWS URL cache TTL wrong variable, feedback queryset N+1) are all real production correctness/performance issues. The main feature is cleanly layered, properly permission-gated, and idempotent by design. Main actionable items before merge: (1) align the DeleteMetadataColumn permission check to the standard idiom to avoid future misreads, and (2) add a canSetup: false CT test variant for IntelligenceSetupBanner. The remaining_count UX gap is a good candidate for a follow-up issue.

[claude]

Code Review

This is a high-quality PR with a well-designed core feature. The architecture is solid, the idempotency story is well thought through, and several significant bug fixes are bundled in. Below are my findings ordered by severity.

---

Critical / Must-fix

Nothing blocking. The permission model, IDOR prevention, and service layer conventions are all followed correctly.

---

Significant / Worth discussing

1. CorpusAction.get_or_create in _setup_reference_enrichment has no DB constraint backing it

intelligence_setup.py lines ~268–280:

action, created = CorpusAction.objects.get_or_create(
    corpus=corpus,
    trigger=CorpusActionTrigger.ADD_DOCUMENT.value,
    analyzer=analyzer,  # ← converged to one row by EnrichmentService
    defaults={"name": REFERENCE_ENRICHMENT_ACTION_NAME, "creator": user},
)

The code's own comment correctly notes: "No DB constraint backs this up (source_template is NULL) — a duplicate from a photo-finish race is wasted work, not corruption." Since EnrichmentService already converges the analyzer to a single row, the lookup fields (corpus, trigger, analyzer) are as tight as they can be. This is fine as long as the get_or_create tuple is unique — but since there's no unique_together, two concurrent setup() calls on a fresh corpus could still race into a double-insert.

Suggestion: This is survivable (idempotent writer), but if the schema doesn't already have a partial unique constraint on (corpus, trigger, analyzer) where source_template IS NULL, adding one (in a follow-up migration) would close the window entirely and make the comment redundant.

2. batch_run_action trusts its callers for permission verification (by convention only)

corpus_actions.py — the new batch_run_action classmethod is documented as a "trusted-caller variant" that skips the re-fetch + permission re-check. Both current callers (batch_run_on_corpus and _setup_templates) do verify before calling. But the method is public on CorpusActionService, so a future caller in config/graphql/ that forgets the check would bypass the permission gate silently.

Suggestion: Either rename to _batch_run_action (private convention) or add an assertion: assert cls.user_has(action.corpus, user, PermissionTypes.UPDATE). A leading underscore costs nothing and makes the intent clear.

---

Minor / Quality

3. is_fully_set_up relies on graphene's implicit attribute resolver

corpus_types.py — CorpusIntelligenceSetupStatusType declares is_fully_set_up = graphene.Boolean(required=True) but has no explicit resolve_is_fully_set_up. This works because graphene falls back to getattr(obj, "is_fully_set_up") and the dataclass has a @property, but it's non-obvious to a future reader who'll wonder where the value comes from.

Suggestion: Either add a one-liner resolve_is_fully_set_up = lambda self, info: self.is_fully_set_up resolver, or add a brief comment in the type class: # resolved from IntelligenceSetupStatus.is_fully_set_up @property.

4. IntelligenceSetupBanner has no loading skeleton — potential layout shift

IntelligenceSetupBanner.tsx:

const { data, refetch } = useQuery<...>(GET_CORPUS_INTELLIGENCE_SETUP_STATUS, ...);
// ...
if (!status || status.isFullySetUp || !status.canSetup) return null;

The !status guard means the banner renders nothing while the query loads, then abruptly appears. For IntelligencePanel, which already has its own stats-loading skeleton, this creates a visible layout shift after the panel's stats load (the banner area is zero-height, then expands). This is minor but noticeable on slow connections.

Suggestion: Render a fixed-height placeholder (matching the banner's height, ~52px) while !data to prevent the shift. Or accept the shift as a known limitation given this is a secondary UI element.

5. DeleteMetadataColumn and UpdateFieldset — no changelog fragment for new server-side mutations

These two mutations (extract_mutations.py) are new production API surface. They appear in the bulk "26 invalid documents fixed" changelog entry as a side note, but they're new write mutations that could warrant their own added fragment, especially since the frontend was calling them before they existed (i.e., the UI has had a silently-broken delete-field button).

Suggestion: Add a short added fragment for these, or expand the existing graphql-spec-validation.fixed.md entry's bullet to make it clearer that these were blocking UX fixes.

6. analyses query variable type change (ID → String)

queries.ts:

-query analysesForCorpusEnrichment($corpusId: ID) {
-  analyses(corpusId: $corpusId) {
+query analysesForCorpusEnrichment($corpusId: String) {
+  analyses(analyzedCorpusId: $corpusId) {

analyzedCorpusId is correct (was a broken argument before). But changing the variable type from ID to String is a bit unusual — the caller passes a Relay global ID string, which is semantically an ID. This works only if the backend resolver accepts a raw Relay ID string as a String argument. If the backend expects a decoded integer PK, the filter would silently return nothing. Worth verifying this is intentional (i.e., the resolver handles the raw global ID string) vs. the type should remain ID.

---

Praise / Highlights

These are genuinely good changes worth calling out:

• GraphQL spec validation fix (schema.py) is the most important fix in the PR. Silently discarding unknown fields/arguments allowed ~26 broken documents to reach production undetected. The [*specified_rules, ...] pattern is the right fix, the architectural test pins it, and the sweep test in CI will catch future regressions. Excellent.

• UserFeedback.visible_to_user correlated Exists (QuerySets.py): converting __in=<full-table subquery> to a correlated Exists is a real perf fix. The comment quantifying the before/after (~0.8s per evaluation → negligible) is exactly the kind of evidence-backed comment that belongs in production code.

• AWS presigned URL TTL fix (base.py): _AWS_EXPIRY (HTTP Cache-Control max-age, 7 days) vs. AWS_QUERYSTRING_EXPIRE (URL signature validity, 1 hour by default) is a genuinely easy-to-miss confusion. The fix plus the explicit clamp_shared_url_cache_ttl guard is correct and the comment explains why.

• CorpusActionService.install_template refactoring: extracting the savepoint-wrapped clone + IntegrityError recovery + CRUD grant into one place, shared by both AddTemplateToCorpus and the bundle setup, is a solid DRY improvement. The two code paths can no longer drift.

• can_setup on the status response (hiding the CTA from read-only viewers) is a clean design — the client shows "Set up" only to users whose click can actually succeed. Avoids a guaranteed-to-fail UX path.

• Non-@login_required status endpoint with the deliberate justification in the docstring is correct and well-documented. Anonymous viewers of public corpora get read-only status; can_setup is always false for them.

• Test coverage: the service tests exercise install, idempotency, permission gating, partial-batch cap, analyzer-absent path, and in-flight analysis dedup. The CT tests cover offer → run → hide, already-set-up silent, viewer-can't-setup silent, deferred-remainder toast, and partial-template-error. That's thorough for new greenfield feature code.

---

Nit

corpus_mutations.py SetupCorpusIntelligence.mutate — the bare except Exception: around from_global_id parsing swallows ValueError from a malformed ID silently (returns the generic failure_msg). This is intentional and correct for IDOR prevention, but the comment could say so explicitly rather than relying on the reader to infer it.

---

Overall this is merge-ready. The race-condition note (#1 above) is the only item I'd consider pursuing in a follow-up issue; the rest are suggestions. The bundle of concurrent fixes (spec validation, AWS TTL, N+1 queryset, 26 broken GraphQL documents) significantly improves the baseline even before the new feature is considered.

[claude]

Code Review: One-click Collection Intelligence Setup

Overall this is a well-structured PR. The new feature is coherent, the service layer follows project conventions, and the three bug fixes bundled here fix real production issues.

---

Bundled Bug Fixes

1. GraphQL spec validation was silently disabled (schema.py)

This is the most important fix in the PR. Passing only custom rules to validate() replaced (not extended) the spec rule set, so 26 invalid frontend documents were executing silently. The fix -- [*specified_rules, DepthLimitValidationRule] -- is correct, and the three new security-hardening tests plus the architectural test_all_literal_documents_are_schema_valid properly pin the invariant.

Worth calling out: the removal of $includeMetadata: Boolean! from GET_DOCUMENTS is a necessary cleanup -- it was declared but never used in the query body, triggering NoUnusedVariables once spec rules were re-enabled. Several callers (CorpusDocumentCards.tsx, Corpuses.tsx, SelectDocumentsModal.tsx) still pass includeMetadata as a runtime variable; those are now silently ignored and can be cleaned up in a follow-up.

2. AWS presigned URL cache TTL fix (settings/base.py)

_signed_url_lifetime_seconds was derived from _AWS_EXPIRY (7-day HTTP CacheControl max-age) instead of AWS_QUERYSTRING_EXPIRE (1-hour presign expiry), allowing the shared URL cache to serve dead 403 links for up to 5 hours. Fixed correctly.

Minor note: settings/base.py now imports from opencontractserver.utils.files at module level. Safe today (clamp_shared_url_cache_ttl is a pure stdlib function), but it sets a precedent -- if files.py ever imports a Django model, the settings module will break at startup. Consider inlining the two-liner in settings or extracting it to a separate stdlib-only module.

3. UserFeedbackQuerySet.visible_to_user IN -> correlated EXISTS (QuerySets.py)

The correlated Exists() fix is correct. The comment explaining the measured 0.8s evaluation cost on the old IN subquery is exactly the right documentation for a non-obvious performance change.

---

New Feature: CorpusIntelligenceSetupService

Service Layer

Follows project conventions: get_or_none + require_permission for the CRUD gate, deferred model imports, ServiceResult envelope, log_action for audit trail. Permission tier at CRUD (not UPDATE) is correct -- AddTemplateToCorpus and CreateCorpusAction already gate the identical writes at CRUD.

install_template extraction into CorpusActionService is a good DRY fix -- both AddTemplateToCorpus and the bundle now share the same fast-path dedupe, savepoint clone, IntegrityError recovery, and CRUD grant recipe. The allow_partial=True mode on batch_run_action is clean.

Idempotency

Thorough: action rows deduplicated, already-run documents skipped, in-flight reference analysis not duplicated, get_or_create on the reference enrichment action. The race-window comment on the non-atomic check+start for the analysis is honest and correct (enrichment writer is idempotent -- duplicate is wasted work, not corruption). The None guard on install_template return value in _setup_templates is present and correct.

remaining_count calculation

max(0, batch_total - skipped_already_run_count - queued_count) is correct. batch_total is the full active-document count (not the capped eligible set), so the result correctly represents documents deferred for a future call.

Minor: is_fully_set_up is a @Property, not a dataclass field

IntelligenceSetupStatus.is_fully_set_up is a @Property on the dataclass, but CorpusIntelligenceSetupStatusType exposes it as a plain graphene.Boolean. Graphene reads it via getattr which works for properties -- functional. Worth noting for anyone who adds dataclass-field introspection later.

---

GraphQL Surface

corpusIntelligenceSetupStatus (no @login_required)

The deliberate omission is well-justified in the docstring: anonymous users can reach the intelligence overview for public corpora, and can_setup mirrors the CRUD gate so the CTA never renders for users whose click would fail.

setupCorpusIntelligence mutation

Rate-limited at WRITE_HEAVY. try/except on from_global_id decoding returning the same opaque failure message is correct IDOR prevention.

createCorpus.objId

DRFMutation already exposes obj_id = graphene.ID() (camelCased to objId), so selecting it is safe.

---

Frontend

IntelligenceSetupBanner

Visibility logic is correct: hide on load, on error, when fully set up, and when canSetup is false. Toast messages correctly distinguish success-with-docs-queued, warning-with-template-errors, and generic success. await refetch() after the mutation correctly hides the banner once status flips.

Minor: the banner renders null during the initial status query -- a visible layout shift if the query takes >200ms. Not blocking.

GovernanceGraphLive bootstrap -- extra round-trip

The new fetchSetupStatus call before installing the reference action is correct (prevents double-installation), but adds a sequential network request. network-only means it cannot be served from cache even when IntelligenceSetupBanner already fetched it. Consider cache-first: the bootstrap fires right after the page renders when the banner query has likely populated the cache, and a stale hit that incorrectly skips installation is equivalent to pre-PR behaviour for users who already ran one-click setup.

useReferenceMentions -- client.query over useLazyQuery

Well-documented fix (observed live hang on the first awaited lazy query re-execution). client.query for an imperative await-in-a-loop is the right call.

Mutation type fixes (ID! -> String!, conversation -> obj, etc.)

All verified: PinThread/UnpinThread returns obj (confirmed via moderation_mutations.py); UpdateFieldset returns message (not msg); UpvoteMessage/DownvoteMessage/RemoveVote argument type is String server-side.

---

Test Coverage

Good overall:

• 7 service tests: install, idempotence, CRUD gate, no-analyzer path, status query, GraphQL smoke.
• 2 CT tests: offer->run->hide and silent-when-set-up.
• allow_partial=True batch run test added.
• TestServedValidationRulesIncludeSpecRules pins the spec-rule fix.
• test_all_literal_documents_are_schema_valid architectural sweep.

One gap: no test for the no-enrichment-analyzer path through setup() (where reference_available stays False and only the LLM half runs). IntelligenceSetupStatusWithoutEnrichmentTestCase covers the status query but not this setup path. Low risk -- fallback is log + continue -- but worth a follow-up.

---

Summary

The three bundled bug fixes are important (spec validation regression, AWS TTL bug, IN-subquery perf). The new feature is well-designed and follows project conventions. No blockers. Most actionable suggestions: the settings import precedent and switching the GovernanceGraph status fetch to cache-first.

[claude]

Code Review — PR #1982: One-click collection-intelligence setup

The PR is well-structured with a clean service-layer composite. Idempotency design and fault-tolerance are solid. Below are findings ordered by severity.

---

Security

[MODERATE] Double-permission check in setup() — diverges from canonical pattern

intelligence_setup.py ~lines 179–190: setup() calls get_or_none(Corpus, corpus_pk, user) (READ gate), then immediately calls require_permission(corpus, user, PermissionTypes.CRUD). No bypass is possible because the CRUD check always runs, but this diverges from the codebase pattern. Canonical fix:

corpus = cls.get_or_none(Corpus, corpus_pk, user, PermissionTypes.CRUD, request=request)
if corpus is None:
    raise PermissionError(...)

This eliminates the separate require_permission call and collapses both into one IDOR-safe lookup — the same pattern status() already uses correctly at ~line 118.

[LOW] reference_analysis_started = False is misleading on the already-in-flight path

intelligence_setup.py ~lines 313–324: when in_flight is True, the method returns early with summary.reference_analysis_started = False. Both "start failed" and "weave already running" return the same summary state, so the frontend toast silently omits the "reference web weaving" note even though analysis IS running. Recommend one of:

• Add a reference_analysis_already_running: bool field to IntelligenceSetupSummary, or
• Set reference_analysis_started = True for the in-flight case (the analysis is running, just not started by this call).

---

Architecture / Pattern Adherence

[MODERATE] batch_run_action calls corpus._get_active_documents() directly — bypasses service layer

corpus_actions.py ~line 121:

active_doc_ids = set(corpus._get_active_documents().values_list("id", flat=True))

_get_active_documents() is a private model method (leading _). CLAUDE.md designates CorpusDocumentService.get_corpus_documents(user, corpus) as the canonical pipeline-facing caller for operations over a whole readable corpus. This is a pipeline-facing batch operation. Not an IDOR risk, but the coupling to a private method is fragile and violates the service contract.

[MINOR] install_template trust assumptions undocumented

corpus_actions.py ~lines 202–256: the method is a trusted-caller method (callers are responsible for pre-verifying template.is_active and corpus write permission), but the method docstring doesn't say so. Callers currently do verify correctly, but a future caller may not know.

---

Performance

[MODERATE] status() executes 6 DB queries per call

intelligence_setup.py ~lines 95–154: each render of the intelligence panel fires ~6 queries (corpus lookup, analyzer lookup, action existence checks, template queries, guardian permission). Acceptable for the current single-panel context, but this should be flagged before any corpus-list-per-row badge or polling surface is built on top.

[MINOR] _already_run_document_ids materializes full document ID set into Python memory

corpus_actions.py ~lines 259–275: a values_list over all QUEUED/RUNNING/COMPLETED executions materializes all IDs into a Python set. A partial unique index on (corpus_action, document, status) would eliminate the materialization. The inline comment already notes this — good, but worth calling out here for tracking.

---

Testing

[MODERATE] No TransactionTestCase test — on_commit Celery dispatch path is untested

test_intelligence_setup.py: all tests use TestCase. Under TestCase, transaction.on_commit callbacks never fire. The docstring acknowledges this (observable contract is queued CorpusActionExecution rows), but the analogous test_batch_run_corpus_action.py uses TransactionTestCase for exactly this path. Recommend adding a TransactionTestCase (or mock-on_commit) test verifying the Celery task actually dispatches.

[MODERATE] No GraphQL-layer IDOR test for setupCorpusIntelligence mutation

test_intelligence_setup.py: test_setup_requires_crud_permission tests the service layer directly, but there's no test that fires the mutation as a stranger (user without corpus access) and asserts ok=False plus an IDOR-safe error message. The pattern exists in test_security_hardening.py for analogous mutations. Should be added.

[MINOR] Magic string "migration_admin" in test setup

test_intelligence_setup.py ~line 67:

User.objects.get_or_create(username="migration_admin", defaults={"is_superuser": True, ...})

CLAUDE.md: no magic strings. Extract to a named constant.

[MINOR] test_mutation_and_status_query covers two concerns in one method

Fails are ambiguous — is the mutation broken or the status query? Split into test_setup_mutation and test_status_query_after_setup for clearer failure attribution.

[MINOR] IntelligenceSetupBanner CT tests lack a loading-state test

The component renders null while the status query is in-flight. No test covers this null-render case. Low-risk, but inconsistent with banner test patterns elsewhere.

---

Frontend

[LOW] CorpusModal setupIntelligence defaults to true on open — correct

CorpusModal.tsx ~lines 371, 487–507: the useState(true) default + the reset useEffect work correctly. The "default-on, user can opt out" intent is properly implemented.

[LOW] handleSetup early return inside try with finally — not a bug

IntelligenceSetupBanner.tsx ~lines 135–140: return inside a try block still runs finally, so setSubmitting(false) fires in all paths. Counterintuitive but correct.

---

Actionable Recommendations (priority order)

1. Fix double-permission pattern in setup(): replace get_or_none(READ) + require_permission(CRUD) with a single get_or_none(Corpus, corpus_pk, user, PermissionTypes.CRUD, request=request).
2. Add GraphQL-layer IDOR test for the setupCorpusIntelligence mutation (stranger calls → ok=False + IDOR-safe message).
3. Fix reference_analysis_started misleading state for the already-in-flight case.
4. Migrate corpus._get_active_documents() in batch_run_action to CorpusDocumentService.get_corpus_documents(user, corpus).
5. Add TransactionTestCase test to verify Celery dispatch wiring fires from on_commit.
6. Extract magic string "migration_admin" to a named test constant.
7. Split test_mutation_and_status_query into two separate methods.

---

No issues found in: core IDOR protection (CRUD gate always enforces), idempotency design, partial-batch fault tolerance, two changelog fragments for two change categories, can_setup=False for anonymous users, createCorpus returning objId for mutation chaining.

[claude]

Code Review — PR #1982 «One-click collection-intelligence setup»

Overview

This is a large, multi-concern PR (3 759 additions / 426 deletions, 61 files). It ships the headlining CorpusIntelligenceSetupService feature but also bundles several significant bug fixes that piggyback on the investigation work:

Concern	Size
New: CorpusIntelligenceSetupService + GraphQL + frontend banner + CorpusModal opt-in	~1 700 lines
Fix: GraphQL spec validation disabled on the live endpoint (security)	~100 lines
Fix: PDF inline citations stored as wrong annotation type	~300 lines
Fix: UserFeedbackQuerySet correlated EXISTS vs IN (3-minute queries)	~30 lines
Fix: AWS presigned-URL cache TTL derived from wrong constant	~40 lines
Fix: 26 invalid frontend GraphQL documents	~400 lines
Fix: useReferenceMentions lazy-query promise not settling	~60 lines
New: DeleteMetadataColumn, UpdateFieldset mutations	~150 lines

The bundling is understandable since the fixes were discovered during the feature work, but it makes the review surface area very wide. Calling that out — not a blocker.

---

Security / Critical

GraphQL spec validation was disabled on the live endpoint — fixed here. The change in config/graphql/schema.py from validation_rules: list = [DepthLimitValidationRule] to validation_rules: list = [*specified_rules, DepthLimitValidationRule] is the correct fix. graphql-core's validate() replaces the default ruleset when an explicit list is passed; the missing *specified_rules silently disabled every standard validation (unknown fields, argument types, etc.) in all environments. The test TestServedValidationRulesIncludeSpecRules pins this correctly. Ship this fast.

AWS presigned-URL cache TTL was derived from _AWS_EXPIRY (7-day HTTP CacheControl max-age) instead of AWS_QUERYSTRING_EXPIRE (1-hour presign lifetime), causing cached PDF/PAWLS links to serve dead 403 responses for up to 5 hours. The fix is correct, clamp_shared_url_cache_ttl is well-tested, and the regression guard added to test_files_utils.py is appropriate.

---

Service Architecture

The CorpusIntelligenceSetupService is well-designed:

• Idempotency is handled at every level: install_template uses a fast-path dedupe + savepoint clone + IntegrityError race recovery, batch_run_action skips already-executed documents, and an in-flight analysis is not duplicated.
• Permission gating is correctly pegged at CRUD (the same tier AddTemplateToCorpus requires), and the IDOR-unified not-found message is used throughout.
• Partial-success (allow_partial=True) propagates remaining_count all the way to the banner toast, so large corpora don't silently appear fully-enqueued when only the first batch was queued.
• status() without @login_required is intentional and correctly documented: anonymous viewers of a public corpus see read-only status, and can_setup is derived from CRUD (which anon never holds).

---

Issues to Address

1. install_template returns (None, False) on a pathological race — verify the caller handles it

In corpus_actions.py, when an IntegrityError race happens and the re-query returns nothing, the method returns (None, False). The comment says "practically impossible" but the return type acknowledges | None. The comment in the diff says intelligence_setup.py skips on action is None, but this couldn't be confirmed in the visible portion of the diff. Please confirm (or add a comment cross-referencing) that _setup_templates guards against action is None before calling batch_run_action.

2. clamp_shared_url_cache_ttl import at settings module level

config/settings/base.py imports this with # noqa: E402 and a comment saying "pure helper, no app/model imports at module level." This is true today, but files.py has grown and is likely to keep growing. A future import there that pulls in a model or signal will silently break startup with a cryptic error at manage.py load time. Consider inlining the two-liner (max(0, min(requested, signed_lifetime // 2))) or at minimum adding a comment that warns contributors to keep files.py import-safe at settings load time.

3. GovernanceGraphLive.tsx adds a sequential network round-trip on "Map the reference web"

Before creating the add_document corpus action, the component now fetches corpusIntelligenceSetupStatus with fetchPolicy: "network-only". This is correct for correctness (can't tolerate a stale "not installed" cache) but adds a sequential hop to a flow that previously fired one mutation. Since the server-side createCorpusAction already uses get_or_create, you could let the server absorb the race and skip the client check entirely. The existing behavior is safe and correct as-is; this is a latency note, not a bug.

4. Missing test: partial-batch through a full setup() call

test_over_cap_allow_partial_queues_up_to_cap tests batch_run_action(allow_partial=True) in isolation. There is no test that calls CorpusIntelligenceSetupService.setup() on a corpus with more than BATCH_RUN_MAX_DOCS active documents and asserts that remaining_count is propagated in the summary. This path is exercised indirectly via the per-template outcome but a direct corpus-level test would be more explicit.

5. Default-on intelligence setup in CorpusModal

const [setupIntelligence, setSetupIntelligence] = useState(true);

Defaulting to true is reasonable UX but will surprise operators of tight-budget or non-LLM deployments (where the agent runs have real cost). The checkbox label is clear, but consider whether this should be gated on whether the bundle templates are actually active/available on the deployment (derivable from corpusIntelligenceSetupStatus.missingTemplateNames). Alternatively, a note in the deployment docs is sufficient. Not a code bug — just raising it.

---

Minor / Polish

• status() fires 5-6 separate DB queries per call. The docstring acknowledges this. Acceptable now; flag if it's ever rendered in a corpus list.
• IntelligenceSetupBanner fires useQuery unconditionally on mount. The banner renders null once fully set up but still pays the query cost. Low impact — the query is fast and the server-side visibility gate ensures no leaked data. No change needed now.
• Toast text math: queued in the success toast counts across all templates (templates.reduce((sum, t) => sum + t.queuedCount, 0)). If two templates each queue 12 docs, the toast says "24 document enrichment runs queued" — accurate but potentially surprising to users expecting document-level counts. Fine as-is.
• CorpusIntelligenceSetupStatusType exposes isFullySetUp backed by a Python @property. Graphene resolves @property via getattr, so this works correctly. A short comment in the type class would help future maintainers understand why it isn't a resolve_* method.

---

Test Coverage

Coverage is solid for a feature this size:

• test_intelligence_setup.py: install/idempotence/permission-gating/status + GraphQL smoke
• IntelligenceSetupBanner.ct.tsx: offer -> run -> hide + silent-when-set-up
• TestServedValidationRulesIncludeSpecRules: pins spec validation (3 cases)
• TestVisibilityQueryShape: pins correlated EXISTS form
• SharedUrlCacheTtlTests: 4 unit tests for the cache clamp
• test_over_cap_allow_partial_*: two cases for partial-batch mode

Gaps: no integration test for the post-create setup chain in Corpuses.tsx (the fire-and-forget .then()/.catch()) — but this is UI-flow territory, acceptable without a CT test.

---

Summary

The core changes are well-executed. Three things worth resolving before merge:

1. Confirm intelligence_setup.py guards the action is None return from install_template.
2. Consider defusing the settings-level import of clamp_shared_url_cache_ttl (or add a fragility warning comment).
3. Consider the default-on intelligence setup opt-in vis-a-vis LLM cost on deployments where bundle templates aren't active.

Everything else is polish or acknowledged trade-offs. The security fix (spec validation) and the 3-minute query fix alone justify landing this quickly.