feat(libsy): stage router (signals only)#135
Conversation
messiaen
changed the title
7cb8a7b to
96fc5e3
Compare
c8d9195 to
ac31ad5
Compare
|
Will you also be updating docs? |
My plan was to update the user docs when we have the full router working. |
WalkthroughChangesStage Router
Estimated code review effort: 4 (Complex) | ~60 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: 3
🤖 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/stage_router.rs`:
- Line 310: Make StageClassifier public and add its documentation in
crates/libsy/src/algorithms/util/stage_router.rs:310-310, then re-export it from
stage_router in crates/libsy/src/lib.rs:92-96 and forward that export through
the components bridge in crates/switchyard-components/src/stage_router.rs:12-15
so consumers can construct and register it.
- Around line 329-359: Update the PickOutcome::Resolved and
PickOutcome::ConsultClassifier branches to use the outcome-provided confidence
for resolved selections instead of deriving it from signed score. Keep fall-open
results genuinely ambiguous: do not remap the signed score onto default_tier or
emit high confidence for the default weak target, while preserving
default_target state handling.
In `@crates/libsy/src/algorithms/util/tool_signals.rs`:
- Around line 259-262: Add /// documentation for ToolSignalProcessor and its
recent_window field in crates/libsy/src/algorithms/util/tool_signals.rs:259-262.
Add a /// comment in crates/libsy/src/core/state.rs:24 describing the lifecycle
and absence semantics of tool_signals, and document the public stage_router
module in crates/libsy/src/lib.rs:92; use Rust documentation comments for all
three public APIs.
🪄 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: d6162c00-6a3b-48de-ae14-2c8c1702ca6b
📒 Files selected for processing (7)
crates/libsy/src/algorithms/util.rscrates/libsy/src/algorithms/util/stage_router.rscrates/libsy/src/algorithms/util/tool_signals.rscrates/libsy/src/core/state.rscrates/libsy/src/lib.rscrates/libsy/src/signal.rscrates/switchyard-components/src/stage_router.rs
💤 Files with no reviewable changes (1)
- crates/libsy/src/signal.rs
| } | ||
| } | ||
|
|
||
| struct StageClassifier {} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Expose the classifier through the intended bridge. StageClassifier is private inside a pub(crate) module, and neither forwarding layer exports it. No consumer can construct or register this Classifier; only in-module tests can execute it.
crates/libsy/src/algorithms/util/stage_router.rs#L310-L310: makeStageClassifierpublic and document it.crates/libsy/src/lib.rs#L92-L96: re-exportStageClassifierfromstage_router.crates/switchyard-components/src/stage_router.rs#L12-L15: forwardStageClassifierthrough the components bridge.
📍 Affects 3 files
crates/libsy/src/algorithms/util/stage_router.rs#L310-L310(this comment)crates/libsy/src/lib.rs#L92-L96crates/switchyard-components/src/stage_router.rs#L12-L15
🤖 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/stage_router.rs` at line 310, Make
StageClassifier public and add its documentation in
crates/libsy/src/algorithms/util/stage_router.rs:310-310, then re-export it from
stage_router in crates/libsy/src/lib.rs:92-96 and forward that export through
the components bridge in crates/switchyard-components/src/stage_router.rs:12-15
so consumers can construct and register it.
| PickOutcome::Resolved { tier, score, .. } => { | ||
| let target = match tier { | ||
| Tier::Capable => "strong", | ||
| Tier::Efficient => "weak", | ||
| }; | ||
| let conf = (score + 1.0) / 2.0; | ||
| // TODO add the non-target to this score set? | ||
| Ok(Classification::Scores(vec![Score { | ||
| target: target.to_string(), | ||
| confidence: conf, | ||
| }])) | ||
| } | ||
| PickOutcome::ConsultClassifier { | ||
| score, | ||
| default_tier, | ||
| .. | ||
| } => { | ||
| let target = match default_tier { | ||
| Tier::Capable => "strong", | ||
| Tier::Efficient => "weak", | ||
| }; | ||
| state.extra.insert( | ||
| "default_target".to_string(), | ||
| StateValue::String(target.to_string()), | ||
| ); | ||
| let conf = (score + 1.0) / 2.0; | ||
| // TODO add the non-target to this score set? | ||
| Ok(Classification::Ambiguous(vec![Score { | ||
| target: target.to_string(), | ||
| confidence: conf, | ||
| }])) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve confidence for the selected target.
score is signed toward Capable, but target switches to "weak" for negative scores. A -0.6 resolved weak result is emitted with 0.2 confidence, while hard override/de-escalation outcomes with score = 0.0 emit 0.5 despite being resolved. Fall-open can also report high confidence for the default weak target when the signal points capable. Use PickOutcome’s confidence for resolved outcomes and represent fall-open as genuinely ambiguous rather than remapping the signed score.
🤖 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/stage_router.rs` around lines 329 - 359,
Update the PickOutcome::Resolved and PickOutcome::ConsultClassifier branches to
use the outcome-provided confidence for resolved selections instead of deriving
it from signed score. Keep fall-open results genuinely ambiguous: do not remap
the signed score onto default_tier or emit high confidence for the default weak
target, while preserving default_target state handling.
There was a problem hiding this comment.
confidence is just the magnitude of the score. The logic is correct here.
There was a problem hiding this comment.
@messiaen, agreed that confidence is the score’s magnitude—that is exactly why the current mapping is problematic.
StageClassifier::score emits (score + 1.0) / 2.0, which is not the magnitude:
score = -0.6selects"weak"but emits0.2, rather than0.6.- An override has
score = 0.0and provided confidenceSome(1.0), but emits0.5. - A fall-open positive score can still default to
"weak"underEfficientFirst, yet the mapping reports a high confidence for that default target.
So the concern is not with confidence = score.abs(); it is that Lines 342 and 359 remap the signed score instead of using the outcome confidence (and, for fall-open, associate directional evidence with the default tier).
| #[derive(Debug, Clone)] | ||
| pub struct ToolSignalProcessor { | ||
| pub recent_window: usize, | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the new public Rust API.
crates/libsy/src/algorithms/util/tool_signals.rs#L259-L262: add///docs forToolSignalProcessorandrecent_window.crates/libsy/src/core/state.rs#L24-L24: add a///doc explaining the lifecycle and absence semantics oftool_signals.crates/libsy/src/lib.rs#L92-L92: add a///doc comment for the publicstage_routermodule.
As per coding guidelines, “Use /// documentation comments for public Rust items.”
📍 Affects 3 files
crates/libsy/src/algorithms/util/tool_signals.rs#L259-L262(this comment)crates/libsy/src/core/state.rs#L24-L24crates/libsy/src/lib.rs#L92-L92
🤖 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/tool_signals.rs` around lines 259 - 262, Add
/// documentation for ToolSignalProcessor and its recent_window field in
crates/libsy/src/algorithms/util/tool_signals.rs:259-262. Add a /// comment in
crates/libsy/src/core/state.rs:24 describing the lifecycle and absence semantics
of tool_signals, and document the public stage_router module in
crates/libsy/src/lib.rs:92; use Rust documentation comments for all three public
APIs.
Source: Coding guidelines
| @@ -0,0 +1,527 @@ | |||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
There was a problem hiding this comment.
can we make the structure src/algorithsm//*.rs
with src/algorithsm/.rs
There was a problem hiding this comment.
My goal was to separate the public stuff (algorithms) and the non-public (util).
The public part of this would be some instantiation FallThrough ... probably.
| //! Stage-router tier selection — thin re-export of libsy's decision core. | ||
| //! | ||
| //! Decides whether a coding-agent turn goes to the **capable** (strong) or | ||
| //! **efficient** (weak) tier from its [`ToolResultSignal`]. Single source of |
There was a problem hiding this comment.
we should also replace/delete these stage router files in switchyard-components
There was a problem hiding this comment.
The only dependency is dimension_collector I think. Which I don't think we need to move.
tool_signal was moved previous PR
| pub struct ToolSignalProcessor { | ||
| pub recent_window: usize, | ||
| } | ||
|
|
There was a problem hiding this comment.
we should also replace any parsing happening here, to switchyard-protocol/IR
too much API specific parsing happening here
There was a problem hiding this comment.
Yes, I agree. I think some of it is necessary. @sabhatinas can clarify.
Let's do this is a separate PR.
Signed-off-by: Greg Clark <grclark@nvidia.com> feat(libsy): bridge back to components for stage_router Signed-off-by: Greg Clark <grclark@nvidia.com>
ac31ad5 to
0775ad6
Compare
3 participants
What
How tested
uv run ruff check .cleanuv run mypy switchyardcleanuv run pytest tests/greenChecklist
snake_caseof the primary class.switchyard/__init__.py.__all__if intended for downstream use.--helpupdated if customer-facing surface changed.Signed-off-by: Your Name <email>) per the DCO.Notes for reviewers
Anything reviewers should pay extra attention to — risky paths, follow-up tickets, intentional trade-offs.
Summary by CodeRabbit