fix(contradiction): stop Path C orphaning a conflicted row (09/22 L-12) - #1690
Conversation
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
8e8709b to
df0173b
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
|
CI green on the rebase onto |
df0173b to
0e398b7
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
0e398b7 to
671e460
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
671e460 to
12dcbda
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
12dcbda to
2fbb6f1
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
2fbb6f1 to
0f0bfb0
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
|
@Eldad-Caura please approve |
Path C's entity-overlap forward loop gated ALL supersession-edge writes
on one `found` flag covering both the canonical and the flipped branch,
while the `"conflicted"` status write at the top of the loop stayed
ungated. So in any mixed-direction run the first confirmed conflict
suppressed every later edge, leaving a row `conflicted` with nothing
pointing at it — neither active nor properly superseded, and invisible
to both.
Both orderings produced it:
flipped -> canonical: `new_memory` never got its outgoing edge, so the
older canonical candidate was conflicted and unreachable.
canonical -> flipped: the newer candidate never got its edge, so
`new_memory` itself was conflicted and unreachable.
This is exactly what Path A splits its two guards to prevent — the
comment on the RDF loop's canonical branch names it ("otherwise the
older canonical candidate is left orphaned ... unreachable via the
chain") and `test_mixed_conflicts_complete_three_way_chain` pins it.
Path C now uses the same split: `new_memory_is_outdated` keeps the
status-reversion guard, and a separate `chain_supersedes_id` gates only
the canonical branch, so `new_memory` still carries at most one outgoing
edge while every flipped candidate wires its own edge back at it.
`chain_supersedes_id` is seeded from the row's current `supersedes_id`,
as the semantic loop's is. `memory_update_status` has no
`supersedes_id IS NULL` guard — its only CAS is the explicit
`expected_supersedes_id` the forward paths never pass — so an unseeded
canonical write would silently re-point a row that already supersedes
something and orphan its previous target.
Tests assert on the resulting chain (status + supersedes_id per row)
read off the `batch_update_status` payload, not on source text. The two
mixed-direction tests fail on the pre-fix loop.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Arkady Mankovsky <arkash20@gmail.com>
0f0bfb0 to
4505ae7
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
🤖 I have created a release *beep* *boop* --- <details><summary>backend: 3.18.1</summary> ## [3.18.1](backend-v3.18.0...backend-v3.18.1) (2026-09-23) ### Bug Fixes * **contradiction:** stop Path C orphaning a conflicted row (09/22 L-12) ([#1690](#1690)) ([469f597](469f597)) * **documents:** answer a memories-shaped write body with the document shape (09/17 m-13) ([#1688](#1688)) ([9f3aecc](9f3aecc)) * **forge:** stop skipped clusters from spending the run's write budget ([#1687](#1687)) ([5132e20](5132e20)) * **memory:** make caller ownership of summary/tags durable on the row (09/02 L-08) ([#1693](#1693)) ([6cce76a](6cce76a)) * **sentinel:** scan name + tags, and settle the support_files checks (09/02 L-01, L-02) ([#1689](#1689)) ([7527a71](7527a71)) * **storage:** say when an unapplied migration is the expected outcome ([#1700](#1700)) ([d83a0a3](d83a0a3)) ### Dependencies * bump the uv-majors group across 2 directories with 1 update ([#1590](#1590)) ([5c86e9d](5c86e9d)) </details> <details><summary>plugin: 2.23.2</summary> ## [2.23.2](plugin-v2.23.1...plugin-v2.23.2) (2026-09-23) ### Bug Fixes * **memory:** make caller ownership of summary/tags durable on the row (09/02 L-08) ([#1693](#1693)) ([6cce76a](6cce76a)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: release-please[bot] <release-please[bot]@users.noreply.github.com> Signed-off-by: Eldad Caura <eldad@caura.ai> Co-authored-by: caura-deploy-bot[bot] <265395343+caura-deploy-bot[bot]@users.noreply.github.com>
Row
oss-0814-l-12— "Path C forward detection gates ALL supersession-edge writes onif not found— a canonical match after a flipped match leaves the older candidateconflictedbut orphaned from the chain, the exact corruption shape Path A explicitly guards against."Reproduced, and it is worse than the row states: both mixed-direction orderings orphan a row, not just flipped-then-canonical.
The bug
detect_contradictions_by_entities_async's forward loop wrapped both attribution branches in oneif not found:, while the"conflicted"status write at the top of the loop stayed ungated. The first confirmed conflict in a run therefore setfoundand suppressed every later supersession edge — but the later candidates were still markedconflicted.conflictedwith nothing pointing at itnew_memory → older_candolder_candnewer_cand → new_memorynew_memoryEither way the row is neither active nor properly superseded: retrieval penalises it as
conflicted, and no chain edge reaches it, so nothing can explain or undo it.Actual pre-fix payload for the flipped-then-canonical case (from the new test):
How Path A guards it
Both Path A loops (
_rdf_conflict_passand the semantic loop in_detect) split the guard in two:new_memory_is_outdated— the status-reversion guard. Stops a later canonical iteration writingnew_memoryback to"active".supersedes_id— the chain-edge guard, gating only the canonical branch. The flipped branch is ungated, because many newer rows may legitimately supersede one older row.The RDF loop's comment names the exact failure: "otherwise the older canonical candidate is left orphaned (outdated but unreachable via the chain)", and
test_mixed_conflicts_complete_three_way_chainpins it. Path C had collapsed both guards intofound.The fix
Path C now uses the same split —
new_memory_is_outdatedunchanged, plus a separatechain_supersedes_idgating only the canonical branch. No new mechanism; the branch bodies are byte-identical, only their guards moved.chain_supersedes_idis seeded from the row's currentsupersedes_id, as the semantic loop's is. Worth noting because the in-file comments claim a storage-side CAS that does not exist:memory_update_statushas nosupersedes_id IS NULLguard — its only CAS is the explicitexpected_supersedes_id, which the forward paths never pass. Without the seed, a canonical write would silently re-point a row that already supersedes something and orphan its previous target.reg-a73 interaction
The tracker flagged these as adjacent, and they are — in the same direction. Bulk ingest is the main real-world producer of multi-candidate mixed-direction Path C runs: dozens of rows about one subject land within seconds, so each row is judged against siblings both older and newer than itself.
write.bulk_subject_batchingreduces that to the last row per subject (every sibling then older, all-canonical), so with the flag on the orphan shape is rarer. It is off by default, and the flag does not remove the shape — a batch still lands against a pre-existing store. No conflict between the two changes; this one makes the un-batched path correct rather than depending on the flag.Tests
tests/test_l12_path_c_canonical_edge_orphan.py— 5 tests asserting on the resulting chain (status +supersedes_idper row) read off thebatch_update_statuspayload, not on source text. Includes an invariant helper: no row this run marksconflictedmay be left without an inbound edge.Revert-tested against the committed blob — with
main's detector restored and the new tests kept, 3 of the 5 fail:The other 2 pin behaviour the fix must preserve (one outgoing edge per row; the flipped branch's existing-
supersedes_idguard).pytest -k "contradiction or supersession or path_c or pathc": 367 passed, 1 skipped on this branch vs 362 passed, 1 skipped onmain(+5 new). No failures on either side.mypyon the file: same 6 pre-existing errors asmain, none new.ruff check/format --checkclean at CI's 0.16.2.Out of scope
A row whose single outgoing edge is already owned (by Path A, un-retracted) still leaves a second canonical candidate conflicted-and-orphaned. That is the chain's one-outgoing-edge design and Path A has the identical shape — a separate question from this row.
🤖 Generated with Claude Code