diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index cfd0143..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,257 +0,0 @@ -# Implementation Plan: Minimal D&D Scene Chunking - -Status: Ready for implementation - -This plan implements the target state in -[Minimal D&D Scene Chunking](minimal-dnd-scene-chunking.md). Complete the -stages in order. The feature roadmap owns product intent and boundary policy; -this document owns implementation sequence and acceptance criteria. - -## Decisions And Constraints - -- Keep the production module key `dnd/scenes`, prompt ID `dnd.scenes`, private - response-schema key and ID, schema filename, schema name, and schema version - `v1`. This application is pre-release, so update the private v1 contract in - place rather than adding a second prompt or response decoder. -- The model response contains only a required, non-empty `scenes` array whose - objects contain exactly `start_unit_id` and `end_unit_id`. -- A newly generated D&D scene plan has no plan-level or range-level - annotations and no model-derived warnings. Do not retain deprecated fields - internally, hide them in metadata, or translate them into another free-form - output. -- Preserve the shared D&D prompt assets, prompt message layout, reference - slots, configured profile behavior, module registration, structured LLM - boundary, manifest prompt/schema metadata, and deterministic coverage rules. -- Do not change the generic `source.ChunkPlan`, `source.ChunkRange`, materialized - chunk, or `source/chunk-map` contracts. Empty annotation maps are already - valid generic behavior. -- Invalidate all pre-change canonical chunk-plan records once by changing the - cache record schema from `notarius.chunk-plan.v1` to - `notarius.chunk-plan.v2`. This is intentionally a global, recoverable - pre-release cache transition. Do not compare prompt hashes, response-schema - hashes, chunker keys, profiles, references, or annotations during lookup; - after the version transition, ADR-0005's source-digest-only reuse policy - remains unchanged. -- Follow the testing policy: protect the private schema and observable plan - invariants, remove obsolete annotation tests, and avoid exact prompt-text, - prompt-length, prompt-hash, or message-count change detectors. Default tests - remain offline and must not call a live or paid model. -- Do not implement scene-aware combat skipping, ordered dependencies, new - validators, repair calls, semantic post-processing, or changes to the - `dnd/scene-descriptions` lane. - -## Stage 1: Replace The Scene Chunker With Its Minimal Contract - -### Goal - -Make every newly generated `dnd/scenes` plan depend only on the model-proposed -scene endpoints and contain no D&D-specific annotations or warnings. - -### Production changes - -1. Simplify - `internal/modules/dnd/chunk/scenes/assets/schemas/dnd_scenes.v1.json` in - place: - - keep the existing JSON Schema draft and `$id`; - - retain one top-level object with `additionalProperties: false`; - - make `scenes` the sole required and permitted top-level field; - - retain `type: array` and `minItems: 1`; and - - define each item as a closed object requiring only positive integer - `start_unit_id` and `end_unit_id`. -2. Rewrite the scene-specific content in - `internal/modules/dnd/chunk/scenes/assets/prompts/task.md` and - `instructions.md`: - - retain the feature roadmap's definition of a scene, positive and negative - boundary guidance, preference for fewer coherent scenes, and full ordered - coverage requirement; - - ask only for inclusive source-unit endpoints; - - continue to forbid gaps, overlaps, reordering, final chunk IDs, and chunk - indexes; and - - remove every request or definition for titles, modes, participants, - summaries, boundary notes, confidence, and caveats. - Leave `dnd.scenes.yaml` and the shared prompt assets unchanged. Their - existing message structure, cache controls, inputs, prompt identity, profile, - and schema path remain authoritative. -3. Reduce the private DTOs in - `internal/modules/dnd/chunk/scenes/model.go`: - - `chunkResponse` contains only `Scenes []sceneResponse`; - - `sceneResponse` contains only the two existing `shared.UnitRef` endpoint - fields; and - - delete `normalizedScene` rather than preserving a second endpoints-only - representation. -4. Simplify `internal/modules/dnd/chunk/scenes/chunker.go`: - - remove `annotationNamespace`, annotation JSON encoding, caveat-to-warning - conversion, semantic field trimming, participant copying, enum helpers, - and imports used only by those behaviors; - - keep request validation, source-document validation, prompt inputs, - response decoding, module registration, reference declarations, and - manifest metadata unchanged; - - have `Plan` pass the minimal response to `planFromResponse` and return the - resulting plan with no warnings; - - have `planFromResponse` resolve both `shared.UnitRef` endpoints through - `shared.ResolveUnitID`, validate them against a precomputed map of source - unit ID to document position, enforce first-to-last contiguous coverage, - and append ranges containing only `StartUnitID` and `EndUnitID`; and - - return a plan containing only the source digest and ranges. Treat nil and - zero-length annotation maps as equivalent absence; do not allocate empty - maps solely for presentation. -5. Preserve validation by document position. Do not compare endpoint IDs - numerically or assume that adjacent document units have consecutive IDs. - -### Focused tests - -Update the existing tests instead of layering parallel coverage onto obsolete -cases: - -- In `schema_test.go`, make the valid fixture use only the minimal fields. - Retain schema identity/loading and defensive-copy coverage. Verify a valid - minimal response, a missing or empty `scenes` array, non-positive or - non-integer endpoints, unknown top-level fields, and unknown scene fields. - A removed legacy field is sufficient to exercise scene-level unknown-field - rejection; do not enumerate every removed field. -- In `chunker_test.go`, replace - `TestPlanReturnsSceneRangesAndAnnotationsFromStructuredOutput` with a - behavior-level test that verifies the structured request, exact resolved - ranges, source digest, absent plan/range annotations, and absent warnings. - Remove the whitespace-caveat and annotation-defensive-copy tests and remove - obsolete semantic-field cases from the malformed-output table. -- Retain request, reference-input, legacy-roster mapping, LLM error, module - registration, manifest metadata, missing/empty scene, unknown endpoint, - reversed endpoint, gap, overlap, and incomplete-coverage behavior. -- Add one planner case whose source units have positive IDs in a nonnumeric - document order, such as `10, 3, 20`. Prove that valid ranges follow document - order and that reversal is judged by positions rather than numeric values. -- Simplify test builders such as `scene`, `validSceneResponse`, and schema - fixtures so they construct only endpoints. -- Keep the prompt preparation and diagnostics tests in - `scriptorium_assets_test.go`. Preparation through the real embedded assets is - sufficient; do not assert the exact wording or hash of the rewritten - scene-specific prompt. - -### Stage validation - -Run: - -```sh -gofmt -w internal/modules/dnd/chunk/scenes/*.go -go test -count=1 ./internal/modules/dnd/chunk/scenes -git diff --check -``` - -Stage 1 is complete when the focused package accepts the minimal structured -response, rejects malformed ranges, returns annotation-free and warning-free -plans, and contains no production references to the removed response fields. - -## Stage 2: Retire Old Cached Plans And Align Maintained Contracts - -### Goal - -Ensure a pre-change cached plan cannot reintroduce removed D&D annotations, -while preserving generic annotations and source-addressed plan reuse for all -new records. - -### Cache compatibility changes - -1. Change `pipeline.ChunkPlanSchemaVersion` in - `internal/framework/pipeline/chunk_plan_store.go` from - `notarius.chunk-plan.v1` to `notarius.chunk-plan.v2`. -2. Do not add a v1 decoder, migration, deletion routine, or D&D-specific cache - branch. `internal/framework/chunkplan` must continue treating a schema - mismatch as a recoverable invalid record. In `auto` mode the existing runner - path then regenerates and atomically replaces it; `refresh` and `bypass` - retain their existing meanings. -3. Update `internal/framework/chunkplan/store_test.go` so its valid records use - v2 and its invalid-record table explicitly proves that a well-formed v1 - record is reported as invalid/recoverable. It is appropriate to assert both - version literals here because this test owns a deliberate wire - compatibility transition. -4. Preserve existing runner tests proving that valid current-version plans are - reused across chunker or configuration differences and that structurally - invalid hits regenerate. Do not add a second runner test that merely repeats - the store's schema-version rejection and the runner's existing invalid-hit - behavior. - -### Generic fixture cleanup - -The generic chunk-map codec must continue exercising arbitrary annotations, -but its fixtures should not imply that the minimal `dnd/scenes` producer still -emits them: - -1. In `internal/framework/chunkmap/codec_test.go` and - `internal/framework/chunkmap/testdata/source_chunk_map.v1.json`, replace the - illustrative D&D identities and semantic scene values with neutral test - data. Use `chunk/requested` as the requested chunker, `chunk/producer` as the - producer, and `test/chunker` as the annotation namespace; retain small JSON - objects as annotation values so canonicalization remains exercised. -2. Preserve all existing annotation canonicalization, strict decoding, - defensive ownership, digest, and round-trip assertions. Do not remove - generic annotation coverage or change the durable chunk-map schema. -3. Retain the distinct requested and producing chunker identities to exercise - ADR-0005 reuse without associating generic annotation behavior with a - production module. - -### Documentation and roadmap alignment - -1. Update only the canonical current-behavior documentation that becomes - inaccurate: - - revise the `internal/modules/dnd/chunk/scenes` section of - `docs/internal/modules.md` to describe the boundary-only response, - deterministic coverage validation, annotation-free plan, and absence of - boundary warnings; - - in `docs/roadmap/future.md`, remove the completed chunker-minimization - bullet and rename `Minimize And Use D&D Scene Chunking` to `Use D&D Scene - Chunking`, retaining its still-future combat gating, ordered dependency, - and reassessment work; and - - set the feature roadmap status to `Implemented` only after all production, - test, and current-documentation changes pass. -2. Do not change `docs/integrations/chunk-map.md`: it already owns the unchanged - generic artifact contract, permits empty maps, and correctly treats - annotations as optional module-specific JSON. -3. Do not change configuration, CLI, operations, LLM runtime, or - scene-description integration documentation unless implementation reveals a - statement that is factually false after this work. Those documents do not - own the removed private response fields. -4. Do not add live-model evaluation to the offline suite. At handoff, recommend - evaluating the simpler prompt against the human-reviewed cases described in - the feature roadmap; credentials and human quality judgment are not - implementation completion gates. - -### Stage validation - -Run focused checks first: - -```sh -go test -count=1 ./internal/modules/dnd/chunk/scenes -go test -count=1 ./internal/framework/chunkplan -go test -count=1 ./internal/framework/chunkmap -go test -count=1 ./internal/framework/pipeline -``` - -Then run the repository-wide checks required for shared contracts: - -```sh -gofmt -w internal/modules/dnd/chunk/scenes/*.go \ - internal/framework/chunkplan/*.go \ - internal/framework/chunkmap/*.go \ - internal/framework/pipeline/chunk_plan_store.go -go test -count=1 ./... -go vet ./... -go build ./cmd/notarius -git diff --check -``` - -Review the final diff and confirm: - -- only endpoint fields remain in the private response, prompt instructions, - DTOs, and focused fixtures; -- newly generated D&D scene plans have no annotations or warnings; -- v1 cache records are recoverably invalid and new records use v2; -- valid v2 plans still follow source-only canonical reuse; -- generic chunk-map annotations remain supported; -- no current-behavior document claims that `dnd/scenes` produces semantic - annotations; and -- no scene-aware combat or ordered-dependency work entered the change. - -## Open Questions - -None. The feature and migration choices are decision-complete. diff --git a/docs/roadmap/minimal-dnd-scene-chunking.md b/docs/roadmap/minimal-dnd-scene-chunking.md deleted file mode 100644 index 32a4952..0000000 --- a/docs/roadmap/minimal-dnd-scene-chunking.md +++ /dev/null @@ -1,172 +0,0 @@ -# Minimal D&D Scene Chunking - -Status: Implemented - -## Purpose - -Narrow the `dnd/scenes` chunk module to one responsibility: divide a transcript -into coherent, contiguous Dungeons & Dragons scenes. The current model response -also asks for titles, modes, participants, summaries, boundary explanations, -confidence labels, and plan-wide caveats. Those fields increase prompt and -response complexity, overlap with dedicated extraction artifacts, and are not -needed to materialize or validate chunks. - -This change applies the same minimal-contract policy used by the D&D extraction -lanes. The model should propose only facts that require model judgment and that -the chunking operation consumes. Notarius should derive or validate everything -else deterministically. - -## Desired End State - -The `dnd/scenes` chunker asks the model for an ordered, non-empty list of -inclusive source-unit ranges. Each range represents one coherent scene and -contains exactly: - -- `start_unit_id`; and -- `end_unit_id`. - -The response contains no scene title, summary, kind, participant list, -boundary note, confidence label, or plan-wide caveat list. The accepted -`source.ChunkPlan` contains the validated ranges and source digest, with no D&D -scene-specific plan or range annotations. - -The module key, prompt identity, configured model profile, registration, and -the generic accepted chunk-map contract remain unchanged. The private prompt -and response-schema content changes in place; this pre-release application does -not need a compatibility layer for prior private responses. - -Existing canonical chunk plans may contain the annotations removed by this -feature. Invalidate the pre-change chunk-plan cache format once so those records -are regenerated. This is a storage compatibility transition, not a change to -the source-addressed lookup policy: after the transition, canonical plans -remain keyed only by source digest and continue to be reused independently of -prompt, schema, profile, or module configuration. Prompt and schema hashes -remain provenance rather than cache-key inputs. - -## Scene Boundary Policy - -A scene is a coherent unit of play. A new scene is appropriate when the -transcript establishes a meaningful change in location, objective, threat, -activity, encounter, or mode of play. Examples include: - -- moving to a materially different location; -- beginning or ending combat; -- entering a substantially different phase of an encounter; -- changing between combat, exploration, social interaction, planning, travel, - rest, or downtime; -- shifting the central NPC, faction, threat, or immediate objective; or -- a sustained table-level interruption that materially changes the activity. - -A scene should not begin solely because a speaker changes, a combat round -changes, a routine turn occurs, or the table briefly digresses. The chunker -should prefer a smaller number of coherent scenes over speculative or -fine-grained boundaries. - -The prompt owns this semantic boundary guidance. Deterministic code owns all -structural invariants. - -## Model And Prompt Boundary - -Keep the existing shared D&D system, transcript, and reference assets and the -established prompt ordering. Simplify only the scene-specific task and -instructions needed to request boundary ranges. Do not move scene-specific -wording into shared assets unless another module needs exactly identical -content. - -The private structured-output schema must: - -- require one top-level `scenes` array; -- require at least one scene; -- permit only integer `start_unit_id` and `end_unit_id` fields on each scene; -- require positive unit IDs; and -- reject unknown fields at every object boundary. - -Schema validation is intentionally structural. The model is not responsible -for chunk IDs, chunk indexes, source identity, digests, annotations, or -diagnostics. - -The prompt must require full transcript coverage in source order without gaps -or overlaps. These instructions guide the model, but deterministic validation -remains authoritative. - -## Deterministic Planning And Validation - -Convert an accepted model response directly into a `source.ChunkPlan`. Preserve -the existing guarantees that: - -- the scene list is present and non-empty; -- every endpoint identifies a unit in the current source document; -- each start appears at or before its corresponding end in document order; -- the first range starts at the first source unit; -- adjacent ranges are contiguous and do not overlap; -- ranges preserve source-document order; and -- the final range ends at the final source unit. - -Do not assume that source-unit IDs are numerically contiguous or that numeric -ID order is document order. Continue to validate positions against the source -document's ordered unit collection. - -Materialization remains responsible for deterministic chunk IDs, indexes, unit -membership, and generic plan validation. The chunker should not reproduce -framework-owned checks except where rejecting the private model response is -necessary to construct a valid plan. - -Because the minimal response has no advisory fields, the chunker emits no -model-derived boundary warnings. Transport, schema, endpoint, coverage, order, -gap, and overlap failures remain ordinary chunk-planning errors. Do not replace -removed caveats or confidence labels with free-form diagnostics or hidden -annotations. - -## Ownership Of Removed Semantics - -Removed fields do not move into another chunker contract: - -- `kind`, `title`, and `summary` belong to the accepted - `dnd/scene-description-list` artifact produced by the - `dnd/scene-descriptions` extraction lane. -- NPC occurrence evidence belongs to `dnd/npc-interactions`; a scene - participant view may later be derived deterministically by intersecting - artifact evidence ranges with scene ranges. -- Combat participation and turns belong to their dedicated artifacts. -- Boundary confidence, boundary notes, and plan-wide caveats are omitted until - a concrete validator or operator workflow demonstrates a durable need for - them. - -The generic `source/chunk-map` artifact continues to represent the exact -accepted materialized chunks. For plans produced by `dnd/scenes`, its -`plan_annotations` and each chunk's `annotations` map are empty. Do not add -D&D-specific fields to the generic chunk-map schema. - -## Quality Policy - -Protect the minimal response contract and deterministic range invariants at -their stable behavioral boundaries. Do not add tests that freeze incidental -prompt wording, message length, or exact prompt hashes. Cache compatibility -tests may assert the chunk-plan storage version because it is a deliberate wire -compatibility boundary. - -Evaluate boundary quality on a small human-reviewed transcript set containing -combat transitions, location changes, planning, social interaction, brief -digressions, and ambiguous gradual transitions. Review whether the simpler -contract improves full-coverage success and boundary usefulness on the smaller -models the application is intended to support. Treat this evaluation as a -human development aid, not a deterministic correctness oracle or an offline -test-suite requirement. - -## Non-Goals - -This scope does not: - -- change how source documents or chunks are represented; -- change generic chunk validation or materialization; -- change the durable `source/chunk-map` schema or output option; -- add scene descriptions, classifications, summaries, or participants to the - chunker; -- change the `dnd/scene-descriptions` artifact; -- make combat extraction depend on scene descriptions or skip non-combat - chunks; -- introduce ordered pipeline dependencies, a DAG, or cross-lane - reconciliation; -- add a new LLM validator, repair pass, or semantic post-processing step; or -- preserve compatibility with prior private scene-chunker responses or cached - plans. diff --git a/internal/core/artifacts/artifacts_test.go b/internal/core/artifacts/artifacts_test.go index 1888a29..cca5f2f 100644 --- a/internal/core/artifacts/artifacts_test.go +++ b/internal/core/artifacts/artifacts_test.go @@ -20,7 +20,7 @@ func TestRunManifestOmitsEmptyOptionalFields(t *testing.T) { func TestRunManifestChunkPlanIsAdditiveAndOmitsPlanContent(t *testing.T) { manifest := RunManifest{ChunkPlan: &ChunkPlanManifest{ Mode: "auto", Action: "reused", SourceDigest: "sha256:source", PlanDigest: "sha256:plan", - PlanSchemaVersion: "notarius.chunk-plan.v1", RequestedModule: "chunk/current", + PlanSchemaVersion: "notarius.chunk-plan.v2", RequestedModule: "chunk/current", ProducerInputModule: "input/original", ProducerModule: "chunk/original", }} encoded, err := json.Marshal(manifest) diff --git a/internal/framework/pipeline/runner_chunk_plan_test.go b/internal/framework/pipeline/runner_chunk_plan_test.go index 641a2c9..bfce124 100644 --- a/internal/framework/pipeline/runner_chunk_plan_test.go +++ b/internal/framework/pipeline/runner_chunk_plan_test.go @@ -402,19 +402,19 @@ func TestRunnerRegeneratesStructurallyInvalidHit(t *testing.T) { } } -func TestRunnerReusesCrossDomainAnnotationsAsOptionalData(t *testing.T) { +func TestRunnerReusesAnnotationsFromDifferentChunkerAsOptionalData(t *testing.T) { prepared, plan := preparedTerminalDebugPipeline(t) - plan.Annotations = source.ChunkAnnotations{"dnd/scenes": json.RawMessage(`{"boundary_caveats":["uncertain"]}`)} - plan.Ranges[0].Annotations = source.ChunkAnnotations{"dnd/scenes": json.RawMessage(`{"title":"Opening"}`)} + plan.Annotations = source.ChunkAnnotations{"test/chunker": json.RawMessage(`{"label":"fixture"}`)} + plan.Ranges[0].Annotations = source.ChunkAnnotations{"test/chunker": json.RawMessage(`{"category":"sample"}`)} record := chunkPlanRecord(t, prepared, plan) - record.Producer.ChunkModule = "dnd/scenes" + record.Producer.ChunkModule = "chunk/producer" store := &recordingChunkPlanStore{record: record, decision: ChunkPlanDecision{Status: ChunkPlanHit}} output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), ChunkCacheMode: ChunkCacheAuto, ChunkPlans: store}) if err != nil { t.Fatal(err) } - if output.Manifest.ChunkPlan.ProducerModule != "dnd/scenes" || output.Manifest.ChunkPlan.Action != "reused" { - t.Fatalf("cross-domain annotation plan was not reused: %#v", output.Manifest.ChunkPlan) + if output.Manifest.ChunkPlan.ProducerModule != "chunk/producer" || output.Manifest.ChunkPlan.Action != "reused" { + t.Fatalf("different-chunker annotation plan was not reused: %#v", output.Manifest.ChunkPlan) } } diff --git a/internal/modules/dnd/chunk/scenes/chunker_test.go b/internal/modules/dnd/chunk/scenes/chunker_test.go index cf67b7f..1df9cba 100644 --- a/internal/modules/dnd/chunk/scenes/chunker_test.go +++ b/internal/modules/dnd/chunk/scenes/chunker_test.go @@ -170,14 +170,14 @@ func TestPlanReturnsAnnotationFreeSceneRangesFromStructuredOutput(t *testing.T) if got.StartUnitID != want.StartUnitID || got.EndUnitID != want.EndUnitID { t.Fatalf("range[%d] = %#v, want %#v", i, result.Plan.Ranges[i], want) } - if got.Annotations != nil { + if len(got.Annotations) != 0 { t.Fatalf("range[%d] annotations = %#v, want absent", i, got.Annotations) } } - if result.Plan.Annotations != nil { + if len(result.Plan.Annotations) != 0 { t.Fatalf("plan annotations = %#v, want absent", result.Plan.Annotations) } - if result.Warnings != nil { + if len(result.Warnings) != 0 { t.Fatalf("warnings = %#v, want absent", result.Warnings) } }