Skip to content

refactor(libsy): source advisor-gate guards from shared ToolSignals counts - #591

Merged
eric-liu-nvidia merged 2 commits into
mainfrom
claude/pr-review-gate-guards-661b66
Sep 1, 2026
Merged

refactor(libsy): source advisor-gate guards from shared ToolSignals counts#591
eric-liu-nvidia merged 2 commits into
mainfrom
claude/pr-review-gate-guards-661b66

Conversation

@eric-liu-nvidia

@eric-liu-nvidia eric-liu-nvidia commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

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 shared ToolSignals extraction instead of bespoke counters — one definition of tool/turn counting across algorithms.

  • ToolSignals gains two buckets: tool_result_count (per ToolResult block, including empty-content results, matching the gate's previous counting) and assistant_turn_count (Role::Assistant messages; documented in contrast to turn_depth, which counts all messages regardless of role).
  • AdvisorGate::route_inner calls ToolSignals::from_request once and reads both counts; the bespoke count_tool_results / assistant_turns helpers in advisor_gate/turn.rs are deleted.
  • No behavior change: the two new gate e2e tests were written first and passed against the old helpers; after the swap, both suites re-ran green with zero test modifications.

Tests

  • ToolSignals unit tests pin counting parity on both message shapes: one ToolResult per user message (OpenAI-chat normalized role: tool) and several blocks batched in one message (Anthropic) — plus empty-content results still counting, and the assistant_turn_count vs turn_depth contrast.
  • Gate e2e: batched result blocks satisfy gate_min_tool_results per 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

    • Improved advisor gating accuracy when processing tool results and assistant responses.
    • Batched messages now count each tool result individually, including empty results.
    • Stall detection now counts assistant turns correctly rather than counting all conversation messages.
    • Threshold-based safeguards now respond more reliably to conversations with multiple tool calls or results.
  • Tests

    • Added coverage for batched tool results, empty results, and varied conversation formats.

…ounts

Signed-off-by: zengyuanl <zengyuanl@nvidia.com>
@eric-liu-nvidia
eric-liu-nvidia requested a review from a team as a code owner September 1, 2026 19:43
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

🚀 View preview at
https://NVIDIA-NeMo.github.io/Switchyard/pr-preview/pr-591/

Built to branch gh-pages at 2026-09-01 20:41 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9e2dc465-4128-462e-b33e-eb1da59db666

📥 Commits

Reviewing files that changed from the base of the PR and between 035d702 and 28c14bd.

📒 Files selected for processing (4)
  • crates/libsy/src/algorithms/advisor_gate.rs
  • crates/libsy/src/algorithms/advisor_gate/tests.rs
  • crates/libsy/src/algorithms/advisor_gate/turn.rs
  • crates/libsy/src/algorithms/util/tool_signals.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

ToolSignals now extracts assistant-message and raw tool-result counts. AdvisorGate::route_inner uses these shared signals for stall and minimum-tool-result thresholds. Tests cover batched and empty results, and assistant-turn counting.

Changes

Advisor gate signal extraction

Layer / File(s) Summary
Conversation-shape signal extraction
crates/libsy/src/algorithms/util/tool_signals.rs
ToolSignals now counts assistant messages and every ToolResult block, including batched and empty results. Tests cover the new counts.
AdvisorGate threshold integration
crates/libsy/src/algorithms/advisor_gate.rs, crates/libsy/src/algorithms/advisor_gate/tests.rs, crates/libsy/src/algorithms/advisor_gate/turn.rs
route_inner uses shared counts for stall and minimum-tool-result checks. Tests cover both thresholds. Obsolete local helpers and imports are removed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 28c14

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

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: refactoring advisor-gate guards to use shared ToolSignals counts.
Docstring Coverage ✅ Passed Docstring coverage is 86.67% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 4 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

Comment thread crates/libsy/src/algorithms/util/tool_signals.rs Outdated
Comment thread crates/libsy/src/algorithms/util/tool_signals.rs Outdated
@eric-liu-nvidia
eric-liu-nvidia merged commit 7bda077 into main Sep 1, 2026
20 checks passed
@eric-liu-nvidia
eric-liu-nvidia deleted the claude/pr-review-gate-guards-661b66 branch September 1, 2026 20:49
Signed-off-by: zengyuanl <zengyuanl@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants