Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
before awaiting it. See the
[host integration guide](docs/getting_started.md#drive-the-algorithm).
([#948](https://github.com/NVIDIA-NeMo/Switchyard/pull/948))
- **Fallback on capability rejections** - an upstream 400 saying the model cannot
take this input, such as image input on a text-only model, now moves the answer
call to the route's next candidate. Calls made during routing still stop on it.
**Rust source-breaking change:** callers that match exhaustively on
`RoutingFallbackReason` must handle the new `Capability` variant.

### Changed

Expand Down
158 changes: 158 additions & 0 deletions crates/libsy-llm-client/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use switchyard_protocol::{
};
use switchyard_translation::prepare_request_for_target;

use crate::error::is_overflow_body;
use crate::observation::{ModelCallObservation, RunObservation, RunObserver};
use crate::{metrics, observability};

Expand Down Expand Up @@ -518,6 +519,15 @@ async fn buffer_routing_stream(
Ok((stream::iter(events.into_iter().map(Ok)).boxed(), usage))
}

// Upstream wording for "this model cannot take this input at all". Kept narrow:
// a miss keeps today's error, a false positive moves the request to another target.
const CAPABILITY_PHRASES: &[&str] = &[
"does not support image input", // Fireworks, Moonshot
"is not a multimodal model", // vLLM; the model name prefix varies
"image input is not supported", // llama.cpp wording, when a proxy answers 400
"image_url is only supported by certain models", // OpenAI
];

/// Whether a failed candidate is worth routing around.
fn fallback_reason(
error: &LibsyError,
Expand All @@ -538,6 +548,13 @@ fn fallback_reason(
if policy_denial && !allow_policy_fallback {
return None;
}
// Reuses the overflow helper's message parsing with non-overflow phrases.
if *status == StatusCode::BAD_REQUEST
&& !policy_denial
&& is_overflow_body(body, |_| false, CAPABILITY_PHRASES)
{
return Some(RoutingFallbackReason::Capability);
}
((*status == StatusCode::BAD_REQUEST && policy_denial)
|| matches!(
*status,
Expand Down Expand Up @@ -1211,6 +1228,7 @@ mod tests {
ContextWindow,
ContentPolicy(StatusCode),
Unauthorized,
BadRequest(&'static str),
StreamSuccess,
MidStreamError,
}
Expand Down Expand Up @@ -1241,6 +1259,10 @@ mod tests {
status: StatusCode::UNAUTHORIZED,
body: "unauthorized".to_string(),
}),
FirstOutcome::BadRequest(body) => Err(LlmClientError::UpstreamHttp {
status: StatusCode::BAD_REQUEST,
body: body.to_string(),
}),
FirstOutcome::StreamSuccess => Ok(stream_response(vec![
LlmResponseChunk::TextDelta {
index: 0,
Expand Down Expand Up @@ -2475,9 +2497,13 @@ mod tests {
Ok(())
}

const IMAGE_REJECT: &str = r#"{"error":{"type":"invalid_request_error","message":"This model does not support image inputs"}}"#;

#[test]
fn fallback_only_accepts_context_policy_and_unavailable_failures() {
let error = |source| LibsyError::client_call("target", source);
let capability = Some(RoutingFallbackReason::Capability);
assert_eq!(RoutingFallbackReason::Capability.as_str(), "capability");
assert_eq!(
fallback_reason(
&error(LlmClientError::ContextWindowExceeded {
Expand Down Expand Up @@ -2517,6 +2543,56 @@ mod tests {
),
(r#"{"error":{"message":"content_policy_violation"}}"#, None),
("content_policy_violation", None),
(IMAGE_REJECT, capability),
(
r#"{"error":{"message":"judge is not a multimodal model","type":"BadRequestError","code":400}}"#,
capability,
),
(
r#"{"error":{"message":"image input is not supported - hint: provide the mmproj"}}"#,
capability,
),
(
r#"{"error":{"message":"Invalid content type. image_url is only supported by certain models."}}"#,
capability,
),
(
r#"{"object":"error","message":"m is not a multimodal model"}"#,
capability,
),
("This model does not support image inputs", capability),
(
r#"{"error":{"message":"THIS MODEL DOES NOT SUPPORT IMAGE INPUT"}}"#,
capability,
),
(
r#"{"error":{"code":"invalid_request_error","message":"does not support image input"}}"#,
capability,
),
(r#"{"error":{"message":"invalid image data"}}"#, None),
(
r#"{"error":{"message":"This model does not support images larger than 20MB"}}"#,
None,
),
(
r#"{"error":{"message":"does not support image generation"}}"#,
None,
),
(r#"{"error":{"message":"image too large"}}"#, None),
(
r#"{"error":{"message":"Multimodal processing failed"}}"#,
None,
),
(r#"{"error":{"message":"bad request"}}"#, None),
(
r#"{"error":{"message":"response_format json_schema is not supported"}}"#,
None,
),
(
r#"{"error":{"message":"rejected"},"metadata":"does not support image input"}"#,
None,
),
(r#"{"message":"does not support image input"}"#, None),
] {
assert_eq!(
fallback_reason(
Expand Down Expand Up @@ -2549,6 +2625,39 @@ mod tests {
None
);
}

let mmproj =
r#"{"error":{"message":"image input is not supported - hint: provide the mmproj"}}"#;
let policy = r#"{"error":{"code":"content_policy_violation","message":"does not support image input"}}"#;
let unavailable = Some(RoutingFallbackReason::Unavailable);
for (status, body, allow_policy_fallback, expected) in [
(401, IMAGE_REJECT, true, None),
(404, IMAGE_REJECT, true, None),
(422, IMAGE_REJECT, true, None),
(403, IMAGE_REJECT, true, unavailable),
(408, IMAGE_REJECT, true, unavailable),
(429, IMAGE_REJECT, true, unavailable),
(500, IMAGE_REJECT, true, unavailable),
(503, IMAGE_REJECT, true, unavailable),
(500, mmproj, true, unavailable),
// A policy denial keeps its own result even when the message also matches.
(400, policy, true, unavailable),
(400, policy, false, None),
// The next candidate serves the arrived request, so the flag does not apply.
(400, IMAGE_REJECT, false, capability),
] {
assert_eq!(
fallback_reason(
&error(LlmClientError::UpstreamHttp {
status: StatusCode::from_u16(status).expect("valid status"),
body: body.to_string(),
}),
allow_policy_fallback,
),
expected,
"{status} {body} {allow_policy_fallback}"
);
}
}

#[tokio::test]
Expand Down Expand Up @@ -2579,6 +2688,29 @@ mod tests {
);
assert_eq!(response.served_model().map(ModelId::as_str), Some("strong"));

// A model that cannot take the input at all also permits another candidate.
let (client, result) = run_candidates(FirstOutcome::BadRequest(IMAGE_REJECT)).await;
let (_, response) = result?;
assert_eq!(
&*client.calls.lock(),
&[ModelId::from("weak"), "strong".into()]
);
assert_eq!(response.served_model().map(ModelId::as_str), Some("strong"));

// Any other 400 stays terminal, so the second candidate is untouched.
let (client, result) = run_candidates(FirstOutcome::BadRequest("failed")).await;
assert!(matches!(
result,
Err(LibsyError::ClientCall {
source: LlmClientError::UpstreamHttp {
status: StatusCode::BAD_REQUEST,
..
},
..
})
));
assert_eq!(&*client.calls.lock(), &[ModelId::from("weak")]);

// Authentication failure is not retryable, so the second candidate is untouched.
let (client, result) = run_candidates(FirstOutcome::Unauthorized).await;
assert!(matches!(
Expand Down Expand Up @@ -2667,6 +2799,32 @@ mod tests {
Ok(())
}

#[tokio::test]
async fn capability_rejection_gives_the_next_candidate_the_fallback_request() -> Result<()> {
let client = Arc::new(CandidateClient {
calls: Mutex::new(Vec::new()),
requests: Mutex::new(Vec::new()),
first: FirstOutcome::BadRequest(IMAGE_REJECT),
});
run(
Arc::new(RewritingAlgorithm {}),
ClientRouter::single(client.clone()),
request(),
to_category_map(&["weak", "strong"]),
None,
)
.await?;
assert_eq!(
&*client.calls.lock(),
&[ModelId::from("weak"), "strong".into()]
);
assert_eq!(
client.requests.lock()[1].llm_request.messages,
request().llm_request.messages
);
Ok(())
}

#[tokio::test]
async fn retry_exhaustion_falls_back_and_optional_cooldown_skips_failed_backend() -> Result<()>
{
Expand Down
3 changes: 3 additions & 0 deletions crates/protocol/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ pub enum RoutingFallbackReason {
ContextWindow,
/// The selected target was unavailable after its client retries finished.
Unavailable,
/// The selected target rejected the request as something it cannot serve at all.
Capability,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Downstream crates that exhaustively match this public enum stop compiling after adding Capability, making this a source-breaking API change.

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes · 🏷️ Api mismatch

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** crates/protocol/src/client.rs
**Line:** 139:139
**Comment:**
	*Api Mismatch: Downstream crates that exhaustively match this public enum stop compiling after adding `Capability`, making this a source-breaking API change.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

}

impl RoutingFallbackReason {
Expand All @@ -143,6 +145,7 @@ impl RoutingFallbackReason {
match self {
Self::ContextWindow => "context_window",
Self::Unavailable => "unavailable",
Self::Capability => "capability",
}
}
}
Expand Down
Loading
Loading