feat: Implement Tool Coordinator for efficient multi-step tool execution #375
Conversation
Feat: Improve prompting for exploration efficiency
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a tool coordination flow: a new coordinator that plans, executes, and aggregates multi-step tool calls; app/actions invokes the coordinator (optionally), injects a coordinator summary into final messages, and falls back to the original streaming researcher flow on coordinator errors. Researcher and geospatial tool signatures updated to support the flow. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as app/actions
participant Coord as toolCoordinator
participant Exec as executeToolPlan
participant Tool as Tool (various)
participant Agg as aggregateToolResults
participant Rsrch as researcher
UI->>Coord: request ToolPlan (messages)
Coord->>Coord: build & zod-validate ToolPlan
Coord-->>UI: ToolPlan
UI->>Exec: execute ToolPlan
loop for each step
Exec->>Exec: resolve dependencyIndices -> _dependencyResults
Exec->>Tool: call toolName with toolArgs (+ _dependencyResults)
Tool-->>Exec: ToolResultPart (success / error)
end
Exec-->>UI: ToolResultPart[]
UI->>Agg: aggregateToolResults(plan, results)
Agg-->>UI: markdown summary
alt coordinator succeeded
UI->>Rsrch: call researcher(finalMessages, useTools=false)
else coordinator failed
UI->>UI: log & notify user
UI->>Rsrch: fallback researcher(originalMessages, useTools=true)
end
Rsrch-->>UI: final synthesis response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
Overall, this is a very solid improvement that addresses systemic workflow issues. The only remaining asks: add some extra doc/examples, and double-check testing for both paths and new error cases. |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||
|
Manus AI seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||
…ed EOF build error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
app/actions.tsx(3 hunks)lib/agents/index.tsx(1 hunks)lib/agents/researcher.tsx(2 hunks)lib/agents/tool-coordinator.tsx(1 hunks)lib/agents/tools/geospatial.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
lib/agents/index.tsx (1)
lib/agents/tools/index.tsx (2)
ToolProps(7-11)ToolProps(14-39)
lib/agents/researcher.tsx (1)
lib/agents/tools/index.tsx (2)
getTools(14-39)ToolProps(7-11)
lib/agents/tools/geospatial.tsx (1)
lib/schema/geospatial.tsx (1)
geospatialQuerySchema(13-121)
app/actions.tsx (2)
lib/db/schema.ts (1)
messages(26-37)lib/agents/researcher.tsx (1)
researcher(76-154)
🪛 Biome (2.1.2)
lib/agents/tool-coordinator.tsx
[error] 77-77: unexpected token \
(parse)
app/actions.tsx
[error] 365-365: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 372-372: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
🔇 Additional comments (2)
lib/agents/index.tsx (1)
6-6: Expose Tool Coordinator via agents index is appropriateRe‑exporting
./tool-coordinatorfrom the agents index cleanly surfaces the new orchestration APIs to callers and matches howapp/actions.tsximports them. No issues here.lib/agents/researcher.tsx (1)
81-82: NewuseToolstoggle on researcher looks correct; ensure callers pass intent explicitlyAdding
useTools: boolean = trueand wiring it intotools: useTools ? getTools(...) : undefinedgives a clean, backwards‑compatible way to disable tool calls. Existing callers remain unaffected, andapp/actions.tsxcan now explicitly turn tools off when the coordinator has already run. Just ensure all new call sites intentionally decide whether to passfalse(as in the coordinator path) vs relying on the default.If you want to double‑check usage, run a quick search for
researcher(to confirm all non‑coordinator paths still rely on the default and don’t accidentally disable tools.Also applies to: 105-105
| import { Section } from '@/components/section' | ||
| import { FollowupPanel } from '@/components/followup-panel' | ||
| import { inquire, researcher, taskManager, querySuggestor, resolutionSearch } from '@/lib/agents' | ||
| import { inquire, researcher, taskManager, querySuggestor, resolutionSearch, toolCoordinator, executeToolPlan, aggregateToolResults } from '@/lib/agents' |
There was a problem hiding this comment.
🧩 Analysis chain
Fix syntax around researcher call and preserve coordinator tool results
There are two separate issues in this block:
-
Broken syntax (parse error) around
researchercall
Lines 365‑374 contain stray characters (c,)cAPI,, duplicated!useToolCoordinator), which Biome also flags. This makes the file invalid TypeScript/JS.You likely intended the loop body to look like this:
- while (
- useSpecificAPI
- ? answer.length === 0
- : answer.length === 0 && !errorOccurred
- ) {
- // If coordinator was used, pass finalMessages and disable tools for researcher
- c const { fullResponse, hasError, toolResponses } = await researcher(
- currentSystemPrompt,
- uiStream,
- streamText,
- finalMessages,
- useSpecificAPI,
- !useToolCoordinator // Pass a flag to disable tools if coordinator was used
- )cAPI,
- !useToolCoordinator // Pass a flag to disable tools if coordinator was used
- ) !useToolCoordinator // Pass a flag to disable tools if coordinator was used
- )
+ while (
+ useSpecificAPI
+ ? answer.length === 0
+ : answer.length === 0 && !errorOccurred
+ ) {
+ // If coordinator was used, pass finalMessages and disable tools for researcher
+ const { fullResponse, hasError, toolResponses } = await researcher(
+ currentSystemPrompt,
+ uiStream,
+ streamText,
+ finalMessages,
+ useSpecificAPI,
+ !useToolCoordinator // Pass a flag to disable tools if coordinator was used
+ )- Coordinator results are lost before being persisted
- When
useToolCoordinatoristrue, you correctly settoolOutputs = resultsfromexecuteToolPlan(plan). - But inside the loop you unconditionally do
toolOutputs = toolResponses. - In the coordinator path,
researcheris called withuseTools = !useToolCoordinator→false, sotoolResponsesis always[]. That overwrites the actual coordinator results. - As a result, the block that persists tool outputs to
aiState:
- When
if (toolOutputs.length > 0) {
toolOutputs.map(output => {
aiState.update({ ... })
})
}never sees the coordinator’s tool results.
To fix this while keeping current behavior for non‑coordinator runs, guard the assignment:
- answer = fullResponse
- toolOutputs = toolResponses
+ answer = fullResponse
+ if (!useToolCoordinator) {
+ toolOutputs = toolResponses
+ }Together with the cleaned‑up call, that will:
- Restore valid syntax.
- Ensure that, when the coordinator is enabled,
toolOutputscontinues to refer to the executed plan’s results and gets persisted intoaiStateas intended, while non‑coordinator flows keep usingresearcher’s tool responses.
After applying the fixes, re‑run your linter/formatter and verify:
- The file parses cleanly.
- With
USE_TOOL_COORDINATOR=true, you seetoolmessages in saved chats corresponding to the coordinator’s executed steps, and with itfalseyou still see the regular streaming-tool behavior.
Also applies to: 128-129, 323-357, 359-379
🏁 Script executed:
#!/bin/bash
# Inspect the actual code around the syntax errors at lines 320-380
cat -n app/actions.tsx | sed -n '320,380p'Length of output: 2626
🏁 Script executed:
#!/bin/bash
# Check lines around USE_TOOL_COORDINATOR flag (128-129)
cat -n app/actions.tsx | sed -n '125,135p'Length of output: 513
🏁 Script executed:
#!/bin/bash
# Check the toolOutputs assignment pattern in the file
rg -n 'toolOutputs\s*=' app/actions.tsx -C2Length of output: 362
Fix syntax errors in researcher call and preserve coordinator results
Lines 365–375 contain malformed syntax blocking file parsing:
- Line 365: stray
cbeforeconst - Line 372: malformed
)cAPI, - Lines 373–375: duplicated and misplaced
!useToolCoordinatorcomments with extra closing parentheses
Clean up the call to match the intended function signature:
while (
useSpecificAPI
? answer.length === 0
: answer.length === 0 && !errorOccurred
) {
// If coordinator was used, pass finalMessages and disable tools for researcher
- c const { fullResponse, hasError, toolResponses } = await researcher(
- currentSystemPrompt,
- uiStream,
- streamText,
- finalMessages,
- useSpecificAPI,
- !useToolCoordinator // Pass a flag to disable tools if coordinator was used
- )cAPI,
- !useToolCoordinator // Pass a flag to disable tools if coordinator was used
- ) !useToolCoordinator // Pass a flag to disable tools if coordinator was used
- )
+ const { fullResponse, hasError, toolResponses } = await researcher(
+ currentSystemPrompt,
+ uiStream,
+ streamText,
+ finalMessages,
+ useSpecificAPI,
+ !useToolCoordinator // Pass a flag to disable tools if coordinator was used
+ )Additionally, line 377 unconditionally overwrites toolOutputs (set from coordinator results at line 331) with toolResponses, which is empty when coordinator is enabled since researcher is called with tools disabled. Guard the assignment to preserve coordinator results:
answer = fullResponse
+ if (!useToolCoordinator) {
+ toolOutputs = toolResponses
+ }This ensures the block at line 380–396 that persists tool outputs receives the coordinator's executed plan results when enabled, while non-coordinator flows continue using researcher tool responses.
Committable suggestion skipped: line range outside the PR's diff.
| execute: async (params: z.infer<typeof geospatialQuerySchema> & { _dependencyResults?: any[] }) => { | ||
| const { queryType, includeMap = true, _dependencyResults } = params; | ||
| console.log('[GeospatialTool] Execute called with:', params); | ||
|
|
||
| if (_dependencyResults && _dependencyResults.length > 0) { | ||
| console.log('[GeospatialTool] Processing dependency results:', _dependencyResults); | ||
| // Logic to process dependency results can be added here. | ||
| // For example, if a previous step was a search, the result might contain coordinates | ||
| // that can be used as input for a subsequent directions query. | ||
| // Since the full logic for dependency injection is complex and depends on the | ||
| // specific tool schema, we will log it for now and ensure the tool can handle it. | ||
| // The LLM planning step is responsible for generating the correct 'params' | ||
| // based on the dependency results. The tool only needs to be aware of them. | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Dependency results plumbing is sound; consider scope of logging and persisted payload
Extending the executor signature to accept { _dependencyResults?: any[] } and simply ignoring it when absent is a good way to make the tool coordinator compatible without impacting existing callers.
Two things to be aware of:
- The new log
console.log('[GeospatialTool] Processing dependency results:', _dependencyResults);will print the full dependency payloads. Depending on size/sensitivity, you may want to redact or summarize these rather than dumping raw objects. - Returning
originalUserInput: JSON.stringify(params)now means_dependencyResults(including upstream tool outputs) will be embedded in the stored payload whenever the coordinator is involved. If that object can be large or contain redundant/sensitive data, consider omitting_dependencyResultsfrom the serialized copy:
- return { type: 'MAP_QUERY_TRIGGER', originalUserInput: JSON.stringify(params), queryType, ... }
+ const { _dependencyResults, ...cleanParams } = params
+ return {
+ type: 'MAP_QUERY_TRIGGER',
+ originalUserInput: JSON.stringify(cleanParams),
+ queryType,
+ ...
+ }Functionally the current change is correct; this is mainly about log hygiene and avoiding unnecessary data retention.
Also applies to: 346-347
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
app/actions.tsx (1)
374-374: GuardtoolOutputsassignment to preserve coordinator results.When
useToolCoordinatoristrue, line 331 setstoolOutputs = resultsfrom the executed plan. However, line 374 unconditionally overwrites this withtoolResponses. Sinceresearcheris called withuseTools = !useToolCoordinator(i.e.,falsewhen coordinator is active),toolResponseswill be an empty array, causing the coordinator's results to be lost before persistence at lines 377–393.Apply this diff to preserve coordinator results:
answer = fullResponse + if (!useToolCoordinator) { toolOutputs = toolResponses + } errorOccurred = hasErrorThis ensures that when the coordinator is enabled,
toolOutputsretains the executed plan's results for persistence, while non-coordinator flows continue using the researcher's tool responses. Based on learnings (past review).lib/agents/tool-coordinator.tsx (2)
5-5: Fix import path and correctgetToolsusage.Three related issues require fixing:
Wrong import path: Line 5 imports from
@/lib/tools, which doesn't exist. The correct path is@/lib/agents/tools(confirmed by the relevant code snippets fromlib/agents/tools/index.tsx).Missing required parameters: Lines 32 and 68 call
getTools({})with an empty object, butgetToolsrequires{ uiStream, fullResponse }parameters (seelib/agents/tools/index.tsxlines 6-10 for theToolPropsinterface).Wrong return type assumption: Lines 34 and 69 call
.map()directly on the result ofgetTools, butgetToolsreturns an object map of tools (e.g.,{ search: ..., retrieve: ..., geospatialQueryTool: ... }), not an array. You must useObject.values()to convert it to an array first.Apply these fixes:
Update the import at line 5:
-import { getTools } from '@/lib/tools' +import { getTools } from '@/lib/agents/tools' +import { createStreamableUI } from 'ai/rsc'Fix lines 32-38 in
toolCoordinator:const model = getModel() - const tools = getTools({}) + const toolsObj = getTools({ + uiStream: createStreamableUI(), + fullResponse: '' + }) - const toolDescriptions = tools.map(tool => ({ + const toolDescriptions = Object.values(toolsObj).map(tool => ({ name: tool.toolName, description: tool.description, parameters: tool.parameters }))Fix lines 68-69 in
executeToolPlan:- const allTools = getTools({}) - const toolMap = new Map(allTools.map(tool => [tool.toolName, tool])) + const toolsObj = getTools({ + uiStream: createStreamableUI(), + fullResponse: '' + }) + const toolMap = new Map(Object.values(toolsObj).map(tool => [tool.toolName, tool]))Based on learnings (past review).
Also applies to: 32-38, 68-69
87-91: Push aToolResultPartplaceholder for missing tools to maintain aggregation alignment.When a tool is not found, the code logs an error and stores it in
resultsbut doesn't push a correspondingToolResultParttotoolResults. This breaks the 1:1 alignment betweenplan.stepsandtoolResultsthataggregateToolResultsassumes at line 144-145, where it directly indexesplan.steps[index]based ontoolResultsiteration.If any step's tool is missing,
toolResults.length < plan.steps.length, causing incorrect step-to-result mapping and missing steps in the summary.Apply this diff to maintain alignment:
if (!tool) { console.error(`Tool ${step.toolName} not found.`) const errorResult = { error: `Tool ${step.toolName} not found.` } results.set(i, errorResult) + toolResults.push({ + toolName: step.toolName, + toolCallId: `coord-${i}`, + result: errorResult + }) continue }This ensures every step produces a corresponding
ToolResultPart, allowingaggregateToolResultsto correctly report all steps including failures. Based on learnings (past review).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
app/actions.tsx(3 hunks)lib/agents/tool-coordinator.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/actions.tsx (3)
lib/db/schema.ts (1)
messages(26-37)lib/agents/tool-coordinator.tsx (1)
toolCoordinator(30-60)lib/agents/researcher.tsx (1)
researcher(76-154)
lib/agents/tool-coordinator.tsx (7)
lib/utils/index.ts (1)
getModel(19-62)lib/agents/tools/index.tsx (2)
getTools(14-39)ToolProps(7-11)lib/agents/tools/retrieve.tsx (1)
ToolProps(8-67)lib/agents/tools/search.tsx (1)
ToolProps(8-54)lib/agents/researcher.tsx (1)
researcher(76-153)lib/agents/tools/video-search.tsx (1)
ToolProps(8-50)lib/agents/tools/geospatial.tsx (1)
mcpClient(247-260)
🪛 Biome (2.1.2)
lib/agents/tool-coordinator.tsx
[error] 77-77: unexpected token \
(parse)
🔇 Additional comments (1)
app/actions.tsx (1)
365-372: Researcher function signature is correct and matches the call.Verification confirms the
researcherfunction atlib/agents/researcher.tsx:76-83accepts the six-parameter signature as called. The function correctly defines:
- Parameter 6:
useTools: boolean = true- Return object includes
fullResponse,hasError, andtoolResponsesThe call at
app/actions.tsx:365-372passes all six arguments correctly, including the boolean flag!useToolCoordinatorfor theuseToolsparameter. No syntax or signature mismatch issues exist.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/agents/tool-coordinator.tsx (1)
24-27: Markdown special characters in error messages can break formatting.As noted in a previous review, line 26 embeds
result.errordirectly into the markdown output without escaping special characters. If the error message contains characters like#,*,[, or backticks, they could break the markdown rendering.The previous review suggested escaping or wrapping dynamic content in code blocks. Consider applying that fix:
if (hasError) { - summary += `**Status:** Failed + summary += `**Status:** ❌ Failed **Error:** ${result.error} `Or wrap the error in a code block for safer rendering:
if (hasError) { - summary += `**Status:** Failed -**Error:** ${result.error} + summary += `**Status:** ❌ Failed +**Error:** +\`\`\` +${result.error} +\`\`\` `
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/agents/tool-coordinator.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/agents/tool-coordinator.tsx (2)
app/actions.tsx (2)
output(339-353)msg(360-368)lib/agents/researcher.tsx (1)
researcher(76-153)
lib/agents/tool-coordinator.tsx
Outdated
| toolResults.forEach((toolResult, index) => { | ||
| const step = plan.steps[index] | ||
| const result = toolResult.result | ||
| const hasError = result && typeof result === 'object' && 'error' in result | ||
|
|
||
| summary += ` | ||
| #### Step ${index + 1}: ${step.purpose} (\`${step.toolName}\`) | ||
| ` |
There was a problem hiding this comment.
Add bounds checking for plan.steps[index] access.
The code assumes a 1:1 correspondence between toolResults and plan.steps, accessing plan.steps[index] at line 16 without verifying the index is valid. If toolResults.length > plan.steps.length (which can occur if the alignment issue noted in past reviews is not fixed in executeToolPlan), then step will be undefined and line 21 will throw a TypeError when accessing step.purpose and step.toolName.
Add a guard to handle misaligned arrays:
toolResults.forEach((toolResult, index) => {
const step = plan.steps[index]
+ if (!step) {
+ console.error(`No plan step found for tool result at index ${index}`)
+ return
+ }
const result = toolResult.resultBetter yet, validate the lengths upfront:
+ if (toolResults.length !== plan.steps.length) {
+ console.warn(`Mismatch: ${toolResults.length} tool results but ${plan.steps.length} plan steps`)
+ }
+
toolResults.forEach((toolResult, index) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| toolResults.forEach((toolResult, index) => { | |
| const step = plan.steps[index] | |
| const result = toolResult.result | |
| const hasError = result && typeof result === 'object' && 'error' in result | |
| summary += ` | |
| #### Step ${index + 1}: ${step.purpose} (\`${step.toolName}\`) | |
| ` | |
| if (toolResults.length !== plan.steps.length) { | |
| console.warn(`Mismatch: ${toolResults.length} tool results but ${plan.steps.length} plan steps`) | |
| } | |
| toolResults.forEach((toolResult, index) => { | |
| const step = plan.steps[index] | |
| if (!step) { | |
| console.error(`No plan step found for tool result at index ${index}`) | |
| return | |
| } | |
| const result = toolResult.result | |
| const hasError = result && typeof result === 'object' && 'error' in result | |
| summary += ` | |
| #### Step ${index + 1}: ${step.purpose} (\`${step.toolName}\`) | |
| ` |
🤖 Prompt for AI Agents
In lib/agents/tool-coordinator.tsx around lines 15 to 22, the code indexes
plan.steps[index] without checking bounds which can produce a TypeError if
toolResults is longer than plan.steps; add a guard so you only read step when
plan.steps[index] is defined (e.g. compute const validLen =
Math.min(toolResults.length, plan.steps.length) or skip iterations where
plan.steps[index] is undefined), and handle the misaligned case by either
skipping extra toolResults or appending a safe fallback summary entry and
logging a warning about the mismatch; also consider validating array lengths
once before the loop and early-return or trim toolResults to validLen.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/agents/tool-coordinator.tsx (1)
170-197: Aggregation assumes 1:1 alignment betweentoolResultsandplan.steps; consider a defensive guard
aggregateToolResultsindexesplan.steps[i]for eachtoolResults[i]without checking bounds. With the current implementation ofexecuteToolPlan, you always push exactly oneToolResultPartper step, so the invariant holds internally.However, if in the future someone:
- Appends extra
ToolResultParts, or- Calls
aggregateToolResultswith atoolResultsarray not produced byexecuteToolPlan,then
plan.steps[i]could beundefinedandstep.purposewill throw at runtime.Given the label on this PR and that this function formats user‑visible output, I’d add a small guard:
toolResults.forEach((tr, i) => { const step = plan.steps[i] if (!step) { console.warn(`[ToolCoordinator] No plan step for tool result at index ${i}`) return } // existing logic... })Not strictly required today, but it hardens the API against misuse. Since a similar concern was raised in a past review, this is effectively reinforcing that suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/agents/tool-coordinator.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/agents/tool-coordinator.tsx (2)
lib/utils/index.ts (1)
getModel(19-62)lib/agents/tools/index.tsx (1)
getTools(14-39)
🔇 Additional comments (3)
lib/agents/tool-coordinator.tsx (3)
70-103: Planner flow looks correct; just ensure model and tools metadata stay alignedThe planner correctly:
- Fetches tools via
getTools({ uiStream: createStreamableUI(), fullResponse: '' }).- Projects them to
{ name, description, parameters }and embeds that JSON into the system prompt.- Uses
generateObjectwithtoolPlanSchemato get a typedToolPlan.Two things to keep in mind:
- If
getModel()throws (e.g., because the dynamic require fallback fails), the error will bubble from here; confirm callers wraptoolCoordinatorin a try/catch and fall back to the legacy path (per PR description).- The planning call instantiates real tool instances just to read metadata. If tool construction ever becomes expensive or side‑effectful, you may want a lightweight, metadata‑only registry instead of calling
getToolshere.Overall, the logic is sound and matches the intended orchestrator pattern.
109-164: Execution logic and dependency handling are solid; logging and error shaping look reasonableThis executor is in good shape:
- Tools are resolved once into
toolMapkeyed bytoolName.resolveDepsenforces that dependencies refer only to already‑executed steps (results.has(i)), which prevents using undefined results.- A missing tool or dependency just yields
{ error: ... }for that step; you still record it in bothresultsandtoolResults, keeping alignment withplan.steps._dependencyResultsis passed only when there are dependencies, avoiding noisy args for tools that don’t support it.Minor considerations:
- Dependency indices that point to future steps or to the current step are treated as “missing” and surface as errors. That’s acceptable, but if you want stricter guarantees you could validate
dependencyIndicesduring planning (e.g., all< current index).console.log/console.errorcontain step index and tool name only, which is good from a privacy standpoint; ensure no PII‑bearing args get logged elsewhere.Functionally, this executor looks correct for sequential orchestrated tool calls.
11-45: Dynamicrequirefallbacks forgetModel/createStreamableUImay not behave as intended under bundlersThis block assumes that missing modules like
@/lib/modelsor@/lib/streamablewill be handled at runtime viatry/catch, but most bundlers (Next/Webpack) still need to statically resolve theserequire()paths. If those modules don’t exist, you may get a build‑time “module not found” error despite thetry/catch. Also, there is already a canonicalgetModelinlib/utils/index.ts(see snippet), so this introduces a second, slightly different resolution path.I recommend either:
- Re‑using the existing
getModelexport (or a single shared@/lib/modelsmodule that it re‑exports from), and- Ensuring the
createStreamableUIpath matches how other agents obtain it, or centralizing this in a shared utility.At minimum, please verify that
@/lib/modelsand@/lib/streamable(or@/lib/ui/streamable) actually exist and resolve correctly in this repo’s build pipeline.
| const toolStepSchema = z.object({ | ||
| toolName: z.string(), | ||
| toolArgs: z.record(z.any()), | ||
| dependencyIndices: z.array(z.number()).optional(), | ||
| purpose: z.string() | ||
| }) | ||
|
|
||
| const toolPlanSchema = z.object({ | ||
| reasoning: z.string(), | ||
| steps: z.array(toolStepSchema) | ||
| }) | ||
|
|
||
| export type ToolPlan = z.infer<typeof toolPlanSchema> | ||
| export type ToolStep = z.infer<typeof toolStepSchema> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Schema is functional but could be tightened for safety and guidance
The toolStepSchema/toolPlanSchema are straightforward and work, but they’re very permissive:
toolArgs: z.record(z.any())allows arbitrary shapes, including non‑JSON‑safe values.- There’s no bound on
stepslength, so a misbehaving model could produce a very large plan.
Consider:
- Switching
toolArgstoz.record(z.unknown())(or a narrower shape if you standardize tool args) to avoid implicitly “any” values. - Adding
.max(n)tosteps(e.g. 6–10) to guard against pathological plans and control tool‑call blast radius.
These changes aren’t required for correctness but would improve robustness.
🤖 Prompt for AI Agents
In lib/agents/tool-coordinator.tsx around lines 51–64, the zod schemas are too
permissive: change toolArgs from z.record(z.any()) to z.record(z.unknown()) (or
a narrower shape if you have a standardized arg schema) to avoid implicit
any/unsafe values, and add a max bound to steps (e.g., .max(10)) on the
toolPlanSchema's steps array to limit plan size; keep dependencyIndices optional
as-is. Ensure types exported (ToolPlan/ToolStep) remain in sync after these
schema changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/actions.tsx (1)
373-374: Coordinator tool results are lost before persistenceLine 374 unconditionally overwrites
toolOutputswithtoolResponses. WhenuseToolCoordinatoristrue:
- Line 331 sets
toolOutputs = results(coordinator's executed plan results)- Line 371 calls researcher with
useTools = false, sotoolResponsesis[]- Line 374 overwrites coordinator results with
[]- The persistence block at lines 377-393 never sees the coordinator's tool results
This was flagged in the previous review.
Guard the assignment to preserve coordinator results:
answer = fullResponse + if (!useToolCoordinator) { + toolOutputs = toolResponses + } - toolOutputs = toolResponses errorOccurred = hasErrorThis ensures coordinator results persist to
aiState, while non-coordinator flows continue using researcher tool responses.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
app/actions.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/actions.tsx (4)
lib/db/schema.ts (1)
messages(26-37)lib/agents/tool-coordinator.tsx (3)
toolCoordinator(70-103)executeToolPlan(114-164)aggregateToolResults(170-197)components/message.tsx (1)
BotMessage(11-31)lib/agents/researcher.tsx (1)
researcher(76-154)
🔇 Additional comments (2)
app/actions.tsx (2)
15-15: LGTM!The coordinator function imports are correctly added and align with the module exports.
365-372: LGTM!The researcher call syntax is now clean and correctly passes
finalMessagesand disables tools via!useToolCoordinatorwhen the coordinator is active.
| if (useToolCoordinator) { | ||
| uiStream.update(<div><Spinner /> Planning tool execution...</div>) | ||
| try { | ||
| const plan = await toolCoordinator(messages) | ||
| uiStream.update(<div><Spinner /> Executing tool plan...</div>) | ||
| const results = await executeToolPlan(plan) | ||
| toolOutputs = results | ||
| const summary = aggregateToolResults(results, plan) | ||
|
|
||
| // Add the summary to the messages for the final synthesis agent | ||
| finalMessages = [ | ||
| ...messages, | ||
| { | ||
| id: nanoid(), | ||
| role: 'tool', | ||
| content: summary, | ||
| type: 'tool_coordinator_summary' | ||
| } as any // Cast to any to satisfy CoreMessage type for custom type | ||
| ] | ||
|
|
||
| // Stream a message to the user about the tool execution completion | ||
| uiStream.append( | ||
| <BotMessage content="Tool execution complete. Synthesizing final answer..." /> | ||
| ) | ||
| } catch (e) { | ||
| console.error('Tool Coordinator failed:', e) | ||
| uiStream.append( | ||
| <BotMessage content="Tool Coordinator failed. Falling back to streaming researcher." /> | ||
| ) | ||
| // Fallback: continue with the original messages and let the researcher handle it | ||
| finalMessages = messages | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix missing parameter, type mismatches, and BotMessage usage
Three issues in the coordinator flow:
-
Missing required context parameter (line 330)
executeToolPlanexpects(plan, context)but is called with onlyplan. This will cause a runtime error. -
Type mismatch for tool message content (lines 335-343)
Role'tool'inCoreMessageexpectscontentto beToolResultPart[], not a string. The cast toanyat line 342 hides this type error and may cause issues whenresearcherprocessesfinalMessages. -
BotMessage expects StreamableValue (lines 347, 352)
BotMessagecomponent requirescontent: StreamableValue<string>, but plain strings are passed.
Apply these fixes:
if (useToolCoordinator) {
uiStream.update(<div><Spinner /> Planning tool execution...</div>)
try {
const plan = await toolCoordinator(messages)
uiStream.update(<div><Spinner /> Executing tool plan...</div>)
- const results = await executeToolPlan(plan)
+ const results = await executeToolPlan(plan, { uiStream, fullResponse: '' })
toolOutputs = results
const summary = aggregateToolResults(results, plan)
- // Add the summary to the messages for the final synthesis agent
+ // Add coordinator summary as an assistant message for synthesis
finalMessages = [
...messages,
{
id: nanoid(),
- role: 'tool',
+ role: 'assistant',
content: summary,
type: 'tool_coordinator_summary'
- } as any // Cast to any to satisfy CoreMessage type for custom type
+ }
]
- // Stream a message to the user about the tool execution completion
+ const completionMsg = createStreamableValue<string>()
+ completionMsg.done("Tool execution complete. Synthesizing final answer...")
uiStream.append(
- <BotMessage content="Tool execution complete. Synthesizing final answer..." />
+ <BotMessage content={completionMsg.value} />
)
} catch (e) {
console.error('Tool Coordinator failed:', e)
+ const errorMsg = createStreamableValue<string>()
+ errorMsg.done("Tool Coordinator failed. Falling back to streaming researcher.")
uiStream.append(
- <BotMessage content="Tool Coordinator failed. Falling back to streaming researcher." />
+ <BotMessage content={errorMsg.value} />
)
- // Fallback: continue with the original messages and let the researcher handle it
finalMessages = messages
}
}Changing the role from 'tool' to 'assistant' ensures type safety and allows the summary to flow naturally to the researcher as context, while the actual tool results are already captured in toolOutputs for persistence.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/actions.tsx around lines 325 to 357, fix three issues: call
executeToolPlan with the missing context parameter (pass the current context
object used elsewhere when invoking tools), change the synthesized tool-summary
message to use role 'assistant' (not 'tool') and remove the any cast so the
content is a plain string (or appropriate assistant content type) to satisfy
CoreMessage typing, and ensure BotMessage receives a StreamableValue<string> by
wrapping the plain string messages with the project’s streamable value helper
(or creating a StreamableValue) before passing to uiStream.append/update.
User description
This pull request implements the Tool Coordinator worker, which introduces the Orchestrator-Worker pattern for tool use, as outlined in the Agent workflow documentation.
Key Improvements for Mapbox/Geospatial Tool Efficiency:
geospatialToolis updated to accept_dependencyResults, allowing it to consume outputs from previous steps in the coordinated plan.app/actions.tsxand controlled by theUSE_TOOL_COORDINATORenvironment variable for safe, gradual rollout.This change significantly improves the reliability and efficiency of multi-tool queries, especially those involving the Mapbox/Geospatial tool.
PR Type
Enhancement
Description
Implements Tool Coordinator worker for structured multi-step tool execution planning
Enables sequential tool workflows with dependency tracking between steps
Updates geospatialTool to accept and process dependency results from previous steps
Integrates coordinator into app actions with USE_TOOL_COORDINATOR feature flag
Adds fallback mechanism to researcher when coordinator execution fails
Diagram Walkthrough
File Walkthrough
tool-coordinator.tsx
New Tool Coordinator module for multi-step planninglib/agents/tool-coordinator.tsx
toolStepSchemaandtoolPlanSchemafor structured toolexecution planning
toolCoordinator()function to analyze queries and generatemulti-step plans using LLM
executeToolPlan()function to sequentially execute plannedsteps with dependency resolution
aggregateToolResults()function to summarize executionresults for final synthesis
actions.tsx
Integrate Tool Coordinator into action submission flowapp/actions.tsx
toolCoordinator,executeToolPlan,aggregateToolResults)USE_TOOL_COORDINATORenvironment variable check for feature flagcontrol
enabled
useToolsflag to researcher to disable tools when coordinatoris active
geospatial.tsx
Add dependency results support to geospatial toollib/agents/tools/geospatial.tsx
execute()function signature to accept optional_dependencyResultsparametersteps
researcher.tsx
Add conditional tool usage to researcher agentlib/agents/researcher.tsx
useToolsparameter (defaults totrue) to control toolavailability
streamText()based onuseToolsflagactive
index.tsx
Export Tool Coordinator functionslib/agents/index.tsx
application
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.