feat: improve searchtools #146
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func (c *UtcpClient) SearchTools(providerPrefix string, limit int) ([]Tool, error) { | ||
| all, err := c.toolRepository.GetTools(context.Background()) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Convert []*Tool to []Tool if needed | ||
| result := make([]Tool, len(tools)) | ||
| for i, tool := range tools { | ||
| switch t := any(tool).(type) { | ||
| case Tool: | ||
| result[i] = t | ||
| case *Tool: | ||
| result[i] = *t | ||
| default: | ||
| // fallback (shouldn't happen) | ||
| result[i] = Tool{} | ||
| var filtered []Tool | ||
| for _, t := range all { | ||
| if strings.HasPrefix(t.Name, providerPrefix+".") { | ||
| filtered = append(filtered, t) | ||
| } | ||
| } | ||
| return result, nil | ||
| if len(filtered) == 0 { | ||
| return nil, fmt.Errorf("no tools found for provider %q", providerPrefix) | ||
| } | ||
| return filtered, nil |
There was a problem hiding this comment.
The new SearchTools implementation no longer delegates to the configured searchStrategy; it simply filters tools whose names begin with providerPrefix and returns an error when none match. Existing callers still pass natural‑language queries or an empty string to discover tools (for example src/plugins/codemode/orchestrator.go invokes client.SearchTools("", limit) and the codemode plugin passes user queries). Those calls now either fail with "no tools found" or only match provider names, so tag/description search is effectively disabled. This regressions breaks tool discovery across the SDK. Consider keeping the strategy-based search and providing provider filtering as a separate helper.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="src/tag/tag_search.go">
<violation number="1" location="src/tag/tag_search.go:98">
Appending the tool before testing `len(result) >= limit` lets SearchTools return a result even when `limit <= 0`, breaking the limit contract for zero/negative requests.</violation>
</file>
<file name="utcp_client.go">
<violation number="1" location="utcp_client.go:527">
SearchTools now requires a provider prefix and breaks all existing query-based discovery (e.g., passing empty string) by always returning an error.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| break | ||
| for _, st := range scored { | ||
| if st.score > 0 { | ||
| result = append(result, st.tool) |
There was a problem hiding this comment.
Appending the tool before testing len(result) >= limit lets SearchTools return a result even when limit <= 0, breaking the limit contract for zero/negative requests.
Prompt for AI agents
Address the following comment on src/tag/tag_search.go at line 98:
<comment>Appending the tool before testing `len(result) >= limit` lets SearchTools return a result even when `limit <= 0`, breaking the limit contract for zero/negative requests.</comment>
<file context>
@@ -43,57 +43,74 @@ func (s *TagSearchStrategy) SearchTools(ctx context.Context, query string, limit
- break
+ for _, st := range scored {
+ if st.score > 0 {
+ result = append(result, st.tool)
+ if len(result) >= limit {
+ break
</file context>
| result[i] = Tool{} | ||
| var filtered []Tool | ||
| for _, t := range all { | ||
| if strings.HasPrefix(t.Name, providerPrefix+".") { |
There was a problem hiding this comment.
SearchTools now requires a provider prefix and breaks all existing query-based discovery (e.g., passing empty string) by always returning an error.
Prompt for AI agents
Address the following comment on utcp_client.go at line 527:
<comment>SearchTools now requires a provider prefix and breaks all existing query-based discovery (e.g., passing empty string) by always returning an error.</comment>
<file context>
@@ -517,26 +517,21 @@ func (c *UtcpClient) CallTool(
- result[i] = Tool{}
+ var filtered []Tool
+ for _, t := range all {
+ if strings.HasPrefix(t.Name, providerPrefix+".") {
+ filtered = append(filtered, t)
}
</file context>
Summary by cubic
Improves tool discovery with more relevant tag/description scoring and adds provider-prefixed filtering in UtcpClient.SearchTools. The example now searches for http tools.
New Features
Migration
Written for commit 802b552. Summary will update automatically on new commits.