From 10ac29d149c108f0238025a642895c5ae5093c56 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 25 Sep 2026 21:56:29 +0000 Subject: [PATCH] Configure PromptKit transient retries --- docs/config.md | 2 + docs/internal/configuration.md | 6 +- docs/internal/llm.md | 10 +-- docs/operations.md | 11 ++-- docs/roadmap/implementation.md | 2 + examples/dnd-complete.config.yml | 1 + internal/cli/catalog.go | 14 ++-- internal/cli/production_contract_test.go | 12 +++- internal/core/config/config.go | 13 ++-- internal/core/config/document.go | 7 +- internal/core/config/load.go | 3 + internal/core/config/loaded_config.go | 3 + internal/core/config/loaded_config_test.go | 39 +++++++++++ internal/core/config/operational_document.go | 12 ++++ .../core/config/operational_document_test.go | 7 ++ internal/core/config/validation.go | 3 + .../core/config/validation_contract_test.go | 16 +++++ internal/framework/llm/promptkit_client.go | 26 ++++---- .../framework/llm/promptkit_client_test.go | 64 +++++++++++++++++++ 19 files changed, 214 insertions(+), 37 deletions(-) diff --git a/docs/config.md b/docs/config.md index f0ab8403..80e7cef1 100644 --- a/docs/config.md +++ b/docs/config.md @@ -102,6 +102,7 @@ version: 5 promptkit: profile_dir: ./profiles # profile_file: ./profiles.yml + transient_retries: 2 local_backend: endpoint: http://localhost:8000/v1 concurrency_limit: 2 @@ -111,6 +112,7 @@ promptkit: | --- | --- | --- | | **profile_dir** | string | Non-empty directory containing profile files. | | **profile_file** | string | Non-empty profile file. | +| **transient_retries** | integer | Non-negative additional built-in HTTP attempts for eligible transient provider failures; defaults to 2. Set 0 to disable these retries. | | **local_backend** | object | Optional registration for the conventional PromptKit backend ID **local**. | | **local_backend.endpoint** | string | Required when **local_backend** is present; absolute HTTP or HTTPS URL with a host. | | **local_backend.concurrency_limit** | integer | Optional non-negative limit; defaults to 0. | diff --git a/docs/internal/configuration.md b/docs/internal/configuration.md index 3448ed51..714965cf 100644 --- a/docs/internal/configuration.md +++ b/docs/internal/configuration.md @@ -44,7 +44,9 @@ concurrency settings, malformed bindings and references, invalid retries, and invalid pipeline, step, or lane structure. PromptKit local-backend validation accepts only an absolute HTTP or HTTPS endpoint with a host and no user information, query, or fragment, and rejects a negative local concurrency -limit. Its errors retain the closest known pipeline, lane, and binding context. +limit. It also rejects a negative `promptkit.transient_retries` value; decoded +omission resolves to two, while explicit zero remains a distinct opt-out. Its +errors retain the closest known pipeline, lane, and binding context. It deliberately does not require modules to be registered: that requires a catalog and belongs to resolution. @@ -119,6 +121,8 @@ recording arbitrary environment values or provider requests. - Whitespace-normalized identifiers are unique wherever they identify a pipeline, step, lane, worker, or reference slot. - Runtime conversion and resolved-pipeline publication return detached data. +- The resolved PromptKit transient-retry budget is always passed to the shared + built-in client; document presence preserves an explicit zero. - The resolved digest changes when resolved composition changes and never includes itself. diff --git a/docs/internal/llm.md b/docs/internal/llm.md index 509d67b6..7473898e 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -264,10 +264,12 @@ deadline. The HTTP client timeout is a separate transport-wide cap. Notarius forwards the caller context and does not install another timeout wrapper around PromptKit. -The selected PromptKit profile owns generation settings. PromptKit v0.11.0 -performs up to two additional built-in attempts for eligible transient HTTP -failures in one generation, while Notarius binding retries remain outside the -adapter and repeat the complete module operation and validation chain. +The selected PromptKit profile owns generation settings. The resolved global +`promptkit.transient_retries` configuration is passed to the shared PromptKit +engine and controls additional built-in attempts for eligible transient HTTP +failures in one generation. It defaults to two; explicit zero disables those +attempts. Notarius binding retries remain outside the adapter and repeat the +complete module operation and validation chain. Operator-facing behavior is summarized in [Operations](../operations.md#operational-limits), and the pinned upstream contract is identified in diff --git a/docs/operations.md b/docs/operations.md index 001135e1..d3454d62 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -150,10 +150,13 @@ their exit classes are defined in the [CLI reference](cli.md#output-streams-and- Each producer binding has one outer **retries** budget. It covers complete producer attempts for operational failures, invalid structured output, normalizer fallback retry, and semantic correction. It is independent from -PromptKit's structural-repair calls inside one completion and from an -LLM-backed validator's own retry budget. A semantic correction rebuilds the -ordinary producer request and supplies only the latest rejected model response -plus aggregated validator guidance; it is not a conversation replay. +PromptKit's structural-repair calls inside one completion, its configured +`promptkit.transient_retries` built-in HTTP attempts, and an LLM-backed +validator's own retry budget. The transient setting defaults to two additional +attempts for eligible provider statuses; set it explicitly to zero when an +operator needs one HTTP attempt per generation. A semantic correction rebuilds +the ordinary producer request and supplies only the latest rejected model +response plus aggregated validator guidance; it is not a conversation replay. After the applicable budgets are exhausted, the resolved [`validation_policy`](config.md#pipelines) determines the result. Structural diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 517b979c..5132a570 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -133,6 +133,8 @@ dependency's default retry behavior and license accurately. ## Stage 3 — Wire The Explicit Transient Retry Setting +**Status:** Completed 2026-09-25. + **Goal:** Make retry policy inspectable and configurable through the one shared engine. One prompt covering a single vertical configuration slice. diff --git a/examples/dnd-complete.config.yml b/examples/dnd-complete.config.yml index 9d0ddc7e..20a8c7e3 100644 --- a/examples/dnd-complete.config.yml +++ b/examples/dnd-complete.config.yml @@ -4,6 +4,7 @@ configkit: - path: pipelines/dnd-session.yml promptkit: profile_file: ./llm-profiles/dnd-extraction.yml + transient_retries: 2 concurrency: total_llm: 2 stage_workers: diff --git a/internal/cli/catalog.go b/internal/cli/catalog.go index d65ded93..48991212 100644 --- a/internal/cli/catalog.go +++ b/internal/cli/catalog.go @@ -157,13 +157,15 @@ func buildProductionLLMClient(ctx context.Context, cfg config.Config, profileID return nil, nil, fmt.Errorf("production asset registry must not be nil") } recorder := llm.NewLLMProfileRecorder() + transientRetries := cfg.PromptKit.TransientRetries client, err := llm.NewPromptKitClient(llm.PromptKitClientConfig{ - ProfileDir: cfg.PromptKit.ProfileDir, - ProfileFile: cfg.PromptKit.ProfileFile, - LocalBackend: mapPromptKitLocalBackend(cfg.PromptKit.LocalBackend), - Assets: assets, - Recorder: recorder, - ReasoningEffort: overrides.ReasoningEffort, + ProfileDir: cfg.PromptKit.ProfileDir, + ProfileFile: cfg.PromptKit.ProfileFile, + TransientRetries: &transientRetries, + LocalBackend: mapPromptKitLocalBackend(cfg.PromptKit.LocalBackend), + Assets: assets, + Recorder: recorder, + ReasoningEffort: overrides.ReasoningEffort, }) if err != nil { return nil, nil, fmt.Errorf("create PromptKit-backed LLM client: %w", err) diff --git a/internal/cli/production_contract_test.go b/internal/cli/production_contract_test.go index 396827a9..e712d52c 100644 --- a/internal/cli/production_contract_test.go +++ b/internal/cli/production_contract_test.go @@ -557,11 +557,16 @@ func TestNormalizeOptionsSharesProductionProfileAssetsWithDefaultRuntime(t *test func TestProductionLLMClientFactoryUsesConfiguredLocalBackend(t *testing.T) { var providerCalls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - providerCalls.Add(1) + call := providerCalls.Add(1) if r.URL.Path != "/v1/chat/completions" { t.Errorf("provider path = %q, want /v1/chat/completions", r.URL.Path) } w.Header().Set("Content-Type", "application/json") + if call == 1 { + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(`{"error":{"message":"transient"}}`)) + return + } _, _ = w.Write([]byte(`{ "choices": [{"message": {"role": "assistant", "content": "{\"ok\":true}"}}], "usage": {"prompt_tokens": 3, "completion_tokens": 4, "total_tokens": 7} @@ -596,6 +601,7 @@ output: cfg := config.Default() cfg.PromptKit.ProfileFile = profilePath + cfg.PromptKit.TransientRetries = 1 cfg.PromptKit.LocalBackend = &config.PromptKitLocalBackendConfig{ Endpoint: server.URL + "/v1", ConcurrencyLimit: 2, @@ -624,8 +630,8 @@ output: if err != nil { t.Fatalf("CompleteStructured() error = %v, want nil", err) } - if providerCalls.Load() != 1 { - t.Fatalf("provider calls = %d, want 1", providerCalls.Load()) + if providerCalls.Load() != 2 { + t.Fatalf("provider calls = %d, want 2", providerCalls.Load()) } provider, ok := client.(contracts.LLMProfileManifestProvider) if !ok { diff --git a/internal/core/config/config.go b/internal/core/config/config.go index bb329a11..a0a31e06 100644 --- a/internal/core/config/config.go +++ b/internal/core/config/config.go @@ -14,9 +14,10 @@ type Config struct { } type PromptKitConfig struct { - ProfileDir string `json:"profile_dir,omitempty"` - ProfileFile string `json:"profile_file,omitempty"` - LocalBackend *PromptKitLocalBackendConfig `json:"local_backend,omitempty"` + ProfileDir string `json:"profile_dir,omitempty"` + ProfileFile string `json:"profile_file,omitempty"` + TransientRetries int `json:"transient_retries"` + LocalBackend *PromptKitLocalBackendConfig `json:"local_backend,omitempty"` } type PromptKitLocalBackendConfig struct { @@ -29,7 +30,10 @@ type ConcurrencyConfig struct { StageWorkers map[string]int `json:"stage_workers"` } -const defaultLLMConcurrency = 16 +const ( + defaultLLMConcurrency = 16 + defaultTransientRetries = 2 +) type OutputConfig struct { Directory string `json:"directory"` @@ -55,6 +59,7 @@ type DebugConfig struct { func Default() Config { return Config{ + PromptKit: PromptKitConfig{TransientRetries: defaultTransientRetries}, Pipelines: map[string]pipeline.PipelineProfile{}, Concurrency: ConcurrencyConfig{ TotalLLM: defaultLLMConcurrency, diff --git a/internal/core/config/document.go b/internal/core/config/document.go index bb0ef288..c99b0924 100644 --- a/internal/core/config/document.go +++ b/internal/core/config/document.go @@ -16,9 +16,10 @@ type Document struct { } type PromptKitDocument struct { - ProfileDir *string `yaml:"profile_dir,omitempty"` - ProfileFile *string `yaml:"profile_file,omitempty"` - LocalBackend *PromptKitLocalBackendDocument `yaml:"local_backend,omitempty"` + ProfileDir *string `yaml:"profile_dir,omitempty"` + ProfileFile *string `yaml:"profile_file,omitempty"` + TransientRetries *int `yaml:"transient_retries,omitempty"` + LocalBackend *PromptKitLocalBackendDocument `yaml:"local_backend,omitempty"` } type PromptKitLocalBackendDocument struct { diff --git a/internal/core/config/load.go b/internal/core/config/load.go index a83ac5a9..f0ec9b80 100644 --- a/internal/core/config/load.go +++ b/internal/core/config/load.go @@ -63,6 +63,9 @@ func documentDefinition(state *documentLoadState) configkit.Definition[Document] func documentDefaults() Document { return Document{ + PromptKit: &PromptKitDocument{ + TransientRetries: documentValue(defaultTransientRetries), + }, Pipelines: map[string]PipelineDocument{}, Concurrency: &ConcurrencyDocument{ TotalLLM: documentValue(defaultLLMConcurrency), diff --git a/internal/core/config/loaded_config.go b/internal/core/config/loaded_config.go index b62f11d4..6d70ff55 100644 --- a/internal/core/config/loaded_config.go +++ b/internal/core/config/loaded_config.go @@ -94,6 +94,9 @@ func documentToConfig(document Document) (Config, error) { if document.PromptKit.ProfileFile != nil { config.PromptKit.ProfileFile = *document.PromptKit.ProfileFile } + if document.PromptKit.TransientRetries != nil { + config.PromptKit.TransientRetries = *document.PromptKit.TransientRetries + } if document.PromptKit.LocalBackend != nil { backend := PromptKitLocalBackendConfig{} if document.PromptKit.LocalBackend.Endpoint != nil { diff --git a/internal/core/config/loaded_config_test.go b/internal/core/config/loaded_config_test.go index 9a35934b..c2b6b62d 100644 --- a/internal/core/config/loaded_config_test.go +++ b/internal/core/config/loaded_config_test.go @@ -2,6 +2,7 @@ package config import ( "bytes" + "strconv" "strings" "testing" @@ -58,6 +59,44 @@ pipelines: } } +func TestLoadedConfigResolvesPromptKitTransientRetriesAndRetainsInspection(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + document string + want int + }{ + {name: "omitted", document: "version: 5\n", want: defaultTransientRetries}, + {name: "disabled", document: "version: 5\npromptkit:\n transient_retries: 0\n", want: 0}, + {name: "configured", document: "version: 5\npromptkit:\n transient_retries: 5\n", want: 5}, + } { + t.Run(test.name, func(t *testing.T) { + loaded, err := loadConfigSource(configkit.Bytes("document.yml", []byte(test.document)), LoadOptions{}) + if err != nil { + t.Fatal(err) + } + if got := loaded.Config.PromptKit.TransientRetries; got != test.want { + t.Fatalf("transient retries = %d, want %d", got, test.want) + } + var effective bytes.Buffer + if err := loaded.WriteEffective(&effective); err != nil { + t.Fatal(err) + } + if !strings.Contains(effective.String(), "transient_retries: "+strconv.Itoa(test.want)) { + t.Fatalf("effective configuration = %q", effective.String()) + } + var explanation bytes.Buffer + if err := loaded.WriteExplanation(&explanation, "/promptkit/transient_retries"); err != nil { + t.Fatal(err) + } + if !strings.Contains(explanation.String(), strconv.Itoa(test.want)) { + t.Fatalf("explanation = %q", explanation.String()) + } + }) + } +} + func TestLoadedConfigOwnsRuntimeValuesAndKeepsFrozenInspection(t *testing.T) { t.Parallel() diff --git a/internal/core/config/operational_document.go b/internal/core/config/operational_document.go index 192fe8ad..e966c0f7 100644 --- a/internal/core/config/operational_document.go +++ b/internal/core/config/operational_document.go @@ -167,6 +167,11 @@ func validateDocumentPromptKit(promptKit *PromptKitDocument, validation *configk if err := validatePromptKitProfileSources(profileDir, profileFile); err != nil { validation.Error(configkit.Keys("promptkit"), err.Error()) } + if promptKit.TransientRetries == nil { + validation.Error(configkit.Keys("promptkit", "transient_retries"), "must be an integer") + } else if err := validatePromptKitTransientRetries(*promptKit.TransientRetries); err != nil { + validation.Error(configkit.Keys("promptkit", "transient_retries"), err.Error()) + } if promptKit.LocalBackend == nil { return } @@ -283,6 +288,13 @@ func validatePromptKitLocalBackendConcurrencyLimit(concurrencyLimit int) error { return nil } +func validatePromptKitTransientRetries(transientRetries int) error { + if transientRetries < 0 { + return fmt.Errorf("promptkit.transient_retries must not be negative") + } + return nil +} + func validateRequiredPath(name, value string) error { if strings.TrimSpace(value) == "" { return fmt.Errorf("%s must not be empty", name) diff --git a/internal/core/config/operational_document_test.go b/internal/core/config/operational_document_test.go index 3657f261..f18c0268 100644 --- a/internal/core/config/operational_document_test.go +++ b/internal/core/config/operational_document_test.go @@ -223,6 +223,13 @@ cache: assertDocumentDiagnostic(t, err, configkit.CodeValidateError, "/promptkit/local_backend/endpoint") } +func TestDocumentLoadRejectsNegativePromptKitTransientRetries(t *testing.T) { + t.Parallel() + + _, err := loadDocumentSource(configkit.Bytes("promptkit.yml", []byte("version: 5\npromptkit:\n transient_retries: -1\n")), LoadOptions{}) + assertDocumentDiagnostic(t, err, configkit.CodeValidateError, "/promptkit/transient_retries") +} + func assertDocumentDiagnostic(t *testing.T, err error, code configkit.Code, path string) { t.Helper() var loadError *configkit.LoadError diff --git a/internal/core/config/validation.go b/internal/core/config/validation.go index 65257fda..0af9a745 100644 --- a/internal/core/config/validation.go +++ b/internal/core/config/validation.go @@ -46,6 +46,9 @@ func validatePromptKit(cfg PromptKitConfig) error { if err := validatePromptKitProfileSources(cfg.ProfileDir, cfg.ProfileFile); err != nil { return err } + if err := validatePromptKitTransientRetries(cfg.TransientRetries); err != nil { + return err + } if cfg.LocalBackend == nil { return nil } diff --git a/internal/core/config/validation_contract_test.go b/internal/core/config/validation_contract_test.go index d810f1c1..b863c652 100644 --- a/internal/core/config/validation_contract_test.go +++ b/internal/core/config/validation_contract_test.go @@ -89,6 +89,22 @@ func TestValidatePromptKitSourcesAreMutuallyExclusive(t *testing.T) { assertValidationContains(t, cfg, "promptkit profile_dir and profile_file are mutually exclusive") } +func TestValidatePromptKitTransientRetries(t *testing.T) { + if got := Default().PromptKit.TransientRetries; got != defaultTransientRetries { + t.Fatalf("default transient retries = %d, want %d", got, defaultTransientRetries) + } + for _, retries := range []int{0, 4} { + cfg := Default() + cfg.PromptKit.TransientRetries = retries + if err := cfg.Validate(); err != nil { + t.Fatalf("Validate() with transient retries %d error = %v", retries, err) + } + } + negative := Default() + negative.PromptKit.TransientRetries = -1 + assertValidationContains(t, negative, "promptkit.transient_retries must not be negative") +} + func TestValidatePromptKitLocalBackendEndpoints(t *testing.T) { tests := []struct { name string diff --git a/internal/framework/llm/promptkit_client.go b/internal/framework/llm/promptkit_client.go index 293055b6..d91aab18 100644 --- a/internal/framework/llm/promptkit_client.go +++ b/internal/framework/llm/promptkit_client.go @@ -26,15 +26,16 @@ type PromptKitLocalBackendConfig struct { } type PromptKitClientConfig struct { - ProfileDir string - ProfileFile string - LocalBackend *PromptKitLocalBackendConfig - Assets *AssetRegistry - Timeout time.Duration - HTTPClient *http.Client - EngineOptions []promptkit.Option - Recorder *LLMProfileRecorder - ReasoningEffort *string + ProfileDir string + ProfileFile string + TransientRetries *int + LocalBackend *PromptKitLocalBackendConfig + Assets *AssetRegistry + Timeout time.Duration + HTTPClient *http.Client + EngineOptions []promptkit.Option + Recorder *LLMProfileRecorder + ReasoningEffort *string } type PromptKitClient struct { @@ -79,9 +80,10 @@ func NewPromptKitClient(cfg PromptKitClientConfig) (*PromptKitClient, error) { options = append(options, cfg.EngineOptions...) engine, err := promptkit.NewEngine(promptkit.Config{ - ProfileDir: profileSource.ProfileDir, - Timeout: cfg.Timeout, - HTTPClient: cfg.HTTPClient, + ProfileDir: profileSource.ProfileDir, + Timeout: cfg.Timeout, + HTTPClient: cfg.HTTPClient, + TransientRetries: cfg.TransientRetries, }, options...) if err != nil { return nil, fmt.Errorf("create PromptKit engine: %w", err) diff --git a/internal/framework/llm/promptkit_client_test.go b/internal/framework/llm/promptkit_client_test.go index 1b7ff767..48deb48a 100644 --- a/internal/framework/llm/promptkit_client_test.go +++ b/internal/framework/llm/promptkit_client_test.go @@ -971,6 +971,70 @@ model: local-model } } +func TestPromptKitClientAppliesConfiguredBuiltInTransientRetries(t *testing.T) { + for _, test := range []struct { + name string + retries int + statuses []int + wantCalls int32 + wantErr bool + }{ + {name: "eligible failure then success", retries: 1, statuses: []int{http.StatusServiceUnavailable, http.StatusOK}, wantCalls: 2}, + {name: "disabled", retries: 0, statuses: []int{http.StatusServiceUnavailable}, wantCalls: 1, wantErr: true}, + {name: "exhausted", retries: 1, statuses: []int{http.StatusServiceUnavailable, http.StatusServiceUnavailable}, wantCalls: 2, wantErr: true}, + {name: "noneligible client error", retries: 2, statuses: []int{http.StatusBadRequest}, wantCalls: 1, wantErr: true}, + } { + t.Run(test.name, func(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + index := calls.Add(1) - 1 + if int(index) >= len(test.statuses) { + t.Errorf("unexpected HTTP attempt %d", index+1) + w.WriteHeader(http.StatusInternalServerError) + return + } + status := test.statuses[index] + w.Header().Set("Content-Type", "application/json") + if status != http.StatusOK { + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"error":{"message":"provider failure"}}`)) + return + } + _, _ = w.Write([]byte(`{"choices":[{"message":{"content":"{\"ok\":true}"}}]}`)) + })) + defer server.Close() + + profilePath := filepath.Join(t.TempDir(), "profile.yml") + if err := os.WriteFile(profilePath, []byte("id: retry-profile\nendpoint: "+server.URL+"/v1\nmodel: retry-model\n"), 0o600); err != nil { + t.Fatal(err) + } + client, err := NewPromptKitClient(PromptKitClientConfig{ + Assets: newTestPromptKitAssets(t), + ProfileFile: profilePath, + TransientRetries: &test.retries, + }) + if err != nil { + t.Fatal(err) + } + var out map[string]any + _, err = client.CompleteStructured(context.Background(), contracts.StructuredCompletionRequest{ + PromptID: "adapter.test", + ProfileID: "retry-profile", + SessionID: "retry-test", + Inputs: contracts.LLMInputSet{ + "transcript": contracts.NewLLMInputMaterial("transcript", "application/json", []byte(`{"source":true}`), "", ""), + }, + }, &out) + if (err != nil) != test.wantErr { + t.Fatalf("CompleteStructured() error = %v, want error=%t", err, test.wantErr) + } + if got := calls.Load(); got != test.wantCalls { + t.Fatalf("HTTP calls = %d, want %d", got, test.wantCalls) + } + }) + } +} + func TestLLMProfileRecorderDistinguishesEffectiveTargets(t *testing.T) { recorder := NewLLMProfileRecorder() for _, profile := range []artifacts.LLMProfileManifest{