Remove unused reverse source conversions

2026-10-25 12:50:11 +00:00
parent 455a4a355d
commit 19f5c2a2d5
6 changed files with 106 additions and 136 deletions
--- a/conversion.go
+++ b/conversion.go
@@ -52,16 +52,6 @@ func fromEngineSourceInfo(value engine.SourceInfo) SourceInfo {
 		ImportSelections: fromEngineImportSelections(engine.SourceInfoImportSelectionsForFacade(value)),
 	}
 }
-func toEngineSourceInfo(value SourceInfo) engine.SourceInfo {
-	result := engine.SourceInfo{
-		ID: value.ID, Label: value.Label, Path: value.Path, Destination: toEnginePath(value.Destination),
-		Layer: engine.Layer(value.Layer), Kind: engine.SourceKind(value.Kind), UnderlayIndex: value.UnderlayIndex,
-		ImportedFrom: toEngineLocationPointer(value.ImportedFrom), VersionCheck: toEngineSourceVersionCheck(value.VersionCheck),
-	}
-	result = engine.SourceInfoWithImportSelections(result, toEngineImportSelections(value.ImportSelections))
-	return engine.SourceInfoWithPlacement(result, uint8(value.PlacementMode), value.DestinationUnresolved)
-}
-
 func fromEngineImportSelections(values []engine.ImportSelectionForFacade) []ImportSelection {
 	if values == nil {
 		return nil
@@ -76,15 +66,14 @@ func fromEngineImportSelections(values []engine.ImportSelectionForFacade) []Impo
 	return result
 }

-func toEngineImportSelections(values []ImportSelection) []engine.ImportSelectionForFacade {
+func toEngineOverrides(values []Override) []engine.Override {
 	if values == nil {
 		return nil
 	}
-	result := make([]engine.ImportSelectionForFacade, len(values))
+	result := make([]engine.Override, len(values))
 	for index, value := range values {
-		result[index] = engine.ImportSelectionForFacade{
-			Path: toEnginePath(value.Path), Location: toEngineLocation(value.Location),
-			Selector: string(value.Selector), Outcome: string(value.Outcome),
+		result[index] = engine.Override{
+			Path: toEnginePath(value.Path), Value: value.Value, Flag: value.Flag,
 		}
 	}
 	return result
@@ -127,16 +116,6 @@ func fromEngineSourceVersionCheck(value *engine.SourceVersionCheck) *SourceVersi
 		Location: fromEngineLocationPointer(value.Location), Redacted: value.Redacted,
 	}
 }
-func toEngineSourceVersionCheck(value *SourceVersionCheck) *engine.SourceVersionCheck {
-	if value == nil {
-		return nil
-	}
-	return &engine.SourceVersionCheck{
-		Path:     toEnginePath(value.Path),
-		Version:  engine.Version{Present: value.Version.Present, Null: value.Version.Null, Text: value.Version.Text},
-		Location: toEngineLocationPointer(value.Location), Redacted: value.Redacted,
-	}
-}
 func fromEngineOrigin(value engine.Origin) Origin {
 	result := Origin{Layer: Layer(value.Layer), Locations: fromEngineLocations(value.Locations), Operation: ChangeKind(value.Operation)}
 	if value.Inheritance != nil {
@@ -150,19 +129,6 @@ func fromEngineOrigin(value engine.Origin) Origin {
 	}
 	return result
 }
-func toEngineOrigin(value Origin) engine.Origin {
-	result := engine.Origin{Layer: engine.Layer(value.Layer), Locations: toEngineLocations(value.Locations), Operation: engine.ChangeKind(value.Operation)}
-	if value.Inheritance != nil {
-		result.Inheritance = make([]engine.InheritanceStep, len(value.Inheritance))
-		for index, step := range value.Inheritance {
-			result.Inheritance[index] = engine.InheritanceStep{
-				From: toEnginePath(step.From), To: toEnginePath(step.To), Reference: toEnginePath(step.Reference),
-				Locations: toEngineLocations(step.Locations),
-			}
-		}
-	}
-	return result
-}
 func fromEngineDiagnostic(value engine.Diagnostic) Diagnostic {
 	return Diagnostic{Code: Code(value.Code), Severity: Severity(value.Severity), Path: fromEnginePath(value.Path), Message: value.Message, Primary: fromEngineLocationPointer(value.Primary), Related: fromEngineLocations(value.Related)}
 }
--- a/docs/roadmap/implementation.md
+++ b/docs/roadmap/implementation.md
@@ -1,6 +1,6 @@
 # Consolidation Implementation Plan

-Status: in progress. Stage 1 is complete; follow Stage 2 through Stage 20 in order.
+Status: in progress. Stages 1 and 2 are complete; follow Stage 3 through Stage 20 in order.
 The target executor is a Luna coding agent working through one end-to-end sprint.

 ## Authority, Baseline, And Execution
@@ -173,7 +173,7 @@ detached results and callback ownership (`internal/engine/metadata_test.go`,
 coverage gap surfaced in this map; later focused checks remain assigned in the
 validation groups above.

-## Stage 2 — Remove Unused Reverse Bridges And Share Real Facade Conversion
+## Stage 2 — Remove Unused Reverse Bridges And Share Real Facade Conversion (Complete)

 **Owners:** root `conversion.go`, `records_test.go`, `lifecycle.go`, `layered.go`,
 and engine `records.go`.
@@ -197,6 +197,12 @@ and engine `records.go`.
 **Finish:** the unnecessary cross-package inverse interface is gone and real
 conversion is shared without a new transport layer. Run F.

+Progress notes (2026-10-25): removed the unused reverse source-record bridge
+and engine setters, replaced synthetic round-trip fixtures with engine loads,
+and shared override conversion across ordinary and layered options. Validation
+group F passed with `GOTOOLCHAIN=local GOPROXY=off go test . -run
+'SourceInfo|SourceVersionCheck|ImportSelection|Facade|PublicFacadeBoundary|DependencyImportBoundary' -count=1`.
+
 ## Stage 3 — Normalize Preflight Without Fabricated Options

 **Owners:** engine `options.go`, ordinary entry in `load.go`, grouped entry in
--- a/internal/engine/records.go
+++ b/internal/engine/records.go
@@ -237,22 +237,6 @@ func SourceInfoImportSelectionsForFacade(source SourceInfo) []ImportSelectionFor

 // SourceInfoWithImportSelections copies converted root-owned records into
 // private engine source evidence.
-func SourceInfoWithImportSelections(source SourceInfo, selections []ImportSelectionForFacade) SourceInfo {
-	if selections == nil {
-		source.importSelections = nil
-		return source
-	}
-	source.importSelections = make([]importSelectionEvidence, len(selections))
-	for index, selection := range selections {
-		source.importSelections[index] = importSelectionEvidence{
-			path: selection.Path.clone(), location: selection.Location,
-			selector: importSelectorFromString(selection.Selector),
-			outcome:  importSelectionOutcomeFromString(selection.Outcome),
-		}
-	}
-	return source
-}
-
 func importSelectorString(value importSelector) string {
 	switch value {
 	case importSelectorPath:
@@ -266,19 +250,6 @@ func importSelectorString(value importSelector) string {
 	}
 }

-func importSelectorFromString(value string) importSelector {
-	switch value {
-	case "path":
-		return importSelectorPath
-	case "dir":
-		return importSelectorDirectory
-	case "glob":
-		return importSelectorGlob
-	default:
-		return 0
-	}
-}
-
 func importSelectionOutcomeString(value importSelectionOutcome) string {
 	if value == importSelectionNone {
 		return "none"
@@ -286,13 +257,6 @@ func importSelectionOutcomeString(value importSelectionOutcome) string {
 	return ""
 }

-func importSelectionOutcomeFromString(value string) importSelectionOutcome {
-	if value == "none" {
-		return importSelectionNone
-	}
-	return 0
-}
-
 // SourceInfoPlacementModeForFacade converts private placement evidence to its
 // root-owned numeric representation.
 func SourceInfoPlacementModeForFacade(source SourceInfo) uint8 {
@@ -304,13 +268,6 @@ func SourceInfoDestinationUnresolvedForFacade(source SourceInfo) bool {
 	return source.destinationUnresolved
 }

-// SourceInfoWithPlacement copies facade-owned source evidence into private fields.
-func SourceInfoWithPlacement(source SourceInfo, mode uint8, unresolved bool) SourceInfo {
-	source.placementMode = placementMode(mode)
-	source.destinationUnresolved = unresolved
-	return source
-}
-
 // SourceVersionCheck is a detached, public-safe record of a source-local
 // version declaration observed during an enabled load.
 type SourceVersionCheck struct {
--- a/layered.go
+++ b/layered.go
@@ -88,13 +88,6 @@ func toEngineLayeredOptions[T any](options LayeredOptions[T]) engine.LayeredOpti
 			}
 		}
 	}
-	if options.Overrides != nil {
-		result.Overrides = make([]engine.Override, len(options.Overrides))
-		for index, override := range options.Overrides {
-			result.Overrides[index] = engine.Override{
-				Path: toEnginePath(override.Path), Value: override.Value, Flag: override.Flag,
-			}
-		}
-	}
+	result.Overrides = toEngineOverrides(options.Overrides)
 	return result
 }
--- a/lifecycle.go
+++ b/lifecycle.go
@@ -323,14 +323,7 @@ func toEngineOptions[T any](options Options[T]) engine.Options[T] {
 	if options.Filesystem != nil {
 		result.Filesystem = toEngineFilesystemContext(options.Filesystem)
 	}
-	if options.Overrides != nil {
-		result.Overrides = make([]engine.Override, len(options.Overrides))
-		for index := range options.Overrides {
-			result.Overrides[index] = engine.Override{
-				Path: toEnginePath(options.Overrides[index].Path), Value: options.Overrides[index].Value, Flag: options.Overrides[index].Flag,
-			}
-		}
-	}
+	result.Overrides = toEngineOverrides(options.Overrides)
 	return result
 }

--- a/records_test.go
+++ b/records_test.go
@@ -1,6 +1,8 @@
 package configkit

 import (
+	"os"
+	"path/filepath"
 	"testing"

 	"gitea.maximumdirect.net/eric/ConfigKit/internal/engine"
@@ -27,16 +29,11 @@ func TestOverlayPublicValues(t *testing.T) {

 func TestSourceInfoDestinationConversionsAndCopies(t *testing.T) {
 	destination := Keys("collections", "a/b", "0")
-	public := SourceInfo{
-		ID: "source-1", Destination: destination,
-		PlacementMode: PlacementModeSequenceItem, DestinationUnresolved: true,
-	}
-	internal := toEngineSourceInfo(public)
-	if internal.Destination.String() != destination.String() || engine.SourceInfoPlacementModeForFacade(internal) != uint8(PlacementModeSequenceItem) || !engine.SourceInfoDestinationUnresolvedForFacade(internal) {
-		t.Fatalf("engine source evidence = %#v", internal)
-	}
+	path := writeRecordConfig(t, "value: main\n")
+	options := engine.OptionsWithPlacement(engine.Options[map[string]any]{}, engine.Keys("collections", "a/b", "0"), uint8(PlacementModeSequenceItem))
+	internal := engineSourceInfoFromLoad(t, path, "source-1", options)
 	converted := fromEngineSourceInfo(internal)
-	if converted.Destination.String() != destination.String() || converted.PlacementMode != PlacementModeSequenceItem || !converted.DestinationUnresolved {
+	if converted.Destination.String() != destination.String() || converted.PlacementMode != PlacementModeSequenceItem || converted.DestinationUnresolved {
 		t.Fatalf("public source evidence = %#v", converted)
 	}
 	copy := cloneSourceInfo(converted)
@@ -47,58 +44,116 @@ func TestSourceInfoDestinationConversionsAndCopies(t *testing.T) {
 }

 func TestSourceVersionCheckConversionsAndCopies(t *testing.T) {
-	path := Keys("records").Index(2).Key("version")
-	source := SourceInfo{
-		ID: "source-1",
-		VersionCheck: &SourceVersionCheck{
-			Path: path, Version: Version{Present: true, Text: "123"},
-			Location: &Location{SourceID: "source-1", Line: 4, Column: 8},
+	path := Keys("version")
+	file := writeRecordConfig(t, "version: '123'\nvalue: main\n")
+	options := engine.Options[recordConfig]{
+		Definition: engine.Definition[recordConfig]{
+			Version: &engine.VersionRule{
+				Path: engine.Keys("version"),
+				Check: func(version engine.Version, _ *engine.Validation) error {
+					if !version.Present || version.Text != "123" {
+						t.Fatalf("version callback input = %#v", version)
+					}
+					return nil
+				},
+			},
 		},
+		SourceVersionMode: engine.SourceVersionModeFromFacade(uint8(SourceVersionModeIfPresent)),
 	}
-	internal := toEngineSourceInfo(source)
-	if internal.VersionCheck == nil || internal.VersionCheck.Path.String() != path.String() ||
-		internal.VersionCheck.Version.Text != "123" || internal.VersionCheck.Location == nil || internal.VersionCheck.Location.Line != 4 {
-		t.Fatalf("engine source version check = %#v", internal.VersionCheck)
-	}
+	internal := engineSourceInfoFromLoad(t, file, "source-1", options)
 	converted := fromEngineSourceInfo(internal)
+	if converted.VersionCheck == nil || converted.VersionCheck.Path.String() != path.String() ||
+		!converted.VersionCheck.Version.Present || converted.VersionCheck.Version.Text != "123" ||
+		converted.VersionCheck.Location == nil || converted.VersionCheck.Location.SourceID != internal.ID || converted.VersionCheck.Location.Line != 1 {
+		t.Fatalf("public source version check = %#v", converted.VersionCheck)
+	}
 	copy := cloneSourceInfo(converted)
 	copy.VersionCheck.Path = Keys("changed")
 	copy.VersionCheck.Version.Text = "changed"
 	copy.VersionCheck.Location.Line = 99
 	copy.VersionCheck.Redacted = true
 	if converted.VersionCheck.Path.String() != path.String() || converted.VersionCheck.Version.Text != "123" ||
-		converted.VersionCheck.Location.Line != 4 || converted.VersionCheck.Redacted {
+		converted.VersionCheck.Location.Line != 1 || converted.VersionCheck.Redacted {
 		t.Fatalf("source version check changed through a detached copy: %#v", converted.VersionCheck)
 	}
 }

 func TestImportSelectionConversionsAndCopies(t *testing.T) {
-	declaration, err := ParsePath("/configkit/imports/2")
-	if err != nil {
-		t.Fatal(err)
-	}
-	source := SourceInfo{ID: "source-4", ImportSelections: []ImportSelection{{
-		Path: declaration, Location: Location{SourceID: "source-4", Line: 7, Column: 5},
-		Selector: ImportSelectorGlob, Outcome: ImportSelectionNone,
-	}}}
-	internal := toEngineSourceInfo(source)
-	transport := engine.SourceInfoImportSelectionsForFacade(internal)
-	if len(transport) != 1 || transport[0].Path.String() != declaration.String() || transport[0].Location.SourceID != source.ID || transport[0].Selector != "glob" || transport[0].Outcome != "none" {
-		t.Fatalf("engine selection evidence = %#v", transport)
-	}
-	source.ImportSelections[0].Path = Keys("changed")
+	file := writeRecordConfig(t, "configkit:\n  imports:\n    - glob: absent/*.yml\nvalue: main\n")
+	internal := engineSourceInfoFromLoad(t, file, "source-1", engine.Options[recordConfig]{Imports: true})
 	converted := fromEngineSourceInfo(internal)
-	if len(converted.ImportSelections) != 1 || converted.ImportSelections[0].Path.String() != declaration.String() || converted.ImportSelections[0].Location.Line != 7 || converted.ImportSelections[0].Selector != ImportSelectorGlob || converted.ImportSelections[0].Outcome != ImportSelectionNone {
+	if len(converted.ImportSelections) != 1 || converted.ImportSelections[0].Path.String() != "/configkit/imports/0" ||
+		converted.ImportSelections[0].Location.SourceID != internal.ID || converted.ImportSelections[0].Location.Line != 3 ||
+		converted.ImportSelections[0].Selector != ImportSelectorGlob || converted.ImportSelections[0].Outcome != ImportSelectionNone {
 		t.Fatalf("converted selection evidence = %#v", converted.ImportSelections)
 	}
 	copy := cloneSourceInfo(converted)
 	copy.ImportSelections[0].Path = Keys("copy")
 	copy.ImportSelections[0].Selector = ImportSelectorPath
-	if converted.ImportSelections[0].Path.String() != declaration.String() || converted.ImportSelections[0].Selector != ImportSelectorGlob {
+	if converted.ImportSelections[0].Path.String() != "/configkit/imports/0" || converted.ImportSelections[0].Selector != ImportSelectorGlob {
 		t.Fatalf("source selection evidence changed through copy: %#v", converted.ImportSelections)
 	}
+	if converted.ImportSelections[0].Outcome != ImportSelectionNone {
+		t.Fatalf("source selection outcome = %q", converted.ImportSelections[0].Outcome)
+	}
+	if fromEngineImportSelections(nil) != nil {
+		t.Fatal("converting nil engine selections returned a non-nil slice")
+	}
+	if fromEngineImportSelections([]engine.ImportSelectionForFacade{}) == nil {
+		t.Fatal("converting an empty engine selection slice returned nil")
+	}
 	empty := cloneSourceInfo(SourceInfo{ImportSelections: make([]ImportSelection, 0)})
 	if empty.ImportSelections == nil {
 		t.Fatal("cloning collapsed a non-nil empty selection slice")
 	}
 }
+
+func TestOverrideConversionsPreserveSliceShapeAndCopyRecords(t *testing.T) {
+	if toEngineOverrides(nil) != nil {
+		t.Fatal("nil overrides converted to a non-nil slice")
+	}
+	empty := make([]Override, 0)
+	if toEngineOverrides(empty) == nil || toEngineOptions(Options[recordConfig]{Overrides: empty}).Overrides == nil ||
+		toEngineLayeredOptions(LayeredOptions[recordConfig]{Overrides: empty}).Overrides == nil {
+		t.Fatal("a non-nil empty override slice was collapsed")
+	}
+
+	overrides := []Override{{Path: Keys("feature", "enabled"), Value: "yes", Flag: "--feature-enabled"}}
+	ordinary := toEngineOptions(Options[recordConfig]{Overrides: overrides})
+	layered := toEngineLayeredOptions(LayeredOptions[recordConfig]{Overrides: overrides})
+	overrides[0] = Override{Path: Keys("changed"), Value: "no", Flag: "--changed"}
+	for label, converted := range map[string][]engine.Override{"ordinary": ordinary.Overrides, "layered": layered.Overrides} {
+		if len(converted) != 1 || converted[0].Path.String() != "/feature/enabled" || converted[0].Value != "yes" || converted[0].Flag != "--feature-enabled" {
+			t.Errorf("%s overrides = %#v", label, converted)
+		}
+	}
+}
+
+type recordConfig struct {
+	Value   string `yaml:"value"`
+	Version string `yaml:"version"`
+}
+
+func writeRecordConfig(t *testing.T, content string) string {
+	t.Helper()
+	path := filepath.Join(t.TempDir(), "config.yml")
+	if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	return path
+}
+
+func engineSourceInfoFromLoad[T any](t *testing.T, path, sourceID string, options engine.Options[T]) engine.SourceInfo {
+	t.Helper()
+	loaded, err := engine.Load(engine.File(path), engine.SecretBinding{}, options)
+	if err != nil {
+		t.Fatalf("engine load: %v", err)
+	}
+	for _, source := range loaded.Inspection.Sources() {
+		if source.ID == sourceID {
+			return source
+		}
+	}
+	t.Fatalf("engine inspection contains no source %q", sourceID)
+	return engine.SourceInfo{}
+}