Repository navigation
Budget stack merge order (helpers → models → enforce) #165
Description
Tracks the budget stack that puts ApiKey.budget_limit_cents into the request path.
Merge order
budget/helpers → budget/models → budget/enforce
| PR | Branch | Lands |
|---|---|---|
| #168 | budget/helpers |
pure delivery-pricing helpers (app/budget_pricing.py) — no request-path change |
| #169 | budget/models |
spend ledger + durable park (budget_parks table, fold, pre-check) |
| #170 | budget/enforce |
wires the cap into execute_chat |
Each branch is based on the one above it, so they merge in order and each is
independently reviewable.
Why this ordering
The adapter-fault settlement must land in the same PR as enforcement. An
earlier attempt at this stack split the two, and the result was that a
server-side adapter fault — not a client choice — would charge a budgeted key its
entire remaining lifetime budget. Measured at 100% of cap on a request that
delivered a handful of tokens. That stack was closed unmerged.
In this stack the settlement for an adapter fault lives inside the enforce
layer: _unmeasured_charge prices a _STREAM_ADAPTER_ERROR ending from what
actually reached the client. There is no segment that has to land before
enforcement does, so the three layers merge in one linear order with no ordering
constraint.
Worth knowing if this stack is ever split again: do not separate the
adapter-fault settlement from the pre-check that dispatches.
Status
- schema, migration and
packages.auth.spendlibrary — already onmain
(ec70055). Inert: no request path changed. - feat(budget): add pure delivery pricing helpers [1/3] #168
budget/helpers— pure pricing helpers - feat(budget): spend ledger with a durable park for lost settlements [2/3] #169
budget/models— spend ledger + durable park - feat(budget): enforce the lifetime cap in the chat request path [3/3] #170
budget/enforce— request-path enforcement
What was fixed during review
| Layer | Finding |
|---|---|
budget/models |
settle_parked_spend popped the whole in-memory hold after an await, so a settlement merged into the hold during the park write was dropped without being persisted — reopening the cap by that amount |
budget/enforce |
a cancellation after a late commit was swallowed (return instead of raise) |
budget/enforce |
the completed-without-usage branch charged the whole remaining cap for every model, including known-free ones |
All three are fixed and the layers were re-reviewed afterwards.
Trade-offs accepted deliberately
- Fail-closed double-count. When a commit lands but its ack is lost and the
durability probe also cannot read, the obligation is parked and the key is
billed twice. The alternative — trusting an unreadable database — reopens the
cap for exactly the key the park exists to protect. Over-charging is bounded by
the lifetime cap and visible in the park ledger; under-charging lets delivered
spend through uncounted, silently, for as long as the outage lasts. Recorded in
the commit that introduced it, with both options spelled out. - Measured cost can be raised to the estimate on a no-usage path
(max(actual, charge)). Deliberate never-under-charge. budget_limit_cents=0rejects every request, including free models.
Verification gap
No tests have been executed. The environment these branches were written in
cannot load SQLAlchemy's async stack, so the suite has never run against them.
py_compile passes and a local review gate is clean; neither is a substitute.
CI must run the tests before any of these merge.
Still open
- The park-blocks-dispatch test parks its obligation directly through
record_unsettled_spendrather than arriving there via a failed settlement.
It proves the pre-check reads the park, but not the full
fail-settlement → park → reject sequence end to end. - P2 cancellation paths flagged during review: a give-up during a cancelled
last-chance commit, and the blocking-path commit during a client disconnect.
Activity
xizhuomengcontin commentedon Oct 6, 2026
Status update on the stack: #160 is merged (
ec70055), and that has put the other three into conflict. Details below so the rebases can be done in one pass.#160 — merged, and push 4 verified
I ran the stale-seed fix end-to-end rather than taking the formula on trust.
spent = MIN(MAX(spent, SUM(logs)), cap)holds exactly:spent=0,logs=15→ 15spent=15,logs=25→ 25 — the oldWHERE spent_microcents = 0gate stayed at 15spent=500000>logs=25→ stays 500000logs=2,000,025> cap → 1,000,000budget_limit_cents=1withspent=999999999→ HTTP 200, cap not enforcedPlus
ruffclean, 808 passed, and my cross-cutting regression battery 15/15 identical tomain.Two operational checks, since this one adds a startup migration:
requests_logrows, with and without a capped key: 5.6–6.2s after vs 6.2–7.2s before — all inside the noise of Python import time. With no capped key the seedUPDATEis a no-op (WHERE budget_limit_cents IS NOT NULL).ec70055, ran the pre-merge code (31281db) against it — boots, serves, analytics intact — then switched back. The new columns are additive and nullable, so backing the code out needs no DB work.All three remaining PRs now conflict with
main#160's push 4 rewrotepackages/auth/spend.pyandpackages/db/migrate.py; the stack branches still carry their own older copies.packages/auth/spend.py,packages/db/migrate.py,tests/unit/test_budget_migration.py,tests/unit/test_budget_spend.pypackages/auth/spend.pyAll three need a rebase onto
ec70055. Worth doing #163 first since it is next in the order here — and note it still has to absorb #162's6f8a2a4as you flagged, so that re-sequencing and this rebase are the same piece of work.Remaining blockers per PR
#163 (next to land)
packages/auth/spend.py:403(re-read the spend counter on a fresh transaction after the fold) andapp/routes/chat.py:1458(fail-closed charge discards the authoritative known cost on a blocking response with no usage dict)#162
6aebfc5c, 10-06T03:09): 2 P1s,chat.py:1422(gate the no-usage fail-closed arm on delivered tokens) andchat.py:999(settle fail-closed charges from the raw usage dict, not only prompt/completion tokens)#161
8fbf2450): 1 P1 — "Settle the stream in the AdapterError branch so a budgeted key is not charged its entire remaining allowance"On that #161 finding — my measurement agrees with the bot
I reproduced it independently on 09-30, before the bot raised it on this head. Injecting a fault into the Anthropic transformer mid-stream (so
finish_quietly(failed=True)throwsAdapterErrorinto the engine), on a key withbudget_limit_cents=100:3 of 3 runs, on a request that delivered a handful of tokens. The same harness on #162 and #163 gives
spent = 5, i.e. 0.0% — the fix is9b82eb2 price adapter-fault delivery instead of charging remaining budget, which sits in the segment both of those share.That is the concrete reason the merge order matters: landing #161 before the segment containing
9b82eb2puts a live budget-wipe onmain, and it is a server-side fault wiping a customer's lifetime allowance. Your ordering (#163 → #162 → #161) already avoids it; I am restating the measurement so the constraint has data behind it rather than only the banner text.Happy to re-verify each one end-to-end as it becomes mergeable — the budget harness is reusable (cap enforcement via 429, adapter-fault settlement, migration seed across restarts, concurrent fail-closed rows). For reference, the full stack merged locally earlier passed
ruff, 854 tests, regression 15/15, and behaved correctly on every budget path I could drive.[-]Budget stack merge order (#160 → #163 → #162 → #161)[/-][+]Budget stack merge order (helpers → models → enforce)[/+]