Complete report identity and time audit

docs/roadmap/audit.md

@@ -1,6 +1,6 @@
 # Repository Audit Ledger
 
-Status: In progress; Stages 1-2 complete.
+Status: In progress; Stages 1-3 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
@@ -15,8 +15,13 @@ suite, CLI help check, formatting check, and vet check all pass. Stage 2 found
 that the implemented package graph and principal workflows follow the intended
 dependency direction and assigned ownership. It recorded one low-severity
 candidate finding for unused internal persistence helpers left outside the
-documented stateless workflows. Subsystem conclusions and final disposition
-remain pending the later stages.
+documented stateless workflows. Stage 3 found one medium-severity correctness
+defect in wall-clock window construction on daylight-saving transition days
+and one medium-severity test gap around Daily run-ID uniqueness. Report
+registry identities, command and configuration lookup policies, batch
+membership, civil-day periods, and rolling-hour periods otherwise match their
+documented contracts. Subsystem conclusions and final disposition remain
+pending the later stages.
 
 ## Baseline Metadata
 
@@ -145,7 +150,7 @@ inventory commands, graph index refresh, and graph architecture inspection.
 | ---: | --- | --- |
 | 1 | Establish the baseline and audit ledger | Complete |
 | 2 | Audit architecture and dependency direction | Complete |
-| 3 | Audit report identity and time foundations | Pending |
+| 3 | Audit report identity and time foundations | Complete |
 | 4 | Audit configuration, secrets, and validation | Pending |
 | 5 | Audit CLI parsing, wiring, and output contracts | Pending |
 | 6 | Audit weather data acquisition and collection | Pending |
@@ -180,7 +185,7 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 | Risk area | Current test owner or evidence source | Audit stage | Coverage assessment |
 | --- | --- | ---: | --- |
 | Architecture and dependency direction | Assembled app and CLI tests; graph traces | 2 | Sufficient at the architectural boundary: direct imports are acyclic, external dependency types remain adapter-local, and representative workflow ordering and publication boundaries have focused tests. `AUD-001` records unused persistence APIs that do not participate in normal execution. |
-| Report identity, periods, dates, and timezones | `internal/report`, `internal/timeutil` | 3 | Pending |
+| Report identity, periods, dates, and timezones | `internal/report`, `internal/timeutil`, plus focused `internal/app` and `internal/forecast` callers | 3 | Partly insufficient. Registry identities, alias policies, batch membership, civil-day boundaries, and elapsed six-hour periods are coherent and exercised. `AUD-002` records incorrect wall-clock windows on daylight-saving transition days, and `AUD-003` records the missing regression test for Daily run-ID uniqueness across dates. |
 | Configuration, validation, and secrets | `internal/config` | 4 | Pending |
 | CLI parsing, output, and exit behavior | `internal/cli` | 5 | Pending |
 | Weather transport and normalized collection | `internal/adapters/weatherapi`, `internal/collect`, `internal/weatherdata` | 6 | Pending |
@@ -242,6 +247,81 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 - Related findings: none
 - Remediation reference: pending
 
+### AUD-002: Clock windows shift on daylight-saving transition days
+
+- Stage: 3
+- Status: candidate
+- Severity: medium
+- Confidence: high
+- Category: correctness
+- Area: `internal/timeutil.ClockWindow` and
+  `internal/forecast.ResolveDayparts`
+- Evidence: `ClockWindow` obtains the local civil-day start and creates its
+  boundaries with elapsed-duration additions such as
+  `day.Start.Add(17 * time.Hour)`. In `America/Chicago`, adding 17 elapsed
+  hours to local midnight produces 18:00 on the 2026-03-08 spring transition
+  and 16:00 on the 2026-11-01 fall transition; adding six hours produces
+  07:00 and 05:00 respectively. Its only production caller is
+  `forecast.ResolveDayparts`, which uses those boundaries for daily summaries
+  and facts. The focused time test covers only an overnight window in a fixed
+  offset zone, so both transition cases pass unnoticed.
+- Contract at risk: Configured daypart clocks are local wall-clock boundaries.
+  Civil-day and timezone policy requires those clocks to retain their stated
+  local hour across 23-hour and 25-hour days.
+- Impact: On daylight-saving transition dates, observations can enter or leave
+  a configured daypart an hour early or late, shifting derived daily summaries
+  and any downstream facts or report content that consume them.
+- Recommendation: Construct each boundary as a civil date plus local clock in
+  the target location, including explicit handling for `24:00` and overnight
+  windows, rather than adding elapsed hours to midnight.
+- Test implications: Add table-driven `America/Chicago` coverage for both
+  spring-forward and fall-back dates, asserting the requested wall-clock start
+  and end and the intended overnight date rollover. Retain the existing
+  half-open period assertions.
+- Validation: The new regression test fails against the elapsed-addition
+  implementation and passes when transition-day boundaries retain their
+  configured local clocks; focused `timeutil` and `forecast` suites pass.
+- Related findings: none
+- Remediation reference: pending
+
+### AUD-003: Daily run-ID uniqueness lacks a focused contract test
+
+- Stage: 3
+- Status: candidate
+- Severity: medium
+- Confidence: high
+- Category: testing
+- Area: `internal/report.Resolved.runID`,
+  `validStartDateRunIDDisambiguator`, and focused report tests
+- Evidence: Daily run IDs append the resolved valid-start date to the shared
+  generation timestamp and report ID. This distinguishes multiple Daily
+  reports planned for different dates in one batch, where `GeneratedAt` is the
+  same. Focused report tests assert that registry identities are nonempty and
+  app tests use nonempty or fixture run IDs, but no test resolves two Daily
+  dates at one clock instant and asserts distinct IDs. Downstream
+  `plannedReportsByRunID` rejects duplicate IDs before building a batch
+  notification, and Distributor idempotency also consumes the run ID.
+- Contract at risk: A run ID must identify one resolved report product, while
+  one batch may legitimately contain several Daily products for different
+  valid dates.
+- Impact: An accidental removal or weakening of the date disambiguator can
+  give distinct Daily products the same identity, causing batch notification
+  construction to fail and creating Distributor idempotency collisions even
+  though the existing focused suite remains green.
+- Recommendation: Add a focused report test that resolves two Daily dates with
+  the same injected `Now`, asserts distinct nonempty run IDs, and verifies that
+  each ID carries the corresponding valid-start date. Prefer this relational
+  contract over freezing unrelated timestamp formatting.
+- Test implications: The regression must fail if the Daily disambiguator is
+  removed while allowing harmless changes to the common run-ID timestamp
+  representation. An app-level assertion may additionally protect the
+  multi-Daily batch consumer, but should not duplicate the report invariant.
+- Validation: Removing the date disambiguator makes the new focused test fail;
+  with the invariant intact, report and multi-Daily batch tests pass and
+  `plannedReportsByRunID` receives unique keys.
+- Related findings: none
+- Remediation reference: pending
+
 ## Retained Decisions
 
 ### RET-001: Keep the application package as the explicit composition owner
@@ -287,6 +367,40 @@ tests a dependency-neutral deterministic seam. Reconsider if collection gains
 no additional policy and an equally narrow project-owned adapter contract can
 replace it without leaking transport construction into app.
 
+### RET-005: Keep report definitions explicit and independently reviewable
+
+The four registry definitions were compared across stable ID, prompt and
+version, template and schema, period resolver, modules, output name,
+Distributor paths, and run-ID policy. Their similar structure is contract
+data, while meaningful differences include Daily's required date and dynamic
+output identity, Today's current local day and Morning membership, Tomorrow's
+next local day and both batch memberships, and Hourly's rolling elapsed
+period. Explicit definitions make those differences visible and focused tests
+assert the exact retained registry surface. Reconsider a shared declarative
+builder only if the registry grows enough that it can reduce maintenance while
+preserving independent review of every identity field.
+
+### RET-006: Keep command and configuration report lookup policies separate
+
+`IDForCommandName` accepts only the four exact CLI command names, while
+`IDForConfigKey` deliberately trims space, folds case, and normalizes hyphens
+for configuration keys. Tests also reject retired aliases. Combining the two
+lookups because their successful mappings are similar would risk broadening
+the CLI contract or narrowing configuration compatibility. Reconsider only if
+the accepted-input policies intentionally converge or registry metadata can
+generate both maps without changing either boundary.
+
+### RET-007: Keep civil-day and rolling elapsed-time period policies distinct
+
+Daily, Today, and Tomorrow use `CivilDay`, whose `AddDate` boundary correctly
+produces 23-hour and 25-hour local days. Hourly instead adds six elapsed hours
+to the injected clock, matching its documented rolling-horizon contract.
+Batch coverage likewise walks the actual hourly instants within each civil
+day, and focused app tests exercise daylight-saving dates. Unifying these
+period calculations would conflate calendar and elapsed-time semantics.
+Reconsider only if a report's product contract changes which kind of time it
+represents.
+
 ## Open Questions
 
 No Stage 1 open questions or unexplained baseline failures remain.
@@ -304,6 +418,21 @@ unusual edge as findings:
   and 23 should assess their local behavior and maintainability rather than
   inferring a finding from metrics.
 
+Stage 3 routed these investigation leads to their assigned later stages:
+
+- `LoadLocation` accepts numeric offsets through `23:59`, while the config
+  reference does not state an offset range. Stage 4 should assess the effective
+  configuration contract and validation ownership before judging that bound.
+- The report-registry document says focused tests cover run IDs and output
+  names, but Stage 3 found no behavioral run-ID assertion and only indirect
+  dynamic output-name coverage. `AUD-003` captures the consequential run-ID
+  gap; Stage 24 should reconcile the broader documentation claim with the
+  eventual executable contract.
+- Prompt, template, schema, and module IDs are internally consistent at the
+  report registry. Stages 8 and 10-13 remain responsible for validating the
+  referenced registries, embedded assets, generated-text schemas, and render
+  compatibility rather than duplicating those audits here.
+
 ## Stage Log
 
 ### Stage 1: Establish The Baseline And Audit Ledger
@@ -378,3 +507,56 @@ unusual edge as findings:
 - Retained decisions: `RET-001`, `RET-002`, `RET-003`, and `RET-004`.
 - Open questions: the two leads recorded above are routed to their assigned
   later stages.
