Cleaned up and removed legacy configuration surfaces
This commit is contained in:
@@ -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.<name>.inputs.<key>.path` | string | No | empty |
|
||||
| `pipeline.scriptorium.artifacts.<name>.inputs.<key>.required` | bool | No | `false` |
|
||||
| `pipeline.scriptorium.artifacts.<name>.vars.<key>` | 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.<name>.inputs.<key>.source` values:
|
||||
- `narratio.transcript.trimmed`
|
||||
- `narratio.bounds.session`
|
||||
- `narratio.artifact.<configured_artifact_key>`
|
||||
- `previous_session_artifact` (legacy path-based source; uses `inputs.<key>.path`)
|
||||
|
||||
`pipeline.archive.promote_artifacts[].source` values:
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -12,8 +12,7 @@ Inputs:
|
||||
Source types used by analyze:
|
||||
- built-ins: `narratio.transcript.*`, `narratio.bounds.session`;
|
||||
- configured artifacts: `narratio.artifact.<artifact_key>`;
|
||||
- canonical previous-session artifacts: `narratio.previous_session.artifact.<artifact_key>`;
|
||||
- legacy path-based previous-session source: `previous_session_artifact` (uses `inputs.*.path`).
|
||||
- canonical previous-session artifacts: `narratio.previous_session.artifact.<artifact_key>`.
|
||||
|
||||
Outputs:
|
||||
- promoted configured artifact files at each configured `output_path`;
|
||||
|
||||
159
docs/roadmap/cleanup.md
Normal file
159
docs/roadmap/cleanup.md
Normal file
@@ -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.<configured_artifact_key>`.
|
||||
- Canonical previous-session artifact sources such as `narratio.previous_session.artifact.<configured_artifact_key>`.
|
||||
|
||||
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.<name>.path`.
|
||||
- Tests cover canonical previous-session sources and the rejection of the legacy source.
|
||||
- The canonical replacement is `narratio.previous_session.artifact.<configured_artifact_key>`, 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.<configured_artifact_key>`.
|
||||
- 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.<name>.path` for previous artifacts.
|
||||
- Keep canonical handling for `narratio.previous_session.artifact.<configured_artifact_key>`.
|
||||
- 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.
|
||||
Reference in New Issue
Block a user