Update documentation to reflect the implemented chunking module
This commit is contained in:
@@ -1,245 +0,0 @@
|
||||
# Chunk Follow-Up Implementation Plan
|
||||
|
||||
This plan addresses review findings from the first `dnd/scenes` implementation.
|
||||
It is written for an LLM coding agent that will implement each stage in order.
|
||||
|
||||
Before beginning any stage, review:
|
||||
|
||||
- `docs/policy/architecture.md`
|
||||
- `docs/policy/development.md`
|
||||
- `docs/policy/documentation.md`
|
||||
- `docs/roadmap/chunk.md`
|
||||
|
||||
Do not move planned behavior into non-roadmap docs until the corresponding code
|
||||
is implemented. Do not revert unrelated user changes.
|
||||
|
||||
## Goals
|
||||
|
||||
- Record chunker prompt and response-schema provenance in run manifests.
|
||||
- Ensure downstream extractors receive canonical source units from the source
|
||||
document, not chunker-mutated unit payloads.
|
||||
- Prevent empty or whitespace-only scene caveats from becoming warnings.
|
||||
|
||||
## Stage 1: Run-Manifest Module Metadata
|
||||
|
||||
Goal: make non-lane module provenance auditable without adding
|
||||
chunker-specific fields or D&D-specific framework behavior.
|
||||
|
||||
Design decision:
|
||||
|
||||
- Add a generic top-level run manifest metadata map for singleton pipeline
|
||||
modules:
|
||||
|
||||
```go
|
||||
ModuleMetadata map[string]map[string]any `json:"module_metadata,omitempty"`
|
||||
```
|
||||
|
||||
- Use stable stage keys:
|
||||
- `input`
|
||||
- `chunker`
|
||||
- `output`
|
||||
- Keep existing artifact lane metadata under `ArtifactLaneManifest.Metadata`.
|
||||
Do not move extractor, merger, normalizer, or validator metadata into the
|
||||
top-level map in this stage.
|
||||
- Record metadata only when a built module implements
|
||||
`contracts.ManifestMetadataProvider` and returns non-empty metadata.
|
||||
- Continue to reject raw prompts, raw response schemas, source text, provider
|
||||
payloads, and secrets from manifest metadata by convention and tests.
|
||||
|
||||
Code changes:
|
||||
|
||||
- Add `ModuleMetadata` to `internal/core/artifacts.RunManifest`.
|
||||
- Add a small helper in `internal/framework/pipeline/runner.go` to attach
|
||||
top-level module metadata by stage key.
|
||||
- After building the input adapter, chunker, and output encoder, call that
|
||||
helper with keys `input`, `chunker`, and `output` respectively.
|
||||
- Keep `setLaneManifestMetadata` for lane-owned modules. If practical, share
|
||||
metadata cloning logic with the new helper.
|
||||
- Ensure failed runs that already have a manifest also retain any metadata
|
||||
collected before the failure.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add pipeline runner tests proving top-level metadata is recorded for a fake
|
||||
chunker that implements `ManifestMetadataProvider`.
|
||||
- Add a test proving the existing lane metadata behavior remains unchanged.
|
||||
- Add a CLI or output integration test proving a `dnd/scenes` run manifest
|
||||
contains `module_metadata.chunker` with prompt/schema provenance.
|
||||
- Add a negative assertion that raw prompt text, raw schema JSON, source text,
|
||||
provider payloads, and API key-like fields are not present in the scene
|
||||
chunker metadata.
|
||||
|
||||
Documentation:
|
||||
|
||||
- Update `docs/internal/pipeline.md` to describe top-level metadata for input,
|
||||
chunker, and output modules, and lane metadata for lane modules.
|
||||
- Update `docs/internal/modules.md` to say that `dnd/scenes` prompt/schema
|
||||
provenance appears under `module_metadata.chunker`.
|
||||
- Update `docs/integrations/json-output.md` and `docs/operations.md` if their
|
||||
manifest descriptions need to mention `module_metadata`.
|
||||
|
||||
Validation:
|
||||
|
||||
```sh
|
||||
go test ./internal/core/artifacts
|
||||
go test ./internal/framework/pipeline
|
||||
go test ./internal/cli
|
||||
go test ./internal/modules/chunk/dnd/scenes
|
||||
```
|
||||
|
||||
Stage completion criteria:
|
||||
|
||||
- `dnd/scenes` prompt/schema provenance is visible in durable
|
||||
`manifest.json` and diagnostics `run-manifest.json`.
|
||||
- Existing artifact lane metadata remains in the same JSON location as before.
|
||||
|
||||
## Stage 2: Canonical Source Units In Chunk Results
|
||||
|
||||
Goal: preserve the chunker boundary contract while ensuring extractors always
|
||||
consume source document units, not rewritten units supplied by a chunker.
|
||||
|
||||
Design decision:
|
||||
|
||||
- Keep the chunker contract expressed in terms of `contracts.SourceChunk`.
|
||||
- Continue validating chunk IDs, source IDs, indexes, unit membership, unit
|
||||
uniqueness within each chunk, and source-ordering.
|
||||
- After validation, canonicalize chunk units by replacing each returned
|
||||
`SourceUnit` with a defensive copy of the matching source document unit.
|
||||
- Preserve `SourceChunk.Metadata` as module-owned chunk metadata.
|
||||
- Do not require full source coverage and do not reject overlap between chunks.
|
||||
- Do not preserve chunker-mutated per-unit text, kind, or metadata. A chunker
|
||||
that wants to add scene-level information must use `SourceChunk.Metadata`.
|
||||
|
||||
Code changes:
|
||||
|
||||
- Replace or extend `validateChunkResult` in
|
||||
`internal/framework/pipeline/chunk_validation.go` so it returns canonical
|
||||
chunks, for example:
|
||||
|
||||
```go
|
||||
func validateAndCanonicalizeChunkResult(doc *source.SourceDocument, chunks []contracts.SourceChunk) ([]contracts.SourceChunk, error)
|
||||
```
|
||||
|
||||
- Build a source-unit lookup from the validated source document.
|
||||
- For each chunk:
|
||||
- validate the existing generic invariants;
|
||||
- copy chunk ID, source ID, index, and chunk metadata;
|
||||
- replace the unit slice with cloned source units from the source document in
|
||||
the returned boundary/order.
|
||||
- Update the runner to use canonical chunks for all downstream extraction and
|
||||
merge behavior.
|
||||
- Ensure chunk metadata is cloned so later module or caller mutation cannot
|
||||
affect runner state.
|
||||
- Keep the implementation source-agnostic. Do not inspect transcript-specific
|
||||
metadata keys.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add a runner test where a fake chunker returns a valid unit ID with mutated
|
||||
text, kind, and unit metadata. Assert the extractor receives the original
|
||||
source document unit values.
|
||||
- Add a runner test proving chunk metadata survives canonicalization and is not
|
||||
aliased to the chunker-returned map.
|
||||
- Keep existing tests for invalid chunk IDs, duplicate IDs, wrong source ID,
|
||||
wrong index, empty units, repeated unit IDs, unknown unit IDs, out-of-order
|
||||
units, partial coverage, and overlap.
|
||||
- Add or update tests so generic chunking still behaves unchanged.
|
||||
|
||||
Documentation:
|
||||
|
||||
- Update internal chunk contract docs to state that source units in chunks are
|
||||
canonicalized from the source document by ID before extractors run.
|
||||
- Document that chunk metadata is the supported mechanism for passing
|
||||
chunker-owned context to extractors.
|
||||
|
||||
Validation:
|
||||
|
||||
```sh
|
||||
go test ./internal/framework/pipeline
|
||||
go test ./internal/modules/chunk/generic
|
||||
go test ./internal/modules/chunk/dnd/scenes
|
||||
```
|
||||
|
||||
Stage completion criteria:
|
||||
|
||||
- Extractors cannot observe chunker-rewritten source-unit text, kind, or unit
|
||||
metadata.
|
||||
- Chunker-owned scene metadata still reaches extractors through
|
||||
`SourceChunk.Metadata`.
|
||||
|
||||
## Stage 3: Scene Caveat Hygiene
|
||||
|
||||
Goal: ensure model caveats become useful warnings and never produce blank
|
||||
warnings that can confuse operators or affect diagnostics retention.
|
||||
|
||||
Design decision:
|
||||
|
||||
- Require caveat strings to be non-empty after trimming.
|
||||
- Treat whitespace-only caveats as malformed structured output rather than
|
||||
silently dropping them. This is consistent with the `dnd/scenes` policy of
|
||||
failing explicitly for malformed model output.
|
||||
- Store warning messages as trimmed caveat text.
|
||||
|
||||
Code changes:
|
||||
|
||||
- Update `internal/modules/chunk/dnd/scenes/assets/schemas/dnd_scenes.v1.json`
|
||||
so `boundary_caveats.items` has `minLength: 1`.
|
||||
- Update `warningsFromCaveats` or response validation in
|
||||
`internal/modules/chunk/dnd/scenes/chunker.go` to trim caveats and reject
|
||||
empty results with a module-prefixed malformed-output error.
|
||||
- Prefer validating caveats before constructing chunks so all malformed response
|
||||
checks happen together.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add schema tests proving `boundary_caveats` items require non-empty strings.
|
||||
- Add chunker tests proving:
|
||||
- caveat warning messages are trimmed;
|
||||
- whitespace-only caveats fail explicitly;
|
||||
- valid caveats still produce `scene_boundary_caveat` warnings.
|
||||
- Keep existing warning tests passing.
|
||||
|
||||
Documentation:
|
||||
|
||||
- Update `docs/internal/modules.md` and `docs/troubleshooting.md` if needed to
|
||||
mention that malformed caveats are treated as malformed structured output.
|
||||
|
||||
Validation:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/chunk/dnd/scenes
|
||||
go test ./internal/cli
|
||||
```
|
||||
|
||||
Stage completion criteria:
|
||||
|
||||
- No blank warnings can be emitted from `dnd/scenes` boundary caveats.
|
||||
- Valid caveats remain visible as warnings.
|
||||
|
||||
## Stage 4: Full Verification
|
||||
|
||||
Goal: verify the follow-up work across contracts, production wiring,
|
||||
documentation, and the command entry point.
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./...
|
||||
go vet ./...
|
||||
go build ./cmd/notarius
|
||||
```
|
||||
|
||||
Inspect or test representative output manifests:
|
||||
|
||||
- `manifest.json` includes `module_metadata.chunker` for a `dnd/scenes` run;
|
||||
- `module_metadata.chunker` contains prompt and response-schema provenance;
|
||||
- no raw prompt, raw schema, source text, provider payload, or secret appears in
|
||||
module metadata;
|
||||
- artifact lane metadata remains under `artifact_lanes[].metadata`;
|
||||
- `warnings.json` contains trimmed scene caveats and no blank caveat warnings.
|
||||
|
||||
Stage completion criteria:
|
||||
|
||||
- Full validation commands pass.
|
||||
- Current-behavior docs match implemented behavior.
|
||||
- Any remaining planned or deferred behavior stays under `docs/roadmap/`.
|
||||
Reference in New Issue
Block a user