Support parenthesized NWS discussion headings

This commit is contained in:
2026-08-03 00:13:58 +00:00
parent a62cb87b78
commit b8c6708439
4 changed files with 526 additions and 296 deletions

View File

@@ -2,108 +2,135 @@
## Purpose
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.
Complete the remaining resilience work defined in
[`afd-section-heading-variants.md`](afd-section-heading-variants.md) without
changing the canonical forecast-discussion contract. Stages 1-8 summarize work
already completed. Implement Stages 9-13 in order.
## Cross-Stage Constraints
- Keep all production parsing changes in
- Keep 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.
- Use only the Go standard library and keep new helpers unexported.
- Do not change `model`, `standards`, source configuration or polling, schema
identifiers, event-envelope behavior, Postgres mapping, sinks, consumer docs,
or integration contracts.
- Continue exposing only key messages, short term, and long term.
- Map `KEY POINTS` to the existing key-message role. Keep `NEAR TERM`,
`DISCUSSION`, and every other unmapped identity boundary-only.
- Preserve first-occurrence behavior by canonical role, including across `KEY
MESSAGES` and `KEY POINTS` aliases.
- Preserve raw body lines during structural scanning. Apply provider-specific
cleanup only while parsing a mapped block.
- Prefer small grammar, label, and list-marker helpers over a broad regular
expression or general parsing framework.
- 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 Known Heading Parsing — Completed
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.
The provider parser gained a shared heading classifier and section-block model
for the original key-message, short-term, long-term, and aviation identities.
Discovery, boundary handling, and qualifier extraction stopped using separate
per-section patterns.
## Stage 2: Add Heading-Variant Regressions — Completed
## Stage 2: Add Original Heading-Variant Regressions — Completed
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.
Provider and normalizer tests covered ellipsis-first and slash-qualified
headings, malformed forms, empty bodies, mixed heading families, and unchanged
canonical wire shape.
## Stage 3: Validate and Record the Baseline — Completed
## Stage 3: Validate the Original Feature — Completed
Focused and full tests passed, production changes remained inside the NWS
provider parser, and the original feature roadmap was marked implemented.
Focused and repository-wide checks passed, production changes remained inside
the NWS provider parser, and the original roadmap was reconciled with the
implemented baseline.
## Stage 4: Generalize Heading Syntax and Centralize Canonical Roles
## Stage 4: Generalize Structural Heading Recognition — Completed
Decouple structural heading recognition from canonical section selection.
Heading syntax was decoupled from canonical roles. The parser now accepts a
constrained generic uppercase identity grammar, normalizes identity whitespace,
supports embedded slashes without confusing slash qualifiers, and recognizes
unknown valid identities as structural headings.
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:
## Stage 5: Scan Ordered Section Blocks Once — Completed
- 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:
Repeated extraction was replaced by a one-pass ordered block scanner. Generic
headings, `&&`, `$$`, and the watch/advisory safeguard terminate active blocks;
role lookup happens after scanning and retains first-occurrence behavior.
- 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`.
## Stage 6: Normalize Multiline Preambles and Markers — Completed
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:
Short- and long-term parsing gained next-line parenthesized qualifiers,
case-insensitive `Issued at` handling, and exact change-marker removal. Key
messages gained exact marker cleanup and removal of one leading issue/update
metadata line.
- `.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.
## Stage 7: Add Initial Cross-Office Coverage — Completed
The existing LSX fixture was retained and a compact BOU-style fixture added.
Provider and normalizer regressions cover multiline qualifiers, change markers,
leading metadata, generic boundary isolation, envelope behavior, and unchanged
JSON shape.
## Stage 8: Validate the First Resilience Follow-up — Completed
Focused tests, the full repository suite, static analysis, and diff checks
passed. The public model and schemas remained unchanged, and the roadmap was
updated to describe the completed behavior.
## Stage 9: Add Parenthesized-Terminal Heading Syntax
Recognize the observed `.<IDENTITY> (<qualifier>)...` family without weakening
generic identity validation.
1. In `internal/providers/nws/forecast_discussion.go`, add a dedicated
`parseForecastDiscussionParenthesizedTerminalHeading` helper returning the
existing `forecastDiscussionSectionHeading` type.
2. Update `parseForecastDiscussionSectionHeading` to try forms in this order:
1. if the line ends `/...`, try the slash-qualified helper and return its
result;
2. otherwise, try the parenthesized-terminal helper for any line ending
`...` and return it when it succeeds; and
3. fall back to the existing ellipsis-first helper.
Do not merge the forms into a single regular expression.
3. Implement the parenthesized-terminal helper with these exact rules:
- operate on the already outer-trimmed line and require leading `.` plus a
terminal ASCII `...`;
- remove the leading dot and terminal ellipsis, then remove horizontal
whitespace immediately before the ellipsis;
- require the remaining content to end with `)`;
- locate the first `(` that is preceded by horizontal whitespace; everything
before that separator is the raw identity and everything from `(` through
the final `)` is the qualifier;
- normalize and validate the identity through the existing identity helper;
- require nonempty text after trimming inside the outer parentheses;
- return the qualifier with its outer parentheses and internal punctuation
intact; and
- reject missing separator whitespace, empty qualifiers, missing or misplaced
parentheses, trailing text after the ellipsis, unsupported identity
punctuation, and lowercase or mixed-case identities.
Parentheses inside the qualifier are content; only the first separator `(`
and final `)` delimit the outer qualifier.
4. Extend the heading table in
`internal/providers/nws/forecast_discussion_test.go` with:
- `.DISCUSSION (Today through Thursday)...`;
- a mapped identity such as `.SHORT TERM (Tonight)...`;
- identity whitespace normalization;
- qualifier punctuation and nested parentheses;
- each rejected malformed form listed above; and
- compatibility assertions for every existing ellipsis-first and
slash-qualified case.
5. Add a scanner regression in which a mapped section is followed immediately,
without `&&`, by `.DISCUSSION (Today through Thursday)...`. Assert that the
discussion heading starts a new boundary-only block and its prose cannot leak
into the mapped section.
Run and pass:
@@ -112,46 +139,37 @@ gofmt -w internal/providers/nws/forecast_discussion.go internal/providers/nws/fo
go test -count=1 ./internal/providers/nws
```
Do not proceed until generic syntax tests pass without changing canonical
models or schemas.
Do not proceed until all previous heading and scanner tests still pass.
## Stage 5: Scan Ordered Blocks Once and Use Generic Boundaries
## Stage 10: Add Explicit Canonical Role Aliases
Replace repeated per-identity extraction with one structural pass.
Make true wording synonyms cheap to support while preserving semantic
distinctions.
1. Replace `extractForecastDiscussionSection` with
`parseForecastDiscussionSectionBlocks(lines []string) []forecastDiscussionSectionBlock`.
The scanner must:
1. Add `KEY POINTS` to the existing identity-to-role registry with the key-message
role. Keep the registry as the only production source of identity-to-role
mappings; do not add a parallel alias collection or role-selection switch.
2. Leave `KEY MESSAGES`, `SHORT TERM`, and `LONG TERM` mappings unchanged.
Explicitly do not add role mappings for `NEAR TERM`, `DISCUSSION`,
`AVIATION`, or other generic identities.
3. Update registry tests so they no longer assume one identity per role. Assert
instead that:
- 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:
- every registry identity parses as a heading;
- `KEY MESSAGES` and `KEY POINTS` both map to the key-message role;
- `SHORT TERM` and `LONG TERM` retain their roles; and
- `NEAR TERM`, `DISCUSSION`, and `AVIATION` have no role entry.
4. Add `ParseForecastDiscussionText` tests for role-level first occurrence:
- 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.
- `KEY POINTS` alone populates key messages;
- `KEY POINTS` followed by `KEY MESSAGES` keeps the first block; and
- `KEY MESSAGES` followed by `KEY POINTS` keeps the first block.
Use hyphen messages in this stage so list-tokenization changes remain scoped
to Stage 11.
5. Add a regression containing `NEAR TERM`, `SHORT TERM`, and `LONG TERM` in one
bulletin. Assert that near-term prose terminates adjacent blocks but does not
populate or override either canonical text section.
Run and pass:
@@ -160,85 +178,110 @@ gofmt -w internal/providers/nws/forecast_discussion.go internal/providers/nws/fo
go test -count=1 ./internal/providers/nws
```
## Stage 6: Normalize Multiline Preambles and NWS Presentation Markers
Do not change the canonical model to expose near-term or discussion content.
Handle real section-layout variation without weakening heading syntax.
## Stage 11: Harden Key-Message Metadata and Item Parsing
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:
Replace prefix-sensitive, hyphen-only parsing with conservative metadata and
list classifiers.
- 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:
1. Add a helper that recognizes a leading labeled metadata line only when:
- 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;
- its trimmed text begins with exactly `Issued at` or `Updated at`, compared
ASCII case-insensitively;
- at least one horizontal-whitespace byte follows the label; and
- parsing the remaining text through the existing unlabeled NWS issue-time
grammar succeeds.
Reuse `parseForecastDiscussionIssueTime` for the timestamp grammar after
removing the label. Do not add a second timestamp parser. A label prefix with
no boundary or with an invalid timestamp returns false and remains content.
2. Update key-message cleanup to remove at most one such valid leading metadata
line after presentation-marker removal and blank-line trimming. Later valid
timestamp lines remain message content.
3. Replace the hyphen-only check with a helper that classifies and strips one of
these markers from a trimmed line:
- one leading `-`, preserving current compatibility whether or not whitespace
follows it;
- one leading `*`, whether or not whitespace follows it; or
- one or more ASCII digits forming an integer greater than zero, followed by
`)` or `.`, and then either end-of-line or horizontal whitespace.
After recognizing and stripping a leading hyphen or asterisk plus horizontal
whitespace, also strip one immediately following valid numeric marker. This
supports composite forms such as `- 1.` without retaining either decorator.
Otherwise strip only the recognized marker and following horizontal
whitespace. Reject zero, overflow, alphanumeric prefixes, and numeric
punctuation without the required boundary. Do not interpret these markers
outside key-message parsing.
4. Parse a cleaned key-message body deterministically:
- first determine whether any nonempty line has a recognized marker;
- when markers exist, each marker starts a new message; nonempty unmarked
lines after a marker continue that message; blank lines after the first
marker are ignored; and blank-line-separated prose before the first marker
is flushed as preserved message content;
- when no marker exists, each nonempty paragraph separated by one or more
blank lines becomes one message; and
- in both modes, join wrapped lines with one ASCII space, discard empty
messages, and preserve source order.
5. Add table-driven marker tests for `-`, `*`, `1)`, `2.`, composite `- 1.` and
`* 1)`, multi-digit values, wrapped continuations, marker-only lines, and
rejected zero/malformed numeric prefixes.
6. Add block-level tests covering:
- numbered and asterisk lists producing distinct messages;
- unmarked paragraphs producing distinct messages;
- mixed introductory prose and marked items without data loss;
- valid leading `Issued at` and `Updated at` metadata removal;
- `Updated atmospheric conditions...` remaining content;
- `Updated at not a timestamp` remaining content;
- a later valid timestamp-like line remaining content; and
- all existing hyphen, change-marker, and continuation behavior.
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 12: Add Representative Numbered and Key-Points Fixtures
Prove the new extension points through provider and normalizer boundaries.
1. Retain the existing LSX and BOU fixtures and their regressions.
2. Add two compact HTML fixtures under `internal/providers/nws/testdata/`:
- a BGM/CTP-style fixture with a valid header and issue time, numbered `KEY
MESSAGES`, wrapped item lines, and a boundary-only `DISCUSSION` section; and
- an MFR-style fixture with a valid header and issue time, asterisk `KEY
POINTS`, and an immediately following
`.DISCUSSION (Today through Thursday)...` section without an intervening
`&&`.
Use concise representative text rather than full web pages. Add a short HTML
comment to each fixture naming the real office-format family it represents;
do not claim that edited fixture prose is a verbatim archived product.
3. Add provider end-to-end tests that assert:
- exact ordered key messages and joined continuations;
- `KEY POINTS` alias mapping;
- correct office and top-level issue metadata;
- absence of numeric/asterisk markers from canonical values;
- absence of `DISCUSSION` headings, qualifiers, and prose from key 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.
- nil short- and long-term fields when the fixture contains neither mapped
role.
4. Add table-driven normalizer regressions using both fixtures. For each, verify
input envelope preservation, event kind, canonical schema, effective time,
exact key-message mapping, and successful JSON marshaling.
5. Assert the JSON payload still has no `nearTerm`, `discussion`, `aviation`, or
generic `sections` field. Do not add those fields to provider or canonical
structs.
6. Keep syntax and tokenizer edge cases in focused unit tables; do not add more
full fixtures for cases already proven locally.
Run and pass:
@@ -247,17 +290,18 @@ gofmt -w internal/providers/nws/forecast_discussion_test.go internal/normalizers
go test -count=1 ./internal/providers/nws ./internal/normalizers/nws
```
## Stage 8: Reconcile Documentation and Perform Final Validation
## Stage 13: Reconcile Documentation and Perform Final Validation
Close the resilience follow-up only after all behavior is proven.
Close the second resilience follow-up only after all behavior is implemented and
verified.
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.
1. Review the final diff. Production changes must remain confined to
`internal/providers/nws/forecast_discussion.go`; other Go changes must be
owning provider and normalizer tests. Preserve unrelated user work.
2. Confirm the public contract is unchanged. If implementation appears to
require a canonical near-term/discussion field, schema change, configuration,
or downstream migration, stop and report the conflict rather than expanding
scope.
3. Run:
```sh
@@ -271,14 +315,18 @@ Close the resilience follow-up only after all behavior is proven.
git diff --check
```
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.
passing test, including compatibility with Stages 1-8.
5. Update the feature roadmap status to `Implemented.` and rewrite its remaining
proposed or future-tense language as implemented behavior only after all
checks pass.
6. Update this implementation plan so Stages 9-13 are marked `— Completed` and
the Purpose states that all stages are complete. Preserve the stage details
as a historical implementation record.
## Open Questions
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.
None. The plan makes the required policy choices explicitly: `KEY POINTS` is a
true key-message alias; `NEAR TERM` remains semantically distinct and
boundary-only; parenthesized-terminal syntax is a third constrained grammar;
metadata is removed only after timestamp validation; and key-message variants
are handled by an isolated marker classifier plus paragraph fallback.