[Bug]: ACP derived cost ignores cache/thought buckets, silently zeroes unknown models, and can inflate cost when UsageUpdate cost stalls

[EvolveAegis]

Is there an existing issue for the same bug?

• I have searched existing issues and this is not a duplicate.

Bug Description

For ACP (external agent) sessions where the provider does not send UsageUpdate.cost (e.g. gemini-cli; the code comment at openhands/sdk/agent/acp_agent.py says "gemini-cli: does not send UsageUpdate (cost derived from tokens below)"), cost is derived from token counts. The derivation prices only input and output tokens, ignoring cache_read/cache_write and thought buckets. It also returns 0.0 silently for models unknown to litellm, and there is a path where a token-derived estimate is added even when provider-reported cost exists, inflating accumulated_cost.

Mechanism, verified at software-agent-sdk main (ecf417c), file openhands-sdk/openhands/sdk/agent/acp_agent.py:

1. _extract_token_usage (751-770) parses input/output/cache_read/cache_write/thought from ACP usage. The token metrics DO record these buckets.
2. _estimate_cost_from_tokens (777-793) prices only input_tokens * input_cost + output_tokens * output_cost. The cache/thought buckets are never passed to it (signature is (model, input_tokens, output_tokens)). Cache reads are priced differently from plain input on most providers (cheaper) and cache writes at a premium, so a cache-heavy ACP session is mis-priced when this path runs.
3. Unknown models: cost_map.get(model, {}) yields an empty dict, both prices are 0, and the function returns 0.0. The blanket except Exception: return 0.0 also swallows litellm import failures. The caller does if cost > 0: add_cost(cost), so the silent result is that the turn records no cost at all, with no warning or metric.
4. Inflated-cost path: in _record_usage, when a UsageUpdate arrives with cost but delta = cost.amount - last_cost <= 0 (stale _last_cost_by_session after a session switch, or a cost reset), cost_recorded stays False. The later branch if not cost_recorded and (input_tokens or output_tokens) and self.acp_model: then adds the token-derived estimate on top of the provider cost that was already recorded in an earlier call (the add_cost(delta) sites at ~1815 and ~1846). Session accumulated_cost can therefore exceed the provider's last cumulative cost.
5. Adjacent: failed/cancelled turns (timeout, hard-fail, stop_reason=cancelled) typically never call _finalize_successful_turn / _record_usage, and a pending UsageUpdate can be dropped on the next prepare_usage_sync.

Expected Behavior

• The derived cost should price cache_read/cache_write and thought tokens when the ACP usage reports them (litellm exposes cache_read_input_token_cost, cache_creation_input_token_cost, and reasoning rates).
• A token-derived estimate should only apply when no provider cost was recorded for the session, so the two sources cannot both contribute.
• An unknown model should not silently record $0; at minimum a warning or metric should surface it.

Steps To Reproduce

Static code-path demonstration (no live run required):

1. Configure an ACP agent with a provider that sends usage without UsageUpdate.cost (gemini-cli class).
2. Read _record_usage in acp_agent.py (1792-1864): the provider-cost branch sets cost_recorded = True only when delta > 0; the derived-cost branch fires when not cost_recorded, regardless of whether provider cost was recorded in an earlier call.
3. For the cache/thought gap: pass a usage with nonzero cache_read/cache_write/thought and observe _estimate_cost_from_tokens(model, input_tokens, output_tokens) never receives them.

Environment

• Python 3.12, macOS 15
• openhands-sdk software-agent-sdk main (ecf417c)

Installation Method

pip install from source (software-agent-sdk main, ecf417c)

SDK Version

ecf417c

Operating System

macOS 15 (arm64)

Additional Context

Suggested fix:

1. Extend _estimate_cost_from_tokens to accept and price cache_read/cache_write/thought buckets using litellm's cost fields.
2. Track a per-session "provider cost seen" flag independent of the last-delta, so the derived branch only runs when the provider never reported cost.
3. When a model is unknown to litellm, emit a warning or record an unknown-cost marker instead of silently skipping.

[EvolveAegis]

Dynamic reproduction added (2 tests against the real SDK code at ecf417c):

1. _estimate_cost_from_tokens signature is (model, input_tokens, output_tokens); cache_read/cache_write/thought extracted by _extract_token_usage are never passed to it. For a usage of 1000 input / 100 output / 900 cache_read / 50 cache_write / 30 reasoning on gpt-4o, the derived cost is 0.0035 (input+output only); the 900 cache_read tokens priced at half rate would have saved 0.001125, which is lost.
2. Double-count: after a turn records provider cost 10 via UsageUpdate, a second UsageUpdate with the same cumulative cost (delta <= 0, e.g. stale _last_cost_by_session) leaves cost_recorded False, and the token-derived estimate (0.0035) is added on top, making accumulated_cost 10.0035.

Both tests pass against the installed SDK.

[onatozmenn]

Picked this up in #4444 (draft), unless you were already on it.

One correction worth making on the cache half, since it changes the sign of the fix. The report reads the missing cache pricing as a lost discount, on the assumption that cached reads sit inside input_tokens the way litellm and OpenAI report them. ACP reports the buckets separately: Usage.total_tokens is documented as "Sum of all token types across session", and metrics.py already says as much in cache_hit_rate ("litellm/OpenAI count cached reads inside prompt_tokens; ACP reports them separately"). So those 900 cache-read tokens were not being over-billed at the full input rate, they were being billed at nothing.

Measured on gemini-2.5-flash with real litellm pricing, using your 1000/100/900/50/30 example: 0.00055 before, 0.000667 after. The derived estimate goes up, not down.

The other two are exactly as you described. The double-count reproduces the way you set it out, and the unpriced model now warns once per model rather than per turn.

I left item 5 alone, the failed and cancelled turns never reaching _record_usage. That looks like a turn-lifecycle question rather than a pricing one and I would rather not fold it into the same change. Happy to open it separately if you are not already on it.