Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructures Brave Search server code, and adds Image search #594

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jonathansampson
Copy link

@jonathansampson jonathansampson commented Feb 4, 2025

Description

The code has been restructured into modules for maintainability. Each tool now has its own file, containing the description of the tool, related interfaces, and methods.

This PR also introduces support for a new tool: brave_image_search. Claude may not always immediately share image URLs in its results, but will do so if prompted again. An effort has been made to provide a prompt that will strongly encourage Claude to show clickable-links in its results when querying Brave Search's API for images.

Server Details

  • Server: brave-search
  • Changes to: tools, and structure

Motivation and Context

  • Updated structure helps with maintainability and testing in the future.
  • Introduction of Image search adds new functionality for consumers.

How Has This Been Tested?

Tested with Claude for Desktop on Windows. Claude at times will not always include markdown links in its response, but an effort has been made to carefully construct a tool description that strongly encourages Claude to include this information when relaying results.

Breaking Changes

None

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

N/A

Copy link
Contributor

@jerome3o-anthropic jerome3o-anthropic left a comment

Choose a reason for hiding this comment

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

some comments regarding tool descriptions/prompting


const data = await response.json() as Response;

return data.results.map(result => [
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, this is only relevant for clients that render markdown, I think using the actual ImageContent would be better

@jerome3o-anthropic
Copy link
Contributor

Thanks for the glow up of this server - the new layout is much nicer. Curious about the markdown prompting - is this because the tool result ImageContent in claudeai isn't rendered?

The code has been restructured into components for maintainability. Each tool now has its own file, containing the description of the tool, related interfaces, and methods.

This PR also introduces support for a new tool: brave_image_search. Claude may not always immediately share image URLs in its results, but will do so if prompted again. An effort has been made to provide a prompt that will strongly encourage Claude to show clickable-links in its results when querying Brave Search's API for images.

Update imageSearch.ts
I believe this is better for maintainability as the core description of the tool itself is readily available in the same location across all tool components.
@jonathansampson
Copy link
Author

jonathansampson commented Feb 26, 2025

Thanks for the glow up of this server - the new layout is much nicer. Curious about the markdown prompting - is this because the tool result ImageContent in claudeai isn't rendered?

Great question! I followed the example of earlier code in this server, and didn't think to consider whether or not images could be shown in the results (I am developing/testing in Claude for Desktop). In fact, if we can show the images themselves, that would simplify the prompt/instruction as well, which was largely written to ensure good markdown output. I can check the docs for this, but is there a way for the client to specify to the mcp-server whether or not it prefers/supports image/text results?

Update: I see examples of returning images within ../src/everart/index.ts; I'll follow that and push additional commits to this PR for consideration.

@jonathansampson
Copy link
Author

jonathansampson commented Mar 6, 2025

Thanks for the glow up of this server - the new layout is much nicer. Curious about the markdown prompting - is this because the tool result ImageContent in claudeai isn't rendered?

@jerome3o-anthropic , regarding ImageContent, I see there is a limit of 1048576 mentioned from time to time. Is this limit for UTF-16 characters in the base64-encoded image content, bits in the base64-encoded value, or something else?

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