error reporting fixes
This commit is contained in:
+4
-4
@@ -49,7 +49,7 @@ port = 8080
|
|||||||
[agents]
|
[agents]
|
||||||
[agents.coordinator]
|
[agents.coordinator]
|
||||||
role = "COORDINATOR"
|
role = "COORDINATOR"
|
||||||
model_id = "local"
|
model_id = "default"
|
||||||
enable_shell_exec = true
|
enable_shell_exec = true
|
||||||
streaming = false
|
streaming = false
|
||||||
log_intermediate = true
|
log_intermediate = true
|
||||||
@@ -66,7 +66,7 @@ port = 8080
|
|||||||
|
|
||||||
[agents.coder]
|
[agents.coder]
|
||||||
role = "SPECIALIST"
|
role = "SPECIALIST"
|
||||||
model_id = "local"
|
model_id = "default"
|
||||||
enable_shell_exec = true
|
enable_shell_exec = true
|
||||||
streaming = false
|
streaming = false
|
||||||
log_intermediate = true
|
log_intermediate = true
|
||||||
@@ -82,7 +82,7 @@ port = 8080
|
|||||||
|
|
||||||
[agents.reviewer]
|
[agents.reviewer]
|
||||||
role = "SPECIALIST"
|
role = "SPECIALIST"
|
||||||
model_id = "local"
|
model_id = "fast"
|
||||||
streaming = false
|
streaming = false
|
||||||
log_intermediate = true
|
log_intermediate = true
|
||||||
system_prompt = "You are a Quality Assurance Specialist and Code Reviewer. Your role is to analyze code for potential bugs, security vulnerabilities, and style violations."
|
system_prompt = "You are a Quality Assurance Specialist and Code Reviewer. Your role is to analyze code for potential bugs, security vulnerabilities, and style violations."
|
||||||
@@ -97,7 +97,7 @@ port = 8080
|
|||||||
|
|
||||||
[agents.tester]
|
[agents.tester]
|
||||||
role = "SPECIALIST"
|
role = "SPECIALIST"
|
||||||
model_id = "local"
|
model_id = "fast"
|
||||||
enable_shell_exec = true
|
enable_shell_exec = true
|
||||||
streaming = false
|
streaming = false
|
||||||
log_intermediate = true
|
log_intermediate = true
|
||||||
|
|||||||
+18
-3
@@ -133,7 +133,7 @@ func (s *Sidekick) runStep(ctx context.Context, history *[]Message,
|
|||||||
}
|
}
|
||||||
msg, err := CallLLM(ctx, s.Model, *history, s.ToolRegistry, sc, rc)
|
msg, err := CallLLM(ctx, s.Model, *history, s.ToolRegistry, sc, rc)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return false, "", fmt.Errorf("LLM call failed: %w", err)
|
return false, "", err
|
||||||
}
|
}
|
||||||
s.logIntermediate(msg)
|
s.logIntermediate(msg)
|
||||||
*history = append(*history, msg)
|
*history = append(*history, msg)
|
||||||
@@ -141,10 +141,11 @@ func (s *Sidekick) runStep(ctx context.Context, history *[]Message,
|
|||||||
act, params, parsed, err := s.parseAction(msg)
|
act, params, parsed, err := s.parseAction(msg)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if !parsed {
|
if !parsed {
|
||||||
|
// Not intended as JSON, just treat as RESPOND
|
||||||
act, params = ActionTypeRespond, map[string]interface{}{"response": msg.Content}
|
act, params = ActionTypeRespond, map[string]interface{}{"response": msg.Content}
|
||||||
} else {
|
} else {
|
||||||
*history = append(*history, Message{Role: MessageRoleUser, Content: fmt.Sprintf("Error: %v", err)})
|
// Intended as structured action but failed validation/parsing
|
||||||
return false, "", nil
|
return false, "", fmt.Errorf("the AI returned an invalid response: %w", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if act == ActionTypeRespond {
|
if act == ActionTypeRespond {
|
||||||
@@ -184,6 +185,9 @@ func (s *Sidekick) dispatch(ctx context.Context, a ActionType, p map[string]inte
|
|||||||
}
|
}
|
||||||
s.IntermediateHandler(fmt.Sprintf("Tool Result: %s", truncRes))
|
s.IntermediateHandler(fmt.Sprintf("Tool Result: %s", truncRes))
|
||||||
}
|
}
|
||||||
|
if tName == "read_file" || tName == "grep_file" {
|
||||||
|
return res, nil
|
||||||
|
}
|
||||||
return BufferGate(res, tf, s.Config.ToolResponseThreshold)
|
return BufferGate(res, tf, s.Config.ToolResponseThreshold)
|
||||||
}
|
}
|
||||||
if a == ActionTypeDelegate {
|
if a == ActionTypeDelegate {
|
||||||
@@ -202,12 +206,23 @@ func (s *Sidekick) parseAction(msg Message) (ActionType, map[string]interface{},
|
|||||||
}
|
}
|
||||||
var env ActionEnvelope
|
var env ActionEnvelope
|
||||||
c := strings.TrimSpace(msg.Content)
|
c := strings.TrimSpace(msg.Content)
|
||||||
|
isProbablyJSON := strings.HasPrefix(c, "{")
|
||||||
if strings.HasPrefix(c, "```json") && strings.HasSuffix(c, "```") {
|
if strings.HasPrefix(c, "```json") && strings.HasSuffix(c, "```") {
|
||||||
c = strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(c, "```json"), "```"))
|
c = strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(c, "```json"), "```"))
|
||||||
|
isProbablyJSON = true
|
||||||
}
|
}
|
||||||
if err := json.Unmarshal([]byte(c), &env); err != nil {
|
if err := json.Unmarshal([]byte(c), &env); err != nil {
|
||||||
|
if isProbablyJSON {
|
||||||
|
return "", nil, true, fmt.Errorf("malformed JSON action: %w", err)
|
||||||
|
}
|
||||||
return "", nil, false, err
|
return "", nil, false, err
|
||||||
}
|
}
|
||||||
|
if env.Action == "" {
|
||||||
|
if isProbablyJSON {
|
||||||
|
return "", nil, true, fmt.Errorf("AI response is missing the 'action' field")
|
||||||
|
}
|
||||||
|
return ActionTypeRespond, map[string]interface{}{"response": msg.Content}, false, nil
|
||||||
|
}
|
||||||
return env.Action, env.Params, true, nil
|
return env.Action, env.Params, true, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+22
-2
@@ -5,12 +5,21 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
var (
|
||||||
|
ErrLLMTimeout = errors.New("the request to the AI model timed out. Please try again later")
|
||||||
|
ErrLLMRateLimit = errors.New("rate limit exceeded. Please wait a moment and try again")
|
||||||
|
ErrLLMAuth = errors.New("authentication failed. Please check your API key")
|
||||||
|
ErrLLMServer = errors.New("the AI provider's server encountered an error. Please try again later")
|
||||||
|
ErrLLMInvalidResponse = errors.New("the AI model returned an invalid response structure")
|
||||||
|
)
|
||||||
|
|
||||||
// LLMClient represents an abstraction over the LLM provider
|
// LLMClient represents an abstraction over the LLM provider
|
||||||
type LLMClient interface {
|
type LLMClient interface {
|
||||||
Call(ctx context.Context, model ModelConfig, messages []Message,
|
Call(ctx context.Context, model ModelConfig, messages []Message,
|
||||||
@@ -63,11 +72,22 @@ func (s *standardLLMClient) Call(ctx context.Context, model ModelConfig, msgs []
|
|||||||
|
|
||||||
resp, err := client.Do(req)
|
resp, err := client.Do(req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
if errors.Is(err, context.DeadlineExceeded) || strings.Contains(err.Error(), "timeout") {
|
||||||
|
return Message{}, ErrLLMTimeout
|
||||||
|
}
|
||||||
return Message{}, fmt.Errorf("HTTP request failed: %w", err)
|
return Message{}, fmt.Errorf("HTTP request failed: %w", err)
|
||||||
}
|
}
|
||||||
defer resp.Body.Close()
|
defer resp.Body.Close()
|
||||||
|
|
||||||
if resp.StatusCode != http.StatusOK {
|
if resp.StatusCode != http.StatusOK {
|
||||||
|
switch resp.StatusCode {
|
||||||
|
case http.StatusUnauthorized, http.StatusForbidden:
|
||||||
|
return Message{}, ErrLLMAuth
|
||||||
|
case http.StatusTooManyRequests:
|
||||||
|
return Message{}, ErrLLMRateLimit
|
||||||
|
case http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable:
|
||||||
|
return Message{}, ErrLLMServer
|
||||||
|
}
|
||||||
body, _ := io.ReadAll(resp.Body)
|
body, _ := io.ReadAll(resp.Body)
|
||||||
return Message{}, fmt.Errorf("API returned error (%d): %s", resp.StatusCode, string(body))
|
return Message{}, fmt.Errorf("API returned error (%d): %s", resp.StatusCode, string(body))
|
||||||
}
|
}
|
||||||
@@ -91,7 +111,7 @@ func (s *standardLLMClient) Call(ctx context.Context, model ModelConfig, msgs []
|
|||||||
}
|
}
|
||||||
|
|
||||||
if err := json.Unmarshal(body, &openAIResp); err != nil {
|
if err := json.Unmarshal(body, &openAIResp); err != nil {
|
||||||
return Message{}, fmt.Errorf("failed to unmarshal response: %w", err)
|
return Message{}, fmt.Errorf("%w: failed to parse API response: %v", ErrLLMInvalidResponse, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if openAIResp.Error != nil {
|
if openAIResp.Error != nil {
|
||||||
@@ -99,7 +119,7 @@ func (s *standardLLMClient) Call(ctx context.Context, model ModelConfig, msgs []
|
|||||||
}
|
}
|
||||||
|
|
||||||
if len(openAIResp.Choices) == 0 {
|
if len(openAIResp.Choices) == 0 {
|
||||||
return Message{}, fmt.Errorf("API returned no choices")
|
return Message{}, fmt.Errorf("%w: API returned no choices", ErrLLMInvalidResponse)
|
||||||
}
|
}
|
||||||
|
|
||||||
return openAIResp.Choices[0].Message, nil
|
return openAIResp.Choices[0].Message, nil
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package sidekick
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"os"
|
"os"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -148,3 +149,60 @@ func TestTempFileCleanup(t *testing.T) {
|
|||||||
t.Errorf("leak: before %d, after %d", bc, ac)
|
t.Errorf("leak: before %d, after %d", bc, ac)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type errorLLMClient struct {
|
||||||
|
err error
|
||||||
|
}
|
||||||
|
|
||||||
|
func (e *errorLLMClient) Call(ctx context.Context, model ModelConfig, messages []Message,
|
||||||
|
tools map[string]ToolDefinition, streamCallback, reasoningCallback func(string)) (Message, error) {
|
||||||
|
return Message{}, e.err
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLLMTimeoutError(t *testing.T) {
|
||||||
|
SetLLMClient(&errorLLMClient{err: ErrLLMTimeout})
|
||||||
|
a := NewSidekick(AgentConfig{MaxIterations: 1}, ModelConfig{}, nil)
|
||||||
|
_, err := a.Run(context.Background(), nil, nil)
|
||||||
|
if !errors.Is(err, ErrLLMTimeout) {
|
||||||
|
t.Errorf("expected ErrLLMTimeout, got %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMalformedJSONAction(t *testing.T) {
|
||||||
|
mc := &mockLLMClient{responses: []Message{
|
||||||
|
{Content: `{"action": "TOOL_CALL", "params": { "missing_brace": `},
|
||||||
|
}}
|
||||||
|
SetLLMClient(mc)
|
||||||
|
a := NewSidekick(AgentConfig{MaxIterations: 1}, ModelConfig{}, nil)
|
||||||
|
_, err := a.Run(context.Background(), nil, nil)
|
||||||
|
if err == nil || !strings.Contains(err.Error(), "malformed JSON action") {
|
||||||
|
t.Errorf("expected malformed JSON action error, got %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMissingActionField(t *testing.T) {
|
||||||
|
mc := &mockLLMClient{responses: []Message{
|
||||||
|
{Content: `{"params": {"foo": "bar"}}`},
|
||||||
|
}}
|
||||||
|
SetLLMClient(mc)
|
||||||
|
a := NewSidekick(AgentConfig{MaxIterations: 1}, ModelConfig{}, nil)
|
||||||
|
_, err := a.Run(context.Background(), nil, nil)
|
||||||
|
if err == nil || !strings.Contains(err.Error(), "missing the 'action' field") {
|
||||||
|
t.Errorf("expected missing action field error, got %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestPlainTextResponse(t *testing.T) {
|
||||||
|
mc := &mockLLMClient{responses: []Message{
|
||||||
|
{Content: `This is a plain text response.`},
|
||||||
|
}}
|
||||||
|
SetLLMClient(mc)
|
||||||
|
a := NewSidekick(AgentConfig{MaxIterations: 1}, ModelConfig{}, nil)
|
||||||
|
res, err := a.Run(context.Background(), nil, nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("expected no error, got %v", err)
|
||||||
|
}
|
||||||
|
if res != "This is a plain text response." {
|
||||||
|
t.Errorf("expected plain text response, got %s", res)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user