diff --git a/docs/integrations/dnd-spell-artifacts.md b/docs/integrations/dnd-spell-artifacts.md index 2baaefa..3bac4b8 100644 --- a/docs/integrations/dnd-spell-artifacts.md +++ b/docs/integrations/dnd-spell-artifacts.md @@ -6,6 +6,7 @@ extractor. Selectable extractor keys are cataloged in ## Identity +- Artifact kind: `dnd/spell-list` - Prompt ID: `dnd.spells` - Response schema key: `dnd_spells` - Response schema ID: `notarius.dnd.spells` @@ -13,6 +14,12 @@ extractor. Selectable extractor keys are cataloged in - Response schema version: `v1` - Media type: `application/json` +The durable JSON Schema is owned by the D&D spell artifact codec. The +extractor's private LLM response schema is a separate transport contract: its +source-reference objects omit `source_id`, which the extractor assigns while +mapping the response to the canonical artifact. The LLM DTO and transport +schema are not part of this durable contract. + The output contains canonical spell casts derived from transcript evidence. Source IDs are assigned from the input identity; source-unit ranges identify the evidence location. diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 8af17e6..fe0c444 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -17,15 +17,14 @@ validator registry. Package-family registrars compose those leaf registrations into the production catalog and own family-level policy such as default validator chains and prompt asset collection. -Production input, chunk, and output packages register strict option decoders and -run-local builders. Preparation decodes their options into implementation-owned -values and injects dependencies; their operation requests contain run context, -not raw option maps or LLM clients. Production extract, merge, normalize, and -validator packages still use the explicit legacy raw registration APIs and -temporary adapters around zero-argument constructors. Their raw option maps and -LLM clients remain operation inputs while that part of the catalog migrates. -Typed registration is framework-ready, but no production artifact kind or -codec is registered yet. +Production input, chunk, output, and D&D spell-extract packages register strict +option decoders and run-local builders. Preparation decodes their options into +implementation-owned values and injects dependencies. The spell extractor is +typed over the canonical D&D model; a temporary raw adapter preserves the +current downstream production path. Production merge, normalize, and validator +packages still use the explicit legacy raw registration APIs and temporary +adapters around zero-argument constructors. Their raw option maps and LLM +clients remain operation inputs while that part of the catalog migrates. Specs expose capability and execution metadata without constructing an implementation. Registry entries separately expose option validation and @@ -106,12 +105,16 @@ The spell extractor prepares a structured request from one chunk, the chunk-scoped source input, the session, and optional D&D reference inputs. It decodes the model response, assigns the generic source identity to every source reference, canonicalizes duplicate references, orders spell casts by their -earliest cited unit, and returns raw JSON plus response-schema provenance. +earliest cited unit, and returns `dnd.SpellList`. -The package owns its embedded prompt, response schemas, and prompt/schema -manifest metadata. Shared D&D helpers keep prompt input names and source-unit -reference conversion consistent with the scene chunker. The extractor produces -raw output; production validators own approval policy. +The extractor owns its private model-response DTO, embedded prompt, LLM response +schema, strict option decoder, injected shared LLM client, and prompt/schema +manifest metadata. The separate `internal/modules/dnd/codec/spells` package +owns the durable schema and stable JSON representation for artifact kind +`dnd/spell-list`. Production composition currently wraps the typed extractor +with a raw adapter that encodes through this codec, so existing validators and +later stages remain unchanged. Shared D&D helpers keep prompt input names and +source-unit reference conversion consistent with the scene chunker. The durable payload and manifest metadata shapes are defined in the [D&D spell artifact contract](../integrations/dnd-spell-artifacts.md). diff --git a/docs/internal/overview.md b/docs/internal/overview.md index ddcc97a..b9db5f7 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -62,10 +62,12 @@ run-local construction closures. Preparation injects shared dependencies and constructs input, chunk, validators, ordered lanes, and output before source parsing. Production input, chunk, and output modules use strict construction-time option decoding, and the LLM-backed scene chunker retains the injected shared -client. Production artifact-lane modules and validators do not register typed -variants yet and continue through explicitly named legacy raw registrations and -temporary zero-argument constructor adapters. The current runner rejects a -typed prepared lane instead of routing it through raw execution. +client. The D&D family registers the canonical `dnd/spell-list` codec and a +typed spell extractor. A temporary raw adapter serializes that typed result for +the still-raw production validators, merger, normalizer, and runner. Other +artifact-lane modules and validators continue through explicitly named legacy +raw registrations and temporary zero-argument constructor adapters. The current +runner rejects a typed prepared lane instead of routing it through raw execution. ## Production Extensions @@ -79,7 +81,9 @@ Configuration. The implemented module packages are: | `internal/modules/seriatim/input/transcript` | Parses the supported Seriatim transcript format into the generic source model. | | `internal/modules/generic/chunk/units` | Splits ordered source units by unit count and overlap. | | `internal/modules/dnd/chunk/scenes` | Produces contiguous D&D scene chunks from structured model output. | -| `internal/modules/dnd/extract/spells` | Produces source-grounded D&D spell-cast raw output. | +| `internal/modules/dnd` | Owns the canonical D&D spell-list and spell-cast artifact types. | +| `internal/modules/dnd/codec/spells` | Strictly decodes and stably encodes the durable D&D spell-list representation. | +| `internal/modules/dnd/extract/spells` | Maps private structured model output to canonical source-grounded D&D spell lists. | | `internal/modules/generic/merge/appendorder` | Combines accepted extraction results in chunk order. | | `internal/modules/generic/normalize/noop` | Preserves accepted merged output. | | `internal/modules/generic/output/json` | Encodes manifests, lane payloads, warnings, and rejections as logical JSON files. | diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 4a3c77a..60aa3a3 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -73,9 +73,11 @@ separate target namespaces; serialized registrations declare whether they support chunks, artifacts, or both. Duplicate variants and exact Go-type mismatches are rejected deterministically. -Production composition initializes the artifact codec registry without adding -codec entries, and production artifact-lane modules use the explicitly named -legacy raw registration APIs. A raw registration cannot satisfy a typed lane. +Production composition registers the D&D spell-list codec. The typed spell +extractor also registers a temporary raw adapter, which resolution selects +until its downstream production lane is typed. Other production artifact-lane +modules use the explicitly named legacy raw registration APIs. A standalone raw +registration cannot satisfy a typed lane. A `ModuleSpec` declares its stage plus required and provided capabilities. Chunk, extract, merge, and normalize specs may also declare reference slots. @@ -84,8 +86,10 @@ and verify that a constructed implementation reports the registered key. Builder registrations accept `ModuleDependencies` and cloned raw options through one `BuildRequest`. Production input, chunk, and output builders decode those options and retain typed values or injected dependencies in the constructed -implementation. Remaining production raw-stage registrations are adapted from -their zero-argument constructors during migration. +implementation. A typed extractor registration may explicitly supply a raw +adapter builder for a still-raw downstream lane; the adapter is selected as one +unit and does not expose the typed value to raw consumers. Remaining production +raw-stage registrations are adapted from their zero-argument constructors. A `ValidatorSpec` declares a validator key and execution class. Resolution uses the execution class to reject incompatible profile bindings before execution. diff --git a/internal/cli/compatibility_test.go b/internal/cli/compatibility_test.go index 3a5060d..d581f6a 100644 --- a/internal/cli/compatibility_test.go +++ b/internal/cli/compatibility_test.go @@ -106,13 +106,12 @@ func TestProductionCompatibilitySnapshot(t *testing.T) { }) assertAssetNames(t, assets.SchemaFS, []string{ "dnd_scenes.v1.json", - "dnd_spells.v1.json", "dnd_spells_llm.v1.json", }) identitySnapshot := map[string]map[string]any{ "scenes": sceneManifestMetadata(t), - "spells": spells.New().ManifestMetadata(), + "spells": spellManifestMetadata(t), } for name, metadata := range identitySnapshot { for _, key := range []string{"prompt_id", "prompt_version", "prompt_sha256", "response_schema_key", "response_schema_id", "response_schema_name", "response_schema_version", "response_schema_sha256"} { @@ -348,7 +347,10 @@ func TestProductionLLMCallersShareScheduledClient(t *testing.T) { }() go func() { started.Done() - _, err := spells.New().Extract(context.Background(), contracts.ExtractionRequest{Source: doc, Chunk: &chunk, LLMClient: client}) + extractor, err := spells.New(client, spells.Options{}) + if err == nil { + _, err = extractor.Extract(context.Background(), contracts.TypedExtractionRequest{Source: doc, Chunk: &chunk}) + } errs <- err }() started.Wait() @@ -520,6 +522,15 @@ func sceneManifestMetadata(t *testing.T) map[string]any { return chunker.ManifestMetadata() } +func spellManifestMetadata(t *testing.T) map[string]any { + t.Helper() + extractor, err := spells.New(orderingProductionLLMClient{}, spells.Options{}) + if err != nil { + t.Fatalf("construct spell extractor: %v", err) + } + return extractor.ManifestMetadata() +} + func assertAssetNames(t *testing.T, getFS func() (fs.FS, error), want []string) { t.Helper() fSys, err := getFS() diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index ad9bc3b..2bb0691 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -138,8 +138,8 @@ func TestProductionCatalogIncludesProductionModulesValidatorsAndDefaults(t *test if catalog.ArtifactCodecs == nil { t.Fatal("production artifact codec registry = nil, want initialized empty registry") } - if got := catalog.ArtifactCodecs.RegisteredKinds(); len(got) != 0 { - t.Fatalf("production artifact codec kinds = %#v, want legacy raw production path", got) + if got := catalog.ArtifactCodecs.RegisteredKinds(); !reflect.DeepEqual(got, []contracts.ArtifactKind{"dnd/spell-list"}) { + t.Fatalf("production artifact codec kinds = %#v, want dnd/spell-list", got) } moduleTests := []struct { diff --git a/internal/framework/pipeline/extractor_registry.go b/internal/framework/pipeline/extractor_registry.go index 2c9011c..0a735c7 100644 --- a/internal/framework/pipeline/extractor_registry.go +++ b/internal/framework/pipeline/extractor_registry.go @@ -23,6 +23,7 @@ type typedExtractorEntry struct { valueType reflect.Type validateOptions OptionValidator builder func(BuildRequest) (any, error) + rawBuilder LegacyRawExtractorBuilder } func NewExtractorRegistry() *ExtractorRegistry { @@ -92,6 +93,20 @@ func RegisterExtractor[T any](registry *ExtractorRegistry, spec ModuleSpec, cons } func RegisterExtractorBuilder[T any](registry *ExtractorRegistry, spec ModuleSpec, validateOptions OptionValidator, builder func(BuildRequest) (contracts.Extractor[T], error)) error { + return registerExtractorBuilder(registry, spec, validateOptions, builder, nil) +} + +// RegisterExtractorBuilderWithRawAdapter registers a typed extractor while a +// raw downstream remains in use. Resolution selects the adapter until the +// registration is replaced with the typed-only builder. +func RegisterExtractorBuilderWithRawAdapter[T any](registry *ExtractorRegistry, spec ModuleSpec, validateOptions OptionValidator, builder func(BuildRequest) (contracts.Extractor[T], error), rawBuilder LegacyRawExtractorBuilder) error { + if rawBuilder == nil { + return fmt.Errorf("extractor raw adapter builder for %q must not be nil", strings.TrimSpace(spec.Key)) + } + return registerExtractorBuilder(registry, spec, validateOptions, builder, rawBuilder) +} + +func registerExtractorBuilder[T any](registry *ExtractorRegistry, spec ModuleSpec, validateOptions OptionValidator, builder func(BuildRequest) (contracts.Extractor[T], error), rawBuilder LegacyRawExtractorBuilder) error { if registry == nil { return fmt.Errorf("extractor registry must not be nil") } @@ -118,6 +133,7 @@ func RegisterExtractorBuilder[T any](registry *ExtractorRegistry, spec ModuleSpe builder: func(request BuildRequest) (any, error) { return builder(cloneBuildRequest(request)) }, + rawBuilder: rawBuilder, } if registry.typedEntries == nil { registry.typedEntries = make(map[string]typedExtractorEntry) @@ -143,6 +159,12 @@ func (r *ExtractorRegistry) BuildLegacyRawWithRequest(key string, request BuildR return nil, fmt.Errorf("extractor key must not be empty") } builder, ok := r.legacyBuilders[normalizedKey] + if !ok { + if entry, typedOK := r.typedEntries[normalizedKey]; typedOK && entry.rawBuilder != nil { + builder = entry.rawBuilder + ok = true + } + } if !ok { return nil, fmt.Errorf("legacy raw extractor %q is not registered", normalizedKey) } @@ -193,6 +215,11 @@ func (r *ExtractorRegistry) typedEntry(key string) (typedExtractorEntry, bool) { return entry, ok } +func (r *ExtractorRegistry) usesRawAdapter(key string) bool { + entry, ok := r.typedEntry(key) + return ok && entry.rawBuilder != nil +} + func (r *ExtractorRegistry) RegisteredKeys() []string { if r == nil { return nil diff --git a/internal/framework/pipeline/extractor_registry_test.go b/internal/framework/pipeline/extractor_registry_test.go index 4ee11a4..7992387 100644 --- a/internal/framework/pipeline/extractor_registry_test.go +++ b/internal/framework/pipeline/extractor_registry_test.go @@ -353,10 +353,48 @@ func TestExtractorRegistryBuildRejectsEmptyKey(t *testing.T) { } } +func TestExtractorRegistryTypedRegistrationCanProvideRawAdapter(t *testing.T) { + registry := NewExtractorRegistry() + spec := ModuleSpec{Key: "typed-extractor", Stage: StageExtract, ArtifactKind: "test/value"} + if err := RegisterExtractorBuilderWithRawAdapter(registry, spec, func(map[string]any) error { return nil }, + func(BuildRequest) (contracts.Extractor[registryTypedValue], error) { + return registryTypedExtractor{key: spec.Key}, nil + }, + func(BuildRequest) (contracts.LegacyRawExtractor, error) { + return registryFakeExtractor{key: spec.Key}, nil + }, + ); err != nil { + t.Fatalf("RegisterExtractorBuilderWithRawAdapter() error = %v", err) + } + if !registry.usesRawAdapter(spec.Key) { + t.Fatal("usesRawAdapter() = false, want true") + } + if _, ok := registry.typedEntry(spec.Key); !ok { + t.Fatal("typedEntry() ok = false, want true") + } + adapter, err := registry.BuildLegacyRaw(spec.Key) + if err != nil || adapter.Key() != spec.Key { + t.Fatalf("BuildLegacyRaw() = %#v, %v", adapter, err) + } + if err := RegisterExtractorBuilderWithRawAdapter[registryTypedValue](NewExtractorRegistry(), spec, func(map[string]any) error { return nil }, nil, nil); err == nil || !strings.Contains(err.Error(), "raw adapter") { + t.Fatalf("nil raw builder error = %v, want raw adapter context", err) + } +} + type registryFakeExtractor struct { key string } +type registryTypedValue struct{ Value string } + +type registryTypedExtractor struct{ key string } + +func (extractor registryTypedExtractor) Key() string { return extractor.key } +func (registryTypedExtractor) ReferenceSlots() []contracts.ReferenceSlot { return nil } +func (registryTypedExtractor) Extract(context.Context, contracts.TypedExtractionRequest) (contracts.TypedExtractionResult[registryTypedValue], error) { + return contracts.TypedExtractionResult[registryTypedValue]{}, nil +} + func fakeExtractorConstructor(key string) LegacyRawExtractorConstructor { return func() (contracts.LegacyRawExtractor, error) { return registryFakeExtractor{key: key}, nil diff --git a/internal/framework/pipeline/profile.go b/internal/framework/pipeline/profile.go index 0084e73..1c45292 100644 --- a/internal/framework/pipeline/profile.go +++ b/internal/framework/pipeline/profile.go @@ -393,6 +393,9 @@ func resolveArtifactIdentity(pipelineID, laneID string, lane *ResolvedArtifactLa if !ok { return nil, fmt.Errorf("pipeline %q lane %q extract module %q declares artifact kind %q without a typed registration", pipelineID, laneID, lane.Extract.Module, extractSpec.ArtifactKind) } + if catalog.Extractors.usesRawAdapter(lane.Extract.Module) { + return nil, nil + } if catalog.ArtifactCodecs == nil { return nil, fmt.Errorf("pipeline %q lane %q artifact codec registry must not be nil for kind %q", pipelineID, laneID, extractSpec.ArtifactKind) } @@ -639,12 +642,16 @@ func validatePipelineReferenceDefaults( merge := resolveBinding(laneProfile.Merge, DefaultMergeModule) var artifactType reflect.Type - if extractSpec.ArtifactKind != "" && catalog.Extractors != nil { + artifactKind := extractSpec.ArtifactKind + if catalog.Extractors != nil && catalog.Extractors.usesRawAdapter(extract.Module) { + artifactKind = "" + } + if artifactKind != "" && catalog.Extractors != nil { if entry, ok := catalog.Extractors.typedEntry(extract.Module); ok { artifactType = entry.valueType } } - mergeSpec, err := mergerSpecForArtifact(catalog, merge.Module, extractSpec.ArtifactKind, artifactType) + mergeSpec, err := mergerSpecForArtifact(catalog, merge.Module, artifactKind, artifactType) if err != nil { return moduleLookupError(pipelineID, laneID, StageMerge, merge.Module, err) } @@ -653,7 +660,7 @@ func validatePipelineReferenceDefaults( } normalize := resolveBinding(laneProfile.Normalize, DefaultNormalizeModule) - normalizeSpec, err := normalizerSpecForArtifact(catalog, normalize.Module, extractSpec.ArtifactKind, artifactType) + normalizeSpec, err := normalizerSpecForArtifact(catalog, normalize.Module, artifactKind, artifactType) if err != nil { return moduleLookupError(pipelineID, laneID, StageNormalize, normalize.Module, err) } diff --git a/internal/modules/dnd/extract/spells/assets/schemas/dnd_spells.v1.json b/internal/modules/dnd/codec/spells/assets/schemas/dnd_spells.v1.json similarity index 100% rename from internal/modules/dnd/extract/spells/assets/schemas/dnd_spells.v1.json rename to internal/modules/dnd/codec/spells/assets/schemas/dnd_spells.v1.json diff --git a/internal/modules/dnd/codec/spells/codec.go b/internal/modules/dnd/codec/spells/codec.go new file mode 100644 index 0000000..062ca13 --- /dev/null +++ b/internal/modules/dnd/codec/spells/codec.go @@ -0,0 +1,110 @@ +package spells + +import ( + "bytes" + "embed" + "encoding/json" + "fmt" + "io" + "strings" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" +) + +const ( + SchemaID = "notarius.dnd.spells" + SchemaName = "notarius_dnd_spells_v1" + SchemaVersion = "v1" + MediaType = "application/json" +) + +//go:embed assets/schemas/dnd_spells.v1.json +var schemaAssets embed.FS + +var _ contracts.ArtifactCodec[dnd.SpellList] = (*Codec)(nil) + +type Codec struct{} + +func New() *Codec { return &Codec{} } + +func (c *Codec) Kind() contracts.ArtifactKind { return dnd.SpellListKind } + +func (c *Codec) Schema() contracts.ArtifactSchema { + raw, err := schemaAssets.ReadFile("assets/schemas/dnd_spells.v1.json") + if err != nil { + return contracts.ArtifactSchema{} + } + return contracts.ArtifactSchema{ID: SchemaID, Name: SchemaName, Version: SchemaVersion, JSONSchema: raw} +} + +func (c *Codec) MediaType() string { return MediaType } + +func (c *Codec) Encode(value dnd.SpellList) ([]byte, error) { + if err := validate(value); err != nil { + return nil, fmt.Errorf("encode dnd spell list: %w", err) + } + return c.EncodeCandidate(value) +} + +// EncodeCandidate provides the same stable representation before typed +// validators have approved a value on the temporary raw downstream path. +func (c *Codec) EncodeCandidate(value dnd.SpellList) ([]byte, error) { + content, err := json.Marshal(value) + if err != nil { + return nil, fmt.Errorf("encode dnd spell list: %w", err) + } + return content, nil +} + +func (c *Codec) Decode(content []byte) (dnd.SpellList, error) { + decoder := json.NewDecoder(bytes.NewReader(content)) + decoder.DisallowUnknownFields() + var value dnd.SpellList + if err := decoder.Decode(&value); err != nil { + return dnd.SpellList{}, fmt.Errorf("decode dnd spell list: %w", err) + } + var trailing any + if err := decoder.Decode(&trailing); err != io.EOF { + return dnd.SpellList{}, fmt.Errorf("decode dnd spell list: multiple JSON values") + } + if err := validate(value); err != nil { + return dnd.SpellList{}, fmt.Errorf("decode dnd spell list: %w", err) + } + return value, nil +} + +func validate(value dnd.SpellList) error { + if value.SpellCasts == nil { + return fmt.Errorf("spell_casts must be present") + } + for index, spell := range value.SpellCasts { + if strings.TrimSpace(spell.Caster) == "" { + return fmt.Errorf("spell_casts[%d].caster must not be empty", index) + } + if strings.TrimSpace(spell.Spell) == "" { + return fmt.Errorf("spell_casts[%d].spell must not be empty", index) + } + if strings.TrimSpace(spell.Effect) == "" { + return fmt.Errorf("spell_casts[%d].effect must not be empty", index) + } + if strings.TrimSpace(spell.NarrativeDescription) == "" { + return fmt.Errorf("spell_casts[%d].narrative_description must not be empty", index) + } + if len(spell.SourceRefs) == 0 { + return fmt.Errorf("spell_casts[%d].source_refs must not be empty", index) + } + for refIndex, ref := range spell.SourceRefs { + if strings.TrimSpace(ref.SourceID) == "" { + return fmt.Errorf("spell_casts[%d].source_refs[%d].source_id must not be empty", index, refIndex) + } + if ref.StartUnitID <= 0 { + return fmt.Errorf("spell_casts[%d].source_refs[%d].start_unit_id must be positive", index, refIndex) + } + if ref.EndUnitID <= 0 { + return fmt.Errorf("spell_casts[%d].source_refs[%d].end_unit_id must be positive", index, refIndex) + } + } + } + return nil +} diff --git a/internal/modules/dnd/codec/spells/codec_test.go b/internal/modules/dnd/codec/spells/codec_test.go new file mode 100644 index 0000000..26b5716 --- /dev/null +++ b/internal/modules/dnd/codec/spells/codec_test.go @@ -0,0 +1,111 @@ +package spells + +import ( + "bytes" + "encoding/json" + "os" + "reflect" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/core/source" + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" + "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" +) + +func TestCodecMatchesMaintainedDurableFixture(t *testing.T) { + raw, err := os.ReadFile("testdata/dnd_spells.v1.json") + if err != nil { + t.Fatalf("read durable fixture: %v", err) + } + codec := New() + value, err := codec.Decode(raw) + if err != nil { + t.Fatalf("Decode() error = %v, want nil", err) + } + want := dnd.SpellList{SpellCasts: []dnd.SpellCast{ + {Caster: "Aria", Spell: "Cure Wounds", Effect: "Heals an injured ally.", NarrativeDescription: "Aria restores the fighter after the fight.", SourceRefs: []source.SourceRef{{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}}}, + {Caster: "Borin", Spell: "Fire Bolt", Effect: "Scorches the wight.", NarrativeDescription: "Borin hurls fire at the wight.", SourceRefs: []source.SourceRef{{SourceID: "session-alpha", StartUnitID: 3, EndUnitID: 3}}}, + }} + if !reflect.DeepEqual(value, want) { + t.Fatalf("Decode() = %#v, want %#v", value, want) + } + encoded, err := codec.Encode(value) + if err != nil { + t.Fatalf("Encode() error = %v, want nil", err) + } + var compact bytes.Buffer + if err := json.Compact(&compact, raw); err != nil { + t.Fatalf("compact durable fixture: %v", err) + } + if !bytes.Equal(encoded, compact.Bytes()) { + t.Fatalf("Encode() = %s, want stable durable JSON %s", encoded, compact.Bytes()) + } + second, err := codec.Encode(value) + if err != nil || !bytes.Equal(second, encoded) { + t.Fatalf("second Encode() = %s, %v; want deterministic bytes", second, err) + } +} + +func TestCodecOwnsDurableSchemaAndRegistersExactType(t *testing.T) { + codec := New() + schema := codec.Schema() + if codec.Kind() != dnd.SpellListKind || codec.MediaType() != MediaType { + t.Fatalf("codec identity = %q/%q", codec.Kind(), codec.MediaType()) + } + if schema.ID != SchemaID || schema.Name != SchemaName || schema.Version != SchemaVersion || !json.Valid(schema.JSONSchema) { + t.Fatalf("schema = %#v, want durable spell schema", schema) + } + var document map[string]any + if err := json.Unmarshal(schema.JSONSchema, &document); err != nil || document["$id"] != SchemaID { + t.Fatalf("durable schema document = %#v, %v", document, err) + } + + registry := pipeline.NewArtifactCodecRegistry() + if err := pipeline.RegisterArtifactCodec(registry, codec); err != nil { + t.Fatalf("RegisterArtifactCodec() error = %v", err) + } + spec, ok := registry.Spec(dnd.SpellListKind) + if !ok || spec.SchemaDigest != contracts.DigestArtifactSchema(schema) { + t.Fatalf("registered spec = %#v, %t", spec, ok) + } +} + +func TestCodecStrictlyRejectsInvalidRepresentations(t *testing.T) { + codec := New() + tests := []struct { + name string + raw string + want string + }{ + {name: "unknown", raw: `{"spell_casts":[],"unexpected":true}`, want: "unknown field"}, + {name: "trailing", raw: `{"spell_casts":[]} {}`, want: "multiple JSON values"}, + {name: "missing", raw: `{}`, want: "spell_casts must be present"}, + {name: "invalid evidence", raw: `{"spell_casts":[{"caster":"Aria","spell":"Cure Wounds","effect":"Heals","narrative_description":"Aria heals","source_refs":[{"source_id":"session","start_unit_id":0,"end_unit_id":1}]}]}`, want: "start_unit_id"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := codec.Decode([]byte(test.raw)) + if err == nil || !strings.Contains(err.Error(), test.want) { + t.Fatalf("Decode() error = %v, want %q", err, test.want) + } + }) + } +} + +func TestCodecRejectsInvalidCanonicalValues(t *testing.T) { + _, err := New().Encode(dnd.SpellList{}) + if err == nil || !strings.Contains(err.Error(), "spell_casts must be present") { + t.Fatalf("Encode() error = %v, want strict shape error", err) + } +} + +func TestCodecSchemaIsMutationSafe(t *testing.T) { + first := New().Schema() + first.JSONSchema[0] = '[' + second := New().Schema() + if !json.Valid(second.JSONSchema) || second.JSONSchema[0] == '[' { + t.Fatalf("Schema() returned shared bytes: %s", second.JSONSchema) + } +} diff --git a/internal/modules/dnd/codec/spells/testdata/dnd_spells.v1.json b/internal/modules/dnd/codec/spells/testdata/dnd_spells.v1.json new file mode 100644 index 0000000..ba0763c --- /dev/null +++ b/internal/modules/dnd/codec/spells/testdata/dnd_spells.v1.json @@ -0,0 +1,30 @@ +{ + "spell_casts": [ + { + "caster": "Aria", + "spell": "Cure Wounds", + "effect": "Heals an injured ally.", + "narrative_description": "Aria restores the fighter after the fight.", + "source_refs": [ + { + "source_id": "session-alpha", + "start_unit_id": 1, + "end_unit_id": 2 + } + ] + }, + { + "caster": "Borin", + "spell": "Fire Bolt", + "effect": "Scorches the wight.", + "narrative_description": "Borin hurls fire at the wight.", + "source_refs": [ + { + "source_id": "session-alpha", + "start_unit_id": 3, + "end_unit_id": 3 + } + ] + } + ] +} diff --git a/internal/modules/dnd/extract/spells/assets.go b/internal/modules/dnd/extract/spells/assets.go index 8d9c3aa..3baa086 100644 --- a/internal/modules/dnd/extract/spells/assets.go +++ b/internal/modules/dnd/extract/spells/assets.go @@ -2,5 +2,5 @@ package spells import "embed" -//go:embed assets/schemas/*.json assets/prompts/*.yaml assets/prompts/*.md +//go:embed assets/schemas/dnd_spells_llm.v1.json assets/prompts/*.yaml assets/prompts/*.md var embeddedAssets embed.FS diff --git a/internal/modules/dnd/extract/spells/canonicalize.go b/internal/modules/dnd/extract/spells/canonicalize.go index a1f8128..2031e7e 100644 --- a/internal/modules/dnd/extract/spells/canonicalize.go +++ b/internal/modules/dnd/extract/spells/canonicalize.go @@ -3,15 +3,17 @@ package spells import ( "sort" + "gitea.maximumdirect.net/eric/notarius/internal/core/source" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) -func canonicalizeResponse(response *extractionResponse, sourceID string) { +func canonicalizeResponse(response *extractionResponse) { if response == nil { return } for index := range response.SpellCasts { - canonicalizeSpellCast(&response.SpellCasts[index], sourceID) + canonicalizeSpellCast(&response.SpellCasts[index]) } sort.SliceStable(response.SpellCasts, func(i, j int) bool { left, leftOK := earliestSourceUnit(response.SpellCasts[i]) @@ -26,9 +28,8 @@ func canonicalizeResponse(response *extractionResponse, sourceID string) { }) } -func canonicalizeSpellCast(spell *spellCastResponse, sourceID string) { +func canonicalizeSpellCast(spell *spellCastResponse) { for index := range spell.SourceRefs { - spell.SourceRefs[index].SourceID = sourceID spell.SourceRefs[index].StartUnitID = canonicalUnitRef(spell.SourceRefs[index].StartUnitID) spell.SourceRefs[index].EndUnitID = canonicalUnitRef(spell.SourceRefs[index].EndUnitID) } @@ -51,12 +52,12 @@ func canonicalUnitRef(ref shared.UnitRef) shared.UnitRef { return shared.UnitRefFromInt(value) } -func dedupeSourceRefs(refs []shared.SourceRefResponse) []shared.SourceRefResponse { +func dedupeSourceRefs(refs []spellSourceRefResponse) []spellSourceRefResponse { if len(refs) < 2 { return refs } out := refs[:0] - var previous shared.SourceRefResponse + var previous spellSourceRefResponse for index, ref := range refs { if index > 0 && sameSourceRef(previous, ref) { continue @@ -67,9 +68,8 @@ func dedupeSourceRefs(refs []shared.SourceRefResponse) []shared.SourceRefRespons return out } -func sameSourceRef(left shared.SourceRefResponse, right shared.SourceRefResponse) bool { - return left.SourceID == right.SourceID && - left.StartUnitID.Int() == right.StartUnitID.Int() && +func sameSourceRef(left spellSourceRefResponse, right spellSourceRefResponse) bool { + return left.StartUnitID.Int() == right.StartUnitID.Int() && left.EndUnitID.Int() == right.EndUnitID.Int() } @@ -90,3 +90,28 @@ func unitSortValue(ref shared.UnitRef) int { } return value } + +func canonicalSpellList(response extractionResponse, sourceID string) dnd.SpellList { + spellCasts := make([]dnd.SpellCast, len(response.SpellCasts)) + for index, spell := range response.SpellCasts { + refs := make([]source.SourceRef, len(spell.SourceRefs)) + for refIndex, ref := range spell.SourceRefs { + refs[refIndex] = source.SourceRef{ + SourceID: sourceID, + StartUnitID: ref.StartUnitID.Int(), + EndUnitID: ref.EndUnitID.Int(), + } + } + spellCasts[index] = dnd.SpellCast{ + Caster: spell.Caster, + Spell: spell.Spell, + Effect: spell.Effect, + NarrativeDescription: spell.NarrativeDescription, + SourceRefs: refs, + } + } + if response.SpellCasts == nil { + spellCasts = nil + } + return dnd.SpellList{SpellCasts: spellCasts} +} diff --git a/internal/modules/dnd/extract/spells/extractor.go b/internal/modules/dnd/extract/spells/extractor.go index 30526c5..f10da67 100644 --- a/internal/modules/dnd/extract/spells/extractor.go +++ b/internal/modules/dnd/extract/spells/extractor.go @@ -3,11 +3,11 @@ package spells import ( "bytes" "context" - "encoding/json" "fmt" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) @@ -31,12 +31,31 @@ var referenceSlotDescriptions = shared.ReferenceSlotDescriptions{ Roster: "Deprecated alias for party roster reference material used only for disambiguation.", } -var _ contracts.LegacyRawExtractor = (*Extractor)(nil) +var _ contracts.Extractor[dnd.SpellList] = (*Extractor)(nil) -type Extractor struct{} +type Options struct{} -func New() *Extractor { - return &Extractor{} +type Extractor struct { + llm contracts.StructuredLLMClient +} + +type rawAdapter struct { + extractor *Extractor + codec RawAdapterCodec +} + +type RawAdapterCodec interface { + contracts.ArtifactCodec[dnd.SpellList] + EncodeCandidate(dnd.SpellList) ([]byte, error) +} + +var _ contracts.LegacyRawExtractor = (*rawAdapter)(nil) + +func New(llmClient contracts.StructuredLLMClient, _ Options) (*Extractor, error) { + if llmClient == nil { + return nil, extractorErrorf("LLM client must not be nil") + } + return &Extractor{llm: llmClient}, nil } func (e *Extractor) Key() string { @@ -67,35 +86,35 @@ func (e *Extractor) ManifestMetadata() map[string]any { return metadata } -func (e *Extractor) Extract(ctx context.Context, req contracts.ExtractionRequest) (contracts.ExtractionResult, error) { +func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRequest) (contracts.TypedExtractionResult[dnd.SpellList], error) { if e == nil { - return contracts.ExtractionResult{}, extractorErrorf("extractor must not be nil") + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("extractor must not be nil") + } + if e.llm == nil { + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("LLM client must not be nil") } if ctx == nil { - return contracts.ExtractionResult{}, extractorErrorf("context must not be nil") + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("context must not be nil") } if err := ctx.Err(); err != nil { - return contracts.ExtractionResult{}, extractorErrorf("context error before extraction: %w", err) + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("context error before extraction: %w", err) } if req.Source == nil { - return contracts.ExtractionResult{}, extractorErrorf("source must not be nil") + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("source must not be nil") } if req.Chunk == nil { - return contracts.ExtractionResult{}, extractorErrorf("chunk must not be nil") + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("chunk must not be nil") } if len(req.Chunk.Units) == 0 { - return contracts.ExtractionResult{}, extractorErrorf("chunk %q units must not be empty", req.Chunk.ID) - } - if req.LLMClient == nil { - return contracts.ExtractionResult{}, extractorErrorf("LLM client must not be nil") + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("chunk %q units must not be empty", req.Chunk.ID) } sourceInput, err := chunkSourceInput(req) if err != nil { - return contracts.ExtractionResult{}, err + return contracts.TypedExtractionResult[dnd.SpellList]{}, err } var response extractionResponse - if _, err := req.LLMClient.CompleteStructured(ctx, contracts.StructuredCompletionRequest{ + if _, err := e.llm.CompleteStructured(ctx, contracts.StructuredCompletionRequest{ StageName: Key, PromptID: PromptID, PromptVersion: SchemaVersion, @@ -103,37 +122,13 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.ExtractionRequest SessionID: req.SessionID, Inputs: shared.PromptInputs(sourceInput, req.References), }, &response); err != nil { - return contracts.ExtractionResult{}, extractorErrorf("complete structured output: %w", err) + return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("complete structured output: %w", err) } - canonicalizeResponse(&response, req.Source.ID) - content, err := json.Marshal(response) - if err != nil { - return contracts.ExtractionResult{}, extractorErrorf("marshal canonical output: %w", err) - } - schema, err := loadResponseSchema() - if err != nil { - return contracts.ExtractionResult{}, extractorErrorf("load response schema: %w", err) - } - return contracts.ExtractionResult{ - Output: contracts.ExtractOutput{ - Schema: contracts.ResponseSchema{ - ID: ResponseSchemaID, - Name: ResponseSchemaName, - Version: SchemaVersion, - JSONSchema: append([]byte(nil), schema.JSONSchema...), - }, - Payload: contracts.RawPayload{ - Content: content, - MediaType: "application/json", - Metadata: map[string]any{ - "spell_cast_count": len(response.SpellCasts), - }, - }, - }, - }, nil + canonicalizeResponse(&response) + return contracts.TypedExtractionResult[dnd.SpellList]{Value: canonicalSpellList(response, req.Source.ID)}, nil } -func chunkSourceInput(req contracts.ExtractionRequest) (contracts.LLMInputMaterial, error) { +func chunkSourceInput(req contracts.TypedExtractionRequest) (contracts.LLMInputMaterial, error) { material := req.SourceInput.Clone() if len(material.Content) == 0 { material = contracts.NewLLMInputMaterial("source", req.Chunk.MediaType, req.Chunk.Content, "", "") @@ -159,16 +154,93 @@ func ModuleSpec() pipeline.ModuleSpec { Stage: pipeline.StageExtract, Requires: append([]string(nil), requiredCapabilities...), Provides: append([]string(nil), providedCapabilities...), + ArtifactKind: dnd.SpellListKind, ReferenceSlots: shared.ReferenceSlots(referenceSlotDescriptions), } } func Register(registry *pipeline.ExtractorRegistry) error { - return registry.RegisterLegacyRawWithSpec(ModuleSpec(), func() (contracts.LegacyRawExtractor, error) { - return New(), nil + return pipeline.RegisterExtractorBuilder(registry, ModuleSpec(), validateOptions, func(request pipeline.BuildRequest) (contracts.Extractor[dnd.SpellList], error) { + options, err := DecodeOptions(request.Options) + if err != nil { + return nil, err + } + return New(request.Dependencies.LLM, options) }) } +// RegisterWithRawAdapter keeps existing raw downstream implementations usable +// while the extractor itself produces the canonical typed artifact. +func RegisterWithRawAdapter(registry *pipeline.ExtractorRegistry, codec RawAdapterCodec) error { + if codec == nil { + return extractorErrorf("artifact codec must not be nil") + } + build := func(request pipeline.BuildRequest) (*Extractor, error) { + options, err := DecodeOptions(request.Options) + if err != nil { + return nil, err + } + return New(request.Dependencies.LLM, options) + } + return pipeline.RegisterExtractorBuilderWithRawAdapter(registry, ModuleSpec(), validateOptions, + func(request pipeline.BuildRequest) (contracts.Extractor[dnd.SpellList], error) { + return build(request) + }, + func(request pipeline.BuildRequest) (contracts.LegacyRawExtractor, error) { + extractor, err := build(request) + if err != nil { + return nil, err + } + return &rawAdapter{extractor: extractor, codec: codec}, nil + }, + ) +} + +func (adapter *rawAdapter) Key() string { return Key } + +func (adapter *rawAdapter) ReferenceSlots() []contracts.ReferenceSlot { + return adapter.extractor.ReferenceSlots() +} + +func (adapter *rawAdapter) ManifestMetadata() map[string]any { + return adapter.extractor.ManifestMetadata() +} + +func (adapter *rawAdapter) Extract(ctx context.Context, req contracts.ExtractionRequest) (contracts.ExtractionResult, error) { + result, err := adapter.extractor.Extract(ctx, contracts.TypedExtractionRequest{ + Source: req.Source, Chunk: req.Chunk, AmbientContext: req.AmbientContext, + SourceInput: req.SourceInput, SessionID: req.SessionID, References: req.References, + LLMProfile: req.LLMProfile, Metadata: req.Metadata, + }) + if err != nil { + return contracts.ExtractionResult{}, err + } + content, err := adapter.codec.EncodeCandidate(result.Value) + if err != nil { + return contracts.ExtractionResult{}, extractorErrorf("encode canonical output: %w", err) + } + schema := adapter.codec.Schema() + return contracts.ExtractionResult{ + Output: contracts.ExtractOutput{ + Schema: contracts.ResponseSchema{ID: schema.ID, Name: schema.Name, Version: schema.Version, JSONSchema: append([]byte(nil), schema.JSONSchema...)}, + Payload: contracts.RawPayload{Content: content, MediaType: adapter.codec.MediaType(), Metadata: map[string]any{"spell_cast_count": len(result.Value.SpellCasts)}}, + }, + Warnings: result.Warnings, + }, nil +} + +func validateOptions(options map[string]any) error { + _, err := DecodeOptions(options) + return err +} + +func DecodeOptions(options map[string]any) (Options, error) { + if err := pipeline.RejectUnknownOptions(options); err != nil { + return Options{}, extractorErrorf("%w", err) + } + return Options{}, nil +} + func extractorErrorf(format string, args ...any) error { return fmt.Errorf("dnd spells extractor: "+format, args...) } diff --git a/internal/modules/dnd/extract/spells/extractor_test.go b/internal/modules/dnd/extract/spells/extractor_test.go index 7ac6fd0..d188941 100644 --- a/internal/modules/dnd/extract/spells/extractor_test.go +++ b/internal/modules/dnd/extract/spells/extractor_test.go @@ -2,47 +2,55 @@ package spells import ( "context" - "encoding/json" "errors" + "reflect" "strings" "testing" "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) -func TestExtractReturnsCanonicalOutputFromStructuredResponse(t *testing.T) { - client := &fakeSpellsLLMClient{ - response: extractionResponse{ - SpellCasts: []spellCastResponse{ - { - Caster: " Aria ", - Spell: " Cure Wounds ", - Effect: " Heals an injured ally. ", - NarrativeDescription: " Aria restores the fighter after the fight. ", - SourceRefs: responseSourceRefsInt("transcript", 1, 2), - }, - }, +func TestExtractReturnsCanonicalSpellListFromPrivateResponse(t *testing.T) { + client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{ + { + Caster: " Aria ", + Spell: " Cure Wounds ", + Effect: " Heals an injured ally. ", + NarrativeDescription: " Aria restores the fighter after the fight. ", + SourceRefs: responseSourceRefs(1, 2), }, - content: []byte(`{"spell_casts":[{"caster":" Aria ","spell":" Cure Wounds ","effect":" Heals an injured ally. ","narrative_description":" Aria restores the fighter after the fight. ","source_refs":[{"source_id":"session-alpha","start_unit_id":1,"end_unit_id":2}]}],"raw_marker":true}`), - } + }}} + req := extractionRequest() - extractReq := extractionRequestWithClient(client) - result, err := New().Extract(context.Background(), extractReq) + result, err := newExtractor(t, client).Extract(context.Background(), req) if err != nil { t.Fatalf("Extract() error = %v, want nil", err) } + want := dnd.SpellList{SpellCasts: []dnd.SpellCast{ + { + Caster: " Aria ", + Spell: " Cure Wounds ", + Effect: " Heals an injured ally. ", + NarrativeDescription: " Aria restores the fighter after the fight. ", + SourceRefs: []source.SourceRef{{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}}, + }, + }} + if !reflect.DeepEqual(result.Value, want) { + t.Fatalf("Value = %#v, want %#v", result.Value, want) + } + if len(result.Warnings) != 0 { + t.Fatalf("Warnings = %#v, want none", result.Warnings) + } if len(client.requests) != 1 { t.Fatalf("LLM calls = %d, want 1", len(client.requests)) } llmReq := client.requests[0] - if llmReq.StageName != Key { - t.Fatalf("StageName = %q, want %q", llmReq.StageName, Key) - } - if llmReq.PromptID != PromptID || llmReq.PromptVersion != SchemaVersion { - t.Fatalf("prompt = %q/%q, want %q/%q", llmReq.PromptID, llmReq.PromptVersion, PromptID, SchemaVersion) + if llmReq.StageName != Key || llmReq.PromptID != PromptID || llmReq.PromptVersion != SchemaVersion { + t.Fatalf("LLM request identity = %#v, want spell prompt", llmReq) } if llmReq.SessionID != "session-123" || llmReq.ProfileID != "profile-spells" { t.Fatalf("session/profile = %q/%q, want session-123/profile-spells", llmReq.SessionID, llmReq.ProfileID) @@ -51,45 +59,17 @@ func TestExtractReturnsCanonicalOutputFromStructuredResponse(t *testing.T) { if transcript.Name != "transcript" || transcript.MediaType != "application/json" || transcript.Digest != "sha256:chunk" || transcript.OriginURI != "file:///session-alpha.json" { t.Fatalf("transcript metadata = %#v", transcript) } - if got := string(transcript.Content); got != string(extractReq.Chunk.Content) { - t.Fatalf("transcript content = %q, want chunk content %q", got, extractReq.Chunk.Content) - } - - if result.Output.Payload.MediaType != "application/json" { - t.Fatalf("MediaType = %q, want application/json", result.Output.Payload.MediaType) - } - if result.Output.Schema.ID != ResponseSchemaID || result.Output.Schema.Name != ResponseSchemaName || result.Output.Schema.Version != SchemaVersion { - t.Fatalf("schema = %#v, want response schema provenance", result.Output.Schema) - } - if !json.Valid(result.Output.Schema.JSONSchema) { - t.Fatalf("schema JSON is invalid or missing: %s", result.Output.Schema.JSONSchema) - } - if strings.Contains(string(result.Output.Payload.Content), "raw_marker") { - t.Fatalf("content = %q, want canonical payload without raw completion marker", result.Output.Payload.Content) - } - - var payload extractionResponse - if err := json.Unmarshal(result.Output.Payload.Content, &payload); err != nil { - t.Fatalf("Unmarshal(Content) error = %v, want nil", err) - } - if len(payload.SpellCasts) != 1 || payload.SpellCasts[0].Spell != " Cure Wounds " { - t.Fatalf("payload = %#v, want structured response fields", payload) - } - if got := payload.SpellCasts[0].SourceRefs[0].SourceID; got != "session-alpha" { - t.Fatalf("source_id = %q, want canonical source document ID", got) + if got := string(transcript.Content); got != string(req.Chunk.Content) { + t.Fatalf("transcript content = %q, want chunk content %q", got, req.Chunk.Content) } } -func TestExtractorManifestMetadataIncludesPromptAndSchemaProvenance(t *testing.T) { - metadata := New().ManifestMetadata() - +func TestExtractorManifestMetadataIncludesLLMSchemaProvenance(t *testing.T) { + metadata := newExtractor(t, &fakeSpellsLLMClient{}).ManifestMetadata() tests := map[string]string{ - "prompt_id": PromptID, - "prompt_version": SchemaVersion, - "response_schema_key": string(ResponseSchemaKey), - "response_schema_id": ResponseSchemaID, - "response_schema_name": ResponseSchemaName, - "response_schema_version": SchemaVersion, + "prompt_id": PromptID, "prompt_version": SchemaVersion, + "response_schema_key": string(ResponseSchemaKey), "response_schema_id": ResponseSchemaID, + "response_schema_name": ResponseSchemaName, "response_schema_version": SchemaVersion, } for key, want := range tests { if metadata[key] != want { @@ -106,373 +86,125 @@ func TestExtractorManifestMetadataIncludesPromptAndSchemaProvenance(t *testing.T func TestExtractPassesReferencesAsPromptInputs(t *testing.T) { client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{}}} - req := extractionRequestWithClient(client) - req.References = contracts.ReferenceSet{ - Slots: map[string]contracts.ResolvedReferenceSlot{ - "players": { - Slot: contracts.ReferenceSlot{Name: "players"}, - Items: []contracts.ReferenceItem{ - {SlotName: "players", Content: []byte("Alice: Aria Brightmantle")}, - }, - }, - "party": { - Slot: contracts.ReferenceSlot{Name: "party"}, - Items: []contracts.ReferenceItem{ - {SlotName: "party", Content: []byte("Aria Brightmantle: party cleric")}, - }, - }, - "glossary": { - Slot: contracts.ReferenceSlot{Name: "glossary"}, - Items: []contracts.ReferenceItem{ - {SlotName: "glossary", Content: []byte("Brightmantle: local temple name")}, - }, - }, - }, - } + req := extractionRequest() + req.References = contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ + "players": {Slot: contracts.ReferenceSlot{Name: "players"}, Items: []contracts.ReferenceItem{{SlotName: "players", Content: []byte("Alice: Aria Brightmantle")}}}, + "party": {Slot: contracts.ReferenceSlot{Name: "party"}, Items: []contracts.ReferenceItem{{SlotName: "party", Content: []byte("Aria Brightmantle: party cleric")}}}, + "glossary": {Slot: contracts.ReferenceSlot{Name: "glossary"}, Items: []contracts.ReferenceItem{{SlotName: "glossary", Content: []byte("Brightmantle: local temple name")}}}, + }} - if _, err := New().Extract(context.Background(), req); err != nil { + if _, err := newExtractor(t, client).Extract(context.Background(), req); err != nil { t.Fatalf("Extract() error = %v, want nil", err) } - - if len(client.requests) != 1 { - t.Fatalf("LLM calls = %d, want 1", len(client.requests)) + inputs := client.requests[0].Inputs + if string(inputs["players"].Content) != "Alice: Aria Brightmantle" || string(inputs["party"].Content) != "Aria Brightmantle: party cleric" || string(inputs["glossary"].Content) != "Brightmantle: local temple name" { + t.Fatalf("reference inputs = %#v, want configured content", inputs) } - request := client.requests[0] - if request.PromptID != PromptID || request.PromptVersion != SchemaVersion { - t.Fatalf("prompt = %q/%q, want %q/%q", request.PromptID, request.PromptVersion, PromptID, SchemaVersion) - } - if got := string(request.Inputs["players"].Content); got != "Alice: Aria Brightmantle" { - t.Fatalf("players input = %q, want reference content", got) - } - if got := string(request.Inputs["party"].Content); got != "Aria Brightmantle: party cleric" { - t.Fatalf("party input = %q, want reference content", got) - } - if got := string(request.Inputs["glossary"].Content); got != "Brightmantle: local temple name" { - t.Fatalf("glossary input = %q, want reference content", got) - } - if strings.Contains(string(request.Inputs["transcript"].Content), "Aria Brightmantle: party cleric") { - t.Fatalf("transcript input contains reference content") + if strings.Contains(string(inputs["transcript"].Content), "party cleric") { + t.Fatal("transcript input contains reference content") } } func TestPromptInputsMapLegacyRosterReferenceToParty(t *testing.T) { - inputs := shared.PromptInputs(spellSourceInput(), contracts.ReferenceSet{ - Slots: map[string]contracts.ResolvedReferenceSlot{ - "roster": { - Slot: contracts.ReferenceSlot{Name: "roster"}, - Items: []contracts.ReferenceItem{ - {SlotName: "roster", Content: []byte("Legacy roster text")}, - }, - }, - }, - }) - + inputs := shared.PromptInputs(spellSourceInput(), contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ + "roster": {Slot: contracts.ReferenceSlot{Name: "roster"}, Items: []contracts.ReferenceItem{{SlotName: "roster", Content: []byte("Legacy roster text")}}}, + }}) if got := string(inputs["party"].Content); got != "Legacy roster text" { t.Fatalf("party input = %q, want legacy roster content", got) } if _, ok := inputs["roster"]; ok { - t.Fatalf("roster prompt input was present; want only party input") + t.Fatal("roster prompt input was present; want only party input") } } -func TestExtractReturnsRawOutputForEmptyResponse(t *testing.T) { - client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{}}} - - result, err := New().Extract(context.Background(), extractionRequestWithClient(client)) - if err != nil { - t.Fatalf("Extract() error = %v, want nil", err) - } - var payload extractionResponse - if err := json.Unmarshal(result.Output.Payload.Content, &payload); err != nil { - t.Fatalf("Unmarshal(Content) error = %v, want nil", err) - } - if len(payload.SpellCasts) != 0 { - t.Fatalf("SpellCasts = %#v, want none", payload.SpellCasts) - } -} - -func TestExtractReturnsCanonicalOutputForMalformedStructuredResponse(t *testing.T) { - client := &fakeSpellsLLMClient{response: extractionResponse{}} - - result, err := New().Extract(context.Background(), extractionRequestWithClient(client)) - if err != nil { - t.Fatalf("Extract() error = %v, want nil", err) - } - if string(result.Output.Payload.Content) != `{"spell_casts":null}` { - t.Fatalf("content = %s, want canonical structured output", result.Output.Payload.Content) - } -} - -func TestExtractWrapsLLMClientError(t *testing.T) { - client := &fakeSpellsLLMClient{err: errors.New("provider unavailable")} - - _, err := New().Extract(context.Background(), extractionRequestWithClient(client)) - if err == nil { - t.Fatal("Extract() error = nil, want LLM error") - } - if !strings.Contains(err.Error(), "dnd spells") || !strings.Contains(err.Error(), "provider unavailable") { - t.Fatalf("Extract() error = %q, want wrapped LLM context", err.Error()) - } -} - -func TestExtractRejectsInvalidRequests(t *testing.T) { - validClient := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{}}} - validReq := extractionRequestWithClient(validClient) - canceledCtx, cancel := context.WithCancel(context.Background()) - cancel() - +func TestExtractPreservesEmptyAndMalformedValuesForTypedValidators(t *testing.T) { tests := []struct { - name string - extractor *Extractor - ctx context.Context - req contracts.ExtractionRequest - want string + name string + response extractionResponse + wantNil bool }{ - {name: "nil extractor", extractor: nil, ctx: context.Background(), req: validReq, want: "extractor"}, - {name: "nil context", extractor: New(), ctx: nil, req: validReq, want: "context"}, - {name: "canceled context", extractor: New(), ctx: canceledCtx, req: validReq, want: "context"}, - {name: "nil source", extractor: New(), ctx: context.Background(), req: contracts.ExtractionRequest{Chunk: validReq.Chunk, LLMClient: validReq.LLMClient}, want: "source"}, - {name: "nil chunk", extractor: New(), ctx: context.Background(), req: contracts.ExtractionRequest{Source: validReq.Source, LLMClient: validReq.LLMClient}, want: "chunk"}, - {name: "empty chunk units", extractor: New(), ctx: context.Background(), req: emptyChunkRequest(validReq), want: "units"}, - {name: "nil LLM client", extractor: New(), ctx: context.Background(), req: contracts.ExtractionRequest{Source: validReq.Source, Chunk: validReq.Chunk}, want: "LLM client"}, - {name: "source input mismatches chunk", extractor: New(), ctx: context.Background(), req: mismatchedSourceInputRequest(validReq), want: "must match chunk"}, + {name: "empty", response: extractionResponse{SpellCasts: []spellCastResponse{}}}, + {name: "missing", response: extractionResponse{}, wantNil: true}, } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := tt.extractor.Extract(tt.ctx, tt.req) - if err == nil { - t.Fatal("Extract() error = nil, want error") + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := newExtractor(t, &fakeSpellsLLMClient{response: test.response}).Extract(context.Background(), extractionRequest()) + if err != nil { + t.Fatalf("Extract() error = %v, want nil", err) } - if !strings.Contains(err.Error(), "dnd spells") || !strings.Contains(err.Error(), tt.want) { - t.Fatalf("Extract() error = %q, want %q context", err.Error(), tt.want) + if (result.Value.SpellCasts == nil) != test.wantNil || len(result.Value.SpellCasts) != 0 { + t.Fatalf("SpellCasts = %#v, want empty with nil=%t", result.Value.SpellCasts, test.wantNil) } }) } } -func TestExtractOrdersSpellCastsByEarliestSourceUnit(t *testing.T) { - client := &fakeSpellsLLMClient{ - response: extractionResponse{ - SpellCasts: []spellCastResponse{ - { - Caster: "Bandit Shaman", - Spell: "Fire Bolt", - Effect: "Burns.", - NarrativeDescription: "Second spell.", - SourceRefs: responseSourceRefs("session-alpha", 2, 2), - }, - { - Caster: "Aria", - Spell: "Cure Wounds", - Effect: "Heals.", - NarrativeDescription: "First spell.", - SourceRefs: responseSourceRefs("session-alpha", 1, 1), - }, - { - Caster: "Narrator", - Spell: "Unknown Spell", - Effect: "No cited range.", - NarrativeDescription: "This should sort after cited spell casts.", - }, - }, - }, +func TestExtractWrapsLLMClientError(t *testing.T) { + _, err := newExtractor(t, &fakeSpellsLLMClient{err: errors.New("provider unavailable")}).Extract(context.Background(), extractionRequest()) + if err == nil || !strings.Contains(err.Error(), "dnd spells") || !strings.Contains(err.Error(), "provider unavailable") { + t.Fatalf("Extract() error = %v, want wrapped provider error", err) } +} - result, err := New().Extract(context.Background(), extractionRequestWithClient(client)) +func TestExtractRejectsInvalidRequests(t *testing.T) { + validReq := extractionRequest() + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + validExtractor := newExtractor(t, &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{}}}) + tests := []struct { + name string + extractor *Extractor + ctx context.Context + req contracts.TypedExtractionRequest + want string + }{ + {name: "nil extractor", ctx: context.Background(), req: validReq, want: "extractor"}, + {name: "nil context", extractor: validExtractor, req: validReq, want: "context"}, + {name: "canceled context", extractor: validExtractor, ctx: canceledCtx, req: validReq, want: "context"}, + {name: "nil source", extractor: validExtractor, ctx: context.Background(), req: contracts.TypedExtractionRequest{Chunk: validReq.Chunk}, want: "source"}, + {name: "nil chunk", extractor: validExtractor, ctx: context.Background(), req: contracts.TypedExtractionRequest{Source: validReq.Source}, want: "chunk"}, + {name: "empty chunk units", extractor: validExtractor, ctx: context.Background(), req: emptyChunkRequest(validReq), want: "units"}, + {name: "source input mismatches chunk", extractor: validExtractor, ctx: context.Background(), req: mismatchedSourceInputRequest(validReq), want: "must match chunk"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := test.extractor.Extract(test.ctx, test.req) + if err == nil || !strings.Contains(err.Error(), "dnd spells") || !strings.Contains(err.Error(), test.want) { + t.Fatalf("Extract() error = %v, want %q context", err, test.want) + } + }) + } +} + +func TestExtractOrdersAndDeduplicatesEvidence(t *testing.T) { + client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{ + {Caster: "Borin", Spell: "Fire Bolt", Effect: "Burns.", NarrativeDescription: "Second.", SourceRefs: responseSourceRefs(2, 2)}, + {Caster: "Aria", Spell: "Cure Wounds", Effect: "Heals.", NarrativeDescription: "First.", SourceRefs: []spellSourceRefResponse{{StartUnitID: shared.UnitRefFromInt(1), EndUnitID: shared.UnitRefFromInt(2)}, {StartUnitID: shared.UnitRefFromInt(1), EndUnitID: shared.UnitRefFromInt(2)}}}, + {Caster: "Narrator", Spell: "Unknown", Effect: "Unknown.", NarrativeDescription: "Uncited."}, + }}} + result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) if err != nil { t.Fatalf("Extract() error = %v, want nil", err) } - var payload extractionResponse - if err := json.Unmarshal(result.Output.Payload.Content, &payload); err != nil { - t.Fatalf("Unmarshal(Content) error = %v, want nil", err) + if got := []string{result.Value.SpellCasts[0].Spell, result.Value.SpellCasts[1].Spell, result.Value.SpellCasts[2].Spell}; !reflect.DeepEqual(got, []string{"Cure Wounds", "Fire Bolt", "Unknown"}) { + t.Fatalf("spell order = %#v, want evidence order", got) } - if len(payload.SpellCasts) != 3 || - payload.SpellCasts[0].Spell != "Cure Wounds" || - payload.SpellCasts[1].Spell != "Fire Bolt" || - payload.SpellCasts[2].Spell != "Unknown Spell" { - t.Fatalf("spell order = %#v, want earliest source-unit order with uncited spell last", payload.SpellCasts) + if refs := result.Value.SpellCasts[0].SourceRefs; len(refs) != 1 || refs[0] != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}) { + t.Fatalf("source refs = %#v, want one canonical ref", refs) } } -func TestExtractCanonicalizesSourceRefs(t *testing.T) { - client := &fakeSpellsLLMClient{ - response: extractionResponse{ - SpellCasts: []spellCastResponse{ - { - Caster: "Aria", - Spell: "Cure Wounds", - Effect: "Heals.", - NarrativeDescription: "Aria heals.", - SourceRefs: []shared.SourceRefResponse{ - {SourceID: "gameplay_transcript", StartUnitID: shared.UnitRefFromInt(2), EndUnitID: shared.UnitRefFromInt(2)}, - {SourceID: "", StartUnitID: shared.UnitRefFromInt(1), EndUnitID: shared.UnitRefFromInt(2)}, - {SourceID: "transcript", StartUnitID: shared.UnitRefFromInt(1), EndUnitID: shared.UnitRefFromInt(2)}, - }, - }, - }, - }, - } - - result, err := New().Extract(context.Background(), extractionRequestWithClient(client)) +func TestExtractPreservesInvalidEvidenceForValidators(t *testing.T) { + client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{{ + Caster: "Aria", Spell: "Cure Wounds", Effect: "Heals.", NarrativeDescription: "Aria heals.", + SourceRefs: []spellSourceRefResponse{{StartUnitID: shared.UnitRefFromInt(99), EndUnitID: shared.UnitRefFromString("missing")}}, + }}}} + result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) if err != nil { t.Fatalf("Extract() error = %v, want nil", err) } - var payload extractionResponse - if err := json.Unmarshal(result.Output.Payload.Content, &payload); err != nil { - t.Fatalf("Unmarshal(Content) error = %v, want nil", err) - } - refs := payload.SpellCasts[0].SourceRefs - if len(refs) != 2 { - t.Fatalf("source refs = %#v, want duplicate collapsed", refs) - } - for _, ref := range refs { - if ref.SourceID != "session-alpha" { - t.Fatalf("source ref = %#v, want canonical source_id", ref) - } - } - if refs[0].StartUnitID.Int() != 1 || refs[0].EndUnitID.Int() != 2 || - refs[1].StartUnitID.Int() != 2 || refs[1].EndUnitID.Int() != 2 { - t.Fatalf("source refs = %#v, want sorted unit ranges", refs) + ref := result.Value.SpellCasts[0].SourceRefs[0] + if ref != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 99}) { + t.Fatalf("source ref = %#v, want canonical source with invalid range preserved", ref) } } - -func TestExtractPreservesInvalidSourceRefsForValidators(t *testing.T) { - client := &fakeSpellsLLMClient{ - response: extractionResponse{ - SpellCasts: []spellCastResponse{ - { - Caster: "Aria", - Spell: "Cure Wounds", - Effect: "Heals.", - NarrativeDescription: "Aria heals.", - SourceRefs: []shared.SourceRefResponse{ - {SourceID: "transcript", StartUnitID: shared.UnitRefFromInt(99), EndUnitID: shared.UnitRefFromString("missing")}, - }, - }, - }, - }, - } - - result, err := New().Extract(context.Background(), extractionRequestWithClient(client)) - if err != nil { - t.Fatalf("Extract() error = %v, want nil", err) - } - var payload map[string][]map[string]any - if err := json.Unmarshal(result.Output.Payload.Content, &payload); err != nil { - t.Fatalf("Unmarshal(Content) error = %v, want nil", err) - } - ref := payload["spell_casts"][0]["source_refs"].([]any)[0].(map[string]any) - if ref["source_id"] != "session-alpha" || ref["start_unit_id"] != float64(99) || ref["end_unit_id"] != "" { - t.Fatalf("source ref = %#v, want source_id canonicalized without unit repair", ref) - } -} - -func TestExtractDefensivelyCopiesRawContent(t *testing.T) { - client := &fakeSpellsLLMClient{ - response: extractionResponse{ - SpellCasts: []spellCastResponse{ - { - Caster: "Aria", - Spell: "Cure Wounds", - Effect: "Heals.", - NarrativeDescription: "Aria heals.", - SourceRefs: responseSourceRefs("session-alpha", 1, 2), - }, - }, - }, - } - - result, err := New().Extract(context.Background(), extractionRequestWithClient(client)) - if err != nil { - t.Fatalf("Extract() error = %v, want nil", err) - } - client.response.SpellCasts[0].SourceRefs[0].StartUnitID = shared.UnitRefFromInt(99) - - var payload extractionResponse - if err := json.Unmarshal(result.Output.Payload.Content, &payload); err != nil { - t.Fatalf("Unmarshal(Content) error = %v, want nil", err) - } - if got := payload.SpellCasts[0].SourceRefs[0].StartUnitID.String(); got != "1" { - t.Fatalf("source ref start = %q, want copied 1", got) - } -} - -func extractionRequestWithClient(client contracts.StructuredLLMClient) contracts.ExtractionRequest { - req := promptExtractionRequest() - req.LLMClient = client - req.SourceInput = spellChunkInput(req.Chunk) - req.SessionID = "session-123" - req.LLMProfile = "profile-spells" - return req -} - -const spellTranscriptJSON = `{"id":"session-alpha","segments":[{"id":1,"text":"Aria raises her hand and casts Cure Wounds."}]}` - -func spellSourceInput() contracts.LLMInputMaterial { - return contracts.NewLLMInputMaterial("source", "application/json", []byte(spellTranscriptJSON), "sha256:transcript", "file:///session-alpha.json") -} - -func spellChunkInput(chunk *source.Chunk) contracts.LLMInputMaterial { - return contracts.NewLLMInputMaterial("source", chunk.MediaType, chunk.Content, "sha256:chunk", "file:///session-alpha.json") -} - -func emptyChunkRequest(req contracts.ExtractionRequest) contracts.ExtractionRequest { - req.Chunk = &source.Chunk{ - ID: req.Chunk.ID, - SourceID: req.Chunk.SourceID, - Index: req.Chunk.Index, - } - return req -} - -func mismatchedSourceInputRequest(req contracts.ExtractionRequest) contracts.ExtractionRequest { - req.SourceInput = spellSourceInput() - return req -} - -type fakeSpellsLLMClient struct { - response extractionResponse - content []byte - err error - requests []contracts.StructuredCompletionRequest -} - -func (client *fakeSpellsLLMClient) CompleteStructured(ctx context.Context, req contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { - client.requests = append(client.requests, cloneStructuredCompletionRequest(req)) - if client.err != nil { - return contracts.StructuredCompletionResponse{}, client.err - } - - target, ok := out.(*extractionResponse) - if !ok { - return contracts.StructuredCompletionResponse{}, errors.New("unexpected output target") - } - *target = client.response - content := append([]byte(nil), client.content...) - if len(content) == 0 { - var err error - content, err = json.Marshal(client.response) - if err != nil { - return contracts.StructuredCompletionResponse{}, err - } - } - return contracts.StructuredCompletionResponse{Content: content}, nil -} - -func cloneStructuredCompletionRequest(req contracts.StructuredCompletionRequest) contracts.StructuredCompletionRequest { - req.Inputs = req.Inputs.Clone() - req.Vars = cloneVars(req.Vars) - return req -} - -func cloneVars(in map[string]any) map[string]any { - if len(in) == 0 { - return nil - } - out := make(map[string]any, len(in)) - for key, value := range in { - out[key] = value - } - return out -} diff --git a/internal/modules/dnd/extract/spells/model.go b/internal/modules/dnd/extract/spells/model.go index 278457e..933c6bb 100644 --- a/internal/modules/dnd/extract/spells/model.go +++ b/internal/modules/dnd/extract/spells/model.go @@ -2,21 +2,19 @@ package spells import "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" -type SpellCast struct { - Caster string `json:"caster"` - Spell string `json:"spell"` - Effect string `json:"effect"` - NarrativeDescription string `json:"narrative_description"` -} - type extractionResponse struct { SpellCasts []spellCastResponse `json:"spell_casts"` } type spellCastResponse struct { - Caster string `json:"caster"` - Spell string `json:"spell"` - Effect string `json:"effect"` - NarrativeDescription string `json:"narrative_description"` - SourceRefs []shared.SourceRefResponse `json:"source_refs"` + Caster string `json:"caster"` + Spell string `json:"spell"` + Effect string `json:"effect"` + NarrativeDescription string `json:"narrative_description"` + SourceRefs []spellSourceRefResponse `json:"source_refs"` +} + +type spellSourceRefResponse struct { + StartUnitID shared.UnitRef `json:"start_unit_id"` + EndUnitID shared.UnitRef `json:"end_unit_id"` } diff --git a/internal/modules/dnd/extract/spells/registry_test.go b/internal/modules/dnd/extract/spells/registry_test.go index f6b9c74..b9eb40e 100644 --- a/internal/modules/dnd/extract/spells/registry_test.go +++ b/internal/modules/dnd/extract/spells/registry_test.go @@ -7,13 +7,14 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" ) -func TestNewReturnsExtractorWithMetadata(t *testing.T) { - extractor := New() - if extractor == nil { - t.Fatal("New() = nil, want extractor") +func TestNewRequiresLLMClientAndReturnsExtractor(t *testing.T) { + if _, err := New(nil, Options{}); err == nil || !strings.Contains(err.Error(), "LLM client") { + t.Fatalf("New(nil) error = %v, want LLM client error", err) } + extractor := newExtractor(t, &fakeSpellsLLMClient{}) if extractor.Key() != Key { t.Fatalf("extractor.Key() = %q, want %q", extractor.Key(), Key) } @@ -31,6 +32,7 @@ func TestModuleSpec(t *testing.T) { Provides: []string{ "dnd.spell_casts", }, + ArtifactKind: dnd.SpellListKind, ReferenceSlots: []contracts.ReferenceSlot{ { Name: "glossary", @@ -74,12 +76,11 @@ func TestRegisterMakesExtractorBuildable(t *testing.T) { t.Fatalf("Register() error = %v, want nil", err) } - extractor, err := registry.BuildLegacyRaw(Key) - if err != nil { - t.Fatalf("Build() error = %v, want nil", err) + if _, err := registry.BuildLegacyRaw(Key); err == nil || !strings.Contains(err.Error(), "legacy raw") { + t.Fatalf("BuildLegacyRaw() error = %v, want typed registration error", err) } - if extractor.Key() != Key { - t.Fatalf("extractor.Key() = %q, want %q", extractor.Key(), Key) + if _, err := DecodeOptions(map[string]any{"unexpected": true}); err == nil || !strings.Contains(err.Error(), "unknown option") { + t.Fatalf("DecodeOptions() error = %v, want unknown option error", err) } } @@ -101,7 +102,7 @@ func TestRegisterStoresModuleSpec(t *testing.T) { } func TestRuntimeReferenceSlotsMatchModuleSpec(t *testing.T) { - extractor := New() + extractor := newExtractor(t, &fakeSpellsLLMClient{}) spec := ModuleSpec() if !reflect.DeepEqual(extractor.ReferenceSlots(), spec.ReferenceSlots) { diff --git a/internal/modules/dnd/extract/spells/schema.go b/internal/modules/dnd/extract/spells/schema.go index 022820f..f16f129 100644 --- a/internal/modules/dnd/extract/spells/schema.go +++ b/internal/modules/dnd/extract/spells/schema.go @@ -15,6 +15,6 @@ func loadResponseSchema() (llm.ResponseSchema, error) { ID: ResponseSchemaID, Version: SchemaVersion, Name: ResponseSchemaName, - AssetPath: "assets/schemas/dnd_spells.v1.json", + AssetPath: "assets/schemas/dnd_spells_llm.v1.json", }) } diff --git a/internal/modules/dnd/extract/spells/schema_test.go b/internal/modules/dnd/extract/spells/schema_test.go index 0b2b2a9..4e6e36c 100644 --- a/internal/modules/dnd/extract/spells/schema_test.go +++ b/internal/modules/dnd/extract/spells/schema_test.go @@ -1,130 +1,62 @@ package spells import ( + "bytes" "encoding/json" "strings" "testing" ) -func TestLoadResponseSchemaForSpells(t *testing.T) { +func TestLoadResponseSchemaUsesExtractorOwnedLLMSchema(t *testing.T) { schema, err := loadResponseSchema() if err != nil { t.Fatalf("loadResponseSchema() error = %v, want nil", err) } - if schema.Key != ResponseSchemaKey { - t.Fatalf("schema.Key = %q, want %q", schema.Key, ResponseSchemaKey) + if schema.Key != ResponseSchemaKey || schema.ID != ResponseSchemaID || schema.Version != SchemaVersion || schema.Name != ResponseSchemaName { + t.Fatalf("schema identity = %#v, want maintained response identity", schema) } - if schema.ID != ResponseSchemaID { - t.Fatalf("schema.ID = %q, want %q", schema.ID, ResponseSchemaID) - } - if schema.Version != SchemaVersion { - t.Fatalf("schema.Version = %q, want %q", schema.Version, SchemaVersion) - } - if schema.Name != ResponseSchemaName { - t.Fatalf("schema.Name = %q, want %q", schema.Name, ResponseSchemaName) - } - if !strings.HasPrefix(schema.SHA256, "sha256:") { - t.Fatalf("schema.SHA256 = %q, want sha256 prefix", schema.SHA256) - } - if !json.Valid(schema.JSONSchema) { - t.Fatalf("schema.JSONSchema is invalid JSON: %s", schema.JSONSchema) + if !strings.HasPrefix(schema.SHA256, "sha256:") || !json.Valid(schema.JSONSchema) { + t.Fatalf("schema metadata = %#v, want valid hashed JSON", schema) } var decoded map[string]any if err := json.Unmarshal(schema.JSONSchema, &decoded); err != nil { - t.Fatalf("Unmarshal(schema.JSONSchema) error = %v, want nil", err) + t.Fatalf("Unmarshal(schema.JSONSchema) error = %v", err) } - properties := decoded["properties"].(map[string]any) - spellCastProperties := properties["spell_casts"].(map[string]any)["items"].(map[string]any)["properties"].(map[string]any) - sourceRefProperties := spellCastProperties["source_refs"].(map[string]any)["items"].(map[string]any)["properties"].(map[string]any) - sourceRefRequired := spellCastProperties["source_refs"].(map[string]any)["items"].(map[string]any)["required"].([]any) - if !containsJSONField(sourceRefRequired, "source_id") { - t.Fatalf("canonical source refs required = %#v, want source_id", sourceRefRequired) - } - for _, field := range []string{"start_unit_id", "end_unit_id"} { - property := sourceRefProperties[field].(map[string]any) - if property["type"] != "integer" { - t.Fatalf("%s type = %#v, want integer", field, property["type"]) - } - if property["minimum"] != float64(1) { - t.Fatalf("%s minimum = %#v, want 1", field, property["minimum"]) - } - } -} - -func TestLLMResponseSchemaOmitsSourceID(t *testing.T) { - raw, err := embeddedAssets.ReadFile("assets/schemas/dnd_spells_llm.v1.json") - if err != nil { - t.Fatalf("ReadFile(LLM schema) error = %v, want nil", err) - } - if !json.Valid(raw) { - t.Fatalf("LLM schema is invalid JSON: %s", raw) - } - - var decoded map[string]any - if err := json.Unmarshal(raw, &decoded); err != nil { - t.Fatalf("Unmarshal(LLM schema) error = %v, want nil", err) + if decoded["$id"] != "notarius.dnd.spells.llm" { + t.Fatalf("LLM schema $id = %#v, want extractor transport schema", decoded["$id"]) } properties := decoded["properties"].(map[string]any) spellCastProperties := properties["spell_casts"].(map[string]any)["items"].(map[string]any)["properties"].(map[string]any) sourceRefItems := spellCastProperties["source_refs"].(map[string]any)["items"].(map[string]any) sourceRefProperties := sourceRefItems["properties"].(map[string]any) - sourceRefRequired := sourceRefItems["required"].([]any) if _, ok := sourceRefProperties["source_id"]; ok { - t.Fatalf("LLM source ref schema contains source_id property: %#v", sourceRefProperties) - } - if containsJSONField(sourceRefRequired, "source_id") { - t.Fatalf("LLM source refs required = %#v, want no source_id", sourceRefRequired) + t.Fatalf("LLM source ref schema contains canonical source_id: %#v", sourceRefProperties) } } func TestResponseSchemaJSONIsMutationSafe(t *testing.T) { first, err := loadResponseSchema() if err != nil { - t.Fatalf("loadResponseSchema() error = %v, want nil", err) + t.Fatalf("loadResponseSchema() error = %v", err) } first.JSONSchema[0] = '[' - second, err := loadResponseSchema() - if err != nil { - t.Fatalf("loadResponseSchema() error = %v, want nil", err) - } - if !json.Valid(second.JSONSchema) { - t.Fatalf("schema JSON was mutated: %s", second.JSONSchema) - } - if len(second.JSONSchema) > 0 && second.JSONSchema[0] == '[' { - t.Fatalf("schema JSON did not use defensive copy") + if err != nil || !json.Valid(second.JSONSchema) || bytes.Equal(first.JSONSchema, second.JSONSchema) { + t.Fatalf("second schema = %s, %v; want defensive valid copy", second.JSONSchema, err) } } func TestResponseSchemaDiagnosticsOmitRawSchema(t *testing.T) { schema, err := loadResponseSchema() if err != nil { - t.Fatalf("loadResponseSchema() error = %v, want nil", err) + t.Fatalf("loadResponseSchema() error = %v", err) } diagnostics := schema.DiagnosticsMap() - if diagnostics["key"] != ResponseSchemaKey { - t.Fatalf("diagnostics[key] = %#v, want %q", diagnostics["key"], ResponseSchemaKey) - } - for _, key := range []string{"id", "version", "name", "sha256"} { - if diagnostics[key] == "" { - t.Fatalf("diagnostics[%q] = %#v, want value", key, diagnostics[key]) - } + t.Fatalf("diagnostics = %#v, want response key", diagnostics) } if _, ok := diagnostics["json_schema"]; ok { - t.Fatalf("diagnostics should omit raw schema content: %#v", diagnostics) - } - if _, ok := diagnostics["JSONSchema"]; ok { - t.Fatalf("diagnostics should omit raw schema content: %#v", diagnostics) + t.Fatalf("diagnostics include raw schema: %#v", diagnostics) } } - -func containsJSONField(fields []any, want string) bool { - for _, field := range fields { - if field == want { - return true - } - } - return false -} diff --git a/internal/modules/dnd/extract/spells/scriptorium_assets_test.go b/internal/modules/dnd/extract/spells/scriptorium_assets_test.go index 631a5c8..9113de2 100644 --- a/internal/modules/dnd/extract/spells/scriptorium_assets_test.go +++ b/internal/modules/dnd/extract/spells/scriptorium_assets_test.go @@ -57,7 +57,7 @@ func TestScriptoriumPromptDiagnosticsOmitRawMaterials(t *testing.T) { transcript := []byte(`{"secret":"source text"}`) reference := "private party note" prepared := prepareSpellsPrompt(t, transcript, "private player note", reference, " ") - metadata := New().ManifestMetadata() + metadata := newExtractor(t, &fakeSpellsLLMClient{}).ManifestMetadata() payload, err := json.Marshal(map[string]any{ "prepared": map[string]any{ diff --git a/internal/modules/dnd/extract/spells/test_helpers_test.go b/internal/modules/dnd/extract/spells/test_helpers_test.go index ff46b18..c958fc0 100644 --- a/internal/modules/dnd/extract/spells/test_helpers_test.go +++ b/internal/modules/dnd/extract/spells/test_helpers_test.go @@ -1,7 +1,9 @@ package spells import ( + "context" "encoding/json" + "errors" "testing" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -9,7 +11,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) -func promptExtractionRequest() contracts.ExtractionRequest { +func promptExtractionRequest() contracts.TypedExtractionRequest { doc := promptSourceDocument() chunk := &source.Chunk{ ID: "session-alpha:chunk:0", @@ -25,7 +27,7 @@ func promptExtractionRequest() contracts.ExtractionRequest { Units: append([]source.SourceUnit(nil), doc.Units...), Metadata: map[string]any{"ignored": "chunk metadata"}, } - return contracts.ExtractionRequest{ + return contracts.TypedExtractionRequest{ Source: doc, Chunk: chunk, } @@ -70,22 +72,90 @@ func mustJSON(t *testing.T, value any) string { return string(encoded) } -func responseSourceRefs(sourceID string, startUnitID int, endUnitID int) []shared.SourceRefResponse { - return []shared.SourceRefResponse{ +func responseSourceRefs(startUnitID int, endUnitID int) []spellSourceRefResponse { + return []spellSourceRefResponse{ { - SourceID: sourceID, StartUnitID: shared.UnitRefFromInt(startUnitID), EndUnitID: shared.UnitRefFromInt(endUnitID), }, } } -func responseSourceRefsInt(sourceID string, startUnitID int, endUnitID int) []shared.SourceRefResponse { - return []shared.SourceRefResponse{ - { - SourceID: sourceID, - StartUnitID: shared.UnitRefFromInt(startUnitID), - EndUnitID: shared.UnitRefFromInt(endUnitID), - }, - } +const spellTranscriptJSON = `{"id":"session-alpha","segments":[{"id":1,"text":"Aria raises her hand and casts Cure Wounds."}]}` + +func spellSourceInput() contracts.LLMInputMaterial { + return contracts.NewLLMInputMaterial("source", "application/json", []byte(spellTranscriptJSON), "sha256:transcript", "file:///session-alpha.json") +} + +func spellChunkInput(chunk *source.Chunk) contracts.LLMInputMaterial { + return contracts.NewLLMInputMaterial("source", chunk.MediaType, chunk.Content, "sha256:chunk", "file:///session-alpha.json") +} + +func extractionRequest() contracts.TypedExtractionRequest { + req := promptExtractionRequest() + req.SourceInput = spellChunkInput(req.Chunk) + req.SessionID = "session-123" + req.LLMProfile = "profile-spells" + return req +} + +func emptyChunkRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { + req.Chunk = &source.Chunk{ID: req.Chunk.ID, SourceID: req.Chunk.SourceID, Index: req.Chunk.Index} + return req +} + +func mismatchedSourceInputRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { + req.SourceInput = spellSourceInput() + return req +} + +func newExtractor(t *testing.T, client contracts.StructuredLLMClient) *Extractor { + t.Helper() + extractor, err := New(client, Options{}) + if err != nil { + t.Fatalf("New() error = %v, want nil", err) + } + return extractor +} + +type fakeSpellsLLMClient struct { + response extractionResponse + content []byte + err error + requests []contracts.StructuredCompletionRequest +} + +func (client *fakeSpellsLLMClient) CompleteStructured(_ context.Context, req contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { + client.requests = append(client.requests, cloneStructuredCompletionRequest(req)) + if client.err != nil { + return contracts.StructuredCompletionResponse{}, client.err + } + target, ok := out.(*extractionResponse) + if !ok { + return contracts.StructuredCompletionResponse{}, errors.New("unexpected output target") + } + *target = client.response + content := append([]byte(nil), client.content...) + if len(content) == 0 { + var err error + content, err = json.Marshal(client.response) + if err != nil { + return contracts.StructuredCompletionResponse{}, err + } + } + return contracts.StructuredCompletionResponse{Content: content}, nil +} + +func cloneStructuredCompletionRequest(req contracts.StructuredCompletionRequest) contracts.StructuredCompletionRequest { + req.Inputs = req.Inputs.Clone() + if len(req.Vars) == 0 { + req.Vars = nil + return req + } + vars := make(map[string]any, len(req.Vars)) + for key, value := range req.Vars { + vars[key] = value + } + req.Vars = vars + return req } diff --git a/internal/modules/dnd/register/register.go b/internal/modules/dnd/register/register.go index bda7874..5dc760f 100644 --- a/internal/modules/dnd/register/register.go +++ b/internal/modules/dnd/register/register.go @@ -7,6 +7,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/llm" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/chunk/scenes" + spellcodec "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/codec/spells" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/extract/spells" spellshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/shape" spellsourcerefs "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/source_refs" @@ -24,8 +25,9 @@ func Register(registries pipeline.Registries, assets *llm.AssetRegistry) error { name string register func() error }{ + {name: "spells codec", register: func() error { return pipeline.RegisterArtifactCodec(registries.ArtifactCodecs, spellcodec.New()) }}, {name: "scenes chunker", register: func() error { return scenes.Register(registries.Chunkers) }}, - {name: "spells extractor", register: func() error { return spells.Register(registries.Extractors) }}, + {name: "spells extractor", register: func() error { return spells.RegisterWithRawAdapter(registries.Extractors, spellcodec.New()) }}, {name: "spell shape validator", register: func() error { return spellshape.Register(registries.Validators) }}, {name: "spell source references validator", register: func() error { return spellsourcerefs.Register(registries.Validators) }}, {name: "spell source relatedness validator", register: func() error { return spellrelatedness.Register(registries.Validators) }}, @@ -57,6 +59,8 @@ func validateRegistries(registries pipeline.Registries, assets *llm.AssetRegistr switch { case registries.Chunkers == nil: return fmt.Errorf("dnd registrar: chunker registry must not be nil") + case registries.ArtifactCodecs == nil: + return fmt.Errorf("dnd registrar: artifact codec registry must not be nil") case registries.Extractors == nil: return fmt.Errorf("dnd registrar: extractor registry must not be nil") case registries.Validators == nil: diff --git a/internal/modules/dnd/register/register_test.go b/internal/modules/dnd/register/register_test.go index 79ff5dd..e115d84 100644 --- a/internal/modules/dnd/register/register_test.go +++ b/internal/modules/dnd/register/register_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/llm" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/extract/spells" @@ -20,6 +21,9 @@ func TestRegisterAddsDNDFamily(t *testing.T) { } assertKeys(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"dnd/scenes"}) assertKeys(t, "extractors", registries.Extractors.RegisteredKeys(), []string{"dnd/spells"}) + if got := registries.ArtifactCodecs.RegisteredKinds(); !reflect.DeepEqual(got, []contracts.ArtifactKind{"dnd/spell-list"}) { + t.Fatalf("artifact codec kinds = %#v, want dnd/spell-list", got) + } assertKeys(t, "validators", registries.Validators.RegisteredKeys(), []string{ "extract/dnd/spells/shape", "extract/dnd/spells/source_refs", @@ -51,7 +55,6 @@ func TestRegisterAddsDNDFamily(t *testing.T) { }) assertAssetNames(t, assets.SchemaFS, []string{ "dnd_scenes.v1.json", - "dnd_spells.v1.json", "dnd_spells_llm.v1.json", }) } @@ -63,6 +66,7 @@ func TestRegisterRejectsMissingDNDDependenciesBeforeMutation(t *testing.T) { wantErr string }{ {name: "chunkers", remove: func(r *pipeline.Registries, _ **llm.AssetRegistry) { r.Chunkers = nil }, wantErr: "chunker registry"}, + {name: "artifact codecs", remove: func(r *pipeline.Registries, _ **llm.AssetRegistry) { r.ArtifactCodecs = nil }, wantErr: "artifact codec registry"}, {name: "extractors", remove: func(r *pipeline.Registries, _ **llm.AssetRegistry) { r.Extractors = nil }, wantErr: "extractor registry"}, {name: "validators", remove: func(r *pipeline.Registries, _ **llm.AssetRegistry) { r.Validators = nil }, wantErr: "validator registry"}, {name: "validator chains", remove: func(r *pipeline.Registries, _ **llm.AssetRegistry) { r.ValidatorChains = nil }, wantErr: "validator chain registry"}, @@ -91,7 +95,7 @@ func TestRegisterReportsDuplicateDNDRegistration(t *testing.T) { t.Fatalf("first Register() error = %v, want nil", err) } err := Register(registries, assets) - if err == nil || !strings.Contains(err.Error(), "register dnd scenes chunker") || !strings.Contains(err.Error(), "already registered") { + if err == nil || !strings.Contains(err.Error(), "register dnd spells codec") || !strings.Contains(err.Error(), "already registered") { t.Fatalf("second Register() error = %v, want contextual duplicate error", err) } } diff --git a/internal/modules/dnd/types.go b/internal/modules/dnd/types.go new file mode 100644 index 0000000..0b06f63 --- /dev/null +++ b/internal/modules/dnd/types.go @@ -0,0 +1,21 @@ +// Package dnd owns the canonical in-process artifact types for the D&D domain. +package dnd + +import ( + "gitea.maximumdirect.net/eric/notarius/internal/core/source" + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" +) + +const SpellListKind contracts.ArtifactKind = "dnd/spell-list" + +type SpellList struct { + SpellCasts []SpellCast `json:"spell_casts"` +} + +type SpellCast struct { + Caster string `json:"caster"` + Spell string `json:"spell"` + Effect string `json:"effect"` + NarrativeDescription string `json:"narrative_description"` + SourceRefs []source.SourceRef `json:"source_refs"` +} diff --git a/internal/modules/integration/dnd_spells_config_test.go b/internal/modules/integration/dnd_spells_config_test.go index 83be655..7724687 100644 --- a/internal/modules/integration/dnd_spells_config_test.go +++ b/internal/modules/integration/dnd_spells_config_test.go @@ -10,6 +10,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" + spellcodec "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/codec/spells" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/extract/spells" "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/merge/appendorder" "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/normalize/noop" @@ -152,6 +153,7 @@ func dndSpellsTestCatalog(t *testing.T, specs dndSpellsCatalogSpecs) pipeline.Mo inputs := pipeline.NewInputAdapterRegistry() chunkers := pipeline.NewChunkerRegistry() extractors := pipeline.NewExtractorRegistry() + codecs := pipeline.NewArtifactCodecRegistry() mergers := pipeline.NewMergerRegistry() normalizers := pipeline.NewNormalizerRegistry() outputs := pipeline.NewOutputEncoderRegistry() @@ -177,13 +179,20 @@ func dndSpellsTestCatalog(t *testing.T, specs dndSpellsCatalogSpecs) pipeline.Mo } if specs.extractor.Key == "" { - if err := spells.Register(extractors); err != nil { + codec := spellcodec.New() + if err := pipeline.RegisterArtifactCodec(codecs, codec); err != nil { + t.Fatalf("register dnd spells codec: %v", err) + } + if err := spells.RegisterWithRawAdapter(extractors, codec); err != nil { t.Fatalf("register dnd spells extractor: %v", err) } - } else if err := extractors.RegisterLegacyRawWithSpec(specs.extractor, func() (contracts.LegacyRawExtractor, error) { - return spells.New(), nil - }); err != nil { - t.Fatalf("register dnd spells extractor override: %v", err) + } else { + specs.extractor.ArtifactKind = "" + if err := extractors.RegisterLegacyRawWithSpec(specs.extractor, func() (contracts.LegacyRawExtractor, error) { + return configLegacyExtractor{key: specs.extractor.Key}, nil + }); err != nil { + t.Fatalf("register dnd spells extractor override: %v", err) + } } if err := mergers.RegisterLegacyRawWithSpec(pipeline.ModuleSpec{ @@ -215,7 +224,7 @@ func dndSpellsTestCatalog(t *testing.T, specs dndSpellsCatalogSpecs) pipeline.Mo return pipeline.ModuleCatalog{ Inputs: inputs, Chunkers: chunkers, - ArtifactCodecs: pipeline.NewArtifactCodecRegistry(), + ArtifactCodecs: codecs, Extractors: extractors, Mergers: mergers, Normalizers: normalizers, @@ -224,6 +233,14 @@ func dndSpellsTestCatalog(t *testing.T, specs dndSpellsCatalogSpecs) pipeline.Mo } } +type configLegacyExtractor struct{ key string } + +func (extractor configLegacyExtractor) Key() string { return extractor.key } +func (configLegacyExtractor) ReferenceSlots() []contracts.ReferenceSlot { return nil } +func (configLegacyExtractor) Extract(context.Context, contracts.ExtractionRequest) (contracts.ExtractionResult, error) { + return contracts.ExtractionResult{}, nil +} + func dndSpellsChunkerSpec() pipeline.ModuleSpec { return pipeline.ModuleSpec{ Key: "fake/chunk",