489 lines
21 KiB
Markdown
489 lines
21 KiB
Markdown
# Test Suite Policy Review Implementation
|
|
|
|
## Status
|
|
|
|
Completed on 2026-07-18 by the commit series ending with `4f96abf`. This
|
|
document is retained as the execution record for the completed test-suite
|
|
policy review; its stages are no longer active implementation instructions.
|
|
|
|
## Purpose
|
|
|
|
This document records the decision-complete execution plan used to implement
|
|
the target revisions in [Test Suite Policy Review](tests.md). The stages were
|
|
followed in order. The feature roadmap owns the review evidence, desired test
|
|
ownership, required dispositions, and target state; this document records the
|
|
sequencing, file-level work, validation, and stop conditions used during
|
|
implementation.
|
|
|
|
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.
|