refactor: improve code quality, error handling, and test hygiene
- Standardise error handling and simplify error returns
- Replace `fmt.Errorf` with `errors.New` for static error messages
- Convert `extractArticle` to return `*Article` instead of `(*Article,
error)`
- Use `errors.As` for type assertion on
`viper.ConfigFileNotFoundError`
- Enhance test suite with proper isolation and cleanup
- Replace manual `os.Setenv`/`os.Unsetenv` with `t.Setenv` throughout
- Remove unused test helpers (`writeTestFile`, `writeTempFile`,
`contains`, `hasSuffix`)
- Delete redundant `testFile` struct and `createFile` helpers
- Fix deferred close error checks with `//nolint:errcheck` annotations
- Apply `//nolint:errcheck` to intentional error discards
- stdout writes, file closes, and environment variable unset
operations
- Pipe writes in test stubs
- Simplify string formatting and concatenation
- Replace `fmt.Sprintf` with direct string concatenation where
appropriate
- Use `strconv.Itoa` instead of `fmt.Sprintf("%d", ...)` in dry-run
output
- Clean up dead code and unused constants
- Remove unused `malformedRSSFeed` and `testRSSFeed` constants
- Delete unused color gradient variables from TUI model
- Reorganize switch statements for clarity
- Replace `if-else` chains with `switch` in feed source selection and
TUI view
- Improve code documentation
- Add explanatory comments to default constants
- Document exported function behaviour
- Update HTTP method constant from `"GET"` to `http.MethodGet`
jump to
@@ -0,0 +1,16 @@
+version: "2" + +linters: + enable: + - errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases + - govet # reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - usetesting # reports uses of functions with replacement inside the testing package + - perfsprint # checks that fmt.Sprintf can be replaced with a faster alternative + - unused # checks for unused constants, variables, functions and types + - gocritic # provides diagnostics that check for bugs, performance and style issues + - staticcheck # is a go vet on steroids, applying a ton of static analysis checks + - godoclint # checks Golang's documentation practice + - errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13 + - recvcheck # checks for receiver type consistency + - unparam # reports unused function parameters + - usestdlibvars # detects the possibility to use variables/constants from the Go standard library
@@ -6,6 +6,17 @@ The format is based on [Keep a
Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- Linting using `golangci-lint`, mostly for readability and maintainability. + +### Fixed + +- Fixed all lint issues across the codebase, including unchecked error returns, + string formatting performance, dead code, and documentation style. + ## [0.15.0] - 2026-04-27 ### Added
@@ -4,6 +4,7 @@ import (
"context" "fmt" "os" + "strconv" tea "github.com/charmbracelet/bubbletea" "llm_aggregator/internal/aggregator"@@ -126,7 +127,7 @@ if sh.IsExiting() {
// Signal arrived during execution — output partial result if available sh.Stop() if rt.Summary != "" { - rt.WriteOutput(os.Stdout) + _ = rt.WriteOutput(os.Stdout) //nolint:errcheck // stdout write failure is unrecoverable } fmt.Fprintln(os.Stderr, style.Errorf("interrupted by signal")) os.Exit(130)@@ -177,9 +178,9 @@
// Print configuration summary fmt.Println() fmt.Println(style.Label("Configuration:")) - fmt.Printf(" Max articles per feed: %s\n", style.Value(fmt.Sprintf("%d", rt.MaxArticlesPerFeed))) - fmt.Printf(" Max days old: %s\n", style.Value(fmt.Sprintf("%d", rt.MaxDaysOld))) - fmt.Printf(" Max total articles: %s\n", style.Value(fmt.Sprintf("%d", rt.MaxTotalArticles))) + fmt.Printf(" Max articles per feed: %s\n", style.Value(strconv.Itoa(rt.MaxArticlesPerFeed))) + fmt.Printf(" Max days old: %s\n", style.Value(strconv.Itoa(rt.MaxDaysOld))) + fmt.Printf(" Max total articles: %s\n", style.Value(strconv.Itoa(rt.MaxTotalArticles))) fmt.Printf(" Include keywords: %s\n", style.Value(fmt.Sprintf("%v", rt.IncludeKeywords))) fmt.Printf(" Exclude keywords: %s\n", style.Value(fmt.Sprintf("%v", rt.ExcludeKeywords))) fmt.Printf(" Output format: %s\n", style.Value(rt.Output))@@ -200,7 +201,8 @@
var articles []*aggregator.Article var err error - if rt.Stdin && rt.FeedsFile != "" { + switch { + case rt.Stdin && rt.FeedsFile != "": var stdinArticles []*aggregator.Article var fileArticles []*aggregator.Article@@ -212,13 +214,13 @@ if fileArticles, err = feedAgg.ParseFeedsFromFile(rt.FeedsFile); err != nil {
fmt.Fprintln(os.Stderr, style.Errorf("Failed to parse feeds file: %v", err)) os.Exit(1) } - articles = append(stdinArticles, fileArticles...) - } else if rt.Stdin { + articles = append(stdinArticles, fileArticles...) //nolint:gocritic + case rt.Stdin: if articles, err = feedAgg.ParseFeedFromStdin(); err != nil { fmt.Fprintln(os.Stderr, style.Errorf("Failed to parse stdin feed: %v", err)) os.Exit(1) } - } else { + default: if articles, err = feedAgg.ParseFeedsFromFile(rt.FeedsFile); err != nil { fmt.Fprintln(os.Stderr, style.Errorf("Failed to parse feeds: %v", err)) os.Exit(1)@@ -248,10 +250,10 @@ filteredCount := totalArticles - len(processedArticles)
fmt.Println() fmt.Println(style.Label("Article statistics:")) - fmt.Printf(" Total fetched: %s\n", style.Value(fmt.Sprintf("%d", totalArticles))) - fmt.Printf(" After filtering: %s\n", style.Value(fmt.Sprintf("%d", len(processedArticles)))) + fmt.Printf(" Total fetched: %s\n", style.Value(strconv.Itoa(totalArticles))) + fmt.Printf(" After filtering: %s\n", style.Value(strconv.Itoa(len(processedArticles)))) if filteredCount > 0 { - fmt.Printf(" Filtered out: %s\n", style.Value(fmt.Sprintf("%d", filteredCount))) + fmt.Printf(" Filtered out: %s\n", style.Value(strconv.Itoa(filteredCount))) } // Group articles by source for summary@@ -266,7 +268,7 @@ if len(sourceCounts) > 0 {
fmt.Println() fmt.Println(style.Label("Articles by source:")) for source, count := range sourceCounts { - fmt.Printf(" %s: %s\n", style.Filepath(source), style.Value(fmt.Sprintf("%d", count))) + fmt.Printf(" %s: %s\n", style.Filepath(source), style.Value(strconv.Itoa(count))) } }@@ -274,7 +276,7 @@ // Estimate token count
estimatedTokens := contentProc.EstimateTokenCount(processedArticles, rt.Model) fmt.Println() fmt.Printf("Estimated token count: %s (for model: %s)\n", style.Value(fmt.Sprintf("~%d", estimatedTokens)), style.Value(rt.Model)) - fmt.Printf("Max tokens for response: %s\n", style.Value(fmt.Sprintf("%d", rt.MaxTokens))) + fmt.Printf("Max tokens for response: %s\n", style.Value(strconv.Itoa(rt.MaxTokens))) // Prompt preview fmt.Println()
@@ -19,6 +19,19 @@ # Run tests with coverage
go test ./... -cover ``` +## Linting + +The project uses [golangci-lint](https://golangci-lint.run/) for static analysis. +Configuration is in `.golangci.yml`. + +```bash +# Run the linter +golangci-lint run + +# Auto-fix formatting issues +gofmt -w . +``` + ## Test packages ### `internal/cli`: CLI parsing
@@ -48,7 +48,7 @@ file, err := os.Open(filePath)
if err != nil { return nil, fmt.Errorf("failed to open feeds file: %w", err) } - defer file.Close() + defer file.Close() //nolint:errcheck return fa.parseFeedsFromReader(file, filePath) }@@ -85,13 +85,7 @@ maxItems := min(fa.maxArticlesPerFeed, len(feed.Items))
articles := make([]*Article, 0, maxItems) for i := range feed.Items[:maxItems] { - article, err := fa.extractArticle(feed.Items[i], feedTitle, cutoffTime) - if err != nil { - if fa.progressCtx != nil { - fa.progressCtx.Warningf("Failed to extract article %d from %s: %v", i, sourceName, err) - } - continue - } + article := fa.extractArticle(feed.Items[i], feedTitle, cutoffTime) if article != nil { articles = append(articles, article) }@@ -197,14 +191,8 @@ }
maxItems := min(fa.maxArticlesPerFeed, len(feed.Items)) - for i, item := range feed.Items[:maxItems] { - article, err := fa.extractArticle(item, feedTitle, cutoffTime) - if err != nil { - if fa.progressCtx != nil { - fa.progressCtx.Warningf("Failed to extract article %d from %s: %v", i, feedURL, err) - } - continue - } + for _, item := range feed.Items[:maxItems] { + article := fa.extractArticle(item, feedTitle, cutoffTime) if article != nil { articles = append(articles, article) }@@ -213,7 +201,7 @@
return articles, nil } -func (fa *FeedAggregator) extractArticle(item *gofeed.Item, feedTitle string, cutoffTime time.Time) (*Article, error) { +func (fa *FeedAggregator) extractArticle(item *gofeed.Item, feedTitle string, cutoffTime time.Time) *Article { // Extract metadata title := item.Title if title == "" {@@ -222,7 +210,7 @@ }
link := item.Link if link == "" { - return nil, nil + return nil } // Parse publication date@@ -233,7 +221,7 @@ if !cutoffTime.IsZero() && !published.IsZero() && published.Before(cutoffTime) {
if fa.progressCtx != nil { fa.progressCtx.Debugf("Skipping old article: %s (%s)", title, published.Format("2006-01-02")) } - return nil, nil + return nil } // Extract author@@ -263,7 +251,7 @@ Content: content,
Published: published, Author: author, SourceFeed: feedTitle, - }, nil + } } func (fa *FeedAggregator) parsePublishedDate(item *gofeed.Item) time.Time {@@ -314,7 +302,7 @@ return content
} func (fa *FeedAggregator) fetchWebpageContent(url string) (string, error) { - req, err := http.NewRequest("GET", url, nil) + req, err := http.NewRequest(http.MethodGet, url, nil) if err != nil { return "", err }@@ -324,7 +312,7 @@ resp, err := fa.client.Do(req)
if err != nil { return "", err } - defer resp.Body.Close() + defer resp.Body.Close() //nolint:errcheck if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("HTTP status %d", resp.StatusCode)
@@ -46,22 +46,12 @@ <author><name>Atom Author</name></author>
</entry> </feed>` -const malformedRSSFeed = `<?xml version="1.0" encoding="UTF-8"?> -<rss version="2.0"> - <channel> - <title>Malformed Feed</title> - <item> - <title>Incomplete Article</title> - <!-- missing link --> - </item> - </channel> -</rss>` - func TestNewFeedAggregatorWithProgress(t *testing.T) { t.Run("with nil progress context", func(t *testing.T) { fa := NewFeedAggregatorWithProgress(10, 7, 5000, nil) if fa == nil { t.Error("Expected non-nil FeedAggregator") + return } if fa.progressCtx != nil { t.Error("Expected nil progress context")@@ -134,48 +124,6 @@ if err != nil && strings.Contains(err.Error(), "failed to open") {
t.Errorf("File parsing error: %v", err) } }) -} - -func writeTestFile(path, content string) error { - return writeFile(path, []byte(content)) -} - -func writeFile(path string, data []byte) error { - f, err := createFile(path) - if err != nil { - return err - } - defer f.Close() - _, err = f.Write(data) - return err -} - -func createFile(path string) (*testFile, error) { - return &testFile{path: path}, nil -} - -type testFile struct { - path string -} - -func (tf *testFile) Write(data []byte) (int, error) { - // Using os.Write would be cleaner but let's use os package directly - return len(data), nil -} - -func (tf *testFile) Close() error { - return nil -} - -// Helper to actually write to temp file -func writeTempFile(t *testing.T, content string) string { - t.Helper() - tmpFile := "/tmp/llm_aggregator_test_feeds.txt" - err := os.WriteFile(tmpFile, []byte(content), 0644) - if err != nil { - t.Fatalf("Failed to write temp file: %v", err) - } - return tmpFile } func TestArticleAgeFiltering(t *testing.T) {@@ -452,8 +400,8 @@
// Read from a pipes reader to avoid blocking on real stdin r, w, _ := os.Pipe() go func() { - w.WriteString(validRSSFeed) - w.Close() + _, _ = w.WriteString(validRSSFeed) //nolint:errcheck + w.Close() //nolint:errcheck }() articles, err := fa.ParseFeedFromReader(r, "stdin")
@@ -52,12 +52,12 @@ DryRun bool `arg:"-D,--dry-run" help:"Validate config, show article statistics, and exit without making LLM API calls"`
} // Version returns the version string. -func (Args) Version() string { +func (a *Args) Version() string { return fmt.Sprintf("llm_aggregator v%s (built %s)", Version, BuildDate) } // Description returns the program description. -func (Args) Description() string { +func (a *Args) Description() string { return "LLM Aggregator - Aggregate RSS feeds and summarise with LLM API" }
@@ -19,11 +19,11 @@ feedsFile: "/tmp/feeds.txt",
wantErr: false, setupFunc: func() { // Create a temporary feeds file - f, _ := os.Create("/tmp/feeds.txt") - f.Close() + f, _ := os.Create("/tmp/feeds.txt") //nolint:errcheck + _ = f.Close() //nolint:errcheck }, cleanupFunc: func() { - os.Remove("/tmp/feeds.txt") + _ = os.Remove("/tmp/feeds.txt") //nolint:errcheck }, }, {@@ -177,10 +177,10 @@ envKey: "sk-env-key-67890",
wantAPIKey: nil, // CLI parsing doesn't read env vars directly wantErr: false, setupFunc: func() { - os.Setenv("LLM_AGGREGATOR_API_KEY", "sk-env-key-67890") + t.Setenv("LLM_AGGREGATOR_API_KEY", "sk-env-key-67890") }, cleanupFunc: func() { - os.Unsetenv("LLM_AGGREGATOR_API_KEY") + _ = os.Unsetenv("LLM_AGGREGATOR_API_KEY") //nolint:errcheck }, }, {@@ -190,10 +190,10 @@ envKey: "sk-env-key-should-be-ignored",
wantAPIKey: strPtr("sk-explicit-key"), wantErr: false, setupFunc: func() { - os.Setenv("LLM_AGGREGATOR_API_KEY", "sk-env-key-should-be-ignored") + t.Setenv("LLM_AGGREGATOR_API_KEY", "sk-env-key-should-be-ignored") }, cleanupFunc: func() { - os.Unsetenv("LLM_AGGREGATOR_API_KEY") + _ = os.Unsetenv("LLM_AGGREGATOR_API_KEY") //nolint:errcheck }, }, }@@ -281,10 +281,10 @@ model: "",
wantModel: nil, // CLI parsing doesn't read env vars directly wantErr: false, setupFunc: func() { - os.Setenv("LLM_AGGREGATOR_MODEL", "custom-model-from-env") + t.Setenv("LLM_AGGREGATOR_MODEL", "custom-model-from-env") }, cleanupFunc: func() { - os.Unsetenv("LLM_AGGREGATOR_MODEL") + _ = os.Unsetenv("LLM_AGGREGATOR_MODEL") //nolint:errcheck }, }, {@@ -293,10 +293,10 @@ model: "explicit-model",
wantModel: strPtr("explicit-model"), wantErr: false, setupFunc: func() { - os.Setenv("LLM_AGGREGATOR_MODEL", "env-model-should-be-ignored") + t.Setenv("LLM_AGGREGATOR_MODEL", "env-model-should-be-ignored") }, cleanupFunc: func() { - os.Unsetenv("LLM_AGGREGATOR_MODEL") + _ = os.Unsetenv("LLM_AGGREGATOR_MODEL") //nolint:errcheck }, }, }
@@ -36,7 +36,7 @@ func renderFlag(short, long string) string {
if short != "" { return fmt.Sprintf("%s, %s", short, long) } - return fmt.Sprintf("%s", long) + return long } func getOptionValue(opt HelpOption) string {@@ -316,5 +316,5 @@ }
func WriteHelp(args *Args, w *os.File) { output := BuildStyledHelp(args) - w.WriteString(output) + w.WriteString(output) //nolint:errcheck }
@@ -21,9 +21,7 @@ }
for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - old := os.Getenv("NO_COLOR") - os.Setenv("NO_COLOR", tt.noColorVal) - defer os.Setenv("NO_COLOR", old) + t.Setenv("NO_COLOR", tt.noColorVal) got := shouldStyle() if got != tt.want {@@ -114,9 +112,7 @@ }
} func TestMaxFlagWidth(t *testing.T) { - old := os.Getenv("NO_COLOR") - os.Setenv("NO_COLOR", "1") - defer os.Setenv("NO_COLOR", old) + t.Setenv("NO_COLOR", "1") sections := []HelpSection{ {@@ -144,9 +140,7 @@ }
func TestFormatSection(t *testing.T) { // Test with NO_COLOR to avoid lipgloss dependency in assertions - old := os.Getenv("NO_COLOR") - os.Setenv("NO_COLOR", "1") - defer os.Setenv("NO_COLOR", old) + t.Setenv("NO_COLOR", "1") section := HelpSection{ Title: "Test Section",@@ -189,10 +183,8 @@ }
} func TestFormatSectionWithStyling(t *testing.T) { - // Test with color enabled - old := os.Getenv("NO_COLOR") - os.Unsetenv("NO_COLOR") - defer os.Setenv("NO_COLOR", old) + // Ensure NO_COLOR is not set for this test + _ = os.Unsetenv("NO_COLOR") // Suppress the lipgloss warning during test _ = style.NoColor@@ -222,9 +214,7 @@ }
func TestBuildStyledHelp(t *testing.T) { // Test without color - old := os.Getenv("NO_COLOR") - os.Setenv("NO_COLOR", "1") - defer os.Setenv("NO_COLOR", old) + t.Setenv("NO_COLOR", "1") args := &Args{FeedsFile: "/tmp/feeds.txt", Prompt: "Summarise"} output := BuildStyledHelp(args)@@ -247,9 +237,7 @@ }
} func TestBuildStyledHelpWithColor(t *testing.T) { - old := os.Getenv("NO_COLOR") - os.Unsetenv("NO_COLOR") - defer os.Setenv("NO_COLOR", old) + _ = os.Unsetenv("NO_COLOR") args := &Args{FeedsFile: "/tmp/feeds.txt", Prompt: "Summarise"} output := BuildStyledHelp(args)@@ -269,22 +257,20 @@
func TestWriteHelp(t *testing.T) { args := &Args{FeedsFile: "/tmp/feeds.txt", Prompt: "Summarise"} - old := os.Getenv("NO_COLOR") - os.Setenv("NO_COLOR", "1") - defer os.Setenv("NO_COLOR", old) + t.Setenv("NO_COLOR", "1") - tmpFile, err := os.CreateTemp("", "help_test_*.txt") + tmpFile, err := os.CreateTemp(t.TempDir(), "help_test_*.txt") if err != nil { t.Fatalf("Failed to create temp file: %v", err) } - defer os.Remove(tmpFile.Name()) - tmpFile.Close() + defer func() { _ = os.Remove(tmpFile.Name()) }() //nolint:errcheck + _ = tmpFile.Close() //nolint:errcheck w, err := os.OpenFile(tmpFile.Name(), os.O_WRONLY, 0644) if err != nil { t.Fatalf("Failed to open temp file for write: %v", err) } - defer w.Close() + defer func() { _ = w.Close() }() //nolint:errcheck WriteHelp(args, w)
@@ -1,6 +1,7 @@
package config import ( + "errors" "fmt" "os" "path/filepath"@@ -72,11 +73,10 @@ // Debug: show what config file is being used
// Try to read config file if err := v.ReadInConfig(); err != nil { // If config file doesn't exist, that's OK - we'll use defaults + env vars - if _, ok := err.(viper.ConfigFileNotFoundError); !ok { + var notFoundError viper.ConfigFileNotFoundError + if !errors.As(err, ¬FoundError) { fmt.Fprintln(os.Stderr, style.Warningf("error reading config file: %v", err)) - } else { } - } else { } } return v@@ -210,6 +210,7 @@ return rt
} // bindEnvVars binds environment variables to viper keys. +//nolint:errcheck // viper.BindEnv always returns nil in practice func bindEnvVars(v *viper.Viper) { // Feed aggregation options v.BindEnv("max_articles_per_feed", "LLM_AGGREGATOR_MAX_ARTICLES_PER_FEED")
@@ -63,11 +63,7 @@ // Order from highest to lowest: CLI args > Environment variables > Config file > Defaults
func TestViperToRuntimePrecedence(t *testing.T) { // Create temporary directory for test tempDir := t.TempDir() - oldEnv := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tempDir) - defer func() { - os.Setenv("XDG_CONFIG_HOME", oldEnv) - }() + t.Setenv("XDG_CONFIG_HOME", tempDir) // Save a config file with known values (manually write TOML since DefaultConfig/Save don't exist) configuredModel := "config-file-model"@@ -82,7 +78,7 @@ max_tokens = %d
`, configuredModel, configuredBaseURL, configuredTemperature, configuredMaxTokens) configPath := filepath.Join(tempDir, "llm_aggregator", "config.toml") - os.MkdirAll(filepath.Dir(configPath), 0755) + _ = os.MkdirAll(filepath.Dir(configPath), 0755) //nolint:errcheck if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { t.Fatalf("Failed to write config file: %v", err) }@@ -90,10 +86,10 @@
// Test 1: Config file should override defaults t.Run("config file overrides defaults", func(t *testing.T) { // Clear any env vars - os.Unsetenv("LLM_AGGREGATOR_MODEL") - os.Unsetenv("LLM_AGGREGATOR_BASE_URL") - os.Unsetenv("LLM_AGGREGATOR_TEMPERATURE") - os.Unsetenv("LLM_AGGREGATOR_MAX_TOKENS") + _ = os.Unsetenv("LLM_AGGREGATOR_MODEL") + _ = os.Unsetenv("LLM_AGGREGATOR_BASE_URL") + _ = os.Unsetenv("LLM_AGGREGATOR_TEMPERATURE") + _ = os.Unsetenv("LLM_AGGREGATOR_MAX_TOKENS") v := GetViper() rt := ViperToRuntime(v, "/tmp/feeds.txt", "test prompt")@@ -110,11 +106,11 @@ // Test 2: Environment variables should override config file
t.Run("environment variables override config file", func(t *testing.T) { envModel := "env-override-model" envBaseURL := "https://env-override.example.com" - os.Setenv("LLM_AGGREGATOR_MODEL", envModel) - os.Setenv("LLM_AGGREGATOR_BASE_URL", envBaseURL) + t.Setenv("LLM_AGGREGATOR_MODEL", envModel) + t.Setenv("LLM_AGGREGATOR_BASE_URL", envBaseURL) defer func() { - os.Unsetenv("LLM_AGGREGATOR_MODEL") - os.Unsetenv("LLM_AGGREGATOR_BASE_URL") + _ = os.Unsetenv("LLM_AGGREGATOR_MODEL") + _ = os.Unsetenv("LLM_AGGREGATOR_BASE_URL") }() v := GetViper()@@ -137,11 +133,11 @@ v.SetDefault("base_url", "https://default.example.com")
v.SetDefault("temperature", 0.7) // Set env vars that should be overridden - os.Setenv("LLM_AGGREGATOR_MODEL", "env-should-be-overridden") - os.Setenv("LLM_AGGREGATOR_BASE_URL", "https://env-should-be-overridden.example.com") + t.Setenv("LLM_AGGREGATOR_MODEL", "env-should-be-overridden") + t.Setenv("LLM_AGGREGATOR_BASE_URL", "https://env-should-be-overridden.example.com") defer func() { - os.Unsetenv("LLM_AGGREGATOR_MODEL") - os.Unsetenv("LLM_AGGREGATOR_BASE_URL") + _ = os.Unsetenv("LLM_AGGREGATOR_MODEL") + _ = os.Unsetenv("LLM_AGGREGATOR_BASE_URL") }() // Simulate CLI args binding - using pointer types like the actual Args struct@@ -302,30 +298,12 @@ func intPtr(i int) *int { return &i }
func floatPtr(f float64) *float64 { return &f } func boolPtr(b bool) *bool { return &b } -// Helper function to check if string contains substring -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || - (s[0:len(substr)] == substr || contains(s[1:], substr))) -} - -// Helper function to check if path has suffix -func hasSuffix(path, suffix string) bool { - if len(path) < len(suffix) { - return false - } - return path[len(path)-len(suffix):] == suffix -} - // TestConfigParsingAlwaysPasses ensures that parsing of options always passes. // This tests the integration between CLI args, config loading, and Viper. func TestConfigParsingAlwaysPasses(t *testing.T) { // Create temporary directory for test tempDir := t.TempDir() - oldEnv := os.Getenv("XDG_CONFIG_HOME") - os.Setenv("XDG_CONFIG_HOME", tempDir) - defer func() { - os.Setenv("XDG_CONFIG_HOME", oldEnv) - }() + t.Setenv("XDG_CONFIG_HOME", tempDir) // Create a test feeds file feedsFile := tempDir + "/feeds.txt"@@ -406,10 +384,7 @@ // Build CLI args slice
args := []string{"llm_aggregator"} for k, v := range tt.cliArgs { // Remove leading dashes for go-arg compatibility - key := k - if strings.HasPrefix(key, "--") { - key = key[2:] - } + key := strings.TrimPrefix(k, "--") args = append(args, "--"+key) if v != "" { args = append(args, v)
@@ -7,20 +7,27 @@ // consistent default values across CLI arguments, configuration,
// and runtime settings. const ( - // LLM API defaults - DefaultModel = "deepseek-chat" - DefaultBaseURL = "https://api.deepseek.com" - DefaultMaxTokens = 4000 + // DefaultModel is the default LLM model. + DefaultModel = "deepseek-chat" + // DefaultBaseURL is the default API base URL. + DefaultBaseURL = "https://api.deepseek.com" + // DefaultMaxTokens is the default max tokens for LLM responses. + DefaultMaxTokens = 4000 + // DefaultTemperature is the default sampling temperature. DefaultTemperature = 0.7 - DefaultLLMTimeout = 300 // seconds + // DefaultLLMTimeout is the default LLM request timeout in seconds. + DefaultLLMTimeout = 300 - // Feed aggregation defaults + // DefaultMaxArticlesPerFeed is the default max articles per feed. DefaultMaxArticlesPerFeed = 10 - DefaultMaxDaysOld = 7 - DefaultMaxTotalArticles = 20 + // DefaultMaxDaysOld is the default max age for articles in days. + DefaultMaxDaysOld = 7 + // DefaultMaxTotalArticles is the default max total articles. + DefaultMaxTotalArticles = 20 - // Output defaults - DefaultOutput = "text" + // DefaultOutput is the default output format. + DefaultOutput = "text" + // DefaultIncludeArticles is the default for including articles. DefaultIncludeArticles = false )
@@ -2,6 +2,7 @@ package llm
import ( "context" + "errors" "fmt" "os" "strings"@@ -37,7 +38,7 @@ if apiKey == "" {
apiKey = os.Getenv("LLM_AGGREGATOR_API_KEY") } if apiKey == "" || strings.TrimSpace(apiKey) == "" { - return nil, fmt.Errorf( + return nil, errors.New( "LLM API key is required. " + "Set LLM_AGGREGATOR_API_KEY environment variable or pass apiKey parameter", )@@ -121,28 +122,28 @@ contextParts = append(contextParts, fmt.Sprintf("--- ARTICLE %d ---", i+1))
contextParts = append(contextParts, fmt.Sprintf("Title: %s", article["title"])) if source, ok := article["source_feed"].(string); ok && source != "" { - contextParts = append(contextParts, fmt.Sprintf("Source: %s", source)) + contextParts = append(contextParts, "Source: "+source) } if published, ok := article["published"]; ok { switch pub := published.(type) { case time.Time: if !pub.IsZero() { - contextParts = append(contextParts, fmt.Sprintf("Published: %s", pub.Format(time.RFC3339))) + contextParts = append(contextParts, "Published: "+pub.Format(time.RFC3339)) } case string: - contextParts = append(contextParts, fmt.Sprintf("Published: %s", pub)) + contextParts = append(contextParts, "Published: "+pub) default: contextParts = append(contextParts, fmt.Sprintf("Published: %v", pub)) } } if author, ok := article["author"].(string); ok && author != "" { - contextParts = append(contextParts, fmt.Sprintf("Author: %s", author)) + contextParts = append(contextParts, "Author: "+author) } if link, ok := article["link"].(string); ok && link != "" { - contextParts = append(contextParts, fmt.Sprintf("Link: %s", link)) + contextParts = append(contextParts, "Link: "+link) } if content, ok := article["content"].(string); ok && content != "" {@@ -151,7 +152,7 @@ maxContentLen := 3000
if len(content) > maxContentLen { content = content[:maxContentLen] + "... [truncated]" } - contextParts = append(contextParts, fmt.Sprintf("Content: %s", content)) + contextParts = append(contextParts, "Content: "+content) } contextParts = append(contextParts, "") // Empty line between articles@@ -209,22 +210,23 @@ if dc.logger != nil {
dc.logger.Logf("API error: %s", errStr) } - if strings.Contains(errStr, "401") { - return "", nil, fmt.Errorf("invalid API key. Please check your LLM API key") - } else if strings.Contains(errStr, "429") { - return "", nil, fmt.Errorf("rate limit exceeded. Please try again later") - } else if strings.Contains(errStr, "500") { - return "", nil, fmt.Errorf("LLM API server error. Please try again later") - } else if strings.Contains(errStr, "404") { - return "", nil, fmt.Errorf("API endpoint not found. Please check the base URL and endpoint. OpenAI API uses /chat/completions") - } else if strings.Contains(errStr, "context deadline exceeded") || strings.Contains(errStr, "context canceled") { + switch { + case strings.Contains(errStr, "401"): + return "", nil, errors.New("invalid API key. Please check your LLM API key") + case strings.Contains(errStr, "429"): + return "", nil, errors.New("rate limit exceeded. Please try again later") + case strings.Contains(errStr, "500"): + return "", nil, errors.New("LLM API server error. Please try again later") + case strings.Contains(errStr, "404"): + return "", nil, errors.New("API endpoint not found. Please check the base URL and endpoint. OpenAI API uses /chat/completions") + case strings.Contains(errStr, "context deadline exceeded") || strings.Contains(errStr, "context canceled"): return "", nil, fmt.Errorf("LLM request timed out after %d seconds", dc.llmTimeout) } return "", nil, fmt.Errorf("failed to connect to LLM API: %w", err) } if len(response.Choices) == 0 { - return "", nil, fmt.Errorf("no response choices returned from API") + return "", nil, errors.New("no response choices returned from API") } outputText := response.Choices[0].Message.Content
@@ -46,7 +46,7 @@ var lines []string
// Title title := getString(data, "title", "LLM Aggregator Summary") - lines = append(lines, fmt.Sprintf("# %s", title)) + lines = append(lines, "# "+title) lines = append(lines, "") // Metadata@@ -58,10 +58,10 @@ model := getString(data, "model", "Unknown")
articlesCount := getInt(data, "articles_count", 0) timestamp := getString(data, "timestamp", time.Now().Format(time.RFC3339)) - lines = append(lines, fmt.Sprintf("- **Prompt**: %s", prompt)) - lines = append(lines, fmt.Sprintf("- **Model**: %s", model)) + lines = append(lines, "- **Prompt**: "+prompt) + lines = append(lines, "- **Model**: "+model) lines = append(lines, fmt.Sprintf("- **Articles Analysed**: %d", articlesCount)) - lines = append(lines, fmt.Sprintf("- **Generated**: %s", timestamp)) + lines = append(lines, "- **Generated**: "+timestamp) lines = append(lines, "") // Summary@@ -81,22 +81,22 @@ lines = append(lines, fmt.Sprintf("### Article %d: %s", i+1, article["title"]))
lines = append(lines, "") if source, ok := article["source_feed"].(string); ok && source != "" { - lines = append(lines, fmt.Sprintf("**Source**: %s", source)) + lines = append(lines, "**Source**: "+source) } if published, ok := article["published"]; ok { switch pub := published.(type) { case time.Time: if !pub.IsZero() { - lines = append(lines, fmt.Sprintf("**Published**: %s", pub.Format("2006-01-02 15:04"))) + lines = append(lines, "**Published**: "+pub.Format("2006-01-02 15:04")) } case string: - lines = append(lines, fmt.Sprintf("**Published**: %s", pub)) + lines = append(lines, "**Published**: "+pub) } } if author, ok := article["author"].(string); ok && author != "" { - lines = append(lines, fmt.Sprintf("**Author**: %s", author)) + lines = append(lines, "**Author**: "+author) } if link, ok := article["link"].(string); ok && link != "" {@@ -129,10 +129,10 @@ model := getString(data, "model", "Unknown")
articlesCount := getInt(data, "articles_count", 0) timestamp := getString(data, "timestamp", time.Now().Format(time.RFC3339)) - lines = append(lines, fmt.Sprintf("Prompt: %s", prompt)) - lines = append(lines, fmt.Sprintf("Model: %s", model)) + lines = append(lines, "Prompt: "+prompt) + lines = append(lines, "Model: "+model) lines = append(lines, fmt.Sprintf("Articles Analysed: %d", articlesCount)) - lines = append(lines, fmt.Sprintf("Generated: %s", timestamp)) + lines = append(lines, "Generated: "+timestamp) lines = append(lines, "") // Summary@@ -154,26 +154,26 @@ lines = append(lines, fmt.Sprintf("[Article %d]", i+1))
lines = append(lines, fmt.Sprintf("Title: %s", article["title"])) if source, ok := article["source_feed"].(string); ok && source != "" { - lines = append(lines, fmt.Sprintf("Source: %s", source)) + lines = append(lines, "Source: "+source) } if published, ok := article["published"]; ok { switch pub := published.(type) { case time.Time: if !pub.IsZero() { - lines = append(lines, fmt.Sprintf("Published: %s", pub.Format("2006-01-02 15:04"))) + lines = append(lines, "Published: "+pub.Format("2006-01-02 15:04")) } case string: - lines = append(lines, fmt.Sprintf("Published: %s", pub)) + lines = append(lines, "Published: "+pub) } } if author, ok := article["author"].(string); ok && author != "" { - lines = append(lines, fmt.Sprintf("Author: %s", author)) + lines = append(lines, "Author: "+author) } if link, ok := article["link"].(string); ok && link != "" { - lines = append(lines, fmt.Sprintf("Link: %s", link)) + lines = append(lines, "Link: "+link) } // Show preview of content@@ -182,7 +182,7 @@ preview := content
if len(preview) > 200 { preview = preview[:200] + "..." } - lines = append(lines, fmt.Sprintf("Preview: %s", preview)) + lines = append(lines, "Preview: "+preview) } lines = append(lines, "")
@@ -39,18 +39,18 @@ }
} func (sl *SimpleLogger) Logf(format string, args ...any) { - fmt.Fprintf(sl.writer, format+"\n", args...) + _, _ = fmt.Fprintf(sl.writer, format+"\n", args...) } func (sl *SimpleLogger) Warningf(format string, args ...any) { // Use style.Warningf for orange bold WARNING prefix - fmt.Fprintf(sl.writer, "%s\n", style.Warningf(format, args...)) + _, _ = fmt.Fprintf(sl.writer, "%s\n", style.Warningf(format, args...)) } func (sl *SimpleLogger) Debugf(format string, args ...any) { if sl.debug { // Use style.Debugf for styled debug output - fmt.Fprintf(sl.writer, "%s\n", style.Debugf(format, args...)) + _, _ = fmt.Fprintf(sl.writer, "%s\n", style.Debugf(format, args...)) } }
@@ -2,6 +2,7 @@ package runtime
import ( "context" + "errors" "fmt" "io" "os"@@ -68,7 +69,8 @@
var articles []*aggregator.Article var err error - if r.Stdin && r.FeedsFile != "" { + switch { + case r.Stdin && r.FeedsFile != "": // Both stdin and feeds file: collate results r.Progress.SetSubStage("Parsing stdin feed and feeds file")@@ -83,25 +85,25 @@ if fileArticles, err = feedAgg.ParseFeedsFromFile(r.FeedsFile); err != nil {
return fmt.Errorf("error aggregating feeds: %w", err) } - articles = append(stdinArticles, fileArticles...) - } else if r.Stdin { + articles = append(stdinArticles, fileArticles...) //nolint:gocritic + case r.Stdin: // Stdin only r.Progress.SetSubStage("Parsing stdin feed") if articles, err = feedAgg.ParseFeedFromStdin(); err != nil { return fmt.Errorf("error parsing stdin feed: %w", err) } - } else if r.FeedsFile != "" { + case r.FeedsFile != "": // Feeds file only - r.Progress.SetSubStage(fmt.Sprintf("Parsing feeds from %s", r.FeedsFile)) + r.Progress.SetSubStage("Parsing feeds from " + r.FeedsFile) if articles, err = feedAgg.ParseFeedsFromFile(r.FeedsFile); err != nil { return fmt.Errorf("error aggregating feeds: %w", err) } - } else { - return fmt.Errorf("no feed source specified: use --feeds-file or --stdin") + default: + return errors.New("no feed source specified: use --feeds-file or --stdin") } if len(articles) == 0 { - return fmt.Errorf("no articles found after parsing feeds") + return errors.New("no articles found after parsing feeds") } r.Progress.SetArticleCount(len(articles), 0)@@ -120,7 +122,7 @@
processedArticles := contentProc.ProcessArticles(articles, "date", true) if len(processedArticles) == 0 { - return fmt.Errorf("no articles passed filtering") + return errors.New("no articles passed filtering") } r.Progress.SetArticleCount(len(articles), len(articles)-len(processedArticles))@@ -132,7 +134,7 @@ r.Progress.SetTokenEstimate(tokenEst, tokenEst)
// Step 3: Initialise LLM client r.Progress.SetStage("Connecting to LLM") - r.Progress.SetSubStage(fmt.Sprintf("Using model: %s", r.Model)) + r.Progress.SetSubStage("Using model: " + r.Model) llm, err := llm.NewLLMClient( r.APIKey,@@ -197,7 +199,7 @@ return fmt.Errorf("error creating formatter: %w", err)
} outputData := map[string]any{ - "title": fmt.Sprintf("LLM Aggregator Summary - %s", time.Now().Format("2006-01-02 15:04")), + "title": "LLM Aggregator Summary - " + time.Now().Format("2006-01-02 15:04"), "prompt": r.Prompt, "model": r.Model, "articles_count": len(r.Articles),@@ -231,7 +233,7 @@ file, err := os.Create(r.OutputFile)
if err != nil { return fmt.Errorf("error creating output file: %w", err) } - defer file.Close() + defer file.Close() //nolint:errcheck return r.WriteOutput(file) }
@@ -10,25 +10,6 @@ "llm_aggregator/internal/aggregator"
"llm_aggregator/internal/progress" ) -const testRSSFeed = `<?xml version="1.0" encoding="UTF-8"?> -<rss version="2.0"> - <channel> - <title>File Feed</title> - <link>https://example.com/feed</link> - <description>A test RSS feed</description> - <item> - <title>File Article One</title> - <link>https://example.com/file1</link> - <description>Content from file feed.</description> - </item> - <item> - <title>File Article Two</title> - <link>https://example.com/file2</link> - <description>More content from file feed.</description> - </item> - </channel> -</rss>` - const testAtomFeed = `<?xml version="1.0" encoding="UTF-8"?> <feed xmlns="http://www.w3.org/2005/Atom"> <title>Stdin Feed</title>@@ -131,7 +112,7 @@ t.Fatalf("Unexpected error parsing feeds file: %v", err)
} // Collate: stdin first, then feeds file - articles := append(stdinArticles, fileArticles...) + articles := append(stdinArticles, fileArticles...) //nolint:gocritic // Should have 1 stdin article + 0 file articles (URL unreachable) if len(articles) != 1 {
@@ -38,7 +38,7 @@
// White for headings White = Colour("15") - // Bright black for muted/debug text + // BrightBlack is bright black for muted/debug text. BrightBlack = Colour("8") )
@@ -26,11 +26,9 @@ )
var ( colorSubtle = lipgloss.Color("7") // Gray (dim text) - colorHighlight = lipgloss.Color("13") // Magenta (status) - colorSuccess = lipgloss.Color("2") // Green (completion) - colorError = lipgloss.Color("1") // Red (errors) - colorGradientStart = lipgloss.Color("205") // Pink - colorGradientEnd = lipgloss.Color("226") // Yellow + colorHighlight = lipgloss.Color("13") // Magenta (status) + colorSuccess = lipgloss.Color("2") // Green (completion) + colorError = lipgloss.Color("1") // Red (errors) titleStyle = lipgloss.NewStyle(). MarginLeft(2).@@ -331,21 +329,22 @@ sb.WriteString("\n\n")
// If done, show 100% progress. var progressPercent float64 - if m.done && m.errorMsg == "" { + switch { + case m.done && m.errorMsg == "": progressPercent = 1.0 - } else if m.waiting { + case m.waiting: // Fake progress: 85% base + up to 15% over 60 seconds elapsed := time.Since(m.waitingStart).Seconds() waitFraction := min(elapsed/60.0, 1.0) progressPercent = 0.85 + (waitFraction * 0.15) - } else if m.tokenCount > 0 { + case m.tokenCount > 0: // Use token-based progress once articles are ready // Tokens sent to LLM as a fraction of model's max token window (8k context assumed) progressPercent = float64(m.tokenUsed) / float64(m.tokenCount) if progressPercent > 1.0 { progressPercent = 1.0 } - } else { + default: progressPercent = float64(m.currentStep) / float64(m.totalSteps) }@@ -357,11 +356,12 @@ // Status with spinner
sb.WriteString(statusStyle.Render("Status: ")) // Replace spinner with a checkmark when done. - if m.done && m.errorMsg == "" { + switch { + case m.done && m.errorMsg == "": sb.WriteString(lipgloss.NewStyle().Foreground(colorSuccess).Render("✓")) - } else if m.done && m.errorMsg != "" { + case m.done && m.errorMsg != "": sb.WriteString(lipgloss.NewStyle().Foreground(colorError).Render("✗")) - } else { + default: sb.WriteString(m.spinner.View()) }@@ -424,7 +424,7 @@
return sb.String() } -// Messages for updating the TUI +// StepMsg represents a progress update message. type StepMsg struct { Step int Status string