334 lines
17 KiB
Markdown
334 lines
17 KiB
Markdown
# Implementation Plan: Shared Stage Checkpoint Mechanics
|
|
|
|
Status: Complete.
|
|
|
|
## Scope and Authority
|
|
|
|
Implement the agreed follow-up to v2.2.0: consolidate the thin checkpoint
|
|
mechanism shared by transcribe and analyze without combining their progress
|
|
models or changing operator behavior. There is no separate consolidation
|
|
roadmap in this checkout. This plan uses the accompanying discussion as its
|
|
scope: persistence is already centralized; candidate isolation and
|
|
save-before-promotion are the duplication to remove. The completed
|
|
[transcription roadmap](transcribe-checkpoints-and-replay.md) provides background,
|
|
not authorization to reimplement transcription or HTTP replay. The preceding
|
|
completed implementation plan remains in Git history.
|
|
|
|
Complete the numbered stages in order, one bounded implementation prompt per
|
|
stage for gpt-5.6-terra. Record completion and actual verification results here.
|
|
This planning pass changes documentation only; the steps below are for the
|
|
subsequent implementation sprint.
|
|
|
|
Before each stage, read `AGENTS.md`, `docs/development.md`, and all three
|
|
documents in `docs/policy/`: architecture, documentation, and testing. Use
|
|
maintained Go 1.26. Prefer graph tools for code discovery; inspect current
|
|
files if their index is stale. Read the stage-specific preparation below,
|
|
rather than unrelated historical roadmaps.
|
|
|
|
## Target and Scope Boundaries
|
|
|
|
The existing `internal/manifest.Store` and `LocalStore` remain the session
|
|
manifest persistence interface and implementation. Encoding, schemas,
|
|
validation, normalization, and atomic file replacement remain unchanged.
|
|
There is no new store, journal, file format, dependency, command, configuration,
|
|
or migration.
|
|
|
|
One private helper in `internal/stage` owns candidate preparation, calling the
|
|
existing store, and promoting the selected stage record only after successful
|
|
save. The existing typed analyze/transcribe checkpoint methods become thin
|
|
callers. This is execution plumbing: keep it beside the callers rather than
|
|
moving orchestration into the storage package or exporting a reusable API.
|
|
|
|
Keep these responsibilities stage-owned:
|
|
|
|
- Transcribe's completed-only speaker map, evidence checks, bounded concurrency,
|
|
mutex, checkpoint-failure latch, cancellation, pre-dispatch force reset,
|
|
output installation, and aggregate repair.
|
|
- Analyze's dispositions, selection, dependencies, catalog updates, working and
|
|
committed collections, failure reconciliation, invalidation, and outputs.
|
|
- Each stage's decision whether/when to checkpoint, typed validation, contextual
|
|
errors, and post-save working-state updates.
|
|
- Runner aggregate transitions, session locking, terminal failure saves, and
|
|
fixed dependent-stage invalidation.
|
|
|
|
Do not introduce a common subtask model, scheduler, transaction manager, global
|
|
mutex, automatic retry, additional checkpoint, or batching policy. Do not
|
|
migrate publish/restore, planning clones, cleanup, or ordinary runner saves:
|
|
their operations are not the demonstrated duplication. Leave HTTP replay and
|
|
all adapters unchanged.
|
|
|
|
## Selected Design
|
|
|
|
### Shared Save-and-Promote Operation
|
|
|
|
Add `internal/stage/checkpoint.go` with a private helper shaped as:
|
|
|
|
```go
|
|
func checkpointStageCollection(
|
|
ctx context.Context,
|
|
store manifest.Store,
|
|
path string,
|
|
live *manifest.Manifest,
|
|
stageName string,
|
|
update func(*manifest.StageRecord) error,
|
|
) error
|
|
```
|
|
|
|
Names are implementation choices, not public contracts. Use a function and
|
|
small copying helpers, not a factory/options/interface framework. The two
|
|
existing typed checkpoint methods are the only production callers. They pass
|
|
the already-resolved manifest path; the helper does not discover paths or
|
|
inspect configuration.
|
|
|
|
The helper performs this sequence:
|
|
|
|
1. Check required live/store inputs without mutation. Copy a checkpoint
|
|
candidate and select a copied target record. If absent, provide an empty
|
|
candidate record to the caller, without inferring its name, status, or time.
|
|
2. Apply the supplied update to that candidate record only. On update error,
|
|
return without saving or promoting. The typed caller validates and fills
|
|
its version, collection, and stage-owned initialization/timestamps.
|
|
3. Invoke `Store.Save` once with the supplied context. Preserve error identity
|
|
through `%w` and useful path context. On error, do not promote, retry,
|
|
reload, or synthesize aggregate failure.
|
|
4. After success, install the saved target record and candidate manifest
|
|
`UpdatedAt` into live state. Preserve an existing target record's pointer
|
|
identity by assigning through it. Allocate missing live maps/records only
|
|
after success. Do not replace the entire live manifest or overwrite other
|
|
stage records. Copy mutable checkpoint collections on promotion so saved
|
|
candidates and caller working collections cannot mutate live progress.
|
|
|
|
Callers serialize the whole operation. Transcribe's current mutex does so;
|
|
analyze executes artifacts sequentially. Do not add a lock to the helper or
|
|
promise support for concurrent independent manifest writers. Atomic replacement
|
|
visibility does not become power-loss durability or a transaction covering
|
|
transcript files.
|
|
|
|
### Bounded Candidate Isolation
|
|
|
|
Factor `cloneManifestForTranscribeCheckpoint` into a shared checkpoint-scoped
|
|
copy helper in the new file. Use ordinary typed copies and existing
|
|
`manifest.CloneAnalyzeArtifactCollection` and
|
|
`manifest.CloneTranscribeSpeakerCollection`. Do not introduce reflection
|
|
cloning, JSON round trips, or a public general-purpose clone API.
|
|
|
|
Copy the manifest value, stages map, stage records, and mutable collections
|
|
touched by these updates or `LocalStore.Save`. In particular, Save normalizes
|
|
analyze collections throughout the candidate, including on a transcribe
|
|
checkpoint. Clone those collections for every stage so normalization cannot
|
|
leak into live state on failure. Preserve the existing bounded copies of
|
|
input/artifact and output/log/config slices. Retain nil entries for existing
|
|
validation to reject, rather than silently repairing invalid manifests.
|
|
|
|
This is not arbitrary deep isolation of metadata. Neither the two update
|
|
callbacks nor the current Store mutates `map[string]any` metadata, so it may
|
|
remain shared read-only. Document that boundary and inspect every current
|
|
normalization write. Use existing typed clones for nested checkpoint evidence;
|
|
do not restrict legal metadata values or alter their Go/JSON representations
|
|
to simplify copying.
|
|
|
|
### Differences to Preserve
|
|
|
|
- Analyze still requires an existing stage record. Its callback changes only
|
|
analyze version/collection, not transcribe-style initialization or timestamp
|
|
policy. It refreshes working and committed collections only after success.
|
|
- Transcribe retains missing-record initialization for direct stage calls:
|
|
its typed caller supplies name, running status, and initial timestamps when
|
|
needed. Its callback retains the existing stage `UpdatedAt` update. Copying
|
|
an existing record must not clear aggregate fields or invoke lifecycle
|
|
transitions.
|
|
- Existing transcribe live-record initialization is incomplete on the unusual
|
|
missing-record path; promoting the actual successfully saved candidate makes
|
|
its identity agree with disk. This does not add a new missing-record
|
|
rejection or change ordinary runner status transitions.
|
|
- Analyze rollback/failure handling and transcribe failure latching remain in
|
|
their execution owners. The shared operation never invents failed items.
|
|
- Preserve starting-checkpoint elision, force/reset semantics, empty versioned
|
|
collections, evidence and producer identity, legacy transcription handling,
|
|
supported zero-sized audio, and aggregate repair without HTTP calls.
|
|
- Do not freeze exact error wording, wall-clock values, copy choreography, or
|
|
iteration order into new contracts. No new schema, limits, filename rules,
|
|
statuses, or compatibility guarantees are authorized.
|
|
|
|
## Stage 1: Add the Shared Checkpoint Operation
|
|
|
|
Status: Complete.
|
|
|
|
Completion marker (2026-09-22): Added the private stage-level checkpoint
|
|
save-and-promote helper and bounded candidate copying without migrating either
|
|
production caller. Focused real-store coverage proves live isolation through
|
|
updates, validation, save-time normalization and timestamps, save failures,
|
|
missing-target creation, pointer-preserving promotion, unrelated aggregate
|
|
preservation, caller-copy isolation, context/error identity, and exact one-save
|
|
behavior. Verified with `go test ./internal/manifest ./internal/stage`,
|
|
`go test -race ./internal/stage`, and `git diff --check`.
|
|
|
|
Preparation: `internal/manifest/store.go`, `manifest.go`, `analyze_state.go`,
|
|
`transcribe_state.go`; `internal/stage/analyze_execution_state.go`,
|
|
`transcribe_execution_state.go`; `docs/internal/manifest.md`.
|
|
|
|
1. Add the private helper and bounded candidate copying described above.
|
|
Leave existing production checkpoint paths unchanged until their migration
|
|
stages. Do not duplicate manifest record-validation rules.
|
|
2. Add focused `internal/stage/checkpoint_test.go` coverage with real
|
|
`manifest.LocalStore` and temporary directories for successful save/reload.
|
|
Use a small Store wrapper only to observe/fail the persistence boundary.
|
|
3. Prove live progress remains old during Save, matches saved progress after
|
|
success, and does not change on update, validation, or save failure. Include
|
|
both typed collections and a transcribe checkpoint alongside analyze
|
|
records subject to save-time normalization. A failure test must exercise
|
|
candidate timestamp/normalization mutation before returning an error, not
|
|
merely fail before any work.
|
|
4. Prove unrelated stage/aggregate data survives, existing target references
|
|
observe updates, missing-target creation is invisible on failure, and later
|
|
caller collection edits do not mutate committed records. Preserve context
|
|
and error identity. Test the supported copy/write boundary, not arbitrary
|
|
malicious Store mutation.
|
|
|
|
Verify: `go test ./internal/manifest ./internal/stage` and
|
|
`go test -race ./internal/stage`.
|
|
|
|
Done when the shared integrity boundary is tested without changing existing
|
|
stage behavior. Temporary copying overlap is removed in Stage 3.
|
|
|
|
## Stage 2: Migrate Analyze
|
|
|
|
Status: Complete.
|
|
|
|
Completion marker (2026-09-22): Replaced analyze's local candidate-copy,
|
|
save, and promotion choreography with the shared checkpoint operation while
|
|
retaining its existing-record requirement, typed validation, path resolution,
|
|
and post-save working and committed collection refresh. Existing recovery,
|
|
selection, invalidation, and rollback coverage required no duplication.
|
|
Verified with `go test ./internal/stage ./internal/app ./internal/manifest`,
|
|
`go test -race ./internal/stage ./internal/app`, and `git diff --check`.
|
|
|
|
Preparation: `internal/stage/analyze_execution_state.go`,
|
|
`analyze_checkpoint_test.go`, `analyze_failure_test.go`,
|
|
`internal/app/analyze_checkpoint_recovery_test.go`, and
|
|
`docs/internal/stage-analyze.md`.
|
|
|
|
1. Replace manual candidate copying, saving, and live promotion in analyze
|
|
with the shared helper. Retain typed validation, required-record checking,
|
|
the typed update callback, and committed/working collection refresh.
|
|
2. Preserve starting-checkpoint elision, selected-artifact behavior, catalog
|
|
sequencing, dependent invalidation on changed output, and rollback after
|
|
checkpoint failure. Do not change the runner's terminal failure save or
|
|
joined errors.
|
|
3. Run existing tests before adding coverage. Retain tests proving earlier
|
|
artifacts survive failure/cancellation, unsaved output bytes gain no
|
|
authority, selected/forced work preserves appropriate unrelated artifacts,
|
|
and interrupted runs recover across store reloads. Add only missing
|
|
integration coverage; do not duplicate Stage 1's isolation matrix.
|
|
|
|
Verify: `go test ./internal/stage ./internal/app ./internal/manifest` and
|
|
`go test -race ./internal/stage ./internal/app`.
|
|
|
|
Done when analyze delegates persistence choreography while progress, artifact
|
|
selection, and failure behavior remain unchanged.
|
|
|
|
## Stage 3: Migrate Transcribe and Remove Duplicate Machinery
|
|
|
|
Status: Complete.
|
|
|
|
Completion marker (2026-09-22): Delegated transcription's locked checkpoint
|
|
operation to the shared helper while retaining typed validation, direct-call
|
|
record initialization, stage timestamp updates, post-save working collection
|
|
refresh, and the existing caller-owned mutex and failure latch. Removed the
|
|
duplicate production manifest clone; the read-only resume assertion now uses
|
|
the shared bounded snapshot. Existing real-store, concurrency, force/retry,
|
|
interruption, aggregate-repair, and zero-sized-audio coverage passed unchanged.
|
|
Verified with `go test ./internal/stage ./internal/app ./internal/manifest`,
|
|
`go test -race ./internal/stage ./internal/app`, and `git diff --check`.
|
|
|
|
Preparation: `internal/stage/transcribe_execution_state.go`, `transcribe.go`,
|
|
`transcribe_resume.go`, `transcribe_checkpoint_test.go`,
|
|
`transcribe_reconciliation_test.go`,
|
|
`internal/app/transcribe_checkpoint_recovery_test.go`, and
|
|
`docs/internal/stage-transcribe.md`.
|
|
|
|
1. Delegate `checkpointCollectionLocked` to the shared helper, retaining
|
|
typed initialization/update and successful working-collection refresh.
|
|
Keep the existing mutex across candidate creation, save, and promotion.
|
|
Do not move HTTP/hashing work into that critical section or add a lock.
|
|
2. Preserve failure-latch checks and worker joining, the parent versus worker
|
|
context distinction, force's durable empty starting collection, and
|
|
ordinary retry's reuse of committed forced replacements.
|
|
3. Remove `cloneManifestForTranscribeCheckpoint`. Its current use by a
|
|
read-only resume test is not a reason to retain duplicate production code.
|
|
Use an independent test snapshot or the shared bounded copy where sufficient,
|
|
without exporting a helper for tests.
|
|
4. Retain existing real-store and concurrency coverage: no lost completions;
|
|
pre-dispatch save failure prevents requests/canonical writes; later failure
|
|
retains prior authority; interrupted and aggregate-save recovery; failed
|
|
force followed by retry; run-directory independence; zero-sized audio;
|
|
removed/mismatched aggregate repair without additional WhisperX requests
|
|
or removed-speaker merge input. Do not weaken these regression assertions.
|
|
|
|
Verify: `go test ./internal/stage ./internal/app ./internal/manifest` and
|
|
`go test -race ./internal/stage ./internal/app`.
|
|
|
|
Done when both stages use one save/promote implementation and one bounded
|
|
copy implementation, with their different models and synchronization intact.
|
|
|
|
## Stage 4: Document, Review Scope, and Validate
|
|
|
|
Status: Complete.
|
|
|
|
Completion marker (2026-09-22): Documented the private shared checkpoint
|
|
persistence boundary and linked the stage guides back to it while retaining
|
|
their separate analyze and transcribe progress policies. The refactor removed
|
|
the two local candidate/save/promotion sequences and the transcription-only
|
|
bounded clone; the retained thin wrappers own typed validation, updates,
|
|
working-state refresh, and their existing serialization and failure behavior.
|
|
The collective test review retained focused helper integrity coverage plus
|
|
stage/application recovery tests, without duplicating the helper's isolation
|
|
matrix. A cumulative scope audit found no schema, validation-policy,
|
|
scheduling, force, concurrency, output-selection, stage-order, adapter-call,
|
|
configuration, or public-interface change.
|
|
|
|
Verified with `go test ./...`, `go test -race ./...`, `go vet ./...`,
|
|
`go build ./...`, `go test ./internal/doccheck`,
|
|
`go test ./internal/config -run '^TestExamplesLoadAndValidate$'`, and
|
|
`git diff --check`; all passed.
|
|
|
|
Preparation: `docs/internal/manifest.md`, `docs/internal/stage-transcribe.md`,
|
|
`docs/internal/stage-analyze.md`, documentation/testing policies, and the
|
|
implementation diff against this plan's starting commit.
|
|
|
|
1. Update manifest internals to identify the shared checkpoint owner and copy/
|
|
save/promote boundary. Stage guides retain stage-specific policy and link
|
|
to that explanation instead of duplicating shared implementation details.
|
|
Do not rewrite immutable release notes or announce new operator behavior.
|
|
2. Assess tests collectively. Consolidate genuinely duplicated persistence
|
|
mechanics coverage, but retain stage/application recovery tests proving
|
|
correct integration. Avoid test-count/LOC quotas. Record a brief summary
|
|
here of duplicated mechanisms removed and thin wrappers retained.
|
|
3. Confirm no schema, validation policy, scheduling, force, concurrency, output
|
|
selection, stage order, adapter call, or configuration change slipped into
|
|
the refactor. Additional restrictions require a separate decision, not
|
|
tests that retrospectively declare new policy.
|
|
4. Run all gates below and record actual results. Mark this plan complete only
|
|
when both migrations and verification pass. Do not release, tag, push, delete
|
|
other roadmaps, or expand into a repository-wide cloning refactor.
|
|
|
|
```sh
|
|
go test ./...
|
|
go test -race ./...
|
|
go vet ./...
|
|
go build ./...
|
|
go test ./internal/doccheck
|
|
go test ./internal/config -run '^TestExamplesLoadAndValidate$'
|
|
git diff --check
|
|
```
|
|
|
|
Done when duplication is removed, recovery remains covered, documentation
|
|
matches the implementation, and all gates pass.
|
|
|
|
## Open Questions
|
|
|
|
None requiring a product decision. A private stage-level helper is selected
|
|
because persistence already has an owner and the duplication is execution
|
|
plumbing. Model unification, library extraction, generalized transactions, and
|
|
broader cloning cleanup remain out of scope.
|