Skip to content

fix: enhance count argument handling in WebSearchTool to support string input #741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Xeven777 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: enhance count argument handling in WebSearchTool to support string input #741

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions pkg/tools/web.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"encoding/json"
"fmt"
"io"
"math"
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The math import is only used for the strict integer validation (math.Round and math.Abs). If the fractional value validation is removed (as suggested in other comments), this import would no longer be needed and should be removed to avoid unnecessary dependencies.

Suggested change
"math"

Copilot uses AI. Check for mistakes.
"net/http"
"net/url"
"regexp"
"strconv"
"strings"
"time"
)
Expand Down Expand Up @@ -473,9 +475,22 @@ func (t *WebSearchTool) Execute(ctx context.Context, args map[string]any) *ToolR
}

count := t.maxResults
if c, ok := args["count"].(float64); ok {
if int(c) > 0 && int(c) <= 10 {
count = int(c)
switch v := args["count"].(type) {
case float64:
rounded := int(math.Round(v))
if math.Abs(v-float64(rounded)) > 1e-9 {
return ErrorResult(fmt.Sprintf("count must be an integer, got %v", v))
}
if rounded > 0 && rounded <= 10 {
count = rounded
}
Comment on lines +480 to +486
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new validation that rejects fractional float64 values (lines 481-482) is a breaking change from the previous behavior, which silently converted fractional values to integers. If the count parameter receives 5.5, the old code would have converted it to 5 and used it, but the new code returns an error. Consider whether this breaking change is intentional and justified, or if you should maintain backward compatibility by silently truncating/rounding fractional values as the original code did.

Suggested change
rounded := int(math.Round(v))
if math.Abs(v-float64(rounded)) > 1e-9 {
return ErrorResult(fmt.Sprintf("count must be an integer, got %v", v))
}
if rounded > 0 && rounded <= 10 {
count = rounded
}
n := int(v)
if n > 0 && n <= 10 {
count = n
}

Copilot uses AI. Check for mistakes.
Comment on lines +480 to +486
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strict integer validation for float64 values is inconsistent with how other tools in the codebase handle numeric parameters. For example, skills_search.go (line 62-63) and cron.go (line 155, 161) simply cast float64 to int without checking for fractional parts. When JSON is unmarshaled into map[string]any, integer values become float64 by default in Go, so values like 5.0 are expected and valid. The current implementation would reject 5.0000000001 due to floating-point precision issues, which could cause unexpected failures. Consider either removing this validation to match the codebase pattern or at least using int(v) instead of math.Round if you want to be lenient with very small fractional parts.

Suggested change
rounded := int(math.Round(v))
if math.Abs(v-float64(rounded)) > 1e-9 {
return ErrorResult(fmt.Sprintf("count must be an integer, got %v", v))
}
if rounded > 0 && rounded <= 10 {
count = rounded
}
n := int(v)
if n > 0 && n <= 10 {
count = n
}

Copilot uses AI. Check for mistakes.
case int:
if v > 0 && v <= 10 {
count = v
}
case string:
if n, err := strconv.Atoi(v); err == nil && n > 0 && n <= 10 {
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the string parsing fails or the value is out of range, the code silently falls back to the default maxResults without informing the user. This is inconsistent with the float64 case (lines 481-482) which returns an error for invalid values. Consider either returning an error for invalid string inputs to maintain consistency, or document why strings are treated more leniently than floats. The silent fallback could confuse users who expect their count parameter to be respected.

Suggested change
if n, err := strconv.Atoi(v); err == nil && n > 0 && n <= 10 {
n, err := strconv.Atoi(v)
if err != nil {
return ErrorResult(fmt.Sprintf("count must be an integer, got %q", v))
}
if n > 0 && n <= 10 {

Copilot uses AI. Check for mistakes.
count = n
}
Comment on lines +491 to 494
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding string support for the count parameter deviates from the established pattern in the codebase. Other tools with integer parameters (like skills_search.go line 62, cron.go lines 149-150, i2c_linux.go line 159) only accept float64, not strings. The Parameters() method declares count as type "integer", which means LLM APIs will send it as a number (decoded as float64 in Go). Unless there's a specific use case requiring string support (e.g., manual testing, specific provider quirks), consider removing the string case to maintain consistency with other tools in the codebase.

Suggested change
case string:
if n, err := strconv.Atoi(v); err == nil && n > 0 && n <= 10 {
count = n
}

Copilot uses AI. Check for mistakes.
}
Comment on lines 478 to 495
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new count parameter handling logic (both float64 and string cases) lacks test coverage. The existing test file web_test.go has no tests for the count parameter. Consider adding tests for: valid integer values (as float64), fractional values that should be rejected, string inputs (valid and invalid), boundary values (0, 1, 10, 11), and the default fallback behavior when count is omitted or invalid.

Copilot uses AI. Check for mistakes.

Expand Down