Add brave as alternative WebSearchTool#245
Merged
claude-code-best merged 1 commit intoclaude-code-best:mainfrom Apr 12, 2026
Merged
Add brave as alternative WebSearchTool#245claude-code-best merged 1 commit intoclaude-code-best:mainfrom
claude-code-best merged 1 commit intoclaude-code-best:mainfrom
Conversation
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:
📝 WalkthroughWalkthroughAdds a BraveSearchAdapter that queries Brave’s LLM context endpoint, extracts and normalizes grounding entries into SearchResult[], updates adapter factory to optionally select Brave via WEB_SEARCH_ADAPTER with caching, and adds unit/integration tests plus documentation for the new backend. Changes
Sequence DiagramsequenceDiagram
participant Client
participant BraveAdapter as BraveSearchAdapter
participant HTTP as axios
participant Brave as search.brave.com
participant Parser as extractBraveResults
Client->>BraveAdapter: search(query, { onProgress, signal })
BraveAdapter->>Client: onProgress({ type: "query_update", query })
BraveAdapter->>HTTP: GET /res/v1/llm/context?q=... (headers: X-Subscription-Token, timeout, signal)
HTTP->>Brave: request
Brave-->>HTTP: grounding JSON response
HTTP-->>BraveAdapter: payload
BraveAdapter->>Parser: extractBraveResults(payload)
Parser->>Parser: flatten buckets, normalize snippets, dedupe
Parser-->>BraveAdapter: SearchResult[]
BraveAdapter->>BraveAdapter: apply allowed/blocked domain filtering
BraveAdapter->>Client: onProgress({ type: "search_results_received", resultCount, query })
BraveAdapter-->>Client: return filtered SearchResult[]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds a Brave-backed web search adapter and wires it into the WebSearchTool adapter factory as an optional backend.
Changes:
- Added
BraveSearchAdapterthat fetches Brave Search HTML and extracts organic results. - Updated adapter factory to support
WEB_SEARCH_ADAPTER=brave(and restored env override + API/Bing selection logic). - Added unit tests for Brave HTML extraction and adapter behavior, plus a runnable integration script hitting real Brave Search.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/tools/WebSearchTool/adapters/index.ts |
Adds Brave adapter option and updates factory selection/caching behavior. |
src/tools/WebSearchTool/adapters/braveAdapter.ts |
Implements Brave HTML fetch + regex extraction and domain filtering. |
src/tools/WebSearchTool/__tests__/braveAdapter.test.ts |
Adds unit tests for entity decoding, HTML extraction, and adapter behavior with mocked axios. |
src/tools/WebSearchTool/__tests__/braveAdapter.integration.ts |
Adds a manual integration script that queries Brave and prints/validates results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+18
| // Adapter is stateless — safe to reuse across calls within a session | ||
| if (cachedAdapter) return cachedAdapter |
There was a problem hiding this comment.
createAdapter() caches the adapter instance, but the adapter choice depends on process.env.WEB_SEARCH_ADAPTER and isFirstPartyAnthropicBaseUrl() (which is driven by process.env.ANTHROPIC_BASE_URL). If those settings can change at runtime (e.g., settings reload without restarting), the cache will cause the tool to keep using the old backend. Consider either removing caching, keying the cache by the relevant env/config values, or providing an explicit cache reset when settings/env change.
Comment on lines
+20
to
+39
| // Env override: WEB_SEARCH_ADAPTER=api|bing|brave forces specific backend | ||
| const envAdapter = process.env.WEB_SEARCH_ADAPTER | ||
| if (envAdapter === 'api') { | ||
| cachedAdapter = new ApiSearchAdapter() | ||
| return cachedAdapter | ||
| } | ||
| if (envAdapter === 'bing') { | ||
| cachedAdapter = new BingSearchAdapter() | ||
| return cachedAdapter | ||
| } | ||
| if (envAdapter === 'brave') { | ||
| cachedAdapter = new BraveSearchAdapter() | ||
| return cachedAdapter | ||
| } | ||
|
|
||
| // // Anthropic official URL → API server-side search | ||
| // if (isFirstPartyAnthropicBaseUrl()) { | ||
| // cachedAdapter = new ApiSearchAdapter() | ||
| // return cachedAdapter | ||
| // } | ||
| // Anthropic official URL → API server-side search | ||
| if (isFirstPartyAnthropicBaseUrl()) { | ||
| cachedAdapter = new ApiSearchAdapter() | ||
| return cachedAdapter | ||
| } |
There was a problem hiding this comment.
This PR is titled as adding Brave as an alternative adapter, but this hunk also re-enables the API-vs-Bing selection logic (including defaulting to ApiSearchAdapter on first-party base URLs) instead of always using Bing. If that broader behavior change is intentional, it would help to reflect it in the PR title/description; if not intentional, the factory should probably keep the previous default behavior and only add the Brave override path.
Comment on lines
+47
to
+50
| onProgress?.({ type: 'query_update', query }) | ||
|
|
||
| const url = `https://search.brave.com/search?q=${encodeURIComponent(query)}&source=web` | ||
|
|
There was a problem hiding this comment.
This introduces a new outbound dependency on https://search.brave.com for web search. Please make sure the repo’s external-dependency / network-call documentation is updated accordingly (e.g., docs/external-dependencies.md currently lists Bing Search) and note any new environment switch (WEB_SEARCH_ADAPTER=brave) that controls it.
Comment on lines
+11
to
+34
| const FETCH_TIMEOUT_MS = 30_000 | ||
|
|
||
| /** | ||
| * Browser-like headers to avoid Brave's anti-bot JS-rendered response. | ||
| * These mimic Microsoft Edge on macOS to get full HTML search results. | ||
| */ | ||
| const BROWSER_HEADERS = { | ||
| 'User-Agent': | ||
| 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36 Edg/131.0.0.0', | ||
| Accept: | ||
| 'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8', | ||
| 'Accept-Language': 'en-US,en;q=0.9', | ||
| 'Accept-Encoding': 'gzip, deflate, br', | ||
| 'Cache-Control': 'no-cache', | ||
| Pragma: 'no-cache', | ||
| 'Sec-Ch-Ua': '"Microsoft Edge";v="131", "Chromium";v="131", "Not_A Brand";v="24"', | ||
| 'Sec-Ch-Ua-Mobile': '?0', | ||
| 'Sec-Ch-Ua-Platform': '"macOS"', | ||
| 'Sec-Fetch-Dest': 'document', | ||
| 'Sec-Fetch-Mode': 'navigate', | ||
| 'Sec-Fetch-Site': 'none', | ||
| 'Sec-Fetch-User': '?1', | ||
| 'Upgrade-Insecure-Requests': '1', | ||
| } as const |
There was a problem hiding this comment.
BROWSER_HEADERS, timeout, abort wiring, and domain-filtering logic are now duplicated across Bing and Brave adapters. To reduce maintenance burden when headers/parsing behavior needs updates, consider extracting shared request + filtering helpers/constants into a common module reused by both adapters.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/tools/WebSearchTool/__tests__/braveAdapter.test.ts (1)
1-2: Avoid preloadingbraveAdapterbefore registering module mocks.Line 2 imports the module under test before any axios mock is registered, while Lines 284-286 assume the later dynamic import sees the mock. That breaks the repo's required Bun mocking pattern and makes these tests order-dependent. Keep this file purely dynamic-imported after
mock.module(), or split the pure helper tests into a separate file.As per coding guidelines, "Use
mock.module()with inlineawait import()in test files for testing modules with heavy dependencies".Also applies to: 283-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/__tests__/braveAdapter.test.ts` around lines 1 - 2, The test file currently statically imports braveAdapter (symbols extractBraveResults, decodeHtmlEntities) before registering Bun mocks, causing order-dependent failures; remove the top-level import and instead load the module under test via await import() inside a mock.module() block (or move helper-only tests into a separate file) so that axios/http mocks are registered first and the dynamic import of ../adapters/braveAdapter sees the mocks; ensure all references to extractBraveResults and decodeHtmlEntities are updated to use the dynamically imported module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tools/WebSearchTool/__tests__/braveAdapter.integration.ts`:
- Around line 79-82: The test file calls main() unguarded causing a live Brave
request on import; wrap the manual entrypoint so it only runs when executed
directly by enclosing the main().catch(...) call inside a runtime guard using
import.meta.main (e.g., if (import.meta.main) { main().catch(...) }) so
importing the module during tests won't trigger process.exit; alternatively
consider moving this smoke script out of the __tests__ directory if it’s not an
actual test.
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Line 115: The existing regex resultBlockRegex uses prefix-based class matching
which only finds elements where "snippet" or "result" is the first class token;
update this and the analogous regex at the other occurrence (e.g., the
description/snippet-description regex around line 147) to match class tokens
anywhere by checking the class attribute for whole-word tokens (use a
token-based match like looking for word boundaries or space-delimited tokens
inside the class="..."), so that class="fdb snippet" or class="foo description"
are correctly matched; modify the variables resultBlockRegex and the
description/snippet regex to use this token-aware matching.
- Around line 153-158: The fallback currently only tests the first paragraph
match by using fallbackRegex.exec(block) and returns undefined if that one is
too short; change it to iterate over all paragraph matches (e.g., use matchAll
or a loop with fallbackRegex.exec in a while) and for each match perform the
same stripping and decodeHtmlEntities logic and return the first snippet whose
text.length > 10; keep the existing variables/fns (fallbackRegex,
decodeHtmlEntities, block) and ensure you stop and return as soon as a valid
paragraph is found.
In `@src/tools/WebSearchTool/adapters/index.ts`:
- Around line 17-18: The code currently returns the previously stored
cachedAdapter (symbol: cachedAdapter) immediately, freezing the adapter
selection to the first call and ignoring later changes to process.env-driven
values like WEB_SEARCH_ADAPTER and isFirstPartyAnthropicBaseUrl(); remove or
change the early return so adapter selection logic (which checks
WEB_SEARCH_ADAPTER and calls isFirstPartyAnthropicBaseUrl()) runs on each
invocation, or implement a cache-key strategy (e.g., derive a key from
process.env.WEB_SEARCH_ADAPTER and the result of isFirstPartyAnthropicBaseUrl())
and recreate cachedAdapter when that key changes; update the function that
builds the adapter (look for the adapter creation block near cachedAdapter) to
honor this behavior.
---
Nitpick comments:
In `@src/tools/WebSearchTool/__tests__/braveAdapter.test.ts`:
- Around line 1-2: The test file currently statically imports braveAdapter
(symbols extractBraveResults, decodeHtmlEntities) before registering Bun mocks,
causing order-dependent failures; remove the top-level import and instead load
the module under test via await import() inside a mock.module() block (or move
helper-only tests into a separate file) so that axios/http mocks are registered
first and the dynamic import of ../adapters/braveAdapter sees the mocks; ensure
all references to extractBraveResults and decodeHtmlEntities are updated to use
the dynamically imported module.
🪄 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: b52c7d7e-0445-4514-b431-2292c875407e
📒 Files selected for processing (4)
src/tools/WebSearchTool/__tests__/braveAdapter.integration.tssrc/tools/WebSearchTool/__tests__/braveAdapter.test.tssrc/tools/WebSearchTool/adapters/braveAdapter.tssrc/tools/WebSearchTool/adapters/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/tools/WebSearchTool/adapters/braveAdapter.ts (1)
6-9: Use thesrc/alias for these imports.The new file adds more deep relative imports instead of the repo-standard
src/...paths.As per coding guidelines, "Use
src/path alias for imports; valid paths likeimport { ... } from 'src/utils/...'".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 6 - 9, Replace the deep relative imports with the repo-standard src/ alias: change the import of AbortError (currently from '../../../utils/errors.js') to import from 'src/utils/errors.js' and change the type imports (SearchResult, SearchOptions, WebSearchAdapter) from './types.js' to 'src/tools/WebSearchTool/adapters/types' (or the canonical src path for that module) so all imports use the src/ alias; keep axios and he as-is.src/tools/WebSearchTool/adapters/index.ts (1)
6-10: Use thesrc/alias for these new imports.These additions introduce more relative pathing in a file that already sits fairly deep in the tree. Please switch them to
src/...imports to stay aligned with the repo import convention.As per coding guidelines, "Use
src/path alias for imports; valid paths likeimport { ... } from 'src/utils/...'".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/adapters/index.ts` around lines 6 - 10, Replace the relative imports in this file with the repo's `src/` alias: update the imports that bring in isFirstPartyAnthropicBaseUrl, ApiSearchAdapter, BingSearchAdapter, BraveSearchAdapter and the WebSearchAdapter type so they use paths like 'src/utils/...' and 'src/tools/WebSearchTool/adapters/...' instead of '../../../utils/...'/ './apiAdapter.js' etc.; keep the same named symbols and file basenames (isFirstPartyAnthropicBaseUrl, ApiSearchAdapter, BingSearchAdapter, BraveSearchAdapter, WebSearchAdapter) so only the import module specifiers change to the `src/` alias.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/external-dependencies.md`:
- Line 22: The table cell containing the token `WEB_SEARCH_ADAPTER=bing|brave`
is being parsed as an extra column separator; update that cell to escape the
pipe so the row stays a single cell (e.g., change
`WEB_SEARCH_ADAPTER=bing|brave` to `WEB_SEARCH_ADAPTER=bing\|brave` or use
`WEB_SEARCH_ADAPTER=bing|brave`) in the markdown row that currently reads
`Web Search Pages | `www.bing.com`, `search.brave.com` | HTTPS | WebSearch
工具，可通过 `WEB_SEARCH_ADAPTER=bing|brave` 切换 |`.
In `@docs/features/web-search-tool.md`:
- Around line 44-46: The file index section (section six) is out of sync: add
the Brave adapter and its tests to the section that currently only lists Bing
artifacts. Update the section to include
src/tools/WebSearchTool/adapters/braveAdapter.ts and the corresponding test
entries (src/tools/WebSearchTool/__tests__/braveAdapter*.test.ts and
src/tools/WebSearchTool/__tests__/braveAdapter.integration.ts) so the index
matches the new table above and the Bing entries remain unchanged.
In `@src/tools/WebSearchTool/__tests__/adapterFactory.test.ts`:
- Around line 21-33: Reset the module-level cache after each test: in the
afterEach() teardown add logic to clear the adapter cache by setting
cachedAdapter and cachedAdapterKey back to their initial/undefined state so
createAdapter will re-evaluate on the next import; reference the module-level
symbols cachedAdapter and cachedAdapterKey (used by createAdapter in the
adapters index) and ensure they are reset alongside restoring
process.env.WEB_SEARCH_ADAPTER.
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 83-94: Normalize allowedDomains and blockedDomains entries before
comparison with hostname (which is already lowercase); for example, map both
arrays to trimmed, lowercased domain strings and strip any leading dots so the
checks in the conditions that use allowedDomains?.some(d => hostname === d ||
hostname.endsWith('.' + d)) and blockedDomains?.some(...) compare lowercased,
dot-normalized values. Update the logic in braveAdapter.ts to use these
normalized arrays (or normalize each domain inside the .some callback) so casing
differences like "Example.COM" no longer cause mismatches.
---
Nitpick comments:
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 6-9: Replace the deep relative imports with the repo-standard src/
alias: change the import of AbortError (currently from
'../../../utils/errors.js') to import from 'src/utils/errors.js' and change the
type imports (SearchResult, SearchOptions, WebSearchAdapter) from './types.js'
to 'src/tools/WebSearchTool/adapters/types' (or the canonical src path for that
module) so all imports use the src/ alias; keep axios and he as-is.
In `@src/tools/WebSearchTool/adapters/index.ts`:
- Around line 6-10: Replace the relative imports in this file with the repo's
`src/` alias: update the imports that bring in isFirstPartyAnthropicBaseUrl,
ApiSearchAdapter, BingSearchAdapter, BraveSearchAdapter and the WebSearchAdapter
type so they use paths like 'src/utils/...' and
'src/tools/WebSearchTool/adapters/...' instead of '../../../utils/...'/
'./apiAdapter.js' etc.; keep the same named symbols and file basenames
(isFirstPartyAnthropicBaseUrl, ApiSearchAdapter, BingSearchAdapter,
BraveSearchAdapter, WebSearchAdapter) so only the import module specifiers
change to the `src/` alias.
🪄 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: e4062261-397b-4caa-afed-0b9dd1a9b361
📒 Files selected for processing (9)
docs/external-dependencies.mddocs/features/web-search-tool.mddocs/tools/search-and-navigation.mdxsrc/tools/WebSearchTool/__tests__/adapterFactory.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.extract.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.integration.tssrc/tools/WebSearchTool/__tests__/braveAdapter.test.tssrc/tools/WebSearchTool/adapters/braveAdapter.tssrc/tools/WebSearchTool/adapters/index.ts
✅ Files skipped from review due to trivial changes (1)
- src/tools/WebSearchTool/tests/braveAdapter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tools/WebSearchTool/tests/braveAdapter.integration.ts
Comment on lines
+21
to
+33
| const { createAdapter } = await import('../adapters/index') | ||
|
|
||
| const originalWebSearchAdapter = process.env.WEB_SEARCH_ADAPTER | ||
|
|
||
| afterEach(() => { | ||
| isFirstPartyBaseUrl = true | ||
|
|
||
| if (originalWebSearchAdapter === undefined) { | ||
| delete process.env.WEB_SEARCH_ADAPTER | ||
| } else { | ||
| process.env.WEB_SEARCH_ADAPTER = originalWebSearchAdapter | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the test file structure
cat -n src/tools/WebSearchTool/__tests__/adapterFactory.test.tsRepository: claude-code-best/claude-code
Length of output: 2631
🏁 Script executed:
# Now examine the adapters/index.ts to see the caching mechanism
cat -n src/tools/WebSearchTool/adapters/index.tsRepository: claude-code-best/claude-code
Length of output: 1794
🏁 Script executed:
# Check what isFirstPartyBaseUrl is and where it's defined
rg -n "isFirstPartyBaseUrl" src/tools/WebSearchTool/ -A 2 -B 2Repository: claude-code-best/claude-code
Length of output: 2390
Reset the factory cache between test cases.
The module-level cachedAdapter and cachedAdapterKey state in src/tools/WebSearchTool/adapters/index.ts is never cleared between tests. While cache invalidation works via the adapterKey comparison, proper test isolation requires explicitly resetting this module state in the afterEach() block alongside the environment variable cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/WebSearchTool/__tests__/adapterFactory.test.ts` around lines 21 -
33, Reset the module-level cache after each test: in the afterEach() teardown
add logic to clear the adapter cache by setting cachedAdapter and
cachedAdapterKey back to their initial/undefined state so createAdapter will
re-evaluate on the next import; reference the module-level symbols cachedAdapter
and cachedAdapterKey (used by createAdapter in the adapters index) and ensure
they are reset alongside restoring process.env.WEB_SEARCH_ADAPTER.
Comment on lines
+83
to
+94
| if ( | ||
| allowedDomains?.length && | ||
| !allowedDomains.some( | ||
| d => hostname === d || hostname.endsWith('.' + d), | ||
| ) | ||
| ) { | ||
| return false | ||
| } | ||
| if ( | ||
| blockedDomains?.length && | ||
| blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d)) | ||
| ) { |
Contributor
There was a problem hiding this comment.
Normalize domain filters before comparing hostnames.
new URL(...).hostname is lowercase, but allowedDomains / blockedDomains are compared as-is. Inputs like Example.COM will fail to match, so allowed results can be dropped and blocked results can slip through.
Suggested fix
+ const normalizedAllowedDomains = allowedDomains?.map(domain =>
+ domain.toLowerCase(),
+ )
+ const normalizedBlockedDomains = blockedDomains?.map(domain =>
+ domain.toLowerCase(),
+ )
+
// Client-side domain filtering
const results = rawResults.filter(r => {
if (!r.url) return false
try {
const hostname = new URL(r.url).hostname
if (
- allowedDomains?.length &&
- !allowedDomains.some(
+ normalizedAllowedDomains?.length &&
+ !normalizedAllowedDomains.some(
d => hostname === d || hostname.endsWith('.' + d),
)
) {
return false
}
if (
- blockedDomains?.length &&
- blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d))
+ normalizedBlockedDomains?.length &&
+ normalizedBlockedDomains.some(
+ d => hostname === d || hostname.endsWith('.' + d),
+ )
) {
return false
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 83 - 94,
Normalize allowedDomains and blockedDomains entries before comparison with
hostname (which is already lowercase); for example, map both arrays to trimmed,
lowercased domain strings and strip any leading dots so the checks in the
conditions that use allowedDomains?.some(d => hostname === d ||
hostname.endsWith('.' + d)) and blockedDomains?.some(...) compare lowercased,
dot-normalized values. Update the logic in braveAdapter.ts to use these
normalized arrays (or normalize each domain inside the .some callback) so casing
differences like "Example.COM" no longer cause mismatches.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/tools/WebSearchTool/adapters/braveAdapter.ts (1)
80-90:⚠️ Potential issue | 🟡 MinorNormalize domain filters before hostname comparison.
Line 80/88 compare raw
allowedDomains/blockedDomainsvalues directly; case/format variants (e.g., uppercase or leading dot) can bypass intended matches.Suggested fix
+ const normalizedAllowedDomains = allowedDomains + ?.map(d => d.trim().toLowerCase().replace(/^\.+/, '')) + .filter(Boolean) + const normalizedBlockedDomains = blockedDomains + ?.map(d => d.trim().toLowerCase().replace(/^\.+/, '')) + .filter(Boolean) + const results = rawResults.filter(r => { try { - const hostname = new URL(r.url).hostname + const hostname = new URL(r.url).hostname.toLowerCase() if ( - allowedDomains?.length && - !allowedDomains.some( + normalizedAllowedDomains?.length && + !normalizedAllowedDomains.some( d => hostname === d || hostname.endsWith('.' + d), ) ) { return false } if ( - blockedDomains?.length && - blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d)) + normalizedBlockedDomains?.length && + normalizedBlockedDomains.some( + d => hostname === d || hostname.endsWith('.' + d), + ) ) { return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 80 - 90, Normalize domain filters and hostname before comparison: trim and lower-case hostname and each entry in allowedDomains/blockedDomains and strip any leading '.' from domain entries, then run the existing checks (the .some(...) and .endsWith('.' + d) comparisons) against these normalized values. Precompute normalizedAllowed and normalizedBlocked (mapping allowedDomains/blockedDomains to normalized form) and use a normalizedHostname variable in the existing conditional blocks that reference allowedDomains, blockedDomains, hostname, and the .some/endsWith logic so case or leading-dot variants do not bypass matching.
🧹 Nitpick comments (3)
src/tools/WebSearchTool/adapters/braveAdapter.ts (1)
7-8: Switch new production imports tosrc/alias paths.These new relative imports don’t follow the repository alias rule for TS modules.
As per coding guidelines
Use src/ path alias for imports; valid paths like import { ... } from 'src/utils/...'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 7 - 8, Update the import paths in braveAdapter.ts to use the repository TypeScript alias starting with "src/" instead of relative paths: replace '../../../utils/errors.js' and './types.js' with their corresponding 'src/...' alias imports so that AbortError, SearchResult, SearchOptions and WebSearchAdapter are imported from the aliased module paths; ensure file extensions and exported symbol names remain correct after switching to the src/ alias.src/tools/WebSearchTool/adapters/index.ts (1)
9-9: Usesrc/alias for the new adapter import.Line 9 adds a new relative import; please switch it to the project alias style for consistency with repo rules.
As per coding guidelines
Use src/ path alias for imports; valid paths like import { ... } from 'src/utils/...'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/adapters/index.ts` at line 9, Replace the relative import of BraveSearchAdapter with the project alias style; locate the import statement that currently reads "import { BraveSearchAdapter } from './braveAdapter.js'" and change it to use the src/ alias (e.g. import { BraveSearchAdapter } from 'src/tools/WebSearchTool/adapters/braveAdapter.js') so it follows the repository import convention.src/tools/WebSearchTool/__tests__/braveAdapter.integration.ts (1)
16-16: Usesrc/alias import in this test file too.Line 16 uses a relative import; align this test with the repo alias convention.
As per coding guidelines
Use src/ path alias for imports; valid paths like import { ... } from 'src/utils/...'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/__tests__/braveAdapter.integration.ts` at line 16, The test currently imports BraveSearchAdapter via a relative path; update the import to use the repo's src/ path alias (e.g., import { BraveSearchAdapter } from 'src/...') so the test follows the project's alias convention and aligns with other modules that import BraveSearchAdapter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 76-97: The filter in the braveAdapter.ts rawResults.filter
currently constructs new URL(r.url) but never rejects non-HTTP schemes (e.g.
javascript:, data:), so add a protocol check before domain logic: inside the
predicate for rawResults.filter (the closure that produces results) verify that
new URL(r.url).protocol is exactly 'http:' or 'https:' and return false for any
other protocol (while still catching URL construction errors), then proceed with
the existing allowedDomains/blockedDomains checks using hostname.
---
Duplicate comments:
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 80-90: Normalize domain filters and hostname before comparison:
trim and lower-case hostname and each entry in allowedDomains/blockedDomains and
strip any leading '.' from domain entries, then run the existing checks (the
.some(...) and .endsWith('.' + d) comparisons) against these normalized values.
Precompute normalizedAllowed and normalizedBlocked (mapping
allowedDomains/blockedDomains to normalized form) and use a normalizedHostname
variable in the existing conditional blocks that reference allowedDomains,
blockedDomains, hostname, and the .some/endsWith logic so case or leading-dot
variants do not bypass matching.
---
Nitpick comments:
In `@src/tools/WebSearchTool/__tests__/braveAdapter.integration.ts`:
- Line 16: The test currently imports BraveSearchAdapter via a relative path;
update the import to use the repo's src/ path alias (e.g., import {
BraveSearchAdapter } from 'src/...') so the test follows the project's alias
convention and aligns with other modules that import BraveSearchAdapter.
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 7-8: Update the import paths in braveAdapter.ts to use the
repository TypeScript alias starting with "src/" instead of relative paths:
replace '../../../utils/errors.js' and './types.js' with their corresponding
'src/...' alias imports so that AbortError, SearchResult, SearchOptions and
WebSearchAdapter are imported from the aliased module paths; ensure file
extensions and exported symbol names remain correct after switching to the src/
alias.
In `@src/tools/WebSearchTool/adapters/index.ts`:
- Line 9: Replace the relative import of BraveSearchAdapter with the project
alias style; locate the import statement that currently reads "import {
BraveSearchAdapter } from './braveAdapter.js'" and change it to use the src/
alias (e.g. import { BraveSearchAdapter } from
'src/tools/WebSearchTool/adapters/braveAdapter.js') so it follows the repository
import convention.
🪄 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: 30ea11d2-60d2-49c0-8fea-03588309555f
📒 Files selected for processing (9)
docs/external-dependencies.mddocs/features/web-search-tool.mddocs/tools/search-and-navigation.mdxsrc/tools/WebSearchTool/__tests__/adapterFactory.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.extract.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.integration.tssrc/tools/WebSearchTool/__tests__/braveAdapter.test.tssrc/tools/WebSearchTool/adapters/braveAdapter.tssrc/tools/WebSearchTool/adapters/index.ts
✅ Files skipped from review due to trivial changes (4)
- docs/external-dependencies.md
- docs/tools/search-and-navigation.mdx
- src/tools/WebSearchTool/tests/adapterFactory.test.ts
- docs/features/web-search-tool.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tools/WebSearchTool/tests/braveAdapter.extract.test.ts
- src/tools/WebSearchTool/tests/braveAdapter.test.ts
Comment on lines
+76
to
+97
| const results = rawResults.filter(r => { | ||
| try { | ||
| const hostname = new URL(r.url).hostname | ||
| if ( | ||
| allowedDomains?.length && | ||
| !allowedDomains.some( | ||
| d => hostname === d || hostname.endsWith('.' + d), | ||
| ) | ||
| ) { | ||
| return false | ||
| } | ||
| if ( | ||
| blockedDomains?.length && | ||
| blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d)) | ||
| ) { | ||
| return false | ||
| } | ||
| } catch { | ||
| return false | ||
| } | ||
| return true | ||
| }) |
Contributor
There was a problem hiding this comment.
Filter out non-HTTP(S) URLs before returning results.
At Line 78, new URL(r.url) accepts schemes like javascript:/data:. With no domain filters, those can pass through and be returned downstream.
Suggested fix
- const results = rawResults.filter(r => {
+ const results = rawResults.filter(r => {
try {
- const hostname = new URL(r.url).hostname
+ const parsedUrl = new URL(r.url)
+ if (
+ parsedUrl.protocol !== 'http:' &&
+ parsedUrl.protocol !== 'https:'
+ ) {
+ return false
+ }
+ const hostname = parsedUrl.hostname.toLowerCase()
if (
allowedDomains?.length &&
!allowedDomains.some(
d => hostname === d || hostname.endsWith('.' + d),
)📝 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.
Suggested change
| const results = rawResults.filter(r => { | |
| try { | |
| const hostname = new URL(r.url).hostname | |
| if ( | |
| allowedDomains?.length && | |
| !allowedDomains.some( | |
| d => hostname === d || hostname.endsWith('.' + d), | |
| ) | |
| ) { | |
| return false | |
| } | |
| if ( | |
| blockedDomains?.length && | |
| blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d)) | |
| ) { | |
| return false | |
| } | |
| } catch { | |
| return false | |
| } | |
| return true | |
| }) | |
| const results = rawResults.filter(r => { | |
| try { | |
| const parsedUrl = new URL(r.url) | |
| if ( | |
| parsedUrl.protocol !== 'http:' && | |
| parsedUrl.protocol !== 'https:' | |
| ) { | |
| return false | |
| } | |
| const hostname = parsedUrl.hostname.toLowerCase() | |
| if ( | |
| allowedDomains?.length && | |
| !allowedDomains.some( | |
| d => hostname === d || hostname.endsWith('.' + d), | |
| ) | |
| ) { | |
| return false | |
| } | |
| if ( | |
| blockedDomains?.length && | |
| blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d)) | |
| ) { | |
| return false | |
| } | |
| } catch { | |
| return false | |
| } | |
| return true | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 76 - 97, The
filter in the braveAdapter.ts rawResults.filter currently constructs new
URL(r.url) but never rejects non-HTTP schemes (e.g. javascript:, data:), so add
a protocol check before domain logic: inside the predicate for rawResults.filter
(the closure that produces results) verify that new URL(r.url).protocol is
exactly 'http:' or 'https:' and return false for any other protocol (while still
catching URL construction errors), then proceed with the existing
allowedDomains/blockedDomains checks using hostname.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/tools/WebSearchTool/adapters/braveAdapter.ts (1)
76-97:⚠️ Potential issue | 🟠 MajorHarden URL filtering: reject non-HTTP(S) schemes and normalize domain filters.
Current filtering can pass
javascript:/data:URLs when no domain filters are set, and domain comparisons are case/format sensitive.Suggested fix
- const results = rawResults.filter(r => { + const normalizedAllowedDomains = allowedDomains + ?.map(d => d.trim().replace(/^\./, '').toLowerCase()) + .filter(Boolean) + const normalizedBlockedDomains = blockedDomains + ?.map(d => d.trim().replace(/^\./, '').toLowerCase()) + .filter(Boolean) + + const results = rawResults.filter((r) => { try { - const hostname = new URL(r.url).hostname + const parsedUrl = new URL(r.url) + if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') { + return false + } + const hostname = parsedUrl.hostname.toLowerCase() if ( - allowedDomains?.length && - !allowedDomains.some( + normalizedAllowedDomains?.length && + !normalizedAllowedDomains.some( d => hostname === d || hostname.endsWith('.' + d), ) ) { return false } if ( - blockedDomains?.length && - blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d)) + normalizedBlockedDomains?.length && + normalizedBlockedDomains.some( + d => hostname === d || hostname.endsWith('.' + d), + ) ) { return false } } catch { return false } return true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 76 - 97, The URL filter in the rawResults.filter callback (used to build results) should reject non-HTTP(S) schemes and normalize domain filters before comparison: parse r.url and ensure url.protocol is "http:" or "https:" (otherwise return false), derive hostname via new URL(...).hostname, and compare hostnames against allowedDomains/blockedDomains after normalizing to lower-case and stripping any leading/trailing dots from domain entries (and trimming whitespace) so comparisons use hostname === domain or hostname.endsWith('.' + domain) in a case-insensitive way; keep the existing try/catch but add these checks and normalization for both allowedDomains and blockedDomains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tools/WebSearchTool/__tests__/braveAdapter.extract.test.ts`:
- Line 2: Replace the relative import of extractBraveResults from
'../adapters/braveAdapter' with the project's src/ path alias import (i.e.,
import extractBraveResults via the src/ alias) so tests use the same module
resolution as the app; update the import statement referencing braveAdapter and
the extractBraveResults symbol accordingly.
In `@src/tools/WebSearchTool/__tests__/braveAdapter.test.ts`:
- Line 8: The test currently uses a relative dynamic import for
BraveSearchAdapter (const { BraveSearchAdapter } = await
import('../adapters/braveAdapter')); replace that with the repository path alias
style so the import becomes a dynamic import from the src alias (e.g., await
import('src/...') using the correct module path to the braveAdapter module) and
update the other occurrence the same way; ensure the symbol BraveSearchAdapter
is still destructured from the imported module.
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 7-8: Replace the relative imports in braveAdapter.ts with the
project's src/ path alias: change the import of AbortError from
'../../../utils/errors.js' to import from 'src/utils/errors' and change the
types import (SearchResult, SearchOptions, WebSearchAdapter) from './types.js'
to 'src/tools/WebSearchTool/adapters/types' (or the correct src path to that
types module); update all referenced import specifiers so AbortError,
SearchResult, SearchOptions, and WebSearchAdapter are imported via the src/
alias to match repository conventions.
---
Duplicate comments:
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 76-97: The URL filter in the rawResults.filter callback (used to
build results) should reject non-HTTP(S) schemes and normalize domain filters
before comparison: parse r.url and ensure url.protocol is "http:" or "https:"
(otherwise return false), derive hostname via new URL(...).hostname, and compare
hostnames against allowedDomains/blockedDomains after normalizing to lower-case
and stripping any leading/trailing dots from domain entries (and trimming
whitespace) so comparisons use hostname === domain or hostname.endsWith('.' +
domain) in a case-insensitive way; keep the existing try/catch but add these
checks and normalization for both allowedDomains and blockedDomains.
🪄 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: ab988d1d-3280-4240-9da3-34dc5ef0d0a8
📒 Files selected for processing (9)
docs/external-dependencies.mddocs/features/web-search-tool.mddocs/tools/search-and-navigation.mdxsrc/tools/WebSearchTool/__tests__/adapterFactory.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.extract.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.integration.tssrc/tools/WebSearchTool/__tests__/braveAdapter.test.tssrc/tools/WebSearchTool/adapters/braveAdapter.tssrc/tools/WebSearchTool/adapters/index.ts
✅ Files skipped from review due to trivial changes (2)
- src/tools/WebSearchTool/tests/adapterFactory.test.ts
- src/tools/WebSearchTool/tests/braveAdapter.integration.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/tools/search-and-navigation.mdx
- docs/external-dependencies.md
- src/tools/WebSearchTool/adapters/index.ts
- docs/features/web-search-tool.md
| @@ -0,0 +1,106 @@ | |||
| import { describe, expect, test } from 'bun:test' | |||
| import { extractBraveResults } from '../adapters/braveAdapter' | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use src/ alias import in tests as well.
Switch this relative import to the configured path alias for consistency with project standards.
As per coding guidelines, "Use src/path alias for imports; valid paths likeimport { ... } from 'src/utils/...'``."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/WebSearchTool/__tests__/braveAdapter.extract.test.ts` at line 2,
Replace the relative import of extractBraveResults from
'../adapters/braveAdapter' with the project's src/ path alias import (i.e.,
import extractBraveResults via the src/ alias) so tests use the same module
resolution as the app; update the import statement referencing braveAdapter and
the extractBraveResults symbol accordingly.
|
|
||
| describe('BraveSearchAdapter.search', () => { | ||
| const createAdapter = async () => { | ||
| const { BraveSearchAdapter } = await import('../adapters/braveAdapter') |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace relative dynamic imports with src/ alias imports.
Use the repository alias for these imports to keep test modules consistent with TS path conventions.
As per coding guidelines, "Use src/path alias for imports; valid paths likeimport { ... } from 'src/utils/...'``."
Also applies to: 193-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/WebSearchTool/__tests__/braveAdapter.test.ts` at line 8, The test
currently uses a relative dynamic import for BraveSearchAdapter (const {
BraveSearchAdapter } = await import('../adapters/braveAdapter')); replace that
with the repository path alias style so the import becomes a dynamic import from
the src alias (e.g., await import('src/...') using the correct module path to
the braveAdapter module) and update the other occurrence the same way; ensure
the symbol BraveSearchAdapter is still destructured from the imported module.
Comment on lines
+7
to
+8
| import { AbortError } from '../../../utils/errors.js' | ||
| import type { SearchResult, SearchOptions, WebSearchAdapter } from './types.js' |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use src/ path alias imports instead of relative paths.
Please switch these imports to the configured src/... alias to match repository conventions.
As per coding guidelines, "Use src/path alias for imports; valid paths likeimport { ... } from 'src/utils/...'``."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 7 - 8, Replace
the relative imports in braveAdapter.ts with the project's src/ path alias:
change the import of AbortError from '../../../utils/errors.js' to import from
'src/utils/errors' and change the types import (SearchResult, SearchOptions,
WebSearchAdapter) from './types.js' to 'src/tools/WebSearchTool/adapters/types'
(or the correct src path to that types module); update all referenced import
specifiers so AbortError, SearchResult, SearchOptions, and WebSearchAdapter are
imported via the src/ alias to match repository conventions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/tools/WebSearchTool/adapters/braveAdapter.ts (1)
76-90:⚠️ Potential issue | 🟠 MajorHarden result URL filtering (protocol + domain normalization).
At Line 78, non-HTTP(S) URLs can pass through when no allow/block list is provided. Also, Lines 80-90 compare user-provided domains without normalization, so mixed-case values can mismatch.
Suggested change
+ const normalizedAllowedDomains = allowedDomains + ?.map(d => d.trim().replace(/^\.+/, '').toLowerCase()) + .filter(Boolean) + const normalizedBlockedDomains = blockedDomains + ?.map(d => d.trim().replace(/^\.+/, '').toLowerCase()) + .filter(Boolean) + const rawResults = extractBraveResults(payload) - const results = rawResults.filter(r => { + const results = rawResults.filter(r => { try { - const hostname = new URL(r.url).hostname + const parsedUrl = new URL(r.url) + if ( + parsedUrl.protocol !== 'http:' && + parsedUrl.protocol !== 'https:' + ) { + return false + } + const hostname = parsedUrl.hostname.toLowerCase() if ( - allowedDomains?.length && - !allowedDomains.some( + normalizedAllowedDomains?.length && + !normalizedAllowedDomains.some( d => hostname === d || hostname.endsWith('.' + d), ) ) { return false } if ( - blockedDomains?.length && - blockedDomains.some(d => hostname === d || hostname.endsWith('.' + d)) + normalizedBlockedDomains?.length && + normalizedBlockedDomains.some( + d => hostname === d || hostname.endsWith('.' + d), + ) ) { return false } } catch { return false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/WebSearchTool/adapters/braveAdapter.ts` around lines 76 - 90, The URL filtering in the rawResults.filter callback allows non-HTTP(S) schemes and does case-sensitive domain comparisons; update the filter to first ensure the parsed URL.protocol is 'http:' or 'https:' and skip other schemes, then normalize hostname and user-provided allowedDomains/blockedDomains to a canonical lowercase form (and optionally trim trailing dots) before comparing (use hostname === domain or hostname.endsWith('.' + domain)); keep the existing try/catch around new URL(r.url) and apply these checks where hostname is used so mixed-case or non-HTTP URLs are correctly excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/web-search-tool.md`:
- Line 8: Update the docs text for WebSearchTool to correctly describe Brave:
change the phrase calling Brave an “HTML parsing backend” to state that Brave
(handled by BraveSearchAdapter) uses Brave’s LLM Context API and returns JSON
grounding data that is mapped into the tool, whereas Bing uses the HTML-parsing
backend; reference WebSearchTool, BraveSearchAdapter and the Bing HTML parsing
path to make the distinction clear.
---
Duplicate comments:
In `@src/tools/WebSearchTool/adapters/braveAdapter.ts`:
- Around line 76-90: The URL filtering in the rawResults.filter callback allows
non-HTTP(S) schemes and does case-sensitive domain comparisons; update the
filter to first ensure the parsed URL.protocol is 'http:' or 'https:' and skip
other schemes, then normalize hostname and user-provided
allowedDomains/blockedDomains to a canonical lowercase form (and optionally trim
trailing dots) before comparing (use hostname === domain or
hostname.endsWith('.' + domain)); keep the existing try/catch around new
URL(r.url) and apply these checks where hostname is used so mixed-case or
non-HTTP URLs are correctly excluded.
🪄 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: 9282198b-436a-4055-bbb3-b6d3b3dd2412
📒 Files selected for processing (9)
docs/external-dependencies.mddocs/features/web-search-tool.mddocs/tools/search-and-navigation.mdxsrc/tools/WebSearchTool/__tests__/adapterFactory.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.extract.test.tssrc/tools/WebSearchTool/__tests__/braveAdapter.integration.tssrc/tools/WebSearchTool/__tests__/braveAdapter.test.tssrc/tools/WebSearchTool/adapters/braveAdapter.tssrc/tools/WebSearchTool/adapters/index.ts
✅ Files skipped from review due to trivial changes (3)
- docs/external-dependencies.md
- src/tools/WebSearchTool/tests/braveAdapter.extract.test.ts
- src/tools/WebSearchTool/tests/adapterFactory.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tools/WebSearchTool/adapters/index.ts
- src/tools/WebSearchTool/tests/braveAdapter.test.ts
| ## 一、功能概述 | ||
|
|
||
| WebSearchTool 让模型可以搜索互联网获取最新信息。原始实现仅支持 Anthropic API 服务端搜索（`web_search_20250305` server tool），在第三方代理端点下不可用。现已重构为适配器架构，新增 Bing 搜索页面解析作为 fallback，确保任何 API 端点都能使用搜索功能。 | ||
| WebSearchTool 让模型可以搜索互联网获取最新信息。原始实现仅支持 Anthropic API 服务端搜索（`web_search_20250305` server tool），在第三方代理端点下不可用。现已重构为适配器架构，支持 API 服务端搜索，以及 Bing / Brave 两个 HTML 解析后端，确保任何 API 端点都能使用搜索功能。 |
Contributor
There was a problem hiding this comment.
Correct backend description for Brave.
Line 8 describes Brave as an “HTML parsing backend,” but BraveSearchAdapter calls Brave’s LLM Context API and maps JSON grounding data. Please adjust this wording to avoid conflating it with Bing’s HTML parsing path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/features/web-search-tool.md` at line 8, Update the docs text for
WebSearchTool to correctly describe Brave: change the phrase calling Brave an
“HTML parsing backend” to state that Brave (handled by BraveSearchAdapter) uses
Brave’s LLM Context API and returns JSON grounding data that is mapped into the
tool, whereas Bing uses the HTML-parsing backend; reference WebSearchTool,
BraveSearchAdapter and the Bing HTML parsing path to make the distinction clear.
Owner
|
稍后我验证一下哈 |
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 by CodeRabbit
New Features
Tests
Documentation