Audit workspace architecture implementation plan

docs/development/workspace-implementation-plan.md

@@ -0,0 +1,507 @@
+# Workspace Architecture Implementation Plan (Audit)
+
+## 1. Executive Summary
+
+**Complexity assessment:** **heavy**.
+
+This is not a single path-helper refactor. The current codebase has a hybrid session/run model that works for current behavior, but diverges from `docs/development/workspace.md` in foundational places (workspace root shape, manifest responsibilities, stage output placement, and archive symmetry).
+
+Highest-risk areas:
+
+1. Splitting the current single manifest model into durable **session manifest** vs per-invocation **run manifest** without regressing skip/force/resume UX.
+2. Migrating path helpers and artifact-store interfaces from session-only roots (`work/{session}`) to campaign-aware roots (`work/{campaign}/{session}`) while preserving existing runs.
+3. Introducing run-local stage outputs + immediate promotion while keeping stage tests and archive behavior stable.
+4. Avoiding stale downstream skips after forced upstream reruns.
+
+Surprising findings:
+
+1. Code already has campaign/run-aware helpers (`SessionRunWorkDir`, `SessionSpoolAudioDir`) but core session helpers and manifest pathing remain campaign-unaware.
+2. Archive recently gained run/session fallback behavior for manifest/promotion sources, which confirms an existing hybrid-layout pressure point.
+3. Analyze input resolution is functional but ad hoc and stage-local; there is no centralized artifact registry/resolver.
+
+---
+
+## 2. Current-State Map
+
+## 2.1 Workspace Path Construction
+
+Primary path model:
+
+- [`internal/artifacts/paths.go`](../../internal/artifacts/paths.go)
+  - `SessionWorkDir(rootDir, sessionID)` => `{root}/work/{session_id}`
+  - `buildSessionPaths(workspaceRoot, sessionID)` roots all canonical paths under `{root}/work/{session_id}`
+  - `SessionRunWorkDir(rootDir, campaign, sessionID, runID)` exists, but is not the default session root helper.
+
+Artifact store abstraction:
+
+- [`internal/artifacts/store.go`](../../internal/artifacts/store.go)
+  - `SessionPaths(sessionID string)` / `EnsureLayout(sessionID string)` are session-id-only (no campaign argument).
+- [`internal/artifacts/local.go`](../../internal/artifacts/local.go)
+  - `EnsureLayout` creates session-level folders under `SessionWorkDir`.
+
+Path normalization helper:
+
+- [`internal/artifacts/resolve.go`](../../internal/artifacts/resolve.go)
+  - `ResolveSessionLocalPathForRead` accepts absolute/workspace/session-relative values and probes filesystem.
+
+Where campaign-aware/run-aware support exists:
+
+- Local run path helpers: `SessionRunWorkDir`, `SessionSpoolAudioDir`.
+- S3 key helpers: [`internal/artifacts/s3_keys.go`](../../internal/artifacts/s3_keys.go) (`campaigns/sessions/runs/current`).
+
+Where session-root assumptions remain `{workspace}/work/{session}`:
+
+- Manifest path computation: [`internal/app/runner.go`](../../internal/app/runner.go) `manifestPathFor`.
+- Artifact store layout and most stage `paths := env.ArtifactStore.SessionPaths(sessionID)` calls.
+- Many tests hardcode `workspace/work/<session>/...` (examples below).
+
+Manual/ad hoc path construction (not through a single resolver API):
+
+- Common stage patterns: `filepath.Join(paths.TranscriptsDir, "...")`, `filepath.Join(paths.ArtifactsDir, "...")`, etc.
+- `prepare` run/work selection: [`internal/stage/prepare.go`](../../internal/stage/prepare.go) `pathsWorkDirForManifest`.
+- Analyze input fallback path resolution: `resolveInputPathForRead` in [`internal/stage/analyze.go`](../../internal/stage/analyze.go).
+
+## 2.2 Manifest and Run Identity
+
+Current manifest model:
+
+- [`internal/manifest/manifest.go`](../../internal/manifest/manifest.go) `Manifest` includes both session and run fields:
+  - `SessionID`, `Campaign`
+  - `RunID`, `LocalWorkDir`, `LocalSpoolDir`
+  - `S3Bucket`, `S3SessionPrefix`, `S3RunPrefix`
+  - `Stages`, `Inputs`, stage outputs/logs/generated configs/metadata.
+
+Current persistence:
+
+- [`internal/manifest/store.go`](../../internal/manifest/store.go) `LocalStore` reads/writes one JSON manifest path.
+- Runner always loads/saves one manifest path via `manifestPathFor(cfg)` (session-root path under current layout).
+
+Identity initialization:
+
+- [`internal/app/runner.go`](../../internal/app/runner.go) `ensureManifestIdentity` populates run fields if absent.
+- `RunID` is generated once for an empty manifest and reused thereafter (hybrid semantics).
+
+Interpretation today:
+
+- Best described as a **hybrid session manifest** with run identity fields, not as distinct session + run manifests.
+
+What this means for redesign:
+
+- Session-vs-run split is not just file relocation; it requires new responsibilities and write flows.
+- A backward-compatible evolution path is possible by:
+  - preserving current fields in session manifest for migration/read-compat,
+  - adding explicit run-manifest type + path,
+  - gradually moving invocation-specific details to run manifests.
+
+## 2.3 Stage Output Paths (Current)
+
+All implemented stages currently write canonical artifacts directly into session-level `paths.*` roots (under current session root), with logs/configs typically also session-level.
+
+1. `prepare` ([`internal/stage/prepare.go`](../../internal/stage/prepare.go))
+- Inputs copied to `inputs/` (`session.yml`, `pipeline.resolved.yml`, `speakers.yml`, `autocorrect.yml`, `glossary.yml`)
+- Audio copied to `audio/`
+- Manifest input provenance recorded in `m.Inputs`
+- S3 audio uses run-scoped spool/work helpers when `RunID` is present.
+
+2. `transcribe` ([`internal/stage/transcribe.go`](../../internal/stage/transcribe.go))
+- Outputs: `transcripts/raw/{speaker}.json`
+- Stage metadata only (no stage logs/config files produced here).
+
+3. `merge` ([`internal/stage/merge.go`](../../internal/stage/merge.go))
+- Pre-normalize intermediates: `transcripts/raw/normalized/{basename}.normalized.json`
+- Merge output: `transcripts/merged.json`
+- Report: `artifacts/seriatim.report.json` (if enabled)
+- Logs/configs:
+  - per-input normalize logs/configs in session-level `logs/` + `config/`
+  - merge logs/config in session-level `logs/` + `config/`
+
+4. `polish` ([`internal/stage/polish.go`](../../internal/stage/polish.go))
+- Output: `transcripts/processed.json`
+- Report: `artifacts/audita.report.json` (if enabled)
+- Work dir: `artifacts/audita-work`
+- Logs/config: `logs/audita.*`, `config/audita.generated.yml`
+
+5. `normalize` ([`internal/stage/normalize.go`](../../internal/stage/normalize.go))
+- Output: `transcripts/normalized.json` (configurable)
+- Report: `artifacts/seriatim.normalize.report.json` (if enabled)
+- Logs/config: `logs/seriatim.normalize.*`, `config/seriatim.normalize.generated.yml`
+
+6. `trim` ([`internal/stage/trim.go`](../../internal/stage/trim.go))
+- Output: `transcripts/trimmed.json` (configurable)
+- Bounds output path from config (default examples use `artifacts/session_bounds.json`)
+- Logs/configs in session-level `logs/` and `config/` for scriptorium + seriatim invocations
+
+7. `analyze` ([`internal/stage/analyze.go`](../../internal/stage/analyze.go))
+- Current implemented artifact: `artifacts/session_recap.md`
+- Logs/config in session-level `logs/` + `config/`
+- Optional render diagnostics under session-level artifacts/logs/config.
+
+8. `archive` ([`internal/stage/archive.go`](../../internal/stage/archive.go))
+- Reads from a "workDir" derived by `archiveWorkDir`:
+  - prefers `m.LocalWorkDir` if it exists,
+  - else tries run-scoped campaign/session/run path,
+  - else falls back to legacy session path.
+- Uploads run record and promotions.
+- Current code now resolves manifest and promotion sources via run/session fallback.
+
+9. `notify`
+- Placeholder only in [`internal/stage/placeholders.go`](../../internal/stage/placeholders.go); no durable outputs.
+
+Durable-vs-diagnostic split today:
+
+- Durable artifacts and diagnostics are mixed at session level.
+- No run-local stage directories exist yet.
+
+## 2.4 Idempotency / Force / Resume / Sparse Runs
+
+Run control implementation:
+
+- [`internal/app/run_control.go`](../../internal/app/run_control.go)
+  - skip rule: `!force && stageSucceeded(manifest, stage)`
+  - stale detection TODO only; no invalidation logic.
+
+Command behavior:
+
+- `run`: full plan through `executeStages`.
+- `run-stage`: single selected stage through `executeStages`.
+- `resume`: starts at first non-succeeded stage unless `--force`.
+
+Current assumptions:
+
+- Command invocation mutates a single durable workspace + single manifest for the session path model.
+- There is no per-invocation run manifest write path.
+
+Smallest safe UX-preserving invariant to keep during migration:
+
+- Session manifest remains source-of-truth for skip decisions across invocations.
+
+## 2.5 Archive and S3 Alignment
+
+S3 semantics are relatively mature:
+
+- [`internal/artifacts/s3_keys.go`](../../internal/artifacts/s3_keys.go)
+  - session prefix + run prefix + `current/manifest.json` + `current/run_id.txt`.
+
+Archive stage behavior:
+
+- uploads run records under run prefix,
+- uploads promoted session outputs,
+- uploads current manifest then current run pointer,
+- current pointer is commit marker,
+- required promotions fail; optional promotions skipped.
+
+Local-vs-remote mismatch still present:
+
+- Local canonical session root currently defaults to `work/{session}` (artifact store),
+- while archive/run helpers expect campaign-aware run locations (`work/{campaign}/{session}/{run}`),
+- causing fallback logic and hybrid handling in `archive`.
+
+## 2.6 Artifact Source Resolution in Analyze
+
+Current implementation is stage-local and ad hoc:
+
+- Transcript discoverers:
+  - `discoverProcessedTranscript`
+  - `discoverNormalizedTranscript`
+  - `discoverTrimmedTranscript`
+- Input source switch in `resolveScriptoriumInput` supports:
+  - `processed_transcript`
+  - `normalized_transcript`
+  - `trimmed_transcript`
+  - `previous_session_artifact`
+
+No first-class artifact registry exists yet. Aliases and canonical IDs are not modeled.
+
+## 2.7 Stale/Invalidation
+
+- `manifest.StageStatus` already defines `stale` (`internal/manifest/status.go`), but no stage uses it.
+- Skip logic ignores stale state and only checks `succeeded`.
+- Forced upstream rerun does not invalidate downstream stage success markers.
+
+## 2.8 Test Coverage Relevant to Redesign
+
+High-value existing coverage:
+
+- Workspace helpers:
+  - [`internal/artifacts/paths_model_test.go`](../../internal/artifacts/paths_model_test.go)
+  - [`internal/artifacts/resolve_test.go`](../../internal/artifacts/resolve_test.go)
+- Runner semantics:
+  - [`internal/app/runner_test.go`](../../internal/app/runner_test.go)
+  - [`internal/app/resume_run_stage_test.go`](../../internal/app/resume_run_stage_test.go)
+- Stage path/output behavior:
+  - `internal/stage/*_test.go` for prepare/transcribe/merge/polish/normalize/trim/analyze/archive
+- Archive behavior:
+  - [`internal/stage/archive_test.go`](../../internal/stage/archive_test.go)
+  - [`internal/app/post_archive_cleanup_test.go`](../../internal/app/post_archive_cleanup_test.go)
+
+Tests likely to fail during workspace redesign:
+
+- Any tests hardcoding `work/{session}` manifest and transcript paths (many in `internal/app/*test.go`, `internal/stage/*test.go`).
+- Archive tests assuming current hybrid fallback behavior.
+
+---
+
+## 3. Gap Analysis Against `docs/development/workspace.md`
+
+| Intended concept | Current status | Notes |
+|---|---|---|
+| Session root at `work/{campaign}/{session}` | **Partial / mostly absent** | `SessionWorkDir` and artifact store still use `work/{session}`. Campaign-aware run helper exists separately. |
+| Distinct session manifest vs run manifest | **Absent** | One hybrid manifest model/file is used. |
+| Run-local stage dirs under `runs/{run_id}/{stage}` | **Absent** | Stages write canonical outputs/logs/config directly at session level. |
+| Immediate promotion run-local -> session canonical | **Absent** | No run-local staging area to promote from today. |
+| Session manifest skip source of truth | **Present** | Skip/resume use single manifest stage statuses. |
+| Sparse runs represented under `runs/{run_id}` | **Absent** | No run-manifest/per-run stage records yet. |
+| Artifact resolver with canonical IDs + aliases | **Absent** | Analyze resolves via stage-local source-name switch and fallback helpers. |
+| Downstream invalidation for forced upstream reruns | **Absent** | TODO only; no stale propagation or status clearing. |
+| Local semantics mirror archive semantics | **Partial** | Archive/S3 side models runs/current, local workspace core still session-layout-centric. |
+| Safe path helpers centralization | **Partial** | Good helper base exists, but many stage-level manual joins still encode conventions. |
+
+---
+
+## 4. Recommended Implementation Sequence
+
+## Step 1: Introduce campaign-aware session path model without behavior break
+
+Purpose:
+
+- Add first-class helpers for `work/{campaign}/{session}` and make them available everywhere.
+
+Expected changes:
+
+- `internal/artifacts`: add/extend path helpers and `SessionPaths` constructor variants that accept campaign.
+- `internal/app`: pass campaign into path-model entrypoints where available.
+
+Behavior change:
+
+- None initially (can keep legacy fallback reads).
+
+Tests:
+
+- Add campaign-aware path-model tests.
+- Keep legacy-path compatibility tests.
+
+Risks:
+
+- Wide compile-time touch due `Store` interface signatures.
+
+Rollback:
+
+- Keep legacy helper wrappers until full migration lands.
+
+## Step 2: Split manifest responsibilities (session manifest + run manifest scaffolding)
+
+Purpose:
+
+- Preserve current UX while introducing explicit run execution records.
+
+Expected changes:
+
+- `internal/manifest`: add run manifest type/store helpers.
+- `internal/app/runner.go`: create/load session manifest and initialize per-invocation run manifest path.
+
+Behavior change:
+
+- Session manifest remains skip truth source.
+- Run manifest begins recording invocation metadata/stage actions.
+
+Tests:
+
+- New tests for both manifest files existing and being updated correctly.
+
+Risks:
+
+- Incorrect ordering of saves can regress crash consistency.
+
+Rollback:
+
+- Keep session-manifest-only decision logic until run manifest proves stable.
+
+## Step 3: Move stage execution products to run-local directories with promotion
+
+Purpose:
+
+- Align with workspace architecture (`runs/{run_id}/{stage}/...`) while preserving canonical outputs.
+
+Expected changes:
+
+- `internal/stage`: each implemented stage writes outputs/logs/config/reports to run-local paths.
+- Introduce shared promotion helpers (atomic copy/rename + output validation + manifest provenance).
+
+Behavior change:
+
+- Canonical outputs remain session-level; run-local diagnostics now preserved per run.
+
+Tests:
+
+- Stage tests updated to assert run-local outputs + promoted canonical outputs.
+- New tests for immediate promotion and producer run metadata.
+
+Risks:
+
+- Highest regression risk (all implemented stages touched).
+
+Rollback:
+
+- Stage-by-stage migration flag or phased rollout by stage order.
+
+## Step 4: Archive alignment pass
+
+Purpose:
+
+- Remove hybrid fallback complexity once local layout is canonical.
+
+Expected changes:
+
+- `internal/stage/archive.go`: resolve sources from canonical session outputs and run manifests deterministically.
+- Keep `current/manifest.json` + `current/run_id.txt` publication semantics.
+
+Behavior change:
+
+- Simpler source selection; fewer cross-layout heuristics.
+
+Tests:
+
+- Archive tests for run uploads, promotions, current pointers, and fallback removal/compat gates.
+
+Risks:
+
+- Breaking current mixed-layout compatibility too early.
+
+Rollback:
+
+- Retain fallback compatibility for one migration window.
+
+## Step 5: Artifact registry/resolver (analyze first consumer)
+
+Purpose:
+
+- Replace ad hoc analyze source resolution with canonical artifact IDs and aliases.
+
+Expected changes:
+
+- New resolver package (for example `internal/artifacts/registry` or `internal/stage/artifactresolve`) with IDs:
+  - `narratio.transcript.merged`
+  - `narratio.transcript.polished`
+  - `narratio.transcript.full`
+  - `narratio.transcript.trimmed`
+  - `narratio.bounds.session`
+  - `narratio.artifact.session_recap`
+- Backward-compatible alias map for current source names.
+
+Behavior change:
+
+- Analyze input resolution becomes centralized and consistent.
+
+Tests:
+
+- Resolver unit tests for canonical IDs + aliases + missing-input error clarity.
+- Analyze tests updated to assert resolver usage.
+
+Risks:
+
+- Input resolution edge cases for `previous_session_artifact` and required/optional handling.
+
+Rollback:
+
+- Keep old resolver path behind a temporary compatibility function.
+
+## Step 6: Minimal downstream invalidation after forced upstream reruns
+
+Purpose:
+
+- Prevent stale downstream skips before full checksum stale detection exists.
+
+Expected changes:
+
+- `internal/app/run_control.go` + manifest transition helpers.
+- On forced rerun success of stage `X`, clear/mark downstream success states.
+
+Behavior change:
+
+- Subsequent runs no longer skip stale downstream stages.
+
+Tests:
+
+- New run-control tests for force-induced downstream invalidation.
+- Resume tests with forced sparse runs.
+
+Risks:
+
+- Over-invalidating too broadly and degrading UX.
+
+Rollback:
+
+- Start with deterministic downstream stage list based on pipeline order only.
+
+## Step 7: Legacy layout migration strategy
+
+Purpose:
+
+- Handle existing `work/{session}` data safely.
+
+Expected changes:
+
+- Startup detection/migration path in app layer.
+- Clear failure messages for ambiguous legacy states.
+
+Behavior change:
+
+- Explicit migration semantics instead of implicit fallback drift.
+
+Tests:
+
+- Migration detection tests for legacy-only, new-only, and ambiguous layouts.
+
+Risks:
+
+- Silent duplication if both layouts are partially populated.
+
+Rollback:
+
+- Prefer fail-fast ambiguity policy over auto-merge.
+
+---
+
+## 5. Minimal Viable v1.0 Scope
+
+Must-have for v1.0:
+
+1. Campaign-aware canonical session roots.
+2. Session manifest remains skip/resume authority.
+3. Introduce run manifests + run-local stage records.
+4. Immediate promotion from run-local outputs to canonical session outputs.
+5. Minimal downstream invalidation on forced upstream rerun.
+6. Archive/local semantic alignment with `current/*` behavior preserved.
+7. Backward-compatible analyze aliases (`processed_transcript`, `normalized_transcript`, `trimmed_transcript`).
+
+Nice-to-have / defer if risky:
+
+1. Full checksum-based stale detection graph.
+2. Broad manifest schema-version migration framework.
+3. Full artifact-registry rollout beyond analyze’s initial needs.
+4. Aggressive cleanup of all legacy fallback branches in one release.
+
+---
+
+## 6. Open Questions (with Recommendations)
+
+1. **Should campaign be mandatory for all local path derivation immediately?**
+- Recommendation: yes for new layout writes; keep controlled read compatibility for legacy session-only roots during migration window.
+
+2. **Session manifest location transition policy:** auto-migrate vs explicit migrate command?
+- Recommendation: if user base is small, prefer explicit fail-fast with actionable migration instructions to avoid silent split-brain state.
+
+3. **Run manifest granularity:** per-stage detailed records vs summary + references?
+- Recommendation: start with summary + stage status/paths; avoid duplicating full session artifact state to keep write path simple.
+
+4. **Invalidation marking:** use `stale` status now or clear `succeeded` markers?
+- Recommendation: if invasive to propagate new status semantics quickly, clear/overwrite downstream success states first; add formal `stale` usage in follow-up.
+
+5. **Promotion timing:** promote every stage immediately vs delayed at end of run?
+- Recommendation: immediate per-stage promotion after validation (matches current idempotent skip expectations and simplifies resume behavior).
+
+6. **Docs consistency order after implementation starts:**
+- Recommendation: update `README.md` and `architecture.md` in lockstep with each migration step. Current conflict is explicit:
+  - code/README/architecture still primarily describe session roots at `work/{session}`
+  - `docs/development/workspace.md` defines `work/{campaign}/{session}` plus run manifests.
+