{52.22} — Path-C catalogue idempotent re-run (UPSERT on natural key)

{52.22} — Path-C catalogue idempotent re-run (UPSERT on natural key)

Type: Focused implementation design note (sub-{N.4} scope) for the one remaining pending Subtask of Task ID-52. Companion to PRODUCT.md (Inv-20..Inv-25) and TECH.md (§2.7 Path C, §2.8 instance-side idempotency) in this directory. UK English throughout.

Authored against the live staging schema (turayklvaunphgbgscat) and the as-built Path-C helpers delivered in {52.14}. Every code/schema claim below is grounded with GitNexus + the live migration corpus — see §7 Verification.

---

1. Problem

Re-running Path-C cataloguing over the same form instance is not idempotent today. The write step (confirmAndWriteCatalogue, lib/catalogue/from-instance.ts:370-418) calls a bare supabase.from('form_template_requirements').insert(row) per confirmed row (from-instance.ts:401-404). A second run over the same instance re-classifies, re-embeds, re-confirms, and inserts a fresh set of catalogue rows — duplicating every requirement on the natural key. The catalogue form_template_requirements is the GLOBAL, reusable T10 read target (PRODUCT Inv-22, Inv-23); duplicates corrupt that read boundary and inflate match candidates.

This note makes the catalogue write idempotent: a re-run over the same instance yields zero net new rows and updates only changed fields, preserving row id (and thus the bid_questions.template_requirement_id FK — see §3).

2. The natural key

Natural key = (template_name, template_version, section_ref, question_number).

This is not a new invention — it is the already-existing DB unique constraint on the table. The original CREATE TABLE (pre-squash migration line 4522-4523) declared:

ALTER TABLE ONLY "public"."template_requirements"
    ADD CONSTRAINT "template_requirements_template_name_template_version_sectio_key"
    UNIQUE ("template_name", "template_version", "section_ref", "question_number");

renamed in the T2 split migration (line 247) to form_template_requirements_unique_section.

2.1 Why this key, not the alternatives

Candidate natural key	Verdict	Reason
(template_name, template_version, section_ref, question_number)	CHOSEN	The constraint already exists in the DB. It is the per-template, per-section, per-question-position identity — semantically “this requirement is the Nth question of section S of template T (version V)”. question_number and display_order are both set from field.sequence (from-instance.ts:297,307), so the key tracks reading-order position, which is the same stable secondary key TECH §2.8 chose for the instance side (ftf:{rel_path}:{sequence}). Consistent identity model across both sides.
requirement_type + matching_keywords	Rejected	Both are LLM-derived (classifyField, from-instance.ts:212-229). They are non-deterministic across runs — a re-classification may pick statement vs declaration, or reorder keywords — so they cannot be a stable identity. Worse, many distinct questions share a requirement_type, so it is not unique.
Normalised-text / content hash of requirement_text	Rejected	The same legitimate-edit problem TECH §2.8 calls out for the instance side: a typo fix or clarification revision to a question’s text MUST update the row in place, not orphan the old row and insert a new one. A text hash key would re-key on every edit, defeating idempotency precisely when the form is revised. Position (question_number) is stable across text edits; text-hash is not.
Per-template surrogate (template_id + sequence)	Rejected	form_template_requirements has no template_id / instance FK by design — the catalogue is global and reusable across workspaces and across re-seeding from different instances (Inv-23). It carries only the denormalised template_name / template_version / template_type. So a per-instance surrogate is not available on the catalogue row.

2.2 The NULL-distinctness gap (the one real correctness issue)

The existing constraint is plain UNIQUE (...) — i.e. NULLS DISTINCT (Postgres default; the DDL carries no NULLS NOT DISTINCT clause — verified §7). Two key columns are nullable:

• template_version text (nullable — pre-squash line 4145).
• question_number integer (nullable — pre-squash line 4149).

In NULLS DISTINCT semantics, two rows whose template_version is NULL and whose other three key columns are equal do NOT collide — Postgres treats each NULL as distinct, so ON CONFLICT finds no conflicting row and inserts a duplicate anyway. This silently breaks idempotency.

