From 1d0085450ac088d85d5079a519b4880bdd1591a6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 18 Oct 2026 13:28:29 +0000 Subject: [PATCH] Preserve empty smoke error messages in aggregate reports --- docs/roadmap/remediation.md | 15 ++++++++++++--- integrations/ckprompt/smoke_outcome_test.go | 8 ++++---- integrations/ckprompt/smoke_report.go | 5 +++-- integrations/ckprompt/smoke_report_test.go | 7 ++++++- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 59a8816..85bfe98 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Dependency upgrade and PromptKit smoke review remediation -Status: open; R-1 through R-4 require attention. +Status: in progress; R-1 complete; R-2 through R-4 remain open. ## Review scope and evidence @@ -34,13 +34,14 @@ modules. No test suite was run or added during this review. The completed implementation plan records earlier test runs; those completion markers are historical evidence, not fresh test results from this review. -No production correction has been applied here. The preceding completed +No production correction was applied during the review. Completion records +below describe the subsequent remediation. The preceding completed request-validation review remains available in [Git history](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/730f930b47c929fb2c59a25a75cbe2924dfc75bb/docs/roadmap/remediation.md). ## R-1: Preserve aggregate error presence when its message is empty -Status: open. Priority: P2. Category: report fidelity. +Status: complete. Priority: P2. Category: report fidelity. **Location:** [smoke_report.go](../../integrations/ckprompt/smoke_report.go), `smokeResultDTO` and `encodeSmokeResults`, lines 101–125. @@ -69,6 +70,14 @@ message remains `error: ""`. Confirm the assembled aggregate still returns operational failure and preserves the original error through `errors.Is`. The appropriate focused tests and ckprompt suite pass. +**Completed:** 2026-10-18. Changed the private aggregate error field to an +optional string pointer, preserving non-nil empty messages. Extended the YAML +projection and assembled aggregate regressions to distinguish nil, empty, and +nonempty errors while retaining operational classification and typed cause +identity. Both focused regressions failed before the fix and passed afterward. +The complete ckprompt suite, including the composed consumer, passed with +Go 1.26.8 and offline dependency resolution. + ## R-2: Remove assertions about PromptKit's private smoke-test execution Status: open. Priority: P3. Category: upstream testing ownership. diff --git a/integrations/ckprompt/smoke_outcome_test.go b/integrations/ckprompt/smoke_outcome_test.go index a3c3f8b..f12588d 100644 --- a/integrations/ckprompt/smoke_outcome_test.go +++ b/integrations/ckprompt/smoke_outcome_test.go @@ -141,7 +141,7 @@ func (err *smokeTypedError) Error() string { return err.text } func TestSmokeAggregatePreservesMultipleTypedCauses(t *testing.T) { firstErr := &smokeTypedError{text: "reachability failed"} - secondErr := &smokeTypedError{text: "authentication failed"} + secondErr := &smokeTypedError{text: ""} app := newPromptTestApp(t, Command(Suite[struct{}, struct{}]{ SmokeTest: func(_ context.Context, request SmokeRequest, _ struct{}, _ struct{}) (commandkit.Acquired[*promptkit.SmokeTestReport], error) { var err error @@ -162,14 +162,14 @@ func TestSmokeAggregatePreservesMultipleTypedCauses(t *testing.T) { } var document struct { Tests []struct { - Kind string `yaml:"kind"` - Error string `yaml:"error"` + Kind string `yaml:"kind"` + Error *string `yaml:"error"` } `yaml:"tests"` } if err := yaml.Unmarshal(output.Bytes(), &document); err != nil { t.Fatalf("decode aggregate errors %q: %v", output.String(), err) } - if len(document.Tests) != 4 || document.Tests[0].Kind != string(promptkit.SmokeTestReachability) || document.Tests[0].Error != firstErr.Error() || document.Tests[1].Kind != string(promptkit.SmokeTestAuthentication) || document.Tests[1].Error != secondErr.Error() || document.Tests[2].Error != "" || document.Tests[3].Error != "" { + if len(document.Tests) != 4 || document.Tests[0].Kind != string(promptkit.SmokeTestReachability) || document.Tests[0].Error == nil || *document.Tests[0].Error != firstErr.Error() || document.Tests[1].Kind != string(promptkit.SmokeTestAuthentication) || document.Tests[1].Error == nil || *document.Tests[1].Error != secondErr.Error() || document.Tests[2].Error != nil || document.Tests[3].Error != nil { t.Fatalf("aggregate error entries = %#v", document.Tests) } } diff --git a/integrations/ckprompt/smoke_report.go b/integrations/ckprompt/smoke_report.go index c9e98e6..786f836 100644 --- a/integrations/ckprompt/smoke_report.go +++ b/integrations/ckprompt/smoke_report.go @@ -101,7 +101,7 @@ type smokeAggregateDTO struct { type smokeResultDTO struct { Kind promptkit.SmokeTestKind `yaml:"kind"` Report *smokeReportDTO `yaml:"report"` - Error string `yaml:"error,omitempty"` + Error *string `yaml:"error,omitempty"` } type smokeResult struct { @@ -122,7 +122,8 @@ func encodeSmokeResults(results []smokeResult) ([]byte, error) { Report: smokeReportFromNative(result.Report), } if result.Err != nil { - entries[index].Error = result.Err.Error() + message := result.Err.Error() + entries[index].Error = &message } } return encodePromptYAMLBytes(smokeAggregateDTO{Tests: entries}) diff --git a/integrations/ckprompt/smoke_report_test.go b/integrations/ckprompt/smoke_report_test.go index de22598..8d0c70b 100644 --- a/integrations/ckprompt/smoke_report_test.go +++ b/integrations/ckprompt/smoke_report_test.go @@ -163,6 +163,7 @@ func TestSmokeAggregateYAMLRetainsOrderErrorAndNativeReportKind(t *testing.T) { encoded, err := encodeSmokeResults([]smokeResult{ {Kind: promptkit.SmokeTestTextInference, Report: report, Err: operationErr}, {Kind: promptkit.SmokeTestAuthentication}, + {Kind: promptkit.SmokeTestStructuredOutput, Report: report, Err: errors.New("")}, }) if err != nil { t.Fatal(err) @@ -172,7 +173,7 @@ func TestSmokeAggregateYAMLRetainsOrderErrorAndNativeReportKind(t *testing.T) { t.Fatalf("decode aggregate report: %v\n%s", err, encoded) } tests, ok := document["tests"].([]any) - if !ok || len(tests) != 2 { + if !ok || len(tests) != 3 { t.Fatalf("aggregate entries = %#v", document) } first := tests[0].(map[string]any) @@ -190,6 +191,10 @@ func TestSmokeAggregateYAMLRetainsOrderErrorAndNativeReportKind(t *testing.T) { if _, exists := second["error"]; exists { t.Fatalf("nil error should be omitted: %#v", second) } + third := tests[2].(map[string]any) + if message, exists := third["error"]; !exists || message != "" { + t.Fatalf("empty error message should remain present: %#v", third) + } } func containsSmokeKey(value any, want string) bool {