S352 — Adversarial pre-ratification review of OQ defaults (ID-109, ID-110, ID-59 {59.19})

S352 — Adversarial OQ default review (ID-109 / ID-110 / ID-59 {59.19})

Review-only. No spec file edited. Each verdict is grounded in a file:line against the canonical-pipeline-setup working tree (commit 879fb7f77). Three specs already passed a spec-internal-consistency Checker pass; this pass is deliberately skeptical about whether each encoded DEFAULT is best practice vs a tech-debt / rework / data-quality / lock-in trap.

Verdict table

Spec	OQ	Default (one line)	Verdict	One-line rationale
ID-109	OQ-A mixed-provenance	Named-3rd-party + first-person possessive in one sentence ⇒ abstain-to-null	DEFAULT-SOUND	Abstain is the safe floor for a precision-first rule; mis-attributing a quoted supplier cert to self is the worse failure, and the holder map already drops unresolved sources to null.
ID-109	OQ-B possessive breadth	our/we/our own fire; bare in-house abstains	DEFAULT-SOUND	Conservative trigger set matches the abstain-to-null safety posture; recall loss is cheap (a doc author can add “our”), false-self is expensive and silent. Prompt-only dial, zero migration cost to widen later.
ID-109	(silent) holder_basis in JSONB, unindexed	holder_basis rides entity_mentions.metadata JSONB, no column/index	DEFAULT-SOUND	Consumption is explicitly out-of-scope; only expiry_date is JSONB-indexed today and holder itself is filtered in app code, so the new breadcrumb matches the existing access pattern. Flagged for the future consumption Task.
ID-110	(a) body-of-record gap	Manual refs get Readability/unpdf text, NOT PullMD markdown	NEEDS-LIAM-INPUT	Two body-of-record extraction paths for one reference_items.body column is a genuine data-quality divergence; whether feed-vs-manual body parity matters is a product call, not an engineering one. Identity is stable so a later re-fetch UPSERTs in place — the debt is reversible.
ID-110	(b) no reference-detail nav	Success card shows copyable id only, no “view item” link	DEFAULT-SOUND	Verified: zero reference-detail page/route exists and the ID-75 reference RPCs have zero UI consumers, so any link would 404. Copyable id + omit-dead-link is the only non-broken interim. Follow-on correctly filed.
ID-110	(c) lighter manual classification	primary_domain/primary_subtopic left null when not cheaply available	CHANGE-RECOMMENDED	reference_search already projects both columns; classifier + embedding are already being computed in-request, so the marginal cost of populating domain/subtopic is near-zero — null-when-inconvenient creates an avoidable faceting/display gap on exactly the rows a user just chose to add.
ID-110	(silent) extraction_method=NULL write	RPC writes extraction_method=NULL, true producer in extraction_metadata JSONB	DEFAULT-SOUND	Verified the ID-42 CHECK admits neither readability nor unpdf; NULL + JSONB provenance avoids a CHECK violation without coupling reference provenance to the legacy app extractor. (TECH mis-quotes the enum — see Silent defaults.)
ID-59	OQ-1 auto-batch vs curated	Auto-promote corpus extractions → published in batch	DEFAULT-SOUND	ID-45 INV-23 requires post-run search-visibility with no human in the loop; a curated queue cannot satisfy the cutover gate. Curated-as-v1.1-opt-in on the same primitive is the right layering.
ID-59	OQ-2 stale-pair retirement	On extraction invalidation, leave the published pair live (no active retirement)	NEEDS-LIAM-INPUT	Leave-published trades stale-corpus-answers risk against silent-corpus-loss risk; which is worse for a Q&A surface consumers trust is a product judgement. Schema (superseded_by, archived) already supports either choice, so it is not engineering-blocked.
ID-59	OQ-3 embedding-failure	Surface-and-continue; set promoted_to_pair_id even on embed fail; count the shortfall	CHANGE-RECOMMENDED	Setting promoted_to_pair_id on a pair that never got an embedding makes the failure permanently sticky under the idempotency rule (INV-5) — it is never retried. Decouple the linkage from embedding success (or make embed-failed rows re-eligible) so a transient embedding outage is self-healing on the next run.

