Skip to content

okf-record-model-v2 — CRITIC CORRECTIONS (supersede before driving any Task)

okf-record-model-v2 — CRITIC CORRECTIONS (supersede before driving any Task)

Section titled “okf-record-model-v2 — CRITIC CORRECTIONS (supersede before driving any Task)”

The adversarial critic (SOUND_WITH_CAVEATS) found real defects in okf-record-model-v2.md. These corrections supersede the named sections. Do NOT ratify the v2 record-identity recipe as written.

BLOCKERS (must resolve before ratifying the irreversible seed)

Section titled “BLOCKERS (must resolve before ratifying the irreversible seed)”
  1. Record-identity seed in §2.3 is collision-prone + the namespace claim is FALSE.

    • v2 recommended source_documents.id = uuid5(tenant_ns, 'sd:'+content_hash). Two distinct documents with identical content (same policy PDF in two folders/workspaces — common) collide to ONE uuid → second UPSERT overwrites first → silent data loss. content_hash ≠ identity.
    • _KH_PIPELINE_DOC_NS is a single global constant (flow.py:1640), NOT tenant-scoped — so there is no tenant discriminator; identical content across tenants also collides.
    • Correct seed must combine: per-tenant namespace + per-document discriminator (workspace_id + logical locator) + version hash. Design + verify in Task B BEFORE ID-45.
  2. Moving source_documents OFF rel_path discards DELIBERATE re-ingest idempotency.

    • rel_path seed is by-design (flow.py:1630-1640): re-ingesting the same file (even edited) mints the SAME PK → declare_row UPSERTs in place (INV-4). content_hash seed turns every edit into a NEW row needing parent_id version-threading via read-before-write — logic that does not exist today and complicates cocoindex’s memoised delta model.
    • rel_path is arguably the MORE correct identity for source_documents. The “rel_path is THE offender” framing over-generalises a q_a_pairs migration problem onto source_documents.
    • Net: the seed is per-record-type, and the source_documents seed is a genuine trade-off (idempotency-in-place vs edit=new-version vs base-path-move resilience vs collision-safety) — not a settled “switch to content_hash.”
  3. content_items elimination consumer-surface is UNDERSTATED (“swap is invisible” is false).

    • hybrid_search (squash:3911) selects ~20 fields FROM content_items and RETURNS that shape; filter_by_keywords RETURNS SETOF content_items; find_related_items + get_content_win_rate return content_items-derived shapes. Eliminating the table forces re-authoring this search-RPC family as a polymorphic UNION (source_documents + content_chunks + q_a_pairs + reference_items) with a CHANGED result shape + the TS consumers (search.ts:116,280,320) + cite_content (procurement.ts:463-472). Direction is right; “code refactor, not data migration” is fair on the data axis but understates the consumer-contract rewrite.
  • IMS DROP columns are NOT all contract-free: platform, author_name, source_domain, thumbnail_url, priority are in hybrid_search’s RETURNS TABLE; user_tags in find_related_items. Dropping requires updating those RPC signatures + consumers — not a silent DROP.
  • FK inventory stale: source_document_diffs was already DROPPED (id-117, 20260623004320). Real count = 10 enforced + 2 unenforced FKs, not 11+2. Re-derive from current head, not the squash.
  • Audit quartet omitted from the §3.1 re-home table: created_at/updated_at/created_by/updated_by. created_by is returned by hybrid_search → map created_by → source_documents.uploaded_by explicitly.
  • content_text_hash conflation: content_items.content_text_hash is a GENERATED md5 of NORMALISED text (dedup); source_documents.content_hash is a plain file hash (different semantics). Mapping one onto the other loses the normalised-text dedup that find_exact_duplicates/dedup_status rely on.
  • Manual/TS authoring path (app/api/items, ~58 cols, gen_random_uuid) has no defined seed; “no manual create verb needed” is an unverified product assertion — confirm before assuming.
  • “A Q&A pair would NEVER be an OKF concept” is a design judgement stated as absolute (well-supported by shipped bundles + the client’s own topic index, but not spec-mandated).
  • Task A also owns an irreversibility (the resource-URI scheme emitted into the client-owned bundle) — not SOLELY Task B.

ALREADY CLEARED this session (critic flagged as unverified; we verified)

Section titled “ALREADY CLEARED this session (critic flagged as unverified; we verified)”
  • Zero data lock-in: prod content_items=33 are [PUB-BULK] E2E debris (0 chunks); Phew’s 631 are legacy pre-cocoindex (platform col set, 0 source_documents). Full-replace re-ingest on empty preview branch. CI-DECOMP’s “no data migration” premise holds.