diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..be52356 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,138 @@ +# PromptKit requirements inspection review + +Status: three open findings. This document records required follow-up work; +the review does not change implementation or current contracts. + +## Review scope and authority + +Reviewed the requirements-inspection implementation from planning baseline +[`3935dcd`](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/3935dcd) +through implemented state +[`cfc2d1d`](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/cfc2d1dfd0c17c35ffe0d0a1a46f4dc2df8be5bb). +The review compared production changes, tests, examples, and documentation with +the accepted [feature roadmap](requirements.md), completed +[implementation plan](implementation.md), and applicable +[architecture](../policy/architecture.md), +[documentation](../policy/documentation.md), and +[testing](../policy/testing.md) policies. + +The targeted runtime architecture is present: the optional ckprompt suite owns +requirements registration, selection, coordination, and YAML presentation; +its engine adapter delegates discovery to PromptKit's public API. Core and +ckconfig retain their existing responsibilities and dependency boundaries. +No production defect or additional domain restriction was identified in this +review. The findings concern canonical documentation and one test that constrains +presentation beyond the selected contract. + +## R-1: Correct singleton wording for requirements flags + +Status: open. Priority: normal. Category: documentation contract. + +**Evidence:** [CLI reference](../cli.md), lines 418–420, says that the built-in +flags are singleton bindings and that the last value is used when repeated. +The same section lists `--var` as repeatable. In +[command.go](../../integrations/ckprompt/command.go), `newRequirementsCommand` +uses `RepeatFlag` for variables and builds the map from every occurrence, with +last-value replacement only for repeated keys. + +The unqualified singleton statement contradicts the implemented and accepted +behavior. For example, `--var audience=internal --var mode=full` retains both +keys; the second occurrence does not replace the entire variable context. + +**Required work:** qualify the singleton description to exclude `--var`, or +identify the singleton flags explicitly. Retain the existing explanation that +variable occurrences accumulate and only duplicate keys use the last value. +Suite selectors continue to follow their declared bindings. No parser or handler +change is required. + +**Completion evidence:** the CLI reference gives consistent repetition rules in +its flag table and prose. Check the wording against the existing assignment and +singleton coverage; new generic parser tests are unnecessary. + +## R-2: Complete the canonical requirements YAML schema + +Status: open. Priority: normal. Category: incomplete documentation delivery. + +**Evidence:** [CLI reference](../cli.md), lines 428–451, describes the report +shape but illustrates `variables: []` without defining variable-entry fields +anywhere in the requirements section. The implemented +`requirementVariableReport` in +[command.go](../../integrations/ckprompt/command.go) emits `name`, `kind`, +`supplied`, and `explanation`. Those fields were selected by the feature roadmap +and are covered by `TestRequirementsReportPreservesNativeFieldsAndMetadata` in +[command_test.go](../../integrations/ckprompt/command_test.go). + +Operators and automated consumers cannot obtain the complete CommandKit YAML +schema from its canonical owner. PromptKit's Go API defines native meanings, +but it does not define CommandKit's YAML field names. The missing fields should +remain documented after temporary planning records are retired. + +**Required work:** add a representative variable entry to the CLI example or +provide a compact field inventory for the full report. Define the existing +`supplied` field as native exact-key presence, including an explicitly empty +value, independently of `kind` and `complete`. Keep explanations as upstream +human-readable evidence rather than text to parse. + +Use a coherent partial-discovery example if retaining the illustrated +`missing_variable` issue for `mode`: include its variable entry as native +inspection does. Preserve all already selected fields, false values, empty +sequences, and nullable metadata semantics. Do not change the report or add +new disclosure, completeness, or readiness guarantees. + +**Completion evidence:** all selected report fields are documented in the CLI +owner, and the example matches the existing native/consumer evidence. Review +local links and document whitespace. Production serialization and test fixtures +need no changes to repair this omission. + +## R-3: Remove an incidental YAML key-order requirement from a test + +Status: open. Priority: low. Category: inadvertently strengthened test contract. + +**Evidence:** `TestRequirementsCommandCapturesContextAndVariableAssignments` in +[command_test.go](../../integrations/ckprompt/command_test.go), lines 190–192, +uses `strings.HasPrefix(output.String(), "prompt_id:")` to establish that an +omitted format defaults to YAML. + +That check also requires `prompt_id` to be the first serialized mapping key and +excludes an otherwise valid YAML document-start marker. Neither presentation +detail was selected by the user. A semantic-equivalent report would fail this +test, contrary to the implementation plan's instruction to avoid incidental +key-order assertions and the testing policy's behavior-focused approach. + +**Required work:** replace the prefix assertion with YAML decoding of the second +invocation's output and an assertion on the relevant semantic fields, such as +the selected prompt identity. Retain the existing success, fresh input/state, +and empty discovery-context assertions. Do not add a new key-order guarantee to +the CLI documentation to justify the current test. + +**Completion evidence:** the test proves default YAML output without constraining +mapping-key order or document-marker style. Run the affected requirements test +and owning module suite after changing the assertion. + +## Review validation + +At reviewed commit `cfc2d1d`, on Go 1.26.8: + +- All three workspace module suites passed with `GOTOOLCHAIN=local go test ./...` + from their respective module directories. +- Fresh root tests passed with `GOWORK=off GOTOOLCHAIN=local`, `-mod=readonly`, + and `-count=1`. Root module and production/test import enumeration confirmed + no optional integration or owning-library dependency. +- Fresh ckconfig and ckprompt suites, module graphs, and production/test import + enumeration passed with workspace disabled and readonly disposable copies of + committed module/sum files. Only unpublished CommandKit prerequisites were + replaced with local sources. Temporary metadata hashes remained unchanged + during validation, and distributed manifests were not edited. +- The isolated ckprompt graph selects PromptKit v0.14.0 and ConfigKit v0.7.1; + ckconfig has no ckprompt or PromptKit dependency. Local prerequisite + replacements establish source compatibility, not future tag availability. +- Composed requirements invocations produced the expected partial and complete + inventories with status `0`. The complete report still contained an unsupplied + required variable, consistent with native discovery semantics. +- Changed Markdown local links/fragments and commit-range whitespace checks + passed. No tests or production code were added or changed for this review. + +PromptKit v0.14.0's known lack of a public fixture path for successful engine +configuration warnings remains a validation limitation, not a new implementation +gap. Existing custom-callback coverage exercises command diagnostic handling. +These passing checks do not resolve the three findings above.