diff --git a/internal/phraser/eval/address_multi_test.go b/internal/phraser/eval/address_multi_test.go new file mode 100644 index 0000000..378f8f1 --- /dev/null +++ b/internal/phraser/eval/address_multi_test.go @@ -0,0 +1,33 @@ +package eval + +import ( + "strings" + "testing" +) + +// TestAddressReportsEveryBreak — the real reply from a nudge eval run broke in +// two ways at once and the check named only the plural. Both must print: a +// half-reported failure reads as a milder problem than it is. +func TestAddressReportsEveryBreak(t *testing.T) { + body := "Смотрите на его потребление воды." + res := checkAddress(body) + if res.Pass { + t.Fatalf("checkAddress passed %q", body) + } + for _, want := range []string{"смотрите", "его"} { + if !strings.Contains(res.Detail, want) { + t.Errorf("detail %q does not name %q", res.Detail, want) + } + } +} + +// One word repeated is one problem, so the detail must not say it twice. +func TestAddressDeduplicates(t *testing.T) { + res := checkAddress("Вам стоит поесть, вам это нужно.") + if res.Pass { + t.Fatal("expected failure") + } + if n := strings.Count(res.Detail, "formal"); n != 1 { + t.Errorf("detail repeats the same break %d times: %q", n, res.Detail) + } +} diff --git a/internal/phraser/eval/checks.go b/internal/phraser/eval/checks.go index e88a52a..39ee366 100644 --- a/internal/phraser/eval/checks.go +++ b/internal/phraser/eval/checks.go @@ -434,14 +434,26 @@ func looksVerb(w string) bool { func checkAddress(body string) Result { words := addressWordRE.FindAllString(strings.ToLower(body), -1) + // Every break, not just the first. A bad reply usually breaks in more than + // one way at once — "Смотрите на его потребление воды" is a plural imperative + // AND third person about him — and reporting only the first hid the second, + // which made the failure look milder than it was. + var breaks []string + seen := map[string]bool{} + add := func(msg string) { + if seen[msg] { + return // the same word twice in one message is one problem, not two + } + seen[msg] = true + breaks = append(breaks, msg) + } + for i, w := range words { if formalPronouns[w] { - return Result{CheckAddress, false, - fmt.Sprintf("formal %q — she says ты/тебя/тебе", w)} + add(fmt.Sprintf("formal %q — she says ты/тебя/тебе", w)) } if pluralVerb(w) && !(i > 0 && prepositions[words[i-1]]) { - return Result{CheckAddress, false, - fmt.Sprintf("plural imperative %q — she uses the singular", w)} + add(fmt.Sprintf("plural imperative %q — she uses the singular", w)) } } @@ -455,18 +467,26 @@ func checkAddress(body string) Result { if !unicode.Is(unicode.Cyrillic, []rune(p)[0]) && !isLatinWord(p) { continue // punctuation } - if notAnAntecedent[p] || prepositions[p] || thirdPersonHim[p] || looksVerb(p) { + // pluralVerb as well as looksVerb: looksVerb knows the imperative in + // -й/-йте but not the -те plural ("смотрите"), so "Смотрите на его + // потребление воды" counted "смотрите" as the person being talked + // about and the "его" never printed. Third time a verb form has + // blinded this check — if a fourth turns up, the antecedent test + // wants a real morphology table, not another suffix. + if notAnAntecedent[p] || prepositions[p] || thirdPersonHim[p] || looksVerb(p) || pluralVerb(p) { continue } named = true break } if !named { - return Result{CheckAddress, false, - fmt.Sprintf("third person %q with nobody else named — she talks to him, not about him", w)} + add(fmt.Sprintf("third person %q with nobody else named — she talks to him, not about him", w)) } } + if len(breaks) > 0 { + return Result{CheckAddress, false, strings.Join(breaks, " + ")} + } return Result{CheckAddress, true, ""} }