refactor(libsy): source advisor-gate guards from shared ToolSignals counts - #591
Conversation
…ounts Signed-off-by: zengyuanl <zengyuanl@nvidia.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 (4)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. Walkthrough
ChangesAdvisor gate signal extraction
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized refactor preserves the existing gating behavior and introduces no actionable merge-blocking risk; it is merge-ready after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Signed-off-by: zengyuanl <zengyuanl@nvidia.com>
Summary
Followup committed in the #371 review thread: the advisor gate's request-side guards (
gate_stall_turns,gate_min_tool_results) now read the sharedToolSignalsextraction instead of bespoke counters — one definition of tool/turn counting across algorithms.ToolSignalsgains two buckets:tool_result_count(perToolResultblock, including empty-content results, matching the gate's previous counting) andassistant_turn_count(Role::Assistantmessages; documented in contrast toturn_depth, which counts all messages regardless of role).AdvisorGate::route_innercallsToolSignals::from_requestonce and reads both counts; the bespokecount_tool_results/assistant_turnshelpers inadvisor_gate/turn.rsare deleted.Tests
ToolSignalsunit tests pin counting parity on both message shapes: oneToolResultper user message (OpenAI-chat normalizedrole: tool) and several blocks batched in one message (Anthropic) — plus empty-content results still counting, and theassistant_turn_countvsturn_depthcontrast.gate_min_tool_resultsper block; the stall checkpoint counts assistant turns, not messages.Additive pub-struct fields only (no out-of-crate consumers). The trigger side (buffered-response inspection:
has_tool_use,visible_text) is intentionally untouched and left for a separate followup.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests