Record the architecture audit findings

docs/roadmap/audit.md

@@ -1,6 +1,6 @@
 # Repository Audit Ledger
 
-Status: In progress; Stage 1 complete.
+Status: In progress; Stages 1-2 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
@@ -11,9 +11,12 @@ canonical owners identified by the
 ## Executive Summary
 
 Stage 1 established a clean, reproducible baseline. The repository-wide test
-suite, CLI help check, formatting check, and vet check all pass. No audit
-finding or retained decision was established during baseline collection.
-Subsystem conclusions and final disposition remain pending the later stages.
+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.
 
 ## Baseline Metadata
 
@@ -141,7 +144,7 @@ inventory commands, graph index refresh, and graph architecture inspection.
 | Stage | Scope | Status |
 | ---: | --- | --- |
 | 1 | Establish the baseline and audit ledger | Complete |
-| 2 | Audit architecture and dependency direction | Pending |
+| 2 | Audit architecture and dependency direction | Complete |
 | 3 | Audit report identity and time foundations | Pending |
 | 4 | Audit configuration, secrets, and validation | Pending |
 | 5 | Audit CLI parsing, wiring, and output contracts | Pending |
@@ -176,7 +179,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 | Pending |
+| 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 |
 | Configuration, validation, and secrets | `internal/config` | 4 | Pending |
 | CLI parsing, output, and exit behavior | `internal/cli` | 5 | Pending |
@@ -199,17 +202,107 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 
 ## Findings
 
-No findings were recorded in Stage 1. A passing baseline is not evidence that
-later audit scopes have no findings.
+### AUD-001: Unused persistence helpers remain after the stateless redesign
+
+- Stage: 2
+- Status: candidate
+- Severity: low
+- Confidence: high
+- Category: architecture
+- Area: `internal/promptinput.Save`,
+  `internal/adapters/weatherapi.SaveBundle`, and
+  `internal/fileutil.WriteJSONAtomic`
+- Evidence: Graph inbound traces show `promptinput.Save` has no callers,
+  `weatherapi.SaveBundle` is called only by `TestSaveBundle`, and
+  `fileutil.WriteJSONAtomic` is called only by `SaveBundle` and its focused
+  test. Repository text search found no documentation or other call sites.
+  Representative generate, batch, comparison, and collection traces do not
+  reach any of these helpers. `promptinput.Save` and `SaveBundle` write
+  intermediate prompt input or normalized weather data to arbitrary paths,
+  while normal publication uses `fileutil.WriteFileAtomic` for selected
+  Markdown and `comparison.Publish` for selected bundles.
+- Contract at risk: The architecture policy and ADR 0001 define normal
+  execution as an in-memory stateless transformation whose durable files are
+  operator-selected report outputs, comparison bundles, or explicitly
+  requested secure prompt debugging.
+- Impact: These unreachable exported functions do not create state during
+  current workflows, but they retain an unsupported persistence surface and
+  low-value tests that can invite accidental reintroduction of intermediate
+  artifacts or require maintenance despite having no product caller.
+- Recommendation: Remove the two unused domain/adapter save functions, remove
+  `WriteJSONAtomic` if it then has no production use, and delete or consolidate
+  tests that protect only those retired APIs.
+- Test implications: `TestSaveBundle` and `TestWriteJSONAtomic` protect unused
+  persistence mechanisms; `promptinput.Save` has no focused test. Preserve
+  tests for `WriteFileAtomic`, normal report publication, comparison
+  publication, and explicit prompt-debug writes.
+- Validation: Graph and text searches show no remaining production references
+  to the retired helpers; `go test ./...` passes; normal output, comparison,
+  and debug-publication tests remain unchanged and pass.
+- Related findings: none
+- Remediation reference: pending
 
 ## Retained Decisions
 
