Feature/cloudflare gateway connector - #13
Merged
Merged
Conversation
…hit bugs - extract_openai_native/extract_anthropic_native now prefer the response's own model over the requested alias, matching what actually served the call. - Wrapper cache-hit detection: skip billing when a gateway (e.g. Cloudflare) served the response from cache, via .with_raw_response.create(...). - New lago_agent_sdk.gateway.adapters.cloudflare_gateway: extract_cloudflare_log() and resolve_subscription() for the log-extraction half of a standalone connector, verified live across all three of Cloudflare's ingress methods (REST /ai/run, Unified/compat, Native binding) and across native wraps for Anthropic, Gemini, and Mistral through their dedicated passthrough endpoints.
…queue reliability Follow-on to 2bb89a0 (Cloudflare AI Gateway connector). Adds: - Gemini adapter: prefer the response's resolved model over the requested alias, same fix already applied to OpenAI/Anthropic in the prior commit. Extracted the shared resolve_model() helper to adapters/_common.py. - Mistral '-latest' alias resolution for OpenRouter pricing lookups, via union-find over Mistral's mutually-aliasing /v1/models shape. - Selective/lazy pricing warm-up: Cloudflare Workers AI and Mistral pricing are primed reactively on the first wrap()'d call (via _auto_prime_pricing_for), not eagerly at SDK init — OpenRouter is still warmed eagerly. - EventQueue: split permanent (4xx) vs. transient send failures, bounded shutdown drain instead of silently dropping stranded events on exit. - Added verify_ssl config option for local dev against a self-signed Lago. - README: documented the Cloudflare AI Gateway connector (live + backfill paths), removed stale references and roadmap/phase markers.
Workers AI cached tokens were billed twice. `provider="workers-ai"` was missing from `_INPUT_INCLUDES_CACHE_READ`, but Workers AI is only ever reached through Cloudflare's OpenAI-COMPATIBLE endpoint, so its `prompt_tokens` already includes `prompt_tokens_details.cached_tokens`. With the cached portion never subtracted, those tokens were charged at the full input rate AND again at the cache-read rate, which Cloudflare's catalog does publish (verified live for kimi-k2.6, kimi-k2.7-code and glm-5.2). Measured +583% overbill against a real cached call (prompt 23233 / cached 23168) at live catalog rates. Gateway-backfilled Gemini calls could never be priced. The adapter passed Cloudflare's own provider vocabulary through verbatim; a real entry reports provider="google-ai-studio", which matched no vendor in _VENDOR_MAP, so lookup_openrouter missed every time (confirmed against the live 400-model table: miss as google-ai-studio, hit as gemini). The same miss kept it out of _INPUT_INCLUDES_CACHE_READ, so Gemini's cache_read — a subset of its input — was billed on top of input rather than subtracted. A model already carrying its vendor prefix never matched. A real REST-path log reports model="anthropic/claude-opus-4.8" with provider="anthropic", which built "anthropic/anthropic/claude-opus-4.8". Now stripped, but only when the prefix agrees with the resolved vendor, so the lookup stays vendor-gated: a model naming a different vendor is still a miss. Also: `_parse_price` raised InvalidOperation instead of returning None for values >= 1e16, because `.quantize()` sat outside the try. That escaped into emit()'s catch-all and dropped the event as an unknown error rather than taking the normal "no price" path. Returning None also matches what the JS port returns for the same inputs. All 10 distinct (provider, model) pairs across the real captured fixtures now resolve to a live price; three previously missed. The shared golden fixture gains a `precomputed_cases` section carrying verbatim costs from real gateway log entries, plus an optional `provider` on `cases` so per-provider token semantics are pinned cross-repo.
anassg-lago
force-pushed
the
feature/cloudflare-gateway-connector
branch
from
August 7, 2026 09:01
0bb4207 to
7f3d7c0
Compare
`ruff format --check` is a CI gate, and these two files have been failing it since 9cfde77, the branch's first commit — `main` is clean. Both changes are purely cosmetic line-length wraps at the configured 110-char limit; no behavior changes. Unrelated to the billing fixes in the preceding commit, but the PR cannot go green without it.
ancorcruz
reviewed
Aug 17, 2026
ancorcruz
left a comment
ancorcruz
left a comment
There was a problem hiding this comment.
Review notes
Six items, in the order I'd act on them. The first two I'd fix before merge; the next three are cheap hardening I'd do in the same pass; the last is a behaviour change worth double-checking against live data.
None of these are caught by the current suite — CI is green on all 6 jobs. Two of them (the id collision, the streaming Workers AI path) need a scenario the tests don't construct.
`_is_permanent_failure` treated the whole 400-499 range as unretryable, but 429 (rate limited) and 408 (request timeout) both mean "try again, later". A throttled 100-event batch therefore took the isolate-and-drop path: 100 further requests aimed at the server that had just asked us to slow down, each also throttled, each then logged and discarded. 100 billable events lost, and the isolation actively deepened the throttle. This was a regression — before the permanent/transient split existed, the same 429 went through the 1s->60s backoff and eventually landed. "Permanent" is now an explicit set (400, 401, 403, 404, 409, 422) rather than a range, so an unrecognized 4xx errs toward retrying: waiting on an event that would have been dropped costs latency, dropping one that would have been accepted costs revenue. The isolate-one-by-one recovery is unchanged for the validation statuses it was written for, so a single bad transaction_id still doesn't take its batch down with it. Retry-After is deliberately not honoured yet: LagoApiError carries only (status, body) and the raise site discards headers, so respecting it means changing an exported constructor.
`_infer_provider` matched only a bare "@cf/...", but Cloudflare's OpenAI-compatible /compat endpoint requires the routing-prefixed form — "workers-ai/@cf/meta/llama-3.3-70b-instruct-fp8-fast". That is what the README and the demo notebook prescribe, and the only form a STREAMING call can report, since the synthetic usage payload carries no model and `resolve_model` falls back to the requested string verbatim. Those calls were stamped provider="openai", looked up against OpenRouter as "openai/workers-ai/@cf/...", missed, and silently degraded to token events — with nothing to degrade to in an llm_cost-only billing setup. The `workers-ai` entry in _INPUT_INCLUDES_CACHE_READ never applied either, so on the paths that did price, cache reads were billed twice again. Recognising the prefixed spelling alone is not enough: Cloudflare's catalog keys models as bare "@cf/...", so lookup_cloudflare_workers_ai now strips the routing prefix before matching, and a genuine unknown model still misses rather than becoming a false hit. CanonicalUsage.model deliberately keeps the spelling the customer used, so reporting stays faithful to the request while pricing resolves.
Each stream wrapper rebuilds a synthetic usage payload from the chunks, and
all three discarded the model the response reported — so `resolve_model`
fell back to the requested alias, which is precisely the bug the
non-streaming path was fixed for. A streamed "gpt-5-chat-latest" stayed
"gpt-5-chat-latest" instead of resolving to the dated snapshot OpenRouter
lists, so price mode missed and degraded to token events while the
identical non-streaming call priced correctly.
Each provider hides the resolved name somewhere different:
* OpenAI reports `model` on every chunk, and on `response` for the
Responses API's terminal event.
* Anthropic reports it ONLY on `message_start` under `message.model`, so
the wrapper now keeps it across the whole accumulate-and-merge stream.
* Gemini reports `model_version`, which it hot-swaps server-side for
"-latest" aliases.
It matters most on a gateway, where the resolved name decides which price
table the call is looked up in at all.
The streaming fakes carried no model field and no wrapper test asserted
properties["model"] — both now do, one attribution test per wrapper.
`_emit_token_events` and the per-field branch of `_push_cost_event` both
built `f"{event_id}_{field_name}"` over the same field vocabulary, and both
are reachable for the SAME event_id: emit() falls back to token events when
a price lookup misses, then takes the cost path once the table is warm.
So a backfill re-run over one window sent `backfill_X_input` under
llm_input_tokens on the cold pass, then re-sent the identical id under
llm_cost on the warm pass. Lago rejected it as a duplicate transaction_id,
and because /events/batch is all-or-nothing that rejection failed every
other event in the batch too. The dollar amounts for that window were never
billed — only the raw token counts — and nothing surfaced it. That defeats
the idempotency promise event_id exists to provide.
The two multi-event paths now use disjoint namespaces (`_tok_` / `_cost_`).
The single precomputed-cost event stays unsuffixed: it pushes exactly one
event, so there is nothing to disambiguate.
No migration needed. `event_id` is absent from 0.2.0 entirely, so no
released path has ever emitted the old format — they all use a random UUID.
That is also why both namespaces could be made explicit rather than only
prefixing the cost side.
Regression test asserts a cold run's ids and a warm run's ids over the same
event_id do not intersect; verified to fail without the fix, where all four
ids collided exactly.
`_pick_mistral_canonical` sorted by `(len(n), n)`, but every dated id in one
family is the same length — so the length term always tied and the choice
fell through to the alphabetical term, which for `-2402` / `-2407` / `-2411`
IS the date, ascending. A family of
mistral-large-2402/-2407/-2411/-latest collapsed onto mistral-large-2402 and
the whole family was priced at a two-year-old rate.
Now sorts by a normalized date descending. 4-digit YYMM is widened to
YYYYMM00 first, because mixed suffix widths do not compare correctly as raw
strings ("20250929" sorts below "2411").
Two further parts of the same bug:
* An explicit dated snapshot is no longer remapped. The mapping loop
rewrote every non-canonical member, including the real dated ids, so
asking for `mistral-large-2411` by name was redirected to the group's
canonical and priced at that snapshot's rate. It matched OpenRouter
directly before alias resolution existed, so that was a regression.
* Ordering is by Unicode code point, and the JS port's `localeCompare` is
gone. It is ICU/locale-dependent, so it was not reproducible across
environments, and for a group differing only by case/separator the two
repos picked DIFFERENT canonicals: `Mistral-Small-2603` here vs
`mistral_small_2603` in JS. `_norm` lowercases and maps "." to "-" but
leaves "_" alone, so the JS pick normalized onto a name OpenRouter does
not list and the group fell back to token events there while pricing
correctly here. Both repos now agree.
Regression tests verified to fail without the fix (4 here, 5 in JS).
The cost path spread `dimensions` into `base_properties` — before `unit`,
`value`, `base_cost` and `unit_price` — so those four SDK-computed keys
overwrote a caller dimension of the same name. `_emit_token_events` spreads
`dimensions` last and honoured it. One customer config, two different
outcomes depending on the mode, and no error on either path.
`dimensions` is now spread last in both, so one rule holds: a caller
dimension always wins over an SDK-computed property of the same name.
This is a reachable config rather than a contrived one — `unit`, `value` and
`model` are ordinary words a customer may use for their own breakdown, and a
per-seat biller naturally writes dimensions={"unit": "seat"}.
The accepted consequence is now pinned by a test rather than left implicit: a
dimension named `value` overrides the REPORTED quantity on token events. It
cannot affect the amount actually charged on a cost event, because
`precise_total_amount_cents` is a sibling of `properties`, not a member.
Regression test asserts both emitters for the same dimensions dict; verified
to fail without the fix, where the cost path reported unit='150' not 'seat'.
`__init__` defaulted `api_url` to the production URL, so the `if api_url:`
guard that exists to let explicit args win ALWAYS fired and overwrote
`config.api_url`. That meant
LagoSDK(api_key=k, config=LagoConfig(api_url="http://localhost:3000/api/v1"))
shipped a local-dev customer's usage data to api.getlago.com, silently.
The default is now None, and each explicit arg is guarded on "was it passed?"
rather than on truthiness — so a config value survives when the arg is
omitted, an explicit arg still wins when given, and the production default is
unchanged when nothing is passed.
This was the shortest path to the bug rather than an exotic one: a custom
api_url and verify_ssl=False go together in exactly one setup — a local Lago
behind a self-signed cert, which is what verify_ssl was added for — and
verify_ssl was reachable only through a LagoConfig, so the feature pushed
callers into the clobber. `verify_ssl=` is now accepted directly.
Validated against a real local Lago instance: events emitted with
`LagoSDK(api_key, api_url, verify_ssl=False)` and no LagoConfig land with the
exact expected unit deltas, and the config-only form now resolves to the
local url instead of production.
…eries test Two test-infrastructure fixes found by actually pointing the live suite at a real Lago instead of the in-process mock. Reconciliation could not run against a local Lago at all. It is the ONLY test that proves Lago *accepts* what we emit — every other integration test talks to a mock, so a wrong metric code or a rejected precise_total_amount_cents would pass there and surface only in production. Both halves failed on SSL against a self-signed dev cert: the module's own `requests.get` never passed `verify=`, and the SDK was built without `verify_ssl`. Both now honour LAGO_VERIFY_SSL, mirroring LagoConfig.verify_ssl. Note it had also been skipping silently for a second reason: it gates on LAGO_EXTERNAL_SUBSCRIPTION_ID while the local .env defines LAGO_SUBSCRIPTION_ID, so the skip never announced a misconfiguration. The o-series reasoning test was a coin flip. `o4-mini` spends a variable number of reasoning tokens on the same prompt — measured 0 on some calls and non-zero on others minutes apart — and since the SDK only emits non-zero fields, the hardcoded assertion failed and passed on identical input, alternating between the two repos. It now asserts the SDK's actual contract — emit reasoning tokens WHEN the provider reports them — reading the reported count off the response, checking the emitted value matches it, and skipping when the model answered without reasoning. Verified stable over four consecutive runs per repo.
`unit` on the single-event path was `str(usage.input + usage.output)`, which dropped `reasoning` and `cache_write` entirely and counted a cache-inclusive provider's cached tokens at full weight — while the per-token_type branch directly below reports the de-overlapped `parts["tokens"]`. Two branches of one method, two different bases for the same call. On the captured 16_real_gemini_via_dedicated_endpoint.json row (9 in / 21 out / 852 reasoning) it published unit="30" for a call that consumed 882. New `deoverlapped_token_total()` sums the same PRICED_FIELDS the split path emits one event each for, so the two agree by construction. The charged amount was never affected — that comes from precise_total_amount_cents — but `unit` is the reported quantity a customer points a sum aggregation at. Both _INCLUDES_ sets are applied and deliberately NOT gated on a price existing, unlike compute_cost's subtraction: this is a token count, so whether a rate happens to be published cannot change how many tokens were consumed. The two still agree, because a cache-inclusive provider with no cache_read price keeps those tokens inside `input` and emits no cache_read event. Limited to the five PRICED_FIELDS on purpose: tool_calls is a count of calls rather than tokens, and cache_write_5m/cache_write_1h are a breakdown OF cache_write, so neither belongs in a token total. One under-report the review didn't name: for an ADDITIVE provider the old basis was much wider than the reasoning case. Anthropic with input=1000, cache_read=900, output=100 reported 1100 against 2000 consumed. Verified against a live Lago instance — the llm_cost charge's units moved by exactly 2000, with the dynamic charge carrying the metered $0.05.
emit() returns early whenever the effective mode isn't "price", and never consulted usd_cost on the way out — so a caller who passed a gateway's real metered price got token counts instead, with no log and no on_error. A hand-rolled backfill written from the module docstring (the pattern examples/cloudflare_gateway_demo.ipynb demonstrates) would drop every real cost it read and still look like it succeeded. The configured mode is still respected, deliberately: honouring a per-call usd_cost in token mode would emit an llm_cost event that maps to none of a token-mode customer's configured charges. What changes is that the discard now reaches on_error, the same hook every other billing gap uses. Reported per occurrence rather than deduped — the count of discarded costs is exactly what a caller reconciling on on_error needs, and the documented backfill pattern passes an explicit mode="price", so hitting this at volume means a real misconfiguration rather than normal operation. The common case, no usd_cost supplied, stays silent. Validated against a live Lago instance: on_error fires naming the discarded amount, the call still bills 13/17 token units, and no llm_cost event appears.
`cache_read` checked only `input_cached_tokens` and `cache_write` only `input_cache_creation_tokens`, while `reasoning` on the next line already checked two casings — and that asymmetry was the tell. Surveying every usage_metadata key across all 14 captured fixtures: the gateway's OWN counters are consistently snake_case, but a provider's native key can pass through untouched. The real Gemini entry carries camelCase `reasoningTokens` plus an `input_text_tokens` this adapter maps nowhere. So the spelling of a cache key on a provider we have no CACHED capture for was genuinely unknown. The consequence is an over-bill, not a lost field. `_normalize_provider` maps these entries to `gemini`, and `gemini` is in _INPUT_INCLUDES_CACHE_READ, so compute_cost relies on cache_read being populated to SUBTRACT the cached portion out of input. A silent 0 leaves the whole prompt billed at the full input rate. Measured against the live OpenRouter table on a gemini-2.5-flash call with 9,000 of 10,000 prompt tokens cached: $0.00325 against a true $0.00082, a 3.96x over-bill — and it grows with cache hit rate, so it is worst on the long-cached-system-prompt workload caching exists for. Both fields now check the gateway's snake_case name, its camelCase form, and the provider's own native name (cachedContentTokenCount for Gemini, cache_creation_input_tokens for Anthropic). An extra spelling costs a dict lookup; a missed one costs 4x. Fallthrough is on any falsy value, not just a missing key, so a provider sending both its own name and the gateway's with one zeroed still resolves to the real count. The JS port used `??` here, which only skips null/undefined and resolved to the zero — fixed there too.
…sion OpenRouter marks a MOVING alias with a leading "~" on the vendor — "~anthropic/claude-sonnet-latest", "~openai/gpt-latest", "~google/gemini-flash-latest". parse_openrouter split the id on "/" and took the left half as the vendor, so these indexed under "~anthropic"/"~openai"/"~google", none of which appear in _VENDOR_MAP. A customer in price mode requesting a plain "-latest" alias therefore missed the table and fell back to token events — billing nothing at all in an llm_cost-only setup. Measured live against the 415-model catalog: 11 such ids across 6 vendors, every one carrying real token pricing, every one previously unpriceable, all 11 now resolving. Includes claude-sonnet-latest, claude-opus-latest, claude-haiku-latest, gpt-latest and gpt-mini-latest — names a customer plausibly asks for by hand. The "~" id stays indexed alongside the bare one, so nothing that already worked changes. Verified collision-free: no un-prefixed id duplicates a "~"-prefixed one, so the strip cannot overwrite a real listing. Separately, _VERSION_DATE_SUFFIX now also strips a 3-digit revision. Gemini's model_version can report "-002" where OpenRouter lists only the bare name, so preferring the resolved id turned a hit into a miss. Latent rather than live — every captured real Gemini response reports a bare "gemini-2.5-flash" — but it costs one regex arm and is a config change away. Verified safe: zero of the 415 live ids have a model part ending in exactly three digits. Found while investigating the reviewer's Gemini "-002" question on #13, which turned out to have TWO independent causes; fixing only the suffix would have left that model missing anyway.
The catalog fetch had no test coverage at all and two ways to under-price.
`result_info.total_count` was trusted as a terminator and defaulted to
len(models) when absent, so a missing count broke after page one — keeping 50 of
the 64 models the endpoint actually serves, silently. Measured live, that count
is worse than absent-able: it reports 291 while the endpoint serves 64 (50, then
14, then 0), so `len(models) >= total` can never fire. A short page is the only
reliable end-of-catalog signal and is now the only one used. Today's behaviour
was correct by luck — the short page ended the loop — but any change to that
count would have cost 14 models with no diagnostic.
The `while True` had no page bound, on a loop that runs on the queue's flush
tick AHEAD of the drain: an endpoint returning full pages indefinitely would
stall event delivery, not just waste bandwidth. Capped at 40 pages (~2000 models
against a real catalog of 64), and the truncation is logged, since a short
catalog otherwise reads as "these models are unpriced".
Python-only: one malformed entry unpriced EVERY Workers AI model.
`m.get("properties", [])` only defaults when the key is absent, so an explicit
JSON null returned None and `for p in None` raised TypeError out of
parse_cloudflare_workers_ai into maybe_refresh's handler, which leaves the table
at None. The JS port already isinstance-guarded and dropped only the bad entry.
Five new tests cover the loop that previously had none: the real 50-then-14
shape, a missing total_count, a wrong total_count, the page bound, and a
null-properties sibling surviving. Verified live that the full 64-model catalog
is still walked and yields the same 36 token-priced models.
…from JS
Cross-port audit findings, none of them raised in review.
A dropped event now always reaches on_error rather than only the module logger.
Two paths lost billable events silently from the hook's point of view: the
queue's buffer overflow, and emit() dropping a call when no subscription
resolved. The JS port already reported both, so the two repos disagreed on
whether a lost event is visible at all.
A negative token count could be emitted as a billable quantity.
`nonzero_numeric` filtered on truthiness, so input=-100 survived and was pushed
as value="-100". Nothing upstream should produce one — every adapter clamps at
extraction — but this is the last gate before an event is built, and JS already
filtered on > 0.
`apply_markup` was the one money helper that could raise. A non-numeric input
hit a bare Decimal("abc") and threw InvalidOperation from inside
_push_cost_event, under emit()'s catch-all, so the cost event was dropped and
reported as an unknown "emit" error rather than taking the documented no-price
path — past every caller relying on this module's None-on-bad-input convention.
Now parsed through _parse_price and floored to 0, matching JS and
compute_precomputed_cost's existing behaviour.
money_golden.json is untouched and still byte-identical across repos; both
golden suites pass.
Not fixed here: `_safe_int("1825.0")` returning 0 where JS returns 1825. Every
token value in the real Cloudflare fixtures is an int, so it is unreachable on
this branch — it belongs with the Databricks table columns, which the adapter
documents as arriving as strings.
ancorcruz
reviewed
Aug 19, 2026
ancorcruz
left a comment
ancorcruz
left a comment
There was a problem hiding this comment.
Re-review of the fixes (17b3005..5c514fe)
All five fixes verified in the code, and three went further than the reports asked. Streaming model-drop was fixed in all three wrappers, not just OpenAI's; the cache-key lists gained the providers' native spellings; and the pagination fix corrected my suggestion — I proposed keying on result_info.total_count, you measured it reporting 291 against 64 served and used a short page instead. I also checked deoverlapped_token_total agrees with the split path in all four provider/price combinations, and the _tok_/_cost_ regression test does fail without the fix.
Agreed on deferring prime()/TTL, for the reason you gave — it's the only one of the set that doesn't touch money. Leaving that thread open; resolving the other five.
CI green on all 6 jobs, 513 unit tests pass locally.
Four of the eleven items below are regressions introduced by these fixes, which is the main thing worth acting on: the overflow report can deadlock a customer's thread, api_url now accepts "" and silently stops all billing, deoverlapped_token_total over-reports for workers-ai, and narrowing _PERMANENT_STATUSES re-opened head-of-line blocking for 413. Six were reproduced by running them.
Smaller notes not worth their own threads:
_WORKERS_AI_COMPAT_PREFIXis now defined in bothpricing.py:124andopenai_native.py:52, each with a comment asking a human to keep it in sync — and they're load-bearing on each other (recognise the prefix, then strip it to price).pricing.pyhas no cycle withadapters/, so one definition would do.sdk.py:291's no-subscription path now emitslogger.errorand alogger.warningvia_report_error— two lines per drop.canonical.py'snonzero_numericsilently drops a negative count, while the same commit addson_errorreporting for the other two drop paths. Defensible since adapters clamp upstream, but it's the odd one out now.verify_sslis now a first-classLagoSDK(...)argument and the docstring recommendsverify_ssl=Falsefor local dev — which routes the documented happy path straight throughlago_client.py:25's unguardedrequests.packagesaccess (on the deferred list). Worth pulling that one forward with the rest of the hardening, since it's now the advertised setup.
| # 50 of the 64 available. | ||
| if len(batch) < _CF_PER_PAGE: | ||
| break | ||
| if page >= _CF_MAX_PAGES: |
There was a problem hiding this comment.
The 40-page bound still allows ~400s of blocked event delivery per refresh.
_CF_MAX_PAGES = 40 sequential requests.get calls at the default 10s request_timeout_seconds is up to ~400s, all on the lago-queue thread inside maybe_refresh() — which _run calls before _take_batch(). A slow endpoint, or one that ignores page and keeps returning full pages, stalls every queued billable event for that whole walk; and since _cloudflare_stale clears only on success, the walk repeats.
The changed comment says the bound exists so the loop "must not stall event delivery indefinitely" — which is true, it's now bounded. But 400s is a stall, just a finite one.
This is the same family as the prime()/TTL item you're deferring, so it probably belongs in that same follow-up rather than here. Two cheap options for whenever it lands: drain the buffer before maybe_refresh() in _run, or bound the whole walk by wall clock the way the shutdown drain already does with min(self._max_retry_seconds, 10.0), instead of by page count.
These tests never ran anywhere. Neither CI workflow sets a single provider key, so tests/integration always skipped; running it needed five paid accounts; and every per-provider test pointed at an in-process mock Lago, so it could not prove Lago accepts an event even when it did run. They also never shipped to clients — the wheel is built from src/ only. Adapter behaviour stays pinned by the captured real responses under tests/unit/adapters/fixtures/, which is what the unit tests assert against. CONTRIBUTING now says so explicitly, so the removal does not read as "provider shapes are unverified", and the add-a-provider recipe loses its "add a live integration test" step. Live verification moves to a driver script pointed at a real Lago instance: that is the only way to exercise what these tests could not — llm_cost against a dynamic charge, and precise_total_amount_cents being accepted.
Each of these was a fix that made something else worse, so each carries a
test that fails when the fix is reverted.
Queue overflow could deadlock the customer's thread. _lock is a plain Lock,
so raising the new overflow report while holding it deadlocked any on_error
hook that itself emits — a plausible hook, since reporting a billing gap by
emitting a metric is an obvious thing to do. The report now runs with the
lock released, plus a threading.local guard bounding re-entrancy to one
report per overflow per thread (reporting after release alone recurses,
because the buffer is still full when the hook runs). Covered by a subprocess
test: an in-process deadlock wedges interpreter shutdown and hangs the whole
suite rather than failing it.
api_url="" stopped all billing. Guarding explicit args on "was it passed?"
meant api_url=os.environ.get("LAGO_API_URL", "") with the var unset stored "",
and an empty base URL is unrecoverable downstream: requests raises
MissingSchema, which is not a LagoApiError, so the queue reads it as
transient and retries at the 60s ceiling forever. api_url is now guarded on
non-emptiness, so "" falls through to config and then to the production
default, exactly as an omitted argument does.
workers-ai was missing from _OUTPUT_INCLUDES_REASONING, the output-side twin
of the cache-read entry added for it. The /compat endpoint returns the OpenAI
shape, where reasoning is a subset of output, so counting it additively
inflated the basis — 1900 for a call that consumed 1100. Unreachable today
(Workers AI reports no reasoning tokens at all, verified live), but the two
sets must agree about the same provider for the same reason, and the
cache-read side IS live-reachable.
413/402/415 head-of-line blocked the whole FIFO. These reject a batch AS a
batch, so re-sending it can never succeed — treating them as transient
re-prepended the identical batch and backed off to 60s indefinitely.
Classifying them permanent routes them to _send_individually, which splits
the batch and delivers what is deliverable: the case that path was built for
and could not previously reach. 405/410 stay transient.
Also de-flakes test_overflow_drops_oldest_at_exact_boundary, which set
max_batch_size == max_buffer_size and so let the worker race the assertions
for the lock. The overflow fix widened that window enough to fail 5 runs in
6; the fix is the one its sibling test already carries.
---
A moving "~" alias could overwrite a real listing's price, decided purely by
catalog order. Stripping OpenRouter's "~" marker 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 — that was a property of the day's response, not of the code. On a
synthetic pair the same lookup returned 0.009 or 0.001 depending only on
position in the response. Alias keys are now written only when absent, so a
real listing always wins; the "~"-spelled id still resolves to its own entry,
and non-alias entries keep plain assignment.
The -\d{3} strip arm is now scoped to OpenRouter, the only source that ever
needed it (Gemini's "-002" revision, which OpenRouter omits). The shared
helper also builds the AWS/Bedrock price keys, and there a shortened key does
not merely miss: bedrock_model_key feeds table.setdefault(key, {}) per
direction, so two 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), so this was latent — the split makes
it structurally impossible rather than empirically absent.
apply_markup's two bad-input fallbacks are not equivalent, and the ports
disagreed on one. An unparseable cost means there is nothing to bill, so 0 is
right; an unparseable markup means only the multiplier is unusable, and
returning 0 there discards a good cost. Python returned "0" for both, JS fell
back to 1.0 for a bad markup, so identical input would have billed
differently. Python now matches JS.
That last one is defence in depth, not a live fix, and the code says so:
every emit path already runs the customer's markup through coerce_markup
(which falls back to 1.0 and reports), and both arguments are _fmt_money
output by then, so neither can actually arrive unparseable. What had no
coverage was 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.
---
Five small things, each with a test that fails when reverted.
A recovery path silently reversed FIFO order. _send_individually re-queued
each transiently-failing event as it went, and _replay_failed PREPENDS — so a
413 batch of a,b,c,d,e whose b,c,d failed while isolated came back as d,c,b.
FIFO is the queue's contract: it is what makes the oldest-dropped-first
overflow policy and Lago's own event ordering mean anything. Survivors are now
collected and re-queued once.
A negative token count was dropped without a word. nonzero_numeric correctly
filters it (Lago would otherwise sum a negative billable quantity), but this
was the last drop path that never reached on_error — the same gap already
closed for queue overflow and for an unresolvable subscription. It is
reachable, not theoretical: CanonicalUsage is exported and emit() takes one
directly, which is the documented way to backfill usage the SDK did not
intercept. Reported before the empty-check, so an event whose only fields were
negative still reports instead of returning silently.
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.
verify_ssl=False could crash LagoSDK() construction. The warning suppression
reached through requests.packages, a legacy compatibility alias with no
guarantee of existing, in an unguarded attribute chain inside __init__. Now
imports 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 asserts there is exactly one definition in the tree.
Two comments in the Cloudflare gateway adapter were wrong, and they were the stated justification for code, so they would have misled the next reader. The module docstring claimed usage_metadata's key casing "is NOT normalized by Cloudflare — it passes through whatever convention the provider used". It does not. Across all 14 captured fixtures 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 — is Cloudflare's own inconsistency, not a leaked provider key: Gemini's native spelling is thoughtsTokenCount, absent everywhere. _first_int said a missed cache key is "an over-bill, not an omission". That holds only for a subtractive provider, 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 worse direction. The comment asserted one direction for a function used by both. Anthropic's cache_read_input_tokens and Gemini's thoughtsTokenCount join the spelling fallthrough, labelled in the code as unobserved insurance rather than handling for a known case, since neither has appeared in a fixture. Kept because the fallthrough is free and a missed cache key mis-bills either way. Gemini streaming attributed the requested alias instead of the resolved model when a chunk carried usage without model_version: this port read it off whichever chunk carried usage, the JS port remembered it across chunks, so the two priced the same call differently. It now persists across chunks (sync and async) and accepts both spellings. Not reachable with Gemini as it behaves today — every streaming chunk carries both fields, verified live, which is why the existing fixture could not catch it. That fixture is left faithful to reality; a separate, explicitly synthetic case pins the property instead.
Correct two wrong comments, and widen gateway/Gemini attribution
ancorcruz
approved these changes
Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
wrap()auto-detects a client pointed atgateway.ai.cloudflare.com, skips billing oncf-aig-cache-status: HIT, auto-primes Workers AI pricing) and backfill path (gateway.adapters.cloudflare_gatewayextracts a Logs API entry and bills Cloudflare's own metered cost viaemit(usd_cost=..., event_id=...), idempotent across re-runs).-latest, Gemini resolving to a dated snapshot).-latestaliases against Mistral's own/v1/models(union-find over its mutually-aliasing shape) so OpenRouter price lookups hit the right dated model.wrap()'d call, not eagerly at SDK init.verify_sslconfig option for local dev against a self-signed Lago instance.Testing
current_usageevent counts before/after.