Skip to content

✨ feat(web-search): add web_search built-in tool#117

Open
zadawq wants to merge 10 commits into
awakenworks:mainfrom
zadawq:web_search
Open

✨ feat(web-search): add web_search built-in tool#117
zadawq wants to merge 10 commits into
awakenworks:mainfrom
zadawq:web_search

Conversation

@zadawq
Copy link
Copy Markdown
Contributor

@zadawq zadawq commented Apr 17, 2026

Summary

Moves the existing web_search built-in tool from awaken-runtime to a standalone
awaken-ext-web-search crate, following the project extension convention awaken-ext-*.
This establishes clear dependency boundaries and enables multiple search backend support.

Changes

Implemented all PR review suggestions:

  1. Abstraction Layer - Added SearchProvider trait to support multiple backends
    pub trait SearchProvider: Send + Sync {
        async fn search(&self, query: &str, num_results: usize)
            -> Result<Vec<SearchResult>, ToolError>;
    }
  2. Modular Architecture
    - src/tool.rs - Core SearchProvider trait, SearchResult struct, and WebSearchTool
    - src/providers/serpapi.rs - Concrete SerpAPI implementation
    - Proper feature gating: serpapi feature gates the default backend
  3. Environment Variable Support - Fallback to SERPAPI_KEY environment variable
    // Reads from environment when api_key is empty
    SerpApiProvider::new("", None) // <- gets key from SERPAPI_KEY
  4. Retry Mechanism - Automatic retry on timeout (max 2 retries)
  5. Backward Compatibility - Original module in awaken-runtime kept as compatibility wrapper,
    existing code requires no changes

Checklist

  • Correct feature isolation, --no-default-features build passes
  • All unit tests pass
  • Clippy check clean (no warnings)
  • Full workspace compilation succeeds
  • Follows project conventions: awaken-ext-* naming, thiserror for errors, prelude re-exports

Architecture Notes

The implementation is ready for adding new search backends in the future (Google Custom
Search, Bing, etc.). To add a new backend:

  1. Add a new file under src/providers/
  2. Implement the SearchProvider trait
  3. Gate it with a feature flag

zadawq added 3 commits April 17, 2026 12:46
- Migrate web search tool to independent awaken-ext-web-search crate
- Introduce SearchProvider trait abstraction for multiple backends
- Add SerpApiProvider implementation with env var fallback (SERPAPI_KEY)
- Add timeout retry logic (max 2 retries)
- Maintain backward compatibility in awaken-runtime
- Proper feature gating: web-search in awaken-runtime, serpapi in extension
- Complete test coverage for core functionality and cancellation
@chaizhenhua
Copy link
Copy Markdown
Contributor

Good direction overall: extracting web_search into a separate extension crate, introducing SearchProvider, and keeping runtime compatibility wrappers all make sense.

A few issues should be fixed before merging:

ShutdownConfig was accidentally removed from awaken::prelude, which is an unrelated breaking change. Please restore the re-export.
SerpApiProvider appears to use Authorization: Bearer , while SerpAPI normally expects api_key as a query parameter. Please update this or add tests/docs proving the current approach works.
num_results should have schema and runtime validation instead of silently falling back to 5 for invalid values.
The docs show SerpApiProvider, but it requires the serpapi feature. Please either enable it by default or document the required feature flag.

Once these are addressed, the overall design looks good.

zadawq added 2 commits April 20, 2026 10:09
- Migrate web search tool to independent awaken-ext-web-search crate
- Introduce SearchProvider trait abstraction for multiple backends
- Add SerpApiProvider implementation with env var fallback (SERPAPI_KEY)
- Add timeout retry logic (max 2 retries)
- Maintain backward compatibility in awaken-runtime
- Proper feature gating: web-search in awaken-runtime, serpapi in extension
- Complete test coverage for core functionality and cancellation
@zadawq zadawq force-pushed the web_search branch 2 times, most recently from 311e115 to fba1376 Compare April 20, 2026 02:39
@zadawq
Copy link
Copy Markdown
Contributor Author

