fix(v2): carry llmUsage through to_v2_document so the SaaS can settle its reserve - #537
Closed
behramcelen wants to merge 1 commit into
Closed
behramcelen wants to merge 1 commit into
behramcelen wants to merge 1 commit into
Conversation
Contributor
|
Thanks for the pull request. Before we can merge it, please read our Contributor License Agreement and sign it by posting the comment below in this PR. I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
`V2Document` (`crates/crw-server/src/routes/v2/adapters.rs`) had no `llm_usage`
field, and `to_v2_document()` never copied one. Every `/v2/scrape` response
dropped the token telemetry that `/v1` has always carried on
`ScrapeData::llm_usage`.
That field is billing infrastructure, not diagnostics. A caller prices a
managed-LLM scrape by reserving a worst-case estimate up front and settling it
against the engine's reported usage. With the field absent there is nothing to
settle against, so the reserve stands and the worst case is charged on every
request.
Measured on the same page with the same schema: `/v1/scrape` with `jsonSchema`
returns `llmUsage` (631 in / 45 out), while the `/v2/scrape` object form
`[{type:"json",schema}]` returns none.
The fix is additive to the frozen Firecrawl envelope, which this file already
does deliberately elsewhere for the same reason: `V2CrawlStatus.blocked` carries
the comment "Additive to the Firecrawl envelope (their SDKs ignore unknown
keys)". `skip_serializing_if = "Option::is_none"` keeps the key out of every
non-LLM response, so callers who never trigger a model see the Firecrawl shape
byte-identical to today.
Tests: two regressions in `routes::v2::adapters::tests`. One asserts the usage
survives the mapping AND reaches the wire as `llmUsage.inputTokens`, which is
the exact key a billing caller reads; a field that exists on the struct but
serialises under another name would still be silent. The other asserts a
non-LLM scrape does not grow the key (no `"llmUsage": null`).
cargo test -p crw-server --lib routes::v2::adapters
test result: ok. 39 passed; 0 failed
Whole `crw-server` suite unchanged: `parse_pdf_upload_returns_markdown` fails
identically with and without this commit and is unrelated, a PDF trailer parse
failure.
us
force-pushed
the
fix/v2-scrape-llm-usage
branch
from
September 13, 2026 13:16
45c3f93 to
1757ee5
Compare
Owner
|
Superseded by 538 (same branch, same commit). |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The bug
V2Document(crates/crw-server/src/routes/v2/adapters.rs) had nollm_usagefield, andto_v2_document()never copied one. Every/v2/scraperesponse dropped the token telemetry that/v1has always carried onScrapeData::llm_usage.That field is billing infrastructure, not diagnostics. The SaaS prices a managed-LLM scrape by reserving a worst-case estimate up front and settling it against the engine's reported usage. With the field absent there is nothing to settle against, so it holds the reserve — and the caller pays the worst case on every request, forever.
Measured on production
Same page, same schema, same work:
llmUsage/v1/scrape+jsonSchema/v2/scrapeobject-form[{type:"json",schema}]An 18x over-charge decided by nothing but which API version the caller used. Reproduced end to end through the SaaS with a temporary key; the resulting
UsageEventrow for the v2 call isllmInputTokens: 0, llmModel: "crw-managed-pro", creditsCharged: 91— the SaaS defaults, i.e. the "no telemetry" fallback branch.Why the blast radius is the worst possible
The SaaS
next.config.tsserves this v2 surface at the domain root specifically soFirecrawlApp(api_url="https://api.fastcrw.com")is a drop-in target for the firecrawl-py SDK.So every customer migrating from Firecrawl — the exact audience that compatibility was built to win — landed on
/v2by default and was billed ~18x for it. One of them derived the ratio from his own invoice and reported it as "a 18x cost bump" before we found it internally.The change
Add
llm_usagetoV2Documentand copy it into_v2_document().Additive to the frozen Firecrawl envelope, which this file already does deliberately elsewhere for exactly this reason —
V2CrawlStatus.blockedcarries the comment "Additive to the Firecrawl envelope (their SDKs ignore unknown keys) and load-bearing: the SaaS bills offcompleted".skip_serializing_if = "Option::is_none"keeps the key out of every non-LLM response, so callers who never trigger a model see the Firecrawl shape byte-identical to today.Tests
Two regressions in
routes::v2::adapters::tests:v2_document_carries_llm_usage_for_the_saas_to_bill_on— asserts the usage survives the mapping and reaches the wire asllmUsage.inputTokens, which is the exact key the SaaS reads. A field that exists on the struct but serialises under another name would still be a silent 18x.v2_document_omits_llm_usage_when_no_llm_ran— asserts a non-LLM scrape does not grow the key (no"llmUsage": null).Whole
crw-serversuite unchanged:parse_pdf_upload_returns_markdownfails identically with and without this commit (verified by stashing), and is unrelated — it is a PDF trailer parse failure.Follow-up, not in this PR
Affected customers need refunding for the period this was live. The SaaS side should also grow a safety net so a missing-telemetry response can never again silently bill the worst case in full.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RUNZ9Nm19QSLWf4GhWpayq