From e7b06341c3b426dfcccaf5f583ea437c19704c87 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Mon, 15 Sep 2025 21:29:46 +0200 Subject: [PATCH] Enhance command parsing in ParseLlamaCommand --- pkg/backends/llamacpp/parser.go | 152 ++++++++++++++++--- pkg/backends/llamacpp/parser_test.go | 213 ++++++++++++++++++++++++++- 2 files changed, 342 insertions(+), 23 deletions(-) diff --git a/pkg/backends/llamacpp/parser.go b/pkg/backends/llamacpp/parser.go index c8382bd..2dd7f65 100644 --- a/pkg/backends/llamacpp/parser.go +++ b/pkg/backends/llamacpp/parser.go @@ -3,32 +3,31 @@ package llamacpp import ( "encoding/json" "fmt" + "path/filepath" + "regexp" "strconv" "strings" ) // ParseLlamaCommand parses a llama-server command string into LlamaServerOptions +// Supports multiple formats: +// 1. Full command: "llama-server --model file.gguf" +// 2. Full path: "/usr/local/bin/llama-server --model file.gguf" +// 3. Args only: "--model file.gguf --gpu-layers 32" +// 4. Multiline commands with backslashes func ParseLlamaCommand(command string) (*LlamaServerOptions, error) { - // 1. Validate command starts with llama-server - trimmed := strings.TrimSpace(command) + // 1. Normalize the command - handle multiline with backslashes + trimmed := normalizeMultilineCommand(command) if trimmed == "" { return nil, fmt.Errorf("command cannot be empty") } - // Check if command starts with llama-server (case-insensitive) - lowerCommand := strings.ToLower(trimmed) - if !strings.HasPrefix(lowerCommand, "llama-server") { - return nil, fmt.Errorf("command must start with 'llama-server'") + // 2. Extract arguments from command + args, err := extractArgumentsFromCommand(trimmed) + if err != nil { + return nil, err } - // 2. Extract arguments (everything after llama-server) - parts := strings.Fields(trimmed) - if len(parts) < 1 { - return nil, fmt.Errorf("invalid command format") - } - - args := parts[1:] // Skip binary name - // 3. Parse arguments into map options := make(map[string]any) i := 0 @@ -79,7 +78,8 @@ func ParseLlamaCommand(command string) (*LlamaServerOptions, error) { flagName := strings.ReplaceAll(flag, "-", "_") // Check if next arg is a value (not a flag) - if i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") { + // Special case: allow negative numbers as values + if i+1 < len(args) && !isFlag(args[i+1]) { value := parseValue(args[i+1]) // Handle array flags by checking if flag already exists @@ -129,7 +129,7 @@ func parseValue(value string) any { return false } - // Try to parse as integer + // Try to parse as integer (handle negative numbers) if intVal, err := strconv.Atoi(value); err == nil { return intVal } @@ -139,6 +139,122 @@ func parseValue(value string) any { return floatVal } - // Default to string - return value + // Default to string (remove quotes if present) + return strings.Trim(value, `""`) } + +// normalizeMultilineCommand handles multiline commands with backslashes +func normalizeMultilineCommand(command string) string { + // Handle escaped newlines (backslash followed by newline) + re := regexp.MustCompile(`\\\s*\n\s*`) + normalized := re.ReplaceAllString(command, " ") + + // Clean up extra whitespace + re = regexp.MustCompile(`\s+`) + normalized = re.ReplaceAllString(normalized, " ") + + return strings.TrimSpace(normalized) +} + +// extractArgumentsFromCommand extracts arguments from various command formats +func extractArgumentsFromCommand(command string) ([]string, error) { + // Split command into tokens respecting quotes + tokens, err := splitCommandTokens(command) + if err != nil { + return nil, err + } + + if len(tokens) == 0 { + return nil, fmt.Errorf("no command tokens found") + } + + // Check if first token looks like an executable + firstToken := tokens[0] + + // Case 1: Full path to executable (contains path separator or ends with llama-server) + if strings.Contains(firstToken, string(filepath.Separator)) || + strings.HasSuffix(filepath.Base(firstToken), "llama-server") { + return tokens[1:], nil // Return everything except the executable + } + + // Case 2: Just "llama-server" command + if strings.ToLower(firstToken) == "llama-server" { + return tokens[1:], nil // Return everything except the command + } + + // Case 3: Arguments only (starts with a flag) + if strings.HasPrefix(firstToken, "-") { + return tokens, nil // Return all tokens as arguments + } + + // Case 4: Unknown format - might be a different executable name + // Be permissive and assume it's the executable + return tokens[1:], nil +} + +// splitCommandTokens splits a command string into tokens, respecting quotes +func splitCommandTokens(command string) ([]string, error) { + var tokens []string + var current strings.Builder + inQuotes := false + quoteChar := byte(0) + escaped := false + + for i := 0; i < len(command); i++ { + c := command[i] + + if escaped { + current.WriteByte(c) + escaped = false + continue + } + + if c == '\\' { + escaped = true + current.WriteByte(c) + continue + } + + if !inQuotes && (c == '"' || c == '\'') { + inQuotes = true + quoteChar = c + current.WriteByte(c) + } else if inQuotes && c == quoteChar { + inQuotes = false + quoteChar = 0 + current.WriteByte(c) + } else if !inQuotes && (c == ' ' || c == '\t') { + if current.Len() > 0 { + tokens = append(tokens, current.String()) + current.Reset() + } + } else { + current.WriteByte(c) + } + } + + if inQuotes { + return nil, fmt.Errorf("unterminated quoted string") + } + + if current.Len() > 0 { + tokens = append(tokens, current.String()) + } + + return tokens, nil +} + +// isFlag determines if a string is a command line flag or a value +// Handles the special case where negative numbers should be treated as values, not flags +func isFlag(arg string) bool { + if !strings.HasPrefix(arg, "-") { + return false + } + + // Special case: if it's a negative number, treat it as a value + if _, err := strconv.ParseFloat(arg, 64); err == nil { + return false + } + + return true +} \ No newline at end of file diff --git a/pkg/backends/llamacpp/parser_test.go b/pkg/backends/llamacpp/parser_test.go index bfdaafd..a217840 100644 --- a/pkg/backends/llamacpp/parser_test.go +++ b/pkg/backends/llamacpp/parser_test.go @@ -40,16 +40,42 @@ func TestParseLlamaCommand(t *testing.T) { command: "", expectErr: true, }, - { - name: "invalid command without llama-server", - command: "other-command --model /path/to/model.gguf", - expectErr: true, - }, { name: "case insensitive command", command: "LLAMA-SERVER --model /path/to/model.gguf", expectErr: false, }, + // New test cases for improved functionality + { + name: "args only without llama-server", + command: "--model /path/to/model.gguf --gpu-layers 32", + expectErr: false, + }, + { + name: "full path to executable", + command: "/usr/local/bin/llama-server --model /path/to/model.gguf", + expectErr: false, + }, + { + name: "negative number handling", + command: "llama-server --gpu-layers -1 --model test.gguf", + expectErr: false, + }, + { + name: "multiline command with backslashes", + command: "llama-server --model /path/to/model.gguf \\\n --ctx-size 4096 \\\n --batch-size 512", + expectErr: false, + }, + { + name: "quoted string with special characters", + command: `llama-server --model test.gguf --chat-template "{% for message in messages %}{{ message.role }}: {{ message.content }}\n{% endfor %}"`, + expectErr: false, + }, + { + name: "unterminated quoted string", + command: `llama-server --model test.gguf --chat-template "unterminated quote`, + expectErr: true, + }, } for _, tt := range tests { @@ -167,3 +193,180 @@ func TestParseLlamaCommandTypeConversion(t *testing.T) { t.Errorf("expected no_mmap to be true, got %v", result.NoMmap) } } + +func TestParseLlamaCommandArgsOnly(t *testing.T) { + // Test parsing arguments without llama-server command + command := "--model /path/to/model.gguf --gpu-layers 32 --ctx-size 4096" + result, err := ParseLlamaCommand(command) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if result.Model != "/path/to/model.gguf" { + t.Errorf("expected model '/path/to/model.gguf', got '%s'", result.Model) + } + + if result.GPULayers != 32 { + t.Errorf("expected gpu_layers 32, got %d", result.GPULayers) + } + + if result.CtxSize != 4096 { + t.Errorf("expected ctx_size 4096, got %d", result.CtxSize) + } +} + +func TestParseLlamaCommandFullPath(t *testing.T) { + // Test full path to executable + command := "/usr/local/bin/llama-server --model test.gguf --gpu-layers 16" + result, err := ParseLlamaCommand(command) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if result.Model != "test.gguf" { + t.Errorf("expected model 'test.gguf', got '%s'", result.Model) + } + + if result.GPULayers != 16 { + t.Errorf("expected gpu_layers 16, got %d", result.GPULayers) + } +} + +func TestParseLlamaCommandNegativeNumbers(t *testing.T) { + // Test negative number parsing + command := "llama-server --model test.gguf --gpu-layers -1 --seed -12345" + result, err := ParseLlamaCommand(command) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if result.GPULayers != -1 { + t.Errorf("expected gpu_layers -1, got %d", result.GPULayers) + } + + if result.Seed != -12345 { + t.Errorf("expected seed -12345, got %d", result.Seed) + } +} + +func TestParseLlamaCommandMultiline(t *testing.T) { + // Test multiline command with backslashes + command := `llama-server --model /path/to/model.gguf \ + --ctx-size 4096 \ + --batch-size 512 \ + --gpu-layers 32` + + result, err := ParseLlamaCommand(command) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if result.Model != "/path/to/model.gguf" { + t.Errorf("expected model '/path/to/model.gguf', got '%s'", result.Model) + } + + if result.CtxSize != 4096 { + t.Errorf("expected ctx_size 4096, got %d", result.CtxSize) + } + + if result.BatchSize != 512 { + t.Errorf("expected batch_size 512, got %d", result.BatchSize) + } + + if result.GPULayers != 32 { + t.Errorf("expected gpu_layers 32, got %d", result.GPULayers) + } +} + +func TestParseLlamaCommandQuotedStrings(t *testing.T) { + // Test quoted strings with special characters + command := `llama-server --model test.gguf --api-key "sk-1234567890abcdef" --chat-template "User: {user}\nAssistant: "` + result, err := ParseLlamaCommand(command) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if result.Model != "test.gguf" { + t.Errorf("expected model 'test.gguf', got '%s'", result.Model) + } + + if result.APIKey != "sk-1234567890abcdef" { + t.Errorf("expected api_key 'sk-1234567890abcdef', got '%s'", result.APIKey) + } + + expectedTemplate := "User: {user}\\nAssistant: " + if result.ChatTemplate != expectedTemplate { + t.Errorf("expected chat_template '%s', got '%s'", expectedTemplate, result.ChatTemplate) + } +} + +func TestParseLlamaCommandUnslothExample(t *testing.T) { + // Test with realistic unsloth-style command + command := `llama-server --model /path/to/model.gguf \ + --ctx-size 4096 \ + --batch-size 512 \ + --gpu-layers -1 \ + --temp 0.7 \ + --repeat-penalty 1.1 \ + --top-k 40 \ + --top-p 0.95 \ + --host 0.0.0.0 \ + --port 8000 \ + --api-key "sk-1234567890abcdef"` + + result, err := ParseLlamaCommand(command) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify key fields + if result.Model != "/path/to/model.gguf" { + t.Errorf("expected model '/path/to/model.gguf', got '%s'", result.Model) + } + + if result.CtxSize != 4096 { + t.Errorf("expected ctx_size 4096, got %d", result.CtxSize) + } + + if result.BatchSize != 512 { + t.Errorf("expected batch_size 512, got %d", result.BatchSize) + } + + if result.GPULayers != -1 { + t.Errorf("expected gpu_layers -1, got %d", result.GPULayers) + } + + if result.Temperature != 0.7 { + t.Errorf("expected temperature 0.7, got %f", result.Temperature) + } + + if result.RepeatPenalty != 1.1 { + t.Errorf("expected repeat_penalty 1.1, got %f", result.RepeatPenalty) + } + + if result.TopK != 40 { + t.Errorf("expected top_k 40, got %d", result.TopK) + } + + if result.TopP != 0.95 { + t.Errorf("expected top_p 0.95, got %f", result.TopP) + } + + if result.Host != "0.0.0.0" { + t.Errorf("expected host '0.0.0.0', got '%s'", result.Host) + } + + if result.Port != 8000 { + t.Errorf("expected port 8000, got %d", result.Port) + } + + if result.APIKey != "sk-1234567890abcdef" { + t.Errorf("expected api_key 'sk-1234567890abcdef', got '%s'", result.APIKey) + } +} \ No newline at end of file