Files
weatherfeeder/docs/roadmap/cleanup.md
Eric Rakestraw dec05821bf
All checks were successful
ci/woodpecker/push/build-image Pipeline was successful
Add a staged cleanup roadmap to address the code quality audit
2026-06-10 21:02:07 -05:00

12 KiB

Cleanup Roadmap

Summary

This roadmap turns the findings in docs/roadmap/audit.md into staged, behavior-preserving cleanup work for weatherfeeder.

The goal is to reduce duplication and drift risk before the next major release without changing public contracts. Implement these stages in order. Each stage should be small enough for one focused implementation prompt or commit unless the implementing agent discovers unexpected coupling.

Preserve the current architecture:

  • feedkit remains generic daemon infrastructure.
  • weatherfeeder keeps weather-domain policy in sources, providers, normalizers, model, standards, and Postgres mapping.
  • Current-behavior docs should change only when implementation changes require them.
  • Roadmap-only cleanup instructions belong here until implemented.

Global Guardrails

  • Do not change public schema strings, event kind strings, source driver names, config keys, table names, column names, column nullability, or canonical model JSON field names.
  • Do not include database migrations; this is cleanup-only work.
  • Do not change feedkit in this cleanup pass.
  • Do not introduce broad framework abstractions, plugin systems, workflow engines, generic source frameworks, ORM-style mapping, reflection mapping, or persistence annotations in model.
  • Keep cmd/weatherfeeder focused on runtime composition.
  • Keep source fetching separate from normalizer mapping.
  • Keep provider-specific timestamp parsing and WMO mapping provider-specific unless an existing common helper already has exactly matching semantics.
  • Keep user-facing YAML examples and Markdown examples literal; do not generate docs.
  • Run focused tests after each stage and go test ./... after all stages.

Stage 1: Centralize Event Kind And Driver Constants

Add code constants for repeated weatherfeeder identifiers while preserving the literal string values.

Implementation requirements:

  • Add event kind string constants in standards, for example:
    • KindObservation = "observation"
    • KindForecast = "forecast"
    • KindForecastDiscussion = "forecast_discussion"
    • KindWeatherStory = "weather_story"
    • KindAlert = "alert"
    • KindOutlook = "outlook"
  • Keep kind constants typed as plain strings, not event.Kind, so standards does not import feedkit.
  • Add source driver string constants in the provider source packages to avoid import cycles:
    • NWS driver constants under internal/sources/nws.
    • Open-Meteo driver constants under internal/sources/openmeteo.
    • OpenWeather driver constants under internal/sources/openweather.
    • SPC driver constant under internal/sources/spc.
  • Update source constructors to use driver constants instead of local string literals.
  • Update Kinds() methods and SingleEvent calls to use event.Kind(standards.Kind...).
  • Update source registration to use provider driver constants.
  • Update internal tests to use constants where doing so reduces drift.
  • Keep docs and YAML examples literal because they are user-facing examples.
  • Do not add weather-specific kind constants to feedkit.

Acceptance criteria:

  • All driver and event kind string values remain unchanged.
  • No import cycle is introduced.
  • standards does not import feedkit.
  • Source constructors, emitted events, and advertised kinds behave identically.

Focused tests:

go test ./internal/sources ./cmd/weatherfeeder
go test ./internal/normalizers/...

Stage 2: Table-Drive Source Registry Tests

Reduce source registry test duplication while preserving registry coverage.

Implementation requirements:

  • Replace repeated positive tests in internal/sources/builtins_test.go with one table-driven test derived from pollDriverRegistrations.
  • Keep the explicit negative test proving legacy nws_forecast is not registered.
  • Keep sourceConfigForDriver, but update it to use driver constants or registry-derived driver names.
  • Preserve coverage that every current registered driver builds as a PollSource.
  • Do not weaken ValidateExpectedKinds or scheduler job build coverage in cmd/weatherfeeder tests.

Acceptance criteria:

  • Adding a new driver to pollDriverRegistrations automatically includes it in the positive registry test.
  • Legacy-driver rejection remains explicitly tested.
  • Test behavior remains deterministic and independent of live upstream services.

Focused tests:

go test ./internal/sources ./cmd/weatherfeeder

Stage 3: Add Shared HTTP Config Helper For Multi-Document Sources

Centralize common HTTP config parsing for sources that cannot use feedkit's single-document HTTPSource.

