340 lines
13 KiB
Markdown
340 lines
13 KiB
Markdown
# 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.
|