Skip to content

fix(provider-cost): price a sandbox execution the provider never billed - #1293

Merged
drewstone merged 1 commit into
mainfrom
fix/provider-prices-unreceipted-work
Sep 18, 2026
Merged

drewstone merged 1 commit into
mainfrom
fix/provider-prices-unreceipted-work

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Closes part of #1175. Do not merge this alone — see the measurement at the bottom.

The defect

priceUnreceiptedWork has exactly one caller in the runtime, and it is the bridge executor. The provider path obtains an estimate only by copying receipt.estimatedCostUsd off the harness stream, so a sandbox-rooted run with 14.5M prompt tokens settles usd: 0, usdKnown: false with no estimate at all — not even a catalog floor.

Measured: mech-interp-foundations-sandbox-a-20260917g settled recording ~$0 and actually cost $35.66, recovered by joining each sandbox to its per-sandbox router key.

The change

Settlement asks the catalog when pendingEstimate === undefined, via a local catalogPrice() that tries the model a usage receipt reported, then turn.model, then concreteProfileModel.

Cannot promote an unproven dollar. sawUnknownCostReceipt = true is set unconditionally in that block and Spend.usdKnown is sawCostReceipt && !sawUnknownCostReceipt, unchanged. The estimate rides usdEstimated in full.

Cannot double-count. Prices only when usd === 0 — no dollars of any provenance reached the channel for this execution. This is stricter than the bridge's turnKnownCostSubtotal === 0, which ignores unknown-marked amounts; a partly billed execution keeps its unresolved part unknown, erring toward "missing stays missing".

An unpriced model settles cleanly with no estimate at all. Every sentinel that could reach the catalog by accident ('worker', 'default', 'unknown', '') was checked and is unpriced.

Tests

  • prices unreceipted provider work from the catalog instead of settling a bare zerored on base (1 failed / 106 passed, received {usd: 0, usdKnown: false, provenance: 'uncaptured'}), green after ({usd: 0, usdKnown: false, usdEstimated: 0.4416, provenance: 'catalog-estimate'}). Verified red by restoring the file from git in the same worktree.
  • settles a model the catalog cannot price with no estimate at all — negative case.
  • does not price an execution that already put billed dollars on the channel — double-count guard; without it the case would have been $0.4716 instead of $0.03.

Checks, all exit 0: environment-provider.test.ts 107/107 across four runs · six cost suites 167/167 · src/runtime tests/runtime tests/kernel 2,642 passed / 1 skipped / 187 files · typecheck · lint · check:version-bump · docs:check · check:api-surface · check:model-execution-boundary · verify:static-imports.

The chore(release): 0.241.0 commit was dropped during rebase; the version bump belongs to the release owner.

Why this should not merge alone

I recovered the actual charge for sandbox runs and compared it to Runtime's estimate on the runs where both exist (n=5, thin and stated as thin):

Runtime's estimate is a median 18.7x the charge, range 0.74x to 162.6x.

run Runtime usdEstimated router charged
zkp-frontier-20260911c $54.50 $7.85
q36-quantum-20260911-tangle-r8-164216 $48.90 $0.30

This change replaces a false $0 with an honest but potentially 100x-too-large estimate. $0 on a run that cost $35.66 is the worst of the three states because it is believed, so this is a real improvement — but an estimate landing anywhere from 0.74x to 162x should not be summed into a fleet total, and a caller cannot tell which end of that range it is on.

#1175 asks for two things and they are not alternatives: price the unreceipted work and let a profile or route declare it has no marginal cost so the catalog is skipped rather than producing a large marked estimate. Directors run a subscription route with near-zero marginal cost, which is exactly what produces the 162x.

Merging this without the escape hatch trades a believed zero for a believed overestimate. I would rather land both.

🤖 Generated with Claude Code

