fix(chat): a finished call whose endpoint reports no usage is billed an estimate, not zero (#1432)

[frankbria]

Closes #1432.

Problem

OpenAIProvider.async_stream asks for usage with stream_options={"include_usage": True} and started its counters at 0. Some OpenAI-compatible servers (local ollama/vllm, some proxies) ignore that and never send a usage chunk. The finished call's message_stop then said (0, 0), and StreamingChatAdapter._stream_turn billed it as nothing. That was less than the same call cut off mid-stream, which gets the #1345 estimate, and the spend was invisible to the per-user daily limit (#1303).

Fix

• openai.py: the usage counters start at None, and are set only when a usage chunk arrives. Missing usage reaches message_stop as unknown, not 0.
• streaming_chat.py: when message_stop carries no usage at all (both None), the call is billed with the same estimate a cut-off call gets. That estimate moves into one local _estimate(), used by both paths. Reported usage, including a genuine 0, is billed exactly as before.

Evidence

• tests/api/test_chat_unreported_usage_1432.py: the real OpenAIProvider over a fake SDK stream, through the real producer _run_streaming_adapter.
  • A finished stream with no usage chunk is billed more than zero (red on main: (0, 0)).
  • The same stream with usage is billed exactly (123, 45).
• Mutation check: bypass the fallback → the no-usage test fails.
• Blast radius (tests/adapters plus every test touching async_stream/StreamChunk/StreamingChatAdapter/session_chat/message_stop): 438 passed.
• Demo (a 600-char reply):

== main-wt (5877930)
  endpoint sends usage           : billed input=123, output=45
  endpoint ignores include_usage : billed input=0, output=0
== wt-1432 (0297dcf)
  endpoint sends usage           : billed input=123, output=45
  endpoint ignores include_usage : billed input=96, output=201

Cross-family review

codex review pass 1: no actionable regressions.

Known Limitations

• The estimate is ~3 chars/token over the prompt plus streamed output. It over-counts, the same as the cut-off path. Exact counts need the endpoint to report usage.
• A local model priced at $0 still records $0 cost. What changes is the token counts, which are now recorded instead of zero.
• Only the streaming chat path changes. Non-streaming complete() reads response.usage directly and is unaffected.

https://claude.ai/code/session_01MkaiVsygNAW7aPgbR7fMEi

[frankbria]

Cross-family review (codex), pass 1

The patch distinguishes missing streaming usage from explicitly reported zero usage and applies the existing estimate when usage is absent. Reported token counts remain unchanged, and the added tests cover both paths. No actionable regressions were identified.

[github-actions]

Claude finished @frankbria's task in 10m 10s —— View job

---

GLM precision review — #1438

• Fetch PR diff and context
• Read openai.py usage-counter change and its consumers
• Read streaming_chat.py estimate fallback and its callers
• Trace every consumer of message_stop usage / StreamChunk token fields for None-safety
• Check cross-provider consistency (anthropic/ollama/mock paths)
• Post inline findings + summary

✅ GLM review: no defects found.

Verified against the checked-out repo (not the diff alone):

Area	Check	Result
openai.py:309-310	None counters reach only message_stop (openai.py:399-404); StreamChunk.input_tokens/output_tokens already Optional[int] (base.py:186)	✅ no contract break
streaming_chat.py:576-581	both None → estimate keeps genuine reported 0 exact (0 is not None → else-branch); mock/anthropic message_stop always carry ints, so no estimate mis-fire	✅
streaming_chat.py:520-524	_estimate() identical to the old inline #1345 formula; rebound each tool-loop iteration after streamed_chars = 0, both call sites run in-iteration	✅ no stale closure
Double-billing	finished = True set before _account/COST_UPDATE yield → disconnect-at-yield or SpendLimitExceeded can't re-bill via finally (not finished guard)	✅
Downstream types	_account receives ints on every path; ChatEvent.to_dict, record_llm_usage, TokenUsage unchanged	✅
Test soundness	red on main ((0,0) counters); row isolation via tests/api/conftest.py autouse cleanup + per-class WORKSPACE_ROOT	✅ not vacuous

One non-defect observation (scope of the fix, not a bug in it): the only production constructor of StreamingChatAdapter is session_chat_ws.py:164, and it resolves the provider with provider_flag pinned to "anthropic" (_AGENT_TYPE_TO_PROVIDER, session_chat_ws.py:69; flag-first precedence in llm_resolution.py:95-100). So OpenAIProvider.async_stream is not currently reachable through interactive chat — the openai.py half of the fix matters for the adapter's bare-constructor path and future wirings, while the provider-agnostic streaming_chat.py fallback (which also covers an Anthropic-protocol proxy that omits usage) is the reachable half. No failure scenario against this diff; noting it only in case the issue's repro assumed otherwise.

[claude]

Reviewed the diff directly (no prior substantive reviews on this PR to reconcile with — just the codex cross-family pass noted in the thread, which I agree with).

Correctness

• The core fix is sound: OpenAIProvider.async_stream now starts usage_input/usage_output at None and only sets them (together, via or 0) when a chunk.usage arrives — matching the Optional[int] = None contract already declared on the base StreamChunk (codeframe/adapters/llm/base.py:185-186). Anthropic and the mock provider always report both counters together on message_stop, so _stream_turn's chunk.input_tokens is None and chunk.output_tokens is None check is the correct discriminator for "no usage at all" and can't misfire on those paths.
• Factoring the cut-off estimate ([P2.72] Apply the daily spend limit to PRD, discovery, task generation and session chat #1345) and the new no-usage estimate into one _estimate() closure is a good simplification — same formula, one place, and it correctly captures streamed_chars/current_messages from the enclosing scope.
• No double-billing risk: finished is set True as soon as the message_stop chunk is processed, regardless of whether real or estimated tokens were used, so the finally block's if not finished and not refused guard still only fires for the genuinely-cut-off case.
• Reported usage (including a real 0) is left untouched — only the both-None sentinel path changes behavior, which matches the PR's stated scope.

Tests

• tests/api/test_chat_unreported_usage_1432.py drives the real OpenAIProvider through the real _run_streaming_adapter producer rather than mocking at the streaming_chat boundary, so it actually exercises the code path described in the bug. Covers both the no-usage-estimate case and the reported-usage-exact case. The claimed mutation check (bypassing the fallback fails the no-usage test) is a reasonable sanity bar, consistent with this repo's evidence-based testing norms.

Scope / docs

• Changes are narrowly scoped to the streaming chat path, as the PR description notes complete() is unaffected (verified: it reads response.usage directly in both adapters). CHANGELOG entry is accurate and appropriately placed under ### Fixed.

No actionable issues found. The known limitations called out in the PR description (estimate over-counts, $0-priced local models still show $0 cost) are accurately described and are inherent to the estimate-based approach rather than bugs in this patch.

[frankbria]

Bot review triage

Source	Finding	Decision
GLM / Claude	No defects: None-safety at every message_stop consumer, a reported 0 stays exact, no double billing, no stale closure	Confirmed, no action.
GLM (scope note)	Interactive chat pins its provider to Anthropic (_AGENT_TYPE_TO_PROVIDER, session_chat_ws.py:69), so OpenAIProvider.async_stream is not reachable through chat today	Verified, and accepted as a correction to my own issue. #1432 overstated the user impact. In chat today, the reachable part is the provider-neutral fallback in streaming_chat.py, which also covers an Anthropic-protocol proxy that omits usage. The openai.py change guards the adapter for direct use and any future wiring. No further change; the issue gets a correcting note.

CI green, no review threads. Merging.