Record requirement inspection review findings

docs/roadmap/remediation.md

@@ -1,9 +1,133 @@
-# ReleaseKit Adoption Review Remediation
+# Implementation Review Remediation
+
+## Effective Requirement Inspection Review
+
+Review baseline: `a051edd82d9d20cdc82dfebf25adb689dc58b01f`.
+Scope: implementation commits after `a6e0ffe`, compared with the simplified
+[feature roadmap](inspection.md) and [implementation plan](implementation.md).
+The existing ReleaseKit documentation finding is retained below.
+
+### Finding 1 — Report Lost Caller-Map Scope As Incomplete (P2)
+
+Locations: [requirement walker](../../internal/prompt/requirements.go),
+`visit`'s `WithNode` handling (lines 481–503), `visitCommand` (lines 675–690),
+and `visitVariable` (lines 694–708).
+
+The walker treats every `with` body as nonroot and suppresses an unresolved
+condition issue unless a recognizable dependency was recorded in that body.
+Unsupported local-map accesses are similarly discarded without an issue when
+the dependency counter does not change. Losing the ability to recognize a
+caller key can therefore be mistaken for proof that no key is referenced.
+
+With `Vars: {"audience": "visible"}`, all six of these valid templates render
+`visible`, but inspection reports `Complete: true`, no variables, and no issues:
+
+```gotemplate
+{{with .}}{{.audience}}{{end}}
+{{with $}}{{.audience}}{{end}}
+{{$alias := .}}{{with $alias}}{{.audience}}{{end}}
+{{with .}}{{index . "audience"}}{{end}}
+{{$alias := .}}{{index $alias "audience"}}
+{{$alias := .}}{{$alias.audience}}
+```
+
+This is not merely accepted failure to enumerate a complex reference: the
+result incorrectly claims complete discovery. Consumers can consequently
+accept an incomplete variable inventory as exhaustive.
+
+Remediation: distinguish known nonroot values from unknown scope. Preserve
+straightforward root identity where already determinable, or report an
+unsupported-scope limitation when a reachable access may hide caller keys.
+Unrecognized access must not depend on successfully recording a named
+dependency before it can mark discovery incomplete. Apply the existing unknown
+caller-key effect handling to affected optional entries as needed. Do not add
+alias-value propagation or general assignment interpretation; conservative
+partial results satisfy the approved scope.
+
+Verification: add focused package cases for root-map `with`, local-map field
+access, and local-map `index`. A result may enumerate the known key correctly
+or remain explicitly incomplete, but must not silently omit it from complete
+discovery. Retain known-string dot isolation and inactive-path behavior.
+
+### Finding 2 — Avoid Recursive Rescanning Of Unsupported Names (P2)
+
+Location: [requirement walker](../../internal/prompt/requirements.go),
+`evaluateInputName` (lines 231–244), through `evaluateValue`,
+`evaluateCommand`, and `visitCommand`.
+
+Unsupported name evaluation already scans the expression for recognizable
+references. `evaluateInputName` then calls `visit` on that same subtree again.
+When a computed name contains another `input` with an unsupported computed
+name, both traversals recursively repeat the same fallback. Work grows
+exponentially even though the source grows linearly.
+
+An isolated diagnostic starts with `.selector` and repeatedly wraps it as
+`input (printf "%s" (EXPRESSION))`. Observed visits to that single caller key
+are 2, 4, 8, 64, and 1,024 at depths 1, 2, 3, 6, and 10 respectively. The
+depth-10 source is only 233 bytes. Result deduplication masks the repeated work.
+
+Remediation: collect recognizable references once when simple name evaluation
+falls back to syntax enumeration. Use one conservative traversal rather than
+recursively reevaluating the same unsupported subtree. Preserve classifications,
+known references, and safe issues; more expression support, memoization
+machinery, or new size/depth/work restrictions are unnecessary.
+
+Verification: retain existing computed-name results and exercise a nested
+unsupported expression. Use a small growth benchmark or equivalent focused
+work check to guard against repeated recursive scans, without making exact
+visit counts or a wall-clock threshold a public guarantee.
+
+### Finding 3 — Qualify The Public Variable-Disclosure Statement (P3)
+
+Location: [public request GoDoc](../../requirements.go), `Vars` (lines 22–24).
+
+The comment says that supplied values are not returned in inspection.
+However, `{{input .binding}}` with `Vars: {"binding": "transcript"}` intentionally
+returns `transcript` as an input identifier. The approved roadmap explicitly
+permits that necessary identifier disclosure while excluding unrelated values.
+The current wording can be read as a stronger privacy guarantee than was
+requested or implemented.
+
+Remediation: state that caller values are not returned as values, while a
+resolved input identifier can equal its supplying variable value. Put this
+exception in the canonical GoDoc; preserve existing unrelated-value and
+diagnostic disclosure protections. Do not redact legitimate binding names or
+introduce a new confidentiality mechanism.
+
+Verification: review GoDoc against a direct dynamic-name result and the existing
+privacy contract test. A documentation-only correction does not need another
+full-suite run.
+
+### Review Evidence And Architecture
+
+- The root facade owns exported values, request copying, result copying, and
+  error conversion. The use-case runner reuses captured prompt/profile
+  selection and retrieves the exact snapshot without readiness checks.
+- Configuration captures effective input metadata presence at the final
+  ConfigKit handoff. Parsed templates are shared read-only; request analysis
+  and inventories remain operation-local in `internal/prompt`.
+- Inspection avoids artifact/schema loading, credentials, provider calls,
+  admission, observations, and run IDs. Required declarations, inactive input
+  and variable inventories, exact names, session bypass, and independent
+  returned metadata are implemented and covered by focused tests.
+- The existing full offline race suite passed:
+  `go test -race -vet=off -mod=readonly -count=1 ./...`.
+  `go vet -mod=readonly ./...` and `python3 scripts/check-examples.py` also passed.
+  Commands used
+  `GOTOOLCHAIN=local GOWORK=off GOPROXY=off GONOPROXY=none GOSUMDB=off`.
+- Temporary Go overlay probes, outside the repository, reproduced Finding 1
+  against real template execution and quantified the repeated work in Finding 2.
+  No implementation or maintained test files were changed during review.
+- No new template rejection, caller limit, input-format constraint, readiness
+  requirement, or ConfigKit-boundary expansion was found in these commits.
+  Finding 3 identifies the one overstated documentation guarantee.
+
+## Earlier ReleaseKit Adoption Review
 
 Review baseline: `d5f3464`. The release implementation matches the selected
 architecture; no runtime defect or additional release gate was identified.
 
-## Finding 1 — Complete Documentation Alignment (P3)
+### Finding 1 — Complete Documentation Alignment (P3)
 
 Two descriptions were left behind during the cutover:
 
@@ -26,7 +150,7 @@ profile and runbook, and check changed local links. The disposable fixture
 already demonstrates acceptance of a note without fixed section headings;
 these prose corrections do not require repeating the complete Go suite.
 
-## Review Evidence
+### Review Evidence
 
 - `python3 scripts/test-release.py` passed: successful annotated publication,
   one race-enabled suite and additional check, same-Go tool build, unchanged