Complete step 3 of the migration plan and clean up the implemented roadmap

This commit is contained in:
2026-07-27 19:05:28 -05:00
parent 8703793b0c
commit 3074b3165a
3 changed files with 19 additions and 521 deletions

View File

@@ -1,340 +0,0 @@
# Migration Step 3 Implementation Plan
## Status
Completed on 2026-07-27. The framework characterization baseline is complete,
and Migration Step 4 is the next planned work.
## 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.

View File

@@ -1,178 +0,0 @@
# Migration Step 3: Framework Characterization
## Status
Completed on 2026-07-27. The target state below is characterized by the
passing baseline; its ordered implementation record 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
The completed baseline provides:
- 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 required validation 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 completed implementation roadmap remains the concise characterization
record for the migration. Repository history retains the detailed implementation
record.

View File

@@ -610,6 +610,8 @@ func TestRunPassesPreparedRequestToInjectedLLMClient(t *testing.T) {
}
func TestEngineRunPropagatesCallerCancellation(t *testing.T) {
const synchronizationTimeout = 5 * time.Second
started := make(chan struct{})
transport := roundTripFunc(func(req *http.Request) (*http.Response, error) {
close(started)
@@ -640,10 +642,24 @@ func TestEngineRunPropagatesCallerCancellation(t *testing.T) {
result <- err
}()
<-started
watchdog := time.NewTimer(synchronizationTimeout)
defer watchdog.Stop()
select {
case <-started:
case err := <-result:
t.Fatalf("Engine.Run returned before the transport started: %v", err)
case <-watchdog.C:
t.Fatal("timed out waiting for the transport to start")
}
cancel()
if err := <-result; !errors.Is(err, scriptorium.ErrLLMGenerate) {
t.Fatalf("expected ErrLLMGenerate after caller cancellation, got %v", err)
select {
case err := <-result:
if !errors.Is(err, scriptorium.ErrLLMGenerate) {
t.Fatalf("expected ErrLLMGenerate after caller cancellation, got %v", err)
}
case <-watchdog.C:
t.Fatal("timed out waiting for Engine.Run to return after cancellation")
}
}