Skip to content

fix(v2): accumulate all billable usage fields across retries (#2493) - #2497

Closed
mbsdeepak wants to merge 1 commit into
567-labs:mainfrom
mbsdeepak:fix/retry-usage-accumulation
Closed

fix(v2): accumulate all billable usage fields across retries (#2493)#2497
mbsdeepak wants to merge 1 commit into
567-labs:mainfrom
mbsdeepak:fix/retry-usage-accumulation

Conversation

@mbsdeepak

Copy link
Copy Markdown

Addresses #2493.

Problem

update_total_usage enumerated the token fields to sum by hand, once per provider, so every counter a provider SDK added afterwards was silently dropped:

  • Anthropic (instructor/v2/providers/anthropic/usage.py) — the nested cache_creation and server_tool_use sub-models were never accumulated. initialize_usage() leaves them None and the old loop summed only four flat fields, so the ephemeral cache-write and web-search/fetch breakdowns stayed None for the whole run.
  • OpenAI (instructor/v2/core/usage.py) — the accumulator's *_tokens_details objects (which only carried audio/reasoning/cached) were model_copy'd back onto the response, overwriting accepted_prediction_tokens / rejected_prediction_tokens with None.

Nothing raised, and input_tokens / prompt_tokens stayed correct, so the under-count only surfaced when reconciling against a provider invoice.

Fix

Replace both hand-written lists with a single generic accumulator (accumulate_usage) that walks the pydantic usage model and sums every numeric field, recursing into nested sub-models and adopting a zeroed copy when the accumulator's sub-model is None. Non-numeric fields (service_tier, inference_geo) take the latest reported value. New SDK counters are now picked up automatically — the "generic" shape suggested in the issue. Both provider paths route through it, and the running total is mirrored back onto the response in place, so a CompletionUsage subclass keeps its type.

Before / after

Anthropic, three identical retries, on the repo's pinned anthropic==0.93.0:

before:  total.cache_creation is None       -> AttributeError reading ephemeral_5m_input_tokens
after:   input=300   ephemeral_5m=1500   web_search=6

Tests

tests/v2/test_usage_accumulation.py reproduces both provider bugs (pure accounting logic, no API keys or network), plus a subclass-preservation test and a version-independent test of the accumulation rules (nesting, None handling, non-numeric last-wins).

Full suite locally: 1656 passed, 164 skipped; ruff check clean.

update_total_usage enumerated token fields by hand in two places, so counters
newer SDK releases added were dropped: on Anthropic the nested cache_creation /
server_tool_use sub-models were never accumulated (left None / stale), and on
OpenAI the accumulator's details object was copied onto the response, wiping
accepted_prediction_tokens / rejected_prediction_tokens to None.

Replace both hand-written lists with a generic accumulator that walks the
pydantic usage model and sums every numeric field, recursing into nested
sub-models and adopting a zeroed copy when the accumulator's sub-model is None.
Non-numeric fields (service_tier, inference_geo) take the latest value. New SDK
counters are now picked up automatically.

Addresses 567-labs#2493

@ErenAta16 ErenAta16 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed #2493, so treat this as an interested party. The generic walk is the right shape and this version has a design detail the other two do not. It also has one gap that sits directly under the claim in its own docstring.

The gap: the walk only visits declared fields, so a counter the SDK has not modelled yet is dropped. The SDK usage models are extra="allow", so a field the API starts returning before Stainless regenerates lands in model_extra rather than in model_fields. Measured on real anthropic==0.93.0, three attempts each reporting a counter the pinned SDK does not declare:

Usage.model_config["extra"]              -> 'allow'
Usage.model_validate(raw).model_extra    -> {'brand_new_billable_tokens': 7}
"brand_new_billable_tokens" in Usage.model_fields -> False

after 3 attempts, walking model_fields only:
  input_tokens=300  ephemeral_5m=60  brand_new_billable_tokens=None   (expected 21)

Declared fields all accumulate correctly, so this is narrow. But it is the same failure mode the module docstring here promises to end: "new billable fields are picked up automatically rather than needing a matching edit in two places every time." SDK lag is exactly when that matters, and it is a real window; output_tokens_details did not appear in anthropic until well after the API was returning the data. Walking set(type(m).model_fields) | set(m.model_extra or {}) in _accumulate_into and _zero_numeric closes it, and the same union in _sync_into mirrors it back.

The design detail this version gets right and I would not want lost. _sync_into mutates an existing sub-model in place instead of replacing it:

existing = getattr(response, name, None)
if isinstance(existing, BaseModel):
    _sync_into(existing, value)
else:
    setattr(response, name, value.model_copy(deep=True))

Replacing with a copy of the accumulator's object means the response ends up holding whatever concrete class the accumulator was built with. That is fine while both sides are the same class, but _initialize_usage hands the OpenAI accumulator to every non-Anthropic provider, and any provider shipping a CompletionUsage subclass with extra fields would silently have them swapped out for the base type. Keeping the response's own instance and writing into it avoids that entirely. Worth a comment saying that is deliberate, because it reads like an unnecessary branch.

Also correct, and easy to get wrong. Handling bool before (int, float) in both _zero_numeric and _accumulate_into. bool is an int subclass, so without that ordering a flag would be zeroed and then summed into 2, 3, 4. Two of the three implementations under review got this right and it is not obvious from reading; the explanatory comment earns its place.

Verified separately, and it constrains all three PRs. pyproject.toml pins anthropic==0.93.0, which has no output_tokens_details and no such module. The docstring here lists output_tokens_details (thinking_tokens) as one of the accumulated sub-models; that is true on a newer SDK and a no-op on the pinned one, which the generic walk handles silently and correctly. That is worth saying explicitly in the docstring, because it is the property that makes this approach necessary rather than merely tidier: #2499 names the same sub-model directly and raises ImportError on 0.93.0 on every call.

One correction to my own issue. I wrote #2493 against anthropic==0.116.0 and listed thinking_tokens among the fields left stale. It does not exist on the version instructor installs, so that third of the issue does not apply to the pinned SDK. The cache_creation and server_tool_use halves do, and the OpenAI half does; I measured cache_write_tokens landing as None on main against openai==2.52.0 where the provider had sent it three times.

jxnl added a commit that referenced this pull request Aug 3, 2026
## Summary

- accumulate declared, nested, and unknown numeric OpenAI/Anthropic
usage fields across retries while preserving non-numeric metadata
- add corrective feedback when a Responses API retry receives no tool
call
- preserve raw iterable type hints through sync and async v2
parallel-tool wrappers
- strengthen API-key-free coverage for current and future SDK usage
counters

## Consolidated and superseded items

- closes #2493
- consolidates contributor work from #2498, #2500, and #2501 with
original commit authorship preserved
- supersedes #2497 because it drops unknown `model_extra` counters
- supersedes #2499 because its hand-maintained provider field lists
would drift as SDKs evolve

## Validation

- focused changed-surface suite: `110 passed`
- broad offline v2/coverage suite: `2368 passed, 91 skipped, 73
deselected`
- Ruff check and format check: passed
- scoped `ty check`: passed
- `uv lock --check`: passed
- pre-commit hooks and `git diff --check`: passed

The 73 deselected tests require live provider credentials. An unfiltered
local run confirmed its 22 failures were provider network connections in
the restricted environment; GitHub provider jobs remain the
authoritative validation for those paths.

## Intentionally skipped

- provider additions or expansions: #2436, #2435, #2423, #2409, #2384,
#2322, #2306, #2298, #2283, #2168, #2086; issues #2408, #2383, #2365,
#2260, #2084, #2076
- broad architecture, product, security, or streaming decisions: #2394,
#2392, #2357, #2356, #2355, #2351, #2321, #2307, #2287, #2263; issues
#2479, #2403, #2393, #2391, #2316, #2272, #2056
- dependency batch: #2433
- nontrivial examples and editorial/resource additions: #2468, #2405,
#2401, #2354, #2346, #2311, #2305; issue #2404

These remain open because they need dedicated product, architecture,
provider, security, dependency, or editorial review and are not required
for the `1.15.5` patch release.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes retry usage totals and reask message content on failure paths;
scope is limited and heavily covered by tests, with no auth or
data-store changes.
> 
> **Overview**
> Bundles three v2 retry and wrapper fixes for a patch release.
> 
> **Retry usage accounting** replaces hand-maintained token field sums
with generic `_accumulate_models` on Pydantic usage objects. Numeric
fields (including nested models and `model_extra` counters) add across
retries; booleans and other non-numeric metadata are not treated as
billable. OpenAI and Anthropic paths share this logic.
> 
> **OpenAI Responses reask** appends a user correction when
`RESPONSES_TOOLS` validation fails but the output has no tool calls
(e.g. reasoning-only), so retries include feedback instead of repeating
the same request.
> 
> **Parallel tools** in `patch_v2` skips `prepare_response_model` and
does not replace `response_model` with the handler’s prepared wrapper
for parallel modes, keeping raw `Iterable[...]` hints so schemas and
parsed results include every member type.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
bbddca1. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@jxnl

jxnl commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #2502, now merged to main. The canonical implementation also walks Pydantic model_extra fields, which this patch omitted, so newly added SDK counters remain cumulative. Closing this overlapping source PR in favor of the consolidated fix.

@jxnl jxnl closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants