From 9e3f8809b3ceb384017ae4b88893757a035ad212 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 7 Jul 2026 18:34:23 +0000 Subject: [PATCH] Implement integer source units and chunk payloads --- docs/integrations/dnd-spell-artifacts.md | 10 +- docs/integrations/json-output.md | 4 +- docs/integrations/seriatim.md | 10 +- docs/internal/modules.md | 21 ++- docs/internal/pipeline.md | 10 +- examples/seriatim-minimal-transcript.json | 4 +- internal/cli/run_test.go | 22 +-- internal/core/artifacts/artifacts_test.go | 8 +- internal/core/source/source.go | 6 +- internal/core/source/source_test.go | 63 ++++----- internal/core/source/validation.go | 33 ++--- .../framework/contracts/composition_test.go | 18 ++- internal/framework/contracts/contracts.go | 14 +- .../framework/contracts/contracts_test.go | 46 ++++--- .../framework/pipeline/chunk_validation.go | 49 +++++-- .../framework/pipeline/input_registry_test.go | 2 +- .../pipeline/registry_integration_test.go | 14 +- internal/framework/pipeline/runner_test.go | 128 +++++++++++++----- .../testdata/walking_skeleton_input.json | 6 +- .../testdata/walking_skeleton_output.json | 8 +- .../pipeline/walking_skeleton_test.go | 26 ++-- .../dnd/scenes/assets/prompts/instructions.md | 3 +- internal/modules/chunk/dnd/scenes/chunker.go | 52 +++++-- .../modules/chunk/dnd/scenes/chunker_test.go | 64 +++++---- internal/modules/chunk/dnd/scenes/model.go | 4 +- internal/modules/chunk/generic/chunker.go | 28 +++- .../modules/chunk/generic/chunker_test.go | 31 +++-- .../dnd/spells/assets/prompts/instructions.md | 3 +- .../modules/extract/dnd/spells/config_test.go | 12 +- .../extract/dnd/spells/extractor_test.go | 16 +-- .../modules/extract/dnd/spells/runner_test.go | 8 +- .../extract/dnd/spells/test_helpers_test.go | 24 ++-- .../testdata/seriatim_spell_session.json | 6 +- .../extract/dnd/spells/validator_test.go | 10 +- internal/modules/input/seriatim/adapter.go | 24 ++-- .../modules/input/seriatim/adapter_test.go | 40 +++--- internal/modules/input/seriatim/model.go | 43 +++++- .../modules/input/seriatim/runner_test.go | 30 ++-- .../testdata/duplicate_segment_id.json | 4 +- .../seriatim/testdata/valid_minimal.json | 4 +- .../modules/merge/appendorder/merger_test.go | 8 +- .../modules/normalize/noop/normalizer_test.go | 6 +- internal/modules/output/json/encoder_test.go | 6 +- .../modules/sharedassets/dnd/unit_refs.go | 95 ++++++------- .../sharedassets/dnd/unit_refs_test.go | 46 +++---- 45 files changed, 618 insertions(+), 451 deletions(-) diff --git a/docs/integrations/dnd-spell-artifacts.md b/docs/integrations/dnd-spell-artifacts.md index a8c1947..7d8087c 100644 --- a/docs/integrations/dnd-spell-artifacts.md +++ b/docs/integrations/dnd-spell-artifacts.md @@ -44,8 +44,8 @@ Approved artifacts use the generic artifact envelope documented in "source_refs": [ { "source_id": "session-alpha", - "start_unit_id": "seg-001", - "end_unit_id": "seg-001" + "start_unit_id": 1, + "end_unit_id": 1 } ] } @@ -78,7 +78,7 @@ Each source reference uses the generic source-reference shape: Validation requires: - at least one source reference; -- non-empty source ID and unit IDs; +- non-empty source ID and positive unit IDs; - source ID matching the source document ID; - start and end unit IDs existing in the source document; - start unit appearing before or at the same position as end unit. @@ -98,8 +98,8 @@ The extractor asks the LLM for this top-level response shape: "source_refs": [ { "source_id": "session-alpha", - "start_unit_id": "seg-001", - "end_unit_id": "seg-001" + "start_unit_id": 1, + "end_unit_id": 1 } ] } diff --git a/docs/integrations/json-output.md b/docs/integrations/json-output.md index cfff589..0ee935c 100644 --- a/docs/integrations/json-output.md +++ b/docs/integrations/json-output.md @@ -156,8 +156,8 @@ Each artifact file has this shape: "source_refs": [ { "source_id": "session-alpha", - "start_unit_id": "seg-001", - "end_unit_id": "seg-001" + "start_unit_id": 1, + "end_unit_id": 1 } ] } diff --git a/docs/integrations/seriatim.md b/docs/integrations/seriatim.md index f706f3c..4a33fe1 100644 --- a/docs/integrations/seriatim.md +++ b/docs/integrations/seriatim.md @@ -28,7 +28,7 @@ output that provides the same required segment fields. }, "segments": [ { - "id": "seg-001", + "id": 1, "start": 0, "end": 4, "speaker": "Aria", @@ -56,10 +56,9 @@ The adapter rejects: - missing, null, or non-object `metadata`; - missing, null, non-array, or empty `segments`; - segment values that are not objects; -- segment `id` values that are neither strings nor numbers; +- segment `id` values that are not positive integer JSON numbers or numeric + strings; - non-string `speaker` or `text`; -- empty segment IDs; -- segment IDs with leading or trailing whitespace; - duplicate segment IDs; - missing or empty `speaker`; - missing, empty, invalid, non-finite, or negative `start`; @@ -87,8 +86,7 @@ The adapter maps input to `SourceDocument`: Each segment becomes one `SourceUnit`: -- `segment.id` becomes `SourceUnit.ID`; numeric IDs are converted to their JSON - number text, so `1` becomes `"1"`; +- `segment.id` becomes integer `SourceUnit.ID`; - `segment.text` becomes `SourceUnit.Text`; - `SourceUnit.Kind` is `transcript_segment`; - `speaker`, `start`, and `end` are stored in source-unit metadata. diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 1422160..9db4ea6 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -84,9 +84,10 @@ The `generic` chunker splits source units into ordered chunks. It validates the source document, clones source units, assigns chunk IDs such as `chunk-000001`, and records chunk metadata for start unit, end unit, and unit count. -The pipeline runner canonicalizes chunk units from the source document by ID -before extractors and mergers run. Chunker-owned context should stay in -`SourceChunk.Metadata`. +The pipeline runner canonicalizes chunk units from the source document by +integer ID before extractors and mergers run. Chunkers also populate chunk +start and end unit IDs, content bytes, and media type. Chunker-owned context +should stay in `SourceChunk.Metadata`. Options: @@ -126,12 +127,11 @@ Options: none. Non-empty options are rejected. The chunker enforces full source-unit coverage from the first source unit to the last, sequential contiguous scenes, and no overlap. Its LLM-facing schema uses -integer `start_unit_id` and `end_unit_id` values as 1-based source-unit numbers; -the module canonicalizes valid integer references to source-unit IDs before -producing chunks. It assigns chunk IDs such as `scene-000001` and stores scene -metadata including title, primary mode, participants, summary, boundary note, -confidence, boundary unit IDs, and unit count. Boundary caveats become warnings -with reason code +integer `start_unit_id` and `end_unit_id` values matching source-unit IDs. It +assigns chunk IDs such as `scene-000001`, emits JSON chunk content, and stores +scene metadata including title, primary mode, participants, summary, boundary +note, confidence, boundary unit IDs, and unit count. Boundary caveats become +warnings with reason code `scene_boundary_caveat`. Whitespace-only caveats are treated as malformed structured output rather than silently dropped. @@ -150,8 +150,7 @@ input materials, response schema, and session ID to the runtime; converts spell-cast responses into artifact candidates; and supplies deterministic validators. Its LLM-facing source-reference schema uses integer `start_unit_id` and -`end_unit_id` values as 1-based source-unit numbers; the module canonicalizes -valid integer references to source-unit IDs before validation and output. +`end_unit_id` values matching source-unit IDs. Its prompt definition lives under `assets/prompts` and its schema under `assets/schemas`. Shared reusable D&D prompt fragments are provided by diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index b6c6ff3..422946e 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -136,15 +136,19 @@ invariants before running extractors: - chunk IDs must be non-empty and unique in the chunk result; - each chunk `SourceID` must match the source document ID; - each chunk `Index` must match its zero-based returned order; +- each chunk start and end unit ID must exist in the source document, with the + start unit at or before the end unit; +- each chunk must include non-empty extraction content and media type; - each chunk must contain at least one source unit; - a chunk must not repeat a source unit; - every chunk source unit must exist in the source document; - source units inside each chunk must appear in source-document order. After validation, the runner rebuilds each chunk from source-document units by -ID, preserving the chunk boundary order and cloning chunk metadata. Extractors -and downstream stages therefore see canonical source units, while -`SourceChunk.Metadata` remains the supported place for chunker-owned context. +integer ID, preserving chunk boundaries, content bytes, media type, and cloned +chunk metadata. Extractors and downstream stages therefore see canonical source +units, while `SourceChunk.Metadata` remains the supported place for +chunker-owned context. The framework does not require complete source-unit coverage and does not reject overlap between different chunks. Stricter policies, such as full coverage or diff --git a/examples/seriatim-minimal-transcript.json b/examples/seriatim-minimal-transcript.json index abf9c26..2c88b79 100644 --- a/examples/seriatim-minimal-transcript.json +++ b/examples/seriatim-minimal-transcript.json @@ -5,14 +5,14 @@ }, "segments": [ { - "id": "seg-001", + "id": 1, "start": 0, "end": 4, "speaker": "Aria", "text": "Aria raises her holy symbol and casts Cure Wounds." }, { - "id": "seg-002", + "id": 2, "start": 4, "end": 8, "speaker": "DM", diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index b498065..6987263 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -2012,7 +2012,7 @@ func TestExampleFixtureRunWritesExpectedJSON(t *testing.T) { t.Fatalf("source refs = %#v, want one source ref", artifactFile.Artifacts[0].SourceRefs) } ref := artifactFile.Artifacts[0].SourceRefs[0] - if ref.SourceID != "session-alpha" || ref.StartUnitID != "seg-001" || ref.EndUnitID != "seg-001" { + if ref.SourceID != "session-alpha" || ref.StartUnitID != 1 || ref.EndUnitID != 1 { t.Fatalf("source ref = %#v, want fixture source ref", ref) } @@ -2411,7 +2411,7 @@ func writeSeriatimInput(t *testing.T) string { }, "segments": [ { - "id": "seg-001", + "id": 1, "start": 0, "end": 1, "speaker": "Aria", @@ -2454,8 +2454,8 @@ func (client *fakeRunLLMClient) CompleteStructured(ctx context.Context, req cont payload := map[string]any{ "scenes": []map[string]any{ { - "start_unit_id": "seg-001", - "end_unit_id": "seg-002", + "start_unit_id": 1, + "end_unit_id": 2, "short_title": "Opening spell", "primary_mode": "Narrative", "main_participants": []string{"Aria"}, @@ -2478,9 +2478,9 @@ func (client *fakeRunLLMClient) CompleteStructured(ctx context.Context, req cont } return contracts.StructuredCompletionResponse{Content: encoded}, nil } - startUnitID := "seg-001" + startUnitID := 1 if client.invalidSourceRef { - startUnitID = "missing-segment" + startUnitID = 999 } payload := client.payload if payload == nil { @@ -2491,11 +2491,11 @@ func (client *fakeRunLLMClient) CompleteStructured(ctx context.Context, req cont "spell": "Cure Wounds", "effect": "Heals a wounded ally.", "narrative_description": "Aria casts Cure Wounds.", - "source_refs": []map[string]string{ + "source_refs": []map[string]any{ { "source_id": "session-alpha", "start_unit_id": startUnitID, - "end_unit_id": "seg-001", + "end_unit_id": 1, }, }, }, @@ -2653,7 +2653,7 @@ func (fakeRunInputAdapter) Parse(ctx context.Context, req contracts.ParseRequest Format: "test", Digest: "sha256:source", Units: []source.SourceUnit{ - {ID: "unit-1", Kind: "text", Text: string(req.Raw)}, + {ID: 1, Kind: "text", Text: string(req.Raw)}, }, }, nil } @@ -2671,7 +2671,7 @@ func (fakeRunChunker) ReferenceSlots() []contracts.ReferenceSlot { func (fakeRunChunker) Chunk(ctx context.Context, req contracts.ChunkRequest) (contracts.ChunkResult, error) { return contracts.ChunkResult{ Chunks: []contracts.SourceChunk{ - {ID: "chunk-1", SourceID: req.Source.ID, Index: 0, Units: req.Source.Units}, + {ID: "chunk-1", SourceID: req.Source.ID, Index: 0, StartUnitID: 1, EndUnitID: 1, Content: []byte(`{"units":[1]}`), MediaType: "application/json", Units: req.Source.Units}, }, }, nil } @@ -2704,7 +2704,7 @@ func (fakeRunExtractor) Extract(ctx context.Context, req contracts.ExtractionReq { Payload: []byte(`{"value":true}`), SourceRefs: []source.SourceRef{ - {SourceID: "source", StartUnitID: "unit-1", EndUnitID: "unit-1"}, + {SourceID: "source", StartUnitID: 1, EndUnitID: 1}, }, }, }, diff --git a/internal/core/artifacts/artifacts_test.go b/internal/core/artifacts/artifacts_test.go index 0a63c13..71dc875 100644 --- a/internal/core/artifacts/artifacts_test.go +++ b/internal/core/artifacts/artifacts_test.go @@ -16,7 +16,7 @@ func TestArtifactFromCandidatePreservesCandidateFields(t *testing.T) { SchemaVersion: "v1", Payload: json.RawMessage(`{"name":"example"}`), SourceRefs: []source.SourceRef{ - {SourceID: "source-1", StartUnitID: "u1", EndUnitID: "u2"}, + {SourceID: "source-1", StartUnitID: 1, EndUnitID: 2}, }, Metadata: map[string]any{ "confidence": 0.75, @@ -45,13 +45,13 @@ func TestArtifactFromCandidatePreservesCandidateFields(t *testing.T) { } candidate.Payload[0] = '[' - candidate.SourceRefs[0].StartUnitID = "changed" + candidate.SourceRefs[0].StartUnitID = 99 candidate.Metadata["confidence"] = 0.5 if string(artifact.Payload) != `{"name":"example"}` { t.Fatalf("Payload changed after candidate mutation: %s", artifact.Payload) } - if artifact.SourceRefs[0].StartUnitID != "u1" { + if artifact.SourceRefs[0].StartUnitID != 1 { t.Fatalf("SourceRefs changed after candidate mutation: %#v", artifact.SourceRefs) } if artifact.Metadata["confidence"] != 0.75 { @@ -67,7 +67,7 @@ func TestJSONMarshalUsesExpectedFieldNames(t *testing.T) { SchemaVersion: "v1", Payload: json.RawMessage(`{"value":true}`), SourceRefs: []source.SourceRef{ - {SourceID: "source-1", StartUnitID: "u1", EndUnitID: "u1"}, + {SourceID: "source-1", StartUnitID: 1, EndUnitID: 1}, }, Metadata: map[string]any{ "reviewed": true, diff --git a/internal/core/source/source.go b/internal/core/source/source.go index afecaea..eb16af6 100644 --- a/internal/core/source/source.go +++ b/internal/core/source/source.go @@ -10,7 +10,7 @@ type SourceDocument struct { } type SourceUnit struct { - ID string `json:"id"` + ID int `json:"id"` Kind string `json:"kind"` Text string `json:"text"` Metadata map[string]any `json:"metadata,omitempty"` @@ -18,6 +18,6 @@ type SourceUnit struct { type SourceRef struct { SourceID string `json:"source_id"` - StartUnitID string `json:"start_unit_id"` - EndUnitID string `json:"end_unit_id"` + StartUnitID int `json:"start_unit_id"` + EndUnitID int `json:"end_unit_id"` } diff --git a/internal/core/source/source_test.go b/internal/core/source/source_test.go index a7e2865..4b938dc 100644 --- a/internal/core/source/source_test.go +++ b/internal/core/source/source_test.go @@ -96,13 +96,8 @@ func TestValidateDocumentMissingUnitFields(t *testing.T) { }{ { name: "id", - mutate: func(doc *SourceDocument) { doc.Units[1].ID = "" }, - wantErr: "source unit[1].id must not be empty", - }, - { - name: "id surrounding whitespace", - mutate: func(doc *SourceDocument) { doc.Units[1].ID = " u2 " }, - wantErr: "source unit[1].id \" u2 \" must not contain leading or trailing whitespace", + mutate: func(doc *SourceDocument) { doc.Units[1].ID = 0 }, + wantErr: "source unit[1].id must be positive", }, { name: "kind", @@ -135,14 +130,14 @@ func TestValidateDocumentMissingUnitFields(t *testing.T) { func TestValidateDocumentDuplicateUnitIDs(t *testing.T) { doc := validDocument() - doc.Units[1].ID = "u1" + doc.Units[1].ID = 1 err := ValidateDocument(doc) if err == nil { t.Fatal("ValidateDocument() error = nil, want error") } - if err.Error() != "source unit id \"u1\" is duplicated" { + if err.Error() != "source unit id 1 is duplicated" { t.Fatalf("ValidateDocument() error = %q", err.Error()) } } @@ -151,8 +146,8 @@ func TestValidateRefValid(t *testing.T) { doc := validDocument() ref := SourceRef{ SourceID: "source-1", - StartUnitID: "u1", - EndUnitID: "u2", + StartUnitID: 1, + EndUnitID: 2, } if err := ValidateRef(doc, ref); err != nil { @@ -164,8 +159,8 @@ func TestValidateRefSourceIDMismatch(t *testing.T) { doc := validDocument() ref := SourceRef{ SourceID: "source-2", - StartUnitID: "u1", - EndUnitID: "u2", + StartUnitID: 1, + EndUnitID: 2, } err := ValidateRef(doc, ref) @@ -186,43 +181,33 @@ func TestValidateRefMissingUnitIDs(t *testing.T) { }{ { name: "missing source id", - ref: SourceRef{StartUnitID: "u1", EndUnitID: "u2"}, + ref: SourceRef{StartUnitID: 1, EndUnitID: 2}, wantErr: "source ref source_id must not be empty", }, { name: "source id surrounding whitespace", - ref: SourceRef{SourceID: " source-1 ", StartUnitID: "u1", EndUnitID: "u2"}, + ref: SourceRef{SourceID: " source-1 ", StartUnitID: 1, EndUnitID: 2}, wantErr: "source ref source_id \" source-1 \" must not contain leading or trailing whitespace", }, { name: "missing start id", - ref: SourceRef{SourceID: "source-1", EndUnitID: "u2"}, - wantErr: "source ref start_unit_id must not be empty", - }, - { - name: "start id surrounding whitespace", - ref: SourceRef{SourceID: "source-1", StartUnitID: " u1 ", EndUnitID: "u2"}, - wantErr: "source ref start_unit_id \" u1 \" must not contain leading or trailing whitespace", + ref: SourceRef{SourceID: "source-1", EndUnitID: 2}, + wantErr: "source ref start_unit_id must be positive", }, { name: "missing end id", - ref: SourceRef{SourceID: "source-1", StartUnitID: "u1"}, - wantErr: "source ref end_unit_id must not be empty", - }, - { - name: "end id surrounding whitespace", - ref: SourceRef{SourceID: "source-1", StartUnitID: "u1", EndUnitID: " u2 "}, - wantErr: "source ref end_unit_id \" u2 \" must not contain leading or trailing whitespace", + ref: SourceRef{SourceID: "source-1", StartUnitID: 1}, + wantErr: "source ref end_unit_id must be positive", }, { name: "unknown start id", - ref: SourceRef{SourceID: "source-1", StartUnitID: "u9", EndUnitID: "u2"}, - wantErr: "source ref start_unit_id \"u9\" was not found", + ref: SourceRef{SourceID: "source-1", StartUnitID: 9, EndUnitID: 2}, + wantErr: "source ref start_unit_id 9 was not found", }, { name: "unknown end id", - ref: SourceRef{SourceID: "source-1", StartUnitID: "u1", EndUnitID: "u9"}, - wantErr: "source ref end_unit_id \"u9\" was not found", + ref: SourceRef{SourceID: "source-1", StartUnitID: 1, EndUnitID: 9}, + wantErr: "source ref end_unit_id 9 was not found", }, } @@ -244,8 +229,8 @@ func TestValidateRefReversedUnitOrder(t *testing.T) { doc := validDocument() ref := SourceRef{ SourceID: "source-1", - StartUnitID: "u2", - EndUnitID: "u1", + StartUnitID: 2, + EndUnitID: 1, } err := ValidateRef(doc, ref) @@ -261,7 +246,7 @@ func TestValidateRefReversedUnitOrder(t *testing.T) { func TestUnitIndex(t *testing.T) { doc := validDocument() - index, ok := UnitIndex(doc, "u2") + index, ok := UnitIndex(doc, 2) if !ok { t.Fatal("UnitIndex() ok = false, want true") } @@ -269,7 +254,7 @@ func TestUnitIndex(t *testing.T) { t.Fatalf("UnitIndex() index = %d, want 1", index) } - index, ok = UnitIndex(doc, "u9") + index, ok = UnitIndex(doc, 9) if ok { t.Fatal("UnitIndex() ok = true, want false") } @@ -286,12 +271,12 @@ func validDocument() *SourceDocument { Digest: "sha256:abc123", Units: []SourceUnit{ { - ID: "u1", + ID: 1, Kind: "paragraph", Text: "First unit.", }, { - ID: "u2", + ID: 2, Kind: "paragraph", Text: "Second unit.", }, diff --git a/internal/core/source/validation.go b/internal/core/source/validation.go index dac150a..ffa85c6 100644 --- a/internal/core/source/validation.go +++ b/internal/core/source/validation.go @@ -28,13 +28,10 @@ func ValidateDocument(doc *SourceDocument) error { return fmt.Errorf("source document units must not be empty") } - seenUnitIDs := make(map[string]struct{}, len(doc.Units)) + seenUnitIDs := make(map[int]struct{}, len(doc.Units)) for i, unit := range doc.Units { - if isBlank(unit.ID) { - return fmt.Errorf("source unit[%d].id must not be empty", i) - } - if hasSurroundingWhitespace(unit.ID) { - return fmt.Errorf("source unit[%d].id %q must not contain leading or trailing whitespace", i, unit.ID) + if unit.ID <= 0 { + return fmt.Errorf("source unit[%d].id must be positive", i) } if isBlank(unit.Kind) { return fmt.Errorf("source unit[%d].kind must not be empty", i) @@ -43,7 +40,7 @@ func ValidateDocument(doc *SourceDocument) error { return fmt.Errorf("source unit[%d].text must not be empty", i) } if _, ok := seenUnitIDs[unit.ID]; ok { - return fmt.Errorf("source unit id %q is duplicated", unit.ID) + return fmt.Errorf("source unit id %d is duplicated", unit.ID) } seenUnitIDs[unit.ID] = struct{}{} } @@ -61,17 +58,11 @@ func ValidateRef(doc *SourceDocument, ref SourceRef) error { if hasSurroundingWhitespace(ref.SourceID) { return fmt.Errorf("source ref source_id %q must not contain leading or trailing whitespace", ref.SourceID) } - if isBlank(ref.StartUnitID) { - return fmt.Errorf("source ref start_unit_id must not be empty") + if ref.StartUnitID <= 0 { + return fmt.Errorf("source ref start_unit_id must be positive") } - if hasSurroundingWhitespace(ref.StartUnitID) { - return fmt.Errorf("source ref start_unit_id %q must not contain leading or trailing whitespace", ref.StartUnitID) - } - if isBlank(ref.EndUnitID) { - return fmt.Errorf("source ref end_unit_id must not be empty") - } - if hasSurroundingWhitespace(ref.EndUnitID) { - return fmt.Errorf("source ref end_unit_id %q must not contain leading or trailing whitespace", ref.EndUnitID) + if ref.EndUnitID <= 0 { + return fmt.Errorf("source ref end_unit_id must be positive") } if ref.SourceID != doc.ID { return fmt.Errorf("source ref source_id %q does not match document id %q", ref.SourceID, doc.ID) @@ -79,20 +70,20 @@ func ValidateRef(doc *SourceDocument, ref SourceRef) error { startIndex, ok := UnitIndex(doc, ref.StartUnitID) if !ok { - return fmt.Errorf("source ref start_unit_id %q was not found", ref.StartUnitID) + return fmt.Errorf("source ref start_unit_id %d was not found", ref.StartUnitID) } endIndex, ok := UnitIndex(doc, ref.EndUnitID) if !ok { - return fmt.Errorf("source ref end_unit_id %q was not found", ref.EndUnitID) + return fmt.Errorf("source ref end_unit_id %d was not found", ref.EndUnitID) } if startIndex > endIndex { - return fmt.Errorf("source ref start_unit_id %q appears after end_unit_id %q", ref.StartUnitID, ref.EndUnitID) + return fmt.Errorf("source ref start_unit_id %d appears after end_unit_id %d", ref.StartUnitID, ref.EndUnitID) } return nil } -func UnitIndex(doc *SourceDocument, unitID string) (int, bool) { +func UnitIndex(doc *SourceDocument, unitID int) (int, bool) { if doc == nil { return 0, false } diff --git a/internal/framework/contracts/composition_test.go b/internal/framework/contracts/composition_test.go index 154a34f..0fff29b 100644 --- a/internal/framework/contracts/composition_test.go +++ b/internal/framework/contracts/composition_test.go @@ -150,8 +150,8 @@ func (adapter compositionAdapter) Parse(ctx context.Context, req contracts.Parse Format: "text/plain", Digest: "sha256:abc123", Units: []source.SourceUnit{ - {ID: "u1", Kind: "unit", Text: "First source unit."}, - {ID: "u2", Kind: "unit", Text: "Second source unit."}, + {ID: 1, Kind: "unit", Text: "First source unit."}, + {ID: 2, Kind: "unit", Text: "Second source unit."}, }, }, nil } @@ -177,11 +177,15 @@ func (chunker compositionChunker) Chunk(ctx context.Context, req contracts.Chunk return contracts.ChunkResult{ Chunks: []contracts.SourceChunk{ { - ID: req.Source.ID + ":chunk:0", - SourceID: req.Source.ID, - Index: 0, - Units: append([]source.SourceUnit(nil), req.Source.Units...), - Metadata: map[string]any{"strategy": "whole-document"}, + ID: req.Source.ID + ":chunk:0", + SourceID: req.Source.ID, + Index: 0, + StartUnitID: req.Source.Units[0].ID, + EndUnitID: req.Source.Units[len(req.Source.Units)-1].ID, + Content: []byte(`{"units":[{"id":1,"kind":"unit","text":"First source unit."},{"id":2,"kind":"unit","text":"Second source unit."}]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), req.Source.Units...), + Metadata: map[string]any{"strategy": "whole-document"}, }, }, }, nil diff --git a/internal/framework/contracts/contracts.go b/internal/framework/contracts/contracts.go index 6c923f5..d2e4774 100644 --- a/internal/framework/contracts/contracts.go +++ b/internal/framework/contracts/contracts.go @@ -89,11 +89,15 @@ type InputAdapter interface { } type SourceChunk struct { - ID string `json:"id"` - SourceID string `json:"source_id"` - Index int `json:"index"` - Units []source.SourceUnit `json:"units"` - Metadata map[string]any `json:"metadata,omitempty"` + ID string `json:"id"` + SourceID string `json:"source_id"` + Index int `json:"index"` + StartUnitID int `json:"start_unit_id"` + EndUnitID int `json:"end_unit_id"` + Content []byte `json:"-"` + MediaType string `json:"media_type"` + Units []source.SourceUnit `json:"units"` + Metadata map[string]any `json:"metadata,omitempty"` } type ChunkRequest struct { diff --git a/internal/framework/contracts/contracts_test.go b/internal/framework/contracts/contracts_test.go index 350745e..a6d4020 100644 --- a/internal/framework/contracts/contracts_test.go +++ b/internal/framework/contracts/contracts_test.go @@ -33,7 +33,7 @@ func TestFakeExtractorReturnsCandidateAndValidator(t *testing.T) { Format: "text/plain", Digest: "sha256:abc123", Units: []source.SourceUnit{ - {ID: "u1", Kind: "section", Text: "Source text."}, + {ID: 1, Kind: "section", Text: "Source text."}, }, } @@ -86,7 +86,7 @@ func TestFakeChunkerReturnsSourceChunks(t *testing.T) { Format: "text/plain", Digest: "sha256:abc123", Units: []source.SourceUnit{ - {ID: "u1", Kind: "section", Text: "Source text."}, + {ID: 1, Kind: "section", Text: "Source text."}, }, } chunker := fakeChunker{key: "generic-chunker"} @@ -113,6 +113,12 @@ func TestFakeChunkerReturnsSourceChunks(t *testing.T) { if chunk.Index != 0 { t.Fatalf("SourceChunk.Index = %d, want 0", chunk.Index) } + if chunk.StartUnitID != 1 || chunk.EndUnitID != 1 { + t.Fatalf("SourceChunk boundaries = %d-%d, want 1-1", chunk.StartUnitID, chunk.EndUnitID) + } + if chunk.MediaType != "application/json" || string(chunk.Content) != `{"units":[{"id":1,"kind":"section","text":"Source text."}]}` { + t.Fatalf("SourceChunk payload = %q %s, want JSON units", chunk.MediaType, chunk.Content) + } if len(chunk.Units) != 1 { t.Fatalf("len(SourceChunk.Units) = %d, want 1", len(chunk.Units)) } @@ -125,7 +131,7 @@ func TestFakeChunkerReceivesLLMClient(t *testing.T) { Format: "text/plain", Digest: "sha256:abc123", Units: []source.SourceUnit{ - {ID: "u1", Kind: "section", Text: "Source text."}, + {ID: 1, Kind: "section", Text: "Source text."}, }, } client := fakeLLMClient{} @@ -151,15 +157,19 @@ func TestFakeExtractorReceivesChunkAndAmbientContext(t *testing.T) { Format: "text/plain", Digest: "sha256:abc123", Units: []source.SourceUnit{ - {ID: "u1", Kind: "section", Text: "First source text."}, - {ID: "u2", Kind: "section", Text: "Second source text."}, + {ID: 1, Kind: "section", Text: "First source text."}, + {ID: 2, Kind: "section", Text: "Second source text."}, }, } chunk := SourceChunk{ - ID: "source-1:chunk:1", - SourceID: doc.ID, - Index: 1, - Units: []source.SourceUnit{doc.Units[1]}, + ID: "source-1:chunk:1", + SourceID: doc.ID, + Index: 1, + StartUnitID: 2, + EndUnitID: 2, + Content: []byte(`{"units":[{"id":2,"kind":"section","text":"Second source text."}]}`), + MediaType: "application/json", + Units: []source.SourceUnit{doc.Units[1]}, } result, err := extractor.Extract(context.Background(), ExtractionRequest{ @@ -182,8 +192,8 @@ func TestFakeExtractorReceivesChunkAndAmbientContext(t *testing.T) { t.Fatalf("len(SourceRefs) = %d, want 1", len(candidate.SourceRefs)) } ref := candidate.SourceRefs[0] - if ref.StartUnitID != "u2" || ref.EndUnitID != "u2" { - t.Fatalf("SourceRef = %+v, want u2 range", ref) + if ref.StartUnitID != 2 || ref.EndUnitID != 2 { + t.Fatalf("SourceRef = %+v, want unit 2 range", ref) } } @@ -365,7 +375,7 @@ func TestFakeMergeNormalizeAndOutputContracts(t *testing.T) { SourceID: "source-1", Index: 0, Units: []source.SourceUnit{ - {ID: "u1", Kind: "section", Text: "Source text."}, + {ID: 1, Kind: "section", Text: "Source text."}, }, } merger := fakeMerger{key: "generic-merger"} @@ -487,10 +497,14 @@ func (chunker fakeChunker) Chunk(ctx context.Context, req ChunkRequest) (ChunkRe return ChunkResult{ Chunks: []SourceChunk{ { - ID: req.Source.ID + ":chunk:0", - SourceID: req.Source.ID, - Index: 0, - Units: append([]source.SourceUnit(nil), req.Source.Units...), + ID: req.Source.ID + ":chunk:0", + SourceID: req.Source.ID, + Index: 0, + StartUnitID: req.Source.Units[0].ID, + EndUnitID: req.Source.Units[len(req.Source.Units)-1].ID, + Content: []byte(`{"units":[{"id":1,"kind":"section","text":"Source text."}]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), req.Source.Units...), }, }, }, nil diff --git a/internal/framework/pipeline/chunk_validation.go b/internal/framework/pipeline/chunk_validation.go index 3c5077f..08b6021 100644 --- a/internal/framework/pipeline/chunk_validation.go +++ b/internal/framework/pipeline/chunk_validation.go @@ -9,8 +9,12 @@ import ( ) func validateAndCanonicalizeChunkResult(doc *source.SourceDocument, chunks []contracts.SourceChunk) ([]contracts.SourceChunk, error) { - sourceUnitIndexes := make(map[string]int, len(doc.Units)) - sourceUnits := make(map[string]source.SourceUnit, len(doc.Units)) + if len(chunks) == 0 { + return nil, fmt.Errorf("chunks must not be empty") + } + + sourceUnitIndexes := make(map[int]int, len(doc.Units)) + sourceUnits := make(map[int]source.SourceUnit, len(doc.Units)) for index, unit := range doc.Units { sourceUnitIndexes[unit.ID] = index sourceUnits[unit.ID] = unit @@ -33,25 +37,42 @@ func validateAndCanonicalizeChunkResult(doc *source.SourceDocument, chunks []con if chunk.Index != chunkIndex { return nil, fmt.Errorf("chunk %q index %d does not match returned order %d", chunk.ID, chunk.Index, chunkIndex) } + startIndex, ok := sourceUnitIndexes[chunk.StartUnitID] + if !ok { + return nil, fmt.Errorf("chunk %q start_unit_id %d was not found in source document %q", chunk.ID, chunk.StartUnitID, doc.ID) + } + endIndex, ok := sourceUnitIndexes[chunk.EndUnitID] + if !ok { + return nil, fmt.Errorf("chunk %q end_unit_id %d was not found in source document %q", chunk.ID, chunk.EndUnitID, doc.ID) + } + if startIndex > endIndex { + return nil, fmt.Errorf("chunk %q start_unit_id %d appears after end_unit_id %d", chunk.ID, chunk.StartUnitID, chunk.EndUnitID) + } if len(chunk.Units) == 0 { return nil, fmt.Errorf("chunk %q units must not be empty", chunk.ID) } + if len(chunk.Content) == 0 { + return nil, fmt.Errorf("chunk %q content must not be empty", chunk.ID) + } + if strings.TrimSpace(chunk.MediaType) == "" { + return nil, fmt.Errorf("chunk %q media_type must not be empty", chunk.ID) + } - seenUnitIDs := make(map[string]struct{}, len(chunk.Units)) + seenUnitIDs := make(map[int]struct{}, len(chunk.Units)) previousSourceIndex := -1 canonicalUnits := make([]source.SourceUnit, 0, len(chunk.Units)) for unitIndex, unit := range chunk.Units { - if strings.TrimSpace(unit.ID) == "" { - return nil, fmt.Errorf("chunk %q unit[%d].id must not be empty", chunk.ID, unitIndex) + if unit.ID <= 0 { + return nil, fmt.Errorf("chunk %q unit[%d].id must be positive", chunk.ID, unitIndex) } if _, ok := seenUnitIDs[unit.ID]; ok { - return nil, fmt.Errorf("chunk %q repeats source unit %q", chunk.ID, unit.ID) + return nil, fmt.Errorf("chunk %q repeats source unit %d", chunk.ID, unit.ID) } seenUnitIDs[unit.ID] = struct{}{} sourceIndex, ok := sourceUnitIndexes[unit.ID] if !ok { - return nil, fmt.Errorf("chunk %q source unit %q was not found in source document %q", chunk.ID, unit.ID, doc.ID) + return nil, fmt.Errorf("chunk %q source unit %d was not found in source document %q", chunk.ID, unit.ID, doc.ID) } if sourceIndex <= previousSourceIndex { return nil, fmt.Errorf("chunk %q source units must appear in source document order", chunk.ID) @@ -61,11 +82,15 @@ func validateAndCanonicalizeChunkResult(doc *source.SourceDocument, chunks []con } canonicalChunks = append(canonicalChunks, contracts.SourceChunk{ - ID: chunk.ID, - SourceID: chunk.SourceID, - Index: chunk.Index, - Units: canonicalUnits, - Metadata: cloneMetadata(chunk.Metadata), + ID: chunk.ID, + SourceID: chunk.SourceID, + Index: chunk.Index, + StartUnitID: chunk.StartUnitID, + EndUnitID: chunk.EndUnitID, + Content: append([]byte(nil), chunk.Content...), + MediaType: chunk.MediaType, + Units: canonicalUnits, + Metadata: cloneMetadata(chunk.Metadata), }) } diff --git a/internal/framework/pipeline/input_registry_test.go b/internal/framework/pipeline/input_registry_test.go index dd33c1f..bf8d433 100644 --- a/internal/framework/pipeline/input_registry_test.go +++ b/internal/framework/pipeline/input_registry_test.go @@ -301,7 +301,7 @@ func (adapter fakeAdapter) Parse(ctx context.Context, req contracts.ParseRequest Format: "text/plain", Digest: "sha256:abc123", Units: []source.SourceUnit{ - {ID: "u1", Kind: "unit", Text: "Source unit."}, + {ID: 1, Kind: "unit", Text: "Source unit."}, }, }, nil } diff --git a/internal/framework/pipeline/registry_integration_test.go b/internal/framework/pipeline/registry_integration_test.go index 0fcfc20..19af1a8 100644 --- a/internal/framework/pipeline/registry_integration_test.go +++ b/internal/framework/pipeline/registry_integration_test.go @@ -126,10 +126,14 @@ func (chunker integrationChunker) Chunk(ctx context.Context, req contracts.Chunk return contracts.ChunkResult{ Chunks: []contracts.SourceChunk{ { - ID: "chunk-0", - SourceID: req.Source.ID, - Index: 0, - Units: req.Source.Units, + ID: "chunk-0", + SourceID: req.Source.ID, + Index: 0, + StartUnitID: req.Source.Units[0].ID, + EndUnitID: req.Source.Units[len(req.Source.Units)-1].ID, + Content: []byte(`{"units":[1]}`), + MediaType: "application/json", + Units: req.Source.Units, }, }, }, nil @@ -267,7 +271,7 @@ func integrationSourceDocument() *source.SourceDocument { Format: "text/plain", Digest: "sha256:abc123", Units: []source.SourceUnit{ - {ID: "u1", Kind: "unit", Text: "Source unit."}, + {ID: 1, Kind: "unit", Text: "Source unit."}, }, } } diff --git a/internal/framework/pipeline/runner_test.go b/internal/framework/pipeline/runner_test.go index 5a3194f..ac19ece 100644 --- a/internal/framework/pipeline/runner_test.go +++ b/internal/framework/pipeline/runner_test.go @@ -264,45 +264,70 @@ func TestRunRejectsInvalidChunks(t *testing.T) { }{ { name: "empty chunk id", - chunks: []contracts.SourceChunk{{ID: "", SourceID: "source-1", Index: 0, Units: []source.SourceUnit{unitWithID("u1")}}}, + chunks: []contracts.SourceChunk{chunkWithUnits("", "source-1", 0, unitWithID("u1"))}, want: "id must not be empty", }, { name: "duplicate chunk id", chunks: []contracts.SourceChunk{ - {ID: "chunk-0", SourceID: "source-1", Index: 0, Units: []source.SourceUnit{unitWithID("u1")}}, - {ID: "chunk-0", SourceID: "source-1", Index: 1, Units: []source.SourceUnit{unitWithID("u2")}}, + chunkWithUnits("chunk-0", "source-1", 0, unitWithID("u1")), + chunkWithUnits("chunk-0", "source-1", 1, unitWithID("u2")), }, want: "duplicated", }, { name: "wrong source id", - chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "other-source", Index: 0, Units: []source.SourceUnit{unitWithID("u1")}}}, + chunks: []contracts.SourceChunk{chunkWithUnits("chunk-0", "other-source", 0, unitWithID("u1"))}, want: "source_id", }, { name: "wrong index", - chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 1, Units: []source.SourceUnit{unitWithID("u1")}}}, + chunks: []contracts.SourceChunk{chunkWithUnits("chunk-0", "source-1", 1, unitWithID("u1"))}, want: "index", }, + { + name: "unknown start id", + chunks: []contracts.SourceChunk{chunkWithBounds("chunk-0", "source-1", 0, 9, 1, unitWithID("u1"))}, + want: "start_unit_id", + }, + { + name: "unknown end id", + chunks: []contracts.SourceChunk{chunkWithBounds("chunk-0", "source-1", 0, 1, 9, unitWithID("u1"))}, + want: "end_unit_id", + }, + { + name: "reversed bounds", + chunks: []contracts.SourceChunk{chunkWithBounds("chunk-0", "source-1", 0, 2, 1, unitWithID("u1"), unitWithID("u2"))}, + want: "appears after", + }, { name: "empty units", - chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 0}}, + chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 0, StartUnitID: 1, EndUnitID: 1, Content: []byte(`{"units":[]}`), MediaType: "application/json"}}, want: "units must not be empty", }, + { + name: "empty content", + chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 0, StartUnitID: 1, EndUnitID: 1, MediaType: "application/json", Units: []source.SourceUnit{unitWithID("u1")}}}, + want: "content must not be empty", + }, + { + name: "empty media type", + chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 0, StartUnitID: 1, EndUnitID: 1, Content: []byte(`{"units":[1]}`), Units: []source.SourceUnit{unitWithID("u1")}}}, + want: "media_type must not be empty", + }, { name: "repeated unit inside chunk", - chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 0, Units: []source.SourceUnit{unitWithID("u1"), unitWithID("u1")}}}, + chunks: []contracts.SourceChunk{chunkWithUnits("chunk-0", "source-1", 0, unitWithID("u1"), unitWithID("u1"))}, want: "repeats source unit", }, { name: "unknown unit", - chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 0, Units: []source.SourceUnit{unitWithID("u9")}}}, + chunks: []contracts.SourceChunk{chunkWithUnits("chunk-0", "source-1", 0, unitWithID("u9"))}, want: "was not found", }, { name: "units out of source order", - chunks: []contracts.SourceChunk{{ID: "chunk-0", SourceID: "source-1", Index: 0, Units: []source.SourceUnit{unitWithID("u2"), unitWithID("u1")}}}, + chunks: []contracts.SourceChunk{chunkWithBounds("chunk-0", "source-1", 0, 1, 2, unitWithID("u2"), unitWithID("u1"))}, want: "source document order", }, } @@ -328,8 +353,8 @@ func TestRunRejectsInvalidChunks(t *testing.T) { func TestRunAllowsPartialCoverageAndOverlappingChunks(t *testing.T) { modules := defaultRunnerModules() modules.chunker.chunks = []contracts.SourceChunk{ - {ID: "chunk-0", SourceID: "source-1", Index: 0, Units: []source.SourceUnit{unitWithID("u1"), unitWithID("u2")}}, - {ID: "chunk-1", SourceID: "source-1", Index: 1, Units: []source.SourceUnit{unitWithID("u2")}}, + chunkWithUnits("chunk-0", "source-1", 0, unitWithID("u1"), unitWithID("u2")), + chunkWithUnits("chunk-1", "source-1", 1, unitWithID("u2")), } output, err := New(newRunnerRegistries(t, modules)).Run(context.Background(), RunInput{Pipeline: resolvedPipeline()}) @@ -346,12 +371,16 @@ func TestRunCanonicalizesChunkUnitsBeforeExtraction(t *testing.T) { modules.input.doc = sourceDocumentWithUnitMetadata() modules.chunker.chunks = []contracts.SourceChunk{ { - ID: "chunk-0", - SourceID: "source-1", - Index: 0, + ID: "chunk-0", + SourceID: "source-1", + Index: 0, + StartUnitID: 1, + EndUnitID: 1, + Content: []byte(`{"units":[{"id":1}]}`), + MediaType: "application/json", Units: []source.SourceUnit{ { - ID: "u1", + ID: 1, Kind: "mutated-kind", Text: "mutated text", Metadata: map[string]any{ @@ -380,7 +409,7 @@ func TestRunCanonicalizesChunkUnitsBeforeExtraction(t *testing.T) { if chunk == nil { t.Fatal("extractor chunk = nil, want canonical chunk") } - if chunk.Units[0].ID != "u1" || chunk.Units[0].Kind != "source-kind" || chunk.Units[0].Text != "source text" { + if chunk.Units[0].ID != 1 || chunk.Units[0].Kind != "source-kind" || chunk.Units[0].Text != "source text" { t.Fatalf("chunk unit = %#v, want source document unit values", chunk.Units[0]) } if got := chunk.Units[0].Metadata["speaker"]; got != "source-speaker" { @@ -402,9 +431,13 @@ func TestRunPreservesChunkMetadataDuringCanonicalization(t *testing.T) { modules := defaultRunnerModules() modules.chunker.chunks = []contracts.SourceChunk{ { - ID: "chunk-0", - SourceID: "source-1", - Index: 0, + ID: "chunk-0", + SourceID: "source-1", + Index: 0, + StartUnitID: 1, + EndUnitID: 1, + Content: []byte(`{"units":[{"id":1}]}`), + MediaType: "application/json", Units: []source.SourceUnit{ unitWithID("u1"), }, @@ -1739,9 +1772,9 @@ func validSourceDocument() *source.SourceDocument { Format: "text/plain", Digest: "sha256:source", Units: []source.SourceUnit{ - {ID: "u1", Kind: "unit", Text: "Source unit."}, - {ID: "u2", Kind: "unit", Text: "Second source unit."}, - {ID: "u3", Kind: "unit", Text: "Third source unit."}, + {ID: 1, Kind: "unit", Text: "Source unit."}, + {ID: 2, Kind: "unit", Text: "Second source unit."}, + {ID: 3, Kind: "unit", Text: "Third source unit."}, }, } } @@ -1754,7 +1787,7 @@ func sourceDocumentWithUnitMetadata() *source.SourceDocument { Digest: "sha256:source", Units: []source.SourceUnit{ { - ID: "u1", + ID: 1, Kind: "source-kind", Text: "source text", Metadata: map[string]any{ @@ -1763,7 +1796,7 @@ func sourceDocumentWithUnitMetadata() *source.SourceDocument { }, }, { - ID: "u2", + ID: 2, Kind: "source-kind", Text: "second source text", Metadata: map[string]any{ @@ -1775,26 +1808,53 @@ func sourceDocumentWithUnitMetadata() *source.SourceDocument { } func sourceChunkWithID(id string, index int) contracts.SourceChunk { + unit := unitWithID("u1") return contracts.SourceChunk{ - ID: id, - SourceID: "source-1", - Index: index, - Units: []source.SourceUnit{ - {ID: "u1", Kind: "unit", Text: "Source unit."}, - }, + ID: id, + SourceID: "source-1", + Index: index, + StartUnitID: unit.ID, + EndUnitID: unit.ID, + Content: []byte(`{"units":[{"id":1,"kind":"unit","text":"Source unit."}]}`), + MediaType: "application/json", + Units: []source.SourceUnit{unit}, } } func unitWithID(id string) source.SourceUnit { switch id { case "u1": - return source.SourceUnit{ID: "u1", Kind: "unit", Text: "Source unit."} + return source.SourceUnit{ID: 1, Kind: "unit", Text: "Source unit."} case "u2": - return source.SourceUnit{ID: "u2", Kind: "unit", Text: "Second source unit."} + return source.SourceUnit{ID: 2, Kind: "unit", Text: "Second source unit."} case "u3": - return source.SourceUnit{ID: "u3", Kind: "unit", Text: "Third source unit."} + return source.SourceUnit{ID: 3, Kind: "unit", Text: "Third source unit."} + case "u9": + return source.SourceUnit{ID: 9, Kind: "unit", Text: "Unknown source unit."} default: - return source.SourceUnit{ID: id, Kind: "unit", Text: "Unknown source unit."} + return source.SourceUnit{ID: 99, Kind: "unit", Text: "Unknown source unit."} + } +} + +func chunkWithUnits(id string, sourceID string, index int, units ...source.SourceUnit) contracts.SourceChunk { + startUnitID, endUnitID := 1, 1 + if len(units) > 0 { + startUnitID = units[0].ID + endUnitID = units[len(units)-1].ID + } + return chunkWithBounds(id, sourceID, index, startUnitID, endUnitID, units...) +} + +func chunkWithBounds(id string, sourceID string, index int, startUnitID int, endUnitID int, units ...source.SourceUnit) contracts.SourceChunk { + return contracts.SourceChunk{ + ID: id, + SourceID: sourceID, + Index: index, + StartUnitID: startUnitID, + EndUnitID: endUnitID, + Content: []byte(`{"units":[1]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), units...), } } diff --git a/internal/framework/pipeline/testdata/walking_skeleton_input.json b/internal/framework/pipeline/testdata/walking_skeleton_input.json index addac39..3a6a180 100644 --- a/internal/framework/pipeline/testdata/walking_skeleton_input.json +++ b/internal/framework/pipeline/testdata/walking_skeleton_input.json @@ -2,15 +2,15 @@ "id": "fixture-source", "units": [ { - "id": "u1", + "id": 1, "text": "First event." }, { - "id": "u2", + "id": 2, "text": "Second event." }, { - "id": "u3", + "id": 3, "text": "Third event." } ] diff --git a/internal/framework/pipeline/testdata/walking_skeleton_output.json b/internal/framework/pipeline/testdata/walking_skeleton_output.json index bdae2c4..936644f 100644 --- a/internal/framework/pipeline/testdata/walking_skeleton_output.json +++ b/internal/framework/pipeline/testdata/walking_skeleton_output.json @@ -25,8 +25,8 @@ "source_refs": [ { "source_id": "fixture-source", - "start_unit_id": "u1", - "end_unit_id": "u2" + "start_unit_id": 1, + "end_unit_id": 2 } ] }, @@ -42,8 +42,8 @@ "source_refs": [ { "source_id": "fixture-source", - "start_unit_id": "u3", - "end_unit_id": "u3" + "start_unit_id": 3, + "end_unit_id": 3 } ] } diff --git a/internal/framework/pipeline/walking_skeleton_test.go b/internal/framework/pipeline/walking_skeleton_test.go index b8b5f52..1fe0d6e 100644 --- a/internal/framework/pipeline/walking_skeleton_test.go +++ b/internal/framework/pipeline/walking_skeleton_test.go @@ -185,7 +185,7 @@ func (input walkingSkeletonInput) Parse(ctx context.Context, req contracts.Parse var fixture struct { ID string `json:"id"` Units []struct { - ID string `json:"id"` + ID int `json:"id"` Text string `json:"text"` } `json:"units"` } @@ -227,16 +227,24 @@ func (chunker walkingSkeletonChunker) Chunk(ctx context.Context, req contracts.C return contracts.ChunkResult{ Chunks: []contracts.SourceChunk{ { - ID: req.Source.ID + ":chunk:0", - SourceID: req.Source.ID, - Index: 0, - Units: append([]source.SourceUnit(nil), req.Source.Units[:2]...), + ID: req.Source.ID + ":chunk:0", + SourceID: req.Source.ID, + Index: 0, + StartUnitID: req.Source.Units[0].ID, + EndUnitID: req.Source.Units[1].ID, + Content: []byte(`{"units":[1,2]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), req.Source.Units[:2]...), }, { - ID: req.Source.ID + ":chunk:1", - SourceID: req.Source.ID, - Index: 1, - Units: append([]source.SourceUnit(nil), req.Source.Units[2:]...), + ID: req.Source.ID + ":chunk:1", + SourceID: req.Source.ID, + Index: 1, + StartUnitID: req.Source.Units[2].ID, + EndUnitID: req.Source.Units[len(req.Source.Units)-1].ID, + Content: []byte(`{"units":[3]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), req.Source.Units[2:]...), }, }, }, nil diff --git a/internal/modules/chunk/dnd/scenes/assets/prompts/instructions.md b/internal/modules/chunk/dnd/scenes/assets/prompts/instructions.md index 54669da..fe0437c 100644 --- a/internal/modules/chunk/dnd/scenes/assets/prompts/instructions.md +++ b/internal/modules/chunk/dnd/scenes/assets/prompts/instructions.md @@ -22,8 +22,7 @@ dnd/scenes boundary policy: - return sequential scenes with no gaps; - do not overlap scenes; - preserve source-unit order; -- use 1-based integer source-unit numbers from the transcript, where 1 is the - first provided source unit; +- use integer source-unit IDs from the transcript; - each scene must have start_unit_id and end_unit_id; - do not include final chunk IDs or chunk indexes. diff --git a/internal/modules/chunk/dnd/scenes/chunker.go b/internal/modules/chunk/dnd/scenes/chunker.go index 01cb0a4..e84ad06 100644 --- a/internal/modules/chunk/dnd/scenes/chunker.go +++ b/internal/modules/chunk/dnd/scenes/chunker.go @@ -2,6 +2,7 @@ package scenes import ( "context" + "encoding/json" "fmt" "strings" @@ -142,7 +143,7 @@ func chunksFromResponse(doc *source.SourceDocument, response chunkResponse) ([]c return nil, fmt.Errorf("scenes must not be empty") } - unitIndexes := make(map[string]int, len(doc.Units)) + unitIndexes := make(map[int]int, len(doc.Units)) for i, unit := range doc.Units { unitIndexes[unit.ID] = i } @@ -157,18 +158,18 @@ func chunksFromResponse(doc *source.SourceDocument, response chunkResponse) ([]c startIndex, ok := unitIndexes[normalized.StartUnitID] if !ok { - return nil, fmt.Errorf("scene[%d] start_unit_id %q was not found", i, normalized.StartUnitID) + return nil, fmt.Errorf("scene[%d] start_unit_id %d was not found", i, normalized.StartUnitID) } endIndex, ok := unitIndexes[normalized.EndUnitID] if !ok { - return nil, fmt.Errorf("scene[%d] end_unit_id %q was not found", i, normalized.EndUnitID) + return nil, fmt.Errorf("scene[%d] end_unit_id %d was not found", i, normalized.EndUnitID) } if startIndex > endIndex { - return nil, fmt.Errorf("scene[%d] start_unit_id %q appears after end_unit_id %q", i, normalized.StartUnitID, normalized.EndUnitID) + return nil, fmt.Errorf("scene[%d] start_unit_id %d appears after end_unit_id %d", i, normalized.StartUnitID, normalized.EndUnitID) } if i == 0 && startIndex != 0 { - return nil, fmt.Errorf("first scene must start at first source unit %q", doc.Units[0].ID) + return nil, fmt.Errorf("first scene must start at first source unit %d", doc.Units[0].ID) } if i > 0 { if startIndex <= previousEnd { @@ -181,11 +182,19 @@ func chunksFromResponse(doc *source.SourceDocument, response chunkResponse) ([]c previousEnd = endIndex units := cloneUnits(doc.Units[startIndex : endIndex+1]) + content, err := chunkContent(units) + if err != nil { + return nil, err + } chunks = append(chunks, contracts.SourceChunk{ - ID: fmt.Sprintf("scene-%06d", i+1), - SourceID: doc.ID, - Index: i, - Units: units, + ID: fmt.Sprintf("scene-%06d", i+1), + SourceID: doc.ID, + Index: i, + StartUnitID: units[0].ID, + EndUnitID: units[len(units)-1].ID, + Content: content, + MediaType: "application/json", + Units: units, Metadata: map[string]any{ "scene_title": normalized.ShortTitle, "primary_mode": normalized.PrimaryMode, @@ -201,11 +210,23 @@ func chunksFromResponse(doc *source.SourceDocument, response chunkResponse) ([]c } if previousEnd != len(doc.Units)-1 { - return nil, fmt.Errorf("final scene must end at final source unit %q", doc.Units[len(doc.Units)-1].ID) + return nil, fmt.Errorf("final scene must end at final source unit %d", doc.Units[len(doc.Units)-1].ID) } return chunks, nil } +func chunkContent(units []source.SourceUnit) ([]byte, error) { + content, err := json.Marshal(struct { + Units []source.SourceUnit `json:"units"` + }{ + Units: units, + }) + if err != nil { + return nil, fmt.Errorf("encode chunk content: %w", err) + } + return content, nil +} + func normalizeScene(doc *source.SourceDocument, index int, scene sceneResponse) (normalizedScene, error) { startUnitID, err := dnd.ResolveUnitID(doc, "start_unit_id", scene.StartUnitID) if err != nil { @@ -226,9 +247,16 @@ func normalizeScene(doc *source.SourceDocument, index int, scene sceneResponse) BoundaryConfidence: strings.TrimSpace(scene.BoundaryConfidence), } + requiredInts := map[string]int{ + "start_unit_id": out.StartUnitID, + "end_unit_id": out.EndUnitID, + } + for field, value := range requiredInts { + if value <= 0 { + return normalizedScene{}, fmt.Errorf("scene[%d] %s must be positive", index, field) + } + } required := map[string]string{ - "start_unit_id": out.StartUnitID, - "end_unit_id": out.EndUnitID, "short_title": out.ShortTitle, "primary_mode": out.PrimaryMode, "summary": out.Summary, diff --git a/internal/modules/chunk/dnd/scenes/chunker_test.go b/internal/modules/chunk/dnd/scenes/chunker_test.go index da65705..fdb9da9 100644 --- a/internal/modules/chunk/dnd/scenes/chunker_test.go +++ b/internal/modules/chunk/dnd/scenes/chunker_test.go @@ -170,8 +170,8 @@ func TestChunkReturnsSceneChunksFromStructuredOutput(t *testing.T) { if got := chunkIDs(result.Chunks); !reflect.DeepEqual(got, []string{"scene-000001", "scene-000002"}) { t.Fatalf("chunk IDs = %#v, want deterministic scene IDs", got) } - gotUnits := [][]string{unitIDs(result.Chunks[0].Units), unitIDs(result.Chunks[1].Units)} - wantUnits := [][]string{{"seg-001", "seg-002"}, {"seg-003", "seg-004"}} + gotUnits := [][]int{unitIDs(result.Chunks[0].Units), unitIDs(result.Chunks[1].Units)} + wantUnits := [][]int{{1, 2}, {3, 4}} if !reflect.DeepEqual(gotUnits, wantUnits) { t.Fatalf("chunk units = %#v, want %#v", gotUnits, wantUnits) } @@ -179,13 +179,19 @@ func TestChunkReturnsSceneChunksFromStructuredOutput(t *testing.T) { if first.SourceID != "session-alpha" || first.Index != 0 { t.Fatalf("first chunk = %#v, want source and index fields", first) } + if first.StartUnitID != 1 || first.EndUnitID != 2 { + t.Fatalf("first boundaries = %d-%d, want 1-2", first.StartUnitID, first.EndUnitID) + } + if first.MediaType != "application/json" || len(first.Content) == 0 { + t.Fatalf("first payload = media type %q length %d, want JSON content", first.MediaType, len(first.Content)) + } if first.Metadata["scene_title"] != "Goblin parley" || first.Metadata["primary_mode"] != "Discussion" || first.Metadata["summary"] != "The party negotiates with a scout." || first.Metadata["boundary_note"] != "The scene covers the discussion before fighting starts." || first.Metadata["boundary_confidence"] != "High" || - first.Metadata["start_unit_id"] != "seg-001" || - first.Metadata["end_unit_id"] != "seg-002" || + first.Metadata["start_unit_id"] != 1 || + first.Metadata["end_unit_id"] != 2 || first.Metadata["unit_count"] != 2 { t.Fatalf("first metadata = %#v, want scene metadata", first.Metadata) } @@ -311,11 +317,11 @@ func TestChunkDefensivelyCopiesSourceUnitsAndMetadata(t *testing.T) { t.Fatalf("Chunk() error = %v, want nil", err) } - doc.Units[0].ID = "mutated" + doc.Units[0].ID = 99 doc.Units[0].Metadata["speaker"] = "mutated" client.response.Scenes[0].MainParticipants[0] = "mutated" - if result.Chunks[0].Units[0].ID != "seg-001" { + if result.Chunks[0].Units[0].ID != 1 { t.Fatalf("chunk unit ID changed after source mutation: %#v", result.Chunks[0].Units[0]) } if result.Chunks[0].Units[0].Metadata["speaker"] != "Alice" { @@ -362,7 +368,7 @@ func TestChunkRejectsInvalidRequests(t *testing.T) { canceledCtx, cancel := context.WithCancel(context.Background()) cancel() invalidDoc := sceneSourceDocument() - invalidDoc.Units[0].ID = "" + invalidDoc.Units[0].ID = 0 emptyDoc := sceneSourceDocument() emptyDoc.Units = nil @@ -407,37 +413,37 @@ func TestChunkRejectsMalformedStructuredOutput(t *testing.T) { { name: "unknown boundary id", response: replaceScenes(validSceneResponse(), []sceneResponse{ - scene("seg-001", "seg-999"), + scene(1, 999), }), want: "was not found", }, { name: "out of order boundaries", response: replaceScenes(validSceneResponse(), []sceneResponse{ - scene("seg-003", "seg-002"), + scene(3, 2), }), want: "appears after", }, { name: "gap", response: replaceScenes(validSceneResponse(), []sceneResponse{ - scene("seg-001", "seg-001"), - scene("seg-003", "seg-004"), + scene(1, 1), + scene(3, 4), }), want: "gap", }, { name: "overlap", response: replaceScenes(validSceneResponse(), []sceneResponse{ - scene("seg-001", "seg-002"), - scene("seg-002", "seg-004"), + scene(1, 2), + scene(2, 4), }), want: "overlap", }, { name: "incomplete coverage", response: replaceScenes(validSceneResponse(), []sceneResponse{ - scene("seg-001", "seg-003"), + scene(1, 3), }), want: "final scene", }, @@ -445,8 +451,8 @@ func TestChunkRejectsMalformedStructuredOutput(t *testing.T) { name: "empty metadata field", response: replaceScenes(validSceneResponse(), []sceneResponse{ { - StartUnitID: dnd.UnitRefFromString("seg-001"), - EndUnitID: dnd.UnitRefFromString("seg-004"), + StartUnitID: dnd.UnitRefFromInt(1), + EndUnitID: dnd.UnitRefFromInt(4), ShortTitle: " ", PrimaryMode: "Narrative", MainParticipants: []string{"Aria"}, @@ -461,8 +467,8 @@ func TestChunkRejectsMalformedStructuredOutput(t *testing.T) { name: "empty participant", response: replaceScenes(validSceneResponse(), []sceneResponse{ { - StartUnitID: dnd.UnitRefFromString("seg-001"), - EndUnitID: dnd.UnitRefFromString("seg-004"), + StartUnitID: dnd.UnitRefFromInt(1), + EndUnitID: dnd.UnitRefFromInt(4), ShortTitle: "Title", PrimaryMode: "Narrative", MainParticipants: []string{"Aria", " "}, @@ -511,7 +517,7 @@ func chunkRequestWithClient(client contracts.StructuredLLMClient) contracts.Chun } } -const sceneTranscriptJSON = `{"id":"session-alpha","segments":[{"id":"seg-001","text":"Aria asks whether the goblin will parley."}]}` +const sceneTranscriptJSON = `{"id":"session-alpha","segments":[{"id":1,"text":"Aria asks whether the goblin will parley."}]}` func sceneSourceInput() contracts.LLMInputMaterial { return contracts.NewLLMInputMaterial("source", "application/json", []byte(sceneTranscriptJSON), "sha256:transcript", "file:///session-alpha.json") @@ -529,10 +535,10 @@ func sceneSourceDocument() *source.SourceDocument { Format: "application/vnd.seriatim.minimal+json", Digest: "sha256:source", Units: []source.SourceUnit{ - {ID: "seg-001", Kind: "transcript_segment", Text: "Aria asks whether the goblin will parley.", Metadata: map[string]any{"speaker": "Alice"}}, - {ID: "seg-002", Kind: "transcript_segment", Text: "The goblin scout describes the gate guards."}, - {ID: "seg-003", Kind: "transcript_segment", Text: "The guards rush out with blades drawn."}, - {ID: "seg-004", Kind: "transcript_segment", Text: "The party defeats the ambushers."}, + {ID: 1, Kind: "transcript_segment", Text: "Aria asks whether the goblin will parley.", Metadata: map[string]any{"speaker": "Alice"}}, + {ID: 2, Kind: "transcript_segment", Text: "The goblin scout describes the gate guards."}, + {ID: 3, Kind: "transcript_segment", Text: "The guards rush out with blades drawn."}, + {ID: 4, Kind: "transcript_segment", Text: "The party defeats the ambushers."}, }, } } @@ -540,7 +546,7 @@ func sceneSourceDocument() *source.SourceDocument { func validSceneResponse() chunkResponse { return chunkResponse{ Scenes: []sceneResponse{ - scene("seg-001", "seg-004"), + scene(1, 4), }, BoundaryCaveats: []string{}, } @@ -551,10 +557,10 @@ func replaceScenes(response chunkResponse, scenes []sceneResponse) chunkResponse return response } -func scene(startUnitID string, endUnitID string) sceneResponse { +func scene(startUnitID int, endUnitID int) sceneResponse { return sceneResponse{ - StartUnitID: dnd.UnitRefFromString(startUnitID), - EndUnitID: dnd.UnitRefFromString(endUnitID), + StartUnitID: dnd.UnitRefFromInt(startUnitID), + EndUnitID: dnd.UnitRefFromInt(endUnitID), ShortTitle: "Scene title", PrimaryMode: "Narrative", MainParticipants: []string{"Aria"}, @@ -572,8 +578,8 @@ func chunkIDs(chunks []contracts.SourceChunk) []string { return ids } -func unitIDs(units []source.SourceUnit) []string { - ids := make([]string, 0, len(units)) +func unitIDs(units []source.SourceUnit) []int { + ids := make([]int, 0, len(units)) for _, unit := range units { ids = append(ids, unit.ID) } diff --git a/internal/modules/chunk/dnd/scenes/model.go b/internal/modules/chunk/dnd/scenes/model.go index b114fa1..1918b65 100644 --- a/internal/modules/chunk/dnd/scenes/model.go +++ b/internal/modules/chunk/dnd/scenes/model.go @@ -19,8 +19,8 @@ type sceneResponse struct { } type normalizedScene struct { - StartUnitID string - EndUnitID string + StartUnitID int + EndUnitID int ShortTitle string PrimaryMode string MainParticipants []string diff --git a/internal/modules/chunk/generic/chunker.go b/internal/modules/chunk/generic/chunker.go index df09660..e43909c 100644 --- a/internal/modules/chunk/generic/chunker.go +++ b/internal/modules/chunk/generic/chunker.go @@ -68,11 +68,19 @@ func (c *Chunker) Chunk(ctx context.Context, req contracts.ChunkRequest) (contra end = len(req.Source.Units) } units := cloneUnits(req.Source.Units[start:end]) + content, err := chunkContent(units) + if err != nil { + return contracts.ChunkResult{}, err + } chunks = append(chunks, contracts.SourceChunk{ - ID: fmt.Sprintf("chunk-%06d", len(chunks)+1), - SourceID: req.Source.ID, - Index: len(chunks), - Units: units, + ID: fmt.Sprintf("chunk-%06d", len(chunks)+1), + SourceID: req.Source.ID, + Index: len(chunks), + StartUnitID: units[0].ID, + EndUnitID: units[len(units)-1].ID, + Content: content, + MediaType: "application/json", + Units: units, Metadata: map[string]any{ "start_unit_id": units[0].ID, "end_unit_id": units[len(units)-1].ID, @@ -87,6 +95,18 @@ func (c *Chunker) Chunk(ctx context.Context, req contracts.ChunkRequest) (contra return contracts.ChunkResult{Chunks: chunks}, nil } +func chunkContent(units []source.SourceUnit) ([]byte, error) { + content, err := json.Marshal(struct { + Units []source.SourceUnit `json:"units"` + }{ + Units: units, + }) + if err != nil { + return nil, chunkerErrorf("encode chunk content: %w", err) + } + return content, nil +} + func ModuleSpec() pipeline.ModuleSpec { return pipeline.ModuleSpec{ Key: Key, diff --git a/internal/modules/chunk/generic/chunker_test.go b/internal/modules/chunk/generic/chunker_test.go index 01fa474..ac0577e 100644 --- a/internal/modules/chunk/generic/chunker_test.go +++ b/internal/modules/chunk/generic/chunker_test.go @@ -59,10 +59,16 @@ func TestChunkUsesDefaultsForSingleChunk(t *testing.T) { if chunk.Index != 0 || chunk.SourceID != "source-1" { t.Fatalf("chunk = %#v, want source and index fields", chunk) } - if got := unitIDs(chunk.Units); !reflect.DeepEqual(got, []string{"u001", "u002", "u003"}) { + if got := unitIDs(chunk.Units); !reflect.DeepEqual(got, []int{1, 2, 3}) { t.Fatalf("unit IDs = %#v, want all units", got) } - if chunk.Metadata["start_unit_id"] != "u001" || chunk.Metadata["end_unit_id"] != "u003" || chunk.Metadata["unit_count"] != 3 { + if chunk.StartUnitID != 1 || chunk.EndUnitID != 3 { + t.Fatalf("chunk boundaries = %d-%d, want 1-3", chunk.StartUnitID, chunk.EndUnitID) + } + if chunk.MediaType != "application/json" || len(chunk.Content) == 0 { + t.Fatalf("chunk payload = media type %q length %d, want JSON content", chunk.MediaType, len(chunk.Content)) + } + if chunk.Metadata["start_unit_id"] != 1 || chunk.Metadata["end_unit_id"] != 3 || chunk.Metadata["unit_count"] != 3 { t.Fatalf("metadata = %#v, want chunk bounds", chunk.Metadata) } } @@ -79,8 +85,8 @@ func TestChunkExactBoundaries(t *testing.T) { if got := chunkIDs(result.Chunks); !reflect.DeepEqual(got, []string{"chunk-000001", "chunk-000002", "chunk-000003"}) { t.Fatalf("chunk IDs = %#v, want stable IDs", got) } - gotUnits := [][]string{unitIDs(result.Chunks[0].Units), unitIDs(result.Chunks[1].Units), unitIDs(result.Chunks[2].Units)} - wantUnits := [][]string{{"u001", "u002"}, {"u003", "u004"}, {"u005", "u006"}} + gotUnits := [][]int{unitIDs(result.Chunks[0].Units), unitIDs(result.Chunks[1].Units), unitIDs(result.Chunks[2].Units)} + wantUnits := [][]int{{1, 2}, {3, 4}, {5, 6}} if !reflect.DeepEqual(gotUnits, wantUnits) { t.Fatalf("chunk units = %#v, want %#v", gotUnits, wantUnits) } @@ -95,11 +101,11 @@ func TestChunkOverlap(t *testing.T) { t.Fatalf("Chunk() error = %v, want nil", err) } - gotUnits := make([][]string, 0, len(result.Chunks)) + gotUnits := make([][]int, 0, len(result.Chunks)) for _, chunk := range result.Chunks { gotUnits = append(gotUnits, unitIDs(chunk.Units)) } - wantUnits := [][]string{{"u001", "u002", "u003"}, {"u003", "u004", "u005"}, {"u005", "u006", "u007"}} + wantUnits := [][]int{{1, 2, 3}, {3, 4, 5}, {5, 6, 7}} if !reflect.DeepEqual(gotUnits, wantUnits) { t.Fatalf("chunk units = %#v, want %#v", gotUnits, wantUnits) } @@ -162,10 +168,10 @@ func TestChunkDefensivelyCopiesUnits(t *testing.T) { t.Fatalf("len(Chunks) = %d, want 2", len(result.Chunks)) } - doc.Units[0].ID = "changed" + doc.Units[0].ID = 99 doc.Units[0].Metadata["speaker"] = "changed" - if result.Chunks[0].Units[0].ID != "u001" { + if result.Chunks[0].Units[0].ID != 1 { t.Fatalf("chunk unit ID changed after source mutation: %#v", result.Chunks[0].Units[0]) } if result.Chunks[0].Units[0].Metadata["speaker"] != "speaker-001" { @@ -176,11 +182,10 @@ func TestChunkDefensivelyCopiesUnits(t *testing.T) { func testSource(count int) *source.SourceDocument { units := make([]source.SourceUnit, 0, count) for i := 1; i <= count; i++ { - id := "u" + zeroPad3(i) units = append(units, source.SourceUnit{ - ID: id, + ID: i, Kind: "unit", - Text: "Text for " + id, + Text: "Text for " + zeroPad3(i), Metadata: map[string]any{ "speaker": "speaker-" + zeroPad3(i), }, @@ -207,8 +212,8 @@ func chunkIDs(chunks []contracts.SourceChunk) []string { return ids } -func unitIDs(units []source.SourceUnit) []string { - ids := make([]string, 0, len(units)) +func unitIDs(units []source.SourceUnit) []int { + ids := make([]int, 0, len(units)) for _, unit := range units { ids = append(ids, unit.ID) } diff --git a/internal/modules/extract/dnd/spells/assets/prompts/instructions.md b/internal/modules/extract/dnd/spells/assets/prompts/instructions.md index fbdfdec..c534b75 100644 --- a/internal/modules/extract/dnd/spells/assets/prompts/instructions.md +++ b/internal/modules/extract/dnd/spells/assets/prompts/instructions.md @@ -1,5 +1,4 @@ -Source references must use 1-based integer source-unit numbers from the -transcript, where 1 is the first provided source unit. +Source references must use integer source-unit IDs from the transcript. Return only D&D spell-cast artifacts. For each spell cast, identify the in-world caster, spell name, effect, narrative description, and source references using diff --git a/internal/modules/extract/dnd/spells/config_test.go b/internal/modules/extract/dnd/spells/config_test.go index 02dc07f..c12c31f 100644 --- a/internal/modules/extract/dnd/spells/config_test.go +++ b/internal/modules/extract/dnd/spells/config_test.go @@ -244,10 +244,14 @@ func (dndSpellsChunker) Chunk(ctx context.Context, req contracts.ChunkRequest) ( return contracts.ChunkResult{ Chunks: []contracts.SourceChunk{ { - ID: req.Source.ID + ":chunk:0", - SourceID: req.Source.ID, - Index: 0, - Units: append([]source.SourceUnit(nil), req.Source.Units...), + ID: req.Source.ID + ":chunk:0", + SourceID: req.Source.ID, + Index: 0, + StartUnitID: req.Source.Units[0].ID, + EndUnitID: req.Source.Units[len(req.Source.Units)-1].ID, + Content: []byte(`{"units":[1,2,3]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), req.Source.Units...), }, }, }, nil diff --git a/internal/modules/extract/dnd/spells/extractor_test.go b/internal/modules/extract/dnd/spells/extractor_test.go index 47d141c..86eebb8 100644 --- a/internal/modules/extract/dnd/spells/extractor_test.go +++ b/internal/modules/extract/dnd/spells/extractor_test.go @@ -73,7 +73,7 @@ func TestExtractReturnsSpellCandidateFromStructuredOutput(t *testing.T) { if payload != wantPayload { t.Fatalf("payload = %#v, want %#v", payload, wantPayload) } - wantRef := source.SourceRef{SourceID: "session-alpha", StartUnitID: "seg-001", EndUnitID: "seg-002"} + wantRef := source.SourceRef{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2} if len(candidate.SourceRefs) != 1 || candidate.SourceRefs[0] != wantRef { t.Fatalf("SourceRefs = %#v, want %#v", candidate.SourceRefs, []source.SourceRef{wantRef}) } @@ -254,14 +254,14 @@ func TestExtractPreservesResponseOrder(t *testing.T) { Spell: "Cure Wounds", Effect: "Heals.", NarrativeDescription: "First spell.", - SourceRefs: responseSourceRefs("session-alpha", "seg-001", "seg-001"), + SourceRefs: responseSourceRefs("session-alpha", 1, 1), }, { Caster: "Bandit Shaman", Spell: "Fire Bolt", Effect: "Burns.", NarrativeDescription: "Second spell.", - SourceRefs: responseSourceRefs("session-alpha", "seg-002", "seg-002"), + SourceRefs: responseSourceRefs("session-alpha", 2, 2), }, }, }, @@ -296,7 +296,7 @@ func TestExtractCopiesCandidateSourceRefs(t *testing.T) { Spell: "Cure Wounds", Effect: "Heals.", NarrativeDescription: "Aria heals.", - SourceRefs: responseSourceRefs("session-alpha", "seg-001", "seg-002"), + SourceRefs: responseSourceRefs("session-alpha", 1, 2), }, }, }, @@ -306,10 +306,10 @@ func TestExtractCopiesCandidateSourceRefs(t *testing.T) { if err != nil { t.Fatalf("Extract() error = %v, want nil", err) } - client.response.SpellCasts[0].SourceRefs[0].StartUnitID = dnd.UnitRefFromString("mutated") + client.response.SpellCasts[0].SourceRefs[0].StartUnitID = dnd.UnitRefFromInt(99) - if got := result.Candidates[0].SourceRefs[0].StartUnitID; got != "seg-001" { - t.Fatalf("candidate source ref start = %q, want copied seg-001", got) + if got := result.Candidates[0].SourceRefs[0].StartUnitID; got != 1 { + t.Fatalf("candidate source ref start = %d, want copied 1", got) } } @@ -322,7 +322,7 @@ func extractionRequestWithClient(client contracts.StructuredLLMClient) contracts return req } -const spellTranscriptJSON = `{"id":"session-alpha","segments":[{"id":"seg-001","text":"Aria raises her hand and casts Cure Wounds."}]}` +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") diff --git a/internal/modules/extract/dnd/spells/runner_test.go b/internal/modules/extract/dnd/spells/runner_test.go index 0790f75..e85fb77 100644 --- a/internal/modules/extract/dnd/spells/runner_test.go +++ b/internal/modules/extract/dnd/spells/runner_test.go @@ -26,14 +26,14 @@ func TestRunnerProcessesSeriatimInputWithDNDSpellsExtractor(t *testing.T) { Spell: "Cure Wounds", Effect: "Heals an injured ally.", NarrativeDescription: "Aria restores the fighter after the fight.", - SourceRefs: responseSourceRefs(expectedDoc.ID, "seg-001", "seg-001"), + SourceRefs: responseSourceRefs(expectedDoc.ID, 1, 1), }, { Caster: "Borin", Spell: "Fire Bolt", Effect: "Scorches the wight.", NarrativeDescription: "Borin hurls fire at the wight.", - SourceRefs: responseSourceRefs(expectedDoc.ID, "seg-003", "seg-003"), + SourceRefs: responseSourceRefs(expectedDoc.ID, 3, 3), }, }, }, @@ -122,7 +122,7 @@ func TestRunnerPassesPartyAndGlossaryReferencesToDNDSpellsPrompt(t *testing.T) { Spell: "Fire Bolt", Effect: "Scorches the wight.", NarrativeDescription: "Borin hurls fire at the wight.", - SourceRefs: responseSourceRefs(expectedDoc.ID, "seg-003", "seg-003"), + SourceRefs: responseSourceRefs(expectedDoc.ID, 3, 3), }, }, }, @@ -207,7 +207,7 @@ func TestRunnerRejectsDNDSpellCastWithInvalidSourceRef(t *testing.T) { Spell: "Cure Wounds", Effect: "Heals an injured ally.", NarrativeDescription: "Aria restores the fighter after the fight.", - SourceRefs: responseSourceRefs("spell-session", "seg-999", "seg-999"), + SourceRefs: responseSourceRefs("spell-session", 999, 999), }, }, }, diff --git a/internal/modules/extract/dnd/spells/test_helpers_test.go b/internal/modules/extract/dnd/spells/test_helpers_test.go index 2f668bc..460b410 100644 --- a/internal/modules/extract/dnd/spells/test_helpers_test.go +++ b/internal/modules/extract/dnd/spells/test_helpers_test.go @@ -12,11 +12,15 @@ import ( func promptExtractionRequest() contracts.ExtractionRequest { doc := promptSourceDocument() chunk := &contracts.SourceChunk{ - ID: "session-alpha:chunk:0", - SourceID: doc.ID, - Index: 0, - Units: append([]source.SourceUnit(nil), doc.Units...), - Metadata: map[string]any{"ignored": "chunk metadata"}, + ID: "session-alpha:chunk:0", + SourceID: doc.ID, + Index: 0, + StartUnitID: doc.Units[0].ID, + EndUnitID: doc.Units[len(doc.Units)-1].ID, + Content: []byte(`{"units":[1,2]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), doc.Units...), + Metadata: map[string]any{"ignored": "chunk metadata"}, } return contracts.ExtractionRequest{ Source: doc, @@ -32,7 +36,7 @@ func promptSourceDocument() *source.SourceDocument { Digest: "sha256:test", Units: []source.SourceUnit{ { - ID: "seg-001", + ID: 1, Kind: "transcript_segment", Text: "Aria raises her hand and casts Cure Wounds.", Metadata: map[string]any{ @@ -43,7 +47,7 @@ func promptSourceDocument() *source.SourceDocument { }, }, { - ID: "seg-002", + ID: 2, Kind: "transcript_segment", Text: "The fighter's wounds begin to close.", Metadata: map[string]any{"ignored": "not rendered"}, @@ -61,12 +65,12 @@ func mustJSON(t *testing.T, value any) string { return string(encoded) } -func responseSourceRefs(sourceID string, startUnitID string, endUnitID string) []dnd.SourceRefResponse { +func responseSourceRefs(sourceID string, startUnitID int, endUnitID int) []dnd.SourceRefResponse { return []dnd.SourceRefResponse{ { SourceID: sourceID, - StartUnitID: dnd.UnitRefFromString(startUnitID), - EndUnitID: dnd.UnitRefFromString(endUnitID), + StartUnitID: dnd.UnitRefFromInt(startUnitID), + EndUnitID: dnd.UnitRefFromInt(endUnitID), }, } } diff --git a/internal/modules/extract/dnd/spells/testdata/seriatim_spell_session.json b/internal/modules/extract/dnd/spells/testdata/seriatim_spell_session.json index 091049c..9f5cada 100644 --- a/internal/modules/extract/dnd/spells/testdata/seriatim_spell_session.json +++ b/internal/modules/extract/dnd/spells/testdata/seriatim_spell_session.json @@ -5,21 +5,21 @@ }, "segments": [ { - "id": "seg-001", + "id": 1, "start": 0, "end": 4, "speaker": "Alice", "text": "Aria raises her holy symbol and casts Cure Wounds." }, { - "id": "seg-002", + "id": 2, "start": 4, "end": 8, "speaker": "DM", "text": "The bandit mage casts Shield as the blow lands." }, { - "id": "seg-003", + "id": 3, "start": 8, "end": 12, "speaker": "Bob", diff --git a/internal/modules/extract/dnd/spells/validator_test.go b/internal/modules/extract/dnd/spells/validator_test.go index 10e3182..0c778c4 100644 --- a/internal/modules/extract/dnd/spells/validator_test.go +++ b/internal/modules/extract/dnd/spells/validator_test.go @@ -130,22 +130,22 @@ func TestSourceRefValidatorRejectsInvalidRefs(t *testing.T) { }{ { name: "unknown source id", - ref: source.SourceRef{SourceID: "session-beta", StartUnitID: "seg-001", EndUnitID: "seg-002"}, + ref: source.SourceRef{SourceID: "session-beta", StartUnitID: 1, EndUnitID: 2}, want: "does not match", }, { name: "unknown start unit", - ref: source.SourceRef{SourceID: "session-alpha", StartUnitID: "seg-999", EndUnitID: "seg-002"}, + ref: source.SourceRef{SourceID: "session-alpha", StartUnitID: 999, EndUnitID: 2}, want: "start_unit_id", }, { name: "unknown end unit", - ref: source.SourceRef{SourceID: "session-alpha", StartUnitID: "seg-001", EndUnitID: "seg-999"}, + ref: source.SourceRef{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 999}, want: "end_unit_id", }, { name: "reversed unit range", - ref: source.SourceRef{SourceID: "session-alpha", StartUnitID: "seg-002", EndUnitID: "seg-001"}, + ref: source.SourceRef{SourceID: "session-alpha", StartUnitID: 2, EndUnitID: 1}, want: "appears after", }, } @@ -234,7 +234,7 @@ func validSpellCandidate(index int) artifacts.ArtifactCandidate { Index: index, Payload: spellPayload(validSpellPayload()), SourceRefs: []source.SourceRef{ - {SourceID: "session-alpha", StartUnitID: "seg-001", EndUnitID: "seg-002"}, + {SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}, }, } } diff --git a/internal/modules/input/seriatim/adapter.go b/internal/modules/input/seriatim/adapter.go index 2652d50..1287ece 100644 --- a/internal/modules/input/seriatim/adapter.go +++ b/internal/modules/input/seriatim/adapter.go @@ -69,7 +69,7 @@ func (a *Adapter) Parse(ctx context.Context, req contracts.ParseRequest) (*sourc Metadata: copyMetadata(parsed.Metadata), } - seenSegmentIDs := make(map[string]struct{}, len(parsed.Segments)) + seenSegmentIDs := make(map[int]struct{}, len(parsed.Segments)) for i, segment := range parsed.Segments { unit, err := sourceUnit(segment, i, seenSegmentIDs) if err != nil { @@ -98,39 +98,35 @@ func Register(registry *pipeline.InputAdapterRegistry) error { }) } -func sourceUnit(segment segment, index int, seen map[string]struct{}) (source.SourceUnit, error) { +func sourceUnit(segment segment, index int, seen map[int]struct{}) (source.SourceUnit, error) { segmentLabel := fmt.Sprintf("segment[%d]", index) - segmentID := strings.TrimSpace(segment.ID) - if segmentID == "" { - return source.SourceUnit{}, inputErrorf("%s id must not be empty", segmentLabel) - } - if segmentID != segment.ID { - return source.SourceUnit{}, inputErrorf("%s id %q must not contain leading or trailing whitespace", segmentLabel, segment.ID) + if segment.ID <= 0 { + return source.SourceUnit{}, inputErrorf("%s id must be positive", segmentLabel) } if _, ok := seen[segment.ID]; ok { - return source.SourceUnit{}, inputErrorf("segment id %q is duplicated", segment.ID) + return source.SourceUnit{}, inputErrorf("segment id %d is duplicated", segment.ID) } seen[segment.ID] = struct{}{} speaker := strings.TrimSpace(segment.Speaker) if speaker == "" { - return source.SourceUnit{}, inputErrorf("segment %q speaker must not be empty", segment.ID) + return source.SourceUnit{}, inputErrorf("segment %d speaker must not be empty", segment.ID) } - start, err := validTimestamp(segment.Start, fmt.Sprintf("segment %q start", segment.ID)) + start, err := validTimestamp(segment.Start, fmt.Sprintf("segment %d start", segment.ID)) if err != nil { return source.SourceUnit{}, err } - end, err := validTimestamp(segment.End, fmt.Sprintf("segment %q end", segment.ID)) + end, err := validTimestamp(segment.End, fmt.Sprintf("segment %d end", segment.ID)) if err != nil { return source.SourceUnit{}, err } if end.Cmp(start) < 0 { - return source.SourceUnit{}, inputErrorf("segment %q end must be greater than or equal to start", segment.ID) + return source.SourceUnit{}, inputErrorf("segment %d end must be greater than or equal to start", segment.ID) } if strings.TrimSpace(segment.Text) == "" { - return source.SourceUnit{}, inputErrorf("segment %q text must not be empty", segment.ID) + return source.SourceUnit{}, inputErrorf("segment %d text must not be empty", segment.ID) } return source.SourceUnit{ diff --git a/internal/modules/input/seriatim/adapter_test.go b/internal/modules/input/seriatim/adapter_test.go index 3178f51..9b16384 100644 --- a/internal/modules/input/seriatim/adapter_test.go +++ b/internal/modules/input/seriatim/adapter_test.go @@ -41,8 +41,8 @@ func TestParseValidMinimalTranscript(t *testing.T) { } first := doc.Units[0] - if first.ID != "seg-001" { - t.Fatalf("first.ID = %q, want seg-001", first.ID) + if first.ID != 1 { + t.Fatalf("first.ID = %d, want 1", first.ID) } if first.Kind != UnitKind { t.Fatalf("first.Kind = %q, want %q", first.Kind, UnitKind) @@ -86,13 +86,13 @@ func TestParseAcceptsNumericSegmentIDs(t *testing.T) { if len(doc.Units) != 2 { t.Fatalf("len(doc.Units) = %d, want 2", len(doc.Units)) } - if doc.Units[0].ID != "1" || doc.Units[1].ID != "2" { - t.Fatalf("unit IDs = %#v, want numeric IDs normalized to strings", []string{doc.Units[0].ID, doc.Units[1].ID}) + if doc.Units[0].ID != 1 || doc.Units[1].ID != 2 { + t.Fatalf("unit IDs = %#v, want numeric IDs", []int{doc.Units[0].ID, doc.Units[1].ID}) } ref := source.SourceRef{ SourceID: doc.ID, - StartUnitID: "1", - EndUnitID: "2", + StartUnitID: 1, + EndUnitID: 2, } if err := source.ValidateRef(doc, ref); err != nil { t.Fatalf("ValidateRef() error = %v, want nil", err) @@ -115,7 +115,7 @@ func TestParseRequestSourceIDOverridesMetadataIDs(t *testing.T) { } func TestParseFallbackDocumentIDIsDeterministic(t *testing.T) { - raw := []byte(`{"metadata":{},"segments":[{"id":"s1","start":0,"end":1,"speaker":"Narrator","text":"Synthetic text."}]}`) + raw := []byte(`{"metadata":{},"segments":[{"id":1,"start":0,"end":1,"speaker":"Narrator","text":"Synthetic text."}]}`) first, err := New().Parse(context.Background(), contracts.ParseRequest{Raw: raw}) if err != nil { @@ -138,7 +138,7 @@ func TestParseFallbackDocumentIDIsDeterministic(t *testing.T) { } func TestParseUsesMetadataSourceIDWhenMetadataIDIsAbsent(t *testing.T) { - raw := []byte(`{"metadata":{"source_id":" source-from-metadata "},"segments":[{"id":"s1","start":0,"end":1,"speaker":"Narrator","text":"Synthetic text."}]}`) + raw := []byte(`{"metadata":{"source_id":" source-from-metadata "},"segments":[{"id":1,"start":0,"end":1,"speaker":"Narrator","text":"Synthetic text."}]}`) doc, err := New().Parse(context.Background(), contracts.ParseRequest{Raw: raw}) if err != nil { @@ -203,7 +203,7 @@ func TestParseRejectsInvalidInput(t *testing.T) { { name: "missing segment id", raw: validJSONWithSegment(`"start":0,"end":1,"speaker":"Narrator","text":"Synthetic text."`), - wantErr: []string{"id", "empty"}, + wantErr: []string{"id", "positive"}, }, { name: "invalid segment id type", @@ -212,52 +212,52 @@ func TestParseRejectsInvalidInput(t *testing.T) { }, { name: "whitespace segment id", - raw: validJSONWithSegment(`"id":" s1 ","start":0,"end":1,"speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":" 1 ","start":0,"end":1,"speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"id", "whitespace"}, }, { name: "empty text", - raw: validJSONWithSegment(`"id":"s1","start":0,"end":1,"speaker":"Narrator","text":" "`), + raw: validJSONWithSegment(`"id":1,"start":0,"end":1,"speaker":"Narrator","text":" "`), wantErr: []string{"text", "empty"}, }, { name: "missing speaker", - raw: validJSONWithSegment(`"id":"s1","start":0,"end":1,"text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"start":0,"end":1,"text":"Synthetic text."`), wantErr: []string{"speaker", "empty"}, }, { name: "missing start", - raw: validJSONWithSegment(`"id":"s1","end":1,"speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"end":1,"speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"start", "empty"}, }, { name: "missing end", - raw: validJSONWithSegment(`"id":"s1","start":0,"speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"start":0,"speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"end", "empty"}, }, { name: "negative start", - raw: validJSONWithSegment(`"id":"s1","start":-1,"end":1,"speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"start":-1,"end":1,"speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"start", "negative"}, }, { name: "non-numeric end", - raw: validJSONWithSegment(`"id":"s1","start":0,"end":"late","speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"start":0,"end":"late","speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"segment[0]", "end", "number"}, }, { name: "non-finite timestamp", - raw: validJSONWithSegment(`"id":"s1","start":1e10000,"end":1e10000,"speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"start":1e10000,"end":1e10000,"speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"start", "valid number"}, }, { name: "end before start", - raw: validJSONWithSegment(`"id":"s1","start":2,"end":1,"speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"start":2,"end":1,"speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"end", "start"}, }, { name: "end before start beyond float precision", - raw: validJSONWithSegment(`"id":"s1","start":9007199254740993,"end":9007199254740992,"speaker":"Narrator","text":"Synthetic text."`), + raw: validJSONWithSegment(`"id":1,"start":9007199254740993,"end":9007199254740992,"speaker":"Narrator","text":"Synthetic text."`), wantErr: []string{"end", "start"}, }, } @@ -284,7 +284,7 @@ func TestParseRejectsDuplicateSegmentIDs(t *testing.T) { if err == nil { t.Fatal("Parse() error = nil, want duplicate ID error") } - if !strings.Contains(err.Error(), "duplicated") || !strings.Contains(err.Error(), "seg-001") { + if !strings.Contains(err.Error(), "duplicated") || !strings.Contains(err.Error(), "1") { t.Fatalf("Parse() error = %q, want duplicate segment context", err.Error()) } } diff --git a/internal/modules/input/seriatim/model.go b/internal/modules/input/seriatim/model.go index 56bfb84..8566f99 100644 --- a/internal/modules/input/seriatim/model.go +++ b/internal/modules/input/seriatim/model.go @@ -5,6 +5,8 @@ import ( "encoding/json" "fmt" "io" + "strconv" + "strings" ) type transcript struct { @@ -13,7 +15,7 @@ type transcript struct { } type segment struct { - ID string `json:"id"` + ID int `json:"id"` Start json.Number `json:"start"` End json.Number `json:"end"` Speaker string `json:"speaker"` @@ -78,7 +80,7 @@ func decodeSegment(raw []byte, index int) (segment, error) { var decoded segment if err := decodeOptionalSegmentID(fields, "id", &decoded.ID); err != nil { - return segment{}, fmt.Errorf("segment[%d] id must be a string or number: %w", index, err) + return segment{}, fmt.Errorf("segment[%d] id must be a positive integer string or number: %w", index, err) } if err := decodeOptionalNumber(fields, "start", &decoded.Start); err != nil { return segment{}, fmt.Errorf("segment[%d] start must be a number: %w", index, err) @@ -103,7 +105,7 @@ func decodeOptionalString(fields map[string]json.RawMessage, key string, out *st return decodeJSON(raw, out) } -func decodeOptionalSegmentID(fields map[string]json.RawMessage, key string, out *string) error { +func decodeOptionalSegmentID(fields map[string]json.RawMessage, key string, out *int) error { raw, ok := fields[key] if !ok { return nil @@ -111,17 +113,46 @@ func decodeOptionalSegmentID(fields map[string]json.RawMessage, key string, out var text string if err := decodeJSON(raw, &text); err == nil { - *out = text + parsed, err := parsePositiveInt(text) + if err != nil { + return err + } + *out = parsed return nil } var number json.Number if err := decodeJSON(raw, &number); err == nil { - *out = number.String() + parsed, err := parsePositiveInt(number.String()) + if err != nil { + return err + } + *out = parsed return nil } - return fmt.Errorf("must be a string or number") + return fmt.Errorf("must be a positive integer string or number") +} + +func parsePositiveInt(value string) (int, error) { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return 0, fmt.Errorf("must not be empty") + } + if trimmed != value { + return 0, fmt.Errorf("must not contain leading or trailing whitespace") + } + parsed, err := strconv.Atoi(value) + if err != nil { + return 0, fmt.Errorf("must be an integer") + } + if parsed <= 0 { + return 0, fmt.Errorf("must be positive") + } + if strconv.Itoa(parsed) != value { + return 0, fmt.Errorf("must be a canonical positive integer") + } + return parsed, nil } func decodeOptionalNumber(fields map[string]json.RawMessage, key string, out *json.Number) error { diff --git a/internal/modules/input/seriatim/runner_test.go b/internal/modules/input/seriatim/runner_test.go index 0dfc21a..415a1ed 100644 --- a/internal/modules/input/seriatim/runner_test.go +++ b/internal/modules/input/seriatim/runner_test.go @@ -57,7 +57,7 @@ func TestRunnerProcessesSeriatimInputWithFakeModules(t *testing.T) { if err := source.ValidateRef(expectedDoc, artifact.SourceRefs[0]); err != nil { t.Fatalf("ValidateRef() error = %v, want nil", err) } - if artifact.SourceRefs[0].StartUnitID != "seg-001" || artifact.SourceRefs[0].EndUnitID != "seg-002" { + if artifact.SourceRefs[0].StartUnitID != 1 || artifact.SourceRefs[0].EndUnitID != 2 { t.Fatalf("SourceRefs[0] = %#v, want Seriatim unit IDs", artifact.SourceRefs[0]) } if extractor.calls != 1 { @@ -165,10 +165,14 @@ func (runnerSeriatimChunker) Chunk(ctx context.Context, req contracts.ChunkReque return contracts.ChunkResult{ Chunks: []contracts.SourceChunk{ { - ID: req.Source.ID + ":chunk:0", - SourceID: req.Source.ID, - Index: 0, - Units: append([]source.SourceUnit(nil), req.Source.Units...), + ID: req.Source.ID + ":chunk:0", + SourceID: req.Source.ID, + Index: 0, + StartUnitID: req.Source.Units[0].ID, + EndUnitID: req.Source.Units[len(req.Source.Units)-1].ID, + Content: []byte(`{"units":[1,2]}`), + MediaType: "application/json", + Units: append([]source.SourceUnit(nil), req.Source.Units...), }, }, }, nil @@ -206,21 +210,21 @@ func (e *runnerSeriatimExtractor) Extract(ctx context.Context, req contracts.Ext if req.Chunk == nil { return contracts.ExtractionResult{}, fmt.Errorf("chunk must not be nil") } - if got := unitIDs(req.Source.Units); !equalStrings(got, []string{"seg-001", "seg-002"}) { + if got := unitIDs(req.Source.Units); !equalInts(got, []int{1, 2}) { return contracts.ExtractionResult{}, fmt.Errorf("source unit IDs = %#v, want Seriatim segment IDs", got) } - if got := unitIDs(req.Chunk.Units); !equalStrings(got, []string{"seg-001", "seg-002"}) { + if got := unitIDs(req.Chunk.Units); !equalInts(got, []int{1, 2}) { return contracts.ExtractionResult{}, fmt.Errorf("chunk unit IDs = %#v, want Seriatim segment IDs", got) } for _, unit := range req.Chunk.Units { if speaker, ok := Speaker(unit); !ok || speaker == "" { - return contracts.ExtractionResult{}, fmt.Errorf("unit %q missing speaker metadata", unit.ID) + return contracts.ExtractionResult{}, fmt.Errorf("unit %d missing speaker metadata", unit.ID) } if _, ok := Start(unit); !ok { - return contracts.ExtractionResult{}, fmt.Errorf("unit %q missing start metadata", unit.ID) + return contracts.ExtractionResult{}, fmt.Errorf("unit %d missing start metadata", unit.ID) } if _, ok := End(unit); !ok { - return contracts.ExtractionResult{}, fmt.Errorf("unit %q missing end metadata", unit.ID) + return contracts.ExtractionResult{}, fmt.Errorf("unit %d missing end metadata", unit.ID) } } @@ -254,15 +258,15 @@ func (runnerSeriatimOutput) Encode(ctx context.Context, req contracts.OutputRequ }, nil } -func unitIDs(units []source.SourceUnit) []string { - ids := make([]string, 0, len(units)) +func unitIDs(units []source.SourceUnit) []int { + ids := make([]int, 0, len(units)) for _, unit := range units { ids = append(ids, unit.ID) } return ids } -func equalStrings(a, b []string) bool { +func equalInts(a, b []int) bool { if len(a) != len(b) { return false } diff --git a/internal/modules/input/seriatim/testdata/duplicate_segment_id.json b/internal/modules/input/seriatim/testdata/duplicate_segment_id.json index e504e0e..827fe0f 100644 --- a/internal/modules/input/seriatim/testdata/duplicate_segment_id.json +++ b/internal/modules/input/seriatim/testdata/duplicate_segment_id.json @@ -4,14 +4,14 @@ }, "segments": [ { - "id": "seg-001", + "id": 1, "start": 0, "end": 1, "speaker": "Narrator", "text": "First segment." }, { - "id": "seg-001", + "id": 1, "start": 1, "end": 2, "speaker": "Player", diff --git a/internal/modules/input/seriatim/testdata/valid_minimal.json b/internal/modules/input/seriatim/testdata/valid_minimal.json index 339819e..dbe3af3 100644 --- a/internal/modules/input/seriatim/testdata/valid_minimal.json +++ b/internal/modules/input/seriatim/testdata/valid_minimal.json @@ -6,14 +6,14 @@ }, "segments": [ { - "id": "seg-001", + "id": 1, "start": 0, "end": 4.5, "speaker": "Narrator", "text": "The stone door opens." }, { - "id": "seg-002", + "id": 2, "start": 4.5, "end": 8, "speaker": "Player", diff --git a/internal/modules/merge/appendorder/merger_test.go b/internal/modules/merge/appendorder/merger_test.go index 0b04a80..168cf2e 100644 --- a/internal/modules/merge/appendorder/merger_test.go +++ b/internal/modules/merge/appendorder/merger_test.go @@ -80,7 +80,7 @@ func TestMergeDefensivelyCopiesCandidates(t *testing.T) { input[0].Candidates[0].Index = 99 input[0].Candidates[0].Payload[0] = '[' - input[0].Candidates[0].SourceRefs[0].StartUnitID = "changed" + input[0].Candidates[0].SourceRefs[0].StartUnitID = 99 input[0].Candidates[0].Metadata["name"] = "changed" got := result.Candidates[0] @@ -90,7 +90,7 @@ func TestMergeDefensivelyCopiesCandidates(t *testing.T) { if string(got.Payload) != `{"name":"original"}` { t.Fatalf("Payload = %s, want original payload", got.Payload) } - if got.SourceRefs[0].StartUnitID != "u1" { + if got.SourceRefs[0].StartUnitID != 1 { t.Fatalf("SourceRefs = %#v, want original source ref", got.SourceRefs) } if got.Metadata["name"] != "original" { @@ -119,7 +119,7 @@ func candidate(index int, name string) artifacts.ArtifactCandidate { SchemaVersion: "v1", Payload: json.RawMessage(`{"name":"` + name + `"}`), SourceRefs: []source.SourceRef{ - {SourceID: "source-1", StartUnitID: "u1", EndUnitID: "u1"}, + {SourceID: "source-1", StartUnitID: 1, EndUnitID: 1}, }, Metadata: map[string]any{ "name": name, @@ -141,7 +141,7 @@ func sourceChunk(index int) contracts.SourceChunk { SourceID: "source-1", Index: index, Units: []source.SourceUnit{ - {ID: "u1", Kind: "unit", Text: "Source unit."}, + {ID: 1, Kind: "unit", Text: "Source unit."}, }, } } diff --git a/internal/modules/normalize/noop/normalizer_test.go b/internal/modules/normalize/noop/normalizer_test.go index 7c283df..1fb00de 100644 --- a/internal/modules/normalize/noop/normalizer_test.go +++ b/internal/modules/normalize/noop/normalizer_test.go @@ -84,7 +84,7 @@ func TestNormalizeDefensivelyCopiesCandidates(t *testing.T) { input[0].Index = 99 input[0].Payload[0] = '[' - input[0].SourceRefs[0].EndUnitID = "changed" + input[0].SourceRefs[0].EndUnitID = 99 input[0].Metadata["name"] = "changed" got := result.Candidates[0] @@ -94,7 +94,7 @@ func TestNormalizeDefensivelyCopiesCandidates(t *testing.T) { if string(got.Payload) != `{"name":"original"}` { t.Fatalf("Payload = %s, want original payload", got.Payload) } - if got.SourceRefs[0].EndUnitID != "u1" { + if got.SourceRefs[0].EndUnitID != 1 { t.Fatalf("SourceRefs = %#v, want original source ref", got.SourceRefs) } if got.Metadata["name"] != "original" { @@ -123,7 +123,7 @@ func candidate(index int, name string) artifacts.ArtifactCandidate { SchemaVersion: "v1", Payload: json.RawMessage(`{"name":"` + name + `"}`), SourceRefs: []source.SourceRef{ - {SourceID: "source-1", StartUnitID: "u1", EndUnitID: "u1"}, + {SourceID: "source-1", StartUnitID: 1, EndUnitID: 1}, }, Metadata: map[string]any{ "name": name, diff --git a/internal/modules/output/json/encoder_test.go b/internal/modules/output/json/encoder_test.go index 9d00cfd..b0b17a5 100644 --- a/internal/modules/output/json/encoder_test.go +++ b/internal/modules/output/json/encoder_test.go @@ -247,7 +247,7 @@ func TestEncodeDoesNotMutateInputs(t *testing.T) { } req.Approved[0].Payload[0] = '[' - req.Approved[0].SourceRefs[0].StartUnitID = "changed" + req.Approved[0].SourceRefs[0].StartUnitID = 99 req.Approved[0].Metadata["name"] = "changed" req.Rejected[0].Candidate.Payload[0] = '[' req.Warnings[0].Message = "changed" @@ -286,7 +286,7 @@ func artifact(artifactType, name string) artifacts.Artifact { SchemaVersion: "v1", Payload: stdjson.RawMessage(`{"name":"` + name + `"}`), SourceRefs: []source.SourceRef{ - {SourceID: "source-1", StartUnitID: "u1", EndUnitID: "u1"}, + {SourceID: "source-1", StartUnitID: 1, EndUnitID: 1}, }, Metadata: map[string]any{"name": name}, } @@ -300,7 +300,7 @@ func candidate(artifactType, name string) artifacts.ArtifactCandidate { SchemaVersion: "v1", Payload: stdjson.RawMessage(`{"name":"` + name + `"}`), SourceRefs: []source.SourceRef{ - {SourceID: "source-1", StartUnitID: "u1", EndUnitID: "u1"}, + {SourceID: "source-1", StartUnitID: 1, EndUnitID: 1}, }, Metadata: map[string]any{"name": name}, } diff --git a/internal/modules/sharedassets/dnd/unit_refs.go b/internal/modules/sharedassets/dnd/unit_refs.go index ba31328..66f7701 100644 --- a/internal/modules/sharedassets/dnd/unit_refs.go +++ b/internal/modules/sharedassets/dnd/unit_refs.go @@ -11,9 +11,8 @@ import ( ) type UnitRef struct { - value string + value int fromNumber bool - number int } type SourceRefResponse struct { @@ -23,19 +22,22 @@ type SourceRefResponse struct { } func UnitRefFromString(value string) UnitRef { - return UnitRef{value: value} + parsed, _ := parseUnitRefNumber(value) + return UnitRef{value: parsed} } func UnitRefFromInt(value int) UnitRef { return UnitRef{ - value: strconv.Itoa(value), + value: value, fromNumber: true, - number: value, } } func (ref UnitRef) String() string { - return ref.value + if ref.value == 0 { + return "" + } + return strconv.Itoa(ref.value) } func (ref *UnitRef) UnmarshalJSON(raw []byte) error { @@ -48,13 +50,17 @@ func (ref *UnitRef) UnmarshalJSON(raw []byte) error { if err := json.Unmarshal(raw, &value); err != nil { return err } - *ref = UnitRefFromString(value) + number, err := parseUnitRefNumber(value) + if err != nil { + return err + } + *ref = UnitRef{value: number} return nil } - number, err := strconv.Atoi(string(raw)) + number, err := parseUnitRefNumber(string(raw)) if err != nil { - return fmt.Errorf("unit ref must be a string or integer") + return err } *ref = UnitRefFromInt(number) return nil @@ -62,72 +68,47 @@ func (ref *UnitRef) UnmarshalJSON(raw []byte) error { func (ref UnitRef) MarshalJSON() ([]byte, error) { if ref.fromNumber { - return []byte(strconv.Itoa(ref.number)), nil + return []byte(strconv.Itoa(ref.value)), nil } - return json.Marshal(ref.value) + return json.Marshal(ref.String()) } -func ResolveUnitID(doc *source.SourceDocument, field string, ref UnitRef) (string, error) { - value := strings.TrimSpace(ref.value) - if value == "" { - return "", fmt.Errorf("%s must not be empty", field) +func ResolveUnitID(doc *source.SourceDocument, field string, ref UnitRef) (int, error) { + if ref.value <= 0 { + return 0, fmt.Errorf("%s must be positive", field) } - if id, ok := canonicalUnitID(doc, value); ok { - return id, nil + if _, ok := source.UnitIndex(doc, ref.value); !ok { + return 0, fmt.Errorf("%s %d was not found", field, ref.value) } - if number, ok := unitNumber(value); ok { - if id, ok := unitIDByNumber(doc, number); ok { - return id, nil - } - return "", fmt.Errorf("%s %d was not found as a source-unit ID or 1-based unit number", field, number) - } - return "", fmt.Errorf("%s %q was not found", field, value) + return ref.value, nil } func SourceRefCandidate(doc *source.SourceDocument, ref SourceRefResponse) source.SourceRef { return source.SourceRef{ SourceID: strings.TrimSpace(ref.SourceID), - StartUnitID: unitIDCandidate(doc, ref.StartUnitID), - EndUnitID: unitIDCandidate(doc, ref.EndUnitID), + StartUnitID: unitIDCandidate(ref.StartUnitID), + EndUnitID: unitIDCandidate(ref.EndUnitID), } } -func unitIDCandidate(doc *source.SourceDocument, ref UnitRef) string { - value := strings.TrimSpace(ref.value) - if id, ok := canonicalUnitID(doc, value); ok { - return id - } - if number, ok := unitNumber(value); ok { - if id, ok := unitIDByNumber(doc, number); ok { - return id - } - } - return value +func unitIDCandidate(ref UnitRef) int { + return ref.value } -func canonicalUnitID(doc *source.SourceDocument, value string) (string, bool) { - if doc == nil { - return "", false +func parseUnitRefNumber(value string) (int, error) { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return 0, fmt.Errorf("unit ref must not be empty") } - for _, unit := range doc.Units { - if unit.ID == value { - return unit.ID, true - } + if trimmed != value { + return 0, fmt.Errorf("unit ref must not contain leading or trailing whitespace") } - return "", false -} - -func unitIDByNumber(doc *source.SourceDocument, number int) (string, bool) { - if doc == nil || number < 1 || number > len(doc.Units) { - return "", false - } - return doc.Units[number-1].ID, true -} - -func unitNumber(value string) (int, bool) { number, err := strconv.Atoi(value) if err != nil { - return 0, false + return 0, fmt.Errorf("unit ref must be an integer") } - return number, true + if number <= 0 { + return 0, fmt.Errorf("unit ref must be positive") + } + return number, nil } diff --git a/internal/modules/sharedassets/dnd/unit_refs_test.go b/internal/modules/sharedassets/dnd/unit_refs_test.go index 0c34c14..0023402 100644 --- a/internal/modules/sharedassets/dnd/unit_refs_test.go +++ b/internal/modules/sharedassets/dnd/unit_refs_test.go @@ -8,7 +8,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" ) -func TestUnitRefUnmarshalAcceptsIntegerAndString(t *testing.T) { +func TestUnitRefUnmarshalAcceptsIntegerAndNumericString(t *testing.T) { var integerRef UnitRef if err := json.Unmarshal([]byte(`12`), &integerRef); err != nil { t.Fatalf("Unmarshal(integer) error = %v, want nil", err) @@ -18,55 +18,49 @@ func TestUnitRefUnmarshalAcceptsIntegerAndString(t *testing.T) { } var stringRef UnitRef - if err := json.Unmarshal([]byte(`"seg-001"`), &stringRef); err != nil { + if err := json.Unmarshal([]byte(`"12"`), &stringRef); err != nil { t.Fatalf("Unmarshal(string) error = %v, want nil", err) } - if got := stringRef.String(); got != "seg-001" { - t.Fatalf("string ref = %q, want seg-001", got) + if got := stringRef.String(); got != "12" { + t.Fatalf("string ref = %q, want 12", got) } } -func TestUnitRefUnmarshalRejectsNonIntegerTypes(t *testing.T) { - for _, raw := range []string{`true`, `null`, `1.5`, `{}`} { +func TestUnitRefUnmarshalRejectsNonIntegerValues(t *testing.T) { + for _, raw := range []string{`true`, `null`, `1.5`, `{}`, `"seg-001"`, `" 1 "`, `0`, `-1`} { t.Run(raw, func(t *testing.T) { var ref UnitRef err := json.Unmarshal([]byte(raw), &ref) if err == nil { t.Fatal("Unmarshal() error = nil, want error") } - if !strings.Contains(err.Error(), "string or integer") { - t.Fatalf("Unmarshal() error = %q, want type context", err.Error()) - } }) } } -func TestResolveUnitIDPrefersExactSourceUnitID(t *testing.T) { - doc := unitRefSourceDocument("2", "10") +func TestResolveUnitIDReturnsExistingIntegerSourceUnitID(t *testing.T) { + doc := unitRefSourceDocument(2, 10) got, err := ResolveUnitID(doc, "start_unit_id", UnitRefFromInt(2)) if err != nil { t.Fatalf("ResolveUnitID() error = %v, want nil", err) } - if got != "2" { - t.Fatalf("ResolveUnitID() = %q, want exact source unit ID", got) + if got != 2 { + t.Fatalf("ResolveUnitID() = %d, want exact source unit ID", got) } } -func TestResolveUnitIDFallsBackToOneBasedUnitNumber(t *testing.T) { - doc := unitRefSourceDocument("seg-001", "seg-002") +func TestResolveUnitIDDoesNotFallbackToOneBasedUnitNumber(t *testing.T) { + doc := unitRefSourceDocument(10, 20) - got, err := ResolveUnitID(doc, "end_unit_id", UnitRefFromInt(2)) - if err != nil { - t.Fatalf("ResolveUnitID() error = %v, want nil", err) - } - if got != "seg-002" { - t.Fatalf("ResolveUnitID() = %q, want second source unit ID", got) + _, err := ResolveUnitID(doc, "end_unit_id", UnitRefFromInt(2)) + if err == nil { + t.Fatal("ResolveUnitID() error = nil, want missing source-unit ID") } } func TestResolveUnitIDRejectsMissingUnit(t *testing.T) { - doc := unitRefSourceDocument("seg-001") + doc := unitRefSourceDocument(1) _, err := ResolveUnitID(doc, "start_unit_id", UnitRefFromInt(9)) if err == nil { @@ -78,14 +72,14 @@ func TestResolveUnitIDRejectsMissingUnit(t *testing.T) { } func TestSourceRefCandidateCanonicalizesValidRefsAndPreservesInvalidRefs(t *testing.T) { - doc := unitRefSourceDocument("seg-001", "seg-002") + doc := unitRefSourceDocument(1, 2) valid := SourceRefCandidate(doc, SourceRefResponse{ SourceID: " session-alpha ", StartUnitID: UnitRefFromInt(1), EndUnitID: UnitRefFromInt(2), }) - if valid != (source.SourceRef{SourceID: "session-alpha", StartUnitID: "seg-001", EndUnitID: "seg-002"}) { + if valid != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}) { t.Fatalf("valid candidate = %#v, want canonical source ref", valid) } @@ -94,12 +88,12 @@ func TestSourceRefCandidateCanonicalizesValidRefsAndPreservesInvalidRefs(t *test StartUnitID: UnitRefFromInt(9), EndUnitID: UnitRefFromString("missing"), }) - if invalid != (source.SourceRef{SourceID: "session-alpha", StartUnitID: "9", EndUnitID: "missing"}) { + if invalid != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 9, EndUnitID: 0}) { t.Fatalf("invalid candidate = %#v, want unresolved values for validator", invalid) } } -func unitRefSourceDocument(ids ...string) *source.SourceDocument { +func unitRefSourceDocument(ids ...int) *source.SourceDocument { doc := &source.SourceDocument{ ID: "session-alpha", Kind: "transcript",