Finish the D&D module cleanup
This commit is contained in:
@@ -1,327 +0,0 @@
|
||||
# D&D Module Refactoring Implementation Plan
|
||||
|
||||
## Objective
|
||||
|
||||
Address the five remaining D&D module refactoring findings without changing
|
||||
artifact schemas, prompt content, module keys, validator policy, warning or
|
||||
rejection semantics, durable media types, or pipeline configuration.
|
||||
|
||||
The completed implementation must:
|
||||
|
||||
- make every D&D append-order merger return independently owned nested data and
|
||||
preserve absent versus present-empty list representation consistently;
|
||||
- use one source-document index per operation wherever D&D code repeatedly
|
||||
validates or resolves source references;
|
||||
- centralize the strict candidate JSON encoding and decoding mechanics shared
|
||||
by all five D&D artifact codecs while retaining artifact ownership in each
|
||||
codec package;
|
||||
- remove unused source-reference canonicalization results from the NPC and
|
||||
combat-turn normalizers; and
|
||||
- remove the unused internal NPC identity and registry aliases.
|
||||
|
||||
This plan implements internal refactoring only. It does not add extractors,
|
||||
fields, classifications, prompts, validators, compatibility shims, or user
|
||||
configuration.
|
||||
|
||||
## Governing Constraints
|
||||
|
||||
Follow:
|
||||
|
||||
- [Architecture](../policy/architecture.md), especially explicit stage
|
||||
ownership, domain-first dependencies, and the rule against abstractions that
|
||||
serve only hypothetical reuse;
|
||||
- [Testing Policy](../policy/testing.md), especially behavior-level tests,
|
||||
focused regression coverage for data integrity, and avoidance of tests that
|
||||
merely detect private implementation changes;
|
||||
- [Documentation Policy](../policy/documentation.md), keeping current
|
||||
implementation detail in [Module Internals](../internal/modules.md); and
|
||||
- the implemented [D&D extractor contract](../internal/modules.md#dd-extractor-contract).
|
||||
|
||||
Preserve these decisions throughout the work:
|
||||
|
||||
- Keep the five artifact-specific merger functions. Do not replace them with a
|
||||
callback-driven generic merger.
|
||||
- Use `source.DocumentIndex` as the canonical index for repeated source
|
||||
reference validation and position lookup. Do not introduce another
|
||||
independent unit-ID map or validation implementation.
|
||||
- Keep artifact schemas, approved-value validation, kinds, media types, and
|
||||
contextual errors in their artifact codec packages.
|
||||
- Keep artifact-specific response mapping, chronology sorting, normalization,
|
||||
and validators local. The shared work in this plan is limited to demonstrated
|
||||
mechanics with identical semantics.
|
||||
- Preserve existing error classifications, diagnostic scopes, ordering,
|
||||
truncation, and approval behavior. Exact non-contractual wording may remain
|
||||
unchanged where practical, but tests should assert stable semantics rather
|
||||
than duplicate entire error strings.
|
||||
- Do not add benchmarks or performance-sensitive timing assertions. Verify
|
||||
index reuse through design and ordinary behavioral tests, not through
|
||||
brittle implementation or latency detectors.
|
||||
|
||||
Each stage below is one implementation prompt. Complete and verify a stage
|
||||
before beginning the next.
|
||||
|
||||
## Stage 1: Harmonize D&D Merger Ownership And Presence
|
||||
|
||||
Update the D&D append-order mergers in
|
||||
`internal/modules/dnd/register/merge.go`.
|
||||
|
||||
Implementation:
|
||||
|
||||
1. Add artifact-specific clone functions for spell casts and NPCs, including a
|
||||
fresh copy of each non-nil `SourceRefs` slice. Match the existing
|
||||
combat-turn and NPC-interaction ownership behavior.
|
||||
2. Change `appendSpellLists` and `appendNPCLists` to append cloned records
|
||||
rather than shallow-copying records from their inputs.
|
||||
3. Change `appendSpellLists` to use the same list-presence rule as the other
|
||||
four mergers:
|
||||
- no inputs, or only inputs with a nil `SpellCasts` field, produce a nil
|
||||
`SpellCasts` field;
|
||||
- any non-nil input field makes the result field non-nil, including when all
|
||||
present fields are empty; and
|
||||
- records retain deterministic input and record order.
|
||||
4. Do not alter scene-description merge behavior: its record contains a scalar
|
||||
source reference and requires no nested clone.
|
||||
|
||||
Tests:
|
||||
|
||||
- Extend registrar merger tests at the package-level merge boundary.
|
||||
- For spells and NPCs, mutate a merged record's first source reference and
|
||||
prove that the corresponding input remains unchanged.
|
||||
- Cover no inputs, nil-only inputs, present-empty input, and ordered populated
|
||||
inputs for spells. Retain the existing equivalent NPC, combat-turn,
|
||||
interaction, and scene coverage without duplicating it elsewhere.
|
||||
|
||||
Verification:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/register
|
||||
```
|
||||
|
||||
## Stage 2: Add Indexed Citation Resolution And Migrate Relatedness Validators
|
||||
|
||||
Replace repeated validation and endpoint scans in D&D citation text assembly
|
||||
with one operation-scoped indexed resolver.
|
||||
|
||||
Implementation:
|
||||
|
||||
1. In `internal/modules/dnd/shared`, introduce a small citation resolver that:
|
||||
- is constructed from one non-nil `*source.SourceDocument`;
|
||||
- retains that immutable operation input and one
|
||||
`source.NewDocumentIndex(doc)`;
|
||||
- validates each requested range through `DocumentIndex.ValidateRef`;
|
||||
- resolves endpoints through `DocumentIndex.Position`;
|
||||
- includes overlapping source units only once;
|
||||
- returns cited unit text in document order, joined exactly as today; and
|
||||
- preserves the current contextual failure for an invalid source range.
|
||||
2. Replace the top-level repeated-scan `CitedText` API with the resolver API.
|
||||
Do not retain two production implementations of citation traversal.
|
||||
3. Construct one resolver per relatedness-validation invocation and reuse it
|
||||
for every record in that request.
|
||||
4. Migrate the spell, NPC, combat-turn, NPC-interaction, and scene-description
|
||||
relatedness validators. Preserve all current deferral, warning, token
|
||||
matching, warning-cap, and diagnostic behavior.
|
||||
|
||||
Tests:
|
||||
|
||||
- Adapt shared citation tests to the resolver's package-level behavior.
|
||||
- Preserve coverage for nil documents, invalid ranges, overlapping ranges,
|
||||
disjoint ranges, document ordering, and newline joining where those risks are
|
||||
currently covered.
|
||||
- Run existing relatedness tests unchanged except for construction/API updates
|
||||
required by the refactor. Do not add tests that inspect the resolver's map,
|
||||
count index construction, or assert performance.
|
||||
|
||||
Verification:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/shared \
|
||||
./internal/modules/dnd/validate/spells/source_relatedness \
|
||||
./internal/modules/dnd/validate/npcs/source_relatedness \
|
||||
./internal/modules/dnd/validate/combatturns/source_relatedness \
|
||||
./internal/modules/dnd/validate/npcinteractions/source_relatedness \
|
||||
./internal/modules/dnd/validate/scenedescriptions/source_relatedness
|
||||
```
|
||||
|
||||
## Stage 3: Reuse Document Indexes Across D&D Validators
|
||||
|
||||
Migrate repeated source-reference validation and position lookup in rejecting
|
||||
and invariant validators to one `source.DocumentIndex` per validation
|
||||
invocation.
|
||||
|
||||
Implementation:
|
||||
|
||||
1. Construct the index only after any existing shape-deferral or request
|
||||
precondition logic that should run first.
|
||||
2. Use `DocumentIndex.ValidateRef` inside source-reference loops in the spell,
|
||||
NPC, combat-turn, NPC-interaction, and scene-description source-reference
|
||||
validators.
|
||||
3. Pass an operation-scoped index into invariant helper functions that
|
||||
repeatedly validate references or compare source positions. Apply this to
|
||||
combat turns, NPC interactions, and scene descriptions where relevant.
|
||||
4. Replace scene invariant `source.UnitIndex` calls inside sort comparisons
|
||||
with `DocumentIndex.Position`.
|
||||
5. Preserve extract-stage chunk-containment checks, validator deferral,
|
||||
aggregate diagnostic order, reason codes, and truncation. The index changes
|
||||
lookup mechanics only.
|
||||
6. Do not extract the artifact-specific validator loops into a generic callback
|
||||
helper.
|
||||
|
||||
Tests:
|
||||
|
||||
- Existing validator suites should continue to express the behavioral
|
||||
contract. Add or change tests only if a source-document boundary is not
|
||||
already protected.
|
||||
- Do not add source-code inspection tests or assertions that a particular
|
||||
helper was called.
|
||||
|
||||
Verification:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/validate/...
|
||||
```
|
||||
|
||||
## Stage 4: Reuse Document Indexes In Domain And Normalization Logic
|
||||
|
||||
Finish the operation-scoped index migration outside validators and simplify
|
||||
the NPC and combat-turn canonicalization call sites.
|
||||
|
||||
Implementation:
|
||||
|
||||
1. In spell and combat-turn normalization, construct one
|
||||
`source.DocumentIndex` for the normalization operation and pass it through
|
||||
duplicate-identity helpers instead of calling `source.ValidateRef` for each
|
||||
record and reference.
|
||||
2. In scene-description normalization, replace its local unit-position map and
|
||||
repeated `source.ValidateRef` calls with one `source.DocumentIndex`. Use the
|
||||
same index for validation and chronological ordering.
|
||||
3. Update `internal/modules/dnd/npcinteractions` canonical helpers so callers
|
||||
performing repeated validity checks can supply and reuse an operation-scoped
|
||||
index. Propagate the index through interaction normalization and invariant
|
||||
validation as needed. Do not keep a second loop that delegates to
|
||||
`source.ValidateRef`.
|
||||
4. Review `internal/modules/dnd/shared/unit_refs.go`. Where one extraction
|
||||
mapping operation resolves multiple unit IDs, construct one document index
|
||||
at the operation boundary and resolve all unit IDs through it. Preserve the
|
||||
existing field-scoped errors and raw candidate mapping behavior.
|
||||
5. Remove the NPC and combat-turn `canonicalizeSourceRefs` wrappers that return
|
||||
change and duplicate-count values their callers discard. Call
|
||||
`SourceRefOrder.Canonicalize` directly and perform the single behavioral
|
||||
comparison each normalizer actually needs.
|
||||
6. Keep the spell normalizer's richer `canonicalizeSourceRefs` helper because
|
||||
it intentionally distinguishes ordering changes from duplicate removal for
|
||||
diagnostics.
|
||||
7. Do not alter normalization identities, collapse eligibility, ordering,
|
||||
warning scopes, or checkpoint fingerprints.
|
||||
|
||||
Tests:
|
||||
|
||||
- Run the existing extractor, domain-helper, and normalizer tests as behavioral
|
||||
regression coverage.
|
||||
- Add coverage only for an observable boundary found to be missing; do not
|
||||
test tuple arity, helper presence, map construction, or private call paths.
|
||||
|
||||
Verification:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/extract/... \
|
||||
./internal/modules/dnd/npcinteractions \
|
||||
./internal/modules/dnd/normalize/...
|
||||
```
|
||||
|
||||
## Stage 5: Centralize Strict D&D Candidate JSON Mechanics
|
||||
|
||||
Extract the identical candidate JSON encoding and strict decoding behavior used
|
||||
by the five D&D artifact codecs.
|
||||
|
||||
Implementation:
|
||||
|
||||
1. Add a narrowly named shared package beneath
|
||||
`internal/modules/dnd/codec/` for generic typed candidate JSON mechanics.
|
||||
It may depend only on the Go standard library and must not import an
|
||||
artifact-specific codec or D&D artifact type.
|
||||
2. Provide generic helpers that:
|
||||
- encode a typed candidate with `json.Marshal`;
|
||||
- decode exactly one typed JSON value;
|
||||
- reject unknown fields with `DisallowUnknownFields`;
|
||||
- reject trailing JSON values; and
|
||||
- wrap failures with a caller-supplied artifact label so existing codec
|
||||
errors retain useful context.
|
||||
3. Migrate the spell, NPC, combat-turn, NPC-interaction, and
|
||||
scene-description codecs to delegate only `EncodeCandidate` and
|
||||
`DecodeCandidate` mechanics to the helper.
|
||||
4. Keep `Codec`, `New`, schema loading, kind, version, media type,
|
||||
`Encode`/`Decode` approved-value validation, and artifact-specific
|
||||
validation in each owning package.
|
||||
5. Preserve the exact accepted and rejected JSON language. In particular, do
|
||||
not make candidate decoding permissive, add normalization, or merge durable
|
||||
schemas.
|
||||
|
||||
Tests:
|
||||
|
||||
- Give the shared package focused table-driven tests for successful typed
|
||||
round-trip, unknown-field rejection, malformed JSON, and trailing-value
|
||||
rejection.
|
||||
- Retain each artifact codec's package-level boundary tests and maintained
|
||||
durable fixtures. Remove only tests that become exact redundant copies of
|
||||
helper mechanism tests and provide no artifact-boundary confidence.
|
||||
- Do not consolidate artifact fixtures or schemas.
|
||||
|
||||
Verification:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/codec/...
|
||||
```
|
||||
|
||||
## Stage 6: Remove Unused NPC Aliases And Close Documentation
|
||||
|
||||
Remove the unnecessary internal API surface and update the canonical
|
||||
implemented-behavior documentation after all preceding refactors are present.
|
||||
|
||||
Implementation:
|
||||
|
||||
1. Delete `npcs/registry.New`; `Resolve` remains the single registry
|
||||
construction entry point.
|
||||
2. Delete `npcs/identity.Validate`; `ValidateList` remains the explicit
|
||||
whole-list validation entry point.
|
||||
3. Confirm there are no production or test callers before deletion. Do not add
|
||||
compatibility aliases or deprecation shims: these are internal,
|
||||
pre-release APIs.
|
||||
4. Update [Module Internals](../internal/modules.md) concisely to reflect:
|
||||
- independently owned nested merger output;
|
||||
- operation-scoped indexed source-reference and citation traversal; and
|
||||
- the shared strict candidate JSON mechanism with artifact-owned durable
|
||||
codec policy.
|
||||
5. Do not add these implementation details to user configuration, integration
|
||||
contracts, or ADRs. No user-visible or durable contract changes are
|
||||
intended.
|
||||
6. Run formatting over changed Go files and inspect the final diff for
|
||||
accidental schema, prompt, configuration, or fingerprint changes.
|
||||
|
||||
Tests and verification:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/...
|
||||
go test ./...
|
||||
go vet ./...
|
||||
go build ./cmd/notarius
|
||||
```
|
||||
|
||||
The final implementation is complete when:
|
||||
|
||||
- all commands above pass;
|
||||
- the five artifact mergers preserve order, presence, and independent nested
|
||||
ownership;
|
||||
- repeated D&D source-reference operations use one document index at their
|
||||
operation boundary;
|
||||
- all five codecs share strict candidate JSON mechanics without surrendering
|
||||
artifact-specific policy;
|
||||
- the NPC and combat normalizers compute only the canonicalization results they
|
||||
use;
|
||||
- the two unused aliases are absent; and
|
||||
- `git diff` shows no prompt, schema, durable artifact, module-key,
|
||||
configuration, or checkpoint-policy changes.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None. The implementation choices required for this refactoring are resolved
|
||||
above.
|
||||
@@ -334,6 +334,9 @@ func TestDocumentIndexSnapshotsIdentityAndUnitPositions(t *testing.T) {
|
||||
doc.ID = "changed"
|
||||
doc.Units[0].ID = 99
|
||||
|
||||
if documentID, ok := index.DocumentID(); !ok || documentID != "source-1" {
|
||||
t.Fatalf("DocumentID() = %q, %t, want source-1, true", documentID, ok)
|
||||
}
|
||||
if position, ok := index.Position(30); !ok || position != 0 {
|
||||
t.Fatalf("Position(30) = %d, %t, want 0, true", position, ok)
|
||||
}
|
||||
@@ -348,6 +351,9 @@ func TestDocumentIndexSnapshotsIdentityAndUnitPositions(t *testing.T) {
|
||||
|
||||
func TestZeroDocumentIndexIsSafe(t *testing.T) {
|
||||
var index DocumentIndex
|
||||
if documentID, ok := index.DocumentID(); ok || documentID != "" {
|
||||
t.Fatalf("DocumentID() = %q, %t, want empty, false", documentID, ok)
|
||||
}
|
||||
if position, ok := index.Position(1); ok || position != 0 {
|
||||
t.Fatalf("Position(1) = %d, %t, want 0, false", position, ok)
|
||||
}
|
||||
|
||||
@@ -31,6 +31,14 @@ func NewDocumentIndex(doc *SourceDocument) DocumentIndex {
|
||||
}
|
||||
}
|
||||
|
||||
// DocumentID returns the indexed document identity.
|
||||
func (i DocumentIndex) DocumentID() (string, bool) {
|
||||
if !i.hasDocument {
|
||||
return "", false
|
||||
}
|
||||
return i.documentID, true
|
||||
}
|
||||
|
||||
// Position returns the indexed document position for unitID.
|
||||
func (i DocumentIndex) Position(unitID int) (int, bool) {
|
||||
position, ok := i.positions[unitID]
|
||||
|
||||
@@ -111,7 +111,7 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize
|
||||
return contracts.TypedNormalizeResult[dnd.CombatTurnList]{}, normalizerErrorf("resolve NPC registry: %w", err)
|
||||
}
|
||||
index := source.NewDocumentIndex(req.Source)
|
||||
order := shared.NewSourceRefOrderWithIndex(req.Source, index)
|
||||
order := shared.NewSourceRefOrderFromIndex(index)
|
||||
value, warnings := normalizeList(req.MergeOutput.Value, index, order, npcRegistry)
|
||||
return contracts.TypedNormalizeResult[dnd.CombatTurnList]{Value: value, Warnings: warnings}, nil
|
||||
}
|
||||
|
||||
@@ -119,7 +119,7 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize
|
||||
return contracts.TypedNormalizeResult[dnd.NPCInteractionList]{}, normalizerErrorf("NPC registry reference is required")
|
||||
}
|
||||
index := source.NewDocumentIndex(req.Source)
|
||||
order := shared.NewSourceRefOrderWithIndex(req.Source, index)
|
||||
order := shared.NewSourceRefOrderFromIndex(index)
|
||||
value, warnings := normalizeList(req.MergeOutput.Value, index, order, registry)
|
||||
return contracts.TypedNormalizeResult[dnd.NPCInteractionList]{Value: value, Warnings: warnings}, nil
|
||||
}
|
||||
|
||||
@@ -98,7 +98,7 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize
|
||||
}
|
||||
|
||||
index := source.NewDocumentIndex(req.Source)
|
||||
order := shared.NewSourceRefOrderWithIndex(req.Source, index)
|
||||
order := shared.NewSourceRefOrderFromIndex(index)
|
||||
value, warnings := normalizeSpellList(req.MergeOutput.Value, n.effectiveCatalog, order)
|
||||
value, duplicateWarnings := collapseDuplicateSpellCasts(value, index, n.effectiveCatalog)
|
||||
warnings = append(warnings, duplicateWarnings...)
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package register
|
||||
|
||||
import (
|
||||
"slices"
|
||||
|
||||
"gitea.maximumdirect.net/eric/notarius/internal/core/source"
|
||||
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd"
|
||||
)
|
||||
@@ -110,32 +112,28 @@ func appendSceneDescriptionLists(values []dnd.SceneDescriptionList) (dnd.SceneDe
|
||||
|
||||
func cloneCombatTurn(value dnd.CombatTurn) dnd.CombatTurn {
|
||||
clone := value
|
||||
if value.SourceRefs != nil {
|
||||
clone.SourceRefs = append([]source.SourceRef(nil), value.SourceRefs...)
|
||||
}
|
||||
clone.SourceRefs = cloneSourceRefs(value.SourceRefs)
|
||||
return clone
|
||||
}
|
||||
|
||||
func cloneSpellCast(value dnd.SpellCast) dnd.SpellCast {
|
||||
clone := value
|
||||
if value.SourceRefs != nil {
|
||||
clone.SourceRefs = append([]source.SourceRef(nil), value.SourceRefs...)
|
||||
}
|
||||
clone.SourceRefs = cloneSourceRefs(value.SourceRefs)
|
||||
return clone
|
||||
}
|
||||
|
||||
func cloneNPC(value dnd.NPC) dnd.NPC {
|
||||
clone := value
|
||||
if value.SourceRefs != nil {
|
||||
clone.SourceRefs = append([]source.SourceRef(nil), value.SourceRefs...)
|
||||
}
|
||||
clone.SourceRefs = cloneSourceRefs(value.SourceRefs)
|
||||
return clone
|
||||
}
|
||||
|
||||
func cloneNPCInteraction(value dnd.NPCInteraction) dnd.NPCInteraction {
|
||||
clone := value
|
||||
if value.SourceRefs != nil {
|
||||
clone.SourceRefs = append([]source.SourceRef(nil), value.SourceRefs...)
|
||||
}
|
||||
clone.SourceRefs = cloneSourceRefs(value.SourceRefs)
|
||||
return clone
|
||||
}
|
||||
|
||||
func cloneSourceRefs(refs []source.SourceRef) []source.SourceRef {
|
||||
return slices.Clone(refs)
|
||||
}
|
||||
|
||||
@@ -409,6 +409,28 @@ func TestAppendCombatTurnListsPreservesOrderPresenceAndOwnership(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendListsPreserveNestedSourceReferencePresence(t *testing.T) {
|
||||
spells, err := appendSpellLists([]dnd.SpellList{{SpellCasts: []dnd.SpellCast{{SourceRefs: []source.SourceRef{}}}}})
|
||||
if err != nil || spells.SpellCasts[0].SourceRefs == nil {
|
||||
t.Fatalf("appendSpellLists() = %#v, %v; want present-empty source refs", spells, err)
|
||||
}
|
||||
|
||||
npcs, err := appendNPCLists([]dnd.NPCList{{NPCs: []dnd.NPC{{SourceRefs: []source.SourceRef{}}}}})
|
||||
if err != nil || npcs.NPCs[0].SourceRefs == nil {
|
||||
t.Fatalf("appendNPCLists() = %#v, %v; want present-empty source refs", npcs, err)
|
||||
}
|
||||
|
||||
turns, err := appendCombatTurnLists([]dnd.CombatTurnList{{CombatTurns: []dnd.CombatTurn{{SourceRefs: []source.SourceRef{}}}}})
|
||||
if err != nil || turns.CombatTurns[0].SourceRefs == nil {
|
||||
t.Fatalf("appendCombatTurnLists() = %#v, %v; want present-empty source refs", turns, err)
|
||||
}
|
||||
|
||||
interactions, err := appendNPCInteractionLists([]dnd.NPCInteractionList{{Interactions: []dnd.NPCInteraction{{SourceRefs: []source.SourceRef{}}}}})
|
||||
if err != nil || interactions.Interactions[0].SourceRefs == nil {
|
||||
t.Fatalf("appendNPCInteractionLists() = %#v, %v; want present-empty source refs", interactions, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegisterRejectsMissingDNDDependenciesBeforeMutation(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -15,16 +15,17 @@ type SourceRefOrder struct {
|
||||
|
||||
// NewSourceRefOrder captures the source identity and unit positions from doc.
|
||||
func NewSourceRefOrder(doc *source.SourceDocument) SourceRefOrder {
|
||||
return NewSourceRefOrderWithIndex(doc, source.NewDocumentIndex(doc))
|
||||
return NewSourceRefOrderFromIndex(source.NewDocumentIndex(doc))
|
||||
}
|
||||
|
||||
// NewSourceRefOrderWithIndex captures source identity with a supplied
|
||||
// document index for source-reference comparison and canonicalization.
|
||||
func NewSourceRefOrderWithIndex(doc *source.SourceDocument, index source.DocumentIndex) SourceRefOrder {
|
||||
if doc == nil {
|
||||
// NewSourceRefOrderFromIndex captures source identity and unit positions from
|
||||
// one document index for source-reference comparison and canonicalization.
|
||||
func NewSourceRefOrderFromIndex(index source.DocumentIndex) SourceRefOrder {
|
||||
sourceID, ok := index.DocumentID()
|
||||
if !ok {
|
||||
return SourceRefOrder{}
|
||||
}
|
||||
return SourceRefOrder{sourceID: doc.ID, index: index}
|
||||
return SourceRefOrder{sourceID: sourceID, index: index}
|
||||
}
|
||||
|
||||
// Less orders references by source identity, then document positions when
|
||||
|
||||
@@ -60,19 +60,22 @@ func TestSourceRefOrderCanonicalizePreservesRepresentationAndOwnership(t *testin
|
||||
|
||||
func TestSourceRefOrderSnapshotAndEarliestValid(t *testing.T) {
|
||||
doc := unitRefSourceDocument(30, 10, 20)
|
||||
order := NewSourceRefOrder(doc)
|
||||
sourceID := doc.ID
|
||||
index := source.NewDocumentIndex(doc)
|
||||
order := NewSourceRefOrderFromIndex(index)
|
||||
doc.ID = "changed"
|
||||
doc.Units[0].ID, doc.Units[1].ID = 10, 30
|
||||
refs := []source.SourceRef{
|
||||
{SourceID: doc.ID, StartUnitID: 20, EndUnitID: 20},
|
||||
{SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10},
|
||||
{SourceID: sourceID, StartUnitID: 20, EndUnitID: 20},
|
||||
{SourceID: sourceID, StartUnitID: 10, EndUnitID: 10},
|
||||
{SourceID: "other", StartUnitID: 1, EndUnitID: 1},
|
||||
{SourceID: doc.ID, StartUnitID: 999, EndUnitID: 999},
|
||||
{SourceID: doc.ID, StartUnitID: 20, EndUnitID: 30},
|
||||
{SourceID: sourceID, StartUnitID: 999, EndUnitID: 999},
|
||||
{SourceID: sourceID, StartUnitID: 20, EndUnitID: 30},
|
||||
}
|
||||
if position, ok := order.EarliestValid(refs); !ok || position != 1 {
|
||||
t.Fatalf("EarliestValid() = %d, %t, want 1, true", position, ok)
|
||||
}
|
||||
if position, ok := order.EarliestValid([]source.SourceRef{{SourceID: doc.ID, StartUnitID: 20, EndUnitID: 30}}); ok || position != 0 {
|
||||
if position, ok := order.EarliestValid([]source.SourceRef{{SourceID: sourceID, StartUnitID: 20, EndUnitID: 30}}); ok || position != 0 {
|
||||
t.Fatalf("EarliestValid(invalid) = %d, %t, want 0, false", position, ok)
|
||||
}
|
||||
if position, ok := (SourceRefOrder{}).EarliestValid(refs); ok || position != 0 {
|
||||
|
||||
@@ -55,7 +55,7 @@ func Validate(doc *source.SourceDocument, value dnd.CombatTurnList) error {
|
||||
if !sourceRefsValid(index, value) {
|
||||
return nil
|
||||
}
|
||||
issues := issuesFor(shared.NewSourceRefOrderWithIndex(doc, index), value)
|
||||
issues := issuesFor(shared.NewSourceRefOrderFromIndex(index), value)
|
||||
if len(issues) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -94,7 +94,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq
|
||||
if !npcRegistry.Bound() {
|
||||
return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "invalid NPC interaction normalization: NPC registry reference is required"}, nil
|
||||
}
|
||||
issues := issuesFor(shared.NewSourceRefOrderWithIndex(req.Source, index), req.Value, npcRegistry)
|
||||
issues := issuesFor(shared.NewSourceRefOrderFromIndex(index), req.Value, npcRegistry)
|
||||
if len(issues) == 0 {
|
||||
return contracts.ValidationResult{Approved: true}, nil
|
||||
}
|
||||
|
||||
@@ -45,7 +45,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint {
|
||||
}
|
||||
|
||||
func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.SceneDescriptionList]) (contracts.ValidationResult, error) {
|
||||
if shape.Validate(req.Value) != nil || !sourceRefsValid(req.Source, req.Value) {
|
||||
if shape.Validate(req.Value) != nil {
|
||||
return contracts.ValidationResult{Approved: true}, nil
|
||||
}
|
||||
resolver, err := shared.NewCitationResolver(req.Source)
|
||||
@@ -72,15 +72,6 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq
|
||||
}, nil
|
||||
}
|
||||
|
||||
func sourceRefsValid(doc *source.SourceDocument, value dnd.SceneDescriptionList) bool {
|
||||
for _, scene := range value.Scenes {
|
||||
if source.ValidateRef(doc, scene.SourceRef) != nil {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func tokenSet(value string) map[string]struct{} {
|
||||
tokens := make(map[string]struct{})
|
||||
for _, token := range shared.NormalizedTokens(value) {
|
||||
|
||||
@@ -54,6 +54,14 @@ func TestValidatorUsesTranscriptOnlyAndDefersInvalidInputs(t *testing.T) {
|
||||
if err != nil || !malformed.Approved || len(malformed.Warnings) != 0 {
|
||||
t.Fatalf("malformed Validate() = %#v, %v; want deferral", malformed, err)
|
||||
}
|
||||
|
||||
invalidScene := scene("invalid", "Greencloak", "Greencloak arrives.")
|
||||
invalidScene.SourceRef.StartUnitID = 99
|
||||
invalidSource := dnd.SceneDescriptionList{Scenes: []dnd.SceneDescription{invalidScene}}
|
||||
deferred, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.SceneDescriptionList]{Source: doc, Value: invalidSource})
|
||||
if err != nil || !deferred.Approved || len(deferred.Warnings) != 0 {
|
||||
t.Fatalf("invalid-source Validate() = %#v, %v; want deferral", deferred, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidatorRegistrationAndPolicyFingerprint(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user