fix(consolidation): preserve temporal fields across dedup merges - #3482
Open
Sanderhoff-alt wants to merge 1 commit into
Open
fix(consolidation): preserve temporal fields across dedup merges#3482Sanderhoff-alt wants to merge 1 commit into
Sanderhoff-alt wants to merge 1 commit into
Conversation
CREATE dedup folds source ids and merged text directly into the existing observation, bypassing the normal CREATE writer. Carry all four source-derived temporal fields into that fold and merge them null-safely. UPDATE dedup folds the rewritten observation into the survivor before deleting the redundant row. Merge temporal fields from both rows so the survivor keeps the complete source interval. The ordinary UPDATE path now passes event_date and applies the same rules: event_date and occurred_start use the earliest non-null value; occurred_end and mentioned_at use the latest non-null value. Apply the same merge in external memory stores. Document why event_date uses the store snapshot in that branch: MemoryFact does not carry event_date. Add regression coverage for PostgreSQL CREATE/UPDATE fold SQL, ordinary UPDATE parameters, external-store folds, and null/min/max semantics. The external-store ordinary UPDATE test covers the newly added event_date merge branch. Validation: - 45 consolidation dedup tests - 15 UPDATE/integrity tests - 7 source aggregation tests - Existing real-LLM temporal consolidation integration test
Sanderhoff-alt
force-pushed
the
fix/3477-consolidation-temporal
branch
from
August 14, 2026 09:46
84c473d to
af927ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #3477 reports that dated source facts can produce observations with null
occurred_startandoccurred_end. The normal CREATE path already aggregates source dates, but dedup folds bypassed that writer and updated observations did not passevent_datethrough the PostgreSQL SQL path. As a result, merging observations could silently discard part of their temporal information.This change makes all observation merge paths use the same null-safe rules:
event_dateandoccurred_startkeep the earliest non-null value, whileoccurred_endandmentioned_atkeep the latest non-null value. CREATE dedup carries source-derived fields into the survivor, UPDATE dedup merges both rows before deleting the rewritten row, and external memory stores follow the same rules.As a related fix, the ordinary observation UPDATE path now propagates
event_datefrom the source aggregation instead of leaving it unchanged. Its PostgreSQL SQL update and external-store upsert now apply the same temporal merge rules, so updates cannot lose an earlier event date while extending an observation.Verification
The deterministic regression tests cover CREATE and UPDATE dedup SQL, the ordinary UPDATE parameter contract, external-store folds, and min/max/null behavior. 45 dedup tests, 15 UPDATE/integrity tests, and 7 source aggregation tests pass. The existing real-LLM temporal consolidation integration test was also run with the configured test runtime; this PR does not add a new LLM integration test.
Fixes #3477