Infer artifact dependencies from configured inputs

internal/config/artifact_dependencies.go

@@ -0,0 +1,123 @@
+package config
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	"gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy"
+)
+
+type artifactDependencyOrigin struct {
+	explicitIndex int
+	inputName     string
+	family        string
+}
+
+func finalizeArtifactDependencies(cfg *PipelineConfig) {
+	if cfg == nil || cfg.Scriptorium == nil {
+		return
+	}
+	for artifactName, artifact := range cfg.Scriptorium.Artifacts {
+		priorOrigins := artifact.dependencyOrigins
+		origins := make(map[string][]artifactDependencyOrigin)
+		dependencies := make(map[string]struct{})
+		family := ""
+		if member, ok := cfg.familyCatalog.Members[artifactName]; ok {
+			family = member.Family
+		}
+
+		for index, dependency := range artifact.DependsOn {
+			dependency = strings.TrimSpace(dependency)
+			if prior, finalized := priorOrigins[dependency]; finalized {
+				explicit := explicitOrigins(prior)
+				if len(explicit) > 0 {
+					dependencies[dependency] = struct{}{}
+					origins[dependency] = append(origins[dependency], explicit...)
+				}
+				continue
+			}
+			dependencies[dependency] = struct{}{}
+			origins[dependency] = append(origins[dependency], artifactDependencyOrigin{
+				explicitIndex: index,
+				family:        family,
+			})
+		}
+
+		inputNames := make([]string, 0, len(artifact.Inputs))
+		for inputName := range artifact.Inputs {
+			inputNames = append(inputNames, inputName)
+		}
+		sort.Strings(inputNames)
+		for _, inputName := range inputNames {
+			descriptor, err := artifactpolicy.DescribeScriptoriumInputSource(artifact.Inputs[inputName].Source)
+			if err != nil || descriptor.Source.Kind != artifactpolicy.SourceKindConfiguredArtifact {
+				continue
+			}
+			dependency := descriptor.Source.ConfiguredKey
+			dependencies[dependency] = struct{}{}
+			origins[dependency] = append(origins[dependency], artifactDependencyOrigin{
+				explicitIndex: -1,
+				inputName:     inputName,
+				family:        family,
+			})
+		}
+
+		artifact.DependsOn = sortedDependencyKeys(dependencies)
+		artifact.dependencyOrigins = origins
+		cfg.Scriptorium.Artifacts[artifactName] = artifact
+	}
+}
+
+func explicitOrigins(origins []artifactDependencyOrigin) []artifactDependencyOrigin {
+	result := make([]artifactDependencyOrigin, 0, len(origins))
+	for _, origin := range origins {
+		if origin.explicitIndex >= 0 {
+			result = append(result, origin)
+		}
+	}
+	return result
+}
+
+func sortedDependencyKeys(dependencies map[string]struct{}) []string {
+	result := make([]string, 0, len(dependencies))
+	for dependency := range dependencies {
+		result = append(result, dependency)
+	}
+	sort.Strings(result)
+	return result
+}
+
+func dependencyOriginField(artifactName, dependency string, artifact ScriptoriumArtifactConfig) string {
+	origins := artifact.dependencyOrigins[dependency]
+	for _, origin := range origins {
+		if origin.explicitIndex < 0 {
+			continue
+		}
+		if origin.family != "" {
+			return fmt.Sprintf("pipeline.scriptorium.artifact_families.%s.member_dependencies[%d]", origin.family, origin.explicitIndex)
+		}
+		return fmt.Sprintf("pipeline.scriptorium.artifacts.%s.depends_on[%d]", artifactName, origin.explicitIndex)
+	}
+	for _, origin := range origins {
+		if origin.inputName == "" {
+			continue
+		}
+		if origin.family != "" {
+			return fmt.Sprintf("pipeline.scriptorium.artifact_families.%s.inputs.%s.source", origin.family, origin.inputName)
+		}
+		return fmt.Sprintf("pipeline.scriptorium.artifacts.%s.inputs.%s.source", artifactName, origin.inputName)
+	}
+	return fmt.Sprintf("pipeline.scriptorium.artifacts.%s.depends_on", artifactName)
+}
+
+func cloneDependencyOrigins(in map[string][]artifactDependencyOrigin) map[string][]artifactDependencyOrigin {
+	if in == nil {
+		return nil
+	}
+	out := make(map[string][]artifactDependencyOrigin, len(in))
+	for dependency, origins := range in {
+		out[dependency] = append([]artifactDependencyOrigin(nil), origins...)
+	}
+	return out
+}

