Normalize briefing module options and weather stories

This commit is contained in:
2026-08-13 00:53:31 +00:00
parent 730929e2ed
commit 9b4e53702b
12 changed files with 188 additions and 19 deletions

View File

@@ -305,16 +305,20 @@ func (b *bundleBuilder) fetchDiscussion(ctx context.Context) error {
func (b *bundleBuilder) fetchWeatherStory(ctx context.Context) error {
var story weatherdata.WeatherStory
fetched, ok, err := b.fetchDecodedSource(ctx, sourceRequest{
request := sourceRequest{
name: config.MissingSourceWeatherStory,
endpoint: "/weatherstories/latest",
query: queryOptions{omitUnits: true},
missingMessage: "NWS weather story data is missing",
}, &story)
}
fetched, ok, err := b.fetchDecodedSource(ctx, request, &story)
if err != nil || !ok {
return err
}
source := fetched.source
if !story.HasUsableContent() {
return b.handleMalformed(&source, fmt.Errorf("weather story has no usable content"), request)
}
if !story.StartTime.IsZero() {
source.IssuedAt = &story.StartTime
}

View File

@@ -768,6 +768,44 @@ func TestMalformedWeatherStoryUsesPolicy(t *testing.T) {
}
}
func TestEmptyWeatherStoryUsesPolicy(t *testing.T) {
for _, tt := range []struct {
name string
policy config.MissingSourcePolicy
wantErr bool
}{
{name: "warn", policy: config.MissingSourceWarn},
{name: "error", policy: config.MissingSourceError, wantErr: true},
} {
t.Run(tt.name, func(t *testing.T) {
server := fixtureServer(t, map[string]handlerOverride{
"/weatherstories/latest": {status: http.StatusOK, body: `{"data": {}}`},
}, nil)
client := newTestClient(t, server.URL+"/", map[string]config.MissingSourcePolicy{
"weather_story": tt.policy,
})
bundle, err := client.FetchBundle(context.Background())
if tt.wantErr {
if err == nil || !strings.Contains(err.Error(), "weather story has no usable content") {
t.Fatalf("FetchBundle() error = %v, want unusable weather story error", err)
}
return
}
if err != nil {
t.Fatalf("FetchBundle() error = %v", err)
}
if bundle.WeatherStory != nil {
t.Fatalf("WeatherStory = %#v, want nil for empty source", bundle.WeatherStory)
}
source := sourceByName(t, bundle.Sources, "weather_story")
if !source.Missing || len(source.Warnings) != 1 || source.Warnings[0].Code != "malformed_source" {
t.Fatalf("weather_story source = %#v, want malformed source warning", source)
}
})
}
}
func TestContextCancellation(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
<-r.Context().Done()

View File

@@ -462,19 +462,58 @@ func TestAreaForecastDiscussionModuleCanSelectSections(t *testing.T) {
registry := MustDefaultModuleRegistry()
ctx := testModuleContext()
output, err := registry.BuildModule(ctx, module.ConfigItem{
ID: module.AreaForecastDiscussion,
Options: module.AreaForecastDiscussionOptions{Sections: []string{"short_term"}},
})
for _, tt := range []struct {
name string
options any
}{
{name: "value", options: module.AreaForecastDiscussionOptions{Sections: []string{"short_term"}}},
{name: "pointer", options: &module.AreaForecastDiscussionOptions{Sections: []string{"short_term"}}},
} {
t.Run(tt.name, func(t *testing.T) {
output, err := registry.BuildModule(ctx, module.ConfigItem{
ID: module.AreaForecastDiscussion,
Options: tt.options,
})
if err != nil {
t.Fatalf("BuildModule() error = %v", err)
}
afd := moduleValue[AreaForecastDiscussionModule](t, output)
if afd.ShortTerm != "Showers increase this afternoon." {
t.Fatalf("ShortTerm = %q, want selected short term section", afd.ShortTerm)
}
if afd.Product != "" || len(afd.KeyMessages) != 0 || afd.LongTerm != "" {
t.Fatalf("AFD = %#v, want only short_term section", afd)
}
})
}
}
func TestWeatherStoryModuleOmitsEmptyContent(t *testing.T) {
registry := MustDefaultModuleRegistry()
ctx := testModuleContext()
ctx.Collected.WeatherStory = &weatherdata.WeatherStory{OfficeID: "LSX", Priority: true, Order: 1}
output, err := registry.BuildModule(ctx, module.ConfigItem{ID: module.WeatherStory})
if err != nil {
t.Fatalf("BuildModule() error = %v", err)
}
afd := moduleValue[AreaForecastDiscussionModule](t, output)
if afd.ShortTerm != "Showers increase this afternoon." {
t.Fatalf("ShortTerm = %q, want selected short term section", afd.ShortTerm)
if output != nil {
t.Fatalf("output = %#v, want omitted weather story", output)
}
if afd.Product != "" || len(afd.KeyMessages) != 0 || afd.LongTerm != "" {
t.Fatalf("AFD = %#v, want only short_term section", afd)
}
func TestWeatherStoryModulePreservesZeroPriorityAndOrder(t *testing.T) {
registry := MustDefaultModuleRegistry()
ctx := testModuleContext()
ctx.Collected.WeatherStory = &weatherdata.WeatherStory{Title: "Rain Chances"}
output, err := registry.BuildModule(ctx, module.ConfigItem{ID: module.WeatherStory})
if err != nil {
t.Fatalf("BuildModule() error = %v", err)
}
story := moduleValue[WeatherStoryModule](t, output)
if !story.Available || story.Priority || story.Order != 0 {
t.Fatalf("WeatherStory = %#v, want available story with zero priority and order", story)
}
}

View File

@@ -100,7 +100,8 @@ func (r ModuleRegistry) BuildModule(ctx ModuleContext, item module.ConfigItem) (
if !definition.SupportsReport(ctx.Resolved.Definition.ID) {
return nil, fmt.Errorf("module %q is not compatible with report %q", item.ID, ctx.Resolved.Definition.ID)
}
if err := definition.ValidateOptions(item.Options); err != nil {
options, err := definition.CanonicalOptions(item.Options)
if err != nil {
return nil, err
}
if definition.Builder == nil {
@@ -120,7 +121,6 @@ func (r ModuleRegistry) BuildModule(ctx ModuleContext, item module.ConfigItem) (
return nil, fmt.Errorf("module %q has unknown missing data behavior %q", item.ID, definition.MissingData)
}
}
options := item.Options
if options == nil {
options = definition.DefaultOptions
}
@@ -264,6 +264,24 @@ func (d ModuleDefinition) ValidateOptions(options any) error {
return fmt.Errorf("module %q options have type %s, want %s", d.ID, got, want)
}
// CanonicalOptions validates options and converts an accepted typed pointer to its value form.
func (d ModuleDefinition) CanonicalOptions(options any) (any, error) {
if err := d.ValidateOptions(options); err != nil {
return nil, err
}
if options == nil {
return nil, nil
}
value := reflect.ValueOf(options)
if value.Kind() != reflect.Pointer {
return options, nil
}
if value.IsNil() {
return nil, fmt.Errorf("module %q options must not be a nil pointer", d.ID)
}
return value.Elem().Interface(), nil
}
func defaultModuleDefinitions() []ModuleDefinition {
allReports := []report.ID{report.Daily, report.Today, report.Tomorrow, report.Hourly}
daypartReports := []report.ID{report.Daily, report.Today, report.Tomorrow}

View File

@@ -458,6 +458,7 @@ func TestModuleRegistryAcceptsTypedOptions(t *testing.T) {
err := registry.ValidateComposition(report.Daily, []module.ConfigItem{
{ID: module.Metadata, Options: module.MetadataOptions{}},
{ID: module.CurrentConditions, Options: &module.CurrentConditionsOptions{}},
{ID: module.AreaForecastDiscussion, Options: &module.AreaForecastDiscussionOptions{Sections: []string{"short_term"}}},
})
if err != nil {
t.Fatalf("ValidateComposition() error = %v", err)

View File

@@ -23,7 +23,7 @@ type WeatherStoryModule struct {
func buildWeatherStoryModule(ctx ModuleContext, _ any) (*module.Output, error) {
story := ctx.Collected.WeatherStory
if story == nil {
if story == nil || !story.HasUsableContent() {
return nil, nil
}
period := timeutil.Period{Start: story.StartTime, End: story.EndTime}

View File

@@ -563,6 +563,35 @@ func TestReportModuleOverridesNormalizesConstructedOptionsWithoutMutatingConfig(
}
}
func TestReportModuleOverridesCanonicalizesTypedPointerOptions(t *testing.T) {
cfg := Defaults()
options := &module.AreaForecastDiscussionOptions{Sections: []string{"short_term"}}
cfg.Reports = map[string]ReportConfig{
"daily": {
DeterministicModules: []ModuleConfigItem{
{ID: module.Metadata},
{ID: module.AreaForecastDiscussion, Options: options},
},
deterministicModulesSet: true,
},
}
overrides, err := cfg.ReportModuleOverrides()
if err != nil {
t.Fatalf("ReportModuleOverrides() error = %v", err)
}
got, ok := overrides[report.Daily][1].Options.(module.AreaForecastDiscussionOptions)
if !ok {
t.Fatalf("override options type = %T, want AreaForecastDiscussionOptions", overrides[report.Daily][1].Options)
}
if !reflect.DeepEqual(got, *options) {
t.Fatalf("override options = %#v, want %#v", got, *options)
}
if cfg.Reports["daily"].DeterministicModules[1].Options != options {
t.Fatalf("config options = %#v, want original pointer %#v", cfg.Reports["daily"].DeterministicModules[1].Options, options)
}
}
func TestValidateReportModuleAliasesDirectly(t *testing.T) {
retiredDailyKey := retiredDailyReportKeyForTest()
tests := []struct {

View File

@@ -180,7 +180,7 @@ func normalizeModuleOptions(registry briefing.ModuleRegistry, id module.ID, raw
return nil, fmt.Errorf("module %q does not accept options", id)
}
if err := definition.ValidateOptions(raw); err == nil {
return raw, nil
return definition.CanonicalOptions(raw)
}
optionType := reflect.TypeOf(definition.DefaultOptions)
normalized, err := decodeKnownOptions(raw, optionType)

View File

@@ -4,6 +4,7 @@ package weatherdata
import (
"encoding/json"
"math"
"strings"
"time"
)
@@ -180,6 +181,15 @@ type WeatherStory struct {
DownloadURL string `json:"downloadUrl,omitempty"`
}
// HasUsableContent reports whether the weather story contains displayable content or a valid period.
func (s WeatherStory) HasUsableContent() bool {
return strings.TrimSpace(s.Title) != "" ||
strings.TrimSpace(s.Description) != "" ||
strings.TrimSpace(s.AltText) != "" ||
strings.TrimSpace(s.DownloadURL) != "" ||
(!s.StartTime.IsZero() && !s.EndTime.IsZero() && s.EndTime.After(s.StartTime))
}
type ConvectiveOutlookRun struct {
LocationID string `json:"locationId,omitempty"`
LocationName string `json:"locationName,omitempty"`

View File

@@ -117,6 +117,31 @@ func TestForecastPeriodHasValidPrecipitationProbability(t *testing.T) {
}
}
func TestWeatherStoryHasUsableContent(t *testing.T) {
start := mustParseBundleTime("2026-05-29T13:00:00Z")
end := mustParseBundleTime("2026-05-29T14:00:00Z")
for _, tt := range []struct {
name string
story WeatherStory
want bool
}{
{name: "empty"},
{name: "whitespace title", story: WeatherStory{Title: " \t "}},
{name: "title", story: WeatherStory{Title: "Rain Chances"}, want: true},
{name: "description", story: WeatherStory{Description: "Showers expected."}, want: true},
{name: "alternate text", story: WeatherStory{AltText: "Rain chances graphic"}, want: true},
{name: "download URL", story: WeatherStory{DownloadURL: "https://example.invalid/story.png"}, want: true},
{name: "valid period", story: WeatherStory{StartTime: start, EndTime: end}, want: true},
{name: "reversed period", story: WeatherStory{StartTime: end, EndTime: start}},
} {
t.Run(tt.name, func(t *testing.T) {
if got := tt.story.HasUsableContent(); got != tt.want {
t.Fatalf("HasUsableContent() = %t, want %t", got, tt.want)
}
})
}
}
func mustParseBundleTime(value string) time.Time {
parsed, err := time.Parse(time.RFC3339, value)
if err != nil {