Finish cleanup roadmap follow-through
All checks were successful
ci/woodpecker/tag/release Pipeline was successful
All checks were successful
ci/woodpecker/tag/release Pipeline was successful
This commit is contained in:
@@ -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.
|
||||
Reference in New Issue
Block a user