+
+### Stage 3: Audit Report Identity And Time Foundations
+
+- Status: Complete.
+- Scope reviewed: all production and focused-test code in `internal/report`
+  and `internal/timeutil`; report and timezone contracts in `docs/cli.md`,
+  `docs/config.md`, and `docs/internal/report-registry.md`; and the focused
+  `internal/app` and `internal/forecast` callers that consume batch membership,
+  run IDs, civil periods, clock windows, and hourly coverage.
+- Exclusions: Effective configuration validation and CLI date parsing remain
+  assigned to Stages 4-5. Forecast derivation beyond the time boundary remains
+  assigned to Stage 7. Module, prompt, schema, and template compatibility
+  remains assigned to Stages 8 and 10-13. Batch notification and Distributor
+  idempotency behavior remains assigned to Stage 16, and documentation-wide
+  coherence remains assigned to Stage 24.
+
+#### Contract Accounting
+
+| Contract or risk | Implementation and caller evidence | Disposition |
+| --- | --- | --- |
+| Stable report identities | The default registry contains exactly Daily, Today, Tomorrow, and Hourly. Each definition carries the documented prompt `2.0.0`, matching template/schema identity, modules, output name, and Distributor paths; focused registry tests assert the retained surface. | Matches the registry contract; explicit definitions retained as `RET-005`. Asset compatibility remains for later owner stages. |
+| Alias resolution | Command lookup accepts exact current command names. Config lookup separately normalizes whitespace, case, and hyphens. Both reject retired aliases in focused tests. | Matches the distinct boundary contracts; separation retained as `RET-006`. |
+| Batch membership and ordering | Morning explicitly plans Today then Tomorrow before eligible Daily dates; Evening explicitly plans Tomorrow before eligible Daily dates. Focused app tests cover ordering, flags, multiple Daily dates, and no eligible dates. | Matches CLI and registry contracts. Detailed partial-success and notification semantics remain for Stage 16. |
+| Report period policy | Daily requires an explicit date; Today uses an explicit date or current local date; Tomorrow selects the next local civil day; Hourly spans six elapsed hours from the injected clock. `Resolve` defaults a missing location to UTC and a zero clock to the system clock. | Matches documented selection policy; calendar and elapsed policies retained separately as `RET-007`. |
+| Civil-day and hourly DST behavior | `CivilDay` uses local midnight plus one calendar day, and batch hourly coverage walks actual instants, correctly accommodating 23-hour and 25-hour dates. Focused batch tests exercise DST civil days. | Sufficient for the reviewed policy. |
+| Configured clock windows | `ClockWindow` adds elapsed clock durations to local midnight. The only production path reaches `forecast.ResolveDayparts`, daily summaries, and facts. Fixed-zone tests do not exercise either DST transition. | Incorrect on DST dates; recorded as `AUD-002`. |
+| Date and timezone parsing | `ParseLocalDate` uses strict `YYYY-MM-DD` parsing in the selected location. `LoadLocation` covers IANA names, `America/` fallback, documented aliases, fixed US abbreviations, and signed offsets, returning contextual errors for invalid input. | Implementation is coherent for Stage 3. Effective config validation and CLI error presentation remain for Stages 4-5. |
+| Clock control | All report resolvers accept an injected `Now`; only zero-valued input falls back to `time.Now`. Focused report and batch tests use fixed clocks. | Sufficient for deterministic period selection. |
+| Daily output and run identity | Daily output names derive from the resolved valid-start date, and Daily run IDs append that date so same-clock multi-date products remain distinct. The downstream batch-notification map rejects duplicate IDs. | Implementation is coherent, but the identity invariant lacks a focused behavior test; recorded as `AUD-003`. |
+| Half-open period operations | `Period.Valid`, `Contains`, `Overlaps`, and `Intersection` consistently use `[start,end)` semantics, and focused tests protect overlap boundaries. | Matches the internal period contract. |
+
+#### Commands And Evidence
+
+- Used graph search, source snippets, and inbound/outbound traces for the four
+  report definitions, registry construction and lookup, command/config mapping,
+  batch mapping and planning, `Resolve`, every period resolver, `CivilDay`,
+  `ClockWindow`, `ParseLocalDate`, `LoadLocation`, period overlap operations,
+  Daily output naming, run-ID construction, `ResolveDayparts`, batch hourly
+  coverage, and `plannedReportsByRunID`.
+- Compared the implementation with `docs/cli.md`, `docs/config.md`, and
+  `docs/internal/report-registry.md`, then inspected the focused report, time,
+  forecast, and app tests. Bounded text searches were used for string and test
+  assertion discovery not represented by the code graph.
+- Reproduced the clock-window defect with `America/Chicago` transition dates:
+  17 elapsed hours after midnight yields 18:00 on 2026-03-08 and 16:00 on
+  2026-11-01; six elapsed hours yields 07:00 and 05:00 respectively.
+- Ran
+  `go test ./internal/report ./internal/timeutil ./internal/forecast ./internal/app`;
+  all focused packages passed.
+- Findings: `AUD-002` and `AUD-003`.
+- Retained decisions: `RET-005`, `RET-006`, and `RET-007`.
+- Open questions: the three leads recorded above are routed to their assigned
+  later stages.