Skip to content

feat(nimbus-mcp): add search_icons tool#1230

Open
tylermorrisford wants to merge 6 commits intomainfrom
CRAFT-2136-search-icons-tool
Open

feat(nimbus-mcp): add search_icons tool#1230
tylermorrisford wants to merge 6 commits intomainfrom
CRAFT-2136-search-icons-tool

Conversation

@tylermorrisford
Copy link
Contributor

@tylermorrisford tylermorrisford commented Mar 10, 2026

Summary

  • Add search_icons MCP tool with two-pass search (substring then Fuse.js fuzzy) against icon names and keywords
  • Returns up to 10 matching icons with import paths from @commercetools/nimbus-icons, paginated at 5 per page with an offset parameter
  • Consolidate duplicate IconResult type into shared IconCatalogEntry from types.ts

Closes CRAFT-2136

Test plan

  • pnpm test packages/nimbus-mcp/ — all tests pass
  • search_icons("checkmark") returns CheckCircle, Check, Done with correct import paths
  • Results are capped at 10, with 5 shown per page
  • Passing offset: 5 returns the next page of results
  • Nonsense queries return fewer results than real queries

🤖 Generated with Claude Code

tylermorrisford and others added 3 commits March 10, 2026 17:19
Two-pass search (substring then Fuse.js fuzzy) against icon names and
keywords from the icon catalog. Returns up to 20 matching icons with
import paths from @commercetools/nimbus-icons.

CRAFT-2136

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ntry type

Remove duplicate IconResult type declarations from search-icons tool and
tests, using the existing IconCatalogEntry from types.ts instead. Also
removes the now-unnecessary toResult mapper.

CRAFT-2136

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Avoids confusion with an upcoming tool that uses the same variable name.

CRAFT-2136

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nimbus-documentation Ready Ready Preview, Comment Mar 16, 2026 9:28pm
nimbus-storybook Ready Ready Preview, Comment Mar 16, 2026 9:28pm

Request Review

@changeset-bot
Copy link

changeset-bot bot commented Mar 10, 2026

⚠️ No Changeset found

Latest commit: d6be945

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Reduce token usage by returning 5 icons per page instead of all 20 at once.
Adds optional offset param and structured response metadata (totalIcons,
totalResults, hasMore, hint) so LLMs can paginate when needed. Also lowers
max search results from 20 to 10 since tail matches are low-signal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rch quality

Update tests to validate paginated response shape (totalIcons, totalResults,
offset, pageSize, hasMore, hint) and pagination behavior across pages.

Also fix two pre-existing search quality issues exposed by the lower result cap:
- Skip single-char keywords in substring matching to prevent false positives
- Rank name matches above keyword matches so exact name hits aren't pushed out

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
*/
async function searchIcons(query: string): Promise<IconCatalogEntry[]> {
const catalog = await getCatalog();
const needle = query.toLowerCase();
Copy link
Collaborator

@valoriecarli valoriecarli Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im sure needle means something im not familiar with yet. yes, i can see that nameLower is used further down.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refers to the figurative 'haystack'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was so random that i suspected as much. TIL

const MIN_KEYWORD_LENGTH = 2;

/** Cached catalog + Fuse instance (created on first call). */
let cachedCatalog: IconCatalog | undefined;
Copy link
Collaborator

@valoriecarli valoriecarli Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this caught my eyeball. just one.
is there a reason why someone (naming names not necessary) didn't memoize getIconCatalog() using lazyJson in the data-loader file so you wouldn't have to clean up her mess here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch 🔧 d6be945

Copy link
Collaborator

@valoriecarli valoriecarli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had some questions, but nothing we can get to as we continue to build this out and keep learning how to do this effectively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants