Skip to content

fix(openai): record cached and reasoning tokens on the LLM metrics counters#2758

Merged
benfrank241 merged 1 commit into
vectorize-io:mainfrom
ebarkhordar:fix/openai-metrics-cached-thoughts
Jul 17, 2026
Merged

fix(openai): record cached and reasoning tokens on the LLM metrics counters#2758
benfrank241 merged 1 commit into
vectorize-io:mainfrom
ebarkhordar:fix/openai-metrics-cached-thoughts

Conversation

@ebarkhordar

Copy link
Copy Markdown
Contributor

Root cause

OpenAICompatibleLLM extracts cached_tokens and thoughts_tokens from the provider's
usage object on both call paths and passes them to TokenUsage, but not to
metrics.record_llm_call. That collector already accepts both kwargs and keeps a
bucketed counter for each (llm_tokens_cached_input, llm_tokens_thoughts), so the
values are extracted, used for the return contract, and then dropped on the metrics path.

Two distinct effects, which are worth keeping separate:

  1. Reasoning tokens reach no counter at all (regression). fix(openai): propagate reasoning_tokens into TokenUsage for OpenAI-compatible providers #2378 made output_tokens
    visible-only by subtracting thoughts_tokens, and that subtraction sits directly above
    record_llm_call. So reasoning was removed from the metrics path rather than moved onto
    llm_tokens_thoughts. Before fix(openai): propagate reasoning_tokens into TokenUsage for OpenAI-compatible providers #2378 those tokens were at least still counted inside
    output_tokens.
  2. cached_input_tokens has always read 0 here. It has never been passed by this
    provider since the counter landed in feat(llm): provider prompt-prefix caching — retain + consolidation + reflect (bank-agnostic, default-on) #1936; gemini_llm is the only provider that
    passes it. This half is a gap, not a regression.

The rationale for the counter is already written next to it in metrics.py:

Surfacing them as a distinct counter is required for honest cost attribution: a workload
that "looks cheap" by output volume can be silently expensive if the model is doing long
reasoning chains.

That comment introduces them as the Gemini 2.5+ family, which is where they came from, but
the counter name and the MetricsCollectorBase.record_llm_call signature are both
provider-neutral, and #2378 established that OpenAI-compatible reasoning models report the
same counts. An o-series or deepseek-r1 workload is exactly the one that reads as cheap
today.

Fix

Pass cached_input_tokens and thoughts_tokens at the two call sites that parse a usage
object (call() and call_with_tools()). Four added kwargs, no logic or signature change.

The other two record_llm_call sites are deliberately untouched: the tool_use_failed
fallback records input_tokens=0, output_tokens=0 (it has no usage object), and the Ollama
native path reads prompt_eval_count/eval_count, which carry no reasoning or cached
counts.

Invariant: recorded output_tokens + recorded thoughts_tokens equals the provider's
completion_tokens, so every billed output token lands on exactly one counter, never zero
and never two.

How I verified

Clean python:3.12-slim container, package installed into real site-packages (not a
source-dir import), against current main (9676fc1).

Usage shape: prompt=2000 (cached=1024), completion=83 (reasoning=64).

record_llm_call receives
main output_tokens=19, no thoughts_tokens, no cached_input_tokens
this branch output_tokens=19, thoughts_tokens=64, cached_input_tokens=1024

On main that is 19 of 83 billed output tokens and 0 of 1024 cached; on the branch,
83 of 83 and 1024 of 1024.

For the regression half I installed #2378's parent (701de32) in the same container and ran
the same probe: record_llm_call(output_tokens=83), so all billed output was visible to
the counters before that change.

Tests: three added to tests/test_token_usage_cached_thoughts.py, asserting on a
MagicMock(spec=MetricsCollector). All three fail on main (KeyError: 'thoughts_tokens')
and pass on the branch; the full file is 14/14 green. The existing provider tests patch
get_metrics_collector without asserting on it, which is why this survived them.
ruff check and ruff format --check (0.14.9, per scripts/hooks/lint.sh) are clean on
both changed files, and ty check hindsight_api reports the same 181 diagnostics as
pristine main.

What I did not verify. No live provider call: the usage objects are
SimpleNamespace mocks, and the completion=83, reasoning=64 pair is quoted from #2378's
own commit message rather than measured by me. My claim is only that HEAD drops these
fields given that usage shape. Verification also stops at the provider-to-collector
boundary: I assert the kwargs the provider passes, and read record_llm_call to confirm
each lands on a counter. I did not scrape a live Prometheus endpoint.

Scope

Kept to the OpenAI-compatible provider, following #2378's precedent of scoping to one
provider. anthropic_llm.py has the same cached_input gap (it extracts cached_tokens
at line 320, passes it to the span recorder, and omits it from record_llm_call at line
324), and litellm_llm.py has the same shape. #2378 explicitly left anthropic as an
optional follow-up. Happy to do those as a separate PR if you want them, but a
multi-provider diff seemed more conflict-prone and harder to review than it is worth.

Also out of scope: the OTel span recorder and the DB trace recorder accept cached_tokens
but have no thoughts_tokens parameter, so reasoning is invisible there too. That needs a
new parameter and a GenAI attribute, so it is a bigger change than this one.


AI assistance disclosure: this change was written with AI assistance. I ran the
reproduction, the differential against #2378's parent, and the tests myself, and I can
explain every line.

…unters

The OpenAI-compatible provider extracts cached_tokens and thoughts_tokens on
both call paths and hands them to TokenUsage, but never passes them to
metrics.record_llm_call, which accepts and buckets both. Two separate effects:

- Reasoning tokens reach no counter at all. vectorize-io#2378 made output_tokens
  visible-only by subtracting thoughts_tokens directly above the
  record_llm_call, so the reasoning half of the billed output was removed
  from the metrics path rather than moved onto llm_tokens_thoughts. Before
  vectorize-io#2378 those tokens were still counted inside output_tokens.
- cached_input_tokens has read 0 for every OpenAI-compatible provider since
  the counter was added; only gemini_llm passes it.

Pass both kwargs at the two call sites that parse a usage object. The
fallback path (no usage) and the Ollama native path (no reasoning or cached
fields) are unchanged.

Invariant: recorded output_tokens + recorded thoughts_tokens equals the
provider's completion_tokens, so every billed token lands on exactly one
counter. The new tests assert on the collector itself; the existing ones
patch it without asserting, which is why this went unnoticed.
@benfrank241
benfrank241 merged commit d9bc612 into vectorize-io:main Jul 17, 2026
87 checks passed
@ebarkhordar
ebarkhordar deleted the fix/openai-metrics-cached-thoughts branch July 17, 2026 16:10
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.

2 participants