Implement remaining cleanup items prior to the next release
All checks were successful
ci/woodpecker/push/build-image Pipeline was successful

This commit is contained in:
2026-06-11 10:17:49 -05:00
parent 8041f99782
commit 5d7f604a2c
9 changed files with 171 additions and 308 deletions

View File

@@ -1,82 +0,0 @@
# Code Quality Audit Snapshot
## Purpose
This roadmap file records the current cleanup audit status for `weatherfeeder`.
It is not current-behavior documentation and should not be used as the source of
truth for implemented features. Current behavior belongs in README, config,
operations, internal, consumer, and integration docs.
## Current Assessment
`weatherfeeder` remains architecturally coherent:
- `cmd/weatherfeeder` is runtime composition.
- Source adapters fetch upstream provider payloads.
- Provider helpers isolate provider-specific parsing.
- Normalizers map raw schemas into canonical `model` payloads.
- `standards` owns schema, kind, and WMO constants.
- Postgres sink mapping is isolated under `internal/sinks/postgres`.
- Feedkit owns generic config, scheduling, processing, dispatch, and sink
mechanics.
The highest-value completed cleanup work since the original audit includes:
- event kind constants in `standards`;
- source driver constants in provider source packages;
- source registry tests derived from registered drivers;
- shared HTTP config parsing for multi-document sources;
- config/example load coverage;
- shallow documentation consistency tests for current schemas and source
drivers;
- consumer documentation for public Go packages.
## Remaining Cleanup Opportunities
### Postgres Mapper Boilerplate
The Postgres mapper still repeats event envelope values and parent-row map
patterns across canonical product families.
Recommended next action: implement the Postgres mapper envelope cleanup in
[`cleanup.md`](cleanup.md) without changing table contracts or mapper behavior.
### Package-Local Fixture Helpers
Some tests still use package-local fixture-loading helpers that may be
consolidated where multiple files in the same package duplicate the same logic.
Recommended next action: consolidate only obvious same-package duplication. Do
not add cross-package test helper packages.
### Literal Sweep
Some string literals remain intentional in docs, YAML examples, and negative
tests. Internal code and tests can still be reviewed for opportunities to use
existing constants where that reduces drift risk.
Recommended next action: perform a final literal sweep only after higher-value
cleanup stages are complete.
## Non-Issues
These areas were reviewed and should not be refactored without a new roadmap:
- Normalizer `Match` methods are intentionally explicit schema checks.
- Provider-specific time parsers should remain provider-specific.
- SPC multi-document polling should remain source-local and atomic.
- The current runtime does not need a generic workflow engine.
- The Postgres sink should not become an ORM or reflection mapper.
- Documentation should remain audience-specific rather than generated from code.
## Validation Baseline
The current test suite covers source construction, normalizer registration and
mapping, Postgres schema/mapping, maintained config loading, source-driver docs,
and event-schema docs.
Recommended baseline after any cleanup:
```sh
go test ./...
```

View File

@@ -1,146 +0,0 @@
# Cleanup Roadmap
## Summary
This roadmap tracks remaining behavior-preserving cleanup opportunities for
`weatherfeeder`. Earlier cleanup stages for event kind constants, source driver
constants, registry-derived source tests, shared multi-document HTTP config, and
shallow docs consistency tests have been completed and are no longer listed as
future work.
The remaining work should be implemented only in small, focused changes that do
not alter public event schemas, event kinds, source driver names, config keys,
Postgres table contracts, or canonical JSON field names.
## Guardrails
- Keep `feedkit` as the generic daemon infrastructure boundary.
- Keep weather-domain behavior in sources, provider helpers, normalizers,
`model`, `standards`, and Postgres mapping.
- Keep current-behavior docs accurate and roadmap-only plans under
`docs/roadmap/`.
- Do not introduce plugin systems, workflow engines, generic source frameworks,
ORM-style mapping, reflection mapping, generated docs, or persistence tags on
canonical model structs.
- Run focused tests after each stage and `go test ./...` after all stages.
## Stage 1: Reduce Postgres Mapper Envelope Duplication
Centralize repeated parent event envelope mapping without changing the table
contract.
Implementation requirements:
- Add small helpers inside `internal/sinks/postgres` for parent envelope values:
`event_id`, `event_kind`, `event_source`, `event_schema`,
`event_emitted_at`, and `event_effective_at`.
- Use the helper in every parent table mapper that stores event envelope
columns.
- Keep explicit per-product mapper functions and product-specific validation.
- Optionally centralize envelope column declarations only if `schema.go` remains
easy to scan.
- Preserve every table, column, nullability rule, required-field check, compact
JSON behavior, UTC normalization, child positional index, and write count.
Acceptance criteria:
- Mapper output for existing valid payloads is equivalent before and after the
refactor.
- Unsupported schemas still map to zero writes and no error.
- Required-field failures still include useful product/path context.
Focused tests:
```sh
go test ./internal/sinks/postgres
```
## Stage 2: Consolidate Package-Local Fixture Helpers
Reduce low-value duplicated fixture-reading code only where it is local and
obvious.
Implementation requirements:
- Consolidate duplicated fixture readers only within the same Go package.
- Keep fixtures under each package's `testdata` directory.
- Do not create a cross-package test utility package.
- Do not change fixture contents unless an existing test already requires it.
- Do not mix this cleanup with parser behavior changes.
Acceptance criteria:
- Local test helper duplication is reduced where multiple files in one package
already share the same fixture-reading behavior.
- Tests remain easy to read locally.
- No package imports a helper solely for tests from another package.
Focused tests:
```sh
go test ./internal/providers/nws ./internal/providers/spc
go test ./internal/sources/nws ./internal/sources/spc
go test ./internal/normalizers/nws ./internal/normalizers/spc
```
## Stage 3: Dead-Code And Literal Sweep
Perform a final cleanup sweep after mapper and fixture cleanup.
Implementation requirements:
- Search for stale internal driver, kind, and schema literals.
- Replace internal code/test literals with constants where it reduces typo or
drift risk.
- Keep user-facing docs and YAML examples literal.
- Keep intentional legacy-driver negative tests.
- Do not remove compatibility tests unless they are clearly obsolete.
- Do not broaden the cleanup into unrelated refactors.
Suggested searches:
```sh
rg 'event\.Kind\("|nws_forecast|nws_weatherstories|openmeteo_|openweather_|spc_convective_outlook|weather_story|forecast_discussion|raw\.|weather\.' .
rg 'TODO|legacy|deprecated|unknown source driver' internal cmd docs examples
```
Acceptance criteria:
- Internal literals are reduced where constants already exist.
- Intentional literals in docs, YAML examples, raw schema docs, and negative
tests remain readable.
- No behavior changes are introduced.
Focused tests:
```sh
go test ./internal/sources ./internal/normalizers/... ./internal/sinks/postgres ./cmd/weatherfeeder
```
## Final Verification
After all stages are complete, run:
```sh
go test ./...
git status --short
```
Verify current-behavior docs still describe implemented behavior only and no
public contracts changed unintentionally.
## Refactors To Avoid
Do not perform these changes as part of this cleanup roadmap:
- Generic workflow or stage engine.
- Runtime plugin system.
- Weather-specific constants in `feedkit`.
- Replacing feedkit scheduler, dispatch, HTTP helpers, or sink mechanics.
- Generic source abstraction covering every source type.
- Reflection-based or generated Postgres mapper.
- ORM-style persistence layer.
- Database column metadata on canonical model structs.
- Cross-provider timestamp parser that hides provider-specific formats.
- Broad WMO mapper consolidation beyond existing common text fallback.
- Generated documentation system.

View File

