Skip to content

A role word is recorded against the chunk that says it, so a re-push can say something new - #1428

Open
gedaiu wants to merge 1 commit into
deeplethe:devfrom
re-cinq:feat/role-words-per-chunk
Open

gedaiu wants to merge 1 commit into
deeplethe:devfrom
re-cinq:feat/role-words-per-chunk

Conversation

@gedaiu

@gedaiu gedaiu commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Part of #1267, PR 1 of 3. Thanks for the clear answers on the issue, they made this one easy to cut.

This builds decision 1 of ADR 0070. statement_qualifiers gets a chunk_id (NOT NULL, ON DELETE CASCADE, like fact_evidence), and its key becomes (fact_id, role, chunk_id). add_statement_qualifier now records a role word against the chunk that says it, so a re-push writes a new row for its new chunk and leaves the old one alone. The new edge is guarded the same way as the others under ADR 0048. The trigger checks that the chunk is in the fact's base, the export preflight scans it (squalifier.chunk), and the catalog guard and ADR 0048's table list it. materialize copies the new column, and copies role words only when it creates a typed row: a statement that merges into an existing row may say another value, and the row would read two. The liveness readers come in PR 2 and the re-sync in PR 3.

Reads are unchanged. Migration 0121 copies each existing role word onto every evidence chunk of its fact, superseded chunks included, and statement_qualifiers_for collapses the copies with DISTINCT, so every read returns what it returned before. A typed or implied row takes a role word only on the chunks of the source statements that said the same value, so a value one source said never sits on another source's chunk, and a typed row whose sources disagree still reads back only its own value (PR 3 brings the other value across). A typed role no source carries with that value keeps today's value on the row's evidence. Role words on facts with no evidence, the invalidated rows that documents::purge leaves behind, are dropped, and the migration reports how many with a NOTICE. If a live fact has role words and no evidence, the migration stops, the same way 0070's audit does. The ADR's status moves to In progress, and decision 1 gets a dated revision note for typed rows. CURRENT_SCHEMA_VERSION is 112.

Tests: a role word written on two chunks makes two rows and the same chunk again adds nothing; a role word on another base's chunk is refused by the trigger and reported by the export preflight; and three scratch-database tests run migration 0121 itself: one checks the backfill and that reads still return one value, one that a typed row takes the role word only on the chunk of a source that said the same value (including a source that said 58-70 against the row's 40-52), and one that a live fact with role words and no evidence stops it. a_bound_statement_becomes_a_typed_fact merges a statement saying since 2021 into a row made from one saying 2019, and the row keeps only 2019. Each fails without its change. I ran the store, server, core and cli suites against Postgres with UTOPIA_TEST_REQUIRE_DB=1, the migrations twice, and fmt and clippy with -D warnings.

Size: it adds 551 lines and removes 65, a little over 500. The migration and the test that runs it are most of that and have to ship together, so I don't see a useful split.

@gedaiu
gedaiu force-pushed the feat/role-words-per-chunk branch from 8df2c62 to d49d2dc Compare October 26, 2026 10:27
@gedaiu

gedaiu commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

One fix before review: the backfill now puts a typed row's role word only on the chunk of the source statement that said it, not on every evidence chunk of the row. Otherwise a value from statement A could keep speaking through statement B's chunk after A is re-pushed. There's a new test for a typed row with two sources, and the description says so.

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the per-chunk key, the cascade, the trigger and preflight coverage, and the abort and drop handling all match what we agreed. One thing before merge: backfill step 2 copies each source's own value onto the typed row. A typed row whose two sources gave different values (S1 lines 40-52 on c1, S2 58-70 on c2, row holding 40-52) then reads back two values, so reads change, and decision 1 says they mustn't in PR 1. Please match source rows on the typed row's own value and entity (s.value IS NOT DISTINCT FROM q.value AND s.entity_id IS NOT DISTINCT FROM q.entity_id), let the rest fall through to step 3, and add a test with two sources that disagree. PR 3 will bring S2's value across. Small nit: ADR 0048 still counts 39 edges, 13 by trigger; they're now 40 and 14.

@gedaiu

gedaiu commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for such a careful read, the two-sources case is a great catch and I'm glad you spotted it before it shipped.

  • Backfill step 2 now matches source rows on the typed row's own value and entity (s.value IS NOT DISTINCT FROM q.value AND s.entity_id IS NOT DISTINCT FROM q.entity_id). A source that gave another value no longer lends its chunk, and anything with no matching source falls through to step 3. Every typed row keeps exactly the one value it had.
  • The typed-row migration test now has a third source that says lines 58-70 on the live chunk. The typed row holding 40-52 gets a row only on the first source's chunk. With the old step 2 it also got the live chunk, so the test fails without the fix.
  • ADR 0048 now counts 40 edges, 14 of them by trigger, and says that 0121 added the 40th.

I also rebased onto current dev. Migration 0121 is still free, and CURRENT_SCHEMA_VERSION is now 111. Thanks again!

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the backfill fix and the two-sources test are what we needed. One thing before merge: the runtime path in `materialize.rs` has the same problem. The `statement_qualifiers` copy also runs when a statement merges into an existing typed row, and the conflict key now includes the chunk. So S2 saying 58-70 on c2, merged into a row holding 40-52, leaves that row reading two values. Please copy only when the row is created, as ADR 0070 describes, and add a test. PR 3 can bring the second value across.

Two small things: `docs/design/ledger.md:33` still gives the old key, and 0070 decision 1 still says every evidence chunk, which is no longer true for typed rows.

Note for the schema version: #1466 merges first, so this PR will need 112.

…can say something new

Signed-off-by: Bogdan Szabo <bogdan@re-cinq.com>
@gedaiu
gedaiu force-pushed the feat/role-words-per-chunk branch from dde278f to 787a6a2 Compare October 30, 2026 22:24
@gedaiu

gedaiu commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thank you so much for catching the runtime path too, you were right. Here is what changed:

  • materialize now copies role words only when it creates a typed row. A statement that merges into an existing row adds its evidence and source but not its role words, so the row keeps reading one value. a_bound_statement_becomes_a_typed_fact now gives the merged statement since 2021 against the row's 2019 and checks that the row holds only 2019 on the first chunk. It fails without the change.
  • docs/design/ledger.md gives the new key, (fact, role, chunk, value | entity).
  • ADR 0070 decision 1 has a dated revision note: typed rows take a role word only on the chunk of a source statement that said the same value, and the runtime copy happens at creation only. The decision text itself is untouched.
  • Rebased onto dev after A merge keeps the identity evidence of the name it dedupes #1466, and CURRENT_SCHEMA_VERSION is now 112. Migration 0121 is still free, so it keeps its number.

Thanks again for the careful reviews, they made this much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants