Clean up completed D&D scene chunking work
This commit is contained in:
@@ -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.
|
||||
Reference in New Issue
Block a user