@@ -2,8 +2,6 @@ package spc
import (
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"
"time"
@@ -344,16 +342,6 @@ func geoJSONFixtureForProduct(t *testing.T, key string) []byte {
}
}
func readSPCTestFixture(t *testing.T, name string) []byte {
t.Helper()
path := filepath.Join("..", "..", "providers", "spc", "testdata", name)
raw, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read fixture %s: %v", path, err)
}
return raw
}
func findOutlook(outlooks []model.WeatherOutlook, day int, outlookType string) *model.WeatherOutlook {
for i := range outlooks {
if outlooks[i].Day == day && outlooks[i].OutlookType == outlookType {

View File

@@ -0,0 +1,17 @@
package spc
import (
"os"
"path/filepath"
"testing"
)
func readSPCTestFixture(t *testing.T, name string) []byte {
t.Helper()
path := filepath.Join("..", "..", "providers", "spc", "testdata", name)
raw, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read fixture %s: %v", path, err)
}
return raw
}

View File

@@ -528,6 +528,56 @@ func TestMapPostgresEventForecastDiscussionMalformedPayload(t *testing.T) {
}
}
func TestParentEventValuesAddsEnvelopeAndPreservesProductValues(t *testing.T) {
emittedAt := time.Date(2026, 3, 16, 13, 31, 0, 0, time.FixedZone("CDT", -5*60*60))
effectiveAt := time.Date(2026, 3, 16, 13, 30, 0, 0, time.FixedZone("CDT", -5*60*60))
event := fkevent.Event{
ID: "evt-envelope",
Kind: fkevent.Kind(standards.KindForecast),
Source: "test-source",
Schema: standards.SchemaWeatherForecastV1,
EmittedAt: emittedAt,
EffectiveAt: &effectiveAt,
}
got := parentEventValues(event, map[string]any{"product_col": "product-value"})
assertParentEnvelopeValues(t, got, event)
if got["product_col"] != "product-value" {
t.Fatalf("product_col = %#v, want product-value", got["product_col"])
}
}
func TestParentEventValuesNullEffectiveAt(t *testing.T) {
base := fkevent.Event{
ID: "evt-envelope",
Kind: fkevent.Kind(standards.KindObservation),
Source: "test-source",
Schema: standards.SchemaWeatherObservationV1,
EmittedAt: time.Date(2026, 3, 16, 18, 31, 0, 0, time.UTC),
}
for _, tt := range []struct {
name string
mut func(*fkevent.Event)
}{
{name: "nil", mut: func(*fkevent.Event) {}},
{name: "zero", mut: func(event *fkevent.Event) {
zero := time.Time{}
event.EffectiveAt = &zero
}},
} {
t.Run(tt.name, func(t *testing.T) {
event := base
tt.mut(&event)
got := parentEventValues(event, nil)
if got["event_effective_at"] != nil {
t.Fatalf("event_effective_at = %#v, want nil", got["event_effective_at"])
}
})
}
}
func testEvent(schema string, kind fkevent.Kind, payload any) fkevent.Event {
effectiveAt := time.Date(2026, 3, 16, 18, 30, 0, 0, time.UTC)
return fkevent.Event{
@@ -541,6 +591,30 @@ func testEvent(schema string, kind fkevent.Kind, payload any) fkevent.Event {
}
}
func assertParentEnvelopeValues(t *testing.T, values map[string]any, event fkevent.Event) {
t.Helper()
if got := values["event_id"]; got != event.ID {
t.Fatalf("event_id = %#v, want %q", got, event.ID)
}
if got := values["event_kind"]; got != string(event.Kind) {
t.Fatalf("event_kind = %#v, want %q", got, event.Kind)
}
if got := values["event_source"]; got != event.Source {
t.Fatalf("event_source = %#v, want %q", got, event.Source)
}
if got := values["event_schema"]; got != event.Schema {
t.Fatalf("event_schema = %#v, want %q", got, event.Schema)
}
if got := values["event_emitted_at"]; got != event.EmittedAt.UTC() {
t.Fatalf("event_emitted_at = %#v, want %s", got, event.EmittedAt.UTC())
}
wantEffective := nullableTime(event.EffectiveAt)
if got := values["event_effective_at"]; got != wantEffective {
t.Fatalf("event_effective_at = %#v, want %#v", got, wantEffective)
}
}
func firstWriteForTable(writes []fksinks.PostgresWrite, table string) (fksinks.PostgresWrite, bool) {
for _, w := range writes {
if w.Table == table {

View File

@@ -26,13 +26,7 @@ func PostgresSchema() fksinks.PostgresSchema {
Tables: []fksinks.PostgresTable{
{
Name: tableObservations,
Columns: []fksinks.PostgresColumn{
{Name: "event_id", Type: "TEXT", Nullable: false},
{Name: "event_kind", Type: "TEXT", Nullable: false},
{Name: "event_source", Type: "TEXT", Nullable: false},
{Name: "event_schema", Type: "TEXT", Nullable: false},
{Name: "event_emitted_at", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "event_effective_at", Type: "TIMESTAMPTZ", Nullable: true},
Columns: parentEnvelopeColumns([]fksinks.PostgresColumn{
{Name: "station_id", Type: "TEXT", Nullable: true},
{Name: "station_name", Type: "TEXT", Nullable: true},
{Name: "observed_at", Type: "TIMESTAMPTZ", Nullable: false},
@@ -48,7 +42,7 @@ func PostgresSchema() fksinks.PostgresSchema {
{Name: "visibility_meters", Type: "DOUBLE PRECISION", Nullable: true},
{Name: "relative_humidity_percent", Type: "DOUBLE PRECISION", Nullable: true},
{Name: "apparent_temperature_c", Type: "DOUBLE PRECISION", Nullable: true},
},
}...),
PrimaryKey: []string{"event_id"},
PruneColumn: "observed_at",
Indexes: []fksinks.PostgresIndex{
@@ -73,13 +67,7 @@ func PostgresSchema() fksinks.PostgresSchema {
},
{
Name: tableForecasts,
Columns: []fksinks.PostgresColumn{
{Name: "event_id", Type: "TEXT", Nullable: false},
{Name: "event_kind", Type: "TEXT", Nullable: false},
{Name: "event_source", Type: "TEXT", Nullable: false},
{Name: "event_schema", Type: "TEXT", Nullable: false},
{Name: "event_emitted_at", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "event_effective_at", Type: "TIMESTAMPTZ", Nullable: true},
Columns: parentEnvelopeColumns([]fksinks.PostgresColumn{
{Name: "location_id", Type: "TEXT", Nullable: true},
{Name: "location_name", Type: "TEXT", Nullable: true},
{Name: "issued_at", Type: "TIMESTAMPTZ", Nullable: false},
@@ -89,7 +77,7 @@ func PostgresSchema() fksinks.PostgresSchema {
{Name: "longitude", Type: "DOUBLE PRECISION", Nullable: true},
{Name: "elevation_meters", Type: "DOUBLE PRECISION", Nullable: true},
{Name: "period_count", Type: "INTEGER", Nullable: false},
},
}...),
PrimaryKey: []string{"event_id"},
PruneColumn: "issued_at",
Indexes: []fksinks.PostgresIndex{
@@ -137,13 +125,7 @@ func PostgresSchema() fksinks.PostgresSchema {
},
{
Name: tableForecastDiscussions,
Columns: []fksinks.PostgresColumn{
{Name: "event_id", Type: "TEXT", Nullable: false},
{Name: "event_kind", Type: "TEXT", Nullable: false},
{Name: "event_source", Type: "TEXT", Nullable: false},
{Name: "event_schema", Type: "TEXT", Nullable: false},
{Name: "event_emitted_at", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "event_effective_at", Type: "TIMESTAMPTZ", Nullable: true},
Columns: parentEnvelopeColumns([]fksinks.PostgresColumn{
{Name: "office_id", Type: "TEXT", Nullable: true},
{Name: "office_name", Type: "TEXT", Nullable: true},
{Name: "issued_at", Type: "TIMESTAMPTZ", Nullable: false},
@@ -156,7 +138,7 @@ func PostgresSchema() fksinks.PostgresSchema {
{Name: "long_term_issued_at", Type: "TIMESTAMPTZ", Nullable: true},
{Name: "long_term_text", Type: "TEXT", Nullable: true},
{Name: "key_message_count", Type: "INTEGER", Nullable: false},
},
}...),
PrimaryKey: []string{"event_id"},
PruneColumn: "issued_at",
Indexes: []fksinks.PostgresIndex{
@@ -180,17 +162,11 @@ func PostgresSchema() fksinks.PostgresSchema {
},
{
Name: tableWeatherStoryRuns,
Columns: []fksinks.PostgresColumn{
{Name: "event_id", Type: "TEXT", Nullable: false},
{Name: "event_kind", Type: "TEXT", Nullable: false},
{Name: "event_source", Type: "TEXT", Nullable: false},
{Name: "event_schema", Type: "TEXT", Nullable: false},
{Name: "event_emitted_at", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "event_effective_at", Type: "TIMESTAMPTZ", Nullable: true},
Columns: parentEnvelopeColumns([]fksinks.PostgresColumn{
{Name: "office_id", Type: "TEXT", Nullable: true},
{Name: "as_of", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "story_count", Type: "INTEGER", Nullable: false},
},
}...),
PrimaryKey: []string{"event_id"},
PruneColumn: "as_of",
Indexes: []fksinks.PostgresIndex{
@@ -225,20 +201,14 @@ func PostgresSchema() fksinks.PostgresSchema {
},
{
Name: tableAlertRuns,
Columns: []fksinks.PostgresColumn{
{Name: "event_id", Type: "TEXT", Nullable: false},
{Name: "event_kind", Type: "TEXT", Nullable: false},
{Name: "event_source", Type: "TEXT", Nullable: false},
{Name: "event_schema", Type: "TEXT", Nullable: false},
{Name: "event_emitted_at", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "event_effective_at", Type: "TIMESTAMPTZ", Nullable: true},
Columns: parentEnvelopeColumns([]fksinks.PostgresColumn{
{Name: "location_id", Type: "TEXT", Nullable: true},
{Name: "location_name", Type: "TEXT", Nullable: true},
{Name: "as_of", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "latitude", Type: "DOUBLE PRECISION", Nullable: true},
{Name: "longitude", Type: "DOUBLE PRECISION", Nullable: true},
{Name: "alert_count", Type: "INTEGER", Nullable: false},
},
}...),
PrimaryKey: []string{"event_id"},
PruneColumn: "as_of",
Indexes: []fksinks.PostgresIndex{
@@ -301,13 +271,7 @@ func PostgresSchema() fksinks.PostgresSchema {
},
{
Name: tableOutlookRuns,
Columns: []fksinks.PostgresColumn{
{Name: "event_id", Type: "TEXT", Nullable: false},
{Name: "event_kind", Type: "TEXT", Nullable: false},
{Name: "event_source", Type: "TEXT", Nullable: false},
{Name: "event_schema", Type: "TEXT", Nullable: false},
{Name: "event_emitted_at", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "event_effective_at", Type: "TIMESTAMPTZ", Nullable: true},
Columns: parentEnvelopeColumns([]fksinks.PostgresColumn{
{Name: "location_id", Type: "TEXT", Nullable: true},
{Name: "location_name", Type: "TEXT", Nullable: true},
{Name: "latitude", Type: "DOUBLE PRECISION", Nullable: true},
@@ -315,7 +279,7 @@ func PostgresSchema() fksinks.PostgresSchema {
{Name: "as_of", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "issued_at", Type: "TIMESTAMPTZ", Nullable: true},
{Name: "outlook_count", Type: "INTEGER", Nullable: false},
},
}...),
PrimaryKey: []string{"event_id"},
PruneColumn: "as_of",
Indexes: []fksinks.PostgresIndex{
@@ -362,3 +326,15 @@ func PostgresSchema() fksinks.PostgresSchema {
MapEvent: mapPostgresEvent,
}
}
func parentEnvelopeColumns(extra ...fksinks.PostgresColumn) []fksinks.PostgresColumn {
columns := []fksinks.PostgresColumn{
{Name: "event_id", Type: "TEXT", Nullable: false},
{Name: "event_kind", Type: "TEXT", Nullable: false},
{Name: "event_source", Type: "TEXT", Nullable: false},
{Name: "event_schema", Type: "TEXT", Nullable: false},
{Name: "event_emitted_at", Type: "TIMESTAMPTZ", Nullable: false},
{Name: "event_effective_at", Type: "TIMESTAMPTZ", Nullable: true},
}
return append(columns, extra...)
}

View File

@@ -1,8 +1,11 @@
package postgres
import (
"reflect"
"strings"
"testing"
fksinks "gitea.maximumdirect.net/ejr/feedkit/sinks"
)
func TestWeatherPostgresSchemaShape(t *testing.T) {
@@ -88,6 +91,28 @@ func TestWeatherPostgresSchemaIncludesWeatherStoryColumns(t *testing.T) {
}
}
func TestWeatherPostgresSchemaParentTablesStartWithEnvelopeColumns(t *testing.T) {
for _, table := range []string{
tableObservations,
tableForecasts,
tableForecastDiscussions,
tableWeatherStoryRuns,
tableAlertRuns,
tableOutlookRuns,
} {
t.Run(table, func(t *testing.T) {
columns := orderedColumnsForTable(t, table)
want := parentEnvelopeColumns()
if len(columns) < len(want) {
t.Fatalf("%s has %d columns, want at least %d", table, len(columns), len(want))
}
if !reflect.DeepEqual(columns[:len(want)], want) {
t.Fatalf("%s envelope prefix = %#v, want %#v", table, columns[:len(want)], want)
}
})
}
}
func assertTablePrimaryKey(t *testing.T, table string, want []string) {
t.Helper()
for _, tbl := range PostgresSchema().Tables {
@@ -121,20 +146,26 @@ func assertTableIndex(t *testing.T, table string, name string, want []string) {
t.Fatalf("missing table %q", table)
}
func columnsForTable(t *testing.T, table string) map[string]bool {
func orderedColumnsForTable(t *testing.T, table string) []fksinks.PostgresColumn {
t.Helper()
schema := PostgresSchema()
for _, tbl := range schema.Tables {
if tbl.Name != table {
continue
if tbl.Name == table {
return tbl.Columns
}
cols := make(map[string]bool, len(tbl.Columns))
for _, col := range tbl.Columns {
cols[col.Name] = true
}
return cols
}
t.Fatalf("missing table %q", table)
return nil
}
func columnsForTable(t *testing.T, table string) map[string]bool {
t.Helper()
ordered := orderedColumnsForTable(t, table)
cols := make(map[string]bool, len(ordered))
for _, col := range ordered {
cols[col.Name] = true
}
return cols
}

View File

@@ -4,8 +4,6 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"strings"
"testing"
"time"
@@ -340,16 +338,6 @@ func discussionFixtureForProduct(t *testing.T, key string) []byte {
}
}
func readSPCTestFixture(t *testing.T, name string) []byte {
t.Helper()
path := filepath.Join("..", "..", "providers", "spc", "testdata", name)
raw, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read fixture %s: %v", path, err)
}
return raw
}
const testRSS = `<?xml version="1.0"?>
<rss version="2.0">
<channel>

View File

@@ -0,0 +1,17 @@
package spc
import (
"os"
"path/filepath"
"testing"
)
func readSPCTestFixture(t *testing.T, name string) []byte {
t.Helper()
path := filepath.Join("..", "..", "providers", "spc", "testdata", name)
raw, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read fixture %s: %v", path, err)
}
return raw
}