`priceUnreceiptedWork` had exactly one caller, the bridge executor. The
provider/sandbox executor obtained a dollar figure only by copying
`receipt.estimatedCostUsd` off the harness stream, so a provider that
reported tokens and no dollars settled `usd: 0` with no `usdEstimated` at
all — not even a catalog floor. Reproduced here: a provider fixture
presenting 200,000 prompt and 20,000 completion tokens under a profile
declaring `glm-5.3` settles `{ usd: 0, usdKnown: false, provenance:
'uncaptured' }`, while `estimateCost(200_000, 20_000, 'glm-5.3')` is
0.4416 and `isModelPriced('glm-5.3')` is true. The corpus effect — five
sandbox-rooted discovery runs recording `usd=0` on 252k to 49.3M input
tokens beside a bridge-rooted run recording $54.50 on 48.4M — is reported
from that program's records and was not re-measured here.

Settlement now asks the catalog when the harness supplied no estimate.
Two limits hold the number honest:

  - `usdKnown` stays false and the whole amount rides `usdEstimated`, so
    `usd - usdEstimated` remains the dollars a provider is known to have
    billed. A price approximates what a provider would bill; it never
    measures what it did.
  - It prices only an execution with no dollars on the channel. `tokens`
    is the execution's cumulative total rather than the unresolved
    remainder, so pricing it beside any receipt would charge the same
    tokens twice — the same reason bridge-executor gates on
    `turnKnownCostSubtotal === 0`. A partly billed execution keeps its
    unresolved part unknown, which is true.

A model the catalog cannot price still settles `usd: 0, usdKnown: false`
with `usdEstimated` absent: nothing could be priced, rather than priced at
nothing. Every sentinel that could reach the catalog by accident — the
worker's name, `default`, `unknown` — is unpriced, so none can invent a
dollar.

Admission does not move. An unknown-dollar settlement already zeroed a
usd-capped root's free balance and still does; what changes is the
recorded total.

Tests: a run with 200k/20k tokens on a priced model settles the catalog
amount with `usdKnown: false` and `usd - usdEstimated === 0`; the same run
on an unpriced model settles zero with no estimate; a run holding a
billing receipt plus a later unbilled call keeps its $0.03 and gains no
estimate.

Partly serves #1175, which asks for three things: carry the provider's
reported per-call cost with `usdKnown: true` (untouched here), keep a
subscription route unpriced rather than catalog-estimated (in tension with
this change, which prices any execution the provider did not bill), and
keep an estimate separable from reported cost in the projection (which
this satisfies). #1175 stays open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 88b905f6

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-09-18T23:16:04Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Coverage 1 of 2 lenses (value)
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 189.5s (2 bridge agents)
Total 189.5s

⚠️ Partial audit — the verdict covers only value. usefulness: agent returned no usable verdict. Treat the missing lens as unexamined, not as clear.

💰 Value — sound

The provider-path executor now asks the existing catalog-pricing kernel for an estimate when an execution reported tokens but zero dollars of any provenance, closing a measured $35.66-settled-as-$0 hole while preserving the price-is-never-a-receipt invariant — a clean, in-grain reuse of the module b

  • What it does: In streamProviderExecutor (src/runtime/environment-provider.ts:1062-1086), an execution with pendingUnpricedWork used to settle a bare cost event of usd:0 with provenance 'uncaptured' when no harness estimate (pendingEstimate) existed. Now settlement falls back to a local catalogPrice() (environment-provider.ts:1234-1245) that prices the execution's cumulative token total via the shared priceUnrec
  • Goals it achieves: A sandbox-rooted run that certainly spent money no longer settles reporting a confident-looking $0: the PR measures one run settling ~$0 against an actual $35.66. After merge, budget pools and run records for unreceipted provider work carry a catalog-priced lower-bound estimate (subtractable via usdEstimated) instead of a false zero, matching what the bridge executor already does for the same prob
  • Assessment: Good on its merits. It reuses the exact kernel the bridge executor already uses for identical unreceipted-work pricing (bridge-executor.ts:1005-1022) rather than reinventing pricing, and preserves the kernel's central invariant: sawUnknownCostReceipt is set unconditionally in the settlement block, Spend.usdKnown = sawCostReceipt && !sawUnknownCostReceipt is untouched, and the entire estimate rides
  • Better / existing approach: none — this is the right approach. Searched for existing model-candidate resolution to reuse: model-identity.ts exports only match/snapshot predicates (observedModelMatchesDeclared, observedModelHasSnapshot), model-policy.ts:43-48 exports profileProviderModel (strips a provider prefix from a profile model id) — the only adjacent utility, noted as a weak nit below. Pushing pricing into createSandbo
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error event without a message: {"type":"error","timestamp":1789774115602,"sessionID":"ses_f49264f33ffePh0KflT8cAbIzR","error":{"name":"UnknownError","data":{"message":"Unexpected server error. Check server logs for details.","ref":"err_31319889"}}}

