Repository navigation
fix(sdk/python): report a provider-confirmed zero cost instead of unknown - #1105
Conversation
…nown _resolve_cost() accepted a provider-native cost figure only when it was strictly positive, so a provider-reported 0.0 -- what a zero-priced or free-tier model returns -- was discarded and resolution fell through to the LiteLLM branches, ending at (None, None). detect_multimodal_response() then reported cost_usd=None/cost_source=None, agent_ai.py recorded that into the execution CostTracker, and serialize() emitted total_cost_usd: null for a run whose calls were all free. Accept cost >= 0 in the provider-native branch only. The two LiteLLM branches keep > 0 because litellm.completion_cost() returns 0.0 for models absent from its pricing database, so a zero there means "unpriced" rather than "free". This also matches the TypeScript SDK, which accepts a real zero for the same provider figure (costFromUsageRaw: cost >= 0, "known zero receipts remain reported entries").
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Performance
✓ No regressions detected |
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
ZeroPoint95
left a comment
There was a problem hiding this comment.
@remote-controlled-man, thanks for the focused fix. Provider-reported zero now stays a known cost through the public response and serialized usage receipt, while the unknown-pricing fallback remains intact. The regression checks reproduce the bug on the base and pass here. Looks good from the code-review side.
Summary
_resolve_cost()prefers a provider-native cost figure — OpenRouter'susage.cost, which this SDK asks for on every OpenRouter request (openrouter_attribution.apply_openrouter_usage_accountingsetsusage: {"include": true}) — but it accepted that figure only when it was strictly positive:So a provider that reports the call's cost as exactly
0— the zero-priced / free-tier case — falls through to the two LiteLLM branches, and when those have no price either,_resolve_costreturns(None, None).detect_multimodal_response()then hands the callercost_usd=None, cost_source=None;agent_ai.pyrecords that into the execution'sCostTracker; andCostTracker.serialize()leavesany_cost=Falseand emitstotal_cost_usd: null. The cost/usage surface reports "cost unknown" for a call whose cost the provider confirmed as $0 — including a run whose calls were all free, which is exactly the run where a user most wants to see0.0rather than a hole.This PR accepts
cost >= 0in the provider-native branch only. The two LiteLLM branches keep> 0, which is still correct there:litellm.completion_cost()returns0.0for models absent from its pricing database, so a zero from LiteLLM means "unpriced" while a zero from the provider means "free".Why this is a bug rather than the intended contract
Three places in this repository already treat a known zero as a value:
_resolve_cost's own docstring reserves(None, None)for "when no cost can be determined". A provider-reported$0is determined.CostTracker.serialize()is explicitly built to express a known zero —if e.cost_usd is not None: total_cost += e.cost_usd; any_cost = True→total_cost_usd: 0.0— and the recording site inagent_ai.pydocumentscost_usd=Noneas "when the price is unknown".agent.pyrecordscost_source="provider" if cost is not None else None, andmedia_providers.pydoes the same for its metadata figure.The TypeScript SDK already accepts a real zero for the same provider figure:
sdk/typescript/src/usage/aiUsage.tscostFromUsageRaw()returns it whencost >= 0, andrecordAiSdkUsage's docstring states "Known zero receipts remain reported entries". So today the two SDKs disagree about the same call — TS reports0, Python reportsnull.Type of change
Test plan
Two new regression tests, both failing on the unmodified base with
assert None == 0.0:cd sdk/python && python -m pytest tests/test_usage_transport.py --no-cov→ 45 passed. New cases:TestCostResolution::test_openrouter_native_zero_cost_is_known(the resolver returns(0.0, "provider")) andTestDetectMultimodalUsage::test_openrouter_native_zero_cost_read_through(the publicdetect_multimodal_response()reportscost_usd == 0.0andcost_source == "provider"with tokens intact). Both keep LiteLLM mocked as raising, so a pass can only come from the provider branch.test_usage_transport,test_cost_tracker,test_multimodal_response,test_multimodal_response_additional,test_multimodal_response_cost,test_data_url_media_outputs,test_media_integration,test_media_providers,test_openrouter_audio,test_openrouter_video,test_usage_stream,test_video_output,test_vision,test_issue_589) → 366 passed.ruff check .→All checks passed!with the CI-pinnedruff==0.15.22;mypy --config-file mypy.ini agentfield/→Success: no issues found in 83 source fileswithmypy==1.18.2from the dev extra. Python 3.12.7 on Windows.9bd0a0dd;git range-diffreports=and the stable patch-id is unchanged): 12 failed, 2506 passed, 6 skipped, 38 deselected. All 12 are pre-existing host artifacts, attributed by node ID rather than by name: re-running the same nodes with this patch stashed (target blob back to1ac98a55) produced the identical 10 deterministic failures — 7 ×test_log_writer(CRLF / pipe-timing), 2 ×test_harness_packaging(GBKUnicodeDecodeErrorfrompathlib.read_text), 1 ×test_tool_calling.py::TestExecuteToolCallLoop::test_single_tool_call_then_response— and the remaining 2 (test_litellm_observability) pass on warm re-runs on both trees, which is that file's known cold-cache flake. Recorded run at this head, with those twelve nodes deselected: 2506 passed, 6 skipped, 50 deselected (the 50 are the 38 marker-deselected tests plus those twelve), i.e. zero failures once the proven host artifacts are set aside.tests/test_agent_mesh.pyis excluded because it does not complete on this Windows host — a pre-existing hang, unrelated to this change. Left to CI onubuntu-latest../scripts/test-all.shand./scripts/coverage-summary.shwere not run locally: they also build the Go control plane, the web UI and the desktop app, and the coverage summary needs a Docker engine, which is unavailable here. This PR changes onlysdk/python/**, and.github/workflows/sdk-python.ymlfilters on exactly that path plus its own workflow file, so the Python surface above is the applicable CI.ruff formatwas deliberately not applied. On pristinemain,ruff format --diffalready proposes two unrelated hunks in exactly these two files —_resolve_cost's signature line and aHarnessResult(...)call in the tests — so reformatting would add pre-existing churn to a one-line fix. CI's format step iscontinue-on-error: true.Test coverage
coverage-baseline.json— not applicable; nothing was removed and no source line lost coverage../scripts/coverage-summary.shmeasures all five surfaces and needs a Docker engine.Heads-up so the gate output is not misread:
agentfield.multimodal_responseis not in the--covwhitelist insdk/python/pyproject.toml's pytestaddopts(which lists nine modules), andscripts/coverage-surface.shmeasuressdk-pythonby running barepytest, so this file never reaches the coverage report thatpatch-coverage-gate.shdiffs against. This PR will therefore reportsdk-python | 0 | — | ➖ no changes. That is the whitelist, not a missing test: the changed branch is exercised directly through_resolve_costand through the publicdetect_multimodal_response()entry point, and the neighbouring positive provider-cost cases (0.0042,0.0012) were already pinned.pyproject.tomlandcoverage-baseline.jsonare left untouched to keep this to one seam — adding--cov=agentfield.multimodal_responsetoaddoptswould let the gate see this module, and I can send that separately if you want it.Checklist
_resolve_cost,cost_source,multimodal_response,cost_usd provider,zero costandfree model cost: no report and no competing change. Happy to file one first if you would rather track it that way.Related issues / PRs
None. No file overlap with any open PR: the Python files under review elsewhere are
agent.py/agent_server.py/logger.py(#1090), the prompt-template surface (#1092) and the harness CLI (#1104); neithersdk/python/agentfield/multimodal_response.pynorsdk/python/tests/test_usage_transport.pyis touched by any of them.