Skip to content

feat(libsy-llm-client)!: try next candidate on capability 400 - #14

Open
chethanuk wants to merge 1 commit into
goal-13-basefrom
fix/fallback-on-capability-reject
Open

chethanuk wants to merge 1 commit into
goal-13-basefrom
fix/fallback-on-capability-reject

Conversation

@chethanuk

@chethanuk chethanuk commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

What

  • Add RoutingFallbackReason::Capability (as_str is "capability").
  • In fallback_reason, an upstream 400 whose error message says the model cannot take this input at all now returns Capability, so the candidate loop tries the next candidate instead of returning the error. The match reuses is_overflow_body with a short phrase list: does not support image input, is not a multimodal model, image input is not supported, image_url is only supported by certain models.
  • Docs: context_window.md and troubleshooting.md, plus a changelog entry.

Why

A route can mix targets that take images with targets that do not. A text-only target answers 400 on image input. The loop treated every such 400 as final, so a target that could serve the request was never tried.

Notes for reviewers

  • Start at fallback_reason in crates/libsy-llm-client/src/run.rs. Overflow, rate limit, plain invalid-request 400s and non-400 statuses give the same result as before. A policy denial keeps its own result. If the last candidate rejects, the caller gets the error.
  • The check ignores allow_policy_fallback. The next candidate receives the request as it arrived, and ContextWindow already skips the flag the same way.
  • The design has no maintainer sign-off. I considered reusing Unavailable (no public addition, but the log reason is wrong) and adding an LlmClientError variant (a second public error shape). I picked a new fallback reason. Unavailable would be a smaller diff if a reviewer prefers it.
  • RoutingFallbackReason is not non_exhaustive, so this is a source-breaking change for external code that matches it exhaustively. The changelog says so. I did not check for such callers outside this repo.
  • Limits: the check is per request, so a rejecting target is tried again on the next request. Calls made during routing still stop on the 400. A tool schema the target rejects is a separate kind of 400 and is not handled here. A wording not in the phrase list keeps today's behaviour, and a false positive moves the request to the next target, so the caller may get an answer from a different, possibly more expensive model.
  • Non-test lines added: 46.

Which issue

None linked in this fork PR.

Test plan

  • Extended the fallback_reason table with positives, near-miss negatives (malformed image, size limits, image generation, wrong JSON field), status and policy-flag cases.
  • Candidate tests in run.rs: next candidate is tried and gets the fallback request; any other 400 stays terminal.
  • Two server tests against a mock upstream: the second target serves after a capability 400, and a routing-phase call keeps the 400.
  • cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings and cargo test --workspace all exited 0 on the branch.

Gate substitutions:

  • uv run pytest tests/ -v: branch exit 1, clean base exit 1. Both stop at the first failure, test_closed_book_proxy_allows_switchyard_service_and_blocks_cheat_source, because the user cannot reach the Docker socket (permission denied). Same failing id on both.
  • uv run pytest tests/ -v -o addopts="" (native equivalent): branch exit 1 with 3 failures, base exit 1 with 2 failures. The shared two are test_closed_book_proxy_allows_switchyard_service_and_blocks_cheat_source and test_open_book_proxy_allows_raw_github_hostname, both Docker-denied.
  • tests/test_nemo_gym_run.py::test_interrupt_stops_only_owned_children[2-130] is the third branch failure (20s subprocess timeout) and did not fail on base. Rerun alone on the branch it passed (exit 0, 2 passed). I report it as flaky under load. The fork CI job Tests (Python 3.12) runs uv run pytest tests/ -v -m "not integration" and its log shows both parameters of this test passing.
  • The base checkout was a temporary worktree at fork/goal-13-base (bdd7647). I removed that worktree and nothing else.

A route can mix targets that accept images with targets that do not.
A text-only target answers HTTP 400 on image input, and the candidate
loop treated that as terminal, so a target that could serve the request
was never tried.

Recognize a short list of upstream phrases on 400 responses and report
a new Capability fallback reason, which gives an accurate log label.
Other 400s, policy denials and other statuses keep their results. The
check is per request and ignores allow_policy_fallback, because the
next candidate gets the request as it arrived. A rejecting target is
tried again on the next request, and calls made during routing still
stop on the 400.

Signed-off-by: ChethanUK <chethanuk@outlook.com>
@codeant-ai

codeant-ai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

CodeAnt  CodeAnt Review Status

✅  Review completed

Status Commit Started (UTC) Finished (UTC)
✅  #506bf Reviewed your PR 6468bf3 Oct 31, 2026 · 03:02 03:07

@codeant-ai

codeant-ai Bot commented Oct 31, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Oct 31, 2026
@codeant-ai

codeant-ai Bot commented Oct 31, 2026

Copy link
Copy Markdown

CodeAnt-AI Description

Try another model when a target cannot handle the request

What Changed

  • When an answer target returns HTTP 400 indicating it cannot handle the request, the route now tries its next candidate.
  • Other HTTP 400 errors still stop immediately. If every candidate rejects the request, the caller receives the last target’s error.
  • Calls made during routing still stop on the rejection.

Impact

✅ Image requests can reach a compatible model
✅ Fewer failures on routes with mixed model capabilities
✅ Invalid requests still fail without unnecessary retries

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.2

🚀 View preview at
https://chethanuk.github.io/Switchyard/pr-preview/pr-14/

Built to branch gh-pages at 2026-10-31 03:03 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

/// 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
👍 | 👎

@codeant-ai

codeant-ai Bot commented Oct 31, 2026

Copy link
Copy Markdown

CodeAnt  CodeAnt PR Risk

