Update implementation plan to address additional clean-up items
This commit is contained in:
@@ -2,13 +2,14 @@
|
||||
|
||||
## Status
|
||||
|
||||
The domain-typed pipeline was implemented on 2026-07-17. A subsequent review
|
||||
identified five follow-up issues. The stages below are the decision-complete
|
||||
implementation plan for resolving them.
|
||||
The domain-typed pipeline and the first five remediation stages were completed
|
||||
on 2026-07-17. A subsequent review identified three additional implementation
|
||||
issues. Stages 1 through 5 are retained as the completed implementation record;
|
||||
Stages 6 through 8 are the decision-complete plan for the remaining work.
|
||||
|
||||
Implement the stages in order. Keep each stage independently reviewable and
|
||||
leave the repository passing its full validation suite before beginning the
|
||||
next stage. Unless a stage explicitly says otherwise, preserve public CLI,
|
||||
Implement the pending stages in order. Keep each stage independently reviewable
|
||||
and leave the repository passing its full validation suite before beginning
|
||||
the next stage. Unless a stage explicitly says otherwise, preserve public CLI,
|
||||
configuration, module-key, validator-key, durable output, and checkpoint file
|
||||
contracts.
|
||||
|
||||
@@ -34,9 +35,12 @@ The existing implementation already provides:
|
||||
- bounded chunk-first, lane-second extraction with deterministic public
|
||||
outcomes.
|
||||
|
||||
The following work corrects checkpoint identity, completes retry debugging,
|
||||
removes kind-ambiguous registry lookup, strengthens architectural enforcement,
|
||||
and removes the obsolete sequential extraction path.
|
||||
Stages 1 through 5 corrected checkpoint identity, completed merge and normalize
|
||||
retry debugging, removed kind-ambiguous registry lookup, strengthened the
|
||||
initial architectural guard, and removed the obsolete sequential extraction
|
||||
path. Stages 6 through 8 complete candidate-artifact handling, make attempt
|
||||
debugging comprehensive across all stages, and close the remaining import-guard
|
||||
loopholes.
|
||||
|
||||
## Implementation Rules
|
||||
|
||||
@@ -65,6 +69,11 @@ Apply these rules to every stage:
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Completed Remediation Record
|
||||
|
||||
The following five stages have been implemented and are retained to document
|
||||
the decisions and completion criteria that produced the current baseline.
|
||||
|
||||
## Stage 1: Include Resolved Validator Policy In Pipeline Identity
|
||||
|
||||
### Goal
|
||||
@@ -430,18 +439,278 @@ The concurrent coordinator must be the only code path that executes extraction,
|
||||
and lane continuation must consume finalized extraction state without routing
|
||||
it through a synthetic checkpoint loader.
|
||||
|
||||
## Pending Remediation Plan
|
||||
|
||||
Implement Stages 6 through 8 in order. Do not perform final roadmap closeout
|
||||
until all three stages and their documentation changes have landed.
|
||||
|
||||
## Stage 6: Validate Merge And Normalize Candidates Before Final Encoding
|
||||
|
||||
### Goal
|
||||
|
||||
Preserve the two-zone artifact model at merge and normalize boundaries: a
|
||||
module result remains a candidate artifact until typed validation accepts it,
|
||||
and only an accepted artifact is encoded with the final artifact codec and
|
||||
eligible for checkpointing or downstream use.
|
||||
|
||||
### Required Changes
|
||||
|
||||
1. Refactor the merge and normalize attempt paths in
|
||||
`internal/framework/pipeline/runner_typed.go` so each attempt proceeds in
|
||||
this order:
|
||||
|
||||
1. execute the typed module and collect its warnings;
|
||||
2. serialize the returned value with `ArtifactCodec.EncodeCandidate` for
|
||||
attempt-debug capture;
|
||||
3. execute the resolved typed validator chain against the in-memory value;
|
||||
4. on validator rejection, record the rejected candidate and finish the
|
||||
attempt without invoking `ArtifactCodec.Encode` or creating a
|
||||
checkpoint artifact; and
|
||||
5. only after validator acceptance, invoke `ArtifactCodec.Encode` through
|
||||
the normal checkpoint-artifact path.
|
||||
|
||||
2. Use one private candidate-serialization helper for merge, normalize, and
|
||||
any equivalent pre-validation debug path. The helper must preserve the
|
||||
codec's media type and schema digest in the debug artifact, but its result
|
||||
must never be used as a final checkpoint or downstream artifact.
|
||||
3. Treat candidate-encoding and final-encoding failures as framework errors,
|
||||
not validator rejections. Record the applicable attempt envelope before
|
||||
returning the error using the existing attempt-debug behavior; Stage 7 will
|
||||
consolidate that behavior across every retrying stage.
|
||||
4. Preserve retry behavior:
|
||||
|
||||
- a validator rejection may be retried according to the stage retry policy;
|
||||
- warnings from rejected attempts remain attempt-local unless existing
|
||||
warning-promotion policy says otherwise;
|
||||
- a final accepted value and its serialized checkpoint artifact are
|
||||
published only after the attempt has completed successfully; and
|
||||
- checkpoint identity, checkpoint event reporting, and durable artifact
|
||||
formats remain unchanged.
|
||||
|
||||
5. Do not weaken a domain codec so that its final `Encode` accepts invalid
|
||||
domain values. In particular, keep the D&D spell codec's semantic checks at
|
||||
the final-artifact boundary and use `EncodeCandidate` for its intentionally
|
||||
incomplete pre-validation representation.
|
||||
|
||||
### Tests
|
||||
|
||||
Add focused merge and normalize tests using an instrumented codec for which
|
||||
`EncodeCandidate` accepts an invalid value but `Encode` rejects it. Prove that:
|
||||
|
||||
- the validator, rather than final encoding, determines that the candidate is
|
||||
rejected;
|
||||
- rejected candidates never call final `Encode` and never produce checkpoint
|
||||
artifacts;
|
||||
- an accepted candidate calls final `Encode` exactly once and produces the
|
||||
existing checkpoint representation;
|
||||
- candidate-encoding and post-acceptance final-encoding failures are reported
|
||||
as framework errors and have attempt debug records; and
|
||||
- retry warnings, rejection promotion, and checkpoint event ordering remain
|
||||
deterministic.
|
||||
|
||||
Retain or add a D&D spell regression test demonstrating that an incomplete
|
||||
candidate can reach validation without making the final spell codec permissive.
|
||||
|
||||
### Documentation
|
||||
|
||||
Update `docs/internal/pipeline.md` to state explicitly that merge and normalize
|
||||
debug payloads may contain candidate encodings, while checkpoints and stage
|
||||
outputs always contain validator-approved final encodings. Update
|
||||
`docs/internal/artifacts.md` if it currently implies that every serialized
|
||||
debug artifact is a final Domain Artifact Zone representation.
|
||||
|
||||
### Completion Gate
|
||||
|
||||
No merge or normalize code path may invoke final artifact encoding before typed
|
||||
validation acceptance, and a validator-rejected candidate must remain
|
||||
observable as a rejection even when the codec would refuse to encode it as a
|
||||
final artifact.
|
||||
|
||||
## Stage 7: Record Every Executed Attempt And Its Terminal Outcome
|
||||
|
||||
### Goal
|
||||
|
||||
Make attempt debugging complete and uniform across chunk, extract, merge, and
|
||||
normalize so every executed retry has one terminal attempt envelope, including
|
||||
validator rejection, module error, validator error, and serialization error.
|
||||
|
||||
### Required Changes
|
||||
|
||||
1. Introduce or consolidate a private attempt-terminal recorder used by all
|
||||
four retrying stages. It must write exactly one envelope per executed
|
||||
attempt and support these terminal outcomes:
|
||||
|
||||
- accepted success;
|
||||
- validator rejection;
|
||||
- module execution error;
|
||||
- validator execution error;
|
||||
- candidate-serialization error; and
|
||||
- final-serialization error where final encoding occurs within the attempt.
|
||||
|
||||
2. Preserve the existing debug directory layout and envelope schema. Populate
|
||||
the envelope consistently with the attempt number, warnings accumulated by
|
||||
that attempt, any available candidate payload or rejection details, and the
|
||||
terminal error text when an error occurred. A validator rejection without
|
||||
an execution error remains a rejection and must not acquire a synthetic
|
||||
error string.
|
||||
3. Complete the extract path in
|
||||
`internal/framework/pipeline/runner_concurrent.go`:
|
||||
|
||||
- write an attempt envelope before returning a validator rejection or
|
||||
validator error;
|
||||
- write an attempt envelope before returning a final-serialization error;
|
||||
and
|
||||
- stop discarding errors returned while writing module-error attempt data.
|
||||
|
||||
4. Complete the chunk path in `internal/framework/pipeline/runner.go` by
|
||||
recording validator execution errors in the envelope's error field and by
|
||||
propagating attempt-write failures on every terminal path.
|
||||
5. Route the existing merge and normalize attempt handling through the same
|
||||
terminal-recording behavior without changing their retry or validation
|
||||
semantics. Coordinate this work with Stage 6 so rejected candidates use
|
||||
candidate encoding and accepted values use final encoding.
|
||||
6. Keep LLM-call ownership scoped to the operation that made the call:
|
||||
|
||||
- module calls belong to the module attempt envelope;
|
||||
- validator calls remain in their validator-specific debug scope; and
|
||||
- attaching a call to an attempt must not duplicate it in another module
|
||||
attempt or orphan it from the attempt that initiated it.
|
||||
|
||||
7. Never silently discard a debug write failure. If another error already
|
||||
exists, return an `errors.Join` result that preserves both the primary error
|
||||
and the contextualized debug error. If no primary error exists, return the
|
||||
contextualized debug error. Do not replace a validator rejection with a
|
||||
framework error unless persisting its required debug record fails.
|
||||
8. Keep debug failures subject to the retry boundary already surrounding the
|
||||
relevant attempt; do not add a second retry loop specifically for debug
|
||||
persistence.
|
||||
|
||||
### Tests
|
||||
|
||||
Add table-driven attempt-debug tests covering each terminal outcome for chunk
|
||||
and extract, and retain equivalent merge and normalize coverage. At minimum,
|
||||
prove that:
|
||||
|
||||
- a first-attempt extract rejection followed by success writes both envelopes;
|
||||
- a terminal extract rejection, validator error, module error, candidate-codec
|
||||
error, and final-codec error each write an envelope with the correct fields;
|
||||
- a chunk validator execution error appears in its attempt envelope;
|
||||
- debug write failures are returned, and are joined with the primary error
|
||||
when both occur;
|
||||
- module LLM calls are attached to the correct attempt while validator LLM
|
||||
calls remain isolated; and
|
||||
- success, rejection, warning, and retry ordering remains deterministic.
|
||||
|
||||
Use a shared assertion helper to verify the invariant that the number and
|
||||
indices of attempt envelopes equal the attempts actually executed.
|
||||
|
||||
### Documentation
|
||||
|
||||
Update `docs/internal/pipeline.md` and `docs/operations.md` so their attempt
|
||||
debug guarantees name all terminal outcomes and explain the separation between
|
||||
module-attempt and validator-call scopes. Do not promise that a payload exists
|
||||
when failure occurred before a candidate value was available.
|
||||
|
||||
### Completion Gate
|
||||
|
||||
For chunk, extract, merge, and normalize, every entered attempt must leave
|
||||
exactly one terminal envelope or return an error that explicitly reports why
|
||||
that envelope could not be persisted. No attempt-related debug write error may
|
||||
be ignored.
|
||||
|
||||
## Stage 8: Close Production Import-Guard Loopholes
|
||||
|
||||
### Goal
|
||||
|
||||
Make the automated import guard enforce the complete domain-first dependency
|
||||
policy for production code, including the framework, the application
|
||||
composition root, and the integration-test-only package.
|
||||
|
||||
### Required Changes
|
||||
|
||||
1. Extend `internal/modules/import_boundaries_test.go` so validation considers
|
||||
both the importing file's repository-relative path and whether it is a test
|
||||
file. Do not return early merely because the importer is outside
|
||||
`internal/modules`.
|
||||
2. Treat `internal/modules/integration` as test infrastructure, not as a module
|
||||
family or a production dependency target:
|
||||
|
||||
- reject every import of `internal/modules/integration` from a non-test Go
|
||||
file; and
|
||||
- continue allowing designated black-box tests under
|
||||
`internal/modules/integration` to import concrete families for composition
|
||||
coverage.
|
||||
|
||||
3. Reject imports of any `internal/modules/**` package from non-test files
|
||||
under `internal/framework/**` or `internal/core/**`. These packages define
|
||||
inward framework and core layers and must remain independent of all module
|
||||
implementations, including `generic`.
|
||||
4. For non-test files under the production CLI composition root
|
||||
`internal/cli/**`, allow module imports only when the target is the exact
|
||||
family registrar package `internal/modules/<family>/register`. Reject direct
|
||||
imports of family roots, domain leaves, adapters, and generic implementation
|
||||
leaves. Apply the same rule automatically to newly added families.
|
||||
5. Preserve test-only assembly allowances needed for black-box and
|
||||
compatibility coverage. In particular, `_test.go` files in the CLI,
|
||||
framework, and core trees may import module packages, and designated
|
||||
integration black-box tests may compose concrete families. These allowances
|
||||
must be based on the importing file being a test file, not on a directory
|
||||
exemption that production files could inherit.
|
||||
6. Retain all Stage 4 within-module rules: concrete peer isolation,
|
||||
registrar-only concrete-to-generic composition, family-root direction, and
|
||||
same-family child relationships. Keep path parsing structural so new module
|
||||
families receive the rules without a hard-coded family list.
|
||||
7. Improve failure messages to name the importing file, import target, and the
|
||||
applicable boundary rule.
|
||||
|
||||
### Tests
|
||||
|
||||
Add table-driven fixtures proving that the guard:
|
||||
|
||||
- rejects production imports of `internal/modules/integration` from module and
|
||||
non-module packages;
|
||||
- rejects production framework and core imports of concrete and generic module
|
||||
packages;
|
||||
- accepts an exact registrar import from production CLI code;
|
||||
- rejects production CLI imports of a family root, concrete leaf, generic
|
||||
leaf, or other non-registrar module package;
|
||||
- accepts equivalent direct imports from `_test.go` compatibility tests;
|
||||
- accepts designated black-box integration-test composition while rejecting a
|
||||
production `.go` file in the same directory; and
|
||||
- retains every cross-family, registrar, and family-root case covered by Stage
|
||||
4.
|
||||
|
||||
Run the guard against the full repository and ensure all current production
|
||||
imports comply without adding path-specific exemptions.
|
||||
|
||||
### Documentation
|
||||
|
||||
Update `docs/internal/modules.md` to distinguish production composition through
|
||||
family registrars from test-only direct composition, and to state that
|
||||
`internal/modules/integration` is not a production dependency target. Update
|
||||
other current-behavior documentation only if it describes broader composition
|
||||
permissions.
|
||||
|
||||
### Completion Gate
|
||||
|
||||
A new production file cannot bypass the domain-first dependency rules by being
|
||||
placed outside `internal/modules`, under `internal/modules/integration`, or in
|
||||
the CLI composition tree. Test-only allowances must remain explicit and
|
||||
file-scoped.
|
||||
|
||||
## Final Verification And Closeout
|
||||
|
||||
After all five stages:
|
||||
After Stages 6 through 8:
|
||||
|
||||
1. Run the full validation commands from this document on a clean worktree.
|
||||
2. Exercise the maintained D&D production example with both a fresh workspace
|
||||
and checkpoint resume.
|
||||
3. Verify that changing the registered default spell validator chain selects a
|
||||
different checkpoint identity.
|
||||
4. Verify merge and normalize attempt debug behavior with instrumented
|
||||
LLM-backed test modules.
|
||||
5. Confirm current production imports satisfy the generic boundary guard.
|
||||
3. Verify that a merge or normalize candidate rejected by typed validation is
|
||||
not passed to final encoding or checkpoint creation.
|
||||
4. Verify that every executed chunk, extract, merge, and normalize retry has a
|
||||
terminal attempt envelope with correctly scoped LLM-call data.
|
||||
5. Confirm current production imports satisfy the strengthened framework,
|
||||
registrar-only CLI, integration-target, and module-family boundary rules.
|
||||
6. Re-read current-behavior documentation for statements made true or obsolete
|
||||
by these stages.
|
||||
7. Replace this roadmap's status with a concise completion record only after all
|
||||
@@ -449,5 +718,6 @@ After all five stages:
|
||||
|
||||
## Open Questions
|
||||
|
||||
None. The implementation choices required for these five remediation stages
|
||||
are specified above.
|
||||
None. The implementation choices required for Stages 6 through 8 are specified
|
||||
above. Final roadmap closeout is intentionally deferred until those stages are
|
||||
complete.
|
||||
|
||||
Reference in New Issue
Block a user