From e58e545686afe79abead547f0eb27a4846e837ef Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 17 May 2026 13:22:09 -0500 Subject: [PATCH] Audit workspace architecture implementation plan --- .../workspace-implementation-plan.md | 507 ++++++++++++++++++ 1 file changed, 507 insertions(+) create mode 100644 docs/development/workspace-implementation-plan.md diff --git a/docs/development/workspace-implementation-plan.md b/docs/development/workspace-implementation-plan.md new file mode 100644 index 0000000..e82b43d --- /dev/null +++ b/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//...` (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. +