-No retained decisions were recorded in Stage 1.
+### RET-001: Keep the application package as the explicit composition owner
+
+The package import graph, outbound traces from `GenerateDetailed`,
+`RunBatchDetailed`, and `compareDetailed`, and their focused tests were
+inspected. `internal/app` has deliberately broad outbound dependencies because
+it sequences project-owned domain, collection, execution, publication, and
+notification contracts; those dependencies do not point back into app. Splitting
+that fan-out merely to reduce a graph metric would obscure workflow ownership.
+Reconsider only if a second composition owner emerges or a coherent workflow
+can move behind a narrower contract without duplicating ordering policy.
+
+### RET-002: Keep external dependencies behind repository-owned contracts
+
+Production import and data-flow inspection found Promptkit imports only in
+`internal/adapters/promptkit` and Distributor imports only in
+`internal/adapters/distributor`. The Promptkit adapter implements
+`promptexec.Executor`; Weather API collection returns `weatherdata.Bundle`;
+and Distributor results are translated before application and CLI summaries.
+This explicit translation prevents dependency types and sensitive diagnostics
+from becoming application contracts. Reconsider only when an upstream type is
+intentionally adopted as a public repository contract with corresponding
+architecture and compatibility changes.
+
+### RET-003: Keep publication mechanisms separate by artifact contract
+
+Filesystem-write discovery and call traces were inspected for single reports,
+comparison bundles, and secure prompt debugging. `fileutil.WriteFileAtomic`
+owns one-file replacement, `comparison.Publish` owns guarded transactional
+directory replacement and recovery, and `promptdebug` owns secure explicit
+diagnostic files. Their shared use of temporary paths and rename operations is
+mechanical similarity, while their authorization, permissions, commit, and
+recovery semantics differ materially. Reconsider common abstraction only if
+multiple artifact kinds acquire the same complete transaction contract.
+
+### RET-004: Keep collection as a narrow application-facing seam
+
+The `app.Collector` contract, `collect.Run`, the Weather API adapter boundary,
+and focused collection tests were inspected. Although `collect.Run` is small,
+it keeps adapter creation and error context out of orchestration and gives app
+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.
 
 ## Open Questions
 
-No Stage 1 open questions or unexplained baseline failures remain. Later
-stages may add scoped questions according to the audit plan.
+No Stage 1 open questions or unexplained baseline failures remain.
+
+Stage 2 routed these investigation leads without treating graph metrics or an
+unusual edge as findings:
+
+- `internal/config` imports `internal/briefing` to use module definitions while
+  validating report-module options. Stages 4, 8, and 23 should determine
+  whether this remains the smallest single-owner validation path or creates
+  avoidable registry coupling.
+- The graph identifies `internal/cli.Run`, `internal/app.RunBatchDetailed`, and
+  `internal/app.compareDetailed` as relatively complex orchestration paths.
+  Their ownership and top-level ordering are coherent; Stages 5, 16, 18, 22,
+  and 23 should assess their local behavior and maintainability rather than
+  inferring a finding from metrics.
 
 ## Stage Log
 
@@ -227,3 +320,61 @@ stages may add scoped questions according to the audit plan.
 - Findings: none.
 - Retained decisions: none.
 - Open questions: none.
