Plan request validation in focused stages
This commit is contained in:
189
docs/roadmap/implementation.md
Normal file
189
docs/roadmap/implementation.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user