Repository navigation
feat(ai): execute_tool dispatcher — drop 29 non-core tool schemas from model context - #1310
Merged
Merged
Conversation
Drops non-core tool names+descriptions from the model context by removing all 29 non-core tools from the tool list. Core tools (list/read/search/create) are sent with full schemas as before. The model uses execute_tool to call any non-core tool after discovering its schema via tool_search, shrinking the visible tool list from ~37 down to ~10. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSplit Global Assistant tools into core vs non-core: core tools are exposed directly to the model; non-core tools are discovered via ChangesTool Dispatch Model Refactor
sequenceDiagram
participant Client
participant Route as GlobalRoute
participant LLM
participant ToolSearch as tool_search
participant ExecuteTool as execute_tool
participant Tools as ToolCatalog
Client->>Route: POST /api/ai/global/[id]/messages
Route->>Tools: build filteredAllTools (pageSpaceTools + flags)
Route->>Tools: split coreTools / nonCoreTools (CORE_TOOL_NAMES)
Route->>LLM: send system prompt + coreTools + tool_search
LLM->>ToolSearch: tool_search("select:tool_name")
ToolSearch->>Route: return schema/info from filteredAllTools
LLM->>ExecuteTool: execute_tool({tool_name, parameters})
ExecuteTool->>Tools: lookup non-core tool in allowedTools
ExecuteTool->>Tools: validate parameters -> call tool.execute
Tools-->>ExecuteTool: tool result
ExecuteTool-->>LLM: execution response / error
LLM-->>Client: stream responses
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c839609e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/lib/ai/tools/execute-tool.ts (1)
22-29: ⚖️ Poor tradeoffType assertions in dispatch logic are pragmatic but limit type safety.
Lines 22 and 29 use type assertions to work around TypeScript's inability to preserve precise types through the dynamic dispatch pattern. This is a reasonable tradeoff for the flexibility of runtime tool routing, but it does mean:
- Line 22: The actual schema type might be more specific than
z.ZodType- Line 29: Return type is erased to
unknownIf stricter typing is needed in the future, consider using generics or a registry pattern with mapped types.
🤖 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 `@apps/web/src/lib/ai/tools/execute-tool.ts` around lines 22 - 29, The dynamic dispatch currently uses broad type assertions (realSchema = t.inputSchema as z.ZodType and t.execute as (args: unknown, opts: unknown) => unknown) which erases useful type information; fix by updating the tool registry/type definitions so t.inputSchema is declared as z.ZodTypeAny (or a generic z.ZodType<unknown>) and t.execute has a typed signature that accepts the inferred input (use z.infer on the schema in the registry or make ToolDefinition generic), then remove the local type assertions in execute-tool.ts and rely on the stronger ToolDefinition types for realSchema, parsed.data, and the execute call (referencing symbols: realSchema, t.inputSchema, parsed, t.execute, tool_name).
🤖 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.
Nitpick comments:
In `@apps/web/src/lib/ai/tools/execute-tool.ts`:
- Around line 22-29: The dynamic dispatch currently uses broad type assertions
(realSchema = t.inputSchema as z.ZodType and t.execute as (args: unknown, opts:
unknown) => unknown) which erases useful type information; fix by updating the
tool registry/type definitions so t.inputSchema is declared as z.ZodTypeAny (or
a generic z.ZodType<unknown>) and t.execute has a typed signature that accepts
the inferred input (use z.infer on the schema in the registry or make
ToolDefinition generic), then remove the local type assertions in
execute-tool.ts and rely on the stronger ToolDefinition types for realSchema,
parsed.data, and the execute call (referencing symbols: realSchema,
t.inputSchema, parsed, t.execute, tool_name).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 145369b4-cb2d-4787-90ca-ea1308bd77b3
📒 Files selected for processing (6)
apps/web/src/app/api/ai/global/[id]/messages/__tests__/stream-socket-events.test.tsapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/lib/ai/core/ai-tools.tsapps/web/src/lib/ai/core/stub-tools.tsapps/web/src/lib/ai/core/system-prompt.tsapps/web/src/lib/ai/tools/execute-tool.ts
💤 Files with no reviewable changes (1)
- apps/web/src/lib/ai/core/stub-tools.ts
The model now sees a compact grouped list of all 34 non-core tool names in the system prompt (~100-150 tokens), giving it a complete map of what is callable without schema overhead. Previously non-core tools were completely invisible after the execute_tool dispatcher change. Also removes the now-obsolete buildStubbedTools tests from stub-tools.test.ts and adds unit tests for buildNonCoreToolNamesPrompt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pageSpaceToolsStubbed was removed in the execute_tool dispatcher refactor. Replace its test block with equivalent corePageSpaceTools assertions covering export existence, key containment in CORE_TOOL_NAMES, and object reference identity with pageSpaceTools. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
Re: CodeRabbit nitpick on The assertions are intentional and correct for dynamic dispatch:
The tradeoff is explicitly acceptable here — runtime correctness is ensured by the |
…te-tool tests - execute-tool.ts: add guard for tools with no execute function (Tool.execute is optional in AI SDK; calling undefined throws TypeError) - route.ts: store buildNonCoreToolNamesPrompt result before concatenation so empty nonCoreTools doesn't append a stray '\n\n' to finalSystemPrompt - execute-tool.test.ts: 6 unit tests covering unknown tool, no-execute tool, invalid params, valid dispatch, and empty registry cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/lib/ai/tools/__tests__/execute-tool.test.ts (1)
7-10: ⚡ Quick winAssert the full dispatched payload.
The happy-path test only proves
startDatesurvived dispatch. IfcreateExecuteToolaccidentally drops or reshapesendDate, this still passes. Have the fake tool echo both inputs, or capture the args and assert the full object.Suggested test tightening
const calendarTool: Tool = { description: 'List calendar events', inputSchema: z.object({ startDate: z.string(), endDate: z.string() }), - execute: async ({ startDate }: { startDate: string; endDate: string }) => ({ events: [], startDate }), + execute: async ({ startDate, endDate }: { startDate: string; endDate: string }) => ({ + events: [], + startDate, + endDate, + }), };- ) as { events: unknown[]; startDate: string }; + ) as { events: unknown[]; startDate: string; endDate: string }; assert({ given: 'valid parameters for list_calendar_events', - should: 'return the real execute result', - actual: result.startDate, - expected: '2024-01-01', + should: 'forward the full parsed parameter object to the real execute', + actual: { startDate: result.startDate, endDate: result.endDate }, + expected: { startDate: '2024-01-01', endDate: '2024-01-31' }, });Also applies to: 67-78
🤖 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 `@apps/web/src/lib/ai/tools/__tests__/execute-tool.test.ts` around lines 7 - 10, The test's fake tool (calendarTool) only returns startDate so the test doesn't verify endDate survives dispatch; update the fake tool used in execute-tool.test.ts (the calendarTool object: inputSchema and execute) to echo both startDate and endDate (or capture the execute args) and then assert the full dispatched payload in the happy-path test (and the related test at lines 67-78) to ensure createExecuteTool doesn't drop or reshape endDate.
🤖 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 `@apps/web/src/app/api/ai/global/`[id]/messages/route.ts:
- Around line 711-715: The context-size accounting is underreporting because
calculateTotalContextSize(...) is still passed systemPrompt while the actual
prompt sent is finalSystemPrompt (which includes nonCoreToolNamesPrompt and
agentAwarenessPrompt additions); update the call to
calculateTotalContextSize(...) to use finalSystemPrompt instead of systemPrompt
and re-run tests/logging to ensure the reported token usage matches the
constructed prompt (refer to calculateTotalContextSize, finalSystemPrompt,
systemPrompt in the route handler).
---
Nitpick comments:
In `@apps/web/src/lib/ai/tools/__tests__/execute-tool.test.ts`:
- Around line 7-10: The test's fake tool (calendarTool) only returns startDate
so the test doesn't verify endDate survives dispatch; update the fake tool used
in execute-tool.test.ts (the calendarTool object: inputSchema and execute) to
echo both startDate and endDate (or capture the execute args) and then assert
the full dispatched payload in the happy-path test (and the related test at
lines 67-78) to ensure createExecuteTool doesn't drop or reshape endDate.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f0b5418-4c9e-4bd3-bd85-ae51cabbc9dd
📒 Files selected for processing (3)
apps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/lib/ai/tools/__tests__/execute-tool.test.tsapps/web/src/lib/ai/tools/execute-tool.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/lib/ai/tools/execute-tool.ts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rate token accounting Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 12, 2026
2witstudios
added a commit
that referenced
this pull request
May 15, 2026
…m model context (#1310) * feat(ai): replace stub tool schemas with execute_tool dispatcher Drops non-core tool names+descriptions from the model context by removing all 29 non-core tools from the tool list. Core tools (list/read/search/create) are sent with full schemas as before. The model uses execute_tool to call any non-core tool after discovering its schema via tool_search, shrinking the visible tool list from ~37 down to ~10. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(ai): inject non-core tool names into system prompt The model now sees a compact grouped list of all 34 non-core tool names in the system prompt (~100-150 tokens), giving it a complete map of what is callable without schema overhead. Previously non-core tools were completely invisible after the execute_tool dispatcher change. Also removes the now-obsolete buildStubbedTools tests from stub-tools.test.ts and adds unit tests for buildNonCoreToolNamesPrompt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(tests): update ai-tools.test.ts to use corePageSpaceTools pageSpaceToolsStubbed was removed in the execute_tool dispatcher refactor. Replace its test block with equivalent corePageSpaceTools assertions covering export existence, key containment in CORE_TOOL_NAMES, and object reference identity with pageSpaceTools. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ai): guard t.execute undefined + fix trailing newline + add execute-tool tests - execute-tool.ts: add guard for tools with no execute function (Tool.execute is optional in AI SDK; calling undefined throws TypeError) - route.ts: store buildNonCoreToolNamesPrompt result before concatenation so empty nonCoreTools doesn't append a stray '\n\n' to finalSystemPrompt - execute-tool.test.ts: 6 unit tests covering unknown tool, no-execute tool, invalid params, valid dispatch, and empty registry cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ai): use z.record(z.string(), z.unknown()) for Zod v4 compatibility Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ai): pass finalSystemPrompt to calculateTotalContextSize for accurate token accounting Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 27, 2026
This was referenced Jun 12, 2026
5 of 6 tasks
6 of 9 tasks
2witstudios
added a commit
that referenced
this pull request
Jul 27, 2026
The previous note said only "belongs in its own change", which reads as an unexamined punt. The actual reason is a trade-off worth recording, because the constant's own definition implies web_fetch should be in the set: - Page chat: genuinely broken. An agent whose saved allowlist omits web_fetch but who toggles web search on gets it rejected at dispatch. - Global Assistant: not broken. That route sets no allowlist, so web_fetch still dispatches through execute_tool. Adding it there only spends context on the one surface deliberately engineered to keep upfront schemas small (#1310 dropped 29 schemas for ~5-12k tokens/turn). So promoting it in the SHARED set would charge the Global Assistant for a page-chat-only fix. The likely correct fix is page-chat-specific, which this design already allows without further change: alwaysUpfront remains a per-call parameter, so that route can pass `new Set([...ALWAYS_UPFRONT_TOOLS, 'web_fetch'])` in isolation. Comment-only change; no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ukUtoKKTgQDH35docNcE6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes all 34 non-core tool names, descriptions, and schemas from the model's visible tool list. Reduces model context from ~37 tools to 10 (~5–12k token savings per request). The model gains a complete awareness map of what exists via a compact grouped tool name section in the system prompt (~120 tokens).
Architecture
Before (stub approach): model saw name + description + `{}` schema for all 37 tools
After: model sees only ~10 tools directly:
Non-core tool names appear as a compact grouped list in the system prompt:
```
NON-CORE TOOLS (use execute_tool to call; use tool_search("select:tool_name") for parameter schemas):
drive: create_drive, rename_drive, update_drive_context
pages: list_trash, list_conversations, read_conversation, rename_page, trash, restore, move_page, edit_sheet_cells
search: glob_search, web_search
tasks: update_task, get_assigned_tasks
agents: update_agent_config, list_agents, multi_drive_list_agents, ask_agent
activity: get_activity
calendar: list_calendar_events, get_calendar_event, check_calendar_availability, create_calendar_event, update_calendar_event, delete_calendar_event, rsvp_calendar_event, invite_calendar_attendees, remove_calendar_attendee
channels: send_channel_message
```
Changes
Tests
🤖 Generated with Claude Code