seriatim/docs/roadmap/cleanup.md

# Pre-1.0 Cleanup Implementation Plan

## Purpose

This roadmap defines the staged cleanup work recommended by
`docs/roadmap/audit.md`. It is written for coding agents that will implement the
cleanup in order.

This is a roadmap document. It may describe planned refactors because it lives
under `docs/roadmap/`. Implementation agents must not document planned cleanup
as current behavior outside `docs/roadmap/`.

## Governing Policies

Follow these policy documents before implementing any stage:

- `docs/policy/architecture.md`
- `docs/policy/development.md`
- `docs/policy/documentation.md`

The cleanup must preserve these project rules:

- Keep CLI code thin: parse flags, build validated config, delegate.
- Keep config normalization and validation in `internal/config`.
- Keep public output contracts and validation in `schema`.
- Keep merge pipeline orchestration in `internal/pipeline`.
- Keep trim and normalize artifact-level; do not run merge modules from those
  commands.
- Keep behavior deterministic and sequential.
- Prefer narrow helpers over broad frameworks.
- Do not add concurrency, dynamic plugins, durable state, manifests, remote
  storage, or resume behavior.

## Global Implementation Rules

Each stage should be implemented as a small, behavior-preserving change.

For every stage:

- Start with `git status --short`.
- Read the files listed in that stage before editing.
- Do not modify unrelated files.
- Preserve public CLI flags, defaults, output schemas, JSON shapes, report event
  ordering, report event text, and error messages unless the stage explicitly
  says otherwise.
- Prefer private helpers unless a cross-package helper is genuinely needed.
- Do not introduce generic engines, plugin systems, broad adapter layers, or
  generics-heavy abstractions.
- Run the stage-specific tests listed in the stage.
- Run `go test ./...` before considering the full cleanup sequence complete.

Documentation updates during cleanup should be minimal. If a refactor changes
implemented package boundaries that are described in `docs/internal/` or
`docs/policy/architecture.md`, update those docs in the same implementation
stage. Do not update user-facing docs if behavior did not change.

## Stage 1: Centralize Output Schema Names

### Goal

Make `schema` the canonical source for public output schema names and keep
command-specific default/precedence policy in `internal/config`.

### Decision

Use the `schema` package as the canonical home for output schema name constants
because public output contracts already live there.

Add these exported string constants to `schema`:

- `OutputSchemaMinimal = "seriatim-minimal"`
- `OutputSchemaIntermediate = "seriatim-intermediate"`
- `OutputSchemaFull = "seriatim-full"`

Add narrow schema-name helpers in `schema`, not CLI-specific helpers:

- `ValidOutputSchemaName(value string) bool`
- `OutputSchemaNames() []string`

Do not put CLI flag names or default selection policy in `schema`.

Keep the `internal/config` constants as aliases for compatibility inside the
repository:

- `config.OutputSchemaMinimal = schema.OutputSchemaMinimal`
- `config.OutputSchemaIntermediate = schema.OutputSchemaIntermediate`
- `config.OutputSchemaFull = schema.OutputSchemaFull`

Remove the duplicate string constants from `internal/trim/artifact.go`; use the
canonical schema constants instead. A type alias or package-local aliases are
acceptable only if they point directly to the `schema` constants and do not
repeat string literals.

### Files To Inspect

- `schema/output.go`
- `schema/output_test.go`
- `internal/config/config.go`
- `internal/config/config_test.go`
- `internal/trim/artifact.go`
- `internal/trim/artifact_test.go`
- `internal/artifact/transcript.go`
- `internal/normalize/build.go`
- `internal/cli/*_test.go`

### Implementation Steps

1. Add canonical output schema constants and helpers to `schema/output.go`.
2. Update `internal/config` schema constants to alias `schema` constants.
3. Update config validation to use `schema.ValidOutputSchemaName` while keeping
   the existing `--output-schema must be one of ...` error text.
4. Update `internal/trim` to stop repeating schema string constants.
5. Update affected tests only where they refer to moved constants.
6. Search for raw schema string literals outside tests and docs. Keep raw values
   in docs and test fixtures where they are intentionally testing serialized
   public JSON.

### Acceptance Criteria

- There is one canonical set of output schema string constants in `schema`.
- `merge` and `normalize` still default to `seriatim-intermediate` and still
  honor `SERIATIM_OUTPUT_SCHEMA`.
- `trim` still preserves the input artifact schema when `--output-schema` is
  omitted.
- Invalid schema error text remains stable unless tests are intentionally
  updated.
- No new schema registry or plugin-style abstraction exists.

### Validation Commands

