Record Stage 7 derivation audit findings

docs/roadmap/audit.md

@@ -1,6 +1,6 @@
 # Repository Audit Ledger
 
-Status: In progress; Stages 1-6 complete.
+Status: In progress; Stages 1-7 complete.
 
 This temporary roadmap document is the evidence ledger for the staged audit
 defined by [the audit plan](audit-plan.md). It records audit evidence and
@@ -41,8 +41,16 @@ required hourly products accept periods without usable time bounds. It also
 found one low-severity retry-policy defect because warmup retries permanent
 HTTP failures. Source availability policy, checked-empty products, query
 construction, cancellation, provenance, normalization ownership, and focused
-offline fixtures otherwise match their contracts. Subsystem conclusions and
-final disposition remain pending the later stages.
+offline fixtures otherwise match their contracts. Stage 7 found three
+medium-severity derivation defects: metric fallback values lose their unit and
+are compared or presented as US customary values, overnight dayparts omit
+alerts that begin after midnight, and precipitation probabilities outside the
+percentage domain flow into summaries. It also found one low-severity text
+classification defect because ordinary “icy” conditions do not set the ice
+indicator. Half-open period selection, deterministic ordering, alert parsing,
+precipitation-window grouping, checked-empty SPC handling, report-family fact
+selection, and realistic derivation cost otherwise match their contracts.
+Subsystem conclusions and final disposition remain pending the later stages.
 
 ## Baseline Metadata
 
@@ -175,7 +183,7 @@ inventory commands, graph index refresh, and graph architecture inspection.
 | 4 | Audit configuration, secrets, and validation | Complete |
 | 5 | Audit CLI parsing, wiring, and output contracts | Complete |
 | 6 | Audit weather data acquisition and collection | Complete |
-| 7 | Audit forecast and fact derivation | Pending |
+| 7 | Audit forecast and fact derivation | Complete |
 | 8 | Audit module contracts, registry, and source-facing briefing modules | Pending |
 | 9 | Audit derived, planning, formatting, and SPC briefing modules | Pending |
 | 10 | Audit prompt inputs, assets, and neutral execution contracts | Pending |
@@ -210,7 +218,7 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 | Configuration, validation, and secrets | `internal/config`, focused `internal/cli` and Promptkit adapter mapping, maintained examples | 4 | Partly insufficient. Defaults, precedence, known-field parsing, active URL/duration/template checks, output-path preservation, profile-source rules, secret redaction, and examples are exercised. `AUD-004` through `AUD-007` record unsupported missing-source keys, blank notification identities, constructed-report override inconsistency, and non-atomic secret environment mutation. |
 | CLI parsing, output, and exit behavior | `cmd/weatherreporter`, `internal/cli`, representative app-facing CLI tests | 5 | Partly insufficient. Command/flag ownership, paths, one-executor mapping, stream separation, quiet mode, comparison safe errors, and visible result/exit rules are coherent. `AUD-008` records missing signal cancellation, `AUD-009` records late generate date validation, and `AUD-010` records missing assembled non-comparison CLI protection. |
 | Weather transport and normalized collection | `internal/adapters/weatherapi`, `internal/collect`, `internal/weatherdata` | 6 | Partly insufficient. All eight source requests, required/optional policy, checked-empty products, retryable source statuses, cancellation, normalized provenance, and collection error ownership have focused offline coverage. `AUD-011` through `AUD-015` record unsupported schemes, unsafe response-body diagnostics, an unenforced body-size limit, structurally invalid required hourly periods, and overbroad warmup retries. |
-| Forecast and fact derivation | `internal/forecast`, `internal/facts` | 7 | Pending |
+| Forecast and fact derivation | `internal/forecast`, `internal/facts` | 7 | Partly insufficient. Half-open selection, stable ordering, alert parsing and clipping, precipitation-window grouping, report-family selection, and SPC checked-empty behavior have focused owners. `AUD-016` through `AUD-019` record unit collapse, overnight alert omission, invalid percentage acceptance, and incomplete ice-text classification. |
 | Module and briefing contracts | `internal/module`, `internal/briefing` | 8-9 | Pending |
 | Prompt inputs, embedded assets, and execution contracts | `internal/promptinput`, `internal/promptassets`, `internal/promptexec` | 10 | Pending |
 | Promptkit boundary and sensitive debug output | `internal/adapters/promptkit`, `internal/promptdebug` | 11 | Pending |
