Add request metadata and cache-aware usage to UniResponse#111
Merged
Conversation
Contributor
Reviewer's GuideIntroduce RequestMetadata and enhanced usage tracking into core response types and adapters, wiring OpenAI and Anthropic adapters to populate metadata/usage consistently and updating docs and tests to reflect the new behavior and recent API deprecations/removals. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
Author
|
@sourcery-ai title |
sourcery-ai
Bot
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/amrita_core/adapters/openai.py" line_range="183" />
<code_context>
)
+ meta = RequestMetadata(
+ model=completion.model,
+ original_request_id=completion._request_id,
+ stop_sequence=None,
+ stop_reason=(
</code_context>
<issue_to_address>
**issue (bug_risk):** Accessing the private `_request_id` attribute directly risks breaking if the OpenAI SDK changes.
Since `_request_id` is private, future SDK changes could drop or rename it and cause `AttributeError`. Please either guard access with something like `getattr(completion, "_request_id", None)` so this fails gracefully when absent, or route this through a supported/public API once OpenAI provides one.
</issue_to_address>
### Comment 2
<location path="src/amrita_core/adapters/anthropic.py" line_range="289-295" />
<code_context>
+ stop_reason=getattr(last_msg, "stop_reason", None),
+ model=getattr(last_msg, "model", "__NOT_GIVEN__"),
+ )
usage: UniResponseUsage[int] = UniResponseUsage[int](
prompt_tokens=last_msg.usage.input_tokens,
completion_tokens=last_msg.usage.output_tokens,
total_tokens=last_msg.usage.input_tokens
+ last_msg.usage.output_tokens,
+ cache_creation=last_msg.usage.cache_creation_input_tokens,
+ cache_hit=last_msg.usage.cache_read_input_tokens,
)
else:
</code_context>
<issue_to_address>
**issue:** Unconditional access to `cache_*` usage fields may fail on Anthropic responses that don’t include these attributes.
In both streaming and non-streaming paths you access `last_msg.usage.cache_creation_input_tokens` and `last_msg.usage.cache_read_input_tokens` directly. On SDK versions or models without caching support these attributes may be missing, causing an `AttributeError`. Please use `getattr(last_msg.usage, "cache_creation_input_tokens", None)` and `getattr(last_msg.usage, "cache_read_input_tokens", None)` so the adapter remains compatible when these fields are absent.
</issue_to_address>
### Comment 3
<location path="src/amrita_core/adapters/anthropic.py" line_range="384-387" />
<code_context>
) = self._convert_tool_choice(tool_choice)
- response = await client.messages.create(
+ response: Message = await client.messages.create(
model=preset.model,
messages=anthropic_msgs,
</code_context>
<issue_to_address>
**issue (bug_risk):** The `cache_*` usage fields in the tools path have the same unconditional access risk as in `call_api`.
In `call_tools`, `usage` reads `response.usage.cache_creation_input_tokens` and `response.usage.cache_read_input_tokens` directly. If the SDK/model doesn’t populate these fields, this will raise `AttributeError`. For consistency with `call_api` and better tolerance of missing cache stats, use `getattr(response.usage, "cache_creation_input_tokens", None)` and `getattr(response.usage, "cache_read_input_tokens", None)` when building `usage`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying amritacore with
|
| Latest commit: |
cedbe12
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://74937379.amritacore.pages.dev |
| Branch Preview URL: | https://feat-fix.amritacore.pages.dev |
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
1 participant
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.
Summary by Sourcery
Add per-request metadata and stop reason tracking to unified responses, extend usage statistics with cache information, and update adapters, tests, and documentation accordingly.
New Features:
Enhancements:
Build:
Tests: