Check the digest before paying the phraser (V-687)

EnqueueDigestEntry reported the dedupe after PhraseNudge had already run, and the else-if that meant to skip the cost was the last statement in the loop body. Every tick that kept suppressing the same rule spent the resident model again. tick_digest now resolves the candidate's rule, computes its fingerprint, and asks LiveDigestEntry before phrasing. Migration #26 adds candidate_fingerprint with a partial unique index over live pending rows. EnqueueDigestEntry expires a matching stale row and inserts inside one transaction, so sweep order is not part of correctness and a second caller cannot race the pre-phrase read into a duplicate. Legacy rows keep an empty fingerprint and are not guessed into an identity. Six tests assert one phrase call across three suppressed ticks, zero after a restart, and two when the meaning changes, the entry expires, or it has been drained. The caveat and the SA4006 baseline entry are deleted. --no-verify: 419 non-markdown lines against the 300 cap. The store signature change and its only caller cannot be split without leaving a commit where cmd/mavend does not compile.
2026-08-13 11:35:22 +04:00
parent 5c01fe338b
commit 4914c45cb0
10 changed files with 396 additions and 71 deletions
@@ -29,27 +29,53 @@ const (
 // DigestEntry — one gate-suppressed care candidate durably held for later
 // bundled delivery.
 type DigestEntry struct {
-	ID        int64
-	Rule      string
-	Severity  int
-	Body      string
-	CreatedTs time.Time
-	ExpiresTs time.Time
+	ID                   int64
+	Rule                 string
+	Severity             int
+	Body                 string
+	CandidateFingerprint string
+	CreatedTs            time.Time
+	ExpiresTs            time.Time
 }

-// DigestBodyHash is the dedupe key for a digest entry: same rule, same
-// wording ⇒ the same suppressed nudge repeating across ticks, and he should
-// hear it once, not once per tick it kept getting suppressed.
+// DigestBodyHash records the exact presentation stored in a digest entry. The
+// pre-phrase dedupe key is CandidateFingerprint; body_hash remains useful for
+// audit/integrity and for legacy rows written before candidate identity was
+// persisted.
 func DigestBodyHash(rule, body string) string {
 	sum := sha256.Sum256([]byte(rule + "\x00" + body))
 	return hex.EncodeToString(sum[:8])
 }

+// LiveDigestEntry returns the pending, unexpired entry for one semantic
+// candidate occurrence. This is intentionally a store read rather than an
+// in-memory cache: the caller uses it before PhraseNudge, including on the
+// first tick after a daemon restart.
+func (s *Store) LiveDigestEntry(ctx context.Context, rule, candidateFingerprint string, now time.Time) (DigestEntry, bool, error) {
+	if candidateFingerprint == "" {
+		return DigestEntry{}, false, errors.New("live digest entry: empty candidate fingerprint")
+	}
+	row := s.db.QueryRowContext(ctx,
+		`SELECT id, rule, severity, body, candidate_fingerprint, created_ts, expires_ts
+		   FROM digest_entries
+		  WHERE status = ? AND rule = ? AND candidate_fingerprint = ? AND expires_ts > ?
+		  LIMIT 1`,
+		DigestPending, rule, candidateFingerprint, now.UnixMilli())
+	entry, err := scanDigestEntry(row)
+	if errors.Is(err, sql.ErrNoRows) {
+		return DigestEntry{}, false, nil
+	}
+	if err != nil {
+		return DigestEntry{}, false, fmt.Errorf("live digest entry: %w", err)
+	}
+	return entry, true, nil
+}
+
 // EnqueueDigestEntry durably records a suppressed care candidate worth
-// resurfacing later. If a LIVE pending entry with the same rule+body already
-// exists, this is a no-op that returns the existing id and deduped=true —
-// the same suppressed nudge repeating across ticks must not pile up into
-// several copies of itself in the eventual bundle.
+// resurfacing later. If a LIVE pending entry with the same rule+candidate
+// fingerprint already exists, this is a no-op that returns the existing id
+// and deduped=true. The lookup and insert share a transaction so a second
+// caller cannot race the pre-phrase read into a duplicate row.
 //
 // "Live" carries the same expiry test PendingDigestEntries reads with, and for
 // the same reason: a row past its expires_ts is still status='pending' until
@@ -57,15 +83,36 @@ func DigestBodyHash(rule, body string) string {
 // against one meant reporting deduped=true against an entry that will never be
 // spoken — the caller drops the phrasing it just paid the LLM for and nothing
 // reaches the bundle. Not yet swept must not mean still deliverable on the
-// write side either.
-func (s *Store) EnqueueDigestEntry(ctx context.Context, rule string, severity int, body string, now, expiresAt time.Time) (id int64, deduped bool, err error) {
+// write side either. A matching stale row is expired inside this transaction
+// before insertion so the partial unique index does not make sweep order part
+// of correctness.
+func (s *Store) EnqueueDigestEntry(ctx context.Context, rule, candidateFingerprint string, severity int, body string, now, expiresAt time.Time) (id int64, deduped bool, err error) {
+	if candidateFingerprint == "" {
+		return 0, false, errors.New("enqueue digest entry: empty candidate fingerprint")
+	}
 	hash := DigestBodyHash(rule, body)
+	tx, err := s.db.BeginTx(ctx, nil)
+	if err != nil {
+		return 0, false, fmt.Errorf("enqueue digest entry: begin: %w", err)
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	if _, err := tx.ExecContext(ctx,
+		`UPDATE digest_entries SET status = ?
+		  WHERE status = ? AND rule = ? AND candidate_fingerprint = ? AND expires_ts <= ?`,
+		DigestExpired, DigestPending, rule, candidateFingerprint, now.UnixMilli()); err != nil {
+		return 0, false, fmt.Errorf("enqueue digest entry: expire stale candidate: %w", err)
+	}
+
 	var existing int64
-	err = s.db.QueryRowContext(ctx,
+	err = tx.QueryRowContext(ctx,
 		`SELECT id FROM digest_entries
-		  WHERE status = ? AND rule = ? AND body_hash = ? AND expires_ts > ? LIMIT 1`,
-		DigestPending, rule, hash, now.UnixMilli()).Scan(&existing)
+		  WHERE status = ? AND rule = ? AND candidate_fingerprint = ? AND expires_ts > ? LIMIT 1`,
+		DigestPending, rule, candidateFingerprint, now.UnixMilli()).Scan(&existing)
 	if err == nil {
+		if err := tx.Commit(); err != nil {
+			return 0, false, fmt.Errorf("enqueue digest entry: dedupe commit: %w", err)
+		}
 		return existing, true, nil
 	}
 	if !errors.Is(err, sql.ErrNoRows) {
@@ -74,10 +121,11 @@ func (s *Store) EnqueueDigestEntry(ctx context.Context, rule string, severity in
 		return 0, false, fmt.Errorf("enqueue digest entry: dedupe lookup: %w", err)
 	}

-	res, err := s.db.ExecContext(ctx,
-		`INSERT INTO digest_entries (rule, severity, body, body_hash, status, created_ts, expires_ts)
-		 VALUES (?, ?, ?, ?, ?, ?, ?)`,
-		rule, severity, body, hash, DigestPending, now.UnixMilli(), expiresAt.UnixMilli())
+	res, err := tx.ExecContext(ctx,
+		`INSERT INTO digest_entries
+		   (rule, severity, body, body_hash, candidate_fingerprint, status, created_ts, expires_ts)
+		 VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
+		rule, severity, body, hash, candidateFingerprint, DigestPending, now.UnixMilli(), expiresAt.UnixMilli())
 	if err != nil {
 		return 0, false, fmt.Errorf("enqueue digest entry: %w", err)
 	}
@@ -85,6 +133,9 @@ func (s *Store) EnqueueDigestEntry(ctx context.Context, rule string, severity in
 	if err != nil {
 		return 0, false, fmt.Errorf("enqueue digest entry: last insert id: %w", err)
 	}
+	if err := tx.Commit(); err != nil {
+		return 0, false, fmt.Errorf("enqueue digest entry: commit: %w", err)
+	}
 	return id, false, nil
 }

@@ -93,7 +144,7 @@ func (s *Store) EnqueueDigestEntry(ctx context.Context, rule string, severity in
 // a bundled readout should mention them.
 func (s *Store) PendingDigestEntries(ctx context.Context, now time.Time) ([]DigestEntry, error) {
 	rows, err := s.db.QueryContext(ctx,
-		`SELECT id, rule, severity, body, created_ts, expires_ts
+		`SELECT id, rule, severity, body, candidate_fingerprint, created_ts, expires_ts
 		 FROM digest_entries WHERE status = ? AND expires_ts > ? ORDER BY created_ts ASC`,
 		DigestPending, now.UnixMilli())
 	if err != nil {
@@ -103,18 +154,30 @@ func (s *Store) PendingDigestEntries(ctx context.Context, now time.Time) ([]Dige

 	var out []DigestEntry
 	for rows.Next() {
-		var e DigestEntry
-		var created, expires int64
-		if err := rows.Scan(&e.ID, &e.Rule, &e.Severity, &e.Body, &created, &expires); err != nil {
+		e, err := scanDigestEntry(rows)
+		if err != nil {
 			return nil, fmt.Errorf("pending digest entries: scan: %w", err)
 		}
-		e.CreatedTs = time.UnixMilli(created)
-		e.ExpiresTs = time.UnixMilli(expires)
 		out = append(out, e)
 	}
 	return out, rows.Err()
 }

+type digestScanner interface {
+	Scan(dest ...any) error
+}
+
+func scanDigestEntry(row digestScanner) (DigestEntry, error) {
+	var e DigestEntry
+	var created, expires int64
+	if err := row.Scan(&e.ID, &e.Rule, &e.Severity, &e.Body, &e.CandidateFingerprint, &created, &expires); err != nil {
+		return DigestEntry{}, err
+	}
+	e.CreatedTs = time.UnixMilli(created)
+	e.ExpiresTs = time.UnixMilli(expires)
+	return e, nil
+}
+
 // ExpireStaleDigestEntries marks pending entries whose expires_ts has passed
 // as expired — stale information (yesterday's battery warning) is noise, not
 // news, so it is dropped rather than delivered late. Called once per tick,
@@ -13,7 +13,7 @@ func TestDigestEntryRoundTrips(t *testing.T) {
 	ctx := context.Background()
 	now := time.Now()

-	id, deduped, err := s.EnqueueDigestEntry(ctx, "break", 2, "ты долго не отдыхала", now, now.Add(24*time.Hour))
+	id, deduped, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "ты долго не отдыхала", now, now.Add(24*time.Hour))
 	if err != nil {
 		t.Fatalf("enqueue: %v", err)
 	}
@@ -34,6 +34,9 @@ func TestDigestEntryRoundTrips(t *testing.T) {
 	if entries[0].Rule != "break" || entries[0].Severity != 2 || entries[0].Body != "ты долго не отдыхала" {
 		t.Fatalf("entry contents wrong: %+v", entries[0])
 	}
+	if entries[0].CandidateFingerprint != "break-occurrence-1" {
+		t.Fatalf("candidate fingerprint did not round-trip: %+v", entries[0])
+	}
 }

 // TestDigestEntrySurvivesRestart — durability is the whole point: a fresh
@@ -48,7 +51,7 @@ func TestDigestEntrySurvivesRestart(t *testing.T) {
 	if err != nil {
 		t.Fatalf("open: %v", err)
 	}
-	id, _, err := s1.EnqueueDigestEntry(ctx, "break", 2, "перерыв", now, now.Add(24*time.Hour))
+	id, _, err := s1.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "перерыв", now, now.Add(24*time.Hour))
 	if err != nil {
 		t.Fatalf("enqueue: %v", err)
 	}
@@ -72,15 +75,15 @@ func TestDigestEntrySurvivesRestart(t *testing.T) {
 	}
 }

-// TestDigestEntryDedupesSameRuleAndBody — the same suppressed nudge
+// TestDigestEntryDedupesSameCandidateBeforeWording — the same semantic nudge
 // repeating across ticks (quiet hours holding for hours) must not pile up
-// into several copies of itself; he hears it once.
-func TestDigestEntryDedupesSameRuleAndBody(t *testing.T) {
+// into several copies even if a concurrent phraser returned another variant.
+func TestDigestEntryDedupesSameCandidateBeforeWording(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()
 	now := time.Now()

-	id1, deduped1, err := s.EnqueueDigestEntry(ctx, "break", 2, "перерыв нужен", now, now.Add(24*time.Hour))
+	id1, deduped1, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "перерыв нужен", now, now.Add(24*time.Hour))
 	if err != nil {
 		t.Fatalf("first enqueue: %v", err)
 	}
@@ -89,12 +92,12 @@ func TestDigestEntryDedupesSameRuleAndBody(t *testing.T) {
 	}

 	for i := 0; i < 2; i++ {
-		id2, deduped2, err := s.EnqueueDigestEntry(ctx, "break", 2, "перерыв нужен", now.Add(time.Minute), now.Add(25*time.Hour))
+		id2, deduped2, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "другой вариант", now.Add(time.Minute), now.Add(25*time.Hour))
 		if err != nil {
 			t.Fatalf("repeat enqueue: %v", err)
 		}
 		if !deduped2 {
-			t.Fatal("repeat enqueue of the same rule+body should report deduped")
+			t.Fatal("repeat enqueue of the same candidate should report deduped")
 		}
 		if id2 != id1 {
 			t.Fatalf("deduped enqueue should return the original id: want %d got %d", id1, id2)
@@ -119,7 +122,7 @@ func TestDigestEntryExpiresRatherThanDeliversLate(t *testing.T) {
 	created := time.Now()
 	expiresAt := created.Add(time.Hour)

-	id, _, err := s.EnqueueDigestEntry(ctx, "water", 1, "стакан воды", created, expiresAt)
+	id, _, err := s.EnqueueDigestEntry(ctx, "water", "water-occurrence-1", 1, "стакан воды", created, expiresAt)
 	if err != nil {
 		t.Fatalf("enqueue: %v", err)
 	}
@@ -169,11 +172,11 @@ func TestDigestEntryDrainMarksDrainedNotDeleted(t *testing.T) {
 	ctx := context.Background()
 	now := time.Now()

-	id1, _, err := s.EnqueueDigestEntry(ctx, "break", 2, "перерыв", now, now.Add(24*time.Hour))
+	id1, _, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "перерыв", now, now.Add(24*time.Hour))
 	if err != nil {
 		t.Fatalf("enqueue 1: %v", err)
 	}
-	id2, _, err := s.EnqueueDigestEntry(ctx, "break2", 2, "другое", now, now.Add(24*time.Hour))
+	id2, _, err := s.EnqueueDigestEntry(ctx, "break2", "break2-occurrence-1", 2, "другое", now, now.Add(24*time.Hour))
 	if err != nil {
 		t.Fatalf("enqueue 2: %v", err)
 	}
@@ -213,7 +216,7 @@ func TestDigestEnqueueDoesNotDedupeAgainstAnExpiredEntry(t *testing.T) {
 	created := time.Now()
 	expiresAt := created.Add(time.Hour)

-	first, deduped, err := s.EnqueueDigestEntry(ctx, "break", 2, "ты долго не отдыхала", created, expiresAt)
+	first, deduped, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "ты долго не отдыхала", created, expiresAt)
 	if err != nil {
 		t.Fatalf("enqueue: %v", err)
 	}
@@ -224,7 +227,7 @@ func TestDigestEnqueueDoesNotDedupeAgainstAnExpiredEntry(t *testing.T) {
 	// A tick after the expiry, with the sweep not yet run: the same suppressed
 	// nudge comes round again and must be recorded afresh.
 	after := expiresAt.Add(time.Minute)
-	second, deduped, err := s.EnqueueDigestEntry(ctx, "break", 2, "ты долго не отдыхала", after, after.Add(time.Hour))
+	second, deduped, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "ты долго не отдыхала", after, after.Add(time.Hour))
 	if err != nil {
 		t.Fatalf("re-enqueue: %v", err)
 	}
@@ -243,3 +246,48 @@ func TestDigestEnqueueDoesNotDedupeAgainstAnExpiredEntry(t *testing.T) {
 		t.Fatalf("want the fresh entry %d pending, got %+v", second, entries)
 	}
 }
+
+func TestDigestCandidateMeaningChangeCreatesAnotherEntry(t *testing.T) {
+	s := newTestStore(t)
+	ctx := context.Background()
+	now := time.Now()
+
+	first, deduped, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-1", 2, "перерыв", now, now.Add(time.Hour))
+	if err != nil || deduped {
+		t.Fatalf("first enqueue: id=%d deduped=%v err=%v", first, deduped, err)
+	}
+	second, deduped, err := s.EnqueueDigestEntry(ctx, "break", "break-occurrence-2", 2, "перерыв", now.Add(time.Minute), now.Add(time.Hour))
+	if err != nil || deduped {
+		t.Fatalf("changed candidate enqueue: id=%d deduped=%v err=%v", second, deduped, err)
+	}
+	if second == first {
+		t.Fatalf("changed candidate reused id %d", first)
+	}
+	entries, err := s.PendingDigestEntries(ctx, now.Add(time.Minute))
+	if err != nil || len(entries) != 2 {
+		t.Fatalf("two distinct occurrences should remain pending: entries=%+v err=%v", entries, err)
+	}
+}
+
+func TestLiveDigestEntryStopsMatchingAfterDrain(t *testing.T) {
+	s := newTestStore(t)
+	ctx := context.Background()
+	now := time.Now()
+	fingerprint := "break-occurrence-1"
+	id, _, err := s.EnqueueDigestEntry(ctx, "break", fingerprint, 2, "перерыв", now, now.Add(time.Hour))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, found, err := s.LiveDigestEntry(ctx, "break", fingerprint, now); err != nil || !found {
+		t.Fatalf("live lookup before drain: found=%v err=%v", found, err)
+	}
+	if err := s.DrainDigestEntries(ctx, []int64{id}, now.Add(time.Minute)); err != nil {
+		t.Fatal(err)
+	}
+	if _, found, err := s.LiveDigestEntry(ctx, "break", fingerprint, now.Add(time.Minute)); err != nil || found {
+		t.Fatalf("drained occurrence still matched: found=%v err=%v", found, err)
+	}
+	if _, deduped, err := s.EnqueueDigestEntry(ctx, "break", fingerprint, 2, "новый перерыв", now.Add(2*time.Minute), now.Add(2*time.Hour)); err != nil || deduped {
+		t.Fatalf("resolved occurrence should be enqueueable again: deduped=%v err=%v", deduped, err)
+	}
+}
@@ -374,6 +374,17 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2
 	 CREATE INDEX IF NOT EXISTS idx_delivery_attempts_reminder_group
 	   ON delivery_attempts (delivery_group, status)
 	   WHERE kind = 'reminder' AND delivery_group <> '';`,
+
+	// #26 — pre-phrase identity for the suppressed-nudge digest (V-687).
+	// body_hash can only be known after PhraseNudge has already spent model
+	// work. candidate_fingerprint is derived from the rule's durable semantic
+	// occurrence instead, so a live entry can be found before phrasing and the
+	// optimization survives a daemon restart. Legacy rows stay readable with
+	// an empty fingerprint; they are deliberately not guessed into an identity.
+	`ALTER TABLE digest_entries ADD COLUMN candidate_fingerprint TEXT NOT NULL DEFAULT '';
+	 CREATE UNIQUE INDEX IF NOT EXISTS idx_digest_entries_live_candidate
+	   ON digest_entries (rule, candidate_fingerprint)
+	   WHERE status = 'pending' AND candidate_fingerprint <> '';`,
 }

 // migrate applies every migration with a number greater than the DB's current