Skip to content

fix(libsy): allow message_hash_fallback with classify_trigger = user_turn - #815

Merged
afourniernv merged 1 commit into
NVIDIA-NeMo:mainfrom
deepujain:fix/allow-message-hash-fallback-user-turn
Sep 23, 2026
Merged

afourniernv merged 1 commit into
NVIDIA-NeMo:mainfrom
deepujain:fix/allow-message-hash-fallback-user-turn

Conversation

@deepujain

@deepujain deepujain commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

What

Relaxes the classifier config validation so message_hash_fallback is accepted with classify_trigger = "user_turn" in addition to "new_session". Only "every_request" is still rejected: it retains no target, so a fallback identity has nothing to key on. Docs updated in docs/reference/toml_schema.md, docs/routing_algorithms/llm_classifier_routing.md, and crates/switchyard-server/README.md. Adds a regression test covering the accepted and rejected combinations.

Why

Fixes the runtime half of #495. With user_turn and no session id, every turn is classified, so user_turn behaves like every_request for callers that send no session id. message_hash_fallback was meant to fix exactly that by keying the retained target on a hash of the first user message, but config validation rejected the combination. This also revives the runtime change from #519, which was closed for inactivity with only the docs update outstanding; the requested docs are included here.

Notes for reviewers

Start at affinity_router in crates/libsy/src/algorithms/llm_class.rs: the runtime already supports the combination, so this change only lifts the spurious validation rejection in the three validate sites. New test: message_hash_fallback_accepts_retaining_triggers. Local: cargo fmt clean, clippy -p switchyard-libsy clean, cargo test -p switchyard-libsy 314 passed.

Summary by CodeRabbit

  • New Features

    • message_hash_fallback now supports classification on both new sessions and user turns.
    • Routing retains targets using the first user message hash when no session ID is available.
    • The setting remains disabled by default.
  • Bug Fixes

    • Configuration validation consistently accepts the supported triggers and rejects unsupported request-level triggering.
  • Documentation

    • Updated configuration references, routing guidance, and server documentation to reflect the expanded trigger support.

…turn

Signed-off-by: Deepak Jain <deepujain@users.noreply.github.com>
@deepujain
deepujain requested a review from a team as a code owner September 22, 2026 06:06
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: NVIDIA-NeMo/Switchyard/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0e1a8b7e-c903-4c61-a6c0-65963d54e145

📥 Commits

Reviewing files that changed from the base of the PR and between 06e88c0 and bb9cf35.

📒 Files selected for processing (4)
  • crates/libsy/src/algorithms/llm_class.rs
  • crates/switchyard-server/README.md
  • docs/reference/toml_schema.md
  • docs/routing_algorithms/llm_classifier_routing.md

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The change expands message_hash_fallback support to NewSession and UserTurn. Validation and route construction still reject EveryRequest. Tests and documentation reflect the updated rule.

Changes

Message Hash Fallback

Layer / File(s) Summary
Classifier validation and route construction
crates/libsy/src/algorithms/llm_class.rs
Task, custom, and shared classifier paths accept NewSession and UserTurn. Tests verify both accepted triggers and continued rejection of EveryRequest.
Trigger requirement documentation
crates/switchyard-server/README.md, docs/reference/toml_schema.md, docs/routing_algorithms/llm_classifier_routing.md
Documentation now states that message_hash_fallback supports new_session and user_turn.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to bb9cf

The classifier now supports message-hash fallback for user turns while continuing to reject every-request classification; validation, routing, tests, and documentation are aligned, with no actionable merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: allowing message_hash_fallback with classify_trigger = user_turn.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (3 skipped: 3 u…
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.

A rabbit checks the trigger gate
New sessions hop beside user turns
Every request stays outside
Hashes guide the route with care
Green tests nibble at the rule

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

@afourniernv afourniernv left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. This carries forward the runtime change from #519. Ayush had already reviewed that implementation and only asked for the routing guide and server README updates: #519 (review). Both are included here.

@ryan-lempka tagging you since this sits on the classify_trigger/AffinityRouter path you added. Holding the merge until the v0.3 freeze is lifted.

@deepujain

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Noted that the merge is on hold until the v0.3 freeze lifts.

@afourniernv
afourniernv merged commit 9d0ae83 into NVIDIA-NeMo:main Sep 23, 2026
20 checks passed
@afourniernv

Copy link
Copy Markdown
Contributor

Thanks for your contribution @deepujain !

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.

2 participants