Repository navigation
feat: retire OpenResearch, fold 17-source search into flare_search - #326
Conversation
Add 13 new type arms to flare_search: - 6 rivalsearch-backed: social, news, github, academic, datasets, websites - 7 inline API: weather, financial, crypto, fx, indicators, youtube, bluesky - DRY gateway delegation via gateway_execute helper Agentflare-Agent: 1 Agentflare-Branch: feat/retire-openresearch-into-global-search
|
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:
📝 WalkthroughWalkthroughThe MCP search tool now advertises and dispatches 17 source types. Gateway execution is centralized, while new RivalSearch and HTTP-backed implementations provide social, news, repository, academic, dataset, weather, market, video, and Bluesky searches. ChangesExpanded MCP Search
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant search_impl
participant gateway_execute
participant RivalSearch
MCPClient->>search_impl: submit SearchRequest
search_impl->>gateway_execute: execute named search tool
gateway_execute->>RivalSearch: invoke gateway tool
RivalSearch-->>gateway_execute: return results or error
gateway_execute-->>search_impl: return JSON response
search_impl-->>MCPClient: return search response
sequenceDiagram
participant MCPClient
participant search_impl
participant ureq
participant ExternalAPIs
MCPClient->>search_impl: submit HTTP-backed SearchRequest
search_impl->>ureq: build source-specific request
ureq->>ExternalAPIs: fetch source data
ExternalAPIs-->>ureq: return response or failure
ureq-->>search_impl: return parsed result
search_impl-->>MCPClient: return JSON result or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
… search arms
- urlencoding() used Rust's debug-escape (\xHH) instead of percent-encoding
(%HH) for any non-alphanumeric byte, breaking weather/indicators queries
with accented or special characters.
- search_financial called a nonexistent SEC endpoint
(data.sec.gov/submissions/CIKLookup/{ticker}.json). The real openresearch-mcp
source (tools/sec.py) resolves ticker->CIK via the static
www.sec.gov/files/company_tickers.json file first, then queries
data.sec.gov/submissions/CIK{cik:010}.json -- restored that two-step flow.
- search_crypto passed raw ticker symbols into CoinGecko's `ids` param, which
only accepts id slugs ("bitcoin", not "btc"). Restored the symbol->id
table openresearch-mcp's tools/crypto.py carries for exactly this reason.
ReviewFound and fixed 3 correctness bugs introduced while porting from
All three were verified against the real The 6 Build clean ( |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/mcp_server/search.rs (1)
235-332: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSix near-identical arms — consider one shared helper.
Each arm differs only in source label, tool name, default/clamp, and arg keys. A small helper would collapse ~90 lines and keep the success/error JSON shape consistent as arms are added.
♻️ Sketch
async fn rivalsearch_arm( &self, source: &str, tool: &str, q: &str, args: serde_json::Value, ) -> String { match self.gateway_execute("rivalsearch", tool, args).await { Ok(val) => serde_json::json!({"source": source, "query": q, "results": val}).to_string(), Err(e) => serde_json::json!({ "source": source, "query": q, "error": format!("rivalsearch {tool} failed: {e}"), "results": [] }) .to_string(), } }🤖 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 `@src/mcp_server/search.rs` around lines 235 - 332, Add a shared async helper near the search methods, such as rivalsearch_arm, to perform gateway_execute and build the common success/error JSON response. Refactor search_social, search_news, search_github, search_academic, search_datasets, and search_websites to retain their existing validation, limits, argument construction, source labels, and tool names while delegating response handling to the helper.
🤖 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 `@src/mcp_server/search.rs`:
- Around line 418-424: Encode all user-controlled URL query parameters in the
search handlers: in src/mcp_server/search.rs lines 418-424, wrap coin with the
existing urlencoding helper; in lines 439-451, encode base and symbols or
validate each as exactly three ASCII letters; in lines 485-487, encode video_id
and reject unrecognized IDs; and in lines 503-514, encode handle in both
getProfile and getAuthorFeed URLs.
- Around line 384-397: The financial query path currently downloads and parses
the SEC ticker file for every request. Update the ticker lookup around the
`ureq::get` call and `cik` derivation to reuse a process-lifetime or TTL-based
cached ticker-to-CIK map, fetching and parsing `company_tickers.json` only when
the cache is absent or expired while preserving the existing error response.
- Around line 34-48: Update gateway_execute to pass execution errors through
gateway_registry::redact_error_for_llm before converting them to the returned
String, matching the existing tool execution path and ensuring all callers
receive redacted errors.
- Around line 579-596: Update the YouTube extraction logic around the URL
parsing branch to extract the 11-character ID from the path segment following
/shorts/ while preserving query-parameter extraction for watch URLs. Tighten the
bare-ID validation to ASCII alphanumeric characters plus '-' and '_' so
non-ASCII characters are rejected.
- Around line 342-362: Update the weather query parsing around `parts`,
`location`, and `days` so the trailing segment is removed only when it parses as
a valid day count; preserve the full query such as “Paris, France” for geocoding
otherwise. Also change the `lon` extraction to return the same
location-not-found error response as the `lat` extraction when longitude is
missing or invalid, instead of defaulting to 0.0.
- Around line 336-373: Move all synchronous HTTP calls in search_weather,
search_financial, search_crypto, search_fx, search_indicators, search_bluesky,
and search_youtube off the Tokio runtime, including the direct search_youtube
invocation from search_impl, by using spawn_blocking or an equivalent
blocking-task boundary. Introduce and reuse a shared ureq::Agent configured with
connect and overall timeouts for every upstream request, preserving each
method’s existing response and error behavior.
In `@src/mcp_server/types.rs`:
- Line 962: Update the schemars description for the limit field near the
relevant type definition to accurately document the implemented per-tool
defaults, including news/github/datasets at 10 and websites at 5, and state that
limit is ignored for weather, financial, crypto, fx, indicators, youtube, and
bluesky. Remove the inaccurate generic default claims without changing runtime
behavior.
---
Nitpick comments:
In `@src/mcp_server/search.rs`:
- Around line 235-332: Add a shared async helper near the search methods, such
as rivalsearch_arm, to perform gateway_execute and build the common
success/error JSON response. Refactor search_social, search_news, search_github,
search_academic, search_datasets, and search_websites to retain their existing
validation, limits, argument construction, source labels, and tool names while
delegating response handling to the helper.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 13a49215-dad8-46cf-a819-00ceded30b54
📒 Files selected for processing (3)
src/mcp_server.rssrc/mcp_server/search.rssrc/mcp_server/types.rs
…rch arms, add helper tests - Wrap every ureq call in the 7 inline-API arms (weather/financial/crypto/fx/ indicators/youtube/bluesky) in spawn_blocking, backed by a shared ureq::Agent with explicit connect/read timeouts, so a stalled upstream parks a blocking-pool thread instead of a Tokio runtime worker. - Collapse the six rivalsearch-backed arms (social/news/github/academic/ datasets/websites) into a shared rivalsearch_arm helper per CodeRabbit's review nitpick. - Add unit tests for the urlencoding and extract_youtube_id pure helpers.
Replaces the OpenResearch MCP server with 13 new
flare_searchtype arms:RivalSearch-backed (gateway delegation):
social— 9 platforms via rivalsearch social_searchnews— 5 sources via rivalsearch news_aggregationgithub— repo search via rivalsearch github_searchacademic— 5 dbs via rivalsearch scientific_researchdatasets— 4 hubs via rivalsearch scientific_researchwebsites— crawl via rivalsearch map_websiteInline API (same free APIs OpenResearch used):
weather— Open-Meteo (geocoding + forecast)financial— SEC EDGAR company lookupcrypto— CoinGecko pricefx— Frankfurter/ECB exchange ratesindicators— World Bank indicator searchyoutube— YouTube transcript APIbluesky— AT Protocol profile + feedAlso DRY'd gateway delegation into a shared
gateway_executehelper.Summary by CodeRabbit
New Features
Bug Fixes
Tests