@@ -785,6 +793,145 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 - Related findings: none
 - Remediation reference: pending
 
+### AUD-016: Metric fallback values lose their unit during daypart derivation
+
+- Stage: 7
+- Status: candidate
+- Severity: medium
+- Confidence: high
+- Category: correctness
+- Area: `internal/forecast.SummarizeDaypart`, `numericIndicators`, and
+  unitless `Range`/`TimedValue` summary fields
+- Evidence: Normalized forecast periods keep temperatures in explicit
+  Fahrenheit/Celsius fields and winds in explicit mph/km/h fields.
+  `SummarizeDaypart` selects the first populated temperature or wind field and
+  stores the raw number in a summary field with no unit. `numericIndicators`
+  likewise selects mph before km/h but compares either result to the mph
+  thresholds of 35 for gusts and 25 for sustained wind. Metric mode is an
+  accepted runtime setting, and focused derivation fixtures populate only
+  Fahrenheit and mph. Consequently, a metric-only gust of 35 km/h is marked
+  hazardous even though it is about 22 mph, while metric temperature and wind
+  summary values are indistinguishable from US customary values to immediate
+  facts consumers.
+- Contract at risk: Derivation must preserve or normalize measurement units so
+  equivalent meteorological inputs produce equivalent summaries and hazard
+  classifications under every supported units setting.
+- Impact: Metric reports can exaggerate wind hazards and carry Celsius or km/h
+  numbers into downstream fields that consumers interpret as Fahrenheit or
+  mph. The defect can materially change planning facts and generated report
+  content.
+- Recommendation: Give derived ranges and timed values an explicit unit, or
+  convert metric fallbacks into one documented canonical unit before storing
+  or comparing them. Apply thresholds only in their declared unit and avoid
+  silently preferring one of two populated representations unless their
+  equivalence is validated.
+- Test implications: Add paired US/metric cases for temperature, apparent
+  temperature, wind speed, and gust. Assert equivalent canonical summaries and
+  indicators at values immediately below, at, and above the thresholds,
+  including periods with both unit fields populated.
+- Validation: Equivalent US and metric inputs produce the same heat, cold, and
+  wind decisions and unambiguous summary values; all immediate derived-fact
+  consumers retain correct units.
+- Related findings: none
+- Remediation reference: pending
+
+### AUD-017: Overnight dayparts omit alerts that begin after midnight
+
+- Stage: 7
+- Status: candidate
+- Severity: medium
+- Confidence: high
+- Category: correctness
+- Area: `internal/forecast.BuildDailySummary` and `overlapsWithin`
+- Evidence: `BuildDailySummary` first limits alerts to the civil-day period and
+  then derives each daypart's alerts only from that reduced set. Configured
+  daypart windows may cross midnight, such as 22:00-06:00, and hourly period
+  selection correctly includes the next morning. An alert beginning at 01:00
+  the next day does not overlap the prior civil day, so it is discarded before
+  the overnight window is evaluated even though it overlaps that window.
+  Existing tests separately protect direct alert overlap and overnight hourly
+  grouping, but do not combine the two through daily-summary construction.
+- Contract at risk: Each daypart summary must be derived from its complete
+  configured half-open period, including the next-day portion of a wrapping
+  window.
+- Impact: A report can include next-morning forecast conditions in its
+  overnight daypart while omitting an active alert for exactly those hours,
+  understating overnight risk.
+- Recommendation: Compute daypart alert overlaps directly from the source
+  alert run and each daypart period. Keep the top-level daily alert list scoped
+  to the civil day so its separate contract does not widen.
+- Test implications: Add an integration case with an overnight daypart and an
+  alert wholly after midnight. Assert that the alert appears clipped in the
+  daypart, does not appear in the prior day's top-level list, and an alert
+  beginning exactly at the daypart end remains excluded.
+- Validation: Every wrapping daypart includes all and only alerts overlapping
+  its full period, while daily alert summaries retain civil-day boundaries.
+- Related findings: `AUD-002`
+- Remediation reference: pending
+
+### AUD-018: Invalid precipitation percentages flow into derived summaries
+
+- Stage: 7
+- Status: candidate
+- Severity: medium
+- Confidence: high
+- Category: correctness
+- Area: `internal/forecast.BuildPrecipTiming`, `SummarizeDaypart`, and
+  `weatherdata.ForecastPeriod.ProbabilityOfPrecipitationPercent`
+- Evidence: Both derivation paths accept any non-nil probability without
+  enforcing the percentage domain. Values above 100 can become daypart or
+  timing maxima and open precipitation windows, while negative values can
+  become maxima in otherwise sparse input. The Weather API normalization
+  boundary decodes the field but does not range-check it. Focused tests cover
+  zero, ordinary values, and values immediately around the default 40-percent
+  threshold, but not values below zero or above 100.
+- Contract at risk: A field named and consumed as a percentage must remain
+  within 0 through 100 before meteorological comparisons or presentation.
+- Impact: Malformed upstream data can produce impossible precipitation facts
+  and misleading timing windows while collection and derivation both report
+  success.
+- Recommendation: Enforce the numeric domain at the normalization boundary so
+  every consumer receives valid percentages. Define whether a malformed value
+  makes required hourly input fail or is omitted with an explicit warning;
+  derivation should not silently clamp it because that hides source corruption.
+- Test implications: Add normalization and derivation cases for negative,
+  100, and above-100 values, plus non-finite values if the wire decoder can
+  produce them. Protect the exact threshold and zero semantics already tested.
+- Validation: No accepted derived fact contains a probability outside 0-100,
+  and malformed source behavior is deterministic and visible under the chosen
+  source-failure policy.
+- Related findings: `AUD-014`
+- Remediation reference: pending
+
+### AUD-019: Ordinary “icy” forecast text does not set the ice indicator
+
+- Stage: 7
+- Status: candidate
+- Severity: low
+- Confidence: high
+- Category: correctness
+- Area: `internal/forecast.indicatorsForText`
+- Evidence: Ice detection checks for the literal substrings `ice`, `freezing`,
+  and `sleet`. The common condition adjective `icy` contains none of those
+  byte sequences, so a description such as “Icy roads” does not set the ice
+  indicator. Focused tests exercise broader fixture summaries but provide no
+  table of representative positive and negative condition phrases.
+- Contract at risk: The documented text indicators identify snow, ice, fog,
+  and wind mentions in normalized forecast descriptions.
+- Impact: Reports can omit an ice hazard indicator for plainly icy conditions.
+  Other text or numerical signals may still reveal the condition, which keeps
+  severity low.
+- Recommendation: Define a small explicit vocabulary or token-aware matcher
+  for supported condition forms, including `icy`, rather than relying only on
+  incidental substrings. Keep the vocabulary bounded to avoid false positives.
+- Test implications: Add table-driven phrases for ice/icy/freezing/sleet,
+  mixed case, punctuation, and nearby negative words; retain separate tests
+  for snow, fog, wind, heat, and cold.
+- Validation: Representative icy descriptions set the ice indicator without
+  broadening matches to unrelated words.
+- Related findings: none
+- Remediation reference: pending
+
 ## Retained Decisions
 
 ### RET-001: Keep the application package as the explicit composition owner
