mirror of
https://github.com/vxcontrol/pentagi.git
synced 2026-07-27 06:21:34 +00:00
Merge pull request #303 from mason5052/codex/issue-280-ollama-tool-support-validation
fix: clarify Ollama models without tool support
This commit is contained in:
@@ -44,6 +44,33 @@ 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 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.
|
||||
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 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)
|
||||
}
|
||||
|
||||
type reflectorRetryContextKey struct{}
|
||||
|
||||
// isReflectorRetry checks if we are already in a reflector retry cycle
|
||||
|
||||
@@ -2,7 +2,10 @@ package providers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
@@ -1174,3 +1177,99 @@ 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",
|
||||
"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,
|
||||
},
|
||||
{
|
||||
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",
|
||||
"metadata advertises tool/function calling",
|
||||
},
|
||||
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")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -445,7 +445,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{
|
||||
@@ -591,7 +591,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{
|
||||
|
||||
Reference in New Issue
Block a user