fix(langchain): emit cache creation tokens#4261
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesCache creation token tracking in LangChain span utilities
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py`:
- Around line 428-430: The `cache_creation_tokens` accumulation at the line with
`cache_creation_tokens += input_token_details.get("cache_creation", 0)` assumes
the value is always numeric, but it can be an object, causing a TypeError that
gets caught by the broad exception handler and skips remaining generations. Fix
this by safely extracting the numeric value from the `cache_creation` field,
handling both numeric values and object cases (where you should extract a
numeric property from the object), and provide a sensible default value when the
data is missing or cannot be converted.
- Around line 440-441: The write-guard conditions using `>= 0` for
cache_read_tokens and cache_creation_tokens always evaluate to true since these
counters are initialized to 0, causing the usage metadata block at line 443 to
execute unconditionally. Change the comparison operators from `>= 0` to `> 0`
for both cache_read_tokens and cache_creation_tokens to ensure the block only
executes when actual token values are present. Additionally, set
has_cache_details to True only when input_token_details is actually present and
successfully parsed, not based on these conditions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1572374-5dd7-4029-bba9-14cb3afbe46a
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py
| cache_read_tokens = (cache_read_tokens or 0) + raw_cache_read | ||
| raw_cache_creation = input_token_details.get("cache_creation") | ||
| if isinstance(raw_cache_creation, (int, float)): | ||
| cache_creation_tokens = (cache_creation_tokens or 0) + raw_cache_creation |
There was a problem hiding this comment.
Why not init the 'cache_creation_tokens' to 0 ? same for cache_read_tokens
There was a problem hiding this comment.
cache_creation_tokens and cache_read_tokens might not be provided at all. so we want to distinguish between not provided and 0.
If not provided we omit them, but we do want to log them if they have value of 0.
f5272ae to
bc84aaf
Compare
Summary by CodeRabbit