# Test Suite Policy Review Implementation ## Purpose This document is the decision-complete execution plan for implementing the target revisions in [Test Suite Policy Review](tests.md). Follow the stages in order. The feature roadmap owns the review evidence, desired test ownership, required dispositions, and target state; this document owns sequencing, file-level work, validation, and stop conditions. This is test-suite harmonization, not a production feature change. Do not modify production behavior. If a new durable test exposes incorrect production behavior, preserve the failing evidence, stop the affected stage, and report the defect separately for explicit scoping. ## Governing Policies Before implementation, read and follow: - [Testing Policy](../policy/testing.md), especially behavioral ownership, semantic duplication, test doubles, exact diagnostics, and deletion criteria; - [Architecture](../policy/architecture.md), especially dependency direction, typed artifact boundaries, validation ownership, checkpoint safety, and source/reference separation; and - [Documentation Policy](../policy/documentation.md), especially canonical ownership, current-versus-future behavior, and maintenance of test-routing links. The following constraints apply to every stage: - preserve unrelated user changes in a dirty worktree; - make no production-code changes unless separately authorized after reporting a confirmed production defect; - add no coverage-percentage target and do not use line coverage as a success metric; - do not mechanically convert tests to tables, consolidate leaf registration tests, or introduce new test frameworks; - keep the default suite deterministic, offline, credential-free, and safe for repeated and parallel execution; - use real filesystem collaborators with `t.TempDir()` for checkpoint behavior; - assert typed/structured outcomes or stable category fragments rather than complete incidental error wording; and - do not update fixtures automatically or add a golden-update path. ## Stage 0 - Establish the implementation baseline 1. Read [Test Suite Policy Review](tests.md) completely, including the retained ownership map and every required finding. 2. Inspect the worktree and preserve unrelated changes. Limit planned edits to tests and the internal documentation routing identified below. 3. Run: ```sh go test ./... go vet ./... go build ./cmd/notarius go test -race ./internal/framework/pipeline ./internal/framework/llm ./internal/cli ./internal/modules/integration ``` 4. If the baseline fails for a reason unrelated to the planned work, stop and report it. Do not rewrite expectations merely to obtain a clean baseline. Stage 0 is complete when the starting state and any pre-existing worktree changes are recorded and all baseline commands pass. ## Stage 1 - Protect filesystem checkpoint compatibility and recovery Complete this stage before deleting or consolidating existing tests so the highest-risk protection is strengthened first. ### 1.1 Checkpoint identity Create `internal/framework/checkpoint/identity_test.go`. Test through `NewIdentity` and `Identity.RelativePath`, not private normalization helpers. Build one representative `pipeline.ResolvedPipeline` with two artifact lanes, an input module, stable pipeline ID/digest, selected lanes, runtime fingerprints, reference provenance, and provenance fingerprints. Add tests that prove: - reordering selected lanes, runtime fingerprints, references, provenance fingerprints, or resolved lanes does not change the resulting identity; - duplicate or blank selected-lane entries do not change the normalized lane set, and blank fingerprint entries do not change the identity; - changing each meaningful identity input independently changes the digest: pipeline ID or digest, input key, raw/source digest, selected lane set, runtime override value, reference digest or target identity, and provenance fingerprint; - an omitted explicit input key falls back to the resolved input module; - missing pipeline ID, pipeline digest, input key, and both source/input digests are rejected with stable category context; and - `RelativePath` is deterministic, slash-separated, relative, confined, and includes the normalized pipeline/input and digest-derived hierarchy without asserting incidental private prefix lengths beyond the documented layout. Use independent inputs for each mutation so one case cannot pass because a different field also changed. ### 1.2 All-stage recorder/loader round trips Create `internal/framework/checkpoint/filesystem_test.go`; leave the existing schema-identifier test in `recorder_test.go`. Using one real root and one identity, exercise the exported recorder and loader for: - source success with a valid document, self-references, metadata, and digest; - extract success with at least one serialized artifact, schema identity, content metadata, chunk provenance, warnings, dependency fingerprints, and one rejected output so `StatusSucceededWithRejections` is round-tripped; - merge success with one serialized artifact, warnings, and dependencies; and - normalize success with one serialized artifact, warnings, and dependencies. For each stage, assert the loader returns `Reused`, restores the meaningful values, and preserves serialized bytes and codec identity. Mutate the original inputs after recording and mutate one loaded result before reloading; neither mutation may alter persisted or subsequently loaded state. Inspect representative created directories and files to retain the `0700`/ `0600` permission contract on platforms where Unix permission bits are meaningful. Do not snapshot the full directory tree or complete JSON documents. ### 1.3 Invalid and incompatible checkpoint state Seed valid state through the recorder, then copy or edit one artifact per subtest. Drive every case through the exported loader method for that stage. Require a non-reused decision and a short category fragment for: - missing artifact and malformed JSON; - `WorkspaceSchemaVersionV1` and an unknown workspace schema version; - mismatched checkpoint identity digest; - wrong stage, lane, module, terminal status, or dependency fingerprint; - incomplete serialized artifact kind/schema/schema digest; - malformed base64 and content-digest mismatch; - source document validation or source/output digest mismatch; and - extract, merge, and normalize output-digest mismatch. Include one successful `StatusSucceededWithRejections` extract case and prove that non-reusable running, failed, pending, or invalidated statuses remain non-reused. Assert categories, not complete sentences. Missing state should be a normal non-reuse decision; corrupt or incompatible state must never panic or silently reuse. ### 1.4 Stage validation Run: ```sh go test ./internal/framework/checkpoint ./internal/framework/pipeline ./internal/cli go test -race ./internal/framework/checkpoint ./internal/framework/pipeline ./internal/cli ``` Stage 1 is complete when identity selection, every persisted stage, mutation ownership, compatibility rejection, and corrupt-state recovery are protected at the real filesystem boundary without production changes. ## Stage 2 - Remove fake-only, obsolete, and misleading tests ### 2.1 Framework contracts and helpers In `internal/framework/contracts/contracts_test.go`, delete exactly: - `TestFakeExtractorReturnsTypedOutput`; - `TestFakeChunkerReturnsSourcePlan`; - `TestFakeChunkerReceivesPerRunContext`; - `TestFakeExtractorReceivesChunkAndAmbientContext`; - `TestFakeMergeNormalizeAndOutputContracts`; and - `TestReferenceSetDataTypes`. After removing those tests, delete fake methods/types or imports only when they have no remaining test use. Retain the compile-time interface assertions and the reference/material/artifact clone and JSON non-leakage tests. Delete `TestHelpersReturnValidationResults` from `internal/framework/validate/validate_test.go`. Retain `TestApproved` and `TestRejectedTrimsReasonAndMessage`. Delete `internal/modules/generic/normalize/noop/typed_test.go`. Do not remove the no-op normalizer's resolver, registration, runner, production-composition, or maintained-example coverage. Retain the direct append-order merger ordering test. ### 2.2 Obsolete Scriptorium grounding Delete `internal/framework/llm/scriptorium_api_test.go` in full. Do not move its unused API inventory elsewhere. Retain and run `scriptorium_client_test.go`, `asset_registry_test.go`, module-local prompt preparation, cancellation, validation, profile, and credential-redaction tests. ### 2.3 Misleading D&D integration tests In `internal/modules/integration/dnd_spells_runner_test.go`, delete exactly: - `TestRunnerDoesNotExtractSpellMentionedOnlyInPartyReference`; - `TestRunnerCarriesDNDSpellCastWithInvalidSourceRefToSerializedOutput`; and - `TestRunnerRejectsMalformedDNDSpellsArtifactAtSerializationBoundary`. Remove helpers/imports only if unused afterward. Retain `TestRunnerProcessesSeriatimInputWithDNDSpellsExtractor` and `TestRunnerPassesPartyAndGlossaryReferencesToDNDSpellsPrompt` as the two representative cross-family workflows. ### 2.4 Stage validation Run: ```sh go test ./internal/framework/contracts ./internal/framework/llm ./internal/framework/validate go test ./internal/modules/generic/normalize/noop ./internal/modules/integration ./internal/modules/dnd/... ``` Stage 2 is complete when all named low-value tests are gone, retained owners still pass, and no production file changed. ## Stage 3 - Consolidate configuration, CLI state, and resolution ownership ### 3.1 Version 3 configuration tests Before deleting `internal/core/config/v3_test.go`, preserve its only distinct protections: - add a version-2 input to the strict file-decoding cases in `file_config_contract_test.go`; require rejection and the migration category, not the complete diagnostic; and - ensure `env_contract_test.go` positively proves that empty configured cache roots resolve to distinct `notarius/chunk-plans` and `notarius/checkpoints` descendants of the supplied per-user cache root. Then delete `v3_test.go`. Do not duplicate its defaults, precedence, redaction, invalid-source, or removed-field cases elsewhere. ### 3.2 CLI state and maintained examples Move `emptyLookup` from `internal/cli/state_surfaces_test.go` to `internal/cli/contract_test_helpers_test.go`, then delete `state_surfaces_test.go` in full. Confirm its remaining behavior is still owned by command/run contracts and `state_hardening_test.go`; do not transplant its tests. In `internal/cli/production_contract_test.go`: - keep `TestDefaultCLICompositionResolvesMaintainedConfigurations`, but rename it to `TestDefaultCLICompositionValidatesRepresentativeConfiguration` and reduce it to one representative `config validate` command using empty/default `Options`; this test owns fallback production composition, not maintained-example enumeration; - remove maintained-example resolution from `TestProductionCatalogCoversMaintainedConfigurations` while retaining required production registry members, typed codec/variant wiring, catalog conversion, and the exact documented spell validator chain; and - remove the maintained-example loop from `TestProductionConfigValidationCoversModuleAndVariantFailures`, retaining one valid baseline and each distinct failure mutation. Do not weaken `example_contract_test.go`; it remains the sole owner for loading, resolving, listing, and executing the maintained examples. ### 3.3 Default and Seriatim resolver duplication Delete `internal/framework/pipeline/default_modules_test.go` in full. Delete `internal/modules/seriatim/input/transcript/config_test.go` in full. Delete `internal/modules/seriatim/input/transcript/testdata/pipeline.yml` with it; the fixture is owned only by that deleted test file. Retain adapter parsing, leaf registration, the Seriatim runner integration, generic resolver tests, and production examples. ### 3.4 D&D capability integration Rewrite `internal/modules/integration/dnd_spells_config_test.go` to own only two cross-family capability failures: 1. removing `source.transcript` from the Seriatim input spec must make the D&D spell extractor incompatible; and 2. removing `dnd.spell_casts` from the spell extractor spec must make the append-order merger incompatible. Use one compact table over a programmatically constructed profile and the smallest catalog capable of resolution. Delete the successful-resolution, stable-digest, and unknown-lane cases. Simplify or remove fixture/catalog helpers that become unnecessary, but preserve helpers used by the retained runner integration in sibling test files. ### 3.5 Stage validation Run: ```sh go test ./internal/core/config ./internal/cli ./internal/framework/pipeline go test ./internal/modules/seriatim/... ./internal/modules/integration ``` Stage 3 is complete when each behavior has the owner specified above, the maintained examples are not redundantly enumerated, and all distinct migration, default-composition, and capability protections remain. ## Stage 4 - Rewrite brittle assertions at durable behavioral boundaries ### 4.1 Source validation ownership and diagnostics In `internal/core/source/source_test.go`: - consolidate malformed `SourceRef` categories under the `ValidateRef` tests; - reduce `TestValidateDocumentUnitReferences` to one case proving nested reference failures receive unit/document context and one case proving the document-only self-reference invariant; - retain valid documents, required document/unit fields, duplicate IDs, non-empty units, valid/reversed/missing references, unit lookup, and digest sensitivity/determinism; and - replace exact complete error equality with the minimum stable field/category fragments needed to distinguish each failure. Do not add typed production errors during this pass. If stable fragments cannot distinguish meaningful categories without a production change, retain the narrowest current assertion and report that limitation rather than changing production code. ### 4.2 Private LLM response schemas In `internal/modules/dnd/chunk/scenes/schema_test.go`, replace the nested `map[string]any` schema-structure walk with actual JSON Schema validation. Add a small test helper that parses the instance and schema with `jsonschema.UnmarshalJSON`, registers the schema with `jsonschema.NewCompiler().AddResource`, compiles it, and calls `schema.Validate`, matching the existing production validator boundary. Use a representative valid scene response and mutations that reject: - obsolete segment-based boundary fields; - non-positive start/end unit IDs; - invalid `primary_mode` and `boundary_confidence` values; - empty boundary caveats; and - unknown properties. Retain identity/hash validity, DTO integer decoding, and mutation safety. In `internal/modules/dnd/extract/spells/schema_test.go`, validate one legal private LLM response whose references omit `source_id`, and prove that adding a canonical `source_id` is rejected. Retain response identity/hash, mutation safety, and diagnostics non-leakage. Do not conflate this private transport schema with the durable codec schema or alter the maintained durable fixture. ### 4.3 Framework schema enumeration In `internal/framework/llm/schema_registry_test.go`, remove only the assertion that `RegisteredResponseSchemas` has exactly two entries. Continue to assert: - returned keys are sorted; - both required framework test schemas are present and valid; - returned bytes are mutation-safe; - diagnostics omit raw schema content; and - D&D schemas are not registered in the domain-neutral framework registry. ### 4.4 Stage validation Run: ```sh go test ./internal/core/source ./internal/framework/llm go test ./internal/modules/dnd/chunk/scenes ./internal/modules/dnd/extract/spells ./internal/modules/dnd/codec/spells ``` Stage 4 is complete when failures express behavioral categories, private schemas are validated by accepted/rejected instances, and durable schema compatibility coverage remains unchanged. ## Stage 5 - Make composition and architecture checks extension-friendly ### 5.1 Family registrars Rewrite the success assertions in: - `internal/modules/generic/register/register_test.go`; - `internal/modules/seriatim/register/register_test.go`; and - `internal/modules/dnd/register/register_test.go`. Use required-membership helpers rather than exact equality for registered keys and asset names. Prove representative family-owned entries are retrievable or buildable through their registry boundary. Preserve: - validation of all required registry/asset dependencies before any mutation; - contextual failure on duplicate family registration; - absence of cross-family composition where that is an explicit architectural ownership rule; and - exact D&D spell default-validator order, because it is documented production policy. Do not delete leaf spec/constructor/registration tests and do not move the CLI production catalog into family tests. ### 5.2 Central import-boundary enforcement Move the two rules from `internal/framework/chunkplan/import_boundaries_test.go` into `internal/modules/import_boundaries_test.go`: - production files under `internal/core/source` may import neither `internal/framework` nor `internal/modules`; and - production files under `internal/framework/chunkplan` may not import `internal/modules`. Extend the centralized checker so its repository walk enforces those rules. Add rule-level synthetic cases for both allowed and forbidden imports, using the existing table/checker style. Retain the existing rejected fixture that proves generic-to-concrete enforcement. Once both unique rules and checker activation are protected centrally, delete `internal/framework/chunkplan/import_boundaries_test.go`. ### 5.3 Internal documentation routing Update only the `Tests To Inspect` routing needed to match the final suite in: - `docs/internal/state.md`; - `docs/internal/pipeline.md`; - `docs/internal/modules.md`; and - `docs/internal/llm.md`. Replace deleted or nonexistent names such as `internal/cli/state_surfaces_test.go` and `internal/cli/run_test.go` with concise links or paths to the retained command/run contracts, state-hardening, production-composition, maintained-example, checkpoint filesystem, and cross-family integration owners. Do not create an exhaustive test inventory or repeat subsystem contracts owned elsewhere. ### 5.4 Stage validation Run: ```sh go test ./internal/modules/... ./internal/framework/chunkplan go test ./internal/cli ./internal/framework/pipeline ./internal/framework/llm ``` Stage 5 is complete when legitimate family additions no longer require closed inventory edits, all architectural rules remain executable in one checker, and internal documentation names only existing retained owners. ## Stage 6 - Repository acceptance and handoff 1. Review the final diff against every required finding and retained-owner statement in [Test Suite Policy Review](tests.md). Confirm that no production `.go` file changed. 2. Run formatting on changed Go test files, then run: ```sh go test ./... go vet ./... go build ./cmd/notarius go test -race ./internal/framework/pipeline ./internal/framework/llm ./internal/framework/checkpoint ./internal/cli ./internal/modules/integration go test ./... -shuffle=on -count=5 ``` 3. Generate one coverage report for investigation. Confirm the new checkpoint tests execute source, extract, merge, normalize, manifest-validation, and corruption paths. Do not compare the percentage to the review baseline and do not add tests merely to increase it. 4. Check documentation links and `git diff --check`. Confirm no fixtures were updated automatically and no secrets or external-service requirements were introduced. Implementation is complete when: - every checkpoint stage has a real filesystem round trip and corrupted or incompatible state is never silently reused; - every deletion or rewrite in the feature roadmap is complete while its named retained owner still passes; - maintained examples have one example-contract owner plus one narrow default production-composition smoke path; - registrar and architecture checks permit legitimate extension without weakening documented ownership or validator order; - the full suite is deterministic, offline, credential-free, race-clean, and diagnostically useful; - internal documentation points to existing test owners; and - all validation commands pass with no production behavior change. ## Open Questions None. The feature roadmap contains enough evidence and policy decisions to implement every stage without additional testing-policy choices. If a new test reveals a production defect, that is a scope boundary rather than an open planning question: stop the affected stage and request explicit authorization before changing production behavior.