refactor: prompt cache internals#91
Merged
Merged
Conversation
kxzk
changed the title
Reverses the over-abstraction introduced earlier in this branch. The five new "collaborator" classes added ~1000 net lines for a refactor that the PR description claimed had no behavior change — the cost didn't match the value. - Delete PromptCacheCapabilities (170 LoC). 13 forwarding methods around cache.foo / cache.respond_to?(:foo) ? cache.foo : default. PromptCache and RailsCacheAdapter both provide the full surface; only fetch_with_lock differs, handled now with one is_a? check in the coordinator. - Flatten PromptCacheCoordinator (326 → 288 LoC). Single dispatch in fetch_cached: SWR > distributed lock > simple get/set, no more distributed_enabled = nil sentinel re-checks. - Delete Resources::Batches and Resources::Traces. Each wrapped two methods through a lambda hop with no encapsulation gain. Methods inlined back onto ApiClient. - Delete PromptClientFactory (116 LoC). Was a case statement plus five thin Client delegators (one of which reached private methods via send). Inlined back onto Client. - Delete PromptClientMetadata mixin. The initialize_prompt_metadata ceremony was more cognitive load than the duplicated attr_readers it removed. api_client_spec: replace mock-heavy SWR/lock/cache-detection contexts with integration tests using real PromptCache and RailsCacheAdapter backends. The mocked tests asserted on respond_to? plumbing that no longer exists; the cache mechanics they covered are tested directly in prompt_cache_spec / rails_cache_adapter_spec. Verification: - bundle exec rspec — 1342 examples, 0 failures, 96.91% coverage - bundle exec rubocop --cache false — 0 offenses - scratchpad/aai_114_refactor/run_local.rb — all 5 scripts pass - scratchpad/aai_114_refactor/run_platform.rb — passed against https://us.cloud.langfuse.com; temporary prompt was deleted
… trivial passthroughs Continues the simplification pass after the prompt-cache cleanup. - Inline Resources::Datasets (257 LoC) and Resources::Prompts (107 LoC) back into ApiClient. Same shape as the just-deleted Batches/Traces: the classes held no state, just lambda-forwarded back to ApiClient private methods. Drops the 5-callable initializer + the 9 stub forwarder methods on each class. - Add private `request(verb, path, params: nil, body: nil)` helper. Collapses the repeated `with_faraday_error_handling do handle_response(connection.<verb>(path, ...)) end` shape across ~20 call sites. Faraday's verb-specific signatures still work because `body || params` lands in the same second positional slot. - Use `Forwardable.def_delegators :api_client, ...` for the 12 trivial Client → ApiClient pass-throughs (`list_prompts`, `list_traces`, `get_trace`, `prompt_cache_*`, `validate_prompt_cache_backend!`, etc.). Kills ~250 LoC of duplicated YARD blocks; YARD `@!method` directives keep the methods documented on Client. - Replace `payload[:foo] = bar if bar` chains with `.compact` in `ScoreClient#build_score_event` and the inlined dataset/prompt payload builders. Drops 2 rubocop disables on score_client and collapses the `add_dataset_item_fields`/`add_source_fields` micro- helpers that existed only to fit the 22-line cap. Verification: - bundle exec rspec — 1342 examples, 0 failures, 96.84% coverage - bundle exec rubocop --cache false — 0 offenses - scratchpad/aai_114_refactor/run_local.rb — 5/5 pass - scratchpad/aai_114_refactor/run_platform.rb — passed against https://us.cloud.langfuse.com; temporary prompt was deleted Net: -482 lines.
There was a problem hiding this comment.
Pull request overview
This PR refactors Langfuse’s prompt caching and prompt-client construction internals into smaller collaborators (notably a new PromptCacheCoordinator) while aiming to preserve the existing public SDK surface and prompt/cache semantics. It also updates internal call sites (rake tasks, cache warmer, request shaping) to rely on the public cache APIs and centralizes prompt-cache event payload construction.
Changes:
- Introduces
Langfuse::PromptCacheCoordinatorand shifts prompt fetch/cache branching + invalidation behavior out ofApiClient. - Consolidates prompt-cache event payload construction via
PromptCacheEvents.build_payloadand expands cache stats to include TTL/size/max_size where applicable. - Updates rake/cache-warming flows and various API request builders to use public cache APIs and a shared
ApiClient#requesthelper; refreshes docs/specs accordingly.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/langfuse/rake_cache_contract_spec.rb | Adds contract test ensuring rake cache tasks use public cache APIs. |
| spec/langfuse/prompt_cache_coordinator_spec.rb | Adds focused tests for coordinator cache statuses, events, and invalidation. |
| spec/langfuse/api_client_spec.rb | Updates cache-related specs to exercise the new coordinator-backed behavior. |
| lib/tasks/langfuse.rake | Switches cache clear flow to Client public cache APIs and prints generation. |
| lib/langfuse/text_prompt_client.rb | Minor documentation tweaks around prompt/compile behavior. |
| lib/langfuse/score_client.rb | Refactors score event body building using compact to remove nils. |
| lib/langfuse/rails_cache_adapter.rb | Enhances stats payload and makes global generation lookup safer. |
| lib/langfuse/prompt_cache.rb | Enhances stats payload with ttl/size/max_size fields. |
| lib/langfuse/prompt_cache_events.rb | Adds shared payload builder and improves observer arity handling. |
| lib/langfuse/prompt_cache_coordinator.rb | New coordinator encapsulating prompt cache read/write/SWR/lock branching and invalidation. |
| lib/langfuse/config.rb | Clarifies tracing_async and job_queue documentation. |
| lib/langfuse/client.rb | Uses Forwardable for pass-through APIs; small prompt client/fallback refactors. |
| lib/langfuse/chat_prompt_client.rb | Minor documentation/attr_reader reordering for prompt. |
| lib/langfuse/cache_warmer.rb | Uses public cache stats APIs and normalizes backend names for reporting. |
| lib/langfuse/api_client.rb | Delegates cache behavior to coordinator; introduces request helper; refactors several endpoints. |
| lib/langfuse.rb | Requires the new coordinator. |
| docs/CONFIGURATION.md | Updates text around tracing async scheduling and job queue being reserved/no-op. |
| docs/API_REFERENCE.md | Updates config table descriptions for tracing_async and job_queue. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
423
to
433
| def list_traces(**) | ||
| list_traces_paginated(**)["data"] || [] | ||
| end | ||
| # rubocop:enable Metrics/ParameterLists | ||
|
|
||
| # Full paginated response including "meta" for internal pagination use | ||
| # | ||
| # @api private | ||
| # @return [Hash] Full response hash with "data" array and "meta" pagination info | ||
| # rubocop:disable Metrics/ParameterLists | ||
| def list_traces_paginated(page: nil, limit: nil, user_id: nil, name: nil, session_id: nil, | ||
| from_timestamp: nil, to_timestamp: nil, order_by: nil, | ||
| tags: nil, version: nil, release: nil, environment: nil, | ||
| fields: nil, filter: nil) | ||
| with_faraday_error_handling do | ||
| params = build_traces_params( | ||
| page: page, limit: limit, user_id: user_id, name: name, | ||
| session_id: session_id, from_timestamp: from_timestamp, | ||
| to_timestamp: to_timestamp, order_by: order_by, tags: tags, | ||
| version: version, release: release, environment: environment, | ||
| fields: fields, filter: filter | ||
| ) | ||
| response = connection.get("/api/public/traces", params) | ||
| handle_response(response) | ||
| end | ||
| def list_traces_paginated(**) | ||
| request(:get, "/api/public/traces", params: build_traces_params(**)) | ||
| end |
Comment on lines
+151
to
+154
| generation = Langfuse.client.clear_prompt_cache | ||
| puts "Cache cleared successfully! ✓" | ||
| puts "Backend: #{Langfuse.configuration.cache_backend}" | ||
| puts "Generation: #{generation}" unless generation.nil? |
- Restore explicit keyword args on `list_traces` / `list_traces_paginated`. The `**` forwarding I introduced was silently swallowing typoed filter keys (e.g. `usre_id:`) instead of raising ArgumentError. Pre-refactor behavior is now restored: unknown kwargs raise immediately at the ApiClient boundary. Forwardable on Client preserves kwarg semantics so the strict signature still applies through `client.list_traces`. - Clarify the rake `langfuse:clear_cache` output. The task calls `clear_prompt_cache`, which is a logical generation bump — old backend entries remain until TTL/eviction. The previous "Cache cleared successfully!" message could mislead operators expecting immediate key deletion. New output explicitly states it's a logical clear and that subsequent fetches will miss the old generation and re-populate. Verification: rspec 1342/0, rubocop clean, scratchpad local + platform (against https://us.cloud.langfuse.com) pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DRSimplifies the prompt-cache and API-client internals. Public SDK behavior unchanged.
WhyThis branch started as a refactor that introduced focused collaborator classes around prompt cache, prompt clients, and API resources. Two simplification passes later, those collaborators were unwound — the abstractions weren't earning their keep against the project's "no abstractions beyond what's required" rule. The result is a net simplification of the SDK that preserves all public behavior.
ChecklistSummary
PromptCacheCoordinator(288 LoC) — single dispatch (SWR > distributed lock > simple get/set), no more sentinel-param re-checks. Replaces the 326-LoC, 11-method-deep version that preceded it.PromptCacheCapabilities— 13 forwarding methods aroundcache.foo/cache.respond_to?(:foo) ? cache.foo : default. Coordinator now calls cache methods directly; the only real branch (fetch_with_lockonly onRailsCacheAdapter) is oneis_a?check.PromptClientFactoryandPromptClientMetadata— inlined the case statement and the metadataattr_readersback intoClient/ the prompt clients.Resources::Batches,Resources::Traces,Resources::Datasets,Resources::Prompts— the lambda-passing pattern (each class took 4-5 callables that round-tripped back toApiClient) added no encapsulation. Methods inlined back onApiClient.request(verb, path, params:, body:)helper onApiClient— collapses the repeatedwith_faraday_error_handling { handle_response(connection.<verb>(...)) }shape across ~20 call sites.Forwardable.def_delegatorsfor 12 trivialClient→ApiClientpass-throughs (list_prompts,list_traces,prompt_cache_*, etc.). YARD@!methoddirectives keep them documented.payload[:foo] = bar if barchains with.compactinScoreClient#build_score_eventand the dataset/prompt payload builders. Drops 2 rubocop disables onscore_client.clear_cacheto use public SDK cache APIs.tracing_asyncand reserved/no-opjob_queueconfig.Net change
+750 / −1196 lines (−446 net). 5 collaborator classes removed plus 4 spec files for the deleted classes. Coverage delta: +0.38pp.
Verification
RBENV_VERSION=3.2.0 rbenv exec bundle _2.4.7_ exec rspec— 1342 examples, 0 failures, 96.84% coverageRBENV_VERSION=3.2.0 rbenv exec bundle _2.4.7_ exec rubocop --cache false— no offensesRBENV_VERSION=3.2.0 rbenv exec bundle _2.4.7_ exec ruby scratchpad/aai_114_refactor/run_local.rb— 5/5 scripts passRBENV_VERSION=3.2.0 rbenv exec bundle _2.4.7_ exec ruby scratchpad/aai_114_refactor/run_platform.rb— passed againsthttps://us.cloud.langfuse.com; temporary prompt was deletedgit status --short --ignored scratchpad/aai_114_refactor— scratchpad remains ignored