feat(libsy): add escalation as config knob on LlmTaskClassifier - #211
Conversation
45caf7a to
15e3dd8
Compare
messiaen
left a comment
messiaen
left a comment
There was a problem hiding this comment.
leaving initial comment
cba756f to
f355e8a
Compare
Signed-off-by: Lin Jia <linj@nvidia.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
…tion constructor Signed-off-by: Lin Jia <linj@nvidia.com>
8810d80 to
e1c69bc
Compare
WalkthroughThis PR adds trajectory-based escalation routing and response stream conversion. The escalation feature evaluates agent progress using a judge model, summarizes conversation history for efficient request shaping, and routes to a capable model after a configured count of consecutive escalation verdicts. The stream conversion feature allows buffered responses to be consumed as synthetic chunk streams. Server configuration gains an optional escalation field to enable the feature per route. ChangesEscalation routing
Stream utilities
Estimated code review effort: 4 (Complex) | ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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: 2
🤖 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 366-370: Remove the unwrap from the assistant-message construction
in the judge request flow. Build the assistant message directly from the
existing agg value before it is moved into efficient_response, preserving the
completed trajectory behavior and avoiding any production panic.
In `@crates/libsy/src/algorithms/util/escalation.rs`:
- Around line 290-313: Update the local assemble closure in the escalation
routine to construct the “showing the last N of M messages” header from the
current window length on every call, rather than capturing a header computed
before the trimming loop. Preserve the existing anchors, window assembly, and
truncation behavior while ensuring the header reflects entries removed by
window.remove(0).
🪄 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: 3d52a83e-9259-46e1-8eaa-901d66f5aedb
📒 Files selected for processing (7)
crates/libsy/src/algorithms.rscrates/libsy/src/algorithms/llm_class.rscrates/libsy/src/algorithms/util.rscrates/libsy/src/algorithms/util/escalation.rscrates/libsy/src/prompts/escalation/prompt.mdcrates/libsy/src/prompts/escalation/schema.jsoncrates/protocol/src/client.rs
Signed-off-by: Lin Jia <linj@nvidia.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
78b8f41 to
570221b
Compare
…ifier route Signed-off-by: Lin Jia <linj@nvidia.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
|
Terminal-Bench 2.0 smoke through the Rust libsy server (
Observed
Are the results expected?Yes. Neither terminal state is a routing fault:
This validates plumbing and routing behaviour, not routing quality — neither run passed its task, and solve rates need the full sweep. |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/switchyard-server/src/config.rs (1)
190-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMandatory
classifier_configfields go unused onceescalationis set.
classifier_config: TaskClassifierConfig(Line 195-196) is notOptionand has no top-level#[serde(default)], sobuild_algorithm'sSome(judge_config)branch (Lines 345-353) never reads it. Operators enablingescalationon anllm_classifierroute must still supply whatever required fieldsTaskClassifierConfighas (e.g.base_threshold, always present inVALID_CONFIG), even though those values have no effect.Consider modeling the two modes so unused fields cannot be supplied, for example by giving
TaskClassifierConfig's scalar fields defaults, or restructuringLlmClassifiersoclassifier_configandescalationare two branches of one field rather than two independently-required/optional fields.#!/bin/bash # Description: Check whether TaskClassifierConfig's scalar fields have serde defaults. set -euo pipefail rg -n -B2 -A 20 'struct TaskClassifierConfig' crates/libsy/src/algorithms/llm_class.rs🤖 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 190 - 205, The LlmClassifier configuration currently requires classifier_config fields even when escalation mode ignores them. Update TaskClassifierConfig or the LlmClassifier representation so escalation and normal classification are modeled as distinct configuration modes, allowing escalation routes to omit unused classifier settings while preserving required fields for non-escalation routes.
🤖 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/protocol/src/stream.rs`:
- Around line 109-121: Update the ContentBlock-to-LlmResponseChunk conversion to
preserve ContentBlock::Reasoning.signature through into_agg(), and handle every
ContentBlock variant rather than silently dropping unmapped blocks via _ => {}.
Extend the chunk/accumulator contract as needed so all content is retained, or
return an explicit conversion error when a variant cannot be represented.
- Around line 103-119: Update the conversion loop around the output iteration
and ContentBlock::ToolCall branch to maintain a separate, unique index for each
tool call rather than reusing the outer output index. Use that tool-call index
for ToolCallDelta.index while preserving the existing output index for text and
reasoning chunks and ensuring each tool call increments independently.
---
Nitpick comments:
In `@crates/switchyard-server/src/config.rs`:
- Around line 190-205: The LlmClassifier configuration currently requires
classifier_config fields even when escalation mode ignores them. Update
TaskClassifierConfig or the LlmClassifier representation so escalation and
normal classification are modeled as distinct configuration modes, allowing
escalation routes to omit unused classifier settings while preserving required
fields for non-escalation routes.
🪄 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: 1889bbbd-7a8f-4aae-ab51-e9252195ffa9
📒 Files selected for processing (3)
crates/libsy/src/algorithms/llm_class.rscrates/protocol/src/stream.rscrates/switchyard-server/src/config.rs
Signed-off-by: Lin Jia <linj@nvidia.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
2 participants
Draft of Ryan and Sabhatina's proposed design: escalation as a builder method on the existing capability classifier rather than a separate algorithm.
LlmTaskClassifier.with_escalation(EscalationJudgeConfig) swaps the inner classifier for an EscalationClassifier that calls the efficient model first, appends its reply to the transcript, consults the trajectory judge, and either returns the efficient response directly (on decline) or routes to capable (once the streak confirms). The session latch is the streak counter in State, replacing the AffinityRouter.
Targets PR 203 (the Option interface change) because EscalationClassifier relies on returning Some(efficient_response) to avoid a second model call on non-escalating turns.
Summary by CodeRabbit