feat: add per-tool circuit breakers - #35
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Summary by CodeRabbit
WalkthroughAdded opt-in, process-local, per-tool circuit breakers. The runtime supports failure thresholds, open and half-open states, permit invalidation, recovery probes, inspection, reset operations, metrics, MCP/task results, documentation, examples, and compatibility exports. ChangesPer-tool circuit-breaker support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ToolRuntime
participant CircuitBreaker
participant Tool
Caller->>ToolRuntime: Invoke registered tool
ToolRuntime->>CircuitBreaker: Acquire and validate permit
CircuitBreaker-->>ToolRuntime: Permit or circuit_open rejection
ToolRuntime->>Tool: Execute after capacity and rate checks
Tool-->>ToolRuntime: Success or failure
ToolRuntime->>CircuitBreaker: Record outcome
CircuitBreaker-->>ToolRuntime: Update circuit state
ToolRuntime-->>Caller: Return tool result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 5
🤖 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 @.github/workflows/ci.yml:
- Around line 60-61: Replace the duplicated inline wheel smoke-check commands
with one shared checked-in scripts/smoke_check.py invocation, preserving all
existing export and behavior assertions. Update .github/workflows/ci.yml lines
60-61 to invoke it with .smoke/bin/python, and .github/workflows/release.yml
lines 46-47 to invoke the same script with .release-smoke/bin/python.
In `@README.md`:
- Around line 249-255: Update the ToolRuntime.invoke() runtime status list to
include circuit_open, matching the circuit-breaker behavior documented in the
surrounding README section and keeping the existing statuses unchanged.
In `@src/samsarix_core/runtime.py`:
- Around line 656-657: Replace the inline circuit failure handling at the shown
call site and the corresponding failure paths around the other circuit-breaker
calls with `_record_circuit_failure`, passing the relevant circuit attempt and
current time. Preserve the existing ordering so `fail()` occurs before execution
cancellation, and keep the trip counter update centralized in the helper.
In `@tests/test_runtime.py`:
- Around line 1551-1557: Update the async timeout test around the
slow_async_dependency invocations to wait for started using the same one-second
synchronization pattern as the synchronous counterpart, replacing the immediate
started.is_set() assertion while preserving the existing circuit-breaker
assertions.
- Around line 1687-1698: Add a parametrized test case in the existing
ToolCircuitBreaker validation matrix using recovery_timeout_seconds=1e308 with a
valid failure_threshold and expecting ValueError, so execution reaches the
isfinite(retry_after_ms) guard; optionally include negative and non-numeric
recovery timeout cases if consistent with the surrounding validation coverage.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 47c46609-6bed-4dc4-8ff2-15cea2d79ac0
📒 Files selected for processing (22)
.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdREADME.mdROADMAP.mddocs/API_REFERENCE.mddocs/ARCHITECTURE.mddocs/BEST_PRACTICES.mddocs/CIRCUIT_BREAKERS.mddocs/GETTING_STARTED.mddocs/MCP.mddocs/OBSERVABILITY.mddocs/RATE_LIMITS.mdexamples/circuit_breaker_api.pysrc/helix_core/__init__.pysrc/samsarix_core/__init__.pysrc/samsarix_core/models.pysrc/samsarix_core/runtime.pytests/test_definitions.pytests/test_mcp.pytests/test_mcp_tasks.pytests/test_runtime.py
1 participant
Summary
Failure boundary
Counts tool exceptions, output validation failures, and caller-visible timeouts. Does not count invalid input, policy outcomes, runtime/rate admission rejection, progress-handler failure, or caller cancellation. Core performs no automatic retry.
Verification
Candidate artifacts (local, not a release):
2a93ebb8fb8d283c4e878b2a0046ac44f5d656a586195f0e951279b9571ea56d0bf48553d4c94f928e5b5ab807586c6529bd9d08fd7dd5459babcdc231935c05