From 16bfdb4f028e27282b802900e853e6b4ba3a29ed Mon Sep 17 00:00:00 2001 From: Arik Levinsky Date: Mon, 20 Jul 2026 11:41:00 -0600 Subject: [PATCH 1/2] fix(pricing): deterministic price tiebreak; admit a zero-cost reserve with a zero hold (#98, #104) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #98: resolve_price ordered only by published_at DESC, but published_at defaults to the transaction timestamp, so a bulk seed/migration publishes several versions at the same instant and the "current" pick was arbitrary and could vary run to run. The resolved version is stamped on the reservation and drives commit reconciliation, so an ambiguous pick silently shifts the cost basis. Add version DESC as a deterministic tiebreak so "latest published" is a total order. #104: reserve raised NonPositiveEstimate (422) when the worst-case estimate rounded to 0 — which, because ceil_micro keeps any positive worst case at >= 1 micro, only happens for a zero-token request or a genuinely free (zero-priced) model. A free model could therefore never transit the gate. Admit a zero estimate with a zero-micro hold instead, so free and paid models route through the same guard; the reserve walks every node at amount 0 (always succeeds on a node with any non-negative headroom) and commit reconciles to 0. NonPositiveEstimate was raised only by reserve, so it is now dead — an error the server can never emit, the same vestige pattern flagged in #96/#100. Removed the type and its api/sdk mappings (the SDK's 422 fallback already resolves to InvalidRequest, so client behavior is unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BjJ4QScUKbrMzF1jujL5Bg --- .../adapters/integrations/sdk/errors.py | 1 - .../adapters/postgres/price_book_repo.py | 15 ++++++---- src/tollgate/api/errors.py | 2 -- src/tollgate/application/handlers/reserve.py | 9 +++--- src/tollgate/domain/errors.py | 10 ------- tests/integration/test_price_book_repo.py | 30 +++++++++++++++++++ tests/unit/test_api_errors.py | 2 -- tests/unit/test_errors.py | 2 -- tests/unit/test_reserve.py | 24 ++++++++------- 9 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/tollgate/adapters/integrations/sdk/errors.py b/src/tollgate/adapters/integrations/sdk/errors.py index eed6781..a98c69d 100644 --- a/src/tollgate/adapters/integrations/sdk/errors.py +++ b/src/tollgate/adapters/integrations/sdk/errors.py @@ -73,7 +73,6 @@ class InternalError(TollgateApiError): "reservation_not_held": ReservationNotHeld, "unknown_model": UnknownModel, "amount_out_of_range": InvalidRequest, - "non_positive_estimate": InvalidRequest, "enforcement_unavailable": EnforcementUnavailable, "internal_error": InternalError, "conflicting_budget_scope": InternalError, diff --git a/src/tollgate/adapters/postgres/price_book_repo.py b/src/tollgate/adapters/postgres/price_book_repo.py index 7547631..0ed43f5 100644 --- a/src/tollgate/adapters/postgres/price_book_repo.py +++ b/src/tollgate/adapters/postgres/price_book_repo.py @@ -1,9 +1,10 @@ """PostgresPriceBookRepository: resolve the current price for a (provider, model) (§3, ADR 0028). -The current price-book version is the one with the latest ``published_at`` (ADR 0028). A single -join from ``price`` to ``price_book``, filtered to the pair and ordered by ``published_at`` -descending, takes that version and its rates — or returns ``None`` when the pair is unpriced, which -the reserve turns into an ``UnknownModel`` denial. Explicit async SQLAlchemy Core, no ORM; like the +The current price-book version is the one with the latest ``published_at`` (ADR 0028), with +``version`` as a deterministic tiebreak when several versions share a ``published_at`` (#98). A +single join from ``price`` to ``price_book``, filtered to the pair and ordered by that total +order descending, takes the current version and its rates — or returns ``None`` when the pair is +unpriced, which the reserve turns into an ``UnknownModel`` denial. Explicit async SQLAlchemy Core, no ORM; like the other repositories it never imports ``application`` and satisfies the port structurally. """ @@ -39,7 +40,11 @@ async def resolve_price(self, provider: str, model: str) -> PricedModel | None: price.join(price_book, price.c.price_book_version == price_book.c.version) ) .where(price.c.provider == provider, price.c.model == model) - .order_by(price_book.c.published_at.desc()) + # version is a deterministic tiebreak: published_at defaults to the transaction + # timestamp, so a bulk seed publishes several versions at the same instant and + # published_at alone leaves an arbitrary, run-varying pick — but the resolved + # version is stamped on the reservation and drives commit reconciliation (#98). + .order_by(price_book.c.published_at.desc(), price_book.c.version.desc()) .limit(1) ) ).first() diff --git a/src/tollgate/api/errors.py b/src/tollgate/api/errors.py index c25fba3..804f145 100644 --- a/src/tollgate/api/errors.py +++ b/src/tollgate/api/errors.py @@ -26,7 +26,6 @@ EnforcementUnavailable, IdempotencyKeyReuse, InsufficientBudget, - NonPositiveEstimate, ReservationNotHeld, ScopeNotAuthorized, TollgateError, @@ -65,7 +64,6 @@ ReservationNotHeld: (409, "reservation_not_held", "reservation is not held"), UnknownModel: (422, "unknown_model", "unknown (provider, model) pair"), AmountOutOfRange: (422, "amount_out_of_range", "amount out of representable range"), - NonPositiveEstimate: (422, "non_positive_estimate", "reserve estimate must be positive"), BalanceGuardViolation: (500, "balance_guard_violation", "balance guard matched no row"), ConflictingBudgetScope: ( 500, diff --git a/src/tollgate/application/handlers/reserve.py b/src/tollgate/application/handlers/reserve.py index 213d7d2..90b5a1d 100644 --- a/src/tollgate/application/handlers/reserve.py +++ b/src/tollgate/application/handlers/reserve.py @@ -30,7 +30,6 @@ from tollgate.domain.errors import ( IdempotencyKeyReuse, InsufficientBudget, - NonPositiveEstimate, TollgateError, UnknownModel, ) @@ -137,15 +136,15 @@ async def reserve(self, auth: AuthContext, command: ReserveCommand) -> ReserveRe nodes = await resolve_applicable_nodes(tx.budgets, auth, command.project_id) + # A zero estimate (a genuinely free, zero-priced model, or a zero-token request) is + # admitted with a zero-micro hold rather than rejected, so free and paid models transit + # the same guard; the reserve walks every node at amount 0 and commit reconciles to 0 + # (#104). ``estimate_micro`` never returns negative, so estimate >= 0 always holds. estimate = estimate_micro( priced.price, input_bound_tokens=command.input_bound_tokens, max_output_tokens=command.max_output_tokens, ) - if estimate == 0: - raise NonPositiveEstimate( - "worst-case estimate is zero; a reserve must gate a positive amount" - ) now = self._clock.now() period_start = calendar_month_start(now) ttl_deadline = now + timedelta(seconds=self._ttl_seconds) diff --git a/src/tollgate/domain/errors.py b/src/tollgate/domain/errors.py index 450c3a2..7b16dec 100644 --- a/src/tollgate/domain/errors.py +++ b/src/tollgate/domain/errors.py @@ -34,16 +34,6 @@ class BudgetNotFound(TollgateError): """No budget governs the requested scope.""" -class NonPositiveEstimate(TollgateError): - """A reserve's worst-case estimate is zero, so it would gate nothing. - - A budget is only real if a reserve can stop the request that would breach it; an - estimate of zero consumes no headroom and admits a call whose entire cost then books - as unguarded overage. Such a reserve is denied so callers cannot under-declare their - bounds past the gate (§4, ADR 0003 caller-declared-bounds trust model). - """ - - class BalanceGuardViolation(TollgateError): """A guarded balance mutation matched a number of rows other than one. diff --git a/tests/integration/test_price_book_repo.py b/tests/integration/test_price_book_repo.py index a9ef433..ab604f6 100644 --- a/tests/integration/test_price_book_repo.py +++ b/tests/integration/test_price_book_repo.py @@ -62,6 +62,36 @@ async def test_resolve_price_takes_the_latest_published_version(db_conn: AsyncCo assert priced.price.cached_input_micro_per_token == Decimal("1.5") +async def test_resolve_price_breaks_a_published_at_tie_deterministically( + db_conn: AsyncConnection, +) -> None: + # published_at defaults to the transaction timestamp, so a bulk seed publishes several versions + # with the SAME published_at. Ordering by published_at alone then leaves an arbitrary pick that + # can differ run to run; version is the deterministic tiebreak so "latest published" is a total + # order (#98). The stamped version drives commit reconciliation, so it must not be ambiguous. + same_instant = datetime(2026, 6, 1, tzinfo=UTC) + await _publish( + db_conn, + version="v-a", + published_at=same_instant, + input_rate="1", + output_rate="2", + cached_rate="0.5", + ) + await _publish( + db_conn, + version="v-b", + published_at=same_instant, + input_rate="3", + output_rate="4", + cached_rate="1.5", + ) + priced = await PostgresPriceBookRepository(db_conn).resolve_price("anthropic", "claude") + assert priced is not None + assert priced.version == "v-b" # highest version wins the tie, deterministically + assert priced.price.input_micro_per_token == Decimal("3") + + async def test_resolve_price_returns_none_for_an_unpriced_pair(db_conn: AsyncConnection) -> None: await _publish( db_conn, diff --git a/tests/unit/test_api_errors.py b/tests/unit/test_api_errors.py index eee91f9..e923de3 100644 --- a/tests/unit/test_api_errors.py +++ b/tests/unit/test_api_errors.py @@ -17,7 +17,6 @@ EnforcementUnavailable, IdempotencyKeyReuse, InsufficientBudget, - NonPositiveEstimate, ReservationNotHeld, ScopeNotAuthorized, TollgateError, @@ -46,7 +45,6 @@ async def boom() -> None: (ReservationNotHeld(), 409, "reservation_not_held"), (UnknownModel("anthropic", "unpriced"), 422, "unknown_model"), (AmountOutOfRange("amount out of range"), 422, "amount_out_of_range"), - (NonPositiveEstimate(), 422, "non_positive_estimate"), (BalanceGuardViolation(), 500, "balance_guard_violation"), (ConflictingBudgetScope("user", "u1"), 500, "conflicting_budget_scope"), (EnforcementUnavailable(), 503, "enforcement_unavailable"), diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py index 61cc14e..f790c49 100644 --- a/tests/unit/test_errors.py +++ b/tests/unit/test_errors.py @@ -11,7 +11,6 @@ EnforcementUnavailable, IdempotencyKeyReuse, InsufficientBudget, - NonPositiveEstimate, ReservationNotHeld, ScopeNotAuthorized, TollgateError, @@ -31,7 +30,6 @@ def test_every_error_descends_from_base() -> None: AuthenticationFailed, ScopeNotAuthorized, AmountOutOfRange, - NonPositiveEstimate, BalanceGuardViolation, ): assert issubclass(exc, TollgateError) diff --git a/tests/unit/test_reserve.py b/tests/unit/test_reserve.py index 0f5e818..b0d9f27 100644 --- a/tests/unit/test_reserve.py +++ b/tests/unit/test_reserve.py @@ -19,7 +19,6 @@ BudgetNotFound, IdempotencyKeyReuse, InsufficientBudget, - NonPositiveEstimate, ScopeNotAuthorized, UnknownModel, ) @@ -393,16 +392,21 @@ async def test_reserve_denies_insufficient_budget_naming_the_binding_node() -> N assert uow.rolled_back is True -async def test_reserve_denies_a_zero_worst_case_estimate() -> None: - # A reserve whose worst-case estimate is zero gates nothing; deny it before touching - # any balance, and roll back so no idempotency key is cached (#65). +async def test_reserve_admits_a_zero_estimate_with_a_zero_hold() -> None: + # A genuinely free (zero-priced) model or a zero-token request estimates to 0. Rather than 4xx, + # admit it with a zero-micro hold so free and paid models route through the same guard; the + # reserve walks every node at amount 0 and commit later reconciles to 0 (#104). handler, uow = _build() - with pytest.raises(NonPositiveEstimate): - await handler.reserve(_auth(), _command(input_bound_tokens=0, max_output_tokens=0)) - assert uow._ctx.reserve_tx.calls == [] # never reached the balance guard - assert uow._ctx.reservations.inserted is None - assert uow._ctx.idempotency.stored == [] - assert uow.rolled_back is True + result = await handler.reserve(_auth(), _command(input_bound_tokens=0, max_output_tokens=0)) + assert result.estimated_micro == 0 + ctx = uow._ctx + assert ctx.reserve_tx.calls == [([_ORG_NODE, _USER_NODE], _PERIOD, 0)] # zero-hold guard walk + assert ctx.reservations.inserted is not None + record, lines = ctx.reservations.inserted + assert record.estimated_micro == 0 + assert all(line.amount_micro == 0 for line in lines) + assert ctx.idempotency.stored != [] # a success caches its response + assert uow.rolled_back is False async def test_reserve_includes_an_authorized_project_budget() -> None: From 1e6ed7fe6b9b213374845b66ecd2353879e5688f Mon Sep 17 00:00:00 2001 From: Arik Levinsky Date: Mon, 20 Jul 2026 11:43:08 -0600 Subject: [PATCH 2/2] style: reflow price-book repo docstring under the line-length limit Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BjJ4QScUKbrMzF1jujL5Bg --- src/tollgate/adapters/postgres/price_book_repo.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tollgate/adapters/postgres/price_book_repo.py b/src/tollgate/adapters/postgres/price_book_repo.py index 0ed43f5..9e8aff7 100644 --- a/src/tollgate/adapters/postgres/price_book_repo.py +++ b/src/tollgate/adapters/postgres/price_book_repo.py @@ -4,8 +4,9 @@ ``version`` as a deterministic tiebreak when several versions share a ``published_at`` (#98). A single join from ``price`` to ``price_book``, filtered to the pair and ordered by that total order descending, takes the current version and its rates — or returns ``None`` when the pair is -unpriced, which the reserve turns into an ``UnknownModel`` denial. Explicit async SQLAlchemy Core, no ORM; like the -other repositories it never imports ``application`` and satisfies the port structurally. +unpriced, which the reserve turns into an ``UnknownModel`` denial. Explicit async SQLAlchemy Core, +no ORM; like the other repositories it never imports ``application`` and satisfies the port +structurally. """ from __future__ import annotations