fix(logging): flag failures that may have been charged; guarantee every exit logs (0.7.6) - #27
Closed
VickyXAI wants to merge 3 commits into
Closed
Conversation
added 2 commits
July 16, 2026 00:57
cost_usd=None reads as "$0", but it's also what we write when we don't know. On Solana those are different facts worth real money. Several Solana routes settle OPTIMISTICALLY — chat, search, both image routes, music — firing settle in parallel with the upstream work so it lands inside the ~60-90s blockhash window. A call that verifies and then fails upstream IS charged, and its error response carries no settlement header. So "no proof of payment" and "you were charged" co-occur precisely when it matters, and the audit row said cost_usd=None: indistinguishable from a free failure. A nano-banana-pro edit that dies at the provider costs $0.10 and the ledger recorded zero. That row is the sidecar's answer to its own docstring — "media calls are the priciest per-request traffic, so they must show up in spend reconciliation" — and the one case where money leaves silently was the case it couldn't see. Failed media calls now carry settlement_status="unknown". Base failures don't: Base settles only after a successful upstream call, so a failure there is genuinely free, and a flag on every Base error is noise that gets ignored. One case is flagged on both chains — the gateway settles, returns 200, and the SDK can't parse the body (ValidationError → 502). The money moved regardless of chain, so the chain check must not gate it. It's one bit of chain, not a table of which Solana routes are optimistic: that table lives in the gateway and would drift here — the same drift that made the changelog wrong three times. The asymmetry decides it. A false "unknown" costs a glance at the ledger; a false "$0" loses a real charge. Solana's video route settles on the completed poll and will sometimes be flagged for nothing; that's the trade, on purpose. Only media needed it. The chat passthrough proxies raw responses and already reads cost/settlement from headers even on errors; media goes through the SDK, which raises, so the sidecar never sees a response. Mutation-verified three ways: regressing to always-$0 fails, dropping the chain check (over-flagging Base) fails, and forgetting the both-chains parse case fails.
…very exit
/review on this PR found the flag missed the failures it exists to catch.
The gate was `http_status >= 500 and is_solana`, reasoning that a 4xx is the
gateway refusing before settle. False, and the expensive kind of wrong: the
optimistic settle fires FIRST, so charged failures come back as 4xx. A content
filter rejection is 400 (blockrun-sol images/generations:376), a rate limit is
429 (:359), and blockrun-llm image.py:368 preserves the gateway status verbatim
on the PAID retry. Both are routine and client-triggerable — the most common
charged failure on the chain was the one the flag ignored. My own comment named
"policy" as a pre-settle 4xx, and test_gateway_refusal_is_not_flagged locked the
false negative in.
402 wasn't exempt either: Solana settles at POST, so the poll returns 402 on
wallet-binding AFTER the money moved ("the POST already settled").
So: on Solana flag ANY failure. The single exemption is a proof, not a guess —
SDK request validation raises before anything goes on the wire. On Base, 504 is
now flagged too: it's our own await ceiling, which abandons the wait but leaves
the worker running to settle.
Every exit now logs. Two paths wrote no row at all, which is worse than an
unflagged one — a call that moves money and leaves no trace is invisible to
reconciliation:
- json.JSONDecodeError subclasses ValueError, and the SDK parses the paid 200
with a bare .json(); a truncated body answered 400 (blaming the caller for a
call they paid for) and returned via `raise` before the log ran. Catching
ValidationError first never helped — .json() fails before pydantic.
- httpx.ReadTimeout on a 10-minute image call matched no arm and escaped raw.
Wired all five log sites, not one. The changelog claimed "only the media routes
needed this — the chat passthrough already reads cost/settlement from headers
even on errors". Reading headers only helps when there IS one: Solana chat logs
"CHARGED BUT REQUEST FAILED — refund manually" and throws, carrying no header.
The claim justifying the scope was the one that didn't survive checking, and it
left the highest-volume paid route silent. _run_video_job also swallowed
ValidationError into ValueError, having drifted from its sibling already.
The tx_hash guard stays and is no longer dead: unreachable from _media_endpoint
(result is None on every error arm), but live for the passthrough, which reads
the settlement header off the response and can see a settled error.
All five regressions mutation-verified to fail the tests.
VickyXAI
changed the title
…sm, chain race
The remaining /review items, now that _settlement_status serves the passthrough
and not just media.
The "Base settles only after success" line stopped being safe the moment this
served chat. Base STREAMING chat settles inside metadata.then(), which resolves
even when the stream loop's catch fires — the only skip is
shouldSkipSettleForTruncation, which gates abortReason=="deadline" alone and
sits behind a default-off flag. So a Base stream that opens, emits tokens and
then dies IS charged. It can't reach this function (a mid-stream death never
produces a >=400 row; the 200 was logged when the headers arrived, and a >=400
at header time means the stream never ran) — but the sentence is now written so
it can't be read later as license it doesn't grant.
The Solana success fixture was a Base response wearing a Solana label. It
asserted a Solana row carries tx_hash 0xabc; a real one carries no hash at all,
because settle is still in flight ("Settlement is optimistic, so there is no tx
hash yet"). It manufactured confidence that Solana successes come with proof of
payment. The test now states the real shape — and that successful Solana spend
is ALSO absent from this ledger, which settlement_status does not fix and does
not claim to.
The chain is snapshotted when the call starts instead of resolved at log time.
BLOCKRUN_API_URL is a mutable global and these calls run for minutes; a flip in
flight would classify a Solana charge as Base and write it off as free — the
same false negative, reintroduced as a race. Mutation-verified: resolving late
fails the new test.
Contributor
Author
VickyXAI
added a commit
that referenced
this pull request
Jul 16, 2026
…ry exit logs (0.7.6) (#28) cost_usd=None reads as $0, but it's also what the sidecar writes when it doesn't know. On Solana those are different facts worth real money: several routes settle OPTIMISTICALLY (settle fires in parallel with the upstream work), so a call that verifies and then fails IS charged, and the error carries no settlement header. Failed calls now carry settlement_status='unknown' across all five log sites. Reworked after /review found the first cut wrong. The gate was 'http_status >= 500 and is_solana', reasoning a 4xx is the gateway refusing before settle. False: the settle fires first, so charged failures come back as 4xx — content filter 400 (blockrun-sol images/generations:376), rate limit 429 (:359) — the most common charged failure on the chain was the one the flag ignored, and a test locked the false negative in. Also fixed: 402 isn't exempt (Solana settles at POST; the poll 402 lands after the money moved); JSONDecodeError subclasses ValueError and wrote NO row at all while blaming the caller for a call they paid for; httpx.ReadTimeout matched no arm; Base 504 is our own await ceiling and can still settle; the chat passthrough (highest-volume paid route) was left silent by a scope claim that didn't survive checking; _run_video_job swallowed ValidationError; the chain is now snapshotted at call start rather than re-read from a mutable env at log time. Every exit logs — a call that moves money and leaves no trace is invisible to reconciliation. The gateway ledger stays the authority; this says go look. 294 tests; all six regressions mutation-verified to fail. Supersedes #27 (auto-closed when its stacked base was deleted).
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
1 participant
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.
cost_usd=Nonereads as "$0". It's also what the sidecar writes when it doesn't know. On Solana those are different facts worth real money.Reworked after
/reviewfound the first cut was wrong. Details below.The gap
Solana settles optimistically on chat, search, both image routes and music — settle fires in parallel with the upstream work, so a call that verifies and then fails is charged, and the error carries no settlement header. The audit row said
cost_usd=None, settlement=None: indistinguishable from a free failure.What review caught in the first cut
The gate was
http_status >= 500 and is_solana, reasoning that a 4xx is the gateway refusing before settle. False, and the expensive kind of wrong — the settle fires first:images/generationsimage2imageaudio/generationsblockrun-llm/image.py:368preserves the gateway status verbatim on the paid retry. So the most common charged failure on the chain — a content-filter rejection — was the one the flag ignored. My own comment named "policy" as a pre-settle 4xx, andtest_gateway_refusal_is_not_flaggedlocked the false negative in.Also fixed from review:
json.JSONDecodeErrorsubclassesValueError, and the SDK parses the paid 200 with a bare.json(), so a truncated body answered 400 (blaming the caller for a call they paid for) and returned viaraisebefore the log ran.httpx.ReadTimeouton a 10-minute image call matched no arm at all._run_video_jobswallowedValidationErrorintoValueError— the sibling drift had already happened.Now
Every exit logs, across all five log sites (media, Videos job, chat/messages passthrough). The
tx_hashguard is no longer dead — unreachable from_media_endpoint, but live for the passthrough, which can see a settled error.The check stays one bit of chain, not a per-route table — that table lives in the gateway and would drift here. The asymmetry decides it: a false "unknown" costs a glance at the ledger; a false "$0" loses a real charge. The gateway ledger stays the authority; this says go look.
Verification
293 tests. All five regressions mutation-verified to fail:
restore the
>=500gate · JSONDecodeError → ValueError arm · drop the transport catch-all · un-flag Base 504 · drop the local-validation exemption (over-flagging).