Skip to content

feat(cancel): billing accuracy + orthogonal delivery/billing status taxonomy - #78

Merged
songkuan-zheng merged 9 commits into
ship/v1.87.0from
fix/billing-accuracy-phase-1
Jun 10, 2026
Merged

feat(cancel): billing accuracy + orthogonal delivery/billing status taxonomy#78
songkuan-zheng merged 9 commits into
ship/v1.87.0from
fix/billing-accuracy-phase-1

Conversation

@songkuan-zheng

@songkuan-zheng songkuan-zheng commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Tier classification

  • C — Universal bug fix in litellm/ core (cursor=1 reset, CancelledError catch)
  • D — Universal mechanism + company opinion in litellm/ core (orthogonal delivery_status / billing_status taxonomy, shield-and-wait billing policy)

If Tier C or D, did you try upstream first?

  • No — justification: 9 commits across 3 phases. Phase 3 settled the design; we want one internal release cycle of dogfooding before opening upstream PRs. Candidates already registered in UPSTREAM_PR_QUEUE.md (Tier C: cursor=1 + CancelledError catch as standalone PRs; Tier D: cancel taxonomy as ISSUE-FIRST). Filing upstream now would split the design discussion across nine commits and three forks; we instead ship a coherent end-to-end fork delta, prove it in real Anthropic traffic, then upstream the pieces.

Summary

End-to-end fix for the LiteLLM cancellation black hole. Before this PR:

  1. Client cancel mid-stream → no SpendLogs row, orphaned Langfuse trace, upstream still billed us for compute that never reached the client.
  2. Anthropic message_start cursor placeholder (output_tokens=1) leaked into final billing when no message_delta arrived before cancel, inflating cancelled-row token counts.
  3. Failure-path cost was hardcoded to 0.0 — masking real upstream consumption even when we had usage evidence.

After this PR:

  • Every cancellation produces a SpendLogs row with proper cost attribution.
  • The row carries status="success" (cancel is not a system failure) plus metadata.cancellation_indicator="client_disconnect" plus two derived orthogonal dimensions in metadata:
    • delivery_status: full | partial | none — what reached the client
    • billing_status: full | partial | none — what we actually charged
  • The UI dashboard renders cancelled rows with an amber "Cancel 499" badge inside the Success bucket (/ui/?page=logs).
  • The failure-rate Prometheus counter no longer counts cancellations.

Commit walk-through (9 commits, 3 phases)

Phase 1 (initial fix; introduced status="success_partial" sentinel):

  • 80127194b5 fix(streaming): reset Anthropic message_start cursor when no message_delta arrives
  • e0a2058de4 feat(spend_logs): add success_partial status + cancellation metadata fields
  • ebab077c4d feat(cancel): catch CancelledError + re-route streaming/non-stream cancels
  • b1420d0d7e feat(cancel): compute partial cost + bridge cancel metadata to SpendLogs
  • a0993142ef test(e2e): add case 26 cancel-billing + plumb success_partial through SpendLogs

Phase 2 (expansion):

  • 987df5f72c test(e2e): cases 27-32 cancel-billing expansion + Phase 2 gaps documented
  • 6e3e4c31c4 feat(cancel): non-stream cancel detection + /v1/messages markers

