fix(libsy): warn when the scorer cannot reach a picker's non-default tier - #394
fix(libsy): warn when the scorer cannot reach a picker's non-default tier#394WUKUNTAI-0211 wants to merge 3 commits into
Conversation
Signed-off-by: WUKUNTAI <WUKUNTAI-0211@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change adds scorer confidence ceilings and detects thresholds that cannot select the non-default tier. Route construction warns about unreachable thresholds while preserving existing routing behavior. The crate publicly re-exports both confidence helpers. Tests cover ceiling calculations and picker outcomes. ChangesStage routing threshold diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to This adds route-construction warnings for unreachable scorer thresholds without changing routing behavior; the reported checks pass, and no actionable merge-blocking risk remains beyond normal review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
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/stage.rs`:
- Around line 170-177: The error message in pick_tier’s confidence-threshold
warning overstates fall-through behavior by saying every turn is affected.
Update the message to limit the claim to scorer-only turns, while preserving the
existing explanation of the judge or capable-tier fallback and the threshold
guidance.
In `@crates/libsy/src/algorithms/util/stage.rs`:
- Around line 355-357: Update scorer_cannot_leave_default to account for
PickerMode::EfficientFirst when confidence_threshold is 1.0, reflecting that
finite tanh scores cannot satisfy the inclusive pick_tier gate; alternatively,
narrow and rename the public helper to explicitly cover only
PickerMode::CapableFirst. Update the related test covering Lines 715-725 to
assert the corrected contract.
🪄 Autofix
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: 44ee6220-b2c9-43db-853e-963c61a17194
📒 Files selected for processing (3)
crates/libsy/src/algorithms/stage.rscrates/libsy/src/algorithms/util/stage.rscrates/libsy/src/lib.rs
Signed-off-by: WUKUNTAI <WUKUNTAI-0211@users.noreply.github.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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/util/stage.rs`:
- Around line 360-378: Update max_capable_confidence and the shared
signal-normalization path used by dimensions_from_signal so the capable ceiling
is derived from the exact normalized value accepted by the inclusive pick_tier
gate, making the strongest capable signal attainable. Add an end-to-end test at
max_capable_confidence covering the capable-threshold boundary, distinct from
efficient_first_at_its_ceiling_never_picks_capable.
🪄 Autofix
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: 0353e5c9-5a04-4ec2-91c9-d22861e2c40b
📒 Files selected for processing (3)
crates/libsy/src/algorithms/stage.rscrates/libsy/src/algorithms/util/stage.rscrates/libsy/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/libsy/src/algorithms/stage.rs
…carry Signed-off-by: WUKUNTAI <WUKUNTAI-0211@users.noreply.github.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
What
Warns at route construction when a
stage_routerroute pairs a picker with aconfidence_thresholdthe scorer can never reach in the direction that leaves the default tier. No routing behaviour changes.Why
Closes #264
score_signalsums one positive axis against one negative dimension, then squashes withtanh. Each direction has a hard ceiling below1.0:production_intensityonly, bounded to1.0byratio0.4621severityat itsHARD_SEVERITYcap, plus one ofspinning/exploring(they partition the not-producing case)0.7616Above its ceiling the gate never opens, so the scorer can only ever confirm the default tier. The reporter measured 439 consecutive turns at
capable_first+0.5with zero turns routed to the efficient tier and zero upstream errors. Nothing in the logs or inrouting_decisions.stage_routerseparates "signals were weak this turn" from "this branch cannot fire at all".efficient_first+1.0is the mirror case, and the[0.0, 1.0]range check accepts it today.How tested
cargo test --workspacegreen (23 test binaries, 0 failed)cargo clippy --workspace --all-targetscleancargo fmt --all --checkcleanuv run ruff check .cleanuv run mypy switchyardcleanuv run pytest tests/ --ignore=tests/e2egreen (135 passed)capable_first+0.5andefficient_first+1.0warn;0.45and0.5respectively stay silent.tests/e2e/test_closed_book_proxy_integration.pyfails locally, but it fails identically on a clean checkout ofmain— a pre-existing network-dependent failure, unrelated to this change.Seven unit tests added. The two that pin the ceilings derive them from the dimension set rather than asserting literals, so they fail if the signal set ever changes.
Checklist
crates/libsy/src/lib.rs.Notes for reviewers
Which of the issue's three options this takes. Option 1 (lower the default) no longer applies:
confidence_thresholdis a required TOML field with no default, and the Python paths the issue cites were removed when routing moved to Rust. Option 3 (add a second negative signal) changes routing behaviour and needs recalibration. This is option 2 — happy to re-scope if you prefer option 3.Warn, not reject. The hard overrides (
should_escalate/should_deescalate) run before the scorer and still fire, and the judge still resolves turns, so the route is not inert end to end — only the scorer is. Rejecting at load would break running deployments, including theconfidence_threshold = 1.0fixture inconfig.rs.Placement. The guard lives in
libsyso it covers the TOML server path and the PyO3 bindings from one place, and the ceilings sit next to theSCORE_GAIN/SIGNAL_UNIT/HARD_SEVERITYconstants they are derived from. Deployments oncapable_firstwill see this alongside the existing "capable_first is experimental" warning inconfig.rs; collapsing the pair is an option if it reads as noise.The capable ceiling depends on the severity table. It assumes
HARD_SEVERITYis the strongest severity the scorer sees, which is what that constant's doc comment already states (critical is intercepted by the override). If a pattern with a severity between0.7and1.0is ever added toERROR_PATTERNS,max_capable_confidencemoves and its test fails — intentionally.Docs left alone. #264 also flags four lines in
docs/routing_algorithms/stage_router_routing.md. #288 is already rewriting that page, so touching it here would only conflict. A follow-up can point it at these helpers once that lands.Summary by CodeRabbit
New Features
Bug Fixes
Tests