diff --git a/convert.go b/convert.go index 403c731..9a0413d 100644 --- a/convert.go +++ b/convert.go @@ -29,6 +29,7 @@ func toDomainRunRequest(req RunRequest) (domain.RunRequest, error) { Execution: execution, Validation: toDomainOutputContractPtr(req.Validation), AppendedMessages: appendedMessages, + TransientRetries: copyIntPtr(req.TransientRetries), }, nil } diff --git a/docs/consumers/pkg-promptkit.md b/docs/consumers/pkg-promptkit.md index 5b57d96..1a4a576 100644 --- a/docs/consumers/pkg-promptkit.md +++ b/docs/consumers/pkg-promptkit.md @@ -187,15 +187,26 @@ request.Validation = &promptkit.OutputContract{ } ``` -Each repair attempt is another model call, so it can increase latency and +Each repair attempt is another model generation, so it can increase latency and usage; `RunResult.Usage` is cumulative and `Validation.RepairAttempts` reports -calls actually started. Exhaustion still returns the final failed validation -result. `basic` validation can also repair an empty candidate, but structural +repair generations rather than transient HTTP attempts. Exhaustion still +returns the final failed validation result. `basic` validation can also repair +an empty candidate, but structural validity is not evidence of factual or domain correctness. See the [output-contract format reference](../formats.md#output-contract) and [`OutputContract` GoDoc](../../types.go) for the exact budget and eligibility rules. +The built-in client also retries eligible temporary HTTP statuses. By default, +each generation can make two additional attempts; a repair generation receives +its own budget. Set `Config.TransientRetries` to a pointer to zero for the +previous single-attempt behavior, or set `RunRequest.TransientRetries` to +override one execution. A custom HTTP transport still uses this policy; +injected `LLMClient` implementations retain their own retry policy. Application +semantic retries are separate runs and can multiply provider traffic further. +See the [provider integration contract](../integrations/openai-compatible-chat.md#transient-status-retries) +for eligible statuses, waits, timeouts, and provider timing. + ### Append Already-Rendered Messages An application can include an earlier assistant response and its own corrective @@ -548,13 +559,13 @@ var generationErr *promptkit.GenerationError if errors.As(err, &generationErr) { status := generationErr.StatusCode() message := generationErr.ProviderMessage() - _, _ = status, message // Apply application retry and presentation policy. + _, _ = status, message // Apply application presentation policy. } ``` All provider fields are untrusted and can contain sensitive request or schema fragments. Do not log, display, or return them without an application-specific -disclosure policy. Promptkit does not assign retry or presentation behavior. +disclosure policy. Promptkit does not assign presentation behavior. The [`GenerationError` GoDoc](../../generation_error.go) owns the exact typed error contract. diff --git a/docs/integrations/openai-compatible-chat.md b/docs/integrations/openai-compatible-chat.md index 3613baa..29edea8 100644 --- a/docs/integrations/openai-compatible-chat.md +++ b/docs/integrations/openai-compatible-chat.md @@ -139,6 +139,32 @@ identity and the exact transport error for `errors.Is` and `errors.As` checks. The rendered error does not include the selected endpoint, request headers, request content, credentials, or provider body. +## Transient Status Retries + +The built-in path retries only HTTP 429, 500, 502, 503, and 504. It makes two +additional attempts by default; engine configuration or an individual run can +choose a different nonnegative count, including zero for single-attempt +behavior. The same budget applies separately to initial generation and each +structural repair generation. An injected `LLMClient` is dispatched once by +Promptkit regardless of this setting. A custom `http.Client` on the built-in +path remains subject to these retries. + +Each retry uses equal jitter between half and all of an exponential local +delay, starting at 500 milliseconds and capped at 8 seconds. The first valid +`Retry-After` header value, as unsigned decimal seconds or an HTTP date, sets +a minimum not-before time measured from response receipt. Invalid values use +local delay; past dates impose no extra delay. The provider minimum is not +capped at 8 seconds. Retry waiting is cancellable and holds no active backend +permit. A caller without a deadline can wait a long time for provider timing. + +The request body, selected URL, and resolved authorization are fixed across +attempts of one generation. Each attempt creates a fresh HTTP request and +receives its own configured generation and HTTP-client timeout after capacity +admission. The caller context covers every attempt and wait without a reset. +Transport failures, malformed responses, noneligible statuses, and invalid +generated content are not transient retries. A positive budget ending in +eligible statuses reports retry exhaustion with the final provider cause. + ## Timeout And Cancellation Timeouts are layered: diff --git a/docs/internal/capacity.md b/docs/internal/capacity.md index 2168f6d..f812f35 100644 --- a/docs/internal/capacity.md +++ b/docs/internal/capacity.md @@ -54,32 +54,32 @@ reserve another bounded slot. ## FIFO Generation Permits -`NewClient` wraps the engine's selected internal model client after public -client adaptation or built-in client construction. Initial generation and the -default repairer receive the same wrapper. Their requests retain the same -effective backend, credential, numeric-presence metadata, and structured-output -settings, so scheduling does not change provider omission semantics between -calls. +The engine supplies one [generation coordinator](generation.md) to initial +generation and the default repairer. Their requests retain the same effective +backend, credential, numeric-presence metadata, structured-output settings, +and retry budget. Scheduling does not change provider omission semantics +between calls. -For each `Generate` call, the wrapper selects a pool from the request's -effective backend ID. An unlimited call passes directly to the next client. A -limited call acquires an active permit, invokes the next client, and defers -permit release so ordinary returns and panic unwinding both restore capacity. +For each model attempt, the coordinator selects a pool from the request's +effective backend ID. An unlimited call uses the unrestricted path. A limited +call acquires an active permit and defers release around that attempt, so +ordinary returns and panic unwinding both restore capacity. Preparation and validation never hold an active permit. The manager's `Acquire` operation owns that active-permit acquisition and -returns an idempotent release function. The existing wrapper delegates to it; -the internal generation coordinator uses the same operation around individual -attempts. There is one queue and one active limit per limited backend pool. +returns an idempotent release function. The coordinator uses it around +individual attempts. The earlier one-shot wrapper still delegates to the same +operation for focused internal callers and tests, but engine assembly no +longer uses it. There is one queue and one active limit per limited backend pool. When all active permits are occupied, calls join a mutex-protected FIFO waiter list. Releasing a permit transfers it directly to the oldest remaining waiter before making it generally available. Pools do not order work relative to other backend IDs. -The wrapper passes generation requests, responses, and collaborator errors -through unchanged. It owns scheduling only; the concrete model client remains -responsible for provider transport behavior. The runner, rather than the +The coordinator passes injected generation requests, responses, and errors +through unchanged. The built-in HTTP client remains responsible for provider +transport behavior. The runner, rather than the capacity layer, sums all five usage fields from the initial response and every completed repair response into the successful run result. @@ -87,10 +87,10 @@ completed repair response into the successful run result. Admission checks the caller context before reserving a slot. A call canceled while waiting for an active permit removes its waiter under the same pool lock -used to grant permits. If cancellation removes the waiter first, the wrapped -client is not invoked. If a concurrent grant wins first, the call owns the -permit and invokes the client with the original context, allowing the client -to observe cancellation normally. +used to grant permits. If cancellation removes the waiter first, no model +attempt is dispatched. If a concurrent grant wins first, the call owns the +permit; the coordinator checks the context before dispatch and releases the +permit if cancellation has already occurred. This grant-or-cancel decision prevents lost and double-released permits. Admission leases and active permits are released after success, collaborator diff --git a/docs/internal/generation.md b/docs/internal/generation.md index e65f6e5..e62ba3e 100644 --- a/docs/internal/generation.md +++ b/docs/internal/generation.md @@ -2,16 +2,17 @@ `internal/generation` coordinates one model-generation request through the existing engine-local [capacity manager](capacity.md) and the -[built-in HTTP client](llm.md). The current engine still uses the existing -capacity wrapper; engine assembly moves to this coordinator when public retry -configuration is wired. +[built-in HTTP client](llm.md). The engine supplies the same coordinator to +initial generation and the default output repairer. The coordinator has explicit built-in and injected-client constructors. The built-in path prepares the provider request once, then acquires and releases an active backend permit around each actual HTTP attempt. Only HTTP 429, 500, 502, 503, and 504 can lead to another attempt. The injected-client path acquires a permit and invokes its client once, regardless of the internal retry budget. -The runner continues to own the logical execution's admission lease. +The runner continues to own the logical execution's admission lease. A request +can override the engine's retry budget; the runner freezes that choice for +prepared execution and passes it to each repair generation. Retry waits hold no active permit. The local delay uses equal jitter between half and all of a capped exponential base, beginning at 500 milliseconds and diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 826dfc6..14392bc 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -19,7 +19,7 @@ contributor workflow and validation. | `internal/capacity` | Owns engine-local bounded execution admission and FIFO model-generation permits for limited backend IDs, including cancellation-safe waiter removal and client wrapping. | [Internal capacity management](capacity.md) | | `internal/domain` | Defines internal framework values for requests, artifacts, prompt definitions, profiles, execution targets, rendering, generation, and validation, and owns source-neutral invariants for shared execution settings, OpenAI-compatible base endpoints, session identifiers, and output contracts. Source parsing, required fields, other source-specific normalization, defaulting, and boundary-specific error classification remain with their callers. | [Domain declarations](../../internal/domain/domain.go), [endpoint invariant](../../internal/domain/endpoint.go) | | `internal/engine` | Assembles external catalogs, Promptkit-owned profiles, repositories, registry, capacity, collaborators, repairer, and the one use-case runner for each engine instance. | [Engine assembly](../../internal/engine/engine.go), [internal sources](sources.md#profiles-and-built-ins), [internal runner](runner.md) | -| `internal/generation` | Coordinates frozen built-in HTTP attempts and one-shot injected calls through active backend permits, including bounded status retries and cancellable backoff; engine assembly has not switched to it yet. | [Internal generation coordination](generation.md) | +| `internal/generation` | Coordinates frozen built-in HTTP attempts and one-shot injected calls through active backend permits, including bounded status retries and cancellable backoff. | [Internal generation coordination](generation.md) | | `internal/defaults` | Defines application-neutral framework constants and constructs the default execution target. It contains no CLI, server, or inbound HTTP limits. | [Framework defaults](../../internal/defaults/defaults.go) | | `internal/filecatalog` | Provides deterministic YAML discovery and path helpers for operating-system filesystems and `fs.FS` sources. | [File catalog](../../internal/filecatalog/catalog.go) | | `internal/jsonvalue` | Validates and deeply copies bounded JSON-compatible extra-parameter and prepared-schema trees while preserving supported concrete value types and rejecting cycles or excessive depth and work. | [JSON values](../../internal/jsonvalue/jsonvalue.go) | diff --git a/docs/internal/runner.md b/docs/internal/runner.md index 5473739..ee55ce3 100644 --- a/docs/internal/runner.md +++ b/docs/internal/runner.md @@ -20,11 +20,11 @@ and override semantics consumed by the runner. profiles, backend resolution, artifacts, rendering, model generation, and validation. The internal engine supplies one immutable registry containing the built-in backend and validated consumer additions, one engine-local run -admitter, and a model client wrapped by the same capacity manager. Validation +admitter, and a generation coordinator using the same capacity manager. Validation plans and provider-facing schema metadata come from the validator's preparation interface. The internal engine supplies one default output repairer through the explicit -runner constructor, using the same capacity-wrapped client as initial +runner constructor, using the same coordinator as initial generation. The no-repair runner constructor remains available for focused internal callers and tests. @@ -145,8 +145,8 @@ method must return before cancellation can take precedence over its outcome. The admission lease covers completion-phase preparation, initial generation, validation, every repair, and every exit. It bounds accepted work without serializing preparation or validation behind the active-generation limit. -The wrapped model client separately acquires a FIFO active permit only around -each actual generation call. +The coordinator separately acquires a FIFO active permit only around each +actual model attempt. It releases the permit before a transient retry wait. After a failed `basic`, JSON, or JSON Schema validation with a positive frozen budget, the installed repairer can make a bounded corrective call. Each request @@ -158,9 +158,10 @@ correction carries validation diagnostics as JSON data bounded to 64 KiB; the full diagnostics remain in the validation result. Repair receives the effective execution target, explicit numeric-presence bits, -credential, backend identity, session ID, and structured-output specification. +credential, backend identity, session ID, structured-output specification, and +frozen transient retry budget. The same request constructor supplies those common fields to initial and repair -generation. The default repairer uses the same wrapped client as initial +generation. The default repairer uses the same coordinator as initial generation, so each repair reacquires the selected backend's active permit while remaining inside its original admission lease. Repair never performs a second bounded admission, and repaired outputs use the operation's existing @@ -218,8 +219,9 @@ behavior, hashing, generation and validation outcomes, backend propagation, bounded repair progression, initial/repair request parity, cumulative usage, shared initial/repair capacity, credentials and redaction, error categories, artifact metadata, and timing. The -[capacity subsystem document](capacity.md) identifies the focused pool, -waiter, and wrapped-client tests. +[capacity subsystem document](capacity.md) identifies the focused pool and +waiter tests. The [generation document](generation.md) owns attempt and retry +coordination tests. Changes to orchestration should continue to use the existing package interfaces, keep request state local to an invocation, and preserve the shared diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 751fb9d..88d1533 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -309,6 +309,8 @@ and panic must not leak permits. Run focused race tests and commit. ## Stage 5 — Wire Retry Configuration Through Both Execution Paths +**Completed:** 2026-09-23. + Add public/internal configuration and request fields. Thread the copied effective retry count through ordinary resolution, private prepared payloads, `executePreparedRun`, `newGenerationRequest`, and `RepairRequest`. Do not add it diff --git a/engine.go b/engine.go index 04a28dd..6dd1e62 100644 --- a/engine.go +++ b/engine.go @@ -62,14 +62,18 @@ var ( // available through errors.Is. ErrLLMGenerate = errors.New("failed to generate output") // ErrTransport identifies a failed HTTP request by the built-in model - // client. It also matches ErrLLMGenerate and retains the transport cause - // for errors.Is and errors.As. It does not imply that the provider received - // no work. + // client. Generation errors in this category also match ErrLLMGenerate and + // retain the transport cause for errors.Is and errors.As. It does not imply + // that the provider received no work. ErrTransport = errors.New("model transport failed") // ErrMalformedResponse identifies an invalid successful model response or - // a nil successful response from an injected client. It also matches - // ErrLLMGenerate. + // a nil successful response from an injected client. Generation errors in + // this category also match ErrLLMGenerate. ErrMalformedResponse = errors.New("malformed model response") + // ErrRetriesExhausted identifies a built-in generation sequence whose + // positive transient retry budget ended with another eligible HTTP status. + // RetryExhaustedError retains the final cause and actual attempt count. + ErrRetriesExhausted = errors.New("transient model retries exhausted") // ErrValidation identifies an operational failure to load or compile a // schema or validate output. A completed validation whose Status is // ValidationFailed is returned in RunResult without this error. @@ -113,6 +117,11 @@ type Config struct { // Timeout or the 10-minute default. The supplied client is not mutated. This // field is ignored when WithLLMClient is used. HTTPClient *http.Client + // TransientRetries selects additional built-in HTTP attempts after eligible + // provider statuses. Nil defaults to two; zero disables retries. Negative + // values are invalid. Injected LLMClient implementations are never retried + // automatically. + TransientRetries *int } // Option customizes engine construction. @@ -301,7 +310,8 @@ func WithSchemaFile(path string) Option { // NewEngine returns an error matching ErrInvalidConfig for invalid // configuration, options, or backend-capacity policies. Each constructed // Engine has independent backend-capacity pools. Construction does not perform -// model requests or require credentials. +// model requests or require credentials. A negative Config.TransientRetries +// is invalid. func NewEngine(cfg Config, opts ...Option) (*Engine, error) { var options engineOptions for _, opt := range opts { @@ -314,7 +324,7 @@ func NewEngine(cfg Config, opts ...Option) (*Engine, error) { } assembled, err := internalengine.New( - internalengine.Config{PromptDir: cfg.PromptDir, ProfileDir: cfg.ProfileDir, SchemaDir: cfg.SchemaDir, Timeout: cfg.Timeout, HTTPClient: cfg.HTTPClient}, + internalengine.Config{PromptDir: cfg.PromptDir, ProfileDir: cfg.ProfileDir, SchemaDir: cfg.SchemaDir, Timeout: cfg.Timeout, HTTPClient: cfg.HTTPClient, TransientRetries: copyIntPtr(cfg.TransientRetries)}, options.internal, ) if err != nil { @@ -428,7 +438,7 @@ func (e *Engine) InspectProfile(ctx context.Context, profileID string) (*Profile // applicable. Cancellation is passed to the active collaborator and is // reported in the applicable operation category; no general errors.Is // relationship to ctx.Err is promised. Prepare returns no partial result on -// error. +// error. It validates RunRequest.TransientRetries but makes no model attempts. func (e *Engine) Prepare(ctx context.Context, req RunRequest) (*PreparedRun, error) { if e == nil || e.internal == nil { return nil, fmt.Errorf("%w: engine is nil", ErrInvalidConfig) @@ -461,7 +471,8 @@ func (e *Engine) Prepare(ctx context.Context, req RunRequest) (*PreparedRun, err // returns does not invalidate the handle or propagate to RunPrepared. // PrepareExecution returns the same error categories as [Engine.Prepare] and // returns no handle on error. A nil Engine returns an error matching -// ErrInvalidConfig. +// ErrInvalidConfig. The effective transient retry budget is frozen privately +// for RunPrepared and is absent from Details. func (e *Engine) PrepareExecution(ctx context.Context, req RunRequest) (*PreparedExecution, error) { if e == nil || e.internal == nil { return nil, fmt.Errorf("%w: engine is nil", ErrInvalidConfig) @@ -491,6 +502,13 @@ func (e *Engine) PrepareExecution(ctx context.Context, req RunRequest) (*Prepare // repair attempts. An inability to generate or validate returns an error and // no partial result. // +// The built-in client retries eligible provider statuses using the effective +// Config or request budget. With R additional transient attempts and P repair +// generations, one run can dispatch at most (R+1)×(P+1) Promptkit-controlled +// model attempts. Each dispatched attempt receives its own generation and +// HTTP-client timeout; the caller context governs the full operation. +// Injected LLMClient implementations are not retried automatically. +// // Run can return every error category documented by [Engine.Prepare], plus // ErrCapacityExceeded and ErrLLMGenerate. Built-in HTTP execution failures // additionally match ErrTransport; malformed successful responses and nil @@ -500,8 +518,9 @@ func (e *Engine) PrepareExecution(ctx context.Context, req RunRequest) (*Prepare // occurs before artifacts, schemas, rendering, or model generation because the // selected backend's admission capacity is full; it does not match // ErrInvalidRequest or ErrLLMGenerate. A built-in OpenAI-compatible non-2xx -// response is discoverable as [GenerationError]. Errors from injected clients -// remain available through errors.Is. Cancellation while waiting for +// response is discoverable as [GenerationError]. Eligible-status exhaustion +// with a positive budget is discoverable as [RetryExhaustedError]. Injected +// errors remain available through errors.Is. Cancellation while waiting for // model-generation capacity matches both ErrLLMGenerate and the context error. // Cancellation otherwise follows the active collaborator's documented // behavior. A nil Engine returns ErrInvalidConfig. Run returns no partial @@ -546,9 +565,12 @@ func (e *Engine) Run(ctx context.Context, req RunRequest) (*RunResult, error) { // ErrCapacityExceeded. A built-in OpenAI-compatible non-2xx response is // discoverable as [GenerationError]. Built-in HTTP execution failures match // ErrTransport; malformed successful responses and nil successful injected -// responses match ErrMalformedResponse. A completed content-validation rejection, -// including repair exhaustion, is returned in RunResult, not as an operational -// error. An operational error returns no partial RunResult. +// responses match ErrMalformedResponse. Positive-budget eligible-status +// exhaustion is discoverable as [RetryExhaustedError]. A completed +// content-validation rejection, including repair exhaustion, is returned in +// RunResult, not as an operational +// error. RunPrepared uses the retry choice frozen during PrepareExecution. An +// operational error returns no partial RunResult. func (e *Engine) RunPrepared(ctx context.Context, prepared *PreparedExecution) (*RunResult, error) { if e == nil || e.internal == nil { return nil, fmt.Errorf("%w: engine is nil", ErrInvalidConfig) diff --git a/errors.go b/errors.go index 014bd31..d0f351a 100644 --- a/errors.go +++ b/errors.go @@ -6,6 +6,7 @@ import ( "strings" "gitea.maximumdirect.net/eric/promptkit/internal/capacity" + "gitea.maximumdirect.net/eric/promptkit/internal/generation" "gitea.maximumdirect.net/eric/promptkit/internal/llm" "gitea.maximumdirect.net/eric/promptkit/internal/profile" "gitea.maximumdirect.net/eric/promptkit/internal/promptdef" @@ -21,6 +22,17 @@ func mapPublicError(err error) error { strings.TrimSpace(internalCapacityError.BackendID) != "" { return &CapacityError{BackendID: internalCapacityError.BackendID} } + var internalExhausted *generation.ExhaustedError + if errors.As(err, &internalExhausted) && internalExhausted != nil { + mapped := newRetryExhaustedError(internalExhausted.Attempts(), mapPublicError(internalExhausted.Unwrap())) + if errors.Is(err, usecase.ErrValidation) && !errors.Is(mapped, ErrValidation) { + return fmt.Errorf("%w: %w", ErrValidation, mapped) + } + if publicErr := publicErrorFor(err); publicErr != nil && !errors.Is(mapped, publicErr) { + return fmt.Errorf("%w: %w", publicErr, mapped) + } + return mapped + } publicErr := publicErrorFor(err) var providerHTTPError *llm.ProviderHTTPError if errors.As(err, &providerHTTPError) && providerHTTPError != nil { diff --git a/generation_error_contract_test.go b/generation_error_contract_test.go index 20ace5f..47907ce 100644 --- a/generation_error_contract_test.go +++ b/generation_error_contract_test.go @@ -208,6 +208,8 @@ func newBuiltInGenerationErrorEngine(t *testing.T, statusCode int, body string) t.Helper() config := contractConfig(frameworkSchemaDir) + noRetries := 0 + config.TransientRetries = &noRetries config.HTTPClient = &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { return &http.Response{ StatusCode: statusCode, diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 65f1843..190898f 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -70,6 +70,7 @@ type RunRequest struct { Execution *ExecutionTargetOverride Validation *OutputContract AppendedMessages []RenderedMessage + TransientRetries *int } // RunResult represents the complete result of a prompt execution run. diff --git a/internal/domain/retries.go b/internal/domain/retries.go new file mode 100644 index 0000000..6ec1c1e --- /dev/null +++ b/internal/domain/retries.go @@ -0,0 +1,15 @@ +package domain + +import "fmt" + +// ResolveTransientRetries selects a nonnegative retry budget. A nil selection +// inherits the supplied default. +func ResolveTransientRetries(defaultBudget uint64, selection *int) (uint64, error) { + if selection == nil { + return defaultBudget, nil + } + if *selection < 0 { + return 0, fmt.Errorf("transient retries must not be negative") + } + return uint64(*selection), nil +} diff --git a/internal/engine/engine.go b/internal/engine/engine.go index a29174f..e05e7de 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -19,6 +19,7 @@ import ( "gitea.maximumdirect.net/eric/promptkit/internal/catalog" "gitea.maximumdirect.net/eric/promptkit/internal/defaults" "gitea.maximumdirect.net/eric/promptkit/internal/domain" + "gitea.maximumdirect.net/eric/promptkit/internal/generation" "gitea.maximumdirect.net/eric/promptkit/internal/llm" "gitea.maximumdirect.net/eric/promptkit/internal/profile" "gitea.maximumdirect.net/eric/promptkit/internal/prompt" @@ -29,11 +30,12 @@ import ( // Config carries construction settings selected by the public facade. type Config struct { - PromptDir string - ProfileDir string - SchemaDir string - Timeout time.Duration - HTTPClient *http.Client + PromptDir string + ProfileDir string + SchemaDir string + Timeout time.Duration + HTTPClient *http.Client + TransientRetries *int } // Options collects construction choices from the public facade. @@ -157,6 +159,10 @@ type Engine struct { // New assembles an engine from facade-selected configuration and collaborators. func New(cfg Config, options Options) (*Engine, error) { + defaultRetries, err := domain.ResolveTransientRetries(2, cfg.TransientRetries) + if err != nil { + return nil, err + } promptDefinitions := options.promptDefinitions if !options.promptSource { if strings.TrimSpace(cfg.PromptDir) == "" { @@ -196,20 +202,22 @@ func New(cfg Config, options Options) (*Engine, error) { } validator = validate.NewStandardValidator(schemaDir) } - client := options.llmClient - if client == nil { - client, err = llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{Timeout: cfg.Timeout, HTTPClient: cfg.HTTPClient}) - if err != nil { - return nil, err + var client llm.Client + if options.llmClient == nil { + builtIn, newErr := llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{Timeout: cfg.Timeout, HTTPClient: cfg.HTTPClient}) + if newErr != nil { + return nil, newErr } + client = generation.NewBuiltIn(manager, builtIn) + } else { + client = generation.NewInjected(manager, options.llmClient) } - client = capacity.NewClient(manager, client) reader := options.artifactReader if !options.artifactSource { reader = artifact.NewCompositeReader() } - return &Engine{runner: usecase.NewRunnerWithRepairer(promptDefinitions, profiles, registry, reader, prompt.NewGoRenderer(), client, validator, usecase.NewDefaultOutputRepairer(client), manager)}, nil + return &Engine{runner: usecase.NewRunnerWithRepairer(promptDefinitions, profiles, registry, reader, prompt.NewGoRenderer(), client, validator, usecase.NewDefaultOutputRepairer(client), manager, defaultRetries)}, nil } func newProfileRepository(profileDir string, options Options, maintained profile.Repository) profile.Repository { diff --git a/internal/usecase/generation_request.go b/internal/usecase/generation_request.go index cccf36b..ab950c6 100644 --- a/internal/usecase/generation_request.go +++ b/internal/usecase/generation_request.go @@ -8,6 +8,7 @@ func newGenerationRequest( target domain.ExecutionTarget, targetPresence domain.ExecutionTargetPresence, structuredOutput *domain.StructuredOutputSpec, + transientRetries uint64, ) domain.GenerateRequest { prompt.SessionID = sessionID return domain.GenerateRequest{ @@ -15,5 +16,6 @@ func newGenerationRequest( Target: target, TargetPresence: targetPresence, StructuredOutput: structuredOutput, + TransientRetries: transientRetries, } } diff --git a/internal/usecase/prepared_execution.go b/internal/usecase/prepared_execution.go index 9ca0e88..8fe6310 100644 --- a/internal/usecase/prepared_execution.go +++ b/internal/usecase/prepared_execution.go @@ -29,9 +29,10 @@ type PreparedExecution struct { } type preparedExecutionPayload struct { - prepared *domain.PreparedRun - validation validate.PreparedValidation - directKey string + prepared *domain.PreparedRun + validation validate.PreparedValidation + directKey string + transientRetries uint64 } // PrepareExecution completes preparation without generation or admission and @@ -62,9 +63,10 @@ func (r *Runner) PrepareExecution(ctx context.Context, req domain.RunRequest) (* state: preparedExecutionReady, details: details, payload: &preparedExecutionPayload{ - prepared: executionSnapshot, - validation: operation.validation, - directKey: state.effectiveModel.APIKey, + prepared: executionSnapshot, + validation: operation.validation, + directKey: state.effectiveModel.APIKey, + transientRetries: state.transientRetries, }, }, nil } @@ -129,7 +131,7 @@ func (r *Runner) RunPrepared(ctx context.Context, prepared *PreparedExecution) ( } defer release() - return r.executePreparedRun(ctx, payload.prepared, payload.directKey, runID, start, func( + return r.executePreparedRun(ctx, payload.prepared, payload.directKey, payload.transientRetries, runID, start, func( ctx context.Context, artifact *domain.Artifact, attemptsUsed int, @@ -169,6 +171,7 @@ func (p *preparedExecutionPayload) clear() { p.prepared = nil p.validation = nil p.directKey = "" + p.transientRetries = 0 } type noOpPreparedValidation struct { diff --git a/internal/usecase/repairer.go b/internal/usecase/repairer.go index 9128957..741755e 100644 --- a/internal/usecase/repairer.go +++ b/internal/usecase/repairer.go @@ -31,6 +31,7 @@ type RepairRequest struct { Target domain.ExecutionTarget TargetPresence domain.ExecutionTargetPresence StructuredOutput *domain.StructuredOutputSpec + TransientRetries uint64 Attempt int MaxAttempts int Mode domain.ValidationMode @@ -91,6 +92,7 @@ func (r *defaultOutputRepairer) Repair(ctx context.Context, req RepairRequest) ( req.Target, req.TargetPresence, req.StructuredOutput, + req.TransientRetries, )) if err != nil { return nil, err diff --git a/internal/usecase/runner.go b/internal/usecase/runner.go index 9e74a1e..ec285b7 100644 --- a/internal/usecase/runner.go +++ b/internal/usecase/runner.go @@ -43,15 +43,16 @@ var ( // Runner executes the Promptkit core use case. type Runner struct { - promptDefs promptdef.Repository - profiles profile.Repository - backends BackendResolver - artifacts artifact.Reader - renderer prompt.Renderer - llm llm.Client - validator validate.Validator - repairer OutputRepairer - admitter RunAdmitter + promptDefs promptdef.Repository + profiles profile.Repository + backends BackendResolver + artifacts artifact.Reader + renderer prompt.Renderer + llm llm.Client + validator validate.Validator + repairer OutputRepairer + admitter RunAdmitter + defaultTransientRetries uint64 } // BackendResolver resolves one normalized backend ID. @@ -73,6 +74,7 @@ type preparationState struct { targetPresence domain.ExecutionTargetPresence effectiveContract domain.OutputContract start time.Time + transientRetries uint64 } type preparedOperation struct { @@ -113,17 +115,23 @@ func NewRunnerWithRepairer( validator validate.Validator, repairer OutputRepairer, admitter RunAdmitter, + defaultTransientRetries ...uint64, ) *Runner { + retries := uint64(0) + if len(defaultTransientRetries) > 0 { + retries = defaultTransientRetries[0] + } return &Runner{ - promptDefs: promptDefs, - profiles: profiles, - backends: backends, - artifacts: artifacts, - renderer: renderer, - llm: llmClient, - validator: validator, - repairer: repairer, - admitter: admitter, + promptDefs: promptDefs, + profiles: profiles, + backends: backends, + artifacts: artifacts, + renderer: renderer, + llm: llmClient, + validator: validator, + repairer: repairer, + admitter: admitter, + defaultTransientRetries: retries, } } @@ -152,7 +160,7 @@ func (r *Runner) Run(ctx context.Context, req domain.RunRequest) (*domain.RunRes } directAPIKey := state.effectiveModel.APIKey - return r.executePreparedRun(ctx, operation.run, directAPIKey, runID, start, func( + return r.executePreparedRun(ctx, operation.run, directAPIKey, state.transientRetries, runID, start, func( ctx context.Context, artifact *domain.Artifact, attemptsUsed int, @@ -173,6 +181,7 @@ func (r *Runner) executePreparedRun( ctx context.Context, prepared *domain.PreparedRun, directAPIKey string, + transientRetries uint64, runID string, start time.Time, validateArtifact preparedValidationFunc, @@ -185,6 +194,7 @@ func (r *Runner) executePreparedRun( executionTarget, prepared.TargetPresence, prepared.StructuredOutput, + transientRetries, )) if err != nil { return nil, wrapGenerationError(err) @@ -213,6 +223,7 @@ func (r *Runner) executePreparedRun( Target: executionTarget, TargetPresence: prepared.TargetPresence, StructuredOutput: prepared.StructuredOutput, + TransientRetries: transientRetries, Attempt: attemptsUsed, MaxAttempts: prepared.OutputContract.RepairAttempts, Mode: prepared.OutputContract.ValidationMode, @@ -295,6 +306,10 @@ func (r *Runner) resolvePreparation( req domain.RunRequest, start time.Time, ) (*preparationState, error) { + transientRetries, err := domain.ResolveTransientRetries(r.defaultTransientRetries, req.TransientRetries) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrInvalidRequest, err) + } if strings.TrimSpace(req.PromptID) == "" { return nil, fmt.Errorf("%w: prompt id is required", ErrInvalidRequest) } @@ -346,6 +361,7 @@ func (r *Runner) resolvePreparation( targetPresence: targetPresence, effectiveContract: effectiveContract, start: start, + transientRetries: transientRetries, }, nil } diff --git a/retry_exhausted_error.go b/retry_exhausted_error.go new file mode 100644 index 0000000..b3d4173 --- /dev/null +++ b/retry_exhausted_error.go @@ -0,0 +1,49 @@ +package promptkit + +import "fmt" + +// RetryExhaustedError reports a built-in generation sequence whose positive +// transient retry budget ended with an eligible provider status. Its cause +// remains available through Unwrap, including GenerationError accessors. +// Engine-produced values are immutable. Default and Go-syntax formatting +// redact provider details, and there is no stable JSON representation. +type RetryExhaustedError struct { + attempts uint64 + cause error +} + +func newRetryExhaustedError(attempts uint64, cause error) *RetryExhaustedError { + return &RetryExhaustedError{attempts: attempts, cause: cause} +} + +// Attempts returns the number of model attempts actually dispatched. +func (e *RetryExhaustedError) Attempts() uint64 { + if e == nil { + return 0 + } + return e.attempts +} + +// Error returns a redacted diagnostic. It is safe on a nil receiver. +func (e *RetryExhaustedError) Error() string { + if e == nil || e.attempts == 0 { + return ErrRetriesExhausted.Error() + } + return fmt.Sprintf("%s after %d attempts", ErrRetriesExhausted, e.attempts) +} + +// GoString returns the same redacted diagnostic as Error. +func (e *RetryExhaustedError) GoString() string { return e.Error() } + +// Unwrap returns the final failure cause, if present. +func (e *RetryExhaustedError) Unwrap() error { + if e == nil { + return nil + } + return e.cause +} + +// Is matches ErrRetriesExhausted, including on nil and zero values. +func (e *RetryExhaustedError) Is(target error) bool { + return target == ErrRetriesExhausted +} diff --git a/transient_retries_contract_test.go b/transient_retries_contract_test.go new file mode 100644 index 0000000..6c691d6 --- /dev/null +++ b/transient_retries_contract_test.go @@ -0,0 +1,171 @@ +package promptkit_test + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "strconv" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/promptkit" +) + +func retryHTTPResponse(status int, content string) *http.Response { + body := `{"error":{"message":"private-provider-marker"}}` + if status == http.StatusOK { + body = `{"choices":[{"message":{"content":` + strconv.Quote(content) + `}}]}` + } + return &http.Response{StatusCode: status, Body: io.NopCloser(strings.NewReader(body))} +} + +func retryTestEngine(t *testing.T, budget *int, next func(int) *http.Response) (*promptkit.Engine, *int) { + t.Helper() + calls := 0 + config := contractConfig(frameworkSchemaDir) + config.TransientRetries = budget + config.HTTPClient = &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + calls++ + return next(calls), nil + })} + engine, err := promptkit.NewEngine(config) + if err != nil { + t.Fatal(err) + } + return engine, &calls +} + +func TestBuiltInRetryDefaultsAndRequestOverride(t *testing.T) { + engine, calls := retryTestEngine(t, nil, func(call int) *http.Response { + if call < 3 { + return retryHTTPResponse(503, "") + } + return retryHTTPResponse(200, "ok") + }) + result, err := engine.Run(context.Background(), generationErrorRunRequest()) + if err != nil || result == nil || result.RawOutput != "ok" || *calls != 3 { + t.Fatalf("default result=%+v, error=%v, calls=%d", result, err, *calls) + } + + engineBudget := 1 + engine, calls = retryTestEngine(t, &engineBudget, func(int) *http.Response { return retryHTTPResponse(503, "") }) + engineBudget = 5 // Construction copied the configured value. + zero := 0 + req := generationErrorRunRequest() + req.TransientRetries = &zero + _, err = engine.Run(context.Background(), req) + if !errors.Is(err, promptkit.ErrLLMGenerate) || errors.Is(err, promptkit.ErrRetriesExhausted) || *calls != 1 { + t.Fatalf("zero override: error=%v, calls=%d", err, *calls) + } + req.TransientRetries = nil + _, err = engine.Run(context.Background(), req) + var exhausted *promptkit.RetryExhaustedError + if !errors.As(err, &exhausted) || exhausted.Attempts() != 2 || *calls != 3 { + t.Fatalf("inherited budget: error=%v, calls=%d", err, *calls) + } +} + +func TestRetryValidationAndPreparedFreezing(t *testing.T) { + negative := -1 + config := contractConfig(frameworkSchemaDir) + config.TransientRetries = &negative + if _, err := promptkit.NewEngine(config); !errors.Is(err, promptkit.ErrInvalidConfig) { + t.Fatalf("negative engine budget = %v", err) + } + + engine, calls := retryTestEngine(t, nil, func(int) *http.Response { return retryHTTPResponse(503, "") }) + req := generationErrorRunRequest() + req.TransientRetries = &negative + if _, err := engine.Prepare(context.Background(), req); !errors.Is(err, promptkit.ErrInvalidRequest) { + t.Fatalf("Prepare negative budget = %v", err) + } + if _, err := engine.PrepareExecution(context.Background(), req); !errors.Is(err, promptkit.ErrInvalidRequest) { + t.Fatalf("PrepareExecution negative budget = %v", err) + } + if _, err := engine.Run(context.Background(), req); !errors.Is(err, promptkit.ErrInvalidRequest) { + t.Fatalf("Run negative budget = %v", err) + } + if *calls != 0 { + t.Fatalf("invalid requests dispatched %d calls", *calls) + } + maximum := int(^uint(0) >> 1) + largeBudgetEngine, largeCalls := retryTestEngine(t, &maximum, func(int) *http.Response { + return retryHTTPResponse(200, "ok") + }) + if _, err := largeBudgetEngine.Run(context.Background(), generationErrorRunRequest()); err != nil || *largeCalls != 1 { + t.Fatalf("large valid budget: error=%v, calls=%d", err, *largeCalls) + } + + zero := 0 + req.TransientRetries = &zero + prepared, err := engine.PrepareExecution(context.Background(), req) + if err != nil { + t.Fatal(err) + } + zero = 2 // The handle retains the selected zero. + _, err = engine.RunPrepared(context.Background(), prepared) + if !errors.Is(err, promptkit.ErrLLMGenerate) || errors.Is(err, promptkit.ErrRetriesExhausted) || *calls != 1 { + t.Fatalf("frozen prepared budget: error=%v, calls=%d", err, *calls) + } +} + +func TestBuiltInRetryBudgetAppliesToRepairGeneration(t *testing.T) { + one := 1 + engine, calls := retryTestEngine(t, &one, func(call int) *http.Response { + switch call { + case 1, 3: + return retryHTTPResponse(503, "") + case 2: + return retryHTTPResponse(200, "not-json") + default: + return retryHTTPResponse(200, `{"ok":true}`) + } + }) + req := generationErrorRunRequest() + req.Validation = &promptkit.OutputContract{Format: promptkit.FormatJSON, ValidationMode: promptkit.ValidationJSON, RepairAttempts: 1} + result, err := engine.Run(context.Background(), req) + if err != nil || result == nil || !result.Validation.IsValid || *calls != 4 { + t.Fatalf("repair result=%+v, error=%v, calls=%d", result, err, *calls) + } +} + +func TestRetryExhaustionRetainsPublicProviderCause(t *testing.T) { + one := 1 + engine, calls := retryTestEngine(t, &one, func(int) *http.Response { return retryHTTPResponse(503, "") }) + result, err := engine.Run(context.Background(), generationErrorRunRequest()) + if result != nil || *calls != 2 || !errors.Is(err, promptkit.ErrRetriesExhausted) || !errors.Is(err, promptkit.ErrLLMGenerate) { + t.Fatalf("exhaustion result=%+v, error=%v, calls=%d", result, err, *calls) + } + var exhausted *promptkit.RetryExhaustedError + var provider *promptkit.GenerationError + if !errors.As(err, &exhausted) || exhausted.Attempts() != 2 || !errors.As(err, &provider) || provider.StatusCode() != 503 || provider.ProviderMessage() != "private-provider-marker" { + t.Fatalf("exhaustion cause=%v", err) + } + for _, rendered := range []string{fmt.Sprintf("%v", err), fmt.Sprintf("%#v", err)} { + if strings.Contains(rendered, "private-provider-marker") { + t.Fatalf("provider detail leaked: %s", rendered) + } + } + var zero *promptkit.RetryExhaustedError + if zero.Attempts() != 0 || !errors.Is(zero, promptkit.ErrRetriesExhausted) || zero.Error() == "" || zero.GoString() == "" || zero.Unwrap() != nil { + t.Fatalf("nil error behavior = %v", zero) + } + zeroValue := &promptkit.RetryExhaustedError{} + if zeroValue.Attempts() != 0 || !errors.Is(zeroValue, promptkit.ErrRetriesExhausted) || zeroValue.Error() == "" || zeroValue.GoString() == "" || zeroValue.Unwrap() != nil { + t.Fatalf("zero error behavior = %v", zeroValue) + } +} + +func TestInjectedClientIsNotAutomaticallyRetried(t *testing.T) { + client := &fakeLLMClient{err: errors.New("injected failure")} + engine := newContractEngineWithOptions(t, frameworkSchemaDir, promptkit.WithLLMClient(client)) + count := 3 + req := generationErrorRunRequest() + req.TransientRetries = &count + _, err := engine.Run(context.Background(), req) + if len(client.requests) != 1 || !errors.Is(err, client.err) || errors.Is(err, promptkit.ErrRetriesExhausted) { + t.Fatalf("injected calls=%d, error=%v", len(client.requests), err) + } +} diff --git a/types.go b/types.go index 2197716..787d0ad 100644 --- a/types.go +++ b/types.go @@ -138,6 +138,11 @@ type RunRequest struct { // source or model work. Malformed values return an error matching // ErrInvalidRequest. AppendedMessages []RenderedMessage + // TransientRetries overrides the engine's built-in HTTP retry budget for + // this execution. Nil inherits the engine value; zero disables retries. + // Negative values are invalid for Prepare, PrepareExecution, and Run. + // Injected LLMClient implementations remain single-dispatch. + TransientRetries *int } // PreparedRun contains prepared prompt execution state returned by