Skip to content

fix(kg): symmetric inversion guard on invalidate (#1371) - #1409

Closed
arnoldwender wants to merge 1 commit into
MemPalace:developfrom
arnoldwender:fix/kg-invalidate-inversion-guard
Closed

fix(kg): symmetric inversion guard on invalidate (#1371)#1409
arnoldwender wants to merge 1 commit into
MemPalace:developfrom
arnoldwender:fix/kg-invalidate-inversion-guard

Conversation

@arnoldwender

Copy link
Copy Markdown
Contributor

What and Why

#1214 added an inverted-interval guard to KnowledgeGraph.add_triple() so a row with valid_to < valid_from cannot be persisted on the create path. The same foot-gun was reachable through invalidate(): a caller passing ended=\"2020-01-01\" against an open row with valid_from=\"2026-01-01\" silently produced the same inverted state — invisible to every query_entity / query_relationship / timeline call (because the as_of filter valid_from <= ? AND valid_to >= ? can never be satisfied), durable in SQLite forever.

@igorls flagged this in #1371 during review of #1214.

Root Cause

mempalace/knowledge_graph.py:232-245 (pre-fix) — invalidate() issued an UPDATE triples SET valid_to=? with no comparison against the existing valid_from. The asymmetry with add_triple()'s line-177 guard meant the same invariant was enforced on creation but not on closure.

Reproduction

kg.add_triple('Alice', 'knows', 'Bob', valid_from='2026-01-01')
kg.invalidate('Alice', 'knows', 'Bob', ended='2020-01-01')

# Before this PR: silently succeeds. The row now has
# valid_from='2026-01-01', valid_to='2020-01-01' — invisible to every query.
results = kg.query_entity('Alice', as_of='2025-01-01')  # → []
results = kg.query_entity('Alice', as_of='2026-06-01')  # → []
# The relationship is gone from the graph permanently.

# After this PR:
# ValueError: ended='2020-01-01' is before valid_from='2026-01-01';
# an inverted interval would be invisible to every KG query

Change Summary

  • mempalace/knowledge_graph.py:232-264invalidate() now SELECTs valid_from for all matched open rows before the UPDATE. If ended < valid_from for any matched row, raise ValueError with the same message format as add_triple(). The SELECT and UPDATE share one with conn: transaction so a concurrent insert cannot race a row past the guard.
  • ended == valid_from is allowed (point-in-time fact, mirrors test_add_triple_accepts_equal_dates).
  • Rows with valid_from IS NULL (open intervals) pass through — the original invalidate() contract for date-less triples is preserved.
  • The guard fires if any matched row would become inverted (subject/predicate/object can repeat across time, so a single call may target multiple open rows; rejecting on any one prevents partial corruption).

Test Plan

  • New test_invalidate_rejects_ended_before_valid_from — exercises the rejection path and asserts the row stays open after the failed call.
  • New test_invalidate_accepts_ended_equal_to_valid_from — same-day point-in-time close still works.
  • New test_invalidate_passes_through_open_intervals — non-regression: rows without valid_from invalidate normally.
  • All existing tests pass: test_invalidate_sets_valid_to (seeded_kg has valid_from=\"2024-06-01\", ended \"2026-01-01\" — guard does not fire), test_invalidated_triple_allows_re_add, test_add_triple_allows_only_one_bound.
  • Full tests/test_knowledge_graph.py: 27 passed (24 existing + 3 new).
  • ruff check . and ruff format --check . clean on both files.

Closes #1371. Surfaced by @igorls during review of #1214.

MemPalace#1214 added an inverted-interval guard to KnowledgeGraph.add_triple() so a
row with valid_to < valid_from cannot be persisted on the create path.
The same foot-gun was reachable through invalidate(): a caller passing
ended="2020-01-01" against an open row with valid_from="2026-01-01"
silently produced the same inverted state — invisible to every
query_entity / query_relationship / timeline call, durable in SQLite
forever.

Mirror the add_triple() guard inside invalidate(). Before the UPDATE,
SELECT the matched rows' valid_from values and reject with ValueError if
ended is strictly before any of them. Equality is allowed (point-in-time
fact). Open intervals (valid_from IS NULL) pass through unchanged. The
SELECT and UPDATE share one transaction (`with conn:`) so a concurrent
insert cannot race a row past the guard.

Tests:
- test_invalidate_rejects_ended_before_valid_from: rejection + asserts
  the row stays open after the failed call.
- test_invalidate_accepts_ended_equal_to_valid_from: same-day close.
- test_invalidate_passes_through_open_intervals: non-regression for the
  original invalidate() contract on rows without valid_from.

Closes MemPalace#1371. Surfaced by @igorls during review of MemPalace#1214.
@arnoldwender

Copy link
Copy Markdown
Contributor Author

Closing — superseded by @fatkobra's #1432 (commit 4adc99f on develop, 2026-05-08), which landed an equivalent inversion guard inside invalidate() plus a stronger _temporal_end_key() / _temporal_start_key() helper pair that handles full datetime, not just YYYY-MM-DD. Same error message phrasing as this PR. Tests on develop now cover the timezone-offset ended case via test_invalidate_rejects_timezone_offset_ended. No additional value left to merge from this branch.

Thanks to @igorls for surfacing the symmetry concern in #1371; happy this got addressed in a more general form. Closes #1371 from this PR's side.

@arnoldwender

Copy link
Copy Markdown
Contributor Author

Closing — fully superseded by upstream #1432 / commit 4adc99f. Details in previous comment.

@arnoldwender
arnoldwender deleted the fix/kg-invalidate-inversion-guard branch May 18, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kg Knowledge graph bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kg: invalidate() should symmetrically reject ended < valid_from for the target row

2 participants