---

Prose — CHANGE-RECOMMENDED and NEEDS-LIAM-INPUT items

ID-110 (a) body-of-record gap — NEEDS-LIAM-INPUT

Default. Manual single-URL references get their reference_items.body from the in-request extractFromUrl (Readability for HTML, unpdf for PDF — confirmed lib/extraction/url.ts, extractionMethod: 'readability' | 'unpdf'), whereas the feed path’s body of record is PullMD/Docling markdown. Two extraction paths fill one column.

Why this is a real divergence, not a non-issue. Verified there is genuinely no TS PullMD client (PullMD is a Python-only httpx client, scripts/cocoindex_pipeline/adapters.py:_pullmd_http_get:169-249; grep for pullmd/PULLMD over lib/+app/ .ts returns nothing) and uuid is not a direct npm dependency (grep -c uuid package.json = 0). So the spec’s premise — building a TS PullMD client is materially larger than a re-point — is correct. The adversarial concern is downstream: reference_items.body is the verbatim substrate the ID-75 reference layer is supposed to make trustworthy, and Readability/unpdf text is a measurably lower-fidelity body than PullMD markdown (table structure, heading hierarchy, boilerplate stripping). A consumer doing reference_get_verbatim cannot tell which fidelity tier a given reference is — the only provenance signal is source_documents.extraction_metadata.extractor, which no consumer reads.

Why it is a product call, not an engineering one. The TECH spec’s mitigation is architecturally sound and makes the debt reversible: the ri:/sd: uuid5 identity is deterministic (flow.py:2710-2712, namespace _KH_PIPELINE_DOC_NS at flow.py:1601), so a future PullMD re-fetch UPSERTs the same row in place with zero row churn (BI-2). The question “is a temporary fidelity gap between manual and feed references acceptable for v1” is a judgement about how much users will rely on manual-reference body fidelity before the follow-on lands — that is Liam’s call.

Recommendation for Liam. Ratify as-is if manual references are understood as “evidence pointers” (the URL + title + summary carry the weight, the body is secondary) for v1. If manual-reference body is expected to be quotable at feed parity, the divergence should block until the PullMD follow-on, because the stable-identity re-fetch argument only helps if the follow-on is actually scheduled. No engineering change either way — purely a sequencing/expectation decision.

ID-110 (c) lighter manual-path classification — CHANGE-RECOMMENDED

Default. “Pass the classifier output when cheaply available, else null” for primary_domain / primary_subtopic (TECH §2 step 3); both columns are nullable.

Adversarial analysis. The “cheaply available” escape hatch is doing a lot of work, and the cost asymmetry argues against it. The route is already running generateEmbedding in-request (TECH §2 step 3, kept for ri.embedding) and already has classifyContent available in the module. Verified that reference_search projects primary_domain and primary_subtopic in both its output contract and its verbatim variant (20260606130224_id75_reference_search_rpcs.sql:85-86, 122-123, 139-140, 192-216) — they are not WHERE-clause filters today, so a NULL does not break retrieval, but it does degrade (i) any domain/subtopic facet or badge a reference-detail UI will want (the OQ-N follow-on), and (ii) any future “references in domain X” query, which would silently miss exactly the manually-added rows. Because the feed path classifies inline (extract_classification), null-on-manual also creates a populated-for-feed, null-for-manual skew in the same column — the precise shape that makes later analytics quietly wrong.

Proposed alternative. Make domain/subtopic population the default for the manual path too (run the classifier’s domain/subtopic extraction unconditionally, the same call the feed path makes), and only fall back to NULL on an actual classifier error — i.e. invert the default from “null unless cheap” to “populate unless it fails”. Keep the columns nullable (error tolerance), but stop electively leaving them null.

