Repository navigation
fix(recall): a year-1/9999 date no longer breaks temporal recall and consolidation (#5820) - #5828
Merged
Merged
Conversation
…consolidation (#5820) A memory date equal to datetime.min/max is stored by Postgres as -infinity/infinity, which asyncpg reads back without a timezone while every finite value is aware. A fact with a real start and an 'ongoing' end of 9999 (the first fact of a batch gets no ordering offset) then failed the temporal arm's midpoint arithmetic, failing every recall whose window reached it and the consolidation that runs that recall, on every retry. - stored_dates: year 1 / 9999 is an unknown bound, not a date; one helper for it. - recall: normalize each date (aware UTC, placeholders dropped) before any arithmetic; the two duplicated best-date blocks become one helper. - extraction: a placeholder year from the model is dropped; the prompt says not to write one for an open-ended event. - retain/edit/MCP timestamps reject a placeholder year (400); bank import drops it. - migration c7e2f9a4d1b8 clears placeholder dates already stored (PG + Oracle). - tests: unit, migration, real-LLM extraction, and a blackbox story that reproduces the 500 end to end.
…r date too Review findings fixed: - The shared retain date parser (parse_datetime_flexible) now rejects a year-1/9999 value, so every retain path — JSON, files, MCP, direct engine calls — is covered, not only the ones with a request-level check. A file upload's timestamp also fails at request time (FileRetainMetadata). - The migration test says why it reads the result back with SQL.
Review finding fixed: a document retained before this change keeps its year-1/9999 timestamp in retain_params; retain now refuses that date, so every reprocess of such a document would fail. Reprocess now replays it as undated.
This was referenced Oct 20, 2026
nicoloboschi
added a commit
that referenced
this pull request
Oct 21, 2026
…s; consolidation reads them as unknown (#5922) * fix(migrations): stop c7e2f9a4d1b8 scanning all of memory_units; consolidation reads placeholder dates as unknown The migration added in #5828 cleared year-1/9999 dates with an UPDATE filtered on four date columns that no index leads with, so it read every memory_units row in every schema at startup to fix a handful of rows. It is now a no-op (the revision stays so databases that already ran it keep a valid history). Instead, consolidation's date merging (_as_dt, _min_date, _max_date) normalises each date with finite_utc, as recall already does, so a stored infinity reads as an unknown bound rather than raising 'can't compare offset-naive and offset-aware datetimes'. * fix(migrations): keep clearing placeholder event_date, via its own index Review finding: on PostgreSQL 16 and older, retain's temporal-link SQL subtracts event_date, and an old infinity there fails with 'cannot subtract infinite timestamps' - the read-side fix can't reach SQL. event_date has a standalone index (idx_memory_units_event_date), so the migration now clears just that column as two index range probes instead of scanning the table. PG only: Oracle has no infinite timestamps. - test: the migration nulls placeholder event_date and leaves the other columns - test: say why the consolidation test forges and reads rows with raw SQL
Merged
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
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.
Fixes #5820.
What was happening
A memory date equal to the smallest or largest possible date (year 1 / 9999) is stored by Postgres as
-infinity/infinity. The driver reads infinity back without a timezone, while every real date comes back with one. A fact with a real start and an "ongoing" end of 9999 (the first fact of a batch gets no ordering offset, so it hits the exact maximum) therefore crashed the temporal arm:Every recall whose window reached the fact failed with a 500, and so did the recall consolidation runs — on every retry, leaving the bank stuck. That's how a single memory ends up with "two different timezones".
Fix
engine/stored_dates.py: year 1 / 9999 is an unknown bound, not a date — one helper used everywhere a date enters a memory.timestamp, memory edit, and MCP timestamps reject a placeholder year (400). Bank import drops it.c7e2f9a4d1b8clears placeholder dates already stored (PG and Oracle; Oracle's NOT NULLevent_datefalls back tocreated_at).Behaviour changes
Tests
tests/test_temporal_infinite_dates.py: recall with an infinite start or end, the parser, the API/MCP/edit rejections, and the migration. Recall + parser tests fail before the fix.tests/test_extraction_open_ended_dates.py(hs_llm_core): open-ended events produce no placeholder years.test_08_extreme_dates::test_an_open_ended_range_still_recalls_and_consolidates: reproduces the 500 end to end without the fix and passes with it.Note: the two existing stories in
test_08_extreme_dates.pydon't setfact_kind="event", so their dates are dropped before storage and never exercise the extreme-date path. Left as is here; worth a follow-up.