diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..5ecfb68 --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,429 @@ +# Structured Generation Errors Implementation Plan + +## Purpose + +Implement the target state defined in the +[structured generation errors roadmap](structured-generation-errors.md): turn +every non-2xx response from the built-in OpenAI-compatible client into a +bounded, immutable public error that exposes deliberate provider diagnostics +through `errors.As` while retaining `ErrLLMGenerate` through `errors.Is`. + +This document owns implementation sequencing. The feature roadmap owns the +consumer intent, public-policy decisions, safety limits, compatibility +boundaries, and non-goals. Follow the architecture, documentation, and testing +policies under [`docs/policy/`](../policy/) throughout the work. + +## Fixed Decisions + +- The public type is `GenerationError`. Engine-produced values are pointers; + fields are unexported; no public constructor or mutation API is added. +- Public methods are `StatusCode() int`, `ProviderCode() string`, + `ProviderType() string`, `ProviderMessage() string`, `Error() string`, + `GoString() string`, and `Unwrap() error`. +- Public `Unwrap` returns `ErrLLMGenerate`. Accessors, formatting, and unwrapping + are safe on a nil receiver and a zero value. +- Engine-produced `Error()` text is exactly + `failed to generate output: provider returned HTTP status N`, where `N` is + the received status. A nil receiver or zero status returns exactly + `failed to generate output`. `GoString()` returns the same redacted text as + `Error()` so `%#v` cannot reveal unexported provider fields. +- Default formatting contains no provider-controlled code, type, message, or + raw response content. The type has no stable JSON representation. +- The recognized body is one JSON document with a top-level object-valued + `error`. `message` and `type` accept strings; `code` accepts a string or an + exact `json.Number`; supported fields are independent and unknown fields are + ignored. +- The non-success-body limit is 65,536 bytes. Declared oversize bodies are not + read; other bodies are read through a 65,537-byte bound. Oversize, malformed, + unrecognized, or unreadable bodies produce status-only detail. +- Normalization converts invalid UTF-8, collapses Unicode whitespace, control, + and format-character runs to one ASCII space, trims the result, omits blank + values, and produces one-line strings. +- Codes and types longer than 256 Unicode code points are omitted. Messages + longer than 4,096 code points retain the first 4,095 code points followed by + `…`, for a total limit of 4,096. +- Only the concrete built-in transport error is converted into a new public + `GenerationError`. Arbitrary injected-client errors are never inspected or + enriched. +- The use-case and domain layers remain provider-neutral. No retryability, + retry, logging, presentation, provider-specific envelope, header, or success- + response behavior is added. + +## Execution Rules + +- Complete the stages in numerical order. Each stage is scoped for one + gpt-5.6-terra implementation prompt and must finish its focused tests before + the next begins. +- Treat all stages as one feature delivery. Intermediate stages intentionally + create internal machinery before exposing it; do not release, tag, or claim + the feature is available until Stage 5 is complete. +- At the start of each stage, reread the feature roadmap and the task-specific + references in [`docs/development.md`](../development.md). Preserve unrelated + working-tree changes. +- Use classical behavior tests at the narrowest owner. Table-drive parser and + boundary cases, use controlled transports or local servers, and do not + duplicate the internal envelope matrix at the root engine boundary. +- Do not contact a live provider, add dependencies, commit, tag, push, or edit + release documentation unless separately instructed. +- Current-state prose documentation changes in Stage 5. Public GoDoc changes + alongside the public declarations in Stage 4 because GoDoc owns that API. + +## Stage 1: Add the Internal Structured Status Error and Envelope Parser + +### Objective + +Create the transport-owned structured value and pure parsing and normalization +logic without changing `OpenAICompatibleClient.Generate` yet. + +### Implementation + +1. Add `internal/llm/provider_http_error.go`. Keep all provider HTTP mechanics + in `internal/llm`; do not add an HTTP error DTO to `internal/domain` or + `internal/usecase`. +2. Define these private constants: + - `maxProviderErrorResponseBytes int64 = 64 << 10`; + - `maxProviderErrorIdentifierRunes = 256`; and + - `maxProviderErrorMessageRunes = 4096`. +3. Add an exported-within-`internal` `ProviderHTTPError` type with unexported + `statusCode`, `providerCode`, `providerType`, and `providerMessage` fields. + The root facade will need to name this concrete type in Stage 4, but no + representation is public outside the module's `internal` boundary. +4. Give `ProviderHTTPError` nil-safe read-only accessors with the same four + names as the planned public type. Implement: + - `Error()` as `llm returned non-success status: status=N` when status is + nonzero and `llm returned non-success status` otherwise; + - `GoString()` by returning `Error()`; and + - `Unwrap()` by returning `ErrUnexpectedStatus`. + Never include provider-derived strings in either formatter. +5. Add a private `providerErrorDetails` value and a private constructor that + builds `*ProviderHTTPError` from a status plus already normalized details. +6. Add `parseProviderErrorEnvelope([]byte) providerErrorDetails` with these + rules: + - use `json.Decoder` with `UseNumber` and require EOF after trailing JSON + whitespace; + - require a top-level object and object-valued `error` member; + - retain supported fields as `json.RawMessage` so each can be decoded and + validated independently; + - accept string `message` and `type` values; + - accept string or `json.Number` `code`, preserving validated number text + without float conversion; + - ignore unknown fields and treat invalid supported-field values as absent; + and + - return empty details for malformed framing, a missing or invalid `error` + object, or an object with no usable fields. +7. Add private normalization helpers that implement the roadmap's UTF-8, + single-line, whitespace/control/format handling and exact rune limits. Use + rune-aware operations; do not truncate bytes in the middle of UTF-8. Omit + overlong code and type identifiers, and truncate overlong messages to 4,095 + code points plus `…`. + +### Tests + +Add `internal/llm/provider_http_error_test.go` in package `llm` with focused, +table-driven tests: + +1. `TestProviderHTTPErrorEnvelopeParsing` covers: + - all supported string fields; + - string, integer, fractional, and exponent-form numeric codes without + float coercion; + - `null` and invalid field types handled independently; + - unknown top-level and nested fields; + - missing, null, scalar, and empty `error` values; + - malformed, truncated, trailing-garbage, and second-document input; and + - no raw or unsupported metadata retained. +2. `TestProviderErrorTextNormalizationAndLimits` covers valid multibyte text, + invalid UTF-8 replacement, leading/trailing and repeated whitespace, + newline/tab/control/format characters, blank normalization, exact identifier + and message boundaries, identifier omission one rune over, and rune-safe + message ellipsis one rune over. +3. `TestProviderHTTPErrorIdentityAndFormatting` covers exact accessors, + `errors.Is(err, ErrUnexpectedStatus)`, nil receivers, zero values, and safe + `%v`, `%+v`, and `%#v` formatting with distinctive provider markers absent. + +Do not test body reading, HTTP response ownership, the root public type, or +assembled engine behavior in this stage. + +### Verification + +```sh +go test ./internal/llm -run 'TestProvider' +go test ./internal/llm +go test ./... +``` + +Stage 1 is complete when the parser and internal error are fully protected but +the live non-2xx branch remains unchanged. + +**Status:** Complete. + +## Stage 2: Add the Bounded Non-Success Body Reader + +### Objective + +Implement and test bounded response-body extraction independently from HTTP +client integration, keeping status preservation separate from envelope +validity. + +### Implementation + +1. In `internal/llm/provider_http_error.go`, add a private helper with the + equivalent contract of: + + ```go + func providerHTTPErrorFromBody( + statusCode int, + contentLength int64, + body io.Reader, + ) *ProviderHTTPError + ``` + +2. Always return a nonnil `ProviderHTTPError` carrying `statusCode`. +3. When `contentLength` is greater than 65,536, return status-only detail + without reading `body`. +4. Otherwise read through an `io.LimitedReader` capped at 65,537 bytes. Return + status-only detail on a read error or when the extra byte is consumed. Do + not parse a bounded prefix of an incomplete oversized body. +5. For a complete body at or below the limit, call + `parseProviderErrorEnvelope` and construct the error from its normalized + details. +6. The helper does not close or drain `body`; the HTTP caller retains response- + body ownership. It must never read beyond the one-byte overflow probe. + +### Tests + +Extend `internal/llm/provider_http_error_test.go` with +`TestProviderHTTPErrorBodyBounds`, using counting, failing, and guarded readers +instead of an HTTP server. Cover: + +- an ordinary recognized envelope; +- an exact 65,536-byte body, using trailing JSON whitespace to reach the + boundary while remaining one valid document; +- a declared 65,537-byte body with zero reads; +- unknown-length and underreported 65,537-byte bodies with exactly 65,537 bytes + read and status-only detail; +- an early read failure with status-only detail; and +- an empty body with status-only detail. + +Assert the provider markers are absent whenever extraction is discarded. Do +not add body-closure assertions here because this helper does not own closing. + +### Verification + +```sh +go test ./internal/llm -run 'TestProviderHTTPErrorBodyBounds|TestProvider' +go test ./internal/llm +go test ./... +``` + +Stage 2 is complete when every read path is deterministically bounded and the +HTTP client's current branch is still untouched. + +## Stage 3: Integrate Structured Status Errors into the Built-In Client + +### Objective + +Replace the built-in client's status-only discard branch with the bounded +internal error while preserving all successful, cancellation, transport, and +body-ownership behavior. + +### Implementation + +1. In `internal/llm/openai_compatible_client.go`, replace the non-2xx branch's + 4,096-byte discard and formatted sentinel with + `providerHTTPErrorFromBody(httpResp.StatusCode, httpResp.ContentLength, + httpResp.Body)`. +2. Keep the existing `defer httpResp.Body.Close()` as the single body-closure + owner. Do not close in the helper, drain after the bound, or reuse the 16 MiB + successful-response decoder or limit. +3. Return no partial `GenerateResponse` for every non-2xx response. +4. Preserve `errors.Is(err, ErrUnexpectedStatus)` through + `ProviderHTTPError.Unwrap`. Do not change `requestFailedError`, endpoint or + request validation, authentication, timeout handling, successful response + decoding, or response-size behavior. + +### Tests + +1. Update the existing non-success case in + `internal/llm/openai_compatible_client_test.go` to assert + `errors.As(err, &providerHTTPError)`, exact status, and continued + `ErrUnexpectedStatus` identity. Keep the existing raw-body redaction check. +2. Add `internal/llm/provider_http_error_transport_test.go` with: + - `TestOpenAICompatibleClientStructuredNonSuccessResponse`, proving a + recognized envelope supplies all normalized internal fields and returns + no generation result; + - `TestOpenAICompatibleClientNonSuccessBodyOwnership`, table-driving normal, + declared-oversize, streamed-oversize, underreported, malformed, and read- + failure cases through a controlled transport; and + - assertions that every body is closed, no case reads beyond its bound, + declared oversize performs no read, and discarded details remain empty. +3. Reuse existing controlled-transport and counting-reader helpers when they + are clear and package-local. Do not duplicate successful-response framing, + timeout, authentication, or endpoint matrices. + +### Verification + +```sh +go test ./internal/llm -run 'TestOpenAICompatibleClient.*NonSuccess|TestProvider' +go test ./internal/llm +go test ./... +``` + +Stage 3 is complete when the built-in client emits the structured internal +error for every non-2xx response and all prior internal identities remain +green. + +## Stage 4: Add the Public Error and Root Mapping Contract + +### Objective + +Translate only the built-in transport's structured error at the root facade, +publish the immutable consumer API, and prove ordinary, prepared, and injected- +client behavior. + +### Implementation + +1. Add `generation_error.go` in package `promptkit` with an immutable public + `GenerationError` whose four fields are unexported strings or integers. Add + one private constructor that accepts the four normalized scalar values. The + public error file must not import `internal/llm` or retain the internal error + or raw body; `errors.go` performs that adaptation at the facade boundary. +2. Implement the four public accessors exactly as fixed above. Each returns + zero or empty on a nil receiver. +3. Implement `Error()` with the fixed strings from this plan, `GoString()` by + returning `Error()`, and `Unwrap()` by returning `ErrLLMGenerate` even for a + nil receiver. Do not implement mutable fields, an exported constructor, + retry helpers, HTTP mapping, `fmt.Formatter`, or JSON methods. +4. Write complete GoDoc covering: + - built-in-client and non-2xx scope; + - ordinary and prepared execution; + - `errors.Is` and pointer-target `errors.As` usage; + - immutable and caller-owned semantics; + - nil and zero behavior; + - lack of stable JSON; + - safe default formatting; and + - the fact that every provider accessor is untrusted and may contain + sensitive request or schema fragments. +5. In `errors.go`, after the special capacity conversion and before generic + sentinel wrapping, use `errors.As` for a nonnil concrete + `*llm.ProviderHTTPError`. Convert it to `*GenerationError` and return that + public value directly. Do not parse error text or recognize an interface + that an injected client could accidentally satisfy. +6. Preserve the existing generic `publicErrorFor` path for all other failures. + In particular, arbitrary injected-client errors remain wrapped with + `ErrLLMGenerate` and retain their original identity. +7. Update public GoDoc in the same stage: + - the `ErrLLMGenerate` declaration points to `GenerationError` for built-in + non-2xx responses; + - `Engine.Run` and `Engine.RunPrepared` mention the typed error without + restating its accessors; + - `doc.go` distinguishes mutable `CapacityError` from immutable + `GenerationError`, lists both as lacking stable JSON, and calls out the + provider-detail trust boundary; and + - injected `LLMClient` GoDoc remains clear that arbitrary client errors are + preserved rather than translated. + +### Tests + +1. Add package-internal tests for `GenerationError` nil and zero receivers, + exact fixed formatting, and unwrapping. Do not expose a test-only public + constructor or turn the lack of stable JSON into a serialized-output + contract. +2. Add a focused external-package contract test, preferably in + `generation_error_contract_test.go`, that obtains errors through a real + assembled engine with a controlled HTTP transport: + - an ordinary `Run` with a recognized envelope asserts a nil result, + `errors.Is(err, ErrLLMGenerate)`, pointer-target `errors.As`, all four + accessors, exact status-bearing formatting, and absence of distinctive + code/type/message markers from `%v`, `%+v`, and `%#v`; + - one `RunPrepared` case proves the same public type and status cross the + prepared boundary without repeating every parser field; and + - neither case contacts a live provider or uses a real credential. +3. Extend the existing injected-client preservation owner with one assertion + that an arbitrary injected error does not become a `*GenerationError`, while + still matching both `ErrLLMGenerate` and the injected error. +4. Keep detailed envelope, normalization, size, and body-ownership matrices in + `internal/llm`; root tests remain representative. + +### Verification + +```sh +go test . -run 'TestGenerationError|TestBuiltInGenerationError|TestRunAddsLLMGenerate' +go test ./internal/llm +go test ./... +``` + +Stage 4 is complete when consumers can inspect built-in non-2xx details through +the stable root contract and injected errors remain untouched. + +## Stage 5: Update Canonical Documentation and Run Full Validation + +### Objective + +Make durable documentation match the implemented contract, remove the roadmap +links that describe parsing as future work, and complete repository-wide +validation. + +### Documentation + +1. Update `docs/integrations/openai-compatible-chat.md` as the canonical wire + owner. Replace the status-only paragraph with: + - the recognized top-level envelope and independent field types; + - strict single-document framing and unknown-field behavior; + - the exact 65,536-byte read policy and status-only fallbacks; + - exact normalization and field limits; + - response closure and no-overread behavior; and + - the prohibition on raw bodies, headers, endpoints, credentials, request + data, schemas, and generated content. +2. Update `docs/internal/llm.md` with the internal `ProviderHTTPError`, bounded + reader and parser flow, retained `ErrUnexpectedStatus` identity, root + conversion boundary, and narrow test ownership. Remove its link that defers + parsing to the feature roadmap. +3. Update `docs/consumers/pkg-promptkit.md` under `Handle Errors` with one short + `errors.As` example. Show status and deliberate message access, warn that all + provider fields are untrusted and potentially sensitive, leave retry and + presentation policy to the application, and link to `GenerationError` + GoDoc rather than duplicating its full contract. +4. Update `docs/internal/overview.md` only enough to inventory implemented + responsibilities: the root facade owns typed capacity and generation error + mapping, and `internal/llm` owns bounded structured non-success response + decoding. Do not duplicate limits or accessor details there. +5. Do not change `docs/policy/architecture.md`, `docs/formats.md`, backend + documentation, release notes, or the README unless implementation reveals a + concrete inaccurate statement. Their canonical topics do not own this + contract. + +### Final Validation + +Run the complete maintainer workflow from +[`docs/development.md#maintainer-validation`](../development.md#maintainer-validation): + +```sh +go test ./... +go test -race ./... +go vet ./... +go build ./... +go run ./examples/go-library/prepare +go run ./examples/go-library/run +``` + +Also perform the documented Go-formatting, local Markdown-link, repository- +hygiene, ignored-file, and credential scans. Review both example outputs and +confirm that all commands remain deterministic, offline, and credential-free. + +Finally review the complete diff against the feature roadmap and confirm: + +- every built-in non-2xx response produces a status-bearing public type; +- malformed and oversized bodies cannot erase status or leak partial content; +- provider-derived strings appear only through deliberate accessors; +- default and Go-syntax formatting are redacted; +- successful, cancellation, capacity, validation, repair, and injected-client + behavior is unchanged; +- no provider policy entered the use-case or domain layers; and +- each exact contract has one canonical documentation and test owner. + +## Open Questions + +None. The roadmap and this plan fix the public API, internal representation, +wire envelope, normalization and bounds, status fallback, formatting, +propagation, compatibility, documentation, and verification decisions required +for implementation. diff --git a/docs/roadmap/structured-generation-errors.md b/docs/roadmap/structured-generation-errors.md index 5d232aa..d159915 100644 --- a/docs/roadmap/structured-generation-errors.md +++ b/docs/roadmap/structured-generation-errors.md @@ -5,67 +5,256 @@ 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. +provider response body. This makes ordinary configuration failures, such as an +unsupported strict JSON Schema keyword, unnecessarily difficult to diagnose. + +This feature supplies bounded facts about the provider response. It does not +make retry, presentation, or logging decisions for consumers. ## 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: +Every non-2xx response received by Promptkit's built-in OpenAI-compatible +client becomes a public typed generation error. A consumer can use +`errors.As` to obtain the HTTP status and any safely extracted provider fields, +and `errors.Is` continues to match `ErrLLMGenerate`. -- 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 typed contract is available from both `Run` and `RunPrepared`. It is not +produced during preparation, which performs no model request. Successful +responses, transport failures before a response is received, cancellation, +capacity failures, validation failures, and nil responses from injected model +clients retain their existing categories and behavior. -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. +An unusable response body never hides the known HTTP status. Empty, malformed, +unrecognized, unreadable, or oversized bodies therefore produce the same typed +error with status-only detail rather than falling back to an unstructured +error or becoming a malformed-success response. -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. +## Public Contract -## Safety And Compatibility Boundaries +The root package exposes an immutable `GenerationError` type with unexported +state and these read-only accessors: -- 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. +- `StatusCode() int` returns the received HTTP status code; +- `ProviderCode() string` returns a normalized provider code, when present; +- `ProviderType() string` returns a normalized provider error type, when + present; and +- `ProviderMessage() string` returns the bounded normalized diagnostic message, + when present. -## Recommended API Direction +The engine returns a `*GenerationError`, so the idiomatic inspection form is: -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. +```go +var generationErr *promptkit.GenerationError +if errors.As(err, &generationErr) { + status := generationErr.StatusCode() + message := generationErr.ProviderMessage() + _, _ = status, message +} +``` -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. +There is no public constructor or mutation API. The type implements `error`, +unwraps to `ErrLLMGenerate`, and provides safe ordinary and Go-syntax +formatting. `Error()` and `GoString()` include the HTTP status but no provider- +controlled code, type, or message. Consumers must use the accessors +deliberately when they want provider details and must not classify failures by +matching error text. + +The zero value and a nil `*GenerationError` receiver are safe: accessors return +zero or empty values, formatting returns a generic redacted generation-failure +description, and unwrapping still identifies `ErrLLMGenerate`. Engine-produced +values always have the non-2xx status received from the provider. The type has +no stable JSON representation. + +All provider-derived strings remain untrusted even after normalization. GoDoc +must warn consumers that provider fields can contain sensitive request or +schema fragments and must not be logged, displayed, or returned to another +caller without an application-appropriate disclosure policy. + +## Recognized Provider Envelope + +Promptkit recognizes only the conventional OpenAI-compatible top-level error +object: + +```json +{ + "error": { + "message": "diagnostic text", + "type": "invalid_request_error", + "code": "unsupported_parameter" + } +} +``` + +The envelope must be one JSON document followed only by JSON whitespace. The +top-level `error` value must be an object. Unknown top-level and error-object +fields are ignored. The optional supported fields are interpreted +independently: + +- `message` and `type` must be JSON strings; +- `code` may be a JSON string or number and is exposed as normalized text; + numeric codes retain their validated JSON number text without floating-point + coercion; and +- `null`, booleans, arrays, objects, or otherwise invalid values are treated as + absent for that field. + +An invalid optional field does not discard other valid supported fields. An +absent `error` object, malformed or multiply framed JSON, or an object with no +usable supported fields simply leaves all provider accessors empty while +preserving the typed status error. + +Promptkit does not expose `param`, metadata objects, nested causes, headers, or +provider-specific extensions in this feature. + +## Bounded Reading And Normalization + +Non-success bodies have a separate fixed limit of 64 KiB (65,536 bytes). This +is intentionally much smaller than the successful completion-body limit while +remaining large enough for useful schema diagnostics. + +- A declared `Content-Length` above the limit is rejected without reading the + body for detail extraction. +- Otherwise Promptkit reads at most one byte beyond the limit so streamed, + chunked, and underreported bodies are bounded. +- A body over the limit contributes no provider fields; Promptkit does not + parse or retain a prefix as though it were a complete envelope. +- Read failures likewise discard provider fields while preserving the status. +- The response body is closed on every outcome and is not drained beyond the + bounded read. + +Extracted strings are converted to valid UTF-8, trimmed, and made single-line: +invalid UTF-8 is replaced, and runs of Unicode whitespace, control characters, +and formatting controls are replaced with one ASCII space. Empty normalized +values are treated as absent. + +Normalized provider codes and types are retained only when they contain at +most 256 Unicode code points. Longer values are omitted rather than truncated +so consumers never classify on a fabricated partial identifier. A provider +message is limited to 4,096 Unicode code points; a longer normalized message is +truncated at a code-point boundary with a visible ellipsis inside that limit. +The raw response body and pre-normalized strings are never exposed or retained +in the public error. + +## Error Propagation And Compatibility + +- Every built-in-client non-2xx response matches `ErrLLMGenerate` and supports + `errors.As` to `*GenerationError`, including status-only cases. +- The internal model client retains its non-success-status identity for its + own package tests. The use-case layer remains provider-neutral and continues + to add only its generation category. +- The root error boundary converts only the built-in transport's structured + status error. It does not parse arbitrary error text, inspect consumer error + fields, or fabricate HTTP details for an injected `LLMClient`. +- Errors returned by injected clients remain in the chain exactly as today. + If an injected client deliberately returns an existing `*GenerationError`, + its identity may pass through ordinary wrapping, but Promptkit does not + construct or enrich one on that client's behalf. +- Existing cancellation and deadline identities, capacity errors, validation + behavior, repair behavior, and successful response decoding remain + unchanged. +- This is an additive public API. Existing consumers that use + `errors.Is(err, ErrLLMGenerate)` continue to work; consumers should not rely + on the previous rendered wording of non-success errors. + +## Architecture And Ownership + +The provider-envelope parser and bounded body reader belong in `internal/llm`, +which owns the OpenAI-compatible transport. The internal transport error owns +only normalized status facts and continues to match the package's existing +non-success-status sentinel. + +The use-case package does not gain HTTP DTOs, status policy, or a provider- +specific branch. Its existing wrapping carries the internal error to the root +facade. The root error mapper recognizes the internal structured status error +and constructs the public `GenerationError` without exposing an internal type +or raw cause through public fields. No transport error is added to +`internal/domain`. + +The public type and its exact Go semantics are owned by its declaration and +GoDoc. The +[OpenAI-compatible integration contract](../integrations/openai-compatible-chat.md) +owns recognized wire shapes, limits, and observable response behavior. The +[internal model-client document](../internal/llm.md) owns implementation flow, +internal failure categories, and test ownership. Architecture policy does not +need a new package or dependency rule for this feature. + +## Documentation End State + +Canonical documentation at the target state has these responsibilities: + +- the `GenerationError` declaration and GoDoc define the exact public methods, + formatting, unwrapping, zero-value behavior, and trust boundary; +- `Engine.Run` and `Engine.RunPrepared` GoDoc identify the typed error without + duplicating its accessor contract; +- the consumer guide includes one short `errors.As` example and links to the + public declaration; +- the integration document replaces its status-only description with the + bounded envelope contract; and +- the internal model-client document describes parsing, conversion ownership, + and narrow test owners. + +The architecture policy, framework format reference, and built-in backend +catalog do not duplicate this API or wire contract. + +## Verification Expectations + +Verification protects each behavior at its narrowest stable owner: + +- internal model-client tests cover recognized string and numeric codes, + independent optional-field handling, unknown fields, empty and malformed + envelopes, single-document framing, read failures, declared and streamed + size boundaries, body closure, normalization, field limits, and absence of + raw provider content from rendered errors; +- root error-boundary tests cover conversion to the immutable public type, + every accessor, `errors.Is`, `errors.As`, and safe `%v`, `%+v`, and `%#v` + formatting; +- one representative ordinary run and one prepared run prove that the built-in + transport contract crosses the assembled engine boundary, without repeating + the complete parser matrix; +- existing injected-client tests continue to prove preservation of consumer + error identity without fabricated provider details; and +- all tests use controlled transports or local servers and never contact a + live or paid provider. + +Security limits and their exact boundaries are contractual enough to warrant +literal boundary tests. Higher-level tests should remain representative and +must not duplicate the internal transport matrix. ## 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. +- A consumer can distinguish an HTTP 400 from other generation failures and + deliberately obtain a bounded provider explanation when one is available. +- The same typed error remains available through ordinary and prepared + execution and still satisfies `errors.Is(err, ErrLLMGenerate)`. +- Default and Go-syntax error formatting cannot disclose any provider-derived + string or raw response content. +- Empty, malformed, unreadable, unrecognized, and oversized bodies preserve a + typed status-only error. +- No read, retained field, or formatted representation can exceed its stated + bound, and the body is closed on every outcome. +- Existing success, cancellation, capacity, validation, repair, and injected- + client contracts remain unchanged. +- Current-state documentation changes only when the implementation exists and + follows the repository's canonical ownership policy. + +## Non-Goals + +This feature does not add: + +- retryability classification, retry loops, backoff, failover, or routing; +- parsing of success bodies as errors or changes to successful-response limits; +- provider-specific envelope variants beyond the conventional top-level + `error` object; +- response headers such as `Retry-After`, raw bodies, request data, endpoints, + credentials, schema documents, generated content, or provider metadata; +- logging, telemetry, redaction policy for downstream applications, HTTP status + mapping for consumer servers, or user-facing presentation; +- translation or enrichment of arbitrary injected-client errors; or +- a new public package, public constructor, mutable error value, or transport + type in the domain model. + +## Open Questions + +None. The public type direction, accessor surface, formatting and error-chain +behavior, envelope scope, normalization, safety limits, fallback behavior, +layer ownership, compatibility boundaries, documentation ownership, and test +boundaries are fixed by this roadmap. diff --git a/internal/llm/provider_http_error.go b/internal/llm/provider_http_error.go new file mode 100644 index 0000000..2bcbd47 --- /dev/null +++ b/internal/llm/provider_http_error.go @@ -0,0 +1,173 @@ +package llm + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "strings" + "unicode" +) + +const ( + maxProviderErrorResponseBytes int64 = 64 << 10 + maxProviderErrorIdentifierRunes = 256 + maxProviderErrorMessageRunes = 4096 +) + +// ProviderHTTPError describes a non-success response from an LLM provider. +type ProviderHTTPError struct { + statusCode int + providerCode string + providerType string + providerMessage string +} + +func (e *ProviderHTTPError) StatusCode() int { + if e == nil { + return 0 + } + return e.statusCode +} + +func (e *ProviderHTTPError) ProviderCode() string { + if e == nil { + return "" + } + return e.providerCode +} + +func (e *ProviderHTTPError) ProviderType() string { + if e == nil { + return "" + } + return e.providerType +} + +func (e *ProviderHTTPError) ProviderMessage() string { + if e == nil { + return "" + } + return e.providerMessage +} + +func (e *ProviderHTTPError) Error() string { + if e == nil || e.statusCode == 0 { + return ErrUnexpectedStatus.Error() + } + return fmt.Sprintf("%s: status=%d", ErrUnexpectedStatus, e.statusCode) +} + +func (e *ProviderHTTPError) GoString() string { + return e.Error() +} + +func (e *ProviderHTTPError) Unwrap() error { + return ErrUnexpectedStatus +} + +type providerErrorDetails struct { + providerCode string + providerType string + providerMessage string +} + +func newProviderHTTPError(statusCode int, details providerErrorDetails) *ProviderHTTPError { + return &ProviderHTTPError{ + statusCode: statusCode, + providerCode: details.providerCode, + providerType: details.providerType, + providerMessage: details.providerMessage, + } +} + +func parseProviderErrorEnvelope(body []byte) providerErrorDetails { + decoder := json.NewDecoder(strings.NewReader(string(body))) + decoder.UseNumber() + + var envelope map[string]json.RawMessage + if err := decoder.Decode(&envelope); err != nil { + return providerErrorDetails{} + } + var trailing any + if err := decoder.Decode(&trailing); !errors.Is(err, io.EOF) { + return providerErrorDetails{} + } + + rawError, ok := envelope["error"] + if !ok { + return providerErrorDetails{} + } + var providerError map[string]json.RawMessage + if err := json.Unmarshal(rawError, &providerError); err != nil || providerError == nil { + return providerErrorDetails{} + } + + var details providerErrorDetails + if raw, ok := providerError["message"]; ok { + var value string + if json.Unmarshal(raw, &value) == nil { + details.providerMessage = normalizeProviderErrorMessage(value) + } + } + if raw, ok := providerError["type"]; ok { + var value string + if json.Unmarshal(raw, &value) == nil { + details.providerType = normalizeProviderErrorIdentifier(value) + } + } + if raw, ok := providerError["code"]; ok { + var value any + fieldDecoder := json.NewDecoder(strings.NewReader(string(raw))) + fieldDecoder.UseNumber() + if fieldDecoder.Decode(&value) == nil { + switch value := value.(type) { + case string: + details.providerCode = normalizeProviderErrorIdentifier(value) + case json.Number: + details.providerCode = normalizeProviderErrorIdentifier(value.String()) + } + } + } + + return details +} + +func normalizeProviderErrorIdentifier(value string) string { + normalized := normalizeProviderErrorText(value) + if len([]rune(normalized)) > maxProviderErrorIdentifierRunes { + return "" + } + return normalized +} + +func normalizeProviderErrorMessage(value string) string { + normalized := normalizeProviderErrorText(value) + runes := []rune(normalized) + if len(runes) <= maxProviderErrorMessageRunes { + return normalized + } + return string(runes[:maxProviderErrorMessageRunes-1]) + "…" +} + +func normalizeProviderErrorText(value string) string { + value = strings.ToValidUTF8(value, "�") + + var result strings.Builder + result.Grow(len(value)) + separatorPending := false + for _, r := range value { + if unicode.IsSpace(r) || unicode.IsControl(r) || unicode.In(r, unicode.Cf) { + if result.Len() > 0 { + separatorPending = true + } + continue + } + if separatorPending { + result.WriteByte(' ') + separatorPending = false + } + result.WriteRune(r) + } + return result.String() +} diff --git a/internal/llm/provider_http_error_test.go b/internal/llm/provider_http_error_test.go new file mode 100644 index 0000000..7f8088a --- /dev/null +++ b/internal/llm/provider_http_error_test.go @@ -0,0 +1,138 @@ +package llm + +import ( + "errors" + "fmt" + "reflect" + "strings" + "testing" + "unicode/utf8" +) + +func TestProviderHTTPErrorEnvelopeParsing(t *testing.T) { + tests := []struct { + name string + body string + want providerErrorDetails + }{ + { + name: "all supported string fields", + body: `{"error":{"message":"diagnostic","type":"invalid_request_error","code":"unsupported_parameter"}}`, + want: providerErrorDetails{providerMessage: "diagnostic", providerType: "invalid_request_error", providerCode: "unsupported_parameter"}, + }, + { + name: "integer code", + body: `{"error":{"code":17}}`, + want: providerErrorDetails{providerCode: "17"}, + }, + { + name: "fractional code", + body: `{"error":{"code":1.25}}`, + want: providerErrorDetails{providerCode: "1.25"}, + }, + { + name: "exponent code", + body: `{"error":{"code":6.02e+23}}`, + want: providerErrorDetails{providerCode: "6.02e+23"}, + }, + { + name: "invalid fields do not discard valid fields", + body: `{"error":{"message":null,"type":"invalid_request_error","code":false}}`, + want: providerErrorDetails{providerType: "invalid_request_error"}, + }, + { + name: "unknown fields are ignored", + body: `{"trace":"do not retain","error":{"param":"temperature","metadata":{"secret":"x"}}}`, + want: providerErrorDetails{}, + }, + {name: "missing error", body: `{}`, want: providerErrorDetails{}}, + {name: "null error", body: `{"error":null}`, want: providerErrorDetails{}}, + {name: "scalar error", body: `{"error":"nope"}`, want: providerErrorDetails{}}, + {name: "empty error", body: `{"error":{}}`, want: providerErrorDetails{}}, + {name: "malformed", body: `{"error":`, want: providerErrorDetails{}}, + {name: "truncated", body: `{"error":{"message":"x"`, want: providerErrorDetails{}}, + {name: "trailing garbage", body: `{"error":{"message":"x"}} garbage`, want: providerErrorDetails{}}, + {name: "second document", body: `{"error":{"message":"x"}} {}`, want: providerErrorDetails{}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := parseProviderErrorEnvelope([]byte(tc.body)); !reflect.DeepEqual(got, tc.want) { + t.Fatalf("parseProviderErrorEnvelope() = %#v, want %#v", got, tc.want) + } + }) + } +} + +func TestProviderErrorTextNormalizationAndLimits(t *testing.T) { + validIdentifier := strings.Repeat("界", maxProviderErrorIdentifierRunes) + validMessage := strings.Repeat("界", maxProviderErrorMessageRunes) + tests := []struct { + name string + got string + want string + }{ + {name: "multibyte text", got: "Grüße 世界", want: "Grüße 世界"}, + {name: "invalid UTF-8", got: string([]byte{'a', 0xff, 'b'}), want: "a�b"}, + {name: "whitespace control and format runs", got: " \n\talpha\x00\u200b\u200bbeta \r ", want: "alpha beta"}, + {name: "blank normalization", got: "\t\u200b\n", want: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := normalizeProviderErrorText(tc.got); got != tc.want { + t.Fatalf("normalizeProviderErrorText() = %q, want %q", got, tc.want) + } + }) + } + + if got := normalizeProviderErrorIdentifier(validIdentifier); got != validIdentifier { + t.Fatalf("exact identifier boundary = %q, want retained value", got) + } + if got := normalizeProviderErrorIdentifier(validIdentifier + "界"); got != "" { + t.Fatalf("overlong identifier = %q, want empty", got) + } + if got := normalizeProviderErrorMessage(validMessage); got != validMessage { + t.Fatalf("exact message boundary = %q, want retained value", got) + } + wantTruncatedMessage := strings.Repeat("界", maxProviderErrorMessageRunes-1) + "…" + if got := normalizeProviderErrorMessage(validMessage + "界"); got != wantTruncatedMessage { + t.Fatalf("overlong message length = %d, want %d", utf8.RuneCountInString(got), maxProviderErrorMessageRunes) + } +} + +func TestProviderHTTPErrorIdentityAndFormatting(t *testing.T) { + const marker = "provider-secret-marker" + err := newProviderHTTPError(429, providerErrorDetails{ + providerCode: marker + "-code", + providerType: marker + "-type", + providerMessage: marker + "-message", + }) + + if err.StatusCode() != 429 || err.ProviderCode() != marker+"-code" || err.ProviderType() != marker+"-type" || err.ProviderMessage() != marker+"-message" { + t.Fatalf("accessors returned unexpected values: %#v", err) + } + if !errors.Is(err, ErrUnexpectedStatus) { + t.Fatalf("errors.Is(%v, ErrUnexpectedStatus) = false", err) + } + for _, rendered := range []string{fmt.Sprintf("%v", err), fmt.Sprintf("%+v", err), fmt.Sprintf("%#v", err)} { + if rendered != "llm returned non-success status: status=429" { + t.Fatalf("formatted error = %q", rendered) + } + if strings.Contains(rendered, marker) { + t.Fatalf("formatted error exposed provider marker: %q", rendered) + } + } + + var nilError *ProviderHTTPError + if nilError.StatusCode() != 0 || nilError.ProviderCode() != "" || nilError.ProviderType() != "" || nilError.ProviderMessage() != "" { + t.Fatal("nil accessors returned provider values") + } + if nilError.Error() != "llm returned non-success status" || nilError.GoString() != "llm returned non-success status" || !errors.Is(nilError, ErrUnexpectedStatus) { + t.Fatalf("nil error behavior is not safe: %v", nilError) + } + + zero := &ProviderHTTPError{} + if zero.Error() != "llm returned non-success status" || zero.GoString() != "llm returned non-success status" || !errors.Is(zero, ErrUnexpectedStatus) { + t.Fatalf("zero error behavior is not safe: %v", zero) + } +}