@@ -933,6 +1080,30 @@ table only if it can express every source's availability, empty-value,
 timestamp, and validation policy without callbacks that recreate the current
 methods indirectly.
 
+### RET-014: Keep selected forecast periods structurally intact
+
+Hourly, narrative, and daily selection uses half-open overlap and returns the
+complete normalized source periods in stable chronological order. It does not
+fabricate clipped forecast periods at the requested boundary; alert overlap is
+separately clipped because it explicitly represents an intersection. Focused
+tests protect inclusion of an already-active hourly period at a rolling
+horizon and exclusion at exact end boundaries. This preserves source identity,
+values, and timestamps for downstream reasoning. Reconsider only if a consumer
+requires synthetic boundary-aligned values and that interpolation or clipping
+policy is made explicit rather than implied by selection.
+
+### RET-015: Keep fact projection shallow under immutable workflow ownership
+
+`BuildCollected` and `CollectedFacts.Bundle` allocate independent top-level
+provenance and warning slices but retain pointers to normalized weather
+products and nested values. Derivation treats those products as immutable, and
+prepared-report construction later deep-copies complete inputs before handing
+them to consumers. Deep-copying the full weather bundle at every fact boundary
+would add repeated allocation without protecting a currently mutable seam.
+Reconsider if facts begin mutating normalized products, an external consumer
+can retain and mutate them concurrently, or the fact API promises complete
+snapshot isolation rather than immutable workflow ownership.
+
 ## Open Questions
 
 No Stage 1 open questions or unexplained baseline failures remain.
