Repository navigation
Conversation
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>
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
CodeAnt-AI DescriptionTry another model when a target cannot handle the request What Changed
Impact
💡 Usage GuideChecking Your Pull RequestEvery 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 AIGot 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: This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code. ExamplePreserve Org Learnings with CodeAntYou 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: This helps CodeAnt AI learn and adapt to your team's coding style and standards. ExampleRetrigger reviewAsk CodeAnt AI to review the PR again, by typing: Check Your Repository HealthTo 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. |
|
| /// 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, |
There was a problem hiding this comment.
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
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
chethanuk
left a comment
There was a problem hiding this comment.
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. ContextWindowExceededis decided inclient.rsbefore 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 setfailure_cooldown_ms = 0, so they do not show this. The run below does. fallback_requestis 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 passedcargo test -p switchyard-llm-client --lib -- fallback candidate capability: 9 passedcargo fmt --all --check: exit 0test_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
- Title. The body and changelog call this a source-breaking change, but the title has no
!.CONTRIBUTING.md:117asks for!or aBREAKING CHANGE:footer, and the PR title can become the squash commit. Usefix(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 suggestsfeat; pick one. - Body, gate section. "The base's one earlier failure of the same test is not claimed" has nothing behind it. In
gate.logthe 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'sTests (Python 3.12)job runsuv run pytest tests/ -v -m "not integration"and its log shows both parameters of the test passing. - 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:523already 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-14lists only context-window and availability failures.docs/routing_algorithms/vgr_routing.md:206-209says only an unavailable capable tier reaches the unverified efficient answer. By reading the code, a capable-tier capability 400 now reaches it too whenefficient_supports_imagesis set. I did not run a VGR route. The new section incontext_window.mdcovers only VGR's efficient-tier call.run.rs:2503: the test namefallback_only_accepts_context_policy_and_unavailable_failuresis 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
LintandRustchecks passing and ongate.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.
What
RoutingFallbackReason::Capability(as_stris"capability").fallback_reason, an upstream 400 whose error message says the model cannot take this input at all now returnsCapability, so the candidate loop tries the next candidate instead of returning the error. The match reusesis_overflow_bodywith 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.context_window.mdandtroubleshooting.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
fallback_reasonincrates/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.allow_policy_fallback. The next candidate receives the request as it arrived, andContextWindowalready skips the flag the same way.Unavailable(no public addition, but the log reason is wrong) and adding anLlmClientErrorvariant (a second public error shape). I picked a new fallback reason.Unavailablewould be a smaller diff if a reviewer prefers it.RoutingFallbackReasonis notnon_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.Which issue
None linked in this fork PR.
Test plan
fallback_reasontable with positives, near-miss negatives (malformed image, size limits, image generation, wrong JSON field), status and policy-flag cases.run.rs: next candidate is tried and gets the fallback request; any other 400 stays terminal.cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warningsandcargo test --workspaceall 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 aretest_closed_book_proxy_allows_switchyard_service_and_blocks_cheat_sourceandtest_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 jobTests (Python 3.12)runsuv run pytest tests/ -v -m "not integration"and its log shows both parameters of this test passing.fork/goal-13-base(bdd7647). I removed that worktree and nothing else.