From 201beec4c7430d17459730d1e30e4f07b32bfd9c Mon Sep 17 00:00:00 2001 From: stefanwalcz Date: Tue, 7 Jul 2026 16:05:39 +0200 Subject: [PATCH] pickTool: name the chosen tool in the error and match tool names leniently Two small robustness fixes for local-model tool selection, verified against production traffic (repeated hard turn-failures across two Qwen-class models). - pickTool now includes the chosen name in its error: "chosen tool %q not found" instead of a bare "chosen tool not found" (the name was only xlog.Debug-logged, leaving operators at info level with no diagnosability). - Tools.Find gains a lenient fallback that triggers ONLY when no exact match exists: it strips a leading namespace segment (functions./tools//ns::), case-folds, and treats '-' == '_'. Local models routinely emit variants like functions.foo, Foo or foo-bar. Exact matches are unaffected (fast path first). Adds table-driven tests for normalizeToolName, lenient Find, and the exact-wins-over-lenient guarantee. Signed-off-by: stefanwalcz --- tools.go | 30 ++++++++++++++++++++- tools_lenient_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 tools_lenient_test.go diff --git a/tools.go b/tools.go index 36e453b..211cb54 100644 --- a/tools.go +++ b/tools.go @@ -133,15 +133,43 @@ func (t *ToolDefinition[T]) Execute(args map[string]any) (string, any, error) { type Tools []ToolDefinitionInterface +// Find returns the tool whose function name matches name. It first tries an +// exact match (fast path, unchanged behavior). If none is found, it falls back +// to a lenient match: local models frequently emit namespaced/case/separator +// variants (e.g. "functions.foo", "tools/foo", "Foo", "foo-bar" for "foo_bar"). +// The lenient pass never overrides an exact match, so existing resolutions are +// unaffected. func (t Tools) Find(name string) ToolDefinitionInterface { for _, tool := range t { if tool.Tool().Function.Name == name { return tool } } + norm := normalizeToolName(name) + if norm == "" { + return nil + } + for _, tool := range t { + if normalizeToolName(tool.Tool().Function.Name) == norm { + return tool + } + } return nil } +// normalizeToolName reduces a tool name to a comparison key: it drops a leading +// namespace segment ("functions.", "tools/", "namespace::"), lower-cases, trims +// spaces, and treats '-' and '_' as equivalent. Used only as a lenient fallback +// in Find; it never affects exact-match resolution. +func normalizeToolName(s string) string { + if i := strings.LastIndexAny(s, "./:"); i >= 0 { + s = s[i+1:] // segment after the last separator (may be empty → empty key) + } + s = strings.ToLower(strings.TrimSpace(s)) + s = strings.ReplaceAll(s, "-", "_") + return s +} + func (t Tools) ToOpenAI() []openai.Tool { openaiTools := []openai.Tool{} for _, tool := range t { @@ -821,7 +849,7 @@ func pickTool(ctx context.Context, llm LLM, fragment Fragment, tools Tools, opts chosenTool := tools.Find(intentionResponse.Tool) if chosenTool == nil { xlog.Debug("[pickTool] Chosen tool not found", "tool", intentionResponse.Tool) - return nil, fmt.Errorf("chosen tool not found") + return nil, fmt.Errorf("chosen tool %q not found", intentionResponse.Tool) } toolChoices = append(toolChoices, &ToolChoice{ diff --git a/tools_lenient_test.go b/tools_lenient_test.go new file mode 100644 index 0000000..dc448bd --- /dev/null +++ b/tools_lenient_test.go @@ -0,0 +1,61 @@ +package cogito + +import "testing" + +func TestNormalizeToolName(t *testing.T) { + cases := []struct{ in, want string }{ + {"foo", "foo"}, + {"Foo", "foo"}, + {"functions.foo", "foo"}, + {"tools/foo", "foo"}, + {"namespace::foo", "foo"}, + {"foo-bar", "foo_bar"}, + {"Foo-Bar", "foo_bar"}, + {"functions.Coach_Regeln", "coach_regeln"}, + {" foo ", "foo"}, + {"", ""}, + {"functions.", ""}, // nothing after the separator → empty key + } + for _, c := range cases { + if got := normalizeToolName(c.in); got != c.want { + t.Errorf("normalizeToolName(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestFindLenient(t *testing.T) { + tools := Tools{newNamedTool("coach_regeln")} + + // exact + lenient variants local models routinely emit + for _, n := range []string{ + "coach_regeln", // exact + "functions.coach_regeln", // namespaced + "tools/coach_regeln", + "Coach_Regeln", // case + "coach-regeln", // separator + } { + if tools.Find(n) == nil { + t.Errorf("Find(%q) = nil, want match", n) + } + } + + for _, n := range []string{"other_tool", "", "functions."} { + if tools.Find(n) != nil { + t.Errorf("Find(%q) matched, want nil", n) + } + } +} + +// An exact match must always win, even when a lenient candidate appears earlier +// in the slice — the lenient pass is a fallback, never an override. +func TestFindExactWinsOverLenient(t *testing.T) { + tools := Tools{newNamedTool("functions.foo"), newNamedTool("foo")} + got := tools.Find("foo") + if got == nil || got.Tool().Function.Name != "foo" { + name := "" + if got != nil { + name = got.Tool().Function.Name + } + t.Errorf(`Find("foo") = %q, want exact "foo"`, name) + } +}