Preserve empty smoke error messages in aggregate reports

This commit is contained in:
2026-10-18 13:28:29 +00:00
parent 0fe77baa29
commit 1d0085450a
4 changed files with 25 additions and 10 deletions

View File

@@ -1,6 +1,6 @@
# Dependency upgrade and PromptKit smoke review remediation # 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 ## 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 The completed implementation plan records earlier test runs; those completion
markers are historical evidence, not fresh test results from this review. 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 request-validation review remains available in
[Git history](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/730f930b47c929fb2c59a25a75cbe2924dfc75bb/docs/roadmap/remediation.md). [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 ## 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), **Location:** [smoke_report.go](../../integrations/ckprompt/smoke_report.go),
`smokeResultDTO` and `encodeSmokeResults`, lines 101–125. `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`. operational failure and preserves the original error through `errors.Is`.
The appropriate focused tests and ckprompt suite pass. 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 ## R-2: Remove assertions about PromptKit's private smoke-test execution
Status: open. Priority: P3. Category: upstream testing ownership. Status: open. Priority: P3. Category: upstream testing ownership.

View File

@@ -141,7 +141,7 @@ func (err *smokeTypedError) Error() string { return err.text }
func TestSmokeAggregatePreservesMultipleTypedCauses(t *testing.T) { func TestSmokeAggregatePreservesMultipleTypedCauses(t *testing.T) {
firstErr := &smokeTypedError{text: "reachability failed"} firstErr := &smokeTypedError{text: "reachability failed"}
secondErr := &smokeTypedError{text: "authentication failed"} secondErr := &smokeTypedError{text: ""}
app := newPromptTestApp(t, Command(Suite[struct{}, struct{}]{ app := newPromptTestApp(t, Command(Suite[struct{}, struct{}]{
SmokeTest: func(_ context.Context, request SmokeRequest, _ struct{}, _ struct{}) (commandkit.Acquired[*promptkit.SmokeTestReport], error) { SmokeTest: func(_ context.Context, request SmokeRequest, _ struct{}, _ struct{}) (commandkit.Acquired[*promptkit.SmokeTestReport], error) {
var err error var err error
@@ -162,14 +162,14 @@ func TestSmokeAggregatePreservesMultipleTypedCauses(t *testing.T) {
} }
var document struct { var document struct {
Tests []struct { Tests []struct {
Kind string `yaml:"kind"` Kind string `yaml:"kind"`
Error string `yaml:"error"` Error *string `yaml:"error"`
} `yaml:"tests"` } `yaml:"tests"`
} }
if err := yaml.Unmarshal(output.Bytes(), &document); err != nil { if err := yaml.Unmarshal(output.Bytes(), &document); err != nil {
t.Fatalf("decode aggregate errors %q: %v", output.String(), err) 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) t.Fatalf("aggregate error entries = %#v", document.Tests)
} }
} }

View File

@@ -101,7 +101,7 @@ type smokeAggregateDTO struct {
type smokeResultDTO struct { type smokeResultDTO struct {
Kind promptkit.SmokeTestKind `yaml:"kind"` Kind promptkit.SmokeTestKind `yaml:"kind"`
Report *smokeReportDTO `yaml:"report"` Report *smokeReportDTO `yaml:"report"`
Error string `yaml:"error,omitempty"` Error *string `yaml:"error,omitempty"`
} }
type smokeResult struct { type smokeResult struct {
@@ -122,7 +122,8 @@ func encodeSmokeResults(results []smokeResult) ([]byte, error) {
Report: smokeReportFromNative(result.Report), Report: smokeReportFromNative(result.Report),
} }
if result.Err != nil { if result.Err != nil {
entries[index].Error = result.Err.Error() message := result.Err.Error()
entries[index].Error = &message
} }
} }
return encodePromptYAMLBytes(smokeAggregateDTO{Tests: entries}) return encodePromptYAMLBytes(smokeAggregateDTO{Tests: entries})

View File

@@ -163,6 +163,7 @@ func TestSmokeAggregateYAMLRetainsOrderErrorAndNativeReportKind(t *testing.T) {
encoded, err := encodeSmokeResults([]smokeResult{ encoded, err := encodeSmokeResults([]smokeResult{
{Kind: promptkit.SmokeTestTextInference, Report: report, Err: operationErr}, {Kind: promptkit.SmokeTestTextInference, Report: report, Err: operationErr},
{Kind: promptkit.SmokeTestAuthentication}, {Kind: promptkit.SmokeTestAuthentication},
{Kind: promptkit.SmokeTestStructuredOutput, Report: report, Err: errors.New("")},
}) })
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@@ -172,7 +173,7 @@ func TestSmokeAggregateYAMLRetainsOrderErrorAndNativeReportKind(t *testing.T) {
t.Fatalf("decode aggregate report: %v\n%s", err, encoded) t.Fatalf("decode aggregate report: %v\n%s", err, encoded)
} }
tests, ok := document["tests"].([]any) tests, ok := document["tests"].([]any)
if !ok || len(tests) != 2 { if !ok || len(tests) != 3 {
t.Fatalf("aggregate entries = %#v", document) t.Fatalf("aggregate entries = %#v", document)
} }
first := tests[0].(map[string]any) first := tests[0].(map[string]any)
@@ -190,6 +191,10 @@ func TestSmokeAggregateYAMLRetainsOrderErrorAndNativeReportKind(t *testing.T) {
if _, exists := second["error"]; exists { if _, exists := second["error"]; exists {
t.Fatalf("nil error should be omitted: %#v", second) 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 { func containsSmokeKey(value any, want string) bool {