mirror of
https://github.com/langchain-ai/langgraph.git
synced 2026-10-01 05:55:14 +02:00
c0279f0910
## Summary `get_delta_channel_history` on Postgres returns an empty history for any `DeltaChannel` on a target checkpoint that is not within the first stage-1 pagination page (1024 rows) of the thread. No exception, no warning: the channel just hydrates empty. Fixes #8448 ## Problem Stage 1 pages `checkpoints` newest-first from the head of the thread, and after each page `_try_advance_walks` tries to move every not-yet-seeded channel's walk along the partial `parent_of` map accumulated so far. The walk starts at the target's parent: ```python if ch not in walk_cursor_by_ch: walk_cursor_by_ch[ch] = parent_of.get(target_id) ``` The target can be any checkpoint in the thread, not just the head, so on the first page `parent_of` frequently has no row for it yet. `.get` then returns `None`, which is also what a target with no parent returns, and the two are stored identically. Because the initialisation is guarded by `ch not in walk_cursor_by_ch`, it never runs again: once the walk is parked at `None` it stays there even after the target's real row and real parent load on a later page. The result is an empty chain and no seed. Downstream `channels_from_checkpoint` does ```python replay_ch = delta_spec.from_checkpoint(history.get("seed", MISSING)) replay_ch.replay_writes(history["writes"]) ``` so `get_state`, `get_state_history` and `update_state` against an older checkpoint reconstruct a `messages` channel as `[]` on a thread with hundreds of real messages. ## Fix Start the walk only once `target_id` is actually present in `parent_of`, so "the target has not loaded yet" stops sharing a representation with "the target is a root": ```python if ch not in walk_cursor_by_ch: if target_id not in parent_of: continue walk_cursor_by_ch[ch] = parent_of[target_id] ``` `_try_advance_walks` is a static method on `BasePostgresSaver`, so `PostgresSaver` and `AsyncPostgresSaver` are both covered by the one change. ## Why it's safe `continue` leaves the channel exactly as it was, so a later page retries. The three existing stop conditions are untouched: a channel that finds its seed still seeds, one that reaches a real root still parks at `None`, and one waiting on an ancestor still keeps its cursor. Paging still terminates on a short page, which is what ends the run for a target that really is a root. ## Long-term The sibling sqlite implementation avoids this class of bug differently, by starting its stage-1 scan at the target (`checkpoint_id <= ?`) instead of at the head. Postgres could adopt the same bound and would then never fetch a checkpoint newer than the target at all, which looks like the bigger win on a long thread. It makes the read path depend on ancestors always sorting below their descendants, though, which sqlite already assumes but the Postgres fast path currently does not. #8550 now reports that assumption as a bug in sqlite, on the grounds that ancestry is defined by `parent_checkpoint_id` and the contract does not require ids to be monotonic, so the bound is the wrong direction to move Postgres in. Paging the full thread and following parent pointers is what keeps this path correct when ids are not monotonic, and with this fix Postgres returns the right history for #8550's scenario at every page size. ## Test plan New `libs/checkpoint-postgres/tests/test_delta_pagination.py`. Page size is monkeypatched rather than writing 1024+ real checkpoints per case, since the only thing that decides the behaviour is which page the target lands on. - [x] `test_async_target_older_than_the_first_page` and its sync twin, parametrised over page sizes `[_DELTA_PAGE_SIZE, 3, 2, 1]`. The thread has 8 checkpoints with a snapshot at step 1 and the target at step 4, so every size at or below 3 leaves the target off the first page. The real page size is the control. - [x] `test_root_target_has_no_history_and_still_terminates` covers the case where a `None` cursor is the correct answer, at page size 1 so the paging loop runs the length of the thread. - [x] 6 of the 9 fail on `main` (`expected a snapshot seed, got '<missing>'`); the 3 that pass are the two controls and the root case. - [x] `make format`, `make lint_package`, `make lint_tests` clean. - [x] Full `libs/checkpoint-postgres` suite, rebased on current `main`: 279 passed, 3 skipped on Postgres 16. - [x] Graph-level repro with `_DELTA_PAGE_SIZE = 5`: 10 invocations, then `get_state` on the 8th-newest checkpoint returns `[]` on `main` and the full history on this branch. Thanks to @Navneet-Scaler for the report, the mechanism write-up, and the fix in #8453, which this matches. Co-authored-by: Navneet-Scaler <147032454+Navneet-Scaler@users.noreply.github.com>