From 07e1dbfcc8462ab99c63050b1b5c28c129245c20 Mon Sep 17 00:00:00 2001 From: eleith Date: Thu, 4 Sep 2025 05:03:57 +0000 Subject: [PATCH] refactor(config): Simplify digest grouping with view parameter Replaces the `group_by`, `sub_group_by`, and `sort_by` parameters with a single `view` parameter in the digest configuration. The new `view` parameter can be set to one of three values: - `date`: Groups by category, then by date. - `category`: Groups by category, then by feed. - `ai`: Groups by category, then uses the LLM to create sub-groups. The default view is `date`. This change simplifies the configuration for the user and makes the digest generation logic easier to maintain. --- config.yaml.example | 4 +- internal/app/app_test.go | 2 +- internal/app/services/digest.go | 2 +- internal/config/config.go | 16 ++-- internal/config/config_test.go | 9 +- internal/digest/digest.go | 18 +++- internal/digest/digest_test.go | 118 ++++++++++++++++++--------- internal/processor/processor.go | 4 +- internal/processor/processor_test.go | 12 ++- scripts/preview/main.go | 8 +- 10 files changed, 120 insertions(+), 73 deletions(-) diff --git a/config.yaml.example b/config.yaml.example index 97828b7..9f5d8ad 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -17,9 +17,7 @@ digest: compress: true mark_as_read: true run_on_startup: false - group_by: "category" # choices: category, feed - sub_group_by: "day" # choices: date, feed, ai - sort_by: "date" # choices: date, title, ai + view: "date" # choices: date, category, ai ai: api_key: "YOUR_GEMINI_API_KEY" diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 7be24f6..f78e36c 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -40,7 +40,7 @@ func (m *mockMinifluxClientService) UpdateEntries(entryIDs []int64, status strin type mockDigestService struct{} -func (m *mockDigestService) BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, groupBy string, subGroupBy string, sortBy string, minifluxHost string) *models.OverviewTemplateData { +func (m *mockDigestService) BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, view string, minifluxHost string) *models.OverviewTemplateData { return nil } diff --git a/internal/app/services/digest.go b/internal/app/services/digest.go index b2692f3..05229d2 100644 --- a/internal/app/services/digest.go +++ b/internal/app/services/digest.go @@ -5,5 +5,5 @@ import ( ) type DigestService interface { - BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, groupBy string, subGroupBy string, sortBy string, minifluxHost string) *models.OverviewTemplateData + BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, view string, minifluxHost string) *models.OverviewTemplateData } diff --git a/internal/config/config.go b/internal/config/config.go index 9435504..2a65344 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -45,11 +45,9 @@ type ConfigDigest struct { Email ConfigDigestEmail `koanf:"email"` Schedule string `koanf:"schedule" validate:"gocron"` Host string `koanf:"host"` - Compress bool `koanf:"compress"` - GroupBy string `koanf:"group_by" validate:"omitempty,oneof=category feed"` - SubGroupBy string `koanf:"sub_group_by" validate:"omitempty,oneof=date feed ai"` - SortBy string `koanf:"sort_by" validate:"omitempty,oneof=date ai"` - MarkAsRead bool `koanf:"mark_as_read"` + Compress bool `koanf:"compress"` + View string `koanf:"view" validate:"oneof=date category ai"` + MarkAsRead bool `koanf:"mark_as_read"` RunOnStartup bool `koanf:"run_on_startup"` } @@ -74,8 +72,8 @@ func (c *Config) Validate() error { validate.RegisterStructValidation(func(sl validator.StructLevel) { cfg := sl.Current().Interface().(Config) - if cfg.Digest.SubGroupBy == "ai" && cfg.AI.ApiKey == "" { - sl.ReportError(cfg.AI.ApiKey, "AI.ApiKey", "ApiKey", "required_if", "Digest.SubGroupBy is 'ai'") + if cfg.Digest.View == "ai" && cfg.AI.ApiKey == "" { + sl.ReportError(cfg.AI.ApiKey, "AI.ApiKey", "ApiKey", "required_if", "Digest.View is 'ai'") } }, Config{}) @@ -119,9 +117,7 @@ func Load(path string) (*Config, error) { func setDefaultValues(k *koanf.Koanf) error { return k.Load(confmap.Provider(map[string]any{ "digest.compress": true, - "digest.group_by": "category", - "digest.sub_group_by": "feed", - "digest.sort_by": "date", + "digest.view": "date", "digest.schedule": "@weekly", "digest.mark_as_read": true, "digest.run_on_startup": false, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index cba9863..ed3c656 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -199,8 +199,9 @@ func TestLoad(t *testing.T) { }, wantErr: true, }, + { - name: "invalid group_by value", + name: "invalid view value", config: map[string]any{ "miniflux": map[string]any{ "host": "miniflux.example.com", @@ -208,13 +209,13 @@ func TestLoad(t *testing.T) { }, "digest": map[string]any{ "schedule": "@daily", - "group_by": "magic", + "view": "magic", }, }, wantErr: true, }, { - name: "missing ai.api_key when sub_group_by is ai", + name: "missing ai.api_key when view is ai", config: map[string]any{ "miniflux": map[string]any{ "host": "miniflux.example.com", @@ -222,7 +223,7 @@ func TestLoad(t *testing.T) { }, "digest": map[string]any{ "schedule": "@daily", - "sub_group_by": "ai", + "view": "ai", }, }, wantErr: true, diff --git a/internal/digest/digest.go b/internal/digest/digest.go index a51aaf0..7018ccd 100644 --- a/internal/digest/digest.go +++ b/internal/digest/digest.go @@ -56,12 +56,28 @@ type digestServiceImpl struct { llmService services.LLMService } -func (s *digestServiceImpl) BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, groupBy string, subGroupBy string, sortBy string, minifluxHost string) *models.OverviewTemplateData { +func (s *digestServiceImpl) BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, view string, minifluxHost string) *models.OverviewTemplateData { iconsSlice := make([]*models.FeedIcon, 0, len(icons)) for _, icon := range icons { iconsSlice = append(iconsSlice, icon) } + var groupBy, subGroupBy string + switch view { + case "date": + groupBy = "category" + subGroupBy = "date" + case "category": + groupBy = "category" + subGroupBy = "feed" + case "ai": + groupBy = "category" + subGroupBy = "ai" + default: // also "category" + groupBy = "category" + subGroupBy = "feed" + } + primaryGroups := GroupEntries(entries, groupBy) grouper := NewSubGrouper(subGroupBy, s.llmService) allPrimaryGroups, overallDigestSummary := grouper.GroupEntries(primaryGroups) diff --git a/internal/digest/digest_test.go b/internal/digest/digest_test.go index 60ffea7..6c51a44 100644 --- a/internal/digest/digest_test.go +++ b/internal/digest/digest_test.go @@ -150,8 +150,21 @@ func TestLLMGrouper_GroupEntries(t *testing.T) { } } -func TestDigestService_BuildDigestData_NonAI(t *testing.T) { - mockLLM := &mockLLMService{} +func TestDigestService_BuildDigestData(t *testing.T) { + mockLLM := &mockLLMService{ + GenerateContentFunc: func(ctx context.Context, prompt string, schema *genai.Schema) ([]byte, error) { + return []byte(`{ + "overview": "This is a summary of all entries.", + "primary_group_summaries": [], + "sub_groups": [ + { + "title": "Sub-group 1", + "entry_ids": [1, 2] + } + ] + }`), nil + }, + } digestService := NewDigestService(mockLLM) entries := testutil.CreateMockEntries(0) @@ -160,41 +173,72 @@ func TestDigestService_BuildDigestData_NonAI(t *testing.T) { 102: {FeedID: 102, Data: "iconB"}, } - overviewData := digestService.BuildDigestData(entries, icons, "category", "day", "date", "http://miniflux.test") + t.Run("view=date", func(t *testing.T) { + overviewData := digestService.BuildDigestData(entries, icons, "date", "http://miniflux.test") + if overviewData == nil { + t.Fatal("overviewData is nil") + } + if len(overviewData.PrimaryGroups) != 3 { + t.Errorf("Expected 3 primary groups, got %d", len(overviewData.PrimaryGroups)) + } + // Check for date-based sub-grouping + catAGroup := findPrimaryGroup(overviewData.PrimaryGroups, "Category A") + if catAGroup == nil || len(catAGroup.SubGroups) != 1 { + t.Fatalf("Incorrect sub-groups for Category A: %+v", catAGroup) + } + subGroup := findSubGroup(catAGroup, "Category A - Jan 2, 2024") + if subGroup == nil { + t.Fatalf("Did not find expected sub-group 'Category A - Jan 2, 2024'") + } + }) - if overviewData == nil { - t.Fatal("overviewData is nil") - } - if len(overviewData.PrimaryGroups) != 3 { - t.Errorf("Expected 3 primary groups, got %d", len(overviewData.PrimaryGroups)) - } -} + t.Run("view=category", func(t *testing.T) { + overviewData := digestService.BuildDigestData(entries, icons, "category", "http://miniflux.test") + if overviewData == nil { + t.Fatal("overviewData is nil") + } + if len(overviewData.PrimaryGroups) != 3 { + t.Errorf("Expected 3 primary groups, got %d", len(overviewData.PrimaryGroups)) + } + // Check for feed-based sub-grouping + catAGroup := findPrimaryGroup(overviewData.PrimaryGroups, "Category A") + if catAGroup == nil || len(catAGroup.SubGroups) != 3 { + t.Fatalf("Incorrect sub-groups for Category A: %+v", catAGroup) + } + }) -func TestDigestService_BuildDigestData_LLMFallback(t *testing.T) { - entries := testutil.CreateMockEntries(0) - icons := map[int64]*models.FeedIcon{ - 100: {FeedID: 100, Data: "icon100"}, - 200: {FeedID: 200, Data: "icon200"}, - } + t.Run("view=ai", func(t *testing.T) { + // Note: Using different entries for AI test for simplicity + aiEntries := createLLMGrouperMockEntries() + overviewData := digestService.BuildDigestData(aiEntries, icons, "ai", "http://miniflux.test") + if overviewData == nil { + t.Fatal("overviewData is nil") + } + if overviewData.OverviewSummary != "This is a summary of all entries." { + t.Errorf("Expected summary, got %q", overviewData.OverviewSummary) + } + if len(overviewData.PrimaryGroups) != 1 { + t.Errorf("Expected 1 primary group, got %d", len(overviewData.PrimaryGroups)) + } + }) - mockLLM := &mockLLMService{ - GenerateContentFunc: func(ctx context.Context, prompt string, schema *genai.Schema) ([]byte, error) { - return nil, errors.New("LLM service error during test") - }, - } - digestService := NewDigestService(mockLLM) - - overviewData := digestService.BuildDigestData(entries, icons, "category", "ai", "date", "http://miniflux.test") - - if overviewData == nil { - t.Fatal("overviewData is nil") - } - - if overviewData.OverviewSummary != "" { - t.Errorf("Expected OverviewSummary to be empty, got %q", overviewData.OverviewSummary) - } - - if len(overviewData.PrimaryGroups) != 3 { // Category A, Category B, Category C - t.Fatalf("Expected 3 primary groups after fallback, got %d", len(overviewData.PrimaryGroups)) - } -} + t.Run("view=ai fallback", func(t *testing.T) { + llmErrorService := NewDigestService(&mockLLMService{ + GenerateContentFunc: func(ctx context.Context, prompt string, schema *genai.Schema) ([]byte, error) { + return nil, errors.New("LLM service error") + }, + }) + overviewData := llmErrorService.BuildDigestData(entries, icons, "ai", "http://miniflux.test") + if overviewData == nil { + t.Fatal("overviewData is nil") + } + if overviewData.OverviewSummary != "" { + t.Errorf("Expected empty summary on fallback, got %q", overviewData.OverviewSummary) + } + // On fallback, it should behave like view=category + catAGroup := findPrimaryGroup(overviewData.PrimaryGroups, "Category A") + if catAGroup == nil || len(catAGroup.SubGroups) != 3 { + t.Fatalf("Incorrect sub-groups for Category A on fallback: %+v", catAGroup) + } + }) +} \ No newline at end of file diff --git a/internal/processor/processor.go b/internal/processor/processor.go index 4ce605b..77cb4ad 100644 --- a/internal/processor/processor.go +++ b/internal/processor/processor.go @@ -35,9 +35,7 @@ func ProcessDigest(application *app.App) (*os.File, []*os.File, *models.Overview data := application.DigestService.BuildDigestData( entries, icons, - application.Config.Digest.GroupBy, - application.Config.Digest.SubGroupBy, - application.Config.Digest.SortBy, + application.Config.Digest.View, application.Config.Miniflux.Host, ) diff --git a/internal/processor/processor_test.go b/internal/processor/processor_test.go index 381c284..4d2d087 100644 --- a/internal/processor/processor_test.go +++ b/internal/processor/processor_test.go @@ -30,11 +30,11 @@ func (m *mockMinifluxClientService) UpdateEntries(entryIDs []int64, status strin } type mockDigestService struct { - buildDigestDataFunc func(entries []*models.Entry, icons map[int64]*models.FeedIcon, groupBy string, subGroupBy string, sortBy string, minifluxHost string) *models.OverviewTemplateData + buildDigestDataFunc func(entries []*models.Entry, icons map[int64]*models.FeedIcon, view string, minifluxHost string) *models.OverviewTemplateData } -func (m *mockDigestService) BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, groupBy string, subGroupBy string, sortBy string, minifluxHost string) *models.OverviewTemplateData { - return m.buildDigestDataFunc(entries, icons, groupBy, subGroupBy, sortBy, minifluxHost) +func (m *mockDigestService) BuildDigestData(entries []*models.Entry, icons map[int64]*models.FeedIcon, view string, minifluxHost string) *models.OverviewTemplateData { + return m.buildDigestDataFunc(entries, icons, view, minifluxHost) } type mockArchiveService struct { @@ -78,7 +78,7 @@ func TestProcessDigest_Success(t *testing.T) { }, } mockDigestService := &mockDigestService{ - buildDigestDataFunc: func(entries []*models.Entry, icons map[int64]*models.FeedIcon, groupBy string, subGroupBy string, sortBy string, minifluxHost string) *models.OverviewTemplateData { + buildDigestDataFunc: func(entries []*models.Entry, icons map[int64]*models.FeedIcon, view string, minifluxHost string) *models.OverviewTemplateData { return expectedOverviewData }, } @@ -95,9 +95,7 @@ func TestProcessDigest_Success(t *testing.T) { Digest: config.ConfigDigest{ MarkAsRead: true, Compress: true, - GroupBy: "category", - SubGroupBy: "feed", - SortBy: "date", + View: "category", }, Miniflux: config.ConfigMiniflux{Host: "http://miniflux.test"}, }), diff --git a/scripts/preview/main.go b/scripts/preview/main.go index f7e5212..f52122b 100644 --- a/scripts/preview/main.go +++ b/scripts/preview/main.go @@ -82,9 +82,7 @@ func generateMockDigest(cfg *config.Config) *models.OverviewTemplateData { data := digestSvc.BuildDigestData( entries, icons, - cfg.Digest.GroupBy, - cfg.Digest.SubGroupBy, - cfg.Digest.SortBy, + cfg.Digest.View, cfg.Miniflux.Host, ) @@ -135,9 +133,7 @@ func generateMinifluxDigest(cfg *config.Config, minifluxClientService services.M return digestSvc.BuildDigestData( entries, icons, - cfg.Digest.GroupBy, - cfg.Digest.SubGroupBy, - cfg.Digest.SortBy, + cfg.Digest.View, cfg.Miniflux.Host, ) } -- 2.54.0