feat(chat): let tools claim the names a server derives from them - #7185
Draft
Haroenv wants to merge 8 commits into
Draft
feat(chat): let tools claim the names a server derives from them#7185Haroenv wants to merge 8 commits into
Haroenv wants to merge 8 commits into
Conversation
Resolving a tool part by prefix is a guess: `a_b` is ambiguous between the tool `a_b` and the tool `a` addressing `b`, and no naming rule tells those apart. Preferring the shorter key renders `foo_bar_products` with `foo` when both `foo` and `foo_bar` are registered; preferring the longer one hands `search_index` on the `products` index to `search_index_products`. Which one wins is the registration site's call, not something the resolver can infer. Add `matchesToolName` to the tool contract, so a tool opts in to the names a server derives from it. `findTool` resolves an exact registration first and otherwise only considers tools that claim the name, so two overlapping registrations no longer collide at all. Among claimants the most specific one wins and a dev warning names the conflict, so the winner never depends on registration order. The Algolia MCP Server's per-index search tool names are declared in `connectChat` rather than guessed: it is the Algolia-specific layer both flavors and the renderer read their tools from, and it covers users who go through the connector without the widget's default tools. That leaves `instantsearch-ui-components` with no tool-name knowledge at all. `chat.tsx` held a third copy of the resolution rule, still first-match-wins and unreachable since the connector keys its tools the same way the widget does; it goes through `findTool` now. A dev warning fires when a name goes unresolved while a registered tool is a prefix of it, pointing at `matchesToolName`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 13 |
TIP This summary will be updated as you push new changes.
More templates
algoliasearch-helper
instantsearch-ui-components
instantsearch.css
instantsearch.js
react-instantsearch
react-instantsearch-core
react-instantsearch-nextjs
react-instantsearch-router-nextjs
vue-instantsearch
commit: |
|
Size Change: +1.1 kB (+0.09%) Total Size: 1.24 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Chat tool-resolution contract so that derived / suffixed tool names (e.g. my_tool_products) are no longer inferred from string prefixes, and instead must be explicitly “claimed” by the registered tool via a new optional matchesToolName(toolName) predicate. This centralizes resolution in findTool, avoids ambiguous overlaps like a_b vs a + b, and keeps the Algolia MCP Server naming convention declared at the Algolia-specific layer (connectChat) rather than guessed in UI components.
Changes:
- Add
matchesToolName?: (toolName: string) => booleanto theClientSideToolcontract and updatefindToolto only resolve derived names when a tool explicitly claims them (with deterministic conflict handling + dev warnings). - Declare the Algolia MCP Server’s per-index search-tool naming (
algolia_search_index_<index>) inconnectChatvia a defaultmatchesToolNameforalgolia_search_index. - Update widget/connector/common tests to cover claimed vs unclaimed derived names and overlapping registrations.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/common/widgets/chat/options.tsx | Adds common-widgets coverage ensuring only tools that claim derived names render for suffixed tool parts. |
| packages/instantsearch.js/src/widgets/chat/chat.tsx | Removes bespoke prefix-guessing and delegates tool lookup to shared findTool. |
| packages/instantsearch.js/src/lib/chat/index.ts | Introduces matchesSearchIndexToolName helper for Algolia MCP Server’s suffixed search tool naming. |
| packages/instantsearch.js/src/connectors/chat/connectChat.ts | Injects a default matchesToolName for algolia_search_index when not explicitly set by the user. |
| packages/instantsearch.js/src/connectors/chat/tests/connectChat-test.ts | Adds connector tests for claimed vs unclaimed derived tool names during tool-call dispatch. |
| packages/instantsearch-ui-components/src/lib/utils/chat.ts | Updates findTool resolution logic to prefer exact matches, otherwise require explicit claims; adds dev warnings for conflicts/unresolved prefixes. |
| packages/instantsearch-ui-components/src/lib/utils/tests/chat-test.ts | Rewrites findTool unit tests around the new claim-based contract and warnings. |
| packages/instantsearch-ui-components/src/components/chat/types.ts | Extends ClientSideTool public type with matchesToolName documentation and signature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+106
to
+110
| `No tool is registered for "${toolName}". The registered ${prefixes | ||
| .map((key) => `"${key}"`) | ||
| .join( | ||
| ', ' | ||
| )} is a prefix of it, but a prefix alone doesn't resolve: declare \`matchesToolName\` on it to also handle "${toolName}".` |
Contributor
Author
There was a problem hiding this comment.
Good catch — fixed in 28ebb06. The message now branches on the count:
- one prefix:
The registered tool "foo" is a prefix of it - several:
The registered tools "foo", "foo_bar" are prefixes of it
The tail also changed from "declare matchesToolName on it" to "on the tool that should handle …", since "it" no longer refers to a single tool. Added a unit test for the plural case.
— via Claude
The message named every registered prefix but always read "The registered … is a prefix of it", which is wrong as soon as two tools qualify. Branch on the count, and point at "the tool that should handle" it rather than "it", which no longer refers to a single tool. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Haroenv
force-pushed
the
fix/chat-tool-name-resolution
branch
from
August 20, 2026 15:20
a588d5e to
28ebb06
Compare
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
Stacked on #7172, addressing #7172 (comment).
That thread chased a tiebreak rule for overlapping tool names, but the rule isn't the problem — the guessing is.
a_bis genuinely ambiguous between the toola_band the toolaaddressingb, and nothing in the string tells you which. Every rule is therefore wrong for someone, which is exactly what the thread found:master) →fooregistered beforefoo_bar, sofoo_bar_productsrenders withfoo.search_instant_search_productssteals the call meant forsearch_instant_searchon theproductsindex.Rejecting overlapping names doesn't work either:
algolia_search_indexand a hypotheticalalgolia_search_index_v2are both legitimate registrations.So this applies the same move #7172 already makes for rendering — the registration site declares intent, the resolver stops inferring.
shouldRenderlets a tool opt out of rendering;matchesToolNamelets it opt in to a name.shouldRender's siblingmatchesToolName?: (toolName: string) => booleantoClientSideTool; it reaches consumers through the existingUserClientSideToolomit.findToolresolves an exact registration first, and otherwise only considers tools that claim the name. Two overlapping registrations no longer collide at all — which answers "can we reject overlapping names?" with "they're no longer overlapping".connectChatrather than guessed.chat.tsxheld a third copy of the resolution rule (still first-match-wins, and unreachable — the connector keys its tools the same way the widget does). It goes throughfindToolnow.matchesToolName.Why
connectChatand not the default toolsconnectChatis the Algolia-specific layer — it already sends thex-algolia-*headers — and it's the single normalization point both flavors and the renderer read their tools from. Putting the MCP naming there means:instantsearch-ui-componentsends up with zero tool-name knowledge, which is stronger than what feat(chat): let tools control their own rendering #7172 achieves on its own.connectChat/useChatusers keep working unchanged. A defaults-only approach (declaring it in each flavor'screateDefaultTools) would have quietly broken them, since they never go through the widget's defaults.It's a default, not a branch: an explicit
matchesToolNamewins, and so does a tool registered under the derived name itself.Public API
Additive — one optional property, no breaking change (semver minor):
Behaviour change to note
#7172's "the name-suffix shim is generic now" widening is reverted, and narrowed past
master: the loader and connector previously prefix-matched any tool throughfindTool, undocumented and accidental. That now requires an explicit claim. The dev warning covers the migration by naming the tool and the property to add. The one real case —algolia_search_index— is unaffected, and its existing test passes unmodified.Tests
findToolunit tests rewritten around the new contract, including both failure cases from the thread.connectChattests: a tool claiming derived names, and one not claiming them.helloclaimshello_productsand renders;goodbyedoesn't claimgoodbye_productsand doesn't.yarn type-check,yarn lint:changedand the full chat surface (153 suites, 2111 tests) are green.🤖 Generated with Claude Code