-
Notifications
You must be signed in to change notification settings - Fork 10
Bsb/new notebooks fix key issues #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to test successfully all the examples that use OpenAI APIs. I couldn't test the ones that use Anthropic as I don't have an API key to use. Most of the examples work, except for these two:
115c145
to
8084a03
Compare
Core changes: - Fix Redis key parsing to handle additional components in _parse_redis_checkpoint_writes_key method - Modify error handling to accept keys with more than 6 components by slicing (parts[:6]) - Update error message to indicate "at least 6 parts" needed rather than "expected 6" Added comprehensive test suites: - Basic key parsing tests (test_key_parsing.py) - Subgraph key parsing tests (test_subgraph_key_parsing.py) - Fix verification tests (test_fix_verification.py) - Semantic search key tests (test_semantic_search_keys.py) - Streaming tests (test_streaming_modes.py, test_streaming.py) Added new example notebooks: - Memory management examples (add-summary-conversation-history, delete-messages, etc.) - Human-in-the-loop examples (breakpoints, edit-graph-state, time-travel, etc.) - Subgraph management notebooks (subgraph-persistence, subgraphs-manage-state) - Agent creation examples with history management and HITL
8084a03
to
e458d19
Compare
pipeline = self._redis.pipeline(transaction=True) | ||
|
||
# Store minimal checkpoint data | ||
checkpoint_data = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smart!
@@ -0,0 +1,88 @@ | |||
"""Focused test for Redis key parsing fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent testing as usual
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good! I focused on the logic changes more than the notebooks.
@riferrei discovered that I have committed the wrong version of the notebooks (the original LangGraph non-Redis ones) - This PR addresses that, but also, as I added several new notebooks testing advanced scenarios, I discovered some issues, mainly key construction and parsing, most of which surfaced when working with subgraphs.
This PR brings the number of notebooks to 20