From 8b790c35e0b78809cf32fd64eb25cc12bba48a0d Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Sun, 17 Aug 2025 21:52:17 +0300 Subject: review changes --- internal/appconfig/config.go | 78 +++++++++++++++++++++++------------------- internal/hexaicli/run.go | 1 - internal/llm/provider.go | 81 ++++++++++++++++++++++---------------------- 3 files changed, 83 insertions(+), 77 deletions(-) diff --git a/internal/appconfig/config.go b/internal/appconfig/config.go index 6b8df4a..4b42ffc 100644 --- a/internal/appconfig/config.go +++ b/internal/appconfig/config.go @@ -1,32 +1,34 @@ // Summary: Application configuration model and loader; reads ~/.config/hexai/config.json and merges defaults. -// Not yet reviewed by a human package appconfig import ( - "encoding/json" - "log" - "os" - "path/filepath" - "strings" + "encoding/json" + "fmt" + "log" + "os" + "path/filepath" + "slices" + "strings" ) // App holds user-configurable settings read from ~/.config/hexai/config.json. type App struct { - MaxTokens int `json:"max_tokens"` - ContextMode string `json:"context_mode"` - ContextWindowLines int `json:"context_window_lines"` - MaxContextTokens int `json:"max_context_tokens"` - LogPreviewLimit int `json:"log_preview_limit"` - NoDiskIO bool `json:"no_disk_io"` - TriggerCharacters []string `json:"trigger_characters"` - Provider string `json:"provider"` - // Provider-specific options - OpenAIBaseURL string `json:"openai_base_url"` - OpenAIModel string `json:"openai_model"` - OllamaBaseURL string `json:"ollama_base_url"` - OllamaModel string `json:"ollama_model"` - CopilotBaseURL string `json:"copilot_base_url"` - CopilotModel string `json:"copilot_model"` + MaxTokens int `json:"max_tokens"` + ContextMode string `json:"context_mode"` + ContextWindowLines int `json:"context_window_lines"` + MaxContextTokens int `json:"max_context_tokens"` + LogPreviewLimit int `json:"log_preview_limit"` + NoDiskIO bool `json:"no_disk_io"` + TriggerCharacters []string `json:"trigger_characters"` + Provider string `json:"provider"` + + // Provider-specific options + OpenAIBaseURL string `json:"openai_base_url"` + OpenAIModel string `json:"openai_model"` + OllamaBaseURL string `json:"ollama_base_url"` + OllamaModel string `json:"ollama_model"` + CopilotBaseURL string `json:"copilot_base_url"` + CopilotModel string `json:"copilot_model"` } // Load reads configuration from a file and merges with defaults. @@ -40,23 +42,14 @@ func Load(logger *log.Logger) App { LogPreviewLimit: 100, NoDiskIO: true, } - if logger == nil { return cfg // Return defaults if no logger is provided (e.g. in tests) } - var configPath string - if xdgConfigHome := os.Getenv("XDG_CONFIG_HOME"); xdgConfigHome != "" { - configPath = filepath.Join(xdgConfigHome, "hexai", "config.json") - } else { - home, err := os.UserHomeDir() - if err != nil { - if logger != nil { - logger.Printf("cannot find user home directory: %v", err) - } - return cfg // Return defaults if home dir is not found - } - configPath = filepath.Join(home, ".config", "hexai", "config.json") + configPath, err := getConfigPath() + if err != nil { + logger.Printf("%v", err) + return cfg } f, err := os.Open(configPath) @@ -95,11 +88,12 @@ func Load(logger *log.Logger) App { } cfg.NoDiskIO = fileCfg.NoDiskIO if len(fileCfg.TriggerCharacters) > 0 { - cfg.TriggerCharacters = append([]string{}, fileCfg.TriggerCharacters...) + cfg.TriggerCharacters = slices.Clone(fileCfg.TriggerCharacters) } if strings.TrimSpace(fileCfg.Provider) != "" { cfg.Provider = fileCfg.Provider } + // Provider-specific options if strings.TrimSpace(fileCfg.OpenAIBaseURL) != "" { cfg.OpenAIBaseURL = fileCfg.OpenAIBaseURL @@ -121,3 +115,17 @@ func Load(logger *log.Logger) App { } return cfg } + +func getConfigPath() (string, error) { + var configPath string + if xdgConfigHome := os.Getenv("XDG_CONFIG_HOME"); xdgConfigHome != "" { + configPath = filepath.Join(xdgConfigHome, "hexai", "config.json") + } else { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("cannot find user home directory: %v", err) + } + configPath = filepath.Join(home, ".config", "hexai", "config.json") + } + return configPath, nil +} diff --git a/internal/hexaicli/run.go b/internal/hexaicli/run.go index 018b5d2..11e64b3 100644 --- a/internal/hexaicli/run.go +++ b/internal/hexaicli/run.go @@ -1,6 +1,5 @@ // Summary: Hexai CLI runner; reads input, creates an LLM client, builds messages, // streams or collects the model output, and prints a short summary to stderr. -// Not yet reviewed by a human package hexaicli import ( diff --git a/internal/llm/provider.go b/internal/llm/provider.go index 09e97e6..c605081 100644 --- a/internal/llm/provider.go +++ b/internal/llm/provider.go @@ -1,5 +1,4 @@ // Summary: LLM provider interfaces, request options, configuration, and factory to build a client from config. -// Not yet reviewed by a human package llm import ( @@ -17,22 +16,22 @@ type Message struct { // Client is a minimal LLM provider interface. // Future providers (Ollama, etc.) should implement this. type Client interface { - // Chat sends chat messages and returns the assistant text. - Chat(ctx context.Context, messages []Message, opts ...RequestOption) (string, error) - // Name returns the provider's short name (e.g., "openai", "ollama"). - Name() string - // DefaultModel returns the configured default model name. - DefaultModel() string + // Chat sends chat messages and returns the assistant text. + Chat(ctx context.Context, messages []Message, opts ...RequestOption) (string, error) + // Name returns the provider's short name (e.g., "openai", "ollama"). + Name() string + // DefaultModel returns the configured default model name. + DefaultModel() string } // Streamer is an optional interface that providers may implement to support // token-by-token streaming responses. Callers can type-assert to Streamer and // fall back to Client.Chat when not implemented. type Streamer interface { - // ChatStream sends chat messages and invokes onDelta with incremental text - // chunks as they are produced by the model. Implementations should call - // onDelta with empty strings sparingly (prefer only non-empty chunks). - ChatStream(ctx context.Context, messages []Message, onDelta func(string), opts ...RequestOption) error + // ChatStream sends chat messages and invokes onDelta with incremental text + // chunks as they are produced by the model. Implementations should call + // onDelta with empty strings sparingly (prefer only non-empty chunks). + ChatStream(ctx context.Context, messages []Message, onDelta func(string), opts ...RequestOption) error } // Options for a request. Providers may ignore unsupported fields. @@ -55,40 +54,40 @@ func WithStop(stop ...string) RequestOption { // Config defines provider configuration read from the Hexai config file. type Config struct { - Provider string - // OpenAI options - OpenAIBaseURL string - OpenAIModel string - // Ollama options - OllamaBaseURL string - OllamaModel string - // Copilot options - CopilotBaseURL string - CopilotModel string + Provider string + // OpenAI options + OpenAIBaseURL string + OpenAIModel string + // Ollama options + OllamaBaseURL string + OllamaModel string + // Copilot options + CopilotBaseURL string + CopilotModel string } // NewFromConfig creates an LLM client using only the supplied configuration. // The OpenAI API key is supplied separately and may be read from the environment // by the caller; other environment-based configuration is not used. func NewFromConfig(cfg Config, openAIAPIKey, copilotAPIKey string) (Client, error) { - p := strings.ToLower(strings.TrimSpace(cfg.Provider)) - if p == "" { - p = "openai" - } - switch p { - case "openai": - if strings.TrimSpace(openAIAPIKey) == "" { - return nil, errors.New("missing OPENAI_API_KEY for provider openai") - } - return newOpenAI(cfg.OpenAIBaseURL, cfg.OpenAIModel, openAIAPIKey), nil - case "ollama": - return newOllama(cfg.OllamaBaseURL, cfg.OllamaModel), nil - case "copilot": - if strings.TrimSpace(copilotAPIKey) == "" { - return nil, errors.New("missing COPILOT_API_KEY for provider copilot") - } - return newCopilot(cfg.CopilotBaseURL, cfg.CopilotModel, copilotAPIKey), nil - default: - return nil, errors.New("unknown LLM provider: " + p) - } + p := strings.ToLower(strings.TrimSpace(cfg.Provider)) + if p == "" { + p = "openai" + } + switch p { + case "openai": + if strings.TrimSpace(openAIAPIKey) == "" { + return nil, errors.New("missing OPENAI_API_KEY for provider openai") + } + return newOpenAI(cfg.OpenAIBaseURL, cfg.OpenAIModel, openAIAPIKey), nil + case "ollama": + return newOllama(cfg.OllamaBaseURL, cfg.OllamaModel), nil + case "copilot": + if strings.TrimSpace(copilotAPIKey) == "" { + return nil, errors.New("missing COPILOT_API_KEY for provider copilot") + } + return newCopilot(cfg.CopilotBaseURL, cfg.CopilotModel, copilotAPIKey), nil + default: + return nil, errors.New("unknown LLM provider: " + p) + } } -- cgit v1.2.3