@@ -1032,6 +1203,23 @@ Stage 6 routed these investigation leads to their assigned later stages:
   of `AUD-011`, rather than Stage 6 inventing undocumented authentication or
   query behavior.
 
+Stage 7 routed these investigation leads to their assigned later stages:
+
+- `DifferenceAtLeast`, `CrossesAtOrAbove`, and `CrossesBelow` are exported
+  forecast helpers with focused tests but no production callers in the graph.
+  Stage 23 should assess removing this unused comparison surface rather than
+  treating locally correct arithmetic as a derivation defect.
+- The forecast derivation document describes
+  `BuildPeriodDailySummaries`, but no such symbol or equivalent multi-day
+  constructor exists in the implementation. Current facts intentionally build
+  one civil-day summary for Daily, Today, and Tomorrow. Stage 24 should
+  reconcile the named API and actual one-day report contract.
+- Planning and derived briefing consumers assign Fahrenheit/mph meaning to
+  the unitless daypart values identified by `AUD-016`, and hazard wording may
+  amplify the missed text classification in `AUD-019`. Stage 9 should account
+  for those downstream effects while avoiding duplicate findings for the same
+  derivation causes.
+
 ## Stage Log
 
 ### Stage 1: Establish The Baseline And Audit Ledger
@@ -1323,3 +1511,54 @@ Stage 6 routed these investigation leads to their assigned later stages:
   explicit source normalization.
 - Open questions: the four leads recorded above are routed to their assigned
   later stages.
