9.0 KiB
PromptKit requirements inspection review
Status: all three findings completed. This document records review and remediation evidence; production behavior and selected contracts are unchanged.
Review scope and authority
Reviewed the requirements-inspection implementation from planning baseline
3935dcd
through implemented state
cfc2d1d.
The review compared production changes, tests, examples, and documentation with
the accepted feature roadmap, completed
implementation plan, and applicable
architecture,
documentation, and
testing 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: completed. Priority: normal. Category: documentation contract.
Evidence: CLI reference, 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, 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.
Completed: Qualified the CLI singleton statement to exclude --var, retaining
the existing per-key replacement and selector-binding descriptions. Reviewed the
wording against newRequirementsCommand and the existing request/assignment
coverage. Local links/fragments and git diff --check passed. No parser, handler,
or test changes were needed.
R-2: Complete the canonical requirements YAML schema
Status: completed. Priority: normal. Category: incomplete documentation delivery.
Evidence: CLI reference, 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 emits name, kind,
supplied, and explanation. Those fields were selected by the feature roadmap
and are covered by TestRequirementsReportPreservesNativeFieldsAndMetadata in
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.
Completed: Documented all four variable-entry fields and native exact-key
presence independently of classification/completeness. The partial-report example
now includes the unsupplied mode entry and upstream human-readable summaries.
Reviewed all example fields against the private report types and existing native
and report coverage; a field-inventory check, local links/fragments, and
git diff --check passed. Production serialization and fixtures are unchanged.
R-3: Remove an incidental YAML key-order requirement from a test
Status: completed. Priority: low. Category: inadvertently strengthened test contract.
Evidence: TestRequirementsCommandCapturesContextAndVariableAssignments in
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.
Completed: Replaced the prefix assertion with decoding into a fresh YAML map
and checking the selected prompt identity. Existing success, invocation lifecycle,
and empty-context assertions remain intact. gofmt and git diff --check passed.
From integrations/ckprompt, both
GOTOOLCHAIN=local go test -count=1 -run '^TestRequirementsCommandCapturesContextAndVariableAssignments$' .
and GOTOOLCHAIN=local go test -count=1 ./... passed, including the composed
consumer. The test no longer requires a mapping-key order or document-marker style.
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. Those review-time checks preceded remediation; completion evidence is recorded under each finding above.