Repository navigation
feat(frontend): refuse media before routing for text-only models - #17874
Conversation
Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
|
No actionable comments were generated in the recent review. WalkthroughThis change adds opt-in frontend rejection for requests containing image, video, or audio content. The rejection status is configurable. OpenAI-compatible and Anthropic routes return refusals before generation or model lookup and classify them as ChangesFrontend media refusal
Estimated code review effort: 4 (Complex) | ~45 minutes Severity of issue fixed: Medium Merge Risk:
|
luckyq
left a comment
There was a problem hiding this comment.
Summary: The PR adds an opt-in --reject-media gate that classifies deserialized user/tool content parts and refuses image/video/audio requests after validation and before engine lookup on chat, responses and /v1/messages, with a configurable 4xx/5xx status and an unsupported_modality metric. The chat path is correct and well tested. Two things need attention before merge: the single merge conflict sits on the Anthropic error middleware whose main version rewrites 415 to 400, and no test pins the default-415 Anthropic refusal, so a straightforward conflict resolution would silently break the documented status; and Anthropic URL-source images are rejected by conversion with 400 before the refusal runs.
Blocking
lib/llm/src/http/service/anthropic.rs:132— PR intercepts only 422; main (#17367) rewrites 422 and 415 to 400, and this function is the one conflicting hunk → after rebase the default-415/v1/messagesrefusal becomes 400 unless the pass-through is kept → addcheck_reject_media_anthropic(StatusCode::UNSUPPORTED_MEDIA_TYPE)(tests athttp-service.rs:4313only run 429 and 422) and keep shaped-body pass-through for 415 when resolving.
Non-blocking
anthropic.rs:570—imageblocks withsource.type == "url"failtry_into(types.rs:303, "only base64 is supported") with 400 beforemedia_refusalruns, so the configured gateway-failover status never fires for the most common Anthropic SDK shape (P2; Anthropic counterpart of jh-nv's Responsesfile_idpoint).openai.rs:1063—smart_json_error_middlewarenow passes any handler-rendered 422 through unconditionally, so a backend 422 forwarded viaForwardClientErrorreaches clients as 422 instead of 400 even with--reject-mediaoff (P3, unannounced).anthropic.rs:584— engine lookup moved after conversion/validation unconditionally: with the flag off, unknown model + invalid body now returns 400 where main returned 404 (P3, unannounced; fine if intended).
Checked: diff read against merge base 7c195fd and git merge-tree against main (one conflict, anthropic_error_middleware); classification covers every non-text variant in dynamo-protocols 8.4.0 user/tool parts, text-only arrays and null assistant content pass; ordering readiness → validation → refusal → lookup on all three APIs; status range enforced in Rust new(), Python validate() and env parsing (invalid env warns, no panic); default off short-circuits; UNSUPPORTED_MODALITY mirrored in Rust and Python name files, exhaustive matches updated; refusal is a pre-stream HTTP error; one failure + one error mark per refusal. Tests not run; CI green on 6969cca.
Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
luckyq
left a comment
There was a problem hiding this comment.
Approving at 4aab4fe; this lifts my earlier request for changes.
Addressed
anthropic_error_middlewarekeeps main's request-id handling and passes shaped{"type":"error"}bodies through with the status untouched;test_reject_media_preserves_anthropic_default_415pins the default.- Anthropic media is now classified on the raw body before
try_into(anthropic/validation.rs::media_content_type), so URL-source images in user content andtool_resultget the configured status; tests cover base64/url × top-level/nested. - Engine lookup restored before conversion;
test_reject_media_disabled_preserves_anthropic_model_lookup_precedencepins 404 with the flag off. - The unconditional 422 pass-through in
smart_json_error_middlewareis documented as an intentional default change and tested. - Responses classification moved before conversion (
responses_media_content_type), coveringfile_idimages for every role andfunction_call_output, which also closes jh-nv's point.
Non-blocking
lib/llm/src/http/service/anthropic.rs:439— Anthropic refusal now runs before typed validation (validate_anthropic_messages/ tools /max_tokens), whereas chat and responses keep validation → refusal; media + invalidtools[0].typeyields 415 instead of 400. Fine for gateway failover, just an inconsistency to be aware of (P3).lib/llm/src/http/service/openai.rs:480—Item::ComputerCallOutputscreenshots are not classified and the converter skips them; computer-use is unsupported end to end, so follow-up at most (P3).
Checked: interdiff 6969cca → 4aab4fe (non-test changes confined to anthropic.rs, anthropic/validation.rs, openai.rs, docs; preprocessor.rs delta byte-identical); single refusal call site per handler, one failure record + one error mark per refusal; readiness still precedes refusal on chat/responses; no new unwrap/panic. The red rust-tests (.) is the unrelated http-transcriptions connection-error flake (upload_too_large_…), seen on other PRs today; rerunning it.
4aab4fe to
2fa369c
Compare
✅ Dynamo PR CI passed — run 41751352487 (attempt 1) on
|
Absorbs the 95 upstream commits since 464ff29. Each upstream change lands in the crate that owns that code at the stack head; the stack's moves stay intact. Router-touching PRs and where they landed: - ai-dynamo#17712 host-path routing modes for worker-selection plugins: &self + Send + Sync plugin traits and WorkerInputs::ACTIVE_REQUESTS (deprecated OCCUPANCY alias) in dynamo-worker-selection and dynamo-router-protocol; HostedWorkerPicker and HostedSelectionRequest in dynamo-worker-selection (hosted.rs), reporting SelectionFailure, which the kernel converts into KvSchedulerError; the InstanceSelector adapter in lib/llm (HostedInstanceSelector); the compat test's hosted coverage in kv-router's prelude test. - ai-dynamo#17713 random/round-robin/least-loaded as builtin plugins: router_modes in dynamo-router-policies (renamed from dynamo-custom-policy-builtin) importing the prelude; RouterPluginRegistry::resolve_type in kv-router; the frontend's hosted path through dynamo_worker_selection::host. - ai-dynamo#17899 1.7.0 removals: taken whole; the alias and the setter were gone at the head; power-of-two never arrives. - ai-dynamo#17657 single-source fetch continuations: KvTransferSourceInterval and source_intervals in dynamo-kv-indexer, the bucketed summary in dynamo-kv-hints, the interval filter in the kernel's resolver; tests in the kernel. - ai-dynamo#17856 KVCR hints in offline replay: mocker and lib/llm bodies; supports_kv_transfer_chain_retention on the flat config; its indexer-vs-config test in kv-router's core host tests. - ai-dynamo#17794 vLLM partial-tail filter and ai-dynamo#17126 SGLang engine-owned multimodal pads: dynamo-kv-events zmq_wire (mm_exclusion.rs); the codec-plus-indexer tests in dynamo-kv-indexer; the engine_owned_mm_identity worker flag through the kernel's WorkerRequest, kv-router's indexer registry and run config, and the bindings. - ai-dynamo#17653 SGLang 0.5.23: pad constant docs in dynamo-multimodal. - ai-dynamo#15333, ai-dynamo#17874, ai-dynamo#17484, ai-dynamo#17688, ai-dynamo#17237, ai-dynamo#17894, ai-dynamo#17958: host-side changes merged in place (bindings, lib/llm, CI). Lockfiles regenerated; no shim reintroduced. Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
Overview
Text-only frontends can opt in to refusing media before routing or opening an SSE stream.
--reject-mediadefaults to off;--reject-media-status-codeaccepts 400–599 and defaults to 415 so gateways can choose a failover status.Key Changes
Approximately 331 added production lines, excluding tests, documentation, comments, and whitespace; no new dependencies.
unsupported_modalityindependently of their HTTP status. This reuses the request counter family, with at most six additional series per model across three endpoints and unary/streaming requests.Validation
input_imagewith onlyfile_idAll 44 HTTP service tests pass on the integrated revision (
cargo test -p dynamo-llm --no-default-features --test http-service), including seven media regression tests. Removing the review fixes and retaining main's conflicting middleware produces six regression failures; restoring the fixes makes the full suite pass again (7.71 seconds). Changed-page documentation lint passes. The Fern CLI is unavailable locally, so Fern configuration/link checks were not rerun for this revision.Initial implementation validation on 6969cca included 41 HTTP integration, 477 HTTP unit, 7 configuration unit, and 41 Python configuration tests, plus binding cargo check. That initial Python run used an existing native overlay. No rebuilt native Python roundtrip or live GPU deployment has been run for this update.
Issues and Ongoing Work
Pull Request Checklist
/pr-checklistskill.