zadawq commented Apr 20, 2026

All four issues have been addressed in the latest commit:

✅ 1. ShutdownConfig restored

  • Re-added ShutdownConfig to awaken::prelude exports (crates/awaken/src/prelude.rs:97)

✅ 2. SerpAPI authentication corrected

  • Changed from Authorization: Bearer header to api_key query parameter
    (crates/awaken-ext-web-search/src/providers/serpapi.rs:88)
  • This matches SerpAPI's standard authentication method
  • Updated tests to verify query parameter authentication

✅ 3. num_results validation added

  • Schema validation: Added minimum: 1, maximum: 20 constraints in JSON schema (tool.rs:84-87)
  • Runtime validation: Explicit bounds checking that returns ToolError::InvalidArguments for
    out-of-range values (tool.rs:100-112)
  • No more silent fallbacks

✅ 4. serpapi feature documented

  • Added doc comment on WebSearchTool::new() stating it requires the serpapi feature
    (tool.rs:59)
  • The web-search feature in awaken-runtime automatically enables
    awaken-ext-web-search/serpapi (Cargo.toml:35)

@chaizhenhua
Copy link
Copy Markdown
Contributor

Thanks for the PR — the overall direction looks good. Splitting web search into awaken-ext-web-search and introducing a SearchProvider abstraction is a solid design.

Before merging, I’d request changes on a few points:

  1. API key leakage risk
    SerpApiProvider currently sends the API key as a URL query parameter. Since reqwest::Error may include the full URL, errors propagated through ToolError::ExecutionFailed(e.to_string()) could leak the key into logs or telemetry. Please either move auth out of the URL if supported, or make sure all reqwest errors are sanitized/redacted, for example via without_url(). A regression test for HTTP error paths would be useful.

  2. Feature/docs mismatch
    The docs show SerpApiProvider as the default quick-start provider, but it is gated behind the serpapi feature while default = []. Either enable serpapi by default or update the docs to explicitly require features = ["serpapi"].

  3. Multiple backend support
    The current SearchProvider trait supports a single pluggable backend, which is a good start. Could we also make the design support multiple backends, either as:

    • fallback/chain mode: try backend A, then B if A fails or returns insufficient results;
    • fanout/merge mode: query several providers and merge results.

    For merged results, we should define clear behavior for deduplication, ranking, provider attribution, timeout handling, and partial failures. This could be implemented as a CompositeSearchProvider without changing the individual provider trait too much.

Other follow-ups worth considering:

  • validate missing/empty API keys locally instead of sending an invalid external request;
  • apply cancellation to the full request lifecycle, including body reading and JSON parsing;
  • add limited retry/backoff for 429/5xx/timeouts;
  • make provider-specific dependencies such as reqwest optional behind the relevant feature.

@zadawq zadawq closed this Apr 23, 2026
@zadawq zadawq reopened this Apr 23, 2026
@zadawq
Copy link
Copy Markdown
Contributor Author

zadawq commented Apr 23, 2026

Thanks for the detailed review! I've addressed all the points you raised

@chaizhenhua
Copy link
Copy Markdown
Contributor

Nice refactor overall — extracting web_search into awaken-ext-web-search and introducing SearchProvider looks like the right direction. 👍

A few issues to fix before merge:

search_merge can exceed num_results
It only breaks the inner loop. Should stop the outer loop or return early once the limit is reached.
search_fallback misclassifies empty results
If all providers return Ok([]), it currently returns "no providers configured". This should instead return Ok([]) and only error if providers.is_empty().
search_merge hides failure states
It always returns Ok(merged), so callers can’t distinguish “no results” vs “all providers failed” or “no providers”. These cases should return an error.
Tests are too thin
Please add coverage for merge limits, fallback empty results, and no-provider / all-failed cases.
Minor: provider attribution may be lost
Consider backfilling SearchResult.provider if not set by the provider.

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