```sh
go test ./schema ./internal/config ./internal/artifact ./internal/trim ./internal/normalize ./internal/cli
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 2: Extract Trim Projection Policy

### Goal

Use one implementation for trim selector policy, ID validation, retained-index
selection, ID renumbering metadata, removed IDs, and empty-output handling.

### Decision

Add a private projection helper in `internal/trim/apply.go`. Do not introduce a
generic artifact framework.

Recommended shape:

```go
type projection struct {
    retainedIndexes []int
    oldToNewID      map[int]int
    removedIDs      []int
}

func projectSegmentIDs(ids []int, opts Options) (projection, error)
```

The helper should:

- validate `Mode`;
- reject an empty selector;
- validate input IDs are positive, unique, and sequential from `1..n`;
- validate selected IDs exist;
- apply keep/remove semantics;
- return retained input indexes in original order;
- return deterministic old-to-new ID mappings;
- return removed input IDs in original order;
- enforce `AllowEmpty`.

Keep schema-specific segment reconstruction in `Apply`, `ApplyIntermediate`,
and `ApplyMinimal`. Keep full-schema overlap group recomputation only in the
full-schema path.

### Files To Inspect

- `internal/trim/apply.go`
- `internal/trim/apply_test.go`
- `internal/cli/trim_test.go`
- `schema/output.go`

### Implementation Steps

1. Add the private projection helper and small supporting types.
2. Refactor `Apply` to call the helper, reconstruct full segments from retained
   indexes, renumber IDs, clear old overlap group IDs, and recompute overlap
   groups.
3. Refactor `ApplyIntermediate` to call the helper and reconstruct intermediate
   segments.
4. Refactor `ApplyMinimal` to call the helper and reconstruct minimal segments.
5. Preserve existing error messages from `validateMode`, empty selectors,
   invalid input IDs, missing selected IDs, and empty output.
6. Add a focused table test proving the same selector policy is applied to all
   three schema shapes.

### Acceptance Criteria

- Keep/remove behavior is unchanged for all supported artifact schemas.
- Segment IDs are still reassigned sequentially from `1`.
- Full-schema trim still recomputes overlap groups.
- Intermediate and minimal trim still do not create overlap groups.
- The common selector and ID policy exists in one helper.

### Validation Commands

```sh
go test ./internal/trim ./internal/cli
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 3: Move Trim Orchestration Into `internal/trim`

### Goal

Make `trim` match the repository's adapter boundary: CLI parses flags and
delegates; artifact-level orchestration lives in `internal/trim`.

### Decision

Add `Run(ctx context.Context, cfg config.TrimConfig) error` to `internal/trim`.

Move from `internal/cli/trim.go` into `internal/trim`:

- selector parsing from validated config;
- input file read;
- artifact parse;
- `ApplyArtifact`;
- output schema selection and conversion;
- output validation;
- transcript output JSON write;
- trim audit payload construction;
- report event construction;
- old-to-new ID mapping ordering.

Keep in `internal/cli/trim.go`:

- Cobra command definition;
- flag registration;
- `cmd.Flags().Changed("output-schema")` handling;
- `config.NewTrimConfig`;
- delegation to `trim.Run`.

Temporarily keep any moved JSON writer local to `internal/trim` if Stage 4 has
not yet been implemented. Stage 4 will centralize file writing.

### Files To Inspect

- `internal/cli/trim.go`
- `internal/cli/trim_test.go`
- `internal/trim/*.go`
- `internal/trim/*_test.go`
- `internal/config/config.go`
- `internal/report/report.go`
- `internal/normalize/normalize.go`

### Implementation Steps

1. Create `internal/trim/run.go` or another appropriately named file.
2. Move trim audit structs to `internal/trim`. Keep JSON field names unchanged.
3. Add `Run(ctx, cfg)` and check `ctx.Err()` before doing work.
4. Move helper functions needed only by trim orchestration, including ordered
   ID mapping.
5. Update `internal/cli/trim.go` to delegate to `trim.Run`.
6. Add direct `internal/trim` tests for service-level behavior if existing CLI
   tests do not cover moved report/output behavior clearly.
7. Keep all existing trim CLI tests passing.

### Acceptance Criteria

- `internal/cli/trim.go` contains flag wiring, config construction, and a call
  to `trim.Run`; it no longer performs artifact I/O or report construction.
- Trim output files and report files are unchanged for existing tests.
- Trim error wrapping is not weakened for user-facing file, parse, validation,
  or report-write failures.
- No merge pipeline modules are invoked by trim.

### Validation Commands

