fix: always refresh TTL when refresh_on_read is enabled #95
Conversation
Remove 60% threshold optimization to prevent TTL drift between checkpoint keys and user-managed external keys. When refresh_on_read=True, TTL is now always refreshed on checkpoint reads, ensuring predictable behavior for TTL synchronization use cases. - Remove unnecessary threshold logic in both sync and async implementations - Simplify TTL refresh behavior to be more intuitive - Add comprehensive tests for new TTL behavior - Update documentation to be clearer about TTL functionality
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a TTL synchronization issue in RedisSaver by removing a 60% threshold optimization that caused TTL drift between checkpoint keys and external user-managed keys. The main purpose is to ensure predictable TTL behavior when refresh_on_read=True.
- Remove 60% TTL threshold logic that only refreshed TTL when it dropped below 60% of the original value
- Always refresh TTL on checkpoint reads when
refresh_on_read=Trueis enabled - Add comprehensive test coverage for the new TTL synchronization behavior
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_ttl_synchronization.py | New comprehensive test suite for sync TTL behavior |
| tests/test_async_ttl_synchronization.py | New comprehensive test suite for async TTL behavior |
| langgraph/checkpoint/redis/aio.py | Remove 60% threshold logic in async implementation |
| langgraph/checkpoint/redis/init.py | Remove 60% threshold logic in sync implementation |
| README.md | Update documentation to clarify TTL functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| from langgraph.checkpoint.redis.util import ( | ||
| to_storage_safe_id, | ||
| to_storage_safe_str, | ||
| ) |
There was a problem hiding this comment.
[nitpick] The import statements for checkpoint key generation are repeated in multiple test functions. Consider moving these imports to the top of the file or creating a helper function to reduce code duplication.
| ) | |
| # Imports moved to top of file |
Copilot uses AI. Check for mistakes.
| from langgraph.checkpoint.redis.util import ( | ||
| to_storage_safe_id, | ||
| to_storage_safe_str, | ||
| ) |
There was a problem hiding this comment.
[nitpick] The import statements for checkpoint key generation are repeated in multiple test functions. Consider moving these imports to the top of the file or creating a helper function to reduce code duplication.
| ) |
Copilot uses AI. Check for mistakes.
Remove 60% threshold optimization that caused TTL drift between checkpoint keys and user-managed external keys. When refresh_on_read=True, TTL is now always refreshed on checkpoint reads, ensuring predictable behavior for TTL synchronization use cases.