Added a staged roadmap to address the issues identified by the code audit
This commit is contained in:
@@ -197,6 +197,7 @@ free of secrets or private transcript data.
|
||||
|
||||
- Keep core/domain logic separate from CLI, config, filesystem, reporting, and
|
||||
other adapter concerns.
|
||||
- Centralize default configuration values as constants defined in internal/config/config.go.
|
||||
- Keep modules narrowly scoped, explicitly configured, and composable by
|
||||
registry name.
|
||||
- Preserve deterministic ordering, final segment ID assignment, and schema
|
||||
|
||||
@@ -52,6 +52,7 @@ Current toolchain note:
|
||||
- Keep validation in `internal/config` and package-specific validators.
|
||||
- Return errors from deep logic; do not print inside internal packages.
|
||||
- Preserve clear package boundaries between adapters and domain transforms.
|
||||
- Define configuration defaults as constants in internal/config/config.go.
|
||||
|
||||
## Dependency policy
|
||||
|
||||
|
||||
784
docs/roadmap/cleanup.md
Normal file
784
docs/roadmap/cleanup.md
Normal file
@@ -0,0 +1,784 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user