fix(checkpoint): apply list(before=...) to LangGraph's UUIDv6 checkpoint ids - #206
Open
ericdelorefice wants to merge 1 commit into
Open
ericdelorefice wants to merge 1 commit into
ericdelorefice wants to merge 1 commit into
Conversation
…int ids The before filter parsed the checkpoint id as a ULID and dropped the filter when that failed. LangGraph creates ids with uuid6(), so get_state_history(before=...) returned the whole history. A shared helper now reads the timestamp out of a ULID or a UUIDv6 id, and the savers use it both to stamp checkpoint_ts and to build the before filter, so the two agree.
This branch has not been deployed
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.
Problem
list(before=...)builds its filter by parsing thebeforecheckpoint id as a ULID, and when the parse fails it drops the filter:LangGraph creates checkpoint ids with
uuid6(), so the parse fails for every checkpoint a graph writes, andget_state_history(config, before=...)returns the whole history. All four savers do this.test_list_before_with_uuid6_no_warningnotes it: "The current implementation silently ignores non-ULID 'before' IDs". #136 removed the warning, but the filter is still skipped.Reproduction
A one-node graph on
RedisSaveragainst Redis 8, three turns, thenget_state_history(config, before=<the middle checkpoint>), onmain:Change
A new helper,
checkpoint_id_timestamp()inutil.py, returns the time a checkpoint id encodes:ULID.timestamp, the value the savers already store for oneNoneAll four savers now use it in two places.
put/aputuse it to stampcheckpoint_ts, falling back to the current time (full savers) orcheckpoint["ts"](shallow savers) as before.list/alistuse it to build thebeforefilter. Stamping and filtering come from the same id, so a checkpoint saved in the background after the next one was created is still placed correctly. Abeforeid that encodes no time is still not filtered, as today.Two things I noticed and left alone:
ULID.timestampis in seconds, while the comments said milliseconds and the fallback stores milliseconds. I kept ULIDs exactly as they are so existing ULID data still compares correctly.beforeon old data is approximate at the boundary and exact for new data.#186 also touches
beforefor non-ULID ids, as part of a larger change that has been open since April and now conflicts withmain. It post-filters in Python over the first 10,000 documents. This PR only fixesbeforeand keeps the filter in the index. If you would rather take #186, I am happy to close this.Verification
New
tests/test_list_before_uuid6.py: the reproduction above forRedisSaverandAsyncRedisSaver, assertingbeforereturns exactly the checkpoints whose id sorts before the middle one, and a unit test of the helper. With the source reverted tomainand the tests kept, both end-to-end tests fail. The helper test fails there too, but only because the helper is new.Full suite with the repo's docker-compose Redis,
tests/integrationandtests/sentinelexcluded:main: 714 passed, 13 skipped (the same run minus the 3 new tests)poetry run format,sort-importsandcheck-mypyare clean.