Complete the Stage 22 efficiency audit

docs/roadmap/audit.md

@@ -325,7 +325,7 @@ inventory commands, graph index refresh, and graph architecture inspection.
 | 19 | Audit test hermeticity and execution hygiene | Complete |
 | 20 | Audit test risk coverage and ownership | Complete |
 | 21 | Audit test durability, duplication, and maintenance cost | Complete |
-| 22 | Audit cross-cutting efficiency and complexity | Pending |
+| 22 | Audit cross-cutting efficiency and complexity | Complete |
 | 23 | Audit cross-cutting refactoring and deduplication opportunities | Pending |
 | 24 | Audit documentation coherence and executable contracts | Pending |
 | 25 | Run dynamic robustness and final diagnostic validation | Pending |
@@ -359,7 +359,8 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 | Test hermeticity and execution hygiene | All Go tests, fixtures, test assets, and helper mechanisms | 19 | Partly insufficient. The suite is offline, uses controlled clocks/roots/environment for almost every stateful case, has no subprocesses or automatic golden updates, passes race and shuffled-repeat checks, and succeeds with a scrubbed environment and broken external proxies. `AUD-059` records one ambient credential assumption; `AUD-060` records unguarded symlink-capability assumptions. |
 | Test risk coverage and ownership | Repository-wide suite | 20 | Partly insufficient. Consequential contracts have narrow owners and representative cross-boundary workflows; every identified missing regression is already attached to the production or test-gap finding for the defect it would catch. Coverage diagnostics found no additional unowned critical branch. Accepted omissions are thin entry-point/delegation wrappers, defensive accessors, and dormant helpers already routed under `AUD-001`; broad duplication and durability remain Stage 21. |
 | Test durability, duplication, and maintenance cost | Repository-wide suite | 21 | Partly insufficient. Most tests assert stable behavior with direct fixtures and appropriately narrow fakes. `AUD-039`, `AUD-061`, and `AUD-063` identify duplicated or implementation-coupled assertions; `AUD-062` identifies avoidable production waits; `AUD-064` identifies incomplete failure cleanup. Each consolidation retains a named behavioral owner. |
-| Cross-cutting efficiency and maintainability | Graph metrics plus focused tests | 22-23 | Pending |
+| Cross-cutting efficiency | Graph metrics, workflow traces, and focused latency/allocation diagnostics | 22 | Partly insufficient. Fixed-size registries, service-sized derivation, template parsing, batch composition, and ordinary profile fan-out are proportionate. `AUD-065` records one redundant Weather API request plus serial independent source fetches; `AUD-066` records repeated full comparison-bundle reads during replacement. |
+| Cross-cutting maintainability | Graph metrics plus focused structural review | 23 | Pending |
 | Documentation and executable-contract coherence | Canonical documents, code, schemas, templates, examples | 24 | Pending |
 | Dynamic robustness and diagnostic checks | Repository-wide deterministic checks | 25 | Pending |
 
@@ -2805,6 +2806,97 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 - Related findings: `AUD-058`
 - Remediation reference: pending
 
