diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..892c6b3 --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,339 @@ +# Migration Step 3 Implementation Plan + +## Status + +Ready for implementation. No implementation work described here has started. + +## Objective + +Implement the target state in the +[Step 3 framework-characterization roadmap](step3.md): make +Promptkit-destined tests independent of Scriptorium-owned executable examples, +close the identified public contract gaps, and leave every migration-critical +behavior with a clear test owner. + +Follow the accepted ownership boundary in +[ADR 0002](../adr/0002-split-promptkit-from-scriptorium.md) and the test-value +and non-duplication rules in the [testing policy](../policy/testing.md). + +## Constraints + +- Execute the stages in order and satisfy each gate before proceeding. +- Do not change production behavior, public types, method signatures, package + boundaries, or application interfaces. +- Limit implementation changes to testdata, tests, and roadmap status. +- Keep all default-suite tests deterministic, offline, and independent of real + credentials. +- Preserve unrelated working-tree changes. +- Do not copy the complete `examples/` tree into testdata. +- Do not add tests merely to raise statement coverage. +- Prefer extending or consolidating an existing test over adding a parallel + test for the same behavior. + +## Stage 1: Create The Framework Contract Corpus + +Create this Promptkit-destined fixture tree: + +```text +testdata/framework/ +├── fixtures/ +│ ├── glossary.yml +│ └── transcript.md +├── profiles/ +│ ├── contract-fast.yaml +│ └── contract-quality.yaml +├── prompts/ +│ ├── contract.markdown_summary.system.md +│ ├── contract.markdown_summary.user.md +│ ├── contract.markdown_summary.yaml +│ ├── contract.structured_events.system.md +│ ├── contract.structured_events.user.md +│ └── contract.structured_events.yaml +└── schemas/ + └── structured_events.schema.json +``` + +Define the corpus as follows: + +- `contract.markdown_summary` + - version `1.0.0`; + - default profile `contract-fast`; + - required `transcript` input and optional `glossary` input; + - system and user messages loaded through relative `content_file` paths; and + - Markdown output with basic validation and no repair attempts. +- `contract.structured_events` + - version `1.0.0`; + - default profile `contract-quality`; + - the same two inputs; + - system and user messages loaded through relative `content_file` paths; and + - JSON output with JSON Schema validation through + `structured_events.schema.json` and no repair attempts. +- `contract-fast` + - endpoint `http://localhost:8000/v1`; + - model `contract-fast-model`; + - temperature `0.2`, max tokens `500`, top-p `1`, and timeout `90`; and + - no credential requirement. +- `contract-quality` + - endpoint `http://localhost:8000/v1`; + - model `contract-quality-model`; + - temperature `0.1`, max tokens `1000`, top-p `0.9`, and timeout `120`; and + - no credential requirement. +- The schema requires an object containing an `events` array. Keep it small but + valid for the same JSON Schema draft currently used by the maintained + structured-output example. +- The transcript and glossary contain short synthetic values suitable for + rendering and hash assertions. They must contain no private or real-world + data. + +Do not add application configuration, HTTP requests, executable scripts, or +provider credentials to this corpus. + +In `engine_test.go`, add shared constants for the contract root, prompt IDs, +profile IDs, and fixture paths. Replace +`TestPrepareWorksWithExampleDirectoriesAndFileInputs` with +`TestPrepareWorksWithFrameworkContractCorpus`, using table cases for the +ordinary and structured prompts. Construct the public engine from the new +directories, use the new file artifacts, and assert that each prompt renders +with its intended profile. For the structured case, also assert that +`PreparedRun.StructuredOutput` contains the loaded JSON Schema specification. +This test is the real-parser acceptance check for the corpus. + +Run: + +```bash +go test . +go test ./internal/promptdef ./internal/profile ./internal/validate +``` + +### Stage 1 Gate + +- Every corpus file loads through its real owning parser. +- Relative prompt content resolves from the prompt file location. +- The structured schema decodes through the real validator's schema-document + loader and appears in the prepared structured-output specification. +- No production or executable-example file changed. + +## Stage 2: Move Framework Tests Onto Framework-Owned Fixtures + +Update `engine_test.go` to use the new corpus. + +1. Reuse the contract constants introduced in Stage 1. +2. Rename: + - `newExampleEngine` to `newContractEngine`; + - `newExampleEngineWithOptions` to `newContractEngineWithOptions`; and + - `exampleConfig` to `contractConfig`. +3. Make `contractConfig` point at the corpus prompt, profile, and schema + directories. +4. Replace each `./examples/...` dependency in `engine_test.go` with the + matching contract fixture or a purpose-built `t.TempDir`, + `fstest.MapFS`, or in-memory profile. +5. Update assertions that intentionally identify fixture prompt IDs, profile + IDs, models, rendered text, or hashes to the contract values. Do not change + assertions that express independent public behavior. +6. Rename tests whose names say “example” when they now exercise contract + testdata. + +Add `TestEngineRunWithDirectorySourcesAndFileInputs` to `engine_test.go`. It +must assemble the public engine from the contract prompt, profile, schema, and +file-artifact directories; inject a deterministic `LLMClient`; run +`contract.structured_events`; and assert: + +- the prompt-selected `contract-quality` profile; +- a non-empty run ID, prompt hash, rendered-prompt hash, and both input hashes; +- provider-level JSON Schema structured output on the captured generation + request; +- passed JSON Schema validation; +- `application/json` artifact content; +- preserved raw output and injected token usage; and +- non-zero ordered timestamps with non-negative duration. + +Move the unique protection from +`internal/usecase/integration_test.go` into this public test, then delete that +internal integration test. Do not retain both assembled workflows. + +The only test references to `examples/` after this stage should be +Scriptorium-owned adapter or maintained-example checks. In particular, this +command must return no matches: + +```bash +rg -n 'examples/' engine_test.go internal/usecase +``` + +Run: + +```bash +go test . +go test ./internal/usecase +``` + +### Stage 2 Gate + +- Public and framework-internal tests pass without reading Scriptorium-owned + executable examples. +- The new public assembled workflow subsumes the deleted internal integration + test. +- Scriptorium's maintained examples are unchanged. +- No production file changed. + +## Stage 3: Consolidate And Complete Public Characterization + +### Execution-Setting Precedence + +Add a table-driven `TestEngineExecutionSettingPrecedence` in `engine_test.go`. +Run through the public engine with an injected recording `LLMClient`. Cover +these cases: + +1. a profile with zero-valued optional settings receives the documented + framework numeric defaults; +2. non-zero profile settings replace those defaults; +3. request settings replace profile settings; and +4. explicit request numeric zero replaces non-zero profile settings. + +Across the table, verify the effective endpoint, model, temperature, +max-tokens, top-p, timeout, service tier, reasoning effort, API-key environment +name, and `extra_params` where the relevant layer supplies them. Verify +`ExecutionTargetPresence` is false for omitted numeric request settings and +true for every explicitly supplied numeric setting, including zero. + +Use relationally distinct values for each layer. Assert literal framework +defaults only in the framework-default case because those values are part of +the documented public contract. Use `t.Setenv` for every non-empty profile or +request API-key environment name, assert only the environment-variable names, +and never expose the test secret values. + +Consolidate overlapping assertions: + +- remove `TestPreparePreservesExplicitZeroExecutionOverrides` once the new + table protects that behavior; and +- retain `TestRunPassesPreparedRequestToInjectedLLMClient` for rendered prompt, + direct-key, and structured-output handoff, but remove execution-precedence + assertions now owned by the table. + +### Caller Cancellation + +Add `TestEngineRunPropagatesCallerCancellation` using the built-in +OpenAI-compatible client and a custom `RoundTripper`. + +- The transport must signal through a channel when `RoundTrip` begins. +- It must block on `req.Context().Done()` and return the context error. +- Start `Engine.Run` in a goroutine, wait for the transport signal, cancel the + caller context, and collect the result through a buffered channel. +- Assert that the call returns and the error matches `ErrLLMGenerate`. +- Do not use sleeps or elapsed-time assertions. + +### Injected Nil Response + +Add a valid-request case to `TestPublicErrorsSupportErrorsIs` whose injected +`LLMClient` returns `(nil, nil)`. Assert `ErrLLMGenerate`. Extend the existing +fake only as needed to express this case; do not create a mock framework. + +### Reserved Provider Parameters + +Add `TestRunRejectsReservedExtraParamsBeforeProviderCall`. + +- Use the built-in client with a custom immediate `RoundTripper` that records + whether it was invoked. +- Supply a valid contract prompt and profile plus request + `ExtraParams: map[string]any{"model": "collision"}`. +- Assert `ErrInvalidRequest`. +- Assert that the transport was not invoked. + +Run: + +```bash +go test . +go test -count=20 . +``` + +### Stage 3 Gate + +- The four-layer precedence table and presence assertions pass. +- Cancellation is deterministic and contains no wall-clock sleeps. +- Nil injected responses and reserved parameters preserve their public error + categories. +- Superseded assertions or tests have been removed rather than duplicated. +- No production file changed. + +## Stage 4: Audit Ownership And Validate The Baseline + +### Ownership Audit + +Review the behavior list in [step3.md](step3.md) against the final suite. +Confirm: + +- root facade and public contract tests are Promptkit-destined; +- `internal/domain`, `internal/usecase`, `internal/promptdef`, + `internal/prompt`, `internal/profile`, `internal/profile/builtin`, + `internal/filecatalog`, general `internal/artifact`, `internal/validate`, and + `internal/llm` tests move with Promptkit-owned behavior; +- CLI, application configuration, prepared formatting, HTTP DTO, strict JSON, + HTTP limit, and rooted artifact-containment tests remain Scriptorium-owned; +- HTTP and CLI tests that currently construct internal runners or inspect + internal sentinels retain their observable assertions and are explicitly + deferred for boundary rewrites in Migration Step 4; and +- no consequential behavior in the feature roadmap lacks a test owner. + +Do not create a permanent test-inventory document. Record any unexpected +ownership exception in `step3.md`; otherwise the ownership table there is the +complete disposition. + +### Full Validation + +Run: + +```bash +go test ./... +go vet ./... +build_dir="$(mktemp -d)" +go build -o "$build_dir/scriptorium" ./cmd/scriptorium +go test -count=20 . +go test ./internal/adapter/http -run TestMaintainedHTTPRunExampleMatchesRequestContract +bash ./examples/render-markdown-summary.sh +go run ./cmd/scriptorium render \ + --config ./examples/config.full.yml \ + --prompt generic.markdown_summary \ + --input transcript=./examples/fixtures/transcript.md \ + --input glossary=./examples/fixtures/glossary.yml \ + --format text +go run ./examples/go-library/prepare +git diff --check +``` + +Validate every local Markdown link and path in the changed roadmap files. +Confirm both configuration examples were accepted through the real +configuration loader by the two render commands. + +Inspect the final diff and confirm the implementation changed only: + +- `testdata/framework/**`; +- `engine_test.go`; +- `internal/usecase/integration_test.go` by deletion; +- `docs/roadmap/step3.md`; +- `docs/roadmap/implementation.md`; and +- the Step 3 status in `docs/roadmap/migration.md`. + +If a necessary change falls outside that list, stop and revise the plan or +request direction rather than expanding scope implicitly. + +### Completion Bookkeeping + +After every check passes: + +1. update `step3.md` to state that the target state is complete and summarize + the characterized baseline without reintroducing an implementation log; +2. add a Step 3 gate-status entry to `migration.md` with the completion date + and a short validation summary; and +3. mark this implementation plan complete. + +Do not begin Migration Step 4 in the same change. + +### Stage 4 Gate + +- Every completion criterion in `step3.md` is satisfied. +- The full suite and maintained examples pass offline. +- The diff contains no production behavior or API change. +- The main migration roadmap identifies Step 3 as complete and Step 4 as next. + +## Open Questions + +None. diff --git a/docs/roadmap/migration.md b/docs/roadmap/migration.md index 05df3e4..0028b7b 100644 --- a/docs/roadmap/migration.md +++ b/docs/roadmap/migration.md @@ -125,6 +125,9 @@ is accepted and records the required ownership and coordination decisions. Strengthen or add contract-focused tests where needed so extraction can be verified without relying on package placement. +The accepted implementation scope and intended completion state are in the +[Step 3 framework-characterization roadmap](step3.md). + Preserve coverage of: - `Prepare` and `Run` behavior; diff --git a/docs/roadmap/step3.md b/docs/roadmap/step3.md new file mode 100644 index 0000000..aca0148 --- /dev/null +++ b/docs/roadmap/step3.md @@ -0,0 +1,177 @@ +# Migration Step 3: Framework Characterization + +## Status + +Accepted scope. Implementation has not started. The ordered execution plan is +in [implementation.md](implementation.md). + +## Purpose + +Characterize the framework and adapter contracts that must survive the +Promptkit extraction. The work should make those contracts portable across the +future repository boundary without duplicating behavior at every test layer or +changing production APIs ahead of Migration Step 4. + +The [testing policy](../policy/testing.md) governs test value, ownership, and +sufficiency. The [accepted split decision](../adr/0002-split-promptkit-from-scriptorium.md) +governs which project will own each behavior. + +## Baseline Findings + +The current suite already provides broad coverage at the public facade, +framework package, CLI, HTTP, source, validation, and model-client boundaries. +Step 3 is therefore a portability and risk-closing exercise rather than a +general coverage expansion. + +The principal extraction risk is fixture ownership: + +- public `Engine` tests rely extensively on executable assets under + `examples/`; +- the assembled runner integration test also reads those assets; and +- the accepted split leaves executable examples in Scriptorium while moving + the public facade and framework tests to Promptkit. + +Those dependencies would either prevent the tests from moving or create +unwanted cross-repository fixture coupling. A smaller set of public-boundary +gaps also remains around complete setting precedence, cancellation, malformed +injected-client behavior, and reserved provider parameters. + +## Target State + +At completion: + +- tests destined for Promptkit use only Promptkit-destined testdata or + fixtures generated within the test; +- Scriptorium's maintained executable examples remain independently validated + by Scriptorium-owned checks; +- each behavior named in the main migration roadmap has one clear test owner; +- representative public `Engine` tests protect assembled framework behavior; +- focused package tests continue to own strict parsing, source mechanics, + validation rules, and provider mapping without higher-level duplication; +- Scriptorium adapter tests continue to protect CLI, HTTP, containment, limits, + and transport mappings; and +- no production API or package boundary has changed as part of Step 3. + +## Policy Choices + +### Framework-Owned Testdata + +Promptkit-destined tests will use a compact framework contract corpus under root +`testdata/` or a purpose-built fixture created inside the test. The shared +corpus will contain only the assets needed to express durable framework +behavior: + +- one ordinary prompt definition; +- one JSON Schema structured-output prompt definition; +- representative execution profiles; +- one JSON Schema document; and +- small file-artifact inputs. + +The corpus should exercise directory-backed loading and relative prompt content +where those behaviors matter. Use small inline `fstest.MapFS` or temporary +fixtures for cases that do not benefit from shared files. + +The complete executable example tree will not be copied. Testdata will remain +minimal, synthetic, secret-free, and distinct from user-facing examples. + +### Test Boundaries And Consolidation + +Representative assembled behavior belongs at the public `Engine` boundary. +Focused parsing, source, validation, provider, and adapter mechanics remain +with their package-level owners. Existing tests should be consolidated when a +new public contract test would otherwise duplicate the same risk. + +The assembled runner integration behavior will be protected through the public +facade rather than through a second test tied to internal domain and runner +types. Promptkit-destined tests will not read Scriptorium-owned `examples/` +assets. Scriptorium adapter and maintained-example checks may continue to do so +where the example itself is the contract under test. + +### Required Public Characterization + +The public suite will characterize: + +- an assembled directory-backed `Engine.Run` workflow with file inputs, + structured output, schema validation, hashes, usage, and timing; +- framework-default, profile, and request execution-setting precedence; +- explicit numeric-zero propagation and target-presence metadata; +- caller-context cancellation at the outbound generation boundary and its + public error classification; +- nil responses from injected model clients; and +- reserved provider parameters failing before a provider call. + +These tests will use deterministic synchronization, real local collaborators +where inexpensive, and fakes only at the model-provider boundary. + +## Target Test Ownership + +| Test category | Future disposition | +| --- | --- | +| Public `Engine`, facade, model-client extension, and public error contracts | Move to Promptkit. | +| Framework domain, runner, prompt, profile, built-in registry, general artifact, validation, and LLM package tests | Move with their Promptkit-owned implementation. | +| CLI parsing, application configuration, prepared-run formatting, and process behavior | Remain in Scriptorium. | +| HTTP DTOs, strict JSON, limits, response mapping, and rooted artifact containment | Remain in Scriptorium. | +| Tests that construct internal runners or classify internal framework sentinels from Scriptorium adapters | Preserve their observable assertions, then rewrite against the public Promptkit boundary during Step 4. | +| Maintained Go consumer example | Move to Promptkit. | +| Maintained executable configuration, render, HTTP, and fixture examples | Remain in Scriptorium. | + +Existing focused tests remain the owners of: + +- strict prompt, profile, and application YAML decoding; +- strict HTTP JSON decoding; +- prompt, profile, schema, and artifact source mechanics; +- built-in profile validation and overlay fallback; +- structured-output encoding and schema validation; +- validation content failures versus operational failures; +- credential handling and redaction; +- OpenAI-compatible wire behavior; and +- HTTP artifact restrictions and transport mappings. + +Coverage will be added only if the ownership audit identifies a consequential +behavior with no credible existing owner. + +## Required Validation Outcome + +The characterized baseline must pass the complete Go test and vet suites, a +temporary-output executable build, repeated public contract tests, both +maintained application configurations, maintained render and Go consumer +examples, the maintained HTTP request-example check, documentation-link +validation, and whitespace validation. All checks must remain offline and +independent of real credentials. + +## Out Of Scope + +Step 3 does not: + +- add artifact-reader, repository, validator, or other production extension + APIs; +- refactor CLI or HTTP adapters to consume the public facade; +- move restricted HTTP artifact behavior out of its current package; +- create the Promptkit repository or change the Go module path; +- move implementation packages between repositories; +- create compatibility aliases or forwarding APIs; +- redesign the public facade; or +- add tests solely to increase a coverage percentage. + +Those changes belong to later migration steps. + +## Completion Criteria + +Step 3 is complete when: + +- Promptkit-destined tests have no dependency on Scriptorium-owned executable + examples; +- the targeted public contract gaps are covered with deterministic tests; +- the full roadmap behavior list has a clear, non-duplicative test owner; +- tests that require Step 4 rewrites are explicitly identified; +- no production behavior or public API changed; +- all validation in Stage 5 passes; and +- the main migration roadmap records the Step 3 gate as complete. + +Migration Step 4 must not begin until these criteria are satisfied. + +## Lifecycle + +This is a temporary implementation roadmap. Once Step 3 is complete and its +gate status is recorded in the main migration roadmap, this file may be removed; +repository history retains the detailed implementation record.