feat(usage): add agent_name and model_name to RequestUsage#2914
feat(usage): add agent_name and model_name to RequestUsage#2914adityasingh2400 wants to merge 5 commits intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80262bcf4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
We're not yet fully convinced to add these data because they could be incomplete in many cases. Refer to the discussion at #2114 for more details. |
80262bc to
b5480cb
Compare
|
Thanks for the feedback @seratch! Bug fix: The On completeness concerns: I've read the discussion at #2114. The fields are already guarded against the incompleteness issue:
Both fields are typed Happy to drop |
|
@seratch quick FYI while we wait, pushed 5ac2475 with three small hardening follow-ups that are directly relevant to the "could be incomplete in many cases" concern:
The completeness contract is unchanged: both fields are |
There was a problem hiding this comment.
💡 Codex Review
openai-agents-python/src/agents/usage.py
Lines 226 to 230 in 5ac2475
When other.requests == 1, this fast path runs even if other.request_usage_entries already exists, so the new RequestUsage is rebuilt from aggregate totals and uses only the passed agent_name/model_name values. In scenarios like re-aggregating deserialized usage, this can silently drop or overwrite existing per-request attribution (entry.agent_name / entry.model_name), while the elif other.request_usage_entries branch explicitly preserves existing names. Please prefer merging other.request_usage_entries first (or copy attribution from the existing entry) so single-request usages keep their original metadata.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Added optional `agent_name: str | None = None` and `model_name: str | None = None` fields to the `RequestUsage` dataclass (backward-compatible) - Modified `Usage.add()` to accept keyword-only `agent_name` and `model_name` parameters and annotate the resulting `RequestUsage` entry with them - Added `_get_model_name()` helper in run_loop.py to safely extract the model name string from any Model implementation via duck-typing - Updated both `run_single_turn_streamed` and `get_new_response` call-sites in run_loop.py to pass agent.name and the resolved model name when adding usage - When merging pre-existing `request_usage_entries`, agent/model names are applied to entries that don't already have them set (existing names preserved) - Updated `serialize_usage` / `deserialize_usage` in usage.py to round-trip the new fields through JSON - Added 9 new tests covering: default None values (backward compat), explicit field population, Usage.add() propagation, entry merge semantics, single-agent runner integration, model_name detection, and multi-agent per-agent attribution scenario - Full test suite passes (2198 tests); lint and pyright clean Closes openai#2100
The elif branch in Usage.add() was mutating agent_name/model_name directly on the RequestUsage objects inside other.request_usage_entries, bypassing the non-overwrite guard and causing silent mis-attribution when the same Usage instance was added to multiple aggregators. Fix: create a new RequestUsage copy for each entry with the annotation applied, leaving the originals unchanged. Adds a regression test that verifies the original entries are not mutated.
…, defensive _get_model_name Three small follow-ups from the CodeRabbit review of PR #4: * test_multi_agent_run_attributes_usage_to_correct_agents now sets a distinct model attribute on each FakeModel and asserts model_name on both the triage and specialist RequestUsage entries, so a regression that drops or cross-wires model_name during handoff aggregation will surface here too. * Adds test_serialize_deserialize_roundtrip_preserves_agent_and_model_names that builds a Usage with both a named and an unnamed RequestUsage entry, runs serialize_usage then deserialize_usage, and asserts both fields survive on the named entry while staying None on the unnamed one. This exercises both branches of the conditional emit in serialize_usage. * Hardens _get_model_name against custom Model implementations whose descriptors or properties raise on attribute access: the helper now swallows exceptions, falls back to model_name when model is missing, and rejects empty strings. This keeps usage accounting from ever crashing the run on a third-party Model.
5ac2475 to
fa0a85f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa0a85f3b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…add() When other.requests == 1 but other.request_usage_entries is already populated, merge those entries (with the same agent_name/model_name annotation rules as the multi-entry path) instead of creating a fresh RequestUsage that only used add() kwargs and dropped prior attribution. Adds regression test for Codex review on PR openai#2914.
|
Follow-up pushed ( Addressed both open Codex threads on
Added Threads marked resolved; CI should re-run on the push. |
Summary
Adds optional
agent_nameandmodel_namefields toRequestUsageso developers can attribute token usage and costs to specific agents and models in multi-agent workflows.Problem
In complex multi-agent systems,
result.context_wrapper.usage.request_usage_entriescontains oneRequestUsageper LLM call, but there is no way to know which agent or model generated each entry. This makes per-agent cost attribution impossible.Solution
agent_name: str | None = Noneandmodel_name: str | None = NonetoRequestUsage(backward-compatible — defaults toNone)Usage.add()to accept keyword-onlyagent_nameandmodel_nameparameters and annotate the resultingRequestUsageentry_get_model_name()helper that safely duck-types the model name from anyModelimplementation (e.g.OpenAIResponsesModel.model,OpenAIChatCompletionsModel.model)run_single_turn_streamedandget_new_responsecall-sites inrun_loop.pyto passagent.nameand resolved model namerequest_usage_entries, agent/model names are applied only to entries that don't already have them (existing names preserved)serialize_usage/deserialize_usageto round-trip the new fields through JSONRequestUsageor callingUsage.add()without these fields continues to workBefore / After
Usage Example
Files Changed
src/agents/usage.pyagent_name/model_namefields toRequestUsage; updatedUsage.add(),serialize_usage,deserialize_usagesrc/agents/run_internal/run_loop.py_get_model_name()helper; updated bothusage.add()call-sitestests/test_usage.pyTest Plan
RequestUsagefield population (explicit + default None)Usage.add()propagation and backward compat testsagent_nameis set.modelattributeRequestUsageentry has the correctagent_nameCloses #2100
Summary by CodeRabbit
New Features
Tests