feat(generations): add cost_details and usage_details support#61
feat(generations): add cost_details and usage_details support#61
Conversation
Greptile SummaryAdds Key changes:
Note: The Confidence Score: 5/5
Important Files Changed
Last reviewed commit: eeba1bb |
There was a problem hiding this comment.
Pull request overview
This PR adds support for usage_details and cost_details setters to the Generation observation class, deprecating the legacy usage= setter. The change moves away from the old camelCase attribute conversion behavior to a simpler approach that preserves the original key format. All generation-focused documentation has been updated to use the new usage_details API.
Changes:
- Added
Generation#usage_details=andGeneration#cost_details=setters that delegate through the unified observation update path - Deprecated
Generation#usage=with a deprecation warning that forwards tousage_details= - Updated all documentation examples to use
usage_detailsinstead ofusage
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/langfuse/observations.rb | Added new usage_details= and cost_details= setters, deprecated usage= with warning mechanism |
| spec/langfuse/base_observation_spec.rb | Added comprehensive tests for new setters and deprecation behavior |
| docs/TRACING.md | Updated examples to use usage_details instead of usage |
| docs/SCORING.md | Updated examples to use usage_details instead of usage |
| docs/RAILS.md | Updated examples to use usage_details instead of usage |
| docs/PROMPTS.md | Updated examples to use usage_details instead of usage |
| docs/GETTING_STARTED.md | Updated examples to use usage_details instead of usage |
| docs/ARCHITECTURE.md | Updated examples to use usage_details instead of usage |
| docs/API_REFERENCE.md | Added documentation for new setters and deprecation notice |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def usage=(value) | ||
| return unless @otel_span.recording? | ||
| warn_usage_deprecation | ||
| self.usage_details = value | ||
| end |
There was a problem hiding this comment.
The Embedding class has a usage= setter (line 681) that silently forwards to usage_details without any deprecation warning, while Generation#usage= logs a deprecation warning. For consistency, the Embedding class should either also have a deprecation warning for its usage= setter, or should have a corresponding usage_details= setter added. This inconsistency could confuse users about which API to use.
| def usage_details=(value) | ||
| update_observation_attributes(usage_details: value) | ||
| end | ||
|
|
||
| usage_json = usage_hash.to_json | ||
| @otel_span.set_attribute("langfuse.observation.usage", usage_json) | ||
| # @param value [Hash] Cost details hash (e.g., total_cost and provider-specific fields) | ||
| # @return [void] | ||
| def cost_details=(value) | ||
| update_observation_attributes(cost_details: value) | ||
| end |
There was a problem hiding this comment.
The model= and model_parameters= setters directly call @otel_span.set_attribute() and check @otel_span.recording?, while the new usage_details= and cost_details= setters use update_observation_attributes() which internally handles the recording check. This inconsistency in implementation patterns within the same class may lead to maintenance issues. Consider refactoring model= and model_parameters= to also use update_observation_attributes() for consistency, or document why they need different implementation patterns.
| Langfuse.configuration.logger&.warn( | ||
| "Langfuse::Generation#usage= is deprecated; use #usage_details= instead." | ||
| ) |
There was a problem hiding this comment.
The deprecation warning uses Langfuse.configuration.logger&.warn(...) with the safe navigation operator. This means if the logger is nil, the warning will be silently suppressed. While this is likely intentional for cases where logging isn't configured, consider whether this is the desired behavior. If deprecation warnings should always be visible regardless of logger configuration, you might want to fall back to warn or $stderr.puts when the logger is nil.
| Langfuse.configuration.logger&.warn( | |
| "Langfuse::Generation#usage= is deprecated; use #usage_details= instead." | |
| ) | |
| message = "Langfuse::Generation#usage= is deprecated; use #usage_details= instead." | |
| logger = Langfuse.configuration.logger | |
| if logger | |
| logger.warn(message) | |
| else | |
| warn(message) | |
| end |
| "Langfuse::Generation#usage= is deprecated; use #usage_details= instead." | ||
| ) | ||
| end | ||
|
|
There was a problem hiding this comment.
The test verifies that the deprecation warning is logged when usage= is called, but it doesn't test that the warning is only logged once per instance even if usage= is called multiple times. Consider adding a test case that calls usage= twice and verifies the warning is logged exactly once, to ensure the @usage_deprecation_logged flag works as intended.
| it "logs usage deprecation warning only once per instance" do | |
| allow(Langfuse.configuration.logger).to receive(:warn) | |
| generation.usage = { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 } | |
| generation.usage = { prompt_tokens: 20, completion_tokens: 10, total_tokens: 30 } | |
| span_data = generation.otel_span.to_span_data | |
| usage_attr_value = span_data.attributes["langfuse.observation.usage_details"] | |
| expect(usage_attr_value).not_to be_nil | |
| usage_attr = JSON.parse(usage_attr_value) | |
| expect(usage_attr["prompt_tokens"]).to eq(20) | |
| expect(usage_attr["completion_tokens"]).to eq(10) | |
| expect(usage_attr["total_tokens"]).to eq(30) | |
| expect(span_data.attributes).not_to have_key("langfuse.observation.usage") | |
| expect(Langfuse.configuration.logger).to have_received(:warn).once.with( | |
| "Langfuse::Generation#usage= is deprecated; use #usage_details= instead." | |
| ) | |
| end |
- Delegate Generation#model= and #model_parameters= to update_observation_attributes, matching the pattern used by usage_details=, cost_details=, and the Embedding class. This also fixes a pre-existing bug where these setters used wrong OTel attribute names (langfuse.observation.model vs langfuse.observation.model.name). - Simplify warn_usage_deprecation by removing unnecessary defined? guard. - Update tests to expect correct OTel attribute names.
Supersedes #55 after branch rename to conventional format.
TL;DRWhyChecklistCloses #
Summary
Generation#usage_details=andGeneration#cost_details=setters that delegate through the unified observation update path.Generation#usage=as a compatibility alias, but now logs a deprecation warning and forwards tousage_details.langfuse.observation.usage/camelCase remap behavior from the generation setter path so usage payload shape is preserved.usage_detailsand included deprecation guidance forusage.bundle exec rspec(1156 examples, 0 failures, 98.04% coverage),bundle exec rubocop(no offenses), and Langfuse CLI schema/OTLP endpoint validation vianpx langfuse-cli.Architecture (Core Change)
Mermaid not applicable.