fix(libsy): harden the classifier judge path - #351
Open
gburachas wants to merge 3 commits into
Open
Conversation
Signed-off-by: Giedrius Burachas <gburachas@nvidia.com>
Signed-off-by: Giedrius Burachas <gburachas@nvidia.com>
…ntent Signed-off-by: Giedrius Burachas <gburachas@nvidia.com>
WalkthroughThe classifier now reinforces windowed prompts, supports optional judge deadlines across Rust, Python, and server configuration, bounds judge consultations, and redacts untrusted error content from judge logs. ChangesWindowed prompt reinforcement
Configurable judge deadlines
Judge error redaction
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/libsy/src/algorithms/util/robustness.rs`:
- Around line 74-75: Update the error matching logic in the surrounding
robustness function to add an explicit LlmClientError::General(_) arm returning
the same redacted "client call failed" message, while retaining the wildcard arm
for future non-exhaustive variants.
- Line 34: Update the error-to-string handling around the
LibsyError::AlgorithmError match arm so it no longer returns the unconstrained
message field verbatim. Replace it with a fixed safe summary, and apply the same
treatment to LlmClientError::Configuration::message; only include explicitly
whitelisted structural fields if needed.
In `@crates/switchyard-server/src/config.rs`:
- Around line 397-403: Ensure judge_deadline_ms is not silently ignored: either
propagate it through escalation and custom routes or keep it capability-only and
reject it during mode validation. Update the relevant configuration validation
flow around the mode handling block, and document the supported mode scope in
docs/reference/toml_schema.md at line 121.
In `@docs/reference/toml_schema.md`:
- Line 121: Update the judge_deadline_ms entry in the TOML schema reference to
state that configured values must be greater than 0, while preserving its
existing millisecond deadline and fallback behavior description.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 11fbe7ac-a93b-4219-8ce6-08fb14cbfb42
📒 Files selected for processing (7)
crates/libsy/src/algorithms/llm_class.rscrates/libsy/src/algorithms/util.rscrates/libsy/src/algorithms/util/llm_judge.rscrates/libsy/src/algorithms/util/robustness.rscrates/switchyard-py/src/libsy_bindings.rscrates/switchyard-server/src/config.rsdocs/reference/toml_schema.md
| ) | ||
| } | ||
| // Built by libsy from static text plus a serde position, so safe verbatim. | ||
| LibsyError::AlgorithmError { message } => message.clone(), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the error definitions and all direct constructors.
rg -n -C 5 'enum\s+(LibsyError|LlmClientError)\b' crates
rg -n -C 3 'LibsyError::AlgorithmError\s*\{|LlmClientError::Configuration\s*\{' cratesRepository: NVIDIA-NeMo/Switchyard
Length of output: 41652
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- error definitions ---'
cat -n crates/libsy/src/error.rs | sed -n '1,95p'
cat -n crates/protocol/src/client.rs | sed -n '1,125p'
printf '%s\n' '--- robustness implementation and tests ---'
cat -n crates/libsy/src/algorithms/util/robustness.rs | sed -n '1,165p'
printf '%s\n' '--- dynamic AlgorithmError constructors ---'
rg -n -C 8 'LibsyError::AlgorithmError\s*\{' crates/libsy/src crates/protocol/src crates/libsy-llm-client/src
printf '%s\n' '--- dynamic Configuration constructors ---'
cat -n crates/libsy-llm-client/src/client.rs | sed -n '125,155p;375,405p;600,630p'
cat -n crates/libsy-llm-client/src/run.rs | sed -n '232,255p'
cat -n crates/switchyard-translation/src/helpers.rs | sed -n '82,102p'Repository: NVIDIA-NeMo/Switchyard
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
roots = [Path("crates/libsy/src"), Path("crates/libsy-llm-client/src"), Path("crates/switchyard-translation/src")]
pattern = re.compile(r"(LibsyError::AlgorithmError|LlmClientError::Configuration)\s*\{")
for root in roots:
for path in root.rglob("*.rs"):
lines = path.read_text(errors="replace").splitlines()
for i, line in enumerate(lines):
if not pattern.search(line):
continue
window = "\n".join(lines[i:min(i + 10, len(lines))])
msg = re.search(r"message\s*:\s*(.*)", window)
if msg:
rhs = msg.group(1).strip()
kind = "literal/static" if rhs.startswith('"') else "dynamic/propagated"
print(f"{path}:{i+1}: {kind}: {rhs[:180]}")
PY
printf '%s\n' '--- relevant message inputs and call sites ---'
rg -n -C 6 'invalid_weights\(|failed to aggregate efficient response|parse_json_verdict|response schema is invalid|Configuration \{' \
crates/libsy/src crates/libsy-llm-client/src crates/switchyard-translation/srcRepository: NVIDIA-NeMo/Switchyard
Length of output: 30449
Do not log message fields verbatim. AlgorithmError::message and LlmClientError::Configuration::message accept unconstrained dynamic content, including response, request, and upstream error details. Return fixed summaries for these variants, or whitelist only structural fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/libsy/src/algorithms/util/robustness.rs` at line 34, Update the
error-to-string handling around the LibsyError::AlgorithmError match arm so it
no longer returns the unconstrained message field verbatim. Replace it with a
fixed safe summary, and apply the same treatment to
LlmClientError::Configuration::message; only include explicitly whitelisted
structural fields if needed.
Comment on lines
+74
to
+75
| // `General` and any future variant are unvetted by construction. | ||
| _ => "client call failed".to_string(), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify enum variants and whether Rust requires a wildcard arm.
rg -n -C 12 'enum\s+LlmClientError\b|non_exhaustive|LlmClientError::General' cratesRepository: NVIDIA-NeMo/Switchyard
Length of output: 27974
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- LlmClientError definition ---'
sed -n '18,95p' crates/protocol/src/client.rs
printf '%s\n' '--- robustness implementation ---'
sed -n '1,90p' crates/libsy/src/algorithms/util/robustness.rs
printf '%s\n' '--- workspace/package boundaries ---'
rg -n -C 3 'name\s*=\s*"switchyard-protocol"|name\s*=\s*"libsy"|switchyard_protocol' crates/protocol/Cargo.toml crates/libsy/Cargo.toml crates/libsy/src/algorithms/util/robustness.rsRepository: NVIDIA-NeMo/Switchyard
Length of output: 8475
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- remaining LlmClientError variants ---'
sed -n '85,125p' crates/protocol/src/client.rs
printf '%s\n' '--- all safe_client_error arms and enum constructors ---'
rg -n -C 2 'LlmClientError::|pub enum LlmClientError' crates/libsy/src/algorithms/util/robustness.rs crates/protocol/src crates/libsy/srcRepository: NVIDIA-NeMo/Switchyard
Length of output: 22808
Match LlmClientError::General explicitly.
LlmClientError is #[non_exhaustive], so retain _ for future variants. Add an explicit General(_) arm to document the redaction policy for every current variant.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/libsy/src/algorithms/util/robustness.rs` around lines 74 - 75, Update
the error matching logic in the surrounding robustness function to add an
explicit LlmClientError::General(_) arm returning the same redacted "client call
failed" message, while retaining the wildcard arm for future non-exhaustive
variants.
Comment on lines
+397
to
+403
| /// Bounds one judge consultation, in milliseconds. | ||
| /// | ||
| /// The judge call sits in front of the routed call, so a stalled judge stalls | ||
| /// the turn. On expiry the judge counts as unavailable and the route falls back | ||
| /// as it does for any other judge failure. Omit to leave it unbounded. | ||
| #[serde(default)] | ||
| judge_deadline_ms: Option<u64>, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Do not silently ignore judge_deadline_ms outside capability mode.
Escalation and custom routes accept this setting, but only capability mode forwards it to TaskClassifierConfig. A configured deadline therefore has no effect for those modes.
crates/switchyard-server/src/config.rs#L397-L403: Scopejudge_deadline_msto capability mode, or add deadline support for every documented mode.crates/switchyard-server/src/config.rs#L605-L663: If deadline support remains capability-only, rejectjudge_deadline_msfor escalation and custom modes.docs/reference/toml_schema.md#L121-L121: Document the supported mode scope.
📍 Affects 2 files
crates/switchyard-server/src/config.rs#L397-L403(this comment)crates/switchyard-server/src/config.rs#L605-L663docs/reference/toml_schema.md#L121-L121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/switchyard-server/src/config.rs` around lines 397 - 403, Ensure
judge_deadline_ms is not silently ignored: either propagate it through
escalation and custom routes or keep it capability-only and reject it during
mode validation. Update the relevant configuration validation flow around the
mode handling block, and document the supported mode scope in
docs/reference/toml_schema.md at line 121.
| | `mode` | No | `capability` | Classifier behavior. Set it explicitly for new configurations. | | ||
| | `classifier_target` | Yes | — | Target the judge is called through. Not a routing destination. | | ||
| | `max_output_tokens` | No | `4096` | Maximum completion tokens for the judge verdict. Must be at least `1`. | | ||
| | `judge_deadline_ms` | No | unset | Bounds one judge consultation, in milliseconds. On expiry the judge counts as unavailable and the route falls back as it does for any other judge failure. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the zero-value restriction.
judge_deadline_ms = 0 is rejected during classifier construction. State that the value must be greater than 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/reference/toml_schema.md` at line 121, Update the judge_deadline_ms
entry in the TOML schema reference to state that configured values must be
greater than 0, while preserving its existing millisecond deadline and fallback
behavior description.
Contributor
|
@gburachas can you address coderabbit comments first, then I can review. |
ayushag-nv
reviewed
Aug 11, 2026
Comment on lines
+36
to
+45
| /// Restates the task after the conversation the judge is asked to route. | ||
| /// | ||
| /// The rubric leads the request, which works while the payload is a single short | ||
| /// message. Once `recent_turn_window` includes real conversation, those instructions | ||
| /// sit far from the generation point and the judge sometimes *answers* the | ||
| /// conversation instead of classifying it — the reply then fails to parse, the | ||
| /// verdict is unavailable, and the turn silently falls back. Repeating the task at | ||
| /// the end is what reliably prevents that. | ||
| const END_POSITION_REINFORCEMENT: &str = | ||
| "Route the conversation above. Output ONLY the routing JSON object, nothing else."; |
Contributor
There was a problem hiding this comment.
This looks like some new feature add, may be we should discuss this once. Can you break down into smaller directed PRs for ease of review. Looks like it solves three issues simultaneously
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #278, closes #346, closes #279.
Three related fixes to the
llm_classifierjudge path, kept in one PR because they touch the same two functions and would conflict as separate branches. Each is one reviewable commit.1. Redact upstream error content from judge warning logs (#278)
report_fail_openlogserror = %error, andLlmClientError::UpstreamHttp'sDisplayinterpolates the raw upstream body — on a warning that fires on every turn whose judge is unavailable. Providers routinely echo the offending request back in a 400, and the judge's request is a condensed view of the user's conversation.This keeps everything #205 built —
report_fail_open, the boundedreasontaxonomy,record_classifier_fail_open— and changes one parameter: it now takes an already-redacted summary produced by an exhaustive match overLibsyError/LlmClientError.Two properties worth preserving in review:
to_string(), so a new error variant cannot start leaking by omission. Resolving a future compile error there with a_ =>arm would discard the whole guarantee.error.to_string()still contains it. The second assertion documents that the hazard is still live inDisplay.Not covered by #271, which redacts
Debugand caps body size; theDisplaypath is untouched by it.2. Bound judge consultations with a configurable deadline (#346)
judge_deadline_mson the route. The judge call sits in front of the routed call, so a stalled judge stalls the turn with no bound of its own.The whole consultation is bounded, not just the HTTP call — a judge that returns headers promptly then stalls mid-stream would otherwise hold the turn just as long. Expiry folds into the existing fail-open path with
reason = "deadline_exceeded", so it appears on/metricsalongside the other reasons rather than as an untracked silent path. DefaultNonepreserves current behaviour exactly.3. Reinforce the routing task after windowed conversation content (#279)
With
recent_turn_windowset, the judge sees conversation content after the task, and tends to assess the content rather than the request. This appends a short reinforcement line end-positioned, which is the only position where it works — the test assertscontents.last(), not merely presence.Only the windowed path is affected. The default path carries user task messages only (the anchor and latest follow-up, per #309), so there is nothing there to outrank.
How tested
cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace, and again with--test-threads=1(this path registers process-global OTel instruments)uv run pytest tests/ -m "not integration"— 871 passed, 9 skippeduv run ruff check .uv run --only-group docs mkdocs build --strictRebased onto current
main, which required re-siting all three against #309 (build_messagesrewritten), #312 (judge system prompt moved toinstructions), #332/#338/#340 (verdict()rewritten) and #337 (LibsyError::MissingClientremoved).One rebase detail worth flagging, since it is the kind of thing that passes review while being wrong: a test here asserted
messages.len() == 2, which held only while the judge system prompt lived inmessages[0]. After #312 it lives ininstructions. The assertion is now== 1plus an assertion thatinstructionsis non-empty — decrementing the number alone would have silently stopped checking that the rubric reaches the model at all.Summary by CodeRabbit
New Features
judge_deadline_msconfiguration through Python and server configuration.Bug Fixes
Documentation