Separate ConfigKit messages from explanations

2026-10-26 03:10:18 +00:00
parent f3e9326544
commit 9a4d1db955
5 changed files with 231 additions and 78 deletions
--- a/integrations/ckconfig/comparison_adapter_test.go
+++ b/integrations/ckconfig/comparison_adapter_test.go
@@ -192,14 +192,14 @@ func TestFromConfigKitCompareRetainsSideDiagnosticsAndLoadError(t *testing.T) {
 	if !errors.As(gotErr, &typed) || typed != loadErr {
 		t.Fatalf("load error type not preserved: %#v", typed)
 	}
-	if len(acquired.Diagnostics) != 2 || acquired.Diagnostics[0].Severity != commandkit.SeverityWarning || acquired.Diagnostics[1].Severity != commandkit.SeverityError {
-		t.Fatalf("retained diagnostics = %#v", acquired.Diagnostics)
+	if len(acquired.Diagnostics) != 1 || acquired.Diagnostics[0].Severity != commandkit.SeverityWarning || len(acquired.Explanation.Details) != 1 {
+		t.Fatalf("retained diagnostics/explanation = %#v / %#v", acquired.Diagnostics, acquired.Explanation)
 	}
 	var leftText, rightText bytes.Buffer
 	if err := acquired.Diagnostics[0].Write(&leftText); err != nil {
 		t.Fatal(err)
 	}
-	if err := acquired.Diagnostics[1].Write(&rightText); err != nil {
+	if err := acquired.Explanation.Details[0](&rightText); err != nil {
 		t.Fatal(err)
 	}
 	if !strings.Contains(leftText.String(), `Configuration scope "app" left:`) || !strings.Contains(leftText.String(), `"left-source"`) {
@@ -231,8 +231,15 @@ func TestFromConfigKitCompareStopsAfterLeftLoadFailure(t *testing.T) {
 		t.Fatalf("left failure/right load = %v / %v", err, rightCalled)
 	}
 	var typed *configkit.LoadError
-	if !errors.As(err, &typed) || typed != loadErr || len(acquired.Diagnostics) != 1 || acquired.Diagnostics[0].Severity != commandkit.SeverityError {
-		t.Fatalf("left failure identity/diagnostics = %v / %#v", err, acquired.Diagnostics)
+	if !errors.As(err, &typed) || typed != loadErr || len(acquired.Explanation.Details) != 1 {
+		t.Fatalf("left failure identity/explanation = %v / %#v", err, acquired.Explanation)
+	}
+	var terminal bytes.Buffer
+	if err := acquired.Explanation.Details[0](&terminal); err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(terminal.String(), `Configuration scope "app" left:`) || !strings.Contains(terminal.String(), `"left-source"`) {
+		t.Fatalf("left terminal context lost side/source identity: %q", terminal.String())
 	}
 }

--- a/integrations/ckconfig/configkit_adapter.go
+++ b/integrations/ckconfig/configkit_adapter.go
@@ -33,53 +33,58 @@ func FromConfigKit[I, S, T any](

 	scope.Acquire = func(ctx context.Context, request Request, input I, state S) (commandkit.Acquired[View], error) {
 		result, err := load(ctx, request.Selection, input, state)
+		acquired := commandkit.Acquired[View]{}
+		if result != nil {
+			acquired.Value = View{Effective: result, Sources: result, Explanation: result}
+			acquired.Diagnostics, acquired.Explanation = configKitDiagnostics(name, result.Sources(), result.Diagnostics())
+		}
 		if err != nil {
 			var loadError *configkit.LoadError
 			if errors.As(err, &loadError) && loadError != nil {
-				return commandkit.Acquired[View]{Diagnostics: configKitDiagnostics(name, loadError.Sources(), loadError.Diagnostics())}, err
+				warnings, explanation := configKitDiagnostics(name, loadError.Sources(), loadError.Diagnostics())
+				acquired.Diagnostics = append(acquired.Diagnostics, warnings...)
+				acquired.Explanation = appendConfigKitExplanation(acquired.Explanation, explanation)
 			}
-			return commandkit.Acquired[View]{}, err
+			return acquired, err
 		}
 		if result == nil {
 			return commandkit.Acquired[View]{}, fmt.Errorf("ConfigKit scope %q returned a nil result", name)
 		}
-
-		sources := result.Sources()
-		return commandkit.Acquired[View]{
-			Value:       View{Effective: result, Sources: result, Explanation: result},
-			Diagnostics: configKitDiagnostics(name, sources, result.Diagnostics()),
-		}, nil
+		return acquired, nil
 	}
 	scope.Compare = func(ctx context.Context, request ComparisonRequest[I], state S) (commandkit.Acquired[[]configkit.Difference], error) {
 		if err := ctx.Err(); err != nil {
 			return commandkit.Acquired[[]configkit.Difference]{}, fmt.Errorf("ConfigKit scope %q comparison before left load: %w", name, err)
 		}

-		left, leftDiagnostics, err := loadComparisonSide(ctx, name, "left", load, request.Left, state)
-		diagnostics := append([]commandkit.Diagnostic(nil), leftDiagnostics...)
+		acquired := commandkit.Acquired[[]configkit.Difference]{}
+		left, leftDiagnostics, leftExplanation, err := loadComparisonSide(ctx, name, "left", load, request.Left, state)
+		acquired.Diagnostics = append(acquired.Diagnostics, leftDiagnostics...)
+		acquired.Explanation = appendConfigKitExplanation(acquired.Explanation, leftExplanation)
 		if err != nil {
-			return commandkit.Acquired[[]configkit.Difference]{Diagnostics: diagnostics}, err
+			return acquired, err
 		}

 		if err := ctx.Err(); err != nil {
-			return commandkit.Acquired[[]configkit.Difference]{Diagnostics: diagnostics}, fmt.Errorf("ConfigKit scope %q comparison after left load: %w", name, err)
+			return acquired, fmt.Errorf("ConfigKit scope %q comparison after left load: %w", name, err)
 		}

-		right, rightDiagnostics, err := loadComparisonSide(ctx, name, "right", load, request.Right, state)
-		diagnostics = append(diagnostics, rightDiagnostics...)
+		right, rightDiagnostics, rightExplanation, err := loadComparisonSide(ctx, name, "right", load, request.Right, state)
+		acquired.Diagnostics = append(acquired.Diagnostics, rightDiagnostics...)
+		acquired.Explanation = appendConfigKitExplanation(acquired.Explanation, rightExplanation)
 		if err != nil {
-			return commandkit.Acquired[[]configkit.Difference]{Diagnostics: diagnostics}, err
+			return acquired, err
 		}

 		if err := ctx.Err(); err != nil {
-			return commandkit.Acquired[[]configkit.Difference]{Diagnostics: diagnostics}, fmt.Errorf("ConfigKit scope %q comparison before native diff: %w", name, err)
+			return acquired, fmt.Errorf("ConfigKit scope %q comparison before native diff: %w", name, err)
 		}

-		differences, err := configkit.Diff(left, right)
+		acquired.Value, err = configkit.Diff(left, right)
 		if err != nil {
-			return commandkit.Acquired[[]configkit.Difference]{Diagnostics: diagnostics}, fmt.Errorf("ConfigKit scope %q native diff: %w", name, err)
+			return acquired, fmt.Errorf("ConfigKit scope %q native diff: %w", name, err)
 		}
-		return commandkit.Acquired[[]configkit.Difference]{Value: differences, Diagnostics: diagnostics}, nil
+		return acquired, nil
 	}
 	return scope
 }
@@ -90,67 +95,110 @@ func loadComparisonSide[I, S, T any](
 	load func(context.Context, Selection, I, S) (*configkit.Result[T], error),
 	request ComparisonSide[I],
 	state S,
-) (*configkit.Result[T], []commandkit.Diagnostic, error) {
+) (*configkit.Result[T], []commandkit.Diagnostic, commandkit.Explanation, error) {
 	result, err := load(ctx, request.Selection, request.Input, state)
+	var diagnostics []commandkit.Diagnostic
+	var explanation commandkit.Explanation
+	if result != nil {
+		warnings, terminal := configKitComparisonDiagnostics(scope, side, result.Sources(), result.Diagnostics())
+		diagnostics = append(diagnostics, warnings...)
+		explanation = appendConfigKitExplanation(explanation, terminal)
+	}
 	if err != nil {
 		var loadError *configkit.LoadError
-		var diagnostics []commandkit.Diagnostic
 		if errors.As(err, &loadError) && loadError != nil {
-			diagnostics = configKitComparisonDiagnostics(scope, side, loadError.Sources(), loadError.Diagnostics())
+			warnings, terminal := configKitComparisonDiagnostics(scope, side, loadError.Sources(), loadError.Diagnostics())
+			diagnostics = append(diagnostics, warnings...)
+			explanation = appendConfigKitExplanation(explanation, terminal)
 		}
-		return nil, diagnostics, fmt.Errorf("ConfigKit scope %q %s load: %w", scope, side, err)
+		return result, diagnostics, explanation, fmt.Errorf("ConfigKit scope %q %s load: %w", scope, side, err)
 	}
 	if result == nil {
-		return nil, nil, fmt.Errorf("ConfigKit scope %q %s loader returned a nil result", scope, side)
+		return nil, diagnostics, explanation, fmt.Errorf("ConfigKit scope %q %s loader returned a nil result", scope, side)
 	}
-	return result, configKitComparisonDiagnostics(scope, side, result.Sources(), result.Diagnostics()), nil
+	return result, diagnostics, explanation, nil
 }

-func configKitDiagnostics(scope string, sources []configkit.SourceInfo, records []configkit.Diagnostic) []commandkit.Diagnostic {
+func configKitDiagnostics(scope string, sources []configkit.SourceInfo, records []configkit.Diagnostic) ([]commandkit.Diagnostic, commandkit.Explanation) {
 	return configKitDiagnosticsWithHeading("Configuration scope "+strconv.Quote(scope)+":\n", sources, records)
 }

-func configKitComparisonDiagnostics(scope, side string, sources []configkit.SourceInfo, records []configkit.Diagnostic) []commandkit.Diagnostic {
+func configKitComparisonDiagnostics(scope, side string, sources []configkit.SourceInfo, records []configkit.Diagnostic) ([]commandkit.Diagnostic, commandkit.Explanation) {
 	return configKitDiagnosticsWithHeading("Configuration scope "+strconv.Quote(scope)+" "+side+":\n", sources, records)
 }

-func configKitDiagnosticsWithHeading(heading string, sources []configkit.SourceInfo, records []configkit.Diagnostic) []commandkit.Diagnostic {
+func configKitDiagnosticsWithHeading(heading string, sources []configkit.SourceInfo, records []configkit.Diagnostic) ([]commandkit.Diagnostic, commandkit.Explanation) {
 	if len(records) == 0 {
-		return nil
+		return nil, commandkit.Explanation{}
 	}

-	severity := commandkit.SeverityWarning
+	sources = append([]configkit.SourceInfo(nil), sources...)
+	records = cloneConfigKitDiagnostics(records)
+	warnings := make([]configkit.Diagnostic, 0, len(records))
+	terminal := make([]configkit.Diagnostic, 0, len(records))
 	for _, record := range records {
 		if record.Severity == configkit.SeverityError {
-			severity = commandkit.SeverityError
-			break
+			terminal = append(terminal, record)
+		} else {
+			warnings = append(warnings, record)
 		}
 	}

-	// Capture the upstream snapshots so later calls to Write do not depend on
-	// slices returned by the provider.
-	sources = append([]configkit.SourceInfo(nil), sources...)
-	records = append([]configkit.Diagnostic(nil), records...)
-	return []commandkit.Diagnostic{{
-		Severity: severity,
-		Write: func(writer io.Writer) error {
-			if err := writeDiagnosticText(writer, heading); err != nil {
+	if len(warnings) == 0 {
+		return nil, explanationWithConfigKitRecords(heading, sources, terminal)
+	}
+	warning := commandkit.Diagnostic{
+		Severity: commandkit.SeverityWarning,
+		Write:    configKitDiagnosticWriter(heading, sources, warnings),
+	}
+	return []commandkit.Diagnostic{warning}, explanationWithConfigKitRecords(heading, sources, terminal)
+}
+
+func explanationWithConfigKitRecords(heading string, sources []configkit.SourceInfo, records []configkit.Diagnostic) commandkit.Explanation {
+	if len(records) == 0 {
+		return commandkit.Explanation{}
+	}
+	return commandkit.Explanation{Details: []commandkit.Detail{configKitDiagnosticWriter(heading, sources, records)}}
+}
+
+func configKitDiagnosticWriter(heading string, sources []configkit.SourceInfo, records []configkit.Diagnostic) commandkit.Detail {
+	return func(writer io.Writer) error {
+		if err := writeDiagnosticText(writer, heading); err != nil {
+			return err
+		}
+		if len(sources) > 0 {
+			if err := writeDiagnosticText(writer, "Source IDs:\n"); err != nil {
 				return err
 			}
-			if len(sources) > 0 {
-				if err := writeDiagnosticText(writer, "Source IDs:\n"); err != nil {
+			for _, source := range sources {
+				line := "  " + strconv.Quote(source.ID) + ": label=" + strconv.Quote(source.Label) + ", path=" + strconv.Quote(source.Path) + "\n"
+				if err := writeDiagnosticText(writer, line); err != nil {
 					return err
 				}
-				for _, source := range sources {
-					line := "  " + strconv.Quote(source.ID) + ": label=" + strconv.Quote(source.Label) + ", path=" + strconv.Quote(source.Path) + "\n"
-					if err := writeDiagnosticText(writer, line); err != nil {
-						return err
-					}
-				}
 			}
-			return configkit.WriteDiagnostics(writer, records)
-		},
-	}}
+		}
+		return configkit.WriteDiagnostics(writer, records)
+	}
+}
+
+func cloneConfigKitDiagnostics(records []configkit.Diagnostic) []configkit.Diagnostic {
+	cloned := append([]configkit.Diagnostic(nil), records...)
+	for index := range cloned {
+		if records[index].Primary != nil {
+			primary := *records[index].Primary
+			cloned[index].Primary = &primary
+		}
+		cloned[index].Related = append([]configkit.Location(nil), records[index].Related...)
+	}
+	return cloned
+}
+
+func appendConfigKitExplanation(left, right commandkit.Explanation) commandkit.Explanation {
+	if left.Message == "" {
+		left.Message = right.Message
+	}
+	left.Details = append(left.Details, right.Details...)
+	return left
 }

 func writeDiagnosticText(writer io.Writer, value string) error {
--- a/integrations/ckconfig/contracts.go
+++ b/integrations/ckconfig/contracts.go
@@ -98,12 +98,13 @@ type Scope[I, S any] struct {
 	// Operations selects the standard handlers exposed for this scope.
 	Operations []Operation
 	// Acquire loads the selected configuration for one ordinary inspection
-	// invocation.
+	// invocation. It returns independent diagnostics and terminal context
+	// separately from the native value and optional operation error.
 	Acquire func(context.Context, Request, I, S) (commandkit.Acquired[View], error)
 	// Compare compares two selections for this scope using one shared state.
-	// Its Acquired result carries native difference records and diagnostics;
-	// returned errors retain their supported cause identity. It is required when
-	// Diff is declared in Operations.
+	// Its Acquired result carries native difference records, independent
+	// diagnostics, and terminal context; returned errors retain their supported
+	// cause identity. It is required when Diff is declared in Operations.
 	Compare func(context.Context, ComparisonRequest[I], S) (commandkit.Acquired[[]configkit.Difference], error)

 	issues []error
--- a/integrations/ckconfig/contracts_adapter_test.go
+++ b/integrations/ckconfig/contracts_adapter_test.go
@@ -126,14 +126,20 @@ func TestFromConfigKitPassesInvocationValuesAndNativeFacets(t *testing.T) {
 }

 func TestFromConfigKitRetainsLoadErrorAndDiagnostics(t *testing.T) {
-	type settings struct {
-		Name string `yaml:"name"`
-	}
-	_, loadErr := configkit.Load(configkit.Mapping("bad source", map[string]any{"unknown": true}), configkit.Options[settings]{Definition: configkit.Definition[settings]{}})
+	path := configkit.Keys("schema_version")
+	_, loadErr := configkit.Load(configkit.Bytes("bad source", []byte("{}\n")), configkit.Options[struct{}]{
+		Definition: configkit.Definition[struct{}]{Version: &configkit.VersionRule{
+			Path: path,
+			Check: func(_ configkit.Version, validation *configkit.Validation) error {
+				validation.Warning(path, "review schema compatibility")
+				return errors.New("private checker error")
+			},
+		}},
+	})
 	if loadErr == nil {
 		t.Fatal("fixture did not produce a ConfigKit load error")
 	}
-	scope := FromConfigKit[struct{}, struct{}, settings]("app", "", func(context.Context, Selection, struct{}, struct{}) (*configkit.Result[settings], error) {
+	scope := FromConfigKit[struct{}, struct{}, struct{}]("app", "", func(context.Context, Selection, struct{}, struct{}) (*configkit.Result[struct{}], error) {
 		return nil, loadErr
 	})
 	acquired, gotErr := scope.Acquire(context.Background(), Request{}, struct{}{}, struct{}{})
@@ -144,38 +150,121 @@ func TestFromConfigKitRetainsLoadErrorAndDiagnostics(t *testing.T) {
 	if !errors.As(gotErr, &typed) {
 		t.Fatalf("load error type was not retained: %T", gotErr)
 	}
-	if len(acquired.Diagnostics) != 1 || acquired.Diagnostics[0].Severity != commandkit.SeverityError {
+	if len(acquired.Diagnostics) != 1 || acquired.Diagnostics[0].Severity != commandkit.SeverityWarning {
 		t.Fatalf("failure diagnostics = %#v", acquired.Diagnostics)
 	}
-	var rendered bytes.Buffer
-	if err := acquired.Diagnostics[0].Write(&rendered); err != nil {
+	if acquired.Explanation.Message != "" || len(acquired.Explanation.Details) != 1 {
+		t.Fatalf("failure explanation = %#v", acquired.Explanation)
+	}
+	var warning, terminal bytes.Buffer
+	if err := acquired.Diagnostics[0].Write(&warning); err != nil {
 		t.Fatal(err)
 	}
-	if !strings.Contains(rendered.String(), `Configuration scope "app":`) || !strings.Contains(rendered.String(), `"bad source"`) {
-		t.Fatalf("diagnostic output lacks scope/source legend: %q", rendered.String())
+	if err := acquired.Explanation.Details[0](&terminal); err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(warning.String(), `Configuration scope "app":`) || !strings.Contains(warning.String(), `review schema compatibility`) {
+		t.Fatalf("warning output lost its scope or record: %q", warning.String())
+	}
+	if !strings.Contains(terminal.String(), `Configuration scope "app":`) || !strings.Contains(terminal.String(), `"bad source"`) || strings.Contains(terminal.String(), "review schema compatibility") {
+		t.Fatalf("terminal explanation lost native context or included an independent warning: %q", terminal.String())
+	}
+	if strings.Contains(terminal.String(), "private checker error") {
+		t.Fatalf("terminal explanation exposed the upstream cause: %q", terminal.String())
+	}
+}
+
+func TestFromConfigKitPreservesValueAndDiagnosticsAlongsideError(t *testing.T) {
+	result, err := adapterSettingsLoad(configkit.Mapping("partial source", map[string]any{"greeting": "Hello"}))
+	if err != nil {
+		t.Fatal(err)
+	}
+	loadErr := errors.New("loader returned a value with an error")
+	scope := FromConfigKit[struct{}, struct{}, adapterSettings]("app", "", func(context.Context, Selection, struct{}, struct{}) (*configkit.Result[adapterSettings], error) {
+		return result, loadErr
+	})
+
+	acquired, gotErr := scope.Acquire(context.Background(), Request{}, struct{}{}, struct{}{})
+	if gotErr != loadErr {
+		t.Fatalf("load error identity = %v, want %v", gotErr, loadErr)
+	}
+	if acquired.Value.Effective != result || acquired.Value.Sources != result || acquired.Value.Explanation != result {
+		t.Fatalf("acquired value facets were lost: %#v", acquired.Value)
+	}
+	if len(acquired.Diagnostics) != 1 || acquired.Diagnostics[0].Severity != commandkit.SeverityWarning {
+		t.Fatalf("independent diagnostics = %#v", acquired.Diagnostics)
+	}
+	if len(acquired.Explanation.Details) != 0 {
+		t.Fatalf("warning-only result acquired terminal context: %#v", acquired.Explanation)
 	}
 }

 func TestConfigKitDiagnosticsMapSeverityAndPropagateWriterFailures(t *testing.T) {
-	warning := configKitDiagnostics("app", nil, []configkit.Diagnostic{{Severity: configkit.SeverityWarning, Message: "safe warning"}})
+	warning, noExplanation := configKitDiagnostics("app", nil, []configkit.Diagnostic{{Severity: configkit.SeverityWarning, Message: "safe warning"}})
 	if len(warning) != 1 || warning[0].Severity != commandkit.SeverityWarning {
 		t.Fatalf("warning diagnostics = %#v", warning)
 	}
-	withError := configKitDiagnostics("app", nil, []configkit.Diagnostic{
+	if noExplanation.Message != "" || len(noExplanation.Details) != 0 {
+		t.Fatalf("warning-only explanation = %#v", noExplanation)
+	}
+	withWarning, withExplanation := configKitDiagnostics("app", nil, []configkit.Diagnostic{
 		{Severity: configkit.SeverityWarning, Message: "safe warning"},
 		{Severity: configkit.SeverityError, Message: "safe error"},
 	})
-	if withError[0].Severity != commandkit.SeverityError {
-		t.Fatalf("mixed diagnostics severity = %q", withError[0].Severity)
+	if len(withWarning) != 1 || withWarning[0].Severity != commandkit.SeverityWarning || len(withExplanation.Details) != 1 {
+		t.Fatalf("mixed diagnostics = %#v, explanation = %#v", withWarning, withExplanation)
 	}

-	if err := withError[0].Write(shortWriter{}); !errors.Is(err, io.ErrShortWrite) {
+	if err := withWarning[0].Write(shortWriter{}); !errors.Is(err, io.ErrShortWrite) {
 		t.Fatalf("short writer error = %v, want %v", err, io.ErrShortWrite)
 	}
+	if err := withExplanation.Details[0](shortWriter{}); !errors.Is(err, io.ErrShortWrite) {
+		t.Fatalf("terminal short writer error = %v, want %v", err, io.ErrShortWrite)
+	}
 	wantErr := errors.New("writer failed")
-	if err := withError[0].Write(failingWriter{err: wantErr}); !errors.Is(err, wantErr) {
+	if err := withWarning[0].Write(failingWriter{err: wantErr}); !errors.Is(err, wantErr) {
 		t.Fatalf("writer error = %v, want %v", err, wantErr)
 	}
+	if err := withExplanation.Details[0](failingWriter{err: wantErr}); !errors.Is(err, wantErr) {
+		t.Fatalf("terminal writer error = %v, want %v", err, wantErr)
+	}
+
+}
+
+func TestConfigKitTerminalContextRetainsRelatedLocationsAndSnapshots(t *testing.T) {
+	primary := &configkit.Location{SourceID: "primary-source", Line: 4, Column: 2}
+	related := []configkit.Location{{SourceID: "related-source", Line: 2, Column: 1}}
+	sources := []configkit.SourceInfo{
+		{ID: "primary-source", Label: "primary", Path: "primary.yml"},
+		{ID: "related-source", Label: "related", Path: "related.yml"},
+	}
+	records := []configkit.Diagnostic{{
+		Severity: configkit.SeverityError,
+		Path:     configkit.Keys("name"),
+		Message:  "native terminal context",
+		Primary:  primary,
+		Related:  related,
+	}}
+	_, explanation := configKitDiagnostics("app", sources, records)
+	primary.SourceID = "mutated-primary"
+	related[0].SourceID = "mutated-related"
+	sources[0].Path = "mutated.yml"
+
+	if len(explanation.Details) != 1 {
+		t.Fatalf("explanation = %#v", explanation)
+	}
+	var output bytes.Buffer
+	if err := explanation.Details[0](&output); err != nil {
+		t.Fatal(err)
+	}
+	for _, fragment := range []string{"primary.yml", "primary-source", "related-source", "native terminal context"} {
+		if !strings.Contains(output.String(), fragment) {
+			t.Errorf("terminal context %q omits %q", output.String(), fragment)
+		}
+	}
+	if strings.Contains(output.String(), "mutated-") || strings.Contains(output.String(), "mutated.yml") {
+		t.Fatalf("terminal renderer retained caller mutations: %q", output.String())
+	}
 }

 type shortWriter struct{}