internal/config/artifact_dependencies_test.go

@@ -0,0 +1,112 @@
+package config
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+)
+
+func TestFinalizeArtifactDependenciesUnionsNormalizesAndIsIdempotent(t *testing.T) {
+	cfg := dependencyTestPipeline([]string{" beta ", "alpha", "beta"})
+	artifact := cfg.Scriptorium.Artifacts["result"]
+	artifact.Inputs["gamma"] = ScriptoriumInputConfig{Source: "narratio.artifact.gamma"}
+	artifact.Inputs["alpha"] = ScriptoriumInputConfig{Source: "narratio.artifact.alpha"}
+	cfg.Scriptorium.Artifacts["result"] = artifact
+	cfg.Scriptorium.Artifacts["gamma"] = ScriptoriumArtifactConfig{}
+
+	finalizeArtifactDependencies(cfg)
+	want := []string{"alpha", "beta", "gamma"}
+	got := cfg.Scriptorium.Artifacts["result"]
+	if !reflect.DeepEqual(got.DependsOn, want) {
+		t.Fatalf("dependencies = %#v, want %#v", got.DependsOn, want)
+	}
+	if len(got.dependencyOrigins["alpha"]) != 2 {
+		t.Fatalf("alpha origins = %#v, want explicit and input origins", got.dependencyOrigins["alpha"])
+	}
+
+	firstOrigins := cloneDependencyOrigins(got.dependencyOrigins)
+	finalizeArtifactDependencies(cfg)
+	got = cfg.Scriptorium.Artifacts["result"]
+	if !reflect.DeepEqual(got.DependsOn, want) {
+		t.Fatalf("dependencies after second finalization = %#v, want %#v", got.DependsOn, want)
+	}
+	if !reflect.DeepEqual(got.dependencyOrigins, firstOrigins) {
+		t.Fatalf("origins changed after second finalization: %#v != %#v", got.dependencyOrigins, firstOrigins)
+	}
+}
+
+func TestInferredArtifactDependencyDiagnosticsPointToInputs(t *testing.T) {
+	for _, test := range []struct {
+		name      string
+		artifacts map[string]ScriptoriumArtifactConfig
+		want      []string
+	}{
+		{
+			name: "missing",
+			artifacts: map[string]ScriptoriumArtifactConfig{
+				"result": {Inputs: map[string]ScriptoriumInputConfig{"missing": {Source: "narratio.artifact.unknown"}}},
+			},
+			want: []string{"pipeline.scriptorium.artifacts.result.inputs.missing.source", "not configured"},
+		},
+		{
+			name: "self",
+			artifacts: map[string]ScriptoriumArtifactConfig{
+				"result": {Inputs: map[string]ScriptoriumInputConfig{"self": {Source: "narratio.artifact.result"}}},
+			},
+			want: []string{"pipeline.scriptorium.artifacts.result.inputs.self.source", "itself"},
+		},
+		{
+			name: "cycle",
+			artifacts: map[string]ScriptoriumArtifactConfig{
+				"alpha": {Inputs: map[string]ScriptoriumInputConfig{"beta": {Source: "narratio.artifact.beta"}}},
+				"beta":  {Inputs: map[string]ScriptoriumInputConfig{"alpha": {Source: "narratio.artifact.alpha"}}},
+			},
+			want: []string{".inputs.", "dependencies must not contain cycles"},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			cfg := &PipelineConfig{Scriptorium: &ScriptoriumConfig{Artifacts: test.artifacts}}
+			finalizeArtifactDependencies(cfg)
+			err := ValidateScriptoriumArtifactDependencies(cfg.Scriptorium.Artifacts)
+			if err == nil {
+				t.Fatal("expected dependency validation error")
+			}
+			for _, fragment := range test.want {
+				if !strings.Contains(err.Error(), fragment) {
+					t.Fatalf("error = %q, want fragment %q", err, fragment)
+				}
+			}
+		})
+	}
+}
+
+func TestExplicitAndInferredDependencyGraphsHaveEqualDigests(t *testing.T) {
+	explicit := dependencyTestPipeline([]string{"alpha"})
+	inferred := dependencyTestPipeline(nil)
+	for _, cfg := range []*PipelineConfig{explicit, inferred} {
+		finalizeArtifactDependencies(cfg)
+		cfg.resolution = &pipelineResolutionMetadata{}
+		if err := recomputePipelineEffectiveDigest(cfg); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if got, want := EffectivePipelineDigest(explicit), EffectivePipelineDigest(inferred); got != want {
+		t.Fatalf("equivalent dependency graph digests differ: %s != %s", got, want)
+	}
+}
+
+func dependencyTestPipeline(dependsOn []string) *PipelineConfig {
+	return &PipelineConfig{Scriptorium: &ScriptoriumConfig{
+		Binary: "scriptorium",
+		Artifacts: map[string]ScriptoriumArtifactConfig{
+			"alpha": {},
+			"beta":  {},
+			"result": {
+				DependsOn: dependsOn,
+				Inputs: map[string]ScriptoriumInputConfig{
+					"alpha": {Source: "narratio.artifact.alpha"},
+				},
+			},
+		},
+	}}
+}

internal/config/artifact_families.go

@@ -127,6 +127,11 @@ func expandPipelineArtifactFamilies(cfg *PipelineConfig, party ResolvedParty) er
 	cfg.Scriptorium.ArtifactFamilies = nil
 	cfg.familyCatalog = catalog
 	cfg.resolution.artifactFamiliesExpanded = true
+	finalizeArtifactDependencies(cfg)
+	for key, member := range cfg.familyCatalog.Members {
+		member.Dependencies = append([]string(nil), cfg.Scriptorium.Artifacts[key].DependsOn...)
+		cfg.familyCatalog.Members[key] = member
+	}
 	if err := rejectConcreteMemberArtifactSources(expanded); err != nil {
 		return err
 	}
@@ -261,9 +266,6 @@ func expandFamilyInputs(prefix string, family ScriptoriumArtifactFamilyConfig, c
 		if !matched {
 			continue
 		}
-		if !familyDependencyContains(family.MemberDependencies, familyKey) {
-			return nil, fmt.Errorf("%s.inputs.%s.source %q requires member_dependencies entry %q", prefix, name, input.Source, familyKey)
-		}
 		input.Source = artifactpolicy.ConfiguredSourceID(familyKey + "_" + characterID)
 		inputs[name] = input
 	}
@@ -299,15 +301,6 @@ func rejectConcreteMemberArtifactSources(artifacts map[string]ScriptoriumArtifac
 	return nil
 }
 
-func familyDependencyContains(values []string, want string) bool {
-	for _, value := range values {
-		if strings.TrimSpace(value) == want {
-			return true
-		}
-	}
-	return false
-}
-
 func inputSourceMap(inputs map[string]ScriptoriumInputConfig) map[string]string {
 	if len(inputs) == 0 {
 		return nil
@@ -392,6 +385,7 @@ func cloneArtifactDefinitions(in map[string]ScriptoriumArtifactConfig) map[strin
 	out := make(map[string]ScriptoriumArtifactConfig, len(in))
 	for key, artifact := range in {
 		artifact.DependsOn = append([]string(nil), artifact.DependsOn...)
+		artifact.dependencyOrigins = cloneDependencyOrigins(artifact.dependencyOrigins)
 		artifact.Inputs = cloneArtifactInputs(artifact.Inputs)
 		artifact.Vars = cloneArtifactVars(artifact.Vars)
 		out[key] = artifact

internal/config/artifact_families_test.go

@@ -223,7 +223,6 @@ characters:
     prompt_id: dnd.character_items
     output_path_pattern: artifacts/characters/{character_id}/items.md
     depends_on: [session_recap]
-    member_dependencies: [character_meta]
     inputs:
       transcript: {source: narratio.transcript.final_trimmed, required: true}
       prior_meta: {source: narratio.member_artifact.character_meta, required: true}
@@ -234,7 +233,7 @@ characters:
 		t.Fatal(err)
 	}
 	items := cfg.Pipeline.Scriptorium.Artifacts["character_items_arannis"]
-	if !reflect.DeepEqual(items.DependsOn, []string{"session_recap", "character_meta_arannis"}) {
+	if !reflect.DeepEqual(items.DependsOn, []string{"character_meta_arannis", "session_recap"}) {
 		t.Fatalf("member dependencies = %#v", items.DependsOn)
 	}
 	if got := items.Inputs["prior_meta"].Source; got != "narratio.artifact.character_meta_arannis" {
@@ -262,11 +261,8 @@ func TestArtifactFamiliesRejectInvalidMemberDependenciesAndSources(t *testing.T)
     for_each: party.characters
     output_path_pattern: artifacts/characters/{character_id}/other.md
 `, want: "duplicates family"},
-		{name: "undeclared source dependency", families: strings.Replace(validFamilyYAML, "member_vars:", "inputs:\n      prior: {source: narratio.member_artifact.other, required: true}\n    member_vars:", 1) + `
-  other:
-    for_each: party.characters
-    output_path_pattern: artifacts/characters/{character_id}/other.md
-`, want: "requires member_dependencies"},
+		{name: "missing inferred family dependency", families: strings.Replace(validFamilyYAML, "member_vars:", "inputs:\n      prior: {source: narratio.member_artifact.missing, required: true}\n    member_vars:", 1), want: "pipeline.scriptorium.artifact_families.character_meta.inputs.prior.source"},
+		{name: "self inferred family dependency", families: strings.Replace(validFamilyYAML, "member_vars:", "inputs:\n      prior: {source: narratio.member_artifact.character_meta, required: true}\n    member_vars:", 1), want: "pipeline.scriptorium.artifact_families.character_meta.inputs.prior.source"},
 		{name: "transitive cycle", families: strings.Replace(validFamilyYAML, "member_vars:", "member_dependencies: [other]\n    member_vars:", 1) + `
   other:
     for_each: party.characters

internal/config/config.go

@@ -244,6 +244,8 @@ type ScriptoriumArtifactConfig struct {
 	Timeout     string                            `yaml:"timeout" json:"timeout"`
 	Inputs      map[string]ScriptoriumInputConfig `yaml:"inputs" json:"inputs"`
 	Vars        map[string]any                    `yaml:"vars" json:"vars"`
+
+	dependencyOrigins map[string][]artifactDependencyOrigin
 }
 
 // ScriptoriumArtifactFamilyConfig declares the shared configuration expanded

internal/config/load.go

@@ -54,6 +54,7 @@ func finalizeLoadedPipeline(path string, cfg *PipelineConfig) (*PipelineConfig,
 	if err := resolveNotariusPaths(cfg, path); err != nil {
 		return nil, fmt.Errorf("load pipeline config: %w", err)
 	}
+	finalizeArtifactDependencies(cfg)
 	if err := finalizePipelineResolution(cfg); err != nil {
 		return nil, fmt.Errorf("load pipeline config: %w", err)
 	}

internal/config/scriptorium_test.go

@@ -395,7 +395,7 @@ func TestScriptoriumLoadAndValidate(t *testing.T) {
 			wantValidateErr: []string{"pipeline.scriptorium.artifacts.session_recap.output_path", "under artifacts/"},
 		},
 		{
-			name: "missing depends_on for artifact source fails validation",
+			name: "artifact source infers dependency",
 			scriptoriumYAML: `scriptorium:
   binary: scriptorium
   artifacts:
@@ -416,7 +416,13 @@ func TestScriptoriumLoadAndValidate(t *testing.T) {
           source: narratio.artifact.session_recap
           required: true
 `,
-			wantValidateErr: []string{"pipeline.scriptorium.artifacts.player_handout.inputs.recap.source", "requires depends_on"},
+			assert: func(t *testing.T, cfg *Config) {
+				t.Helper()
+				dependencies := cfg.Pipeline.Scriptorium.Artifacts["player_handout"].DependsOn
+				if len(dependencies) != 1 || dependencies[0] != "session_recap" {
+					t.Fatalf("inferred dependencies = %#v, want [session_recap]", dependencies)
+				}
+			},
 		},
 		{
 			name: "dependency on unknown artifact fails validation",

internal/config/validate_scriptorium.go

@@ -108,10 +108,9 @@ func validateScriptoriumDependenciesAndInputs(
 	referencedArtifacts := make(map[string]struct{})
 	for _, artifactName := range artifactNames {
 		artifactCfg := artifacts[artifactName]
-		depSet := make(map[string]struct{}, len(artifactCfg.DependsOn))
-		for i, depName := range artifactCfg.DependsOn {
+		for _, depName := range artifactCfg.DependsOn {
 			trimmedDep := strings.TrimSpace(depName)
-			field := fmt.Sprintf("pipeline.scriptorium.artifacts.%s.depends_on[%d]", artifactName, i)
+			field := dependencyOriginField(artifactName, trimmedDep, artifactCfg)
 			if trimmedDep == "" {
 				return nil, fmt.Errorf("%s must be non-empty", field)
 			}
@@ -119,9 +118,8 @@ func validateScriptoriumDependenciesAndInputs(
 				return nil, fmt.Errorf("%s %q is not a configured artifact key", field, depName)
 			}
 			if trimmedDep == artifactName {
-				return nil, fmt.Errorf("pipeline.scriptorium.artifacts.%s.depends_on must not include itself", artifactName)
+				return nil, fmt.Errorf("%s must not include itself", field)
 			}
-			depSet[trimmedDep] = struct{}{}
 			referencedArtifacts[trimmedDep] = struct{}{}
 		}
 
@@ -146,15 +144,6 @@ func validateScriptoriumDependenciesAndInputs(
 				return nil, err
 			}
 			if referencedArtifact != "" {
-				if _, ok := depSet[referencedArtifact]; !ok {
-					return nil, fmt.Errorf(
-						"pipeline.scriptorium.artifacts.%s.inputs.%s.source %q requires depends_on entry %q",
-						artifactName,
-						inputName,
-						source,
-						referencedArtifact,
-					)
-				}
 				referencedArtifacts[referencedArtifact] = struct{}{}
 			}
 		}
@@ -281,13 +270,17 @@ func ValidateScriptoriumArtifactDependencies(artifacts map[string]ScriptoriumArt
 		for _, dep := range cfg.DependsOn {
 			trimmedDep := strings.TrimSpace(dep)
 			if trimmedDep == "" {
-				return fmt.Errorf("pipeline.scriptorium.artifacts.%s.depends_on entries must be non-empty", name)
+				field := dependencyOriginField(name, trimmedDep, cfg)
+				if field == fmt.Sprintf("pipeline.scriptorium.artifacts.%s.depends_on", name) {
+					return fmt.Errorf("%s entries must be non-empty", field)
+				}
+				return fmt.Errorf("%s must be non-empty", field)
 			}
 			if _, ok := artifacts[trimmedDep]; !ok {
-				return fmt.Errorf("pipeline.scriptorium.artifacts.%s dependency %q is not configured", name, dep)
+				return fmt.Errorf("%s dependency %q is not configured", dependencyOriginField(name, trimmedDep, cfg), dep)
 			}
 			if trimmedDep == name {
-				return fmt.Errorf("pipeline.scriptorium.artifacts.%s.depends_on must not include itself", name)
+				return fmt.Errorf("%s must not include itself", dependencyOriginField(name, trimmedDep, cfg))
 			}
 			if _, duplicate := seen[trimmedDep]; duplicate {
 				continue
@@ -303,14 +296,17 @@ func ValidateScriptoriumArtifactDependencies(artifacts map[string]ScriptoriumArt
 
 	var visit func(node string) error
 	visit = func(node string) error {
-		if visiting[node] {
-			return fmt.Errorf("pipeline.scriptorium.artifacts dependencies must not contain cycles")
-		}
 		if visited[node] {
 			return nil
 		}
 		visiting[node] = true
 		for _, dep := range graph[node] {
+			if visiting[dep] {
+				return fmt.Errorf(
+					"%s creates a cycle; pipeline.scriptorium.artifacts dependencies must not contain cycles",
+					dependencyOriginField(node, dep, artifacts[node]),
+				)
+			}
 			if err := visit(dep); err != nil {
 				return err
 			}

internal/stage/analyze_test.go

@@ -627,25 +627,36 @@ func TestAnalyzeRunsDependenciesBeforeDependents(t *testing.T) {
 	paths := sessionPathsForEnv(env, m.SessionID)
 	writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "final.trimmed.json"), `{"segments":[]}`)
 
-	env.Config.Pipeline.Scriptorium.Artifacts["player_handout"] = config.ScriptoriumArtifactConfig{
-		Enabled:    true,
-		DependsOn:  []string{"session_recap"},
-		PromptID:   "dnd.player_handout",
-		ProfileID:  "local-quality",
-		OutputPath: "artifacts/player_handout.md",
-		Inputs: map[string]config.ScriptoriumInputConfig{
-			"recap": {
-				Source:   "narratio.artifact.session_recap",
-				Required: true,
-			},
-			"transcript": {
-				Source:   "narratio.transcript.final_trimmed",
-				Required: true,
-			},
-		},
+	writeAnalyzeFile(t, env.Config.PipelinePath, "workspace:\n  root: "+env.Config.Pipeline.Workspace.Root+`
+scriptorium:
+  binary: scriptorium
+  artifacts:
+    session_recap:
+      enabled: true
+      prompt_id: dnd.session_recap
+      profile_id: local-quality
+      output_path: artifacts/session_recap.md
+      inputs:
+        transcript: {source: narratio.transcript.final_trimmed, required: true}
+    player_handout:
+      enabled: true
+      prompt_id: dnd.player_handout
+      profile_id: local-quality
+      output_path: artifacts/player_handout.md
+      inputs:
+        recap: {source: narratio.artifact.session_recap, required: true}
+        transcript: {source: narratio.transcript.final_trimmed, required: true}
+`)
+	loadedPipeline, err := config.LoadPipeline(env.Config.PipelinePath)
+	if err != nil {
+		t.Fatalf("LoadPipeline() error = %v", err)
+	}
+	env.Config.Pipeline = loadedPipeline
+	if dependencies := loadedPipeline.Scriptorium.Artifacts["player_handout"].DependsOn; len(dependencies) != 1 || dependencies[0] != "session_recap" {
+		t.Fatalf("materialized dependencies = %#v, want [session_recap]", dependencies)
 	}
 
-	_, err := (analyzeStage{}).Run(context.Background(), env, m)
+	_, err = (analyzeStage{}).Run(context.Background(), env, m)
 	if err != nil {
 		t.Fatalf("Run() error = %v", err)
 	}