buildCatalogueRow today sets template_version: args.templateVersion ?? null (from-instance.ts:294) and main never passes a templateVersion (scripts/catalogue-from-instance.ts:219-227) — so every catalogued row currently has template_version = NULL, which is exactly the case that breaks ON CONFLICT. question_number is always set non-null (= field.sequence, an integer), so only template_version is the live hazard.

Resolution (no migration): the writer must emit a non-NULL template_version sentinel. buildCatalogueRow defaults template_version to a stable sentinel 'v1' (constant DEFAULT_TEMPLATE_VERSION) instead of null when no explicit version is supplied. A non-NULL version makes all four key columns non-NULL, so ON CONFLICT matches deterministically. The existing catalogue-standard-sq.ts already always sets a non-null TEMPLATE_VERSION (scripts/catalogue-standard-sq.ts:30-31,158), so this aligns Path-C with the proven hand-script.

Alternative (documented, NOT chosen for v1): a belt-and-braces migration converting the constraint to NULLS NOT DISTINCT would also close the gap and tolerate genuinely version-less rows. It is the safer long-term shape but is not required for {52.22} and introduces DDL + the db push cycle for a case the sentinel already covers. Recorded in §5 as the fallback if a future requirement needs NULL versions to be conflict-equal.

3. The UPSERT mechanism

Change the write from .insert(row) to .upsert(row, { onConflict, ignoreDuplicates: false }).

// lib/catalogue/from-instance.ts — confirmAndWriteCatalogue, replacing the .insert at L401-404
const CATALOGUE_CONFLICT_TARGET =
  'template_name,template_version,section_ref,question_number';

const upsertResult = await tryQuery(
  supabase
    .from('form_template_requirements')
    .upsert(row, { onConflict: CATALOGUE_CONFLICT_TARGET }),
  'form_template_requirements.upsert',
);

• onConflict target = the four natural-key columns, comma-joined, matching the existing form_template_requirements_unique_section constraint exactly.
• ignoreDuplicates: false (the supabase-js default for upsert) → on conflict, the row is UPDATEd, not skipped. This is what makes “updates only changed fields” hold.

3.1 Update vs preserve split

buildCatalogueRow already builds a full row from the (re-classified, re-embedded) instance field, so the UPSERT’s UPDATE branch overwrites every supplied column. The split is therefore which columns the row carries vs which the DB owns:

Column	On re-run	Why
id (uuid)	PRESERVE	UPSERT-on-conflict keeps the existing row’s PK. Critical: bid_questions.template_requirement_id FK-references form_template_requirements.id (verified §7) — a delete-and-reinsert would orphan or violate that FK. This is the catalogue-side analogue of TECH §2.8’s “UPSERT preserves row identity, clear-and-rewrite invalidates downstream FKs”.
created_at	PRESERVE	Not in the insert row (DB default now() only fires on INSERT). Original authorship timestamp survives.
updated_at	AUTO-UPDATE	The set_template_requirements_updated_at BEFORE-UPDATE trigger (pre-squash line 5227) bumps it on every UPDATE. No app action needed.
requirement_text, requirement_type, matching_keywords, matching_guidance, is_mandatory, section_name, description, word_limit_guidance, display_order, is_current, template_type	UPDATE	Re-classified / re-read from the current instance field. If the form was revised (e.g. question text edited, mandatory flag flipped), the catalogue row reflects the current content — same intent as TECH §2.8 instance-side “edit updates in place”.
requirement_embedding	UPDATE per policy (§3.2)	The expensive field; recompute is conditional.
template_name, template_version, section_ref, question_number	MATCH (key)	These ARE the conflict target — by definition equal on both sides of the conflict, so the UPDATE is a no-op on them.

3.2 Embedding-recompute policy — recompute on text change only

The embedding (text-embedding-3-large, dims 1024) is the costly field — one OpenAI call per row (generateRequirementEmbedding, from-instance.ts:239-272). Blindly recomputing on every re-run wastes spend and emits an identical vector when the source text is unchanged.

