From 50bc35516a8d5e125a6ad3f9ef042d12965842ea Mon Sep 17 00:00:00 2001
From: Eric Rakestraw <eric@rakestraw.us>
Date: Mon, 12 Oct 2026 19:31:54 +0000
Subject: [PATCH] Consolidate request validation parity assertions

---
 docs/roadmap/remediation.md       | 15 +++++++++++--
 validate_request_contract_test.go | 35 ++++---------------------------
 2 files changed, 17 insertions(+), 33 deletions(-)

diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md
index 8ab2153..1a532f2 100644
--- a/docs/roadmap/remediation.md
+++ b/docs/roadmap/remediation.md
@@ -5,9 +5,10 @@ Reviewed commits `f315620`, `f3148bf`, and `60784e7` against the finalized
 
 ## 1. Reuse The Existing Preparation-Parity Test Helper
 
-Priority: P3 (test maintainability). Status: open.
+Priority: P3 (test maintainability). Status: complete.
 
-In [validate_request_contract_test.go](../../validate_request_contract_test.go),
+At reviewed commit `60784e7`, 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
@@ -31,6 +32,16 @@ This is a small cleanup, not a demonstrated functional or release-blocking bug.
 Verify with `go test -race . -run '^TestValidateRequest'` and
 `git diff --check`, formatting the changed Go file.
 
+### Completion
+
+Removed the local closure and reused `assertValidateRequestParity` for all four
+cases, including the nil-engine `ErrInvalidConfig` expectation. Existing cases,
+public-error assertions, and no-model-call checks are preserved; production code
+and public contracts are unchanged.
+
+Formatted the Go file. The uncached, race-enabled `TestValidateRequest` tests
+and `git diff --check` passed.
+
 ## Review Outcome
 
 The public method and shared facade helper match the intended architecture.
diff --git a/validate_request_contract_test.go b/validate_request_contract_test.go
index b547af3..ff943df 100644
--- a/validate_request_contract_test.go
+++ b/validate_request_contract_test.go
@@ -21,43 +21,16 @@ func TestValidateRequestMatchesPrepareContract(t *testing.T) {
 		return requirementsEngine(t, oneVersionPrompt("synthetic", "v1", definition), profiles,
 			promptkit.WithLLMClient(countingLLMClient{calls: &modelCalls}))
 	}
-	testParity := func(t *testing.T, engine *promptkit.Engine, request promptkit.RunRequest, want error) {
-		t.Helper()
-		prepared, prepareErr := engine.Prepare(context.Background(), request)
-		validateErr := engine.ValidateRequest(context.Background(), request)
-		if want == nil {
-			if prepareErr != nil || prepared == nil {
-				t.Fatalf("Prepare() = (%+v, %v), want a prepared result", prepared, prepareErr)
-			}
-			if validateErr != nil {
-				t.Fatalf("ValidateRequest() error = %v, want nil", validateErr)
-			}
-			return
-		}
-		if prepared != nil || !errors.Is(prepareErr, want) {
-			t.Fatalf("Prepare() = (%+v, %v), want error matching %v", prepared, prepareErr, want)
-		}
-		if !errors.Is(validateErr, want) {
-			t.Fatalf("ValidateRequest() error = %v, want error matching %v", validateErr, want)
-		}
-	}
 
 	t.Run("successful request", func(t *testing.T) {
 		engine := newEngine(t, contractPrompt("offline", "hello", nil, "", noValidationOutput))
-		testParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, nil)
+		assertValidateRequestParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, nil)
 	})
 
 	t.Run("nil engine", func(t *testing.T) {
 		var engine *promptkit.Engine
 		request := promptkit.RunRequest{PromptID: "synthetic"}
-		prepared, prepareErr := engine.Prepare(context.Background(), request)
-		validateErr := engine.ValidateRequest(context.Background(), request)
-		if prepared != nil || !errors.Is(prepareErr, promptkit.ErrInvalidConfig) {
-			t.Fatalf("Prepare() = (%+v, %v), want ErrInvalidConfig", prepared, prepareErr)
-		}
-		if !errors.Is(validateErr, promptkit.ErrInvalidConfig) {
-			t.Fatalf("ValidateRequest() error = %v, want ErrInvalidConfig", validateErr)
-		}
+		assertValidateRequestParity(t, engine, request, promptkit.ErrInvalidConfig)
 	})
 
 	t.Run("invalid appended message", func(t *testing.T) {
@@ -68,7 +41,7 @@ func TestValidateRequestMatchesPrepareContract(t *testing.T) {
 				Role: "unsupported-role", Content: "not sent to a model",
 			}},
 		}
-		testParity(t, engine, request, promptkit.ErrInvalidRequest)
+		assertValidateRequestParity(t, engine, request, promptkit.ErrInvalidRequest)
 	})
 
 	t.Run("missing required input", func(t *testing.T) {
@@ -76,7 +49,7 @@ func TestValidateRequestMatchesPrepareContract(t *testing.T) {
 			map[string]any{"name": "required", "required": true},
 		}, "", noValidationOutput)
 		engine := newEngine(t, definition)
-		testParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, promptkit.ErrPromptRender)
+		assertValidateRequestParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, promptkit.ErrPromptRender)
 	})
 
 	if got := modelCalls.Load(); got != 0 {