366 lines
15 KiB
Markdown
366 lines
15 KiB
Markdown
# D&D NPC Semantic Normalization Follow-up Plan
|
|
|
|
## Status
|
|
|
|
Ready for implementation.
|
|
|
|
## Objective
|
|
|
|
Close the four findings from the post-implementation review of
|
|
[D&D NPC Semantic Normalization](dnd-npc-semantic-normalization.md) without
|
|
changing its durable artifact schema, deterministic identity rules,
|
|
conflict-aware partial-application policy, retry accounting, or generated
|
|
reference handoff.
|
|
|
|
Complete the four follow-up stages below in order. Each stage should leave its
|
|
affected packages passing before the next stage begins.
|
|
|
|
## Completed Work Summary
|
|
|
|
The original five-stage implementation is complete:
|
|
|
|
1. **Framework retryable normalize fallback.** The framework now classifies
|
|
invalid structured output separately from operational LLM failures.
|
|
Normalizers can return a safe candidate and retry directive; the runner owns
|
|
retry accounting, attempt debug artifacts, final fallback validation,
|
|
accepted-only warning promotion, and checkpoint recording.
|
|
2. **NPC prompt and context material.** `dnd/npcs` owns a private normalization
|
|
prompt and structural response schema. It sends display-name candidates and
|
|
coalesced transcript windows with a fixed two-unit radius, without exposing
|
|
stable NPC IDs or using the full source input.
|
|
3. **Semantic proposal validation and application.** The normalizer performs
|
|
deterministic preprocessing, resolves returned names through the existing
|
|
comparison-key policy, applies independent safe groups, discards locally
|
|
invalid and conflicting groups, preserves evidence and ordering, and emits
|
|
bounded D&D diagnostics.
|
|
4. **Production integration and checkpoint identity.** Production registration
|
|
includes the normalize prompt and schema. Prompt, schema, identity,
|
|
normalization, and context policies contribute checkpoint fingerprints.
|
|
Assembled tests prove retry behavior and the ordered normalized-NPC handoff
|
|
to downstream consumers.
|
|
5. **Configuration and documentation.** The maintained complete D&D example
|
|
demonstrates an explicit normalize profile and retry count. Configuration,
|
|
operations, integration, LLM, module, pipeline, and component documentation
|
|
describe the implemented feature.
|
|
|
|
The completed implementation passed the full test suite, vet, build, diff
|
|
checks, and the targeted race suite. The stages below are limited follow-up
|
|
corrections and do not reopen the original feature design.
|
|
|
|
## Decisions Applying To Every Follow-up Stage
|
|
|
|
- Retain module key `dnd/npcs`, artifact kind `dnd/npc-list`, durable schema
|
|
`notarius.dnd.npcs` v1, prompt ID `dnd.npcs.normalize`, response-schema
|
|
identity, empty reference-slot contract, strict empty options, and default
|
|
normalize validator chain.
|
|
- Retain the context radius of two and the existing candidate and transcript
|
|
input shapes.
|
|
- Retain conflict-aware partial application: independently safe groups may be
|
|
applied, every group participating in a resolved-member conflict is
|
|
discarded, retries restart from the original merge input, and final
|
|
exhaustion accepts only the final attempt's safe candidate.
|
|
- Retain `retries: 0` as the application-wide default. Changing that default
|
|
remains separate work.
|
|
- Retain accepted-only warning promotion, checkpoint format, public
|
|
configuration shape, and durable output JSON.
|
|
- Do not add an LLM-backed validator, normalizer-local retry loop, generic
|
|
deduplication framework, prompt-language change-detector test, or live
|
|
provider test.
|
|
|
|
## Stage 1: Make Context-Material Failures Content-Safe
|
|
|
|
### Goal
|
|
|
|
Prevent arbitrary source metadata keys or other source-derived details from
|
|
appearing in errors returned by the NPC normalization context builder.
|
|
|
|
### Implementation
|
|
|
|
Keep `source.CloneMetadata` unchanged. Its detailed errors remain useful to
|
|
trusted direct callers and its own package tests. Redaction belongs at the
|
|
external source-material consumption boundary in
|
|
`internal/modules/dnd/normalize/npcs/context_material.go`.
|
|
|
|
At that boundary:
|
|
|
|
- When copying source-unit metadata fails, return a fixed category such as
|
|
`build NPC normalization context: invalid source metadata`.
|
|
- Do not wrap the underlying clone error with `%w`.
|
|
- The returned error must not include metadata keys, values, traversal
|
|
locations, transcript text, NPC names, source IDs, origin paths, encoded
|
|
material, concrete Go types derived from source metadata, or the underlying
|
|
error text.
|
|
- Apply the same fixed-error treatment to candidate and transcript JSON
|
|
encoding failures. Those paths are effectively defensive after metadata
|
|
validation, but future DTO changes must not create a content-exposure path.
|
|
- Keep the negative-radius error detailed: the radius is code-owned policy,
|
|
not source content.
|
|
- Keep failure timing unchanged. Context construction still fails before the
|
|
normalization LLM call, retry directive creation, artifact validation, or
|
|
checkpoint recording.
|
|
|
|
The outer `Normalize` error may retain its existing operation context because
|
|
the nested context-builder error is fixed and content-safe.
|
|
|
|
### Tests
|
|
|
|
Add narrow boundary coverage in the NPC normalization package:
|
|
|
|
- Use source metadata with a recognizable sensitive key and an unsupported or
|
|
non-finite value.
|
|
- Assert that normalization fails before the LLM fake is called.
|
|
- Assert that the error contains a stable context-material failure category
|
|
but none of the recognizable key, value, transcript text, NPC names, source
|
|
ID, or origin path.
|
|
- Retain direct `source.CloneMetadata` tests as the owner of detailed cloning
|
|
behavior; do not duplicate its type matrix at the normalizer boundary.
|
|
|
|
Do not assert the complete error sentence. Protect the fixed category and
|
|
absence of source-derived sentinels.
|
|
|
|
### Completion Gate
|
|
|
|
Every context-material construction failure exposed by `dnd/npcs` is
|
|
content-safe, while trusted source-package callers retain detailed clone
|
|
diagnostics.
|
|
|
|
## Stage 2: Remove Conflicting Shared Identity Prompt Guidance
|
|
|
|
### Goal
|
|
|
|
Give the NPC normalizer one unambiguous canonical-name policy and avoid telling
|
|
it about campaign references that it does not receive.
|
|
|
|
### Implementation
|
|
|
|
The existing `common-dnd-identity.md` fragment serves extraction prompts: it
|
|
asks for the most specific supported in-world identity and discusses campaign
|
|
and registry references. It is not wholly applicable to reconciliation of an
|
|
already extracted NPC list.
|
|
|
|
Remove `common-dnd-identity.md` from the NPC normalization prompt only:
|
|
|
|
- Remove its message from
|
|
`assets/prompts/dnd.npcs.normalize.yaml`.
|
|
- Remove it from the normalization package's `PromptAssetManifest.SharedFiles`.
|
|
- Do not change the shared fragment or any extraction prompt that currently
|
|
consumes it.
|
|
- Do not replace it with a copied or newly shared identity fragment. NPC
|
|
extraction has already established that the candidates are in-world NPC
|
|
records; normalization needs only the package-owned duplicate and canonical
|
|
selection instructions.
|
|
- Retain `common-dnd-system.md` and `common-dnd-transcript.md`.
|
|
|
|
After removing the identity message, preserve two intentional prompt-cache
|
|
tiers:
|
|
|
|
1. Mark the common D&D system message as the end of the stable shared tier.
|
|
2. Retain the existing boundary after the package-owned normalization
|
|
instructions.
|
|
|
|
The package-owned instructions remain authoritative:
|
|
|
|
- consolidate only when transcript context clearly identifies one individual;
|
|
- select a supplied canonical name;
|
|
- prefer an unadorned proper name over the same name plus a contextual class,
|
|
role, title, or relationship descriptor unless established as part of the
|
|
name; and
|
|
- prefer no consolidation when identity is ambiguous.
|
|
|
|
Do not add references, stable IDs, new prompt inputs, new natural-language
|
|
repair material, or schema constraints. The prompt digest will change and
|
|
prior NPC normalize checkpoints will intentionally become cold misses. Keep
|
|
the existing prompt ID and v1 version because checkpoint compatibility is
|
|
already content-fingerprinted and no external prompt protocol is versioned by
|
|
this private asset.
|
|
|
|
Update the canonical internal LLM documentation to describe the normalization
|
|
prompt's actual shared tier and to stop claiming that it uses shared identity
|
|
guidance.
|
|
|
|
### Tests
|
|
|
|
Update the existing offline prepared-prompt test to prove:
|
|
|
|
- the prompt prepares with five messages in the intended order;
|
|
- the first cache boundary follows the common D&D system message;
|
|
- the second follows the package-owned instructions;
|
|
- candidate and transcript messages remain in the variable tail without cache
|
|
boundaries;
|
|
- only the two declared dynamic inputs are rendered in their corresponding
|
|
messages; and
|
|
- prompt metadata and fingerprint construction remain valid.
|
|
|
|
Tests may assert asset/message identity, ordering, inputs, and cache-control
|
|
structure. They must not require the presence or absence of particular words
|
|
or phrases in natural-language prompt content.
|
|
|
|
### Completion Gate
|
|
|
|
The normalization prompt contains no extraction-specific identity or reference
|
|
guidance, preserves its intended cache structure, and still carries one clear
|
|
package-owned canonical-name policy.
|
|
|
|
## Stage 3: Enforce Generic Normalize-Retry Diagnostic Bounds
|
|
|
|
### Goal
|
|
|
|
Make the framework-level `NormalizeRetry` contract safely reusable by enforcing
|
|
the mechanical parts of its diagnostic contract at the runner boundary.
|
|
|
|
### Contract
|
|
|
|
Add provider-neutral limits in `internal/framework/contracts`:
|
|
|
|
- maximum retry reason-code length: 128 bytes;
|
|
- maximum retry message length: 4,096 bytes; and
|
|
- both values must be valid UTF-8 and nonblank after trimming surrounding
|
|
whitespace for the blank check.
|
|
|
|
Expose the limits as named constants alongside `NormalizeRetry` so normalizer
|
|
authors can construct compliant diagnostics without importing a D&D package.
|
|
Do not import `internal/modules/dnd/shared/diagnostics` into the framework.
|
|
|
|
These are encoded-byte limits, not rune limits. The framework validates but
|
|
does not truncate or rewrite caller-supplied diagnostics. Silent truncation
|
|
could collapse stable reason identities or conceal a module defect.
|
|
|
|
Content safety remains the module's semantic responsibility: the framework
|
|
cannot determine whether otherwise valid text contains transcript content,
|
|
credentials, paths, names, or other sensitive values.
|
|
|
|
### Runner Implementation
|
|
|
|
Extract the current inline blank check into a small framework-owned validator
|
|
for `*contracts.NormalizeRetry` and call it before placing the directive in a
|
|
debug payload.
|
|
|
|
The validator must reject:
|
|
|
|
- a blank reason code;
|
|
- a blank message;
|
|
- invalid UTF-8 in either field;
|
|
- a reason code exceeding 128 bytes; and
|
|
- a message exceeding 4,096 bytes.
|
|
|
|
Return a fixed module-contract error that identifies the invalid field and
|
|
failure category without echoing either supplied value. Preserve existing
|
|
retry-loop behavior for module contract errors; do not create a new error
|
|
class, rejection, or durable warning.
|
|
|
|
Do not impose a generic count or text-size policy on `FallbackWarnings` in this
|
|
stage. Warning limits remain owned by their existing domain and validator
|
|
contracts; broad warning-policy unification is separate work.
|
|
|
|
Update `docs/internal/pipeline.md` to document the normalize-retry limits and
|
|
the division of responsibility between mechanical framework validation and
|
|
module-owned content safety.
|
|
|
|
### Tests
|
|
|
|
Extend the generic runner retry tests with table-driven boundary cases:
|
|
|
|
- a reason code and message exactly at their byte limits are accepted;
|
|
- one byte over either limit is rejected;
|
|
- invalid UTF-8 in either field is rejected;
|
|
- blank fields remain rejected; and
|
|
- errors contain only fixed field/category context and do not echo supplied
|
|
diagnostic sentinels.
|
|
|
|
Use test-controlled strings and the exported contract constants. Keep the
|
|
existing NPC diagnostic tests as the owner of D&D aggregation and omission
|
|
behavior.
|
|
|
|
### Completion Gate
|
|
|
|
Any typed normalizer receives the same reusable retry mechanism, and the runner
|
|
cannot write a blank, invalid-UTF-8, or unbounded retry diagnostic into debug
|
|
state.
|
|
|
|
## Stage 4: Correct Safe-Fallback Documentation
|
|
|
|
### Goal
|
|
|
|
Make canonical pipeline documentation accurately describe the generic
|
|
framework contract and the NPC module's model-derived safe partial fallback.
|
|
|
|
### Documentation
|
|
|
|
Update `docs/internal/pipeline.md`:
|
|
|
|
- replace `deterministic safe candidate` with `module-supplied safe candidate`
|
|
or equivalent wording;
|
|
- state that the framework treats the candidate as opaque and relies on the
|
|
normalizer plus the configured validator chain for its safety;
|
|
- retain the rules that intermediate candidates are attempt-local, only the
|
|
final fallback is validated for acceptance, and accepted-only warnings and
|
|
checkpoints remain unchanged; and
|
|
- keep the generic contract separate from the NPC-specific conflict and
|
|
partial-application rules.
|
|
|
|
Review the affected current-behavior documentation for the same inaccurate
|
|
determinism claim. The NPC integration and operations documents may describe
|
|
deterministic validation and mutation, but must not imply that every safe
|
|
partial fallback is independent of the LLM proposal.
|
|
|
|
Do not modify the completed feature roadmap except if a factual contradiction
|
|
with the implemented target state is discovered. Do not add tests for prose
|
|
wording.
|
|
|
|
### Verification
|
|
|
|
- Validate local documentation links.
|
|
- Confirm configuration defaults, module keys, prompt IDs, and schema versions
|
|
against current code.
|
|
- Run `git diff --check`.
|
|
|
|
### Completion Gate
|
|
|
|
Canonical documentation distinguishes the generic opaque safe-candidate
|
|
contract from deterministic NPC validation and application, with no duplicated
|
|
or contradictory retry semantics.
|
|
|
|
## Final Verification
|
|
|
|
From the repository root, run:
|
|
|
|
```sh
|
|
git diff --check
|
|
go test ./...
|
|
go vet ./...
|
|
go build ./cmd/notarius
|
|
go test -race ./internal/framework/contracts ./internal/framework/llm ./internal/framework/pipeline ./internal/modules/dnd/normalize/npcs ./internal/modules/dnd/register ./internal/modules/integration
|
|
```
|
|
|
|
Review the final diff for:
|
|
|
|
- no durable artifact, checkpoint-format, or public configuration change;
|
|
- no model-facing stable NPC IDs or full-transcript normalization input;
|
|
- no source-derived metadata keys, values, paths, names, or transcript content
|
|
in context-material errors;
|
|
- no extraction-specific shared identity fragment in the normalize prompt;
|
|
- no prompt-wording change-detector or live-provider test;
|
|
- no framework dependency on D&D diagnostic packages;
|
|
- no silent retry-diagnostic truncation;
|
|
- no change to conflict-aware partial application, retry accounting, warning
|
|
promotion, validator ordering, or downstream handoff; and
|
|
- documentation that describes only implemented behavior in its canonical
|
|
owner.
|
|
|
|
## Assumptions
|
|
|
|
- Detailed `source.CloneMetadata` errors remain appropriate inside the trusted
|
|
source package; only the NPC context-material boundary requires redaction.
|
|
- The normalizer does not need general extraction identity guidance because its
|
|
candidates are already accepted NPC artifacts.
|
|
- Mechanical UTF-8 and byte limits are appropriate framework invariants;
|
|
semantic content safety remains a module obligation.
|
|
- Prompt-content and cache-boundary changes intentionally invalidate existing
|
|
NPC normalize checkpoints through the current fingerprint mechanism.
|
|
- Backward compatibility for development checkpoints is not required.
|
|
|
|
## Open Questions
|
|
|
|
None. The review findings and decisions above are sufficient to implement the
|
|
follow-up work without additional product or architecture choices.
|