Implement final changes from the code quality and deduplication opportunity audit

This commit is contained in:
2026-05-23 10:05:14 -05:00
parent 5620fc5bcf
commit 72deccb4e2
32 changed files with 27 additions and 1339 deletions

View File

@@ -1,31 +0,0 @@
// Package storage declares archive/storage backend adapter boundaries.
package storage
import "context"
// TODO: implement remote storage/archive backends (S3/SFTP/etc.).
// Backend is the adapter boundary for archive/storage operations.
type Backend interface {
Archive(ctx context.Context, req ArchiveRequest) (ArchiveResult, error)
}
// ArchiveItem describes one item to archive.
type ArchiveItem struct {
Kind string
LocalPath string
RemoteKey string
}
// ArchiveRequest describes one archive operation.
type ArchiveRequest struct {
SessionID string
ManifestPath string
Items []ArchiveItem
}
// ArchiveResult describes archive operation output.
type ArchiveResult struct {
Archived []ArchiveItem
Metadata map[string]any
}

View File

@@ -10,23 +10,8 @@ import (
"time"
)
// NoopBackend is a deterministic no-op archive/storage adapter.
type NoopBackend struct{}
// Archive returns the requested items as archived with placeholder metadata.
func (n *NoopBackend) Archive(ctx context.Context, req ArchiveRequest) (ArchiveResult, error) {
if err := ctx.Err(); err != nil {
return ArchiveResult{}, err
}
return ArchiveResult{Archived: append([]ArchiveItem(nil), req.Items...), Metadata: map[string]any{"placeholder": true}}, nil
}
// FakeBackend captures archive requests and returns deterministic responses.
// FakeBackend provides a deterministic in-memory object store for tests.
type FakeBackend struct {
Requests []ArchiveRequest
Err error
Result ArchiveResult
Objects map[string]FakeObject
Uploads []FakeUploadCall
Downloads []FakeDownloadCall
@@ -50,25 +35,6 @@ type FakeDownloadCall struct {
LocalPath string
}
// Archive records request and returns configured response.
func (f *FakeBackend) Archive(ctx context.Context, req ArchiveRequest) (ArchiveResult, error) {
if err := ctx.Err(); err != nil {
return ArchiveResult{}, err
}
f.Requests = append(f.Requests, req)
if f.Err != nil {
return ArchiveResult{}, f.Err
}
res := f.Result
if res.Archived == nil {
res.Archived = append([]ArchiveItem(nil), req.Items...)
}
if res.Metadata == nil {
res.Metadata = map[string]any{"fake": true}
}
return res, nil
}
// FakeObject is a deterministic fake object-store record.
type FakeObject struct {
Key string

View File

@@ -9,30 +9,6 @@ import (
"testing"
)
func TestFakeBackendCapturesRequestAndReturnsItems(t *testing.T) {
fake := &FakeBackend{}
req := ArchiveRequest{SessionID: "s1", Items: []ArchiveItem{{Kind: "artifact", LocalPath: "artifacts/log.md"}}}
res, err := fake.Archive(context.Background(), req)
if err != nil {
t.Fatalf("Archive() error = %v", err)
}
if len(fake.Requests) != 1 || fake.Requests[0].SessionID != "s1" {
t.Fatalf("requests = %#v, want captured request", fake.Requests)
}
if len(res.Archived) != 1 {
t.Fatalf("archived len = %d, want 1", len(res.Archived))
}
}
func TestFakeBackendError(t *testing.T) {
fake := &FakeBackend{Err: errors.New("boom")}
_, err := fake.Archive(context.Background(), ArchiveRequest{})
if err == nil {
t.Fatal("expected error, got nil")
}
}
func TestFakeBackendListPrefixFiltering(t *testing.T) {
fake := &FakeBackend{}
fake.SeedObject(FakeObject{Key: "dnd/campaigns/forsaken/audio/a.flac", Data: []byte("a")})

View File

@@ -5,7 +5,7 @@ import (
"time"
)
// ObjectStore is a remote object storage boundary used by future prepare/archive work.
// ObjectStore is a remote object storage boundary used by prepare, restore, and publish work.
//
// Key invariant:
// callers pass full bucket-relative object keys. Backend implementations do not

View File

@@ -145,7 +145,7 @@ func TestHTTPClientDoesNotRetryOnNonRetryableStatus(t *testing.T) {
}
}
func TestHTTPClientInvalidJSONFailsAndDoesNotPromote(t *testing.T) {
func TestHTTPClientInvalidJSONFailsAndDoesNotInstallOutput(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte(`not-json`))
}))

