Clean up completed roadmap work
This commit is contained in:
@@ -1,365 +0,0 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user