Add D&D module code audit implementation plan
This commit is contained in:
@@ -427,7 +427,8 @@ reference ordering assume numeric unit IDs instead of source-document order,
|
||||
and spell/NPC checkpoint identities omit stable mapping policies. Both can
|
||||
change durable results or reuse results produced under different semantics.
|
||||
Smaller drift exists in common request preflight, scene semantic-validation
|
||||
ownership, prompt cache-boundary coverage, and package surface conventions.
|
||||
ownership, prompt cache-boundary coverage, package surface conventions, and
|
||||
the documentation of the common extractor contract for future lanes.
|
||||
|
||||
The shared-code question is **yes, but only for two narrow D&D-wide
|
||||
responsibilities**: common chunk-extraction preflight and document-aware
|
||||
@@ -630,6 +631,61 @@ hidden result aliasing, or unregistered audited extractor was found.
|
||||
- **Migration/validation:** graph search found no production consumers;
|
||||
compile all internal packages and add one spell zero-value metadata test.
|
||||
|
||||
7. **The common D&D extractor contract is observable but only partially
|
||||
documented as a requirement for future lanes.**
|
||||
- **Category:** documentation and convention drift.
|
||||
- **Affected modules:** all five extractors and future D&D extraction
|
||||
modules.
|
||||
- **Evidence:** all five reject unknown options, perform common request
|
||||
preflight, return independently owned results, use package-local private
|
||||
response types, and expose prompt/schema provenance plus checkpoint
|
||||
fingerprints. The extension checklist in
|
||||
[module internals](../internal/modules.md#adding-an-extension) covers
|
||||
registration, package-owned assets, prompt ordering, and general option
|
||||
and validation coverage, while
|
||||
[LLM internals](../internal/llm.md#prompt-and-schema-assets) documents the
|
||||
private-schema boundary. Neither location consolidates the remaining
|
||||
behaviors into a normative D&D extractor contract. The missing spell/NPC
|
||||
mapping fingerprints and uneven preflight and prompt-contract coverage
|
||||
demonstrate that conventions discoverable from current packages can
|
||||
still drift.
|
||||
- **Impact:** an additional extractor can appear locally consistent while
|
||||
silently accepting misspelled options, omitting a stable semantic input
|
||||
from checkpoint identity, returning aliased mutable data, implementing
|
||||
incomplete preflight, or missing focused contract coverage.
|
||||
- **Target state and owner:** add a compact **D&D extractor contract**
|
||||
subsection to `docs/internal/modules.md`, incorporated into or placed
|
||||
immediately after **Adding An Extension**. Specify behavioral
|
||||
responsibilities rather than filenames or boilerplate:
|
||||
- reject unknown options unless the option namespace is intentionally
|
||||
extensible;
|
||||
- use the shared request preflight contract while retaining receiver,
|
||||
dependency, and lane-specific checks locally;
|
||||
- return results and exposed metadata that are independently owned and
|
||||
safe for caller mutation;
|
||||
- keep the private response DTO, structural schema, schema identity,
|
||||
provider-response mapping, durable artifact conversion, and
|
||||
lane-specific diagnostics package-owned;
|
||||
- include every stable semantic input that can change durable results in
|
||||
checkpoint identity, including prompt, schema, mapping,
|
||||
canonicalization, prepared reference projection, identity,
|
||||
normalization, and trimming policies when applicable; and
|
||||
- consider focused behavioral coverage for construction and
|
||||
registration, option rejection, preflight, provider failures,
|
||||
structured-output decoding, mapping and ownership, prompt
|
||||
role/input/cache order, and checkpoint invalidation.
|
||||
- **Why documentation rather than another abstraction:** these are shared
|
||||
obligations, not one shared implementation. A checklist makes omissions
|
||||
visible without introducing a configurable metadata builder, generic
|
||||
mapper, shared test fixture, mandatory file layout, or exact-output
|
||||
change-detector tests. Detailed prompt, schema, pipeline, and testing
|
||||
policies should remain linked rather than duplicated.
|
||||
- **Migration/validation:** update the extension checklist and relevant
|
||||
cross-links when the shared preflight helper is documented. Review the
|
||||
text against all five lanes and the completed audit matrix. Do not require
|
||||
exact prompt text, hashes, prefix lengths, test counts, filenames, or
|
||||
fixture layouts.
|
||||
|
||||
#### Optional improvement
|
||||
|
||||
The current module documentation lists only four production consumers of
|
||||
@@ -679,6 +735,7 @@ error wrapping, or secret handling.
|
||||
| Harmonize prompt contract tests | Four citation extractor test suites | Each manifest owns its role/input/cache sequence; consistent assertions should stay beside distinct assets. |
|
||||
| Harmonize preflight coverage | All five package suites after shared preflight | Shared tests own common branches; local tests retain package-visible context and specializations. |
|
||||
| Harmonize package hygiene | Spell/combat extractors and D&D shared | Delete unused exports/artificial API and align spell metadata nil behavior without introducing a new abstraction. |
|
||||
| Document the D&D extractor contract | `docs/internal/modules.md`, linked to existing LLM and testing policies | The five lanes share behavioral obligations that should guide future extensions, but they do not justify a generic implementation or rigid package template. |
|
||||
|
||||
### Rejected Or Deferred Candidates
|
||||
|
||||
@@ -717,9 +774,12 @@ error wrapping, or secret handling.
|
||||
5. **Close prompt contract coverage.** Add full role/input/cache-boundary
|
||||
assertions to each citation prompt suite without shared fixtures or exact
|
||||
text/hash assertions.
|
||||
6. **Apply low-risk package/documentation cleanup.** Remove unused
|
||||
`ArtifactType` constants, add the spell metadata nil guard/test, and update
|
||||
module documentation for the shared helper consumers and boundaries.
|
||||
6. **Apply low-risk package cleanup.** Remove unused `ArtifactType` constants
|
||||
and add the spell metadata nil guard/test.
|
||||
7. **Document the common extractor contract.** Update module documentation for
|
||||
the shared helper consumers and boundaries, then add the D&D extractor
|
||||
contract checklist described in Finding 7. Link to existing prompt, schema,
|
||||
pipeline, and testing policies rather than copying them.
|
||||
|
||||
Each scope can be reviewed and reverted independently. The sequence is not a
|
||||
decision-complete implementation plan; implementation should still pin exact
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user