Implementation requirements:

  • Add a narrow helper under internal/sources/internal/httpconfig.
  • The helper should parse only the common source HTTP client params needed by non-HTTPSource sources:
    • trimmed source name;
    • required params.user_agent / params.userAgent;
    • optional params.http_timeout using feedkit config duration semantics;
    • optional params.http_response_body_limit_bytes as a positive integer.
  • The helper should return values sufficient for callers to build a transport.NewHTTPClient(timeout) and pass a body limit to transport.FetchBodyWithLimit.
  • Refactor only the SPC source to use this helper.
  • Preserve SPC-specific config parsing in the SPC source:
    • latitude;
    • longitude;
    • location_id / locationID;
    • location_name / locationName;
    • geojson_urls;
    • discussion_urls;
    • rss_url / rssURL.
  • Do not replace fksources.NewHTTPSource for normal single-URL sources.
  • Preserve SPC atomic bundle fetch behavior, bundle hash behavior, effective-time policy, raw payload shape, and error context.

Acceptance criteria:

  • SPC constructor accepts and rejects the same configs as before.
  • SPC still fetches required documents atomically and emits no partial bundle.
  • Existing SPC source tests pass unchanged except for expected helper-related error wording if the wording becomes more consistent.
  • No single-document source is refactored away from fksources.NewHTTPSource.

Focused tests:

go test ./internal/sources/spc ./internal/sources

Stage 4: Reduce Postgres Mapper Envelope Duplication

Centralize repeated Postgres parent envelope mapping without changing the table contract.

Implementation requirements:

  • Add small helper functions inside internal/sinks/postgres.
  • Centralize parent event envelope values:
    • event_id;
    • event_kind;
    • event_source;
    • event_schema;
    • event_emitted_at;
    • event_effective_at.
  • Use the helper in every parent table mapper that stores event envelope columns.
  • Optionally centralize event envelope column declarations if it keeps schema.go readable. If it makes the schema definition harder to scan, leave column declarations explicit.
  • Keep explicit per-product mapper functions.
  • Keep product-specific validation in product-specific functions where policy differs.
  • Do not introduce reflection mapping, struct tags, ORM-style abstractions, generated table mapping, or persistence annotations in model.
  • Preserve every existing 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.
  • Feedkit Postgres schema validation still receives complete rows for every declared column.

Focused tests:

go test ./internal/sinks/postgres

Stage 5: Tighten Config Example And Documentation Consistency Tests

Add shallow tests that detect identifier drift without generating or over-parsing documentation.

Implementation requirements:

  • Extend maintained config tests so cmd/weatherfeeder/config.yml and every examples/*.yml remain loadable and source-buildable.
  • Add shallow docs consistency tests for stable identifiers only:
    • every schema constant in standards appears in docs/integrations/events.md when it is part of the current event contract;
    • every registered source driver name appears in docs/config.md;
    • every registered source driver name appears in docs/internal/sources.md.
  • Avoid parsing Markdown tables deeply; simple file-content checks are sufficient.
  • Do not generate docs.
  • Do not modify current-behavior docs unless the implementation uncovers an actual stale documented identifier.
  • Keep documentation policy intact: implemented behavior outside docs/roadmap/, future plans under docs/roadmap/.

Acceptance criteria:

  • Identifier consistency tests fail when a new source driver or current schema is added without updating canonical docs.
  • Tests are shallow and low maintenance.
  • Tests do not assert prose formatting or table layout.

Focused tests:

go test ./cmd/weatherfeeder ./standards ./internal/sources

Stage 6: Consolidate Package-Local Test Fixture Helpers

Remove 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.
  • Do not create a cross-package test utility package.
  • Keep fixtures under each package's testdata directory.
  • If a package has only one fixture helper, leave it alone.
  • Do not change fixture contents unless a test already requires it.
  • Do not mix this stage with parser behavior changes.

Acceptance criteria:

  • Test helper duplication is reduced where multiple files in one package 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:

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 7: Dead-Code And Literal Sweep

Perform a final cleanup sweep after constants and helper stages are complete.

Implementation requirements:

  • Search for stale driver, kind, and schema literals after earlier stages.
  • 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 and no longer document supported behavior.
  • Do not broaden the cleanup into unrelated refactors.

Suggested searches:

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 now exist.
  • Intentional literals in docs, YAML examples, raw schema docs, and negative tests remain readable.
  • No behavior changes are introduced.

Focused tests:

go test ./internal/sources ./internal/normalizers/... ./internal/sinks/postgres ./cmd/weatherfeeder

Final Verification

After all stages are complete, run:

go test ./...
git status --short

Before committing the implemented cleanup, verify:

  • No public contracts changed unintentionally.
  • Current-behavior docs still describe implemented behavior only.
  • docs/roadmap/cleanup.md is either updated to remove completed work or moved to future/remediation tracking according to the repository's roadmap practice.
  • No unrelated changes are included.

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.