feat(libsy): make LlmClassifier recent_turn_window configurable - #156
feat(libsy): make LlmClassifier recent_turn_window configurable#156sabhatinas wants to merge 1 commit into
Conversation
Signed-off-by: Sabhatina Selvam <sabhatinas@nvidia.com>
WalkthroughThe classifier now supports configurable trailing prompt context. A default is defined in ChangesClassifier Context Configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 1
🧹 Nitpick comments (2)
crates/switchyard-server/src/config.rs (1)
275-285: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd coverage for the TOML-to-classifier configuration contract.
The construction wiring is correct, but no corresponding server configuration test is shown. Test an omitted field (internal default) and an explicit value (overridden prompt trimming) so deserialization or wiring regressions are caught.
🤖 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 275 - 285, Add server configuration tests around the LlmClassifier construction path to cover the TOML-to-classifier contract: verify an omitted recent_turn_window uses the classifier’s internal default, and verify an explicitly configured value is propagated and overrides prompt trimming. Reuse the existing config parsing and classifier inspection/testing patterns rather than changing the construction logic.crates/libsy/src/algorithms/llm_class.rs (1)
299-305: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest prompt trimming, not only field assignment.
This test would pass even if
create_run_taskignored the configured value and continued using a hardcoded window. Add a multi-message test that inspects the classifier request for both the default and overridden windows, including the intended zero-window behavior.🤖 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/llm_class.rs` around lines 299 - 305, Expand recent_turn_window_defaults_and_is_configurable to exercise create_run_task with a multi-message conversation and inspect the generated classifier request. Verify the default window, an overridden window of 2, and a zero-window configuration each trim the prompt to the expected messages, ensuring the configured value—not a hardcoded constant—is used.
🤖 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 100-105: Update the docstring for with_recent_turn_window to state
that a zero window includes the first user message and, when available, the
latest user message. Add a prompt-level test covering
with_recent_turn_window(0), verifying both messages are included when distinct.
---
Nitpick comments:
In `@crates/libsy/src/algorithms/llm_class.rs`:
- Around line 299-305: Expand recent_turn_window_defaults_and_is_configurable to
exercise create_run_task with a multi-message conversation and inspect the
generated classifier request. Verify the default window, an overridden window of
2, and a zero-window configuration each trim the prompt to the expected
messages, ensuring the configured value—not a hardcoded constant—is used.
In `@crates/switchyard-server/src/config.rs`:
- Around line 275-285: Add server configuration tests around the LlmClassifier
construction path to cover the TOML-to-classifier contract: verify an omitted
recent_turn_window uses the classifier’s internal default, and verify an
explicitly configured value is propagated and overrides prompt trimming. Reuse
the existing config parsing and classifier inspection/testing patterns rather
than changing the construction logic.
🪄 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: 494804e8-b4b1-4358-8e21-0da5e22debd5
📒 Files selected for processing (2)
crates/libsy/src/algorithms/llm_class.rscrates/switchyard-server/src/config.rs
| /// Set the number of trailing conversation turns included in the classifier | ||
| /// prompt (default [`DEFAULT_RECENT_TURN_WINDOW`]). `0` sends only the first | ||
| /// user message. | ||
| #[must_use] | ||
| pub fn with_recent_turn_window(mut self, recent_turn_window: usize) -> Self { | ||
| self.recent_turn_window = recent_turn_window; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure and inspect the relevant Rust source.
git ls-files crates/libsy/src/algorithms/llm_class.rs crates/libsy/src/algorithms | cat
wc -l crates/libsy/src/algorithms/llm_class.rs
sed -n '1,220p' crates/libsy/src/algorithms/llm_class.rs
printf '\n--- TESTS ---\n'
sed -n '220,380p' crates/libsy/src/algorithms/llm_class.rsRepository: NVIDIA-NeMo/Switchyard
Length of output: 14296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for zero-window handling and related tests elsewhere in the crate.
rg -n "recent_turn_window|trim_messages|first user message|latest user message|window 0|0 sends" crates/libsy/srcRepository: NVIDIA-NeMo/Switchyard
Length of output: 1688
Update the zero-window contract
with_recent_turn_window(0) keeps the first user message and, when present, the latest user message. Update the docstring and add a prompt-level test for that case.
🤖 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/llm_class.rs` around lines 100 - 105, Update the
docstring for with_recent_turn_window to state that a zero window includes the
first user message and, when available, the latest user message. Add a
prompt-level test covering with_recent_turn_window(0), verifying both messages
are included when distinct.
3 participants
LlmClassifier's classifier-prompt window (recent_turn_window— first user message + last N turns) was hardcoded to5(trim_messages(..., 5)). This makes it user-configurable, closing a parity gap with the v2ClassifierConfig.recent_turn_window.LlmClassifier: newrecent_turn_windowfield +DEFAULT_RECENT_TURN_WINDOW = 5+with_recent_turn_window(n)builder (non-breaking;new()signature unchanged); call site usesself.recent_turn_window.recent_turn_window: Option<usize>on thellm_classifierroute type, threaded to the classifier when set (unset → default 5).libsy + server build clean; cargo fmt + clippy clean; libsy + server tests pass.
Linear: SWITCH-1110 (parity work item)
Summary by CodeRabbit