View File

@@ -4,7 +4,6 @@ import (
"context"
"fmt"
"os"
"path/filepath"
"strings"
"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
@@ -59,7 +58,7 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, campaign
if err != nil {
return nil, missingSessionConfigError(discoveredSession.Searched, err.Error())
}
sessionTempPath, err := downloadRemoteSessionConfig(ctx, store, remoteKey)
sessionTempPath, err := storage.DownloadObjectToTemp(ctx, store, remoteKey, "narratio-session-*.yml")
if err != nil {
return nil, missingSessionConfigError(discoveredSession.Searched, fmt.Sprintf("remote session %q download failed: %v", remoteKey, err))
}
@@ -134,21 +133,6 @@ func findRemoteSessionConfig(ctx context.Context, store storage.ObjectStore, ses
return storage.ObjectInfo{}, fmt.Errorf("remote session %q not found", remoteKey)
}
func downloadRemoteSessionConfig(ctx context.Context, store storage.ObjectStore, remoteKey string) (string, error) {
f, err := os.CreateTemp("", "narratio-session-*.yml")
if err != nil {
return "", fmt.Errorf("create temp file: %w", err)
}
path := f.Name()
if err := f.Close(); err != nil {
return "", fmt.Errorf("close temp file %q: %w", path, err)
}
if err := store.Download(ctx, remoteKey, path); err != nil {
return "", err
}
return filepath.Clean(path), nil
}
func s3BucketName(cfg *config.PipelineConfig) string {
if cfg == nil || cfg.Storage.S3 == nil {
return ""

View File

@@ -34,7 +34,7 @@ func Restore(ctx context.Context, args []string, out io.Writer) error {
addCommonConfigFlags(fs, &flags)
fs.BoolVar(&dryRun, "dry-run", false, "plan restore actions without writing local files")
fs.BoolVar(&force, "force", false, "overwrite local conflicts with remote state")
fs.BoolVar(&includeAudio, "include-audio", false, "include archived session-level audio objects")
fs.BoolVar(&includeAudio, "include-audio", false, "include remote session-level audio objects")
fs.Usage = func() {
_, _ = fmt.Fprintln(out, "Usage: narratio session restore <session_id> [--config <path>] [--campaign <id>] [--campaign-file <path>] [--session <path>] [--previous-session-id <value>] [--dry-run] [--force] [--include-audio]")
_, _ = fmt.Fprintln(out)

View File

@@ -39,7 +39,7 @@ func TestExecuteRestoreNonDryRunRestoresDurableFiles(t *testing.T) {
if stderr.Len() != 0 {
t.Fatalf("stderr = %q, want empty", stderr.String())
}
if !strings.Contains(stdout.String(), "Restored session archive for sample-campaign/2026-05-03") {
if !strings.Contains(stdout.String(), "Restored session state for sample-campaign/2026-05-03") {
t.Fatalf("stdout = %q, want completion summary", stdout.String())
}

View File

@@ -22,7 +22,7 @@ func TestRestorePlanDefaultScope(t *testing.T) {
seedRestoreObject(store, current.SessionPrefix+"artifacts/session_recap.md", []byte("# recap\n"))
seedRestoreObject(store, current.SessionPrefix+"audio/alice.flac", []byte("audio"))
seedRestoreObject(store, current.SessionPrefix+"runs/20260519T010203Z-a1b2/manifest.json", []byte("{}"))
seedRestoreObject(store, current.SessionPrefix+"logs/archive.log", []byte("log"))
seedRestoreObject(store, current.SessionPrefix+"logs/publish.log", []byte("log"))
plan, err := buildRestorePlan(context.Background(), cfg, current, store, RestorePlanOptions{})
if err != nil {

View File

@@ -201,7 +201,7 @@ func writeRestoreSuccessSummary(out io.Writer, report *RestoreReport) error {
if report == nil {
return fmt.Errorf("restore report is required")
}
if _, err := fmt.Fprintf(out, "Restored session archive for %s/%s\n", report.Campaign, report.SessionID); err != nil {
if _, err := fmt.Fprintf(out, "Restored session state for %s/%s\n", report.Campaign, report.SessionID); err != nil {
return err
}
if _, err := fmt.Fprintf(out, "Remote run: %s\n", report.RunID); err != nil {

View File

@@ -369,7 +369,7 @@ func TestExecuteRestoreNonDryRunForceExecutesPlan(t *testing.T) {
if code != 0 {
t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String())
}
if !strings.Contains(stdout.String(), "Restored session archive for sample-campaign/2026-05-03") {
if !strings.Contains(stdout.String(), "Restored session state for sample-campaign/2026-05-03") {
t.Fatalf("stdout = %q, want completion summary", stdout.String())
}
if stderr.Len() != 0 {

View File

@@ -11,7 +11,6 @@ import (
"gitea.maximumdirect.net/eric/narratio/internal/adapters/notify"
"gitea.maximumdirect.net/eric/narratio/internal/adapters/scriptorium"
"gitea.maximumdirect.net/eric/narratio/internal/adapters/seriatim"
"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
"gitea.maximumdirect.net/eric/narratio/internal/adapters/whisperx"
"gitea.maximumdirect.net/eric/narratio/internal/artifacts"
"gitea.maximumdirect.net/eric/narratio/internal/config"
@@ -83,9 +82,6 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage
if env.Scriptorium == nil {
env.Scriptorium = scriptorium.NewSubprocessRunner()
}
if env.Storage == nil {
env.Storage = &storage.NoopBackend{}
}
if env.ObjectStore == nil && needsObjectStoreForRun(env.Config, stages) {
objectStore, err := newCommandObjectStore(ctx, env.Config, nil)
if err != nil {

View File

@@ -14,7 +14,7 @@ import (
"gitea.maximumdirect.net/eric/narratio/internal/manifest"
)
func TestBuildPlanResolvesPromotedArtifactFromPreviousManifest(t *testing.T) {
func TestBuildPlanResolvesPublishedArtifactFromPreviousManifest(t *testing.T) {
cfg, paths := previousCacheTestConfig(t)
store := &storage.FakeBackend{}
seedPreviousCurrent(t, store, cfg, previousManifestWithOutput(t, cfg, "artifacts/session_recap.md", []string{"artifacts/session_recap.md"}))

View File

@@ -255,7 +255,7 @@ func TestAnalyzeOmitsOptionalCanonicalPreviousRecapWhenUnavailable(t *testing.T)
}
}
func TestAnalyzeUsesRunLocalPathsAndPromotesCanonical(t *testing.T) {
func TestAnalyzeUsesRunLocalPathsAndMaterializesCanonical(t *testing.T) {
env, m, fake := setupAnalyzeEnv(t)
env.Config.Session.Campaign = "sample-campaign"
m.Campaign = "sample-campaign"

View File

@@ -156,7 +156,7 @@ func (mergeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*Sta
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("merge: promote merged transcript: %w", err)
return nil, fmt.Errorf("merge: materialize canonical base transcript: %w", err)
}
outputs := []artifacts.Ref{materializedMerged}
if reportEnabled {
@@ -166,7 +166,7 @@ func (mergeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*Sta
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("merge: promote report: %w", err)
return nil, fmt.Errorf("merge: materialize canonical report: %w", err)
}
outputs = append(outputs, materializedReport)
}

View File

@@ -289,7 +289,7 @@ func TestMergeStageFailsWhenNormalizedOutputInvalid(t *testing.T) {
}
}
func TestMergeStageUsesRunLocalPathsAndPromotesCanonical(t *testing.T) {
func TestMergeStageUsesRunLocalPathsAndMaterializesCanonical(t *testing.T) {
env, m := setupMergeEnv(t)
env.Config.Session.Campaign = "sample-campaign"
m.Campaign = "sample-campaign"

View File

@@ -139,7 +139,7 @@ func (normalizeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("normalize: promote normalized transcript: %w", err)
return nil, fmt.Errorf("normalize: materialize canonical final transcript: %w", err)
}
outputs := []artifacts.Ref{materializedNormalized}
if reportEnabled {
@@ -149,7 +149,7 @@ func (normalizeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("normalize: promote report: %w", err)
return nil, fmt.Errorf("normalize: materialize canonical report: %w", err)
}
outputs = append(outputs, materializedReport)
}

View File

@@ -211,7 +211,7 @@ func TestNormalizeStageReportEnabledFailsWhenReportMissing(t *testing.T) {
}
}
func TestNormalizeStageUsesRunLocalPathsAndPromotesCanonical(t *testing.T) {
func TestNormalizeStageUsesRunLocalPathsAndMaterializesCanonical(t *testing.T) {
env, m, ser := setupNormalizeEnv(t)
env.Config.Session.Campaign = "sample-campaign"
m.Campaign = "sample-campaign"

View File

@@ -96,7 +96,6 @@ func TestStagesReturnExpectedMetadata(t *testing.T) {
Seriatim: sf,
Audita: af,
Scriptorium: sc,
Storage: st,
ObjectStore: st,
Notifier: nf,
}
@@ -221,9 +220,6 @@ func TestStagesReturnExpectedMetadata(t *testing.T) {
if len(sc.RunRequests) != 0 {
t.Fatalf("scriptorium run calls = %d, want 0 when scriptorium config is absent", len(sc.RunRequests))
}
if len(st.Requests) != 0 {
t.Fatalf("storage publish calls = %d, want 0", len(st.Requests))
}
if _, ok := st.Objects["dnd/campaigns/sample-campaign/sessions/2026-05-03/runs/"+m.RunID+"/manifest.json"]; !ok {
t.Fatalf("publish upload missing manifest key in fake object store")
}

View File

@@ -155,7 +155,7 @@ func (polishStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*St
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("polish: promote processed transcript: %w", err)
return nil, fmt.Errorf("polish: materialize canonical polished transcript: %w", err)
}
outputs := []artifacts.Ref{materializedProcessed}
if reportEnabled {
@@ -165,7 +165,7 @@ func (polishStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*St
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("polish: promote report: %w", err)
return nil, fmt.Errorf("polish: materialize canonical report: %w", err)
}
outputs = append(outputs, materializedReport)
}

View File

@@ -238,7 +238,7 @@ func TestPolishStageFailsWhenReportInvalid(t *testing.T) {
}
}
func TestPolishStageUsesRunLocalPathsAndPromotesCanonical(t *testing.T) {
func TestPolishStageUsesRunLocalPathsAndMaterializesCanonical(t *testing.T) {
env, m := setupPolishEnv(t)
env.Config.Session.Campaign = "sample-campaign"
m.Campaign = "sample-campaign"

View File

@@ -27,7 +27,6 @@ type Env struct {
Seriatim seriatim.Runner
Audita audita.Runner
Scriptorium scriptorium.Runner
Storage storage.Backend
ObjectStore storage.ObjectStore
Notifier notify.Sender
}

View File

@@ -191,7 +191,7 @@ func TestTranscribeStageInvalidJSONFails(t *testing.T) {
}
}
func TestTranscribeStageUsesRunLocalOutputAndPromotesCanonical(t *testing.T) {
func TestTranscribeStageUsesRunLocalOutputAndMaterializesCanonical(t *testing.T) {
env, m := setupTranscribeEnv(t, []string{"alice.flac"})
env.Config.Session.Campaign = "sample-campaign"
m.Campaign = "sample-campaign"

View File

@@ -106,7 +106,7 @@ func (trimStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*Stag
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("trim: promote trimmed transcript: %w", err)
return nil, fmt.Errorf("trim: materialize canonical final trimmed transcript: %w", err)
}
metadata["trim_action"] = "copy_disabled"
return &StageResult{
@@ -357,7 +357,7 @@ func (trimStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*Stag
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("trim: promote trimmed transcript: %w", err)
return nil, fmt.Errorf("trim: materialize canonical final trimmed transcript: %w", err)
}
materializedBounds, err := materializeRunLocalOutput(env.ArtifactStore, finalBoundsOutputPath, canonicalBoundsOutputPath, artifacts.Ref{
Kind: "session_bounds",
@@ -365,7 +365,7 @@ func (trimStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*Stag
SessionID: sessionID,
})
if err != nil {
return nil, fmt.Errorf("trim: promote session bounds: %w", err)
return nil, fmt.Errorf("trim: materialize canonical session bounds: %w", err)
}
return &StageResult{

View File

@@ -298,7 +298,7 @@ func TestTrimStageDisabledCopiesNormalizedTranscript(t *testing.T) {
}
}
func TestTrimStageUsesRunLocalPathsAndPromotesCanonical(t *testing.T) {
func TestTrimStageUsesRunLocalPathsAndMaterializesCanonical(t *testing.T) {
env, m, scr, ser := setupTrimEnv(t)
env.Config.Session.Campaign = "sample-campaign"
m.Campaign = "sample-campaign"
@@ -322,7 +322,7 @@ func TestTrimStageUsesRunLocalPathsAndPromotesCanonical(t *testing.T) {
t.Fatalf("trim output path = %q, want run-local path", ser.TrimRequests[0].OutputTrimmedPath)
}
if len(result.Outputs) < 2 {
t.Fatalf("outputs = %#v, want promoted trimmed+bounds outputs", result.Outputs)
t.Fatalf("outputs = %#v, want materialized trimmed+bounds outputs", result.Outputs)
}
for _, out := range result.Outputs {
if strings.Contains(out.AbsolutePath, string(filepath.Separator)+"runs"+string(filepath.Separator)) {