Skip to content

Add cache-stat fields to Response.usage#136

Merged
chris-colinsky merged 4 commits into
mainfrom
feature/0047-response-usage-cache-fields
Jun 6, 2026
Merged

Add cache-stat fields to Response.usage#136
chris-colinsky merged 4 commits into
mainfrom
feature/0047-response-usage-cache-fields

Conversation

@chris-colinsky

@chris-colinsky chris-colinsky commented Jun 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Extends the Usage model with two optional non-negative integer fields: cached_tokens (prefix-cache hit count) and cache_creation_tokens (cache-write count). Both default to None to preserve the absent-vs-reported-zero distinction the spec mandates.
  • The OpenAI adapter sources cached_tokens from the nested response path usage.prompt_tokens_details.cached_tokens; vLLM and other OpenAI-compatible servers that surface implicit-cache stats follow the same nesting. cache_creation_tokens stays None for this mapping per the OpenAI-compat wire surface. Malformed values surface as ProviderInvalidResponse via the existing Pydantic validation path.
  • First PR of the v0.13.0 LLM hardening cycle (proposal 0047). Foundation for OTel cache-attribute emission (openarmature.llm.cache_read.input_tokens) and the typed LlmCompletionEvent work landing in subsequent PRs.

Test plan

  • uv run pytest tests/unit -x (84 passing; 9 new tests covering construction defaults, validation rejection, and end-to-end source paths)
  • uv run pyright src/openarmature/llm/response.py src/openarmature/llm/providers/openai.py tests/unit/test_llm_provider.py (clean)
  • uv run ruff check (clean)
  • Eyeball the Pydantic field shapes and the nested extraction in openai.py

Extend the Usage model with two optional non-negative integer fields:
cached_tokens (count of input tokens that hit a prefix cache) and
cache_creation_tokens (count of input tokens written to the cache).
Both default to None to preserve the absent-vs-reported-zero
distinction the spec mandates.

The OpenAI adapter sources cached_tokens from the nested response
path usage.prompt_tokens_details.cached_tokens; vLLM and other
OpenAI-compatible servers that surface implicit-cache stats follow
the same nesting. cache_creation_tokens stays None for this mapping
per the OpenAI-compat wire surface. Malformed values surface as
ProviderInvalidResponse via the existing Pydantic validation path.

Foundation for OTel cache-attribute emission and the typed LLM
completion event work landing in subsequent PRs of the v0.13.0
LLM hardening cycle.
Copilot AI review requested due to automatic review settings June 6, 2026 18:25

Copilot AI 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.

Pull request overview

Extends the LLM Response.usage surface to include cache-related token counters, and wires the OpenAI-compatible provider mapping to populate cached_tokens from usage.prompt_tokens_details.cached_tokens while preserving the absent-vs-reported-zero distinction.

Changes:

  • Added optional Usage.cached_tokens and Usage.cache_creation_tokens fields (non-negative ints, default None).
  • Updated OpenAI provider response parsing to source cached_tokens from usage.prompt_tokens_details.cached_tokens.
  • Added unit tests covering defaults, validation, and end-to-end extraction behavior for the new cache-stat field.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/openarmature/llm/response.py Extends the Usage Pydantic model with optional cache-stat fields and documentation.
src/openarmature/llm/providers/openai.py Sources cached_tokens from the nested OpenAI-compatible usage payload.
tests/unit/test_llm_provider.py Adds unit tests for defaulting, validation, and OpenAI parsing behavior for cache stats.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/openarmature/llm/providers/openai.py Outdated
Conformance fixtures pre-dating proposal 0047 assert the three base
token-count fields only and don't mention the new cache-stat fields.
Adding cached_tokens and cache_creation_tokens to Usage made the
exact-equality comparison fail against any fixture whose expected
usage shape doesn't include the cache fields.

Treat the fixture's expected usage as the floor: only compare keys
the fixture explicitly asserts about. A spec-required field absent
from actual still fails the comparison (the filtered dict won't
contain it). Impl-extension fields that the fixture is silent about
no longer trip exact-equality, so additive Usage extensions don't
break pre-existing fixtures.
Address PR review feedback: passing cached_tokens=None
unconditionally marks the Pydantic field as "set", so
model_dump(exclude_unset=True) includes it as None — undistinguishable
from a wire response that explicitly carried a null cached_tokens.

Switch the if-branch to a kwargs-dict pattern that only sets
cached_tokens when prompt_tokens_details is a dict AND the nested
cached_tokens key is present. Attribute access (usage.cached_tokens)
still returns None when the wire didn't report; exclude_unset dumps
now reflect the wire shape exactly. The else-branch's explicit
cached_tokens=None (added in the previous commit for if/else symmetry)
is dropped — symmetry now sits at "reflect wire reality", not "pass
identical args".

Two regression tests pin the bidirectional projection: exclude_unset
omits cached_tokens when the wire didn't report it; includes it with
the int value when it did.
Copilot AI review requested due to automatic review settings June 6, 2026 18:34

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread tests/conformance/test_llm_provider.py Outdated
Address PR review feedback: the conformance fixture's typed model
allows usage: null. When that happens, expected_usage is None and
the subset filter's k in expected_usage raises TypeError instead of
producing a clean assertion failure.

