Skip to content

Commit 978f0ab

Browse files
committed
refactor: Improve logging setup and configuration loading process, enhancing clarity and providing fallback mechanisms for config file retrieval
1 parent d5c7868 commit 978f0ab

File tree

4 files changed

+109
-82
lines changed

4 files changed

+109
-82
lines changed

cmd/llmscript/main.go

+21-26
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ func main() {
3939
}
4040
flag.Parse()
4141

42+
// Set up logging first
43+
if *verbose {
44+
log.SetLevel(log.DebugLevel)
45+
} else {
46+
log.SetLevel(log.InfoLevel)
47+
}
48+
4249
if *writeConfig {
4350
cfg := config.DefaultConfig()
4451
if err := config.WriteConfig(cfg); err != nil {
@@ -61,6 +68,7 @@ func main() {
6168
// Override config with command line flags
6269
if *llmProvider != "" {
6370
cfg.LLM.Provider = *llmProvider
71+
log.Debug("Provider overridden by command line flag: %s", *llmProvider)
6472
}
6573
if *llmModel != "" {
6674
switch cfg.LLM.Provider {
@@ -85,13 +93,6 @@ func main() {
8593
cfg.ExtraPrompt = *extraPrompt
8694
}
8795

88-
// Set up logging
89-
if *verbose {
90-
log.SetLevel(log.DebugLevel)
91-
} else {
92-
log.SetLevel(log.InfoLevel)
93-
}
94-
9596
if len(flag.Args()) == 0 {
9697
flag.Usage()
9798
os.Exit(1)
@@ -111,25 +112,19 @@ func runScript(cfg *config.Config, scriptFile string) error {
111112
}
112113

113114
log.Info("Creating LLM provider: %s", cfg.LLM.Provider)
114-
provider, err := llm.NewProvider(cfg.LLM.Provider, struct {
115-
Provider string `yaml:"provider"`
116-
Ollama struct {
117-
Model string `yaml:"model"`
118-
Host string `yaml:"host"`
119-
} `yaml:"ollama"`
120-
Claude struct {
121-
APIKey string `yaml:"api_key"`
122-
Model string `yaml:"model"`
123-
} `yaml:"claude"`
124-
OpenAI struct {
125-
APIKey string `yaml:"api_key"`
126-
Model string `yaml:"model"`
127-
} `yaml:"openai"`
128-
}{
129-
Provider: cfg.LLM.Provider,
130-
Ollama: cfg.LLM.Ollama,
131-
Claude: cfg.LLM.Claude,
132-
OpenAI: cfg.LLM.OpenAI,
115+
provider, err := llm.NewProvider(cfg.LLM.Provider, map[string]interface{}{
116+
"ollama": map[string]interface{}{
117+
"model": cfg.LLM.Ollama.Model,
118+
"host": cfg.LLM.Ollama.Host,
119+
},
120+
"claude": map[string]interface{}{
121+
"api_key": cfg.LLM.Claude.APIKey,
122+
"model": cfg.LLM.Claude.Model,
123+
},
124+
"openai": map[string]interface{}{
125+
"api_key": cfg.LLM.OpenAI.APIKey,
126+
"model": cfg.LLM.OpenAI.Model,
127+
},
133128
})
134129
if err != nil {
135130
return fmt.Errorf("failed to create LLM provider: %w", err)

internal/config/config.go

+32-5
Original file line numberDiff line numberDiff line change
@@ -88,33 +88,58 @@ func interpolateEnvVars(data []byte) []byte {
8888
func LoadConfig() (*Config, error) {
8989
config := DefaultConfig()
9090

91+
// Try XDG_CONFIG_HOME first
9192
configDir := os.Getenv("XDG_CONFIG_HOME")
9293
if configDir == "" {
93-
var err error
94-
configDir, err = os.UserConfigDir()
94+
// If XDG_CONFIG_HOME is not set, try ~/.config
95+
homeDir, err := os.UserHomeDir()
9596
if err != nil {
96-
return nil, fmt.Errorf("failed to get config dir: %w", err)
97+
return nil, fmt.Errorf("failed to get home directory: %w", err)
9798
}
99+
configDir = filepath.Join(homeDir, ".config")
98100
}
99101

100102
configPath := filepath.Join(configDir, "llmscript", "config.yaml")
103+
fmt.Printf("Looking for config file at: %s\n", configPath)
101104
data, err := os.ReadFile(configPath)
102105
if err != nil {
103106
if os.IsNotExist(err) {
104-
return config, nil
107+
// If not found in ~/.config, try UserConfigDir() as fallback
108+
configDir, err = os.UserConfigDir()
109+
if err != nil {
110+
return nil, fmt.Errorf("failed to get config dir: %w", err)
111+
}
112+
configPath = filepath.Join(configDir, "llmscript", "config.yaml")
113+
fmt.Printf("Looking for config file at fallback location: %s\n", configPath)
114+
data, err = os.ReadFile(configPath)
115+
if err != nil {
116+
if os.IsNotExist(err) {
117+
fmt.Printf("Config file not found, using defaults\n")
118+
return config, nil
119+
}
120+
return nil, fmt.Errorf("failed to read config file: %w", err)
121+
}
122+
} else {
123+
return nil, fmt.Errorf("failed to read config file: %w", err)
105124
}
106-
return nil, fmt.Errorf("failed to read config file: %w", err)
107125
}
108126

127+
fmt.Printf("Found config file: %s\n", configPath)
128+
fmt.Printf("Read config file: %s\n", string(data))
129+
109130
// Interpolate environment variables before unmarshaling
110131
data = interpolateEnvVars(data)
111132

133+
fmt.Printf("After env var interpolation: %s\n", string(data))
134+
112135
// Create a temporary config to unmarshal into
113136
var loadedConfig Config
114137
if err := yaml.Unmarshal(data, &loadedConfig); err != nil {
115138
return nil, fmt.Errorf("failed to parse config file: %w", err)
116139
}
117140

141+
fmt.Printf("Loaded config from file: provider=%s\n", loadedConfig.LLM.Provider)
142+
118143
// Merge the loaded config with the default config
119144
config.LLM.Provider = loadedConfig.LLM.Provider
120145
config.LLM.Ollama.Model = loadedConfig.LLM.Ollama.Model
@@ -124,6 +149,8 @@ func LoadConfig() (*Config, error) {
124149
config.LLM.OpenAI.APIKey = loadedConfig.LLM.OpenAI.APIKey
125150
config.LLM.OpenAI.Model = loadedConfig.LLM.OpenAI.Model
126151

152+
fmt.Printf("Merged config: provider=%s\n", config.LLM.Provider)
153+
127154
// Only update numeric values if they are explicitly set in the YAML
128155
if loadedConfig.MaxFixes != 0 {
129156
config.MaxFixes = loadedConfig.MaxFixes

internal/llm/claude.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,9 @@ func (p *ClaudeProvider) generate(ctx context.Context, prompt string) (string, e
117117
url := "https://api.anthropic.com/v1/messages"
118118

119119
reqBody := map[string]interface{}{
120-
"model": p.config.Model,
121-
"messages": []map[string]string{{"role": "user", "content": prompt}},
120+
"model": p.config.Model,
121+
"messages": []map[string]string{{"role": "user", "content": prompt}},
122+
"max_tokens": 4096,
122123
}
123124

124125
jsonData, err := json.Marshal(reqBody)

internal/llm/provider.go

+53-49
Original file line numberDiff line numberDiff line change
@@ -71,27 +71,16 @@ func NewProvider(name string, config interface{}) (Provider, error) {
7171

7272
// If config is provided, try to extract values
7373
if config != nil {
74-
cfg, ok := config.(struct {
75-
Provider string `yaml:"provider"`
76-
Ollama struct {
77-
Model string `yaml:"model"`
78-
Host string `yaml:"host"`
79-
} `yaml:"ollama"`
80-
Claude struct {
81-
APIKey string `yaml:"api_key"`
82-
Model string `yaml:"model"`
83-
} `yaml:"claude"`
84-
OpenAI struct {
85-
APIKey string `yaml:"api_key"`
86-
Model string `yaml:"model"`
87-
} `yaml:"openai"`
88-
})
89-
if ok {
90-
if cfg.Ollama.Model != "" {
91-
ollamaConfig.Model = cfg.Ollama.Model
92-
}
93-
if cfg.Ollama.Host != "" {
94-
ollamaConfig.Host = cfg.Ollama.Host
74+
// Try to convert the config to a map[string]interface{} first
75+
if cfgMap, ok := config.(map[string]interface{}); ok {
76+
// Handle Ollama config
77+
if ollamaCfg, ok := cfgMap["ollama"].(map[string]interface{}); ok {
78+
if model, ok := ollamaCfg["model"].(string); ok && model != "" {
79+
ollamaConfig.Model = model
80+
}
81+
if host, ok := ollamaCfg["host"].(string); ok && host != "" {
82+
ollamaConfig.Host = host
83+
}
9584
}
9685
}
9786
}
@@ -100,41 +89,56 @@ func NewProvider(name string, config interface{}) (Provider, error) {
10089
case "ollama":
10190
return NewOllamaProvider(ollamaConfig)
10291
case "claude":
103-
cfg, ok := config.(struct {
104-
Provider string `yaml:"provider"`
105-
Ollama struct {
106-
Model string `yaml:"model"`
107-
Host string `yaml:"host"`
108-
} `yaml:"ollama"`
109-
Claude struct {
110-
APIKey string `yaml:"api_key"`
111-
Model string `yaml:"model"`
112-
} `yaml:"claude"`
113-
OpenAI struct {
114-
APIKey string `yaml:"api_key"`
115-
Model string `yaml:"model"`
116-
} `yaml:"openai"`
117-
})
118-
if !ok || cfg.Claude.APIKey == "" {
92+
if config == nil {
93+
return nil, fmt.Errorf("a Claude API key is required")
94+
}
95+
// Try to convert the config to a map[string]interface{}
96+
cfgMap, ok := config.(map[string]interface{})
97+
if !ok {
98+
return nil, fmt.Errorf("invalid config type for Claude provider")
99+
}
100+
// Extract Claude config
101+
claudeCfg, ok := cfgMap["claude"].(map[string]interface{})
102+
if !ok {
103+
return nil, fmt.Errorf("missing Claude configuration")
104+
}
105+
apiKey, ok := claudeCfg["api_key"].(string)
106+
if !ok || apiKey == "" {
119107
return nil, fmt.Errorf("a Claude API key is required")
120108
}
109+
model, _ := claudeCfg["model"].(string)
110+
if model == "" {
111+
model = "claude-3-opus-20240229" // Default model
112+
}
121113
return NewClaudeProvider(ClaudeConfig{
122-
APIKey: cfg.Claude.APIKey,
123-
Model: cfg.Claude.Model,
114+
APIKey: apiKey,
115+
Model: model,
124116
})
125117
case "openai":
126-
cfg, ok := config.(struct {
127-
OpenAI struct {
128-
APIKey string `yaml:"api_key"`
129-
Model string `yaml:"model"`
130-
} `yaml:"openai"`
131-
})
132-
if !ok || cfg.OpenAI.APIKey == "" {
133-
return nil, fmt.Errorf("OpenAI API key is required")
118+
if config == nil {
119+
return nil, fmt.Errorf("an OpenAI API key is required")
120+
}
121+
// Try to convert the config to a map[string]interface{}
122+
cfgMap, ok := config.(map[string]interface{})
123+
if !ok {
124+
return nil, fmt.Errorf("invalid config type for OpenAI provider")
125+
}
126+
// Extract OpenAI config
127+
openaiCfg, ok := cfgMap["openai"].(map[string]interface{})
128+
if !ok {
129+
return nil, fmt.Errorf("missing OpenAI configuration")
130+
}
131+
apiKey, ok := openaiCfg["api_key"].(string)
132+
if !ok || apiKey == "" {
133+
return nil, fmt.Errorf("an OpenAI API key is required")
134+
}
135+
model, _ := openaiCfg["model"].(string)
136+
if model == "" {
137+
model = "gpt-4-turbo-preview" // Default model
134138
}
135139
return NewOpenAIProvider(OpenAIConfig{
136-
APIKey: cfg.OpenAI.APIKey,
137-
Model: cfg.OpenAI.Model,
140+
APIKey: apiKey,
141+
Model: model,
138142
})
139143
default:
140144
return nil, fmt.Errorf("unsupported provider: %s", name)

0 commit comments

Comments
 (0)