mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 04:14:56 +00:00
feat(web_search): add provider arg to force a specific engine (#1108)
Currently the web_search tool resolves a tenant-configured provider
chain (e.g. tavily → exa → brave) and stops at the first success.
That's the right default for cost/latency, but it makes cross-engine
corroboration impossible at the agent level: the caller has no way to
ask "search the same query on Exa specifically" once Tavily has
already returned a hit.
Add an optional `provider` argument. When set, the chain is narrowed
to the named provider (case-insensitive); other providers are not
queried. The first-success-wins fallback is preserved when the arg
is omitted.
Use case (concrete): a research agent verifying source freshness
calls web_search twice for the same query — once with provider="tavily"
and once with provider="exa" — and compares the URLs returned. URLs
that appear in both engines are high-confidence original primary
sources; URLs only in one signal a republish or low-circulation outlet
worth verifying further.
Other changes:
- Cache key now includes the requested provider so per-engine
results don't collide. Without this, the second call would just
replay the first engine's cached result, defeating the purpose.
- Unknown provider returns a clear error listing what IS configured
for the tenant — better DX than a silent no-op.
Tests: 5 new cases covering narrowing, case-insensitivity, unknown
provider, default first-success behaviour preservation, and cache
isolation across providers.
This commit is contained in:
1 parent
9e97031099
commit
cfb7f46324
2 files changed
+180
-4
No files matched your search
@@ -163,6 +163,10 @@ func (t *WebSearchTool) Parameters() map[string]any {
|
||||
"type": "string",
|
||||
"description": "Filter results by discovery time. Supports 'pd' (past day), 'pw' (past week), 'pm' (past month), 'py' (past year), and date range 'YYYY-MM-DDtoYYYY-MM-DD'.",
|
||||
},
|
||||
"provider": map[string]any{
|
||||
"type": "string",
|
||||
"description": "Optional: force a specific provider (e.g., 'tavily', 'exa', 'brave', 'duckduckgo'). When omitted, the tenant's configured provider chain is used (first-success-wins). Use this to force cross-engine corroboration — call once with each provider and compare results.",
|
||||
},
|
||||
},
|
||||
"required": []string{"query"},
|
||||
}
|
||||
@@ -183,6 +187,7 @@ func (t *WebSearchTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
searchLang, _ := args["search_lang"].(string)
|
||||
uiLang, _ := args["ui_lang"].(string)
|
||||
freshness, _ := args["freshness"].(string)
|
||||
requestedProvider, _ := args["provider"].(string)
|
||||
|
||||
params := searchParams{
|
||||
Query: query,
|
||||
@@ -193,18 +198,40 @@ func (t *WebSearchTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
Freshness: freshness,
|
||||
}
|
||||
|
||||
// Check cache (scoped per channel to prevent cross-channel cache poisoning)
|
||||
// Check cache (scoped per channel + provider to prevent cross-engine cache mixing)
|
||||
channel := ToolChannelFromCtx(ctx)
|
||||
cacheKey := fmt.Sprintf("%s:%s", channel, buildSearchCacheKey(params))
|
||||
cacheKey := fmt.Sprintf("%s:%s:%s", channel, requestedProvider, buildSearchCacheKey(params))
|
||||
if cached, ok := t.cache.get(cacheKey); ok {
|
||||
slog.Debug("web_search cache hit", "query", query)
|
||||
slog.Debug("web_search cache hit", "query", query, "provider", requestedProvider)
|
||||
return NewResult(cached)
|
||||
}
|
||||
|
||||
// Resolve per-request provider chain from tenant config_secrets + settings overlay.
|
||||
chain := t.resolveChain(ctx)
|
||||
|
||||
// Try providers in order (first success wins)
|
||||
// If caller explicitly named a provider, narrow the chain to just that one.
|
||||
// This unlocks cross-engine corroboration (caller invokes once per engine
|
||||
// and compares results) — without a provider param, the first-success-wins
|
||||
// chain hides everything after the first hit.
|
||||
if requestedProvider != "" {
|
||||
filtered := make([]SearchProvider, 0, 1)
|
||||
for _, p := range chain {
|
||||
if strings.EqualFold(p.Name(), requestedProvider) {
|
||||
filtered = append(filtered, p)
|
||||
break
|
||||
}
|
||||
}
|
||||
if len(filtered) == 0 {
|
||||
available := make([]string, 0, len(chain))
|
||||
for _, p := range chain {
|
||||
available = append(available, p.Name())
|
||||
}
|
||||
return ErrorResult(fmt.Sprintf("provider %q not configured for this tenant; available: %v", requestedProvider, available))
|
||||
}
|
||||
chain = filtered
|
||||
}
|
||||
|
||||
// Try providers in order (first success wins, unless narrowed above)
|
||||
var lastErr error
|
||||
for _, provider := range chain {
|
||||
results, err := provider.Search(ctx, params)
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
)
|
||||
|
||||
// searchStubProvider returns a fixed result so we can assert which provider ran.
|
||||
type searchStubProvider struct {
|
||||
name string
|
||||
called *bool
|
||||
result searchResult
|
||||
failure error
|
||||
}
|
||||
|
||||
func (s *searchStubProvider) Name() string { return s.name }
|
||||
func (s *searchStubProvider) Search(_ context.Context, _ searchParams) ([]searchResult, error) {
|
||||
if s.called != nil {
|
||||
*s.called = true
|
||||
}
|
||||
if s.failure != nil {
|
||||
return nil, s.failure
|
||||
}
|
||||
return []searchResult{s.result}, nil
|
||||
}
|
||||
|
||||
// makeStubbedTool installs a fixed chain of searchStubProviders for the master tenant
|
||||
// so Execute() bypasses the real provider chain resolution.
|
||||
func makeStubbedTool(t *testing.T, providers ...*searchStubProvider) (*WebSearchTool, context.Context) {
|
||||
t.Helper()
|
||||
tool := &WebSearchTool{
|
||||
secrets: newFakeSecretsStore(),
|
||||
cache: newWebCache(defaultCacheMaxEntries, defaultCacheTTL),
|
||||
chainCache: newTenantChainCache(),
|
||||
}
|
||||
tid := uuid.New()
|
||||
ctx := store.WithTenantID(context.Background(), tid)
|
||||
chain := make([]SearchProvider, len(providers))
|
||||
for i, p := range providers {
|
||||
chain[i] = p
|
||||
}
|
||||
tool.chainCache.Set(tid, chain)
|
||||
return tool, ctx
|
||||
}
|
||||
|
||||
// TestExecute_ProviderParam_NarrowsChainToOne — when caller passes provider="exa",
|
||||
// the brave provider must NOT be called even though it sits earlier in the chain.
|
||||
func TestExecute_ProviderParam_NarrowsChainToOne(t *testing.T) {
|
||||
braveCalled, exaCalled := false, false
|
||||
tool, ctx := makeStubbedTool(t,
|
||||
&searchStubProvider{name: "brave", called: &braveCalled, result: searchResult{Title: "brave-hit", URL: "https://b.example/x"}},
|
||||
&searchStubProvider{name: "exa", called: &exaCalled, result: searchResult{Title: "exa-hit", URL: "https://e.example/x"}},
|
||||
)
|
||||
|
||||
res := tool.Execute(ctx, map[string]any{
|
||||
"query": "solana dex volume",
|
||||
"provider": "exa",
|
||||
})
|
||||
|
||||
if res.IsError {
|
||||
t.Fatalf("unexpected error: %s", res.ForLLM)
|
||||
}
|
||||
if braveCalled {
|
||||
t.Errorf("brave was called but provider=\"exa\" requested a narrowed chain")
|
||||
}
|
||||
if !exaCalled {
|
||||
t.Errorf("exa not called despite provider=\"exa\"")
|
||||
}
|
||||
if !strings.Contains(res.ForLLM, "exa-hit") {
|
||||
t.Errorf("expected exa result in output, got: %s", res.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestExecute_ProviderParam_CaseInsensitive — "EXA", "Exa", "exa" all work.
|
||||
func TestExecute_ProviderParam_CaseInsensitive(t *testing.T) {
|
||||
for _, want := range []string{"EXA", "Exa", "exa"} {
|
||||
t.Run(want, func(t *testing.T) {
|
||||
exaCalled := false
|
||||
tool, ctx := makeStubbedTool(t,
|
||||
&searchStubProvider{name: "brave", called: nil, result: searchResult{Title: "brave-hit"}},
|
||||
&searchStubProvider{name: "exa", called: &exaCalled, result: searchResult{Title: "exa-hit"}},
|
||||
)
|
||||
res := tool.Execute(ctx, map[string]any{"query": "q", "provider": want})
|
||||
if res.IsError {
|
||||
t.Fatalf("error: %s", res.ForLLM)
|
||||
}
|
||||
if !exaCalled {
|
||||
t.Errorf("exa should be called for provider=%q", want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestExecute_ProviderParam_Unknown — caller asking for a provider not in the
|
||||
// tenant chain gets a clear error listing what IS available.
|
||||
func TestExecute_ProviderParam_Unknown(t *testing.T) {
|
||||
tool, ctx := makeStubbedTool(t,
|
||||
&searchStubProvider{name: "brave", result: searchResult{}},
|
||||
&searchStubProvider{name: "exa", result: searchResult{}},
|
||||
)
|
||||
res := tool.Execute(ctx, map[string]any{"query": "q", "provider": "google"})
|
||||
if !res.IsError {
|
||||
t.Fatalf("expected error for unknown provider, got success: %s", res.ForLLM)
|
||||
}
|
||||
if !strings.Contains(res.ForLLM, "google") || !strings.Contains(res.ForLLM, "brave") || !strings.Contains(res.ForLLM, "exa") {
|
||||
t.Errorf("error should name unknown provider AND list available ones, got: %s", res.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestExecute_NoProviderParam_FallsBackToFirstSuccessWins — omitting provider
|
||||
// preserves the existing behaviour: chain order is honoured, first success wins.
|
||||
func TestExecute_NoProviderParam_FallsBackToFirstSuccessWins(t *testing.T) {
|
||||
braveCalled, exaCalled := false, false
|
||||
tool, ctx := makeStubbedTool(t,
|
||||
&searchStubProvider{name: "brave", called: &braveCalled, result: searchResult{Title: "brave-hit"}},
|
||||
&searchStubProvider{name: "exa", called: &exaCalled, result: searchResult{Title: "exa-hit"}},
|
||||
)
|
||||
res := tool.Execute(ctx, map[string]any{"query": "q"})
|
||||
if res.IsError {
|
||||
t.Fatalf("error: %s", res.ForLLM)
|
||||
}
|
||||
if !braveCalled {
|
||||
t.Errorf("brave should be called first when no provider param")
|
||||
}
|
||||
if exaCalled {
|
||||
t.Errorf("exa should NOT be called when brave succeeded (first-success-wins)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestExecute_ProviderParam_CacheIsolated — same query but different `provider`
|
||||
// args must NOT collide in the cache, otherwise cross-engine corroboration
|
||||
// would just replay one engine's result twice.
|
||||
func TestExecute_ProviderParam_CacheIsolated(t *testing.T) {
|
||||
braveCalled, exaCalled := false, false
|
||||
tool, ctx := makeStubbedTool(t,
|
||||
&searchStubProvider{name: "brave", called: &braveCalled, result: searchResult{Title: "brave-hit"}},
|
||||
&searchStubProvider{name: "exa", called: &exaCalled, result: searchResult{Title: "exa-hit"}},
|
||||
)
|
||||
_ = tool.Execute(ctx, map[string]any{"query": "q", "provider": "brave"})
|
||||
_ = tool.Execute(ctx, map[string]any{"query": "q", "provider": "exa"})
|
||||
if !braveCalled || !exaCalled {
|
||||
t.Fatalf("both providers should run for distinct provider args (brave=%v, exa=%v)", braveCalled, exaCalled)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user