Improve item occurrence holder corrections
This commit is contained in:
@@ -16,7 +16,7 @@ import (
|
||||
const (
|
||||
Key = "extract/dnd/item-occurrences/shape"
|
||||
ReasonCode = "invalid_item_occurrence_shape"
|
||||
policy = "dnd.item_occurrences.shape.v1"
|
||||
policy = "dnd.item_occurrences.shape.v2"
|
||||
)
|
||||
|
||||
type Options struct{}
|
||||
@@ -35,47 +35,181 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint {
|
||||
}
|
||||
|
||||
func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.ItemOccurrenceList]) (contracts.ValidationResult, error) {
|
||||
if err := Validate(req.Value); err != nil {
|
||||
return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return a complete item-occurrence list with every required field present, valid contextual item names, supported event kinds and holder combinations, positive quantities, and valid source references."}, nil
|
||||
assessment := assess(req.Value)
|
||||
if len(assessment.operatorIssues) != 0 {
|
||||
return contracts.ValidationResult{
|
||||
Approved: false,
|
||||
ReasonCode: ReasonCode,
|
||||
Message: assessment.operatorMessage(),
|
||||
CorrectionGuidance: assessment.correctionGuidance(),
|
||||
}, nil
|
||||
}
|
||||
return contracts.ValidationResult{Approved: true}, nil
|
||||
}
|
||||
|
||||
// Validate returns one bounded error for every owned item-occurrence shape issue.
|
||||
func Validate(value dnd.ItemOccurrenceList) error {
|
||||
issues := issuesFor(value)
|
||||
if len(issues) == 0 {
|
||||
assessment := assess(value)
|
||||
if len(assessment.operatorIssues) == 0 {
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("%s", diagnostics.Aggregate("invalid item occurrence shape", issues))
|
||||
return fmt.Errorf("%s", assessment.operatorMessage())
|
||||
}
|
||||
|
||||
func issuesFor(value dnd.ItemOccurrenceList) []string {
|
||||
type validationAssessment struct {
|
||||
operatorIssues []string
|
||||
correctionGroups []correctionGroup
|
||||
groupIndexes map[string]int
|
||||
}
|
||||
|
||||
type correctionGroup struct {
|
||||
label string
|
||||
rule string
|
||||
records []string
|
||||
recordsSeen map[string]struct{}
|
||||
}
|
||||
|
||||
func assess(value dnd.ItemOccurrenceList) validationAssessment {
|
||||
assessment := validationAssessment{groupIndexes: make(map[string]int)}
|
||||
if value.Occurrences == nil {
|
||||
return []string{"occurrences must be present"}
|
||||
assessment.operatorIssues = append(assessment.operatorIssues, "occurrences must be present")
|
||||
assessment.addCorrection("occurrences", "item-occurrence list", "Return an `occurrences` array; use an empty array when the transcript establishes no occurrences.", "")
|
||||
return assessment
|
||||
}
|
||||
issues := make([]string, 0)
|
||||
for index, occurrence := range value.Occurrences {
|
||||
prefix := fmt.Sprintf("occurrences[%d]", index)
|
||||
context := occurrenceContext(occurrence)
|
||||
if strings.TrimSpace(occurrence.ItemID) == "" {
|
||||
issues = append(issues, prefix+".item_id must not be empty: "+diagnostics.Quote(occurrence.ItemID))
|
||||
assessment.operatorIssues = append(assessment.operatorIssues, prefix+".item_id must not be empty: "+diagnostics.Quote(occurrence.ItemID))
|
||||
assessment.addCorrection("item-name", "item name", "Select a contextual item name from the supplied item registry for every occurrence.", context)
|
||||
}
|
||||
if strings.TrimSpace(occurrence.Name) == "" {
|
||||
issues = append(issues, prefix+".name must not be empty: "+diagnostics.Quote(occurrence.Name))
|
||||
assessment.operatorIssues = append(assessment.operatorIssues, prefix+".name must not be empty: "+diagnostics.Quote(occurrence.Name))
|
||||
assessment.addCorrection("item-name", "item name", "Select a contextual item name from the supplied item registry for every occurrence.", context)
|
||||
}
|
||||
if !itemoccurrences.SupportedKind(occurrence.Kind) {
|
||||
issues = append(issues, prefix+".kind is unsupported: "+diagnostics.Quote(string(occurrence.Kind)))
|
||||
assessment.operatorIssues = append(assessment.operatorIssues, prefix+".kind is unsupported: "+diagnostics.Quote(string(occurrence.Kind)))
|
||||
assessment.addCorrection("kind", "occurrence kind", "Set `kind` to exactly one of `discovered`, `acquired`, `lost`, `consumed`, or `transferred`.", context)
|
||||
} else if !itemoccurrences.ValidHolderCombination(occurrence.Kind, occurrence.From, occurrence.To) {
|
||||
issues = append(issues, prefix+".from and .to are incompatible with "+diagnostics.Quote(string(occurrence.Kind)))
|
||||
assessment.operatorIssues = append(assessment.operatorIssues, prefix+holderOperatorIssue(occurrence))
|
||||
assessment.addCorrection("holders:"+string(occurrence.Kind), string(occurrence.Kind)+" holders", holderCorrection(occurrence.Kind), context)
|
||||
}
|
||||
if occurrence.Quantity != nil && *occurrence.Quantity < 1 {
|
||||
issues = append(issues, prefix+".quantity must be positive when present")
|
||||
assessment.operatorIssues = append(assessment.operatorIssues, prefix+".quantity must be positive when present")
|
||||
assessment.addCorrection("quantity", "quantity", "Set `quantity` to a positive integer when the transcript states one, or to JSON null when it does not.", context)
|
||||
}
|
||||
if len(occurrence.SourceRefs) == 0 {
|
||||
issues = append(issues, prefix+".source_refs must contain at least one reference")
|
||||
assessment.operatorIssues = append(assessment.operatorIssues, prefix+".source_refs must contain at least one reference")
|
||||
assessment.addCorrection("source-refs", "source references", "Provide at least one transcript source range that directly supports every occurrence.", context)
|
||||
}
|
||||
}
|
||||
return issues
|
||||
return assessment
|
||||
}
|
||||
|
||||
func (assessment *validationAssessment) addCorrection(key, label, rule, record string) {
|
||||
index, ok := assessment.groupIndexes[key]
|
||||
if !ok {
|
||||
index = len(assessment.correctionGroups)
|
||||
assessment.groupIndexes[key] = index
|
||||
assessment.correctionGroups = append(assessment.correctionGroups, correctionGroup{label: label, rule: rule, recordsSeen: make(map[string]struct{})})
|
||||
}
|
||||
if record != "" {
|
||||
group := &assessment.correctionGroups[index]
|
||||
if _, seen := group.recordsSeen[record]; seen {
|
||||
return
|
||||
}
|
||||
group.recordsSeen[record] = struct{}{}
|
||||
group.records = append(group.records, record)
|
||||
}
|
||||
}
|
||||
|
||||
func (assessment validationAssessment) operatorMessage() string {
|
||||
return diagnostics.Aggregate("invalid item occurrence shape", assessment.operatorIssues)
|
||||
}
|
||||
|
||||
func (assessment validationAssessment) correctionGuidance() string {
|
||||
issues := make([]string, 0, len(assessment.correctionGroups)+len(assessment.operatorIssues))
|
||||
for _, group := range assessment.correctionGroups {
|
||||
issues = append(issues, group.rule)
|
||||
}
|
||||
for _, group := range assessment.correctionGroups {
|
||||
for _, record := range group.records {
|
||||
issues = append(issues, "Affected "+group.label+" record: "+record)
|
||||
}
|
||||
}
|
||||
return diagnostics.Aggregate("Correct every rejected item occurrence and return the complete replacement list", issues)
|
||||
}
|
||||
|
||||
func holderOperatorIssue(occurrence dnd.ItemOccurrence) string {
|
||||
return fmt.Sprintf(
|
||||
".from and .to are incompatible with %s: expected %s; got from %s and to %s",
|
||||
diagnostics.Quote(string(occurrence.Kind)), holderExpectation(occurrence.Kind),
|
||||
holderDisplay(occurrence.From), holderDisplay(occurrence.To),
|
||||
)
|
||||
}
|
||||
|
||||
func holderExpectation(kind dnd.ItemOccurrenceKind) string {
|
||||
switch kind {
|
||||
case dnd.ItemOccurrenceKindDiscovered:
|
||||
return "both holders absent"
|
||||
case dnd.ItemOccurrenceKindAcquired:
|
||||
return "from absent and to present"
|
||||
case dnd.ItemOccurrenceKindLost:
|
||||
return "from present and to absent"
|
||||
case dnd.ItemOccurrenceKindConsumed:
|
||||
return "from present and to absent"
|
||||
case dnd.ItemOccurrenceKindTransferred:
|
||||
return "distinct named non-party holders"
|
||||
default:
|
||||
return "a supported holder combination"
|
||||
}
|
||||
}
|
||||
|
||||
func holderCorrection(kind dnd.ItemOccurrenceKind) string {
|
||||
switch kind {
|
||||
case dnd.ItemOccurrenceKindDiscovered:
|
||||
return "For `discovered` occurrences, set both `from` and `to` to JSON null."
|
||||
case dnd.ItemOccurrenceKindAcquired:
|
||||
return "For `acquired` occurrences, set `from` to JSON null and `to` to `party` or the named party member gaining possession."
|
||||
case dnd.ItemOccurrenceKindLost:
|
||||
return "For `lost` occurrences, set `from` to `party` or the named party member losing possession and set `to` to JSON null."
|
||||
case dnd.ItemOccurrenceKindConsumed:
|
||||
return "For `consumed` occurrences, set `from` to `party` or the named party member consuming the item and set `to` to JSON null."
|
||||
case dnd.ItemOccurrenceKindTransferred:
|
||||
return "For `transferred` occurrences, set `from` and `to` to two distinct named party members; never use `party` for either holder."
|
||||
default:
|
||||
return "Use the holder combination required by the selected supported occurrence kind."
|
||||
}
|
||||
}
|
||||
|
||||
func occurrenceContext(occurrence dnd.ItemOccurrence) string {
|
||||
name := itemoccurrences.DisplayValue(occurrence.Name)
|
||||
context := "item " + diagnostics.Quote(name)
|
||||
if name == "" {
|
||||
context = "item with a blank contextual name"
|
||||
}
|
||||
if len(occurrence.SourceRefs) == 0 {
|
||||
context += " without a cited source range"
|
||||
} else {
|
||||
ref := occurrence.SourceRefs[0]
|
||||
if ref.StartUnitID == ref.EndUnitID {
|
||||
context += fmt.Sprintf(" at source unit %d", ref.StartUnitID)
|
||||
} else {
|
||||
context += fmt.Sprintf(" at source units %d-%d", ref.StartUnitID, ref.EndUnitID)
|
||||
}
|
||||
if len(occurrence.SourceRefs) > 1 {
|
||||
context += fmt.Sprintf(" (first of %d cited ranges)", len(occurrence.SourceRefs))
|
||||
}
|
||||
}
|
||||
return context + " with from " + holderDisplay(occurrence.From) + " and to " + holderDisplay(occurrence.To)
|
||||
}
|
||||
|
||||
func holderDisplay(value string) string {
|
||||
value = itemoccurrences.DisplayValue(value)
|
||||
if value == "" {
|
||||
return "JSON null"
|
||||
}
|
||||
return diagnostics.Quote(value)
|
||||
}
|
||||
|
||||
func Spec() pipeline.ValidatorSpec {
|
||||
|
||||
@@ -2,6 +2,7 @@ package shape
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -68,16 +69,94 @@ func TestValidatorRejectsOwnedSemanticBoundaries(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidatorProvidesActionableContextualHolderGuidance(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
occurrence dnd.ItemOccurrence
|
||||
wantGuidance []string
|
||||
}{
|
||||
{
|
||||
name: "discovered",
|
||||
occurrence: dnd.ItemOccurrence{ItemID: "item:sha256:opaque", Name: "Ring", Kind: dnd.ItemOccurrenceKindDiscovered, From: "Aria", SourceRefs: refs(1, 2)},
|
||||
wantGuidance: []string{"For `discovered` occurrences", "both `from` and `to` to JSON null"},
|
||||
},
|
||||
{
|
||||
name: "acquired",
|
||||
occurrence: dnd.ItemOccurrence{ItemID: "item:sha256:opaque", Name: "Ring", Kind: dnd.ItemOccurrenceKindAcquired, From: "Merchant", To: "party", SourceRefs: refs(1, 2)},
|
||||
wantGuidance: []string{"For `acquired` occurrences", "`from` to JSON null", "named party member gaining possession"},
|
||||
},
|
||||
{
|
||||
name: "lost",
|
||||
occurrence: dnd.ItemOccurrence{ItemID: "item:sha256:opaque", Name: "Ring", Kind: dnd.ItemOccurrenceKindLost, From: "party", To: "Merchant", SourceRefs: refs(1, 2)},
|
||||
wantGuidance: []string{"For `lost` occurrences", "named party member losing possession", "`to` to JSON null"},
|
||||
},
|
||||
{
|
||||
name: "consumed",
|
||||
occurrence: dnd.ItemOccurrence{ItemID: "item:sha256:opaque", Name: "Ring", Kind: dnd.ItemOccurrenceKindConsumed, SourceRefs: refs(1, 2)},
|
||||
wantGuidance: []string{"For `consumed` occurrences", "named party member consuming the item", "`to` to JSON null"},
|
||||
},
|
||||
{
|
||||
name: "transferred",
|
||||
occurrence: dnd.ItemOccurrence{ItemID: "item:sha256:opaque", Name: "Ring", Kind: dnd.ItemOccurrenceKindTransferred, From: "party", To: "Borin", SourceRefs: refs(1, 2)},
|
||||
wantGuidance: []string{"For `transferred` occurrences", "two distinct named party members", "never use `party`"},
|
||||
},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemOccurrenceList]{Value: listWith(test.occurrence)})
|
||||
if err != nil || result.Approved {
|
||||
t.Fatalf("Validate() = %#v, %v; want rejection", result, err)
|
||||
}
|
||||
for _, fragment := range append(test.wantGuidance, "item \"Ring\"", "source units 1-2") {
|
||||
if !strings.Contains(result.CorrectionGuidance, fragment) {
|
||||
t.Fatalf("CorrectionGuidance = %q, want %q", result.CorrectionGuidance, fragment)
|
||||
}
|
||||
}
|
||||
for _, forbidden := range []string{test.occurrence.ItemID, "sha256", Key, ReasonCode, "occurrences[0]"} {
|
||||
if strings.Contains(result.CorrectionGuidance, forbidden) {
|
||||
t.Fatalf("CorrectionGuidance leaked implementation identifier %q: %q", forbidden, result.CorrectionGuidance)
|
||||
}
|
||||
}
|
||||
if !strings.Contains(result.Message, "expected") || !strings.Contains(result.Message, "got from") {
|
||||
t.Fatalf("operator message = %q, want expected and observed holders", result.Message)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidatorGroupsRepeatedHolderCorrectionsAndRetainsDistinctRules(t *testing.T) {
|
||||
value := dnd.ItemOccurrenceList{Occurrences: []dnd.ItemOccurrence{
|
||||
{ItemID: "item-1", Name: "Silver Pieces", Kind: dnd.ItemOccurrenceKindAcquired, From: "Orc", To: "party", SourceRefs: refs(1, 1)},
|
||||
{ItemID: "item-2", Name: "Torch", Kind: dnd.ItemOccurrenceKindAcquired, From: "Chest", To: "Aria", SourceRefs: refs(2, 2)},
|
||||
{ItemID: "item-3", Name: "Potion", Kind: dnd.ItemOccurrenceKindConsumed, SourceRefs: refs(3, 3)},
|
||||
}}
|
||||
result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemOccurrenceList]{Value: value})
|
||||
if err != nil || result.Approved {
|
||||
t.Fatalf("Validate() = %#v, %v; want rejection", result, err)
|
||||
}
|
||||
if strings.Count(result.CorrectionGuidance, "For `acquired` occurrences") != 1 || strings.Count(result.CorrectionGuidance, "For `consumed` occurrences") != 1 {
|
||||
t.Fatalf("CorrectionGuidance = %q, want one rule per affected kind", result.CorrectionGuidance)
|
||||
}
|
||||
for _, fragment := range []string{"item \"Silver Pieces\"", "source unit 1", "item \"Torch\"", "source unit 2", "item \"Potion\"", "source unit 3"} {
|
||||
if !strings.Contains(result.CorrectionGuidance, fragment) {
|
||||
t.Fatalf("CorrectionGuidance = %q, want contextual fragment %q", result.CorrectionGuidance, fragment)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidatorAggregatesBoundedIndexedDiagnosticsAndRegistration(t *testing.T) {
|
||||
value := dnd.ItemOccurrenceList{Occurrences: make([]dnd.ItemOccurrence, 24)}
|
||||
for index := range value.Occurrences {
|
||||
value.Occurrences[index] = dnd.ItemOccurrence{ItemID: "item", Name: " \n", Kind: "unsupported"}
|
||||
value.Occurrences[index] = dnd.ItemOccurrence{ItemID: "item", Name: fmt.Sprintf("Item %d", index), Kind: "unsupported"}
|
||||
}
|
||||
result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemOccurrenceList]{Value: value})
|
||||
if err != nil || result.Approved || result.ReasonCode != ReasonCode || len([]byte(result.Message)) > 4096 || !utf8.ValidString(result.Message) || !strings.Contains(result.Message, "occurrences[0]") || !strings.Contains(result.Message, "additional issue(s) omitted") {
|
||||
t.Fatalf("Validate() = %#v, %v", result, err)
|
||||
}
|
||||
if value.Occurrences[0].Name != " \n" {
|
||||
if len([]byte(result.CorrectionGuidance)) > 4096 || !utf8.ValidString(result.CorrectionGuidance) || !strings.Contains(result.CorrectionGuidance, "additional issue(s) omitted") {
|
||||
t.Fatalf("CorrectionGuidance = %q, want bounded aggregate", result.CorrectionGuidance)
|
||||
}
|
||||
if value.Occurrences[0].Name != "Item 0" {
|
||||
t.Fatal("Validate() mutated input")
|
||||
}
|
||||
if got := New(Options{}).CheckpointFingerprints(); !reflect.DeepEqual(got, []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}}) {
|
||||
|
||||
Reference in New Issue
Block a user