Phase 3 (taxonomy refactor — supersedes Phase 1's success_partial sentinel):

  • 4dcdd73f1a feat(cancel): orthogonal delivery_status/billing_status taxonomy
  • 9f3e9433da docs(cancel): post-Phase-3 cleanup (rename case 26, refresh docs, harden case 30 preflight)

The Phase 3 refactor was driven by a post-Phase-2 review:
success_partial turned out to be DB-only — none of the external observability integrations (Prometheus, Langfuse, Custom Callback API, OTel) ever learned about it, so cancellations were inconsistently classified depending on dispatch path. Phase 3 collapses StandardLoggingPayloadStatus back to upstream's binary success | failure and puts the cancel taxonomy in optional metadata dimensions instead.

Verification

  • 217/217 Python unit tests pass (tests/test_litellm/litellm_core_utils/test_cancel_*.py + tests/test_litellm/proxy/spend_tracking/)
  • 6/6 cancel e2e cases pass on mock provider (cases 26, 27, 28, 29, 30, 32)
  • Case 33 (real Anthropic streaming cancel) passes:
    • status=success, delivery=partial, billing=partial
    • real billing: prompt=90 tokens, completion=256-288 tokens, spend=$0.00411 / $0.00459
    • markers: cancellation_indicator=client_disconnect, cancel_phase=streaming_partial
  • UI builds clean (npm run build in ui/litellm-dashboard/)
  • black --check clean on touched Python files

Conflict resolutions (if any cherry-pick conflicted)

N/A — no cherry-picks; branch is linear from ship/v1.87.0 fork point f96d5c56a6.

Cross-references

  • Design spec: e2e/cases/26_cancel_billing_partial.md + e2e/cases/33_real_anthropic_cancel.md
  • Single source of truth for the derivation: spend_tracking_utils._derive_delivery_billing_status (7-row semantic mapping in the docstring)
  • Upstream queue: UPSTREAM_PR_QUEUE.md rows added for the Tier-C cursor=1 + CancelledError pieces and the Tier-D taxonomy

Pre-submission

  • Tests in tests/test_litellm/ (3 files + 1 new under proxy/spend_tracking/)
  • make test-unit equivalent passes (217/217)
  • PR scope: 9 commits across one coherent feature
  • Greptile review (will request after PR opens)

…) when no message_delta arrives

The Anthropic streaming protocol emits `message_start.usage.output_tokens=1`
as a placeholder cursor; the real cumulative output count only arrives in
the final `message_delta` event. When a stream is cancelled before
`message_delta` lands (common for thinking models on long-tail prompts),
ChunkProcessor._calculate_usage_per_chunk's last-wins accumulator left
completion_tokens stuck at 1. Because 1 is truthy, the
`completion_tokens or token_counter(text=...)` fallback in
calculate_usage() never fired, and requests were billed for 1 output
token even when several thousand tokens of text had actually streamed.

Fix: track whether any chunk's completion_tokens exceeded 1
(saw_non_cursor_completion). If the only update we saw was the cursor,
reset completion_tokens to 0 so the text-based fallback estimates from
the real completion content.

Legitimate 1-token completions (model returns "Yes." etc.) are unaffected
in practice — token_counter on a 1-token completion_output also yields
~1, so billing stays approximately correct.

Tier: C (universal bug fix — affects every LiteLLM user calling Anthropic
with streaming + cancel/timeout). Upstream PR candidate once landed here.

Tests added:
- TestAnthropicCursorBug (6 cases) — pins the post-fix behavior
- TestNonAnthropicStreamingIntact (2 cases) — guards against regression on
  providers without the cursor pattern

All 9 existing streaming_chunk_builder_utils tests still pass.
…fields

Adds the schema scaffolding for tracking client-cancelled requests as a
billable "success_partial" status (instead of dropping them into the
failure bucket, which both pollutes the proxy failure rate metric and
zeroes out billing for compute the upstream provider already charged us
for).

New StandardLoggingPayloadStatus value:
  - "success_partial": client disconnected mid-flight (or upstream cut
    early) but upstream consumed billable compute. Bills prompt + the
    output that was generated up to cancellation. Does NOT count toward
    the proxy failure rate.

New Literal types in litellm/types/utils.py (used by PR #3 cancel-billing
path to populate metadata fields):
  - CancelPhase: before_upstream | during_upstream | streaming_partial |
                 during_parsing
  - CancelUsageSource: upstream_truth | tokenizer_estimate |
                       upstream_completed_after_cancel | shield_timeout |
                       no_completion

New StandardLoggingMetadata + SpendLogsMetadata fields (all Optional,
populated only when the cancel-billing path fires):
  - cancellation_indicator: client_disconnect | upstream_disconnect
  - cancel_phase: lifecycle phase when cancel was detected
  - bytes_delivered_to_client: total bytes ACK'd to client before disconnect
  - upstream_completed: whether upstream call ran to completion
  - usage_source: provenance of the recorded usage numbers

DB compatibility:
  - LiteLLM_SpendLogs.status is `String?` (not enum) — accepts new value
    without migration.
  - LiteLLM_SpendLogs.metadata is `Json?` — new fields land inside the JSON
    blob, no migration required.
  - Existing `metadata.error_information.error_code` (used by nginx/
    Prometheus dashboards) is unchanged; cancellation fields are additive.