Impact estimate. Small — one extra classifier call on a path that is already doing an embedding round-trip and is now faster than today (it dropped inferLayer/suggestTopic/ suggestGuideSections/quality-score). No schema change. The TECH already imports classifyContent; this is a few lines in the route plus a test asserting non-null domain/subtopic on a classifiable URL. If a future reference-classify backfill is built (already filed as a follow-up), this default still reduces the backfill’s surface.

ID-59 OQ-3 embedding-failure policy — CHANGE-RECOMMENDED

Default. On embedding failure for a promoted pair: surface-and-continue (don’t abort the batch), but still set promoted_to_pair_id on the source extraction so it “is not re-attempted blindly”, and surface the failure count in the batch summary (PRODUCT INV-11

• OQ-59-19-3).

Adversarial analysis — this default manufactures permanent silent invisibility. The idempotency contract (INV-5/INV-8, verified: q_a_extractions.promoted_to_pair_id IS NULL is the sole “eligible” predicate — the table PK is gen_random_uuid() at 20260520225456_t6_q_a_pairs_full_schema.sql:104, so there is no deterministic-key fallback; the boolean flag is the only idempotency mechanism) means: once promoted_to_pair_id is set, that extraction is never reconsidered. So an extraction whose embedding failed on a transient outage (rate limit, network blip on lib/ai/embed) gets a pair that is forever question_embedding IS NULL ⇒ forever invisible to q_a_search (predicate verified verbatim, 20260520231524_t6_q_a_search_rpcs.sql:117-118: WHERE qap.question_embedding IS NOT NULL AND qap.publication_status = 'published'), and the next route-i run will skip it because the link is set. The failure is surfaced once in that run’s summary and then becomes invisible debt — a re-run does not re-surface it. For a feature whose entire purpose is satisfying ID-45 INV-23 (“post-run search-visibility”), baking in a class of pairs that are published-but-invisible-and-never- retried is the exact anti-pattern the spec is trying to prevent.

Proposed alternative. Decouple the lineage write from embedding success. Two clean options, pick one in TECH:

• (preferred) Don’t set promoted_to_pair_id until the pair has a non-NULL embedding. On embed-fail, either don’t create the pair at all (extraction stays eligible → retried next run) or create it draft+unembedded and leave the extraction unlinked so the next run re-attempts the embed and links on success. This makes a transient embedding outage self-healing on the next route-i run.
• (alternative) Keep the link but make embed-failed pairs re-eligible via a distinct predicate (e.g. promote eligibility = invalidated_at IS NULL AND (promoted_to_pair_id IS NULL OR linked_pair.question_embedding IS NULL)), so re-runs retry the embedding for linked-but-unembedded pairs.

Impact estimate. Logic-only, no schema change (the columns already exist). It is a change to the eligibility predicate / link-timing in the {59.20} TECH and route-i implementation — a handful of lines plus a test: “an extraction whose embed failed is re-attempted (not skipped) on the next run, and becomes search-visible once the embed succeeds.” This is the difference between a robust promotion primitive and one that silently leaks invisible pairs under load.

ID-59 OQ-2 stale-pair retirement — NEEDS-LIAM-INPUT

Default. When cocoindex invalidates an extraction after its pair was published, route-i leaves the stale pair published (records superseded_by lineage where the schema allows) and defers active retirement to v1.1 / a reconciliation step (INV-9 + OQ-59-19-2).

Why it is genuinely a product tradeoff. The schema supports either choice — verified q_a_pairs.publication_status admits 'archived' and superseded_by is a UUID lineage column (20260520225456...:81, 87-92; current enum confirmed draft/in_review/published/ archived after the ID-64 rename, 20260601180058_id64_origin_kind_rename_form_response.sql). So this is not engineering-blocked; it is a judgement about which failure a Q&A surface should prefer:

• leave-published (default): consumers may get a stale answer (the source content changed) — but corpus coverage never silently drops. Risk = wrong-but-present.
• active-retire: consumers never see a known-stale answer — but a re-extraction that produces no replacement (e.g. the new content has no usable answer text, which INV-7 skips) silently removes a previously-available answer. Risk = silent-absence.

