Skip to content

fix(libsy): validate classifier scores, fix packaging and strict rustdoc#104

Open
elyasmnvidian wants to merge 2 commits into
mainfrom
emehtabuddin/qa-libsy
Open

fix(libsy): validate classifier scores, fix packaging and strict rustdoc#104
elyasmnvidian wants to merge 2 commits into
mainfrom
emehtabuddin/qa-libsy

Conversation

@elyasmnvidian

@elyasmnvidian elyasmnvidian commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Three fixes to the libsy LLM-classifier router (crates/libsy/src/algorithms/llm_class.rs), plus a packaging and a rustdoc fix.

Streaming classifier verdicts were ignored. The router read the score with as_agg(), which returns None for a streaming response, so a classifier target that streamed its score always fell open to the strong model no matter what it returned. It now drains the stream with into_agg() and routes on the real score. A genuine stream error still propagates instead of being swallowed.

Invalid scores were trusted as verdicts. A classifier reply that parsed as a float but was not a probability — NaN, ±inf, or anything outside [0, 1] — was compared against the threshold directly. NaN and negative values compared below the threshold and were routed to the weak model. The router now keeps only scores in [0.0, 1.0]; everything else fails open to the strong model, same as an unparseable reply.

Packaging: cargo package -p switchyard-libsy now succeeds. The switchyard-protocol path dependency carries version = "0.1.0", which cargo requires before it will package a crate that has a path dependency.

Docs: RUSTDOCFLAGS="-D warnings" cargo doc is clean again. Fixed two intra-doc links that pointed at private items (DriverRequest, Driver::stream) and dropped redundant explicit link targets in lib.rs.

New regression tests cover the streaming drain and the invalid-score fail-open.

Note for reviewers: LlmClassifier is a Rust library type. The Python bindings (crates/switchyard-py/src/libsy_bindings.rs) expose only noop and random, so this router is not reachable through switchyard serve yet — the tests are the proof of behavior.

How to try it: cargo test -p switchyard-libsy runs the new tests.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Changes

LLM classifier routing

Layer / File(s) Summary
Classifier scoring and routing tests
crates/libsy/src/algorithms/llm_class.rs
Classifier responses are fully aggregated before scoring, probabilities are restricted to [0.0, 1.0], and buffered, streaming, invalid-score, and threshold routing cases are covered.
Crate metadata and documentation
crates/libsy/Cargo.toml, crates/libsy/src/core/algorithm.rs, crates/libsy/src/lib.rs
The protocol dependency adds version metadata, while documentation wording and intra-documentation links are updated.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a rabbit with a streaming score,
Draining every delta to the floor.
NaN hops out, the bounds stand clear,
Strong or weak now routes sincere.
Tests twitch whiskers: all is right!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: classifier score validation, packaging fixes, and rustdoc cleanup.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/llm_class.rs`:
- Around line 417-419: Remove the tracker reference “NvBug 6485976” from the NaN
fail-open comment at crates/libsy/src/algorithms/llm_class.rs lines 417-419
while preserving its rationale; likewise remove “NvBug 6485975” from the
streamed-score rationale at lines 439-441 without changing the surrounding
behavior or explanation.
🪄 Autofix (Beta)

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: 81f87868-d92e-4236-97ce-2e768d7abaac

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa6b87 and 2e417fe.

📒 Files selected for processing (4)
  • crates/libsy/Cargo.toml
  • crates/libsy/src/algorithms/llm_class.rs
  • crates/libsy/src/core/algorithm.rs
  • crates/libsy/src/lib.rs

Comment thread crates/libsy/src/algorithms/llm_class.rs Outdated
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
(cherry picked from commit 2e2c773)
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
(cherry picked from commit 0467a07)
@elyasmnvidian
elyasmnvidian force-pushed the emehtabuddin/qa-libsy branch from 2e417fe to 3d3b31c Compare July 21, 2026 21:30
@elyasmnvidian

elyasmnvidian commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Before/after — this fix is library-mode, so the proof is the classifier's own tests

LlmClassifier lives in crates/libsy and is not reachable through switchyard serve (the Python bindings expose only noop and random). The triggers — an invalid score, or a streamed verdict — also can't be produced on demand by a real gateway model. So the reproduction is the regression test: red on main's logic, green on the fix.

I ran the branch's llm_class tests twice: once after reverting the score extraction to main's version (as_agg() + no range filter), once with the fix.

With main's logic (as_agg(), no [0,1] filter):

out_of_range_score_defaults_to_strong ......... FAILED   (NaN routed weak)
below_range_score_defaults_to_strong .......... FAILED   (-0.1 routed weak)
streamed_score_below_threshold_routes_weak .... FAILED   (streamed "0.2" dropped -> strong)
test result: FAILED. 4 passed; 3 failed

With the fix (into_agg() drains the stream; .filter(|s| (0.0..=1.0).contains(s))):

test result: ok. 7 passed; 0 failed

Command: cargo test -p switchyard-libsy --lib llm_class. cargo clippy -p switchyard-libsy --all-targets is clean.

Audit follow-up applied in this push: added below_range_score_defaults_to_strong (a parseable -0.1 that main routes weak).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant