Add a roadmap and implementation plan for an initial public library package
This commit is contained in:
@@ -1,111 +1,144 @@
|
||||
# Runtime Parameter Implementation Plan
|
||||
# Library API Implementation Plan
|
||||
|
||||
This plan implements the target state in `docs/roadmap/params.md`.
|
||||
This plan implements the target state in `docs/roadmap/library.md`.
|
||||
|
||||
Audience: LLM coding agents implementing the feature in order. Follow `docs/policy/architecture.md`, `docs/policy/development.md`, and `docs/policy/documentation.md` before changing code.
|
||||
|
||||
## Constraints
|
||||
|
||||
- Keep adapters thin. CLI and HTTP should capture caller intent and map it into domain request types; merge decisions belong in `internal/usecase`.
|
||||
- Keep external decoding strict. Unknown YAML/JSON fields must continue to fail.
|
||||
- Do not accept or emit raw API key values.
|
||||
- Do not add dependencies unless there is a clear need. This feature should use the standard library plus existing dependencies.
|
||||
- Do not expand the HTTP API surface beyond `POST /v1/runs`.
|
||||
- Do not add provider-specific adapter packages.
|
||||
- Add a public root package named `scriptorium`; keep existing `internal/*` packages internal.
|
||||
- Define public facade types and convert to/from internal domain types. Do not alias internal domain types as the public API.
|
||||
- Do not rewire CLI or HTTP through the public facade in this implementation.
|
||||
- Preserve current CLI, HTTP, prompt/profile loading, validation, secret-handling, and outbound LLM behavior.
|
||||
- Do not add dependencies.
|
||||
- Keep each stage passing `go test ./...` before moving to the next stage.
|
||||
|
||||
## Stage 1: Presence-Aware Request Overrides
|
||||
## Stage 1: Public Types, Engine Construction, And Prepare
|
||||
|
||||
Goal: make per-request numeric execution overrides presence-aware while keeping resolved execution settings concrete.
|
||||
Goal: make prompt preparation usable from an imported root package without calling an LLM.
|
||||
|
||||
### Domain Changes
|
||||
### Public Package
|
||||
|
||||
1. In `internal/domain/domain.go`, add a request-only type:
|
||||
Create Go files at the module root using:
|
||||
|
||||
```go
|
||||
type ExecutionTargetOverride struct {
|
||||
Endpoint string `json:"endpoint,omitempty"`
|
||||
Model string `json:"model,omitempty"`
|
||||
Temperature *float64 `json:"temperature,omitempty"`
|
||||
MaxTokens *int `json:"max_tokens,omitempty"`
|
||||
TopP *float64 `json:"top_p,omitempty"`
|
||||
TimeoutSeconds *int `json:"timeout_seconds,omitempty"`
|
||||
ServiceTier string `json:"service_tier,omitempty"`
|
||||
ReasoningEffort string `json:"reasoning_effort,omitempty"`
|
||||
APIKeyEnv string `json:"api_key_env,omitempty"`
|
||||
ExtraParams map[string]any `json:"extra_params,omitempty"`
|
||||
package scriptorium
|
||||
```
|
||||
|
||||
Expose:
|
||||
|
||||
```go
|
||||
type Engine struct { /* unexported fields */ }
|
||||
|
||||
type Config struct {
|
||||
PromptDir string
|
||||
ProfileDir string
|
||||
SchemaDir string
|
||||
Timeout time.Duration
|
||||
HTTPClient *http.Client
|
||||
}
|
||||
|
||||
type Option func(*engineOptions) error
|
||||
|
||||
func NewEngine(cfg Config, opts ...Option) (*Engine, error)
|
||||
func (e *Engine) Prepare(ctx context.Context, req RunRequest) (*PreparedRun, error)
|
||||
```
|
||||
|
||||
Construction rules:
|
||||
|
||||
- `PromptDir` and `ProfileDir` are required.
|
||||
- `SchemaDir` defaults to the same built-in default used by app config.
|
||||
- `Timeout`, when non-zero, configures the default OpenAI-compatible client timeout.
|
||||
- `HTTPClient`, when non-nil, is used by the default OpenAI-compatible client.
|
||||
- `NewEngine` wires the same internal components used by CLI/HTTP: filesystem prompt/profile repositories, composite artifact reader, Go template renderer, standard validator, and OpenAI-compatible LLM client.
|
||||
- Return public `ErrInvalidConfig` for invalid engine configuration.
|
||||
|
||||
### Public Types
|
||||
|
||||
Define public facade types with exported fields:
|
||||
|
||||
- `RunRequest`
|
||||
- `PreparedRun`
|
||||
- `ArtifactRef`
|
||||
- `Artifact`
|
||||
- `ExecutionTarget`
|
||||
- `ExecutionTargetOverride`
|
||||
- `ExecutionTargetPresence`
|
||||
- `OutputContract`
|
||||
- `ValidationResult`
|
||||
- `TokenUsage`
|
||||
- `RenderedMessage`
|
||||
- `CacheControl`
|
||||
- `StructuredOutputSpec`
|
||||
|
||||
Use the same enum string values as internal domain types for formats, validation modes, validation statuses, artifact ref types, cache-control type, and structured-output type.
|
||||
|
||||
Required request shape:
|
||||
|
||||
```go
|
||||
type RunRequest struct {
|
||||
PromptID string
|
||||
PromptVersion string
|
||||
ProfileID string
|
||||
Inputs map[string]ArtifactRef
|
||||
Vars map[string]string
|
||||
Execution *ExecutionTargetOverride
|
||||
Validation *OutputContract
|
||||
Metadata map[string]string
|
||||
}
|
||||
```
|
||||
|
||||
2. Change `domain.RunRequest.Execution` from `*ExecutionTarget` to `*ExecutionTargetOverride`.
|
||||
3. Change `ExecutionProfile.ExtraParams` and `ExecutionTarget.ExtraParams` from `map[string]string` to `map[string]any`.
|
||||
4. Keep `ExecutionTarget` concrete. It represents the resolved effective runtime target after defaults, profile, and request overrides are merged.
|
||||
`ExecutionTargetOverride` must preserve numeric override presence using pointer fields:
|
||||
|
||||
### Runner Changes
|
||||
```go
|
||||
Temperature *float64
|
||||
MaxTokens *int
|
||||
TopP *float64
|
||||
TimeoutSeconds *int
|
||||
```
|
||||
|
||||
1. Update `internal/usecase/runner.go` so profile values still merge over built-in defaults and request overrides merge over that result.
|
||||
2. Keep the existing concrete profile merge semantics for profile numeric fields.
|
||||
3. Add a separate request override merge path that uses pointer presence:
|
||||
- `nil` numeric pointer means omitted; preserve the current value.
|
||||
- non-nil numeric pointer means explicit override, even when the value is `0`.
|
||||
4. Validate request override numeric values before or during merge:
|
||||
- `temperature`: `0 <= value <= 2`
|
||||
- `max_tokens`: `value >= 0`
|
||||
- `top_p`: `0 <= value <= 1`
|
||||
- `timeout_seconds`: `value >= 0`
|
||||
5. Preserve existing validation after merge:
|
||||
- effective endpoint required
|
||||
- effective model required
|
||||
- `api_key_env`, when set, must name a non-empty environment variable
|
||||
6. Preserve secret handling. The resolved API key value must never be stored in `PreparedRun`, `RunResult`, logs, or HTTP responses.
|
||||
`PreparedRun` should include the same user-observable fields as internal `domain.PreparedRun`, but should not expose internal-only target presence metadata.
|
||||
|
||||
### CLI Changes
|
||||
### Input Helpers
|
||||
|
||||
1. Update `internal/adapter/cli/run.go` request construction to build `domain.ExecutionTargetOverride`.
|
||||
2. Use the existing `flagWasSet` booleans to populate numeric pointers only when the user provided the flag.
|
||||
3. Required behavior:
|
||||
- omitted `--temperature` preserves profile/default temperature;
|
||||
- `--temperature 0` explicitly sets temperature to zero;
|
||||
- omitted `--top-p` preserves profile/default top-p;
|
||||
- `--top-p 0` explicitly sets top-p to zero;
|
||||
- omitted `--max-tokens` preserves profile/default max tokens;
|
||||
- `--max-tokens 0` explicitly sets max tokens to zero;
|
||||
- omitted `--timeout` preserves profile/default timeout;
|
||||
- `--timeout 0s` explicitly sets timeout seconds to zero.
|
||||
4. Do not add new CLI flags in this stage.
|
||||
Expose:
|
||||
|
||||
### HTTP Changes
|
||||
```go
|
||||
func File(path string) ArtifactRef
|
||||
func Inline(body string) ArtifactRef
|
||||
func InlineWithURI(uri string, body string) ArtifactRef
|
||||
```
|
||||
|
||||
1. Update `internal/adapter/http/dto.go` so numeric model override fields are pointers:
|
||||
- `Temperature *float64`
|
||||
- `MaxTokens *int`
|
||||
- `TopP *float64`
|
||||
- `TimeoutSeconds *int`
|
||||
2. Update DTO mapping in `internal/adapter/http/handler.go` to build `domain.ExecutionTargetOverride`.
|
||||
3. Preserve strict JSON decoding and existing error mapping.
|
||||
4. Required behavior:
|
||||
- omitted numeric JSON fields preserve profile/default values;
|
||||
- explicit numeric zero JSON fields override profile/default values.
|
||||
Mapping:
|
||||
|
||||
- `File(path)` maps to artifact type `file` with `URI: path`.
|
||||
- `Inline(body)` maps to artifact type `inline` with `Body: body`.
|
||||
- `InlineWithURI(uri, body)` maps to artifact type `inline` with both fields set.
|
||||
|
||||
### Conversion Layer
|
||||
|
||||
Implement unexported conversion helpers in the public package:
|
||||
|
||||
- public run request to internal `domain.RunRequest`
|
||||
- internal `domain.PreparedRun` to public `PreparedRun`
|
||||
- internal artifacts/messages/contracts/validation/usage/structured-output to public equivalents
|
||||
- public execution override to internal `domain.ExecutionTargetOverride`
|
||||
|
||||
Conversions must deep-copy maps and slices that cross the public/internal boundary.
|
||||
|
||||
### Tests
|
||||
|
||||
Add or update tests in:
|
||||
Add root package tests.
|
||||
|
||||
- `internal/usecase/runner_test.go`
|
||||
- `internal/adapter/cli/run_test.go`
|
||||
- `internal/adapter/http/handler_test.go`
|
||||
Required tests:
|
||||
|
||||
Required test coverage:
|
||||
|
||||
- Runner preserves profile value when request numeric override is omitted.
|
||||
- Runner applies explicit zero request override for `temperature`.
|
||||
- Runner applies explicit zero request override for `top_p`.
|
||||
- Runner applies explicit zero request override for `max_tokens`.
|
||||
- Runner applies explicit zero request override for `timeout_seconds`.
|
||||
- Invalid request override ranges fail as invalid request errors.
|
||||
- CLI `--temperature 0` reaches effective settings as zero.
|
||||
- HTTP `"temperature": 0` reaches effective settings as zero.
|
||||
- HTTP omitted `temperature` preserves profile/default value.
|
||||
- `NewEngine` rejects missing `PromptDir`.
|
||||
- `NewEngine` rejects missing `ProfileDir`.
|
||||
- `Prepare` works with `examples/config.yml` directories when passed directly through `Config`.
|
||||
- `Prepare` works with `File` input refs.
|
||||
- `Prepare` works with `Inline` input refs.
|
||||
- `Prepare` output does not expose raw API-key values or internal target presence metadata in JSON.
|
||||
- Explicit zero execution overrides survive into prepared effective settings.
|
||||
|
||||
### Verification
|
||||
|
||||
@@ -115,85 +148,109 @@ Run:
|
||||
go test ./...
|
||||
```
|
||||
|
||||
## Stage 2: JSON-Compatible `extra_params`
|
||||
## Stage 2: Run, LLM Injection, And Public Errors
|
||||
|
||||
Goal: allow provider-specific parameters to carry JSON-compatible values throughout profile, HTTP, prepared output, metadata, and LLM request construction.
|
||||
Goal: make full execution usable and testable without real provider credentials.
|
||||
|
||||
### Domain And Loader Changes
|
||||
### Public Run Method
|
||||
|
||||
1. Complete all compile fixes from changing `ExtraParams` to `map[string]any`.
|
||||
2. Ensure `internal/profile/filesystem_repository.go` continues to decode profiles strictly while allowing nested JSON-compatible values under `extra_params`.
|
||||
3. Add profile repository tests for `extra_params` containing:
|
||||
- string
|
||||
- number
|
||||
- boolean
|
||||
- nested object or array
|
||||
4. Ensure formatter output remains deterministic:
|
||||
- keep sorting `extra_params` keys in `internal/format/prepared_run.go`;
|
||||
- render non-string values with stable JSON encoding in text output.
|
||||
5. Preserve JSON formatter behavior through normal `encoding/json` output.
|
||||
Expose:
|
||||
|
||||
### HTTP Changes
|
||||
|
||||
1. Change HTTP model override `ExtraParams` to `map[string]any`.
|
||||
2. Add handler tests proving HTTP accepts JSON-compatible `extra_params` values.
|
||||
3. Preserve strict rejection of unknown fields and raw API-key payload fields.
|
||||
|
||||
### Verification
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
go test ./...
|
||||
```go
|
||||
func (e *Engine) Run(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
```
|
||||
|
||||
## Stage 3: Outbound Serialization
|
||||
`RunResult` should expose:
|
||||
|
||||
Goal: serialize `reasoning_effort` and `extra_params` to the OpenAI-compatible chat-completions request.
|
||||
- run ID
|
||||
- artifact
|
||||
- raw output
|
||||
- validation result
|
||||
- prompt/profile/model metadata
|
||||
- effective model params
|
||||
- input hashes
|
||||
- token/cache usage
|
||||
- start/end/duration timing
|
||||
|
||||
### LLM Adapter Changes
|
||||
Do not expose raw API-key values.
|
||||
|
||||
1. In `internal/llm/openai_compatible_client.go`, add first-class outbound support for `reasoning_effort`.
|
||||
2. Add `extra_params` support by flattening `domain.ExecutionTarget.ExtraParams` into additional top-level JSON request fields.
|
||||
3. Implement reserved-field collision checks before the HTTP request is made.
|
||||
4. Reserved keys must include:
|
||||
- `model`
|
||||
- `session_id`
|
||||
- `messages`
|
||||
- `temperature`
|
||||
- `max_tokens`
|
||||
- `top_p`
|
||||
- `service_tier`
|
||||
- `reasoning_effort`
|
||||
- `response_format`
|
||||
5. Reject empty `extra_params` keys.
|
||||
6. Ensure each `extra_params` value can be marshaled as JSON. If marshaling fails, return `ErrInvalidRequest` with context.
|
||||
7. Keep existing request behavior unchanged when `reasoning_effort` and `extra_params` are unset.
|
||||
### Public LLM Injection
|
||||
|
||||
### Recommended Implementation Shape
|
||||
Expose:
|
||||
|
||||
Use a custom marshal path for the outbound chat request rather than string manipulation.
|
||||
```go
|
||||
type LLMClient interface {
|
||||
Generate(context.Context, GenerateRequest) (*GenerateResponse, error)
|
||||
}
|
||||
|
||||
One acceptable shape:
|
||||
func WithLLMClient(client LLMClient) Option
|
||||
```
|
||||
|
||||
- Add `ReasoningEffort string` and `ExtraParams map[string]any` to the internal `openAIChatRequest`.
|
||||
- Add a helper that converts `openAIChatRequest` into `map[string]any`, inserts first-class fields when set, then inserts `ExtraParams` after collision validation.
|
||||
- Marshal that map with `encoding/json`.
|
||||
Public `GenerateRequest` must include:
|
||||
|
||||
Do not construct outbound JSON with manual string concatenation.
|
||||
- rendered prompt
|
||||
- effective execution target
|
||||
- execution target presence
|
||||
- structured-output spec
|
||||
|
||||
Public `GenerateResponse` must include:
|
||||
|
||||
- content
|
||||
- token usage
|
||||
|
||||
Implementation rule:
|
||||
|
||||
- `WithLLMClient` wraps the public client in an unexported adapter that satisfies `internal/llm.Client`.
|
||||
- The adapter converts internal generate requests to public generate requests and converts public generate responses back to internal responses.
|
||||
- A nil client passed to `WithLLMClient` returns `ErrInvalidConfig`.
|
||||
|
||||
Default behavior:
|
||||
|
||||
- If no custom LLM client is supplied, `NewEngine` uses `internal/llm.NewOpenAICompatibleClient`.
|
||||
- `Config.Timeout` and `Config.HTTPClient` apply only to the default OpenAI-compatible client.
|
||||
|
||||
### Public Errors
|
||||
|
||||
Define public sentinel errors:
|
||||
|
||||
- `ErrInvalidConfig`
|
||||
- `ErrInvalidRequest`
|
||||
- `ErrPromptNotFound`
|
||||
- `ErrProfileNotFound`
|
||||
- `ErrPromptLoad`
|
||||
- `ErrProfileLoad`
|
||||
- `ErrArtifactLoad`
|
||||
- `ErrPromptRender`
|
||||
- `ErrLLMGenerate`
|
||||
- `ErrValidation`
|
||||
|
||||
Public methods must map internal errors to public sentinels while preserving wrapped context. Callers must be able to use `errors.Is`.
|
||||
|
||||
Mapping rules:
|
||||
|
||||
- missing/invalid public engine config -> `ErrInvalidConfig`
|
||||
- internal `usecase.ErrInvalidRequest` -> `ErrInvalidRequest`
|
||||
- internal prompt not found -> `ErrPromptNotFound`
|
||||
- internal profile not found -> `ErrProfileNotFound`
|
||||
- internal prompt load errors -> `ErrPromptLoad`
|
||||
- internal profile load errors -> `ErrProfileLoad`
|
||||
- internal artifact load errors -> `ErrArtifactLoad`
|
||||
- internal prompt render errors -> `ErrPromptRender`
|
||||
- internal LLM generate errors -> `ErrLLMGenerate`
|
||||
- internal validation runtime errors -> `ErrValidation`
|
||||
|
||||
Do not expose internal sentinel values as public API.
|
||||
|
||||
### Tests
|
||||
|
||||
Update `internal/llm/openai_compatible_client_test.go`.
|
||||
Required tests:
|
||||
|
||||
Required test coverage:
|
||||
|
||||
- outbound JSON includes `reasoning_effort` when set;
|
||||
- outbound JSON omits `reasoning_effort` when unset;
|
||||
- outbound JSON includes string, number, boolean, object, and array `extra_params`;
|
||||
- reserved `extra_params` keys fail before provider call;
|
||||
- empty `extra_params` keys fail before provider call;
|
||||
- existing message, cache-control, service-tier, response-format, and usage parsing tests continue to pass.
|
||||
- `Run` succeeds with `WithLLMClient` fake and returns typed artifact, raw output, validation, metadata, and usage.
|
||||
- `Run` passes rendered prompt, effective execution target, and target presence to the injected LLM client.
|
||||
- `Run` validation failure returns a successful result with failed validation, not an error.
|
||||
- public errors support `errors.Is` for invalid request, prompt not found, profile not found, artifact load, render failure, LLM failure, and validation runtime failure.
|
||||
- nil `WithLLMClient(nil)` returns `ErrInvalidConfig`.
|
||||
- default OpenAI-compatible client can still be constructed without real provider credentials.
|
||||
|
||||
### Verification
|
||||
|
||||
@@ -203,39 +260,39 @@ Run:
|
||||
go test ./...
|
||||
```
|
||||
|
||||
## Stage 4: Documentation And Examples
|
||||
## Stage 3: Public Documentation And Consumer Examples
|
||||
|
||||
Goal: move implemented behavior from roadmap to canonical docs after code is complete.
|
||||
Goal: document implemented library behavior in canonical public-consumer docs.
|
||||
|
||||
Update only after Stages 1 through 3 are implemented.
|
||||
### Docs
|
||||
|
||||
### Required Docs
|
||||
After Stages 1 and 2 are implemented, update:
|
||||
|
||||
Update:
|
||||
- `README.md`: add a short link to library usage without turning the README into a manual.
|
||||
- `docs/internal/adapters.md`: list the public library facade as an implemented adapter surface.
|
||||
- `docs/consumers/api.md`: describe the public consumer API at a high level.
|
||||
- `docs/consumers/pkg-scriptorium.md`: document the root package usage, types, errors, and examples.
|
||||
|
||||
- `docs/config.md`
|
||||
- `docs/cli.md`
|
||||
- `docs/integrations/http-api.md`
|
||||
- `docs/integrations/openai-compatible-chat.md`
|
||||
- `docs/internal/runner.md`
|
||||
- `docs/internal/adapters.md`
|
||||
Create `docs/consumers/` if it does not exist.
|
||||
|
||||
Required documentation content:
|
||||
|
||||
- `reasoning_effort` is serialized outbound when set.
|
||||
- `extra_params` serializes as provider-specific top-level outbound JSON fields.
|
||||
- `extra_params` supports JSON-compatible values.
|
||||
- reserved `extra_params` fields are rejected.
|
||||
- per-request numeric overrides distinguish omitted values from explicit zero values.
|
||||
- CLI explicit zero behavior for existing numeric flags.
|
||||
- HTTP explicit zero behavior for model override numeric fields.
|
||||
- no raw API-key values are accepted or emitted.
|
||||
Do not document unimplemented future library features outside `docs/roadmap/`.
|
||||
|
||||
### Examples
|
||||
|
||||
Update examples only if needed to keep them accurate and runnable.
|
||||
Add copyable library examples only if they can be tested without real credentials.
|
||||
|
||||
If adding an `extra_params` example, keep it secret-free and simple. Prefer a harmless provider-routing example over a vendor-specific feature that requires special credentials.
|
||||
Recommended example:
|
||||
|
||||
- `examples/go-library/prepare/main.go` or equivalent prepare-only example using `examples/` prompt/profile/fixture assets.
|
||||
|
||||
If adding a run example, it must use an injected fake LLM client and must not require provider credentials.
|
||||
|
||||
### Tests
|
||||
|
||||
Required tests:
|
||||
|
||||
- doc/example smoke coverage for any added Go example using `go test` or `go test ./...`.
|
||||
- existing CLI/HTTP tests continue to pass unchanged.
|
||||
|
||||
### Verification
|
||||
|
||||
@@ -255,8 +312,10 @@ go run ./cmd/scriptorium render \
|
||||
|
||||
Before considering the feature complete:
|
||||
|
||||
1. Confirm `git diff` contains only intended code, test, doc, and example changes.
|
||||
2. Confirm all non-roadmap docs describe implemented behavior only.
|
||||
3. Confirm no output path exposes raw API key values.
|
||||
4. Confirm `go test ./...` passes.
|
||||
5. Confirm the render smoke command passes.
|
||||
1. Confirm the root package can be imported as `gitea.maximumdirect.net/eric/scriptorium`.
|
||||
2. Confirm public package tests do not require real provider credentials.
|
||||
3. Confirm `go test ./...` passes.
|
||||
4. Confirm the render smoke command passes.
|
||||
5. Confirm non-roadmap docs describe only implemented behavior.
|
||||
6. Confirm no public result or rendered/prepared output exposes raw API-key values.
|
||||
7. Confirm `git diff` does not include unrelated CLI/HTTP behavior changes.
|
||||
|
||||
Reference in New Issue
Block a user