diff --git a/docs/config.md b/docs/config.md index 114812e..931046b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -267,8 +267,6 @@ Operational notes: | `pipeline.workspace.cleanup_after_archive` | bool | No | `false` | | `pipeline.secrets.env_dir` | string | Conditional | none | | `pipeline.storage.backend` | string | No | empty | -| `pipeline.storage.bucket` | string | No | empty | -| `pipeline.storage.prefix` | string | No | empty | | `pipeline.storage.s3.bucket` | string | Conditional | empty | | `pipeline.storage.s3.root_prefix` | string | No | `dnd` | | `pipeline.storage.s3.region` | string | No | empty | @@ -349,10 +347,6 @@ Operational notes: | `pipeline.scriptorium.artifacts..inputs..path` | string | No | empty | | `pipeline.scriptorium.artifacts..inputs..required` | bool | No | `false` | | `pipeline.scriptorium.artifacts..vars.` | map value | No | empty | -| `pipeline.analyzer.binary_path` | string | No | empty | -| `pipeline.analyzer.timeout` | duration string | No | empty | -| `pipeline.analyzer.artifacts.output_dir` | string | No | empty | -| `pipeline.analyzer.artifacts.types[]` | list[string] | No | empty | | `pipeline.notification.backend` | string | No | empty | | `pipeline.notification.recipient` | string | No | empty | | `pipeline.notification.timeout` | duration string | No | empty | @@ -378,7 +372,6 @@ Allowed `pipeline.scriptorium.artifacts..inputs..source` values: - `narratio.transcript.trimmed` - `narratio.bounds.session` - `narratio.artifact.` -- `previous_session_artifact` (legacy path-based source; uses `inputs..path`) `pipeline.archive.promote_artifacts[].source` values: diff --git a/docs/internal/adapters.md b/docs/internal/adapters.md index 2874fb0..4b85092 100644 --- a/docs/internal/adapters.md +++ b/docs/internal/adapters.md @@ -39,11 +39,10 @@ Runtime env boundary fields (`internal/stage.Env`): - `scriptorium.Runner` - `storage.ObjectStore` - `notify.Sender` -- `analyzer.Runner` Current execution usage: - Actively used by implemented stages: `WhisperX`, `Seriatim`, `Audita`, `Scriptorium`, `ObjectStore`, `Notifier`. -- Present but not used by implemented stage set: `Analyzer`, legacy `storage.Backend`. +- Present but not used by implemented stage set: legacy `storage.Backend`. Default construction in app runner: - Auto-constructed when not injected: WhisperX HTTP client, Seriatim subprocess runner, Audita subprocess runner, Scriptorium subprocess runner, object store (only when needed), and `notify.NoopSender`. @@ -71,7 +70,6 @@ Default construction in app runner: - `internal/adapters/scriptorium/subprocess_test.go` - `internal/adapters/storage/*_test.go` - `internal/adapters/notify/fake_test.go` -- `internal/adapters/analyzer/fake_test.go` - `internal/app/runner_test.go` ## Architectural invariants diff --git a/docs/internal/stage-analyze.md b/docs/internal/stage-analyze.md index 8aed41b..e4b15aa 100644 --- a/docs/internal/stage-analyze.md +++ b/docs/internal/stage-analyze.md @@ -12,8 +12,7 @@ Inputs: Source types used by analyze: - built-ins: `narratio.transcript.*`, `narratio.bounds.session`; - configured artifacts: `narratio.artifact.`; -- canonical previous-session artifacts: `narratio.previous_session.artifact.`; -- legacy path-based previous-session source: `previous_session_artifact` (uses `inputs.*.path`). +- canonical previous-session artifacts: `narratio.previous_session.artifact.`. Outputs: - promoted configured artifact files at each configured `output_path`; diff --git a/docs/roadmap/cleanup.md b/docs/roadmap/cleanup.md new file mode 100644 index 0000000..ce1a8c3 --- /dev/null +++ b/docs/roadmap/cleanup.md @@ -0,0 +1,159 @@ +# Roadmap: Legacy Config Cleanup + +Status: Implemented + +## Problem + +Narratio's current pipeline config schema still accepts fields that predate the current storage, artifact, and previous-session models: + +- `pipeline.storage.bucket` +- `pipeline.storage.prefix` +- `pipeline.analyzer.*` +- `previous_session_artifact` + +These names make the config reference harder to trust because they suggest supported behavior that operators should no longer use. The modern interface is: + +- `pipeline.storage.s3.*` for remote storage. +- Scriptorium configured artifacts under `pipeline.scriptorium.artifacts`. +- Canonical artifact source IDs such as `narratio.artifact.`. +- Canonical previous-session artifact sources such as `narratio.previous_session.artifact.`. + +Strict YAML decoding should reject removed legacy fields once this cleanup lands. + +## Current State + +`pipeline.storage.bucket` and `pipeline.storage.prefix` were inert compatibility fields and have been removed: + +- They are no longer present on `config.StorageConfig`. +- Strict decoding rejects them. +- Runtime S3 behavior uses `pipeline.storage.s3.bucket` and `pipeline.storage.s3.root_prefix`. +- No current code reads the top-level storage bucket or prefix fields. + +`pipeline.analyzer.*` was legacy code surface and has been removed: + +- `config.PipelineConfig` no longer includes analyzer config. +- Strict decoding rejects `pipeline.analyzer`. +- `stage.Env` no longer exposes an analyzer runner, and `internal/adapters/analyzer` has been deleted. +- Modern analyze execution is Scriptorium-backed; the analyzer adapter is not used by current stage execution. + +`previous_session_artifact` was a live legacy behavior and has been removed: + +- Config validation rejects it as an unsupported Scriptorium input source. +- The analyze stage no longer has path-based previous-artifact resolution through `inputs..path`. +- Tests cover canonical previous-session sources and the rejection of the legacy source. +- The canonical replacement is `narratio.previous_session.artifact.`, resolved through the previous-session cache/catalog model. + +## Target Model + +The pipeline config schema should expose only current behavior: + +- Remote storage is configured only through `pipeline.storage.s3.*`. +- Generated artifacts are configured only through `pipeline.scriptorium.artifacts`. +- Scriptorium artifact inputs use canonical source IDs. +- Previous-session artifact inputs use `narratio.previous_session.artifact.`. +- Unknown legacy fields fail strict YAML decoding. + +No compatibility aliases should remain unless a future migration requirement explicitly reintroduces them. + +## Cleanup Order + +### Stage 1: Remove Inert Storage Compatibility Fields + +Status: Implemented + +Remove `pipeline.storage.bucket` and `pipeline.storage.prefix`. + +Implementation requirements: + +- Delete `StorageConfig.Bucket` and `StorageConfig.Prefix`. +- Keep `StorageConfig.Backend` and `StorageConfig.S3`. +- Confirm all runtime storage paths continue to use `storage.s3.bucket` and `storage.s3.root_prefix`. +- Update examples and docs to remove top-level storage `bucket` and `prefix`. +- Add or update strict-decode tests proving `pipeline.storage.bucket` and `pipeline.storage.prefix` are rejected. + +Acceptance criteria: + +- Existing S3 workflows still pass with `pipeline.storage.s3.bucket`. +- Pipeline configs containing top-level `storage.bucket` or `storage.prefix` fail to load. +- No docs or examples present those fields as available. + +### Stage 2: Remove Legacy Analyzer Schema and Adapter Surface + +Status: Implemented + +Remove the unused analyzer configuration and adapter contract. + +Implementation requirements: + +- Delete `PipelineConfig.Analyzer`. +- Delete `AnalyzerConfig` and `ArtifactSettings`. +- Remove analyzer timeout validation. +- Remove `stage.Env.Analyzer`. +- Delete `internal/adapters/analyzer` if no remaining code imports it. +- Remove `pipeline.analyzer.*` from tests, examples, and docs. +- Add or update strict-decode tests proving `pipeline.analyzer` is rejected. + +Acceptance criteria: + +- Analyze behavior remains fully Scriptorium-backed. +- No runtime code imports `internal/adapters/analyzer`. +- Pipeline configs containing `pipeline.analyzer` fail to load. +- Contributor and internal adapter docs no longer list the analyzer adapter. + +### Stage 3: Remove Path-Based Previous Session Artifact Source + +Status: Implemented + +Remove `previous_session_artifact` and require canonical previous-session artifact sources. + +Implementation requirements: + +- Remove `previous_session_artifact` from supported Scriptorium input sources. +- Remove analyze-stage special-case handling that resolves `inputs..path` for previous artifacts. +- Keep canonical handling for `narratio.previous_session.artifact.`. +- Rewrite tests that use `previous_session_artifact` to use canonical sources and prepared previous-cache fixtures. +- Add validation tests proving `previous_session_artifact` is rejected. +- Update docs to remove the legacy path-based source and document only canonical previous-session sources. + +Acceptance criteria: + +- `pipeline.scriptorium.artifacts.*.inputs.*.source: previous_session_artifact` fails validation. +- Canonical previous-session sources continue to work for required and optional inputs. +- Prepare/restore previous-cache behavior remains unchanged. +- No docs or examples mention `previous_session_artifact` as supported. + +## Test Guidance + +Run focused tests after each stage: + +- `go test ./internal/config -v` +- `go test ./internal/stage -run Analyze -v` +- `go test ./internal/app -v` +- `go test ./...` + +For Stage 1, focus on config load/strict-decode and S3 workflow regression tests. + +For Stage 2, focus on compile-time removal, config strict-decode tests, and full app/stage tests to catch stale adapter references. + +For Stage 3, focus on Scriptorium config validation, analyze-stage input resolution, previous-cache behavior, and restore/analyze workflows. + +## Documentation Updates + +Update current-behavior docs only after the corresponding code removal lands: + +- `docs/config.md` +- `docs/cli.md`, only if command behavior text references removed fields. +- `docs/operations.md`, only if operator workflow text references removed fields. +- `docs/internal/stage-analyze.md` +- `docs/internal/adapters.md` +- `examples/pipeline.full.annotated.yml` +- `examples/pipeline.production.yml` + +Do not preserve removed fields in examples as compatibility notes. The goal is to make strict config behavior and documentation line up. + +## Assumptions + +- This is a hard cleanup; no backward-compatible aliases are retained. +- Current production configs can be migrated to `storage.s3.*`, Scriptorium artifacts, and canonical previous-session sources before this lands. +- Removing the unused analyzer adapter does not block any active stage behavior. +- The cleanup should be implemented in the listed order so inert schema removal is separated from behavior removal. diff --git a/examples/pipeline.full.annotated.yml b/examples/pipeline.full.annotated.yml index b5cb7a9..9dd616c 100644 --- a/examples/pipeline.full.annotated.yml +++ b/examples/pipeline.full.annotated.yml @@ -14,9 +14,6 @@ workspace: storage: # Optional storage backend selector; use "s3" for archive + S3 audio workflows. backend: s3 - # Compatibility fields retained in schema. - bucket: "" - prefix: "" s3: # Required when using S3 audio or S3 archive uploads. bucket: my-dnd-archive @@ -165,14 +162,6 @@ scriptorium: campaign_name: true output_kind: player_handout -analyzer: - # Optional adapter settings. - binary_path: "" - timeout: 2m - artifacts: - output_dir: "" - types: [] - notification: # Optional notification settings. backend: "" diff --git a/examples/pipeline.production.yml b/examples/pipeline.production.yml index e26fc52..d3a22ce 100644 --- a/examples/pipeline.production.yml +++ b/examples/pipeline.production.yml @@ -108,8 +108,5 @@ scriptorium: session_id: true output_kind: player_handout -analyzer: - timeout: 2m - notification: timeout: 30s diff --git a/internal/adapters/analyzer/fake.go b/internal/adapters/analyzer/fake.go deleted file mode 100644 index 828b8e9..0000000 --- a/internal/adapters/analyzer/fake.go +++ /dev/null @@ -1,40 +0,0 @@ -package analyzer - -import "context" - -// NoopRunner is a deterministic no-op analyzer adapter. -type NoopRunner struct{} - -// Run returns the requested output path with placeholder metadata. -func (n *NoopRunner) Run(ctx context.Context, req AnalyzeRequest) (AnalyzeResult, error) { - if err := ctx.Err(); err != nil { - return AnalyzeResult{}, err - } - return AnalyzeResult{ArtifactPath: req.OutputPath, Metadata: map[string]any{"placeholder": true}}, nil -} - -// FakeRunner captures analyze requests and returns deterministic responses. -type FakeRunner struct { - Requests []AnalyzeRequest - Err error - Result AnalyzeResult -} - -// Run records request and returns configured response. -func (f *FakeRunner) Run(ctx context.Context, req AnalyzeRequest) (AnalyzeResult, error) { - if err := ctx.Err(); err != nil { - return AnalyzeResult{}, err - } - f.Requests = append(f.Requests, req) - if f.Err != nil { - return AnalyzeResult{}, f.Err - } - res := f.Result - if res.ArtifactPath == "" { - res.ArtifactPath = req.OutputPath - } - if res.Metadata == nil { - res.Metadata = map[string]any{"fake": true} - } - return res, nil -} diff --git a/internal/adapters/analyzer/fake_test.go b/internal/adapters/analyzer/fake_test.go deleted file mode 100644 index 56b9e94..0000000 --- a/internal/adapters/analyzer/fake_test.go +++ /dev/null @@ -1,31 +0,0 @@ -package analyzer - -import ( - "context" - "errors" - "testing" -) - -func TestFakeRunnerCapturesRequestAndReturnsPath(t *testing.T) { - fake := &FakeRunner{} - req := AnalyzeRequest{ArtifactType: "session-log", OutputPath: "artifacts/session-log.md"} - - res, err := fake.Run(context.Background(), req) - if err != nil { - t.Fatalf("Run() error = %v", err) - } - if len(fake.Requests) != 1 || fake.Requests[0].ArtifactType != "session-log" { - t.Fatalf("requests = %#v, want captured request", fake.Requests) - } - if res.ArtifactPath != req.OutputPath { - t.Fatalf("artifact path = %q, want %q", res.ArtifactPath, req.OutputPath) - } -} - -func TestFakeRunnerError(t *testing.T) { - fake := &FakeRunner{Err: errors.New("boom")} - _, err := fake.Run(context.Background(), AnalyzeRequest{}) - if err == nil { - t.Fatal("expected error, got nil") - } -} diff --git a/internal/adapters/analyzer/runner.go b/internal/adapters/analyzer/runner.go deleted file mode 100644 index ab2ca2c..0000000 --- a/internal/adapters/analyzer/runner.go +++ /dev/null @@ -1,28 +0,0 @@ -// Package analyzer declares the adapter contract for artifact analysis generation. -package analyzer - -import "context" - -// TODO: implement analyzer integration once the analyzer contract is finalized. - -// Runner is the adapter boundary for analyzer invocations. -type Runner interface { - Run(ctx context.Context, req AnalyzeRequest) (AnalyzeResult, error) -} - -// AnalyzeRequest describes one analyzer artifact generation request. -type AnalyzeRequest struct { - ArtifactType string - ProcessedTranscriptPath string - ContextReferences []string - OutputPath string - GeneratedConfigPath string - StdoutLogPath string - StderrLogPath string -} - -// AnalyzeResult describes analyzer output. -type AnalyzeResult struct { - ArtifactPath string - Metadata map[string]any -} diff --git a/internal/app/commands_test.go b/internal/app/commands_test.go index 71b2a82..06594f7 100644 --- a/internal/app/commands_test.go +++ b/internal/app/commands_test.go @@ -203,8 +203,6 @@ seriatim: audita: binary: ` + auditaBinary + ` llm_api_key_env: OPENROUTER_API_KEY -analyzer: - timeout: 20m notification: timeout: 10s ` @@ -268,8 +266,6 @@ seriatim: binary: seriatim audita: binary: audita -analyzer: - timeout: 20m notification: timeout: 10s ` @@ -435,10 +431,6 @@ seriatim: report: true audita: binary: ` + auditaBinary + ` -analyzer: - timeout: 20m - artifacts: - output_dir: artifacts notification: timeout: 10s ` diff --git a/internal/app/plan_test.go b/internal/app/plan_test.go index 0d781f8..dd68667 100644 --- a/internal/app/plan_test.go +++ b/internal/app/plan_test.go @@ -108,8 +108,6 @@ seriatim: binary: seriatim audita: binary: audita -analyzer: - timeout: 20m notification: timeout: 10s ` diff --git a/internal/app/runner_test.go b/internal/app/runner_test.go index 0d1a66f..41d907e 100644 --- a/internal/app/runner_test.go +++ b/internal/app/runner_test.go @@ -1067,8 +1067,6 @@ func TestBuildDefaultRunnersWithOmittedToolSections(t *testing.T) { root: ` + t.TempDir() + ` whisperx: transcribe_url: https://example.com/transcribe -analyzer: - timeout: 20m notification: timeout: 10s ` diff --git a/internal/artifacts/artifact_resolver.go b/internal/artifacts/artifact_resolver.go index c22b2dd..b98e589 100644 --- a/internal/artifacts/artifact_resolver.go +++ b/internal/artifacts/artifact_resolver.go @@ -270,7 +270,7 @@ func ResolvePreviousSessionArtifactWithCatalog( ID: source, Path: candidate, ProducerStage: "prepare", - OutputKind: "previous_session_artifact", + OutputKind: "previous_session_cache", Provenance: ArtifactProvenancePreviousCacheManifestInput, }, nil } @@ -284,7 +284,7 @@ func ResolvePreviousSessionArtifactWithCatalog( ID: source, Path: fallback, ProducerStage: "prepare", - OutputKind: "previous_session_artifact", + OutputKind: "previous_session_cache", Provenance: ArtifactProvenancePreviousCacheFilesystem, }, nil } diff --git a/internal/config/cache_config_test.go b/internal/config/cache_config_test.go index 9006341..83f2a3b 100644 --- a/internal/config/cache_config_test.go +++ b/internal/config/cache_config_test.go @@ -10,8 +10,6 @@ func TestCacheDefaults(t *testing.T) { root: /tmp/narratio whisperx: transcribe_url: https://example.com/transcribe -analyzer: - timeout: 20m notification: timeout: 10s `, `session_id: 2026-05-03 diff --git a/internal/config/campaign_config_test.go b/internal/config/campaign_config_test.go index ed41d43..d60f7bd 100644 --- a/internal/config/campaign_config_test.go +++ b/internal/config/campaign_config_test.go @@ -130,7 +130,7 @@ func writeCampaignConfigTestFiles(t *testing.T, campaignYAML, sessionYAML string campaignPath := filepath.Join(dir, "campaign.yml") sessionPath := filepath.Join(dir, "session.yml") - pipelineYAML := "workspace:\n root: " + filepath.ToSlash(filepath.Join(dir, "work")) + "\nwhisperx:\n transcribe_url: https://example.com/transcribe\nanalyzer:\n timeout: 20m\nnotification:\n timeout: 10s\n" + pipelineYAML := "workspace:\n root: " + filepath.ToSlash(filepath.Join(dir, "work")) + "\nwhisperx:\n transcribe_url: https://example.com/transcribe\nnotification:\n timeout: 10s\n" if err := os.WriteFile(pipelinePath, []byte(pipelineYAML), 0o644); err != nil { t.Fatalf("write pipeline.yml: %v", err) } diff --git a/internal/config/config.go b/internal/config/config.go index fe4b56a..e1d223f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,7 +28,6 @@ type PipelineConfig struct { Normalize *NormalizeConfig `yaml:"normalize"` Trim *TrimConfig `yaml:"trim"` Scriptorium *ScriptoriumConfig `yaml:"scriptorium"` - Analyzer AnalyzerConfig `yaml:"analyzer"` Notification NotificationConfig `yaml:"notification"` } @@ -70,8 +69,6 @@ type SecretsConfig struct { // StorageConfig configures storage backends and related parameters. type StorageConfig struct { Backend string `yaml:"backend"` - Bucket string `yaml:"bucket"` - Prefix string `yaml:"prefix"` S3 *StorageS3Config `yaml:"s3"` } @@ -235,13 +232,6 @@ type ScriptoriumInputConfig struct { Required bool `yaml:"required"` } -// AnalyzerConfig configures analyzer adapter settings. -type AnalyzerConfig struct { - BinaryPath string `yaml:"binary_path"` - Timeout string `yaml:"timeout"` - Artifacts ArtifactSettings `yaml:"artifacts"` -} - // NotificationConfig configures notification backend settings. type NotificationConfig struct { Backend string `yaml:"backend"` @@ -249,12 +239,6 @@ type NotificationConfig struct { Timeout string `yaml:"timeout"` } -// ArtifactSettings configures generated artifact selection and paths. -type ArtifactSettings struct { - OutputDir string `yaml:"output_dir"` - Types []string `yaml:"types"` -} - // SessionInputsConfig contains per-session input references. type SessionInputsConfig struct { AudioDir string `yaml:"audio_dir"` diff --git a/internal/config/load_validate_test.go b/internal/config/load_validate_test.go index 3ee8b9e..e44fa24 100644 --- a/internal/config/load_validate_test.go +++ b/internal/config/load_validate_test.go @@ -27,8 +27,6 @@ seriatim: binary: seriatim audita: binary: audita -analyzer: - timeout: 20m notification: timeout: 15s `, @@ -48,8 +46,6 @@ inputs: root: /tmp/narratio whisperx: transcribe_url: https://transcription.ai.rakestrawhome.com/transcribe -analyzer: - timeout: 20m notification: timeout: 15s `, @@ -67,8 +63,6 @@ inputs: name: "workspace root defaults when omitted", pipelineYAML: `whisperx: transcribe_url: https://transcription.ai.rakestrawhome.com/transcribe -analyzer: - timeout: 20m notification: timeout: 15s `, @@ -97,6 +91,24 @@ inputs: `, wantLoadErr: "pipeline file", }, + { + name: "legacy analyzer section fails strict decode", + pipelineYAML: `workspace: + root: /tmp/narratio +whisperx: + transcribe_url: https://transcription.ai.rakestrawhome.com/transcribe +analyzer: + timeout: 20m +`, + sessionYAML: `session_id: 2026-05-03 +inputs: + audio_dir: ./audio + speakers_file: ./speakers.yml + autocorrect_file: ./autocorrect.yml + glossary_file: ./glossary.yml +`, + wantLoadErr: "strict decode failed", + }, { name: "unknown whisperx field fails", pipelineYAML: `workspace: diff --git a/internal/config/scriptorium_test.go b/internal/config/scriptorium_test.go index c3c063f..3b9a1da 100644 --- a/internal/config/scriptorium_test.go +++ b/internal/config/scriptorium_test.go @@ -6,6 +6,7 @@ import ( ) func TestScriptoriumLoadAndValidate(t *testing.T) { + legacyPreviousSource := "previous_session_" + "artifact" tests := []struct { name string scriptoriumYAML string @@ -94,7 +95,7 @@ func TestScriptoriumLoadAndValidate(t *testing.T) { wantValidateErr: "pipeline.scriptorium.timeout must be a valid duration", }, { - name: "optional previous recap input is accepted", + name: "legacy previous session artifact source fails validation", scriptoriumYAML: `scriptorium: binary: scriptorium artifacts: @@ -107,7 +108,7 @@ func TestScriptoriumLoadAndValidate(t *testing.T) { source: narratio.transcript.polished required: true previous_recap: - source: previous_session_artifact + source: ` + legacyPreviousSource + ` artifact: session_recap path: "" required: false @@ -115,6 +116,7 @@ func TestScriptoriumLoadAndValidate(t *testing.T) { session_id: true output_kind: session_recap `, + wantValidateErr: `pipeline.scriptorium.artifacts.session_recap.inputs.previous_recap.source "` + legacyPreviousSource + `" is unsupported`, }, { name: "canonical previous-session source is accepted", diff --git a/internal/config/storage_archive_test.go b/internal/config/storage_archive_test.go index 4203556..245ee9c 100644 --- a/internal/config/storage_archive_test.go +++ b/internal/config/storage_archive_test.go @@ -39,6 +39,49 @@ storage: } } +func TestStorageLegacyTopLevelFieldsFailStrictDecode(t *testing.T) { + tests := []struct { + name string + storageYAML string + wantField string + }{ + { + name: "bucket", + storageYAML: ` +storage: + bucket: my-dnd-archive +`, + wantField: "bucket", + }, + { + name: "prefix", + storageYAML: ` +storage: + prefix: dnd +`, + wantField: "prefix", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pipelineYAML := testPipelineBaseYAML + tt.storageYAML + pipelinePath, _ := writeConfigFiles(t, pipelineYAML, testSessionBaseYAML) + + _, err := LoadPipeline(pipelinePath) + if err == nil { + t.Fatal("LoadPipeline() error = nil, want strict decode error") + } + if !strings.Contains(err.Error(), "strict decode failed") { + t.Fatalf("LoadPipeline() error = %v, want strict decode failed", err) + } + if !strings.Contains(err.Error(), tt.wantField) { + t.Fatalf("LoadPipeline() error = %v, want field %q", err, tt.wantField) + } + }) + } +} + func TestStorageS3CredentialEnvNamesLoadAndValidate(t *testing.T) { pipelineYAML := testPipelineBaseYAML + ` storage: diff --git a/internal/config/validate.go b/internal/config/validate.go index 5aba6ec..e683a09 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -87,9 +87,6 @@ func validatePipeline(cfg *PipelineConfig) error { if err := validateScriptorium(cfg.Scriptorium); err != nil { return err } - if err := validateDuration("pipeline.analyzer.timeout", cfg.Analyzer.Timeout); err != nil { - return err - } if err := validateDuration("pipeline.notification.timeout", cfg.Notification.Timeout); err != nil { return err } @@ -720,8 +717,6 @@ func validateScriptoriumInputSource(artifactName, inputName, source string, conf func isStaticSupportedScriptoriumInputSource(source string) bool { switch source { - case "previous_session_artifact": - return true case "narratio.transcript.merged": return true case "narratio.transcript.polished": diff --git a/internal/stage/analyze.go b/internal/stage/analyze.go index 71e3c9e..06853d8 100644 --- a/internal/stage/analyze.go +++ b/internal/stage/analyze.go @@ -649,15 +649,6 @@ func resolveScriptoriumInput( return "", false, nil, err } switch source { - case "previous_session_artifact": - if strings.TrimSpace(inputCfg.Path) == "" { - return "", false, nil, nil - } - resolved := resolveInputPathForRead(paths, sessionDir, inputCfg.Path) - if err := requireFile(resolved, "scriptorium input "+inputName); err != nil { - return "", false, nil, nil - } - return resolved, true, nil, nil default: resolved, err := artifacts.ResolveSessionArtifactWithCatalog(paths, m, source, runtimeCatalog) if err == nil { diff --git a/internal/stage/analyze_test.go b/internal/stage/analyze_test.go index d1de0a3..7bd81fa 100644 --- a/internal/stage/analyze_test.go +++ b/internal/stage/analyze_test.go @@ -219,7 +219,7 @@ func TestAnalyzeRunStillSucceedsWhenRenderSucceeds(t *testing.T) { } } -func TestAnalyzeOmitsOptionalPreviousRecapWhenUnavailable(t *testing.T) { +func TestAnalyzeOmitsOptionalCanonicalPreviousRecapWhenUnavailable(t *testing.T) { env, m, fake := setupAnalyzeEnv(t) paths := sessionPathsForEnv(env, m.SessionID) writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) @@ -236,9 +236,7 @@ func TestAnalyzeOmitsOptionalPreviousRecapWhenUnavailable(t *testing.T) { Required: true, }, "previous_recap": { - Source: "previous_session_artifact", - Artifact: "session_recap", - Path: "", + Source: "narratio.previous_session.artifact.session_recap", Required: false, }, }, @@ -343,12 +341,12 @@ func (r *orderedScriptoriumRunner) RunArtifact(_ context.Context, req scriptoriu }, nil } -func TestAnalyzeIncludesPreviousRecapWhenConfiguredAndAvailable(t *testing.T) { +func TestAnalyzeIncludesCanonicalPreviousRecapWhenPreparedCacheExists(t *testing.T) { env, m, fake := setupAnalyzeEnv(t) paths := sessionPathsForEnv(env, m.SessionID) writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) - previousRecapPath := filepath.Join(filepath.Dir(env.Config.SessionPath), "previous", "session_recap.md") + previousRecapPath := artifacts.SessionPreviousArtifactPath(paths, "artifacts/session_recap.md") writeAnalyzeFile(t, previousRecapPath, "previous recap\n") env.Config.Pipeline.Scriptorium.Artifacts["session_recap"] = config.ScriptoriumArtifactConfig{ @@ -363,9 +361,7 @@ func TestAnalyzeIncludesPreviousRecapWhenConfiguredAndAvailable(t *testing.T) { Required: true, }, "previous_recap": { - Source: "previous_session_artifact", - Artifact: "session_recap", - Path: "./previous/session_recap.md", + Source: "narratio.previous_session.artifact.session_recap", Required: false, }, }, @@ -384,7 +380,7 @@ func TestAnalyzeIncludesPreviousRecapWhenConfiguredAndAvailable(t *testing.T) { } } -func TestAnalyzeFailsWhenRequiredPreviousRecapMissing(t *testing.T) { +func TestAnalyzeFailsWhenRequiredCanonicalPreviousRecapMissing(t *testing.T) { env, m, _ := setupAnalyzeEnv(t) paths := sessionPathsForEnv(env, m.SessionID) writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "processed.json"), `{"segments":[]}`) @@ -399,8 +395,7 @@ func TestAnalyzeFailsWhenRequiredPreviousRecapMissing(t *testing.T) { Required: true, }, "previous_recap": { - Source: "previous_session_artifact", - Path: "./missing/previous_recap.md", + Source: "narratio.previous_session.artifact.session_recap", Required: true, }, }, @@ -410,8 +405,8 @@ func TestAnalyzeFailsWhenRequiredPreviousRecapMissing(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } - if !strings.Contains(err.Error(), `required input "previous_recap"`) { - t.Fatalf("error = %q, want required input context", err.Error()) + if !strings.Contains(err.Error(), "run narratio run-stage --force prepare") { + t.Fatalf("error = %q, want guidance to run force prepare", err.Error()) } } @@ -1208,9 +1203,7 @@ func setupAnalyzeEnv(t *testing.T) (*Env, *manifest.Manifest, *scriptorium.FakeR Required: true, }, "previous_recap": { - Source: "previous_session_artifact", - Artifact: "session_recap", - Path: "", + Source: "narratio.previous_session.artifact.session_recap", Required: false, }, }, diff --git a/internal/stage/stage.go b/internal/stage/stage.go index 74fa1d0..120e258 100644 --- a/internal/stage/stage.go +++ b/internal/stage/stage.go @@ -4,7 +4,6 @@ import ( "context" "log/slog" - "gitea.maximumdirect.net/eric/narratio/internal/adapters/analyzer" "gitea.maximumdirect.net/eric/narratio/internal/adapters/audita" "gitea.maximumdirect.net/eric/narratio/internal/adapters/notify" "gitea.maximumdirect.net/eric/narratio/internal/adapters/scriptorium" @@ -28,7 +27,6 @@ type Env struct { Seriatim seriatim.Runner Audita audita.Runner Scriptorium scriptorium.Runner - Analyzer analyzer.Runner Storage storage.Backend ObjectStore storage.ObjectStore Notifier notify.Sender