Compare commits
10 Commits
75fa0a030a
...
v0.9.0
| Author | SHA1 | Date | |
|---|---|---|---|
| bc099a31ad | |||
| 4ff55221a3 | |||
| 8d8024099f | |||
| 18792fd8d1 | |||
| 8860aa033c | |||
| 3ca14d8b6e | |||
| 099e9c4a3e | |||
| cfe6b9408a | |||
| 6ececc749f | |||
| 79901fbb86 |
@@ -105,6 +105,7 @@ Validator:
|
|||||||
HTTP error mapping:
|
HTTP error mapping:
|
||||||
|
|
||||||
- maps domain/use-case errors to stable HTTP code + error code/message.
|
- maps domain/use-case errors to stable HTTP code + error code/message.
|
||||||
|
- distinguishes missing profile selection and missing `api_key_env` variable using stable use-case sentinel errors.
|
||||||
- avoids returning internal wrapped-cause details in response payload.
|
- avoids returning internal wrapped-cause details in response payload.
|
||||||
|
|
||||||
## CLI Adapter Semantics
|
## CLI Adapter Semantics
|
||||||
@@ -140,4 +141,4 @@ Behavior highlights:
|
|||||||
- Adapter packages do not own runner decision logic.
|
- Adapter packages do not own runner decision logic.
|
||||||
- External request/response strictness is part of contract stability.
|
- External request/response strictness is part of contract stability.
|
||||||
- Prepared-render output never includes resolved API key values.
|
- Prepared-render output never includes resolved API key values.
|
||||||
- Outbound OpenAI-compatible request includes only currently serialized fields (`model`, `messages`, optional `temperature`, `max_tokens`, `top_p`, optional `response_format`).
|
- Outbound OpenAI-compatible request includes only currently serialized fields (`model`, `messages`, optional `temperature`, `max_tokens`, `top_p`, optional `service_tier`, optional `response_format`).
|
||||||
|
|||||||
@@ -66,14 +66,21 @@ It receives fully constructed repositories/readers/validators from adapters. Eff
|
|||||||
|
|
||||||
## Failure Behavior
|
## Failure Behavior
|
||||||
|
|
||||||
Key error classes surfaced from `Runner`:
|
Primary runner error classes:
|
||||||
|
|
||||||
- `ErrInvalidRequest`: invalid prompt/profile/request/runtime/API-key-env prerequisites.
|
- `ErrInvalidRequest`: invalid run request envelope.
|
||||||
- `ErrProfileLoad`: prompt or profile load failures.
|
- `ErrProfileRequired`: specific invalid-request reason when neither request `profile_id` nor prompt `default_profile` is available.
|
||||||
|
- `ErrAPIKeyEnvMissing`: specific invalid-request reason when `api_key_env` is set but the named environment variable is unset/empty.
|
||||||
|
- `ErrProfileLoad`: prompt/profile repository load failures.
|
||||||
- `ErrArtifactLoad`: artifact read failures.
|
- `ErrArtifactLoad`: artifact read failures.
|
||||||
- `ErrPromptRender`: template render failures.
|
- `ErrPromptRender`: template render failures.
|
||||||
- `ErrLLMGenerate`: model request failures.
|
- `ErrLLMGenerate`: outbound model request failures.
|
||||||
- `ErrValidation`: validation runtime failures (including schema load/compile failures).
|
- `ErrValidation`: validation runtime failures (including structured-output schema load/compile failures).
|
||||||
|
|
||||||
|
Reason sentinel behavior:
|
||||||
|
|
||||||
|
- `ErrProfileRequired` and `ErrAPIKeyEnvMissing` are wrapped with `ErrInvalidRequest`.
|
||||||
|
- Adapters can use `errors.Is` for stable reason mapping without matching runner prose.
|
||||||
|
|
||||||
Validation content failures are not run errors:
|
Validation content failures are not run errors:
|
||||||
|
|
||||||
@@ -90,13 +97,15 @@ Validation content failures are not run errors:
|
|||||||
3. select profile ID:
|
3. select profile ID:
|
||||||
- explicit request profile ID
|
- explicit request profile ID
|
||||||
- prompt `default_profile`
|
- prompt `default_profile`
|
||||||
- otherwise request error
|
- otherwise return an invalid request with `ErrProfileRequired`
|
||||||
4. load execution profile.
|
4. load execution profile.
|
||||||
5. merge effective runtime target:
|
5. merge effective runtime target:
|
||||||
- built-in execution defaults
|
- built-in execution defaults
|
||||||
- selected profile values
|
- selected profile values
|
||||||
- request overrides
|
- request overrides
|
||||||
6. verify required `api_key_env` environment variable (name only; value is not returned).
|
6. verify required `api_key_env` environment variable:
|
||||||
|
- missing/empty env value returns an invalid request with `ErrAPIKeyEnvMissing`
|
||||||
|
- only the environment-variable name is retained; secret value is never returned
|
||||||
7. resolve output contract and structured-output schema payload when `json_schema` mode is active.
|
7. resolve output contract and structured-output schema payload when `json_schema` mode is active.
|
||||||
8. read input artifacts.
|
8. read input artifacts.
|
||||||
9. render prompt messages.
|
9. render prompt messages.
|
||||||
|
|||||||
@@ -1,519 +0,0 @@
|
|||||||
# Code Quality And Deduplication Audit
|
|
||||||
|
|
||||||
## Executive Summary
|
|
||||||
|
|
||||||
Overall code quality is solid for the current project size. The repository follows the documented shape: thin CLI/HTTP adapters, a central `Runner`, strict config/prompt/profile loading, narrow filesystem and LLM adapters, and package tests around the important public behaviors.
|
|
||||||
|
|
||||||
The top three refactoring targets are:
|
|
||||||
|
|
||||||
1. centralize execution-target field mapping across profiles, CLI overrides, HTTP DTOs, prepared output, and LLM request serialization;
|
|
||||||
2. share filesystem repository scanning helpers between prompt and profile repositories;
|
|
||||||
3. reduce CLI command wiring duplication for app settings, runner construction, and common preflight behavior.
|
|
||||||
|
|
||||||
The codebase appears ready for a limited cleanup pass. No major architectural rewrite is warranted. The main risk before a release is maintenance drift when adding fields or policy to execution settings, repositories, or command setup.
|
|
||||||
|
|
||||||
## Repository Map Reviewed
|
|
||||||
|
|
||||||
Main areas inspected:
|
|
||||||
|
|
||||||
- `cmd/scriptorium`: process entrypoint path, via repository layout and command docs.
|
|
||||||
- `internal/adapter/cli`: `run`, `render`, `serve` parsing, config loading, app wiring, stdout/stderr behavior, summary output, exit-code policy.
|
|
||||||
- `internal/adapter/http`: request DTOs, strict JSON decoding, domain mapping, response metadata mapping, error mapping.
|
|
||||||
- `internal/config`: app config loading, defaults, strict YAML decoding, CLI override precedence.
|
|
||||||
- `internal/defaults`: built-in runtime/config/content-type defaults.
|
|
||||||
- `internal/domain`: domain request/result/profile/target/artifact/validation types.
|
|
||||||
- `internal/usecase`: `Runner.Prepare`, `Runner.Run`, execution target merge, schema structured-output setup, validation and repair policy.
|
|
||||||
- `internal/promptdef`: recursive prompt YAML loading, prompt normalization, content-file resolution, prompt output-contract validation.
|
|
||||||
- `internal/profile`: recursive profile YAML loading, strict decoding, duplicate ID handling, profile validation, raw API-key rejection.
|
|
||||||
- `internal/artifact`: `inline` and `file` artifact resolution.
|
|
||||||
- `internal/prompt`: Go template rendering and required input checks.
|
|
||||||
- `internal/llm`: OpenAI-compatible request construction, timeout/API-key handling, response parsing.
|
|
||||||
- `internal/validate`: basic/JSON/JSON Schema validation and schema document loading.
|
|
||||||
- `internal/format`: prepared-run text/JSON output.
|
|
||||||
- `examples/`: maintained example config, prompts, profiles, schemas, fixtures, HTTP/render examples.
|
|
||||||
- Package tests under `internal/**`.
|
|
||||||
|
|
||||||
Major execution paths reviewed:
|
|
||||||
|
|
||||||
- CLI `run`: parse flags/config, build `RunRequest`, construct runner, call LLM, write artifact, print summary, choose exit code.
|
|
||||||
- CLI `render`: parse flags/config, build `RunRequest`, construct runner without LLM, prepare only, format prepared run.
|
|
||||||
- CLI `serve`: parse flags/config, construct runner, expose HTTP `POST /v1/runs`.
|
|
||||||
- HTTP `POST /v1/runs`: strict decode DTO, map to `RunRequest`, map `RunResult` to JSON response.
|
|
||||||
- Runner `Prepare` and `Run`: prompt/profile/artifact/schema resolution, render, LLM generate, validation, repair hook.
|
|
||||||
|
|
||||||
Areas not deeply inspected:
|
|
||||||
|
|
||||||
- No `internal/app`, `internal/stage`, `internal/modules`, `internal/storage`, `internal/manifest`, `pkg`, or external test-suite directories exist in the current repository.
|
|
||||||
- Full test execution was not run because this is a report-only pass; inspection was static.
|
|
||||||
|
|
||||||
## High-Confidence Deduplication Opportunities
|
|
||||||
|
|
||||||
### 1. Execution Target Field Mapping Is Repeated Across Boundaries
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/domain/domain.go`
|
|
||||||
- `internal/usecase/runner.go`
|
|
||||||
- `internal/adapter/cli/run.go`
|
|
||||||
- `internal/adapter/http/dto.go`
|
|
||||||
- `internal/adapter/http/handler.go`
|
|
||||||
- `internal/llm/openai_compatible_client.go`
|
|
||||||
- `internal/format/prepared_run.go`
|
|
||||||
- tests in `internal/usecase`, `internal/adapter/http`, `internal/adapter/cli`, `internal/llm`, and `internal/format`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- Execution settings fields are listed and copied in several places: profile-to-target conversion, target merge, CLI runtime override construction, HTTP request mapping, HTTP metadata mapping, prepared-run text output, and OpenAI-compatible request construction.
|
|
||||||
- Adding `service_tier` required coordinated edits across many of these sites, which is a strong signal that field-level mapping is too scattered.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- A future profile/runtime field can easily be parsed but not sent, displayed but not merged, accepted over HTTP but not included in metadata, or tested in one command but not another.
|
|
||||||
- This is public-interface drift risk because CLI, HTTP, render output, and provider requests all expose different slices of the same effective execution target.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Keep transport DTOs local to adapters, but add small mapping helpers at each boundary:
|
|
||||||
- a use-case/domain helper for profile-to-target copy and execution-target merge;
|
|
||||||
- an HTTP helper such as `executionTargetFromModelOverrideDTO` and `modelParamsDTOFromExecutionTarget`;
|
|
||||||
- an LLM helper such as `openAIChatRequestFromGenerateRequest` for outbound serialization.
|
|
||||||
- Add a table-driven test that constructs an `ExecutionTarget` with every supported field and verifies the HTTP metadata DTO and LLM request payload retain the intended fields.
|
|
||||||
- Avoid reflection-based generic copying; explicit mapping is still clearer here.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- Expand runner merge tests to use an all-fields target.
|
|
||||||
- Add HTTP DTO mapping tests that fail when a domain execution field is omitted from request or metadata mapping.
|
|
||||||
- Add LLM request serialization tests for all serialized fields and explicit omission of non-serialized fields.
|
|
||||||
|
|
||||||
Risk level: high. The refactor itself is low-to-medium implementation risk, but the duplicated behavior has high drift risk.
|
|
||||||
|
|
||||||
### 2. Prompt And Profile Filesystem Repository Scanning Is Duplicated
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/promptdef/filesystem_repository.go`
|
|
||||||
- `internal/profile/filesystem_repository.go`
|
|
||||||
- repository tests in `internal/promptdef` and `internal/profile`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- Both repositories recursively walk configured directories, filter `.yaml`/`.yml`, sort paths, compute relative paths, derive likely IDs from filenames, use strict YAML decoding, partially decode IDs to decide whether to surface malformed likely-target files, detect duplicate IDs, and include relative paths in errors.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- Prompt/profile repository policy should remain aligned: nested scanning, stable ordering, duplicate ID errors, likely-target malformed file behavior, and relative-path diagnostics.
|
|
||||||
- Future changes to repository scanning or extension rules would need to be made in two packages.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Introduce a narrow internal helper for filesystem catalog behavior, for example `internal/filecatalog` or another small package whose scope is only:
|
|
||||||
- recursive YAML file discovery with context cancellation;
|
|
||||||
- stable sorting;
|
|
||||||
- relative path formatting;
|
|
||||||
- YAML extension checks;
|
|
||||||
- filename stem extraction.
|
|
||||||
- Keep prompt/profile-specific normalization and validation in their current packages.
|
|
||||||
- Do not make a generic repository framework.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- Add helper-level tests for recursive YAML discovery, ordering, extension filtering, and relative path output.
|
|
||||||
- Keep existing prompt/profile behavior tests unchanged to confirm public error behavior survives.
|
|
||||||
|
|
||||||
Risk level: high for drift prevention; low implementation risk if the helper stays small.
|
|
||||||
|
|
||||||
### 3. CLI Command Setup And Runner Wiring Are Repeated
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/adapter/cli/run.go`
|
|
||||||
- `internal/adapter/cli/run_test.go`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- `run`, `render`, and `serve` each construct similar runner dependencies.
|
|
||||||
- `run` and `serve` both create an OpenAI-compatible client with default timeout.
|
|
||||||
- `run`, `render`, and `serve` all resolve app settings, clean dirs, validate required prompt/profile dirs, and create filesystem repositories/validator/renderer/readers.
|
|
||||||
- `run` and `render` share most execution request flags and request construction, with command-specific differences around `--schema-dir`, `--format`, and LLM use.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- Adding or changing a dependency, default, or preflight check can drift between commands.
|
|
||||||
- The current duplication is still readable, but it is large enough that future command additions or option changes will be error-prone.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Add a small CLI-local wiring helper, such as `newRunnerFromDirs(promptDir, profileDir, schemaDir string, llmClient llm.Client) *usecase.Runner`.
|
|
||||||
- Add a small CLI-local resolved settings struct for common `prompt_dir`, `profile_dir`, `schema_dir`, and render-format handling.
|
|
||||||
- Preserve the existing command-specific parse functions and flag surfaces; do not hide command behavior behind a generic command framework.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- Keep parser tests for each command.
|
|
||||||
- Add one test that verifies `run`, `render`, and `serve` use the same configured prompt/profile/schema dirs by exercising config-derived dirs.
|
|
||||||
- Keep command-level success tests for `run` and `render`.
|
|
||||||
|
|
||||||
Risk level: medium. Behavior is public, but a small CLI-local helper can be behavior-preserving.
|
|
||||||
|
|
||||||
### 4. HTTP Error Mapping Depends On Error Message Substrings
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/usecase/runner.go`
|
|
||||||
- `internal/adapter/http/handler.go`
|
|
||||||
- `internal/adapter/http/handler_test.go`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- Runner creates `ErrInvalidRequest` with human-readable details for cases such as missing profile selection and missing API-key env.
|
|
||||||
- HTTP maps some specific invalid-request cases by checking `strings.Contains(err.Error(), ...)`.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- User-facing HTTP error codes can drift if a runner error message is clarified.
|
|
||||||
- This crosses package boundaries in a brittle way: HTTP should depend on stable error identity, not exact prose from `internal/usecase`.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Add narrow sentinel errors or typed invalid-request reasons in `internal/usecase`, for example profile-required and API-key-env-missing.
|
|
||||||
- Keep HTTP response messages stable and adapter-owned.
|
|
||||||
- Do not expose HTTP-specific error codes from the runner.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- HTTP error mapping tests should assert `profile_required` and `api_key_env_missing` via sentinel wrapping, not via message matching.
|
|
||||||
- Runner tests should assert `errors.Is` for the new reason errors.
|
|
||||||
|
|
||||||
Risk level: high for public API stability; low-to-medium implementation risk.
|
|
||||||
|
|
||||||
## Medium-Confidence Opportunities
|
|
||||||
|
|
||||||
### 1. Strict YAML Decode Setup Is Repeated
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/config/config.go`
|
|
||||||
- `internal/promptdef/filesystem_repository.go`
|
|
||||||
- `internal/profile/filesystem_repository.go`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- Each package constructs a YAML decoder and enables `KnownFields(true)`.
|
|
||||||
|
|
||||||
Semantic differences that may be intentional:
|
|
||||||
|
|
||||||
- Prompt/profile loaders need likely-target behavior and raw `api_key` handling.
|
|
||||||
- Config loading has its own explicit/implicit file search policy.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- Strict decoding is an architectural invariant. A future YAML loader could forget to enable it.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Consider a tiny helper for strict YAML decode from bytes.
|
|
||||||
- Keep package-specific error wrapping and partial ID decode logic local.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- Existing unknown-field tests in config, promptdef, and profile should remain.
|
|
||||||
- Add any new YAML-consuming package with an unknown-field test.
|
|
||||||
|
|
||||||
Risk level: medium.
|
|
||||||
|
|
||||||
### 2. Schema Path Resolution Is Centralized, But Schema Loading Happens Twice
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/usecase/runner.go`
|
|
||||||
- `internal/validate/standard_validator.go`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- For `json_schema`, `Runner.Prepare` asks the validator to load the schema document for provider-level structured output.
|
|
||||||
- Later validation resolves and compiles the same schema path again.
|
|
||||||
|
|
||||||
Semantic differences that may be intentional:
|
|
||||||
|
|
||||||
- Provider structured-output payload needs the raw JSON schema document.
|
|
||||||
- Runtime validation needs a compiled schema.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- The current behavior is correct, but schema file errors can surface at different phases and the same file is read more than once during `Run`.
|
|
||||||
- Future caching or schema behavior changes should have one clear owner.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Keep path resolution inside `internal/validate`.
|
|
||||||
- Consider a schema service/loader interface that can return both raw document and compiled schema from one path, only if schema-related work grows.
|
|
||||||
- Do not add caching unless repeated schema loads become a measured cost.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- Keep existing nested schema path tests.
|
|
||||||
- Add a test that `Prepare` fails before LLM call when structured-output schema cannot load.
|
|
||||||
|
|
||||||
Risk level: medium.
|
|
||||||
|
|
||||||
### 3. Artifact Hashing And Output Artifact Construction Are Split
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/artifact/reader.go`
|
|
||||||
- `internal/usecase/runner.go`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- Input artifacts and generated output artifacts both compute SHA-256 hashes and fill size/body/content-type fields.
|
|
||||||
|
|
||||||
Semantic differences that may be intentional:
|
|
||||||
|
|
||||||
- Input artifacts derive content type from source extension or inline default.
|
|
||||||
- Output artifacts derive content type from prompt output format and always use the default output artifact name.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- Hash algorithm and artifact metadata policy should remain consistent.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Consider an `artifact.Build` or `artifact.HashBody` helper only for shared hash/size construction.
|
|
||||||
- Keep source-specific content type and naming decisions local.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- Existing artifact reader tests and runner output content-type tests should cover behavior.
|
|
||||||
- Add a small helper test if a shared hash function is introduced.
|
|
||||||
|
|
||||||
Risk level: medium-low.
|
|
||||||
|
|
||||||
### 4. Domain Contains An Unsupported `s3` Artifact Reference Constant
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/domain/domain.go`
|
|
||||||
- `internal/artifact/reader.go`
|
|
||||||
- docs that state only `inline` and `file` are implemented
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- Not a duplication issue. This is a cleanup issue: `domain.ArtifactRefS3` exists, but no reader supports it and docs do not document it as implemented.
|
|
||||||
|
|
||||||
Semantic differences that may be intentional:
|
|
||||||
|
|
||||||
- The constant may be a placeholder for a future adapter.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- Future-facing code outside `docs/roadmap/` can confuse maintainers and tests. It also weakens the otherwise clean "implemented behavior only" policy.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Remove the constant if no near-term S3 implementation is planned.
|
|
||||||
- If kept, add a code comment that it is intentionally unsupported today and ensure docs continue to state only `inline` and `file` are supported.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- Existing unsupported artifact type tests should continue to pass.
|
|
||||||
|
|
||||||
Risk level: medium-low.
|
|
||||||
|
|
||||||
### 5. Test Fixture Setup Is Repeated In CLI And Use-Case Tests
|
|
||||||
|
|
||||||
Affected files/packages:
|
|
||||||
|
|
||||||
- `internal/adapter/cli/run_test.go`
|
|
||||||
- `internal/usecase/runner_test.go`
|
|
||||||
- `internal/usecase/integration_test.go`
|
|
||||||
|
|
||||||
Duplicated or near-duplicated behavior:
|
|
||||||
|
|
||||||
- Tests repeatedly construct temp prompt/profile dirs, write prompt/profile YAML, set up fake LLMs/readers/renderers/validators, and build runners.
|
|
||||||
|
|
||||||
Semantic differences that may be intentional:
|
|
||||||
|
|
||||||
- Package-local tests avoid exporting test helpers and keep each package independent.
|
|
||||||
|
|
||||||
Why it matters:
|
|
||||||
|
|
||||||
- Repeated setup makes behavior-preserving refactors noisier and can obscure the exact behavior under test.
|
|
||||||
|
|
||||||
Recommended refactor:
|
|
||||||
|
|
||||||
- Add package-local helper builders where duplication is highest, especially in CLI command tests.
|
|
||||||
- Avoid a cross-package test utility package unless multiple packages need the same public fixture contract.
|
|
||||||
|
|
||||||
Suggested tests:
|
|
||||||
|
|
||||||
- This is test cleanup only; existing test assertions should remain equivalent.
|
|
||||||
|
|
||||||
Risk level: low.
|
|
||||||
|
|
||||||
## Boundary And Responsibility Concerns
|
|
||||||
|
|
||||||
- HTTP error mapping currently depends on runner error message text. Stable reason identity should live in `internal/usecase`; HTTP status/code/message mapping should remain in `internal/adapter/http`.
|
|
||||||
- Execution-target merge policy lives in `internal/usecase`, which fits the architecture. The concern is not placement but incomplete centralization of field mapping around that policy.
|
|
||||||
- CLI app wiring currently constructs concrete repositories/readers/renderers/validators inline in each command. That is acceptable adapter responsibility, but repeated wiring should be centralized within the CLI adapter package.
|
|
||||||
- Prompt/profile filesystem traversal is duplicated in two repository packages. A narrow filesystem catalog helper would fit the architecture because it would not own prompt/profile policy.
|
|
||||||
- LLM provider request shape is properly isolated in `internal/llm`. It should remain there; do not move OpenAI/OpenRouter request-field policy into runner or prompt definitions.
|
|
||||||
|
|
||||||
## Path, Key, And Naming Construction Review
|
|
||||||
|
|
||||||
Path and naming construction is mostly explicit and low-risk:
|
|
||||||
|
|
||||||
- Config paths are cleaned in `internal/config` and again in CLI finalization.
|
|
||||||
- Prompt `content_file` paths are resolved relative to the prompt YAML file in `internal/promptdef`.
|
|
||||||
- Schema paths are resolved through `internal/validate.StandardValidator`.
|
|
||||||
- OpenAI-compatible endpoint paths use `defaults.OpenAIChatCompletionsPath`.
|
|
||||||
- Output artifact name comes from `defaults.OutputArtifactName`.
|
|
||||||
|
|
||||||
Areas needing cleanup:
|
|
||||||
|
|
||||||
- Prompt/profile recursive YAML discovery and relative-path formatting should share one helper.
|
|
||||||
- CLI path cleaning after config resolution is repeated and should be consolidated with common command settings finalization.
|
|
||||||
- Structured schema names are derived in `internal/usecase`; this is currently one place and should stay there unless structured-output support expands.
|
|
||||||
|
|
||||||
No remote keys, cache paths, manifest paths, lock files, or generated report paths exist in the current implementation.
|
|
||||||
|
|
||||||
## Resolution And Catalog Review
|
|
||||||
|
|
||||||
Named concept resolution is mostly consistent:
|
|
||||||
|
|
||||||
- Prompt resolution uses YAML `id` and optional `version`, not file path.
|
|
||||||
- Profile resolution uses YAML `id`, not file path.
|
|
||||||
- Prompt/profile subdirectories are organizational only.
|
|
||||||
- Duplicate prompt/profile IDs fail instead of choosing first match.
|
|
||||||
- Schema resolution is explicit path-based relative to `schema_dir`; no basename search.
|
|
||||||
- Input artifacts are resolved by `artifact.Reader`, with `inline` and `file` as implemented types.
|
|
||||||
- Prompt required-input checks happen in the renderer.
|
|
||||||
|
|
||||||
Recommended centralization:
|
|
||||||
|
|
||||||
- Share only filesystem catalog mechanics between prompt/profile repositories.
|
|
||||||
- Keep prompt ID/version, profile ID, schema path, and input resolution policies in their current owning packages.
|
|
||||||
|
|
||||||
## Config And Command-Loading Review
|
|
||||||
|
|
||||||
Config precedence is consistent with policy:
|
|
||||||
|
|
||||||
1. built-in defaults;
|
|
||||||
2. config file values;
|
|
||||||
3. CLI overrides.
|
|
||||||
|
|
||||||
Intentional differences:
|
|
||||||
|
|
||||||
- `render` supports `--format`; `run` does not.
|
|
||||||
- `serve` supports `--addr`; `run`/`render` do not.
|
|
||||||
- `serve` rejects runtime model override flags.
|
|
||||||
- `render` does not expose `--schema-dir`, even though it can prepare `json_schema` prompts through config/default schema settings.
|
|
||||||
- CLI runtime model override flags are narrower than HTTP model override fields; this is currently documented by omission in CLI docs.
|
|
||||||
|
|
||||||
Likely cleanup areas:
|
|
||||||
|
|
||||||
- Common command settings finalization can be made smaller and less repetitive.
|
|
||||||
- Runner dependency construction should be a CLI-local helper.
|
|
||||||
- The `runConfig` flag-set booleans work, but adding more runtime override fields will continue to require touching several fields and the override condition.
|
|
||||||
|
|
||||||
## State, Manifest, Or Progress Handling Review
|
|
||||||
|
|
||||||
Scriptorium has no durable state, manifests, checkpoints, progress records, cache state, or resume behavior. This matches `docs/policy/architecture.md` and `docs/operations.md`.
|
|
||||||
|
|
||||||
There is no drift affecting resume, retry, force, dry-run, or audit behavior because those features do not exist. Validation repair hooks exist in the runner but are not wired by CLI/HTTP today; that boundary is documented.
|
|
||||||
|
|
||||||
## Refactors To Avoid
|
|
||||||
|
|
||||||
- Do not introduce a generic workflow/stage engine. Scriptorium intentionally executes one prompt request.
|
|
||||||
- Do not build a plugin architecture for prompt/profile/schema/artifact backends before another backend exists.
|
|
||||||
- Do not replace explicit CLI parse functions with a broad command framework.
|
|
||||||
- Do not merge CLI and HTTP adapters. Their public interfaces and failure semantics differ.
|
|
||||||
- Do not create a generic reflection-based mapper for domain/DTO/request structs.
|
|
||||||
- Do not centralize prompt/profile normalization into one generic YAML repository; their validation and error semantics are different.
|
|
||||||
- Do not add schema caching or a manifest/state store as part of deduplication.
|
|
||||||
- Do not document or implement unsupported artifact backends as cleanup.
|
|
||||||
|
|
||||||
## Recommended Implementation Sequence
|
|
||||||
|
|
||||||
1. Execution-target mapping cleanup
|
|
||||||
- Goal: reduce drift when adding profile/runtime/provider fields.
|
|
||||||
- Files to update: `internal/usecase`, `internal/adapter/http`, `internal/llm`, targeted tests.
|
|
||||||
- Acceptance criteria: all existing field mapping tests pass; a new all-fields test fails if a mapped execution field is omitted.
|
|
||||||
- Suggested validation: `go test ./internal/usecase ./internal/adapter/http ./internal/llm ./internal/format`.
|
|
||||||
- Small enough for one implementation prompt: yes.
|
|
||||||
|
|
||||||
2. Prompt/profile filesystem catalog helper
|
|
||||||
- Goal: centralize recursive YAML discovery, path sorting, extension filtering, and relative path formatting.
|
|
||||||
- Files to update: prompt/profile repositories plus new narrow helper package.
|
|
||||||
- Acceptance criteria: existing repository tests pass unchanged; new helper tests cover nested discovery and stable ordering.
|
|
||||||
- Suggested validation: `go test ./internal/promptdef ./internal/profile`.
|
|
||||||
- Small enough for one implementation prompt: yes.
|
|
||||||
|
|
||||||
3. CLI wiring and config finalization helper
|
|
||||||
- Goal: centralize common command settings resolution and runner construction.
|
|
||||||
- Files to update: `internal/adapter/cli/run.go` and CLI tests.
|
|
||||||
- Acceptance criteria: no flag behavior changes; run/render/serve config precedence tests pass.
|
|
||||||
- Suggested validation: `go test ./internal/adapter/cli`.
|
|
||||||
- Small enough for one implementation prompt: yes.
|
|
||||||
|
|
||||||
4. Stable use-case invalid-request reasons
|
|
||||||
- Goal: remove HTTP dependency on runner error message substrings.
|
|
||||||
- Files to update: `internal/usecase/runner.go`, `internal/adapter/http/handler.go`, tests.
|
|
||||||
- Acceptance criteria: HTTP error codes/messages remain unchanged; runner exposes stable `errors.Is` reason errors.
|
|
||||||
- Suggested validation: `go test ./internal/usecase ./internal/adapter/http`.
|
|
||||||
- Small enough for one implementation prompt: yes.
|
|
||||||
|
|
||||||
5. Test helper cleanup
|
|
||||||
- Goal: reduce repeated fixture construction after behavior-preserving refactors are complete.
|
|
||||||
- Files to update: primarily `internal/adapter/cli/run_test.go`, optionally `internal/usecase/runner_test.go`.
|
|
||||||
- Acceptance criteria: test intent remains clear; no cross-package helper package unless strongly justified.
|
|
||||||
- Suggested validation: `go test ./internal/adapter/cli ./internal/usecase`.
|
|
||||||
- Small enough for one implementation prompt: yes.
|
|
||||||
|
|
||||||
6. Dead-code/legacy sweep
|
|
||||||
- Goal: remove or clearly mark unsupported placeholders such as `ArtifactRefS3`.
|
|
||||||
- Files to update: domain/artifact tests/docs only if needed.
|
|
||||||
- Acceptance criteria: docs continue to describe only implemented behavior outside roadmap.
|
|
||||||
- Suggested validation: `go test ./internal/domain ./internal/artifact`.
|
|
||||||
- Small enough for one implementation prompt: yes.
|
|
||||||
|
|
||||||
## Test Strategy
|
|
||||||
|
|
||||||
Tests to add before or during cleanup:
|
|
||||||
|
|
||||||
- Execution target all-fields mapping tests:
|
|
||||||
- runner profile/default/request merge;
|
|
||||||
- HTTP request DTO to domain target;
|
|
||||||
- HTTP domain result to metadata DTO;
|
|
||||||
- LLM domain target to outbound JSON payload.
|
|
||||||
- Repository catalog helper tests:
|
|
||||||
- recursive scan;
|
|
||||||
- `.yaml` and `.yml` filtering;
|
|
||||||
- stable sorted paths;
|
|
||||||
- relative clean path formatting;
|
|
||||||
- context cancellation if the helper preserves current behavior.
|
|
||||||
- HTTP error mapping tests:
|
|
||||||
- profile-required and API-key-env-missing should rely on sentinel errors, not message text.
|
|
||||||
- CLI wiring regression tests:
|
|
||||||
- `run`, `render`, and `serve` preserve config precedence and required directory checks.
|
|
||||||
- Schema behavior tests:
|
|
||||||
- `Prepare` fails before LLM generation when `json_schema` structured-output schema cannot be loaded.
|
|
||||||
|
|
||||||
Validation commands for cleanup work:
|
|
||||||
|
|
||||||
- `go test ./internal/usecase ./internal/adapter/http ./internal/llm ./internal/format`
|
|
||||||
- `go test ./internal/promptdef ./internal/profile`
|
|
||||||
- `go test ./internal/adapter/cli`
|
|
||||||
- `go test ./...` before merging broader cleanup.
|
|
||||||
|
|
||||||
No automated docs or link checker is currently present.
|
|
||||||
|
|
||||||
## Appendix: Findings Not Worth Acting On
|
|
||||||
|
|
||||||
- Repeated `select { case <-ctx.Done(): ... }` checks are acceptable. They are local, simple, and appear at IO/loop boundaries where behavior is easy to read.
|
|
||||||
- CLI and HTTP request validation should remain separate. Their external contracts differ, and centralizing all request validation would blur adapter responsibilities.
|
|
||||||
- Prompt and profile validation should not be merged. They both use YAML and IDs, but their schemas, normalization rules, and error policies differ.
|
|
||||||
- Prepared-run text formatting is verbose but intentionally presentation-specific. Avoid abstracting it until another formatter needs the same layout policy.
|
|
||||||
- Hashing appears in artifact loading and runner metadata, but not all hashes represent the same thing. A tiny body-hash helper may be useful later; a generic hashing subsystem is not justified.
|
|
||||||
- HTTP DTOs duplicate domain field names by design. They should remain transport-owned so JSON compatibility can evolve deliberately.
|
|
||||||
- Config `applyConfig` and `ApplyCLIOverrides` look similar, but they apply different source labels and validation contexts. A broad merge abstraction would likely reduce clarity.
|
|
||||||
@@ -1,504 +0,0 @@
|
|||||||
# Cleanup Implementation Roadmap
|
|
||||||
|
|
||||||
## Purpose
|
|
||||||
|
|
||||||
This roadmap turns the findings in [audit.md](audit.md) into a staged, decision-complete cleanup plan for Scriptorium.
|
|
||||||
|
|
||||||
Audience: LLM coding agents implementing the cleanup in order.
|
|
||||||
|
|
||||||
Controlling policies:
|
|
||||||
|
|
||||||
- [Documentation policy](../policy/documentation.md)
|
|
||||||
- [Architecture policy](../policy/architecture.md)
|
|
||||||
- [Development guide](../policy/development.md)
|
|
||||||
|
|
||||||
## Global Implementation Rules
|
|
||||||
|
|
||||||
- Implement stages in order.
|
|
||||||
- Keep public CLI flags, HTTP request/response shapes, config precedence, prompt/profile ID semantics, and validation behavior stable unless a stage explicitly says otherwise.
|
|
||||||
- Keep adapters thin and use-case policy in `internal/usecase`.
|
|
||||||
- Prefer explicit helpers over reflection, generic workflow abstractions, or broad framework-style rewrites.
|
|
||||||
- Update non-roadmap docs only when implemented behavior changes or when a stale implemented-behavior statement is found during a stage.
|
|
||||||
- Do not document future behavior outside `docs/roadmap/`.
|
|
||||||
- Run the stage-specific tests before moving to the next stage.
|
|
||||||
- Run `go test ./...` after the final stage.
|
|
||||||
|
|
||||||
## Stage 1: Execution Target Mapping Cleanup
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Reduce drift when adding or changing execution/runtime fields such as `service_tier`, `api_key_env`, `reasoning_effort`, or future provider request keys.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
Update only explicit execution-target mapping and serialization paths. Do not add new CLI flags or new provider features.
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
In `internal/usecase`:
|
|
||||||
|
|
||||||
- Keep execution-target merge policy in `internal/usecase`.
|
|
||||||
- Add focused helper coverage around `resolveExecutionTarget`, `mergeExecutionTarget`, and profile-to-target conversion.
|
|
||||||
- Keep the existing semantics:
|
|
||||||
- built-in defaults first;
|
|
||||||
- profile values override defaults;
|
|
||||||
- request overrides override profile values;
|
|
||||||
- zero numeric values do not override;
|
|
||||||
- empty/whitespace string values do not override;
|
|
||||||
- non-empty `ExtraParams` replaces the previous map with a copy.
|
|
||||||
|
|
||||||
In `internal/adapter/http`:
|
|
||||||
|
|
||||||
- Add local helper functions:
|
|
||||||
- `executionTargetFromModelOverrideDTO(*modelOverrideRequestDTO) *domain.ExecutionTarget`
|
|
||||||
- `modelParamsDTOFromExecutionTarget(domain.ExecutionTarget) modelParamsDTO`
|
|
||||||
- Use those helpers in `handler.go`.
|
|
||||||
- Keep DTO types unexported and transport-owned.
|
|
||||||
- Keep HTTP response field names and omission behavior unchanged.
|
|
||||||
|
|
||||||
In `internal/llm`:
|
|
||||||
|
|
||||||
- Add a local helper such as `openAIChatRequestFromGenerateRequest(req domain.GenerateRequest, defaultModel string) (openAIChatRequest, error)` or an equivalent small function.
|
|
||||||
- Keep endpoint construction, HTTP client timeout handling, API-key environment lookup, and response parsing in `Generate`.
|
|
||||||
- Keep outbound serialization behavior unchanged:
|
|
||||||
- send `model` and `messages`;
|
|
||||||
- send `temperature`, `max_tokens`, `top_p`, and `service_tier` only when currently sent;
|
|
||||||
- send `response_format` only when structured output is present;
|
|
||||||
- do not serialize `reasoning_effort` or `extra_params`.
|
|
||||||
|
|
||||||
Do not:
|
|
||||||
|
|
||||||
- use reflection to copy fields;
|
|
||||||
- move HTTP DTOs into `internal/domain`;
|
|
||||||
- add generic mapper packages;
|
|
||||||
- change prepared-run JSON tags.
|
|
||||||
|
|
||||||
### Tests
|
|
||||||
|
|
||||||
Add or update tests so an all-fields `domain.ExecutionTarget` catches omissions.
|
|
||||||
|
|
||||||
Required tests:
|
|
||||||
|
|
||||||
- Runner merge/profile conversion:
|
|
||||||
- profile values populate all supported execution fields;
|
|
||||||
- runtime overrides beat profile values for all overrideable fields;
|
|
||||||
- empty string overrides do not erase profile values;
|
|
||||||
- empty `ExtraParams` does not erase profile values.
|
|
||||||
- HTTP adapter:
|
|
||||||
- request `model` object maps every supported field into `RunRequest.Execution`;
|
|
||||||
- response `metadata.model_params` includes every supported field according to current DTO tags.
|
|
||||||
- LLM client:
|
|
||||||
- outbound JSON includes every serialized execution field;
|
|
||||||
- outbound JSON omits `service_tier` when empty;
|
|
||||||
- outbound JSON still omits `reasoning_effort` and `extra_params`.
|
|
||||||
|
|
||||||
### Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./internal/usecase ./internal/adapter/http ./internal/llm ./internal/format
|
|
||||||
```
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- No public behavior changes.
|
|
||||||
- Adding a new execution target field later has obvious mapping/test locations.
|
|
||||||
- Existing HTTP and LLM behavior remains stable.
|
|
||||||
- Stage is small enough for one implementation prompt.
|
|
||||||
|
|
||||||
## Stage 2: Prompt/Profile Filesystem Catalog Helper
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Centralize shared recursive YAML discovery mechanics while preserving prompt/profile-specific validation and error behavior.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
Create a narrow helper package for filesystem catalog mechanics only.
|
|
||||||
|
|
||||||
Recommended package:
|
|
||||||
|
|
||||||
- `internal/filecatalog`
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
Add helper functions with explicit, small responsibilities:
|
|
||||||
|
|
||||||
- recursively find YAML files under a root directory;
|
|
||||||
- honor context cancellation during walking;
|
|
||||||
- accept `.yaml` and `.yml`;
|
|
||||||
- return stable sorted full paths;
|
|
||||||
- compute clean relative paths from a root;
|
|
||||||
- return filename stems with `.yaml`/`.yml` stripped.
|
|
||||||
|
|
||||||
Use the helper in:
|
|
||||||
|
|
||||||
- `internal/promptdef/filesystem_repository.go`
|
|
||||||
- `internal/profile/filesystem_repository.go`
|
|
||||||
|
|
||||||
Preserve existing behavior:
|
|
||||||
|
|
||||||
- prompt/profile lookup uses YAML `id`, not file path;
|
|
||||||
- subdirectories are organizational only;
|
|
||||||
- duplicate prompt/profile IDs are invalid;
|
|
||||||
- malformed likely-target files still surface errors;
|
|
||||||
- relative nested paths still appear in errors;
|
|
||||||
- prompt `content_file` resolution remains relative to the prompt YAML file;
|
|
||||||
- prompt/profile strict YAML and validation stay in their existing packages.
|
|
||||||
|
|
||||||
Do not:
|
|
||||||
|
|
||||||
- create a generic repository framework;
|
|
||||||
- merge prompt and profile normalization;
|
|
||||||
- move prompt/profile domain policy into the helper;
|
|
||||||
- change error messages except for unavoidable wording caused by helper extraction.
|
|
||||||
|
|
||||||
### Tests
|
|
||||||
|
|
||||||
Add tests for `internal/filecatalog`:
|
|
||||||
|
|
||||||
- nested YAML discovery;
|
|
||||||
- `.yaml` and `.yml` accepted;
|
|
||||||
- non-YAML files ignored;
|
|
||||||
- returned paths sorted deterministically;
|
|
||||||
- relative path formatting works for nested files;
|
|
||||||
- filename stem stripping handles both extensions.
|
|
||||||
|
|
||||||
Keep existing prompt/profile repository tests passing.
|
|
||||||
|
|
||||||
### Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./internal/filecatalog ./internal/promptdef ./internal/profile
|
|
||||||
```
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- Prompt/profile repository tests pass without behavior expectation changes.
|
|
||||||
- Shared filesystem scanning logic exists in one place.
|
|
||||||
- Prompt/profile packages still own their own validation and normalization.
|
|
||||||
- Stage is small enough for one implementation prompt.
|
|
||||||
|
|
||||||
## Stage 3: CLI Wiring And Settings Finalization Cleanup
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Reduce duplicated command setup while preserving each command's public flag surface and behavior.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
Clean up `internal/adapter/cli` only, except for tests.
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
Add CLI-local helpers. Recommended helpers:
|
|
||||||
|
|
||||||
- `commonCommandSettings` or similar struct containing resolved `promptDir`, `profileDir`, `schemaDir`, `serverAddr`, and `defaultRenderFormat` where applicable.
|
|
||||||
- `resolveCommonSettings(fs *flag.FlagSet, configPath string, overrides appconfig.CLIOverrides) (commonCommandSettings, error)`.
|
|
||||||
- `validateRequiredLibraryDirs(promptDir, profileDir string) error`.
|
|
||||||
- `newRunner(promptDir, profileDir, schemaDir string, llmClient llm.Client) *usecase.Runner`.
|
|
||||||
- Optionally `newOpenAIClient() (*llm.OpenAICompatibleClient, error)` if it removes exact duplication without obscuring command behavior.
|
|
||||||
|
|
||||||
Preserve command differences:
|
|
||||||
|
|
||||||
- `run` exposes runtime model override flags and `--schema-dir`;
|
|
||||||
- `render` exposes runtime model override flags and `--format`, but not `--schema-dir`;
|
|
||||||
- `serve` exposes `--addr` and `--schema-dir`, but no runtime model override flags;
|
|
||||||
- `render` default format comes from `defaults.render_format` unless `--format` is set;
|
|
||||||
- deprecated `--prompt-id` and `--profile-id` aliases remain accepted.
|
|
||||||
|
|
||||||
Keep existing parse functions:
|
|
||||||
|
|
||||||
- `parseRunArgs`
|
|
||||||
- `parseRenderArgs`
|
|
||||||
- `parseServeArgs`
|
|
||||||
|
|
||||||
Do not:
|
|
||||||
|
|
||||||
- replace the standard library `flag` package;
|
|
||||||
- introduce a command framework;
|
|
||||||
- make `serve` accept runtime model override flags;
|
|
||||||
- change error prefixes such as `run parse error`, `render parse error`, or `serve parse error`;
|
|
||||||
- change CLI output behavior.
|
|
||||||
|
|
||||||
### Tests
|
|
||||||
|
|
||||||
Required regression tests:
|
|
||||||
|
|
||||||
- `run`, `render`, and `serve` still apply config precedence correctly.
|
|
||||||
- Missing effective `prompt_dir` and `profile_dir` still return the same guidance.
|
|
||||||
- `render` still uses config default render format and explicit `--format` override.
|
|
||||||
- `serve` still rejects runtime model override flags.
|
|
||||||
- `run` and `render` still build equivalent runtime override requests for shared flags.
|
|
||||||
|
|
||||||
### Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./internal/adapter/cli
|
|
||||||
```
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- No CLI flag, output, exit-code, or precedence changes.
|
|
||||||
- Runner dependency construction is centralized inside the CLI adapter.
|
|
||||||
- Command-specific behavior remains easy to read.
|
|
||||||
- Stage is small enough for one implementation prompt.
|
|
||||||
|
|
||||||
## Stage 4: Stable Use-Case Error Reasons For HTTP Mapping
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Remove HTTP error mapping's dependency on runner error message substrings.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
Change error identity, not public HTTP error responses.
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
In `internal/usecase`:
|
|
||||||
|
|
||||||
- Add stable sentinel errors for invalid-request reasons that HTTP currently distinguishes by message text.
|
|
||||||
- Required sentinels:
|
|
||||||
- missing profile selection, for the case where neither request profile nor prompt `default_profile` is available;
|
|
||||||
- missing API-key environment value, for the case where `api_key_env` is set but the named environment variable is unset or empty.
|
|
||||||
- Wrap these sentinels with `ErrInvalidRequest` so existing broad invalid-request checks keep working.
|
|
||||||
- Preserve clear human-readable runner errors.
|
|
||||||
|
|
||||||
In `internal/adapter/http`:
|
|
||||||
|
|
||||||
- Replace `strings.Contains(err.Error(), ...)` checks for these cases with `errors.Is`.
|
|
||||||
- Keep current HTTP status codes, error codes, and response messages:
|
|
||||||
- `400 profile_required`;
|
|
||||||
- `400 api_key_env_missing`.
|
|
||||||
|
|
||||||
Do not:
|
|
||||||
|
|
||||||
- expose HTTP-specific error codes from `internal/usecase`;
|
|
||||||
- change the HTTP JSON error body shape;
|
|
||||||
- remove broad fallback handling for `usecase.ErrInvalidRequest`.
|
|
||||||
|
|
||||||
### Tests
|
|
||||||
|
|
||||||
Required tests:
|
|
||||||
|
|
||||||
- Runner tests assert `errors.Is(err, usecase.ErrProfileRequired)` or the chosen sentinel name for missing profile selection.
|
|
||||||
- Runner tests assert `errors.Is(err, usecase.ErrAPIKeyEnvMissing)` or the chosen sentinel name for missing API-key environment value.
|
|
||||||
- HTTP handler tests still assert unchanged status/code/message for both cases.
|
|
||||||
- HTTP handler tests should not construct errors by relying on exact runner prose for these two cases.
|
|
||||||
|
|
||||||
### Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./internal/usecase ./internal/adapter/http
|
|
||||||
```
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- HTTP mapping no longer depends on runner message substrings for the two distinguished invalid-request cases.
|
|
||||||
- Public HTTP behavior is unchanged.
|
|
||||||
- Runner errors remain clear in CLI output.
|
|
||||||
- Stage is small enough for one implementation prompt.
|
|
||||||
|
|
||||||
## Stage 5: Schema Failure Regression Coverage
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Protect the current structured-output invariant before future schema cleanup: `Prepare` must load a `json_schema` document before any LLM call.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
Add regression coverage only. Do not add schema caching or change schema loading architecture in this stage.
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
In `internal/usecase/runner_test.go` or an appropriate package test:
|
|
||||||
|
|
||||||
- Add a test where a prompt uses `validation_mode: json_schema` with a missing or failing schema document.
|
|
||||||
- Assert `Runner.Prepare` fails with `ErrValidation`.
|
|
||||||
- Assert no LLM call is made for `Runner.Run` when structured-output schema loading fails.
|
|
||||||
|
|
||||||
If existing tests already cover part of this behavior, consolidate assertions without making the test suite harder to read.
|
|
||||||
|
|
||||||
Do not:
|
|
||||||
|
|
||||||
- cache compiled schemas;
|
|
||||||
- change `validate.StandardValidator` behavior;
|
|
||||||
- introduce a schema service abstraction.
|
|
||||||
|
|
||||||
### Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./internal/usecase ./internal/validate
|
|
||||||
```
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- Missing structured-output schema fails before generation.
|
|
||||||
- Existing JSON Schema validation behavior is unchanged.
|
|
||||||
- Stage is small enough for one implementation prompt.
|
|
||||||
|
|
||||||
## Stage 6: Test Fixture Cleanup
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Reduce repeated test setup after behavior-preserving production refactors are complete.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
Prefer package-local test helpers. Avoid cross-package test utility packages unless a helper is needed by more than two packages and represents a stable public fixture contract.
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
In `internal/adapter/cli/run_test.go`:
|
|
||||||
|
|
||||||
- Consolidate repeated temp prompt/profile/input setup into local helper functions.
|
|
||||||
- Keep helper names behavior-focused, for example:
|
|
||||||
- `newCLITestLibrary`
|
|
||||||
- `writePromptFileWithDefaultProfile`
|
|
||||||
- `writeProfileFile`
|
|
||||||
- `runCLICommand`
|
|
||||||
- Do not hide assertions inside helpers unless the assertion is truly setup validation.
|
|
||||||
|
|
||||||
In `internal/usecase/runner_test.go`:
|
|
||||||
|
|
||||||
- Keep existing fake interfaces package-local.
|
|
||||||
- Remove only high-volume duplication that obscures test intent.
|
|
||||||
|
|
||||||
Do not:
|
|
||||||
|
|
||||||
- move package-private fake types into production code;
|
|
||||||
- create a broad `internal/testutil` package unless a later cleanup stage proves it necessary;
|
|
||||||
- rewrite tests into table-driven form when cases have meaningfully different setup.
|
|
||||||
|
|
||||||
### Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./internal/adapter/cli ./internal/usecase
|
|
||||||
```
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- Test intent is at least as clear as before.
|
|
||||||
- No production behavior changes.
|
|
||||||
- Test fixture setup has less repeated boilerplate in CLI tests.
|
|
||||||
- Stage is small enough for one implementation prompt.
|
|
||||||
|
|
||||||
## Stage 7: Unsupported Placeholder Sweep
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Remove code that suggests unimplemented artifact behavior outside roadmap documentation.
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
Remove unsupported placeholders only when they are not needed by current tests or public docs.
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
Remove `domain.ArtifactRefS3` from `internal/domain/domain.go` unless new evidence shows it is intentionally needed by implemented code.
|
|
||||||
|
|
||||||
Preserve current behavior:
|
|
||||||
|
|
||||||
- supported artifact reference types remain `inline` and `file`;
|
|
||||||
- unsupported artifact reference types still return `artifact.ErrUnsupportedRefType`;
|
|
||||||
- docs continue to describe only `inline` and `file` outside roadmap files.
|
|
||||||
|
|
||||||
Update tests only if they reference the removed constant. Prefer testing unsupported artifact behavior with a literal custom type such as `domain.ArtifactRefType("s3")` or `domain.ArtifactRefType("unsupported")`.
|
|
||||||
|
|
||||||
Do not:
|
|
||||||
|
|
||||||
- add S3 support;
|
|
||||||
- document S3 as implemented;
|
|
||||||
- add future-backend placeholders elsewhere.
|
|
||||||
|
|
||||||
### Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./internal/domain ./internal/artifact
|
|
||||||
```
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- Unsupported placeholder constant is removed.
|
|
||||||
- Unsupported artifact-type behavior remains covered.
|
|
||||||
- No non-roadmap doc claims unimplemented artifact support.
|
|
||||||
- Stage is small enough for one implementation prompt.
|
|
||||||
|
|
||||||
## Stage 8: Final Verification And Documentation Alignment
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Confirm the cleanup sequence preserved behavior and documentation accuracy.
|
|
||||||
|
|
||||||
### Implementation
|
|
||||||
|
|
||||||
Run the full test suite:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go test ./...
|
|
||||||
```
|
|
||||||
|
|
||||||
Run the maintained render smoke command:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
go run ./cmd/scriptorium render --config ./examples/config.yml --prompt generic.markdown_summary --input transcript=./examples/fixtures/transcript.md --input glossary=./examples/fixtures/glossary.yml --format json
|
|
||||||
```
|
|
||||||
|
|
||||||
Search for stale or unsupported terms:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
rg -n "ArtifactRefS3|s3|strings\\.Contains\\(err\\.Error\\(\\)|TODO|future|planned" internal docs README.md examples
|
|
||||||
```
|
|
||||||
|
|
||||||
Review results manually:
|
|
||||||
|
|
||||||
- `s3` should not appear as an implemented artifact type.
|
|
||||||
- `strings.Contains(err.Error())` should not be used for stable use-case reason mapping.
|
|
||||||
- Any `future` or `planned` wording outside `docs/roadmap/` must describe current boundaries, not aspirational behavior.
|
|
||||||
|
|
||||||
Update docs only if cleanup changed implemented behavior or if the search reveals stale implemented-behavior docs.
|
|
||||||
|
|
||||||
### Acceptance Criteria
|
|
||||||
|
|
||||||
- Full test suite passes.
|
|
||||||
- Maintained render smoke command succeeds.
|
|
||||||
- No stale unsupported artifact placeholder remains.
|
|
||||||
- Non-roadmap docs describe implemented behavior only.
|
|
||||||
- Working tree contains only intentional cleanup changes.
|
|
||||||
|
|
||||||
## Deferred Work
|
|
||||||
|
|
||||||
Do not implement these during the staged cleanup unless a later audit makes them high-confidence:
|
|
||||||
|
|
||||||
- schema caching or a combined raw/compiled schema service;
|
|
||||||
- artifact hash/build helper beyond a small helper introduced opportunistically during touched code;
|
|
||||||
- generic YAML repository framework;
|
|
||||||
- generic CLI command framework;
|
|
||||||
- plugin architecture for future prompt/profile/schema/artifact backends;
|
|
||||||
- durable state, manifests, checkpoints, or resume behavior.
|
|
||||||
|
|
||||||
## Completion Criteria
|
|
||||||
|
|
||||||
The cleanup roadmap is complete when all stages have been implemented in order, the final verification passes, and the resulting code still satisfies:
|
|
||||||
|
|
||||||
- `Runner.Run` reuses `Runner.Prepare`;
|
|
||||||
- CLI and HTTP adapters instantiate `Runner` without a repairer;
|
|
||||||
- unknown config/prompt/profile YAML and HTTP JSON fields are rejected;
|
|
||||||
- raw API key values are not accepted or emitted;
|
|
||||||
- prompt/profile subdirectories remain organizational only;
|
|
||||||
- schema paths remain explicit and relative to `schema_dir` when not absolute;
|
|
||||||
- public CLI and HTTP behavior remains stable.
|
|
||||||
@@ -81,6 +81,14 @@ type serveConfig struct {
|
|||||||
schemaDir string
|
schemaDir string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type commonCommandSettings struct {
|
||||||
|
promptDir string
|
||||||
|
profileDir string
|
||||||
|
schemaDir string
|
||||||
|
serverAddr string
|
||||||
|
defaultRenderFormat renderformat.PreparedRunOutputFormat
|
||||||
|
}
|
||||||
|
|
||||||
type listFlag []string
|
type listFlag []string
|
||||||
|
|
||||||
func (l *listFlag) String() string {
|
func (l *listFlag) String() string {
|
||||||
@@ -125,22 +133,13 @@ func runCommand(args []string, stdout, stderr io.Writer) int {
|
|||||||
return ExitRuntimeError
|
return ExitRuntimeError
|
||||||
}
|
}
|
||||||
|
|
||||||
llmClient, err := llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{
|
llmClient, err := newOpenAIClient()
|
||||||
Timeout: defaults.LLMRequestTimeoutDefault,
|
|
||||||
})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(stderr, "llm client error: %v\n", err)
|
fmt.Fprintf(stderr, "llm client error: %v\n", err)
|
||||||
return ExitRuntimeError
|
return ExitRuntimeError
|
||||||
}
|
}
|
||||||
|
|
||||||
runner := usecase.NewRunner(
|
runner := newRunner(cfg.promptDir, cfg.profileDir, cfg.schemaDir, llmClient)
|
||||||
promptdef.NewFilesystemRepository(cfg.promptDir),
|
|
||||||
profile.NewFilesystemRepository(cfg.profileDir),
|
|
||||||
artifactadapter.NewCompositeReader(),
|
|
||||||
prompt.NewGoRenderer(),
|
|
||||||
llmClient,
|
|
||||||
validate.NewStandardValidator(cfg.schemaDir),
|
|
||||||
)
|
|
||||||
|
|
||||||
res, runErr := runner.Run(context.Background(), req)
|
res, runErr := runner.Run(context.Background(), req)
|
||||||
if runErr != nil {
|
if runErr != nil {
|
||||||
@@ -170,14 +169,7 @@ func renderCommand(args []string, stdout, stderr io.Writer) int {
|
|||||||
return ExitRuntimeError
|
return ExitRuntimeError
|
||||||
}
|
}
|
||||||
|
|
||||||
runner := usecase.NewRunner(
|
runner := newRunner(cfg.promptDir, cfg.profileDir, cfg.schemaDir, nil)
|
||||||
promptdef.NewFilesystemRepository(cfg.promptDir),
|
|
||||||
profile.NewFilesystemRepository(cfg.profileDir),
|
|
||||||
artifactadapter.NewCompositeReader(),
|
|
||||||
prompt.NewGoRenderer(),
|
|
||||||
nil,
|
|
||||||
validate.NewStandardValidator(cfg.schemaDir),
|
|
||||||
)
|
|
||||||
|
|
||||||
prepared, prepErr := runner.Prepare(context.Background(), req)
|
prepared, prepErr := runner.Prepare(context.Background(), req)
|
||||||
if prepErr != nil {
|
if prepErr != nil {
|
||||||
@@ -205,22 +197,13 @@ func serveCommand(args []string, stderr io.Writer) int {
|
|||||||
return ExitRuntimeError
|
return ExitRuntimeError
|
||||||
}
|
}
|
||||||
|
|
||||||
llmClient, err := llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{
|
llmClient, err := newOpenAIClient()
|
||||||
Timeout: defaults.LLMRequestTimeoutDefault,
|
|
||||||
})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(stderr, "llm client error: %v\n", err)
|
fmt.Fprintf(stderr, "llm client error: %v\n", err)
|
||||||
return ExitRuntimeError
|
return ExitRuntimeError
|
||||||
}
|
}
|
||||||
|
|
||||||
runner := usecase.NewRunner(
|
runner := newRunner(cfg.promptDir, cfg.profileDir, cfg.schemaDir, llmClient)
|
||||||
promptdef.NewFilesystemRepository(cfg.promptDir),
|
|
||||||
profile.NewFilesystemRepository(cfg.profileDir),
|
|
||||||
artifactadapter.NewCompositeReader(),
|
|
||||||
prompt.NewGoRenderer(),
|
|
||||||
llmClient,
|
|
||||||
validate.NewStandardValidator(cfg.schemaDir),
|
|
||||||
)
|
|
||||||
|
|
||||||
h := httpadapter.NewHandler(runner)
|
h := httpadapter.NewHandler(runner)
|
||||||
srv := &http.Server{
|
srv := &http.Server{
|
||||||
@@ -307,7 +290,7 @@ func parseServeArgs(args []string) (*serveConfig, error) {
|
|||||||
return nil, fmt.Errorf("unexpected positional args: %v", fs.Args())
|
return nil, fmt.Errorf("unexpected positional args: %v", fs.Args())
|
||||||
}
|
}
|
||||||
|
|
||||||
settings, err := resolveAppSettings(fs, cfg.configPath, appconfig.CLIOverrides{
|
settings, err := resolveCommonSettings(fs, cfg.configPath, appconfig.CLIOverrides{
|
||||||
PromptDir: cfg.promptDirIfSet(fs),
|
PromptDir: cfg.promptDirIfSet(fs),
|
||||||
ProfileDir: cfg.profileDirIfSet(fs),
|
ProfileDir: cfg.profileDirIfSet(fs),
|
||||||
SchemaDir: cfg.schemaDirIfSet(fs),
|
SchemaDir: cfg.schemaDirIfSet(fs),
|
||||||
@@ -317,16 +300,13 @@ func parseServeArgs(args []string) (*serveConfig, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
cfg.promptDir = settings.PromptDir
|
cfg.promptDir = settings.promptDir
|
||||||
cfg.profileDir = settings.ProfileDir
|
cfg.profileDir = settings.profileDir
|
||||||
cfg.schemaDir = settings.SchemaDir
|
cfg.schemaDir = settings.schemaDir
|
||||||
cfg.addr = settings.ServerAddr
|
cfg.addr = settings.serverAddr
|
||||||
|
|
||||||
if strings.TrimSpace(cfg.promptDir) == "" {
|
if err := validateRequiredLibraryDirs(cfg.promptDir, cfg.profileDir); err != nil {
|
||||||
return nil, errors.New(errPromptDirRequired)
|
return nil, err
|
||||||
}
|
|
||||||
if strings.TrimSpace(cfg.profileDir) == "" {
|
|
||||||
return nil, errors.New(errProfileDirRequired)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
cfg.promptDir = filepath.Clean(cfg.promptDir)
|
cfg.promptDir = filepath.Clean(cfg.promptDir)
|
||||||
@@ -359,7 +339,7 @@ func finalizeExecutionRequestConfig(fs *flag.FlagSet, cfg *runConfig) error {
|
|||||||
return fmt.Errorf("unexpected positional args: %v", fs.Args())
|
return fmt.Errorf("unexpected positional args: %v", fs.Args())
|
||||||
}
|
}
|
||||||
|
|
||||||
settings, err := resolveAppSettings(fs, cfg.configPath, appconfig.CLIOverrides{
|
settings, err := resolveCommonSettings(fs, cfg.configPath, appconfig.CLIOverrides{
|
||||||
PromptDir: cfg.promptDirIfSet(fs),
|
PromptDir: cfg.promptDirIfSet(fs),
|
||||||
ProfileDir: cfg.profileDirIfSet(fs),
|
ProfileDir: cfg.profileDirIfSet(fs),
|
||||||
SchemaDir: cfg.schemaDirIfSet(fs),
|
SchemaDir: cfg.schemaDirIfSet(fs),
|
||||||
@@ -368,16 +348,13 @@ func finalizeExecutionRequestConfig(fs *flag.FlagSet, cfg *runConfig) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
cfg.promptDir = settings.PromptDir
|
cfg.promptDir = settings.promptDir
|
||||||
cfg.profileDir = settings.ProfileDir
|
cfg.profileDir = settings.profileDir
|
||||||
cfg.schemaDir = settings.SchemaDir
|
cfg.schemaDir = settings.schemaDir
|
||||||
cfg.defaultRenderFormat = settings.DefaultRenderFormat
|
cfg.defaultRenderFormat = settings.defaultRenderFormat
|
||||||
|
|
||||||
if strings.TrimSpace(cfg.promptDir) == "" {
|
if err := validateRequiredLibraryDirs(cfg.promptDir, cfg.profileDir); err != nil {
|
||||||
return errors.New(errPromptDirRequired)
|
return err
|
||||||
}
|
|
||||||
if strings.TrimSpace(cfg.profileDir) == "" {
|
|
||||||
return errors.New(errProfileDirRequired)
|
|
||||||
}
|
}
|
||||||
if strings.TrimSpace(cfg.promptID) == "" {
|
if strings.TrimSpace(cfg.promptID) == "" {
|
||||||
return errors.New("--prompt is required")
|
return errors.New("--prompt is required")
|
||||||
@@ -476,6 +453,47 @@ func resolveAppSettings(fs *flag.FlagSet, configPath string, overrides appconfig
|
|||||||
return merged, nil
|
return merged, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func resolveCommonSettings(fs *flag.FlagSet, configPath string, overrides appconfig.CLIOverrides) (commonCommandSettings, error) {
|
||||||
|
settings, err := resolveAppSettings(fs, configPath, overrides)
|
||||||
|
if err != nil {
|
||||||
|
return commonCommandSettings{}, err
|
||||||
|
}
|
||||||
|
return commonCommandSettings{
|
||||||
|
promptDir: settings.PromptDir,
|
||||||
|
profileDir: settings.ProfileDir,
|
||||||
|
schemaDir: settings.SchemaDir,
|
||||||
|
serverAddr: settings.ServerAddr,
|
||||||
|
defaultRenderFormat: settings.DefaultRenderFormat,
|
||||||
|
}, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func validateRequiredLibraryDirs(promptDir, profileDir string) error {
|
||||||
|
if strings.TrimSpace(promptDir) == "" {
|
||||||
|
return errors.New(errPromptDirRequired)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(profileDir) == "" {
|
||||||
|
return errors.New(errProfileDirRequired)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func newRunner(promptDir, profileDir, schemaDir string, llmClient llm.Client) *usecase.Runner {
|
||||||
|
return usecase.NewRunner(
|
||||||
|
promptdef.NewFilesystemRepository(promptDir),
|
||||||
|
profile.NewFilesystemRepository(profileDir),
|
||||||
|
artifactadapter.NewCompositeReader(),
|
||||||
|
prompt.NewGoRenderer(),
|
||||||
|
llmClient,
|
||||||
|
validate.NewStandardValidator(schemaDir),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
func newOpenAIClient() (*llm.OpenAICompatibleClient, error) {
|
||||||
|
return llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{
|
||||||
|
Timeout: defaults.LLMRequestTimeoutDefault,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func buildRunRequestFromConfig(cfg *runConfig) (domain.RunRequest, error) {
|
func buildRunRequestFromConfig(cfg *runConfig) (domain.RunRequest, error) {
|
||||||
inputMappings, err := parseMappings(cfg.inputRaw, false)
|
inputMappings, err := parseMappings(cfg.inputRaw, false)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import (
|
|||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"reflect"
|
||||||
"strings"
|
"strings"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -410,6 +411,28 @@ defaults:
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestParseRenderArgsExplicitFormatOverridesConfigDefaultFormat(t *testing.T) {
|
||||||
|
configPath := writeAppConfigFile(t, `
|
||||||
|
prompt_dir: ./from-config/prompts
|
||||||
|
profile_dir: ./from-config/profiles
|
||||||
|
defaults:
|
||||||
|
render_format: json
|
||||||
|
`)
|
||||||
|
|
||||||
|
cfg, err := parseRenderArgs([]string{
|
||||||
|
"--config", configPath,
|
||||||
|
"--prompt", "p",
|
||||||
|
"--input", "a=b",
|
||||||
|
"--format", "text",
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected valid args, got %v", err)
|
||||||
|
}
|
||||||
|
if cfg.outputFormat != renderformat.PreparedRunFormatText {
|
||||||
|
t.Fatalf("expected explicit --format text to override config default, got %q", cfg.outputFormat)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestParseServeArgsWithExplicitConfigLoadsSettingsAndCLIAddrOverrides(t *testing.T) {
|
func TestParseServeArgsWithExplicitConfigLoadsSettingsAndCLIAddrOverrides(t *testing.T) {
|
||||||
configPath := writeAppConfigFile(t, `
|
configPath := writeAppConfigFile(t, `
|
||||||
prompt_dir: ./from-config/prompts
|
prompt_dir: ./from-config/prompts
|
||||||
@@ -471,6 +494,59 @@ server:
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunAndRenderBuildEquivalentRuntimeOverrideRequestsForSharedFlags(t *testing.T) {
|
||||||
|
runCfg, err := parseRunArgs([]string{
|
||||||
|
"--prompt-dir", "./prompts",
|
||||||
|
"--profile-dir", "./profiles",
|
||||||
|
"--prompt", "prompt-1",
|
||||||
|
"--profile", "profile-1",
|
||||||
|
"--input", "transcript=./transcript.md",
|
||||||
|
"--var", "session_date=2026-05-01",
|
||||||
|
"--llm-base-url", "http://localhost:8000/v1",
|
||||||
|
"--model", "model-x",
|
||||||
|
"--temperature", "0.8",
|
||||||
|
"--max-tokens", "123",
|
||||||
|
"--top-p", "0.6",
|
||||||
|
"--timeout", "90s",
|
||||||
|
"--api-key-env", "SCRIPTORIUM_API_KEY",
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected valid run args, got %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
renderCfg, err := parseRenderArgs([]string{
|
||||||
|
"--prompt-dir", "./prompts",
|
||||||
|
"--profile-dir", "./profiles",
|
||||||
|
"--prompt", "prompt-1",
|
||||||
|
"--profile", "profile-1",
|
||||||
|
"--input", "transcript=./transcript.md",
|
||||||
|
"--var", "session_date=2026-05-01",
|
||||||
|
"--llm-base-url", "http://localhost:8000/v1",
|
||||||
|
"--model", "model-x",
|
||||||
|
"--temperature", "0.8",
|
||||||
|
"--max-tokens", "123",
|
||||||
|
"--top-p", "0.6",
|
||||||
|
"--timeout", "90s",
|
||||||
|
"--api-key-env", "SCRIPTORIUM_API_KEY",
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected valid render args, got %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
runReq, err := buildRunRequestFromConfig(runCfg)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected run request build success, got %v", err)
|
||||||
|
}
|
||||||
|
renderReq, err := buildRunRequestFromConfig(&renderCfg.runConfig)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected render request build success, got %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !reflect.DeepEqual(runReq, renderReq) {
|
||||||
|
t.Fatalf("expected run/render shared flag requests to match.\nrun=%#v\nrender=%#v", runReq, renderReq)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestParseRunArgsFailsClearlyWhenNoEffectivePromptDir(t *testing.T) {
|
func TestParseRunArgsFailsClearlyWhenNoEffectivePromptDir(t *testing.T) {
|
||||||
configPath := writeAppConfigFile(t, `
|
configPath := writeAppConfigFile(t, `
|
||||||
profile_dir: ./profiles
|
profile_dir: ./profiles
|
||||||
@@ -586,42 +662,29 @@ func TestRunCommandVarsOptional(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestRunCommandSucceedsWithPromptAndProfileDirsFromConfig(t *testing.T) {
|
func TestRunCommandSucceedsWithPromptAndProfileDirsFromConfig(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
ts := newTestLLMServer("from-config-dirs", nil)
|
ts := newTestLLMServer("from-config-dirs", nil)
|
||||||
defer ts.Close()
|
defer ts.Close()
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.default", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.default", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", ts.URL+"/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", ts.URL+"/v1", "profile-model")
|
||||||
configPath := writeAppConfigFile(t, fmt.Sprintf(`
|
configPath := writeAppConfigFile(t, fmt.Sprintf(`
|
||||||
prompt_dir: %s
|
prompt_dir: %s
|
||||||
profile_dir: %s
|
profile_dir: %s
|
||||||
`, promptDir, profileDir))
|
`, lib.promptDir, lib.profileDir))
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, runCommand, []string{
|
||||||
var stderr bytes.Buffer
|
|
||||||
code := runCommand([]string{
|
|
||||||
"--config", configPath,
|
"--config", configPath,
|
||||||
"--prompt", "prompt.default",
|
"--prompt", "prompt.default",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if stdout.String() != "from-config-dirs" {
|
if stdout != "from-config-dirs" {
|
||||||
t.Fatalf("unexpected stdout output: %q", stdout.String())
|
t.Fatalf("unexpected stdout output: %q", stdout)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -630,28 +693,15 @@ func TestRenderCommandDefaultFormatTextIncludesPreparedDetailsAndNoSecrets(t *te
|
|||||||
const secret = "super-secret-render-key"
|
const secret = "super-secret-render-key"
|
||||||
t.Setenv(envName, secret)
|
t.Setenv(envName, secret)
|
||||||
|
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello transcript")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello transcript"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
writePromptFileWithTemplate(t, promptDir, "prompt.render", "local-default", "Date {{.session_date}} - Summarize: {{input \"transcript\"}}")
|
writePromptFileWithTemplate(t, lib.promptDir, "prompt.render", "local-default", "Date {{.session_date}} - Summarize: {{input \"transcript\"}}")
|
||||||
writeProfileFile(t, profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, renderCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := renderCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.render",
|
"--prompt", "prompt.render",
|
||||||
"--profile", "local-default",
|
"--profile", "local-default",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
@@ -663,15 +713,15 @@ func TestRenderCommandDefaultFormatTextIncludesPreparedDetailsAndNoSecrets(t *te
|
|||||||
"--top-p", "0.2",
|
"--top-p", "0.2",
|
||||||
"--timeout", "20s",
|
"--timeout", "20s",
|
||||||
"--api-key-env", envName,
|
"--api-key-env", envName,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if stderr.Len() != 0 {
|
if stderr != "" {
|
||||||
t.Fatalf("expected empty stderr on success, got %q", stderr.String())
|
t.Fatalf("expected empty stderr on success, got %q", stderr)
|
||||||
}
|
}
|
||||||
|
|
||||||
out := stdout.String()
|
out := stdout
|
||||||
for _, want := range []string{
|
for _, want := range []string{
|
||||||
"prompt: prompt.render",
|
"prompt: prompt.render",
|
||||||
"selected_profile_id: local-default",
|
"selected_profile_id: local-default",
|
||||||
@@ -698,111 +748,72 @@ func TestRenderCommandDefaultFormatTextIncludesPreparedDetailsAndNoSecrets(t *te
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderCommandSucceedsWithPromptAndProfileDirsFromConfig(t *testing.T) {
|
func TestRenderCommandSucceedsWithPromptAndProfileDirsFromConfig(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello transcript")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello transcript"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.render", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.render", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
||||||
configPath := writeAppConfigFile(t, fmt.Sprintf(`
|
configPath := writeAppConfigFile(t, fmt.Sprintf(`
|
||||||
prompt_dir: %s
|
prompt_dir: %s
|
||||||
profile_dir: %s
|
profile_dir: %s
|
||||||
`, promptDir, profileDir))
|
`, lib.promptDir, lib.profileDir))
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, renderCommand, []string{
|
||||||
var stderr bytes.Buffer
|
|
||||||
code := renderCommand([]string{
|
|
||||||
"--config", configPath,
|
"--config", configPath,
|
||||||
"--prompt", "prompt.render",
|
"--prompt", "prompt.render",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if !strings.Contains(stdout.String(), "prompt: prompt.render") {
|
if !strings.Contains(stdout, "prompt: prompt.render") {
|
||||||
t.Fatalf("expected rendered output, got %q", stdout.String())
|
t.Fatalf("expected rendered output, got %q", stdout)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderCommandExplicitTextFormatWorks(t *testing.T) {
|
func TestRenderCommandExplicitTextFormatWorks(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello transcript")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello transcript"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.render", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.render", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, renderCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := renderCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.render",
|
"--prompt", "prompt.render",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
"--format", "text",
|
"--format", "text",
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if !strings.Contains(stdout.String(), "prompt: prompt.render") {
|
if !strings.Contains(stdout, "prompt: prompt.render") {
|
||||||
t.Fatalf("expected text output for explicit --format text, got %q", stdout.String())
|
t.Fatalf("expected text output for explicit --format text, got %q", stdout)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderCommandExplicitJSONFormatOutputsValidJSON(t *testing.T) {
|
func TestRenderCommandExplicitJSONFormatOutputsValidJSON(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello transcript")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello transcript"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.render", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.render", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, renderCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := renderCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.render",
|
"--prompt", "prompt.render",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
"--format", "json",
|
"--format", "json",
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
|
|
||||||
var payload map[string]any
|
var payload map[string]any
|
||||||
if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil {
|
if err := json.Unmarshal([]byte(stdout), &payload); err != nil {
|
||||||
t.Fatalf("expected valid json output, got %v\nbody=%s", err, stdout.String())
|
t.Fatalf("expected valid json output, got %v\nbody=%s", err, stdout)
|
||||||
}
|
}
|
||||||
if payload["prompt_id"] != "prompt.render" {
|
if payload["prompt_id"] != "prompt.render" {
|
||||||
t.Fatalf("expected prompt_id, got %#v", payload["prompt_id"])
|
t.Fatalf("expected prompt_id, got %#v", payload["prompt_id"])
|
||||||
@@ -834,38 +845,25 @@ func TestRenderCommandUnknownFormatFailsClearly(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderCommandOutWritesToFile(t *testing.T) {
|
func TestRenderCommandOutWritesToFile(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello transcript")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
outPath := filepath.Join(lib.rootDir, "render.txt")
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello transcript"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
outPath := filepath.Join(tmp, "render.txt")
|
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.render", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.render", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", "http://127.0.0.1:1/v1", "profile-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, renderCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := renderCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.render",
|
"--prompt", "prompt.render",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
"--out", outPath,
|
"--out", outPath,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if stdout.Len() != 0 {
|
if stdout != "" {
|
||||||
t.Fatalf("expected empty stdout when --out is set, got %q", stdout.String())
|
t.Fatalf("expected empty stdout when --out is set, got %q", stdout)
|
||||||
}
|
}
|
||||||
out, err := os.ReadFile(outPath)
|
out, err := os.ReadFile(outPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -877,36 +875,23 @@ func TestRenderCommandOutWritesToFile(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderCommandPromptDefaultProfileWorksThroughCLIPath(t *testing.T) {
|
func TestRenderCommandPromptDefaultProfileWorksThroughCLIPath(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.default", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.default", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", "http://127.0.0.1:1/v1", "default-model")
|
writeProfileFile(t, lib.profileDir, "local-default", "http://127.0.0.1:1/v1", "default-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, renderCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := renderCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.default",
|
"--prompt", "prompt.default",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
|
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
out := stdout.String()
|
out := stdout
|
||||||
if !strings.Contains(out, "selected_profile_id: local-default") {
|
if !strings.Contains(out, "selected_profile_id: local-default") {
|
||||||
t.Fatalf("expected prompt default profile in output, got %q", out)
|
t.Fatalf("expected prompt default profile in output, got %q", out)
|
||||||
}
|
}
|
||||||
@@ -916,38 +901,25 @@ func TestRenderCommandPromptDefaultProfileWorksThroughCLIPath(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderCommandExplicitProfileOverridesPromptDefault(t *testing.T) {
|
func TestRenderCommandExplicitProfileOverridesPromptDefault(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.default", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.default", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", "http://127.0.0.1:1/v1", "default-model")
|
writeProfileFile(t, lib.profileDir, "local-default", "http://127.0.0.1:1/v1", "default-model")
|
||||||
writeProfileFile(t, profileDir, "quality", "http://127.0.0.1:1/v1", "quality-model")
|
writeProfileFile(t, lib.profileDir, "quality", "http://127.0.0.1:1/v1", "quality-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, renderCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := renderCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.default",
|
"--prompt", "prompt.default",
|
||||||
"--profile", "quality",
|
"--profile", "quality",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
|
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
out := stdout.String()
|
out := stdout
|
||||||
if !strings.Contains(out, "selected_profile_id: quality") {
|
if !strings.Contains(out, "selected_profile_id: quality") {
|
||||||
t.Fatalf("expected explicit profile in output, got %q", out)
|
t.Fatalf("expected explicit profile in output, got %q", out)
|
||||||
}
|
}
|
||||||
@@ -957,104 +929,67 @@ func TestRenderCommandExplicitProfileOverridesPromptDefault(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestRunCommandPromptDefaultProfileWorksThroughCLIPath(t *testing.T) {
|
func TestRunCommandPromptDefaultProfileWorksThroughCLIPath(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
ts := newTestLLMServer("default-output", nil)
|
ts := newTestLLMServer("default-output", nil)
|
||||||
defer ts.Close()
|
defer ts.Close()
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.default", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.default", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", ts.URL+"/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", ts.URL+"/v1", "profile-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, runCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := runCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.default",
|
"--prompt", "prompt.default",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
|
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if stdout.String() != "default-output" {
|
if stdout != "default-output" {
|
||||||
t.Fatalf("unexpected stdout output: %q", stdout.String())
|
t.Fatalf("unexpected stdout output: %q", stdout)
|
||||||
}
|
}
|
||||||
if !strings.Contains(stderr.String(), "selected_profile=local-default") {
|
if !strings.Contains(stderr, "selected_profile=local-default") {
|
||||||
t.Fatalf("expected selected profile in summary, got %q", stderr.String())
|
t.Fatalf("expected selected profile in summary, got %q", stderr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRunCommandExplicitProfileOverridesPromptDefault(t *testing.T) {
|
func TestRunCommandExplicitProfileOverridesPromptDefault(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
defaultServer := newTestLLMServer("from-default", nil)
|
defaultServer := newTestLLMServer("from-default", nil)
|
||||||
defer defaultServer.Close()
|
defer defaultServer.Close()
|
||||||
overrideServer := newTestLLMServer("from-override", nil)
|
overrideServer := newTestLLMServer("from-override", nil)
|
||||||
defer overrideServer.Close()
|
defer overrideServer.Close()
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.default", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.default", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", defaultServer.URL+"/v1", "default-model")
|
writeProfileFile(t, lib.profileDir, "local-default", defaultServer.URL+"/v1", "default-model")
|
||||||
writeProfileFile(t, profileDir, "quality", overrideServer.URL+"/v1", "quality-model")
|
writeProfileFile(t, lib.profileDir, "quality", overrideServer.URL+"/v1", "quality-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, runCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := runCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.default",
|
"--prompt", "prompt.default",
|
||||||
"--profile", "quality",
|
"--profile", "quality",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if stdout.String() != "from-override" {
|
if stdout != "from-override" {
|
||||||
t.Fatalf("expected explicit profile output, got %q", stdout.String())
|
t.Fatalf("expected explicit profile output, got %q", stdout)
|
||||||
}
|
}
|
||||||
if !strings.Contains(stderr.String(), "selected_profile=quality") {
|
if !strings.Contains(stderr, "selected_profile=quality") {
|
||||||
t.Fatalf("expected selected profile quality, got %q", stderr.String())
|
t.Fatalf("expected selected profile quality, got %q", stderr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRunCommandRuntimeFlagsOverrideSelectedProfileValues(t *testing.T) {
|
func TestRunCommandRuntimeFlagsOverrideSelectedProfileValues(t *testing.T) {
|
||||||
tmp := t.TempDir()
|
lib := newCLITestLibrary(t)
|
||||||
promptDir := filepath.Join(tmp, "prompts")
|
inputPath := lib.writeInputFile(t, "transcript.md", "hello")
|
||||||
profileDir := filepath.Join(tmp, "profiles")
|
|
||||||
if err := os.MkdirAll(promptDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if err := os.MkdirAll(profileDir, 0o755); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
inputPath := filepath.Join(tmp, "transcript.md")
|
|
||||||
if err := os.WriteFile(inputPath, []byte("hello"), 0o644); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
var baseHits int32
|
var baseHits int32
|
||||||
baseServer := newTestLLMServer("base", &baseHits)
|
baseServer := newTestLLMServer("base", &baseHits)
|
||||||
@@ -1071,14 +1006,12 @@ func TestRunCommandRuntimeFlagsOverrideSelectedProfileValues(t *testing.T) {
|
|||||||
}))
|
}))
|
||||||
defer overrideServer.Close()
|
defer overrideServer.Close()
|
||||||
|
|
||||||
writePromptFile(t, promptDir, "prompt.default", "local-default")
|
writePromptFile(t, lib.promptDir, "prompt.default", "local-default")
|
||||||
writeProfileFile(t, profileDir, "local-default", baseServer.URL+"/v1", "profile-model")
|
writeProfileFile(t, lib.profileDir, "local-default", baseServer.URL+"/v1", "profile-model")
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
code, stdout, stderr := runCLICommand(t, runCommand, []string{
|
||||||
var stderr bytes.Buffer
|
"--prompt-dir", lib.promptDir,
|
||||||
code := runCommand([]string{
|
"--profile-dir", lib.profileDir,
|
||||||
"--prompt-dir", promptDir,
|
|
||||||
"--profile-dir", profileDir,
|
|
||||||
"--prompt", "prompt.default",
|
"--prompt", "prompt.default",
|
||||||
"--input", "transcript=" + inputPath,
|
"--input", "transcript=" + inputPath,
|
||||||
"--llm-base-url", overrideServer.URL + "/v1",
|
"--llm-base-url", overrideServer.URL + "/v1",
|
||||||
@@ -1087,9 +1020,9 @@ func TestRunCommandRuntimeFlagsOverrideSelectedProfileValues(t *testing.T) {
|
|||||||
"--max-tokens", "55",
|
"--max-tokens", "55",
|
||||||
"--top-p", "0.2",
|
"--top-p", "0.2",
|
||||||
"--timeout", "20s",
|
"--timeout", "20s",
|
||||||
}, &stdout, &stderr)
|
})
|
||||||
if code != ExitOK {
|
if code != ExitOK {
|
||||||
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr.String())
|
t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr)
|
||||||
}
|
}
|
||||||
if atomic.LoadInt32(&baseHits) != 0 {
|
if atomic.LoadInt32(&baseHits) != 0 {
|
||||||
t.Fatalf("expected base profile endpoint not to be hit, got %d", baseHits)
|
t.Fatalf("expected base profile endpoint not to be hit, got %d", baseHits)
|
||||||
@@ -1097,8 +1030,8 @@ func TestRunCommandRuntimeFlagsOverrideSelectedProfileValues(t *testing.T) {
|
|||||||
if atomic.LoadInt32(&overrideHits) != 1 {
|
if atomic.LoadInt32(&overrideHits) != 1 {
|
||||||
t.Fatalf("expected override endpoint to be hit once, got %d", overrideHits)
|
t.Fatalf("expected override endpoint to be hit once, got %d", overrideHits)
|
||||||
}
|
}
|
||||||
if stdout.String() != "override" {
|
if stdout != "override" {
|
||||||
t.Fatalf("unexpected stdout output: %q", stdout.String())
|
t.Fatalf("unexpected stdout output: %q", stdout)
|
||||||
}
|
}
|
||||||
if !strings.Contains(observedBody, `"model":"override-model"`) {
|
if !strings.Contains(observedBody, `"model":"override-model"`) {
|
||||||
t.Fatalf("expected override model in request body, got %s", observedBody)
|
t.Fatalf("expected override model in request body, got %s", observedBody)
|
||||||
@@ -1133,6 +1066,49 @@ func TestWriteOutputAndSummaryUseSeparateWriters(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type cliTestLibrary struct {
|
||||||
|
rootDir string
|
||||||
|
promptDir string
|
||||||
|
profileDir string
|
||||||
|
}
|
||||||
|
|
||||||
|
func newCLITestLibrary(t *testing.T) *cliTestLibrary {
|
||||||
|
t.Helper()
|
||||||
|
root := t.TempDir()
|
||||||
|
lib := &cliTestLibrary{
|
||||||
|
rootDir: root,
|
||||||
|
promptDir: filepath.Join(root, "prompts"),
|
||||||
|
profileDir: filepath.Join(root, "profiles"),
|
||||||
|
}
|
||||||
|
if err := os.MkdirAll(lib.promptDir, 0o755); err != nil {
|
||||||
|
t.Fatalf("failed to create prompt fixture directory: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.MkdirAll(lib.profileDir, 0o755); err != nil {
|
||||||
|
t.Fatalf("failed to create profile fixture directory: %v", err)
|
||||||
|
}
|
||||||
|
return lib
|
||||||
|
}
|
||||||
|
|
||||||
|
func (l *cliTestLibrary) writeInputFile(t *testing.T, name, body string) string {
|
||||||
|
t.Helper()
|
||||||
|
path := filepath.Join(l.rootDir, name)
|
||||||
|
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||||
|
t.Fatalf("failed to create input fixture directory: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(path, []byte(body), 0o644); err != nil {
|
||||||
|
t.Fatalf("failed to write input fixture: %v", err)
|
||||||
|
}
|
||||||
|
return path
|
||||||
|
}
|
||||||
|
|
||||||
|
func runCLICommand(t *testing.T, command func([]string, io.Writer, io.Writer) int, args []string) (int, string, string) {
|
||||||
|
t.Helper()
|
||||||
|
var stdout bytes.Buffer
|
||||||
|
var stderr bytes.Buffer
|
||||||
|
code := command(args, &stdout, &stderr)
|
||||||
|
return code, stdout.String(), stderr.String()
|
||||||
|
}
|
||||||
|
|
||||||
func writePromptFile(t *testing.T, dir, id, defaultProfile string) {
|
func writePromptFile(t *testing.T, dir, id, defaultProfile string) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
writePromptFileWithTemplate(t, dir, id, defaultProfile, "Summarize: {{input \"transcript\"}}")
|
writePromptFileWithTemplate(t, dir, id, defaultProfile, "Summarize: {{input \"transcript\"}}")
|
||||||
|
|||||||
@@ -63,18 +63,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
var model *domain.ExecutionTarget
|
var model *domain.ExecutionTarget
|
||||||
if req.Model != nil {
|
if req.Model != nil {
|
||||||
model = &domain.ExecutionTarget{
|
model = executionTargetFromModelOverrideDTO(req.Model)
|
||||||
Endpoint: req.Model.Endpoint,
|
|
||||||
Model: req.Model.Model,
|
|
||||||
Temperature: req.Model.Temperature,
|
|
||||||
MaxTokens: req.Model.MaxTokens,
|
|
||||||
TopP: req.Model.TopP,
|
|
||||||
TimeoutSeconds: req.Model.TimeoutSeconds,
|
|
||||||
ServiceTier: req.Model.ServiceTier,
|
|
||||||
ReasoningEffort: req.Model.ReasoningEffort,
|
|
||||||
APIKeyEnv: req.Model.APIKeyEnv,
|
|
||||||
ExtraParams: req.Model.ExtraParams,
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
res, err := h.runner.Run(r.Context(), domain.RunRequest{
|
res, err := h.runner.Run(r.Context(), domain.RunRequest{
|
||||||
@@ -110,19 +99,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
|||||||
SelectedProfileID: res.SelectedProfileID,
|
SelectedProfileID: res.SelectedProfileID,
|
||||||
ModelName: res.ModelName,
|
ModelName: res.ModelName,
|
||||||
Endpoint: res.Endpoint,
|
Endpoint: res.Endpoint,
|
||||||
ModelParams: modelParamsDTO{
|
ModelParams: modelParamsDTOFromExecutionTarget(res.EffectiveModelParams),
|
||||||
Endpoint: res.EffectiveModelParams.Endpoint,
|
InputHashes: res.InputHashes,
|
||||||
Model: res.EffectiveModelParams.Model,
|
|
||||||
Temperature: res.EffectiveModelParams.Temperature,
|
|
||||||
MaxTokens: res.EffectiveModelParams.MaxTokens,
|
|
||||||
TopP: res.EffectiveModelParams.TopP,
|
|
||||||
TimeoutSeconds: res.EffectiveModelParams.TimeoutSeconds,
|
|
||||||
ServiceTier: res.EffectiveModelParams.ServiceTier,
|
|
||||||
ReasoningEffort: res.EffectiveModelParams.ReasoningEffort,
|
|
||||||
APIKeyEnv: res.EffectiveModelParams.APIKeyEnv,
|
|
||||||
ExtraParams: res.EffectiveModelParams.ExtraParams,
|
|
||||||
},
|
|
||||||
InputHashes: res.InputHashes,
|
|
||||||
Usage: tokenUsageDTO{
|
Usage: tokenUsageDTO{
|
||||||
PromptTokens: res.Usage.PromptTokens,
|
PromptTokens: res.Usage.PromptTokens,
|
||||||
CompletionTokens: res.Usage.CompletionTokens,
|
CompletionTokens: res.Usage.CompletionTokens,
|
||||||
@@ -143,6 +121,39 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
|||||||
writeJSON(w, http.StatusOK, resp)
|
writeJSON(w, http.StatusOK, resp)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func executionTargetFromModelOverrideDTO(dto *modelOverrideRequestDTO) *domain.ExecutionTarget {
|
||||||
|
if dto == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
return &domain.ExecutionTarget{
|
||||||
|
Endpoint: dto.Endpoint,
|
||||||
|
Model: dto.Model,
|
||||||
|
Temperature: dto.Temperature,
|
||||||
|
MaxTokens: dto.MaxTokens,
|
||||||
|
TopP: dto.TopP,
|
||||||
|
TimeoutSeconds: dto.TimeoutSeconds,
|
||||||
|
ServiceTier: dto.ServiceTier,
|
||||||
|
ReasoningEffort: dto.ReasoningEffort,
|
||||||
|
APIKeyEnv: dto.APIKeyEnv,
|
||||||
|
ExtraParams: dto.ExtraParams,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func modelParamsDTOFromExecutionTarget(target domain.ExecutionTarget) modelParamsDTO {
|
||||||
|
return modelParamsDTO{
|
||||||
|
Endpoint: target.Endpoint,
|
||||||
|
Model: target.Model,
|
||||||
|
Temperature: target.Temperature,
|
||||||
|
MaxTokens: target.MaxTokens,
|
||||||
|
TopP: target.TopP,
|
||||||
|
TimeoutSeconds: target.TimeoutSeconds,
|
||||||
|
ServiceTier: target.ServiceTier,
|
||||||
|
ReasoningEffort: target.ReasoningEffort,
|
||||||
|
APIKeyEnv: target.APIKeyEnv,
|
||||||
|
ExtraParams: target.ExtraParams,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func mapValidation(v domain.ValidationResult) validationDTO {
|
func mapValidation(v domain.ValidationResult) validationDTO {
|
||||||
return validationDTO{
|
return validationDTO{
|
||||||
Status: string(v.Status),
|
Status: string(v.Status),
|
||||||
@@ -164,9 +175,9 @@ func mapRunError(err error) (int, string, string) {
|
|||||||
return http.StatusBadRequest, "prompt_load_failed", "failed to load prompt definition"
|
return http.StatusBadRequest, "prompt_load_failed", "failed to load prompt definition"
|
||||||
case errors.Is(err, profile.ErrInvalidYAML), errors.Is(err, profile.ErrInvalidProfile):
|
case errors.Is(err, profile.ErrInvalidYAML), errors.Is(err, profile.ErrInvalidProfile):
|
||||||
return http.StatusBadRequest, "profile_load_failed", "failed to load execution profile"
|
return http.StatusBadRequest, "profile_load_failed", "failed to load execution profile"
|
||||||
case errors.Is(err, usecase.ErrInvalidRequest) && strings.Contains(err.Error(), "profile id is required either in request or prompt default_profile"):
|
case errors.Is(err, usecase.ErrProfileRequired):
|
||||||
return http.StatusBadRequest, "profile_required", "profile_id is required when prompt default_profile is not set"
|
return http.StatusBadRequest, "profile_required", "profile_id is required when prompt default_profile is not set"
|
||||||
case errors.Is(err, usecase.ErrInvalidRequest) && strings.Contains(err.Error(), "api key environment variable"):
|
case errors.Is(err, usecase.ErrAPIKeyEnvMissing):
|
||||||
return http.StatusBadRequest, "api_key_env_missing", "api_key_env is set but the environment variable is missing"
|
return http.StatusBadRequest, "api_key_env_missing", "api_key_env is set but the environment variable is missing"
|
||||||
case errors.Is(err, usecase.ErrInvalidRequest):
|
case errors.Is(err, usecase.ErrInvalidRequest):
|
||||||
return http.StatusBadRequest, "invalid_request", "invalid run request"
|
return http.StatusBadRequest, "invalid_request", "invalid run request"
|
||||||
|
|||||||
@@ -4,10 +4,10 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
|
"reflect"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
@@ -173,6 +173,136 @@ func TestHandlerPostRunsSuccessUsingPromptDefaultProfile(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestHandlerModelOverrideMapsAllSupportedExecutionFields(t *testing.T) {
|
||||||
|
r := &fakeRunner{result: &domain.RunResult{
|
||||||
|
Artifact: domain.Artifact{Body: []byte("ok")},
|
||||||
|
Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true},
|
||||||
|
EffectiveModelParams: domain.ExecutionTarget{Endpoint: "http://llm/v1", Model: "m1"},
|
||||||
|
}}
|
||||||
|
h := NewHandler(r)
|
||||||
|
|
||||||
|
reqBody := `{
|
||||||
|
"prompt_id": "prompt-1",
|
||||||
|
"inputs": {"transcript": {"type": "file", "uri": "./t.md"}},
|
||||||
|
"model": {
|
||||||
|
"endpoint": "http://override/v1",
|
||||||
|
"model": "override-model",
|
||||||
|
"temperature": 0.6,
|
||||||
|
"max_tokens": 250,
|
||||||
|
"top_p": 0.85,
|
||||||
|
"timeout_seconds": 33,
|
||||||
|
"service_tier": "flex",
|
||||||
|
"reasoning_effort": "medium",
|
||||||
|
"api_key_env": "SCRIPTORIUM_API_KEY",
|
||||||
|
"extra_params": {"provider_option":"on"}
|
||||||
|
}
|
||||||
|
}`
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(reqBody))
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
|
||||||
|
h.ServeHTTP(w, req)
|
||||||
|
|
||||||
|
if w.Code != http.StatusOK {
|
||||||
|
t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
if r.last.Execution == nil {
|
||||||
|
t.Fatalf("expected execution override in run request")
|
||||||
|
}
|
||||||
|
got := r.last.Execution
|
||||||
|
if got.Endpoint != "http://override/v1" ||
|
||||||
|
got.Model != "override-model" ||
|
||||||
|
got.Temperature != 0.6 ||
|
||||||
|
got.MaxTokens != 250 ||
|
||||||
|
got.TopP != 0.85 ||
|
||||||
|
got.TimeoutSeconds != 33 ||
|
||||||
|
got.ServiceTier != "flex" ||
|
||||||
|
got.ReasoningEffort != "medium" ||
|
||||||
|
got.APIKeyEnv != "SCRIPTORIUM_API_KEY" {
|
||||||
|
t.Fatalf("unexpected mapped execution target: %+v", got)
|
||||||
|
}
|
||||||
|
if !reflect.DeepEqual(got.ExtraParams, map[string]string{"provider_option": "on"}) {
|
||||||
|
t.Fatalf("unexpected mapped extra_params: %#v", got.ExtraParams)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestHandlerResponseMetadataModelParamsIncludesAllSupportedFields(t *testing.T) {
|
||||||
|
r := &fakeRunner{result: &domain.RunResult{
|
||||||
|
Artifact: domain.Artifact{
|
||||||
|
Name: "output",
|
||||||
|
ContentType: "text/plain",
|
||||||
|
Body: []byte("ok"),
|
||||||
|
Size: 2,
|
||||||
|
Hash: "abc",
|
||||||
|
},
|
||||||
|
Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true},
|
||||||
|
EffectiveModelParams: domain.ExecutionTarget{
|
||||||
|
Endpoint: "http://llm/v1",
|
||||||
|
Model: "gpt-test",
|
||||||
|
Temperature: 0.4,
|
||||||
|
MaxTokens: 321,
|
||||||
|
TopP: 0.7,
|
||||||
|
TimeoutSeconds: 45,
|
||||||
|
ServiceTier: "priority",
|
||||||
|
ReasoningEffort: "high",
|
||||||
|
APIKeyEnv: "SCRIPTORIUM_API_KEY",
|
||||||
|
ExtraParams: map[string]string{
|
||||||
|
"provider_option": "on",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}}
|
||||||
|
h := NewHandler(r)
|
||||||
|
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{"prompt_id":"p","inputs":{"x":{"type":"file","uri":"a"}}}`))
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(w, req)
|
||||||
|
|
||||||
|
if w.Code != http.StatusOK {
|
||||||
|
t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
var resp map[string]any
|
||||||
|
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||||
|
t.Fatalf("invalid JSON response: %v", err)
|
||||||
|
}
|
||||||
|
metadata := resp["metadata"].(map[string]any)
|
||||||
|
params := metadata["model_params"].(map[string]any)
|
||||||
|
|
||||||
|
if params["endpoint"] != "http://llm/v1" {
|
||||||
|
t.Fatalf("unexpected endpoint: %#v", params["endpoint"])
|
||||||
|
}
|
||||||
|
if params["model"] != "gpt-test" {
|
||||||
|
t.Fatalf("unexpected model: %#v", params["model"])
|
||||||
|
}
|
||||||
|
if params["temperature"] != 0.4 {
|
||||||
|
t.Fatalf("unexpected temperature: %#v", params["temperature"])
|
||||||
|
}
|
||||||
|
if params["max_tokens"] != float64(321) {
|
||||||
|
t.Fatalf("unexpected max_tokens: %#v", params["max_tokens"])
|
||||||
|
}
|
||||||
|
if params["top_p"] != 0.7 {
|
||||||
|
t.Fatalf("unexpected top_p: %#v", params["top_p"])
|
||||||
|
}
|
||||||
|
if params["timeout_seconds"] != float64(45) {
|
||||||
|
t.Fatalf("unexpected timeout_seconds: %#v", params["timeout_seconds"])
|
||||||
|
}
|
||||||
|
if params["service_tier"] != "priority" {
|
||||||
|
t.Fatalf("unexpected service_tier: %#v", params["service_tier"])
|
||||||
|
}
|
||||||
|
if params["reasoning_effort"] != "high" {
|
||||||
|
t.Fatalf("unexpected reasoning_effort: %#v", params["reasoning_effort"])
|
||||||
|
}
|
||||||
|
if params["api_key_env"] != "SCRIPTORIUM_API_KEY" {
|
||||||
|
t.Fatalf("unexpected api_key_env: %#v", params["api_key_env"])
|
||||||
|
}
|
||||||
|
extraParams, ok := params["extra_params"].(map[string]any)
|
||||||
|
if !ok {
|
||||||
|
t.Fatalf("expected extra_params object, got %#v", params["extra_params"])
|
||||||
|
}
|
||||||
|
if extraParams["provider_option"] != "on" {
|
||||||
|
t.Fatalf("unexpected extra_params.provider_option: %#v", extraParams["provider_option"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestHandlerInvalidJSON(t *testing.T) {
|
func TestHandlerInvalidJSON(t *testing.T) {
|
||||||
h := NewHandler(&fakeRunner{})
|
h := NewHandler(&fakeRunner{})
|
||||||
req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString("{"))
|
req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString("{"))
|
||||||
@@ -216,10 +346,10 @@ func TestHandlerUsecaseErrorMapping(t *testing.T) {
|
|||||||
}{
|
}{
|
||||||
{name: "prompt not found", err: wrap(usecase.ErrProfileLoad, promptdef.ErrPromptDefinitionNotFound), status: http.StatusNotFound, code: "prompt_not_found", message: "prompt definition not found"},
|
{name: "prompt not found", err: wrap(usecase.ErrProfileLoad, promptdef.ErrPromptDefinitionNotFound), status: http.StatusNotFound, code: "prompt_not_found", message: "prompt definition not found"},
|
||||||
{name: "prompt load invalid", err: wrap(usecase.ErrProfileLoad, promptdef.ErrInvalidPromptDefinition), status: http.StatusBadRequest, code: "prompt_load_failed", message: "failed to load prompt definition"},
|
{name: "prompt load invalid", err: wrap(usecase.ErrProfileLoad, promptdef.ErrInvalidPromptDefinition), status: http.StatusBadRequest, code: "prompt_load_failed", message: "failed to load prompt definition"},
|
||||||
{name: "missing profile/default", err: wrap(usecase.ErrInvalidRequest, errors.New("profile id is required either in request or prompt default_profile")), status: http.StatusBadRequest, code: "profile_required", message: "profile_id is required when prompt default_profile is not set"},
|
{name: "missing profile/default", err: wrap(usecase.ErrInvalidRequest, usecase.ErrProfileRequired), status: http.StatusBadRequest, code: "profile_required", message: "profile_id is required when prompt default_profile is not set"},
|
||||||
{name: "profile not found", err: wrap(usecase.ErrProfileLoad, profile.ErrProfileNotFound), status: http.StatusNotFound, code: "profile_not_found", message: "execution profile not found"},
|
{name: "profile not found", err: wrap(usecase.ErrProfileLoad, profile.ErrProfileNotFound), status: http.StatusNotFound, code: "profile_not_found", message: "execution profile not found"},
|
||||||
{name: "profile invalid", err: wrap(usecase.ErrProfileLoad, profile.ErrInvalidProfile), status: http.StatusBadRequest, code: "profile_load_failed", message: "failed to load execution profile"},
|
{name: "profile invalid", err: wrap(usecase.ErrProfileLoad, profile.ErrInvalidProfile), status: http.StatusBadRequest, code: "profile_load_failed", message: "failed to load execution profile"},
|
||||||
{name: "api key env missing", err: wrap(usecase.ErrInvalidRequest, errors.New(`api key environment variable "SCRIPTORIUM_API_KEY" is not set`)), status: http.StatusBadRequest, code: "api_key_env_missing", message: "api_key_env is set but the environment variable is missing"},
|
{name: "api key env missing", err: wrap(usecase.ErrInvalidRequest, usecase.ErrAPIKeyEnvMissing), status: http.StatusBadRequest, code: "api_key_env_missing", message: "api_key_env is set but the environment variable is missing"},
|
||||||
{name: "artifact", err: wrap(usecase.ErrArtifactLoad, fmt.Errorf("read failed")), status: http.StatusBadRequest, code: "artifact_read_failed", message: "failed to read input artifact", avoidCause: "read failed"},
|
{name: "artifact", err: wrap(usecase.ErrArtifactLoad, fmt.Errorf("read failed")), status: http.StatusBadRequest, code: "artifact_read_failed", message: "failed to read input artifact", avoidCause: "read failed"},
|
||||||
{name: "prompt render", err: wrap(usecase.ErrPromptRender, fmt.Errorf("render failed")), status: http.StatusBadRequest, code: "prompt_render_failed", message: "failed to render prompt", avoidCause: "render failed"},
|
{name: "prompt render", err: wrap(usecase.ErrPromptRender, fmt.Errorf("render failed")), status: http.StatusBadRequest, code: "prompt_render_failed", message: "failed to render prompt", avoidCause: "render failed"},
|
||||||
{name: "llm", err: wrap(usecase.ErrLLMGenerate, fmt.Errorf("llm failed")), status: http.StatusBadGateway, code: "llm_failed", message: "model generation request failed", avoidCause: "llm failed"},
|
{name: "llm", err: wrap(usecase.ErrLLMGenerate, fmt.Errorf("llm failed")), status: http.StatusBadGateway, code: "llm_failed", message: "model generation request failed", avoidCause: "llm failed"},
|
||||||
|
|||||||
@@ -46,8 +46,8 @@ func TestCompositeReader_Read(t *testing.T) {
|
|||||||
|
|
||||||
t.Run("unsupported ref type", func(t *testing.T) {
|
t.Run("unsupported ref type", func(t *testing.T) {
|
||||||
ref := domain.ArtifactRef{
|
ref := domain.ArtifactRef{
|
||||||
Type: domain.ArtifactRefS3,
|
Type: domain.ArtifactRefType("unsupported"),
|
||||||
URI: "s3://bucket/key",
|
URI: "unsupported://bucket/key",
|
||||||
}
|
}
|
||||||
_, err := reader.Read(ctx, ref)
|
_, err := reader.Read(ctx, ref)
|
||||||
if !errors.Is(err, ErrUnsupportedRefType) {
|
if !errors.Is(err, ErrUnsupportedRefType) {
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ type ArtifactRefType string
|
|||||||
const (
|
const (
|
||||||
ArtifactRefInline ArtifactRefType = "inline"
|
ArtifactRefInline ArtifactRefType = "inline"
|
||||||
ArtifactRefFile ArtifactRefType = "file"
|
ArtifactRefFile ArtifactRefType = "file"
|
||||||
ArtifactRefS3 ArtifactRefType = "s3"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// OutputFormat defines the desired format of the generated artifact.
|
// OutputFormat defines the desired format of the generated artifact.
|
||||||
|
|||||||
54
internal/filecatalog/catalog.go
Normal file
54
internal/filecatalog/catalog.go
Normal file
@@ -0,0 +1,54 @@
|
|||||||
|
package filecatalog
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"sort"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
|
// FindYAMLFiles returns sorted full paths for .yaml and .yml files under root.
|
||||||
|
func FindYAMLFiles(ctx context.Context, root string) ([]string, error) {
|
||||||
|
var files []string
|
||||||
|
err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
select {
|
||||||
|
case <-ctx.Done():
|
||||||
|
return ctx.Err()
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
if d.IsDir() {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
if !isYAMLFile(d.Name()) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
files = append(files, path)
|
||||||
|
return nil
|
||||||
|
})
|
||||||
|
sort.Strings(files)
|
||||||
|
return files, err
|
||||||
|
}
|
||||||
|
|
||||||
|
// RelativePath computes a clean relative path from root to path.
|
||||||
|
func RelativePath(root string, path string) string {
|
||||||
|
rel, err := filepath.Rel(root, path)
|
||||||
|
if err != nil {
|
||||||
|
return filepath.Clean(path)
|
||||||
|
}
|
||||||
|
return filepath.Clean(rel)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Stem strips .yaml or .yml from a file name.
|
||||||
|
func Stem(name string) string {
|
||||||
|
name = strings.TrimSuffix(name, ".yaml")
|
||||||
|
name = strings.TrimSuffix(name, ".yml")
|
||||||
|
return name
|
||||||
|
}
|
||||||
|
|
||||||
|
func isYAMLFile(name string) bool {
|
||||||
|
return strings.HasSuffix(name, ".yaml") || strings.HasSuffix(name, ".yml")
|
||||||
|
}
|
||||||
84
internal/filecatalog/catalog_test.go
Normal file
84
internal/filecatalog/catalog_test.go
Normal file
@@ -0,0 +1,84 @@
|
|||||||
|
package filecatalog
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"reflect"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFindYAMLFilesNestedSortedAndFiltered(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
mustWriteFile(t, filepath.Join(root, "z", "prompt.yml"), "id: z")
|
||||||
|
mustWriteFile(t, filepath.Join(root, "a", "profile.yaml"), "id: a")
|
||||||
|
mustWriteFile(t, filepath.Join(root, "a", "ignore.txt"), "not yaml")
|
||||||
|
mustWriteFile(t, filepath.Join(root, "b", "ignore.yaml.bak"), "not yaml")
|
||||||
|
|
||||||
|
got, err := FindYAMLFiles(context.Background(), root)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected no error, got %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
want := []string{
|
||||||
|
filepath.Join(root, "a", "profile.yaml"),
|
||||||
|
filepath.Join(root, "z", "prompt.yml"),
|
||||||
|
}
|
||||||
|
if !reflect.DeepEqual(got, want) {
|
||||||
|
t.Fatalf("expected sorted YAML files %v, got %v", want, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestFindYAMLFilesHonorsContextCancellation(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
mustWriteFile(t, filepath.Join(root, "one.yaml"), "id: one")
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel()
|
||||||
|
|
||||||
|
_, err := FindYAMLFiles(ctx, root)
|
||||||
|
if !errors.Is(err, context.Canceled) {
|
||||||
|
t.Fatalf("expected context.Canceled, got %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRelativePathNested(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
path := filepath.Join(root, "nested", "profiles", "local.yaml")
|
||||||
|
got := RelativePath(root, path)
|
||||||
|
want := filepath.Join("nested", "profiles", "local.yaml")
|
||||||
|
if got != want {
|
||||||
|
t.Fatalf("expected relative path %q, got %q", want, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestStemStripsYAMLExtensions(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
in string
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{name: "yaml", in: "prompt.yaml", want: "prompt"},
|
||||||
|
{name: "yml", in: "profile.yml", want: "profile"},
|
||||||
|
{name: "other", in: "file.txt", want: "file.txt"},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
if got := Stem(tc.in); got != tc.want {
|
||||||
|
t.Fatalf("expected %q, got %q", tc.want, got)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func mustWriteFile(t *testing.T, path string, content string) {
|
||||||
|
t.Helper()
|
||||||
|
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||||
|
t.Fatalf("failed to create directory: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
|
||||||
|
t.Fatalf("failed to write file %q: %v", path, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -75,14 +75,6 @@ func (c *OpenAICompatibleClient) Generate(ctx context.Context, req domain.Genera
|
|||||||
return nil, fmt.Errorf("%w: timeout_seconds must be greater than or equal to 0", ErrInvalidRequest)
|
return nil, fmt.Errorf("%w: timeout_seconds must be greater than or equal to 0", ErrInvalidRequest)
|
||||||
}
|
}
|
||||||
|
|
||||||
model := strings.TrimSpace(req.Target.Model)
|
|
||||||
if model == "" {
|
|
||||||
model = strings.TrimSpace(c.defaultModel)
|
|
||||||
}
|
|
||||||
if model == "" {
|
|
||||||
return nil, fmt.Errorf("%w: model is required", ErrInvalidRequest)
|
|
||||||
}
|
|
||||||
|
|
||||||
endpoint := strings.TrimSpace(req.Target.Endpoint)
|
endpoint := strings.TrimSpace(req.Target.Endpoint)
|
||||||
if endpoint == "" {
|
if endpoint == "" {
|
||||||
endpoint = c.baseURL
|
endpoint = c.baseURL
|
||||||
@@ -92,36 +84,9 @@ func (c *OpenAICompatibleClient) Generate(ctx context.Context, req domain.Genera
|
|||||||
}
|
}
|
||||||
endpoint = strings.TrimRight(endpoint, "/") + defaults.OpenAIChatCompletionsPath
|
endpoint = strings.TrimRight(endpoint, "/") + defaults.OpenAIChatCompletionsPath
|
||||||
|
|
||||||
wireReq := openAIChatRequest{
|
wireReq, err := openAIChatRequestFromGenerateRequest(req, c.defaultModel)
|
||||||
Model: model,
|
if err != nil {
|
||||||
}
|
return nil, fmt.Errorf("%w: %v", ErrInvalidRequest, err)
|
||||||
|
|
||||||
wireReq.Messages = make([]openAIChatMessage, 0, len(req.Prompt.Messages))
|
|
||||||
for _, msg := range req.Prompt.Messages {
|
|
||||||
wireReq.Messages = append(wireReq.Messages, openAIChatMessage{
|
|
||||||
Role: msg.Role,
|
|
||||||
Content: msg.Content,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
if req.Target.Temperature != 0 {
|
|
||||||
wireReq.Temperature = &req.Target.Temperature
|
|
||||||
}
|
|
||||||
if req.Target.MaxTokens != 0 {
|
|
||||||
wireReq.MaxTokens = &req.Target.MaxTokens
|
|
||||||
}
|
|
||||||
if req.Target.TopP != 0 {
|
|
||||||
wireReq.TopP = &req.Target.TopP
|
|
||||||
}
|
|
||||||
if strings.TrimSpace(req.Target.ServiceTier) != "" {
|
|
||||||
wireReq.ServiceTier = req.Target.ServiceTier
|
|
||||||
}
|
|
||||||
if req.StructuredOutput != nil {
|
|
||||||
responseFormat, err := toOpenAIResponseFormat(req.StructuredOutput)
|
|
||||||
if err != nil {
|
|
||||||
return nil, fmt.Errorf("%w: %v", ErrInvalidRequest, err)
|
|
||||||
}
|
|
||||||
wireReq.ResponseFormat = responseFormat
|
|
||||||
}
|
}
|
||||||
|
|
||||||
payload, err := json.Marshal(wireReq)
|
payload, err := json.Marshal(wireReq)
|
||||||
@@ -190,6 +155,50 @@ func (c *OpenAICompatibleClient) Generate(ctx context.Context, req domain.Genera
|
|||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func openAIChatRequestFromGenerateRequest(req domain.GenerateRequest, defaultModel string) (openAIChatRequest, error) {
|
||||||
|
model := strings.TrimSpace(req.Target.Model)
|
||||||
|
if model == "" {
|
||||||
|
model = strings.TrimSpace(defaultModel)
|
||||||
|
}
|
||||||
|
if model == "" {
|
||||||
|
return openAIChatRequest{}, errors.New("model is required")
|
||||||
|
}
|
||||||
|
|
||||||
|
wireReq := openAIChatRequest{
|
||||||
|
Model: model,
|
||||||
|
}
|
||||||
|
|
||||||
|
wireReq.Messages = make([]openAIChatMessage, 0, len(req.Prompt.Messages))
|
||||||
|
for _, msg := range req.Prompt.Messages {
|
||||||
|
wireReq.Messages = append(wireReq.Messages, openAIChatMessage{
|
||||||
|
Role: msg.Role,
|
||||||
|
Content: msg.Content,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
if req.Target.Temperature != 0 {
|
||||||
|
wireReq.Temperature = &req.Target.Temperature
|
||||||
|
}
|
||||||
|
if req.Target.MaxTokens != 0 {
|
||||||
|
wireReq.MaxTokens = &req.Target.MaxTokens
|
||||||
|
}
|
||||||
|
if req.Target.TopP != 0 {
|
||||||
|
wireReq.TopP = &req.Target.TopP
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(req.Target.ServiceTier) != "" {
|
||||||
|
wireReq.ServiceTier = req.Target.ServiceTier
|
||||||
|
}
|
||||||
|
if req.StructuredOutput != nil {
|
||||||
|
responseFormat, err := toOpenAIResponseFormat(req.StructuredOutput)
|
||||||
|
if err != nil {
|
||||||
|
return openAIChatRequest{}, err
|
||||||
|
}
|
||||||
|
wireReq.ResponseFormat = responseFormat
|
||||||
|
}
|
||||||
|
|
||||||
|
return wireReq, nil
|
||||||
|
}
|
||||||
|
|
||||||
type openAIChatRequest struct {
|
type openAIChatRequest struct {
|
||||||
Model string `json:"model"`
|
Model string `json:"model"`
|
||||||
Messages []openAIChatMessage `json:"messages"`
|
Messages []openAIChatMessage `json:"messages"`
|
||||||
|
|||||||
@@ -96,6 +96,15 @@ func TestOpenAICompatibleClientGenerateSuccess(t *testing.T) {
|
|||||||
if got, ok := obs.Body["model"].(string); !ok || got != "gpt-test" {
|
if got, ok := obs.Body["model"].(string); !ok || got != "gpt-test" {
|
||||||
t.Fatalf("unexpected model payload: %#v", obs.Body["model"])
|
t.Fatalf("unexpected model payload: %#v", obs.Body["model"])
|
||||||
}
|
}
|
||||||
|
if got, ok := obs.Body["temperature"].(float64); !ok || got != 0.4 {
|
||||||
|
t.Fatalf("unexpected temperature payload: %#v", obs.Body["temperature"])
|
||||||
|
}
|
||||||
|
if got, ok := obs.Body["max_tokens"].(float64); !ok || got != 123 {
|
||||||
|
t.Fatalf("unexpected max_tokens payload: %#v", obs.Body["max_tokens"])
|
||||||
|
}
|
||||||
|
if got, ok := obs.Body["top_p"].(float64); !ok || got != 0.7 {
|
||||||
|
t.Fatalf("unexpected top_p payload: %#v", obs.Body["top_p"])
|
||||||
|
}
|
||||||
if got, ok := obs.Body["service_tier"].(string); !ok || got != "priority" {
|
if got, ok := obs.Body["service_tier"].(string); !ok || got != "priority" {
|
||||||
t.Fatalf("unexpected service_tier payload: %#v", obs.Body["service_tier"])
|
t.Fatalf("unexpected service_tier payload: %#v", obs.Body["service_tier"])
|
||||||
}
|
}
|
||||||
@@ -166,6 +175,43 @@ func TestOpenAICompatibleClientOmitsResponseFormatWhenNoStructuredOutput(t *test
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestOpenAICompatibleClientOmitsReasoningEffortAndExtraParams(t *testing.T) {
|
||||||
|
var observedBody map[string]any
|
||||||
|
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
defer r.Body.Close()
|
||||||
|
if err := json.NewDecoder(r.Body).Decode(&observedBody); err != nil {
|
||||||
|
t.Fatalf("failed to decode request body: %v", err)
|
||||||
|
}
|
||||||
|
_, _ = w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`))
|
||||||
|
}))
|
||||||
|
defer ts.Close()
|
||||||
|
|
||||||
|
client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{BaseURL: ts.URL + "/v1"})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = client.Generate(context.Background(), domain.GenerateRequest{
|
||||||
|
Prompt: domain.RenderedPrompt{Messages: []domain.RenderedMessage{{Role: "user", Content: "hi"}}},
|
||||||
|
Target: domain.ExecutionTarget{
|
||||||
|
Model: "model",
|
||||||
|
ReasoningEffort: "high",
|
||||||
|
ExtraParams: map[string]string{
|
||||||
|
"provider_option": "on",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected no error, got %v", err)
|
||||||
|
}
|
||||||
|
if _, exists := observedBody["reasoning_effort"]; exists {
|
||||||
|
t.Fatalf("expected reasoning_effort omitted, got %#v", observedBody["reasoning_effort"])
|
||||||
|
}
|
||||||
|
if _, exists := observedBody["extra_params"]; exists {
|
||||||
|
t.Fatalf("expected extra_params omitted, got %#v", observedBody["extra_params"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestOpenAICompatibleClientNoAuthorizationHeaderWhenNoAPIKey(t *testing.T) {
|
func TestOpenAICompatibleClientNoAuthorizationHeaderWhenNoAPIKey(t *testing.T) {
|
||||||
hadAuth := false
|
hadAuth := false
|
||||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
|||||||
@@ -7,10 +7,10 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"sort"
|
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
|
"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
|
||||||
|
"gitea.maximumdirect.net/eric/scriptorium/internal/filecatalog"
|
||||||
"gopkg.in/yaml.v3"
|
"gopkg.in/yaml.v3"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -34,7 +34,7 @@ func (r *filesystemRepository) GetProfile(ctx context.Context, id string) (*doma
|
|||||||
return nil, fmt.Errorf("%w: profile id is required", ErrInvalidProfile)
|
return nil, fmt.Errorf("%w: profile id is required", ErrInvalidProfile)
|
||||||
}
|
}
|
||||||
|
|
||||||
files, err := r.yamlFiles(ctx)
|
files, err := filecatalog.FindYAMLFiles(ctx, r.dir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to read profile directory: %w", err)
|
return nil, fmt.Errorf("failed to read profile directory: %w", err)
|
||||||
}
|
}
|
||||||
@@ -47,24 +47,25 @@ func (r *filesystemRepository) GetProfile(ctx context.Context, id string) (*doma
|
|||||||
default:
|
default:
|
||||||
}
|
}
|
||||||
|
|
||||||
relPath := r.relativePath(fullPath)
|
relPath := filecatalog.RelativePath(r.dir, fullPath)
|
||||||
fileMatch := profileIDFromFileName(filepath.Base(fullPath)) == id
|
fileMatch := filecatalog.Stem(filepath.Base(fullPath)) == id
|
||||||
data, err := os.ReadFile(fullPath)
|
data, err := os.ReadFile(fullPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to read profile file %s: %w", relPath, err)
|
return nil, fmt.Errorf("failed to read profile file %s: %w", relPath, err)
|
||||||
}
|
}
|
||||||
|
metadata := readProfileFileMetadata(data)
|
||||||
|
idMatch := fileMatch || metadata.id == id
|
||||||
|
if metadata.hasRawAPIKey {
|
||||||
|
if idMatch {
|
||||||
|
return nil, fmt.Errorf("%w: %s", ErrRawAPIKeyNotAllowed, relPath)
|
||||||
|
}
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
var prof domain.ExecutionProfile
|
var prof domain.ExecutionProfile
|
||||||
decoder := yaml.NewDecoder(bytes.NewReader(data))
|
decoder := yaml.NewDecoder(bytes.NewReader(data))
|
||||||
decoder.KnownFields(true)
|
decoder.KnownFields(true)
|
||||||
if err := decoder.Decode(&prof); err != nil {
|
if err := decoder.Decode(&prof); err != nil {
|
||||||
idMatch := fileMatch || profileFileHasID(data, id)
|
|
||||||
if strings.Contains(err.Error(), "field api_key not found") {
|
|
||||||
if idMatch {
|
|
||||||
return nil, fmt.Errorf("%w: %s", ErrRawAPIKeyNotAllowed, relPath)
|
|
||||||
}
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
if idMatch {
|
if idMatch {
|
||||||
return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, relPath, err)
|
return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, relPath, err)
|
||||||
}
|
}
|
||||||
@@ -106,56 +107,36 @@ type profileMatch struct {
|
|||||||
path string
|
path string
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *filesystemRepository) yamlFiles(ctx context.Context) ([]string, error) {
|
type profileFileMetadata struct {
|
||||||
var files []string
|
id string
|
||||||
err := filepath.WalkDir(r.dir, func(path string, d os.DirEntry, err error) error {
|
hasRawAPIKey bool
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
select {
|
|
||||||
case <-ctx.Done():
|
|
||||||
return ctx.Err()
|
|
||||||
default:
|
|
||||||
}
|
|
||||||
if d.IsDir() {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
if !isYAMLFile(d.Name()) {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
files = append(files, path)
|
|
||||||
return nil
|
|
||||||
})
|
|
||||||
sort.Strings(files)
|
|
||||||
return files, err
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *filesystemRepository) relativePath(path string) string {
|
func readProfileFileMetadata(data []byte) profileFileMetadata {
|
||||||
rel, err := filepath.Rel(r.dir, path)
|
var node yaml.Node
|
||||||
if err != nil {
|
if err := yaml.NewDecoder(bytes.NewReader(data)).Decode(&node); err != nil {
|
||||||
return filepath.Clean(path)
|
return profileFileMetadata{}
|
||||||
}
|
}
|
||||||
return filepath.Clean(rel)
|
if node.Kind != yaml.DocumentNode || len(node.Content) == 0 {
|
||||||
}
|
return profileFileMetadata{}
|
||||||
|
|
||||||
func isYAMLFile(name string) bool {
|
|
||||||
return strings.HasSuffix(name, ".yaml") || strings.HasSuffix(name, ".yml")
|
|
||||||
}
|
|
||||||
|
|
||||||
func profileIDFromFileName(name string) string {
|
|
||||||
name = strings.TrimSuffix(name, ".yaml")
|
|
||||||
name = strings.TrimSuffix(name, ".yml")
|
|
||||||
return name
|
|
||||||
}
|
|
||||||
|
|
||||||
func profileFileHasID(data []byte, id string) bool {
|
|
||||||
var raw struct {
|
|
||||||
ID string `yaml:"id"`
|
|
||||||
}
|
}
|
||||||
if err := yaml.NewDecoder(bytes.NewReader(data)).Decode(&raw); err != nil {
|
mapping := node.Content[0]
|
||||||
return false
|
if mapping.Kind != yaml.MappingNode {
|
||||||
|
return profileFileMetadata{}
|
||||||
}
|
}
|
||||||
return strings.TrimSpace(raw.ID) == id
|
|
||||||
|
var metadata profileFileMetadata
|
||||||
|
for i := 0; i+1 < len(mapping.Content); i += 2 {
|
||||||
|
key := mapping.Content[i]
|
||||||
|
value := mapping.Content[i+1]
|
||||||
|
switch key.Value {
|
||||||
|
case "id":
|
||||||
|
metadata.id = strings.TrimSpace(value.Value)
|
||||||
|
case "api_key":
|
||||||
|
metadata.hasRawAPIKey = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return metadata
|
||||||
}
|
}
|
||||||
|
|
||||||
func validateProfile(p *domain.ExecutionProfile) error {
|
func validateProfile(p *domain.ExecutionProfile) error {
|
||||||
|
|||||||
@@ -133,6 +133,20 @@ api_key: secret
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("raw api_key in non-target profile is ignored", func(t *testing.T) {
|
||||||
|
writeProfileTestFile(t, filepath.Join(tmpDir, "raw-api-key-non-target.yaml"), `
|
||||||
|
id: raw-api-key-non-target
|
||||||
|
endpoint: http://localhost:8000/v1
|
||||||
|
model: m
|
||||||
|
api_key: secret
|
||||||
|
`)
|
||||||
|
|
||||||
|
_, err := repo.GetProfile(ctx, "does-not-exist-with-raw-key-nearby")
|
||||||
|
if !errors.Is(err, ErrProfileNotFound) {
|
||||||
|
t.Fatalf("expected ErrProfileNotFound for non-target raw api_key file, got %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
t.Run("invalid yaml", func(t *testing.T) {
|
t.Run("invalid yaml", func(t *testing.T) {
|
||||||
_, err := repo.GetProfile(ctx, "invalid_yaml")
|
_, err := repo.GetProfile(ctx, "invalid_yaml")
|
||||||
if !errors.Is(err, ErrInvalidYAML) {
|
if !errors.Is(err, ErrInvalidYAML) {
|
||||||
|
|||||||
@@ -7,10 +7,10 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"sort"
|
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
|
"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
|
||||||
|
"gitea.maximumdirect.net/eric/scriptorium/internal/filecatalog"
|
||||||
"gopkg.in/yaml.v3"
|
"gopkg.in/yaml.v3"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -63,7 +63,7 @@ func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id strin
|
|||||||
return nil, fmt.Errorf("%w: prompt id is required", ErrInvalidPromptDefinition)
|
return nil, fmt.Errorf("%w: prompt id is required", ErrInvalidPromptDefinition)
|
||||||
}
|
}
|
||||||
|
|
||||||
files, err := r.yamlFiles(ctx)
|
files, err := filecatalog.FindYAMLFiles(ctx, r.dir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to read prompt definition directory: %w", err)
|
return nil, fmt.Errorf("failed to read prompt definition directory: %w", err)
|
||||||
}
|
}
|
||||||
@@ -76,8 +76,8 @@ func (r *filesystemRepository) GetPromptDefinition(ctx context.Context, id strin
|
|||||||
default:
|
default:
|
||||||
}
|
}
|
||||||
|
|
||||||
relPath := r.relativePath(fullPath)
|
relPath := filecatalog.RelativePath(r.dir, fullPath)
|
||||||
fileMatch := promptIDFromFileName(filepath.Base(fullPath)) == id
|
fileMatch := filecatalog.Stem(filepath.Base(fullPath)) == id
|
||||||
|
|
||||||
raw, err := loadPromptDefinitionFile(fullPath)
|
raw, err := loadPromptDefinitionFile(fullPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -130,38 +130,6 @@ type promptDefinitionMatch struct {
|
|||||||
path string
|
path string
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *filesystemRepository) yamlFiles(ctx context.Context) ([]string, error) {
|
|
||||||
var files []string
|
|
||||||
err := filepath.WalkDir(r.dir, func(path string, d os.DirEntry, err error) error {
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
select {
|
|
||||||
case <-ctx.Done():
|
|
||||||
return ctx.Err()
|
|
||||||
default:
|
|
||||||
}
|
|
||||||
if d.IsDir() {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
if !isYAMLFile(d.Name()) {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
files = append(files, path)
|
|
||||||
return nil
|
|
||||||
})
|
|
||||||
sort.Strings(files)
|
|
||||||
return files, err
|
|
||||||
}
|
|
||||||
|
|
||||||
func (r *filesystemRepository) relativePath(path string) string {
|
|
||||||
rel, err := filepath.Rel(r.dir, path)
|
|
||||||
if err != nil {
|
|
||||||
return filepath.Clean(path)
|
|
||||||
}
|
|
||||||
return filepath.Clean(rel)
|
|
||||||
}
|
|
||||||
|
|
||||||
func loadPromptDefinitionFile(path string) (*promptDefinitionFile, error) {
|
func loadPromptDefinitionFile(path string) (*promptDefinitionFile, error) {
|
||||||
data, err := os.ReadFile(path)
|
data, err := os.ReadFile(path)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -306,16 +274,6 @@ func normalizePromptDefinition(raw *promptDefinitionFile, sourcePath string) (*d
|
|||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func isYAMLFile(name string) bool {
|
|
||||||
return strings.HasSuffix(name, ".yaml") || strings.HasSuffix(name, ".yml")
|
|
||||||
}
|
|
||||||
|
|
||||||
func promptIDFromFileName(name string) string {
|
|
||||||
name = strings.TrimSuffix(name, ".yaml")
|
|
||||||
name = strings.TrimSuffix(name, ".yml")
|
|
||||||
return name
|
|
||||||
}
|
|
||||||
|
|
||||||
func isValidOutputFormat(f domain.OutputFormat) bool {
|
func isValidOutputFormat(f domain.OutputFormat) bool {
|
||||||
switch f {
|
switch f {
|
||||||
case domain.FormatText, domain.FormatMarkdown, domain.FormatJSON:
|
case domain.FormatText, domain.FormatMarkdown, domain.FormatJSON:
|
||||||
|
|||||||
@@ -24,12 +24,14 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
var (
|
var (
|
||||||
ErrInvalidRequest = errors.New("invalid run request")
|
ErrInvalidRequest = errors.New("invalid run request")
|
||||||
ErrProfileLoad = errors.New("failed to load prompt definition")
|
ErrProfileRequired = errors.New("profile selection is required")
|
||||||
ErrArtifactLoad = errors.New("failed to load artifact")
|
ErrAPIKeyEnvMissing = errors.New("api_key_env points to an unset environment variable")
|
||||||
ErrPromptRender = errors.New("failed to render prompt")
|
ErrProfileLoad = errors.New("failed to load prompt definition")
|
||||||
ErrLLMGenerate = errors.New("failed to generate output")
|
ErrArtifactLoad = errors.New("failed to load artifact")
|
||||||
ErrValidation = errors.New("failed to validate output")
|
ErrPromptRender = errors.New("failed to render prompt")
|
||||||
|
ErrLLMGenerate = errors.New("failed to generate output")
|
||||||
|
ErrValidation = errors.New("failed to validate output")
|
||||||
)
|
)
|
||||||
|
|
||||||
// Runner executes the Scriptorium core use case.
|
// Runner executes the Scriptorium core use case.
|
||||||
@@ -177,7 +179,7 @@ func (r *Runner) Prepare(ctx context.Context, req domain.RunRequest) (*domain.Pr
|
|||||||
selectedProfileID = strings.TrimSpace(def.DefaultProfile)
|
selectedProfileID = strings.TrimSpace(def.DefaultProfile)
|
||||||
}
|
}
|
||||||
if selectedProfileID == "" {
|
if selectedProfileID == "" {
|
||||||
return nil, fmt.Errorf("%w: profile id is required either in request or prompt default_profile", ErrInvalidRequest)
|
return nil, fmt.Errorf("%w: %w: profile id is required either in request or prompt default_profile", ErrInvalidRequest, ErrProfileRequired)
|
||||||
}
|
}
|
||||||
|
|
||||||
execProfile, err := r.profiles.GetProfile(ctx, selectedProfileID)
|
execProfile, err := r.profiles.GetProfile(ctx, selectedProfileID)
|
||||||
@@ -376,7 +378,7 @@ func validateAPIKeyEnv(apiKeyEnv string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
if strings.TrimSpace(os.Getenv(envName)) == "" {
|
if strings.TrimSpace(os.Getenv(envName)) == "" {
|
||||||
return fmt.Errorf("api key environment variable %q is not set", envName)
|
return fmt.Errorf("%w: api key environment variable %q is not set", ErrAPIKeyEnvMissing, envName)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -234,6 +234,9 @@ func TestRunnerPrepareMissingExplicitProfileAndMissingDefaultProfileFails(t *tes
|
|||||||
if !errors.Is(err, ErrInvalidRequest) {
|
if !errors.Is(err, ErrInvalidRequest) {
|
||||||
t.Fatalf("expected ErrInvalidRequest, got %v", err)
|
t.Fatalf("expected ErrInvalidRequest, got %v", err)
|
||||||
}
|
}
|
||||||
|
if !errors.Is(err, ErrProfileRequired) {
|
||||||
|
t.Fatalf("expected ErrProfileRequired, got %v", err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRunnerPrepareSelectedProfileDoesNotExistFails(t *testing.T) {
|
func TestRunnerPrepareSelectedProfileDoesNotExistFails(t *testing.T) {
|
||||||
@@ -487,6 +490,35 @@ func TestRunnerPrepareJSONSchemaBuildsStructuredOutputSpec(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunnerPrepareJSONSchemaSchemaLoadFailureReturnsValidationError(t *testing.T) {
|
||||||
|
def := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 0)
|
||||||
|
def.Validation.SchemaPath = "missing.schema.json"
|
||||||
|
validator := &fakeValidator{schemaErr: errors.New("schema unavailable")}
|
||||||
|
runner := NewRunner(
|
||||||
|
&fakePromptRepo{def: def},
|
||||||
|
&fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{"exec": defaultExecutionProfile()}},
|
||||||
|
defaultArtifactReader(),
|
||||||
|
defaultRenderer(),
|
||||||
|
&fakeLLM{forbid: true},
|
||||||
|
validator,
|
||||||
|
)
|
||||||
|
|
||||||
|
_, err := runner.Prepare(context.Background(), domain.RunRequest{
|
||||||
|
PromptID: "p",
|
||||||
|
ProfileID: "exec",
|
||||||
|
Inputs: singleInputRef(),
|
||||||
|
})
|
||||||
|
if !errors.Is(err, ErrValidation) {
|
||||||
|
t.Fatalf("expected ErrValidation, got %v", err)
|
||||||
|
}
|
||||||
|
if validator.schemaLoads != 1 {
|
||||||
|
t.Fatalf("expected one schema load attempt, got %d", validator.schemaLoads)
|
||||||
|
}
|
||||||
|
if validator.schemaLoadPath != "missing.schema.json" {
|
||||||
|
t.Fatalf("expected schema path missing.schema.json, got %q", validator.schemaLoadPath)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestRunnerRunJSONSchemaSchemaLoadFailureFailsBeforeLLM(t *testing.T) {
|
func TestRunnerRunJSONSchemaSchemaLoadFailureFailsBeforeLLM(t *testing.T) {
|
||||||
def := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 0)
|
def := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 0)
|
||||||
def.Validation.SchemaPath = "missing.schema.json"
|
def.Validation.SchemaPath = "missing.schema.json"
|
||||||
@@ -862,6 +894,9 @@ func TestRunnerRunAPIKeyEnvMissingEnvironmentValueFailsClearly(t *testing.T) {
|
|||||||
if !errors.Is(err, ErrInvalidRequest) {
|
if !errors.Is(err, ErrInvalidRequest) {
|
||||||
t.Fatalf("expected ErrInvalidRequest, got %v", err)
|
t.Fatalf("expected ErrInvalidRequest, got %v", err)
|
||||||
}
|
}
|
||||||
|
if !errors.Is(err, ErrAPIKeyEnvMissing) {
|
||||||
|
t.Fatalf("expected ErrAPIKeyEnvMissing, got %v", err)
|
||||||
|
}
|
||||||
if !strings.Contains(err.Error(), "SCRIPTORIUM_MISSING_KEY") {
|
if !strings.Contains(err.Error(), "SCRIPTORIUM_MISSING_KEY") {
|
||||||
t.Fatalf("expected missing env name in error, got %v", err)
|
t.Fatalf("expected missing env name in error, got %v", err)
|
||||||
}
|
}
|
||||||
@@ -1122,6 +1157,174 @@ func TestRunnerRunJSONSchemaRepairCarriesStructuredOutputSpec(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestExecutionProfileToTargetPopulatesAllFieldsAndCopiesExtraParams(t *testing.T) {
|
||||||
|
src := &domain.ExecutionProfile{
|
||||||
|
ID: "exec",
|
||||||
|
Endpoint: "http://profile/v1",
|
||||||
|
Model: "profile-model",
|
||||||
|
Temperature: 0.2,
|
||||||
|
MaxTokens: 123,
|
||||||
|
TopP: 0.75,
|
||||||
|
TimeoutSeconds: 90,
|
||||||
|
ServiceTier: "priority",
|
||||||
|
ReasoningEffort: "medium",
|
||||||
|
APIKeyEnv: "SCRIPTORIUM_API_KEY",
|
||||||
|
ExtraParams: map[string]string{
|
||||||
|
"provider_option": "on",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
target := executionProfileToTarget(src)
|
||||||
|
if target.Endpoint != src.Endpoint ||
|
||||||
|
target.Model != src.Model ||
|
||||||
|
target.Temperature != src.Temperature ||
|
||||||
|
target.MaxTokens != src.MaxTokens ||
|
||||||
|
target.TopP != src.TopP ||
|
||||||
|
target.TimeoutSeconds != src.TimeoutSeconds ||
|
||||||
|
target.ServiceTier != src.ServiceTier ||
|
||||||
|
target.ReasoningEffort != src.ReasoningEffort ||
|
||||||
|
target.APIKeyEnv != src.APIKeyEnv {
|
||||||
|
t.Fatalf("expected all profile fields to populate target, got %+v", target)
|
||||||
|
}
|
||||||
|
if !reflect.DeepEqual(target.ExtraParams, src.ExtraParams) {
|
||||||
|
t.Fatalf("expected extra_params to match, got %#v", target.ExtraParams)
|
||||||
|
}
|
||||||
|
|
||||||
|
src.ExtraParams["provider_option"] = "changed"
|
||||||
|
if target.ExtraParams["provider_option"] != "on" {
|
||||||
|
t.Fatalf("expected extra_params copy to be independent, got %#v", target.ExtraParams)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestResolveExecutionTargetProfileValuesPopulateAllSupportedFields(t *testing.T) {
|
||||||
|
profileValue := &domain.ExecutionProfile{
|
||||||
|
ID: "exec",
|
||||||
|
Endpoint: "http://profile/v1",
|
||||||
|
Model: "profile-model",
|
||||||
|
Temperature: 0.3,
|
||||||
|
MaxTokens: 222,
|
||||||
|
TopP: 0.6,
|
||||||
|
TimeoutSeconds: 77,
|
||||||
|
ServiceTier: "priority",
|
||||||
|
ReasoningEffort: "low",
|
||||||
|
APIKeyEnv: "PROFILE_KEY",
|
||||||
|
ExtraParams: map[string]string{
|
||||||
|
"profile_option": "enabled",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
target := resolveExecutionTarget(profileValue, nil)
|
||||||
|
if target.Endpoint != profileValue.Endpoint ||
|
||||||
|
target.Model != profileValue.Model ||
|
||||||
|
target.Temperature != profileValue.Temperature ||
|
||||||
|
target.MaxTokens != profileValue.MaxTokens ||
|
||||||
|
target.TopP != profileValue.TopP ||
|
||||||
|
target.TimeoutSeconds != profileValue.TimeoutSeconds ||
|
||||||
|
target.ServiceTier != profileValue.ServiceTier ||
|
||||||
|
target.ReasoningEffort != profileValue.ReasoningEffort ||
|
||||||
|
target.APIKeyEnv != profileValue.APIKeyEnv {
|
||||||
|
t.Fatalf("expected profile values to populate target, got %+v", target)
|
||||||
|
}
|
||||||
|
if !reflect.DeepEqual(target.ExtraParams, profileValue.ExtraParams) {
|
||||||
|
t.Fatalf("expected profile extra_params in target, got %#v", target.ExtraParams)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestResolveExecutionTargetRuntimeOverridesBeatProfileForAllOverrideableFields(t *testing.T) {
|
||||||
|
profileValue := &domain.ExecutionProfile{
|
||||||
|
ID: "exec",
|
||||||
|
Endpoint: "http://profile/v1",
|
||||||
|
Model: "profile-model",
|
||||||
|
Temperature: 0.2,
|
||||||
|
MaxTokens: 200,
|
||||||
|
TopP: 0.8,
|
||||||
|
TimeoutSeconds: 90,
|
||||||
|
ServiceTier: "priority",
|
||||||
|
ReasoningEffort: "medium",
|
||||||
|
APIKeyEnv: "PROFILE_KEY",
|
||||||
|
ExtraParams: map[string]string{
|
||||||
|
"profile_only": "yes",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
override := &domain.ExecutionTarget{
|
||||||
|
Endpoint: "http://override/v1",
|
||||||
|
Model: "override-model",
|
||||||
|
Temperature: 0.9,
|
||||||
|
MaxTokens: 111,
|
||||||
|
TopP: 0.5,
|
||||||
|
TimeoutSeconds: 30,
|
||||||
|
ServiceTier: "flex",
|
||||||
|
ReasoningEffort: "high",
|
||||||
|
APIKeyEnv: "RUNTIME_KEY",
|
||||||
|
ExtraParams: map[string]string{
|
||||||
|
"runtime_only": "yes",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
target := resolveExecutionTarget(profileValue, override)
|
||||||
|
if target.Endpoint != override.Endpoint ||
|
||||||
|
target.Model != override.Model ||
|
||||||
|
target.Temperature != override.Temperature ||
|
||||||
|
target.MaxTokens != override.MaxTokens ||
|
||||||
|
target.TopP != override.TopP ||
|
||||||
|
target.TimeoutSeconds != override.TimeoutSeconds ||
|
||||||
|
target.ServiceTier != override.ServiceTier ||
|
||||||
|
target.ReasoningEffort != override.ReasoningEffort ||
|
||||||
|
target.APIKeyEnv != override.APIKeyEnv {
|
||||||
|
t.Fatalf("expected runtime overrides to win for all fields, got %+v", target)
|
||||||
|
}
|
||||||
|
if !reflect.DeepEqual(target.ExtraParams, override.ExtraParams) {
|
||||||
|
t.Fatalf("expected runtime extra_params to replace profile extra_params, got %#v", target.ExtraParams)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMergeExecutionTargetEmptyStringOverridesDoNotErase(t *testing.T) {
|
||||||
|
base := domain.ExecutionTarget{
|
||||||
|
Endpoint: "http://base/v1",
|
||||||
|
Model: "base-model",
|
||||||
|
ServiceTier: "priority",
|
||||||
|
ReasoningEffort: "medium",
|
||||||
|
APIKeyEnv: "BASE_KEY",
|
||||||
|
}
|
||||||
|
override := domain.ExecutionTarget{
|
||||||
|
Endpoint: "http://override/v1",
|
||||||
|
Model: "override-model",
|
||||||
|
ServiceTier: " ",
|
||||||
|
ReasoningEffort: " ",
|
||||||
|
APIKeyEnv: "",
|
||||||
|
}
|
||||||
|
|
||||||
|
merged := mergeExecutionTarget(base, override)
|
||||||
|
if merged.Endpoint != "http://override/v1" || merged.Model != "override-model" {
|
||||||
|
t.Fatalf("expected endpoint/model to override, got %+v", merged)
|
||||||
|
}
|
||||||
|
if merged.ServiceTier != "priority" {
|
||||||
|
t.Fatalf("expected empty service_tier override to be ignored, got %q", merged.ServiceTier)
|
||||||
|
}
|
||||||
|
if merged.ReasoningEffort != "medium" {
|
||||||
|
t.Fatalf("expected empty reasoning_effort override to be ignored, got %q", merged.ReasoningEffort)
|
||||||
|
}
|
||||||
|
if merged.APIKeyEnv != "BASE_KEY" {
|
||||||
|
t.Fatalf("expected empty api_key_env override to be ignored, got %q", merged.APIKeyEnv)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMergeExecutionTargetEmptyExtraParamsDoesNotErase(t *testing.T) {
|
||||||
|
base := domain.ExecutionTarget{
|
||||||
|
ExtraParams: map[string]string{
|
||||||
|
"keep": "value",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
override := domain.ExecutionTarget{
|
||||||
|
ExtraParams: map[string]string{},
|
||||||
|
}
|
||||||
|
|
||||||
|
merged := mergeExecutionTarget(base, override)
|
||||||
|
if !reflect.DeepEqual(merged.ExtraParams, base.ExtraParams) {
|
||||||
|
t.Fatalf("expected empty extra_params override not to erase base values, got %#v", merged.ExtraParams)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestBuildOutputArtifactDefaults(t *testing.T) {
|
func TestBuildOutputArtifactDefaults(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
|
|||||||
Reference in New Issue
Block a user