Policy: recompute the embedding only when the embed-input text changed. The embed input is ${question_text}\n\nKeywords: ${matching_keywords.join(', ')} (scripts/catalogue-from-instance.ts:217). Mechanism, in main’s per-field loop before classify/embed:

1. Read the existing catalogue row for this natural key (single select on the four key columns), if any.
2. Compute the candidate embed-input text from the current field + fresh classification.
3. If an existing row is found AND its stored embed-input is unchanged → reuse the existing requirement_embedding (skip the OpenAI call); still UPSERT the row so other changed fields (e.g. matching_guidance) update.
4. If no existing row OR the embed-input changed → call generateRequirementEmbedding and write the new vector.

Because matching_keywords is part of the embed input and is LLM-derived (non-deterministic), a strict “text change only” check keyed on question_text alone is cleaner and avoids spurious recomputes from keyword reordering. Recommended embed-change key: question_text only (the deterministic, human-authored signal), NOT the keyword-augmented string. Implementation may store a lightweight requirement_text comparison (already a row column) rather than persisting the raw embed-input — requirement_text IS field.question_text (from-instance.ts:299), so comparing the candidate requirement_text against the existing row’s requirement_text is the change signal. No new column required.

Simpler fallback if the pre-read is deemed not worth the round-trip: “always recompute”. It is correct (idempotent rows, just wasteful). The recommended policy is recompute-on-text- change because the OpenAI call is the dominant cost and forms are re-catalogued rarely-but- repeatedly. The executor brief makes the conditional-recompute the target and notes the always-recompute fallback as acceptable if the pre-read materially complicates the loop.

4. Migration requirement — NONE for the constraint

A DB unique constraint already exists (form_template_requirements_unique_section on the four natural-key columns — §2). No supabase migration new is required to create it. The {52.22} change is TS-side only: switch .insert → .upsert(onConflict) and emit a non-NULL template_version sentinel.

If — and only if — the team later decides version-less rows must be conflict-equal (the §2.2 alternative), the constraint would be re-declared NULLS NOT DISTINCT. That would be DDL and MUST follow the CLAUDE.md discipline: supabase migration new <name> + supabase db push against staging (turayklvaunphgbgscat), never mcp__supabase__apply_migration / execute_sql, and always cat supabase/.temp/project-ref + relink to staging before push. The exact DDL, recorded for completeness (NOT to be applied in {52.22}):

-- OPTIONAL fallback only — NOT part of {52.22}. Created via:
--   supabase migration new id52_catalogue_unique_nulls_not_distinct
ALTER TABLE public.form_template_requirements
  DROP CONSTRAINT form_template_requirements_unique_section;
ALTER TABLE public.form_template_requirements
  ADD CONSTRAINT form_template_requirements_unique_section
  UNIQUE NULLS NOT DISTINCT
    (template_name, template_version, section_ref, question_number);

The v1 {52.22} path takes the sentinel route and ships zero migrations.

5. Idempotency test definition

A new test in __tests__/lib/catalogue/from-instance.test.ts (extending the existing suite), behaviour-not-implementation per docs/reference/test-philosophy.md. The shared createMockSupabaseClient() chain is extended to record upsert calls (mirroring the existing _chain.insert usage at L165, L199, L216).

Idempotency invariant under test:

1. Zero net new rows on unchanged re-run. Given a confirmed set of N rows for one instance, confirmAndWriteCatalogue issues N upsert calls with onConflict = 'template_name,template_version,section_ref,question_number' — NOT insert. Asserts the write verb is upsert and the conflict target string matches the constraint columns exactly. (Real DB UPSERT semantics → N existing rows updated in place, row count unchanged.)
2. Non-NULL version in every written row. buildCatalogueRow with no templateVersion override produces template_version === 'v1' (the sentinel), never null — so the natural key has no NULL member and ON CONFLICT is well-defined. Asserts row.template_version is a non-empty string.
3. Changed field updates; unchanged embedding is preserved. With a stubbed existing-row read returning an unchanged requirement_text, the loop does NOT call generateRequirementEmbedding (assert the injected embed fn is not invoked) yet still UPSERTs the row (assert matching_guidance change flows through). With a changed requirement_text, the embed fn IS invoked (assert called once).
4. Existing gates still hold. The Inv-21 confirmation gate and Inv-24 auth gate behaviour is unchanged — re-run the existing auth/confirm assertions against the upsert path (a declined row issues no upsert; a viewer-role caller issues no upsert).

