-
Notifications
You must be signed in to change notification settings - Fork 209
fix: evict prompt cache on NotFound error #1442
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
base: main
Are you sure you want to change the base?
Conversation
- Add NotFoundError handling in _fetch_prompt_and_update_cache - Add delete() method to PromptCache - Add test for cache eviction on NotFound - Fix test fixture to initialize _resources with PromptCache
|
|
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.
3 files reviewed, no comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
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.
Pull Request Overview
This PR fixes a bug where deleted prompts remained in the cache indefinitely after expiration. When a prompt label is removed in Langfuse UI and the cached entry expires, the SDK now properly evicts the stale cache entry upon receiving a 404 NotFoundError, allowing fallback mechanisms to work correctly.
Key Changes:
- Added
PromptCache.delete(key)method to remove individual cache entries when a prompt is not found - Modified error handling in
_fetch_prompt_and_update_cacheto catch NotFoundError, log a warning, evict the cache entry, and re-raise the error - Enhanced test fixtures to properly initialize
_resourceswith PromptCache and added comprehensive test coverage for the eviction behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| langfuse/_utils/prompt_cache.py | Adds delete method to safely remove individual cache entries using pop(key, None) |
| langfuse/_client/client.py | Adds NotFoundError exception handling to evict cache entries before re-raising the error |
| tests/test_prompt.py | Updates fixture to initialize PromptCache and adds comprehensive test verifying cache eviction on NotFoundError during background refresh |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem
When a prompt label is removed in the Langfuse UI and the cached entry expires,
get_promptkept returning the stale prompt. The refresh worker logged a404 NotFoundErrorbut left the cache entry intact, so the SDK never fell back to another label or the unlabeled version. This bug is described in the issue linked below.Technical details
NotFoundErrorin_fetch_prompt_and_update_cache, log a warning and delete the cache entry before re‑raising the error.PromptCache.delete(key)to remove a single cached entry andPromptCache.invalidate(prompt_name)to drop all entries for a prompt.test_evict_prompt_cache_entry_when_refresh_returns_not_foundand update fixtures to initialise_resourceswithPromptCache.Original issue: langfuse/langfuse#10138
Important
Fixes prompt cache eviction on
NotFoundErrorby updating cache handling logic and adding tests for verification.client.py, catchNotFoundErrorin_fetch_prompt_and_update_cache, log a warning, and delete the cache entry before re-raising the error.PromptCache.delete(key)to remove a single cached entry andPromptCache.invalidate(prompt_name)to drop all entries for a prompt.test_evict_prompt_cache_entry_when_refresh_returns_not_foundintest_prompt.pyto verify cache eviction onNotFoundError.test_prompt.pyto initialize_resourceswithPromptCache.This description was created by
for 3c32bb0. You can customize this summary. It will automatically update as commits are pushed.
Disclaimer: Experimental PR review
Greptile Summary
NotFoundErrorfrom APIPromptCache.delete()method to remove single cache entries when prompts are deleted in Langfuse UIConfidence Score: 5/5
dict.pop(key, None), and comprehensive test coverage verifying the cache eviction flowImportant Files Changed
NotFoundErrorexception handling to evict cache entries when prompts are deleted from Langfuse UINotFoundErrorand updated fixture with_resourcesinitializationSequence Diagram
sequenceDiagram participant User participant Langfuse as "Langfuse.get_prompt()" participant Cache as "PromptCache" participant Worker as "Background Worker" participant API as "Langfuse API" User->>Langfuse: "get_prompt(name)" Langfuse->>Cache: "get(cache_key)" Cache-->>Langfuse: "cached_prompt (expired)" Langfuse->>Cache: "add_refresh_prompt_task()" Langfuse-->>User: "return stale prompt" Cache->>Worker: "schedule refresh task" Worker->>Langfuse: "call _fetch_prompt_and_update_cache()" Langfuse->>API: "prompts.get(name, label)" API-->>Langfuse: "NotFoundError (404)" Langfuse->>Cache: "delete(cache_key)" User->>Langfuse: "get_prompt(name, fallback)" Langfuse->>Cache: "get(cache_key)" Cache-->>Langfuse: "None" Langfuse->>API: "prompts.get(name, label)" API-->>Langfuse: "NotFoundError (404)" Langfuse-->>User: "return fallback prompt"