🟡  Medium Risk

  • The PR needs attention before merging because adding Capability to the public enum breaks downstream exhaustive matches; the changelog identifies this as source-breaking.

Assessed commit: 6468bf3099f5

@chethanuk chethanuk left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Independent review of 6468bf3 against goal-13-base (bdd7647).

Result: the code is correct and needs no change. I am asking for revisions to the PR title and body only. GitHub does not allow approving your own PR, so this is posted as a comment.

Correctness

I traced call_first_available, fallback_reason, is_overflow_body and the backend cooldown, and found no defect.

  • The last-candidate guard runs before fallback_reason, so the caller still gets the error when no target is left.
  • ContextWindowExceeded is decided in client.rs before this code, so overflow results are unchanged.
  • A 400 is not a transient failure in is_transient_failure, so a capability rejection does not put the target into failure cooldown. The server tests set failure_cooldown_ms = 0, so they do not show this. The run below does.
  • fallback_request is set only by VGR, to the arrived request, so the "as it arrived" claim holds in the server. Candidate-specific requests still take priority over it.

I also ran the built switchyard-server against a small local mock upstream, with default cooldown and no code changes. The earlier tests send a text prompt; these requests carried real image parts. The mock returned 400 for image requests to the text-only targets.

Request Result Upstream calls
Chat with image, sent twice 200 from model/strong both times weak, strong, weak, strong
Chat with image, streaming 200, ends with [DONE] weak, strong
Messages with base64 image 200 from model/strong weak, strong
Responses with input_image 200 from model/strong weak, strong
Every target rejects, with different bodies 400 carrying the last target's message weak, then the other target
Route with vision = false 400, "declares vision = false", unsupported_capability none

The log line had reason="capability" on each fallback, and routing_fallbacks stayed at two keys with zeros. That covers two gaps noted in earlier rounds: no test sent a real image, and the last-target error was only asserted with identical bodies. The troubleshooting row's "declares vision = false" wording also matches the real message.

Tests and scope

The new tests assert behaviour at the HTTP boundary and the fallback_reason table pins the near misses. Non-test lines are 46 by my count, matching the body. The diff stays within the stated change, and gh pr diff matches the local branch and the saved diff.patch.

Reruns on this commit:

  • cargo test -p switchyard-server --test server -- capability_rejection: 2 passed
  • cargo test -p switchyard-llm-client --lib -- fallback candidate capability: 9 passed
  • cargo fmt --all --check: exit 0
  • test_interrupt_stops_only_owned_children, alone, 5 runs: 2 passed each time

Fork CI finished with every check passing except Deploy to GitHub Pages, which was skipped. That includes Rust, Tests (Python 3.12), Tests (Python 3.14) and Build docs.

Please revise

  1. Title. The body and changelog call this a source-breaking change, but the title has no !. CONTRIBUTING.md:117 asks for ! or a BREAKING CHANGE: footer, and the PR title can become the squash commit. Use fix(libsy-llm-client)!: try next candidate on capability 400. Some earlier source-breaking commits skipped the marker and the title check is advisory, so this is a rule the repo does not strictly enforce. The changelog puts the entry under Added, which also suggests feat; pick one.
  2. Body, gate section. "The base's one earlier failure of the same test is not claimed" has nothing behind it. In gate.log the only base run shows that test passing, and I found no other record of a base failure in the row's papers or goal files. Remove the sentence, or add the log. In its place, cite this stronger evidence: CI's Tests (Python 3.12) job runs uv run pytest tests/ -v -m "not integration" and its log shows both parameters of the test passing.
  3. Body, limits. "A false positive costs one extra upstream call" understates the effect. A false positive can also mean the answer comes from a different target, possibly a more expensive one, which the comment at run.rs:523 already says. Suggested: "a false positive moves the request to the next target, so the caller may get an answer from a different model."

The other gate statements match gate.log: fmt, clippy and cargo test --workspace at rc 0; pytest rc 1 on both branch and base; 3 failures against 2 with -o addopts=""; the two shared failures are Docker permission errors; the rerun alone passed.

Optional

  • docs/routing_algorithms/stage_router_routing.md:13-14 lists only context-window and availability failures.
  • docs/routing_algorithms/vgr_routing.md:206-209 says only an unavailable capable tier reaches the unverified efficient answer. By reading the code, a capable-tier capability 400 now reaches it too when efficient_supports_images is set. I did not run a VGR route. The new section in context_window.md covers only VGR's efficient-tier call.
  • run.rs:2503: the test name fallback_only_accepts_context_policy_and_unavailable_failures is now incomplete.
  • "from the original report" in the limits has no referent for a reader of this PR, since nothing is linked. "Unsupported tool schema rejections are not handled" says the same thing.
  • The new changelog entry has no PR link, unlike its neighbour. That is right for a fork PR and needs adding if the change goes further.

Rules check

The title, body, commit message and added diff lines have no issue reference, upstream link or mention. The commit carries one DCO sign-off and no AI trailer. The body states that the design has no maintainer sign-off. The base is goal-13-base.

Not checked

  • Live providers: all four phrases rest on the design's sources, and the rejection text in my mock was mine.
  • Docker-backed tests, which cannot run on this machine.
  • Local clippy and full workspace tests, which I did not rerun; I rely on CI's Lint and Rust checks passing and on gate.log.
  • External code that matches RoutingFallbackReason, and hosts driving the algorithm through the C or Python interfaces.
  • The CodeAnt comment on the enum repeats the source-break point, which the body already states. I did not read its suggested fix.

@chethanuk chethanuk changed the title fix(libsy-llm-client): try next candidate on capability 400 feat(libsy-llm-client)!: try next candidate on capability 400 Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant