Generalize NWS forecast discussion heading parsing

This commit is contained in:
2026-08-02 22:53:00 +00:00
parent 29c65971eb
commit 2b19a121fa
4 changed files with 618 additions and 331 deletions

View File

@@ -1,181 +1,264 @@
# NWS AFD Section Heading Variants Implementation Plan
# NWS AFD Section Parsing Resilience Implementation Plan
## Purpose
Implement the end state defined in
[`afd-section-heading-variants.md`](afd-section-heading-variants.md): recognize
legacy ellipsis-first and slash-qualified NWS Area Forecast Discussion headings
consistently during section discovery, boundary detection, and qualifier
extraction without changing the canonical event contract.
Complete the stages below in order. Keep each stage limited to the files and
behavior it names, and leave the repository passing its tests before proceeding.
Complete the resilience end state in
[`afd-section-heading-variants.md`](afd-section-heading-variants.md) while
preserving the current canonical forecast-discussion contract. Stages 1-3 below
summarize completed work. Implement Stages 4-8 in order.
## Cross-Stage Constraints
- Keep all provider-format parsing in `internal/providers/nws`.
- Use only the Go standard library; do not add a dependency.
- Do not change `model`, `standards`, source configuration, polling behavior,
schemas, event-envelope behavior, Postgres mapping, or sink behavior.
- Preserve the existing case-sensitive supported section identities: `KEY
MESSAGES`, `SHORT TERM`, `LONG TERM`, and `AVIATION`.
- Continue exposing only key messages, short term, and long term in the parsed
and canonical forecast-discussion payloads. `AVIATION` is a boundary marker,
not a new output field.
- Preserve source body lines for the existing prose parsers; heading
normalization must not alter section text, issue-time parsing, signature
trimming, or paragraph joining.
- Treat unknown section names and malformed slash-qualified lines as ordinary
body lines, not as recognized boundaries.
- Use deterministic unit tests and the checked-in fixture. Do not contact live
NWS services.
- Keep all production parsing changes in
`internal/providers/nws/forecast_discussion.go`.
- Use only the Go standard library and keep helpers unexported.
- Do not change `model`, `standards`, source configuration or polling, schemas,
event-envelope behavior, Postgres mapping, sinks, consumer docs, or current
integration contracts.
- Continue exposing only key messages, short term, and long term. All other
structurally valid identities are boundary-only.
- Preserve first-occurrence behavior for mapped sections.
- Preserve raw body lines during structural scanning; apply provider-specific
cleanup only when parsing a block's content.
- Keep parsing deterministic. Tests must use local strings and fixtures, never
live NWS requests.
- Preserve all pre-existing user work and avoid unrelated refactors.
## Stage 1: Centralize Heading Parsing and Section Extraction
## Stage 1: Centralize Known Heading Parsing — Completed
Implement the provider-level parsing primitive and make it the sole source of
heading identity and qualifier interpretation.
The provider parser gained one heading classifier, explicit section and block
types, and shared discovery, boundary, and qualifier handling for the original
four identities. Key-message and text-section consumers were moved to the block
representation, and immediately adjacent recognized headings became valid
boundaries.
1. In `internal/providers/nws/forecast_discussion.go`, define unexported string
constants for the four supported section identities. Use those constants in
`ParseForecastDiscussionText` instead of repeating string literals.
2. Add an unexported `forecastDiscussionSectionHeading` value with `section`
and `qualifier` fields, plus a
`parseForecastDiscussionSectionHeading(string) (forecastDiscussionSectionHeading, bool)`
helper. The helper must trim outer line whitespace and implement exactly
these two case-sensitive, whole-line grammars:
## Stage 2: Add Heading-Variant Regressions — Completed
```text
^\.(KEY MESSAGES|SHORT TERM|LONG TERM|AVIATION)\.\.\.(.*)$
^\.(KEY MESSAGES|SHORT TERM|LONG TERM|AVIATION)[ \t]+/([^/]+)/\.\.\.$
```
Provider tests now cover ellipsis-first and slash-qualified headings, malformed
forms, empty bodies, mixed heading families, and slash-qualified aviation
termination. Normalizer coverage verifies canonical propagation and unchanged
wire shape.
Keep the supported-identity alternation in one shared pattern constant used
to build both regular expressions, so the accepted identity list cannot
drift between the two forms.
3. For the ellipsis-first form, trim leading and trailing whitespace from the
text following the ellipsis and otherwise preserve it verbatim. This retains
current results such as `(Through Late Sunday Night)` and permits an empty
qualifier.
4. For the slash-qualified form, require at least one space or tab before the
opening slash, forbid embedded slash characters, require the closing slash
immediately before the final ellipsis, and reject a qualifier that becomes
empty after trimming. Return the trimmed inner text without either slash.
Do not case-fold identities or accept trailing text after the final
ellipsis.
5. Add an unexported `forecastDiscussionSectionBlock` containing the parsed
heading and its body lines. Change `extractForecastDiscussionSection` to
return this value. Find the requested section by calling the new heading
parser and comparing its `section` field to the requested identity; remove
the current constructed exact-prefix lookup.
6. While collecting a block body, retain the existing terminators `&&`, `$$`,
and lines containing `WATCHES/WARNINGS/ADVISORIES`. Also stop before every
subsequent line recognized by the new heading parser, including a heading
immediately following the current heading. Remove the existing
`j > i+1` exception so an empty section cannot absorb the next heading.
Preserve original, untrimmed body lines in the returned block.
7. Update the consumers of the extracted block:
## Stage 3: Validate and Record the Baseline — Completed
- pass only `block.body` to key-message parsing and change
`parseForecastDiscussionKeyMessages` so it no longer assumes the heading
occupies element zero;
- make `parseForecastDiscussionTextSection` consume the block, initialize
`Qualifier` directly from `block.heading.qualifier`, and process
`block.body` with the existing issue-time and prose logic; and
- remove `forecastDiscussionHeaderRE`,
`isForecastDiscussionSectionHeader`, and
`parseForecastDiscussionQualifier` after all callers use the centralized
helper.
8. In `internal/providers/nws/forecast_discussion_test.go`, add a table-driven
unit test for `parseForecastDiscussionSectionHeading` with, at minimum:
Focused and full tests passed, production changes remained inside the NWS
provider parser, and the original feature roadmap was marked implemented.
- ellipsis-first headings for all four identities;
- an ellipsis-first heading with no qualifier;
- slash-qualified headings for all four identities;
- leading/trailing outer whitespace and multiple spaces before a slash;
- exact qualifier expectations showing that legacy parentheses remain and
slash delimiters are removed; and
- negative cases for an unknown identity, missing opening or closing slash,
an embedded slash, a whitespace-only slash qualifier, missing required
whitespace before the slash, missing final ellipsis, trailing text after a
slash-qualified heading, and a lowercase section identity.
## Stage 4: Generalize Heading Syntax and Centralize Canonical Roles
Decouple structural heading recognition from canonical section selection.
1. In `internal/providers/nws/forecast_discussion.go`, replace the four-name
regular-expression alternation with a generic heading parser. Keep the
`forecastDiscussionSectionHeading` result, with normalized `section` and
`qualifier` fields.
2. Implement the generic parser with these decisions:
- trim outer whitespace and require a leading `.`;
- try the slash-qualified form before the legacy ellipsis-first form so the
terminal ellipsis of an all-uppercase slash heading cannot be mistaken for
a legacy identity;
- accept identities made only from uppercase ASCII letters, digits,
horizontal whitespace, `/`, `&`, apostrophes, and hyphens, with at least
one letter or digit;
- trim the identity and collapse every internal run of spaces or tabs to one
ASCII space before returning it or performing role lookup;
- accept optional horizontal whitespace between the identity and the legacy
`...` delimiter;
- for legacy headings, store trimmed text after the first `...` as the
qualifier, preserving parentheses and permitting an empty value;
- for slash-qualified headings, remove the terminal `/...`, then use the
first slash preceded by horizontal whitespace as the opening separator;
trim the identity before that separator and the qualifier after it, reject
an empty qualifier, and allow additional `/` characters inside the
qualifier; slashes inside an identity must therefore be adjacent to its
other identity characters rather than preceded by whitespace;
- reject trailing text after a slash-qualified terminal, lowercase or mixed
case identities, missing delimiters, and lines whose identity contains
other punctuation; and
- keep ASCII `...` as the only delimiter because that is the raw product
convention; do not interpret a Unicode ellipsis.
3. Replace the duplicated identity constants and identity-pattern string with:
- an unexported `forecastDiscussionSectionRole` enum for key messages, short
term, and long term; and
- one `map[string]forecastDiscussionSectionRole` containing exactly `KEY
MESSAGES`, `SHORT TERM`, and `LONG TERM`.
Unknown identities and `AVIATION` intentionally have no role entry; successful
structural parsing is sufficient for boundary behavior.
4. Update the heading table test in
`internal/providers/nws/forecast_discussion_test.go`. Preserve every legacy
positive case and revise the old policy-specific negatives:
- `.SYNOPSIS...`, `.UPDATE...`, `.MARINE...`, `.HYDROLOGY...`, an office
watch/advisory heading, and `.PRELIMINARY POINT TEMPS/POPS ...` are valid
generic headings;
- a slash-qualified heading whose qualifier contains `/` is valid;
- leading/trailing whitespace, multiple separator spaces, repeated internal
identity whitespace, and whitespace before a legacy ellipsis are valid and
produce the normalized identity; and
- lowercase prose, an empty or punctuation-only identity, malformed slash
terminals, missing ellipses, and unsupported identity punctuation remain
invalid.
5. Add assertions that every role-registry key parses successfully, and confirm
that no production switch or second collection repeats the mapped identity
list.
Run and pass:
```sh
gofmt -w internal/providers/nws/forecast_discussion.go internal/providers/nws/forecast_discussion_test.go
go test ./internal/providers/nws
go test -count=1 ./internal/providers/nws
```
Do not proceed until legacy provider tests still pass and the centralized
heading test covers every accepted identity in both forms.
Do not proceed until generic syntax tests pass without changing canonical
models or schemas.
## Stage 2: Add Provider and Normalizer Regression Coverage
## Stage 5: Scan Ordered Blocks Once and Use Generic Boundaries
Prove section discovery, boundary behavior, and canonical propagation using the
centralized parser. No production normalizer changes should be necessary.
Replace repeated per-identity extraction with one structural pass.
1. In `internal/providers/nws/forecast_discussion_test.go`, add focused tests of
`extractForecastDiscussionSection` using small line slices without `&&`
between sections. Cover:
1. Replace `extractForecastDiscussionSection` with
`parseForecastDiscussionSectionBlocks(lines []string) []forecastDiscussionSectionBlock`.
The scanner must:
- a slash-qualified `LONG TERM` block followed by a slash-qualified
`AVIATION` heading, asserting that the long-term qualifier is normalized,
only long-term prose is returned, and aviation heading/body text is
excluded;
- a recognized heading immediately following another recognized heading,
asserting that the first block has an empty body; and
- unknown and malformed heading-like lines inside a block, asserting that
they remain in the body and do not terminate it before a real terminator or
recognized heading.
2. Add a provider end-to-end regression based on
`testdata/forecast_discussion_sample.html`. Derive the input in the test by
replacing selected fixture headings rather than duplicating the full HTML
fixture. Guard each replacement with an assertion that its original heading
exists so fixture drift cannot make the test pass without exercising the new
syntax.
3. Make that derived bulletin intentionally mix heading families: retain the
legacy `KEY MESSAGES` heading, convert `SHORT TERM` and `LONG TERM` to
slash-qualified headings, and convert `AVIATION` to a slash-qualified
heading. Parse it through `ParseForecastDiscussionHTML` and assert:
- traverse lines once in source order;
- start a block on every structurally valid heading;
- finish the active block before a new heading;
- finish and clear the active block on `&&`;
- finish the active block and stop scanning on `$$`;
- retain the existing watch/advisory termination safeguard even though a
normal dotted watch/advisory heading is structurally recognized;
- ignore preamble lines before the first heading and signature lines after
`$$`; and
- append original, untrimmed body lines to the active block.
2. Refactor `ParseForecastDiscussionText` to iterate over the ordered blocks
once and look up each heading identity in the role registry. Ignore blocks
with no role. Populate each mapped output only if it has not already been
populated, so the first occurrence wins. Track seen roles explicitly rather
than inferring them from output values, because an empty first key-message
block still counts as the first occurrence.
3. Preserve contextual errors when a mapped text block fails to parse; include
the parsed heading identity in the wrapped error.
4. Replace extraction tests with scanner tests covering:
- key messages are unchanged;
- short- and long-term sections are non-nil;
- their qualifiers equal the inner slash text with no slash delimiters or
legacy parentheses;
- their existing issued times and representative prose remain intact; and
- long-term text contains no aviation heading or aviation prose.
4. In `internal/normalizers/nws/forecast_discussion_test.go`, add a regression
that derives the same mixed-format HTML from the shared fixture and passes it
through `ForecastDiscussionNormalizer.Normalize`. Assert the existing kind,
canonical schema, and effective time; exact normalized short- and long-term
qualifiers; representative section text; and absence of aviation content
from long-term text.
5. Marshal the slash-qualified normalizer result and assert that it has no
top-level `aviation` or generic `sections` key. Do not add or alter canonical
model fields to satisfy this test.
- consecutive headings and empty bodies;
- `&&`, `$$`, and watch/advisory termination;
- unknown valid headings terminating short- or long-term content even when
no `&&` is present;
- malformed heading-like lines remaining inside the active body;
- preamble and post-signature exclusion;
- ordered block retention; and
- duplicate mapped sections where the parser keeps the first canonical
occurrence.
Run and pass:
```sh
gofmt -w internal/providers/nws/forecast_discussion.go internal/providers/nws/forecast_discussion_test.go
go test -count=1 ./internal/providers/nws
```
## Stage 6: Normalize Multiline Preambles and NWS Presentation Markers
Handle real section-layout variation without weakening heading syntax.
1. Add an exact provider-local marker predicate for lines whose trimmed value is
`-- Changed Discussion --` or `-- End Changed Discussion --`, compared
case-insensitively. Add a helper that removes only those complete marker
lines from a block body. Do not remove arbitrary dashed lines or bullet text.
2. Apply marker removal before parsing both key-message and text-section bodies.
3. Refactor text-section preamble parsing in this exact order:
- trim blank lines after marker removal;
- start with the qualifier parsed from the heading;
- only when that qualifier is empty, consume the first content line as a
qualifier if its trimmed value is a nonempty standalone parenthetical
string beginning with `(` and ending with `)`; preserve the parentheses;
- after the optional qualifier, consume an optional `Issued at` line and
parse it into `ForecastDiscussionSection.IssuedAt`; and
- pass only the remaining lines to existing signature trimming and paragraph
joining.
4. Make recognition of the `Issued at` label ASCII case-insensitive while
retaining the existing timestamp grammar and error behavior. The top-level
header path, which passes an unlabeled timestamp, must remain compatible.
5. Before key-message bullet parsing, remove markers, trim blank lines, and
discard at most one leading metadata line beginning case-insensitively with
`Issued at` or `Updated at`. Do not discard timestamp-like lines after the
first message begins.
6. Add focused tests for:
- same-line legacy and slash qualifiers remaining unchanged;
- next-line parenthesized qualifiers followed by `Issued at`;
- uppercase `ISSUED AT`;
- a heading qualifier taking precedence over a following parenthetical prose
line;
- empty sections;
- invalid issue timestamps retaining contextual errors;
- marker removal at the beginning and end of text sections;
- key-message markers and a leading `Updated at` line not becoming messages;
and
- arbitrary dashed prose remaining content.
Run and pass:
```sh
gofmt -w internal/providers/nws/forecast_discussion.go internal/providers/nws/forecast_discussion_test.go
go test -count=1 ./internal/providers/nws
```
## Stage 7: Add Cross-Office Fixtures and Normalizer Regressions
Prove the resilient parser against representative source shapes rather than only
synthetic heading replacement.
1. Retain the existing LSX fixture and mixed-format test for regression
compatibility.
2. Add one compact, maintained HTML fixture under
`internal/providers/nws/testdata/` representing a second real NWS formatting
family. It must contain:
- a valid AFD header and issue time;
- key-message change markers plus a leading `Updated at` line;
- ellipsis-first short- and long-term headings with qualifiers on the next
line;
- `Issued at` lines following those qualifiers;
- at least one structurally valid boundary-only section; and
- representative prose sufficient to detect metadata or adjacent-section
leakage.
Keep the fixture concise; include no unrelated webpage content, secrets, or
private data.
3. Add a provider end-to-end test that parses the new fixture and asserts exact
key messages, qualifiers, section issue times, representative prose, and
absence of change markers, timestamp metadata, and boundary-only content.
4. Add a normalizer regression using the same fixture. Verify kind, canonical
schema, envelope/effective-time behavior, short- and long-term mapping, and
JSON wire shape without `aviation`, `discussion`, or generic `sections`
fields.
5. Keep small parser and scanner edge cases table-driven. Do not multiply full
fixtures for cases that a short line slice proves more clearly.
Run and pass:
```sh
gofmt -w internal/providers/nws/forecast_discussion_test.go internal/normalizers/nws/forecast_discussion_test.go
go test ./internal/providers/nws ./internal/normalizers/nws
go test -count=1 ./internal/providers/nws ./internal/normalizers/nws
```
Do not introduce a second full-bulletin fixture unless the checked-in fixture
cannot express a required interaction through guarded heading replacement.
## Stage 8: Reconcile Documentation and Perform Final Validation
## Stage 3: Validate the Feature and Close the Roadmap
Close the resilience follow-up only after all behavior is proven.
Verify the complete change against repository policy and record completion only
after all behavior is proven.
1. Review the final diff and confirm production changes are confined to the NWS
provider parser. Test changes should be confined to the owning provider and
NWS normalizer packages. Do not retain incidental model, schema, config,
source, sink, Postgres, or current-behavior documentation edits introduced
during implementation, and preserve all pre-existing user work.
2. Run formatting on every changed Go file, then run uncached focused tests and
the full repository suite:
1. Review the final diff. Production changes must remain confined to the NWS
provider parser; other Go changes should be tests in the owning provider and
normalizer packages. Preserve all unrelated user work.
2. Confirm the public contract is unchanged. Do not edit canonical model,
schema, Postgres, config, consumer, or integration documentation unless an
actual contract change is discovered. If one appears necessary, stop rather
than expanding this plan.
3. Run:
```sh
gofmt -w \
@@ -184,24 +267,18 @@ after all behavior is proven.
internal/normalizers/nws/forecast_discussion_test.go
go test -count=1 ./internal/providers/nws ./internal/normalizers/nws
go test -count=1 ./...
go vet ./...
git diff --check
```
3. Confirm each acceptance criterion in
[`afd-section-heading-variants.md`](afd-section-heading-variants.md) is
represented by a passing automated test. In particular, verify legacy
compatibility, mixed-format input, slash-qualified aviation termination,
malformed-line non-recognition, normalized qualifiers, and unchanged
canonical wire shape.
4. Because this feature does not alter a public contract, do not change
`docs/integrations/events.md`, `docs/integrations/postgres.md`, consumer docs,
configuration docs, or examples. If implementation appears to require such
a change, stop and reassess the implementation against the feature roadmap
instead of expanding scope.
5. After all checks pass, change the feature roadmap status from `Proposed and
unimplemented.` to `Implemented.` Do not mark it implemented earlier.
4. Verify every acceptance criterion in the feature roadmap has a corresponding
passing automated test, including compatibility with all Stage 1-2 cases.
5. After all checks pass, change the feature roadmap status to `Implemented.`
and rewrite any remaining future-tense statements that would misdescribe the
completed parser. Do not mark the follow-up implemented earlier.
## Open Questions
None. The feature roadmap and this plan fix the accepted grammar, normalization
rules, architectural boundary, test coverage, and canonical compatibility
requirements.
None. This plan fixes the parser grammar, boundary policy, canonical role
selection, multiline preamble rules, presentation cleanup, fixture strategy,
and compatibility boundary. Generic or single-section discussion content remains
outside the canonical model by explicit policy.