docs: multi-tool agent loop + permission modes design#60
docs: multi-tool agent loop + permission modes design#60Hongjiseung-ROK wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive design document for transitioning the chemsmart agent from a static planning model to a dynamic tool-loop architecture. Key features include a provider-neutral orchestrator, a handle-based system for managing complex chemistry objects, and distinct Permission and Driving modes for varying levels of autonomy. Feedback focuses on resolving inconsistencies in turn budget definitions across sections and refining the truncation strategy to prevent context window overflows when multiple tool results are returned simultaneously.
| - `max_model_steps_per_turn = 10` | ||
| - `max_total_tool_calls_per_turn = 24` |
There was a problem hiding this comment.
There is a discrepancy between the turn budgets defined in Section 3 (lines 465-466) and Section 5 (lines 722-723). Section 3 recommends a limit of 12 model steps and 32 tool calls per turn, whereas Section 5 specifies 10 steps and 24 tool calls for Driving Mode. Please clarify if Driving Mode is intended to have more restrictive limits or if these values should be synchronized.
| - `max_total_tool_calls_per_turn = 24` | ||
| - `max_prompt_tokens_from_history = 4_000` | ||
| - `max_model_output_tokens_per_step = 2_048` | ||
| - `max_total_tool_result_chars_in_context = 32_000` |
There was a problem hiding this comment.
The proposed per-tool truncation limit of 12,000 characters (line 489) may conflict with the total context limit of 32,000 characters (line 726) when multiple tool calls occur in a single turn. Given that the design allows up to 4 parallel tool calls (line 469), the aggregate length could reach 48,000 characters, exceeding the turn budget. The design should specify a strategy for prioritizing or further truncating results when the combined length of multiple tool outputs exceeds the total context allowance.
Summary
Notes
bin/plan.mdwas intentionally not linked because this task explicitly stated it does not exist for this research-only requestValidation