434 lines
20 KiB
Markdown
434 lines
20 KiB
Markdown
# 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.
|
|
|
|
**Status:** Complete.
|
|
|
|
## 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.
|
|
|
|
**Status:** Complete.
|
|
|
|
## 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.
|