Skip to content

fix(cost): price only the calls no receipt covered - #1185

Merged
drewstone merged 4 commits into
mainfrom
fix/unproven-dollar-partition-20260911
Sep 11, 2026
Merged

fix(cost): price only the calls no receipt covered#1185
drewstone merged 4 commits into
mainfrom
fix/unproven-dollar-partition-20260911

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Follow-up to #1184, addressing the defects its reviews raised. Three commits; one is a behaviour fix, two are evidence and documentation.

What changed and why

fix(cost): price only the calls no receipt covered (#1175).
streamSandboxLeaf read usdEstimated off the loop TOTAL whenever costUsdKnown was false. That flag is an AND — run-loop.ts:725 clears it on any single llm_call carrying usdKnown: false, run-loop.ts:833 clears it for an iteration that saw no call at all, and run-loop.ts:1093 ANDs it across iterations. So one unproven call relabelled every receipt-backed dollar beside it as a price, and usd - usdEstimated — the quantity the estimate channel exists to make readable — reported $0.00 billed against money a provider really had charged. The unproven amount is now summed per call in the loop ledger that already accumulates costUsd, and carried on Iteration.unprovenCostUsd and LoopResult.unprovenCostUsd beside it.

docs(durable): state that the holder liveness answer is advisory.
runDirectoryHolderIsLive reads the lock file outside the mutation guard acquireRunDirectoryLock evaluates the identical rule under. The docstring named a supervisor picking up an abandoned directory as a caller without saying such a caller must still acquire, which invites acting on a false a concurrent acquire has already overtaken. No behaviour change.

test(budget): pin the leaked reservation into the durable settle record.
leakedReservations was asserted only on the in-memory result. #1181's symptom was a persisted failure.json reporting nodes: 0 with no settle record at all, so the durable artifact is the one that mattered. The new case runs the settled result through settleRecordJson — the same serializer result.json is written with — and asserts the leak and its assignment survive.

Capability lifted / owner code extended

  • The cost fix does not introduce a rule. It applies the partition this repo already owns: budget.ts meterUsageEvent folds usdEstimated per cost EVENT (totals.usdEstimated += ev.usdEstimated, only under usdKnown: false), and bridge-executor.ts:986 prices per turn rather than per run for the same reason. streamSandboxLeaf was the one settlement path that folded first and classified afterwards; it now reads the per-call sum the loop ledger already had the facts to produce.
  • The usdEstimated ⊆ usd clamp is assertValidSpend's own rule (budget.ts: usdEstimated must not exceed usd), not a new one.
  • The settle-record test uses settleRecordJson from src/durable/settle-record.ts — the serializer that already refuses any value JSON would misstate — rather than asserting against a hand-built shape.
  • No new dependency; no new journal event kind; no new public function. Two optional fields are added to existing @stable interfaces.

Measured evidence from this session

All of these were run in this worktree, through createExecutor({ backend: 'sandbox', … }) with a scripted box client — the real leaf path, not a stub.

  1. The defect, reproduced. A leaf whose stream carried llm_call {costUsd: 0.5, usdKnown: false} then a terminal done {totalCostUsd: 1.5} settled:
    {usd: 1.5, usdKnown: false, usdEstimated: 1.5}usd - usdEstimated = $0.00 billed. The ledger credits the $1.00 residual of the cumulative terminal receipt as usdKnown: true (sandbox-events.ts buildLlmCall: absent costProvenance on a canonical terminal means the provider receipt), so $1.00 of stated billing read as zero.
  2. After the fix, the same stream settles {usd: 1.5, usdKnown: false, usdEstimated: 0.5}usd - usdEstimated = $1.00, matching the receipt. The emitted cost event carries the same partition.
  3. Revert-sensitivity checked by execution, not by reading. With only the runtime.ts hunk reverted to its previous form, the new mixed-settlement case fails (1 failed, 8 passed in that file). Before this change the whole suite passed with that hunk reverted — the cloud-child settlement had no coverage at all.
  4. Scope constraint verified. Admission and reconciliation read only spent.usd and spent.usdKnown (budget.ts reserve/reconcile), never usdEstimated, so a dollar-capped run admits and spends exactly what it did before. usdKnown is untouched on every path.

Test command and result

pnpm test        # vitest run — the repo's own script

291 test files passed, 3 skipped (294 total); 3758 tests passed, 0 failed, 11 skipped (3769 total); 126.35s.

Also run, all clean: pnpm run lint (biome, 699 files), pnpm run typecheck (tsc --noEmit + examples), pnpm run build (tsdown), pnpm run check:api-surface (1978 exports across 13 entry points, record current), pnpm run check:version-bump (2 export change(s) needing a minor bump, paid for by 0.210.0 -> 0.211.0; 0.210.0 is the last published version, so the bump already on main covers these and no new release commit is needed), pnpm run docs:check, pnpm run verify:package, pnpm run check:publish-workflow, pnpm run check:upstream-evidence-workflow.

Explicitly NOT in scope

  • sandbox-session.ts and environment-provider.ts are unchanged, deliberately. One review read them as the same defect class. They are not: both declare their whole dollar channel unproven at settlement (usdKnown: false whenever a turn ran, independent of what any call reported), and under that declaration usdEstimated == usd is the correct reading — nothing on the channel is claimed as billed. streamSandboxLeaf is different because it honours per-call provenance (usdKnown = result.costUsdKnown !== false), so it is the only one of the three that can hold a mixture. Changing the other two would mean revisiting whether a sandbox-reported dollar is a receipt at all, which is the decision fix(accounting): cloud child spend is recorded as an unreported gap while the provider reports exact per-call cost #1175 was about and a separate change.
  • budget.ts reconcile is unchanged. A review asked for assertValidSpend/validateResourceUnits to be folded into the decided-then-thrown path so an unreadable spend settles its ticket. I implemented it and measured it, and it is the wrong trade: with the change, a run whose child reported tokens.input: -1 returns kind: 'winner' carrying spentTotal.tokens.input: -1 with tokensKnown: true and no spendGaps — an impossible count presented as a measured total on the one arm that promises a complete one. The stated motivation does not hold either: both behaviours charge the full ceiling rather than refunding it, so tokensLeft is identical (measured: 90,000 of 100,000 before and after, and a second spawn asking for the whole remainder is admitted either way). No capacity is stranded; the open ticket is what makes the winner arm refuse the corrupt total, so it is load-bearing. The change is reverted and not proposed.
  • No journal event for leakedReservations. A review asked for a new leaked-reservation journal kind alongside teardown-unconfirmed. The evidence is already durable: result.json serializes the whole SupervisedResult, which is now pinned by test. A second copy in spawn-journal.jsonl would widen the public journal-event union for evidence that already survives.
  • strategy.tssupervise-surface.ts still drops usdEstimated when projecting a supervised spentTotal into AgenticRunResult, so a benchmark surface rebuilds a Spend whose whole dollar reads as billed. Named in fix(observability): settle a leaked-reservation run, expose run-directory liveness, and name unproven dollars #1184 and still out of scope: it widens a public result type and is not the pursuit settlement fix(accounting): cloud child spend is recorded as an unreported gap while the provider reports exact per-call cost #1175 measured.
  • No release action. main declares 0.211.0; nothing is tagged or published. Unchanged by this PR.

One correction to #1184's own description: it claimed leakedReservations is declared on both arms of SupervisedResult and is dead on the winner arm. That is wrong — it is declared only on the two no-winner arms (types.ts:1701 lifecycle, types.ts:1748 driver-failed). There is no dead field.

🤖 Generated with Claude Code

drewstone and others added 4 commits September 11, 2026 10:39
`streamSandboxLeaf` read `usdEstimated` off the loop TOTAL whenever
`costUsdKnown` was false. That flag is an AND: `run-loop.ts` clears it on
any single `llm_call` carrying `usdKnown: false`, and again for an
iteration that saw no call at all. One unproven call therefore relabelled
every receipt-backed dollar beside it as a price, and `usd - usdEstimated`
— the quantity the estimate channel exists to make readable — reported
$0.00 billed against money a provider really had charged.

Measured on the real path with a scripted box: a leaf whose stream carried
`llm_call {costUsd: 0.5, usdKnown: false}` then a terminal
`done {totalCostUsd: 1.5}` (the ledger credits the $1.00 residual as a
provider receipt) settled `{usd: 1.5, usdKnown: false, usdEstimated: 1.5}`.
It now settles `usdEstimated: 0.5`, so the subtraction returns the $1.00
the receipt states.

The unproven part is summed where the per-call fact exists, in the loop
ledger that already accumulates `costUsd`, and carried on `Iteration` and
`LoopResult` beside it. This is the partition `spendFromUsageEvents` and
`bridge-executor` already apply per event; the leaf was the one settlement
path that folded first and classified afterwards.

Nothing is promoted: `usdKnown` is untouched, and budget admission reads
only `usd`/`usdKnown`, so a dollar-capped run admits and spends exactly
what it did before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`runDirectoryHolderIsLive` reads the lock file outside the mutation guard
that `acquireRunDirectoryLock` evaluates the identical rule under, so its
answer can be stale by the time a caller acts on it. The docstring named a
supervisor picking up an abandoned directory as a caller without saying
that such a caller must still acquire, which invites acting on a `false`
that a concurrent acquire has already overtaken.

Behaviour is unchanged; the function is for reporting and listing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`leakedReservations` was asserted only on the in-memory result, so nothing
proved the finding outlives the process that made it. #1181's symptom was
a persisted `failure.json` reporting `nodes: 0` with no settle record at
all, which makes the durable artifact the one that mattered.

The case runs the settled result through `settleRecordJson` — the same
serializer `result.json` is written with, which refuses any value JSON
would misstate — and asserts the leak and its assignment survive.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drewstone
drewstone force-pushed the fix/unproven-dollar-partition-20260911 branch from 4a3cdcc to 91b1d82 Compare September 11, 2026 18:49
@drewstone

Copy link
Copy Markdown
Contributor Author

Rebased onto main (was based on 0.211.0, four releases behind) and prepared as 0.216.0 with the CHANGELOG entry.

Note on local test signal: this branch shows 22 failures when run on the dev host, but unmodified origin/main shows 18 failures in the same worktree on the same machine, at load average 327 with four concurrent research runs. The failures are scattered across credentials, optimizer evidence, catalog pins, SIGKILL recovery and task manifests — no coherent relationship to a cost-accounting change. I am treating the box as unable to give a clean signal and deferring to CI, which was green on this branch before the rebase.

@drewstone

Copy link
Copy Markdown
Contributor Author

Verified locally, not by CI.

Serialized (--fileParallelism=false), same worktree, same machine, back to back:

failures
this branch @ 91b1d82 3
unmodified origin/main 3, the same three

tests/kernel/supervise-live-recovery.test.tsstarts the resumed parent…, preserves accepted child failure=false…, preserves accepted child failure=true… — is broken on main independently of this change.

The earlier 22-vs-18 full-suite numbers were parallelism under load average 327: tests/improvement-cycle.test.ts and tests/kernel/supervise-restart-resource-safety.test.ts both pass when the run is serialized. Only this one file fails either way.

Merging on that evidence.

@drewstone
drewstone merged commit 3906b41 into main Sep 11, 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.

1 participant