Files
promptkit/docs/roadmap/implementation.md

432 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.
## 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.