Add SemanticCache TTL refresh controls - #618
Conversation
Signed-off-by: Omri SirComp <omribz156@gmail.com>
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
There was a problem hiding this comment.
Pull request overview
Adds opt-out controls for SemanticCache’s sliding-window TTL behavior, allowing callers to disable “refresh TTL on hit” at construction time and/or override it per check() / acheck() call, with corresponding documentation and integration coverage.
Changes:
- Add
refresh_ttl_on_hit(defaultTrue) toSemanticCacheplusrefresh_ttl: bool | Noneoverride oncheck()/acheck(). - Update integration tests to verify TTL refresh can be skipped/enabled for sync and async cache hits.
- Update the LLM cache user guide notebook to document the new TTL refresh controls and scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
redisvl/extensions/cache/llm/semantic.py |
Adds cache-level and per-call controls for whether TTL is refreshed on cache hits (sync + async). |
tests/integration/test_llmcache.py |
Adds new integration tests covering skip/override behavior for TTL refresh in check() / acheck(). |
docs/user_guide/03_llmcache.ipynb |
Documents how to disable/override TTL refresh behavior and updates TTL behavior table/examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with patch.object(cache_with_ttl, "aexpire", new_callable=AsyncMock) as aexpire: | ||
| check_result = await cache_with_ttl.acheck(vector=vector, refresh_ttl=True) | ||
|
|
||
| assert len(check_result) == 1 | ||
| aexpire.assert_called_once() |
There was a problem hiding this comment.
Thanks, fixed in 3dfafaa. The async TTL-refresh assertions now use assert_not_awaited() and assert_awaited_once(). Verified with black --check, compileall, and git diff --check.
vishal-bala
left a comment
There was a problem hiding this comment.
Looks good aside from one minor comment! Should be good to go once we have that addressed.
| return_fields: list[str] | None = None, | ||
| filter_expression: FilterExpression | None = None, | ||
| distance_threshold: float | None = None, | ||
| refresh_ttl: bool | None = None, |
There was a problem hiding this comment.
Since this class inherits from BaseLLMCache, you'll want to make the corresponding update on there as well. Otherwise type-checkers will complain
There was a problem hiding this comment.
Thanks, fixed in 362ca90. I added the matching refresh_ttl parameter to BaseLLMCache.check() and BaseLLMCache.acheck(), plus the sync docstring note, so SemanticCache stays compatible with the base interface. Verified with python -m compileall on base.py/semantic.py and git diff --check.
|
Looks like your code hasn't run the formatter before committing - there's a |
Summary
refresh_ttl_on_hittoSemanticCacheso the default sliding-window TTL behavior stays unchanged but can be disabled at cache constructionrefresh_ttlper-call overrides forcheck()andacheck()Closes #603
Verification
uv run black --check redisvl/extensions/cache/llm/semantic.py tests/integration/test_llmcache.pyuv run python -m compileall redisvl/extensions/cache/llm/semantic.py tests/integration/test_llmcache.pypython -c "import json, pathlib; json.loads(pathlib.Path('docs/user_guide/03_llmcache.ipynb').read_text(encoding='utf-8')); print('notebook json ok')"git diff --checkI also attempted
uv run pytest tests/integration/test_llmcache.py -k "ttl_refresh or ttl_refresh_setting", but this local machine does not have Docker on PATH, so testcontainers failed before running the selected tests withFileNotFoundError: [WinError 2]while invokingdocker compose.This was implemented with Codex assistance and manually reviewed before submission.
Note
Low Risk
Backward-compatible API change with default TTL refresh unchanged; only adds opt-out knobs and tests around existing cache hit behavior.
Overview
Adds configurable control over whether cache lookups extend entry lifetime in
SemanticCache, while keeping the existing sliding-window TTL behavior as the default.refresh_ttl_on_hit(defaultTrue) onSemanticCachedisables TTL extension on every hit when set toFalse.check()/acheck()accept optionalrefresh_ttlto override that setting per lookup (e.g.refresh_ttl=Falsefor read-only probes). The base LLM cache interface andLangCacheSemanticCacheaccept the same parameter for API consistency; LangCache logs a warning because it does not implement TTL refresh.The LLM cache user guide documents the new options and examples, and integration tests cover sync/async skip, cache-level off, and per-call override via mocked
expire/aexpire.Reviewed by Cursor Bugbot for commit 2d33396. Bugbot is set up for automated code reviews on this repo. Configure here.