+### AUD-065: Weather collection duplicates one source and serializes independent requests
+
+- Stage: 22
+- Status: candidate
+- Severity: medium
+- Confidence: high
+- Category: efficiency
+- Area: `internal/adapters/weatherapi.Client.FetchBundle`, `warmup`, and the
+  eight `bundleBuilder.fetch*` calls
+- Evidence: Every collection first performs a warmup GET against the default
+  `/conditions/current` endpoint, discards its body, and immediately fetches
+  `/conditions/current` again as one of eight source products. The remaining
+  seven products are independent HTTP endpoints, but `FetchBundle` waits for
+  all eight in a fixed serial chain. A temporary controlled diagnostic added
+  25 milliseconds of server latency to every healthy endpoint: five runs took
+  236-240 milliseconds apiece, made nine requests, and requested current
+  conditions twice. The diagnostic was removed after measurement.
+- Contract at risk: Collection should preserve source policy, deterministic
+  normalized output, and prompt cancellation without making healthy latency
+  the sum of independent network round trips or issuing a request whose data
+  is always discarded.
+- Impact: For approximately equal endpoint latency `L`, the current healthy
+  transport floor is about `9L`; one full `L` and one upstream response are
+  unconditionally redundant, while serialization adds roughly another `7L`
+  over concurrent independent acquisition. Generate, each batch, and each
+  comparison collect once, so every user-visible workflow pays this cost even
+  though later model latency is separate. Slow optional products also delay
+  all otherwise usable source results and their eventual policy decision.
+- Recommendation: Remove the duplicate current-conditions warmup, or make the
+  successful current fetch satisfy readiness as well as collection. Acquire
+  independent products with a bounded, cancellation-aware group into
+  source-local results, then merge them in canonical source order so warning,
+  provenance, required-source, and error-precedence behavior stays explicit.
+  Do not share-mutate the current builder from request goroutines.
+- Test implications: Add a delayed local-server case proving independent
+  requests overlap and current conditions are requested once. Retain focused
+  cases for warmup/retry policy if a distinct readiness operation remains, and
+  assert deterministic warning/provenance order, required-source failure,
+  optional-source policy, cancellation, and worker joining.
+- Validation: A healthy collection performs only necessary requests and its
+  post-readiness duration approaches the slowest independent source rather
+  than their sum; existing normalized fixture output and failure policy remain
+  byte-for-byte or semantically unchanged as appropriate.
+- Related findings: `AUD-013`, `AUD-015`
+- Remediation reference: pending
+
+### AUD-066: Comparison replacement repeatedly rereads complete bundles
+
+- Stage: 22
+- Status: candidate
+- Severity: low
+- Confidence: high
+- Category: efficiency
+- Area: `internal/app.compareDetailed`,
+  `internal/comparison.PlanDestination`, `RecognizeBundle`, and `publish`
+- Evidence: Replacing an existing bundle invokes `PlanDestination` before
+  prompt work, again after execution, at entry to `publish`, and once more at
+  the commit boundary. Every nonempty replacement plan calls
+  `RecognizeBundle`. One recognition reads the manifest once to decode it,
+  reads every declared directory entry in full (including that manifest and
+  the data package), then reads the data package a second time for its digest.
+  For `N` successful reports, one ordinary replacement therefore performs
+  four recognitions and `4N+16` complete file reads: even a two-profile bundle
+  causes 24 reads, while a ten-profile bundle causes 56. Each report is read
+  four times and the manifest and data package eight times.
+- Contract at risk: Early destination rejection and commit-time
+  reauthorization are necessary, but each authorization pass should perform
+  only the content work needed to establish the recognized-bundle invariant.
+- Impact: Small local bundles make the overhead modest, but the cost scales
+  linearly with profile count and full artifact size, including generated
+  Markdown already unbounded under `AUD-038`. Replacement on slower or remote
+  filesystems multiplies both I/O and pre-publication latency; the adjacent
+  post-execution and publisher-entry checks repeat the same scan with almost
+  no intervening stateful work.
+- Recommendation: Preserve one early preflight and a final commit-time
+  reauthorization, but remove or combine adjacent ownership checks after
+  defining which layer owns each. Within recognition, decode the manifest and
+  hash the data package from their first reads, and validate report entries by
+  metadata plus a minimal open/readability check unless the bundle contract
+  adds report-content digests. Keep the final namespace/type checks needed by
+  the existing replacement threat model.
+- Test implications: Instrument file-open/read operations or use a focused
+  filesystem seam to assert bounded passes for two and many reports. Retain
+  all current symlink, unexpected-entry, digest, concurrent replacement,
+  rollback, and commit-time reauthorization cases.
+- Validation: Replacement performs one deliberate early scan and one
+  deliberate commit authorization without rereading any artifact inside a
+  scan; recognized and adversarial bundles retain their current outcomes.
+- Related findings: `AUD-038`, `AUD-053`, `AUD-055`
+- Remediation reference: pending
+
 ## Retained Decisions
 
 ### RET-001: Keep the application package as the explicit composition owner
@@ -3382,6 +3474,47 @@ template changes reviewable without memorializing incidental whitespace.
 Retain that style while consolidating the cross-owner render checks in
 `AUD-063`.
 
