463 lines
25 KiB
Markdown
463 lines
25 KiB
Markdown
# Workstream 6 Implementation Plan — Per-Artifact Analyze Checkpointing
|
||
|
||
## Status
|
||
|
||
Planning document for Workstream 6, **Per-artifact analyze checkpointing and
|
||
recovery**, in [`refactoring.md`](refactoring.md). Workstreams 1 through 5 are
|
||
complete and are prerequisites for this plan. All stages below are pending and
|
||
must be implemented in numerical order.
|
||
|
||
The purpose of this workstream is to make analyze a deterministic resumable
|
||
artifact queue backed only by the session manifest. If artifacts A and B
|
||
succeed, C fails, and D is not attempted, A and B must remain reusable, C must
|
||
be retried, and D must follow after C succeeds. The implementation must remove
|
||
the current end-of-stage analyze projection and rollback machinery rather than
|
||
building checkpointing alongside it as a second durable model.
|
||
|
||
## Authoritative Scope And Decisions
|
||
|
||
The authoritative product and architecture scope is Workstream 6, the
|
||
dependency relationships, compatibility intent, and consolidated target state
|
||
in [`refactoring.md`](refactoring.md). Supporting decisions are H04, H14, H15,
|
||
H36, H37, C02, and C03 in [`audit-decisions.md`](audit-decisions.md), with the
|
||
detailed rationale in H36 and H37 of [`audit.md`](audit.md) and Finding 3 of
|
||
[`contract-audit.md`](contract-audit.md).
|
||
|
||
The following decisions are fixed for this implementation:
|
||
|
||
- Keep analyze as one fixed pipeline stage in `internal/stage`; per-artifact
|
||
work does not become dynamic pipeline stages or a generic workflow engine.
|
||
- Keep the session manifest's `analyze_artifacts` collection as the only durable
|
||
per-artifact authority. Do not introduce an invocation collection, journal,
|
||
sidecar, database, or replacement run manifest.
|
||
- Use the existing `stage.Env.ManifestStore` and the canonical session-manifest
|
||
path for checkpoints. Production execution already supplies that store and
|
||
holds the session lock for the entire `Stage.Run` call. Do not add a callback
|
||
framework or expose application lifecycle types to `internal/stage`.
|
||
- Checkpoint a copy-on-write candidate manifest. Validate and save the candidate
|
||
before installing its analyze collection into the live in-memory manifest.
|
||
A failed current-record checkpoint must never make newly materialized bytes
|
||
authoritative through a later terminal save.
|
||
- Persist the reconciled starting collection before the first adapter call when
|
||
it changes durable authority. This removes obsolete records and stales
|
||
scheduled or invalid current records before replacement work begins.
|
||
- For every successful artifact: validate and materialize its canonical output,
|
||
construct and validate its current record, update the runtime catalog, stale
|
||
transitive dependents when output identity changed, checkpoint the complete
|
||
session collection, and only then attempt the next artifact.
|
||
- For an artifact failure: retain earlier successful checkpoints, replace the
|
||
active artifact with a bounded failed record, stale its current transitive
|
||
dependents, and expose that collection on the live manifest. The application
|
||
runner remains responsible for marking aggregate analyze failed, invalidating
|
||
publish/notify, and atomically persisting the terminal manifest before the
|
||
command returns the error.
|
||
- If persistence of a successful-artifact checkpoint fails, stop immediately.
|
||
Keep the last durable collection, mark the active artifact failed in the live
|
||
collection without promoting its incidental output, and let the runner's
|
||
terminal failure save make that retry state durable when storage permits.
|
||
- Preserve the existing deterministic work planner, reconciliation,
|
||
fingerprints, evidence validation, dependency order, and current-output
|
||
identity comparison. File presence alone never creates authority.
|
||
- Preserve artifact selection and force behavior. With no `--artifacts`, the
|
||
effective configured set is targeted. With `--artifacts`, only the named
|
||
targets and their required dependency closure participate. Force reruns
|
||
explicit targets; a valid current prerequisite is reused unless it is itself
|
||
explicit or becomes non-current through dependency work. Unrelated valid
|
||
records are retained.
|
||
- On aggregate success, return ordinary `StageResult.Outputs` rebuilt in
|
||
deterministic normalized artifact-key order from all current session analyze
|
||
records. Per-artifact records retain their own logs, generated configuration
|
||
references, bounded Scriptorium provenance, and evidence; aggregate logs and
|
||
metadata continue to describe the current invocation.
|
||
- On aggregate failure, aggregate outputs remain empty. Earlier current
|
||
per-artifact records remain available to a later analyze run, but publish and
|
||
notify are invalidated by the existing application-stage dependency relation.
|
||
- A crash or cancellation after a successful checkpoint may leave aggregate
|
||
analyze `running` or later normalized to `interrupted`; the next ordinary run
|
||
must reconcile the durable collection, reuse valid completed artifacts, and
|
||
continue with the first failed, stale, or missing artifact.
|
||
- Keep the manifest schema and analyze-state contract version unchanged. This
|
||
work changes persistence timing and removes an internal projection transport;
|
||
it does not introduce a new serialized representation.
|
||
- Do not enter Workstream 7 configuration decomposition, Workstream 8 publish
|
||
locks/decomposition, or Workstream 9 cleanup-stage work.
|
||
|
||
## Instructions For Every Stage
|
||
|
||
Before each stage:
|
||
|
||
1. Read `docs/development.md`, every document under `docs/policy/`, this plan,
|
||
and Workstream 6 in [`refactoring.md`](refactoring.md). Read the decision
|
||
entries and audit sections named above; do not re-audit unrelated findings.
|
||
2. Read [`../internal/stage-analyze.md`](../internal/stage-analyze.md) and
|
||
[`../internal/manifest.md`](../internal/manifest.md), then inspect the
|
||
production files and focused tests named by the stage.
|
||
3. Prefer the codebase knowledge graph for Go symbol discovery and call tracing.
|
||
Use text search for serialized fields, documentation, fixtures, and removal
|
||
of projection terminology.
|
||
4. Confirm the worktree state and preserve unrelated changes. Do not begin a
|
||
later workstream merely because adjacent code is visible.
|
||
|
||
During every stage:
|
||
|
||
- Keep the repository compiling and focused tests passing. A temporary private
|
||
delegation or projection bridge is permitted only where the next numbered
|
||
stage explicitly removes it; do not leave two durable authorities active.
|
||
- Preserve package ownership and acyclic dependency direction. Keep new types
|
||
private and cohesive; do not add public extension points.
|
||
- Keep all tests deterministic, offline, and independent of real Scriptorium or
|
||
paid APIs. Prefer the existing fake runner, real temporary filesystem, and a
|
||
small recording/failing manifest store only where checkpoint interaction is
|
||
itself the contract.
|
||
- Assert persisted manifest state, adapter calls, dependency order, and retry
|
||
behavior. Do not pin private helper choreography, complete error wording, or
|
||
incidental timestamps.
|
||
- Preserve bounded, non-secret errors and diagnostics. Never place prompt or
|
||
transcript content, variables containing secrets, or raw credentials in new
|
||
manifest fields or error assertions.
|
||
- Use `gofmt` on changed Go files, run the focused tests listed for the stage,
|
||
and run `git diff --check`. Mark a stage `Completed` only after its exit
|
||
criteria pass.
|
||
|
||
## Stage 1 — Decompose The Single-Artifact Transaction
|
||
|
||
**Status:** Completed
|
||
|
||
### Goal
|
||
|
||
Make one artifact execution understandable and independently reviewable before
|
||
changing its persistence timing. This stage is behavior-preserving.
|
||
|
||
### Required Work
|
||
|
||
1. Refactor `executeAnalyzeArtifact` in `internal/stage/analyze.go` into one
|
||
short transaction owner plus private phases with explicit intermediate
|
||
values. Keep the existing `analyzeExecutionContext` and artifact plan, or
|
||
replace them with equally narrow private values; do not restore the former
|
||
broad parameter list.
|
||
2. Separate these concerns without changing their order or contracts:
|
||
- input identity resolution, omitted optional inputs, reused-current input
|
||
reporting, Scriptorium variables, timeout, canonical/run-local paths, and
|
||
request/diagnostic path preparation;
|
||
- optional render-debug invocation and validation;
|
||
- Scriptorium artifact invocation and adapter failure adaptation;
|
||
- authoritative output-path selection, bounded non-empty regular-file
|
||
validation, output-contract construction, canonical materialization, and
|
||
installed-checksum verification; and
|
||
- result/evidence construction, including provenance, logs, generated
|
||
configuration paths, metadata, output size, and reused artifacts.
|
||
3. Place the extracted code in focused analyze-owned files when that improves
|
||
navigation. Suggested responsibilities are preparation, adapter invocation,
|
||
and validated materialization/evidence; exact private filenames are not a
|
||
contract.
|
||
4. Keep Scriptorium request and response types inside the stage/adapter boundary.
|
||
Do not move Narratio artifact, manifest, path, or metadata policy into the
|
||
Scriptorium adapter.
|
||
5. Preserve the current run-local-before-canonical materialization boundary,
|
||
render-debug behavior, useful error context, workspace modes, centralized
|
||
size limit, and checksum comparison. Do not weaken validation or add a new
|
||
write path while moving code.
|
||
6. Retain existing behavior tests at their stable boundaries. Consolidate only
|
||
tests that become purely coupled to moved private helpers; do not add one
|
||
test per extracted function.
|
||
|
||
### Exit Criteria
|
||
|
||
- `executeAnalyzeArtifact` reads as a short ordered transaction over explicit
|
||
phases and no longer mixes all preparation, invocation, materialization, and
|
||
evidence construction in one large function.
|
||
- Existing analyze request, render, validation, materialization, provenance,
|
||
and failure behavior is unchanged.
|
||
- `go test ./internal/stage ./internal/adapters/scriptorium` succeeds.
|
||
- `git diff --check` succeeds.
|
||
|
||
## Stage 2 — Establish Session-Only Analyze Execution State
|
||
|
||
**Status:** Completed
|
||
|
||
### Goal
|
||
|
||
Replace `analyzeStage.Run`'s parallel local collections with one private
|
||
session-oriented execution state that is ready to checkpoint, while retaining
|
||
the existing end-of-stage persistence boundary temporarily.
|
||
|
||
### Required Work
|
||
|
||
1. Introduce a private analyze execution state/accumulator that owns:
|
||
- the shared execution context and deterministic work plan;
|
||
- the complete reconciled session artifact collection;
|
||
- prior current records needed for output-identity comparison;
|
||
- the evaluated/scheduled key set used for dependent invalidation;
|
||
- the mutable runtime artifact catalog; and
|
||
- invocation-only aggregate logs, generated configurations, generated
|
||
metadata, reused-artifact metadata, and its deduplication set.
|
||
2. Keep setup, reconciliation, work planning, adapter-availability validation,
|
||
deterministic iteration, and final `StageResult` assembly visible in
|
||
`analyzeStage.Run`. Move per-item state mutation and diagnostic collection
|
||
behind narrow methods on the private state; do not turn the accumulator into
|
||
a service locator.
|
||
3. Rename `analyzeWorkPlan.ProjectedRecords`, `projectedAnalyzeRecords`, and
|
||
related private terminology to describe the reconciled starting session
|
||
collection. It is the candidate durable state, not an invocation projection.
|
||
Preserve the planner's read-only behavior and existing classification.
|
||
4. Make the accumulator install its complete collection into the passed session
|
||
manifest on success and its failed/staled collection on error. During this
|
||
stage, continue returning the existing `AnalyzeStateProjection` as a
|
||
temporary compatibility bridge for the application runner; both views must
|
||
be derived from the same collection, and no additional save is introduced.
|
||
5. Add optional `ProducerRunID` provenance to `artifacts.Ref`. Update the
|
||
generic application result mapper to use that value when it is non-empty and
|
||
otherwise retain the current behavior of assigning the active run ID. This
|
||
is data needed by ordinary stage-result transport, not an analyze-specific
|
||
branch; cover both explicit and fallback behavior in the mapper tests.
|
||
6. Add a private helper that deterministically converts all current analyze
|
||
records into ordinary `artifacts.Ref` outputs. Preserve source ID, relative
|
||
path, contract, checksum, and producer run ID. Include valid current records
|
||
retained outside a partial selection; do not include stale, missing, or
|
||
failed records. Continue using the projection path for aggregate outputs
|
||
until Stage 3 switches ownership.
|
||
7. Rewrite stage tests to inspect the manifest collection mutated by the real
|
||
analyze stage rather than manually installing returned projections. Retain a
|
||
minimal assertion that the temporary projection matches that collection so
|
||
the transition cannot silently diverge.
|
||
|
||
### Exit Criteria
|
||
|
||
- `analyzeStage.Run` coordinates one private session collection rather than
|
||
parallel record and diagnostic variables.
|
||
- Planning/reconciliation remains read-only, execution order and force/selection
|
||
behavior are unchanged, and stage tests primarily assert manifest state.
|
||
- Ordinary `StageResult.Outputs` can preserve a reused artifact's original
|
||
producer run identity without an analyze-specific application path.
|
||
- The existing application projection path still functions and no checkpoint
|
||
timing has changed yet.
|
||
- `go test ./internal/stage ./internal/app` succeeds.
|
||
- `git diff --check` succeeds.
|
||
|
||
## Stage 3 — Cut Analyze Over To Durable Per-Artifact Checkpoints
|
||
|
||
**Status:** Completed
|
||
|
||
### Goal
|
||
|
||
Make each validated artifact completion durable before later artifact work and
|
||
stop using the end-of-stage projection transport for real analyze execution.
|
||
|
||
### Required Work
|
||
|
||
1. Add a private analyze checkpoint operation in `internal/stage`. It must:
|
||
- require a non-nil session manifest, analyze stage record, and
|
||
`Env.ManifestStore` when analyze has state to persist;
|
||
- derive the canonical manifest path through `artifacts` helpers from the
|
||
resolved workspace, campaign, and session identity;
|
||
- validate the full candidate collection with
|
||
`manifest.ValidateAnalyzeArtifactCollection`;
|
||
- shallow-copy the manifest, clone the top-level stage map, copy the analyze
|
||
stage record, and deep-clone the candidate analyze collection so the save
|
||
cannot mutate live authority before success;
|
||
- preserve the aggregate analyze status (normally `running`) and every
|
||
unrelated manifest field;
|
||
- save the candidate through `Env.ManifestStore`; and
|
||
- only after success, install the candidate analyze version/collection and
|
||
the store-updated manifest timestamp into the live manifest.
|
||
Do not add a general manifest transaction framework or JSON round-trip clone.
|
||
2. Before invoking the first scheduled artifact, checkpoint the reconciled
|
||
starting collection when it differs from durable state. This must remove
|
||
records for removed configuration and stale any selected/forced/non-current
|
||
authority before replacement work begins. If this save fails, invoke no
|
||
adapter.
|
||
3. For each scheduled artifact, preserve this order:
|
||
- compute its execution fingerprint;
|
||
- execute the decomposed artifact transaction;
|
||
- construct and validate its current manifest record;
|
||
- prove the generated record can be added to the runtime catalog for use by
|
||
later dependents;
|
||
- stale unscheduled transitive dependents only when prior and replacement
|
||
output identities differ;
|
||
- checkpoint the complete updated session collection; and
|
||
- only then expose the result to a later artifact.
|
||
4. If a successful artifact's checkpoint fails, stop immediately and do not
|
||
call the next artifact. Build a failed record for the active artifact from
|
||
the last committed collection, with no output authority; stale its current
|
||
transitive dependents; and install that failure candidate in memory so the
|
||
runner's terminal save can persist retry state. Return the checkpoint
|
||
failure with artifact identity and manifest context.
|
||
5. Have real analyze execution return ordinary `StageResult.Outputs` built from
|
||
every current session record plus its existing aggregate diagnostics and
|
||
metadata. Stop returning `AnalyzeStateProjection` from `analyzeStage.Run`.
|
||
The legacy projection type and application handlers may remain only as dead
|
||
transitional code until Stage 4; add no new uses.
|
||
6. In `executeSelectedStage`, make a terminal save failure after any stage's
|
||
apparent success mark that stage failed and invalidate succeeded transitive
|
||
dependents before terminal persistence. This behavior currently occurs only
|
||
on the analyze-projection branch; it must remain correct when real analyze no
|
||
longer returns a projection and should be correct generically.
|
||
7. Update direct analyze fixtures so they supply a real temporary
|
||
`manifest.LocalStore`, complete campaign/run identity, an aggregate analyze
|
||
record in the state appropriate to the test, and an initially persisted
|
||
manifest. Keep unit-only planner/reconciliation tests free of persistence.
|
||
8. Add focused checkpoint tests at the stage/application boundary:
|
||
- the manifest store observes A current before B is invoked and A+B current
|
||
before C is invoked;
|
||
- a failed B checkpoint prevents C from being invoked;
|
||
- the failed B checkpoint never persists B as current merely because its
|
||
canonical file exists; and
|
||
- final aggregate success exposes outputs for all and only current records
|
||
in deterministic normalized artifact-key order, preserving each record's
|
||
producer run identity.
|
||
|
||
### Exit Criteria
|
||
|
||
- Every successful scheduled artifact is durable before the next adapter call.
|
||
- A checkpoint failure stops later work and cannot promote incidental output.
|
||
- Real analyze execution no longer relies on an end-of-stage state projection.
|
||
- Aggregate success/failure persistence and dependent invalidation remain
|
||
application-owned and correct.
|
||
- `go test ./internal/stage ./internal/app ./internal/manifest` succeeds.
|
||
- `git diff --check` succeeds.
|
||
|
||
## Stage 4 — Complete Failure Recovery And Remove Projection Machinery
|
||
|
||
**Status:** Completed
|
||
|
||
### Goal
|
||
|
||
Complete the A/B/C/D recovery contract and delete the superseded projection,
|
||
validation, application, and rollback paths.
|
||
|
||
### Required Work
|
||
|
||
1. On any artifact-specific failure—including fingerprinting, preparation,
|
||
render, adapter execution, result validation, materialization, evidence
|
||
construction, catalog admission, or checkpoint persistence—construct one
|
||
bounded failed record for the active key, clear its output authority, and
|
||
stale current transitive dependents. Preserve unrelated current records and
|
||
all earlier successfully checkpointed records.
|
||
2. Install the failed/staled collection on the live manifest before returning
|
||
the artifact error. The runner must then use its ordinary stage-error path to
|
||
mark aggregate analyze failed, clear aggregate outputs, invalidate publish
|
||
and notify, and persist the terminal session manifest. Join a terminal save
|
||
failure with the originating artifact/checkpoint error; never replace the
|
||
useful cause.
|
||
3. Ensure unattempted artifacts behave according to existing authority:
|
||
previously absent records remain absent/missing on reconciliation; unrelated
|
||
valid current records remain current; and a prior current record that
|
||
depends directly or transitively on the failed artifact becomes stale with
|
||
no output authority.
|
||
4. Remove `AnalyzeStateProjection` from `stage.StageResult`, delete
|
||
`internal/app/analyze_projection.go`, and remove all validation, cloning,
|
||
application, capture, rollback, and special output reconstruction that exists
|
||
solely for that projection. Remove fake projection stages and projection-only
|
||
tests; replace their meaningful risks with real analyze checkpoint/recovery
|
||
coverage.
|
||
5. Simplify `executeSelectedStage` to its generic result/error handling while
|
||
preserving analyze's live manifest collection across aggregate lifecycle
|
||
clearing. Do not add a type assertion, stage-name branch, callback, or new
|
||
result union to replace the projection special case.
|
||
6. Add a durable assembled-run regression for the approved scenario:
|
||
- configure deterministic A, B, C, and D work in dependency order;
|
||
- make A and B succeed, C fail, and verify D is not attempted;
|
||
- reload the manifest and verify A/B current, C failed, aggregate analyze
|
||
failed, and the correct missing/stale state for D;
|
||
- run again without force, verify only C and then D execute, and verify the
|
||
final aggregate and all selected records are current; and
|
||
- verify valid unrelated records and upstream transcript stages were never
|
||
invalidated or rerun.
|
||
7. Add focused variants for failure of the first and last artifact, cancellation
|
||
after at least one checkpoint, changed-output invalidation of an unselected
|
||
transitive dependent, and byte/contract-identical replacement preserving an
|
||
unselected dependent. Consolidate with existing incremental/failure tests so
|
||
each durable risk has one clear owner.
|
||
|
||
### Exit Criteria
|
||
|
||
- A/B remain durable after C fails; an ordinary later run retries C and then D
|
||
without invoking A/B.
|
||
- Artifact failure and checkpoint failure leave no false current authority, and
|
||
publish/notify cannot remain successfully current after aggregate failure.
|
||
- No analyze projection type, field, helper, application branch, rollback path,
|
||
test fixture, or documentation claim remains.
|
||
- `go test ./internal/stage ./internal/app ./internal/manifest ./internal/artifacts`
|
||
succeeds.
|
||
- `git diff --check` succeeds.
|
||
|
||
## Stage 5 — Verify Force Semantics, Align Documentation, And Close Workstream 6
|
||
|
||
**Status:** Pending
|
||
|
||
### Goal
|
||
|
||
Verify the complete operational contract, remove superseded test/documentation
|
||
language, and leave Workstream 6 closed with repository-wide evidence.
|
||
|
||
### Required Work
|
||
|
||
1. Review existing force/selection/reconciliation tests and retain a lean set
|
||
that proves:
|
||
- an ordinary run reuses every valid current selected artifact;
|
||
- unqualified force reruns the full effective configured target set;
|
||
- selected force reruns the explicit targets, includes their dependency
|
||
closure, reuses valid current prerequisites, and leaves unrelated valid
|
||
artifacts untouched;
|
||
- stale, failed, missing, or dependency-invalid prerequisites execute in
|
||
deterministic topological order before their dependents; and
|
||
- an incidental canonical file without current manifest evidence is never
|
||
reused.
|
||
2. Replace projection-oriented test names, helpers, and assertions with
|
||
checkpoint/recovery language. Remove redundant tests whose only purpose was
|
||
to validate the retired transport or rollback choreography. Do not reduce
|
||
coverage of output validation, dependency invalidation, cancellation,
|
||
persistence failure, or assembled resume behavior.
|
||
3. Update the canonical current-behavior owners:
|
||
- `docs/internal/stage-analyze.md` for checkpoint order, resume behavior,
|
||
failure semantics, and the absence of projections;
|
||
- `docs/internal/manifest.md` for per-artifact checkpoint authority,
|
||
aggregate status, interruption recovery, and persistence failure;
|
||
- `docs/policy/architecture.md` only where its stage/persistence description
|
||
currently implies a single end-of-stage analyze transaction; and
|
||
- `docs/operations.md` and `docs/troubleshooting.md` only where operators
|
||
need to understand reuse after partial analyze failure or recover from a
|
||
manifest checkpoint error.
|
||
Do not duplicate CLI or configuration contracts, since syntax and fields do
|
||
not change.
|
||
4. Search production, tests, and current-behavior documentation for
|
||
`AnalyzeStateProjection`, `AnalyzeState`, `analyze projection`,
|
||
`projected records`, and `end-of-stage` language. Remove or revise every
|
||
reference tied to the retired mechanism; preserve unrelated planning uses of
|
||
the ordinary English word “projection” only when accurate.
|
||
5. Run the full repository validation from `docs/development.md`:
|
||
|
||
```sh
|
||
go test ./...
|
||
go test -race ./...
|
||
go vet ./...
|
||
go build ./...
|
||
go test ./internal/doccheck
|
||
go test ./internal/config -run '^TestExamplesLoadAndValidate$'
|
||
```
|
||
|
||
6. Run `go test -race -shuffle=on -count=3 ./internal/stage ./internal/app` to
|
||
stress the changed recovery and persistence boundaries. Run the supported
|
||
release cross-build checks if an implementation change affects build-tagged
|
||
code; otherwise the repository build is sufficient for this workstream.
|
||
7. After all checks pass, mark Stages 1–5 in this document `Completed` and mark
|
||
only Workstream 6 in [`refactoring.md`](refactoring.md) `Complete`.
|
||
Workstream 7 must remain the next pending workstream.
|
||
|
||
### Exit Criteria
|
||
|
||
- Current documentation describes analyze as a session-manifest-backed,
|
||
per-artifact checkpointed queue and contains no retired projection behavior.
|
||
- The force, partial-selection, dependency, interruption, and persistence-error
|
||
cases are protected through stable behavioral boundaries.
|
||
- All repository validation and focused shuffled race checks pass.
|
||
- Workstream 6 is marked complete, Workstream 7 remains pending, and the
|
||
worktree contains no accidental generated or temporary files.
|
||
- `git diff --check` succeeds.
|
||
|
||
## Open Questions
|
||
|
||
None. The audits, approved Workstream 6 scope, existing selection contract, and
|
||
the fixed decisions above resolve the product and implementation choices needed
|
||
for the coding agent to execute this plan.
|