Skip to content

fix(usage): preserve existing request_usage_entries on Usage.add#3213

Merged
seratch merged 1 commit intoopenai:mainfrom
adityasingh2400:fix/usage-audit
May 8, 2026
Merged

fix(usage): preserve existing request_usage_entries on Usage.add#3213
seratch merged 1 commit intoopenai:mainfrom
adityasingh2400:fix/usage-audit

Conversation

@adityasingh2400
Copy link
Copy Markdown
Contributor

Summary

Usage.add(other) discards any pre-populated other.request_usage_entries when other also has requests == 1 and total_tokens > 0, replacing them with a freshly synthesized entry built from only the top-level fields. Nested details such as cached_tokens / reasoning_tokens carried by the existing entries are silently lost.

The two branches in add() were ordered so the synthesizer wins over an explicit per-request breakdown. The fix swaps them: if other already has an authoritative breakdown, merge it; otherwise fall back to synthesizing one from the top-level fields.

Repro (before the fix)

u1 = Usage()
u2 = Usage(
    requests=1, input_tokens=100, output_tokens=50, total_tokens=150,
    request_usage_entries=[
        RequestUsage(
            input_tokens=100, output_tokens=50, total_tokens=150,
            input_tokens_details=InputTokensDetails(cached_tokens=10),
            output_tokens_details=OutputTokensDetails(reasoning_tokens=5),
        )
    ],
)
u1.add(u2)
# u1.request_usage_entries[0].input_tokens_details.cached_tokens == 0   (was 10)
# u1.request_usage_entries[0].output_tokens_details.reasoning_tokens == 0   (was 5)

Changes

  • src/agents/usage.py — reorder the add() branches so an explicit request_usage_entries list is preferred over a synthesized entry built from top-level fields.
  • tests/test_usage.py — regression test (test_usage_add_preserves_existing_entries_when_top_level_also_set).

Test plan

  • New regression test fails on main, passes after the fix.
  • Full tests/test_usage.py passes (14/14).
  • tests/test_agent_runner_streamed.py::test_streamed_run_preserves_request_usage_entries_after_retry and the conversation-locked variant still pass — these exercise the synthesize path with requests == 2, which the fix does not touch.
  • ruff check clean on touched files.

When `Usage.add(other)` was called with `other.requests == 1` and
`other.total_tokens > 0`, the method synthesized a fresh
`RequestUsage` entry from `other`'s top-level fields and silently
discarded any pre-populated `other.request_usage_entries`. The
synthesized entry only carries top-level token details, so nested
fields such as `cached_tokens` and `reasoning_tokens` already
present on the existing entries were lost.

Reorder the branches so that when `other.request_usage_entries` is
non-empty we merge those authoritative entries directly, and only
fall back to synthesizing one when no per-request breakdown is
provided. Add a regression test that exercises this exact shape.
@github-actions github-actions Bot added bug Something isn't working feature:core labels May 8, 2026
@seratch
Copy link
Copy Markdown
Member

seratch commented May 8, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

ℹ️ 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".

@seratch seratch added this to the 0.17.x milestone May 8, 2026
@seratch seratch merged commit ec5523d into openai:main May 8, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working feature:core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants