diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index e9c7f45..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -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. diff --git a/internal/core/source/source_test.go b/internal/core/source/source_test.go index 2bf9307..2ff4f6d 100644 --- a/internal/core/source/source_test.go +++ b/internal/core/source/source_test.go @@ -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) } diff --git a/internal/core/source/validation.go b/internal/core/source/validation.go index df1b1c6..234d5e8 100644 --- a/internal/core/source/validation.go +++ b/internal/core/source/validation.go @@ -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] diff --git a/internal/modules/dnd/normalize/combatturns/normalizer.go b/internal/modules/dnd/normalize/combatturns/normalizer.go index e69bd75..9c53c12 100644 --- a/internal/modules/dnd/normalize/combatturns/normalizer.go +++ b/internal/modules/dnd/normalize/combatturns/normalizer.go @@ -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 } diff --git a/internal/modules/dnd/normalize/npcinteractions/normalizer.go b/internal/modules/dnd/normalize/npcinteractions/normalizer.go index 259093e..e0f00f0 100644 --- a/internal/modules/dnd/normalize/npcinteractions/normalizer.go +++ b/internal/modules/dnd/normalize/npcinteractions/normalizer.go @@ -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 } diff --git a/internal/modules/dnd/normalize/spells/normalizer.go b/internal/modules/dnd/normalize/spells/normalizer.go index 486e2fc..62dfdb9 100644 --- a/internal/modules/dnd/normalize/spells/normalizer.go +++ b/internal/modules/dnd/normalize/spells/normalizer.go @@ -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...) diff --git a/internal/modules/dnd/register/merge.go b/internal/modules/dnd/register/merge.go index aab41ba..74656dc 100644 --- a/internal/modules/dnd/register/merge.go +++ b/internal/modules/dnd/register/merge.go @@ -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) +} diff --git a/internal/modules/dnd/register/register_test.go b/internal/modules/dnd/register/register_test.go index e2fb136..733f3ae 100644 --- a/internal/modules/dnd/register/register_test.go +++ b/internal/modules/dnd/register/register_test.go @@ -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 diff --git a/internal/modules/dnd/shared/source_ref_order.go b/internal/modules/dnd/shared/source_ref_order.go index 7027b75..8425294 100644 --- a/internal/modules/dnd/shared/source_ref_order.go +++ b/internal/modules/dnd/shared/source_ref_order.go @@ -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 diff --git a/internal/modules/dnd/shared/source_ref_order_test.go b/internal/modules/dnd/shared/source_ref_order_test.go index fd74d50..46ff3b6 100644 --- a/internal/modules/dnd/shared/source_ref_order_test.go +++ b/internal/modules/dnd/shared/source_ref_order_test.go @@ -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 { diff --git a/internal/modules/dnd/validate/combatturns/invariants/validator.go b/internal/modules/dnd/validate/combatturns/invariants/validator.go index 0a4ec9b..c314fe5 100644 --- a/internal/modules/dnd/validate/combatturns/invariants/validator.go +++ b/internal/modules/dnd/validate/combatturns/invariants/validator.go @@ -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 } diff --git a/internal/modules/dnd/validate/npcinteractions/invariants/validator.go b/internal/modules/dnd/validate/npcinteractions/invariants/validator.go index a61a40d..f12d544 100644 --- a/internal/modules/dnd/validate/npcinteractions/invariants/validator.go +++ b/internal/modules/dnd/validate/npcinteractions/invariants/validator.go @@ -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 } diff --git a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go index 29b28b9..ee40a4f 100644 --- a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go @@ -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) { diff --git a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go index 4ec9631..5e615c6 100644 --- a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go @@ -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) {