```sh
go test ./internal/trim ./internal/cli
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 4: Centralize Deterministic JSON File Writing

### Goal

Remove repeated `os.Create` plus indented `json.Encoder` boilerplate while
preserving current JSON formatting.

### Decision

Create a small package named `internal/jsonfile`.

Recommended API:

```go
package jsonfile

func Write(path string, value any) error
```

The helper should:

- create or truncate the target path with `os.Create`;
- encode JSON with `encoder.SetIndent("", "  ")`;
- preserve the current trailing newline produced by `Encoder.Encode`;
- return underlying create/encode/close errors with enough context for callers
  to wrap where they already wrap.

Do not implement atomic writes, temporary files, directory creation, lock files,
or fsync behavior in this stage.

### Files To Inspect

- `internal/builtin/output.go`
- `internal/report/report.go`
- `internal/normalize/normalize.go`
- `internal/trim/run.go` or `internal/cli/trim.go`, depending on Stage 3 state
- Existing CLI tests that read output JSON

### Implementation Steps

1. Add `internal/jsonfile/jsonfile.go`.
2. Add a focused `internal/jsonfile/jsonfile_test.go` covering two-space
   indentation and valid JSON.
3. Update merge JSON output writer to use `jsonfile.Write`.
4. Update normalize output writing to use `jsonfile.Write`, preserving
   user-facing error context.
5. Update trim output writing to use `jsonfile.Write`.
6. Update `report.WriteJSON` to use `jsonfile.Write`.
7. Remove duplicated local `writeOutputJSON` helpers.

### Acceptance Criteria

- Merge, trim, normalize, and report JSON are still pretty-printed with two
  spaces and a trailing newline.
- There is one implementation of deterministic JSON file writing.
- Report construction remains in `internal/report` or command packages; only
  file-writing mechanics are shared.
- No new persistence semantics are introduced.

### Validation Commands

```sh
go test ./internal/jsonfile ./internal/builtin ./internal/report ./internal/normalize ./internal/trim ./internal/cli
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 5: Clean Up Config Path Validation

### Goal

Reduce repeated path validation in single-input command config without changing
config precedence or errors.

### Decision

Add private helpers in `internal/config/config.go` only. Do not create a config
builder framework.

Recommended helpers:

```go
func normalizeSingleInputFile(path string, flag string) (string, error)
func normalizeOptionalOutputPath(path string, flag string) (string, error)
```

`normalizeSingleInputFile` should preserve the current `--input-file is
required` and `requireFile` behavior.

`normalizeOptionalOutputPath` should return `""` for empty or whitespace-only
values and otherwise call the existing required output path validation.

Keep `normalizeInputFiles` separate for merge because merge accepts repeated
input files, rejects duplicates, and sorts paths.

### Files To Inspect

- `internal/config/config.go`
- `internal/config/config_test.go`
- `internal/cli/trim_test.go`
- `internal/cli/normalize_test.go`

### Implementation Steps

1. Add private path helpers.
2. Refactor `NewTrimConfig` to use them.
3. Refactor `NewNormalizeConfig` to use them.
4. Keep `NewMergeConfig` behavior unchanged except where it can reuse existing
   `normalizeOutputPath`.
5. Preserve all current error message substrings tested by config and CLI
   tests.

### Acceptance Criteria

- Trim and normalize single-input path normalization is implemented once.
- Optional report path handling is implemented once.
- Merge multi-input normalization remains explicit and unchanged.
- No config struct fields, env vars, flags, defaults, or precedence rules
  change.

### Validation Commands

