From 97357d62b923d22b3f6ac278b8be5bea5a89c43f Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 12 Oct 2026 17:35:25 +0000 Subject: [PATCH] Plan request validation in focused stages --- docs/roadmap/implementation.md | 189 +++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 docs/roadmap/implementation.md diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..55bd543 --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,189 @@ +# Request Validation Implementation Plan + +Implement [the finalized feature roadmap](validate.md) in the numbered order +below. Each stage is scoped for one Luna coding-agent prompt. The feature +roadmap owns the target behavior and downstream intent; this document owns the +implementation sequence. + +## Baseline And Implementation Decisions + +The baseline is commit `0c3651f`. `Engine.Prepare` in `engine.go` clears execution +observation state, checks the engine, converts `RunRequest`, calls +`internal/engine.Prepare`, maps preparation errors, and converts the returned +internal value into a caller-owned public snapshot. The internal engine already +delegates to `internal/usecase.Runner.Prepare`, which runs both preparation +phases without admission or generation. + +Add exactly this public operation: + +```go +func (e *Engine) ValidateRequest(ctx context.Context, req RunRequest) error +``` + +Extract the existing facade preparation plumbing into an unexported +`(*Engine).prepareRequest` helper returning `(*domain.PreparedRun, error)`. +`Prepare` will call that helper and retain its existing public result conversion; +`ValidateRequest` will call it and discard the internal result. This shares +request conversion, context handling, nil-engine checks, and error mapping while +avoiding public snapshot copies for validation. The private helper is an +implementation choice, not a consumer contract. + +No new internal-engine or runner operation is needed. Internal prepared values, +hashing, schema preparation, and rendering may still occur; eliminating them +would change the selected preparation-parity behavior or require work outside +this small addition. + +## Working Instructions + +- Start with [the development guide](../development.md) and read the applicable + [agent](../policy/agents.md), [architecture](../policy/architecture.md), + [testing](../policy/testing.md), and [documentation](../policy/documentation.md) + policies. Use the existing facade GoDoc and + [runner description](../internal/runner.md#shared-preparation-pipeline) as + current-behavior references. +- Preserve `Prepare`'s behavior and error wrapping, including its existing + context classification. Do not add a general `errors.Is(err, ctx.Err())` + guarantee, new error sentinels, aggregated diagnostics, stricter binding + rules, provider checks, or capacity checks. +- Use the full `RunRequest` conversion path, not a separate subset of its fields + or a dependency inventory from `InspectRequirements`. Leave credential, + resource, template, and request-format rules unchanged. +- Keep tests offline, deterministic, and external-package at the public boundary. + Use real preparation collaborators and small injected doubles for external + effects. Test representative forwarding risks rather than reproducing the + complete preparation suite. +- Commit each completed stage after its checks and relevant diff review, with a + plain-English message. Do not push, tag, or run a publication command as part + of this plan. + +## Stage 1: Share Facade Preparation And Add ValidateRequest + +Scope: the small facade change, GoDoc, and basic contract tests. + +1. Review `Engine.Prepare`, `toDomainRunRequest`, `mapPublicError`, and + `fromDomainPreparedRun`. Extract only the existing preparation plumbing into + the private helper described above. Keep the existing operation order and + wrapping at each error boundary; do not change error semantics as incidental + cleanup. +2. Make `Prepare` call the helper and perform the existing + `fromDomainPreparedRun` conversion on success. Its signature, snapshot + ownership, and failure behavior remain unchanged. +3. Add `ValidateRequest` in `engine.go`. Return the helper's error, discarding its + internal prepared value. Do not call the public `Prepare` method, construct a + public snapshot, or introduce an internal validation pipeline. +4. Add GoDoc explaining that nil means the concrete request passed preparation + now, no LLM call occurs, resource I/O still occurs, and no executable snapshot + is retained. State parity with preparation's errors and context handling, + including the nil-engine `ErrInvalidConfig` convention. Link conceptually to + `Prepare` and `PrepareExecution`; do not promise subsequent provider success. +5. Add `validate_request_contract_test.go` in package `promptkit_test`. Reuse + compatible helpers from `requirements_inspection_contract_test.go` and + `consumer_test_helpers_test.go` where helpful. Construct a synthetic prompt + and explicit offline profile rather than depending on catalog model choices. + Cover a successful request, a nil engine, a conversion-level invalid request + such as an invalid appended-message role, and one missing required input. +6. In those tests, invoke both `Prepare` and `ValidateRequest` for the same + test-controlled request and compare success or the relevant public sentinel + through `errors.Is`. Use one intentional failure per case rather than + expecting a particular error from several independently failing inputs. + Inject a counting client and verify neither operation generates a model call. +7. Format changed Go files and run + `go test . ./internal/usecase ./internal/prompt`, followed by + `git diff --check`. Existing `Prepare` tests must still pass. + +Completion: external consumers can call the new error-only method, all facade +preparation plumbing is shared, and the public snapshot conversion remains only +on `Prepare`'s success path. + +## Stage 2: Verify Concrete-Request Parity And Resource Boundaries + +Scope: focused tests of the finalized roadmap's clarifications. + +1. Extend the root contract tests with a prompt using + `{{if eq .mode "detailed"}}{{input "details"}}{{end}}`. For an input not + declared required, verify `mode=brief` succeeds without it and + `mode=detailed` fails without it but succeeds with an inline binding. Use a + second definition with the input declared required to verify absence fails + even on the brief branch. Compare each case with `Prepare`. +2. Cover an evaluated direct variable lookup with an absent key and with an + explicitly supplied empty string. Include a missing `index` lookup to show + its existing optional semantics are retained. These assertions concern + preparation only, not nonempty model-output rules. An extra unrelated caller + variable must remain acceptable where `Prepare` accepts it. +3. Add a case with an unreadable supplied input that is not referenced on the + selected branch. A synthetic missing file under `t.TempDir()` is sufficient. + Both operations must report the applicable artifact-load error, proving + validation uses actual preparation instead of a names-only or lazy subset. +4. Cover representative selected-profile and output-schema preparation failures + with existing valid fixtures: a missing profile should retain + `ErrProfileNotFound`, and an unavailable selected schema should retain the + category already returned by `Prepare`. Avoid adding source-level or + intermediate-configuration validation. +5. Add one successful request with a profile default and a runtime override that + completes a partial profile, plus a valid direct session ID and appended + message where useful. Inspect `Prepare`'s result only as needed to establish + that the fixture exercised those settings; `ValidateRequest` must accept the + same request. Existing tests own the exhaustive override and message matrix. +6. Cover a controlled canceled-context or canceled-loader failure through the + public facade, asserting the existing preparation category for both calls. + Do not assert a stronger universal relationship to `context.Canceled` or + introduce timing-based tests. Keep the model-call count at zero across the + validation cases. +7. Run `go test -race . ./internal/usecase ./internal/prompt` and + `git diff --check`, formatting changed Go files. Review tests for redundant + preparation coverage and assertions on incidental error text or private call + order. + +Completion: success/failure parity includes concrete branch choices, declared +required inputs, variable presence, all supplied artifact bindings, representative +resource/selection errors, and ordinary request forwarding. No provider or +capacity-readiness interpretation has been introduced. + +## Stage 3: Document Consumer Use And Verify The Integrated Feature + +Scope: canonical documentation, the maintained preparation example, and final +validation. + +1. Add a small `ValidateRequest` example to + `docs/consumers/pkg-promptkit.md`, next to requirements inspection and offline + preparation. Show a full `RunRequest` and nil/error handling, and link to + GoDoc for exact behavior. Distinguish concrete-request validation from + dependency discovery and from obtaining a prepared execution handle. +2. Explain that Scriptorium can map the error to its own diagnostics and exit + status, and should build equivalent requests for its `validate` and `run` + commands. Keep this as consumer guidance, not CLI code or a new PromptKit + transport contract. Explicitly note that separate subprocess calls prepare + independently and cannot transfer the opaque handle. +3. Update `examples/go-library/prepare/main.go` to demonstrate the method using + the same request value as its existing `Prepare` demonstration. Keep its + semantic output unchanged. Explain in the consumer guide that showing both + operations is illustrative, not a requirement to validate before `Prepare` + or `Run`; it performs preparation twice. +4. Update `docs/internal/runner.md` to describe the error-only facade's reuse of + the existing preparation path, and update `docs/internal/overview.md` to + include the implemented public operation. Keep business logic under the + existing internal ownership and avoid describing a new validator component. +5. Ensure GoDoc and guidance state the same boundaries: resource I/O and cost + remain, all supplied inputs follow existing loading behavior, only the + concrete invocation is checked, errors are not aggregated, and no future + provider or execution success is certified. Do not change format references + to create new requirements. +6. `v0.14.0` is already tagged at planning time; leave that historical note + unchanged. Record this optional API addition in the next release notes when + that release is prepared. Release publication and new version selection are + separate from this implementation plan. +7. Run `go test -race ./...`, `go vet ./...`, + `python3 scripts/check-examples.py`, and `git diff --check`. Check affected + documentation links and fragments. Review exported declarations to confirm + that only `Engine.ValidateRequest` was added, and review the complete diff + against [the feature roadmap](validate.md). + +Completion: API, focused tests, canonical documentation, and the maintained +example are coherent; existing preparation and execution workflows remain +available; and the integrated checks pass. Report actual check results and stage +commits. The completed roadmap can be retired during later release preparation. + +## Open Questions + +None. The finalized roadmap resolves the public name, request shape, result, +preparation scope, and downstream boundary required for these stages.