Skip to content

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

Merged
merged 3 commits into from
Apr 30, 2025
Merged

Conversation

bsbodden
Copy link
Contributor

@bsbodden bsbodden commented Apr 29, 2025

@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

@bsbodden bsbodden self-assigned this Apr 29, 2025
@bsbodden bsbodden marked this pull request as ready for review April 29, 2025 05:54
Copy link

@riferrei riferrei left a 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:

@riferrei
Copy link

This one fails during compilation for the highlighted dependency.

image

@riferrei
Copy link

And this one fails during runtime when you specify the user. Note that, maybe this was due to the fact that I changed the model from Anthropic to OpenAI. Maybe this influences the internal behavior of LangGraph.

image

@riferrei
Copy link

Also, your Docker Compose maybe be binding to a particular folder of the filesystem that avoids the execution to start.

image

@bsbodden bsbodden force-pushed the bsb/new-notebooks-fix-key-issues branch from 115c145 to 8084a03 Compare April 29, 2025 21:38
@bsbodden bsbodden requested a review from riferrei April 29, 2025 21:39
  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
@bsbodden bsbodden force-pushed the bsb/new-notebooks-fix-key-issues branch from 8084a03 to e458d19 Compare April 29, 2025 22:29
pipeline = self._redis.pipeline(transaction=True)

# Store minimal checkpoint data
checkpoint_data = {
Copy link
Contributor

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent testing as usual

Copy link
Contributor

@abrookins abrookins left a 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.

@bsbodden bsbodden merged commit 5d3a689 into main Apr 30, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants