All checks were successful
ci/woodpecker/tag/release Pipeline was successful
508 lines
20 KiB
Markdown
508 lines
20 KiB
Markdown
# 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.
|
||
|