+
+### Stage 2: Audit Architecture And Dependency Direction
+
+- Status: Complete.
+- Scope reviewed: `cmd/weatherreporter`, production import boundaries for all
+  24 Go packages, principal call and data-flow paths, external adapter
+  containment, project-owned interfaces, publication boundaries, filesystem
+  writes, ADR 0001, focused orchestration tests, and the architecture policy.
+- Exclusions: Detailed report/time, configuration, CLI, weather, domain,
+  module, prompt, rendering, comparison, and notification correctness remains
+  assigned to Stages 3-18. Cross-cutting complexity, deduplication, and full
+  documentation coherence remain assigned to Stages 22-24.
+
+#### Boundary Accounting
+
+| Normative boundary or invariant | Implementation owner and evidence | Disposition |
+| --- | --- | --- |
+| Binary entry and CLI ownership | `cmd/weatherreporter.main` calls `cli.Run`; `internal/cli` owns action parsing, config loading, executor construction, and bounded result output. Graph traces place app calls below action resolution. | Matches policy. |
+| Configuration ownership | `internal/config` owns defaults, YAML loading, normalization, secrets, and validation. CLI loads config and passes effective values down; app does not parse config files. | Matches policy; the config-to-briefing validation edge is routed to Stages 4, 8, and 23. |
+| Application orchestration | `GenerateDetailed`, `RunBatchDetailed`, and `compareDetailed` sequence preflight, collection, preparation, execution, publication, and notification through repository-owned values and interfaces. No lower package imports app. | Matches policy; retained as `RET-001`. |
+| Report and domain ownership | `report`, `timeutil`, `weatherdata`, `forecast`, `facts`, `module`, `briefing`, `promptinput`, `generatedtext`, and `reporttemplate` form one-way deterministic dependencies below app. | Matches policy at package level; local rules remain for Stages 3 and 7-13. |
+| Prompt execution boundary | `promptexec.Executor` is dependency-neutral. CLI constructs the Promptkit adapter once per action, app consumes only `promptexec` requests/results, and the prepared-report data flow supplies curated serialized module packages. | Matches policy; retained as `RET-002`. |
+| Weather API boundary | App calls its `Collector` interface; the production implementation delegates through `collect.Run` to `adapters/weatherapi.FetchBundle`, which returns normalized `weatherdata.Bundle`. Production HTTP imports occur only in external adapters. | Matches policy; retained as `RET-004`. |
+| Distributor boundary | App owns notification timing and repository request/result types; `distributorNotifier` translates them to adapter-local types and the adapter alone imports the Distributor dependency. Comparison exposes no notifier path. | Matches policy. |
+| Prompt preflight and curated inputs | Generate inspects the exact prompt/profile before `collectWeather`; batch inspects all candidates before its one collection; comparison inspects prompt and all profiles before collection, then calls `prepareReport` once. Data-flow traces reach `promptinput.Build` and `MarshalYAML`, not raw bundle serialization into Promptkit. | Matches policy at workflow level; detailed checks remain for Stages 10, 11, 14, and 18. |
+| Single output and notification order | `publishPromptReport` checks context, calls `fileutil.WriteFileAtomic`, records the path, and only then calls `notifyReport`. Focused tests cover pre-publication preservation and notification ordering. | Matches policy; publication split retained as `RET-003`. |
+| Batch output and notification order | `RunBatchDetailed` validates every planned path before sequential execution, suppresses item notification, continues across item failures, and calls `notifyBatch` only after the loop. Focused tests cover preflight, partial failure, and notification-after-publication. | Matches policy at workflow level; detailed review remains for Stage 16. |
+| Comparison publication | App preflights before external work and again before publication. `internal/comparison` owns bundle validation, guarded replacement, staging, commit, restoration, and cleanup results. The comparison request has no notifier and the trace does not reach Distributor. | Matches policy at workflow level; detailed review remains for Stages 17-18. |
+| Stateless execution | Normal traces retain preparation and execution values in memory. Production filesystem writes are limited to selected Markdown publication, selected comparison bundles, explicit prompt-debug capture, and the unreachable helpers in `AUD-001`; no cache, workspace, history, receipt, or resume owner appears in the package graph. | Matches normal-runtime policy; `AUD-001` records the unused persistence surface. |
+| Dependency cycles and direction | The compiler-derived direct-import inventory is acyclic. Entry packages point inward, adapters do not import app/CLI, domain packages do not import orchestration, and no external dependency type appears outside its production adapter. | Matches policy. |
+
+#### Commands And Evidence
+
+- Refreshed graph project `home-eric-Workspace-weatherreporter` in moderate
+  mode. The graph's branch identity remains production baseline `e7c7262`;
+  Stage 1's intervening commit changes only this excluded audit document.
+- Used graph architecture views for structure, dependencies, entry points,
+  hotspots, boundaries, layers, and clusters; queried the graph schema and
+  direct import edges.
+- Used graph search, snippets, inbound/outbound call traces, and data-flow
+  traces for `main`, `cli.Run`, `GenerateDetailed`, `RunBatchDetailed`,
+  `compareDetailed`, `prepareReport`, `executePreparedProfile`, `collect.Run`,
+  all three external adapters, `publishPromptReport`,
+  `comparison.PlanDestination`, `comparison.Publish`, and Distributor
+  notification.
+- Used compiler-derived `go list` direct imports to separate production edges
+  from test-only graph edges and confirm the build has no import cycle.
+- Used graph-augmented code search to inventory external dependency imports,
+  HTTP ownership, and production filesystem writes. A bounded text search
+  confirmed the unused persistence helpers have no documentation or hidden
+  non-code callers.
+- Ran
+  `go test ./internal/app ./internal/cli ./internal/collect ./internal/comparison ./internal/adapters/... ./internal/fileutil ./internal/promptdebug ./internal/promptexec`;
+  all focused packages passed.
+- Findings: `AUD-001`.
+- 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.