+
+### Stage 7: Audit Forecast And Fact Derivation
+
+- Status: Complete.
+- Scope reviewed: all production code and focused tests in
+  `internal/forecast` and `internal/facts`; the forecast derivation and facts
+  internal documents; normalized forecast types; and immediate briefing/app
+  consumers needed to determine unit meaning, mutation ownership, and whether
+  derived facts participate in normal workflows.
+- Exclusions: Detailed source-facing, derived, planning, formatting, and SPC
+  briefing behavior remains assigned to Stages 8-9. Cross-cutting test
+  portfolio and cleanup remain assigned to Stages 19-23, documentation
+  reconciliation to Stage 24, and adversarial dynamic checks to Stage 25.
+
+#### Derivation Rule And Edge-Case Accounting
+
+| Derivation rule or risk | Boundary, ordering, and test evidence | Disposition |
+| --- | --- | --- |
+| Valid-period selection | Hourly, narrative, and daily runs use half-open overlap, retain complete source periods, and stable-sort by start time. Nil runs return no facts. Focused tests cover unsorted input, partial overlap, and exact-boundary exclusion. | Matches the source-period contract; complete-period selection is retained as `RET-014`. Structurally invalid required periods remain `AUD-014`. |
+| Civil-day summaries and dayparts | Daily summaries require hourly data, resolve configured local windows, and select each daypart independently. Empty dayparts remain explicit summaries; overnight hourly selection crosses midnight. | Ordinary and empty behavior matches. DST window construction remains `AUD-002`; after-midnight alert selection is incorrect in `AUD-017`. |
+| Temperature and wind aggregation | Min/max and peak selection is deterministic for populated values; numeric heat/cold thresholds distinguish Fahrenheit and Celsius. Wind and all unitless summary fields fall back between explicit unit fields without conversion. | Incorrect for metric fallback values; `AUD-016`. Paired unit-system regressions are absent. |
+| Condition indicators | Text matching is case-insensitive; numerical heat, cold, and wind signals merge monotonically across periods. Dominant-condition ties resolve lexically and notable conditions sort. | Deterministic, but common “icy” text is missed; `AUD-019`. Broader briefing interpretation remains Stage 9. |
+| Precipitation timing | Input is copied and stable-sorted. Exact threshold inclusion, dry input, gaps, maxima, text aggregation, thunder mentions, and intentionally open final windows have focused tests. | Window mechanics match. The percentage domain is unenforced in `AUD-018`; the open final window is an intentional horizon signal. |
+| Alert parsing and overlap | Supported time-field fallbacks, malformed omission, instruction whitespace, half-open clipping, ordering, and direct boundary cases have focused tests. | Direct behavior matches. Daily prefiltering loses alerts in wrapping dayparts; `AUD-017`. |
+| Collected facts and mutation | Collection projects every normalized product and copies top-level sources/warnings. Bundle reconstruction repeats those top-level copies; derivation reads shared nested values without mutation. | Fits immutable workflow ownership and is retained as `RET-015`; no speculative deep-copy requirement was added. |
+| Report-family derived facts | Every report selects valid-period hourly, narrative, daily, alert, precipitation, and SPC facts. Daily/Today/Tomorrow additionally build one daily/daypart summary; Hourly does not; unknown reports and invalid periods fail. | Matches the implemented report-family contract with focused cases. The stale multi-day documentation name is routed to Stage 24. |
+| SPC outlooks and discussions | Only valid overlapping outlook periods are selected. Output ordering uses day, type, severity rank, validity, label, and ID; discussions are retained only for selected days and sorted deterministically. Nil and checked-empty runs remain empty facts. | Matches documented checked-empty and deterministic ordering behavior with focused tests. Detailed briefing meaning remains Stage 9. |
+| Repeated work and realistic size | One derived build performs several linear selections, then each configured daypart scans the hourly run and its alert candidates; selected precipitation and SPC slices are sorted. With the normal four dayparts and service-sized runs of a few hundred periods, this is only low-thousands of simple overlap checks. | No Stage 7 performance finding. Reassess in Stage 22 only if horizons, daypart counts, or alert volumes become materially larger or derivation is repeated per module. |
+| Test ownership | `forecast` owns selection, summaries, thresholds, precipitation, parsing, overlap, ordering, and fixture integration. `facts` owns projection, report-family derivation, SPC filtering/order, and failure cases. `timeutil` owns the underlying period rules. | Ownership is coherent. Missing high-risk unit, overnight-alert, percentage-domain, and icy-text cases are attached to `AUD-016` through `AUD-019`, not inferred from coverage alone. |
+
+#### Commands And Evidence
+
+- Used graph architecture, symbol search, source snippets, call traces, and
+  graph queries for every scoped derivation function, report-family consumers,
+  unit-bearing normalized fields, comparison helpers, loops, sorting, and
+  immediate unit interpretations. Text search was limited to documents,
+  literals, fixtures, and focused assertions where graph discovery was not
+  applicable.
+- Compared implementation and tests with
+  `docs/internal/forecast-derivation.md`, `docs/internal/facts.md`, and the
+  architecture, documentation, and testing policies. Existing Stage 3 and
+  Stage 6 findings were treated as dependencies rather than duplicated.
+- Ran
+  `go test -coverprofile=/tmp/weatherreporter-stage7-cover.out ./internal/forecast ./internal/facts ./internal/timeutil`;
+  all focused packages passed. Statement coverage was 89.7% for forecast,
+  73.4% for facts, and 54.5% for timeutil; coverage guided branch inspection
+  but was not itself a finding.
+- Findings: `AUD-016`, `AUD-017`, `AUD-018`, and `AUD-019`.
+- Retained decisions: `RET-014` and `RET-015`.
+- Open questions: the three leads recorded above are routed to their assigned
+  later stages.