From af492c9e977217171366ddff564af23c2a60d429 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 10 Aug 2026 22:27:06 +0000 Subject: [PATCH] Centralize effective artifact selection --- docs/roadmap/implementation.md | 2 + internal/app/analyze_artifacts.go | 57 +++++++++-- internal/app/analyze_artifacts_test.go | 14 +++ internal/app/operator_artifact_rendering.go | 26 +++-- internal/app/operator_session_validate.go | 7 +- internal/app/operator_status.go | 6 +- internal/app/restore_plan.go | 6 +- internal/app/run.go | 9 +- internal/app/run_stage.go | 9 +- internal/app/runner.go | 45 +++++++-- internal/artifacts/catalog.go | 66 ++++++++----- internal/artifacts/effective_set.go | 98 +++++++++++++++++++ internal/artifacts/effective_set_test.go | 86 ++++++++++++++++ internal/artifacts/previous_requirements.go | 7 +- .../artifacts/previous_requirements_test.go | 6 +- internal/stage/analyze.go | 62 +++++------- internal/stage/prepare.go | 21 +++- .../stage/prepare_effective_artifacts_test.go | 32 ++++++ internal/stage/publish.go | 27 ++--- internal/stage/stage.go | 1 + 20 files changed, 457 insertions(+), 130 deletions(-) create mode 100644 internal/artifacts/effective_set.go create mode 100644 internal/artifacts/effective_set_test.go create mode 100644 internal/stage/prepare_effective_artifacts_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 4661573..0d10fb0 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -956,6 +956,8 @@ selection, disabled overrides, empty selections, ordering, catalog versions, publication filters, and the exact composed analyze inputs. This is the primary TST-010 stage; Stage 29 adds resolution/error cases. +**Status:** Completed. + ## Stage 29 — Make analyze resolution typed, optional, actionable, and deterministic **Read first:** `audit-findings.md` lines 2424–2455 (COR-021), 2492–2520 diff --git a/internal/app/analyze_artifacts.go b/internal/app/analyze_artifacts.go index 37f4986..eb6b1e2 100644 --- a/internal/app/analyze_artifacts.go +++ b/internal/app/analyze_artifacts.go @@ -5,6 +5,7 @@ import ( "sort" "strings" + "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" ) @@ -47,19 +48,55 @@ func (f *artifactSelectionFlag) Normalize() ([]string, error) { } func validateSelectedArtifacts(cfg *config.Config, selected []string) error { - if len(selected) == 0 { - return nil - } + _, err := resolveEffectiveArtifacts(cfg, selected) + return err +} + +func resolveEffectiveArtifacts(cfg *config.Config, selected []string) (artifacts.EffectiveArtifactSet, error) { if cfg == nil || cfg.Pipeline == nil || cfg.Pipeline.Scriptorium == nil { - return fmt.Errorf("--artifacts requires pipeline.scriptorium.artifacts to be configured") + if len(selected) == 0 { + return artifacts.ResolveEffectiveArtifactSet(nil, nil) + } + return artifacts.EffectiveArtifactSet{}, fmt.Errorf("--artifacts requires pipeline.scriptorium.artifacts to be configured") } - configured := cfg.Pipeline.Scriptorium.Artifacts - if len(configured) == 0 { - return fmt.Errorf("--artifacts requires at least one configured artifact in pipeline.scriptorium.artifacts") + configured := artifacts.ConfiguredArtifactDefinitions(cfg.Pipeline.Scriptorium.Artifacts) + if len(selected) > 0 && len(configured) == 0 { + return artifacts.EffectiveArtifactSet{}, fmt.Errorf("--artifacts requires at least one configured artifact in pipeline.scriptorium.artifacts") } - for _, name := range selected { - if _, ok := configured[name]; !ok { - return fmt.Errorf("--artifacts includes unknown artifact %q", name) + effective, err := artifacts.ResolveEffectiveArtifactSet(configured, selected) + if err != nil { + if strings.Contains(err.Error(), "is not configured") { + return artifacts.EffectiveArtifactSet{}, fmt.Errorf("--artifacts includes unknown artifact %q", selectedArtifactName(err)) + } + return artifacts.EffectiveArtifactSet{}, err + } + if err := validateEffectiveArtifactConfiguration(cfg.Pipeline.Scriptorium.Artifacts, effective); err != nil { + return artifacts.EffectiveArtifactSet{}, err + } + return effective, nil +} + +func selectedArtifactName(err error) string { + message := err.Error() + start := strings.Index(message, "\"") + if start < 0 { + return "" + } + end := strings.Index(message[start+1:], "\"") + if end < 0 { + return "" + } + return message[start+1 : start+1+end] +} + +func validateEffectiveArtifactConfiguration(configured map[string]config.ScriptoriumArtifactConfig, effective artifacts.EffectiveArtifactSet) error { + for _, name := range effective.Keys() { + artifactCfg := configured[name] + if strings.TrimSpace(artifactCfg.PromptID) == "" { + return fmt.Errorf("pipeline.scriptorium.artifacts.%s.prompt_id is required when selected", name) + } + if strings.TrimSpace(artifactCfg.OutputPath) == "" { + return fmt.Errorf("pipeline.scriptorium.artifacts.%s.output_path is required when selected", name) } } return nil diff --git a/internal/app/analyze_artifacts_test.go b/internal/app/analyze_artifacts_test.go index fa4d01a..e112064 100644 --- a/internal/app/analyze_artifacts_test.go +++ b/internal/app/analyze_artifacts_test.go @@ -110,6 +110,20 @@ func TestValidateSelectedArtifacts(t *testing.T) { }, selected: []string{"player_handout", "session_recap"}, }, + { + name: "selected disabled artifact must be executable", + cfg: &config.Config{ + Pipeline: &config.PipelineConfig{ + Scriptorium: &config.ScriptoriumConfig{ + Artifacts: map[string]config.ScriptoriumArtifactConfig{ + "player_handout": {Enabled: false, OutputPath: "artifacts/player_handout.md"}, + }, + }, + }, + }, + selected: []string{"player_handout"}, + wantErr: "pipeline.scriptorium.artifacts.player_handout.prompt_id is required when selected", + }, } for _, tt := range tests { diff --git a/internal/app/operator_artifact_rendering.go b/internal/app/operator_artifact_rendering.go index ca167fe..c8db617 100644 --- a/internal/app/operator_artifact_rendering.go +++ b/internal/app/operator_artifact_rendering.go @@ -14,21 +14,17 @@ import ( ) func buildHelperArtifactCatalog(cfg *config.Config, m *manifest.Manifest) (*artifacts.ArtifactCatalog, error) { - catalog := artifacts.NewArtifactCatalog() - if err := catalog.RegisterBuiltIns(); err != nil { - return nil, err - } - configured := map[string]artifacts.ConfiguredArtifactDefinition{} + configured := artifacts.ConfiguredArtifactDefinitions(nil) if cfg.Pipeline.Scriptorium != nil { - for key, item := range cfg.Pipeline.Scriptorium.Artifacts { - configured[key] = artifacts.ConfiguredArtifactDefinition{Enabled: item.Enabled, OutputPath: item.OutputPath} - } - } - if err := catalog.RegisterConfiguredArtifacts(configured, nil); err != nil { - return nil, err + configured = artifacts.ConfiguredArtifactDefinitions(cfg.Pipeline.Scriptorium.Artifacts) } extractionDefinitions := artifacts.ExtractionDefinitionsFromConfig(cfg.Pipeline.Notarius) - if err := catalog.RegisterExtractionArtifacts(extractionDefinitions); err != nil { + effective, err := artifacts.ResolveEffectiveArtifactSet(configured, nil) + if err != nil { + return nil, err + } + catalog, err := artifacts.BootstrapRuntimeCatalog(configured, effective, extractionDefinitions) + if err != nil { return nil, err } paths := artifacts.NewLocalStore(cfg.Pipeline.Workspace.Root).SessionPathsFor(cfg.Session.Campaign, cfg.Session.SessionID) @@ -58,8 +54,10 @@ func writeArtifactList(out io.Writer, cfg *config.Config, catalog *artifacts.Art writeExtractionArtifactLine(out, entry.SourceID, state, entry.Provenance, lockSet) } fmt.Fprintln(out, "Previous-session:") - for _, req := range artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg)) { - fmt.Fprintf(out, "- %s required=%t\n", artifactpolicy.PreviousSessionSourceID(req.Name), req.Required) + if effective, err := resolveEffectiveArtifacts(cfg, nil); err == nil { + for _, req := range artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg), effective) { + fmt.Fprintf(out, "- %s required=%t\n", artifactpolicy.PreviousSessionSourceID(req.Name), req.Required) + } } fmt.Fprintln(out, "Published:") for _, rule := range cfg.Pipeline.Publish.Outputs { diff --git a/internal/app/operator_session_validate.go b/internal/app/operator_session_validate.go index 7575386..64e1fd0 100644 --- a/internal/app/operator_session_validate.go +++ b/internal/app/operator_session_validate.go @@ -55,7 +55,12 @@ func SessionValidate(ctx context.Context, args []string, out io.Writer) error { } } - requirements := artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg)) + effective, effectiveErr := resolveEffectiveArtifacts(cfg, nil) + if effectiveErr != nil { + findings = append(findings, errorFinding("config", effectiveErr.Error())) + return renderFindings(out, cfg.Session.Campaign, cfg.Session.SessionID, findings) + } + requirements := artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg), effective) previous := inspectPreviousArtifactReadiness(ctx, cfg, store, requirements) if len(previous.Requirements) == 0 { findings = append(findings, okFinding("previous", "no previous-session artifacts required")) diff --git a/internal/app/operator_status.go b/internal/app/operator_status.go index dd07f2b..eb34003 100644 --- a/internal/app/operator_status.go +++ b/internal/app/operator_status.go @@ -69,11 +69,15 @@ func Status(ctx context.Context, args []string, out io.Writer) error { } } writeStatusRemoteAudio(ctx, out, cfg, store, storeErr) + effective, effectiveErr := resolveEffectiveArtifacts(cfg, nil) + if effectiveErr != nil { + return fmt.Errorf("status: resolve effective artifacts: %w", effectiveErr) + } writeStatusPreviousArtifacts(out, inspectPreviousArtifactReadiness( ctx, cfg, store, - artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg)), + artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg), effective), )) lockChecks := inspectEffectiveLocks(ctx, cfg, store) diff --git a/internal/app/restore_plan.go b/internal/app/restore_plan.go index 9e811ee..83a81c1 100644 --- a/internal/app/restore_plan.go +++ b/internal/app/restore_plan.go @@ -343,7 +343,11 @@ func buildPreviousCacheRestoreActions( if cfg == nil || cfg.Pipeline == nil || cfg.Pipeline.Scriptorium == nil { return nil, nil } - requirements := artifacts.CollectPreviousArtifactRequirements(cfg.Pipeline.Scriptorium.Artifacts) + effective, err := resolveEffectiveArtifacts(cfg, nil) + if err != nil { + return nil, err + } + requirements := artifacts.CollectPreviousArtifactRequirements(cfg.Pipeline.Scriptorium.Artifacts, effective) if len(requirements) == 0 { return nil, nil } diff --git a/internal/app/run.go b/internal/app/run.go index a3e9990..7250cda 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -40,14 +40,15 @@ func Run(ctx context.Context, args []string, out io.Writer) error { if err != nil { return fmt.Errorf("run: invalid --artifacts: %w", err) } - if err := validateSelectedArtifacts(cfg, normalizedArtifacts); err != nil { + effectiveArtifacts, err := resolveEffectiveArtifacts(cfg, normalizedArtifacts) + if err != nil { return fmt.Errorf("run: %w", err) } - stages := BuildFullPlan() summary, err := executeStagesFn(ctx, cfg, stages, RunOptions{ - Force: force, - SelectedArtifacts: normalizedArtifacts, + Force: force, + SelectedArtifacts: normalizedArtifacts, + EffectiveArtifacts: effectiveArtifacts, }) if err != nil { return fmt.Errorf("run: %w", err) diff --git a/internal/app/run_stage.go b/internal/app/run_stage.go index ccec48a..492be04 100644 --- a/internal/app/run_stage.go +++ b/internal/app/run_stage.go @@ -216,13 +216,14 @@ func runSingleStageCommand(ctx context.Context, req singleStageCommand) (*RunSum if err := config.Validate(cfg); err != nil { return nil, fmt.Errorf("%s: %w", req.CommandName, err) } - if err := validateSelectedArtifacts(cfg, req.SelectedArtifacts); err != nil { + effectiveArtifacts, err := resolveEffectiveArtifacts(cfg, req.SelectedArtifacts) + if err != nil { return nil, fmt.Errorf("%s: %w", req.CommandName, err) } - summary, err := executeStagesFn(ctx, cfg, stages, RunOptions{ - Force: req.Force, - SelectedArtifacts: req.SelectedArtifacts, + Force: req.Force, + SelectedArtifacts: req.SelectedArtifacts, + EffectiveArtifacts: effectiveArtifacts, }) if err != nil { return nil, fmt.Errorf("%s: %w", req.CommandName, err) diff --git a/internal/app/runner.go b/internal/app/runner.go index 04e780e..d41450c 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -23,10 +23,11 @@ import ( ) type RunOptions struct { - Force bool - SelectedArtifacts []string - Env *Env - RunManifestStore manifest.RunStore + Force bool + SelectedArtifacts []string + EffectiveArtifacts artifacts.EffectiveArtifactSet + Env *Env + RunManifestStore manifest.RunStore } type RunSummary struct { @@ -42,6 +43,21 @@ type RunSummary struct { var executeStagesFn = executeStages func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage, opts RunOptions) (summary *RunSummary, resultErr error) { + effectiveArtifacts := opts.EffectiveArtifacts + if !effectiveArtifacts.Resolved() && cfg != nil && cfg.Pipeline != nil && cfg.Pipeline.Scriptorium != nil { + var err error + effectiveArtifacts, err = resolveEffectiveArtifacts(cfg, nil) + if err != nil { + return nil, fmt.Errorf("resolve effective artifacts: %w", err) + } + } + if !effectiveArtifacts.Resolved() { + var err error + effectiveArtifacts, err = artifacts.ResolveEffectiveArtifactSet(nil, nil) + if err != nil { + return nil, fmt.Errorf("resolve default effective artifacts: %w", err) + } + } runID, err := artifacts.NewRunID() if err != nil { return nil, fmt.Errorf("generate run id: %w", err) @@ -59,6 +75,7 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage // Injected environments supply collaborators, never an alternate config. env.Config = cfg env.SelectedArtifactKeys = append([]string(nil), opts.SelectedArtifacts...) + env.EffectiveArtifacts = effectiveArtifacts if env.ManifestStore == nil { env.ManifestStore = &manifest.LocalStore{} } @@ -188,7 +205,7 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage if env.Scriptorium == nil { env.Scriptorium = scriptorium.NewSubprocessRunner() } - if env.ObjectStore == nil && needsObjectStoreForRun(env.Config, stages) { + if env.ObjectStore == nil && needsObjectStoreForRun(env.Config, stages, effectiveArtifacts) { objectStore, err := newCommandObjectStore(ctx, env.Config, nil) if err != nil { return nil, persistTerminalFailure( @@ -838,10 +855,24 @@ func manifestPathFor(cfg *config.Config) string { ) } -func needsObjectStoreForRun(cfg *config.Config, stages []stage.Stage) bool { +func needsObjectStoreForRun(cfg *config.Config, stages []stage.Stage, effectiveSets ...artifacts.EffectiveArtifactSet) bool { if cfg == nil || cfg.Pipeline == nil || cfg.Session == nil { return false } + effective := artifacts.EffectiveArtifactSet{} + if len(effectiveSets) > 0 { + effective = effectiveSets[0] + } + if !effective.Resolved() && cfg.Pipeline.Scriptorium != nil { + var err error + effective, err = artifacts.ResolveEffectiveArtifactSet( + artifacts.ConfiguredArtifactDefinitions(cfg.Pipeline.Scriptorium.Artifacts), + nil, + ) + if err != nil { + return false + } + } stageRequested := func(name string) bool { for _, s := range stages { if s != nil && s.Name() == name { @@ -855,7 +886,7 @@ func needsObjectStoreForRun(cfg *config.Config, stages []stage.Stage) bool { return true } if stageRequested("prepare") { - requirements := artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg)) + requirements := artifacts.CollectPreviousArtifactRequirements(configuredScriptoriumArtifacts(cfg), effective) if len(requirements) > 0 && strings.TrimSpace(cfg.Session.PreviousSessionID) != "" { return true } diff --git a/internal/artifacts/catalog.go b/internal/artifacts/catalog.go index 6f46852..9114094 100644 --- a/internal/artifacts/catalog.go +++ b/internal/artifacts/catalog.go @@ -138,10 +138,28 @@ func (c *ArtifactCatalog) RegisterBuiltIns() error { return nil } -// RegisterConfiguredArtifacts registers configured artifacts and applies executable selection. +// RegisterConfiguredArtifacts registers configured artifacts. New runtime +// callers should resolve selection first and use RegisterEffectiveConfiguredArtifacts. func (c *ArtifactCatalog) RegisterConfiguredArtifacts( configured map[string]ConfiguredArtifactDefinition, - selected []string, + selected ...[]string, +) error { + var requested []string + if len(selected) > 0 { + requested = selected[0] + } + effective, err := ResolveEffectiveArtifactSet(configured, requested) + if err != nil { + return err + } + return c.RegisterEffectiveConfiguredArtifacts(configured, effective) +} + +// RegisterEffectiveConfiguredArtifacts registers configured artifacts using one +// resolved analyze execution set. +func (c *ArtifactCatalog) RegisterEffectiveConfiguredArtifacts( + configured map[string]ConfiguredArtifactDefinition, + effective EffectiveArtifactSet, ) error { keys := make([]string, 0, len(configured)) for key := range configured { @@ -149,15 +167,6 @@ func (c *ArtifactCatalog) RegisterConfiguredArtifacts( } sort.Strings(keys) - selectedSet := map[string]struct{}{} - for _, key := range selected { - trimmed := strings.TrimSpace(key) - if !artifactpolicy.IsConfiguredKey(trimmed) { - return fmt.Errorf("selected artifact key %q must match ^[a-z][a-z0-9_]*$", key) - } - selectedSet[trimmed] = struct{}{} - } - for _, key := range keys { trimmed := strings.TrimSpace(key) if !artifactpolicy.IsConfiguredKey(trimmed) { @@ -169,11 +178,6 @@ func (c *ArtifactCatalog) RegisterConfiguredArtifacts( return fmt.Errorf("duplicate configured artifact key %q", trimmed) } - executable := def.Enabled - if len(selectedSet) > 0 { - _, executable = selectedSet[trimmed] - } - if err := c.addEntry(CatalogEntry{ SourceID: sourceID, ConfiguredKey: trimmed, @@ -181,24 +185,36 @@ func (c *ArtifactCatalog) RegisterConfiguredArtifacts( ProducerStage: "analyze", OutputKind: "scriptorium_artifact", Planned: true, - Executable: executable, + Executable: effective.Includes(trimmed), }); err != nil { return fmt.Errorf("register configured artifact %q: %w", trimmed, err) } c.configuredIndex[trimmed] = sourceID } - if len(selectedSet) > 0 { - for key := range selectedSet { - if _, ok := c.configuredIndex[key]; !ok { - return fmt.Errorf("selected artifact %q is not configured", key) - } - } - } - return nil } +// BootstrapRuntimeCatalog creates the deterministic catalog definitions shared +// by runtime consumers. Availability remains the caller's responsibility. +func BootstrapRuntimeCatalog( + configured map[string]ConfiguredArtifactDefinition, + effective EffectiveArtifactSet, + extraction map[string]ExtractionArtifactDefinition, +) (*ArtifactCatalog, error) { + catalog := NewArtifactCatalog() + if err := catalog.RegisterBuiltIns(); err != nil { + return nil, err + } + if err := catalog.RegisterEffectiveConfiguredArtifacts(configured, effective); err != nil { + return nil, err + } + if err := catalog.RegisterExtractionArtifacts(extraction); err != nil { + return nil, err + } + return catalog, nil +} + // Lookup returns one catalog entry by source ID. func (c *ArtifactCatalog) Lookup(sourceID string) (CatalogEntry, bool) { if c == nil { diff --git a/internal/artifacts/effective_set.go b/internal/artifacts/effective_set.go new file mode 100644 index 0000000..f710f44 --- /dev/null +++ b/internal/artifacts/effective_set.go @@ -0,0 +1,98 @@ +package artifacts + +import ( + "fmt" + "sort" + "strings" + + "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +// EffectiveArtifactSet is the resolved set of configured artifacts that an +// analyze invocation will execute. +type EffectiveArtifactSet struct { + keys []string + resolved bool +} + +// ResolveEffectiveArtifactSet applies an explicit artifact selection when one +// is supplied; otherwise it selects the configured enabled artifacts. +func ResolveEffectiveArtifactSet( + configured map[string]ConfiguredArtifactDefinition, + selected []string, +) (EffectiveArtifactSet, error) { + configuredKeys := make(map[string]ConfiguredArtifactDefinition, len(configured)) + for key, definition := range configured { + trimmed := strings.TrimSpace(key) + if !artifactpolicy.IsConfiguredKey(trimmed) { + return EffectiveArtifactSet{}, fmt.Errorf("configured artifact key %q must match ^[a-z][a-z0-9_]*$", key) + } + configuredKeys[trimmed] = definition + } + + if len(selected) > 0 { + set := make(map[string]struct{}, len(selected)) + for _, key := range selected { + trimmed := strings.TrimSpace(key) + if !artifactpolicy.IsConfiguredKey(trimmed) { + return EffectiveArtifactSet{}, fmt.Errorf("selected artifact key %q must match ^[a-z][a-z0-9_]*$", key) + } + if _, ok := configuredKeys[trimmed]; !ok { + return EffectiveArtifactSet{}, fmt.Errorf("selected artifact %q is not configured", trimmed) + } + set[trimmed] = struct{}{} + } + return newEffectiveArtifactSet(set), nil + } + + set := make(map[string]struct{}, len(configuredKeys)) + for key, definition := range configuredKeys { + if definition.Enabled { + set[key] = struct{}{} + } + } + return newEffectiveArtifactSet(set), nil +} + +// ConfiguredArtifactDefinitions converts Scriptorium configuration into the +// definition form used by the runtime catalog. +func ConfiguredArtifactDefinitions(items map[string]config.ScriptoriumArtifactConfig) map[string]ConfiguredArtifactDefinition { + if len(items) == 0 { + return nil + } + definitions := make(map[string]ConfiguredArtifactDefinition, len(items)) + for key, item := range items { + definitions[key] = ConfiguredArtifactDefinition{ + Enabled: item.Enabled, + OutputPath: item.OutputPath, + } + } + return definitions +} + +// Keys returns the selected configured artifact keys in deterministic order. +func (s EffectiveArtifactSet) Keys() []string { + return append([]string(nil), s.keys...) +} + +// Includes reports whether a configured artifact belongs to the effective set. +func (s EffectiveArtifactSet) Includes(key string) bool { + trimmed := strings.TrimSpace(key) + index := sort.SearchStrings(s.keys, trimmed) + return index < len(s.keys) && s.keys[index] == trimmed +} + +// Resolved reports whether the set was created by ResolveEffectiveArtifactSet. +func (s EffectiveArtifactSet) Resolved() bool { + return s.resolved +} + +func newEffectiveArtifactSet(set map[string]struct{}) EffectiveArtifactSet { + keys := make([]string, 0, len(set)) + for key := range set { + keys = append(keys, key) + } + sort.Strings(keys) + return EffectiveArtifactSet{keys: keys, resolved: true} +} diff --git a/internal/artifacts/effective_set_test.go b/internal/artifacts/effective_set_test.go new file mode 100644 index 0000000..7450f86 --- /dev/null +++ b/internal/artifacts/effective_set_test.go @@ -0,0 +1,86 @@ +package artifacts + +import ( + "reflect" + "testing" + + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +func TestResolveEffectiveArtifactSetUsesDefaultsOrExplicitSelection(t *testing.T) { + configured := map[string]ConfiguredArtifactDefinition{ + "session_recap": {Enabled: true, OutputPath: "artifacts/session_recap.md"}, + "player_handout": {Enabled: false, OutputPath: "artifacts/player_handout.md"}, + } + + defaults, err := ResolveEffectiveArtifactSet(configured, nil) + if err != nil { + t.Fatalf("ResolveEffectiveArtifactSet(defaults) error = %v", err) + } + if got, want := defaults.Keys(), []string{"session_recap"}; !reflect.DeepEqual(got, want) { + t.Fatalf("default keys = %v, want %v", got, want) + } + + explicit, err := ResolveEffectiveArtifactSet(configured, []string{"player_handout"}) + if err != nil { + t.Fatalf("ResolveEffectiveArtifactSet(explicit) error = %v", err) + } + if got, want := explicit.Keys(), []string{"player_handout"}; !reflect.DeepEqual(got, want) { + t.Fatalf("explicit keys = %v, want %v", got, want) + } +} + +func TestBootstrapRuntimeCatalogUsesEffectiveSetAndDeterministicDefinitions(t *testing.T) { + configured := map[string]ConfiguredArtifactDefinition{ + "zeta": {Enabled: true, OutputPath: "artifacts/zeta.md"}, + "alpha": {Enabled: false, OutputPath: "artifacts/alpha.md"}, + } + effective, err := ResolveEffectiveArtifactSet(configured, []string{"alpha"}) + if err != nil { + t.Fatalf("ResolveEffectiveArtifactSet() error = %v", err) + } + catalog, err := BootstrapRuntimeCatalog(configured, effective, map[string]ExtractionArtifactDefinition{ + "zeta_lane": {LaneID: "zeta"}, + "alpha_lane": {LaneID: "alpha"}, + }) + if err != nil { + t.Fatalf("BootstrapRuntimeCatalog() error = %v", err) + } + + configuredEntries := catalog.ListConfigured() + if got, want := []string{configuredEntries[0].ConfiguredKey, configuredEntries[1].ConfiguredKey}, []string{"alpha", "zeta"}; !reflect.DeepEqual(got, want) { + t.Fatalf("configured order = %v, want %v", got, want) + } + if !configuredEntries[0].Executable || configuredEntries[1].Executable { + t.Fatalf("configured executability = %+v, want only alpha executable", configuredEntries) + } + extractionEntries := catalog.ListExtraction() + if got, want := []string{extractionEntries[0].ExtractionKey, extractionEntries[1].ExtractionKey}, []string{"alpha_lane", "zeta_lane"}; !reflect.DeepEqual(got, want) { + t.Fatalf("extraction order = %v, want %v", got, want) + } +} + +func TestCollectPreviousArtifactRequirementsUsesExactEffectiveSet(t *testing.T) { + configured := map[string]config.ScriptoriumArtifactConfig{ + "default_artifact": { + Enabled: true, + Inputs: map[string]config.ScriptoriumInputConfig{ + "previous": {Source: "narratio.previous_session.artifact.default_artifact", Required: true}, + }, + }, + "explicit_artifact": { + Enabled: false, + Inputs: map[string]config.ScriptoriumInputConfig{ + "previous": {Source: "narratio.previous_session.artifact.explicit_artifact", Required: true}, + }, + }, + } + effective, err := ResolveEffectiveArtifactSet(ConfiguredArtifactDefinitions(configured), []string{"explicit_artifact"}) + if err != nil { + t.Fatalf("ResolveEffectiveArtifactSet() error = %v", err) + } + requirements := CollectPreviousArtifactRequirements(configured, effective) + if len(requirements) != 1 || requirements[0].Name != "explicit_artifact" || !requirements[0].Required { + t.Fatalf("requirements = %#v, want only the explicit artifact prerequisite", requirements) + } +} diff --git a/internal/artifacts/previous_requirements.go b/internal/artifacts/previous_requirements.go index 1c7b9ea..ca9bf34 100644 --- a/internal/artifacts/previous_requirements.go +++ b/internal/artifacts/previous_requirements.go @@ -16,10 +16,11 @@ type PreviousArtifactRequirement struct { Sources []string } -// CollectPreviousArtifactRequirements scans enabled Scriptorium artifacts and returns -// deduplicated previous-session artifact requirements in deterministic order. +// CollectPreviousArtifactRequirements scans the effective Scriptorium artifact +// set and returns deduplicated previous-session requirements in deterministic order. func CollectPreviousArtifactRequirements( artifactsCfg map[string]config.ScriptoriumArtifactConfig, + effective EffectiveArtifactSet, ) []PreviousArtifactRequirement { if len(artifactsCfg) == 0 { return nil @@ -30,7 +31,7 @@ func CollectPreviousArtifactRequirements( for _, artifactName := range artifactNames { artifactCfg := artifactsCfg[artifactName] - if !artifactCfg.Enabled { + if !effective.Includes(artifactName) { continue } diff --git a/internal/artifacts/previous_requirements_test.go b/internal/artifacts/previous_requirements_test.go index 5187404..2aea788 100644 --- a/internal/artifacts/previous_requirements_test.go +++ b/internal/artifacts/previous_requirements_test.go @@ -181,7 +181,11 @@ func TestCollectPreviousArtifactRequirements(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := CollectPreviousArtifactRequirements(tt.artifactsCfg) + effective, err := ResolveEffectiveArtifactSet(ConfiguredArtifactDefinitions(tt.artifactsCfg), nil) + if err != nil { + t.Fatalf("ResolveEffectiveArtifactSet() error = %v", err) + } + got := CollectPreviousArtifactRequirements(tt.artifactsCfg, effective) if !reflect.DeepEqual(got, tt.want) { t.Fatalf("CollectPreviousArtifactRequirements() = %#v, want %#v", got, tt.want) } diff --git a/internal/stage/analyze.go b/internal/stage/analyze.go index 4744c85..1aae38b 100644 --- a/internal/stage/analyze.go +++ b/internal/stage/analyze.go @@ -84,18 +84,29 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S }}, nil } + effective := env.EffectiveArtifacts + if !effective.Resolved() { + effective, err = artifacts.ResolveEffectiveArtifactSet( + artifacts.ConfiguredArtifactDefinitions(env.Config.Pipeline.Scriptorium.Artifacts), + env.SelectedArtifactKeys, + ) + if err != nil { + return nil, fmt.Errorf("analyze: resolve effective artifacts: %w", err) + } + } + runtimeCatalog, err := buildAnalyzeRuntimeArtifactCatalog( paths, m, env.Config.Pipeline.Scriptorium, env.Config.Pipeline.Notarius, - env.SelectedArtifactKeys, + effective, ) if err != nil { return nil, fmt.Errorf("analyze: build runtime artifact catalog: %w", err) } - plans, skipReason, err := buildAnalyzeExecutionPlans(env.Config.Pipeline.Scriptorium, runtimeCatalog) + plans, skipReason, err := buildAnalyzeExecutionPlans(env.Config.Pipeline.Scriptorium, effective, runtimeCatalog) if err != nil { return nil, fmt.Errorf("analyze: %w", err) } @@ -182,6 +193,7 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S func buildAnalyzeExecutionPlans( scriptoriumCfg *config.ScriptoriumConfig, + effective artifacts.EffectiveArtifactSet, catalog *artifacts.ArtifactCatalog, ) ([]analyzeArtifactExecutionPlan, string, error) { if scriptoriumCfg == nil { @@ -191,18 +203,11 @@ func buildAnalyzeExecutionPlans( return nil, "no scriptorium artifacts configured", nil } - entries := catalog.ListConfigured() - selected := make([]string, 0, len(entries)) - for _, entry := range entries { - if entry.Executable { - selected = append(selected, entry.ConfiguredKey) - } - } - if len(selected) == 0 { + if len(effective.Keys()) == 0 { return nil, "no selected scriptorium artifacts to execute", nil } - ordered, err := orderSelectedScriptoriumArtifacts(scriptoriumCfg.Artifacts, selected, catalog) + ordered, err := orderSelectedScriptoriumArtifacts(scriptoriumCfg.Artifacts, effective, catalog) if err != nil { return nil, "", err } @@ -220,16 +225,12 @@ func buildAnalyzeExecutionPlans( func orderSelectedScriptoriumArtifacts( artifactsCfg map[string]config.ScriptoriumArtifactConfig, - selected []string, + effective artifacts.EffectiveArtifactSet, catalog *artifacts.ArtifactCatalog, ) ([]string, error) { selectedSet := map[string]struct{}{} - for _, key := range selected { - trimmed := strings.TrimSpace(key) - if trimmed == "" { - return nil, fmt.Errorf("selected artifact key must be non-empty") - } - selectedSet[trimmed] = struct{}{} + for _, key := range effective.Keys() { + selectedSet[key] = struct{}{} } for selectedKey := range selectedSet { @@ -722,33 +723,20 @@ func buildAnalyzeRuntimeArtifactCatalog( m *manifest.Manifest, scriptoriumCfg *config.ScriptoriumConfig, notariusCfg *config.NotariusConfig, - selectedArtifacts []string, + effective artifacts.EffectiveArtifactSet, ) (*artifacts.ArtifactCatalog, error) { - catalog := artifacts.NewArtifactCatalog() - if err := catalog.RegisterBuiltIns(); err != nil { - return nil, err + configured := artifacts.ConfiguredArtifactDefinitions(nil) + if scriptoriumCfg != nil { + configured = artifacts.ConfiguredArtifactDefinitions(scriptoriumCfg.Artifacts) } extractionDefinitions := artifacts.ExtractionDefinitionsFromConfig(notariusCfg) - if err := catalog.RegisterExtractionArtifacts(extractionDefinitions); err != nil { + catalog, err := artifacts.BootstrapRuntimeCatalog(configured, effective, extractionDefinitions) + if err != nil { return nil, err } if notariusCfg != nil && notariusCfg.Enabled { catalog.HydrateExtractionArtifacts(paths, m, extractionDefinitions) } - if scriptoriumCfg == nil { - return catalog, nil - } - - configured := map[string]artifacts.ConfiguredArtifactDefinition{} - for key, artifactCfg := range scriptoriumCfg.Artifacts { - configured[key] = artifacts.ConfiguredArtifactDefinition{ - Enabled: artifactCfg.Enabled, - OutputPath: artifactCfg.OutputPath, - } - } - if err := catalog.RegisterConfiguredArtifacts(configured, selectedArtifacts); err != nil { - return nil, err - } for _, entry := range catalog.ListConfigured() { if entry.Executable { diff --git a/internal/stage/prepare.go b/internal/stage/prepare.go index 9668da1..8ade5ff 100644 --- a/internal/stage/prepare.go +++ b/internal/stage/prepare.go @@ -203,7 +203,10 @@ func (prepareStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S } } - previousRequirements := collectPreparePreviousRequirements(env.Config) + previousRequirements, err := collectPreparePreviousRequirements(env.Config, env.EffectiveArtifacts) + if err != nil { + return nil, fmt.Errorf("prepare: resolve previous artifact requirements: %w", err) + } var previousHydration *previousSessionHydrationResult if err := clearManagedPreviousState(paths); err != nil { return nil, fmt.Errorf("prepare: clear previous-session cache: %w", err) @@ -513,11 +516,21 @@ func countAudioInputs(inputs []manifest.InputRecord) int { return count } -func collectPreparePreviousRequirements(cfg *config.Config) []artifacts.PreviousArtifactRequirement { +func collectPreparePreviousRequirements(cfg *config.Config, effective artifacts.EffectiveArtifactSet) ([]artifacts.PreviousArtifactRequirement, error) { if cfg == nil || cfg.Pipeline == nil || cfg.Pipeline.Scriptorium == nil { - return nil + return nil, nil } - return artifacts.CollectPreviousArtifactRequirements(cfg.Pipeline.Scriptorium.Artifacts) + if !effective.Resolved() { + var err error + effective, err = artifacts.ResolveEffectiveArtifactSet( + artifacts.ConfiguredArtifactDefinitions(cfg.Pipeline.Scriptorium.Artifacts), + nil, + ) + if err != nil { + return nil, err + } + } + return artifacts.CollectPreviousArtifactRequirements(cfg.Pipeline.Scriptorium.Artifacts, effective), nil } func clearManagedPreviousState(paths artifacts.SessionPaths) error { diff --git a/internal/stage/prepare_effective_artifacts_test.go b/internal/stage/prepare_effective_artifacts_test.go new file mode 100644 index 0000000..25fdf3c --- /dev/null +++ b/internal/stage/prepare_effective_artifacts_test.go @@ -0,0 +1,32 @@ +package stage + +import ( + "testing" + + "gitea.maximumdirect.net/eric/narratio/internal/artifacts" + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +func TestCollectPreparePreviousRequirementsUsesExplicitDisabledArtifact(t *testing.T) { + configured := map[string]config.ScriptoriumArtifactConfig{ + "session_recap": { + Enabled: false, + Inputs: map[string]config.ScriptoriumInputConfig{ + "previous": {Source: "narratio.previous_session.artifact.session_recap", Required: true}, + }, + }, + } + effective, err := artifacts.ResolveEffectiveArtifactSet(artifacts.ConfiguredArtifactDefinitions(configured), []string{"session_recap"}) + if err != nil { + t.Fatalf("ResolveEffectiveArtifactSet() error = %v", err) + } + requirements, err := collectPreparePreviousRequirements(&config.Config{ + Pipeline: &config.PipelineConfig{Scriptorium: &config.ScriptoriumConfig{Artifacts: configured}}, + }, effective) + if err != nil { + t.Fatalf("collectPreparePreviousRequirements() error = %v", err) + } + if len(requirements) != 1 || requirements[0].Name != "session_recap" || !requirements[0].Required { + t.Fatalf("requirements = %#v, want selected disabled artifact requirement", requirements) + } +} diff --git a/internal/stage/publish.go b/internal/stage/publish.go index 853b207..ff87ca0 100644 --- a/internal/stage/publish.go +++ b/internal/stage/publish.go @@ -849,31 +849,22 @@ func buildPublishRuntimeArtifactCatalog( scriptoriumCfg *config.ScriptoriumConfig, notariusCfg *config.NotariusConfig, ) (*artifacts.ArtifactCatalog, error) { - catalog := artifacts.NewArtifactCatalog() - if err := catalog.RegisterBuiltIns(); err != nil { + extractionDefinitions := artifacts.ExtractionDefinitionsFromConfig(notariusCfg) + configured := artifacts.ConfiguredArtifactDefinitions(nil) + if scriptoriumCfg != nil { + configured = artifacts.ConfiguredArtifactDefinitions(scriptoriumCfg.Artifacts) + } + effective, err := artifacts.ResolveEffectiveArtifactSet(configured, nil) + if err != nil { return nil, err } - extractionDefinitions := artifacts.ExtractionDefinitionsFromConfig(notariusCfg) - if err := catalog.RegisterExtractionArtifacts(extractionDefinitions); err != nil { + catalog, err := artifacts.BootstrapRuntimeCatalog(configured, effective, extractionDefinitions) + if err != nil { return nil, err } if notariusCfg != nil && notariusCfg.Enabled { catalog.HydrateExtractionArtifacts(paths, m, extractionDefinitions) } - if scriptoriumCfg == nil { - return catalog, nil - } - - configured := map[string]artifacts.ConfiguredArtifactDefinition{} - for key, artifactCfg := range scriptoriumCfg.Artifacts { - configured[key] = artifacts.ConfiguredArtifactDefinition{ - Enabled: artifactCfg.Enabled, - OutputPath: artifactCfg.OutputPath, - } - } - if err := catalog.RegisterConfiguredArtifacts(configured, nil); err != nil { - return nil, err - } for _, entry := range catalog.ListConfigured() { if strings.TrimSpace(entry.CanonicalRelPath) == "" { diff --git a/internal/stage/stage.go b/internal/stage/stage.go index 2577d2f..badf053 100644 --- a/internal/stage/stage.go +++ b/internal/stage/stage.go @@ -22,6 +22,7 @@ import ( type Env struct { Config *config.Config SelectedArtifactKeys []string + EffectiveArtifacts artifacts.EffectiveArtifactSet ArtifactStore artifacts.Store ManifestStore manifest.Store Logger *slog.Logger