feat(providers): optional token-usage capture via usage_sink#69
Conversation
|
Thanks @tony-zhelonkin for the follow-up and for opening this as a proposal. I took a first pass through the diff. The direction is useful, especially for downstream cost accounting, and the provider-layer default path looks intentionally low-risk. I ran the local checks on the PR branch:
A few things need to be sorted before this is mergeable:
My preference: resolve the |
Provider-layer hook only. Add an opt-in `usage_sink` dict to the OpenAI-compatible
providers (openai, openrouter, deepseek, qwen, grok, stepfun, zhipu, minimax) and
to Gemini, plus the shared call_openai_compatible_api / call_http_api_with_retry
core. When supplied it is populated in place with {prompt_tokens,
completion_tokens, total_tokens} (plus a native USD cost for OpenRouter, which
sends usage:{include:true} only when a sink is passed). Default path is unchanged:
omit the sink and the request body, network behavior, and list[str] return are
byte-identical.
Also expose public extractors extract_chat_completions_usage and
extract_gemini_usage. Motivation: recover per-call token usage / cost for
consensus runs without monkeypatching requests.post / generate_content.
High-level aggregation (annotate_clusters, get_model_response,
interactive_consensus_annotation) is intentionally out of scope and left as a
follow-up. Anthropic is also deferred: its usage block uses a different shape and
was not validated against the live API, so it is not wired here.
- tests: offline behavioral coverage + format-drift resilience (extra/missing/
partial/malformed usage); default-path byte-identical test.
- CHANGELOG: note under Unreleased.
4b2b6ca to
8b378c7
Compare
|
Thanks for the careful review and for running the checks. Addressed all three:
Marked Ready for review. Local: |
|
Thanks @tony-zhelonkin. I did one final review pass and added a small follow-up so a reused |
Context
Follow-up from #68 — thanks again for merging that one (and for the positional-compat shim).
This one is less clear-cut, so I'm opening it as a proposal, it touches the
list[str]return contract that every provider shares,and you'll see call sites, packaging, and CI implications on your side that I can't from a local checkout.
Happy to reshape or narrow it to whatever fits your vision for the provider layer — or to drop it if you'd rather solve usage tracking a different way.
The problem I'm trying to solve
Downstream I need per-call token usage (and OpenRouter's native USD cost) to cost-account a consensus run. Currently the library seems to discard it: every
process_*returnslist[str], the OpenAI-compatible parser keeps onlychoices[0].message.content, and Gemini reads onlyresponse.text. So I currently recover usage by monkeypatchingrequests.postandgoogle.genai'sgenerate_contentaroundinteractive_consensus_annotation— fragile and pinned to an exact version. I'd love a small supported seam so that patch can go away, and figured others doing cost accounting hit the same wall.One way to do it (this PR)
An opt-in
usage_sink: dict | None = None:call_openai_compatible_api/call_http_api_with_retrycore. When passed, it's filled in place with{prompt_tokens, completion_tokens, total_tokens}— pluscost(USD) for OpenRouter, which opts intousage: {include: true}only when a sink is supplied.usage_metadatanormalizes to that same schema.extract_chat_completions_usage,extract_gemini_usage) for callers who parse responses themselves.I went with an opt-in mutable sink specifically to avoid touching the return type — but that's the main thing I'd want your read on.
I picked the one with the smallest default-path footprint, not necessarily the prettiest API.
On breakage
I tried hard to make the default path a no-op, and I think it is locally:
usage_sinkis appended afterbase_urlwith a default, so positional callsprovider_func(prompt, model, api_key, base_url)are unaffected; the sharedcall_*core is already keyword-only.list[str]return are byte-identical (there's a test pinning this).Treat my "non-breaking" as "non-breaking as far as my local checkout shows."
Scope (deliberate)
Provider-layer only.
annotate_clusters,get_model_response, andinteractive_consensus_annotationdo not expose or aggregate usage here. Aggregating across the many per-model/per-cluster consensus calls — and where to surface it in the result — is a separate API decision I'd rather leave to a follow-up once this low-level hook lands.What I've actually tested vs. not
I only have OpenRouter and Gemini API keys, so that's the boundary of my real world tests:
call_openai_compatible_api+extract_chat_completions_usage, which is covered by offline tests and is the same path OpenRouter rides live. So the core logic is exercised — but I did not hit each provider's native endpoint with a real key. If any of them returns a non-standardusageshape, I wouldn't have caught it.usageblock uses a different shape (input_tokens/output_tokens) and I can't live-validate it (no Anthropic key), so rather than ship unverified parsing I leftprocess_anthropicuntouched. Clean follow-up once someone can exercise it against the live API.Tested
Everything offline, mocking the HTTP/SDK seam — no live calls in the suite. Beyond the happy-path surfacing tests (core,
call_openai_compatible_api, and end-to-end viaprocess_openai/process_openrouternative-cost /process_gemini), I leaned on format-drift resilience since the whole point is to survive providers changing their payloads:usagefields → ignored, canonical keys still captured;usageblock → sink stays{}, noKeyError,list[str]return unaffected;None(not0) for the rest — so "not reported" stays distinguishable from a real zero;usage(non-dict / null) →None, no crash;usage_sinkleaves request body and return byte-identical;usage: {include: true}opt-in shows up in the request body only when a sink is passed.I mutation-checked the resilience tests (not just the happy path): breaking the missing-usage guard, the field normalization, the no-fabricate-total rule, the OpenRouter opt-in condition, and the Gemini field mapping each made the corresponding test fail. Full suite: 341 passed, 1 skipped (the
--run-integrationtest).ruff checkclean on the changed files.