test(e2e): scope the negative-spend guard honestly, drop the #25846 claim

The guard was described as covering the bedrock cache-token regression
(#25846). It does not. That bug needs prompt_tokens to exclude cache tokens
so prompt_tokens - cache_hit falls below zero; this test drives uncached
traffic, so cache_hit is 0 and the subtraction cannot go negative. Reverting
the clamp leaves the test green, confirmed by mutation: the only way to turn
it red was an injected fault unrelated to the bug.

State the scope and the limitation in the docstring, mark the cell
fail_before_fix=unproven explicitly, and record the live bedrock cache-token
path as a still-open gap in the matrix. The test keeps its real value as a
cross-provider non-negative invariant sweep.
This commit is contained in:
mubashir1osmani 2026-07-14 17:47:47 -07:00
parent db3b136a35
commit e2fb677f3c
3 changed files with 23 additions and 16 deletions

View file

@ -33,4 +33,4 @@
- {id: quota_management.spend_tracking.failure.writes_failure_row, module: quota_management, tier: P1, behavior: spend_tracking, variant: failure, assertions: [writes_failure_row], exercised_on: [chat_completions], source: "proxy/spend_tracking/spend_log_error_logger.py", rationale: "A failed call writes a failure-status spend row"}
- {id: quota_management.spend_tracking.spend_calculate.returns_cost, module: quota_management, tier: P2, behavior: spend_tracking, variant: spend_calculate, assertions: [returns_cost], exercised_on: [spend_calculate], source: "proxy/spend_tracking/spend_management_endpoints.py", rationale: "/spend/calculate prices a hypothetical request at nonzero cost"}
- {id: quota_management.spend_tracking.pagination.keeps_total, module: quota_management, tier: P2, behavior: spend_tracking, variant: pagination, assertions: [keeps_total], exercised_on: [chat_completions], source: "proxy/spend_tracking/spend_management_endpoints.py", rationale: "Spend-logs v2 pagination caps page size without losing the total"}
- {id: quota_management.spend_tracking.non_negative.never_negative, module: quota_management, tier: P1, behavior: spend_tracking, variant: non_negative, assertions: [never_negative], exercised_on: [chat_completions, embeddings], source: "litellm_core_utils/llm_cost_calc/utils.py", rationale: "No provider (gemini/anthropic/openai/bedrock) writes a negative spend row across streaming/non-streaming chat and embeddings; derived cache-token counts must clamp at zero (BerriAI/litellm#25846)"}
- {id: quota_management.spend_tracking.non_negative.never_negative, module: quota_management, tier: P1, behavior: spend_tracking, variant: non_negative, assertions: [never_negative], exercised_on: [chat_completions, embeddings], source: "litellm_core_utils/llm_cost_calc/utils.py", fail_before_fix: unproven, rationale: "No provider (gemini/anthropic/openai/bedrock) writes a negative spend row across streaming/non-streaming chat and embeddings; negative spend credits a budget back, letting a key outspend its cap. Broad invariant sweep on uncached traffic: it does NOT cover the bedrock cache-token path (#25846), which needs cache_hit > prompt_tokens to fire, so it stays unproven until a test drives that path"}

View file

@ -29,7 +29,8 @@ proxy + SpendLogs rows. Status: `covered` / `partial` / `gap`.
| Modality | Existing | Status | Live e2e |
|----------|----------|--------|----------|
| Non-negative spend (all providers) | `test_anthropic_claude3_transformation.py`, `llm_cost_calc/utils.py` (#25846) | partial | yes (`test_no_provider_logs_negative_spend`) |
| Non-negative spend (all providers, uncached traffic) | `test_anthropic_claude3_transformation.py`, `llm_cost_calc/utils.py` | partial | yes (`test_no_provider_logs_negative_spend`) |
| Negative cost from bedrock cache-token split (#25846) | `test_anthropic_claude3_transformation.py` (mock) | gap (live) | no - needs bedrock streaming + cached prefix; the sweep above does not reach it |
| Chat (non-stream) | `test_cost_calculator.py`, `local_testing/test_completion_cost.py` | covered | yes (`test_chat_completion_writes_nonzero_spend_row`) |
| Chat (streaming) | `test_streaming_interrupt_spend_tracking.py` | partial | yes (`test_streaming_chat_completion_tracks_spend`) |
| Embedding | `test_cost_calculator.py` (#29956) | partial | yes (`test_embedding_writes_nonzero_spend_row`) |
@ -69,7 +70,7 @@ proxy + SpendLogs rows. Status: `covered` / `partial` / `gap`.
| `test_tag_spend_matches_sum_of_tagged_logs` | `/spend/tags` SUM/COUNT == tagged rows |
| `test_end_user_spend_attributed_on_row` | `end_user` attributed + costed |
| `test_each_model_on_a_shared_key_gets_its_own_row` | per-model/provider rows, correct model + cost, distinct request_ids matching response id |
| `test_no_provider_logs_negative_spend` | no provider (gemini/anthropic/openai/bedrock) writes spend < 0 across streaming/non-streaming chat + embeddings; each provider still logs a positive row (non-vacuous) |
| `test_no_provider_logs_negative_spend` | no provider (gemini/anthropic/openai/bedrock) writes spend < 0 across streaming/non-streaming chat + embeddings; each provider still logs a positive row (non-vacuous). Uncached traffic only, so it does not reach the #25846 cache-token path; `fail_before_fix=unproven` |
| `test_failure_call_writes_failure_status_row` | failed call -> `status=failure`, `spend=0` |
| `test_spend_calculate_returns_nonzero_cost` | cost-map smoke (no batch wait) |
| `test_spend_logs_endpoint_returns_spend` | `/spend/logs` returns 200 + the key's spend, never a 5xx (intermittent-500 regression) |

View file

@ -444,10 +444,9 @@ class _SweepModel:
# Chat providers this guard sweeps when the proxy exposes them, each non-streaming and
# streaming: gemini, anthropic, openai, and bedrock - the surface #25846's negative
# streaming-cost bug lived on. gemini/anthropic/bedrock are registered by the suite's
# driver_models; gpt-5.5 is only wired on the stage/docker gateways. Adding a provider
# is one line.
# streaming: gemini, anthropic, openai, and bedrock. gemini/anthropic/bedrock are
# registered by the suite's driver_models; gpt-5.5 is only wired on the stage/docker
# gateways. Adding a provider is one line.
_CHAT_SWEEP: tuple[_SweepModel, ...] = (
_SweepModel("gemini-2.5-flash", "gemini-2.5-flash"),
_SweepModel("claude-haiku-4-5", "anthropic/claude-haiku-4-5"),
@ -461,15 +460,22 @@ _EMBEDDING_SWEEP = _SweepModel("openai-text-embedding-3-small", "text-embedding-
def test_no_provider_logs_negative_spend(
client: SpendClient, scoped_key: str
) -> None:
"""No provider ever writes a negative spend row. One key sweeps every chat
provider the proxy exposes, both non-streaming and streaming, plus an embedding;
then every logged row is asserted non-negative. A negative cost is a real billing
bug: cache-token accounting that lets a derived token count fall below zero
(BerriAI/litellm#25846, a bedrock-anthropic streaming regression) surfaces here as
spend < 0, and both bedrock and the streaming leg are exercised on purpose because
that is the path that regression rode in on. The per-provider positive-row check
keeps the guard non-vacuous - a pipeline that silently logs 0 (or drops the row)
fails instead of sliding past a bare `>= 0`."""
"""No provider ever writes a negative spend row. One key sweeps every chat provider
the proxy exposes, both non-streaming and streaming, plus an embedding; then every
logged row is asserted non-negative. Negative spend is a billing bug in any form: it
credits the customer's budget back, so a key can outspend its cap without ever
tripping it. The per-provider positive-row check keeps the guard non-vacuous - a
pipeline that silently logs 0 (or drops the row) fails instead of sliding past a
bare `>= 0`.
Scope, deliberately stated: this is a broad invariant sweep, NOT a regression test
for the bedrock cache-token bug (#25846). That bug needs prompt_tokens to exclude
cache tokens so `prompt_tokens - cache_hit` goes below zero; the traffic here is
uncached, so `cache_hit == 0` and that subtraction can't go negative. Reverting
#25846's clamp would leave this test green. Verified by mutation on 2026-07-14: the
only way to turn it red was an injected fault unrelated to that bug, so the cell
stays fail_before_fix=unproven. Proving it needs a test that drives the cache-token
path itself (bedrock streaming with a cached prefix)."""
present = frozenset(entry.model_name for entry in client.gateway.model_info())
chat = tuple(m for m in _CHAT_SWEEP if m.call in present)
assert len(chat) >= 2, (