From ebf1602635e108e2a7ac1abd3a3ca24a620104ce Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 14:05:30 +0000 Subject: [PATCH] Prepare the codebase audit plan --- docs/roadmap/audit-sequence.md | 635 +++++++++++++++++++ docs/roadmap/deferred.md | 21 - docs/roadmap/structured-generation-errors.md | 71 +++ 3 files changed, 706 insertions(+), 21 deletions(-) create mode 100644 docs/roadmap/audit-sequence.md create mode 100644 docs/roadmap/structured-generation-errors.md diff --git a/docs/roadmap/audit-sequence.md b/docs/roadmap/audit-sequence.md new file mode 100644 index 0000000..08cb227 --- /dev/null +++ b/docs/roadmap/audit-sequence.md @@ -0,0 +1,635 @@ +# Codebase Audit Sequence + +## Purpose + +This document defines the staged sequence for auditing Promptkit before further +feature development. The audit is intended to identify high-confidence +opportunities to improve correctness, efficiency, duplication, implementation +clarity, and test-suite quality without changing production behavior during the +review itself. + +The audit findings belong in `audit.md`. A later, separate planning pass will +translate accepted findings into a staged remediation plan in +`implementation.md`. Neither this sequence nor the findings log owns current +behavior; the canonical sources identified by the +[documentation policy](../policy/documentation.md) remain authoritative. + +Each stage below is deliberately scoped for one LLM coding-agent prompt. Run +the stages in order and do not combine them. A stage may discover a concern +outside its scope, but it should record that concern for the owning later stage +rather than expanding its own review. + +## Governing Policies And Boundaries + +Every stage must follow: + +- the [development guide](../development.md), including its task-specific + reading guide; +- the [architecture policy](../policy/architecture.md), especially the public + facade, internal-package, dependency-direction, and consumer boundaries; +- the [testing policy](../policy/testing.md), including its risk-based, + behavior-oriented standard; and +- the [documentation policy](../policy/documentation.md), including canonical + ownership and the temporary nature of roadmap documents. + +This is an audit, not an implementation pass: + +- Do not change production code, tests, examples, fixtures, public contracts, + or current-state documentation. +- Limit repository edits to the audit artifacts explicitly authorized for the + stage. +- Do not silently repair an issue while investigating it. +- Do not treat coverage, complexity, similarity, lint, or graph output as a + finding without confirming the underlying behavior in source and tests. +- Do not recommend centralization merely because code looks similar. The code + must implement the same semantic rule, and consolidation must improve + ownership or reduce a credible drift risk. +- Do not recommend performance work without identifying a relevant execution + path and establishing a defensible cost model, measurement, or complexity + problem. +- Preserve unrelated working-tree changes. Record the audit baseline rather + than requiring an otherwise unrelated dirty tree to be cleaned. + +## Finding Standard + +Record each actionable finding in `audit.md` with: + +- a stable ID in the form `SNN-FNN`, where the first number is the stage; +- category: correctness, efficiency, duplication, clarity, testing, or + contract-documentation consistency; +- severity: critical, high, medium, or low; +- confidence: confirmed, high, medium, or low; +- affected packages, files, symbols, and tests; +- the contract, invariant, policy, or maintenance concern at issue; +- concrete evidence and a concise explanation of the failure mode or cost; +- the recommended direction, without implementation-level sequencing; +- the verification or regression protection that remediation would require; + and +- status: accepted, deferred, rejected, superseded, or resolved. + +Use **confirmed** confidence when the problem is reproduced or follows +unavoidably from a complete trace. Use **high** confidence when direct source +and test evidence establishes the problem but a safe reproduction is not +practical. Medium- and low-confidence concerns belong in a separate +observations section until a later stage confirms or rejects them; they must +not enter the remediation plan as if they were findings. + +Severity describes impact, not implementation effort: + +- **Critical:** credible data disclosure, data corruption, deadlock, unbounded + resource consumption, or a broadly unusable public contract. +- **High:** violation of an important public contract or invariant, a likely + concurrency or resource-lifecycle defect, or a failure with substantial + downstream impact. +- **Medium:** a real but narrower behavioral defect, meaningful avoidable cost, + duplicated policy with credible drift risk, or a material testing gap. +- **Low:** a bounded clarity, maintainability, or testing-friction problem with + a concrete improvement and little behavioral risk. + +When a reviewed area yields no finding, record the important behavior or risk +that was inspected and found adequately implemented or tested. This coverage +ledger prevents later reviewers from mistaking silence for omission. + +## Per-Stage Procedure + +Unless a stage says otherwise, its single agent prompt should: + +1. Read the required policies, focused internal documentation, production + files, and tests for that stage. +2. Use the code knowledge graph for symbol discovery, callers, callees, and + cross-package traces; confirm important conclusions against source. +3. Trace normal, boundary, and failure paths through the narrowest relevant + public or package contract. +4. Review correctness, meaningful runtime cost, semantic duplication, + responsibility clarity, and the value and ownership of tests in scope. +5. Run the narrowest existing tests needed to validate conclusions. Use + race-enabled or repeated focused tests when concurrency or nondeterminism is + in scope. Do not add permanent tests during the audit. +6. Add the stage result to `audit.md`: accepted findings, unresolved + observations, areas verified, commands run, and any handoff to a later + stage. +7. Recheck the working tree and confirm that only the authorized audit artifact + changed. + +## Stage 0: Initialize The Audit And Establish The Baseline + +Create `audit.md` and establish a reproducible starting point before reviewing +individual components. + +Record: + +- the audited commit, branch, Go version, module identity, and working-tree + state; +- unrelated pre-existing changes that all later stages must preserve; +- the implemented package and public-facade inventory; +- the baseline validation results; and +- the finding template, status vocabulary, and coverage ledger used by later + stages. + +Refresh the code knowledge graph for the recorded commit. Run the repository's +ordinary tests, race tests, vet, build, maintained offline preparation example, +Go formatting check, Markdown link check, and repository-hygiene checks. Run +package coverage once as a diagnostic and record the result without defining a +coverage target or committing generated output. Measure coarse package test +duration only if it can be done without adding tooling or changing tests. + +Compare the validation requirements stated by the testing policy, development +guide, and release procedure. Record a finding if their ownership or command +sets are materially inconsistent; do not edit those documents in this stage. + +**Exit condition:** `audit.md` contains the baseline, ledger structure, and +validation result, and no component-level audit has begun. + +## Stage 1: Public Values, Conversion, Errors, And Formatting + +Review the root facade's public request, result, inspection, prepared-run, and +error values together with public-to-internal and internal-to-public +conversion. Scope the review to `doc.go`, `types.go`, `convert.go`, `errors.go`, +`capacity_error.go`, `formatting.go`, and `prepared_execution.go`, plus the +directly relevant portions of root tests. + +Focus on: + +- zero-value and nil behavior; +- defensive copying, aliasing, and immutable snapshots; +- lossless conversion and field precedence; +- error identity through `errors.Is` and `errors.As`; +- containment of internal representations; +- safe `String`, `GoString`, and diagnostic formatting; +- accidental disclosure of credentials, prompt content, generated content, or + other private state; and +- conversion or copying logic that represents the same rule in multiple + places. + +Review only tests that own these value and boundary contracts. Defer engine +assembly, execution coordination, and transport behavior to their later +stages. + +**Exit condition:** all root value-conversion and error-formatting paths have a +recorded audit result without evaluating engine orchestration. + +## Stage 2: Public Configuration And Extension Adapters + +Review the smaller public construction and extension surfaces in +`backends.go`, `profiles.go`, `artifact_reader.go`, `json.go`, and +`llm_adapter.go`, together with their directly relevant root and internal +adapter tests. + +Focus on: + +- validation performed at the public boundary; +- ownership and copying of caller-supplied maps, slices, filesystems, readers, + and clients; +- adapter error propagation and cancellation; +- consistency between convenience constructors and general configuration; +- whether extension interfaces are as narrow as their consumers require; +- whether public helpers duplicate internal policy or merely translate it; + and +- whether tests protect consumer-visible behavior rather than private adapter + choreography. + +Do not review how `NewEngine` combines these values; that belongs to Stage 3. + +**Exit condition:** every non-engine public configuration helper and adapter +has a recorded result and any assembly questions are handed to Stage 3. + +## Stage 3: Engine Construction, Options, And Source Assembly + +Review the construction and configuration portions of `engine.go` and the +corresponding tests in `engine_test.go`. Limit the scope to `NewEngine`, option +application, dependency defaults, backend registration, profile and prompt +source composition, fallback-profile placement, validator and client +selection, capacity-manager construction, and construction-time validation. + +Focus on: + +- deterministic option precedence; +- required versus optional dependencies; +- isolation between engine instances; +- freezing or copying consumer configuration at the correct boundary; +- correct dependency direction and absence of process-global mutable state; +- failure atomicity and useful public errors; +- consistency between configured backends and capacity policies; and +- assembly logic that is repeated or split across unclear owners. + +Do not audit the runtime behavior of `Run`, `Prepare`, or inspection methods; +that belongs to Stage 4 and the internal use-case stages. + +**Exit condition:** engine construction and source assembly are fully accounted +for, including tests, without expanding into runtime orchestration. + +## Stage 4: Engine Operations And Root Contract Coverage + +Review the remaining public methods in `engine.go` and their directly relevant +root tests, including the external-package contracts in +`public_contract_test.go` and `prepared_execution_contract_test.go` only where +they exercise the engine boundary under review. + +Focus on: + +- request translation and context propagation; +- ordinary run, preparation, inspection, and prepared-execution entry points; +- public error mapping and preservation of injected dependency errors; +- result and prepared-state ownership; +- consistency between method and package-level convenience functions; +- public behavior that is asserted redundantly in root internal tests and + external-package contract tests; and +- important public behavior that is tested only through internal packages. + +Treat internal runner, transport, validation, and capacity mechanics as black +boxes in this stage. Hand questions about their implementation to their owning +later stages. + +**Exit condition:** the public execution boundary and its contract-test +ownership are recorded without duplicating internal component audits. + +## Stage 5: Internal Domain And JSON-Compatible Values + +Review `internal/domain` and `internal/jsonvalue`, including all of their tests. + +Focus on: + +- domain invariants and invalid states; +- session normalization; +- prepared-run and schema immutability; +- deep-copy correctness for every supported JSON-compatible shape; +- numeric-type preservation and rejection policy; +- cycles, excessive nesting, unsupported values, and nil distinctions; +- avoidable repeated copying on execution paths; and +- whether generic value machinery has a single clear owner. + +Trace important callers to confirm that these packages enforce the invariants +their consumers assume, but do not audit the callers' broader behavior. + +**Exit condition:** shared value semantics and their test ownership are fully +recorded. + +## Stage 6: Backend Registry, Defaults, And Built-In Profiles + +Review `internal/backend`, `internal/defaults`, and +`internal/profile/builtin`, including their focused tests and the relevant +backend-policy traces into engine assembly and the LLM reserved-field rule. + +Focus on: + +- immutable registry construction and lookup; +- built-in versus consumer ID collision rules; +- endpoint, credential-environment, header, parameter, and concurrency + validation; +- defensive copies at registry boundaries; +- application-neutral default ownership; +- built-in profile/backend consistency; +- reserved request-field ownership without dependency inversion; and +- duplicated validation or default policy across public and internal layers. + +Defer scheduling mechanics to Stage 15 and actual HTTP request construction to +Stage 14. + +**Exit condition:** registry and default-policy correctness are recorded, with +transport and scheduling questions handed to their owning stages. + +## Stage 7: File Discovery And Prompt Definitions + +Review `internal/filecatalog` and `internal/promptdef`, including their tests +and fixtures. Read the framework format reference and internal source document +before evaluating behavior. + +Focus on: + +- deterministic discovery and duplicate handling; +- filesystem and `fs.FS` parity; +- root and relative-path normalization; +- strict YAML decoding and version selection; +- prompt ID, message, input, cache-control, and validation declarations; +- inline versus file-backed content rules; +- containment of referenced files where promised; +- malformed input and contextual error behavior; +- unnecessary repeated directory scans or file reads; and +- fixture and case duplication that does not protect distinct parser risks. + +Do not audit rendering, artifact loading, profile loading, or schema validation +in this stage. + +**Exit condition:** discovery and prompt-definition parsing have complete +findings and coverage-ledger entries. + +## Stage 8: Profile Sources And Repository Composition + +Review `internal/profile` excluding its built-in subpackage, including all +repository tests and profile fixtures. Read the profile format contract first. + +Focus on: + +- strict decoding and profile validation; +- filesystem and `fs.FS` parity; +- repository overlay and fallback precedence; +- distinction between absence and a malformed authoritative source; +- preservation of useful error identity and context; +- conversion to immutable execution profiles; +- duplicate IDs and deterministic selection; +- repeated parsing, validation, or copying; and +- whether tests at repository, engine, and public-contract layers have clear, + nonduplicative ownership. + +Defer resolution of a profile with runtime overrides and backend definitions to +Stage 11. + +**Exit condition:** profile-source and repository-composition behavior are +fully recorded. + +## Stage 9: Artifact Loading And Prompt Rendering + +Review `internal/artifact` and `internal/prompt`, including all focused tests. +Read the internal source document and format reference first. + +Focus on: + +- inline and file artifact ownership, metadata, hashing, and error behavior; +- copied versus shared byte storage; +- caller-selected path semantics and architecture-policy boundaries; +- template parsing and execution; +- artifact, variable, session, and cache-control rendering; +- missing, extra, nil, and malformed input behavior; +- deterministic output and safe diagnostics; +- unnecessary repeated reads, hashes, parses, or allocations on common paths; + and +- tests coupled to incidental template or struct implementation. + +Do not audit the runner's decision about when rendering occurs. + +**Exit condition:** input materialization and rendering are accounted for +through their package boundaries. + +## Stage 10: Output Validation And Frozen Validation Plans + +Review `internal/validate`, including all tests, schema fixtures used by the +root contract suite, and traces from preparation into frozen validation plans. +Read the format and internal source documents first. + +Focus on: + +- basic, JSON, and JSON Schema mode semantics; +- schema-path resolution and filesystem/`fs.FS` parity; +- schema compilation, transitive references, and source-lifetime independence; +- output normalization and preservation; +- malformed schema and malformed model-output errors; +- thread safety of reusable validators and prepared plans; +- expensive recompilation or copying on repeated execution; and +- whether parser, validator, runner, and public tests each own distinct risks. + +Do not audit repair decisions or provider request construction. + +**Exit condition:** validation behavior, plan lifetime, and focused test value +are fully recorded. + +## Stage 11: Inspection And Execution-Target Resolution + +Review `internal/usecase/profile_inspection.go`, +`internal/usecase/prompt_inspection.go`, and the preparation and target- +resolution portions of `internal/usecase/runner.go`, together with their +focused tests. Use graph traces to define the exact helper and call-path scope +before reviewing. + +Focus on: + +- prompt and profile selection; +- backend lookup and endpoint overrides; +- reasoning, session, and other runtime precedence; +- merge semantics for default, profile, backend, and per-run values; +- inspection fidelity versus actual execution; +- credential-name versus credential-value handling; +- prompt-definition and schema freezing during preparation; +- stable error identity and context; and +- duplicated resolution rules across inspection, preparation, and execution. + +Do not review model invocation, repair execution, or prepared-handle lifecycle; +those belong to Stages 12 and 13. + +**Exit condition:** all selection, merge, inspection, and preparation rules are +traced and recorded once. + +## Stage 12: Ordinary Execution, Validation, And Repair Coordination + +Review `internal/usecase/runner.go`, `internal/usecase/repairer.go`, and +`internal/usecase/capacity_error.go` only for the ordinary execution path after +preparation, together with the corresponding sections of `runner_test.go`. +Use the Stage 11 resolution result as an established input rather than +reauditing it. + +Focus on: + +- rendering, generation, validation, and optional repair transitions; +- context cancellation and dependency-error propagation; +- partial result and usage accounting; +- exact attempt count and repair eligibility; +- avoidance of unintended retries; +- capacity-error translation; +- cleanup and failure behavior on every exit path; +- repeated orchestration or request construction; and +- oversized tests, helpers, or case matrices that obscure distinct behavior. + +Treat LLM transport and capacity scheduling as injected package contracts; +their mechanics belong to Stages 14 and 15. + +**Exit condition:** the ordinary execution state machine and its test ownership +are fully recorded. + +## Stage 13: Prepared Execution Lifecycle + +Review `internal/usecase/prepared_execution.go`, its focused tests, and the +prepared-execution portions of the root facade and external contract tests. +Do not repeat the public value review from Stages 1 and 4 or the resolution +review from Stage 11. + +Focus on: + +- single-attempt or other lifecycle guarantees; +- concurrent use and synchronization; +- discard behavior and resource release; +- frozen source, target, credential, capacity, timing, and schema semantics; +- independence of returned details and results; +- context and error behavior; +- consistency between ordinary and prepared execution where promised; +- private-state containment in formatting; and +- redundant assertions across internal, root, and external-package tests. + +Run focused race tests and repeated tests for lifecycle behavior where useful. + +**Exit condition:** prepared execution has one complete lifecycle analysis and +a clear map of which test layer owns each guarantee. + +## Stage 14: OpenAI-Compatible Transport + +Review `internal/llm`, including all transport tests. Read the +OpenAI-compatible integration contract and internal LLM document first. + +Focus on: + +- request endpoint, headers, authentication, and JSON body construction; +- omission versus explicit zero-value behavior; +- reserved-field enforcement and extra-parameter collision handling; +- session ID and reasoning encoding; +- structured-output and cache-control translation; +- client and per-generation deadlines; +- cancellation, body closure, bounded response reads, and decode failures; +- non-success HTTP response behavior; +- response choices, usage, and malformed-success handling; +- wire-visible compatibility and safe error disclosure; +- unnecessary marshaling, copying, or buffering; and +- whether the large transport test file can be simplified without losing + protocol-risk coverage. + +Use `httptest`-based existing tests; do not contact a live provider. + +**Exit condition:** every outbound and inbound wire path has a recorded result, +including focused test ownership. + +## Stage 15: Capacity, Admission, And Concurrency + +Review `internal/capacity`, its tests, `capacity_contract_test.go`, and the +integration points already identified in engine and use-case stages. Read the +internal capacity document first. + +Focus on: + +- bounded run admission and queue-capacity enforcement; +- per-backend limited and unlimited scheduling; +- FIFO behavior and cancellation-safe waiter removal; +- permit release on success, error, panic-relevant boundaries, and + cancellation; +- goroutine, timer, and waiter lifecycle; +- starvation, deadlock, race, and engine-isolation risks; +- lock scope and meaningful contention or allocation costs; +- preservation of injected-client concurrency where promised; +- relational testing of configured limits rather than duplicated defaults; + and +- duplication between internal concurrency tests and public contract tests. + +Run focused ordinary, race-enabled, and repeated tests. Repetition must remain +bounded and diagnostic; a test that passes many times is not proof of +correctness without a source-level synchronization analysis. + +**Exit condition:** concurrency invariants have both a source trace and a +test-ownership assessment. + +## Stage 16: Repository-Wide Test Strategy And Maintained Examples + +Perform a suite-level review after every component has been audited. Review +the testing policy, test inventory, fixtures, external-package root tests, +`architecture_test.go`, and both maintained examples. Use the component-stage +coverage ledger instead of repeating every individual test assertion. + +Construct a risk-to-owner matrix for: + +- public compatibility and error identity; +- parsing, validation, and serialization; +- immutability and data integrity; +- external wire behavior; +- cancellation, failure propagation, and recovery; +- concurrency and resource lifecycle; and +- representative assembled consumer workflows. + +Identify only evidence-backed cases of: + +- consequential behavior with no credible test owner; +- the same semantic rule asserted redundantly at several layers; +- tests coupled to private helpers, internal constants, exact noncontractual + wording, or collaborator choreography; +- low-value or obsolete cases whose lifetime cost exceeds their protection; +- missing failure, cancellation, race, or boundary coverage; +- nondeterminism, shared state, environment dependence, fixed ports, or test + ordering assumptions; +- helpers and fixtures whose complexity is not justified; and +- maintained examples that duplicate one another without protecting distinct + workflows. + +Use coverage and timing only to direct attention. Do not propose tests solely +to raise percentages or remove tests solely to shorten the suite. + +**Exit condition:** every important risk has a named test owner or an accepted +finding, and every proposed test deletion or consolidation states what +protection remains. + +## Stage 17: Cross-Cutting Duplication, Efficiency, And Architecture Review + +Review the codebase as a whole using the completed component findings, graph +traces, complexity signals, similarity signals, and package dependency map. +Do not reopen settled package behavior without new cross-cutting evidence. + +Focus on: + +- one semantic policy implemented by multiple packages; +- repeated public/internal transformations with credible drift risk; +- interfaces broader than their actual consumers; +- responsibilities split across packages or concentrated in the facade + contrary to the architecture policy; +- repeated parsing, copying, schema compilation, request construction, or + source traversal on important paths; +- avoidable lock contention or serial work supported by the concurrency audit; +- abstractions that add indirection without enforcing a boundary; and +- discrepancies between implemented package responsibilities and their + canonical architecture or internal documentation. + +For each possible consolidation, state why the code represents one rule, which +package should own it, and why the resulting dependency direction remains +valid. For each efficiency finding, state the path frequency, input scale, +complexity or measurement evidence, and the benchmark or invariant needed to +verify a remediation. + +**Exit condition:** all cross-cutting opportunities are either accepted with +high confidence, retained as explicitly lower-confidence observations, or +rejected with a short rationale. + +## Stage 18: Consolidate And Close The Audit + +Perform a findings-only synthesis. Do not change code and do not write the +remediation plan yet. + +- Recheck every accepted finding against the final audited tree. +- Merge duplicates and mark superseded IDs without erasing their history. +- Separate shared root causes from downstream symptoms. +- Confirm that every accepted item is confirmed or high confidence. +- Confirm that severity describes impact rather than effort. +- Reject speculative cleanup, coverage-driven test work, and centralization + without a clear owner or drift risk. +- Record dependencies and a recommended remediation order. +- Distinguish behavioral fixes, safe refactors, performance work, test gaps, + test consolidation, and documentation synchronization. +- Add an audit summary stating what was reviewed, what validation ran, the + accepted finding counts by category and severity, and any residual + uncertainty. +- Re-run baseline validation if audit-only investigation could have affected + repository state, and confirm that only authorized roadmap files differ from + the recorded baseline. + +The recommended ordering should place correctness, data-integrity, +resource-lifecycle, and concurrency defects first; policy duplication and +missing protection for consequential behavior next; then clarity, test +consolidation, and demonstrated efficiency improvements. Actual implementation +stages must be decided in the later `implementation.md` planning pass, where +files, dependencies, acceptance criteria, and validation can be made +decision-complete. + +**Exit condition:** `audit.md` is a complete, internally consistent input to a +separate remediation-planning prompt, with no code or test changes mixed into +the audit. + +## Completion Criteria + +The audit is complete only when: + +- every production component and public boundary appears in the coverage + ledger; +- every test file and maintained example has been reviewed at its owning stage + or in the suite-wide stage; +- important cross-package paths have been traced end to end; +- concurrency-sensitive behavior has received source and race-test review; +- every accepted finding meets the evidence and confidence standard; +- lower-confidence observations are visibly separated from remediation + candidates; +- proposed test additions, deletions, and consolidations are justified against + the testing policy; +- proposed simplifications identify a durable responsibility owner; +- proposed efficiency work has a relevant cost model or measurement plan; and +- the repository remains unchanged except for the authorized audit roadmap + artifacts. diff --git a/docs/roadmap/deferred.md b/docs/roadmap/deferred.md index 433c414..d973feb 100644 --- a/docs/roadmap/deferred.md +++ b/docs/roadmap/deferred.md @@ -59,24 +59,3 @@ if supported at all, should be an explicit option and must not expose secrets. Reconsider this idea after downstream use of `InspectPrompt`, `InspectProfile`, and fixture-based preparation demonstrates a concrete gap. - -### Structured Generation Errors - -**Reason for deferral:** Existing `ErrLLMGenerate` classification, preserved -injected-client errors, and prepared execution details currently provide the -necessary failure boundary. A typed error should wait for stronger downstream -demand and a transport-neutral field design. - -Promptkit could expose safe structured generation context through -`errors.As` while preserving `errors.Is(err, ErrLLMGenerate)`. Potential -fields include the selected backend ID and model plus an optional HTTP status -when the built-in OpenAI-compatible transport supplies one. - -The design must not expose provider response bodies, endpoints, credential -environment names or values, request content, or generated content. It should -not duplicate prompt and profile provenance already available from a prepared -execution, and it must preserve the identity of errors returned by injected -clients. Retry and backoff policy remains a consumer responsibility. - -Reconsider this idea when consumers need structured generation diagnostics -beyond the existing sentinel, wrapped client error, and preparation record. diff --git a/docs/roadmap/structured-generation-errors.md b/docs/roadmap/structured-generation-errors.md new file mode 100644 index 0000000..5d232aa --- /dev/null +++ b/docs/roadmap/structured-generation-errors.md @@ -0,0 +1,71 @@ +# Structured Generation Errors + +## Purpose + +Promptkit should give downstream applications actionable, machine-readable +details when the built-in OpenAI-compatible client receives a non-success HTTP +response. Today the client reports only the status code and discards the +provider response body. This makes ordinary configuration failures—such as an +unsupported strict JSON Schema keyword—unnecessarily difficult to diagnose. + +## Target End State + +Failures from the built-in transport are available through a public typed error +that works with `errors.As` while continuing to match `ErrLLMGenerate` through +`errors.Is`. The error should expose: + +- the HTTP status code; +- a normalized provider error code or type when supplied; and +- a bounded provider message extracted from a recognized OpenAI-compatible + JSON error envelope. + +The ordinary `Error()` string should remain safe and concise: it should include +the status and provider code or type, but not automatically include the +provider message. Consumers that deliberately want the provider's diagnostic +text can retrieve it from the typed error and apply their own disclosure and +logging policy. + +This contract should be available for both ordinary and prepared execution. +Errors returned by injected model clients must continue to preserve their own +identity and should not be converted into fabricated HTTP details. + +## Safety And Compatibility Boundaries + +- Never expose the raw response body, response headers, endpoint, credentials, + request messages, schema document, or generated content through this API. +- Read only a small fixed maximum response body, reject malformed or + unrecognized envelopes, normalize invalid UTF-8 and control characters, and + cap every retained diagnostic field independently. +- Treat the extracted provider message as untrusted and potentially sensitive: + its GoDoc must tell consumers not to log or display it without applying their + own policy. +- Preserve the existing generic behavior when a response is empty, non-JSON, + oversized, or does not match a recognized error envelope. +- Do not assign retryability from an HTTP status. Promptkit supplies facts; + downstream applications retain retry and presentation policy. + +## Recommended API Direction + +Prefer one immutable public `GenerationError` value, constructed internally and +carrying accessors for HTTP status, provider code or type, and provider message. +This keeps the exact representation evolvable while giving consumers an +idiomatic `errors.As` contract. Public Go declarations and GoDoc should own the +final exact names and semantics. + +The internal OpenAI-compatible client should parse only the conventional +top-level `error` envelope and pass normalized details through the use-case and +public error-mapping layers. The integration documentation should continue to +own wire behavior; the public declarations should own the consumer contract. + +## Acceptance Criteria + +- A downstream consumer can distinguish a provider HTTP 400 from other + generation failures and obtain a bounded provider explanation when present. +- The typed error still satisfies `errors.Is(err, ErrLLMGenerate)`. +- Existing cancellation, capacity, validation, and injected-client error + identities remain unchanged. +- Tests cover recognized string and numeric provider codes, absent and malformed + envelopes, oversized bodies and fields, control characters, and error-chain + behavior without making live provider requests. +- Current-state GoDoc and the OpenAI-compatible integration and internal-client + documents are updated only when the implementation lands.