diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 00000000..8044d55f --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,387 @@ +# ConfigKit And PromptKit Upgrade Implementation Plan + +## Purpose And Scope + +Implement [ConfigKit And PromptKit Upgrades](upgrade-kit.md). That document +owns the desired behavior and operator migration; this document owns execution +order. Each numbered stage is one bounded implementation prompt for a +gpt-5.6-terra coding agent. Complete and validate stages in order. + +The inspected baseline pins ConfigKit v0.3.0 and PromptKit v0.10.0. Adopt +exactly ConfigKit v0.4.0 and PromptKit v0.11.0, not whatever release happens to +be newest during implementation. The release guides and public APIs are +available in the sibling checkouts `../ConfigKit` and `../promptkit`; consult +the target tags or downloaded modules if those checkouts have advanced. +Build and test Notarius with `GOWORK=off`, without local dependency replacements. + +Read `docs/development.md` and all five policies in `docs/policy/` before +implementation. In particular, simplify machinery without narrowing operator +choices or elevating incidental details into requirements. Preserve existing +contracts unless this roadmap expressly changes them. Use graph discovery for +code, then inspect the relevant source and tests; locations below identify the +reviewed baseline, not a requirement to preserve private file structure. + +## Decisions And Boundaries + +- Configuration selection becomes `--config-overlay` and `overlays/.yml`, + with no legacy alias or fallback. Keep empty, omitted, and `default` selection + behavior. Do not rename PromptKit profiles or pipeline/runtime profile types. +- Keep configuration version 5, source-file-relative declarations, CLI override + precedence, strict loading, inspection provenance, and redaction. +- Add only the global `promptkit.transient_retries` setting: nonnegative integer, + default **2 additional attempts**, explicit zero disables. No arbitrary cap, + per-binding setting, new CLI retry flag, or new total-deadline setting. +- PromptKit owns transient HTTP retries and structural repairs; Notarius keeps + stage/validator retry budgets and disposition unchanged. The existing shared + scheduler retains its completion-scoped permit through upstream waiting. +- Add provider-neutral failure metadata and request-local execution summaries. + Do not change domain artifacts, result receipts, prompt content, validators, + acceptance policy, or introduce a new telemetry service/artifact. +- Successful retry recovery is ordinary success, not a warning. Debug remains + opt-in. Existing token fields keep their meaning; observed usage is separate. +- No release, tag, push, source-license change, or binary packaging is part of + these stages. Record upgrade implications in current integration/operator + documentation; the eventual release note is a separate release task. + +For each stage, inspect the starting worktree, implement code with its focused +tests and current-behavior documentation, run the listed relevant checks, and +make one coherent local commit under agent policy. Report checks and any gaps. +Do not repeat passed checks for unchanged content merely to make a commit. + +## Stage 1 — Cut Over Configuration Selection To Overlays + +**Goal:** Adopt ConfigKit v0.4.0 and finish its narrow operator-facing rename in +one coherent change. This is one prompt: it is a naming migration, not a new +configuration engine. + +**Inspect:** `internal/core/config/load.go`, `loaded_config.go`, configuration +document/path/operational tests; `internal/cli/run.go`, +`configkit_commands.go`, CLI configuration and example contract tests; +`examples/profiles/development.yml`; upstream v0.4.0 migration guide. + +**Work:** + +1. Upgrade ConfigKit to v0.4.0 in `go.mod`/`go.sum`. Change `LoadOptions.Profile` + to `Overlay` and pass it into `configkit.Options[Document].Overlay`. Rename + only helpers, fields, locals, fixtures, and descriptions representing the + selected configuration layer. +2. Replace `--config-profile` with `--config-overlay` for `run`, `config validate`, + `config effective`, `config explain`, and `pipelines list`. Update usage, + flag parsing, argument classification, explicit-empty handling, and error + paths together. Retired flags use normal unknown-flag behavior. +3. Move `examples/profiles/development.yml` to + `examples/overlays/development.yml`; update maintained-example discovery and + selected-layer fixtures. Check file-relative paths after moving. Leave + `examples/llm-profiles/`, PromptKit profile sources, and ordinary explicit + imports alone unless an import actually points at the moved example. +4. Consume upstream overlay provenance and diagnostic names directly. Do not + rebuild composition, implement directory fallback, or translate metadata + back to profile vocabulary. +5. Update `docs/config.md`, `docs/cli.md`, `docs/integrations/pkg-configkit.md`, + `docs/internal/configuration.md`, relevant portions of `docs/internal/cli.md`, + and `docs/consumers/subprocess.md`. Review README, operations, and other + consumer examples for affected invocations. Explain the clean migration and + distinction from LLM profiles. Preserve accepted ADR-0018 and release history. + +**Acceptance:** All five commands select overlays consistently. Representative +existing tests cover selected/base/empty/default behavior, overlay and explicit +override precedence, declaration-relative paths, redacted effective/explain +output, and source-aware failure labels. Include a selected legacy-only file +case proving there is no silent fallback. Retain ordinary-import behavior and +LLM profile coverage; do not duplicate every upstream parser case. + +**Validate:** `GOWORK=off go test ./internal/core/config ./internal/cli`, +`GOWORK=off go mod tidy -diff`, `git diff --check`. Review occurrences of +`config-profile` and `examples/profiles` in current docs/code semantically; +historical docs and explicit legacy-rejection tests are not defects. + +## Stage 2 — Upgrade The PromptKit Runtime Baseline + +**Goal:** Adopt the compatible upstream dependency and document its changed +default before adding Notarius controls. One small prompt. + +**Inspect:** `internal/framework/llm/promptkit_client.go`, asset/profile identity +helpers and tests, `go.mod`, and upstream v0.11.0 release/consumer documentation. + +**Work:** + +1. Pin PromptKit v0.11.0 and tidy. Keep the Go version and provider catalog + selections unless the actual dependency graph requires a change; the target + release does not require either. Do not alter embedded D&D model settings. +2. Update `promptKitBuiltinProfileCatalogID` to the v0.11.0 identity in accordance + with the existing checkpoint fingerprint convention. No other cache policy + change or blanket invalidation is part of this work. +3. Update the pinned integration links in `docs/integrations/pkg-promptkit.md`, + including the relocated upstream `docs/policy/release.md`. Record the + LGPL-3.0-or-later dependency license and reference its license/notices; do not + relicense Notarius or invent a new distribution workflow. +4. Remove the obsolete no-provider-retry-loop statement in `docs/internal/llm.md`. + Describe the now-active upstream default and per-attempt timeouts in the + appropriate integration/operations sections. Do not yet describe the setting + or observations from later stages as implemented. + +**Acceptance:** The prepared-execution adapter builds and its existing tests +pass without prompt/catalog changes. Current documentation acknowledges the +dependency's default retry behavior and license accurately. + +**Validate:** `GOWORK=off go test ./internal/framework/llm ./internal/cli`, +`GOWORK=off go mod tidy -diff`, `git diff --check`. + +## Stage 3 — Wire The Explicit Transient Retry Setting + +**Goal:** Make retry policy inspectable and configurable through the one shared +engine. One prompt covering a single vertical configuration slice. + +**Inspect:** `internal/core/config/{document,config,load,operational_document, +loaded_config,validation}.go`, effective-config tests; +`internal/cli/catalog.go:buildProductionLLMClient`; +`internal/framework/llm/promptkit_client.go:NewPromptKitClient` and HTTP tests. + +**Work:** + +1. Add `TransientRetries *int` to `PromptKitDocument` and a resolved integer to + `PromptKitConfig`. Supply 2 through both the document default path and the + runtime `Default()` path. Preserve explicit zero; never use zero as omission. + Include it in document-to-runtime conversion and effective configuration. +2. Reject negative values through both existing document and direct runtime + validation entry points. Follow current source-aware diagnostics; do not + impose a new maximum. Keep ConfigKit's existing decoding behavior. +3. Add `TransientRetries *int` to `PromptKitClientConfig`: nil follows upstream + default, non-nil includes explicit zero. Production construction always + supplies the resolved configuration value. Pass it into `promptkit.Config`, + not per-request options. All calls through the injected shared client thus + share the setting without module changes. +4. Add the setting to `examples/dnd-complete.config.yml` and `docs/config.md`; + keep the minimal example minimal. Update configuration/LLM internal guides + for the new plumbing and operations for the zero-value opt-out. + +**Acceptance/tests:** Demonstrate omitted default, explicit zero, a positive +override, negative rejection, and effective/explain inspection. Use an existing +CLI factory test to prove the resolved value reaches construction. Exercise +built-in generation with a local HTTP server: eligible failure then success, +disabled retries, exhausted configured count, and a noneligible 400 response. +An injected `LLMClient` stub does not test built-in HTTP retry wiring. Avoid +testing the upstream jitter formula or every equivalent HTTP status. + +**Validate:** `GOWORK=off go test ./internal/core/config ./internal/cli ./internal/framework/llm` +and `git diff --check`. + +## Stage 4 — Preserve Typed Failure And Exhaustion Metadata + +**Goal:** Distinguish provider failures without coupling callers to PromptKit. +One prompt; defer execution observations to the next stage. + +**Inspect:** `internal/framework/contracts/errors.go`, adapter error conversion +in `promptkit_client.go`, redaction helpers, and existing pipeline handling of +`ErrLLMGeneration`, `ErrLLMCapacityExceeded`, and invalid structured output. + +**Work:** + +1. Extend the existing application-owned `LLMGenerationError` rather than + creating a parallel provider-error hierarchy. Retain `StatusCode()` and + `errors.Is(err, ErrLLMGeneration)`. Add application-owned failure-kind, + exhaustion, and exhausted-generation attempt metadata/accessors; use uint64 + for upstream attempt counts. Preserve the existing constructor's callers + with sensible unspecified/generation metadata, adding a metadata constructor + or private adapter helper as needed. +2. Define kinds for `transport`, `provider_status`, `malformed_response`, and + generic `generation`. Exhaustion is an independent boolean and attempt count, + not a replacement kind. Zero attempts means unavailable in this error + projection, not proof no request happened. The count comes from + `RetryExhaustedError.Attempts()` and covers the exhausted generation, not all + repair generations combined. +3. Use `errors.Is`/`errors.As` on upstream sentinels/types. Keep cancellation and + deadline identity first, capacity handling intact, and HTTP status available + through an exhaustion wrapper. Do not unwrap application errors to upstream + provider objects or classify by prose. Preserve sanitized diagnostic text. +4. A malformed provider envelope is a generation failure; schema-invalid model + content stays `ErrInvalidStructuredOutput`. Do not change outer retry or + validator disposition logic. Document classification at the integration and + internal LLM boundaries. + +**Acceptance/tests:** Adapter tests cover transport, malformed response, +noneligible status, exhausted eligible status with count/status preserved, and +cancellation/deadline/capacity regression behavior. Check typed fields and +identities, not full messages. Existing credential-redaction cases remain valid +for nested exhaustion diagnostics; use fake secrets only. + +**Validate:** `GOWORK=off go test ./internal/framework/contracts ./internal/framework/llm ./internal/framework/pipeline` +and `git diff --check`. + +## Stage 5 — Project Request-Local Execution Observations + +**Goal:** Return a lightweight terminal summary even when generation fails. +One prompt focused on contracts and adapter mapping, without persistence changes. + +**Inspect:** `internal/framework/contracts/contracts.go`, `CompleteStructured`, +`responseFromResult`, prepared execution tests, upstream `observations.go` and +prepared-execution observer documentation. + +**Work:** + +1. Add an optional `Execution *LLMExecutionSummary` field to + `StructuredCompletionResponse` (`execution,omitempty`). Own these DTOs in + Notarius contracts; do not alias or serialize upstream event types. +2. The summary contains uint64 `attempts`, `transient_retries`, `repair_calls`; + integer millisecond `capacity_wait_ms`, `generation_ms`, `retry_delay_ms`, + `duration_ms`; `outcome`, `failure_kind`, `status_code`, `retries_exhausted`; + and `usage`. Usage contains prompt/completion/total/cached/cache-write counts, + each with `value`, `known`, `complete` fields. Do not omit these booleans or + known-zero values. The summary needs no prompts, response text, endpoints, + credentials, raw errors, or new copies of run/profile identifiers. +3. Map terminal outcome/failure values explicitly to application-owned strings. + Preserve the distinctions exposed by v0.11.0 (including canceled, deadline, + capacity, invalid output, configuration/load/render failures); unknown future + failures map to `other`. Reuse relevant application failure vocabulary from + Stage 4, without forcing every execution error into a generation error. +4. Attach a local observer to the `RunRequest` before `PrepareExecution` freezes + it. Retain the `ExecutionFinished` projection only: upstream supplies aggregate + counts, durations, and usage. Do not sum attempt events and terminal totals, + retain an event log, or reconstruct a second accounting system. Synchronous + request-local collection needs no global state or asynchronous queue. +5. Attach that summary to every subsequent return from `RunPrepared`, including + context/capacity/provider failure and post-result decode/validation failure. + Preparation failure has no execution event and returns no summary. Preserve + upstream execution outcome as such: a later Notarius decode error remains a + separate returned error, not a fabricated upstream terminal event. +6. Keep existing successful-result token and repair fields unchanged. Observed + `repair_calls` represents execution activity and is not a renamed legacy + `RepairAttempts`. No observation field is a billable-cost guarantee. + +**Acceptance/tests:** Success, failed execution, cancellation, and no-observation +preparation failure retain the right presence. Test known zero versus unknown +and partial aggregate usage with controlled provider responses. One concurrent +completion case proves no summary cross-contamination. Check relationships and +nonnegative durations, not exact timing. Document the DTO and mapping in +`docs/internal/llm.md`; public dependency semantics stay in pkg-promptkit. + +**Validate:** `GOWORK=off go test ./internal/framework/contracts ./internal/framework/llm` +and `GOWORK=off go test -race ./internal/framework/llm`. + +## Stage 6 — Carry Summaries Through Existing Debug Paths + +**Goal:** Make observations useful to operators without a new output channel. +One prompt, limited to debug propagation and terminal allowlists. + +**Inspect:** `internal/framework/pipeline/debug.go`, its response-content/fallback +helpers, terminal recording tests, and `debugLLMScope.references`. Trace +`debugLLMCallReference` through the existing timed stage/terminal envelopes; +updating only the detailed response file is insufficient. The CLI's separate +run-summary files do not need a new run-wide accounting mechanism. + +**Work:** + +1. Project the optional execution summary into existing debug response artifacts + and per-call references used by stage/terminal records. Explicitly preserve it + when `Debug.Response` or `RunResult` is absent and through content splitting + and fallback construction. Keep copies detached if existing clone paths + could otherwise share mutable pointers. +2. Use explicit typed fields as the allowlist for this metadata-only terminal + projection. Do not carry raw upstream events, newly copied provider diagnostics, or model + content into safe terminal summaries. Preserve established redaction and + detailed-trace content boundaries. +3. Leave warnings, normal console output, receipts, domain artifacts, checkpoint + payloads, and no-debug behavior unchanged. No new telemetry file or aggregation + of observed token totals into legacy totals. +4. Update `docs/internal/state.md`/relevant debug internals and `docs/operations.md` + to identify where this diagnostic information is available. Link to the + canonical explanation rather than repeating every field in consumer docs. + +**Acceptance/tests:** Through the real debug wrapper and existing recorder seam, +verify success and failed/canceled completions retain summaries in response and +safe terminal records. Verify content splitting/fallback does not discard them, +and debug disabled writes nothing. Reuse existing redaction tests, adding only +the new projection assertions. No new warnings result from recovered retries. + +**Validate:** `GOWORK=off go test ./internal/framework/pipeline ./internal/cli`, +`GOWORK=off go test -race ./internal/framework/pipeline`, `git diff --check`. + +## Stage 7 — Verify Retry Composition And Finish Operator Guidance + +**Goal:** Validate the cross-boundary behavior that individual stages cannot +prove, then reconcile current documentation. One prompt; no generalized retry +test framework or runtime redesign. + +**Inspect:** Existing adapter HTTP fixtures, scheduler cancellation tests, +pipeline stage retry/debug tests, CLI production tests; current config, LLM, +operations, and consumer documentation. + +**Work and acceptance:** + +- Add one representative local-HTTP scenario combining a recoverable transient + status with structurally invalid generated content followed by a successful + repair. Prove transient budgets apply independently per generation and + terminal attempt/repair/usage accounting is not doubled. Use small explicit + budgets rather than executing the illustrative 18-request worst case. +- Exercise cancellation during provider-directed retry waiting without a long + real sleep: signal that the request was received, cancel a context while a + long Retry-After is pending, and wait with a bounded test timeout. Preserve + cancellation identity and the terminal observation. Do not build a second + backoff implementation or test exact random delays. +- Review shared scheduler tests for completion-scoped permit ownership and + cancellation/release. Add a focused integration case only where existing + coverage does not protect that boundary across the new retry path. Keep + upstream backend capacity algorithms upstream. +- Confirm a recovered generation passes through ordinary success without + producing process warnings. Preserve existing outer stage retries and semantic + feedback. Do not alter limits to hide budget multiplication. +- Finish operations guidance: distinguish transient, structural, and stage/ + validator budgets; give the 18-attempt single-producer illustration with its + assumptions; explain per-attempt timeout versus overall caller cancellation, + Retry-After waits, cost/latency implications, incomplete usage accounting, and + the outer scheduler holding its permit during waits. No new total deadline or + exactly-once guarantee is implied. +- Review all canonical docs changed earlier for consistency. Keep exact fields + and defaults in config, flags in CLI, dependency semantics in integrations, + mechanism in internals, and operating implications in operations. Keep the + migration note about overlays distinct from unchanged LLM profiles. Retain + historical ADR/release content; no new ADR is needed for these upgrades. + +**Validate:** Run the focused adapter/pipeline/CLI tests and race tests covering +modified concurrency paths. Review tests for unnecessary upstream duplication +or exact-prose/change-detector assertions; consolidate rather than multiply them. + +## Stage 8 — Final Integration Verification And Handoff + +**Goal:** Verify the complete upgraded application and documentation against the +feature roadmap. One final verification prompt; fix only defects within scope. + +**Work:** Review all feature acceptance items and staged diffs. Confirm no +upstream types escaped the LLM boundary, no references to the retired config +flag remain in current examples/help, no unintended LLM profile rename occurred, +and no source-only/license/release scope expanded. Check edited relative links. +Leave the roadmap available for the user's completion review; do not publish a +release or delete planning documents automatically. + +**Validation commands:** + +```sh +GOWORK=off go test ./... +GOWORK=off go vet ./... +GOWORK=off go build ./... +GOWORK=off go mod tidy -diff +GOWORK=off go test -race ./internal/framework/llm ./internal/framework/pipeline +GOWORK=off go run ./cmd/notarius config validate --config examples/dnd-minimal.config.yml +GOWORK=off go run ./cmd/notarius config validate --config examples/dnd-complete.config.yml +GOWORK=off go run ./cmd/notarius config validate --config examples/dnd-complete.config.yml --config-overlay development +git diff --check +``` + +Reuse already-passed checks for the same unchanged candidate where appropriate. +The example/CLI tests own maintained examples; the commands above are a one-time +migration smoke check, not new permanent release gates. No live model calls or +real API keys are required. Report environment/dependency-access failures +separately from implementation failures; do not claim blocked checks passed. + +**Completion criteria:** Both exact dependency pins are active; overlay migration, +retry setting, typed errors, observations, debug propagation, tests, examples, +and current documentation satisfy the feature roadmap. Report commits and +checks, plus any residual limitation. Future release preparation should explain +the breaking overlay cutover, retry default/opt-out, and upstream license change +in a minor-release note, but is not part of this implementation. + +## Open Questions + +None. The feature roadmap supplies the policy choices needed for implementation. +The DTO naming and stage boundaries above settle remaining implementation choices +without adding new product restrictions or compatibility promises. diff --git a/docs/roadmap/upgrade-kit.md b/docs/roadmap/upgrade-kit.md index 6549b526..af52faf2 100644 --- a/docs/roadmap/upgrade-kit.md +++ b/docs/roadmap/upgrade-kit.md @@ -12,8 +12,9 @@ Adopt ConfigKit v0.4.0 and PromptKit v0.11.0 in one coordinated work set: Preserve application/framework boundaries and keep the two upgrades as distinct implementation workstreams. Neither requires a redesign of pipeline execution. -This is a feature roadmap describing the target state and required changes, -not an implementation sequence. It does not authorize release publication. +This feature roadmap owns the target state and scope. The ordered work and +stage-specific verification belong to the [implementation plan](implementation.md). +Neither document authorizes release publication. ## Current Baseline