mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
fix(web): address sogou search review feedback
This commit is contained in:
@@ -760,6 +760,13 @@ func TestDefaultConfig_WebPreferNativeEnabled(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultConfig_WebProviderIsAuto(t *testing.T) {
|
||||
cfg := DefaultConfig()
|
||||
if cfg.Tools.Web.Provider != "auto" {
|
||||
t.Fatalf("DefaultConfig().Tools.Web.Provider = %q, want auto", cfg.Tools.Web.Provider)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultConfig_ToolFeedbackDisabled(t *testing.T) {
|
||||
cfg := DefaultConfig()
|
||||
if cfg.Agents.Defaults.ToolFeedback.Enabled {
|
||||
|
||||
@@ -278,7 +278,7 @@ func DefaultConfig() *Config {
|
||||
ToolConfig: ToolConfig{
|
||||
Enabled: true,
|
||||
},
|
||||
Provider: "sogou",
|
||||
Provider: "auto",
|
||||
PreferNative: true,
|
||||
Proxy: "",
|
||||
FetchLimitBytes: 10 * 1024 * 1024, // 10MB by default
|
||||
|
||||
+47
-8
@@ -117,6 +117,21 @@ func extractSogouURL(href string) string {
|
||||
return decoded
|
||||
}
|
||||
|
||||
func applySogouRangeHint(query string, rangeCode string) string {
|
||||
switch rangeCode {
|
||||
case "d":
|
||||
return query + " 最近一天"
|
||||
case "w":
|
||||
return query + " 最近一周"
|
||||
case "m":
|
||||
return query + " 最近一个月"
|
||||
case "y":
|
||||
return query + " 最近一年"
|
||||
default:
|
||||
return query
|
||||
}
|
||||
}
|
||||
|
||||
func normalizeSearchRange(raw string) (string, error) {
|
||||
rangeCode := strings.ToLower(strings.TrimSpace(raw))
|
||||
switch rangeCode {
|
||||
@@ -244,6 +259,10 @@ func (p *BraveSearchProvider) Search(
|
||||
count int,
|
||||
rangeCode string,
|
||||
) (string, error) {
|
||||
if p.keyPool == nil || len(p.keyPool.keys) == 0 {
|
||||
return "", errors.New("no API key provided")
|
||||
}
|
||||
|
||||
searchURL := fmt.Sprintf("https://api.search.brave.com/res/v1/web/search?q=%s&count=%d",
|
||||
url.QueryEscape(query), count)
|
||||
if freshness := mapBraveFreshness(rangeCode); freshness != "" {
|
||||
@@ -343,6 +362,10 @@ func (p *TavilySearchProvider) Search(
|
||||
count int,
|
||||
rangeCode string,
|
||||
) (string, error) {
|
||||
if p.keyPool == nil || len(p.keyPool.keys) == 0 {
|
||||
return "", errors.New("no API key provided")
|
||||
}
|
||||
|
||||
searchURL := p.baseURL
|
||||
if searchURL == "" {
|
||||
searchURL = "https://api.tavily.com/search"
|
||||
@@ -462,7 +485,7 @@ func (p *SogouSearchProvider) Search(
|
||||
|
||||
for page := 1; page <= maxPages && len(results) < count; page++ {
|
||||
params := url.Values{}
|
||||
params.Set("keyword", query)
|
||||
params.Set("keyword", applySogouRangeHint(query, rangeCode))
|
||||
params.Set("v", "5")
|
||||
params.Set("p", fmt.Sprintf("%d", page))
|
||||
|
||||
@@ -656,6 +679,10 @@ func (p *PerplexitySearchProvider) Search(
|
||||
count int,
|
||||
rangeCode string,
|
||||
) (string, error) {
|
||||
if p.keyPool == nil || len(p.keyPool.keys) == 0 {
|
||||
return "", errors.New("no API key provided")
|
||||
}
|
||||
|
||||
searchURL := "https://api.perplexity.ai/chat/completions"
|
||||
|
||||
var lastErr error
|
||||
@@ -769,6 +796,10 @@ func (p *SearXNGSearchProvider) Search(
|
||||
count int,
|
||||
rangeCode string,
|
||||
) (string, error) {
|
||||
if p.baseURL == "" {
|
||||
return "", errors.New("no SearXNG URL provided")
|
||||
}
|
||||
|
||||
searchURL := fmt.Sprintf("%s/search?q=%s&format=json&categories=general",
|
||||
strings.TrimSuffix(p.baseURL, "/"),
|
||||
url.QueryEscape(query))
|
||||
@@ -843,6 +874,10 @@ func (p *GLMSearchProvider) Search(
|
||||
count int,
|
||||
rangeCode string,
|
||||
) (string, error) {
|
||||
if p.apiKey == "" {
|
||||
return "", errors.New("no API key provided")
|
||||
}
|
||||
|
||||
searchURL := p.baseURL
|
||||
if searchURL == "" {
|
||||
searchURL = "https://open.bigmodel.cn/api/paas/v4/web_search"
|
||||
@@ -932,6 +967,10 @@ func (p *BaiduSearchProvider) Search(
|
||||
count int,
|
||||
rangeCode string,
|
||||
) (string, error) {
|
||||
if p.apiKey == "" {
|
||||
return "", errors.New("no API key provided")
|
||||
}
|
||||
|
||||
searchURL := p.baseURL
|
||||
if searchURL == "" {
|
||||
searchURL = "https://qianfan.baidubce.com/v2/ai_search/web_search"
|
||||
@@ -1065,7 +1104,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in
|
||||
client: client,
|
||||
}, maxResults, nil
|
||||
case "perplexity":
|
||||
if !opts.PerplexityEnabled || len(opts.PerplexityAPIKeys) == 0 {
|
||||
if !opts.PerplexityEnabled {
|
||||
return nil, 0, nil
|
||||
}
|
||||
client, err := utils.CreateHTTPClient(opts.Proxy, perplexityTimeout)
|
||||
@@ -1082,7 +1121,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in
|
||||
client: client,
|
||||
}, maxResults, nil
|
||||
case "brave":
|
||||
if !opts.BraveEnabled || len(opts.BraveAPIKeys) == 0 {
|
||||
if !opts.BraveEnabled {
|
||||
return nil, 0, nil
|
||||
}
|
||||
client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout)
|
||||
@@ -1099,7 +1138,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in
|
||||
client: client,
|
||||
}, maxResults, nil
|
||||
case "searxng":
|
||||
if !opts.SearXNGEnabled || opts.SearXNGBaseURL == "" {
|
||||
if !opts.SearXNGEnabled {
|
||||
return nil, 0, nil
|
||||
}
|
||||
maxResults := 10
|
||||
@@ -1110,7 +1149,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in
|
||||
baseURL: opts.SearXNGBaseURL,
|
||||
}, maxResults, nil
|
||||
case "tavily":
|
||||
if !opts.TavilyEnabled || len(opts.TavilyAPIKeys) == 0 {
|
||||
if !opts.TavilyEnabled {
|
||||
return nil, 0, nil
|
||||
}
|
||||
client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout)
|
||||
@@ -1144,7 +1183,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in
|
||||
client: client,
|
||||
}, maxResults, nil
|
||||
case "baidu_search":
|
||||
if !opts.BaiduSearchEnabled || opts.BaiduSearchAPIKey == "" {
|
||||
if !opts.BaiduSearchEnabled {
|
||||
return nil, 0, nil
|
||||
}
|
||||
client, err := utils.CreateHTTPClient(opts.Proxy, perplexityTimeout)
|
||||
@@ -1162,7 +1201,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in
|
||||
client: client,
|
||||
}, maxResults, nil
|
||||
case "glm_search":
|
||||
if !opts.GLMSearchEnabled || opts.GLMSearchAPIKey == "" {
|
||||
if !opts.GLMSearchEnabled {
|
||||
return nil, 0, nil
|
||||
}
|
||||
client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout)
|
||||
@@ -1196,7 +1235,7 @@ func NewWebSearchTool(opts WebSearchToolOptions) (*WebSearchTool, error) {
|
||||
}
|
||||
|
||||
if provider == nil {
|
||||
for _, name := range []string{"sogou", "perplexity", "brave", "searxng", "tavily", "duckduckgo", "baidu_search", "glm_search"} {
|
||||
for _, name := range []string{"perplexity", "brave", "searxng", "tavily", "sogou", "duckduckgo", "baidu_search", "glm_search"} {
|
||||
provider, maxResults, err = opts.providerByName(name)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
+54
-3
@@ -385,14 +385,24 @@ func TestWebFetchTool_PayloadTooLarge(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestWebTool_WebSearch_NoApiKey verifies that no tool is created when API key is missing
|
||||
// TestWebTool_WebSearch_NoApiKey verifies missing credentials are surfaced at execution time.
|
||||
func TestWebTool_WebSearch_NoApiKey(t *testing.T) {
|
||||
tool, err := NewWebSearchTool(WebSearchToolOptions{BraveEnabled: true, BraveAPIKeys: nil})
|
||||
if err != nil {
|
||||
t.Fatalf("Unexpected error: %v", err)
|
||||
}
|
||||
if tool != nil {
|
||||
t.Errorf("Expected nil tool when Brave API key is empty")
|
||||
if tool == nil {
|
||||
t.Fatalf("Expected tool when Brave is enabled, even without API keys")
|
||||
}
|
||||
|
||||
result := tool.Execute(context.Background(), map[string]any{
|
||||
"query": "test query",
|
||||
})
|
||||
if !result.IsError {
|
||||
t.Fatalf("Expected missing Brave API key to return error")
|
||||
}
|
||||
if !strings.Contains(result.ForLLM, "no API key provided") {
|
||||
t.Fatalf("Unexpected error message: %s", result.ForLLM)
|
||||
}
|
||||
|
||||
// Also nil when nothing is enabled
|
||||
@@ -1693,6 +1703,29 @@ func TestWebTool_SogouSearch_Success(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplySogouRangeHint(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
query string
|
||||
rangeCode string
|
||||
want string
|
||||
}{
|
||||
{name: "empty range", query: "golang", rangeCode: "", want: "golang"},
|
||||
{name: "day", query: "golang", rangeCode: "d", want: "golang 最近一天"},
|
||||
{name: "week", query: "golang", rangeCode: "w", want: "golang 最近一周"},
|
||||
{name: "month", query: "golang", rangeCode: "m", want: "golang 最近一个月"},
|
||||
{name: "year", query: "golang", rangeCode: "y", want: "golang 最近一年"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := applySogouRangeHint(tt.query, tt.rangeCode); got != tt.want {
|
||||
t.Fatalf("applySogouRangeHint(%q, %q) = %q, want %q", tt.query, tt.rangeCode, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWebTool_SogouPriorityAndExplicitProvider(t *testing.T) {
|
||||
tool, err := NewWebSearchTool(WebSearchToolOptions{
|
||||
SogouEnabled: true,
|
||||
@@ -1722,6 +1755,24 @@ func TestWebTool_SogouPriorityAndExplicitProvider(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestWebTool_AutoProviderPrefersConfiguredProvidersBeforeSogou(t *testing.T) {
|
||||
tool, err := NewWebSearchTool(WebSearchToolOptions{
|
||||
SogouEnabled: true,
|
||||
SogouMaxResults: 5,
|
||||
BraveEnabled: true,
|
||||
BraveAPIKeys: []string{"brave-key"},
|
||||
BraveMaxResults: 5,
|
||||
DuckDuckGoEnabled: true,
|
||||
DuckDuckGoMaxResults: 5,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("NewWebSearchTool() error: %v", err)
|
||||
}
|
||||
if _, ok := tool.provider.(*BraveSearchProvider); !ok {
|
||||
t.Fatalf("expected BraveSearchProvider, got %T", tool.provider)
|
||||
}
|
||||
}
|
||||
|
||||
type roundTripFunc func(*http.Request) (*http.Response, error)
|
||||
|
||||
func (fn roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
|
||||
+38
-12
@@ -43,11 +43,12 @@ type webSearchProviderOption struct {
|
||||
}
|
||||
|
||||
type webSearchProviderConfig struct {
|
||||
Enabled bool `json:"enabled"`
|
||||
MaxResults int `json:"max_results"`
|
||||
BaseURL string `json:"base_url,omitempty"`
|
||||
APIKey string `json:"api_key,omitempty"`
|
||||
APIKeySet bool `json:"api_key_set,omitempty"`
|
||||
Enabled bool `json:"enabled"`
|
||||
MaxResults int `json:"max_results"`
|
||||
BaseURL string `json:"base_url,omitempty"`
|
||||
APIKey string `json:"api_key,omitempty"`
|
||||
APIKeys []string `json:"api_keys,omitempty"`
|
||||
APIKeySet bool `json:"api_key_set,omitempty"`
|
||||
}
|
||||
|
||||
type webSearchConfigResponse struct {
|
||||
@@ -416,23 +417,23 @@ func (h *Handler) handleUpdateWebSearchConfig(w http.ResponseWriter, r *http.Req
|
||||
if settings, ok := req.Settings["brave"]; ok {
|
||||
cfg.Tools.Web.Brave.Enabled = settings.Enabled
|
||||
cfg.Tools.Web.Brave.MaxResults = settings.MaxResults
|
||||
if key := strings.TrimSpace(settings.APIKey); key != "" {
|
||||
cfg.Tools.Web.Brave.SetAPIKey(key)
|
||||
if keys, ok := normalizeWebSearchAPIKeys(settings.APIKeys, settings.APIKey); ok {
|
||||
cfg.Tools.Web.Brave.SetAPIKeys(keys)
|
||||
}
|
||||
}
|
||||
if settings, ok := req.Settings["tavily"]; ok {
|
||||
cfg.Tools.Web.Tavily.Enabled = settings.Enabled
|
||||
cfg.Tools.Web.Tavily.MaxResults = settings.MaxResults
|
||||
cfg.Tools.Web.Tavily.BaseURL = strings.TrimSpace(settings.BaseURL)
|
||||
if key := strings.TrimSpace(settings.APIKey); key != "" {
|
||||
cfg.Tools.Web.Tavily.SetAPIKey(key)
|
||||
if keys, ok := normalizeWebSearchAPIKeys(settings.APIKeys, settings.APIKey); ok {
|
||||
cfg.Tools.Web.Tavily.SetAPIKeys(keys)
|
||||
}
|
||||
}
|
||||
if settings, ok := req.Settings["perplexity"]; ok {
|
||||
cfg.Tools.Web.Perplexity.Enabled = settings.Enabled
|
||||
cfg.Tools.Web.Perplexity.MaxResults = settings.MaxResults
|
||||
if key := strings.TrimSpace(settings.APIKey); key != "" {
|
||||
cfg.Tools.Web.Perplexity.SetAPIKey(key)
|
||||
if keys, ok := normalizeWebSearchAPIKeys(settings.APIKeys, settings.APIKey); ok {
|
||||
cfg.Tools.Web.Perplexity.APIKeys = config.SimpleSecureStrings(keys...)
|
||||
}
|
||||
}
|
||||
if settings, ok := req.Settings["searxng"]; ok {
|
||||
@@ -479,6 +480,31 @@ func normalizeWebSearchProvider(provider string) string {
|
||||
}
|
||||
}
|
||||
|
||||
func normalizeWebSearchAPIKeys(apiKeys []string, apiKey string) ([]string, bool) {
|
||||
if apiKeys != nil {
|
||||
keys := make([]string, 0, len(apiKeys))
|
||||
seen := make(map[string]struct{}, len(apiKeys))
|
||||
for _, key := range apiKeys {
|
||||
trimmed := strings.TrimSpace(key)
|
||||
if trimmed == "" {
|
||||
continue
|
||||
}
|
||||
if _, ok := seen[trimmed]; ok {
|
||||
continue
|
||||
}
|
||||
seen[trimmed] = struct{}{}
|
||||
keys = append(keys, trimmed)
|
||||
}
|
||||
return keys, true
|
||||
}
|
||||
|
||||
if trimmed := strings.TrimSpace(apiKey); trimmed != "" {
|
||||
return []string{trimmed}, true
|
||||
}
|
||||
|
||||
return nil, false
|
||||
}
|
||||
|
||||
func buildWebSearchConfigResponse(cfg *config.Config) webSearchConfigResponse {
|
||||
current := resolveCurrentWebSearchProvider(cfg)
|
||||
settings := map[string]webSearchProviderConfig{
|
||||
@@ -614,7 +640,7 @@ func resolveCurrentWebSearchProvider(cfg *config.Config) string {
|
||||
if selected != "" && selected != "auto" && webSearchProviderConfigured(cfg, selected) {
|
||||
return selected
|
||||
}
|
||||
for _, name := range []string{"sogou", "perplexity", "brave", "searxng", "tavily", "duckduckgo", "baidu_search", "glm_search"} {
|
||||
for _, name := range []string{"perplexity", "brave", "searxng", "tavily", "sogou", "duckduckgo", "baidu_search", "glm_search"} {
|
||||
if webSearchProviderConfigured(cfg, name) {
|
||||
return name
|
||||
}
|
||||
|
||||
@@ -245,6 +245,15 @@ func TestHandleUpdateWebSearchConfig(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
||||
cfg, err := config.LoadConfig(configPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadConfig() error = %v", err)
|
||||
}
|
||||
cfg.Tools.Web.Brave.SetAPIKeys([]string{"brave-old-1", "brave-old-2"})
|
||||
if err := config.SaveConfig(configPath, cfg); err != nil {
|
||||
t.Fatalf("SaveConfig() error = %v", err)
|
||||
}
|
||||
|
||||
h := NewHandler(configPath)
|
||||
mux := http.NewServeMux()
|
||||
h.RegisterRoutes(mux)
|
||||
@@ -294,3 +303,89 @@ func TestHandleUpdateWebSearchConfig(t *testing.T) {
|
||||
t.Fatalf("brave api key not updated")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleUpdateWebSearchConfig_PreservesAndReplacesMultiKeys(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
||||
cfg, err := config.LoadConfig(configPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadConfig() error = %v", err)
|
||||
}
|
||||
cfg.Tools.Web.Brave.SetAPIKeys([]string{"brave-old-1", "brave-old-2"})
|
||||
if err := config.SaveConfig(configPath, cfg); err != nil {
|
||||
t.Fatalf("SaveConfig() error = %v", err)
|
||||
}
|
||||
|
||||
h := NewHandler(configPath)
|
||||
mux := http.NewServeMux()
|
||||
h.RegisterRoutes(mux)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPut,
|
||||
"/api/tools/web-search-config",
|
||||
bytes.NewBufferString(`{
|
||||
"provider":"auto",
|
||||
"prefer_native":true,
|
||||
"proxy":"",
|
||||
"settings":{
|
||||
"brave":{"enabled":true,"max_results":7}
|
||||
}
|
||||
}`),
|
||||
)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
mux.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
|
||||
}
|
||||
|
||||
updated, err := config.LoadConfig(configPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadConfig() error = %v", err)
|
||||
}
|
||||
if got := updated.Tools.Web.Brave.APIKeys.Values(); len(got) != 2 || got[0] != "brave-old-1" || got[1] != "brave-old-2" {
|
||||
t.Fatalf("brave api keys should be preserved, got %#v", got)
|
||||
}
|
||||
|
||||
rec = httptest.NewRecorder()
|
||||
req = httptest.NewRequest(
|
||||
http.MethodPut,
|
||||
"/api/tools/web-search-config",
|
||||
bytes.NewBufferString(`{
|
||||
"provider":"auto",
|
||||
"prefer_native":true,
|
||||
"proxy":"",
|
||||
"settings":{
|
||||
"brave":{"enabled":true,"max_results":7,"api_keys":["brave-new-1","brave-new-2","brave-new-1"]}
|
||||
}
|
||||
}`),
|
||||
)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
mux.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
|
||||
}
|
||||
|
||||
updated, err = config.LoadConfig(configPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadConfig() error = %v", err)
|
||||
}
|
||||
if got := updated.Tools.Web.Brave.APIKeys.Values(); len(got) != 2 || got[0] != "brave-new-1" || got[1] != "brave-new-2" {
|
||||
t.Fatalf("brave api keys should be replaced by api_keys, got %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveCurrentWebSearchProvider_PrefersConfiguredProvidersBeforeSogou(t *testing.T) {
|
||||
cfg := config.DefaultConfig()
|
||||
cfg.Tools.Web.Provider = "auto"
|
||||
cfg.Tools.Web.Sogou.Enabled = true
|
||||
cfg.Tools.Web.Brave.Enabled = true
|
||||
cfg.Tools.Web.Brave.SetAPIKey("brave-test-key")
|
||||
|
||||
if got := resolveCurrentWebSearchProvider(cfg); got != "brave" {
|
||||
t.Fatalf("resolveCurrentWebSearchProvider() = %q, want brave", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user