diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 62aa34d..296b22d 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -13,8 +13,10 @@ execution keeps its backend-completed targets, and requirement discovery keeps its lightweight metadata path. Public projections detach mutable values. Typed sources use ordinary ConfigKit mapping, composition, and resource context. -One documentation finding remains. No correctness defect or unintended new -restriction or guarantee was identified in the reviewed implementation. The +The initial review identified one documentation finding. The subsequent +algorithmic review adds one low-priority optimization opportunity. No +correctness defect or unintended new restriction or guarantee was identified +in the reviewed implementation. The resource-oriented profile result and changed post-handoff null fallback are deliberately selected compatibility changes, not accidental regressions. @@ -64,6 +66,42 @@ passage against the implemented types and method behavior; check edited local links and whitespace. This is a documentation correction and does not require another implementation path, compatibility field, or new runtime test. +### R2. Hoist The Prompt Presence Lookup Out Of The Input Loop [P3] + +**Status:** Open for consideration. **Affected stage:** 4. + +**Location:** [Prompt resource projection](../../internal/configuration/resources.go), +`InspectPrompt`, lines 77–85, especially `state.promptMetadata[id][i]` at line 81. + +Every input repeats a lookup in the same map using the same prompt ID. The map +and selected metadata slice do not change during the loop; only the slice index +changes. This loop organization was not an algorithm selected by the roadmap. + +**Complexity:** Let `I` be the selected prompt's input count, `M` its message +count, and `L` the prompt ID's byte length. With expected hash-table probing, +map lookup is constant in the number of catalog entries, but hashing a string +key can still cost `O(L)`. The current loop can therefore spend `O(I × L)` on +repeated key processing, for `O(L + I × L + I + M)` total adapter projection +time. Valid resource IDs have no fixed length limit. Go 1.27.3's ordinary +string-map lookup hashes the supplied key on each lookup; its small-map fast +path is an optimization, not a general elimination of this cost. + +**Potential refactoring:** Fetch `presence := state.promptMetadata[id]` once +before iterating over the inputs and use `presence[i].required`. This changes +the projection bound to `O(L + I + M)` with constant additional storage. The +existing `O(I + M)` result allocation and public ownership copies remain +unchanged. No new cache, API, validation, or ID limit is needed. + +This is a modest loop-invariant hoist, not a claim that inspection is quadratic +in catalog size. When ID length is treated as a fixed small constant, both +versions are linear in inputs/messages; the improvement removes an avoidable +multiplicative dependence when IDs and input counts grow independently. + +**Verification:** Existing inherited-input presence and generic encoding tests +should continue to pass, including absent versus explicit false. Source review +can verify that the invariant lookup is outside the loop. No timing threshold, +real-world performance study, or test of an exact map-call count is needed. + ## Stage-By-Stage Review Record | Stage | Evidence inspected and result | @@ -93,3 +131,32 @@ Using Go 1.27.3 with `GOTOOLCHAIN=local`, `GOWORK=off`, and `GOPROXY=off`: The review made no production-code or test changes. These checks do not include live provider diagnostics, release publication, or downstream application changes. + +## Algorithmic Review + +This follow-up reviewed the same production revision, now at repository commit +`101a284` (the intervening commit adds only this review document). It used static +loop, call-path, ownership, and data-size analysis, not performance measurements. +The existing architecture, testing, and documentation policies apply. Explicitly +selected algorithms and dependency-owned composition/encoding were not reopened. + +| Stage | Algorithmic assessment | +| --- | --- | +| 1 — Result declarations | Types and tags introduce no traversal algorithm. Optional values preserve presence without requiring consumer-side reconstruction. | +| 2 — Null interpretation | The fixed set of profile-field checks is constant-sized. Existing backend-provider overlay and target materialization are retained decisions, not new algorithms to redesign in this audit. | +| 3 — Backend/profile resources | Resolved-record indexes are captured once. Individual inspection selects one record rather than scanning the catalog; public provider-map detachment visits the selected value graph. | +| 4 — Prompt resources | Input and message loops visit their respective sequences once, but the input loop has the avoidable repeated string-key lookup described in R2. Scalar presence capture does not scan all prompts for each input. | +| 5 — Facade wiring | Namespace qualification is proportional to ID length. Pointer projection is fixed-field work; nested provider copying is proportional to the copied value graph. No per-field whole-catalog lookup is introduced. | +| 6 — Existing consumers | Removed wrappers do not leave a second resolution pass. Requirement discovery continues using the narrow metadata/snapshot path, rather than copying the expanded resource report. | +| 7 — Generic encoding | Standard JSON/YAML encoding and presence-aware result types are selected planning decisions. No Promptkit serialization round trip or generic reflection layer was added. | +| 8 — Isolation/selection | Inspection itself performs no admission, provider work, schema compilation, or file reads. Caller-owned copies scale with the selected report rather than unrelated resources. | +| 9 — Typed sources | Constructors iterate once over supplied records and their nested input/message sequences, then delegate snapshotting to ConfigKit. These are additive nested sizes, not an all-inputs-by-all-messages cross product. | +| 10 — Real composition | Tests exercise the dependency-owned loading path; no second inheritance resolver or normalization pass was added for typed sources. | +| 11 — Documentation | No production algorithm; the previously recorded documentation correction remains R1. No additional performance guarantee is selected. | +| 12 — Combined validation | The reviewed new production paths contain no additional cross-resource quadratic scan or correctness defect requiring a finding. Existing successful suite/vet results remain applicable because production code is unchanged. | + +The R2 complexity analysis accounts for string-key processing, rather than +assuming every map lookup is free of key-size cost. The selected Go toolchain's +`internal/runtime/maps/runtime_faststr.go` was inspected to confirm that point. +No production implementation, test, benchmark, dependency, or public contract was +changed by this follow-up.