fix(store): enforce same-KB provenance references - #832
Conversation
|
Closing pending author's final review of the PR set — will reopen once the series is finalized. |
|
I've approved the CI run — fork PRs need that here. The analysis behind this is strong, and three parts in particular are the work of someone who has been bitten before: enumerating every provenance edge with the row whose Four things before this can land. 1. DCO is failing. The commit carries no 2. This needs an issue and a decision record first. The convention in this repo is that a change touching the data model opens an issue and waits, and that design reasoning lives in 3. The write-path cost is asserted rather than measured. 38 triggers and 23 functions sit on provenance writes, and extraction writes facts, evidence and chunks continuously. "Clean ledgers see only trigger overhead" is the claim that needs a number here — a before/after on the extraction path, or on the existing 100k bench. This repo settles this kind of question by measuring, and a permanent write-path tax is exactly the kind that should not go in on an estimate. 4. Please record why triggers rather than composite foreign keys. One operational note for whoever merges this: §0 fails the migration closed on a ledger that already holds cross-KB rows. That is the correct behaviour, but it means the precondition scan should be run against real bases before this lands, so an upgrade does not stop halfway on a base nobody had checked. |
e114f60 to
de4524f
Compare
|
All four points addressed in the pushed update (
Your operational note is recorded in the record as well: the §0 scan is the pre-upgrade audit to run against real deployments so the migration does not stop halfway on an unchecked base. |
|
All four points answered, and the third and fourth answered better than I asked. Converting 26 of the 39 edges to The point I did not have and that decides it: user triggers go silent under The measured cost section is what I was asking for: no measurable difference on the populate path (medians 9.77s against 9.35s with overlapping ranges) and roughly +6–7µs per row per triggered edge on a focused bulk insert. "A permanent write tax should carry a number, not an adjective" — agreed, and now it does. I have re-approved the CI run, which fork PRs need after each push. I am not merging this one myself. It changes the data model permanently and 0048 is explicitly "pending review" with an open question for maintainers in #842 — hybrid split against uniform triggers. That is the maintainer's call, so it is with them now, along with your operational point that the §0 scan should be run read-only against real deployments before rollout. |
|
CI came back:
Nothing else failed. Once that is in, please push and I will re-approve the run — fork PRs need approval after every push. The hold I described above still stands and is unrelated to this: the data-model decision and #842 are for the maintainer, not for CI. |
de4524f to
31b0a7f
Compare
|
Fixed the remaining backend assertion by bumping |
|
I ran §0 read-only across the local databases, since "does this actually happen" was the one thing the record could not answer. It happens. MethodThe §0 Result19 databases clean on every edge they could run. Three — All three carry the identical row down to the KB ids, so they are copies of one source: one distinct occurrence, not three. It is a genuine cross-KB reference rather than a dangling id — I checked those separately, and the target row exists, in another base. The detail that makes it diagnostic: the owning base has its own Its failure mode is live, not hypothetical#824 landed earlier today and emits What limits itEvery current write path already scopes the target to the same base:
So the row is a fossil of something since removed or of a write that bypassed the store, rather than a leak the API can reproduce today. RepairedSet to
This also clears an upgrade blocker: §0 would have aborted migration 0070 on all three of those databases. What it changes in my readingIt supports the invariant and it supports your mechanism split — this landed on a self-referencing edge that the declarative composite key covers, and a composite key would have refused it at write time. It does not, on its own, carry the 13 trigger-covered edges: one fossil in years of development data, on a path the application layer already closes, is a thin basis for a permanent check on every provenance write. My position from the earlier review is unchanged and now better evidenced — take the 26 declarative edges, and gate the triggers on a completeness test that enumerates reference columns from the catalog and asserts each is covered, so the scheme cannot silently stop covering edges added later. The scan is worth keeping as an operational tool regardless of how that is decided. |
Install a schema-level invariant: every reference an export can resolve must join rows that live in the same knowledge base. A column foreign key proves the target exists, not that it is the same KB's — the exporter would otherwise mint local IRIs naming foreign rows, or silently drop vocabulary references that resolve to nothing. Three layers: a precondition scan that refuses the migration on a dirty ledger, per-edge same-KB enforcement (composite (kb_id, ref) foreign keys where the row carries its own kb_id, row triggers where the kb authority is a parent row, deferred keys on same-table self-references so COPY and multi-row inserts are judged at commit), and kb-ownership immutability on every owned table. Signed-off-by: 南慶麟 <isolated@test.local>
0048 decides the mechanism per edge: composite (kb_id, ref) foreign keys where the row carries its own kb_id, row triggers where the kb authority is a parent row, deferred keys on same-table self-references, kb immutability throughout. Carries the measured write-path cost and the operational precondition-scan requirement for real deployments. Signed-off-by: 南慶麟 <isolated@test.local>
31b0a7f to
4097d0a
Compare
|
Added the catalog completeness guard you suggested. Two scratch-schema probes confirm the sensitivity: a new single-column FK on a kb-owned row lands in Folded into the two existing commits; fork CI will need re-approval. |
PROBLEM
Provenance-bearing rows can reference rows that live in a different knowledge base. A column foreign key proves the target exists — not that it is the same KB's. As a result an export can mint a local IRI that names another KB's row, or silently drop a vocabulary reference that resolves to nothing.
WHY THIS IS A GENERIC UTOPIA BUG OR CONTRACT GAP
The schema enforces per-column FK existence but never same-KB ownership on provenance edges. Any write path that reaches the tables directly — including restores from older backups or SQL that bypasses the store layer — can persist cross-KB references that no later check catches.
FIX
A new migration installs the invariant at three layers:
(kb_id, ref)foreign keys on the 26 edges whose row carries its ownkb_id—DEFERRABLE INITIALLY DEFERREDon the four same-table self-references so COPY batches and multi-row inserts are judged at commit — and row triggers on the 13 edges whose KB authority is a parent row with nokb_idcolumn to key on,Design record:
docs/decisions/0048; mechanism question open as #842.REGRESSION EVIDENCE
Three new database test files cover: clean/dirty-ledger migration (atomic refusal, nothing installed), forward references via multi-row INSERT, COPY, UPDATE and deferred commit,
from_statementedges, relation self-links, and installation under normal/empty/hostilesearch_path. A catalog-derived completeness guard in the same file enumerates every column-level reference inside the ledger surface frompg_catalogand fails on any edge that resolves to no declared composite-FK, owner-derived trigger, or explicit exclusion. 14/14 pass on PostgreSQL 16; the fullutopia-storesuite is green. Write-path cost measured onbench_100k(UTOPIA_BENCH_DOCS=1000,UTOPIA_BENCH_HUB_FACTS=1000, alternating runs, same host / PG16 / toolchain): populate median 9.35s patched vs 9.77s base — within noise. The trigger-covered edges that path never touches measured separately: ~+6–7µs per row per edge (4000-row bulk inserts —fact_evidence63.7ms vs 37.1ms,typed_fact_sources58.1ms vs 34.3ms).COMPATIBILITY RISK
The migration fails closed on ledgers that already contain cross-KB rows — such installs must repair the data before upgrading, and the error tells them where. The §0 scan doubles as the pre-upgrade audit to run against real deployments (recorded in 0048). Clean ledgers pay ~6–7µs/row on the 13 trigger-covered edges and no measurable cost on the populate path.