fix(cost): bill 1h cache writes at 2x using the usage cache_creation TTL split - #103
Conversation
…TTL split _cost.py billed every prompt-cache write at the 5-minute (1.25x) rate, with a comment claiming Anthropic's usage object "returns a single cache_creation_input_tokens count, no TTL split." That comment is stale: the pinned anthropic SDK (1.0.0) already exposes a per-TTL breakdown via `usage.cache_creation` (CacheCreation.ephemeral_5m_input_tokens / .ephemeral_1h_input_tokens), alongside the aggregate cache_creation_input_tokens field. Verified directly against the installed SDK's types (anthropic.types.Usage / CacheCreation). Official Anthropic pricing: 5-minute cache writes = 1.25x base input, 1-hour cache writes = 2x base input. Any session using this module's cache_stable_region_ttl_1h knob had its 1h write costs undercounted by ~37.5% (billed at 1.25x instead of 2x). Changes: - compute_cost() gains optional cache_creation_5m_input_tokens / cache_creation_1h_input_tokens parameters. When either is supplied, billing switches to split mode: 5m tokens at cache_write_per_m (1.25x), 1h tokens at 2x input_per_m. When neither is supplied (older SDK shapes, mocks, other callers), the legacy aggregate-at-1.25x path is unchanged -- a graceful, non-breaking fallback. - If the split is present but doesn't sum to the aggregate cache_creation_input_tokens, the split wins (it's the more precise figure) and the discrepancy is logged at DEBUG only, never WARNING. - __init__.py extracts usage.cache_creation.ephemeral_5m_input_tokens / .ephemeral_1h_input_tokens defensively (isinstance(int) guarded, so a malformed/mocked object degrades to "split absent" instead of crashing) and wires them into compute_cost(). - Corrected the stale comment in _cost.py's Fable-5 rate block: the 1h rate ($20.00/MTok) is exactly 2x Fable-5's input rate, consistent with every other model -- no special-casing needed. - Read-only check: no other consumer in the module reads cache_creation_input_tokens for cost or telemetry. The one other reader (the kernel Usage.cache_write_tokens field) reports the raw token count for observability, not a dollar cost, so it needs no equivalent change. Tests (tests/test_cost.py): split with nonzero 1h tokens bills at 2x; split that is all-5m matches pre-fix legacy behavior exactly; omitting the split kwargs entirely preserves the legacy aggregate-at-1.25x path; explicit 0/None split values contribute zero cost without raising; split/aggregate mismatch prefers the split and logs at DEBUG (asserted via caplog, not WARNING+); and an end-to-end _convert_to_chat_response() test proving the corrected cost is stamped on Usage.cost_usd. All 6 of the new behavior-under-test cases were confirmed to fail against the pre-fix code (TypeError for the new kwargs on old callers, or the stale $3.75 assertion instead of $6.00) before the fix, and pass after. Full suite: 671 passed (662 baseline + 9 new), 0 failed. Root cause independently corroborated by Joi's review on closed PR #91 and by upstream issue microsoft/amplifier#337; ramparte's PR was the signal that first surfaced it. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
|
All required checks are green: This PR is self-authored (by the maintainer, at explicit user direction) to fix a Branch protection requires a review that isn't available in this context, so this Merging via |
…ng 'false' no longer truthy (#105) Follow-up to microsoft/amplifier-module-provider-openai#74 (squash 8485663). That PR's read-only cross-check of this repo (HEAD 9916a68) found this module already has a correct boolean-parsing helper — AnthropicProvider._config_bool() (__init__.py:577-584) — used by six config keys, but FIVE boolean-ish keys bypassed it and read config with no coercion at all. The app-cli wizard writes boolean ConfigField answers as the STRING "true"/"false" (see the field_type="boolean" fields in get_info()). A plain self.config.get(key, default) returns that string unchanged, and every one of these keys is consumed in a truthiness context — any non-empty string, including the literal string "false", is truthy in Python. A user answering "false" in the wizard therefore got the feature turned ON. enable_prompt_caching is the live-reachable instance: it IS exposed as a field_type="boolean" ConfigField with string default "true" (__init__.py:975-982), so a wizard-driven "false" answer silently enabled prompt caching — and, post-#104, also fed the cache_stable_region_ttl_1h beta-header gate (if self.enable_prompt_caching: at __init__.py:812), which then misread too. Audit table (full re-audit of every self.config.get() call in the constructor, not just the 5 keys named in the task): | key | site (pre-fix) | was broken? | fixed how | |-------------------------|----------------|-------------|--------------------------------| | raw | :661 | YES — no coercion | routed through _config_bool() | | use_streaming | :748 | YES — no coercion | routed through _config_bool() | | filtered | :749-751 | YES — no coercion | routed through _config_bool() | | enable_prompt_caching | :752 | YES — no coercion; wizard-exposed boolean ConfigField, LIVE-REACHABLE | routed through _config_bool() | | enable_web_search | :753-755 | YES — no coercion | routed through _config_bool() | | retry_jitter | :676 | already safe | uses _config_bool() (pre-existing) | | fallback_on_overload | :691-693 | already safe | uses _config_bool() (pre-existing) | | enable_1m_context | :703-705 | already safe | uses _config_bool() (pre-existing) | | persist_fallback_state | :712-714 | already safe | uses _config_bool() (pre-existing) | | refusal_fallback_enabled| :720-722 | already safe | uses _config_bool() (pre-existing) | | cache_stable_region_ttl_1h | :773-775 | already safe | uses _config_bool() (pre-existing) | No other uncoerced boolean-ish config key exists in the constructor — every remaining self.config.get() call reads a numeric, string, or choice value (max_tokens, temperature, timeout, model names, thinking_type, speed, etc.), not a boolean. Fix: each of the five keys is now read as `self._config_bool(self.config.get(key, default))`, mirroring the exact call shape already used by the six safe keys. This module's _config_bool() coerces (does not fail loud on garbage — anything outside 1/true/yes/on resolves to False); no new helper was introduced, matching the module's own existing convention exactly, per the task's own guidance to not invent a new helper. Tests: tests/test_config_bool_parsing.py — 21 tests covering, per affected key: string "false" -> False, string "true" -> True, real bool passthrough, absent -> documented default; plus one integration-flavored assertion for the live-reachable key (enable_prompt_caching="false" as a string -> zero cache_control blocks in a built request). Fail-before/pass-after proof (git stash of only the source fix, keeping the new test file in place): 11 failed / 10 passed against pre-fix main (the 10 passes are the real-bool-passthrough and absent-default cases, which were never broken). Restoring the fix: all 21 pass. Full suite: 695 passed (baseline 674 passed confirmed via a clean run before touching anything, post-#103/#104, + 21 new tests = 695 exactly). No regressions. python_check on touched files: amplifier_module_provider_anthropic/__init__.py carries 15 pre-existing pyright errors + 18 pre-existing ruff-lint/stub warnings (SDK Optional-attribute narrowing, a pre-existing unsorted __all__/import block, blind-exception lint nits, a TODO stub comment) — confirmed identical (15 errors / 18 warnings, same codes and same line-number deltas as this diff's own +6 net lines) before and after this change via git stash. `ruff format`/`ruff check` show zero diff needed on this diff's own lines. The new test file is `ruff format`/`ruff check` clean; its two pyright import-resolution errors are the same isolated-file false positive every existing test file in this repo also reports when checked in isolation (verified against tests/test_prompt_cache_breakpoints.py, an unmodified pre-existing file, which reports the identical "AnthropicProvider is unknown import symbol" / "tests._helpers could not be resolved" pair) — a tool-environment artifact of checking a test file outside the project's own pytest rootdir/venv resolution, not a real defect. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
The bug
_cost.py(around line 141-143, in the Fable-5 rate block, with the samebehavior applying to every model) billed all prompt-cache writes at the
5-minute (1.25x) rate, with this comment:
That comment is stale. Verified directly against this repo's pinned SDK
version (
anthropic>=1.0.0,<2.0.0, resolves to exactly1.0.0viauv sync):usage.cache_creation(when present) carries the exact per-TTL split;usage.cache_creation_input_tokensremains the aggregate. Official Anthropicpricing: 5-minute cache writes = 1.25x base input, 1-hour cache writes = 2x
base input. Because the module billed everything at 1.25x, any session using
cache_stable_region_ttl_1hhad its 1h write costs undercounted by~37.5%.
The fix
compute_cost()gains optionalcache_creation_5m_input_tokens/cache_creation_1h_input_tokensparameters. When either is supplied, 5mtokens are billed at
cache_write_per_m(1.25x) and 1h tokens at2 * input_per_m(2x). When neither is supplied (older SDK responseshapes, other callers, existing test doubles), the legacy
aggregate-at-1.25x path is unchanged — a graceful, non-breaking fallback.
cache_creation_input_tokens, the split wins (it's the more precise,billable figure) and the discrepancy is logged at DEBUG only — no
warning spam.
__init__.pyextractsusage.cache_creation.ephemeral_5m_input_tokens/.ephemeral_1h_input_tokensdefensively (isinstance(..., int)-guarded,so a malformed or mocked object degrades to "split absent" rather than
crashing) and wires them into
compute_cost().its input rate, consistent with every other model — no special-casing
needed.
cache_creation_input_tokensfor cost or telemetry. The only other readeris the kernel
Usage.cache_write_tokensfield, which reports the rawaggregate token count for observability, not a dollar cost — it needs no
equivalent change.
Tests
Added to
tests/test_cost.py, following the module's existing conventions:0/Nonesplit fields → zero cost contribution, no crashcaplog, not WARNING+)_convert_to_chat_response()test proving the corrected costreaches
Usage.cost_usdAll 6 of the new behavior-under-test cases were verified to fail against
the pre-fix code (
TypeErrorfor the new kwargs on the old signature, or thestale
$3.75assertion instead of$6.00) before the fix was applied, andpass after.
Full suite: 671 passed (662 baseline + 9 new), 0 failed, on a fresh clone
of
main@e64b114.Credit
This was independently corroborated by Joi's review comment on closed PR
#91, and by upstream issue microsoft/amplifier#337. ramparte's PR was the
signal that first surfaced this as worth investigating. Thank you both.
🤖 Generated with Amplifier