Compare commits

...

5 Commits

Author SHA1 Message Date
kami 1bd2acdc2a Do not exempt Russian words that are both noun and verb 2026-07-31 12:55:45 +04:00
kami 15e5dd8eaa Merge the second-person gender check 2026-07-31 12:54:48 +04:00
kami 10cf6f525c Check that nudges do not address the owner in the feminine
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
2026-07-31 12:54:18 +04:00
kami e2210f6844 Merge the clarify-expiry notice 2026-07-31 12:52:36 +04:00
kami 214a4032cf Tell him when an expired clarify question is dropped
Vikunja #382. A parked clarifying question past its TTL was discarded
silently on read; now she says the old request is gone and the newly
spoken words are still routed as a fresh utterance.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
2026-07-31 12:51:47 +04:00
9 changed files with 274 additions and 26 deletions
+9 -7
View File
@@ -105,11 +105,14 @@ that never says food.
## Broken, found, not fixed
1. **`checkFeminine` only catches half the constraint.** It scans for masculine
self-reference and passed 15/15 both runs — but three messages address the *owner* in
the feminine: "ты давно не отдыхал**а**", "он не ел". The owner is a man. The check has
no second-person gender test, so this scores clean while being exactly the persona
failure the constraint exists to prevent. This is the most important gap in the harness.
1. ~~**`checkFeminine` only catches half the constraint.**~~ **Fixed** (#381). It scanned for
masculine self-reference only, so three messages that addressed the *owner* in the feminine
("ты давно не отдыхал**а**") scored clean. There is now a second check, `hisgender`: a
feminine past-tense verb (-ла/-лась) in a sentence addressed to him ("ты", "тебе", "твой")
fails, unless the verb is hers ("я заметила", "напомнила тебе"). It is a suffix rule, not a
parser — see the comment in `checks.go` for what it misses. A fresh 15-case run after adding
it scored **12/15** with `hisgender` 15/15; the model did not repeat the feminine address in
that sample, and the check is pinned by unit tests on the recorded bad strings instead.
2. **Grammar is not checked at all, and it is bad.** `"Он не ел 11 дней"` (it was 11 hours),
`"Сонуждились 7 дней"` (not a word), `"Они забыли воду"` (wrong person entirely). Every
one of these passes all six checks. The fixture measures properties, not fluency, and at
@@ -125,8 +128,7 @@ that never says food.
## Next steps
1. **Add a second-person gender check** to `checks.go`. Finding 1 above. Until it exists the
feminine column means less than it looks like.
1. ~~**Add a second-person gender check**~~ — done, `hisgender` in `checks.go` (#381).
2. **Decide whether the fallback should count as a pass.** Right now `Score` cannot tell a
model answer from a fallback. Either mark fallback bodies in `PhrasedNudge` or count them
in their own column. Without that, any future prompt change can score well by failing
+34
View File
@@ -45,6 +45,40 @@ var clarifyQuestions = map[dialogue.Slot]string{
// landed. Feminine self-reference ("поняла"), as everywhere.
const clarifyGaveUp = "Прости, я не поняла. Скажи, пожалуйста, по-другому."
// clarifyExpired — his answer came after the TTL, so the parked request is
// already gone. Same tone as clarifyGaveUp, different reason: too much time
// passed, not "I did not understand". Feminine self-reference ("ждала",
// "отпустила"); he is addressed with a plain imperative.
const clarifyExpired = "Прости, я слишком долго ждала ответа и отпустила прошлую просьбу. Если она ещё нужна, скажи заново."
// clarifyExpiredNotice returns that line when a parked question had just timed
// out, and "" when nothing was parked. Call it right after
// resolveClarifyAnswer: a live question is answered there, an expired one is
// only reported here — the words themselves still go on to be routed fresh.
func (h *reactiveHandler) clarifyExpiredNotice() string {
if h.clarifyStore == nil {
return ""
}
if !h.clarifyStore.TakeExpired(voiceDialogueID, h.now()) {
return ""
}
log.Printf("voice: clarify — parked question expired, telling him and routing the words fresh")
return clarifyExpired
}
// withNotice glues the expiry notice in front of this turn's reply. One turn
// carries one reply on the wire, so the notice cannot be a message of its own —
// but neither the notice nor the fresh answer may be dropped.
func withNotice(notice, reply string) string {
if notice == "" {
return reply
}
if reply == "" {
return notice
}
return notice + " " + reply
}
// missingFor returns the slots a decision still needs, most important first.
// Empty ⇒ there is nothing identifiable to ask about.
func missingFor(dec router.Decision) []dialogue.Slot {
+30
View File
@@ -289,6 +289,36 @@ func TestNoQuestionWhenNothingIsMissing(t *testing.T) {
}
}
// TestClarifyExpiryIsAnnouncedAndWordsStillRoute — his answer lands after the
// TTL: she must say the old request is gone AND still answer the new words.
func TestClarifyExpiryIsAnnouncedAndWordsStillRoute(t *testing.T) {
ctx := context.Background()
h, _, now := newClarifyHandler(t)
emb := router.NewHashEmbedder(1024)
h.embedder = emb
h.router = buildRouter(emb, h.matcher, 0.55, nil)
if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked {
t.Fatal("expected a question")
}
*now = now.Add(clarifyTTL + time.Second)
reply := h.handleText(ctx, "как дела")
if !strings.HasPrefix(reply, clarifyExpired) {
t.Fatalf("expired question must be announced first, got %q", reply)
}
if strings.TrimSpace(strings.TrimPrefix(reply, clarifyExpired)) == "" {
t.Fatalf("the new words must still be answered, got only the notice: %q", reply)
}
if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil {
t.Fatal("the expired question must be gone")
}
// The notice is said once, not on every later utterance.
if reply := h.handleText(ctx, "как дела"); strings.Contains(reply, clarifyExpired) {
t.Fatalf("notice repeated on a later turn: %q", reply)
}
}
// TestNoPendingQuestionFallsThrough — with nothing parked, an utterance routes
// normally.
func TestNoPendingQuestionFallsThrough(t *testing.T) {
+23 -13
View File
@@ -402,9 +402,17 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
return h.reply(ctx, reply, nil)
}
// 1b2. clarify answer — if she asked a question last turn, this utterance is
// its answer, not a fresh command. After the confirm check: a y/n gate is
// armed by her own prompt and is the narrower claim on the utterance.
// 1b2. expired clarify — a question was parked but its TTL ran out, so the
// request behind it is gone. Say that out loud (see clarify.go) and carry
// on: these words are still routed as a fresh utterance below, with the
// notice glued in front of whatever the fresh routing answers. Checked
// BEFORE the answer path: reading a parked question drops an expired one.
expiredNotice := h.clarifyExpiredNotice()
// 1b3. clarify answer — if she asked a live question last turn, this
// utterance is its answer, not a fresh command. After the confirm check: a
// y/n gate is armed by her own prompt and is the narrower claim on the
// utterance.
if reply, handled := h.resolveClarifyAnswer(ctx, text); handled {
return h.reply(ctx, reply, nil)
}
@@ -414,7 +422,7 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// unreliably (it's a command, not a free-form query), so we match it
// before routing. Same pattern as the confirm turn above.
if reply, handled := h.resolveQuietToggle(ctx, text); handled {
return h.reply(ctx, reply, nil)
return h.reply(ctx, withNotice(expiredNotice, reply), nil)
}
// 2. router — classify the utterance.
@@ -423,10 +431,10 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// ErrNoIntents ⇒ classifier unseeded (cold boot). reply with a
// "still warming up" rather than a wire error.
if errors.Is(err, router.ErrNoIntents) {
return h.reply(ctx, "я ещё не понимаю свободную речь — скоро научусь.", nil)
return h.reply(ctx, withNotice(expiredNotice, "я ещё не понимаю свободную речь — скоро научусь."), nil)
}
log.Printf("voice: router error: %v", err)
return h.reply(ctx, "не получилось разобрать команду.", nil)
return h.reply(ctx, withNotice(expiredNotice, "не получилось разобрать команду."), nil)
}
// 2b. dialogue — fill this turn's missing slots from a prior same-intent
@@ -447,7 +455,7 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// stands.
if dec.Clarify {
if question, asked := h.askClarify(dec); asked {
return h.reply(ctx, question, nil)
return h.reply(ctx, withNotice(expiredNotice, question), nil)
}
}
@@ -463,7 +471,7 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// 5. tts — synthesise the reply text; return to the voice server which
// ships it back on the conn.
return h.reply(ctx, replyText, nil)
return h.reply(ctx, withNotice(expiredNotice, replyText), nil)
}
// handleText — the core reactive path without stt/tts: confirm check →
@@ -478,7 +486,9 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
return reply
}
// 1b2. clarify answer — same check as HandlePushToTalk.
// 1b2/1b3. expired clarify then clarify answer — same order and reasons as
// HandlePushToTalk.
expiredNotice := h.clarifyExpiredNotice()
if reply, handled := h.resolveClarifyAnswer(ctx, text); handled {
return reply
}
@@ -487,10 +497,10 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
dec, err := h.router.Route(ctx, text, h.now())
if err != nil {
if errors.Is(err, router.ErrNoIntents) {
return "я ещё не понимаю свободную речь — скоро научусь."
return withNotice(expiredNotice, "я ещё не понимаю свободную речь — скоро научусь.")
}
log.Printf("voice: handleText router error: %v", err)
return "не получилось разобрать команду."
return withNotice(expiredNotice, "не получилось разобрать команду.")
}
log.Printf("voice: route result: intent=%s slots=%+v", dec.Intent, dec.Slots)
@@ -507,7 +517,7 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
// 2c. clarify — same as HandlePushToTalk: ask about the one missing thing.
if dec.Clarify {
if question, asked := h.askClarify(dec); asked {
return question
return withNotice(expiredNotice, question)
}
}
@@ -519,7 +529,7 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
if replyText == "" {
replyText = h.replier.Reply(dec)
}
return replyText
return withNotice(expiredNotice, replyText)
}
// applyAction — executes the router's Decision. Intent-by-intent:
+15
View File
@@ -100,6 +100,21 @@ func (s *ClarifyStore) Get(id string, now time.Time) *PendingQuestion {
return q
}
// TakeExpired reports whether a question was parked here but its TTL ran out,
// and drops it. Get drops such a question silently, which leaves the user
// thinking his request is still alive — the caller uses this to tell him it is
// gone before treating his words as a fresh utterance.
func (s *ClarifyStore) TakeExpired(id string, now time.Time) bool {
s.mu.Lock()
defer s.mu.Unlock()
q, ok := s.questions[id]
if !ok || !q.IsExpired(now) {
return false
}
delete(s.questions, id)
return true
}
func (s *ClarifyStore) Delete(id string) {
s.mu.Lock()
delete(s.questions, id)
+22
View File
@@ -60,6 +60,28 @@ func TestClarifyStoreGetPutDelete(t *testing.T) {
}
}
// TestClarifyStoreTakeExpired — TakeExpired reports (and drops) only a question
// whose TTL ran out.
func TestClarifyStoreTakeExpired(t *testing.T) {
s := NewClarifyStore(time.Minute)
if s.TakeExpired("voice", base) {
t.Fatal("nothing parked ⇒ nothing expired")
}
s.Put("voice", &PendingQuestion{Missing: []Slot{SlotTime}, Asked: base, TTL: time.Minute})
if s.TakeExpired("voice", base.Add(30*time.Second)) {
t.Fatal("a live question must not report as expired")
}
if s.Get("voice", base.Add(30*time.Second)) == nil {
t.Fatal("a live question must survive TakeExpired")
}
if !s.TakeExpired("voice", base.Add(2*time.Minute)) {
t.Fatal("a stale question must report as expired")
}
if s.TakeExpired("voice", base.Add(2*time.Minute)) {
t.Fatal("TakeExpired must drop the question, so the second call is false")
}
}
func TestNewClarifyStoreDefaultTTL(t *testing.T) {
s := NewClarifyStore(0)
q := &PendingQuestion{Asked: base}
+127 -1
View File
@@ -17,10 +17,14 @@ const (
CheckFeminine = "feminine" // her self-reference is feminine (hard constraint)
CheckCringe = "cringe" // DESIGN.md § Non-goals, "not a relationship"
CheckOnTopic = "ontopic" // says the thing the rule is about
// CheckHisGender — the other half of the persona rule: SHE is feminine, HE
// is male. "ты давно не отдыхала" addresses the operator as a woman.
CheckHisGender = "hisgender"
)
// CheckNames — report order.
var CheckNames = []string{CheckMood, CheckLang, CheckLength, CheckFeminine, CheckCringe, CheckOnTopic}
var CheckNames = []string{CheckMood, CheckLang, CheckLength, CheckFeminine, CheckHisGender, CheckCringe, CheckOnTopic}
// Result — one check on one message.
type Result struct {
@@ -52,6 +56,7 @@ func RunChecks(c Case, body, mood string) []Result {
checkLang(body),
checkLength(body),
checkFeminine(body),
checkHisGender(body),
checkCringe(body),
checkOnTopic(c, body),
}
@@ -176,6 +181,127 @@ func checkFeminine(body string) Result {
return Result{CheckFeminine, true, ""}
}
// --- he is male ----------------------------------------------------------
//
// The mirror of checkFeminine, and the failure it was written for: the model
// wrote "ты давно не отдыхала", which addresses the operator as a woman. That
// scored clean, because checkFeminine only ever looks at how SHE speaks about
// herself.
//
// How it works: Russian past tense is gendered by suffix, -л (m) / -ла (f). So
// this looks for feminine past-tense words in a sentence that also talks TO him
// ("ты", "тебя", "тебе", "твой", …). A feminine verb that belongs to her ("я
// заметила", "напомнила тебе") is skipped — that one is correct.
//
// Honest about the limits: this is a suffix rule, not a parser.
// - False positives: a feminine noun can be the subject in the same sentence
// ("зарядка была утром, ты её пропустил"). The guard below skips a verb whose
// previous word looks like a feminine noun, which helps but will not always
// be right.
// - False negatives: gender also shows up outside the past tense (short
// adjectives, "сама"), and none of that is checked here.
//
// That is acceptable for an eval check. It is a signal to read the message, not
// a grammar verdict, and every hit prints the word it tripped on so a human can
// disagree.
// hisMarkers — words that mean the sentence is addressed to him.
var hisMarkers = map[string]bool{
"ты": true, "тебя": true, "тебе": true, "тобой": true, "тобою": true,
"твой": true, "твоя": true, "твоё": true, "твое": true, "твои": true, "твою": true,
}
// notFeminineVerb — ordinary words ending in "-ла" that are not verbs. Small on
// purpose: it only has to cover words a nudge might actually use.
//
// Words that are both a noun and a verb are deliberately NOT here. "села",
// "мыла" and "стекла" are nouns on paper, but in a nudge they are almost always
// verbs ("ты села", "ты мыла"), and listing them would make the check miss the
// exact thing it is for. Missing a real hit is worse than one false alarm.
var notFeminineVerb = map[string]bool{
"школа": true, "скала": true, "игла": true, "метла": true, "смола": true,
"дела": true, "тела": true, "масла": true, "весла": true,
"зола": true, "пчела": true, "числа": true,
}
// femininePast reports whether a word looks like a feminine past-tense verb:
// "отдыхала", "поела", "выспалась".
func femininePast(w string) bool {
if len([]rune(w)) < 3 || notFeminineVerb[w] {
return false
}
return strings.HasSuffix(w, "ла") || strings.HasSuffix(w, "лась")
}
// looksFeminineNoun — a crude guard against "зарядка была": a word right before
// the verb that ends in "а"/"я" and is not itself a verb is probably the subject.
func looksFeminineNoun(w string) bool {
if femininePast(w) || len([]rune(w)) < 3 {
return false
}
return strings.HasSuffix(w, "а") || strings.HasSuffix(w, "я")
}
// sentenceRE splits on sentence-ending punctuation, so a feminine verb in one
// sentence is not blamed on a "ты" in the next.
var sentenceRE = regexp.MustCompile(`[.!?;…]+`)
func checkHisGender(body string) Result {
for _, sentence := range sentenceRE.Split(strings.ToLower(body), -1) {
words := wordRE.FindAllString(sentence, -1)
addressed := false
for _, w := range words {
if hisMarkers[w] {
addressed = true
}
}
if !addressed {
continue
}
for i, w := range words {
if !femininePast(w) || hersNotHis(words, i) {
continue
}
if i > 0 && looksFeminineNoun(prevWord(words, i)) {
continue
}
return Result{CheckHisGender, false,
fmt.Sprintf("feminine %q addressed to him — he is male", w)}
}
}
return Result{CheckHisGender, true, ""}
}
// hersNotHis — the verb is Maven's own if "я" comes shortly before it, or if the
// thing she did was done to him ("напомнила тебе", "проверила за тебя").
func hersNotHis(words []string, i int) bool {
for j := i - 1; j >= 0 && j >= i-3; j-- {
if words[j] == "я" {
return true
}
}
if i+1 < len(words) {
switch words[i+1] {
case "тебе", "тебя", "за", "тобой":
return true
}
}
return false
}
// prevWord — the word before i, skipping "не" and punctuation, so "не отдыхала"
// still sees the subject.
func prevWord(words []string, i int) string {
for j := i - 1; j >= 0; j-- {
w := words[j]
if w == "не" || w == "ни" || !unicode.Is(unicode.Cyrillic, []rune(w)[0]) {
continue
}
return w
}
return ""
}
// --- the cringe checks ---------------------------------------------------
//
// "Think Jarvis without the cringe part". DESIGN.md § Non-goals: "Not a
+1 -1
View File
@@ -281,7 +281,7 @@ func (r Report) String() string {
fmt.Fprintf(&b, "%s: %d/%d cases pass every check (%.1f%%), %d errors\n",
r.Name, r.Passed, r.Total, 100*r.Accuracy(), r.Errors)
for _, name := range CheckNames {
fmt.Fprintf(&b, " %-9s %d/%d\n", name, r.ByCheck[name], r.Total)
fmt.Fprintf(&b, " %-10s %d/%d\n", name, r.ByCheck[name], r.Total)
}
fmt.Fprintf(&b, " latency: p50 %s p95 %s max %s\n", r.P50, r.P95, r.Max)
fmt.Fprintf(&b, " by rule: %s\n", renderStats(r.ByRule))
+13 -4
View File
@@ -72,10 +72,11 @@ func TestStubBaseline(t *testing.T) {
// ceiling ("you've been at your desk for 4 hours without a break — step
// away for a bit." is 76 chars but 16 words). Left failing rather than
// raising the ceiling to hide it.
CheckLength: 12,
CheckFeminine: 15,
CheckCringe: 15,
CheckOnTopic: 12,
CheckLength: 12,
CheckFeminine: 15,
CheckHisGender: 15,
CheckCringe: 15,
CheckOnTopic: 12,
}
for name, floor := range floors {
if rep.ByCheck[name] < floor {
@@ -104,6 +105,14 @@ func TestChecksCatchWhatTheyClaim(t *testing.T) {
{"masculine predicative", "я должен сказать: попей воды.", CheckFeminine},
// The other direction: HE is male, so second-person masculine is right.
{"second person masculine ok", "ты не пил воду четыре часа.", ""},
// The real observed failure: she addressed him as a woman.
{"feminine second person", "ты давно не отдыхала — попей воды.", CheckHisGender},
{"feminine second person no dash", "ты пила воду четыре часа назад.", CheckHisGender},
// Her own feminine verb next to "ты" is correct and must not be flagged.
{"her feminine verb near ты", "я заметила, что ты не пил воду.", ""},
{"her feminine verb about him", "напомнила тебе про воду.", ""},
// A feminine noun subject in the same sentence is not him.
{"feminine noun subject ok", "зарядка была утром, ты её пропустил, попей воды.", ""},
{"feminine self ok", "я заметила: воды не было четыре часа.", ""},
{"pet name", "милый, попей воды.", CheckCringe},
{"emoji", "попей воды 💧", CheckCringe},