From 41b86109e30f3929a198dada6fb65af4c8df0cae Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 13 Aug 2026 01:08:20 +0000 Subject: [PATCH] Correct daypart identity and display handling --- docs/config.md | 7 ++ docs/internal/briefing.md | 6 + .../derived_daypart_summaries_module.go | 18 +-- internal/briefing/derived_modules_test.go | 105 ++++++++++++++++++ internal/briefing/precip_timing_module.go | 2 +- internal/briefing/summary_helpers.go | 40 +++++-- internal/config/config_test.go | 34 ++++++ internal/config/validate.go | 7 ++ internal/generatedtext/render_context_test.go | 29 +++++ 9 files changed, 224 insertions(+), 24 deletions(-) diff --git a/docs/config.md b/docs/config.md index 2ab6500..f4e5570 100644 --- a/docs/config.md +++ b/docs/config.md @@ -223,6 +223,13 @@ derivation. Every item needs `name`, `start`, and `end`; start and end use (`06:00`–`10:00`), `midday` (`10:00`–`15:00`), `afternoon` (`15:00`–`17:00`), and `evening` (`17:00`–`24:00`). +Names remain display text, but each name must have a distinct canonical +identity. Canonicalization trims whitespace, lowercases letters, and collapses +punctuation and whitespace to underscores; for example, `Morning`, +`morning!`, and `morning` conflict. Planning recognizes the canonical +identities `morning`, `afternoon`, `evening`, and `overnight` regardless of +their display capitalization or punctuation. + ### `reports` `reports` optionally overrides a report's ordered deterministic modules and diff --git a/docs/internal/briefing.md b/docs/internal/briefing.md index df380ac..c5b21dc 100644 --- a/docs/internal/briefing.md +++ b/docs/internal/briefing.md @@ -46,6 +46,12 @@ missing sources, and prompt-safe SPC values omit geometry and other template-only or source details. The complete module composition is in [module internals](module.md); fact derivation is in [fact contracts](facts.md). +Derived daypart-summary maps use the forecast package's canonical daypart +identity and reject any collision instead of replacing an earlier value. +Planning applies the same identity when recognizing morning, afternoon, +evening, and overnight windows; display labels remain separate and preserve +configured text with rune-safe first-letter capitalization. + `area_forecast_discussion` accepts an optional typed section filter. Accepted typed option pointers are normalized to the declared value type before builder execution. Planning modules are report-specific: `daily_planning` supports Daily, diff --git a/internal/briefing/derived_daypart_summaries_module.go b/internal/briefing/derived_daypart_summaries_module.go index ef27344..e7f6503 100644 --- a/internal/briefing/derived_daypart_summaries_module.go +++ b/internal/briefing/derived_daypart_summaries_module.go @@ -4,7 +4,6 @@ import ( "fmt" "sort" "strings" - "unicode" "gitea.maximumdirect.net/eric/weatherreporter/internal/forecast" "gitea.maximumdirect.net/eric/weatherreporter/internal/module" @@ -80,6 +79,9 @@ func buildDerivedDaypartSummariesModule(ctx ModuleContext, _ any) (*module.Outpu prefixDates := multipleSummaryDates(ctx.Derived.DailySummaries) for _, daypart := range ctx.Derived.DaypartSummaries { key := daypartKey(daypart, prefixDates) + if existing, exists := value[key]; exists { + return nil, fmt.Errorf("daypart summary key %q collides with display name %q", key, existing.DisplayName) + } value[key] = derivedDaypartSummaryValue(daypart, ctx.Timezone) } return &module.Output{ID: module.DerivedDaypartSummaries, StanzaName: "derived_daypart_summaries", Value: value}, nil @@ -135,7 +137,7 @@ func derivedDaypartSummaryValue(daypart forecast.DaypartSummary, timezone string temperature := daypartTemperatureDisplay(daypart) value := DerivedDaypartSummaryModule{ Date: localDateLabel(daypart.Period.Start, timezone), - DisplayName: titleWord(strings.TrimSpace(daypart.Name)), + DisplayName: capitalizeFirst(strings.TrimSpace(daypart.Name)), PeriodBegins: friendlyPeriodBeginsLabel(daypart.Period, timezone), PeriodEnds: friendlyPeriodEndsLabel(daypart.Period, timezone), TempRangeF: rangeLabel(daypart.Temperature), @@ -148,7 +150,7 @@ func derivedDaypartSummaryValue(daypart forecast.DaypartSummary, timezone string ApparentTempRangeF: daypartApparentRangeLabel(daypart.ApparentTemperature), DominantCondition: daypart.DominantCondition, DominantConditionLower: strings.ToLower(daypart.DominantCondition), - DominantConditionDisplay: sentenceCase(daypart.DominantCondition), + DominantConditionDisplay: capitalizeFirst(strings.TrimSpace(daypart.DominantCondition)), NotableConditions: append([]string(nil), daypart.NotableConditions...), Snow: daypart.Indicators.Snow, Ice: daypart.Indicators.Ice, @@ -381,16 +383,6 @@ func temperatureBandQualifier(remainder int) string { } } -func sentenceCase(value string) string { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - return "" - } - runes := []rune(trimmed) - runes[0] = unicode.ToUpper(runes[0]) - return string(runes) -} - func multipleSummaryDates(summaries []forecast.DailySummary) bool { seen := map[string]struct{}{} for _, summary := range summaries { diff --git a/internal/briefing/derived_modules_test.go b/internal/briefing/derived_modules_test.go index eba8c14..fba3fe1 100644 --- a/internal/briefing/derived_modules_test.go +++ b/internal/briefing/derived_modules_test.go @@ -355,6 +355,52 @@ func TestDerivedDaypartSummariesExposeConfiguredKeysAndHazards(t *testing.T) { } } +func TestDerivedDaypartSummariesRejectCanonicalKeyCollisions(t *testing.T) { + registry := MustDefaultModuleRegistry() + ctx := derivedModuleContext(report.Daily) + first := ctx.Derived.DaypartSummaries[0] + first.Name = "Morning" + second := first + second.Name = "morning!" + ctx.Derived.DaypartSummaries = []forecast.DaypartSummary{first, second} + ctx.Derived.DailySummaries = []forecast.DailySummary{{Date: first.Period.Start.Format(timeutil.DateLayout)}} + + _, err := registry.BuildModule(ctx, module.ConfigItem{ID: module.DerivedDaypartSummaries}) + if err == nil || !strings.Contains(err.Error(), "collides") { + t.Fatalf("BuildModule() error = %v, want canonical daypart-key collision", err) + } +} + +func TestDerivedDaypartSummaryDisplayCapitalizesUnicodeNames(t *testing.T) { + value := derivedDaypartSummaryValue(forecast.DaypartSummary{ + Name: "mañana", + DominantCondition: "llovizna", + }, "UTC") + if value.DisplayName != "Mañana" || value.DominantConditionDisplay != "Llovizna" { + t.Fatalf("daypart display = %#v, want rune-safe capitalization", value) + } +} + +func TestDerivedDaypartSummariesKeepDistinctUnicodeKeys(t *testing.T) { + registry := MustDefaultModuleRegistry() + ctx := derivedModuleContext(report.Daily) + first := ctx.Derived.DaypartSummaries[0] + first.Name = "mañana" + second := first + second.Name = "manana" + ctx.Derived.DaypartSummaries = []forecast.DaypartSummary{first, second} + ctx.Derived.DailySummaries = []forecast.DailySummary{{Date: first.Period.Start.Format(timeutil.DateLayout)}} + + output, err := registry.BuildModule(ctx, module.ConfigItem{ID: module.DerivedDaypartSummaries}) + if err != nil { + t.Fatalf("BuildModule() error = %v", err) + } + value := moduleValue[map[string]DerivedDaypartSummaryModule](t, output) + if len(value) != 2 || value["mañana"].DisplayName != "Mañana" || value["manana"].DisplayName != "Manana" { + t.Fatalf("daypart summaries = %#v, want distinct Unicode canonical keys", value) + } +} + func TestDerivedDaypartSummariesPromptExportOmitsTemplateHelpers(t *testing.T) { registry := MustDefaultModuleRegistry() ctx := derivedModuleContext(report.Daily) @@ -670,6 +716,65 @@ func TestOutdoorWindowsScoreSnowIceAndFog(t *testing.T) { } } +func TestPlanningUsesCanonicalDaypartIdentities(t *testing.T) { + timedValue := func(value float64) *forecast.TimedValue { + return &forecast.TimedValue{Value: value} + } + containsText := func(values []string, text string) bool { + return strings.Contains(strings.Join(values, "\n"), text) + } + summary := &forecast.DailySummary{Dayparts: []forecast.DaypartSummary{ + {Name: "Overnight!", MaxPrecipitationProbability: timedValue(60)}, + {Name: "MORNING", MaxPrecipitationProbability: timedValue(60)}, + {Name: "Afternoon!!!", MaxPrecipitationProbability: timedValue(60)}, + {Name: "EVENING!", MaxPrecipitationProbability: timedValue(60)}, + }} + + today := buildTodayPlanning(summary) + if !containsText(today.MorningReadiness, "Morning precipitation chance peaks near 60%.") { + t.Fatalf("today morning readiness = %#v, want canonical morning window", today.MorningReadiness) + } + if !containsText(today.CommuteSchoolWorkdayConcerns, "Afternoon!!! precipitation chance reaches 60%.") || + containsText(today.CommuteSchoolWorkdayConcerns, "Overnight!") || + containsText(today.CommuteSchoolWorkdayConcerns, "EVENING!") { + t.Fatalf("today workday concerns = %#v, want only canonical workday windows", today.CommuteSchoolWorkdayConcerns) + } + if !containsText(today.LateDayChangeWatch, "Afternoon!!! precipitation timing may shift") || + !containsText(today.LateDayChangeWatch, "EVENING! precipitation timing may shift") { + t.Fatalf("today late-day watch = %#v, want canonical afternoon and evening windows", today.LateDayChangeWatch) + } + + base := buildMorningCommuteOvernightPlanning(summary) + if !containsText(base.MorningReadiness, "Morning precipitation chance peaks near 60%.") || + !containsText(base.OvernightChangeWatch, "Overnight precipitation timing may shift") || + containsText(base.CommuteSchoolWorkdayConcerns, "Overnight!") || + containsText(base.CommuteSchoolWorkdayConcerns, "EVENING!") { + t.Fatalf("daily/tomorrow planning = %#v, want canonical daypart treatment", base) + } +} + +func TestCapitalizeFirst(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {name: "empty", input: "", want: ""}, + {name: "ASCII", input: "morning", want: "Morning"}, + {name: "multibyte", input: "mañana", want: "Mañana"}, + {name: "already uppercase", input: "Morning", want: "Morning"}, + {name: "leading space", input: " morning", want: " morning"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := capitalizeFirst(tt.input); got != tt.want { + t.Fatalf("capitalizeFirst(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + func TestOutdoorWindowsAndTomorrowPlanningModulesPreserveDailyContent(t *testing.T) { registry := MustDefaultModuleRegistry() ctx := derivedModuleContext(report.Tomorrow) diff --git a/internal/briefing/precip_timing_module.go b/internal/briefing/precip_timing_module.go index 321d2bb..3229b19 100644 --- a/internal/briefing/precip_timing_module.go +++ b/internal/briefing/precip_timing_module.go @@ -100,7 +100,7 @@ func precipitationWindowExpectationPhrase(maxPopPercent int, precipitationType s case maxPopPercent >= precipTimingExpectLowerBound: return fmt.Sprintf("Expect %s.", precipitationType) case maxPopPercent >= precipTimingLikelyLowerBound: - return fmt.Sprintf("%s likely.", sentenceCase(precipitationType)) + return fmt.Sprintf("%s likely.", capitalizeFirst(strings.TrimSpace(precipitationType))) case maxPopPercent >= precipTimingChanceLowerBound: return fmt.Sprintf("Chance of %s.", precipitationType) default: diff --git a/internal/briefing/summary_helpers.go b/internal/briefing/summary_helpers.go index a0f6fe7..8373f46 100644 --- a/internal/briefing/summary_helpers.go +++ b/internal/briefing/summary_helpers.go @@ -5,6 +5,7 @@ import ( "math" "sort" "strings" + "unicode" "gitea.maximumdirect.net/eric/weatherreporter/internal/forecast" "gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil" @@ -43,6 +44,13 @@ type TodayPlanning struct { const outdoorIndicatorRiskScore = 25 +const ( + morningDaypartIdentity = "morning" + afternoonDaypartIdentity = "afternoon" + eveningDaypartIdentity = "evening" + overnightDaypartIdentity = "overnight" +) + func buildOutdoorWindows(dayparts []forecast.DaypartSummary) OutdoorWindows { var best *OutdoorWindow var worst *OutdoorWindow @@ -74,7 +82,7 @@ func buildTodayPlanning(summary *forecast.DailySummary) *TodayPlanning { } for _, daypart := range summary.Dayparts { - if daypart.Name == "overnight" || daypart.Name == "evening" { + if isOutsideWorkday(daypart) { continue } planning.CommuteSchoolWorkdayConcerns = appendUnique(planning.CommuteSchoolWorkdayConcerns, concernNotes(daypart)...) @@ -93,7 +101,7 @@ func buildTodayPlanning(summary *forecast.DailySummary) *TodayPlanning { planning.OutdoorPlanning = append(planning.OutdoorPlanning, "No standout outdoor weather constraints are evident in the available forecast.") } - for _, name := range []string{"afternoon", "evening"} { + for _, name := range []string{afternoonDaypartIdentity, eveningDaypartIdentity} { daypart := daypartNamed(summary.Dayparts, name) if daypart != nil { planning.LateDayChangeWatch = appendUnique(planning.LateDayChangeWatch, lateDayWatchNotes(*daypart)...) @@ -126,7 +134,7 @@ func buildMorningCommuteOvernightPlanning(summary *forecast.DailySummary) *morni } for _, daypart := range summary.Dayparts { - if daypart.Name == "overnight" || daypart.Name == "evening" { + if isOutsideWorkday(daypart) { continue } planning.CommuteSchoolWorkdayConcerns = appendUnique(planning.CommuteSchoolWorkdayConcerns, concernNotes(daypart)...) @@ -155,10 +163,10 @@ func outdoorPlanningNotes(dayparts []forecast.DaypartSummary) []string { windows := buildOutdoorWindows(dayparts) var notes []string if windows.Best != nil { - notes = append(notes, fmt.Sprintf("Best outdoor window: %s (%s).", titleWord(windows.Best.Daypart), strings.Join(windows.Best.Reasons, ", "))) + notes = append(notes, fmt.Sprintf("Best outdoor window: %s (%s).", capitalizeFirst(windows.Best.Daypart), strings.Join(windows.Best.Reasons, ", "))) } if windows.Worst != nil && (windows.Best == nil || windows.Worst.Daypart != windows.Best.Daypart) { - notes = append(notes, fmt.Sprintf("Toughest outdoor window: %s (%s).", titleWord(windows.Worst.Daypart), strings.Join(windows.Worst.Reasons, ", "))) + notes = append(notes, fmt.Sprintf("Toughest outdoor window: %s (%s).", capitalizeFirst(windows.Worst.Daypart), strings.Join(windows.Worst.Reasons, ", "))) } return appendUnique(nil, notes...) } @@ -185,7 +193,7 @@ func readinessNotes(daypart forecast.DaypartSummary) []string { func lateDayWatchNotes(daypart forecast.DaypartSummary) []string { notes := []string{} - prefix := titleWord(daypart.Name) + prefix := capitalizeFirst(daypart.Name) if prefix == "" { prefix = "Late-day" } @@ -206,7 +214,7 @@ func lateDayWatchNotes(daypart forecast.DaypartSummary) []string { func concernNotes(daypart forecast.DaypartSummary) []string { notes := []string{} - prefix := titleWord(daypart.Name) + prefix := capitalizeFirst(daypart.Name) if prefix == "" { prefix = "Daytime" } @@ -249,8 +257,9 @@ func overnightWatchNotes(daypart forecast.DaypartSummary) []string { } func daypartNamed(dayparts []forecast.DaypartSummary, name string) *forecast.DaypartSummary { + identity := forecast.CanonicalDaypartKey(name) for i := range dayparts { - if strings.EqualFold(dayparts[i].Name, name) { + if forecast.CanonicalDaypartKey(dayparts[i].Name) == identity { return &dayparts[i] } } @@ -397,9 +406,20 @@ func appendUnique(values []string, candidates ...string) []string { return values } -func titleWord(value string) string { +func isOutsideWorkday(daypart forecast.DaypartSummary) bool { + switch forecast.CanonicalDaypartKey(daypart.Name) { + case overnightDaypartIdentity, eveningDaypartIdentity: + return true + default: + return false + } +} + +func capitalizeFirst(value string) string { if value == "" { return "" } - return strings.ToUpper(value[:1]) + value[1:] + runes := []rune(value) + runes[0] = unicode.ToUpper(runes[0]) + return string(runes) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b5e3827..b5e65ae 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -82,6 +82,40 @@ func TestDefaults(t *testing.T) { } } +func TestDaypartNamesMustHaveDistinctCanonicalIdentities(t *testing.T) { + tests := []struct { + name string + names []string + wantErr string + }{ + {name: "exact duplicate", names: []string{"morning", "morning"}, wantErr: "conflicts with"}, + {name: "case-only duplicate", names: []string{"morning", "MORNING"}, wantErr: "conflicts with"}, + {name: "punctuation-normalized duplicate", names: []string{"morning", "morning!"}, wantErr: "conflicts with"}, + {name: "distinct Unicode names", names: []string{"mañana", "manana"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := Defaults() + cfg.Dayparts = make([]DaypartConfig, 0, len(tt.names)) + for _, name := range tt.names { + cfg.Dayparts = append(cfg.Dayparts, DaypartConfig{Name: name, Start: "06:00", End: "12:00"}) + } + + err := Validate(cfg) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("Validate() error = %v", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Validate() error = %v, want %q", err, tt.wantErr) + } + }) + } +} + func TestWeatherAPIBaseURLValidation(t *testing.T) { tests := []struct { name string diff --git a/internal/config/validate.go b/internal/config/validate.go index 65bbb94..29f5dc2 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -5,6 +5,7 @@ import ( "net/url" "strings" + "gitea.maximumdirect.net/eric/weatherreporter/internal/forecast" "gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil" ) @@ -74,10 +75,16 @@ func Validate(cfg Config) error { if len(cfg.Dayparts) == 0 { return fmt.Errorf("dayparts must contain at least one entry") } + daypartNames := make(map[string]int, len(cfg.Dayparts)) for i, daypart := range cfg.Dayparts { if strings.TrimSpace(daypart.Name) == "" { return fmt.Errorf("dayparts[%d].name is required", i) } + key := forecast.CanonicalDaypartKey(daypart.Name) + if previous, exists := daypartNames[key]; exists { + return fmt.Errorf("dayparts[%d].name %q conflicts with dayparts[%d].name after canonicalization", i, daypart.Name, previous) + } + daypartNames[key] = i if _, err := timeutil.ParseClock(daypart.Start); err != nil { return fmt.Errorf("dayparts[%d].start is invalid: %w", i, err) } diff --git a/internal/generatedtext/render_context_test.go b/internal/generatedtext/render_context_test.go index f856c7b..31ba403 100644 --- a/internal/generatedtext/render_context_test.go +++ b/internal/generatedtext/render_context_test.go @@ -244,6 +244,35 @@ func TestBuildTodayRenderContext(t *testing.T) { } } +func TestTodayRenderContextPreservesUnicodeDaypartDisplayNames(t *testing.T) { + metadata := testTodayMetadata() + snapshot := testTodaySnapshot(t) + for i := range snapshot.Outputs { + if snapshot.Outputs[i].ID != module.DerivedDaypartSummaries { + continue + } + dayparts := snapshot.Outputs[i].Value.(map[string]briefing.DerivedDaypartSummaryModule) + morning := dayparts["morning"] + morning.DisplayName = "Mañana" + dayparts["morning"] = morning + } + + ctx, err := BuildTodayRenderContext(metadata, snapshot, Today{ + Summary: "Today summary.", + ForecastDiscussion: []string{"Morning conditions stay quiet."}, + }, testCollected(), testTodayDerived()) + if err != nil { + t.Fatalf("BuildTodayRenderContext() error = %v", err) + } + rendered, err := reporttemplate.Render("today", ctx) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + if !strings.Contains(string(rendered), "Mañana") { + t.Fatalf("rendered report = %q, want Unicode daypart display name", rendered) + } +} + func TestBuildTodayRenderContextAllowsOmittedOptionalModules(t *testing.T) { snapshot, err := module.NewSnapshot(nil) if err != nil {