For a Q&A surface that users and MCP consumers query for authoritative answers, “stale but present” vs “correct but possibly missing” is a product-risk posture, not a best-practice default an engineer can pick. The default is defensible (no silent corpus loss is the conservative Q&A posture), but it should be a conscious ratification, because the stale window is unbounded until the (unspecified) reconciliation step exists.

Recommendation for Liam. Confirm leave-published and put a bound on the staleness window — at minimum, have route-i set superseded_by on the old pair when it creates the new one (the lineage the default already mentions “where the schema allows”), so a thin later reconciliation can find-and-archive deterministically. Without that linkage written at promotion time, the v1.1 reconciliation has no cheap way to pair old↔new.

---

Silent defaults the specs encode (not captured as OQs, should be ratification-visible)

1. ID-110 — TECH mis-quotes the extraction_method CHECK enum (documentation accuracy, not a logic bug). TECH §Context line ~134 quotes the ID-42 enum as ['pullmd_readability','pullmd_playwright','pullmd_cloudflare','pullmd_reddit','pullmd_trafilatura','docling']. The actual constraint (20260526074944_id42_pullmd_provenance.sql:24-26) is 'rss_content','fetch','jina_reader','firecrawl','summary_fallback','pullmd_readability', 'pullmd_playwright','pullmd_cloudflare','pullmd_reddit','pullmd_trafilatura','docling' — five values are missing from the spec’s quote. The load-bearing conclusion is unaffected (neither 'readability' nor 'unpdf' is admitted, so the NULL + extraction_metadata workaround is correct), but the Executor should read the migration, not the spec’s quote, when writing the parity assertion.

2. ID-110 — source_documents NOT NULL columns the RPC must always populate. The RPC signature passes p_filename, p_mime_type, p_file_size, p_content_hash and the table requires filename, original_filename, mime_type, file_size, content_hash NOT NULL. The route derives filename as “last path segment or host” — for a URL like https://host/ (no path) this is the host; for a query-only URL it must not be empty. Not an OQ, but a silent-empty-string risk the Executor must guard (a NOT NULL violation would 500). Worth a one-line test: a path-less URL still lands a non-empty filename.

3. ID-59 — route-i idempotency has no deterministic-key backstop, only the promoted_to_pair_id boolean, and there is no UNIQUE constraint preventing two pairs from one extraction. q_a_extractions.id is gen_random_uuid() (not uuid5-seeded like ID-110’s references), and there is no UNIQUE on (source_content_item_id) or on the extraction→pair link. INV-8’s “no duplicate pairs” therefore rests entirely on the read-then-write of promoted_to_pair_id being race-free. Two concurrent route-i runs (or a retried batch) could each read promoted_to_pair_id IS NULL and both insert a pair. The PRODUCT says the linkage “MUST be written in the same logical operation as the pair insert” (INV-5/INV-8) — TECH {59.20} must make that an atomic compare-and-set (e.g. UPDATE q_a_extractions SET promoted_to_pair_id=... WHERE id=... AND promoted_to_pair_id IS NULL as the guard, pair insert gated on the UPDATE affecting 1 row), or add a UNIQUE partial index, otherwise the idempotency invariant is only single-writer-safe. This is the single most important thing for {59.20} to nail and it is not surfaced as an OQ.

4. ID-109 — holder_basis is written into JSONB that nothing indexes or filters server-side (captured above as DEFAULT-SOUND, repeated here for ratification visibility). Verified the only JSONB-key index on entity_mentions.metadata is idx_entity_mentions_metadata_expiry on metadata->>'expiry_date' (20260416102457_pre_squash_reconciliation.sql), and metadata.holder is filtered in app code (app/api/certifications/route.ts:241,245), not SQL. So a future “show me internal-function-attributed certs” query would be an unindexed JSONB scan or an app-side filter. Acceptable now (consumption is out-of-scope), but the future consumption Task that reads holder_basis should decide between an expression index and promoting it to a column at that point — flag it so it is not forgotten.