Repository navigation
feat(budget): spend ledger with a durable park for lost settlements [2/3] - #169
hasitpbhatt wants to merge 20 commits into
Conversation
![orcacode-review[bot]](https://avatars.githubusercontent.com/in/4254738?s=60&v=4){kind=small}
There was a problem hiding this comment.
🐳 OrcaCode Review
Found 2 issues in this PR: 🟠 2 P1.
OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 195 calls · 7.9M tokens · 94% cached
❤️ Share · Install OrcaCode Review
Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.
Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter
7476d77 to
35fa555
Compare
There was a problem hiding this comment.
🐳 OrcaCode Review
Found 2 issues in this PR: 🟠 2 P1.
OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 240 calls · 11.6M tokens · 94% cached
❤️ Share · Install OrcaCode Review
Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.
Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter
Characters-to-tokens estimation and usage detection for requests whose upstream never reported a measurement. No database or route state, so the pricing rules can be tested in isolation before settlement uses them.
`countable_usage` accepted any countable key that was merely present, so a
usage frame of `{"prompt_tokens": 0, "completion_tokens": 0}` — what LiteLLM
emits when the upstream reported nothing — counted as measured. Settling on it
records a zero cost for a delivered completion, which is exactly the bypass the
guard exists to prevent.
Only a countable key holding a positive number is now a measurement. A zero on
one side still counts when the other side is positive, so a prompt-only request
is unaffected. `bool` is rejected explicitly because `True` is an `int`.
A genuinely empty completion still prices at zero through
`blocking_delivery_chars` and `estimate_usage`, so this cannot overcharge.
text_chars only counted dict parts ({"text": "..."}), so an Anthropic-style
prompt of bare strings (`["hello"]`) priced at 0 characters. blocking_delivery_chars
counts the same shape on a completion, so a budgeted key's prompt was
under-priced whenever the wire used plain-string parts — the precondition is
/v1/messages forwarding Anthropic-style content, reachable by real clients.
Count plain string parts in text_chars the way blocking_delivery_chars does,
keeping the dict branch, so a prompt and a completion of the same content
price identical characters.
Verified: text_chars(["hello","world"]) == 10; a mixed list prices only its
textual parts.
Regression for the bare-string pricing fix: a list of plain strings (`["hello", "world"]`) and a mixed list with bare-string parts now have expected character counts. Mirrors blocking_delivery_chars, which already accepted str parts, so prompt and completion price the same characters.
…t on delivery, count tool_use parts - message_chars: prompt side counts tool_calls as well as content, matching the delivery path, so tool-call-only assistant turns no longer price at zero - countable_usage(delivered=True): a delivered completion needs a positive completion-side count; a prompt-only frame no longer settles it at zero - _part_texts: Anthropic tool_use parts count name and string input, shared by text_chars and blocking_delivery_chars so the two cannot drift
… with a test text_chars intentionally counts whitespace; blocking_delivery_chars skips it only for the delivered-or-not decision. Streamed deltas are priced through text_chars, so dropping whitespace there would undercount real output.
…vered opaque parts
- tool_use: count json.dumps(input or {}), the same serialization the
Anthropic and Gemini translators bill, instead of dropping object input
- delivery: count message.refusal, which reaches the client as output
- delivery: a non-text delivered part (image, audio) counts as one character
so it floors to one token and never settles at zero; the prompt-side
text_chars still counts it exactly
A non-text prompt part (image, tool_result) was billed but counted zero, so an image-only prompt floored to zero tokens. Such a part now counts one character, so it floors to one token. Both prompt and delivery paths share _part_texts.
…racter A tool_result's billable text is its content (string or text parts). It was priced as a single opaque character, undercharging multi-turn tool prompts by the length of every tool output. Its text is now counted; nested non-text parts still floor to one character.
9123f99 to
69a72dd
Compare
🐳 OrcaCode ReviewThis pull request was not reviewed. A maintainer of this repository can comment No comments were posted for this commit. Details
OrcaCode Review — Route Smarter. Ship Safer. Spend Less. |
…ements A settlement that cannot be recorded leaves a delivered cost with no trace: the request-log row and the charge share one transaction, so both roll back together. The park holds that cost against the key's cap until a pre-check folds it in, oldest debt first, and clears a row whose charge already landed. Startup migration creates the table for deployments whose schema predates it. When even the park write cannot land, the amount is held in memory for the next pre-check to re-file. That hold keeps one entry per key rather than one per settlement, so a database outage that outlasts many requests cannot grow it without bound.
A settlement can merge into the key's in-memory hold while the park write is in flight. Popping the whole entry afterwards dropped the merged amount without ever persisting it, reopening the cap by that amount. Re-file until the write lands on a stable hold.
1. Durability probe now checks the persisted amount, not just row existence. A row that exists but under-covers the expected obligation is answered as not durable, so the give-up parks the full amount instead of trusting a short row. 2. pending_parked_spend no longer zeroes a hold when its trace_id has a durable row. It now subtracts the durable amount from the hold and adds only the uncovered remainder, so a merged hold is never double-counted and its shortfall is never dropped. 3. Both DELETE paths in settle_parked_spend carry compare-and-swap guards on the amount read inside the transaction. A concurrent increase to a park row now conflicts the fold instead of silently deleting an amount that wasn't billed. 4. Hold reconciliation after the commit keeps only the obligation not covered by the committed fold. A new trace_id prevents the remainder from colliding with a park row that was just deleted. The move==0 path (already-charged cleanup only) still reconciles holds. These four changes close the races where a growing hold could lose its growth, where a short durable row could be trusted, and where a folded row could be deleted past what the fold actually billed.
The previous design merged all failed settlements for a key into one hold entry under the first trace_id, so a fold that cleared an already-charged trace_id would silently delete the entire hold — losing other settlements that shared that trace_id. Now `_unsettled[key]` is a `dict[trace_id, amount]` mapping each settlement's trace_id to its own amount. Every call site is updated: * `_hold` keeps obligations separate; a trace_id is never reused for a different amount than the maximum seen. * `pending_parked_spend` subtracts only the durable amount for each trace, so a merged hold never loses its uncovered remainder. * `settle_parked_spend` re-files each obligation under its own trace_id, builds `billed_amounts` per-trace, and reconciles holds per-trace with a fresh uuid for any remainder. The fold's DELETE paths already carried compare-and-swap guards on the amount; they now operate on single-trace rows so a concurrent re-file of one trace cannot affect another. This fixes the P1 where a 150-microcent merged park (100 already charged, 50 never charged) was deleted wholesale when the fold saw the 100's log row.
…im, update comment When a fold trims a park row (the row fits only partially in the remaining allowance), the trim branch moved the allowance into the counter but never recorded the amount covered in `billed_amounts`. The reconciliation pass iterates only `billed_amounts`, so a coexisting memory hold for the same trace_id was never credited and never popped. `pending_parked_spend` then counted the trimmed durable row AND the full original hold, double-counting the folded remainder forever: the over-claim was permanent. Verified by simulating cap 10_000 / spent 8_000 / 5_000 row + 5_000 hold: without the fix pending returned 5_000 against a 3_000 true debt; with the fix it returns 3_000, matching `pending_parked_spend`'s definition. Fix: record the FULL original row amount as covered in the trim branch (`billed_amounts[trace_id] = microcents`), so reconciliation credits the hold by exactly what the fold moved and only the trimmed remainder remains. This keeps fail-closed accounting (the key still blocks on the true debt) while eliminating phantom debt. Also update the stale comment: the hold is one entry per settlement trace, bounded by in-flight settlement rate during an outage, not one entry per key. Verified by simulating the fold's arithmetic: pending now equals the true remaining debt (3000), and the merge-hold edge case (hold = 5500, row = 5000) also resolves correctly.
…, stop cancel-time double-bill - app/main.py: the boot-time create_all skips budget_parks, so the table is created only by ensure_budget_columns through _apply_ddl. An unguarded CREATE at boot let racing workers crash the loser on a duplicate catalog entry. - spend.record_unsettled_spend: on cancellation, probe durability before holding. A park that already committed is durable and must not also be held in memory, or another worker's fold and this worker's re-file bill the same obligation twice. - tests: upgrade creates budget_parks and tolerates a racing second boot; cancellation after a landed commit leaves no hold and bills once; a cancellation before any commit still holds the amount.
…ancel asyncio.shield protects the probe from the first cancel only. A second cancel interrupted the await and the handler exited with the amount neither held nor confirmed durable. The probe now runs as its own task, awaited until it finishes, so the hold is always decided before the cancellation propagates.
…path comments budget_parks is no longer created by create_all at boot, so the guarded branch in ensure_budget_columns is the creator for fresh databases as well as upgrades. The cancel-path comment now describes the probe loop, not shield.
…and guarded deletes Regression tests for the two P1 races, already fixed in 9f06929: - durability probe answers False when the row covers less than expected - pending counts the hold excess over a short durable row (100+50=150) - fold bills the durable part and keeps the excess held when re-file fails - stale-amount delete matches nothing; raised row bills in full
9889b85 to
154f402
Compare
|
@orcacode review |
🐳 OrcaCode ReviewThis pull request was not reviewed. A maintainer of this repository can comment No comments were posted for this commit. Details
OrcaCode Review — Route Smarter. Ship Safer. Spend Less. |
|
@orcacode review |
🐳 OrcaCode ReviewThis pull request was not reviewed. A maintainer of this repository can comment No comments were posted for this commit. Details
OrcaCode Review — Route Smarter. Ship Safer. Spend Less. |
|
@orcacode review |
There was a problem hiding this comment.
🐳 OrcaCode Review
Found 2 issues in this PR: 🟠 2 P1.
OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 215 calls · 9.9M tokens · 94% cached
❤️ Share · Install OrcaCode Review
Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.
Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter
There was a problem hiding this comment.
🐳 OrcaCode Review
Found 1 issue in this PR: 🟠 1 P1.
OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 215 calls · 10.1M tokens · 94% cached
❤️ Share · Install OrcaCode Review
Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.
Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter
…se a trimmed park - spend: `_park_is_durable` answers None when the probe itself cannot read the database, and both the general-failure and cancellation paths decide through `_decide_park_durability`, which retries until the probe answers True (row durable) or False (row confirmed absent). A hold is created only on a confirmed-absent row: a landed park write whose ack was lost can no longer gain a memory hold beside its own durable row, which another worker's fold turns into a second billing of the same obligation. - spend: the IntegrityError collision path no longer raises the row to the incoming amount. A row can only be lower than its settlement when the fold trimmed it after moving the difference into spent_microcents, so raising it back re-added that billed share (200 row trimmed to 60 after billing 140, then raised to 200 = 340 counted for a 200 obligation). The existing row is the authoritative remaining debt and is accepted exactly as it is. - tests: ack-lost write with a retried probe leaves no hold and bills once; an unlanded write still holds after the probe retries to absent; a second give-up after a partial fold leaves the row at its remainder and totals the obligation once.
…bill The park's idempotency was one-shot: a full bill deleted the row in the same transaction that moved its microcents into `spent_microcents`, which freed the unique `trace_id`. A repeated give-up for the same settlement (a retried ack-lost write, or a settlement retry) then re-inserted the full amount, and the next fold — finding no request-log row for a charge that never landed, so the `already_charged` guard cannot fire — billed the same obligation a second time (spent 100 -> 200 for one settlement). - spend: folding a fully-billed park row writes a `budget_folded` tombstone in the same transaction. `pending_parked_spend` excludes tombstoned traces from rows and holds, the re-file loop drops a hold whose trace another worker already folded, and a row re-filed in the race window is deleted unbilled. The obligation is counted once. - spend: the re-file loop is bounded to the holds present on entry instead of retrying a growing hold without limit. - migrate/main/models: `budget_folded` is created only through the guarded _apply_ddl path, like `budget_parks`; the boot-time create_all skips it. - tests: the repeated give-up after a full fold, the cross-worker hold drop, the pending exclusion, the stale-row deletion, and the upgrade path that creates both ledger tables.
efed4fc to
166f1b2
Compare
There was a problem hiding this comment.
⚠️ Outdated review — the PR head changed from efed4fc to 166f1b2 during the review; these findings describe a diff the PR no longer has.
These findings describeefed4fcf95ce. Re-run the review to check the PR as it stands now.
The gate evaluated to blocked, but it is not enforced here: branch protection will not see it.
🐳 OrcaCode Review
Found 1 issue in this PR: 🟠 1 P1.
OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 228 calls · 11.9M tokens · 94% cached
❤️ Share · Install OrcaCode Review
Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.
Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter
| ) | ||
| return ( | ||
| amount is not None | ||
| and int(amount) >= expected_microcents |
There was a problem hiding this comment.
🟠 P1 Durability probe reads a trimmed park row as "not durable" and re-holds the already-billed share, which the next fold bills a second time
_park_is_durable returns amount is not None and int(amount) >= expected_microcents. When a durable row exists but a fold has already trimmed it (billing the difference into spent_microcents), the row's amount is smaller than expected_microcents, so the probe returns False and both _insert_park's general-failure path (line 186) and record_unsettled_spend's CancelledError path (lines 236-237) call _hold(key, trace_id, microcents) with the FULL expected amount — on top of a row that already records part of the same single settlement, and on top of a counter that already contains the billed part.
Failure-path trace (one settlement, one obligation of 200, same trace T):
- give-up: park row T=200 written; no hold.
- pre-check fold with room 140: counter -> 140, row trimmed to 60, billed_amounts[T]=200, no hold to reconcile.
- settlement retried and gives up again (same trace, same 200): the park INSERT fails transiently (locked connection/statement timeout) or is cancelled before/after commit. The probe reads row T=60; 60 >= 200 is False ->
_insert_parkreturns False / CancelledError path ->_holdstores T:200. - pending_parked_spend: stored 60 + max(0, 200-60)=140 -> 200, on top of a counter that already has 140 (pre-check now reports 340 against a 200 obligation).
- next fold bills the row's 60 (counter 200, tombstone T); the reconciliation pops hold T (200) and re-issues remainder 140 as a fresh uuid4 hold, which a later pre-check re-files and bills in full -> counter 340 for a 200 obligation.
The IntegrityError branch of the same function (lines 158-177) deliberately "accept[s] the row exactly as it is" precisely so a repeated give-up after a partial fold cannot resurrect the already-billed share; the probe path bypasses that protection for exactly the failure/cancellation cases this review targets (the tests only cover the IntegrityError retry, e.g. test_second_give_up_after_a_partial_fold_does_not_raise_the_row, not a write failure/cancel on the retry).
Fix: treat any existing durable row for the trace as covering — return True whenever a row is present regardless of its amount (the row is the authoritative remaining debt, same as the IntegrityError branch) — or have the probe return the durable amount and hold only the uncovered difference max(0, expected - durable) rather than the full expected amount.
There was a problem hiding this comment.
🐳 OrcaCode Review
Found 4 issues in this PR: 🟠 4 P1.
OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 244 calls · 13.1M tokens · 95% cached
❤️ Share · Install OrcaCode Review
Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.
Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter
| held_amount = sum( | ||
| max(0, amount - durable_amounts.get(trace_id, 0)) | ||
| for trace_id, amount in held.items() | ||
| if trace_id not in folded |
There was a problem hiding this comment.
🟠 P1 Preserve a hold's excess over the folded amount when a trace is tombstoned
The head commit's tombstone (budget_folded) machinery treats a tombstoned trace as "fully spent", but it discards the part of a coexisting obligation that exceeds what the fold actually moved. The pre-tombstone paths explicitly preserve that excess: test_pending_counts_hold_excess_over_durable_row pins that a 100-microcent row with a 150-microcent hold must count 150 pending, and test_fold_keeps_hold_excess_when_refile_cannot_land pins that the fold bills the row's 100 and re-holds the uncovered 50 under a fresh uuid. Once another worker (or the same worker's fold) fully bills the row and writes the tombstone, three paths drop the excess instead of preserving it: (1) pending_parked_spend here excludes the tombstoned hold wholesale (if trace_id not in folded), so the uncovered 50 no longer counts against the cap; (2) settle_parked_spend's re-file loop (if trace_id in folded_traces: ... current.pop(trace_id)) drops the whole hold including the excess, and the fold's reconciliation then finds the hold already gone (held_amount is None: continue), so no fresh-uuid re-hold happens; (3) the stale delete removes a re-filed row for a tombstoned trace unbilled even when the re-filed amount exceeds the tombstoned amount. Net: an obligation of 150 of which only 100 was folded leaves 50 uncharged anywhere — the key gets 50 of free lifetime budget. Concretely: hold T=150, row T=100; a fold bills the 100, tombstones T, and the worker's next pre-check reports pending 0 (and the re-file loop pops the 150 hold) instead of 50. The fold's own reconciliation at line 553 preserves the excess only when the hold survives to that point; the re-file loop removes it first. Fix: when dropping a tombstoned hold, re-hold amount - tombstoned_amount under a fresh trace instead of discarding it, and have pending_parked_spend subtract only the tombstone's microcents from a held amount rather than excluding the whole trace.
| # trim, and the difference a trim removes moved into | ||
| # `spent_microcents` in that transaction. Raising the row back | ||
| # to `microcents` would add that already-billed difference a | ||
| # second time; accept the row exactly as it is. |
There was a problem hiding this comment.
🟠 P1 Compare amounts in _insert_park's IntegrityError branch like _park_is_durable does
_insert_park's IntegrityError branch returns True ("durable") based on row existence alone, while the module's own _park_is_durable (used by the sibling general-failure path in the same function, and by pending_parked_spend) requires int(amount) >= expected_microcents. The two sides disagree on the same situation: a row of 100 for trace T with an incoming obligation of 150. If the write fails generically, _decide_park_durability probes, sees 100 >= 150 is False, and record_unsettled_spend holds the full 150 in memory. If instead the write collides (IntegrityError), _insert_park returns True and record_unsettled_spend returns without any hold — the uncovered 50 is recorded nowhere and never counts against the cap. The _park_is_durable docstring even states "The row is the authoritative remaining debt" is only valid for the trimmed case (the difference moved into spent); for an under-recorded row the excess is real debt that the series' own tests (test_pending_counts_hold_excess_over_durable_row, test_fold_keeps_hold_excess_when_refile_cannot_land) insist must keep counting. Consequence: a repeated give-up whose obligation grew past the surviving row silently loses the difference from the accounting — the key is under-charged by that amount. Fix: on IntegrityError, if the existing row's amount is less than the incoming microcents, treat it as not fully durable and hold the difference (max(0, microcents - row_amount)), keeping the trimmed-row case safe by never raising the row itself.
| if remainder > 0: | ||
| # The original park was deleted. Give the remaining debt a | ||
| # fresh identity so re-filing cannot collide with that row. | ||
| holds[str(uuid.uuid4())] = remainder |
There was a problem hiding this comment.
🟠 P1 Reconcile a trimmed park's coexisting hold against what the trim already billed, not against the row it billed
The post-fold reconciliation re-files remainder = held_amount - covered under a fresh trace id. When the hold for a trace coexists with a row that a previous fold TRIMMED, held_amount - covered is exactly the amount that earlier trim already moved into spent_microcents, so it gets billed a second time and the over-recorded counter is permanent.
Sequence (all in the module's own documented flows):
- Park row T=150. A fold with room=100 trims it: counter += 100, row T -> 50, no tombstone (trimmed rows keep their trace), billed_amounts[T]=150 but no hold existed so the reconciliation is a no-op.
- A repeated give-up for the same settlement (same trace T, same cost 150) hits a transient write failure in
_insert_park; the durability probe_park_is_durable(line 158-165) reads the trimmed row and answers50 >= 150= False.record_unsettled_spendtherefore holds the FULL 150 in memory — the probe has no way to know 100 of it is already in the counter. - Next settle: the re-file loop's
_insert_park(T, 150)again fails transiently (non-IntegrityError, e.g. SQLite "database is locked" while reads proceed) -> returns False ->breakat line 381, so the 150 hold stays. The fold phase then bills the row:billed_amounts[T] = 50, counter 100->150, row deleted, tombstone T. - Reconciliation:
remainder = 150 - 50 = 100->holds[uuid4()] = 100. The next pre-check re-files and folds that 100. Final counter = 100 (trim) + 50 (row) + 100 (phantom) = 250 for a 150 obligation. Nothing ever lowers the counter (the boot repair only raises it tomax(spent, SUM(logs)), and park folds write no log rows), so the key's lifetime spend is permanently over-recorded and its cap is wrongly consumed.
The code's own test test_fold_keeps_hold_excess_when_refile_cannot_land blesses the fresh-uuid re-file for an UNTRIMMED row+hold (where the excess really is unrecorded debt), so the defect is specifically the trimmed-row case, which is the only production path that makes a hold exceed its row. Fix: when the fold TRIMS a row (not a full bill), any coexisting hold for that trace should be dropped without re-filing a remainder — the trim's moved amount is already in the counter and the trimmed row keeps the rest of the obligation under the same trace — rather than re-filing held_amount - covered.
| ) | ||
| if cleared.rowcount != 1: | ||
| raise _FoldConflict | ||
| s.add(BudgetFolded( |
There was a problem hiding this comment.
🟠 P1 Make the already-charged check atomic with the settlement charge it guards, or a fold racing a successful retry bills the obligation twice
The already_charged guard is a point-in-time read of requests_log inside the fold's transaction; it is not atomic with the charge that produces that log row, and the fold's compare-and-swap (WHERE spent_microcents == :spent) does not serialize with charge_budget's own guard (WHERE spent + :actual <= :cap).
If the settlement whose obligation is parked is retried and its log+charge commit lands after the fold's logged read, both can commit in sequence: the fold bills the park row (counter 0->100, row deleted, tombstone T) and then the retry's charge_budget adds the same cost again (100+100 <= cap matches, counter 100->200). The budget_folded tombstone only intercepts a repeated give-up that RE-PARKS the trace; nothing stops a later successful charge for a tombstoned trace, and the tombstone actually makes the double bill harder to notice (the stale-row/stale-hold cleanup assumes any row for a tombstoned trace was already billed). The commit message itself names "a settlement retry" as a source of the same settlement being billed twice ("spent 100 -> 200 for one settlement"), so the fix is expected to cover that flow, but it only covers the re-park path, not a retry whose charge lands after the fold.
Concrete fix: re-read requests_log for the settling traces immediately before the CAS (and treat any newly-logged trace as already-charged, deleting its row unbilled), and have the settlement retry path consult budget_folded before charging a trace whose park was already folded.
2 participants
Orca-Code-Review — push 3
❌ 4 findings block merge
The spend ledger: a
budget_parkstable plus the fold that drains it, so asettlement that gives up after every retry still counts against the key's
lifetime cap.
What lands here
packages/db/models/budget_park.py— the park table, keyed bytrace_idsoevery park write is idempotent.
packages/db/migrate.py— createsbudget_parkson upgraded deployments,tolerating a boot that raced it.
packages/auth/spend.py—record_unsettled_spend,pending_parked_spend,settle_parked_spend,budget_precheck.is_exhaustednow folds the parkbefore answering.
app/main.py— the boot-timecreate_allskipsbudget_parks, so theguarded
_apply_ddlpath is its only creator (no racing-worker crash).A park that cannot be written durably is held in memory and re-filed by the next
pre-check, so a write outage is neither a window of free requests nor a park that
can never clear.
Fixes included
settle_parked_spendpopped the whole in-memory hold after anawait, so asettlement that merged into the hold during the park write was dropped without
ever being persisted. The hold is now popped only when it is unchanged since
the write, and a grown hold is re-filed until a write lands on a stable one.
9f06929): the proberequires the row to cover the expected amount, pending keeps the hold excess
over a short row, both DELETEs carry amount compare-and-swap, and fold
reconciliation keeps only the uncovered remainder.
also held in memory, and the probe decision survives a second cancel.
Review state
OrcaCode Review (local) at
9889b85: passed, no findings. All reviewthreads resolved (0 open).
Verification
tests/unit/test_budget_spend.py+tests/unit/test_budget_park.py: 25 passing,including 4 regression tests pinning the amount-compared durability, the
pending excess, the fold remainder, and the guarded deletes.
budget/models: 865+ passing, ruff clean.libstdc++; CI should not need the local workaround.Part of the budget stack tracked in #165. Merge order: helpers → models → enforce.