docs: improve code documentation and clarify internal interfaces
- Add or expand Go doc comments for all exported types and functions
- Document purpose, parameters, return values, and edge cases
- Examples: NewLLMClient, ViperToRuntime, EncodingForModel, isZero
- Replace vague or outdated comments with precise behavior descriptions
- Clarify feed fetching, content extraction, and webpage fallback
logic
- Explain signal handling, context cancellation, and timeout
composition
- Remove redundant or misleading comments (e.g., obvious step-by-step
remarks)
- Standardise progress logging interface documentation across
implementations
- SimpleLogger, NoopLogger, TUIProgress now have consistent doc
strings
- Add inline explanations for non-obvious logic
- Tokeniser caching, zero-value detection for CLI args, sort fallback
to date
- Improve TUI model comments to reflect Bubbletea command pattern
accurately
- Clean up whitespace and comment formatting for consistency
jump to
@@ -27,44 +27,35 @@ func main() {
cli.BuildDate = buildDate cli.Version = version - // Parse command line arguments args, err := cli.ParseArgs() if err != nil { fmt.Fprintln(os.Stderr, style.Errorf("parsing arguments: %v", err)) os.Exit(1) } - // Set up signal handling before any blocking call. - // SIGINT, SIGTERM, and SIGHUP are all treated the same — clean shutdown. sh := signals.New() sh.Watch() - - // Get viper instance with config file + environment variables + defaults v := config.GetViper() - // Bind CLI args to viper (highest precedence) config.BindCLIArgs(v, args.ToViperMap()) - // Create runtime directly from viper configuration + // FeedsFile and Prompt come from positional CLI args; everything else from viper rt := config.ViperToRuntime(v, args.FeedsFile, args.Prompt) - // Run dry-run mode if requested (validates config, shows statistics, no LLM calls) if args.DryRun { sh.Stop() runDryRun(rt, args.Verbose) os.Exit(0) } - // Validate API key (required for actual execution) if v.GetString("api_key") == "" { sh.Stop() fmt.Fprintln(os.Stderr, style.Errorf("OpenAI-compatible API key is required. Set via --api-key, %s environment variable, or config file.", "LLM_AGGREGATOR_API_KEY")) os.Exit(1) } - // Run with TUI if requested if args.TUI { sh.Stop() runWithTUI(rt)@@ -74,20 +65,13 @@ }
} func runWithTUI(rt *runtime.Runtime) { - // 1. Create the model and pass the runtime to it. + // Build the model and inject progress bridge so runtime can send messages into the TUI model := tui.New(rt) - - // 2. Create the tea.Program. p := tea.NewProgram(model, tea.WithAltScreen()) - - // 3. Create the TUIProgress handler and give it the program instance. - // This is the bridge that allows the runtime to send messages to the TUI. tp := tui.NewTUIProgress(p) - - // 4. Inject the progress handler into the runtime. rt.Progress = tp - // 5. Run the program. This is a blocking call. + // Blocking call; returns on quit if _, err := p.Run(); err != nil { fmt.Fprintln(os.Stderr, style.Errorf("TUI error: %v", err)) os.Exit(1)@@ -95,7 +79,7 @@ }
} func runWithoutTUI(rt *runtime.Runtime, verbose bool, sh *signals.SignalHandler) { - // Setup logger based on verbose flag and inject it into the runtime + // SimpleLogger outputs to stdout; nil uses NoopLogger if verbose { rt.Progress = progress.NewSimpleLogger(os.Stdout, true) } else {
@@ -27,7 +27,8 @@ userAgent string
progressCtx *progress.Context } -// NewFeedAggregatorWithProgress creates a new FeedAggregator with progress context. +// NewFeedAggregatorWithProgress creates a FeedAggregator with progress reporting. +// progressCtx may be nil (e.g. in dry-run mode with verbose disabled). func NewFeedAggregatorWithProgress(maxArticlesPerFeed, maxDaysOld, maxContentLength int, progressCtx *progress.Context) *FeedAggregator { return &FeedAggregator{ maxArticlesPerFeed: maxArticlesPerFeed,@@ -94,8 +95,8 @@
return articles, nil } -// parseFeedsFromReader reads feed URLs from an io.Reader and fetches each one. -// Used by both file-based and stdin-based feed loading. +// parseFeedsFromReader reads feed URLs line-by-line from any io.Reader, +// then fetches each feed concurrently. Used by both file and stdin paths. func (fa *FeedAggregator) parseFeedsFromReader(reader io.Reader, sourceName string) ([]*Article, error) { content, err := io.ReadAll(reader) if err != nil {@@ -277,13 +278,9 @@ return time.Time{}
} func (fa *FeedAggregator) extractContent(item *gofeed.Item, link string) string { - // Priority: Content -> Description -> Fetch webpage - // NOTE: Webpage scraping is only attempted when content is empty or <100 - // chars. This balances content quality against latency (webpage fetch adds - // ~1-2s per article). content := "" - // Try to get content from item + // Feed item content fields are tried in order; gofeed unifies atom Content/Description if item.Content != "" { content = item.Content } else if item.Description != "" {@@ -323,10 +320,10 @@ if err != nil {
return "", err } - // Remove script and style elements + // Remove noise elements before extracting text doc.Find("script, style, nav, footer, header").Remove() - // Try to find article content + // Try article-shaped selectors first, then fall back to paragraphs, then body articleSelectors := []string{ "article", "main",@@ -348,7 +345,7 @@ }
} } - // Fallback: get all paragraphs + // Fallback: collect all paragraphs paragraphs := doc.Find("p") if paragraphs.Length() > 0 { var texts []string@@ -361,7 +358,7 @@ return text, nil
} } - // Last resort: get all text + // Last resort: raw body text text := doc.Text() text = strings.Join(strings.Fields(text), " ") if len(text) > 200 {
@@ -2,7 +2,7 @@ package aggregator
import "time" -// Article represents an article extracted from an RSS feed. +// Article represents an item extracted from an RSS feed. type Article struct { Title string `json:"title"` Link string `json:"link"`@@ -13,7 +13,8 @@ SourceFeed string `json:"source_feed,omitempty"`
Summary string `json:"summary,omitempty"` } -// ToMap converts article to a map for serialisation. +// ToMap serialises the article to a map for LLM input or JSON output. +// Content is truncated to 500 chars to avoid oversized prompts. func (a *Article) ToMap() map[string]any { content := a.Content if len(content) > 500 {
@@ -13,7 +13,7 @@ Version string
BuildDate string ) -// Args represents command-line arguments. +// Args defines all command-line flags supported by the program. type Args struct { // Feed input FeedsFile string `arg:"-f,--feeds-file" help:"Path to file containing RSS feed URLs (one per line)"`@@ -51,7 +51,7 @@ ShowVersion bool `arg:"--version" help:"Show version"`
DryRun bool `arg:"-D,--dry-run" help:"Validate config, show article statistics, and exit without making LLM API calls"` } -// Version returns the version string. +// Version returns the version string (e.g. "llm_aggregator v0.1.0 (built 2025-01-01)"). func (a *Args) Version() string { return fmt.Sprintf("llm_aggregator v%s (built %s)", Version, BuildDate) }@@ -61,7 +61,8 @@ func (a *Args) Description() string {
return "LLM Aggregator - Aggregate RSS feeds and summarise with LLM API" } -// ParseKeywords parses comma-separated keywords string into list. +// ParseKeywords splits a comma-separated keyword string into a trimmed list. +// Empty tokens resulting from malformed input are discarded. func ParseKeywords(keywordString string) []string { if keywordString == "" { return nil@@ -106,13 +107,13 @@
return &args, nil } -// ToViperMap converts Args to a map for binding to Viper. -// Only non-nil values (explicitly provided CLI flags) are included. +// ToViperMap serialises Args to a viper key-value map. +// Only fields with non-nil/non-empty values are included, giving CLI args the +// highest precedence over config file and environment variables. // -// NOTE: FeedsFile and Prompt are strings (not pointers), so they're ALWAYS -// included if non-empty. This differs from optional fields which use pointer -// types to detect explicit provision vs. default zero values. See isZero() in -// config package. +// NOTE: FeedsFile and Prompt are plain strings — they are always included when +// non-empty, unlike optional pointer-typed fields where nil means "not provided". +// This difference is intentional; see isZero in the config package. func (a *Args) ToViperMap() map[string]any { m := map[string]any{} if a.FeedsFile != "" {
@@ -15,7 +15,8 @@ "llm_aggregator/internal/runtime"
"llm_aggregator/internal/style" ) -// Config holds the application configuration loaded from TOML file, environment variables, and CLI arguments. +// Config holds application configuration sourced from TOML, environment variables, and CLI arguments. +// See config.GetViper for precedence and singleton behaviour. type Config struct { // Feed aggregation options MaxArticlesPerFeed int `mapstructure:"max_articles_per_feed"`@@ -45,13 +46,12 @@ IncludeArticles bool `mapstructure:"include_articles"`
Plain bool `mapstructure:"plain"` } -// GetViper returns the global viper instance with all configuration sources set up. -// The precedence order is: CLI flags > Environment variables > Config file > Defaults. +// GetViper returns the global viper instance. +// Precedence (highest first): CLI flags → environment variables → config file → defaults. // -// NOTE: Viper uses a singleton pattern. Subsequent calls to GetViper() return the SAME -// instance with all previously set values. This means defaults are only set once, -// environment vars are bound once, and the config file is read once. Do NOT call this -// expecting a fresh state — if you need a fresh instance for testing, use viper.New() directly. +// NOTE: Viper is a singleton. Subsequent calls return the SAME instance with all +// previously set values. Defaults, env bindings, and config-file reads are idempotent +// but the instance is NOT reset between calls. Use viper.New() directly for testing. func GetViper() *viper.Viper { // Set defaults first (lowest priority) setDefaults()@@ -117,13 +117,12 @@ }
} } -// isZero checks if a value is the zero value for its type. +// isZero reports whether v is the zero value for its type. // -// NOTE: This function is critical for CLI argument handling. Pointer types -// (*string, *int, etc.) return true only when nil (not provided), NOT when -// pointing to a zero value. This allows distinguishing between "flag not -// provided" (nil) vs "flag explicitly set to zero" (e.g., --temperature 0 -// should override default, but no flag means preserve default). +// This is critical for CLI handling: pointer types (*string, *int, …) return true +// when nil (flag not provided), not when dereferencing to zero. This distinguishes +// "--temperature 0" (explicit zero, overrides default) from no flag at all (nil, +// preserves default from config/env). func isZero(v any) bool { // Handle nil interface values first (before type switch) if v == nil {@@ -171,7 +170,8 @@ return false
} } -// ViperToRuntime converts viper configuration directly to runtime.Runtime. +// ViperToRuntime converts viper configuration into a Runtime value. +// FeedsFile and Prompt are passed separately as they come from positional CLI args. func ViperToRuntime(v *viper.Viper, feedsFile, prompt string) *runtime.Runtime { rt := &runtime.Runtime{ FeedsFile: feedsFile,
@@ -1,10 +1,7 @@
package defaults -// Default values for the llm_aggregator application. -// -// These constants are used throughout the codebase to ensure -// consistent default values across CLI arguments, configuration, -// and runtime settings. +// Default values used throughout the application. +// Kept in one place so they are consistent across CLI, config, and runtime. const ( // DefaultModel is the default LLM model.
@@ -25,13 +25,8 @@ llmTimeout int // seconds; 0 means no timeout
logger *progress.Context } -// NewLLMClient creates a new LLM client. -// apiKey: LLM API key (or read from LLM_AGGREGATOR_API_KEY env var) -// baseURL: API base URL (defaults to "https://api.deepseek.com") -// model: Model to use (defaults to "deepseek-chat") -// maxTokens: Maximum tokens in response (defaults to 4000) -// temperature: Sampling temperature (0.0 to 1.0, defaults to 0.7) -// timeoutSeconds: Request timeout in seconds (defaults to 300) +// NewLLMClient creates an LLM API client. +// Set apiKey to "" to read from LLM_AGGREGATOR_API_KEY. func NewLLMClient(apiKey, baseURL, model string, maxTokens int, temperature float64, timeoutSeconds int) (*LLMClient, error) { // Get API key from parameter or environment variable if apiKey == "" {@@ -83,17 +78,15 @@ func (dc *LLMClient) SetLogger(logger *progress.Context) {
dc.logger = logger } -// TokenUsage holds token usage information from the API +// TokenUsage holds token usage information from the API response. type TokenUsage struct { PromptTokens int CompletionTokens int } -// SummariseArticles summarises a list of articles based on user prompt. -// articles: List of article maps -// userPrompt: User's query/summarisation request -// systemPrompt: Optional system prompt (defaults to helpful assistant) -// ctx: Context for cancellation. If cancelled, the LLM API call aborts. +// SummariseArticles sends articles to the LLM for summarisation. +// Returns the LLM response text, token usage, and any error. +// ctx must carry signal cancellation so that SIGINT/SIGTERM aborts the call. func (dc *LLMClient) SummariseArticles( articles []map[string]any, userPrompt string,
@@ -19,7 +19,8 @@ excludeKeywords []string
logger *progress.Context } -// NewContentProcessor creates a new ContentProcessor with the specified options. +// NewContentProcessor creates a processor that filters and truncates articles. +// Keyword comparison is case-insensitive. func NewContentProcessor(maxTotalArticles, maxContentPerArticle int, filterKeywords, excludeKeywords []string) *ContentProcessor { // Convert keywords to lowercase for case-insensitive matching filterLower := make([]string, len(filterKeywords))@@ -45,7 +46,8 @@ func (cp *ContentProcessor) SetLogger(logger *progress.Context) {
cp.logger = logger } -// ProcessArticles processes articles: filter, sort, and prepare for LLM. +// ProcessArticles applies keyword filtering, sorting, and a ceiling on total count, +// then converts each article to a map for the LLM. func (cp *ContentProcessor) ProcessArticles(articles []*aggregator.Article, sortBy string, reverse bool) []map[string]any { if len(articles) == 0 { if cp.logger != nil {@@ -78,6 +80,8 @@
return processed } +// filterArticles applies include/exclude keyword filters to the article list. +// Exclusions take priority over inclusions. If no filters are set, all articles pass. func (cp *ContentProcessor) filterArticles(articles []*aggregator.Article) []*aggregator.Article { if len(cp.filterKeywords) == 0 && len(cp.excludeKeywords) == 0 { return articles@@ -88,7 +92,6 @@
for _, article := range articles { include := true - // Check if article should be excluded if len(cp.excludeKeywords) > 0 { articleText := strings.ToLower(article.Title + " " + article.Content) for _, keyword := range cp.excludeKeywords {@@ -102,7 +105,6 @@ }
} } - // Check if article should be included (only if we have inclusion filters) if include && len(cp.filterKeywords) > 0 { articleText := strings.ToLower(article.Title + " " + article.Content) include = false@@ -134,13 +136,12 @@ if len(articles) == 0 {
return articles } - // Create a copy to avoid modifying the original slice sortedArticles := make([]*aggregator.Article, len(articles)) copy(sortedArticles, articles) - // Define sort functions switch strings.ToLower(sortBy) { case "date": + // Zero times sort to the end when reverse=false (oldest last) sort.Slice(sortedArticles, func(i, j int) bool { iTime := sortedArticles[i].Published jTime := sortedArticles[j].Published@@ -174,7 +175,7 @@ }
return iSource < jSource }) default: - // Default to date sorting + // Unknown sort key: fall back to date order sort.Slice(sortedArticles, func(i, j int) bool { iTime := sortedArticles[i].Published jTime := sortedArticles[j].Published@@ -224,8 +225,8 @@
return processed } -// EstimateTokenCount estimates token count for articles using tiktoken. -// This is the accurate method using OpenAI's tokenisation. +// EstimateTokenCount uses tiktoken to estimate total token count across all articles. +// Falls back to char÷4 on encoding errors (logged as warnings). func (cp *ContentProcessor) EstimateTokenCount(articles []map[string]any, model string) int { totalTokens := 0
@@ -7,14 +7,15 @@
"llm_aggregator/internal/style" ) -// Logger provides logging interface for the aggregator +// Logger is the minimal interface for progress and verbose logging implementations. type Logger interface { Logf(format string, args ...any) Warningf(format string, args ...any) Debugf(format string, args ...any) } -// Progress provides progress reporting interface for TUI +// Progress extends Logger with stage/count/timing reporting. +// Implementations: NoopLogger (no output), SimpleLogger (stdout), TUIProgress (tea messages). type Progress interface { Logger SetStage(stage string)@@ -24,13 +25,13 @@ SetTokenEstimate(total, used int)
StartWaiting() } -// SimpleLogger implements Logger with io.Writer output +// SimpleLogger writes formatted output to an io.Writer. type SimpleLogger struct { writer io.Writer debug bool } -// NewSimpleLogger creates a new SimpleLogger +// NewSimpleLogger creates a SimpleLogger; set debug=true to enable Debugf output. func NewSimpleLogger(writer io.Writer, debug bool) *SimpleLogger { return &SimpleLogger{ writer: writer,@@ -60,7 +61,7 @@ func (sl *SimpleLogger) SetArticleCount(total, processed int) {}
func (sl *SimpleLogger) SetTokenEstimate(total, used int) {} func (sl *SimpleLogger) StartWaiting() {} -// NoopLogger implements Logger with no output +// NoopLogger discards all output. Used as the default when no progress is desired. type NoopLogger struct{} func (nl *NoopLogger) Logf(format string, args ...any) {}@@ -72,12 +73,12 @@ func (nl *NoopLogger) SetArticleCount(total, processed int) {}
func (nl *NoopLogger) SetTokenEstimate(total, used int) {} func (nl *NoopLogger) StartWaiting() {} -// Context provides progress context for components +// Context wraps a Logger and guards against nil receiver. type Context struct { logger Logger } -// NewContext creates a new progress context +// NewContext wraps a Logger. Passing nil produces a no-op context. func NewContext(logger Logger) *Context { return &Context{ logger: logger,
@@ -16,7 +16,8 @@ "llm_aggregator/internal/processor"
"llm_aggregator/internal/progress" ) -// Runtime holds the execution context for the aggregator +// Runtime holds the full execution context for the pipeline. +// It is constructed once in main() and passed through each stage. type Runtime struct { // Configuration FeedsFile string@@ -46,11 +47,13 @@ Summary string
Error error Interrupted bool // Set to true when a termination signal is received - // Logger for verbose output + // Progress is the logger/progress handler injected by the caller. + // nil means no output (NoopLogger). SimpleLogger outputs to stdout. + // TUIProgress bridges into the Bubbletea program for live updates. Progress progress.Progress } -// Execute runs the full aggregation pipeline +// Execute runs the four-stage pipeline: aggregate → process → LLM → (output is separate). func (r *Runtime) Execute(ctx context.Context) error { // The logger/progress handler is injected, so we don't create it here. // We wrap it in a context to pass to sub-modules that expect a *progress.Context.@@ -154,9 +157,8 @@ r.Progress.SetStage("Getting summary")
r.Progress.SetSubStage(fmt.Sprintf("Sending %d articles to LLM", len(processedArticles))) r.Progress.StartWaiting() - // Derive a timeout context for the LLM call from the parent context. - // The parent context carries signal cancellation, so both signal interrupts - // and timeouts will abort the call. + // callCtx wraps the parent context with a timeout derived from LLMTimeout. + // Both signal cancellation and timeout will abort the LLM call. callCtx := ctx if r.LLMTimeout > 0 { var cancel context.CancelFunc@@ -181,7 +183,7 @@ }
return nil } -// WasInterrupted returns true if a termination signal was received during execution. +// was interrupted by a signal during execution. func (r *Runtime) WasInterrupted() bool { return r.Interrupted }
@@ -8,7 +8,7 @@
"github.com/pkoukk/tiktoken-go" ) -// Encoding names for different model families +// Encoding names accepted by tiktoken for common model families. const ( EncodingCl100kBase = "cl100k_base" // GPT-4, GPT-3.5-turbo, embeddings EncodingO200kBase = "o200k_base" // GPT-4o, GPT-4.1, GPT-4.5@@ -24,7 +24,7 @@ }
var cache = &tokenizerCache{encodings: make(map[string]*tiktoken.Tiktoken)} -// GetEncoding returns a Tiktoken encoding for the given encoding name. +// GetEncoding returns a cached Tiktoken encoding, initialising it if necessary. func GetEncoding(encodingName string) (*tiktoken.Tiktoken, error) { // Check cache first cache.mu.RLock()@@ -48,7 +48,8 @@
return enc, nil } -// EncodingForModel returns the appropriate encoding name for a given model. +// EncodingForModel maps a model name to the nearest tiktoken encoding. +// Unknown model families fall back to cl100k_base. func EncodingForModel(model string) (string, error) { model = strings.ToLower(model)@@ -87,7 +88,7 @@ // Default to cl100k_base (most common for modern models)
return EncodingCl100kBase, nil } -// CountTokens counts the tokens in a text string using the appropriate encoding. +// CountTokens returns the number of tokens in text for the given model. func CountTokens(text string, model string) (int, error) { encodingName, err := EncodingForModel(model) if err != nil {
@@ -159,9 +159,8 @@
return tea.Batch(m.spinner.Tick, m.startProcessing) } -// This is now the standard bubbletea command pattern. -// The bubbletea runtime will execute this function in a goroutine. -// We no longer need to manage the goroutine or use program.Send(). +// startProcessing is the standard bubbletea command pattern. +// The runtime executes in a goroutine managed by the bubbletea runtime. func (m *Model) startProcessing() tea.Msg { err := m.runtime.Execute(context.Background()) return processingDoneMsg{err: err}@@ -226,7 +225,7 @@ }
m.viewport.SetContent(summaryContent) } case progressMsg: - // This message type seems unused, but leaving it in case. + // progressMsg drives the progress bar; sent by the runtime via program.Send. progress := float64(msg) if progress >= 1.0 { m.done = true@@ -235,7 +234,7 @@ }
cmd := m.progress.SetPercent(float64(progress)) return m, cmd case spinner.TickMsg: - // MODIFIED: Stop updating the spinner once we're done. + // Stop the spinner once processing completes if m.done { return m, nil }
@@ -8,13 +8,12 @@
tea "github.com/charmbracelet/bubbletea" ) -// TUIProgress implements the progress.Progress interface for TUI +// TUIProgress bridges progress.Progress calls into a running Bubbletea tea.Program. type TUIProgress struct { program *tea.Program } -// NewTUIProgress creates a new TUIProgress. -// It now accepts the program instance instead of creating one. +// NewTUIProgress wraps an existing tea.Program instance (created by the caller). func NewTUIProgress(p *tea.Program) *TUIProgress { return &TUIProgress{ program: p,@@ -56,7 +55,7 @@ func (tp *TUIProgress) Warningf(format string, args ...any) {
tp.Logf("⚠️ Warning: "+format, args...) } -// Debugf does nothing in the TUI. +// Debugf is a no-op in the TUI; debug output is handled via the progress bar. func (tp *TUIProgress) Debugf(format string, args ...any) {} // Run runs the TUI