From 1a9f462fbf9b38065de3357acfbe4d52ed30a5a5 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 10 Jun 2026 20:04:00 -0500 Subject: [PATCH] Audit code quality and deduplication opportunities --- docs/roadmap/audit.md | 558 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 558 insertions(+) create mode 100644 docs/roadmap/audit.md diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md new file mode 100644 index 0000000..a63e122 --- /dev/null +++ b/docs/roadmap/audit.md @@ -0,0 +1,558 @@ +# Code Quality And Deduplication Audit + +## Executive Summary + +`weatherfeeder` is in good shape for a limited cleanup pass before the next major release. The current architecture is coherent: runtime composition is thin, provider fetching lives in source adapters, provider-specific parsing lives under `internal/providers`, canonical mapping lives in normalizers, and Postgres persistence is isolated under `internal/sinks/postgres`. + +The codebase does not show a major architectural risk that would require broad redesign. Most duplication is the predictable result of recent feature growth across sources, canonical payloads, and persistence tables. The highest-value cleanup should be narrow and behavior-preserving. + +Top refactoring targets: + +1. Source adapter HTTP/config scaffolding: single-document sources and the SPC multi-document source share config, request, effective-time, and event-envelope policy in similar but not identical forms. +2. Postgres mapper boilerplate: parent envelope columns, UTC/null conversion, required-field checks, and child-row construction are repeated across every canonical product mapper. +3. Event kind and driver-name strings: event kinds and source driver names are repeated across registries, source implementations, tests, examples, and docs without code constants comparable to the centralized schema constants. + +Recommended posture: perform a limited cleanup pass in small commits. Avoid broad framework changes, generic workflow engines, plugin systems, or ORM-like abstractions. + +## Repository Map Reviewed + +Inspected directories and packages: + +- `cmd/weatherfeeder`: runtime composition, sample config, config-load and pipeline tests. +- `internal/sources`: source registry and provider source adapters for NWS, Open-Meteo, OpenWeather, and SPC. +- `internal/providers`: provider-specific parsing helpers for NWS, Open-Meteo, OpenWeather, and SPC. +- `internal/normalizers`: built-in normalizer registration, common helpers, and provider normalizers. +- `internal/sinks/postgres`: weatherfeeder-owned table schema and canonical-event mapper. +- `internal/geo`: point-in-geometry helper used by SPC outlook normalization. +- `model`: canonical payload structs. +- `standards`: schema and WMO constants. +- `examples`: maintained config examples. +- `docs`: policy, config, CLI, operations, troubleshooting, internal docs, integration docs, and `docs/roadmap/future.md`. + +Major execution paths reviewed: + +- daemon startup from `cmd/weatherfeeder/main.go`; +- source driver registration and source construction; +- source polling for single-document HTTP products and SPC multi-document bundles; +- normalizer registration and raw-schema dispatch; +- raw-to-canonical mapping for observations, forecasts, discussions, weather stories, alerts, and outlooks; +- Postgres schema and write mapping; +- maintained config loading tests. + +Important areas not deeply inspected: + +- Feedkit internals were not audited because they are a dependency and outside this repository's ownership boundary. +- Live upstream service behavior was not tested; this audit used local source inspection, fixtures, and existing docs. +- Full test execution was not run because this is a report-only task and no code behavior changed. + +## High-Confidence Deduplication Opportunities + +### 1. Centralize Repeated Source HTTP And Config Scaffolding + +Affected files/packages: + +- `internal/sources/nws/observation.go` +- `internal/sources/nws/alerts.go` +- `internal/sources/nws/forecast_common.go` +- `internal/sources/nws/forecast_discussion.go` +- `internal/sources/nws/weatherstories.go` +- `internal/sources/openmeteo/observation.go` +- `internal/sources/openmeteo/forecast.go` +- `internal/sources/openweather/observation.go` +- `internal/sources/spc/convective_outlook.go` +- `docs/internal/sources.md` +- `docs/config.md` + +Duplicated or near-duplicated behavior: + +- Most sources wrap `fksources.NewHTTPSource`, implement `Name`, advertise one `Kinds` value, fetch raw content if changed, compute `effectiveAt`, call `DefaultEventID`, and emit a single event. +- The SPC source cannot use `HTTPSource` directly because it fetches an atomic multi-document bundle, but it repeats the same user-agent, HTTP timeout, body limit, request accept header, and unchanged-content policy at a lower level through `transport.FetchBodyWithLimit`. +- HTTP config params are documented as shared, but source construction has no weatherfeeder-owned helper for the common param names and error shape when a source cannot use `HTTPSource`. + +Why it matters: + +- Adding future providers or multi-document products increases the chance of drift in `user_agent`, `http_timeout`, `http_response_body_limit_bytes`, body-limit, and error-message behavior. +- The single-document and SPC paths both implement operator-facing HTTP policy, but the shared policy is visible only in docs and feedkit conventions. +- Bug fixes to source envelope construction or shared HTTP params would likely need multiple package edits. + +Recommended refactor: + +- Add a small source-internal helper package or file, for example `internal/sources/sourceconfig` or `internal/sources/internal/httpconfig`, that owns weatherfeeder-specific HTTP param extraction for sources that cannot directly use `fksources.NewHTTPSource`. +- Keep `fksources.NewHTTPSource` as the implementation for simple sources. Do not replace it with a custom framework. +- Add a helper for common single-event envelope construction only if it remains explicit about `kind`, `source`, `schema`, `eventID`, `emittedAt`, `effectiveAt`, and payload. Avoid hiding provider-specific effective-time selection. +- For SPC, replace local parsing of `user_agent`, `http_timeout`, and `http_response_body_limit_bytes` with the shared helper while preserving its atomic multi-fetch and hash semantics. + +Suggested tests: + +- Add helper-level tests for param aliases, positive timeout/body-limit validation, missing `user_agent`, and error messages. +- Keep existing source tests for emitted kind/schema/effective time unchanged. +- Add one SPC constructor test that proves shared timeout/body-limit validation still applies. + +Risk level: Low to Medium. The behavior is straightforward, but source constructor errors are user-facing and should be protected by tests. + +### 2. Reduce Postgres Mapper Boilerplate Without Creating An ORM + +Affected files/packages: + +- `internal/sinks/postgres/map.go` +- `internal/sinks/postgres/schema.go` +- `internal/sinks/postgres/map_test.go` +- `internal/sinks/postgres/schema_test.go` +- `docs/internal/postgres-sink.md` +- `docs/integrations/postgres.md` + +Duplicated or near-duplicated behavior: + +- Every parent table mapper repeats the same event envelope columns: `event_id`, `event_kind`, `event_source`, `event_schema`, `event_emitted_at`, and `event_effective_at`. +- Every run mapper follows the same pattern: decode payload, validate required run time/product fields, normalize to UTC, write one parent row, then write child rows with positional indexes. +- Nullable conversion helpers already exist, but each mapper repeats the same map literal shape and required-field error phrasing. +- Schema definitions repeat the same envelope column declarations across parent tables. + +Why it matters: + +- The mapper is now the largest concentration of cross-product persistence policy. As more canonical products are added, missing an envelope column, count field, UTC conversion, or required-field check becomes easier. +- Recent SPC work showed that canonical fields can be accidentally omitted from persistence even when the model and docs are correct. +- Refactoring this area would reduce maintenance risk and make mapper tests easier to read. + +Recommended refactor: + +- Add a small `eventEnvelopeValues(e)` helper returning the parent envelope value map, then merge product-specific columns into it. +- Add a small `eventEnvelopeColumns()` helper for schema definitions if feedkit schema construction remains readable. +- Add required-field helper functions for common checks such as `requireTime`, `requireString`, and `requireJSON`, but keep product-specific validation functions where policy differs. +- Keep explicit per-product mapper functions. Do not introduce reflection-based table mapping, struct tags, or a generic ORM layer. + +Suggested tests: + +- Add focused helper tests for envelope value UTC/null behavior. +- Keep product mapper tests asserting important columns per product. +- Add a regression test that each parent table with event envelope columns receives all envelope values from mapper output. +- Keep schema tests for nullable/required columns, especially recent outlook and forecast condition semantics. + +Risk level: Medium. The target is low-level persistence code; refactor only with existing mapper tests passing and add tests before moving column/value construction. + +### 3. Centralize Event Kind And Driver Name Constants + +Affected files/packages: + +- `internal/sources/builtins.go` +- `internal/sources/builtins_test.go` +- `internal/sources/*/*.go` +- `internal/normalizers/*/*_test.go` +- `cmd/weatherfeeder/main_test.go` +- `cmd/weatherfeeder/config.yml` +- `examples/*.yml` +- `docs/config.md` +- `docs/internal/sources.md` +- `docs/integrations/events.md` +- `standards/schema.go` + +Duplicated or near-duplicated behavior: + +- Schemas are centralized in `standards/schema.go`, but event kinds are repeatedly typed as string literals such as `event.Kind("forecast")`, `event.Kind("weather_story")`, and `event.Kind("outlook")`. +- Driver names are repeated in source constructors, registry entries, tests, config examples, docs, and troubleshooting text. +- The all-current-drivers test duplicates the registry table manually. + +Why it matters: + +- Kinds and driver names are public operator-facing strings. A typo or stale test value can produce startup failures or documentation drift. +- The mismatch between centralized schemas and non-centralized kinds/drivers makes future feature additions more error-prone. +- The source registry already has a structured `pollDriverRegistrations` slice that can become the canonical source for driver tests. + +Recommended refactor: + +- Add event kind constants in `standards`, for example `KindObservation`, `KindForecast`, `KindForecastDiscussion`, `KindWeatherStory`, `KindAlert`, and `KindOutlook`, typed as `event.Kind` if dependency direction is acceptable. If `standards` should not import feedkit, use string constants and convert at adapter boundaries. +- Add source driver constants near source registration, for example in `internal/sources/drivers.go`, and have constructors/tests use those constants. +- Update source registry tests to derive the all-current-drivers list from `pollDriverRegistrations`, while keeping explicit negative tests for removed legacy names. +- Keep docs and YAML examples literal; they are user-facing examples and should not be generated for this cleanup pass. + +Suggested tests: + +- Update source registry tests to assert every registered driver builds as a `PollSource` using the registry slice. +- Add a small test that configured source `Kinds()` match the central kind constants. +- Keep example config load tests as the docs/example guardrail. + +Risk level: Low. This is mostly mechanical, but care is needed to avoid import cycles if kind constants are typed with feedkit's `event.Kind`. + +### 4. Centralize Config Example Coverage Around All Maintained YAML Files + +Affected files/packages: + +- `cmd/weatherfeeder/main_test.go` +- `cmd/weatherfeeder/config.yml` +- `examples/config.minimal.yml` +- `examples/config.nats.yml` +- `examples/config.postgres.yml` +- `docs/config.md` + +Duplicated or near-duplicated behavior: + +- The sample and copyable configs repeat driver names, event kinds, route kind lists, NWS user-agent conventions, source cadences, and sink shapes. +- `main_test.go` already verifies that `cmd/weatherfeeder/config.yml` and `examples/*.yml` load and that sources build scheduler jobs. +- There is no single test that compares example route kind lists against current source-advertised kinds or documented current kinds. + +Why it matters: + +- Config examples are part of the operator contract. They tend to drift when a new canonical kind is added or renamed. +- Routes are easy to leave stale because a config can load successfully while omitting newly supported kinds from a production-oriented route example. + +Recommended refactor: + +- Keep examples explicit and copyable. +- Add tests that collect advertised source kinds from configured examples and verify route examples either intentionally match all kinds or document why they are selective. +- Add a small helper in tests for building the weatherfeeder source registry and validating all maintained configs, so config coverage stays obvious. + +Suggested tests: + +- Extend `TestMaintainedConfigExamplesLoad` to assert source expected kinds and scheduler jobs, which it already does, and add route-kind sanity checks if feedkit exposes compiled routes clearly enough. +- Add a docs/config example guard only if it can be kept simple; avoid parsing Markdown tables unless this repo already uses doc extraction tests. + +Risk level: Low. This is test-only cleanup unless route semantics in examples are intentionally selective. + +## Medium-Confidence Opportunities + +### 1. Normalize Required-Time Helper Patterns Where Semantics Match + +Affected files/packages: + +- `internal/providers/nws/time.go` +- `internal/providers/openmeteo/time.go` +- `internal/providers/spc/time.go` +- `internal/normalizers/nws/forecast.go` +- `internal/normalizers/nws/weatherstories.go` +- `internal/normalizers/spc/convective_outlook.go` +- `internal/sources/nws/*` +- `internal/sources/spc/convective_outlook.go` + +Duplicated or near-duplicated behavior: + +- Multiple normalizers implement required timestamp parsing with field-specific error messages. +- Multiple sources parse provider timestamps best-effort for effective-time selection. +- Providers correctly differ in timestamp formats, but callers often repeat trim/empty/UTC/error-context patterns. + +Why it matters: + +- Time parsing is a domain policy hotspot. Small drift in required vs optional parsing, UTC normalization, or error wording can create subtle behavior differences. +- Required field names in errors are useful and should be preserved. + +Recommended refactor: + +- Do not force all providers through one cross-provider parser; NWS, Open-Meteo, and SPC formats differ for good reasons. +- Consider small provider-local helpers such as `ParseRequiredTime(value, field)` and `ParseOptionalTime(value)` where a provider already has a canonical parser. +- Use common helper signatures only when the failure behavior is truly identical. + +Suggested tests: + +- Provider helper tests for empty, malformed, and UTC-normalized timestamps. +- Normalizer tests that assert required timestamp errors include the field path. + +Risk level: Medium. The duplication is real, but over-centralization could obscure provider-specific formats. + +### 2. Table-Drive Source Registry Tests More Aggressively + +Affected files/packages: + +- `internal/sources/builtins_test.go` + +Duplicated or near-duplicated behavior: + +- Several tests independently instantiate a registry and build one named driver. +- `TestRegisterBuiltinsRegistersAllCurrentDrivers` duplicates the same driver list that exists in `pollDriverRegistrations`. + +Why it matters: + +- Adding new drivers currently requires touching both the registration table and a manually duplicated test list. +- The test suite already has the structure needed to derive cases from the registration table. + +Recommended refactor: + +- Replace individual positive registration tests with a table derived from `pollDriverRegistrations`. +- Keep one or two named tests only when they assert special policy, such as legacy driver removal. +- Keep `sourceConfigForDriver` but make it keyed off driver constants. + +Suggested tests: + +- One table-driven positive registration test for every driver. +- One explicit negative test for `nws_forecast` legacy driver. + +Risk level: Low. This is test cleanup with minimal behavior risk. + +### 3. Package-Local Fixture Helpers Are Duplicated + +Affected files/packages: + +- `internal/providers/nws/forecast_discussion_test.go` +- `internal/providers/spc/geojson_test.go` +- `internal/sources/nws/forecast_discussion_test.go` +- `internal/sources/spc/convective_outlook_test.go` +- `internal/normalizers/nws/forecast_discussion_test.go` +- `internal/normalizers/spc/convective_outlook_test.go` + +Duplicated or near-duplicated behavior: + +- Several tests define local helpers that read from `testdata` using `os.ReadFile` and `filepath.Join`. +- Helper names differ by package, but behavior is mostly identical. + +Why it matters: + +- This is low-risk duplication, but fixture read failures and paths could be made more consistent. +- Cleaner fixture helpers would reduce noise in parser/source/normalizer tests. + +Recommended refactor: + +- Prefer package-local test helpers, not a cross-package test utility. Go package tests are easier to understand when fixtures remain near the package under test. +- Within each package with multiple test files, consolidate repeated `readTestFile` helpers into one `_test.go` helper file. + +Suggested tests: + +- No new behavior tests required; this cleanup is test-only. +- Run affected package tests. + +Risk level: Low. + +### 4. Schema, Model, And Postgres Documentation Lists Require Manual Synchronization + +Affected files/packages: + +- `standards/schema.go` +- `model/*.go` +- `docs/integrations/events.md` +- `docs/integrations/postgres.md` +- `docs/internal/normalizers.md` +- `docs/internal/sources.md` +- `docs/config.md` + +Duplicated or near-duplicated behavior: + +- Current schemas, raw mappings, canonical mappings, event kinds, and Postgres table contracts are documented in multiple current-behavior docs. +- This is partly intentional because docs serve different audiences, but all lists must be manually updated when a feature is added. + +Why it matters: + +- Recent feature additions touched many docs. Manual sync is workable now but will remain a recurring release risk. +- Documentation policy requires current-behavior docs to avoid speculative or stale content. + +Recommended refactor: + +- Do not generate docs wholesale. +- Add targeted doc consistency tests only for compact, machine-checkable facts, such as ensuring every schema constant appears in `docs/integrations/events.md` and every source driver appears in `docs/config.md`. +- Keep prose manual. + +Suggested tests: + +- A small standards/docs test that reads selected docs and checks for schema constants and driver constants. +- Keep examples load-tested. + +Risk level: Medium. Doc tests can become brittle if they parse prose too deeply; keep them shallow. + +## Boundary And Responsibility Concerns + +### Source Adapter HTTP Policy Is Split Between Feedkit And Weatherfeeder + +Most single-document sources rely on feedkit `HTTPSource`, while SPC implements a custom multi-document fetch loop. This boundary is acceptable because SPC's atomic bundle semantics differ from single-document polling. The concern is not the custom source itself; the concern is that shared weatherfeeder HTTP config policy is partly reimplemented in the SPC adapter. + +Recommended home: keep generic HTTP mechanics in feedkit, but add a small weatherfeeder source helper for weatherfeeder-owned parameter names and validation when a source cannot use `HTTPSource` directly. + +### Postgres Mapping Is Correctly Isolated But Becoming Too Dense + +The Postgres mapper is in the right package and does not leak into domain or normalizer code. The package responsibility is clear. The concern is density and repeated policy, not boundary drift. + +Recommended home: keep mapper helpers under `internal/sinks/postgres`. Do not move persistence concerns into `model` or normalizers. + +### Event Kind Strings Lack A Canonical Home + +Schemas have a clear home in `standards`; event kinds do not. Because kinds are part of routing and operator config, they deserve a comparable canonical code location. + +Recommended home: `standards` is the best conceptual location if dependency direction remains clean. If importing feedkit's `event` package into `standards` is undesirable, use string constants in `standards` and convert in source adapters. + +### Runtime Composition Is Appropriately Thin + +`cmd/weatherfeeder/main.go` is mostly process wiring. It does not contain provider parsing or sink mapping. No refactor is recommended here beyond possibly extracting tiny helper functions if future CLI flags make startup more complex. + +## Path, Key, And Naming Construction Review + +Centralized enough: + +- Schema strings are centralized in `standards/schema.go`. +- SPC product keys, day numbers, outlook types, and default URLs are centralized in `internal/providers/spc/product.go`. +- Postgres table names are centralized as constants in `internal/sinks/postgres/schema.go`. +- Test fixture paths are local and simple. + +Needs cleanup: + +- Event kind strings are repeated across source adapters, tests, YAML examples, and docs. +- Source driver names are repeated across constructors, registration, tests, docs, and examples. +- Postgres envelope column names are repeated in schema and mapper literals. +- Config route kind lists in examples are manually synchronized with supported canonical kinds. + +Recommended approach: + +- Add code constants for kinds and drivers first. +- Add narrow Postgres helpers for envelope column/value names second. +- Leave user-facing YAML and Markdown examples explicit, but test them against the code constants where practical. + +## Resolution And Catalog Review + +Current resolution model: + +- Source drivers resolve through `internal/sources.RegisterBuiltins` and feedkit's source registry. +- Normalizers resolve by schema matching through `internal/normalizers.RegisterBuiltins` and feedkit's normalize processor. +- Sinks resolve through feedkit's sink registry, with weatherfeeder registering a Postgres schema mapper. +- Schemas resolve through `standards` constants. +- SPC product catalogs resolve through `internal/providers/spc` product metadata helpers. + +Consistency assessment: + +- Normalizer resolution is strong: schema equality is explicit and follows policy. +- Source driver resolution is explicit and readable, but test coverage duplicates driver lists rather than deriving from the registry table. +- SPC product resolution is strong and should remain provider-local. +- There is no artifact, prompt, module, profile, manifest, or object-key catalog in this repository. + +Recommended centralization: + +- Treat source driver constants and event kind constants as small catalogs. +- Avoid building a generic catalog framework; explicit registry tables are appropriate for this codebase. + +## Config And Command-Loading Review + +Current behavior: + +- The executable reads exactly `config.yml` from the current working directory. +- There are no CLI flags, subcommands, profiles, environment-variable config overlays, or config path precedence rules. +- Feedkit owns top-level config loading and validation. +- Weatherfeeder source/sink constructors own driver-specific param validation. +- Maintained examples are load-tested and source-build-tested. + +Consistency assessment: + +- There is no duplicated command-loading behavior because there is only one command path. +- Driver-specific config validation is mostly consistent, but SPC has to duplicate some HTTP param parsing because it cannot use feedkit's single-document `HTTPSource`. +- OpenWeather's `units=metric` invariant is correctly located in `internal/providers/openweather` and enforced by the source constructor. + +Intentional differences: + +- SPC does not require `params.url` because it owns a fixed product catalog plus optional override maps. +- OpenWeather has stricter URL validation because unit semantics affect canonical mapping correctness. +- Single-document sources use conditional HTTP validators; SPC uses a bundle hash because it fetches multiple documents atomically. + +Likely accidental drift risk: + +- HTTP timeout and body-limit validation wording can differ between feedkit-backed HTTP sources and SPC. +- Future multi-document sources may copy SPC's config parsing rather than sharing a narrow helper. + +## State, Manifest, Or Progress Handling Review + +Current state handling: + +- The daemon has no durable internal run state, manifest, checkpoint, or resume marker. +- Feedkit scheduler, dispatcher, sink fanout queues, and dedupe operate in memory. +- Single-document HTTP conditional validators are source-instance memory only. +- SPC unchanged-response behavior uses a source-local hash of the last complete bundle. +- Postgres persistence is external sink state. + +Consistency assessment: + +- The state model is documented and consistent with the architecture policy. +- There is no hidden filesystem state substituting for declared state. +- There is no resume/force/dry-run behavior to drift across commands. + +Cleanup recommendation: + +- No state/manifest refactor is needed now. +- If future durable checkpoints are added, design them explicitly rather than expanding the current in-memory dedupe or source-local hash semantics. + +## Refactors To Avoid + +Avoid these refactors in the next cleanup pass: + +- A generic workflow engine or stage abstraction. The current runtime has source polling, normalization, dedupe, and dispatch; adding a stage framework would be speculative. +- A plugin runtime. The policy explicitly favors built-in registries over a general plugin system. +- Replacing feedkit HTTP, scheduler, dispatch, or sink infrastructure with weatherfeeder-owned equivalents. +- A generic Postgres ORM or reflection-driven mapper. The table contract is explicit and should remain readable. +- Cross-provider timestamp parsing that ignores provider-specific timestamp formats. +- Consolidating WMO mapping too aggressively. Provider-specific condition signals differ; only shared text fallback belongs in common helpers. +- Generating all docs from code. Shallow consistency tests are useful; generated manuals would fight the documentation policy's audience-specific structure. +- Collapsing all source adapters into one generic source type. The effective-time and payload policies are similar but still product-specific. +- Moving persistence tags or database column names into `model`. Canonical payloads should not depend on the Postgres sink. + +## Recommended Implementation Sequence + +1. Add event kind and source driver constants. + + Scope: constants plus mechanical usage in source adapters, registry tests, and normalizer tests where appropriate. Keep docs/YAML literal. Run `go test ./internal/sources ./internal/normalizers/... ./cmd/weatherfeeder`. + +2. Table-drive source registry tests. + + Scope: derive positive source driver cases from `pollDriverRegistrations`; keep legacy-driver negative tests. Run `go test ./internal/sources ./cmd/weatherfeeder`. + +3. Add source HTTP config helper for non-`HTTPSource` adapters. + + Scope: centralize `user_agent`, `http_timeout`, and `http_response_body_limit_bytes` parsing for SPC and future multi-document sources. Do not alter simple `HTTPSource` adapters. Run `go test ./internal/sources/spc ./internal/sources ./cmd/weatherfeeder`. + +4. Add Postgres envelope helper tests, then helper functions. + + Scope: add `eventEnvelopeValues`, optionally envelope column helpers, and product-specific required-field helpers. Keep explicit mapper functions. Run `go test ./internal/sinks/postgres`. + +5. Consolidate package-local fixture helpers. + + Scope: per package only; no cross-package testing utility. Run affected provider/source/normalizer package tests. + +6. Add shallow docs consistency tests. + + Scope: verify schema constants and source driver constants appear in canonical docs. Avoid parsing Markdown tables deeply. Run `go test ./standards ./cmd/weatherfeeder` or place tests in a suitable package that can read repo docs. + +7. Dead-code and legacy sweep. + + Scope: after constants/tests are in place, search for obsolete schema/driver/kind literals such as removed legacy driver names. Keep explicit negative tests where they document supported removals. + +## Test Strategy + +Tests to add before refactoring: + +- Source HTTP config helper tests for aliases, missing params, positive duration/body-limit validation, and error context. +- Postgres mapper tests that assert parent envelope columns are consistently present for every mapped canonical parent row. +- Source driver/kind constant tests if constants are introduced. + +Tests to update during refactoring: + +- `internal/sources/builtins_test.go` for table-driven registry coverage. +- `internal/sources/spc/convective_outlook_test.go` for shared HTTP config validation. +- `internal/sinks/postgres/map_test.go` and `schema_test.go` for envelope helper preservation. +- Existing provider/source/normalizer fixture tests if fixture helpers move. + +Focused verification commands: + +```sh +go test ./cmd/weatherfeeder ./internal/sources ./internal/sources/... ./internal/providers/... ./internal/normalizers/... ./internal/sinks/postgres ./model ./standards +``` + +Full verification command before merging cleanup: + +```sh +go test ./... +``` + +## Appendix: Findings Not Worth Acting On + +### Provider-Specific Source Files Share A Similar Shape + +NWS, Open-Meteo, and OpenWeather source files all implement `Name`, `Kinds`, `Poll`, metadata decode, and event emission. This is acceptable because each product has distinct effective-time and metadata policy. Extract only the clearly shared HTTP/config pieces. + +### Normalizer Match Methods Are Repetitive By Design + +Most normalizers implement a one-line `Match` against a schema constant. This repetition is good: it keeps routing explicit and cheap. A generic schema-to-builder registry would add indirection without meaningful risk reduction. + +### Provider Time Parsers Should Remain Provider-Specific + +NWS, Open-Meteo, and SPC timestamp formats differ. The current provider-local parsers are easier to reason about than a broad cross-provider parser. Only required/optional wrapper patterns should be considered for cleanup. + +### Documentation Repeats Some Lists Intentionally + +`README.md`, `docs/config.md`, `docs/internal/sources.md`, and `docs/integrations/events.md` repeat selected feature lists for different audiences. Do not eliminate that repetition wholesale. Prefer shallow consistency tests for high-risk identifiers. + +### SPC Bundle Hash State Should Stay Local + +The SPC source's last-bundle hash is source-local unchanged-content state, not a general manifest/checkpoint system. Generalizing it now would be premature. + +### Runtime Wiring Could Be Split Into Helpers, But Need Not Be + +`cmd/weatherfeeder/main.go` is readable and policy-aligned. Extracting helper functions now would mostly move code around. Revisit only if CLI flags, config path options, metrics, or health checks are added.