+### RET-050: Keep service-sized derivation and registries straightforward
+
+Graph loop propagation led through daily context construction into fact and
+forecast selection, but the concrete workload is bounded by four default
+dayparts and Weather API runs of a few hundred periods. The work is linear per
+selection, and Stage 7 already counted only low-thousands of simple overlap
+checks for an ordinary derived build. Report and generated-text registries
+contain four definitions, while the module registry is a fixed small catalog;
+rebuilding those values is not a credible hot path at current cardinalities.
+Retain the direct loops and explicit registries. Reconsider indexing or caching
+only if forecast horizons, configured dayparts, or registries become dynamic
+and materially larger.
+
+### RET-051: Keep embedded template parsing until render volume changes
+
+`reporttemplate.Render` reads and parses one top-level template plus four
+embedded partials on each call. A temporary benchmark that deliberately
+reached execution failure only after parsing measured 0.283-0.291 milliseconds
+and about 91 KiB across 1,898 allocations per render on the audit host. The
+fixed three-report batches therefore spend under one millisecond parsing, and
+ordinary comparisons select only a handful of profiles; provider execution is
+orders of magnitude slower. Keep the simple per-call ownership and error
+context for now. A pre-parsed immutable set becomes justified if rendering
+moves into a high-volume service loop or profiles routinely number in the
+hundreds. The benchmark file was removed after measurement.
+
+### RET-052: Keep prepare-once profile execution and fixed batch sequencing
+
+Comparison builds facts, modules, and prompt input once, then starts one
+profile-local execution per explicitly selected profile; the durable order is
+held in a preallocated result slice and Promptkit owns provider capacity.
+Per-profile JSON cloning and rendering provide mutation isolation, while the
+documented and example workload is a human-selected comparison of at least two
+profiles rather than an unbounded service queue. Morning and evening batches
+likewise contain a fixed small report set and intentionally preserve
+independent sequential publication before one notification. Retain both
+designs at current workload. Treat routine selections in the tens or hundreds,
+observable clone contention, or a larger dynamic batch as triggers for a
+bounded worker design rather than inferring a defect from goroutine or graph
+counts alone.
+
 ## Open Questions
 
 No Stage 1 open questions or unexplained baseline failures remain.
@@ -3757,6 +3890,25 @@ Stage 21 routed these investigation leads to their assigned later stages:
   recorded by Stage 20; Stage 21 did not treat it as a durability or test-count
   issue.
 
+Stage 22 routed these investigation leads to their assigned later stages:
+
+- Fixed registries are cheap but are reconstructed through several public
+  helpers. Stage 23 should decide ownership and API shape from duplication and
+  coherence evidence, not introduce caches for a performance problem Stage 22
+  did not find.
+- `AUD-065` needs deterministic error precedence and ordered provenance if
+  acquisition becomes concurrent. Stage 23 may identify a small result-merge
+  abstraction; Stage 26 should preserve Stage 6 source-policy findings instead
+  of treating concurrency alone as the remediation.
+- `AUD-066` must retain early rejection and commit-time namespace
+  reauthorization. Stage 23 should clarify app-versus-publisher ownership, and
+  Stage 25 should exercise concurrent destination changes before Stage 26
+  consolidates reads.
+- Profile fan-out remains proportionate for documented human-selected sets
+  under `RET-052`. Stage 24 should not imply an arbitrary scalability promise;
+  Stage 26 may consider a limit or worker bound only if operational evidence
+  establishes a larger supported workload.
+
 ## Stage Log
 
 ### Stage 1: Establish The Baseline And Audit Ledger
@@ -4931,3 +5083,56 @@ Stage 21 routed these investigation leads to their assigned later stages:
 - Retained decisions: `RET-047` through `RET-049`.
 - Open questions: the four leads recorded above are routed to their assigned
   later stages.