```sh
go test ./internal/config ./internal/cli
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 6: Add Narrow CLI Flag Helpers

### Goal

Reduce typo-prone repeated Cobra flag wiring while keeping command files
explicit and readable.

### Decision

Add helper functions in `internal/cli/flags.go`. Helpers should take a
`*cobra.Command` and a target pointer. This avoids adding a direct import of
`pflag` unless it becomes clearly cleaner.

Recommended helpers:

- `addOutputFileFlag(cmd *cobra.Command, target *string)`
- `addReportFileFlag(cmd *cobra.Command, target *string)`
- `addOutputModulesFlag(cmd *cobra.Command, target *string)`
- `addMergeOutputSchemaFlag(cmd *cobra.Command, target *string)`
- `addNormalizeOutputSchemaFlag(cmd *cobra.Command, target *string)`
- `addTrimOutputSchemaFlag(cmd *cobra.Command, target *string)`

Do not create one generic output schema helper because trim has different
omitted-flag semantics and help text.

For `--input-file`, prefer keeping command-local definitions unless a helper
improves clarity:

- merge uses `StringArrayVar`;
- trim and normalize use `StringVar`;
- usage text differs.

### Files To Inspect

- `internal/cli/merge.go`
- `internal/cli/trim.go`
- `internal/cli/normalize.go`
- `internal/cli/*_test.go`
- `docs/cli.md`, only to verify flag text if behavior or help text changes

### Implementation Steps

1. Add `internal/cli/flags.go`.
2. Move only identical or intentionally paired flag definitions into helpers.
3. Keep command-specific flags and semantic differences local.
4. Run command help manually and compare the important flag names/defaults.
5. Update tests only if they assert help text that intentionally changed.

### Acceptance Criteria

- Public flag names and defaults are unchanged.
- Help text remains at least as accurate as before.
- Command files remain easy to read.
- No command factory or shared command runner is introduced.

### Validation Commands

```sh
go test ./internal/cli
go run ./cmd/seriatim --help
go run ./cmd/seriatim merge --help
go run ./cmd/seriatim trim --help
go run ./cmd/seriatim normalize --help
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 7: Centralize Common Segment Reference Formatting

### Goal

Use one helper for the shared "best available segment reference" policy used by
overlap and coalesce logic.

### Decision

Add a small helper in `internal/model`, because the helper operates on
`model.Segment` and expresses domain reference semantics.

Recommended API:

```go
func SegmentReference(segment Segment) string
```

The helper should:

- return `fmt.Sprintf("%s#%d", segment.Source, *segment.SourceSegmentIndex)`
  when `Source` is non-empty and `SourceSegmentIndex` is non-nil;
- otherwise return `segment.SourceRef` when non-empty;
- otherwise return `""`.

Use this helper only where the existing code already has this exact fallback
policy. Keep generated references such as `word-run:%d:%d:%d`, `coalesce:%d`,
and `resolve-danglers:%d` local to the modules that create them.

### Files To Inspect

- `internal/model/model.go`
- `internal/overlap/detect.go`
- `internal/coalesce/coalesce.go`
- `internal/overlap/*_test.go`
- `internal/coalesce/*_test.go`
- `internal/cli/merge_test.go`

### Implementation Steps

1. Add `SegmentReference` and tests in `internal/model`.
2. Replace duplicated fallback logic in overlap detection.
3. Replace duplicated fallback logic in coalesce.
4. Do not change generated reference prefixes.
5. Do not change sorting behavior or derived-from ordering.

### Acceptance Criteria

- Existing provenance strings in full output remain unchanged.
- `source#index` formatting exists in one shared helper for matching semantics.
- Module-specific generated references stay module-local.

### Validation Commands

```sh
go test ./internal/model ./internal/overlap ./internal/coalesce ./internal/cli
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 8: Reduce Repetition In Schema Semantic Validation

### Goal

Use one private helper for cross-schema segment ID and timing invariants.

### Decision

Keep this helper private to `schema`. Do not use generics unless the resulting
code is clearly simpler than a small projection type.

Recommended shape:

```go
type segmentSemantics struct {
    id    int
    start float64
    end   float64
}

func validateSegmentSemantics(segments []segmentSemantics) error
```

Preserve current error text:

- `segment %d has id %d; want %d`
- `segment %d has end %.3f before start %.3f`

Keep full-schema overlap group timing validation separate.

### Files To Inspect

- `schema/output.go`
- `schema/output_test.go`
- callers in `internal/artifact`, `internal/trim`, and `internal/normalize`

### Implementation Steps

1. Add the private segment semantic helper.
2. Adapt full, intermediate, and minimal validation functions to project their
   segment shapes into the helper.
3. Keep full overlap group semantic validation in the full validation path.
4. Add or preserve tests that cover invalid IDs and invalid timing for each
   public schema shape.

### Acceptance Criteria

- Sequential ID and timing rules are implemented once.
- Public schema validation behavior and error wording remain stable.
- Full overlap group validation remains full-schema only.

### Validation Commands

```sh
go test ./schema ./internal/artifact ./internal/trim ./internal/normalize ./internal/cli
```

### Prompt Size

Small enough for one implementation prompt.

## Stage 9: Test Helper Cleanup

### Goal

Reduce repeated test setup after behavior-preserving refactors are complete.

### Decision

Keep test helpers package-local. Do not add exported test utility packages.

Prioritize helpers that reduce noise without hiding public command behavior:

- config option builders in `internal/config/config_test.go`;
- command execution helpers in `internal/cli` if duplication remains after
  trim orchestration and flag helper cleanup;
- fixture builders for trim schema shapes if they stay repetitive.

Do not abstract command arguments so far that tests no longer show the public
CLI contract being exercised.

### Files To Inspect

- `internal/config/config_test.go`
- `internal/cli/merge_test.go`
- `internal/cli/trim_test.go`
- `internal/cli/normalize_test.go`
- `internal/trim/*_test.go`

### Implementation Steps

1. Identify repeated setup that survived prior stages.
2. Add package-local helper builders for config options with valid defaults.
3. Consolidate duplicate read/write JSON helpers only inside the test package
   where they are used.
4. Keep high-signal command arguments inline in CLI tests.
5. Avoid golden-file rewrites or fixture churn unrelated to cleanup.

### Acceptance Criteria

- Tests are shorter where setup was noisy.
- Public behavior being tested remains obvious.
- No production code changes are made in this stage unless a test-only cleanup
  reveals dead production code from earlier stages.

### Validation Commands

```sh
go test ./internal/config ./internal/cli ./internal/trim
```

### Prompt Size

One implementation prompt if limited to config/CLI/trim tests. Split by package
if the diff becomes large.

## Stage 10: Final Dead-Code And Documentation Sweep

### Goal

Remove stale helpers and update internal documentation only where implemented
cleanup changed current boundaries.

### Decision

This is a cleanup verification stage, not a feature stage.

Do not delete `samples/`, move examples, redesign docs, or alter user-facing
references unless directly required by implemented cleanup.

### Files To Inspect

- All files touched by Stages 1-9
- `docs/policy/architecture.md`
- `docs/policy/development.md`
- `docs/internal/artifacts.md`
- `docs/internal/pipeline.md`
- `docs/internal/modules.md`
- `docs/cli.md`
- `docs/config.md`

### Implementation Steps

1. Search for obsolete helpers, duplicate schema constants, duplicate JSON
   writers, old trim CLI orchestration helpers, and unused imports.
2. Run `go test ./...`.
3. Run command help checks.
4. Update internal docs only if package responsibility changed in a way current
   docs now describe inaccurately.
5. Do not add planned cleanup notes outside `docs/roadmap/`.

### Acceptance Criteria

- No duplicate output schema string constants remain outside canonical aliases
  and intentional docs/test fixtures.
- No duplicate production `writeOutputJSON` helper remains.
- `internal/cli/trim.go` is a thin adapter.
- Docs outside `docs/roadmap/` describe only implemented behavior.
- Full repository tests pass.

### Validation Commands

```sh
go test ./...
go run ./cmd/seriatim --help
go run ./cmd/seriatim merge --help
go run ./cmd/seriatim trim --help
go run ./cmd/seriatim normalize --help
rg -n "SchemaMinimal|SchemaIntermediate|SchemaFull|writeOutputJSON|seriatim-minimal|seriatim-intermediate|seriatim-full" internal schema
```

The `rg` command is a review aid. Raw schema strings may still be appropriate in
canonical constants, docs, JSON Schema files, and tests that assert serialized
public contracts.

### Prompt Size

Small enough for one implementation prompt after prior stages are complete.

## Deferred Or Explicitly Avoided Work

Do not implement these as part of the pre-1.0 cleanup sequence unless a later
audit identifies a concrete bug or larger duplication pattern:

- A generic workflow engine for merge, trim, and normalize.
- Dynamic plugins or runtime-extensible schemas.
- A manifest, checkpoint, resume, dry-run, force, or progress framework.
- Atomic output writes or fsync semantics.
- A broad filesystem adapter layer.
- A combined backchannel/filler classifier framework.
- A generic registry resolver that erases stage-specific error messages.
- A broad public API for internals.
- Destructive changes to `samples/`.

Backchannel and filler share mechanics, but the cleanup decision is to defer
shared category-tagging extraction before 1.0. The current duplication is small,
domain-specific, and well isolated.

## Full Cleanup Validation

After all implemented stages:

```sh
go test ./...
go run ./cmd/seriatim --help
go run ./cmd/seriatim merge --help
go run ./cmd/seriatim trim --help
go run ./cmd/seriatim normalize --help
```

Manual review checklist:

- Public CLI behavior is unchanged.
- Public output schemas and JSON shapes are unchanged.
- Report event ordering and text remain stable unless tests were deliberately
  updated.
- Trim, normalize, and merge boundaries match `docs/policy/architecture.md`.
- No planned or aspirational behavior was added to non-roadmap docs.
- Cleanup reduced duplicated policy without introducing broad abstractions.

## Open Questions

No blocking questions are required to implement this plan.

If a future implementation stage discovers that preserving current behavior
conflicts with one of the structural decisions above, stop that stage and ask
for direction before changing public CLI behavior, public JSON contracts, or
report semantics.