feat: parallel dispatch - #48
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds opt-in parallel tool dispatch with configurable concurrency, resource-key conflict serialization, cancellation-aware recovery, ordered results, and expanded tests. Configuration, tool APIs, mock tooling, documentation, and changelog entries are updated. ChangesParallel tool dispatch
Sequence Diagram(s)sequenceDiagram
participant BareLoop
participant ToolRegistry
participant ToolDependencyGraph
participant ParallelDispatch
participant Tool
BareLoop->>ToolRegistry: Resolve ToolCall tools
ParallelDispatch->>ToolDependencyGraph: Plan waves using safety and resource_key
ParallelDispatch->>Tool: Execute eligible calls with bounded concurrency
Tool-->>ParallelDispatch: Return per-call results
ParallelDispatch-->>BareLoop: Preserve input order and finish post-processing
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/engine/bare/dispatch.rs (1)
1398-1401: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWall-clock timing assertion can be flaky under CI load.
Asserting
elapsed < 250msfor 3×100ms overlapping calls leaves only ~150ms of scheduling headroom; a loaded runner can breach it and produce spurious failures. Consider a wider margin (e.g.< 290ms) or a paused/virtual-time clock so the test asserts overlap without depending on real wall-clock slack.🤖 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 `@src/engine/bare/dispatch.rs` around lines 1398 - 1401, Relax the elapsed-time threshold in the parallel overlap assertion within the relevant dispatch test to provide substantially more CI scheduling headroom, such as checking for less than 290ms. Preserve the test’s intent of rejecting serialized 3×100ms execution without introducing unrelated changes.
🤖 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 `@src/engine/bare/dispatch.rs`:
- Around line 407-418: Update the task closure in dispatch_tools_parallel to
invoke dispatch_tool_with_recovery instead of dispatch_tool, passing the same
tool call, context, timing, cancellation, and turn inputs so
reflector/backoff/correction retries match sequential dispatch. Preserve permit
release and cancellation behavior.
---
Nitpick comments:
In `@src/engine/bare/dispatch.rs`:
- Around line 1398-1401: Relax the elapsed-time threshold in the parallel
overlap assertion within the relevant dispatch test to provide substantially
more CI scheduling headroom, such as checking for less than 290ms. Preserve the
test’s intent of rejecting serialized 3×100ms execution without introducing
unrelated changes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f009a1d6-74f7-46a9-94e8-250e8e9c8438
📒 Files selected for processing (7)
CHANGELOG.mdexamples/chat.rssrc/config.rssrc/engine/bare/dispatch.rssrc/testing.rssrc/tool.rssrc/tool/registry.rs
|
@CodeRabbit review |
✅ Action performedReview finished.
|
No description provided.