Wrap the subset filter in an isinstance(dict) guard. The mapping path
keeps the same subset semantics; the non-mapping path falls back to
direct comparison so the assert fires with a clear shape mismatch.
@chris-colinsky chris-colinsky merged commit dd804e2 into main Jun 6, 2026
6 checks passed
@chris-colinsky chris-colinsky deleted the feature/0047-response-usage-cache-fields branch June 6, 2026 18:49
chris-colinsky added a commit that referenced this pull request Jun 10, 2026
* Correct v0.13.0 release narrative per spec review

Three blocking + three should-fix items spec flagged on the
pre-tag review. All narrative; no code behavior change.

- 0047 CHANGELOG entry mis-attributed pieces 1+2 (Response.usage
  cache fields + OTel cache attributes) to v0.12.0. Verified via
  git: those landed in PRs #136 + #140 post-v0.12.0-tag, so all
  three pieces of 0047 ship in v0.13.0. Reframed.
- conformance.toml [proposals."0047"] leading-comment block had
  the same v0.12.0 mis-attribution. Same correction; added PR
  references for traceability.
- Unreleased section had two ### Added headings with the 0057
  entry orphaned below ### Changed. Consolidated.
- Spec pin advance text undercounted the cycle journey (said
  v0.51.0 → v0.53.0; actual is v0.46.0 → v0.53.0 across three
  hops). Reframed and listed absorbed proposals inline.
- tool_call.arguments JSON encoding now uses sort_keys=True
  (functionally equivalent but byte-different for downstream
  snapshot consumers). Surfaced as its own ### Changed entry
  instead of buried in the 0047 ### Added.
- conformance.toml [proposals."0049"] leading-comment block
  grew the fixture-deferral surface (057-068 + 069-073 parser-
  deferred pending harness directive schema catch-up; behavior
  pinned by unit tests) per spec OQ2.

* Migrate LLM-event docs to typed-event-first

Three docs still pushed the legacy sentinel-namespace pattern as
the primary path for custom observers consuming LLM events and
custom providers emitting them. After v0.13.0 the bundled provider
emits typed LlmCompletionEvent / LlmFailedEvent variants directly;
the bundled OTel + Langfuse observers consume via isinstance
discrimination. Rewrites:

- docs/concepts/observability.md: "Publishing LLM events for
  custom observers" → "Consuming LLM events in custom observers".
  Typed-event consumption shown as primary (isinstance branch on
  LlmCompletionEvent + LlmFailedEvent with the mutual-exclusion +
  field-set notes). Sentinel pattern demoted to a "Legacy
  sentinel-namespace pattern (compatibility surface)" subsection
  for downstream code interoperating with custom providers that
  haven't migrated.
- docs/model-providers/authoring.md: custom-provider emission
  sketch rewritten — dispatch LlmCompletionEvent on success,
  LlmFailedEvent alongside the §7 exception on failure. Shows the
  current-attempt-index / current-fan-out-index / etc. scoping
  fields the typed events carry. Calls out the mutual-exclusion
  + exception-flow-preservation contracts. Legacy sentinel pattern
  retained as a compatibility-surface callout for older providers.
- docs/agent/non-obvious-shapes.md: "filter openarmature.*-
  namespaced events" tip drops the openarmature.llm.complete
  example (v0.13.0 retired the sentinel pattern for LLM events);
  checkpoint sentinels stay since the tip is still applicable for
  those. Adjusted the follow-on paragraph mentioning LLM events.

mkdocs strict build clean.

* Regenerate AGENTS.md for typed-event doc migration

The non-obvious-shapes doc migration changed a generator source
without regenerating the committed AGENTS.md. Bring it back in
sync so the drift guard passes.

* Extend production-observability example for v0.13.0

Add an LlmFailureTracker observer that consumes the typed
LlmFailedEvent and rolls up per-invocation error-category counts,
and extend LlmUsageAccumulator to track cached_tokens and report a
cache-hit ratio. The persist node now reports both rollups and the
OTel formatter surfaces the cache-read attribute.

Also drop spec/proposal references and em dashes from the
example's comments and walk-through, which carry no meaning for
end users reading the code.

* Drop spec and proposal references from examples

Example comments and docstrings quoted proposal numbers and spec
section refs that have no meaning to end users reading the code.
Reword them to describe only the implementation behavior.

* Add tests for production-observability observers

The examples smoke test only proves the demo loads and its
build_graph() compiles. Cover the two queryable observers the
production-observability example ships: cache-token accumulation
and the derived cache-hit ratio, failure-category counting,
mutual exclusion between the success and failure events, the
per-invocation bucket cleanup, and the OTel cache-read attribute.
The persist-output check drives the real persist node offline.

* Guard legacy LLM observer snippet with NodeEvent check

The legacy sentinel-namespace observer example accessed
event.namespace / event.pre_state without narrowing to NodeEvent.
A real observer receives the full ObserverEvent union, where
variants like InvocationCompletedEvent have no namespace, so the
snippet would raise AttributeError. Add an isinstance(event,
NodeEvent) guard so the copy-paste example is correct.
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