An optional integration assertion (gated behind the existing integration suite, real staging) re-runs the generic script twice over a seeded fixture instance and asserts SELECT count(*) FROM form_template_requirements WHERE template_name = <fixture> is identical across both runs, and that a single row’s id is stable across runs (proving the FK-preserving UPSERT). This is the end-to-end “zero net new rows” proof; the unit assertions above are the PR-blocking gate.

6. Scope boundary

In scope for {52.22}: the .insert → .upsert(onConflict) change in confirmAndWriteCatalogue; the non-NULL template_version sentinel in buildCatalogueRow; the conditional embedding recompute in scripts/catalogue-from-instance.ts:main; the idempotency tests; a one-line note in .claude/skills/catalogue-form-requirements/SKILL.md recording that re-runs UPSERT (so a re-catalogue is safe). No new migration (the constraint exists). No change to the classify/embed/auth/confirm contracts. No T10-matching touch (Inv-22 boundary).

7. Verification (code-intelligence grounding)

Claim	Tool	Result
Existing unique constraint columns	grep pre-squash migration L4522-4523	UNIQUE (template_name, template_version, section_ref, question_number) — plain (NULLS DISTINCT), no NULLS NOT DISTINCT clause.
Constraint renamed, still live	grep T2 migration L247	renamed to form_template_requirements_unique_section; not dropped.
template_version / question_number nullable	pre-squash L4145, L4149 + live list_tables (staging)	both nullable → NULL-distinctness gap real.
Catalogue id is FK target	mcp__supabase__list_tables (staging)	bid_questions.template_requirement_id → form_template_requirements.id — UPSERT must preserve id.
Current write is bare .insert	Read lib/catalogue/from-instance.ts:401-404	confirmed supabase.from('form_template_requirements').insert(row).
template_version defaults to NULL today	Read from-instance.ts:294 + scripts/catalogue-from-instance.ts:219-227	args.templateVersion ?? null; main passes no version.
Blast radius of the two edited fns	mcp__gitnexus__impact (upstream)	buildCatalogueRow + confirmAndWriteCatalogue both LOW risk, 1 direct caller each (scripts/catalogue-from-instance.ts:main), module Catalogue, 0 processes.
Caller wiring	mcp__gitnexus__context confirmAndWriteCatalogue	incoming: scripts/catalogue-from-instance.ts:main; outgoing: tryQuery, ensureAuthorisedForWrite.

No external-library API citations requiring pre-ratification import-checks (supabase-js .upsert/onConflict is the pinned major in use across the repo; OpenAI embedding + Anthropic SDK shapes unchanged from the as-built {52.14} helpers).

8. S316 re-review — hold RELEASED ({80.12} sentinel-collision check)

Liam’s S314 hold (pending the 80.2 spec chain) is RELEASED (S316, 05/06/2026). The {80.12} versioning decision landed as option (b) — supersession pointer columns on form_templates, no instance-side version strings anywhere (docs/specs/ID-80-forms-path-b/80.12-versioning-investigation.md §5, §11) — so the template_version='v1' sentinel collision flagged in that investigation’s §8 is structurally impossible: the catalogue sentinel remains the only template_version writer and keeps its “unversioned catalogue default” meaning, unchallenged. No guard (label namespacing / reserved-'v1') is required. This design is dispatchable unchanged. Live exercise still sequences after the S1 Path-B smoke (v1-completion-sequence.md Lane C).