From 552eda8746b428ea07435f049a7f096a3d417504 Mon Sep 17 00:00:00 2001 From: mason5052 Date: Wed, 6 May 2026 18:30:32 -0400 Subject: [PATCH 1/2] fix: clarify Ollama models without tool support ## Summary Surface an actionable hint when an Ollama model that does not support tool/function calling is selected for a flow or assistant session, so users do not have to parse a five-wrap-deep "failed to determine tool call ID template" stack trace to figure out why flow creation fails. ## Problem A user on an Ollama deployment of `gemma3:27b-it-q4_K_M` reports that the Provider/LLM test passes the basic completion checks but flow creation fails with: failed to create flow worker: failed to get flow provider: failed to determine tool call ID template: failed to collect tool call ID samples: all sample collection attempts failed: failed to call LLM: 400 Bad Request: registry.ollama.ai/library/gemma3:27b-it-q4_K_M does not support tools The underlying signal "does not support tools" is emitted by the Ollama API itself; PentAGI buries it five wraps deep, so the user is left guessing whether the issue is configuration, networking, or something internal to PentAGI. ## Solution Add an unexported helper `wrapToolCallIDTemplateError` in `pkg/providers/helpers.go` that detects the upstream "does not support tools" substring and prepends a one-sentence guidance message naming the constraint and a few known-good Ollama tags (`llama3.1`, `qwen2.5`, `mistral-nemo`). All other errors keep the existing wording. The original error is preserved via `%w` so log spans, langfuse traces, and any future `errors.Is/As` callers continue to work. Both call sites in `pkg/providers/providers.go` (`NewFlowProvider`, `NewAssistantProvider`) now route through the helper. No schema, API, or DB changes. No new lifecycle state. No background work. The behavior change is the wording of one error string in one upstream-defined failure case. ## User Impact - Users who pick an Ollama model without tool support get a clear, actionable error during flow / assistant creation instead of a deep stack trace. - Users on tool-capable models (Anthropic, OpenAI, Bedrock, and tool-capable Ollama tags) see no behavioral or message change. - No restart, migration, or configuration step required. ## Test Plan - New table-driven test `TestWrapToolCallIDTemplateError` covers: - nil error -> nil - upstream "does not support tools" chain (matching the real five- wrap shape) -> actionable message + `errors.Is` round-trips to the original error - generic non-tools error -> existing wrap text preserved, no new guidance appended - `go test ./pkg/providers/...` passes locally. - `go build ./...` and `go vet ./pkg/providers/...` clean. Closes #280 Signed-off-by: mason5052 --- backend/pkg/providers/helpers.go | 26 +++++++++ backend/pkg/providers/helpers_test.go | 84 +++++++++++++++++++++++++++ backend/pkg/providers/providers.go | 4 +- 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/backend/pkg/providers/helpers.go b/backend/pkg/providers/helpers.go index 7c714473..cc80eeaf 100644 --- a/backend/pkg/providers/helpers.go +++ b/backend/pkg/providers/helpers.go @@ -44,6 +44,32 @@ type dummyMessage struct { Message string `json:"message"` } +// wrapToolCallIDTemplateError annotates an error returned by +// Provider.GetToolCallIDTemplate so the user sees an actionable hint when a +// known upstream limitation blocks flow creation. Currently it covers the +// Ollama "model does not support tools" case (issue #280): the underlying +// error is returned by the Ollama API itself and surfaces five wraps deep, +// which previously left the user with a cryptic "failed to determine tool +// call ID template" message. +// +// The function preserves the original error chain via %w so downstream code +// (logs, langfuse spans, errors.Is/As) keeps working. +func wrapToolCallIDTemplateError(err error) error { + if err == nil { + return nil + } + if strings.Contains(err.Error(), "does not support tools") { + return fmt.Errorf( + "failed to determine tool call ID template: the selected model "+ + "does not support tool/function calling, which PentAGI requires "+ + "for flow execution; pick an Ollama model that advertises the "+ + "\"tools\" capability (for example llama3.1, qwen2.5, or "+ + "mistral-nemo) and update the provider configuration: %w", + err) + } + return fmt.Errorf("failed to determine tool call ID template: %w", err) +} + type reflectorRetryContextKey struct{} // isReflectorRetry checks if we are already in a reflector retry cycle diff --git a/backend/pkg/providers/helpers_test.go b/backend/pkg/providers/helpers_test.go index 88a610ca..a59188c0 100644 --- a/backend/pkg/providers/helpers_test.go +++ b/backend/pkg/providers/helpers_test.go @@ -2,7 +2,10 @@ package providers import ( "encoding/json" + "errors" + "fmt" "slices" + "strings" "sync" "sync/atomic" "testing" @@ -1171,3 +1174,84 @@ func TestExecutionMonitorDetector_TotalCallsSequence(t *testing.T) { func mockToolCall(name string) llms.ToolCall { return llms.ToolCall{FunctionCall: &llms.FunctionCall{Name: name}} } + +func TestWrapToolCallIDTemplateError(t *testing.T) { + t.Parallel() + + // Mirror the real upstream wrap chain so the test traps regressions in + // substring detection if the inner wraps ever change. + ollamaErr := fmt.Errorf( + "all sample collection attempts failed: %w", + fmt.Errorf("failed to call LLM: %w", + errors.New("400 Bad Request: registry.ollama.ai/library/gemma3:27b-it-q4_K_M does not support tools")), + ) + genericErr := errors.New("connection refused") + + cases := []struct { + name string + input error + wantNil bool + wantContains []string + wantUnwrapsTo error + wantNotContain []string + }{ + { + name: "nil error passes through", + input: nil, + wantNil: true, + }, + { + name: "ollama tools error gets actionable hint", + input: ollamaErr, + wantContains: []string{ + "failed to determine tool call ID template", + "does not support tool/function calling", + "PentAGI requires", + "\"tools\" capability", + "does not support tools", + }, + wantUnwrapsTo: ollamaErr, + }, + { + name: "non-tools error keeps existing wrap", + input: genericErr, + wantContains: []string{ + "failed to determine tool call ID template", + "connection refused", + }, + wantNotContain: []string{ + "does not support tool/function calling", + "\"tools\" capability", + }, + wantUnwrapsTo: genericErr, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := wrapToolCallIDTemplateError(tc.input) + if tc.wantNil { + assert.Nil(t, got) + return + } + if !assert.NotNil(t, got) { + return + } + msg := got.Error() + for _, want := range tc.wantContains { + assert.Truef(t, strings.Contains(msg, want), + "expected error message to contain %q; got %q", want, msg) + } + for _, banned := range tc.wantNotContain { + assert.Falsef(t, strings.Contains(msg, banned), + "expected error message NOT to contain %q; got %q", banned, msg) + } + if tc.wantUnwrapsTo != nil { + assert.True(t, errors.Is(got, tc.wantUnwrapsTo), + "expected wrapped error to unwrap to original via errors.Is") + } + }) + } +} diff --git a/backend/pkg/providers/providers.go b/backend/pkg/providers/providers.go index 9a9a5a7d..e2947a3c 100644 --- a/backend/pkg/providers/providers.go +++ b/backend/pkg/providers/providers.go @@ -439,7 +439,7 @@ func (pc *providerController) NewFlowProvider( tcIDTemplate, err := prv.GetToolCallIDTemplate(ctx, prompter) if err != nil { - return nil, fmt.Errorf("failed to determine tool call ID template: %w", err) + return nil, wrapToolCallIDTemplateError(err) } fp := &flowProvider{ @@ -581,7 +581,7 @@ func (pc *providerController) NewAssistantProvider( tcIDTemplate, err := prv.GetToolCallIDTemplate(ctx, prompter) if err != nil { - return nil, fmt.Errorf("failed to determine tool call ID template: %w", err) + return nil, wrapToolCallIDTemplateError(err) } ap := &assistantProvider{ From 73d5b8366d923565c9d862182dca206f0387a2fb Mon Sep 17 00:00:00 2001 From: mason5052 Date: Wed, 6 May 2026 22:11:12 -0400 Subject: [PATCH 2/2] fix: drop unverified ollama tags and broaden hint context Address review on PR #303. - helpers.go: remove the concrete model examples (llama3.1, qwen2.5, mistral-nemo) from the Ollama tools-unsupported hint. PentAGI does not verify Ollama model capabilities from upstream metadata, so a hard-coded list of supposedly tool-capable tags is risky and ages poorly. The hint now points users to select an Ollama model whose own metadata advertises tool/function calling support. - helpers.go: rewrite the comment and error wording so it is no longer flow-only. The helper is shared by NewFlowProvider and NewAssistantProvider, so the message now refers to tool execution in flows and assistant sessions, and the doc-comment notes that wording must stay context-neutral. - helpers_test.go: tighten TestWrapToolCallIDTemplateError to match the new wording, ban the previous concrete model tags, and ban flow-only language ('flow execution', 'flow creation'). The test still verifies that the original underlying error is wrapped via errors.Is. Signed-off-by: mason5052 --- backend/pkg/providers/helpers.go | 17 +++++++++-------- backend/pkg/providers/helpers_test.go | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/backend/pkg/providers/helpers.go b/backend/pkg/providers/helpers.go index cc80eeaf..31174d14 100644 --- a/backend/pkg/providers/helpers.go +++ b/backend/pkg/providers/helpers.go @@ -46,11 +46,12 @@ type dummyMessage struct { // wrapToolCallIDTemplateError annotates an error returned by // Provider.GetToolCallIDTemplate so the user sees an actionable hint when a -// known upstream limitation blocks flow creation. Currently it covers the -// Ollama "model does not support tools" case (issue #280): the underlying -// error is returned by the Ollama API itself and surfaces five wraps deep, -// which previously left the user with a cryptic "failed to determine tool -// call ID template" message. +// known upstream limitation blocks provider initialization. Currently it +// covers the Ollama "model does not support tools" case (issue #280): the +// underlying error is returned by the Ollama API itself and surfaces five +// wraps deep, which previously left the user with a cryptic "failed to +// determine tool call ID template" message. This helper is shared by the +// flow and assistant provider paths, so the wording stays context-neutral. // // The function preserves the original error chain via %w so downstream code // (logs, langfuse spans, errors.Is/As) keeps working. @@ -62,9 +63,9 @@ func wrapToolCallIDTemplateError(err error) error { return fmt.Errorf( "failed to determine tool call ID template: the selected model "+ "does not support tool/function calling, which PentAGI requires "+ - "for flow execution; pick an Ollama model that advertises the "+ - "\"tools\" capability (for example llama3.1, qwen2.5, or "+ - "mistral-nemo) and update the provider configuration: %w", + "for tool execution in flows and assistant sessions; select an "+ + "Ollama model whose metadata advertises tool/function calling "+ + "support and update the provider configuration: %w", err) } return fmt.Errorf("failed to determine tool call ID template: %w", err) diff --git a/backend/pkg/providers/helpers_test.go b/backend/pkg/providers/helpers_test.go index a59188c0..3fe35b7e 100644 --- a/backend/pkg/providers/helpers_test.go +++ b/backend/pkg/providers/helpers_test.go @@ -1207,9 +1207,24 @@ func TestWrapToolCallIDTemplateError(t *testing.T) { "failed to determine tool call ID template", "does not support tool/function calling", "PentAGI requires", - "\"tools\" capability", + "flows and assistant sessions", + "metadata advertises tool/function calling", "does not support tools", }, + wantNotContain: []string{ + // The hint must not name specific model tags as + // tool-capable, because Ollama model capabilities are + // not verified by PentAGI and any concrete list ages + // poorly. + "llama3.1", + "qwen2.5", + "mistral-nemo", + // The helper is shared by both the flow and assistant + // provider paths, so the message must not imply this + // is a flow-only failure mode. + "flow execution", + "flow creation", + }, wantUnwrapsTo: ollamaErr, }, { @@ -1221,7 +1236,7 @@ func TestWrapToolCallIDTemplateError(t *testing.T) { }, wantNotContain: []string{ "does not support tool/function calling", - "\"tools\" capability", + "metadata advertises tool/function calling", }, wantUnwrapsTo: genericErr, },