diff --git a/CHANGELOG.md b/CHANGELOG.md index c633eb2..fd388fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,11 @@ All notable changes to this project will be documented here. Format follows [Kee - **One log line per dropped event, not two.** `_report_error` already invokes `on_error` AND logs; an extra `logger.error` beside it emitted the same drop twice at two levels, so a customer grepping logs counted one lost call as two. The JS port logged **nothing** for the same event, so the two repos reported 2 lines vs 0 — `reportError` there now logs as well, since `onError` is opt-in and the log is the floor. - **`verify_ssl=False` could crash `LagoSDK()` construction.** The InsecureRequestWarning suppression reached through `requests.packages`, a legacy compatibility alias with no guarantee of existing, in an unguarded attribute chain inside `__init__`. Now `import urllib3` directly, wrapped: suppressing a warning must never fail construction. This sits on an advertised path — `verify_ssl` is a first-class constructor argument the docstring recommends for local dev — so the crash would have hit exactly the setup the flag was added to serve. - **`WORKERS_AI_COMPAT_PREFIX` had drifted into two definitions.** `adapters/openai_native` decides the *provider* from it and `pricing` strips it before a catalog lookup; those two must never import each other, so it now lives in `canonical` (which imports nothing from the package — no cycle either way, and no pulling `pricing`'s ~50KB into a lightweight adapter). A drift between the copies would have been a silently unpriced call rather than a crash, so a test now asserts there is exactly one definition in the tree. +- **Two comments in the Cloudflare gateway adapter were wrong, and the module docstring described behaviour Cloudflare does not have.** Both are corrected, because they were the justification for code and would have misled the next reader: + - The docstring claimed `usage_metadata`'s key casing "is NOT normalized by Cloudflare — it passes through whatever convention the underlying provider's own usage object used". It does not. Across all 14 captured fixtures (Anthropic, Workers AI, Mistral and Gemini, via every ingress method) the only keys that ever appear are Cloudflare's own: `input_tokens`, `output_tokens`, `total_tokens`, `input_cached_tokens`, `input_cache_creation_tokens`, `neurons`, `input_text_tokens`, `reasoningTokens`. **Not one provider-native key shows up.** The cited proof — camelCase `reasoningTokens` in the real Gemini entry — is Cloudflare's own inconsistency, not a leaked provider key: Gemini's native spelling for that quantity is `thoughtsTokenCount`, which appears nowhere. + - `_first_int`'s comment said a missed cache key is "an over-bill, not an omission". That is true only for a **subtractive** provider (`gemini`/`openai`/`workers-ai`, where `compute_cost` subtracts `cache_read` out of `input`). For **additive** Anthropic the same miss means those tokens are never billed at all — an under-bill, the direction this SDK treats as worse. The comment asserted one direction for a function used by both. +- **Added Anthropic's `cache_read_input_tokens` and Gemini's `thoughtsTokenCount` to the gateway adapter's spelling fallthrough**, alongside the `cachedContentTokenCount` / `cache_creation_input_tokens` entries that were already there. Labelled honestly in the code as **unobserved insurance**: neither has ever appeared in a captured fixture, so this guards against Cloudflare one day forwarding a provider's usage object instead of rewriting it, and is not handling for a case we have seen. Kept because `_first_int` fallthrough is free and a missed cache key mis-bills in one direction or the other for every provider. Pinned by synthetic tests marked as such. +- **Gemini streaming attributed the requested alias instead of the resolved model when a chunk carried usage without `model_version` — a real divergence between the two ports.** Python read `model_version` off whichever chunk carried usage; JS remembered it across chunks. On a stream that announces the version early and sends usage last, Python emitted `gemini-flash-latest` where JS emitted `gemini-2.5-flash-002`, so the two repos priced the same call differently. Python now persists it across chunks (both the sync and async stream wrappers), and accepts `modelVersion` as well as `model_version` since `model_dump()` yields snake_case while a raw REST dict is camelCase. **Not reachable with Gemini as it behaves today** — verified live that every streaming chunk carries both `model_version` and `usage_metadata`, which is why the existing fixture (faithful to that) could not catch it. The captured fixture was left faithful rather than edited to expose the bug; a separate, explicitly synthetic case pins the property. The alias hot-swap itself is real and live-verified: `gemini-flash-latest` resolved to `gemini-3.7-flash`. - **A moving `~` alias could overwrite a real listing's price, decided purely by catalog order.** Stripping OpenRouter's `~` marker indexes an alias under its real vendor, which is what makes a plain `-latest` id priceable at all — but it wrote the alias-derived keys with plain assignment. Collision-freedom was verified against the live catalog and still holds, so nothing is mispriced today; it was a property of that day's response rather than of the code. If OpenRouter ever lists both `google/gemini-flash-latest` and `~google/gemini-flash-latest`, whichever arrived later won: measured on a synthetic pair, the same lookup returned `0.009` or `0.001` depending only on position in the response. Alias-derived keys are now written only when absent, so a real listing always wins regardless of order; the `~`-spelled id still resolves to its own entry, and non-alias entries keep plain assignment so genuine duplicates behave exactly as before. - **The `-\d{3}` version-strip arm is now scoped to OpenRouter, where it was the only thing that ever needed it.** Gemini's `model_version` can report a `-002` revision that OpenRouter omits from its ids, so the shared `_strip_version` grew a 3-digit arm — but that helper also builds the AWS/Bedrock price keys, and there a shortened key does not merely miss: `bedrock_model_key` feeds `table.setdefault(key, {})[direction] = price`, so two distinct models collapsing onto one key silently overwrite each other's rate. All four live catalogs are clean (OpenRouter 415 ids, Cloudflare 64, AWS offer 77, captured Bedrock 39 — zero model parts ending in exactly three digits), so this was latent, not active. Splitting it into an OpenRouter-only strip makes the risk structurally zero rather than empirically zero, and `amazon.titan-text-001` / `-002` now stay distinct keys. - **`apply_markup`'s two bad-input fallbacks were not equivalent, and the ports disagreed on one.** An unparseable `usd` means the cost is unusable, so 0 is right; an unparseable `markup` means only the multiplier is unusable, and returning 0 there discards a perfectly good cost. Python returned `"0"` for both, JS fell back to 1.0 for a bad markup, so identical input would have produced different bills. Python now matches JS. This is defence in depth rather than a live fix: every `emit()` path already runs the customer's markup through `coerce_markup` (which falls back to 1.0 and reports under `on_error`), and `CostBreakdown.markup` / `fields[*]["cost"]` are `_fmt_money` output, so neither argument can actually arrive unparseable — the divergence was latent. What *was* untested is the end-to-end consequence of that guard, now pinned: a customer sending `markup="1,5"` gets the cost billed at 1.0 rather than zeroed, and the lost markup reaches `on_error`. diff --git a/src/lago_agent_sdk/gateway/adapters/cloudflare_gateway.py b/src/lago_agent_sdk/gateway/adapters/cloudflare_gateway.py index 878e32e..8b4d21f 100644 --- a/src/lago_agent_sdk/gateway/adapters/cloudflare_gateway.py +++ b/src/lago_agent_sdk/gateway/adapters/cloudflare_gateway.py @@ -9,15 +9,22 @@ tokens_out → output usage_metadata.input_cached_tokens → cache_read usage_metadata.input_cache_creation_tokens → cache_write - usage_metadata.reasoningTokens/reasoning_tokens → reasoning + usage_metadata.reasoningTokens → reasoning model, provider → passed straight through -`usage_metadata`'s exact key casing is NOT normalized by Cloudflare — it passes -through whatever convention the underlying provider's own usage object used -(Anthropic/OpenAI: snake_case `input_cached_tokens`; a real captured Gemini -entry: camelCase `reasoningTokens`). Both cases are checked for every field -we map; this is observed behavior across two providers, not a documented -guarantee, so a third provider could use a convention we haven't seen yet. +Cloudflare reports its OWN counter vocabulary here, not the provider's. Across all +14 captured fixtures — Anthropic, Workers AI, Mistral and Gemini, via every ingress +method — the only keys that ever appear are `input_tokens`, `output_tokens`, +`total_tokens`, `input_cached_tokens`, `input_cache_creation_tokens`, `neurons`, +`input_text_tokens` and `reasoningTokens`. Not one provider-native key shows up: +no Anthropic `cache_read_input_tokens`, no Gemini `thoughtsTokenCount` or +`cachedContentTokenCount`. + +That vocabulary is *mostly* snake_case, with `reasoningTokens` as a camelCase +outlier — Cloudflare's own inconsistency, not a provider key leaking through +(Gemini's native spelling for the same quantity is `thoughtsTokenCount`, which +appears nowhere). The extra spellings checked below are therefore unobserved +insurance against a convention we have not seen, not handling for a known case. Unlike the provider-native adapters (`adapters/openai_native.py`, `adapters/anthropic_native.py`), there is no request-side model kwarg to prefer @@ -58,20 +65,23 @@ def _safe_str(v: Any) -> str: def _first_int(meta: dict[str, Any], *names: str) -> int: """First of `names` present in `meta` with a usable value, as an int. - The gateway does NOT normalize every key it forwards. Its own counters are - consistently snake_case across every captured fixture (`input_tokens`, - `output_tokens`, `total_tokens`, `input_cached_tokens`, - `input_cache_creation_tokens`), but a provider's native key can come through - untouched: the real Gemini entry carries `reasoningTokens`, camelCase, and an - unmapped `input_text_tokens` alongside it. So the spelling of a cache key on a - provider we have no cached capture for is genuinely unknown. - - Checking every plausible spelling is close to free and the downside is - lopsided. A silent 0 here does not merely lose a field — `gemini` is in - `_INPUT_INCLUDES_CACHE_READ`, so `compute_cost` relies on `cache_read` being - populated in order to SUBTRACT the cached portion out of `input`. A missed - cache key therefore bills those tokens at the full prompt rate instead of the - cache rate: an over-bill, not an omission. + Cloudflare's counter names are its own and mostly snake_case, but not + reliably so — `reasoningTokens` is camelCase in the real Gemini entry, right + next to snake_case `input_tokens` in the same object. Since the vocabulary is + internally inconsistent, the spelling it will use for a provider we have no + capture for is genuinely unknown. + + Checking every plausible spelling costs nothing and the downside is lopsided + — though it is lopsided in OPPOSITE DIRECTIONS depending on the provider, so + neither "over-bill" nor "under-bill" describes it alone: + + - For a SUBTRACTIVE provider (`gemini`, `openai`, `workers-ai` — in + `_INPUT_INCLUDES_CACHE_READ`), `compute_cost` subtracts `cache_read` out of + `input`. A missed cache key leaves those tokens billed at the full prompt + rate instead of the cache rate: an OVER-bill. + - For an ADDITIVE provider (`anthropic`), `cache_read` is billed as its own + line on top of `input`. A missed key means those tokens are not billed at + all: an UNDER-bill, which is the direction this SDK treats as worse. Uses `or`-style fallthrough (not "first key present"), so a provider that sends both its own name and the gateway's with one of them zeroed still resolves to @@ -129,21 +139,33 @@ def extract_cloudflare_log(entry: dict[str, Any]) -> CanonicalUsage: return CanonicalUsage( input=_safe_int(entry.get("tokens_in")), output=_safe_int(entry.get("tokens_out")), - # Gateway's own snake_case first (present in 8 of the 14 captured - # fixtures), then its camelCase form, then the providers' own native names - # — Gemini calls it `cachedContentTokenCount`, Anthropic - # `cache_creation_input_tokens`, and the `reasoningTokens` fixture proves - # native keys do reach us unnormalized. + # Cloudflare's own key first — that is the only spelling ever observed + # (`input_cached_tokens` in 8 of the 14 captured fixtures). Everything after + # it is unobserved insurance: its camelCase form, then the two big providers' + # native names, in case Cloudflare ever forwards a provider's usage object + # rather than rewriting it into its own vocabulary. Kept because + # `_first_int` fallthrough is free and a missed cache key mis-bills in one + # direction or the other for EVERY provider (see `_first_int`) — but this is + # belt-and-braces, not handling for a case we have seen. cache_read=_first_int( - usage_meta, "input_cached_tokens", "inputCachedTokens", "cachedContentTokenCount" + usage_meta, + "input_cached_tokens", + "inputCachedTokens", + "cachedContentTokenCount", # Gemini native + "cache_read_input_tokens", # Anthropic native ), cache_write=_first_int( usage_meta, "input_cache_creation_tokens", "inputCacheCreationTokens", - "cache_creation_input_tokens", + "cache_creation_input_tokens", # Anthropic native + ), + reasoning=_first_int( + usage_meta, + "reasoningTokens", # Cloudflare's own camelCase outlier — the observed one + "reasoning_tokens", + "thoughtsTokenCount", # Gemini native ), - reasoning=_first_int(usage_meta, "reasoningTokens", "reasoning_tokens"), model=_safe_str(entry.get("model")), provider=_normalize_provider(entry.get("provider")), api="cloudflare_gateway", diff --git a/src/lago_agent_sdk/wrappers/gemini.py b/src/lago_agent_sdk/wrappers/gemini.py index 74b37b2..a620beb 100644 --- a/src/lago_agent_sdk/wrappers/gemini.py +++ b/src/lago_agent_sdk/wrappers/gemini.py @@ -91,19 +91,33 @@ def _stream(*args: Any, **kwargs: Any) -> Iterator[Any]: def _iter() -> Iterator[Any]: last_with_usage: Any = None + resolved_model: str | None = None try: for chunk in src: payload = chunk.model_dump() if hasattr(chunk, "model_dump") else chunk - if isinstance(payload, dict) and payload.get("usage_metadata"): - # Carry `model_version` too. Gemini hot-swaps "-latest" - # aliases server-side, so the chunk's own version is what - # OpenRouter lists and what pricing must key off; a - # usage-only payload silently reverted to the requested - # alias on every streaming call. - last_with_usage = { - "usage_metadata": payload["usage_metadata"], - "model_version": payload.get("model_version"), - } + if isinstance(payload, dict): + # `model_version` must PERSIST across chunks, not be read + # off whichever chunk happens to carry usage. Gemini + # hot-swaps "-latest" aliases server-side and announces + # the resolved version on an EARLY chunk, while usage + # arrives on the last one — so reading it from the + # usage-bearing chunk alone found nothing and silently + # reverted to the requested alias. Measured on identical + # input: this emitted "gemini-flash-latest" where the JS + # port, which already persisted it, emitted + # "gemini-2.5-flash-002". Pricing keys off the resolved + # version, so the two ports priced the same call + # differently. Both spellings are accepted because + # `model_dump()` yields snake_case while a raw REST dict + # is camelCase. + mv = payload.get("model_version") or payload.get("modelVersion") + if isinstance(mv, str) and mv: + resolved_model = mv + if payload.get("usage_metadata"): + last_with_usage = { + "usage_metadata": payload["usage_metadata"], + "model_version": resolved_model, + } yield chunk finally: if last_with_usage is not None: @@ -122,19 +136,33 @@ async def _stream_async(*args: Any, **kwargs: Any) -> AsyncIterator[Any]: async def _aiter() -> AsyncIterator[Any]: last_with_usage: Any = None + resolved_model: str | None = None try: async for chunk in src: payload = chunk.model_dump() if hasattr(chunk, "model_dump") else chunk - if isinstance(payload, dict) and payload.get("usage_metadata"): - # Carry `model_version` too. Gemini hot-swaps "-latest" - # aliases server-side, so the chunk's own version is what - # OpenRouter lists and what pricing must key off; a - # usage-only payload silently reverted to the requested - # alias on every streaming call. - last_with_usage = { - "usage_metadata": payload["usage_metadata"], - "model_version": payload.get("model_version"), - } + if isinstance(payload, dict): + # `model_version` must PERSIST across chunks, not be read + # off whichever chunk happens to carry usage. Gemini + # hot-swaps "-latest" aliases server-side and announces + # the resolved version on an EARLY chunk, while usage + # arrives on the last one — so reading it from the + # usage-bearing chunk alone found nothing and silently + # reverted to the requested alias. Measured on identical + # input: this emitted "gemini-flash-latest" where the JS + # port, which already persisted it, emitted + # "gemini-2.5-flash-002". Pricing keys off the resolved + # version, so the two ports priced the same call + # differently. Both spellings are accepted because + # `model_dump()` yields snake_case while a raw REST dict + # is camelCase. + mv = payload.get("model_version") or payload.get("modelVersion") + if isinstance(mv, str) and mv: + resolved_model = mv + if payload.get("usage_metadata"): + last_with_usage = { + "usage_metadata": payload["usage_metadata"], + "model_version": resolved_model, + } yield chunk finally: if last_with_usage is not None: diff --git a/tests/unit/gateway/adapters/test_cloudflare_gateway.py b/tests/unit/gateway/adapters/test_cloudflare_gateway.py index 6512ebd..1e6c8cc 100644 --- a/tests/unit/gateway/adapters/test_cloudflare_gateway.py +++ b/tests/unit/gateway/adapters/test_cloudflare_gateway.py @@ -420,3 +420,45 @@ def test_cache_read_still_zero_when_genuinely_absent() -> None: ) assert u.cache_read == 0 assert u.cache_write == 0 + + +def test_provider_native_cache_and_reasoning_spellings_are_accepted() -> None: + """SYNTHETIC entries — no provider-native key appears in ANY of the 14 captured + fixtures (they carry only Cloudflare's own vocabulary). These pin the unobserved + insurance spellings so the fallthrough list cannot be trimmed by accident. + + The direction of the harm differs by provider, which is why both matter: + Anthropic's cache_read is ADDITIVE, so a missed key means those tokens are never + billed (under-bill); Gemini's is SUBTRACTIVE, so a missed key bills them at the + full prompt rate (over-bill). + """ + anthropic_native = extract_cloudflare_log( + { + "tokens_in": 100, + "tokens_out": 10, + "provider": "anthropic", + "usage_metadata": {"cache_read_input_tokens": 4242}, + } + ) + assert anthropic_native.cache_read == 4242 + + gemini_native = extract_cloudflare_log( + { + "tokens_in": 100, + "tokens_out": 10, + "provider": "google-ai-studio", + "usage_metadata": {"thoughtsTokenCount": 852}, + } + ) + assert gemini_native.reasoning == 852 + + # Cloudflare's own spelling still wins when both are present + both = extract_cloudflare_log( + { + "tokens_in": 100, + "tokens_out": 10, + "provider": "anthropic", + "usage_metadata": {"input_cached_tokens": 11, "cache_read_input_tokens": 4242}, + } + ) + assert both.cache_read == 11 diff --git a/tests/unit/test_wrapper_gemini.py b/tests/unit/test_wrapper_gemini.py index c9fe0d1..ba8fd65 100644 --- a/tests/unit/test_wrapper_gemini.py +++ b/tests/unit/test_wrapper_gemini.py @@ -192,6 +192,65 @@ def test_stream_attributes_the_resolved_model_not_the_requested_alias() -> None: assert models == {_RESOLVED_STREAM_MODEL}, f"expected the resolved version, got {models}" +class _VersionOnlyOnEarlyChunk: + """A SYNTHETIC stream: the resolved version arrives on an early chunk, usage on + the last one, with no version on it. + + This is deliberately NOT what real Gemini does — verified live 2026-08-20 that + every streaming chunk carries BOTH `model_version` and `usage_metadata`, which + is why `FakeGeminiClient` puts the version on both and why this hazard is + invisible there. The point of this case is the robustness property, not a + captured shape: `model_version` must be remembered across chunks rather than + read off whichever chunk happens to carry usage. Python read it from the + usage-bearing chunk alone, so on this input it reverted to the requested alias + while the JS port (which already persisted it) reported the resolved version — + the two repos priced the same call differently. + """ + + __module__ = "google.genai.client" # so the detector routes it to the gemini wrapper + + def __init__(self) -> None: + self.models = self + + def generate_content_stream(self, **kwargs: Any) -> Any: + return iter( + [ + FakeStreamChunk( + { + "candidates": [{"content": {"parts": [{"text": "hi"}]}}], + "model_version": _RESOLVED_STREAM_MODEL, + "usage_metadata": None, + } + ), + FakeStreamChunk( + { + "candidates": [{"content": {"parts": [{"text": "."}]}, "finish_reason": "STOP"}], + # no model_version here + "usage_metadata": { + "prompt_token_count": 9, + "candidates_token_count": 4, + "thoughts_token_count": 0, + "total_token_count": 13, + }, + } + ), + ] + ) + + +def test_stream_remembers_the_resolved_version_from_an_earlier_chunk() -> None: + sdk, received = _make_sdk() + client = sdk.wrap(_VersionOnlyOnEarlyChunk()) + list(client.models.generate_content_stream(model="gemini-flash-latest", contents="hi")) + assert sdk.flush(timeout=2.0) + sdk.shutdown(timeout=1.0) + flat = [e for batch in received for e in batch] + models = {e["properties"]["model"] for e in flat} + assert models == {_RESOLVED_STREAM_MODEL}, ( + f"the version from the earlier chunk must survive to the usage chunk; got {models}" + ) + + def test_wrap_generate_content_stream_captures_usage_from_final_chunk() -> None: sdk, received = _make_sdk() fake = FakeGeminiClient()