+
+### Stage 22: Audit Cross-Cutting Efficiency And Complexity
+
+- Status: Complete.
+- Scope reviewed: production graph complexity, fan-in/fan-out, loop depth, and
+  repeated work; Weather API collection; forecast/fact/module construction;
+  prompt-input serialization; generated-text validation and rendering;
+  prepare-once execution; single and fixed-batch workflows; comparison profile
+  fan-out; comparison bundle construction, recognition, and transactional
+  publication; fixed registry construction; allocation and I/O boundaries; and
+  realistic service, report, profile, and artifact cardinalities.
+- Exclusions: structural extraction, API simplification, and code deduplication
+  remain Stage 23; documentation promises remain Stage 24; adversarial and
+  dynamic robustness remain Stage 25; no remediation was implemented. No
+  production code or tests were changed.
+
+#### Efficiency And Workload Accounting
+
+| Candidate | Realistic workload and measured/defensible cost | Disposition |
+| --- | --- | --- |
+| Weather source acquisition | Every workflow collects once. Healthy collection performs a discarded `/conditions/current` warmup, fetches current again, and serially fetches seven other independent products. With equal latency `L`, the transport floor is about `9L`. Five controlled 25-millisecond-delay runs took 236-240 milliseconds, made nine requests, and hit current twice. | Credible user-visible latency and redundant upstream work under `AUD-065`. |
+| Forecast and fact derivation | Weather runs contain a few hundred periods; four default dayparts cause low-thousands of simple linear overlap/selection checks. The graph's propagated depth of three or four reflects composed scans, not one polynomial nested loop over the same growing collection. | Proportionate under `RET-050`; no finding. |
+| Registry and catalog construction | Report/generated-text catalogs contain four definitions and the module catalog is fixed and small. Construction occurs per command/report boundary but does not grow with weather data or generated output. | Retain direct construction under `RET-050`; Stage 23 owns API/duplication questions. |
+| Prompt input and generated validation | One package is built and YAML-serialized per prepared report. Generated output is decoded, semantically normalized, and encoded once per execution; no runtime schema compiler or repeated schema parse exists. Payload-size risk is already `AUD-038`. | Linear necessary work; no separate efficiency finding. |
+| Template rendering | Every render reparses one template and four partials. A temporary parse-through-execution benchmark measured 0.283-0.291 ms and about 91 KiB/1,898 allocations per render. Fixed batches contain three reports and ordinary comparisons a handful of profiles. | Measurable but immaterial at supported volume; retain under `RET-051`. |
+| Prepared comparison fan-out | Facts/modules/input are prepared once. Each explicit profile receives isolated JSON-cloned render inputs, one render, and one result slot; one goroutine starts per profile while Promptkit owns provider capacity. Documentation and examples describe human-selected sets beginning at two profiles. | Proportionate at current scale under `RET-052`; watch tens/hundreds rather than inventing a metric-only limit. |
+| Fixed batch sequencing | Morning/evening batches collect once and sequentially prepare, execute, and publish a fixed three-report set before one notification. Provider calls dominate, but concurrency would change backend-capacity and partial-publication behavior for a bounded gain. | Retain under `RET-052`; no finding from sequentiality alone. |
+| Comparison replacement recognition | Existing recognized destinations are fully scanned four times. Each scan performs `N+4` full reads for `N` successful reports, so ordinary replacement performs `4N+16` reads: 24 for two profiles and 56 for ten. Reports are read four times; manifest and data package eight times. | Credible scaling and filesystem-I/O issue under `AUD-066`; preserve early and commit-time authorization. |
+
+#### Commands And Evidence
+
+- Used graph complexity queries, search, symbol snippets, and call traces to
+  inspect the highest propagated loop depths and production fan-in/fan-out,
+  then followed the concrete generate, batch, comparison, collection,
+  derivation, rendering, and publication paths rather than treating graph
+  scores as findings.
+- Reconciled prior-stage efficiency leads: service-sized derivation remained
+  bounded; template reparsing was measured and retained; extreme profile
+  counts remained a watch condition; full comparison reads became `AUD-066`;
+  and production waits were separated from the test-only delays in `AUD-062`.
+- Ran a temporary local Weather API diagnostic with 25 milliseconds of
+  deterministic server latency for every healthy endpoint. Five runs took
+  236-240 milliseconds, issued nine serial requests, and fetched current
+  conditions twice. Removed the diagnostic file immediately afterward.
+- Ran a temporary report-template benchmark five times. Parsing through the
+  execution boundary took 0.283-0.291 milliseconds with about 91 KiB and 1,898
+  allocations per call. Removed the benchmark file immediately afterward.
+- Ran `go test ./...`, `go vet ./...`,
+  `go run ./cmd/weatherreporter --help`, and `git diff --check`; all passed.
+- Findings: `AUD-065` and `AUD-066`.
+- Retained decisions: `RET-050` through `RET-052`.
+- Open questions: the four leads recorded above are routed to their assigned
+  later stages.