From 3772b308e99dd1115a5bb18d3936475cb521cd4d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 17 Jul 2026 10:06:18 -0500 Subject: [PATCH] Update implementation plan to address additional clean-up items --- docs/roadmap/implementation.md | 304 +++++++++++++++++++++++++++++++-- 1 file changed, 287 insertions(+), 17 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 46ad177..b515fbc 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -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//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.