Skip to content

test(dst): model edges as physical rows, not a set of pairs - #684

Merged
azimafroozeh merged 2 commits into
ModernRelay:mainfrom
azimafroozeh:sibling-merge-readopts-deleted-edge
Sep 8, 2026
Merged

test(dst): model edges as physical rows, not a set of pairs#684
azimafroozeh merged 2 commits into
ModernRelay:mainfrom
azimafroozeh:sibling-merge-readopts-deleted-edge

Conversation

@azimafroozeh

Copy link
Copy Markdown
Contributor

What & why

Closes #681. The DST nightly (run 9, seed 221206) fired its first WorldDifferential on a delete-vs-readd fork: main holds alice -> bob, b0 inserts the pair again, b1 deletes it; merging b1 and then b0 leaves the pair on main. The engine is right under the multiset default and the DST model was wrong.

  • Engine: the merge walk keys rows on id alone. b0's insert is a fresh row, b1's delete removes the inherited one, so the second merge adopts the fresh row. That is the multiset contract for an unkeyed edge type: re-inserting a pair is a second row, and a delete removes only the rows it matched (dedup deferred to bug: merge duplicates an edge added on both sides of the fork, and plain traversal hides the extra row #583, identity opt-in per type via @key(src, dst) in feat(engine): add edge keys with derived identity #593).
  • Model: Model.edges was a set of pairs, one entry per pair, so it saw b0 unchanged and predicted absence. It is now a map from a minted row id to the pair, ids drawn from one counter on WorldModel and cloned across a fork, and predict_merge runs the same per-key three-way over persons and edges.
  • H-A (reject an edge pair born on both sides) was retired by feat(engine): add edge keys with derived identity #593; this diff keeps that and drops its "known gap" note, since the row-id model closes the gap it named.
  • Per-pair counts were rejected: base count 1 and side count 1 cannot tell "untouched" from "deleted and re-added", and that is the shape under test.
  • Oracles now see the multiset: the physical channel (export_jsonl) keeps duplicate rows and assert_physical_matches compares rows ∪ ghosts at the final reopen, so a model whose row count drifts from the engine's goes red; the bound-edge reader gains a row-grain form (knows_rows_bound_target).
  • Crash reconciliation reads rows, not pairs, when it must: an insert of a pair the branch already holds renders identically under Applied and NotApplied at pair grain, and the old Applied-first tie-break minted a phantom row the engine may lack (caught live on the issue-554 panel, seed 11, once the count oracle existed). The tie-break now triggers whenever the two hypotheses differ in rows or ghosts and rules by the bound-edge rows (query+bound); the widened keep-serving arbitration narrows its ties the same way.
  • Pins, all .gqt, no Rust test: issue_681_sibling_merge_readopts_deleted_edge.gqt pins the engine outcome (no rows after the b1 merge, one row after the b0 merge); keyed_edge_delete_wins_over_readd.gqt pins the keyed contract (the pair stays deleted after both merges, RFC 0044 via feat(engine): add edge keys with derived identity #593); selfloop_rows_gated_once.gqt pins that two self-loop rows read once on the gated traversal and count 2 bound, the grain the model's membership oracles rely on. The model's own agreement is exercised by the nightly fleet (seed 221206 is the arm that found the gap) and by every existing pinned universe under the row-grain physical oracle.
  • RFC 0055 (branch statements) updated: the seed-221206 case was specified there as a held-out engine bug with the delete-wins expectation; the Motivation line, the first-cases item, the fragment and a 2026-09-08 decision-log entry now carry the ruling.

Backing issue / RFC

  • Fixes an accepted issue: Closes bug: merging a sibling branch brings back an edge a previous merge deleted #681. issue_681_sibling_merge_readopts_deleted_edge.gqt is the regression evidence: its # red_on: records the red it witnessed under the expectation the issue filed, its # notes: the ruling, and it expects the engine's answer. The keyed twin takes RFC 0045's feature shape (# issue: none): no build was ever wrong on it.

Checklist

  • Change is focused (the DST model's edge identity, its oracles, three .gqt cases and the RFC 0055 record; no engine edit)
  • Tests added/updated for behavior changes (three .gqt cases; the DST suite's existing pinned universes run under the row-grain physical oracle, 50 passed / 27 ignored)
  • Public docs updated if user-facing surface changed (internal harness, no public surface)
  • Reviewed against docs/dev/invariants.md — no Hard Invariant weakened, no deny-list item hit (no engine code touched; the model now predicts the engine's pinned multiset outcome instead of a stricter set one)

Local verification

  • cd crates/omnigraph-dst && cargo test — scenarios 50 passed / 27 ignored (dst_keep_serving_wedge_issue_554 included), lane_b 1 passed, torn_init 1, lib 26
  • cargo test -p omnigraph-gqt --test gq_logic_tests — 51 cases green on main with feat(engine): add edge keys with derived identity #593 (on the pre-feat(engine): add edge keys with derived identity #593 base the keyed twin is refused at parse, as expected)
  • cargo fmt --all -- --check — clean
  • cd crates/omnigraph-dst && cargo clippy --workspace --all-targets — clean
  • seed 221206 replayed against the previous set model (temporary universe, deleted after) — red at op 29 with the nightly's Store(Query)/WorldDifferential; green under this model
  • the row-grain physical oracle against the pair-grain tie-break (temporary, reverted) — red on the issue-554 panel, seed 11: model [(Alice,Bob),(Alice,Charlie),(Alice,Charlie),(Bob,Diana)], export three rows — the phantom row the bound-row tie-break now rules out
  • cargo test -p omnigraph-gqt --lib — not run to green: branch_list_shape_and_rows_are_blessed_like_a_read_step overflows its stack on 5f94a741 without this diff too

Notes for reviewers

  • No engine change: the multiset default for unkeyed edge types stands. With edge Knows: Person -> Person { @key(src, dst) } the identical insert is an upsert of the same row and the pair stays deleted after both merges; the keyed twin case pins that on main with feat(engine): add edge keys with derived identity #593 merged.
  • Lane B's World keeps its own set model; it never merges.
  • The tie-break reads the bound-edge rows, not export_jsonl: an export read inside a reconcile shifted the issue-554 panel's lance-realm universes by test order (the panel is green when run first and red after most predecessors with the export read; deterministic with --test-threads=1), while the bound query, the same read class the reconcile already issues, leaves them unchanged. The export channel keeps its role as the final-reopen physical oracle.
  • Not in this PR: a Milestone recipe that reaches the sibling delete-vs-readd shape deliberately (today it is seed luck), and a count-aware differential at every oracle site; the final-reopen row-grain oracle covers the count claim once per universe.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@azimafroozeh
azimafroozeh force-pushed the sibling-merge-readopts-deleted-edge branch from 645b3a6 to 7963511 Compare September 8, 2026 07:40
@azimafroozeh
azimafroozeh merged commit b8eb17a into ModernRelay:main Sep 8, 2026
23 checks passed
@azimafroozeh
azimafroozeh deleted the sibling-merge-readopts-deleted-edge branch September 8, 2026 08:04
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.

bug: merging a sibling branch brings back an edge a previous merge deleted

1 participant