Four more ruff rules for code the test suite runs but never checks. F601 is the
one that paid: the duplicate key it flagged in a get_form_data fixture was the
mock reproducing the production bug fixed in the previous commit.
B025 removed two unreachable handlers, one of them a pytest.skip shadowed by an
earlier `pass`, so an upstream Vertex flake reported green having asserted
nothing. F632 turned an `is ""` identity check, which passes only on CPython
interning, into the `== ""` it meant. B023 fixed three closures over loop
variables, all latent today but one iteration-order change away from checking the
last case N times.
get_form_data collapsed the FormData multidict with dict(form) before the loop
that rebuilds `foo[]` arrays ever ran, so a request sending
timestamp_granularities[]=word and timestamp_granularities[]=segment reached the
provider as ["segment"] with the first value silently dropped. Read the multidict
with multi_items() instead.
The test could not catch it because its mock was a plain dict carrying the same
key twice, which Python collapses exactly the way the bug did. Every request.form
mock that fed get_form_data now returns real FormData.
* fix(anthropic_messages): gate sampling params on /v1/messages like /chat/completions
/v1/messages forwarded temperature/top_p/top_k raw to models that removed
sampling params (supports_sampling_params: false — Claude 4.7+/Fable 5),
producing provider 400s that router fallbacks mask as silent model
downgrades. The chat path already gates these via
AnthropicModelInfo._apply_sampling_param; reuse it in
get_requested_anthropic_messages_optional_param so both endpoints agree:
drop under drop_params, else raise the clean client-side 400.
Fixes#35053
* test(anthropic): drive new sampling-param tests off the kwarg, not the global
The five tests added here set `litellm.drop_params = True` under a manual
try/finally. That trips TQ005 (module-global mutation, 10 new violations
over the ceiling) and it leaks process-wide if the finally is ever
skipped, which is what the save/restore conftest exists to paper over.
`get_requested_anthropic_messages_optional_param` already takes
`drop_params` as a kwarg, and that is the path /v1/messages actually
uses, so pass it directly. `monkeypatch.setattr` pins the global to
False so each test proves the per-request flag alone is sufficient and
cannot pass on a leaked global.
Verified: TQ gate clean, all 10 tests pass, and the 3 that assert the
new gating still fail with the fix in utils.py reverted.
---------
Co-authored-by: Mihidum Hettiyahandi <55163074+mihidumh@users.noreply.github.com>
* fix(mcp): resolve admin OAuth sessions to the same server set the connect page shows
* fix(mcp): bind admitted admin rows through the entitlement ceiling, not the credential scope clause
The websocket routes under /openai_passthrough and /openai had no e2e
coverage, so nothing catches the regression from issue #36088, where both
prefixes carried HTTP routes only and refused every upgrade with a 403
before a socket ever existed.
Two tests cover it. The realtime one opens /openai_passthrough/v1/realtime
and asserts OpenAI's own session.created frame comes back, which proves the
route is registered and relayed upstream. The responses one asserts
/openai/v1/responses accepts the upgrade, since a responses.connect socket
waits for the client to speak first and has no opening frame to check.
A refused upgrade is an HTTP response rather than a close frame, so both
assert on the handshake. ws_base_url moves into e2e_config now that a
second suite needs it
The three lite-image keys landed on the deploy branch separately while this
branch was open, so merging left every key defined twice in both price maps.
The merge is clean as text and the file still parses, but JSON keeps the last
occurrence of a repeated key, so the first copy's supported_endpoints,
supported_modalities and supports_system_messages were being dropped without
any error.
Each key is now one entry, placed next to its gemini-3.1-flash-image sibling
rather than at the end of the file.
supports_reasoning goes to false on all three, matching every other Gemini
image model. Leaving it off is not neutral: _supports_factory falls through to
the vertex_ai provider config, which answers true, and reasoning_effort then
gets forwarded to an image endpoint that rejects it. That was fixed for the
rest of the family in 75dd70a678 and these entries had drifted back.
Also fills in what the entries were missing against Google's published
pricing: the Vertex implicit cache read rate, batch rates on the Vertex
routes, and the pdf/video input flags.
The two overlapping test files are folded into one, and the price map suite
grows a duplicate-key guard so the next clean-but-lossy merge fails loudly.
* fix(logging): preserve uvicorn color_message args during secret redaction
SecretRedactionFilter clears record.args after substituting record.msg, but
uvicorn's colorized formatter re-renders the separate color_message extra
field against record.args at emit time. With args cleared, uvicorn prints
the raw "%s://%s:%d" template instead of the actual startup URL whenever
output goes to a TTY (colors on).
* fix(logging): narrow color_message fallback to TypeError
Bare except-Exception-pass on the color_message substitution pushed the
BLE001 and S110 strict-rule budgets over their ceiling. Narrow to the one
exception the %-format can actually raise and give it a real fallback
instead of silently swallowing it.
* refactor(logging): move color_message substitution into a helper
The two record.color_message stores put LIT011 over its ceiling. Building the
value in a pure helper leaves one store, marked rebind-ok since scrubbing a
record in place is the logging.Filter contract.
Also pins the ordering that makes the substitution safe: it has to run before
args are cleared, which puts it before the extra-field loop that redacts the
result, so a secret arriving through record.args is still scrubbed out of
color_message.
* fix(proxy): group Codex turns under one session id
Codex puts its conversation uuid in an unprefixed `session-id` header
(`session_id` on builds before the codex-api split), so
`get_chain_id_from_headers` never matched it: the `x-<vendor>-session-id`
regex requires an `x-` prefix. Codex also sends no request metadata the
Anthropic `metadata.user_id` path could parse and no traceparent, so every
turn fell through to a freshly generated per-call trace id and landed as its
own row in the logs.
Read the unprefixed `session-id` / `thread-id` (and the older `session_id` /
`conversation_id`) names, gated on the Codex user agent. Those names are
generic enough that an unrelated client could send one meaning something
else, and colliding values across callers would merge their traces, so the
bare-header path stays Codex-only.
* fix(proxy): match every first-party Codex originator
`is_codex_user_agent` tested `startswith("codex_")`, but the Codex TUI sends
`codex-tui` with a hyphen, and often bare with no version at all. Real values
seen in the wild are `codex-tui` and
`codex-tui/0.149.0 (Mac OS 26.5.1; arm64) ghostty/1.3.1 (codex-tui; 0.149.0)`.
codex-rs's own `is_first_party_originator` lists `codex-tui`, `codex_cli_rs`,
`codex_vscode` and a `Codex ` prefix, which agree only on the `codex` stem.
Match that stem plus a separator so no spelling is missed and an unrelated
`codexfoo` client still is. This also repairs the pre-existing gap where
`should_auto_drop_params_for_agentic_cli` (called on the request path at
litellm_pre_call_utils.py:2049) never fired for the Codex TUI.
* refactor(proxy): take headers as a read-only Mapping in the Codex session lookup
The auto-router savings figure was computed only inside the spend-update
writer, downstream of where logging callbacks consume the standard logging
payload, so Datadog-style callbacks never received it. Compute it once in
the payload builder, stamp it as a top-level payload field beside
cost_breakdown, thread it into the spend log metadata, and have both
spend-writer call sites read the recorded value with recomputation as the
fallback for rows written before the field shipped. Internal sub-calls
(classifier, shadow eval) are never stamped, and a caller-forged metadata
value is discarded by the unconditional overwrite.
Resolves LIT-5973
* fix(mcp): strip root_path before matching the per-server MCP route spelling
The 401 challenge for a gateway-managed oauth2 MCP server advertises the
protected-resource metadata URL in the spelling the client connected on, so a
strict RFC 9728 section 3 client lands on a document whose `resource` equals the
URL it actually called. That spelling test compared `_original_path` against the
root-relative `/{server}/mcp` shape, but `_original_path` and `scope["path"]`
are raw request-line paths that still carry the deployment's `root_path`
On a SERVER_ROOT_PATH deployment the prefix therefore made the legacy test fail
and every request fell through to the standard `/mcp/{server}` branch. A client
connecting on `/litellm/github/mcp` was pointed at the standard-pattern
document, which serves `resource = {base}/litellm/mcp/github`; that is not the
URL the client called, so a strict client aborts discovery before the MCP
request fires
Route the path through `get_route_relative_request_path` first, which removes
`root_path` on a segment boundary the same way
`litellm.proxy.auth.auth_utils.get_request_route` already does for the rest of
the MCP auth path, so `/litellmfoo` is not truncated under `root_path=/litellm`
* fix(mcp): make the gateway-managed 401 challenge root-path aware
The gateway-managed authorization_code challenge in process_mcp_request
built its AS-metadata URL from two root-path-unaware pieces:
- it matched the caller's spelling against `scope["_original_path"]`, a
raw request-line path that still carries the deployment prefix, so on a
SERVER_ROOT_PATH deployment the `/mcp/{server}` branch never matched and
every request fell through to the legacy one-segment form
- it hardcoded `/.well-known/oauth-authorization-server` without the
root-path segment the discovery route decorators bake in, so the URL
404'd under a sub-path deployment regardless of which branch was taken
Route the spelling match through get_route_relative_request_path and the
well-known root through well_known_root_suffix, the same two helpers the
discovery route registrations derive their paths from, so the advertised
URL cannot drift from the route that serves it.
Root-mounted deployments are unaffected: both helpers are no-ops when
SERVER_ROOT_PATH is unset.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
A ConnectionError/TimeoutError on one node of the async Redis Cluster client
made redis-py tear down every node's connections and force every other
concurrent caller through the shared reinit lock, turning one client-side
timeout under event-loop saturation into a proxy-wide latency spike while
Redis itself stayed healthy. Confirmed live against a local 3-master
cluster: pausing one node made 100% of concurrent commands to the other
two, untouched nodes stall for the full pause duration; after this change,
zero.
LiteLLMAsyncRedisCluster overrides only the ConnectionError/TimeoutError
branch of _execute_command to reset the one node that failed, mirroring
what a plain non-cluster Redis client already does when a pooled
connection errors. Every other branch (MOVED, ASK, CLUSTERDOWN,
slot-not-covered) is unchanged, since those already carry real evidence
the topology changed.
* feat(ui): add per-key Savings tab to key detail page
Adds a "Savings" tab to the key detail view, showing the same four metrics
and time-series chart as the proxy-wide Cost Optimization view, but scoped
to a single API key.
For org admins, the tab shows the key's full savings across all requests.
Non-admins see only their own requests on the key, with a scope note
explaining the limitation.
Root cause: userDailyActivityCall and userDailyActivityAggregatedCall
never forwarded an api_key query parameter to the backend, even though
both handlers already accept and filter by it.
Changes:
- networking.tsx: Add optional apiKey param to both daily activity call
wrappers (appended to variadic options tuple for backward compatibility).
- costOptimizationUtils.ts: Extract shared metrics helpers (compressionOf,
cachingOf, autorouterOf, savedTokensOf, cacheHitRatio) and shortDate
so both UsageTab and KeySavingsTab use the same formulas and prevent
divergence.
- useDailyActivityRange.ts: Refactor into useScopedDailyActivityRange(
accessToken, scope: {userId, apiKey?}) for reuse-by-parameter unbundling.
Role resolution stays at the entry point (useDailyActivityRange), not in
a scoped caller. Update test expectations for new 6-arg tuple.
- UsageTab.tsx: Simplify by importing extracted helpers and SummaryCard
component instead of defining them inline. No behavioral change.
- key_info_view.tsx: Insert "Savings" tab trigger between "Overview" and
"Settings"; wire TabsContent to new KeySavingsTab component with lazy
mounting (no keepMounted) to defer daily-activity fetch until tab opened.
- NEW: components/shared/SummaryCard.tsx — Shared presenter for four-tile
summary row (label + value + hint + optional info popover). Extracted
from UsageTab so both surfaces show identical tile layout without CSS
divergence.
- NEW: components/templates/KeySavingsTab.tsx — Per-key view with admin/
non-admin scope branching, empty-state messaging, same chart toggles
and info popovers as UsageTab.
- NEW: components/templates/KeySavingsTab.test.tsx — 7 tests covering mount,
loading state, empty state, scoping, and scope-note visibility.
Authorization: No new permission check. Both backends gate api_key filter
by the same user role check that governs the request itself. Non-admins
must send their own user_id and can only see their own keys.
Tests: 6121 pass (1 pre-existing failure unrelated to this change).
Prior art / collision note:
- PR #37570 (budgets tab) lands in same TabsList hunks as "Savings" tab,
but different tab names so conflict trivial if both merge.
- PR #37659 (my own) adds progress/cancelled/cancel to DailyActivityRange,
but this PR uses stable three-field interface from staging.
* fix(ui): scope spend view by the backend's admin-view contract, not all_admin_roles
Greptile flagged org admin handling on the key savings tab. The live bug it
described does not fire today: useAuthorized supplies session-role labels and
all_admin_roles only carries the raw org_admin spelling, so an org admin was
already scoped. That safety was accidental, so replace the predicate with
spendScopeUserId / hasProxyWideSpendView in utils/roles.ts, mirroring the
backend's user_api_key_has_admin_view (proxy admin and admin viewer only, org
admin excluded in both spellings), and use it in both useDailyActivityRange
and KeySavingsTab
Reclassify the KeySavingsTab render test as an integration test per the
repo's unit/integration split, move scope-resolution coverage to roles.test.ts
as a full role matrix, use real session-role values instead of raw ones, and
assert tile totals against non-empty metrics. Replace the nested ternary in
the chart body (frontend-lint error) with flat conditional rendering
* fix(ui): show auto-router savings as the fourth key-savings tile
Cache hit rate had displaced auto-router savings from the fourth slot,
diverging from the org-wide Cost Optimization page's tile order. Match
it: Total / Compression / Prompt caching / Auto-router, with cache hit
rate as a fifth tile.
* fix(ui): drop cache hit rate from the key savings tiles
Keep the four tiles this page is meant to show: total, compression,
prompt caching, and auto-router savings.
* fix(ui): stop an empty api_key from widening a key-scoped activity read
The paginated and aggregated daily-activity wrappers disagreed on an
empty filter value: the paginated one appended it, the aggregated one
coerced it to undefined with || and dropped it. Since the aggregated
call is the one tried first, an empty key hash would have silently
turned a key-scoped read into a proxy-wide one and reported every
key's savings as this key's. Use ?? so both send the filter through
and it matches nothing instead.
* style(ui): satisfy prettier and the inline-object lint rule in key savings tests
* refactor(ui): drop the cacheHitRatio extraction left over from the removed tile
* fix(ui): pass daily-activity filters raw so both transports agree at the null boundary
* refactor(ui): share the savings tiles and totals between both surfaces
The per-key Savings tab and the proxy-wide Cost Optimization tab carried a byte-identical
four-tile block, three long metric-definition strings included, and five identical useMemo
totals. Both now render SavingsTiles and total through useSavingsTotals, so the donut cannot
slice numbers the tile above it disagrees with.
* docs(ui): say request, not mount, in the savings tab comment
The comment claimed mounting eagerly would fire the rollup sweep, which reads as a claim about
the bundle. Only the request is deferred; the module ships with the key page either way.
* test(ui): pin the daily-activity args array against the real caller signatures
The sibling unit test mocks networking, so it checks the positional array against itself and
stays green when the array and a networking signature drift apart. Swapping user_id and api_key
in the aggregated signature alone passes there and fails here on user_id=hash-abc.
* style(ui): hoist the daily-activity query options out of the call argument
The four-property object literal tripped local/no-large-inline-object-arg. The violation predates
this branch, which only moved the line into the annotated range, and the rule count drops 550 to 549.
* fix(ui): drive auto-router usage from the shared cost-optimization time picker
* fix(ui): extend a live-ending benchmarks range to the current UTC day
* fix(router): don't log 'Could not identify azure model' when the deployment name resolves from the cost map
get_router_model_info already falls back to resolving the azure
deployment's model name against the model cost map when base_model is
unset — and for deployments named after real azure models (e.g.
azure/gpt-4o) that resolution returns correct max tokens and costs. The
unconditional ERROR was therefore spurious for exactly the deployments
that need no operator action, and on busy proxies it logs thousands of
times per day per multi-deployment group.
Log at debug when the fallback entry carries usable limits/costs
(membership alone is not enough: Router init auto-registers every
deployment name as a zeroed stub), keep the ERROR otherwise.
Fixes#33172
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(router): use consistent positive checks in azure base_model fallback gate
Review follow-up: token-limit fields used 'is not None' while the cost
field used '> 0' — a cost-map entry explicitly storing 0 limits could
suppress the error log without carrying usable resolution data. All
three checks now require a positive value.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(router): trim fallback gate comment and reuse the shared local_model_cost_map fixture
---------
Co-authored-by: Mihidum Hettiyahandi <55163074+mihidumh@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
A test that asserts on the error inside its own except block passes when the
call stops raising, because nothing runs the handler. That is the exact case
the test exists to catch, so the regression lands green.
Rewrites all 111 such blocks into pytest.raises, which fails when the call
succeeds, and selects PT017 in ruff-tests.toml so no new one lands.
* fix(vertex_ai): convert messages to contents in gemini count_tokens
acount_tokens passed contents=None to the Vertex Gemini countTokens
endpoint when called with messages=, causing a silent zero token count.
The Gemini branch of VertexAITokenCounter.count_tokens never read the
messages parameter, so the request body was {"contents": null}, which
Vertex accepts with HTTP 200 and no totalTokens field.
Convert messages to Gemini contents format using the existing
_gemini_convert_messages_with_history helper when contents is None.
Treat a response without totalTokens as a failure so the caller falls
back to local token counting instead of returning a silent zero.
Fixes#36921
* style: apply ruff format to common_utils.py
Resolves lint CI failure on PR #36981.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* chore: suppress LIT002 on messages fallback for Gemini token counter
Adds `# mutable-ok:` suppression on the `messages or []` fallback passed
to `_gemini_convert_messages_with_history`. The [] is a None-fallback;
the helper signature requires list[AllMessageValues], so a tuple would
violate the type contract. Resolves type-discipline-budget CI failure
on PR #36981.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* chore: suppress reportPrivateUsage on _gemini_convert_messages_with_history import
Adds `# pyright: ignore[reportPrivateUsage]` on the import of the
shared `_gemini_convert_messages_with_history` helper. The function is
already used by gemini/chat, context_caching, and
vertex_and_google_ai_studio_gemini; reusing it here avoids duplicating
the OpenAI-to-Gemini message conversion. Resolves basedpyright budget
CI failure on PR #36981.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(guardrails): resolve guardrail pipelines from the canonical metadata bucket
Policy-resolved pipelines are stored in litellm_metadata on routes like /v1/messages, but the pre_call reader fell back to the caller-supplied metadata field first, so a request that sends its own top-level metadata (Claude Code sends metadata.user_id) skipped every pipeline-managed guardrail.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test(guardrails): drive the pipeline regression through a registered guardrail
Exercise the real executor with a guardrail in litellm.callbacks instead of patching PipelineExecutor.execute_steps at class scope.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(guardrails): read pipeline state from the bucket the policy engine wrote
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(guardrails): type the policy pipeline state accessors
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(guardrails): annotate policy pipeline state casts
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Mantle reports context overflow as a structured 400 validation_error rather than
the plain-text patterns Bedrock itself uses, so callers such as Claude Code that
key reactive compaction off the phrase "prompt is too long" never see it. Detect
the pattern and normalize the message to that phrase.
The dcr_bridge oauth_delegate connect flow completed for a signed-in user
with no litellm-side grant to the target server: every leg returned 200,
the DCR client showed connected, and tools/list then fail-closed to an
empty list with the upstream never contacted (#36358). The authorize leg
now admits the user the way MCP egress will (same reload_admitted_user
constructor, same get_allowed_mcp_servers resolver) and refuses with an
RFC 6749 access_denied redirect naming the remedy, before any upstream
OAuth runs or an envelope is minted. Availability faults (5xx) propagate;
unknown or deactivated users deny fail-closed
Promotes MCPRequestHandler reload_admitted_user to public: it already had
a cross-module consumer in ui_session_utils, and this gate adds a second,
so the private name no longer reflected its use. Ratchets the freed
reportPrivateUsage budget headroom down
A name bound twice keeps only the second binding. In `tests/` that is nearly
always a repeated import, harmless but misleading, and the same rule is what
catches the cases that are not harmless: a local that shadows an import the
module still calls, and a second `def test_x` that quietly replaces the first.
311 of the 344 sites were repeated imports and came out with ruff's own fix.
The remaining 33 needed a decision. Four modules imported a name they never
used because a local definition below already shadowed it. Two comprehensions
bound `call` over `unittest.mock.call`, which those modules import and use.
One test rebound the two module handles its nested reload closure had captured.
One class attribute shadowed an unused `status` import.
The load-test fixtures move to a conftest, which is how pytest is meant to share
them, so the test module no longer imports three fixture names it never calls.
The nine `prisma_client` parameters keep a narrow `noqa`: pytest resolves that
fixture by name before the body runs, so the parameter never shadows anything.
Adds ownership of litellm-proxy-extras/litellm_proxy_extras/migrations so
schema migration changes get a review request.
Also repoints the two existing entries at @yuneng-berri. GitHub's CODEOWNERS
validator was rejecting @yuneng-jiang as an unknown owner, which left the
/ui/ and _experimental/out/ rules inert.
The sweep ran unbounded whenever no config.yaml deployment was present,
deleting the day's sentinel rows for deployments absent from the run's own
view. A written charge records capacity that was reserved, so the only rows
a run may retract are the ones it can reassess: a deployment it scanned and
then declined to charge, because the window closed or the PTU config was
removed. It is now always bounded to the ids it scanned
A config deployment is otherwise keyed by a hash of its resolved
litellm_params, so rotating a credential or editing an endpoint mints a
second identity and the catch-up bills the reservation again under it. Flat
cost is keyed by that id and a written charge is never retracted, so the
duplicate is permanent.
The id is read before set_model_list mints one, or the rule would inspect
the value it is meant to reject. Duplicates are counted once per config
entry across the whole file, so the check is order-independent and an
organization fan-out cannot collide with itself. The refusal names the id
the deployment already uses, since inventing a fresh one starts exactly the
second identity this prevents
* fix(proxy): scan batch records with the content hooks that are not guardrails
Guardrails were made to run on batch uploads by scanning each record through the pre-call hook
with the walk limited to guardrails. That limit exists because the same branch carries the rate
limiters and budget accounting, which must count an upload once rather than once per line. It
also excluded every enforcement hook written as a plain CustomLogger, so prompt-injection
detection, Azure content safety, banned keywords and the blocked-user check never saw a batch
record at all. Content that is a hard 400 online reached the provider verbatim through batch.
A CustomLogger now declares whether its pre-call hook judges the payload or merely counts the
request. The four that judge it opt in, the walk admits them, and both short-circuits learn
about them, including the one that decides whether the file is streamed off disk in the first
place: a proxy configured only with one of these hooks was skipping the scan entirely. Nothing
that counts a request is marked, so an upload still costs one slot and one budget check.
* refactor(proxy): drop the per-hook comment the attribute contract already states
* test(proxy): make the classification a ledger, and pin the wiring with a real hook
The classification test listed the two non-enterprise hooks by hand, so unmarking either
enterprise one changed nothing and the mutation matrix passed with both surviving. It now walks
the hook registries and fails on any pre-call CustomLogger that is on neither side, which also
gives the flag the forcing function it lacked: an enforcement hook added later would otherwise
default to off and silently skip batch records, which is the bug being fixed here.
Nothing exercised the path the bug actually lived on either, since every test raised its own
exception rather than a real hook's. One test now drives the shipped prompt-injection hook
through the scan, which pins the part no synthetic exception reaches: a chained exception reads
as a failure to judge, so refactoring any of these hooks to `raise ... from` would turn every
per-record drop into an aborted upload.
Also records why a hook that rewrites the payload for routing stays unmarked, and that only the
leaf class is consulted.
* test(proxy): set the callback list through monkeypatch rather than writing the global
* fix(proxy): read batch records the same way the upload validation does
The upload validation parses each JSONL line as bytes, where the json module sniffs the
encoding itself and accepts a leading byte order mark or a lone surrogate. The guardrail scan
that runs immediately after decoded each line to text first, which is stricter, so a file the
validation had just accepted could fail the scan. A `.jsonl` written by any of the editors that
emit a BOM, which includes PowerShell's Out-File and classic Notepad, uploaded fine until a
pre_call guardrail was configured and then returned 500 with a decode error and no indication
of which line or why. The scan now parses the same bytes the validation did, and an untouched
record is copied through as the bytes it arrived as rather than re-encoded.
A numeric custom_id was reported as null. The spec asks for a string, but callers do send
numbers, and null leaves the one field a caller reconciles on empty for exactly the records
that need it.
* fix(proxy): read the load-balancing record the same way, so a byte order mark keeps its routing
The first record is parsed to pick a deployment when batch load balancing is on, and it was
decoded to text before parsing, which rejects a leading byte order mark. The lookup returns None
on any parse failure, so such a file silently lost its routing and went to the default provider
rather than the configured one. That was already reachable for an upload no guardrail changed,
since the original bytes are passed straight through, and preserving the mark through a rewrite
widens it. Parsed as bytes now, like the validation and the scan.
* fix(proxy): find the routing record past a blank first line
The upload validation and the guardrail scan both skip blank lines, but deployment selection
read only the first physical line, so a file starting with a blank line lost its routing model
and went to the default provider rather than the configured one. It now skips blanks the way
the other two readers do, reading lazily so a large file is not read past its first record.
* fix(proxy): do not crash deployment selection on a record whose body is not an object
The upload validation checks that a record has a `body`, not that it is an object, so a record
can carry a string or a list there. Deployment selection called `.get` on it unconditionally and
raised, returning 500. That was already reachable for a plain file, and reading past a byte order
mark or a blank first line widened it to files that previously fell through to the default
provider instead. A record whose body names no readable model now resolves to no model, which is
the same answer the default-provider branch already handled.
* fix(proxy): keep a custom_id that cannot be encoded from failing the whole upload
A record identifier is echoed back in the create response. JSON parses a lone surrogate happily
but it cannot be encoded again, so a file the upload validation accepts returned 500 from the
response renderer rather than a report. Unencodable characters are replaced, which leaves every
ordinary identifier untouched and keeps a pathological one reconcilable.
This predates the reader change; reading past a byte order mark only altered which error the
same file produced first.
* fix(proxy): treat a url the parser rejects as one we do not recognize
Resolving a record's call type from its url runs the url through urlsplit, which raises on a few
malformed authorities such as an unclosed bracket. That happens before the try that wraps the
guardrail call, so it escaped the scan and returned 500 on a file the upload validation had just
accepted. An unreadable url is simply one we cannot recognize, which the body-shape fallback
already handles, so the record is still scanned rather than lost.
Reachable on staging today for a proxy running any guardrail. Enabling the scan for a proxy that
runs only a content-enforcing CustomLogger widens it to that configuration too, which is why it
is fixed here rather than left.
