refactor(prompts): tighten cache event dispatch and generation safety#90
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens and refactors the prompt caching subsystem in langfuse-rb, focusing on making cache event emission cheaper on the hot path and making name-generation invalidation safe under LRU eviction.
Changes:
- Extracts prompt cache event emission into
PromptCacheEvents, adds lazy payload construction, and improves observer/ActiveSupport dispatch behavior. - Centralizes cache-related constants (
CacheStatus,CacheSource,CacheBackend) and threads prompt-fetch helpers through small structs to reduce call-site complexity. - Fixes per-name generation safety by switching to a monotonic global counter so evicted names cannot reintroduce colliding generations; adds specs pinning this behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/langfuse/prompt_cache_spec.rb | Adds specs covering LRU eviction behavior and monotonic generation safety. |
| spec/langfuse/api_client_spec.rb | Adds specs for new observer arity handling, lazy payload building, and AS instrumentation gating. |
| lib/langfuse/stale_while_revalidate.rb | Introduces TtlWindow and a helper to compute TTL/freshness windows for writes. |
| lib/langfuse/rails_cache_adapter.rb | Uses centralized cache constants; refactors TTL computation and namespacing prefix handling. |
| lib/langfuse/prompt_fetch_result.rb | Switches fallback detection to centralized cache source constant; removes cache_key alias. |
| lib/langfuse/prompt_cache.rb | Adds bounded per-name generation map with monotonic counter; updates stats to centralized constants; refactors TTL window usage. |
| lib/langfuse/prompt_cache_events.rb | New module implementing unified prompt cache event emission (observer + ActiveSupport). |
| lib/langfuse/client.rb | Updates fallback result construction to use new event helper and cache constants. |
| lib/langfuse/cache_constants.rb | New shared cache constants module for producers/consumers of cache metadata. |
| lib/langfuse/api_client.rb | Integrates PromptCacheEvents, introduces PromptFetchOptions, and threads constants through event payloads and fetch paths. |
| lib/langfuse.rb | Requires new cache constants and prompt cache events modules in load order. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Use total_ttl if SWR enabled, otherwise just ttl | ||
| window = compute_window(ttl: ttl, stale_ttl: stale_ttl) | ||
| expires_in = swr_enabled? ? window.total_ttl : window.ttl |
There was a problem hiding this comment.
Resolved in 30d0668: inlined the TTL math here, dropped compute_window and TtlWindow so the SWR-disabled write path no longer allocates an intermediate object. Ran the live cache validators after the change — all green.
| window = compute_window(ttl: ttl, stale_ttl: stale_ttl) | ||
| @cache[key] = CacheEntry.new(value, window.fresh_until, window.stale_until) |
There was a problem hiding this comment.
Resolved in 30d0668: inlined the timestamp math in PromptCache#set so the in-memory hot path is allocation-free again apart from the CacheEntry itself. Same fix applied to StaleWhileRevalidate#set_cache_entry; compute_window/TtlWindow are gone.
Addresses Copilot review on #90: compute_window allocated a TtlWindow on every cache set even when callers only needed two timestamps (or one integer). Inline the math in PromptCache#set, RailsCacheAdapter#set, and StaleWhileRevalidate#set_cache_entry; drop compute_window and TtlWindow.
TL;DRHardens prompt cache event dispatch and the per-name generation counter on top of #89, before release.
Why#89 shipped the prompt cache surface; this PR tightens it. The event hot path was allocating payloads even with no listeners and re-resolving the AS notifier per emit. The per-name LRU cap could re-issue a generation value that still indexed orphan
@cacheentries — fixed by switching to a monotonic global counter that never collides.SummaryEvent dispatch:
PromptCacheEventsmixin fromApiClient(observer +ActiveSupport::Notificationsdispatch,PROMPT_CACHE_NOTIFICATIONconstant, payload shape).cache_observeronce at init into a 1-arg callable; the per-event hot path no longer recheckscall.arity.active_support_listening?once per emit (was 2-3×) and thread the result to dispatchers.respond_to?(:listening?)guard on the AS notifier with a comment for non-AS notifier stand-ins.Constants and types:
CacheStatus/CacheSource/CacheBackendinlib/langfuse/cache_constants.rbso producers and consumers share one definition.compute_window's Hash return with a fixed-shapeTtlWindowStruct so per-write cache sets don't allocate a 5-key Hash.PromptFetchOptionsStruct (1 arg vs 4).Generation safety:
PromptCache#invalidate_namenow draws generation values from a monotonic global counter, not a per-name counter. LRU eviction can never return a generation that's still embedded in a stale@cacheentry, so reintroducing an evicted name lands on a fresh storage key. New spec pins the contract.ChecklistVerificationRBENV_VERSION=3.2.0 rbenv exec bundle _2.4.7_ exec rspec— 1344 examples, 0 failures, 96.32% line coverageRBENV_VERSION=3.2.0 rbenv exec bundle _2.4.7_ exec rubocop --format simple— cleanscratchpad/aai_106_prompt_cache/run_all.rb— all four matrix scripts pass againstus.cloud.langfuse.comscratchpad/pr_90_dispatch_and_lru_validate.rb— new validator: lazy block payload, observer arity, AS subscriber, LRU monotonic counter, fallback payload shape, live AS subscriber on real fetch