🎯 Usefulness — error

usefulness agent produced no parseable value-audit JSON.

  • Model: opencode/deepseek/deepseek-v4-pro
  • Bridge attempts: 3
  • Bridge error: opencode/zai-coding-plan/glm-5.2: opencode: opencode error event without a message: {"type":"error","timestamp":1789774191420,"sessionID":"ses_f49264f66ffeqI5Cc1awAomX5d","error":{"name":"APIError","data":{"message":"Rate limit reached for requests","statusCode":429,"isRetryable":true,"responseHeaders":{"alt-svc":"h3=":443"; ma=3600","connection":"keep-alive","date":"Fri, 18 Sep 2026 23:29:51 GM

💰 Value Audit

🟡 Third-priority model candidate could reuse profileProviderModel for prefix stripping [better-architecture] ``

catalogPrice() (environment-provider.ts:1235) passes concreteProfileModel(args.createProfile) to the catalog, which matches bare model ids only. The codebase already has profileProviderModel (src/runtime/supervise/model-policy.ts:43-48) that strips the selected provider's prefix (e.g. 'offline/profile-backend' -> 'profile-backend'); a profile declaring a prefixed default would fail isModelPriced and settle unpriced. Impact is marginal — it only suppresses an estimate (the safe direction) on a th


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260918T233143Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 88b905f6

Review health 100/100 · Reviewer score 89/100 · Confidence 65/100 · 2 findings (2 low)

glm: Correctness 89 · Security 89 · Testing 89 · Architecture 89

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision.

🟡 LOW No test exercises the observedModel-first candidate ordering in catalogPrice — src/runtime/environment-provider.test.ts

The three new tests cover: catalog estimate for a priced profile model, uncaptured fallback for an unpriced model, and suppression under partial billing. The fixture's llm_call events carry no model field, so observedModel is always undefined in them and the documented precedence (receipt-reported id beats turn.model beats profile model — the case the comment at environment-provider.ts:1230-1232 calls out for providers that select their own default) is never executed by a test. A fourth case yielding data: { model: 'glm-5.3', tokensIn, tokensOut } under a profile with an unpriced default would pin that ordering; until then the most specific candidate is dead-in-tests code.

🟡 LOW observedModel guard drops a genuine model report when it equals the worker's profile name — src/runtime/environment-provider.ts

buildLlmCall (sandbox-events.ts:711) stamps receipts that name no model with agentRunName, so creditUsage infers 'receipt actually reported a model' from receipt.model !== (args.profile.name ?? 'agent'). If a provider serves a model whose id string equals the worker's profile name (e.g. a profile named 'glm-5.3' whose provider-selected default is also glm-5.3), the guard excludes the only pricable id and the run settles uncaptured instead of catalog-estimated. Impact is bounded: catalogPrice still falls through to turn.model/concreteProfileModel, and a miss settles as unknown dollars (truthful), never as a wrong price. A marker constant or an explicit 'model was defaulted' flag from the ledger would remove the collision, but the comment documents the tradeoff and the failure direction is s


tangletools · 2026-09-18T23:32:34Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 2 non-blocking findings — 88b905f6

Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-09-18T23:32:34Z · immutable trace

@drewstone
drewstone merged commit 466a7a2 into main Sep 18, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants