Improve GetLLMLogs (#237)

* GetLLMLogs is critical for being able to view the actual
inputs/outputs of the LLM
* Don't require the user to specify the LogFile
* Add a playbook to describe how to use it to view the actual LLM
request and response
* Support OpenAI and Anthropic

app/api/config.go

@@ -7,6 +7,7 @@ const (
 	ModelProviderAnthropic ModelProvider = "anthropic"
 	ModelProviderOpenAI    ModelProvider = "openai"
 	ModelProviderDefault   ModelProvider = "openai"
+	ModelProviderUnknown   ModelProvider = "unknown"
 )
 
 type AgentConfig struct {

app/pkg/analyze/crud.go

@@ -2,6 +2,7 @@ package analyze
 
 import (
 	"context"
+	"sort"
 
 	"connectrpc.com/connect"
 	"github.com/jlewi/foyle/app/pkg/logs"
@@ -54,26 +55,35 @@ func (h *CrudHandler) GetTrace(ctx context.Context, request *connect.Request[log
 }
 
 func (h *CrudHandler) GetLLMLogs(ctx context.Context, request *connect.Request[logspb.GetLLMLogsRequest]) (*connect.Response[logspb.GetLLMLogsResponse], error) {
+	log := logs.FromContext(ctx)
 	getReq := request.Msg
 	if getReq.GetTraceId() == "" {
 		return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("No traceID provided"))
 	}
 
-	if getReq.GetLogFile() == "" {
-		return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("No LogFile provided"))
-	}
-
-	log, err := readAnthropicLog(ctx, getReq.GetTraceId(), getReq.GetLogFile())
+	logFiles, err := findLogFiles(ctx, h.cfg.GetLogDir())
 	if err != nil {
-		// Assume its a not found error.
-		return nil, connect.NewError(connect.CodeInternal, errors.Wrapf(err, "Failed to get prompt for trace id %s; logFile: %s", getReq.GetTraceId(), getReq.GetLogFile()))
+		log.Error(err, "Failed to find log files")
+		return nil, connect.NewError(connect.CodeInternal, errors.Wrap(err, "Failed to find log files"))
 	}
 
-	resp := &logspb.GetLLMLogsResponse{}
-	resp.RequestHtml = renderAnthropicRequest(log.Request)
-	resp.ResponseHtml = renderAnthropicResponse(log.Response)
+	// Sort the slice in descending order
+	sort.Slice(logFiles, func(i, j int) bool {
+		return logFiles[i] > logFiles[j]
+	})
+
+	// We loop over all the logFiles until we find it which is not efficient.
+	for _, logFile := range logFiles {
+		resp, err := readLLMLog(ctx, getReq.GetTraceId(), logFile)
+		if err != nil {
+			return nil, connect.NewError(connect.CodeInternal, errors.Wrapf(err, "Failed to get LLM call log for trace id %s; logFile: %s", getReq.GetTraceId(), getReq.GetLogFile()))
+		}
+		if resp != nil {
+			return connect.NewResponse(resp), nil
+		}
+	}
 
-	return connect.NewResponse(resp), nil
+	return nil, connect.NewError(connect.CodeNotFound, errors.Errorf("No log file found for traceID %v", getReq.GetTraceId()))
 }
 
 func (h *CrudHandler) GetBlockLog(ctx context.Context, request *connect.Request[logspb.GetBlockLogRequest]) (*connect.Response[logspb.GetBlockLogResponse], error) {

app/pkg/analyze/logs.go

@@ -7,6 +7,9 @@ import (
 	"os"
 	"strings"
 
+	"github.com/jlewi/foyle/app/pkg/logs/matchers"
+	logspb "github.com/jlewi/foyle/protos/go/foyle/logs"
+
 	"connectrpc.com/connect"
 	"github.com/jlewi/foyle/app/api"
 	"github.com/jlewi/foyle/app/pkg/logs"
@@ -24,61 +27,82 @@ type AnthropicLog struct {
 	Response *anthropic.MessagesResponse
 }
 
-// readAnthropicRequest reads an Anthropic request from a log file
-//
-// N.B. If there are multiple requests as part of the same trace then only the last request will be returned.
-// TODO(jeremy): Ideally we'd join the request with its response and return the one that succeeded. The reason
-// There might be multiple is because context exceeded length; in which case only one request which has been
-// sufficiently shortened will have an actual response.
-func readAnthropicLog(ctx context.Context, traceId string, logFile string) (*AnthropicLog, error) {
+// readLLMLog tries to fetch the raw LLM request/response from the log
+func readLLMLog(ctx context.Context, traceId string, logFile string) (*logspb.GetLLMLogsResponse, error) {
 	log := logs.FromContext(ctx)
 	file, err := os.Open(logFile)
+
 	if err != nil {
 		return nil, connect.NewError(connect.CodeNotFound, errors.Wrapf(err, "Failed to open file %s", logFile))
 	}
 	d := json.NewDecoder(file)
 
-	aLog := &AnthropicLog{
-		TraceID: traceId,
-		LogFile: logFile,
-	}
-	req := &anthropic.MessagesRequest{}
-	resp := &anthropic.MessagesResponse{}
+	resp := &logspb.GetLLMLogsResponse{}
+
+	provider := api.ModelProviderUnknown
 	for {
 		entry := &api.LogEntry{}
 		if err := d.Decode(entry); err != nil {
 			if err == io.EOF {
-				return aLog, nil
+				return nil, nil
 			}
 			log.Error(err, "Failed to decode log entry")
 		}
 		if entry.TraceID() != traceId {
 			continue
 		}
-		if !strings.HasSuffix(entry.Function(), "anthropic.(*Completer).Complete") {
+		isMatch := false
+		if strings.HasSuffix(entry.Function(), "anthropic.(*Completer).Complete") {
+			provider = api.ModelProviderAnthropic
+			isMatch = true
+		}
+
+		if matchers.IsOAIComplete(entry.Function()) {
+			provider = api.ModelProviderOpenAI
+			isMatch = true
+		}
+
+		if strings.HasSuffix(entry.Function(), "anthropic.(*Completer).Complete") {
+			provider = api.ModelProviderAnthropic
+			isMatch = true
+		}
+
+		// If tis not a matching request ignore it.
+		if !isMatch {
 			continue
 		}
+		if reqBytes := entry.Request(); reqBytes != nil {
+			resp.RequestJson = string(reqBytes)
+		}
 
-		reqBytes := entry.Request()
-		if reqBytes != nil {
-			if err := json.Unmarshal(reqBytes, req); err != nil {
-				// TODO(jeremy): Should we include the error in the response?
-				log.Error(err, "Failed to unmarshal request")
-			} else {
-				aLog.Request = req
-				req = &anthropic.MessagesRequest{}
-			}
+		if resBytes := entry.Response(); resBytes != nil {
+			resp.ResponseJson = string(resBytes)
 		}
 
-		respBytes := entry.Response()
-		if respBytes != nil {
-			if err := json.Unmarshal(respBytes, resp); err != nil {
-				// TODO(jeremy): Should we include the error in the response?
-				log.Error(err, "Failed to unmarshal response")
-			} else {
-				aLog.Response = resp
-				resp = &anthropic.MessagesResponse{}
-			}
+		// Since we have read the request and response less
+		// This isn't a great implementation because we will end up reading all the logs if for some reason
+		// The logs don't have the entries.
+		if resp.RequestJson != "" && resp.ResponseJson != "" {
+			break
+		}
+	}
+
+	if provider == api.ModelProviderAnthropic && resp.ResponseJson != "" {
+		html, err := renderAnthropicRequestJson(resp.RequestJson)
+		if err != nil {
+			log.Error(err, "Failed to render request")
+
+		} else {
+			resp.RequestHtml = html
+		}
+
+		htmlResp, err := renderAnthropicResponseJson(resp.ResponseJson)
+		if err != nil {
+			log.Error(err, "Failed to render response")
+
+		} else {
+			resp.ResponseHtml = htmlResp
 		}
 	}
+	return resp, nil
 }

app/pkg/analyze/logs_test.go

@@ -2,9 +2,12 @@ package analyze
 
 import (
 	"context"
+	"encoding/json"
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/liushuangls/go-anthropic/v2"
 )
 
 func TestReadAnthropicLog(t *testing.T) {
@@ -28,22 +31,32 @@ func TestReadAnthropicLog(t *testing.T) {
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			fullPath := filepath.Join(cwd, "test_data", c.logFile)
-			result, err := readAnthropicLog(context.Background(), c.traceId, fullPath)
+			result, err := readLLMLog(context.Background(), c.traceId, fullPath)
 			if err != nil {
 				t.Errorf("Failed to read Anthropic request: %v", err)
 			}
 			if result == nil {
 				t.Fatalf("Request should not be nil")
 			}
-			if result.Request == nil {
+			if result.RequestHtml == "" {
+				t.Errorf("Request should not be nil")
+			}
+			if result.ResponseHtml == "" {
+				t.Errorf("Response should not be nil")
+			}
+			if result.RequestJson == "" {
 				t.Errorf("Request should not be nil")
 			}
-			if result.Response == nil {
+			if result.ResponseJson == "" {
 				t.Errorf("Response should not be nil")
-			} else {
-				if result.Response.Model == "" {
-					t.Errorf("Model should not be empty")
-				}
+			}
+			resp := &anthropic.MessagesResponse{}
+			if err := json.Unmarshal([]byte(result.ResponseJson), resp); err != nil {
+				t.Fatalf("Failed to unmarshal response: %v", err)
+			}
+
+			if resp.Model == "" {
+				t.Errorf("Model should be set")
 			}
 		})
 	}

app/pkg/analyze/render.go

@@ -3,6 +3,7 @@ package analyze
 import (
 	"bytes"
 	_ "embed"
+	"encoding/json"
 	"fmt"
 	"html/template"
 
@@ -34,6 +35,24 @@ type Message struct {
 	Content template.HTML
 }
 
+func renderAnthropicRequestJson(jsonValue string) (string, error) {
+	req := &anthropic.MessagesRequest{}
+	if err := json.Unmarshal([]byte(jsonValue), req); err != nil {
+		return "", nil
+	}
+
+	return renderAnthropicRequest(req), nil
+}
+
+func renderAnthropicResponseJson(jsonValue string) (string, error) {
+	res := &anthropic.MessagesResponse{}
+	if err := json.Unmarshal([]byte(jsonValue), res); err != nil {
+		return "", nil
+	}
+
+	return renderAnthropicResponse(res), nil
+}
+
 // renderAnthropicRequest returns a string containing the HTML representation of the request
 func renderAnthropicRequest(request *anthropic.MessagesRequest) string {
 	log := zapr.NewLogger(zap.L())

app/pkg/analyze/render_test.go

@@ -1,8 +1,10 @@
 package analyze
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/liushuangls/go-anthropic/v2"
@@ -12,35 +14,38 @@ import (
 
 func TestRenderAnthropicRequest(t *testing.T) {
 	type testCase struct {
-		name    string
-		request *anthropic.MessagesRequest
+		name  string
+		fname string
 	}
 
 	tests := []testCase{
 		{
-			name: "basic",
-			request: &anthropic.MessagesRequest{
-				Model:       "test",
-				MaxTokens:   10,
-				Temperature: proto.Float32(0.5),
-				System:      "This is the system message",
-				Messages: []anthropic.Message{
-					{
-						Role: "User",
-						Content: []anthropic.MessageContent{
-							{
-								Text: proto.String("# md heading\n * item 1 \n * item 2"),
-							},
-						},
-					},
-				},
-			},
+			name:  "basic",
+			fname: "anthropic_request.json",
 		},
 	}
 
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Failed to get current working directory: %v", err)
+	}
+
+	testDataDir := filepath.Join(cwd, "test_data")
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			result := renderAnthropicRequest(test.request)
+
+			fname := filepath.Join(testDataDir, test.fname)
+			data, err := os.ReadFile(fname)
+			if err != nil {
+				t.Fatalf("Failed to read file %s: %v", fname, err)
+			}
+
+			req := &anthropic.MessagesRequest{}
+			if err := json.Unmarshal(data, req); err != nil {
+				t.Fatalf("Failed to unmarshal request: %v", err)
+			}
+
+			result := renderAnthropicRequest(req)
 			if result == "" {
 				t.Errorf("Request should not be empty")
 			}

app/pkg/logs/matchers/names.go

@@ -6,10 +6,17 @@ package matchers
 import "strings"
 
 const (
+	OAIComplete    = "github.com/jlewi/foyle/app/pkg/oai.(*Completer).Complete"
 	LogEvents      = "github.com/jlewi/foyle/app/pkg/agent.(*Agent).LogEvents"
 	StreamGenerate = "github.com/jlewi/foyle/app/pkg/agent.(*Agent).StreamGenerate"
 )
 
+type Matcher func(name string) bool
+
+func IsOAIComplete(name string) bool {
+	return strings.HasPrefix(name, OAIComplete)
+}
+
 func IsLogEvent(fname string) bool {
 	// We need to use HasPrefix because the logging statement is nested inside an anonymous function so there
 	// will be a suffix like "func1"