From 18309f50db2d2f00c96423b1661f22c57d95e814 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 12 Oct 2026 19:05:38 +0000 Subject: [PATCH] Record request validation review finding --- docs/roadmap/remediation.md | 39 +++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 docs/roadmap/remediation.md diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..8ab2153 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,39 @@ +# Request Validation Review Findings + +Reviewed commits `f315620`, `f3148bf`, and `60784e7` against the finalized +[feature roadmap](validate.md) and [implementation plan](implementation.md). + +## 1. Reuse The Existing Preparation-Parity Test Helper + +Priority: P3 (test maintainability). Status: open. + +In [validate_request_contract_test.go](../../validate_request_contract_test.go), +`TestValidateRequestMatchesPrepareContract` defines a local `testParity` closure +at lines 25–45 that duplicates the package-level +`assertValidateRequestParity` helper at lines 87–108. Both invoke `Prepare` and +`ValidateRequest` and assert the same success/result and public-error behavior; +the package-level helper additionally returns the successful prepared value. +The nil-engine subtest repeats the same failure assertions again. + +Retaining separate copies makes future parity-assertion improvements require +multiple edits and increases the risk that related tests diverge unnecessarily. +This is a small cleanup, not a demonstrated functional or release-blocking bug. + +### Requested Remediation + +- Replace calls to the local closure with `assertValidateRequestParity` and + remove the redundant closure. +- Reuse that helper for the nil-engine subtest as well; retain its + `ErrInvalidConfig` expectation. +- Preserve all existing cases, public-error assertions, and no-model-call + checks. No production or public-contract changes are needed. + +Verify with `go test -race . -run '^TestValidateRequest'` and +`git diff --check`, formatting the changed Go file. + +## Review Outcome + +The public method and shared facade helper match the intended architecture. +No implementation gaps or unintended strengthening of contracts were found. +The uncached full race-enabled suite, vet, maintained consumer examples, and +implementation diff whitespace checks passed during review.