Skip to content

test(langgraph): cover JSON overwrite sentinel replay#8126

Closed
Sydney Runkle (sydney-runkle) wants to merge 1 commit into
mainfrom
test-json-overwrite-sentinel
Closed

test(langgraph): cover JSON overwrite sentinel replay#8126
Sydney Runkle (sydney-runkle) wants to merge 1 commit into
mainfrom
test-json-overwrite-sentinel

Conversation

@sydney-runkle

Copy link
Copy Markdown
Collaborator

Refs #8125
Depends on #8125

Adds regression coverage for API-style JSON Overwrite sentinel updates on DeltaChannel, verifying the streamed update shape, snapshot boundary, and later checkpoint replay.

Not run; this test is expected to pass once #8125 lands.

@sydney-runkle

Copy link
Copy Markdown
Collaborator Author

Closing this because the regression belongs in langgraph-api, not langgraph. The issue is API JSON serialization of Overwrite to the reserved overwrite sentinel.

@open-swe open-swe Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open SWE Review found 1 potential issue.

Open in WebView Open SWE trace

first_saved = saver.get_tuple(config)
assert first_saved is not None
snapshot = first_saved.checkpoint["channel_values"].get("items")
assert isinstance(snapshot, _DeltaSnapshot)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 New regression test fails immediately

The test added by this PR fails on the current head: running uv run --directory /langgraph/libs/langgraph pytest /langgraph/libs/langgraph/tests/test_channels.py::test_delta_channel_api_json_overwrite_sentinel_snapshots_and_replays -q reaches this assertion with snapshot is None. With snapshot_frequency=1000, the current checkpoint code does not write an _DeltaSnapshot after the first overwrite update, so this PR would make the test suite fail until the implementation change it depends on is included or the test is marked/structured accordingly.

(Refers to line 437)


Your feedback helps Open SWE learn. React with 👍 or 👎 to tell us if this review comment was useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant