From 3bd3c7ebf7f9c2f4d7217fe0ddf86350ab7959d6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 25 Aug 2026 19:34:27 +0000 Subject: [PATCH] Classify PromptKit generation errors safely --- docs/integrations/pkg-promptkit.md | 4 ++ docs/internal/llm.md | 3 ++ docs/roadmap/implementation.md | 2 +- internal/framework/contracts/contracts.go | 34 ++++++++------ internal/framework/contracts/errors.go | 28 +++++++++++ internal/framework/llm/promptkit_client.go | 32 +++++++++++++ .../framework/llm/promptkit_client_test.go | 46 +++++++++++++++++-- 7 files changed, 131 insertions(+), 18 deletions(-) diff --git a/docs/integrations/pkg-promptkit.md b/docs/integrations/pkg-promptkit.md index 48c9ba46..21d1a693 100644 --- a/docs/integrations/pkg-promptkit.md +++ b/docs/integrations/pkg-promptkit.md @@ -103,6 +103,10 @@ pinned upstream documentation. module assets, maps its transport-neutral completion contract, prepares and executes requests, validates output, records provenance, captures debug material, redacts errors, and preserves timeout ownership. + +PromptKit provider error details do not cross the ordinary completion boundary. +Notarius exposes a provider-neutral generation category and optional status; +redacted provider details are retained only in requested debug material. [D&D Module Internals](../internal/dnd.md) owns the embedded `dnd-extraction` fallback profile and the maintained D&D prompt defaults. [Configuration](../config.md#promptkit-profiles) defines how a Notarius diff --git a/docs/internal/llm.md b/docs/internal/llm.md index 58bc000f..3a25aaaa 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -256,6 +256,9 @@ redacted before it crosses the runtime boundary. Known-secret redaction is available to other runtime collaborators; it does not make prompt or response contents safe for general logging. +Generation failures expose an application-owned category and optional HTTP +status. Provider code, type, and message remain debug-only, after redaction. + ## Failure Boundaries - Construction fails for missing asset registries, mutually exclusive profile diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 1b8537ad..32045292 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -263,7 +263,7 @@ git diff --check - Current documentation accurately describes the implemented profile and credential behavior without duplicating PromptKit's merge algorithm. -## Stage 4: Adapt Structured Generation Errors Safely +## Stage 4: Adapt Structured Generation Errors Safely ✅ ### Goal diff --git a/internal/framework/contracts/contracts.go b/internal/framework/contracts/contracts.go index 052e3df5..00fac69a 100644 --- a/internal/framework/contracts/contracts.go +++ b/internal/framework/contracts/contracts.go @@ -60,19 +60,27 @@ type LLMDebugMessage struct { } type LLMDebugResponse struct { - Content string `json:"content,omitempty"` - RunID string `json:"run_id,omitempty"` - PromptID string `json:"prompt_id,omitempty"` - PromptVersion string `json:"prompt_version,omitempty"` - PromptHash string `json:"prompt_hash,omitempty"` - RenderedPromptHash string `json:"rendered_prompt_hash,omitempty"` - SelectedProfileID string `json:"selected_profile_id,omitempty"` - ModelName string `json:"model_name,omitempty"` - Endpoint string `json:"endpoint,omitempty"` - EffectiveModelParams map[string]any `json:"effective_model_params,omitempty"` - InputHashes map[string]string `json:"input_hashes,omitempty"` - Validation map[string]any `json:"validation,omitempty"` - Usage LLMDebugUsage `json:"usage,omitempty"` + Content string `json:"content,omitempty"` + RunID string `json:"run_id,omitempty"` + PromptID string `json:"prompt_id,omitempty"` + PromptVersion string `json:"prompt_version,omitempty"` + PromptHash string `json:"prompt_hash,omitempty"` + RenderedPromptHash string `json:"rendered_prompt_hash,omitempty"` + SelectedProfileID string `json:"selected_profile_id,omitempty"` + ModelName string `json:"model_name,omitempty"` + Endpoint string `json:"endpoint,omitempty"` + EffectiveModelParams map[string]any `json:"effective_model_params,omitempty"` + InputHashes map[string]string `json:"input_hashes,omitempty"` + Validation map[string]any `json:"validation,omitempty"` + Usage LLMDebugUsage `json:"usage,omitempty"` + ProviderError *LLMDebugProviderError `json:"provider_error,omitempty"` +} + +type LLMDebugProviderError struct { + StatusCode int `json:"status_code,omitempty"` + Code string `json:"code,omitempty"` + Type string `json:"type,omitempty"` + Message string `json:"message,omitempty"` } type LLMDebugUsage struct { diff --git a/internal/framework/contracts/errors.go b/internal/framework/contracts/errors.go index a2bc2765..a9c1d421 100644 --- a/internal/framework/contracts/errors.go +++ b/internal/framework/contracts/errors.go @@ -9,3 +9,31 @@ var ErrInvalidStructuredOutput = errors.New("invalid structured output") // ErrLLMCapacityExceeded identifies backend admission exhaustion before model // generation begins. var ErrLLMCapacityExceeded = errors.New("LLM capacity exceeded") + +// ErrLLMGeneration identifies a provider generation failure. +var ErrLLMGeneration = errors.New("LLM generation failed") + +type LLMGenerationError struct { + status int + diagnostic string +} + +func NewLLMGenerationError(status int, diagnostic string) *LLMGenerationError { + if status < 0 { + status = 0 + } + return &LLMGenerationError{status: status, diagnostic: diagnostic} +} +func (e *LLMGenerationError) Error() string { + if e == nil || e.diagnostic == "" { + return ErrLLMGeneration.Error() + } + return e.diagnostic +} +func (e *LLMGenerationError) Unwrap() error { return ErrLLMGeneration } +func (e *LLMGenerationError) StatusCode() int { + if e == nil { + return 0 + } + return e.status +} diff --git a/internal/framework/llm/promptkit_client.go b/internal/framework/llm/promptkit_client.go index 023b9425..80e5487b 100644 --- a/internal/framework/llm/promptkit_client.go +++ b/internal/framework/llm/promptkit_client.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net/http" + "os" "regexp" "sort" "strings" @@ -169,6 +170,16 @@ func (c *PromptKitClient) CompleteStructured(ctx context.Context, req contracts. redactPromptKitError(err), ) } + if errors.Is(err, promptkit.ErrLLMGenerate) { + var generationErr *promptkit.GenerationError + status := 0 + response := contracts.StructuredCompletionResponse{Debug: &contracts.LLMDebugMaterial{Prompt: promptKitDebugPrompt(&preparedDetails)}} + if errors.As(err, &generationErr) { + status = generationErr.StatusCode() + response.Debug.Response = promptKitDebugGenerationError(&preparedDetails, generationErr) + } + return response, contracts.NewLLMGenerationError(status, fmt.Sprintf("run PromptKit prompt %q: %v", promptID, redactPromptKitError(err))) + } return contracts.StructuredCompletionResponse{}, fmt.Errorf("run PromptKit prompt %q: %v", promptID, redactPromptKitError(err)) } if result == nil { @@ -187,6 +198,27 @@ func (c *PromptKitClient) CompleteStructured(ctx context.Context, req contracts. return response, nil } +func promptKitDebugGenerationError(prepared *promptkit.PreparedRun, generationErr *promptkit.GenerationError) *contracts.LLMDebugResponse { + if generationErr == nil { + return nil + } + var secrets []string + if prepared != nil && strings.TrimSpace(prepared.EffectiveModelParams.APIKeyEnv) != "" { + if value, ok := os.LookupEnv(prepared.EffectiveModelParams.APIKeyEnv); ok { + secrets = append(secrets, value) + } + } + redact := func(value string) string { + return bearerTokenPattern.ReplaceAllString(RedactSecrets(value, secrets), "Bearer "+secretReplacement) + } + return &contracts.LLMDebugResponse{ProviderError: &contracts.LLMDebugProviderError{ + StatusCode: generationErr.StatusCode(), + Code: redact(generationErr.ProviderCode()), + Type: redact(generationErr.ProviderType()), + Message: redact(generationErr.ProviderMessage()), + }} +} + func (c *PromptKitClient) responseFromResult(result *promptkit.RunResult, prepared *promptkit.PreparedRun) contracts.StructuredCompletionResponse { content := result.Artifact.Body if len(content) == 0 { diff --git a/internal/framework/llm/promptkit_client_test.go b/internal/framework/llm/promptkit_client_test.go index 9076bca5..0b4241d8 100644 --- a/internal/framework/llm/promptkit_client_test.go +++ b/internal/framework/llm/promptkit_client_test.go @@ -629,6 +629,44 @@ func TestPromptKitClientAllowsMissingOptionalFilesystemCredential(t *testing.T) } } +func TestPromptKitClientMapsHTTPGenerationErrorToApplicationBoundary(t *testing.T) { + const credential = "selected-test-credential" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(`{"error":{"code":"temporary","type":"provider_error","message":"marker ` + credential + ` Bearer bearer-secret"}}`)) + })) + defer server.Close() + t.Setenv("NOTARIUS_GENERATION_TEST_KEY", credential) + profilePath := filepath.Join(t.TempDir(), "profile.yaml") + if err := os.WriteFile(profilePath, []byte("id: generation-profile\nendpoint: "+server.URL+"/v1\nmodel: test\napi_key_env: NOTARIUS_GENERATION_TEST_KEY\n"), 0o600); err != nil { + t.Fatal(err) + } + client, err := NewPromptKitClient(PromptKitClientConfig{Assets: newTestPromptKitAssets(t), ProfileFile: profilePath}) + if err != nil { + t.Fatal(err) + } + var out map[string]any + response, err := client.CompleteStructured(context.Background(), contracts.StructuredCompletionRequest{PromptID: "adapter.test", ProfileID: "generation-profile", SessionID: "generation-error-test", Inputs: contracts.LLMInputSet{"transcript": contracts.NewLLMInputMaterial("transcript", "application/json", []byte(`{"source":true}`), "", "")}}, &out) + if !errors.Is(err, contracts.ErrLLMGeneration) { + t.Fatalf("error = %v", err) + } + var generation *contracts.LLMGenerationError + if !errors.As(err, &generation) || generation.StatusCode() != http.StatusServiceUnavailable { + t.Fatalf("generation error = %#v", generation) + } + if strings.Contains(err.Error(), credential) || strings.Contains(err.Error(), "marker") { + t.Fatalf("ordinary error leaked provider detail: %v", err) + } + if response.Debug == nil || response.Debug.Response == nil || response.Debug.Response.ProviderError == nil { + t.Fatalf("debug = %#v", response.Debug) + } + debug := response.Debug.Response.ProviderError + if debug.StatusCode != http.StatusServiceUnavailable || debug.Code != "temporary" || strings.Contains(debug.Message, credential) || strings.Contains(debug.Message, "bearer-secret") || !strings.Contains(debug.Message, "marker") { + t.Fatalf("debug provider error = %#v", debug) + } +} + func TestPromptKitClientCheckpointFingerprintTracksFallbackProfileAssets(t *testing.T) { fingerprintFor := func(content string) CheckpointFingerprint { t.Helper() @@ -945,8 +983,8 @@ func TestPromptKitClientProviderFailureIncludesContextAndRedactsBearerToken(t *t t.Fatalf("error chain exposes credential: %v", err) } } - if errors.Unwrap(err) != nil { - t.Fatalf("provider failure must not expose a wrapped diagnostic: %v", err) + if !errors.Is(err, contracts.ErrLLMGeneration) { + t.Fatalf("provider failure = %v, want application generation classification", err) } var recoveredProviderErr *credentialBearingProviderError if errors.As(err, &recoveredProviderErr) { @@ -955,8 +993,8 @@ func TestPromptKitClientProviderFailureIncludesContextAndRedactsBearerToken(t *t if errors.Is(err, promptkit.ErrLLMGenerate) { t.Fatalf("provider failure exposes PromptKit generation sentinel: %v", err) } - if resp.Debug != nil { - t.Fatalf("debug material = %#v, want none for provider failure without result", resp.Debug) + if resp.Debug == nil || resp.Debug.Prompt == nil || resp.Debug.Response != nil { + t.Fatalf("debug material = %#v, want prepared prompt without provider details", resp.Debug) } }