Tier: B (internal change to a TypedDict + helper init; downstream
behavior change ships in PR #3). The success_partial status name and
field semantics are coordinated with the upcoming cancel-billing module.

Tests:
  - 10 new cases covering literal acceptance, None-init, round-trip,
    error_code coexistence, and enum value coverage.
  - All 73 existing spend_tracking tests pass.
…ncels to success_partial path

Closes the 499 black hole. asyncio.CancelledError is a BaseException
subclass since Python 3.8 — every `except Exception` in LiteLLM was
letting it slip through silently:
  * SpendLogs got no row for the cancelled request
  * No callback fired (Langfuse trace stuck "Running", Prometheus
    failed_requests counter not incremented)
  * The upstream provider continued generating after the client TCP
    close and billed us for compute that never reached our ledger

This PR introduces a small finalize layer that catches the cancel
signal, marks the Logging object with cancel_phase / cancellation
metadata (matching PR #4's schema), dispatches the partial response
through the normal async_success_handler path with the new
status="success_partial" classification, and re-raises CancelledError
so asyncio's cancellation contract is preserved.

Files
-----
* litellm/litellm_core_utils/cancel_finalize.py (new)
    Public helpers:
      - mark_logging_obj_cancelled(logging_obj, phase, indicator,
        bytes_delivered): idempotent dict mutation; pathological inputs
        (None logging_obj, missing model_call_details) are no-ops.
      - finalize_streaming_cancel(stream_wrapper, logging_obj, ...):
        runs stream_chunk_builder on accumulated chunks → dispatches
        through async_success_handler with success_partial intent.
        Shielded internally so the SpendLogs write completes even if
        the runtime injects more cancel signals during teardown.
        Falls back to post_call_failure_hook if there are zero chunks.
      - finalize_non_stream_cancel(upstream_task, logging_obj, ...,
        shield_timeout_s=60.0): shields the upstream call past the
        cancel and waits for real usage (strategy A). On timeout,
        cancels upstream and records usage_source="shield_timeout".

* litellm/proxy/proxy_server.py — async_data_generator
    Added `except asyncio.CancelledError:` before `except Exception:`
    that calls finalize_streaming_cancel and re-raises.

* litellm/proxy/common_request_processing.py — async_streaming_data_generator
    Same catch for the /v1/messages and /v1beta/...generateContent
    paths (Anthropic + Google).

* litellm/litellm_core_utils/streaming_handler.py — __anext__
    Defense-in-depth: if cancel reaches the stream wrapper directly
    (not via the proxy generator), mark the Logging object and
    re-raise so whoever does finalize the request gets the marker.

* CLAUDE.md — Test discipline
    Added "No theater tests" rule: mock the boundary (httpx transport,
    DB cursor), not the unit under test. Use a small spy fake to
    capture handler args; assert on the captured data, not on
    `.called`. Example: instead of mocking `stream_chunk_builder`,
    pass real ModelResponseStream chunks in. The rule is enforced in
    this PR's test file.

Tests (18 cases, all real — no mocking of the unit under test):
  * mark_logging_obj_cancelled: dict-mutation contract incl.
    idempotency, None handling, no-pollution-with-Nones
  * is_logging_obj_cancelled: marker detection
  * _get_accumulated_chunks: stream wrapper variants
  * finalize_streaming_cancel:
      - Real Anthropic-shaped chunks (message_start cursor=1 + content
        deltas, NO message_delta) run through real stream_chunk_builder
        → captured response.usage.completion_tokens > 1, exercising
        the cross-PR contract with PR #1's cursor reset
      - No-chunks edge: real post_call_failure_hook spy verifies
        fallback fires with a CancelledError, not a swallowed signal
      - Downstream success_handler raising must NOT propagate
  * finalize_non_stream_cancel:
      - Shield-wait happy path: real asyncio task, real response
        flows through to captured success kwargs
      - Shield timeout: upstream task actually cancelled
      - Upstream exception during shield window
      - No upstream task (before_upstream cancel)

Cost computation for the partial response — the actual implementation
of "modified strategy 5'" billing — lands in PR #3 (cancel_billing.py)
which reads the markers this PR sets. Until PR #3 lands, the
reassembled partial response runs through the existing cost calculator
unchanged, which is already a significant improvement over the prior
"drop everything" behavior because PR #1's cursor=1 fix produces
sensible partial usage for Anthropic.

Tier: C+D (Tier C for the BaseException catch fix — purely a bug;
Tier D for the success_partial taxonomy — opinionated mechanism with
billing implications we want carried in the fork until upstream agrees
on semantics).
Completes the cancel-billing path started in PR #2. cancel_finalize was
catching cancels and dispatching through async_success_handler with a
reassembled partial response, but two gaps remained:

1. The cancellation markers written to logging_obj.model_call_details
   never reached SpendLogs — _get_spend_logs_metadata pulls fields from
   request_data.litellm_params.metadata, not from model_call_details.
   Result: SpendLogs rows existed (good, no more black hole) but had
   no cancel_phase / usage_source / etc., so dashboards couldn't tell
   normal vs partial.

2. The zero-chunk fallback path (finalize → failure hook) still wrote
   response_cost=0.0 — the upstream provider received the prompt and
   started processing, but our ledger said "free". For thinking models
   on long prompts this is a real $ leak ($0.003-$0.01 per 499 in
   production).

Files
-----
* litellm/litellm_core_utils/cancel_billing.py (new):
    - compute_prompt_only_cost(messages, model, custom_llm_provider):
      best-effort prompt-only $ via the real LiteLLM cost map. Returns
      0.0 on any pricing failure — caller is in the failure hook and
      cannot afford an exception. Wraps token_counter + cost_per_token
      so the cost-map lookup path is exercised by real provider IDs.
    - enrich_request_metadata_with_cancel_markers(request_data,
      logging_obj): copies the five cancel markers from
      model_call_details into litellm_params.metadata. Idempotent;
      no-op when there's no marker (normal requests pay zero
      overhead). Overrides metadata.status to "success_partial".

* litellm/litellm_core_utils/cancel_finalize.py:
    - finalize_streaming_cancel now calls
      enrich_request_metadata_with_cancel_markers right after
      mark_logging_obj_cancelled, so the partial response dispatched
      through async_success_handler carries the markers into the
      cost-tracking pipeline.
    - finalize_non_stream_cancel does the same bridge, AND a second
      bridge after the shield-wait outcome is known (upstream_completed
      / usage_source can change after the asyncio.wait_for resolves).

* litellm/proxy/hooks/proxy_track_cost_callback.py:
    - async_post_call_failure_hook: when the failure is actually a
      CancelledError (i.e. the cancel-finalize fallback path because
      no chunks were received), compute the prompt-only cost via
      cancel_billing.compute_prompt_only_cost instead of the hardcoded
      0.0. Other failures (provider 5xx, real timeouts) still write
      0.0 — there's no compute on our side to bill in those cases.
    - Same hook also calls enrich_request_metadata_with_cancel_markers
      so failure-path SpendLogs rows tagged success_partial get the
      right metadata even when called by external code paths that
      didn't go through cancel_finalize first.

Tests (12 new cases, no mocking of cost_per_token / token_counter):
  * TestComputePromptOnlyCost:
      - Real Anthropic & OpenAI pricing paths exercised → cost > 0
        with sanity upper bound (catches both "always 0" and "wrong
        rate" regressions)
      - Unknown model degrades to 0.0 without raising
      - Long-prompt cost scales with token count (sanity that we're
        actually counting, not constants)
  * TestEnrichRequestMetadataWithCancelMarkers:
      - No-op when no marker (must not pollute normal requests)
      - All five markers + status override propagate
      - Creates missing litellm_params.metadata dict
      - Partial markers don't insert keys for absent ones
      - Overwrites stale status (success / failure → success_partial)

All 43 Phase 1 tests pass together (cursor + schema + cancel_finalize
+ cancel_billing). 82 streaming + spend_tracking regression tests pass.

Tier: D (mechanism with billing policy — bills > 0 for cancelled
requests, which is a behavior change downstream consumers can observe).
… SpendLogs

Adds an e2e case that verifies the full Phase 1 cancel-billing chain
end-to-end against the live proxy + Postgres, and fixes three real
plumbing bugs the case exposed:

1. FallbackStreamWrapper.__anext__ did not accumulate chunks
   FallbackStreamWrapper inherits from CustomStreamWrapper but overrides
   __anext__ to delegate to its own async generator — bypassing the
   parent's `self.chunks.append(chunk)`. Result: any streaming request
   that went through the Router fallback wrapper had empty
   wrapper.chunks, so finalize_streaming_cancel's reassembly path saw
   "no chunks" and fell through to the failure-hook fallback. Fix:
   append in __anext__ to mirror the parent.

2. enrich_request_metadata_with_cancel_markers only touched one of two
   metadata dicts
   The proxy failure path reads metadata from
   request_data["litellm_params"]["metadata"]; the success path reads
   from logging_obj.litellm_params["metadata"]. These are usually the
   same object but can diverge after construction-time copies. Fix:
   update both. Detected via case 26 showing status="success"
   completion_tokens=200 with all cancel markers blank — the success
   path was getting the partial response but not the markers.

3. _get_status_for_spend_log only knew "success" / "failure"
   metadata.status was set to "success_partial" by the cancel-finalize
   bridge, but _get_status_for_spend_log collapsed any non-"failure"
   value back to "success" before writing the LiteLLM_SpendLogs.status
   column. Dashboards filtering `WHERE status='success_partial'` would
   have matched zero rows. Fix: widen the Literal to three values and
   pass success_partial through unchanged.

4. _ProxyDBLogger.async_post_call_failure_hook clobbered cancel markers
   and hardcoded status="failure"
   The failure hook rebuilt litellm_params.metadata from scratch,
   preserving only "tags" — cancel markers set by cancel_finalize were
   lost on the zero-chunk fallback path. Also hardcoded metadata.status
   = "failure" even for CancelledError. Fix: detect CancelledError and
   classify as success_partial; preserve the five cancel marker fields
   alongside the existing tags preservation.

5. time.time() vs datetime.now() in finalize success_handler calls
   async_success_handler expects datetime instances for start_time /
   end_time (it does arithmetic on them); passing time.time() floats
   raised TypeError in the cost calculator and silently dropped the
   row. Fix: datetime.now().

Test plan (live proxy + Postgres):
* Case 26 C1: streaming cancel mid-flight       → status=success_partial,
                                                  completion_tokens=214,
                                                  cancel_phase=streaming_partial
* Case 26 C2: long stream cancelled near end    → status=success_partial,
                                                  completion_tokens=760
* Case 26 C3: zero-chunk cancel                 → status=success_partial,
                                                  completion_tokens=14
* Full mock-only e2e suite: 17/17 pass, 0 fail, 8 skip (expected
  real-provider skips). No regression in cases 04, 06, 07, 10, 11,
  12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 23, 25.

Unit tests:
* 43 Phase 1 tests still pass (cursor + schema + cancel_finalize +
  cancel_billing).
* 82 streaming + spend_tracking regression tests still pass.

Tier: C (FallbackStreamWrapper accumulation, _get_status_for_spend_log
widening — universal bug fixes) + D (success_partial classification,
metadata bridging policy).
…nted

Extends the cancel-billing e2e coverage with five new mock-only cases.
Two PASS outright, three honestly SKIP with detailed deferral reasons
that point at the remaining Phase 2 work.

PASSING (new):
  29 failure-not-polluted    — Forces upstream 503; asserts SpendLogs
                               row classifies as status=failure with
                               error metadata populated. Critical
                               regression guard: without this, the
                               CancelledError check in
                               proxy_track_cost_callback could silently
                               grow to catch other exceptions and
                               start polluting success_partial.
  32 concurrent-cancels      — Fires 10 concurrent stream cancels,
                               asserts all 10 SpendLogs rows write
                               with success_partial + the marker,
                               and a control health probe returns
                               in < 3s (no asyncio task leak under
                               contention).

DEFERRED (skip 77, with detailed runbook comments):
  27 non-stream-cancel       — finalize_non_stream_cancel exists and
                               is unit-tested, but no caller wires it
                               up. LiteLLM removed the
                               check_request_disconnection polling
                               task (it's dead code in proxy_server.py),
                               so non-stream cancel detection requires
                               a Phase 2 polling revive. Until then
                               non-stream cancels look like normal
                               successes from the proxy's view (which
                               is at least billing-correct, just
                               unclassified).
  28 v1-messages-cancel      — The CancelledError catch in
                               common_request_processing.py IS firing
                               (verified: SpendLogs row gets correct
                               partial completion_tokens reflecting
                               bytes received before cancel), but the
                               success_partial markers don't propagate
                               because response.logging_obj is None
                               on the anthropic_messages path.
                               Phase 2: pull logging_obj from
                               request_data["litellm_logging_obj"]
                               instead of from response.
  30 shield-timeout          — Depends on case 27 wiring. Implemented
                               the LITELLM_CANCEL_SHIELD_TIMEOUT_S env
                               var (default 60s, e2e sets 2s) so the
                               case can run deterministically once
                               polling is wired.

Other changes:
  * e2e/_config/mock_provider.py: added time.sleep(ttft_ms) to the
    Anthropic non-stream path so case 27/30 can deterministically
    drive client-cancel timing once the proxy plumbing lands.
  * e2e/_config/docker-compose.yml: sets
    LITELLM_CANCEL_SHIELD_TIMEOUT_S=2 on the proxy container.
  * litellm/litellm_core_utils/cancel_finalize.py: reads
    LITELLM_CANCEL_SHIELD_TIMEOUT_S env var at module load for the
    DEFAULT_CANCEL_SHIELD_TIMEOUT_S knob.

Suite outcome: 30 cases · 19 PASS · 0 FAIL · 11 SKIP
(11 skips = 6 pre-existing Tier=real + 1 pre-existing thinking-signature
+ 1 pre-existing anthropic-beta-overrides + 3 new Phase-2-deferred).

Tier: B (e2e infrastructure + documentation of Phase 2 gaps).
…rkers

Closes the remaining Phase 2 gaps documented in commit 987df5f:

1. /v1/messages cancel markers didn't reach SpendLogs (case 28)
   The CancelledError catch in async_streaming_data_generator was firing
   correctly but `getattr(response, "logging_obj", None)` returned None
   on the Anthropic native path (response is an async iterator without
   that attribute), so mark_logging_obj_cancelled became a no-op.
   Additionally, `_get_litellm_metadata_from_kwargs` prefers the newer
   `litellm_metadata` dict over `metadata` — and the cancel markers were
   only written to the latter, so they were filtered out by the SpendLogs
   pipeline even when present on the Logging instance.

2. Non-stream cancel was a silent black hole (case 27, 30)
   No proxy code path detected client disconnect during non-stream LLM
   calls. The dead-code `check_request_disconnection` helper had been in
   proxy_server.py for ages but with zero callers.

Fixes
-----

litellm/proxy/proxy_server.py + litellm/proxy/common_request_processing.py
  Both streaming catches: when response.logging_obj is None, fall back to
  request_data["litellm_logging_obj"]. Required for /v1/messages,
  /v1beta/...streamGenerateContent and any other endpoint where the
  response object isn't a CustomStreamWrapper.

litellm/litellm_core_utils/cancel_billing.py
  enrich_request_metadata_with_cancel_markers now writes to BOTH the
  `metadata` and `litellm_metadata` keys on each litellm_params dict
  (request_data + logging_obj.litellm_params). litellm_metadata is the
  newer-endpoint variant; without writing to it, /v1/messages
  cancellations couldn't surface success_partial markers because the
  cost callback's metadata extractor returns litellm_metadata when both
  are present.

litellm/proxy/common_request_processing.py — base_process_llm_request
  Added a _disconnect_watcher task that polls request.is_disconnected()
  every second while the LLM call is in flight. On detection:

  * Records the disconnect time and KEEPS the LLM call running. We
    don't want to cancel it: the upstream provider has almost certainly
    started processing and will charge us regardless, so the right
    thing is to record real upstream usage.
  * If the LLM call completes within LITELLM_CANCEL_SHIELD_TIMEOUT_S
    (default 60s, env-tunable) of the disconnect, the row gets tagged
    with usage_source=upstream_completed_after_cancel and
    upstream_completed=True.
  * If the budget elapses with the LLM still running, give up waiting,
    synthesize an empty response so the handler can return, and tag
    usage_source=shield_timeout / upstream_completed=False. The real
    response (when it eventually comes back from upstream) will fire
    a SECOND SpendLogs row via the normal success path — that's
    acceptable because it preserves the upstream-billing alignment.

  Implemented as a `wait_for(shield(...), poll_interval)` loop rather
  than a single wait_for + shield, so the watcher task gets a chance
  to set disconnect_flag between polls without being interrupted by
  the timeout itself. Cleaned up via a finally that cancels the
  watcher task regardless of how the LLM call resolved.

e2e/cases/27, 28, 30
  Un-skipped; all three now PASS against the live proxy.

Test plan (mock-only e2e suite, full run):
  * 30 cases · 22 PASS · 0 FAIL · 8 SKIP
  * 8 SKIP = 6 Tier=real + 1 thinking-signature (pre-existing) + 1
    anthropic-beta-overrides (Tier=real, requires Bedrock). All cancel-
    related cases now PASS — no remaining Phase 2 deferrals.
  * 125 unit tests still pass (cancel_billing 12 + cancel_finalize 18 +
    cursor 8 + spend_logs metadata 5 + streaming regression 9 + spend
    tracking regression 73).

Tier: C (universal bug fix — anyone relying on non-stream cancel
detection or the /v1/messages cancel taxonomy was getting wrong rows)
+ D (the disconnect-watcher's shield-and-wait policy is opinionated
billing behavior we want to carry until upstream agrees on semantics).
…onomy

Replace the single `status='success_partial'` marker with two orthogonal
dimensions derived at SpendLog write time from the five cancel markers
plus the raw status:

  delivery_status ∈ {full, partial, none}   # what reached the client
  billing_status  ∈ {full, partial, none}   # what we billed

The top-level SpendLogs status filter stays binary (success | failure);
cancellations carry `status='success'` with metadata markers and the UI
surfaces them through the new derived columns. Callers that need to
target the cancel slice in SQL can filter on
`metadata::jsonb->>'delivery_status'` / `billing_status` directly.

Scope:
- core derivation in `spend_tracking_utils._derive_delivery_billing_status`
  is the single source of truth — no other writer sets these fields.
- e2e cases 26–30 and 32 updated to assert both dimensions; case 33
  (real-anthropic-cancel) added end-to-end.
- UI: view_logs columns and filter_options surface the new fields.
- Tests added for the new derivation and for the binary status filter
  (test_status_filter_condition).
…doc, harden case 30 preflight, log upstream PR candidates

- e2e/cases/26 renamed: success_partial → partial (refactor dropped the
  three-valued status sentinel). Updated dispatcher reference in
  run-all-cases.
- e2e/cases/26 + 33 .md docs rewritten to describe the new binary
  status + orthogonal delivery_status / billing_status taxonomy,
  including the failure-mode lookup table for case 33 (real Anthropic).
- e2e/cases/data/30_shield_timeout.sh: add the mock-container
  healthcheck preflight that the other mock-only cases already have,
  so real-mode runs SKIP (rc=77) instead of FAIL.
- UPSTREAM_PR_QUEUE.md: log two Tier-C candidates (cursor=1 reset +
  CancelledError catch) and the Phase 3 taxonomy as a Tier-D
  ISSUE-FIRST entry. Refresh Last reviewed.

Tier: B (internal infra / docs).
Tried upstream first: N/A — purely internal cleanup.
@songkuan-zheng
songkuan-zheng merged commit b8917f2 into ship/v1.87.0 Jun 10, 2026
songkuan-zheng added a commit that referenced this pull request Jun 10, 2026
…pstream (#81)

Phase 3's _derive_delivery_billing_status mislabeled two cancel
scenarios as (delivery=none, billing=none) when in reality the
SpendLogs row carried a positive ``spend`` from
``compute_prompt_only_cost`` in the failure-hook path:

  - Streaming cancel BEFORE first chunk (phase=streaming_partial,
    bytes_delivered=0) — upstream had received the prompt and started
    generating; cancel arrived before the first SSE chunk reached the
    client. compute_prompt_only_cost bills the prompt baseline.

  - Non-stream cancel during upstream wait when shield gave up via
    _fallback_to_failure_hook with usage_source in {no_completion,
    None} (e.g. upstream errored mid-shield). Same billing logic
    applies — prompt was dispatched, prompt-only cost recovered.

Under the old rule both lands on (none, none), which contradicts the
positive ``spend`` column on the same row. Dashboards filtering by
billing_status would under-report cancel revenue.

Fix:
  - Reorder the derivation so phase=='before_upstream' is the only
    path that returns (none, none). Everything else inside the cancel
    branch falls through to (none, partial) when no positive-delivery
    or shield-success signal matches.
  - Update existing tests that pinned the wrong (none, none) values
    and add a test for the during_upstream no_completion case.

Existing tests covering the unaffected scenarios (normal success,
streaming with chunks, shield_timeout, shield_success, real failure,
before_upstream zero-chunk) all stay green — 217/217 spend_tracking +
cancel_billing + cancel_finalize tests pass.

Tier: D (refines the orthogonal-taxonomy derivation shipped in PR #78).
Tried upstream first: not yet — same dogfood window as #78's Tier-D
candidate; will be folded into the upstream issue once the v1.87.0-
internal.N line has one release of production data confirming the
mapping.
songkuan-zheng added a commit that referenced this pull request Jun 10, 2026
…ions (#82)

* fix(prometheus): track spend + suppress failure counter for cancellations

The cancel-finalize zero-chunk / shield-timeout / no-completion paths
all route through proxy_logging_obj.post_call_failure_hook. That
dispatch reaches every callback in litellm.callbacks, but the failure
side of the integration was systemically undercounting cancel revenue:

Layer 1 — SLP construction (litellm_logging.py):
  _failure_handler_helper_fn hardcoded model_call_details["response_cost"]
  = 0 BEFORE building the StandardLoggingPayload. Every callback that
  reads response_cost off the SLP (Prometheus litellm_spend_metric,
  Langfuse generation cost, Custom Callback API body, OTel spans, etc.)
  saw 0 for client cancellations even though the DB row carried a
  positive spend from proxy_track_cost_callback's compute_prompt_only_cost.

  Fix: detect asyncio.CancelledError and run compute_prompt_only_cost
  inline (best-effort — falls back to 0 on cost-map miss or tokenizer
  failure, never raises).

Layer 2 — Prometheus integration (prometheus.py):
  async_log_failure_event unconditionally bumped
  litellm_llm_api_failed_requests_metric and did NOT call
  _increment_top_level_request_and_spend_metrics at all. Even with
  Layer 1 fixed, the spend counter would still be wrong because the
  failure-event path never touches it.

  Fix: branch on standard_logging_payload.metadata.cancellation_indicator.
  When present:
    • Route SLP.response_cost to litellm_spend_metric.inc(amount=cost)
      — keeps Prometheus spend in lockstep with the SpendLogs spend
      column.
    • SKIP litellm_llm_api_failed_requests_metric and the
      set_llm_deployment_failure_metrics call — cancels are not
      system failures, must not pollute failure-rate dashboards.
    • Org-budget metric tracks the cancel cost too.
  Otherwise (real failures) the original behaviour is preserved
  exactly.

Tests:
  - test_litellm_logging.py: 3 new tests covering the cancel
    response_cost pre-population (positive cost for known model,
    zero/no-raise on unknown model, no change for non-cancel
    exceptions).
  - 217/217 cancel-billing + spend_tracking + cancel_finalize tests
    pass. (The one pre-existing failure in
    test_logfire_logger_accepts_env_vars_for_base_url is a missing-
    `opentelemetry` package on the local Python env, unaffected by
    this change.)

Tier: D — refines the Phase-3 cancel taxonomy by closing a metric
gap inherited from upstream's failure-event design. Both layers are
gated on the cancellation_indicator marker so non-cancel failures
keep identical behaviour. To be folded into the same upstream issue
as PR #78's Tier-D candidate after one internal release dogfood.

* test(e2e): case 35 — Prometheus cancel-vs-failure metric discrimination

End-to-end coverage for PR #82's Layer 2 routing topology. Tests the
real Prometheus client state inside the running proxy (no Counter
mocking, no async_log_failure_event monkey-patching — both would have
been theater).

Two probes against `model=mock-claude`:

  Probe A — streaming cancel (curl --max-time 3 mid-stream):
    expect: litellm_llm_api_failed_requests_metric_total delta = 0
            SpendLogs row status=success + cancellation_indicator

  Probe B — forced X-Mock-Fail: 503 (the control):
    expect: litellm_llm_api_failed_requests_metric_total delta >= 1
            SpendLogs row status=failure + no marker

The A/B discrimination is the load-bearing assertion: both probes
share the same `model` label so the counter delta is purely a
function of which branch of async_log_failure_event fired. Probe B's
positive delta also locks in that the fix isn't over-suppressing.

Spend-VALUE verification (Layer 1's compute_prompt_only_cost
populating SLP.response_cost) lives in
test_litellm_logging.py::TestFailureHandlerCancelCost (uses
anthropic/claude-haiku-4-5 in the cost map) and
e2e/cases/data/33_real_anthropic_cancel.sh (real provider) — both
land positive cost figures the mock provider can't produce because
mock-claude isn't in the cost map.

Wired into run-all-cases under the cancel-billing block (26 → 35).
Verified end-to-end: 7/7 cancel-suite cases PASS including the new
case 35.
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