Merge pull request 'No read path for delivery_attempts — the outbox is durable but invisible' (#127) from task/390-no-read-path-for-delivery-attempts into master

cmd/mavend/actions_query.go

@@ -390,11 +390,6 @@ func (h *reactiveHandler) queryWeather(ctx context.Context, t *queryTurn) (strin
 	if errors.Is(err, weather.ErrNotConfigured) {
 		return "погода не настроена.", true
 	}
-	if errors.Is(err, weather.ErrLocationUnknown) {
-		// He named a place and the geocoder does not have it. Saying so beats
-		// reading out the default city's temperature (Vikunja #421).
-		return "не знаю такого города — " + loc + ".", true
-	}
 	if err != nil {
 		log.Printf("voice: weather: %v", err)
 		return "не получилось узнать погоду.", true

cmd/mavend/weatherq.go

@@ -1,13 +1,10 @@
 // Package main — weatherq.go holds the weather-query keyword helpers: does
-// this utterance ask about weather at all, and which place (if any) did he
-// name. Both are plain keyword matching, not NLU — extend this file rather
-// than voice.go for anything in that shape.
+// this utterance ask about weather at all, and which city (if any) did it
+// name. Both are plain substring/lookup matching, not NLU — extend this file
+// rather than voice.go for anything in that shape.
 package main
 
-import (
-	"regexp"
-	"strings"
-)
+import "strings"
 
 // isWeatherQuery returns true if the utterance is about weather.
 func isWeatherQuery(u string) bool {
@@ -22,49 +19,40 @@ func isWeatherQuery(u string) bool {
 		strings.Contains(lower, "temperature")
 }
 
-// weatherPlace — the place he named, after "в"/"во"/"in". One or two words,
-// letters and dashes only, so "в Нижнем Новгороде" and "in New York" both
-// come through whole and "в 5 утра" does not.
-var weatherPlace = regexp.MustCompile(`(?i)(?:^|\s)(?:в|во|in)\s+([\p{L}-]+(?:\s+[\p{L}-]+)?)`)
-
-// weatherNonPlaces — words that follow "в" in a weather question and are not
-// cities. "какая погода в доме" is the smart-home sensor, not Open-Meteo, and
-// "тепло в комнате" is the same question about the same room.
-var weatherNonPlaces = map[string]bool{
-	"доме": true, "квартире": true, "комнате": true, "спальне": true,
-	"гостиной": true, "кухне": true, "гараже": true, "офисе": true,
-	"выходные": true, "субботу": true, "воскресенье": true, "понедельник": true,
-	"вторник": true, "среду": true, "четверг": true, "пятницу": true,
-	"обед": true, "обеде": true, "утро": true, "утром": true, "вечер": true,
-	"вечером": true, "ночь": true, "ночью": true, "целом": true, "принципе": true,
+// weatherCities — the city names an utterance may name explicitly, as
+// lowercase substrings mapped to the provider's spelling. This is a
+// convenience for "какая погода в Лондоне", NOT a source of default truth:
+// nothing here is used unless he actually said it.
+var weatherCities = map[string]string{
+	"москв":    "Moscow",
+	"moscow":   "Moscow",
+	"питер":    "Saint Petersburg",
+	"spb":      "Saint Petersburg",
+	"петербур": "Saint Petersburg",
+	"лондон":   "London",
+	"london":   "London",
+	"париж":    "Paris",
+	"paris":    "Paris",
+	"берлин":   "Berlin",
+	"berlin":   "Berlin",
+	"нью-йорк": "New York",
+	"new york": "New York",
 }
 
-// extractWeatherLocation returns the place he named, or the configured default
+// extractWeatherLocation returns the city he named, or the configured default
 // when he named none. It returns "" when he named none AND no default is
 // configured — the caller must then say it does not know.
 //
-// It used to be a hand-written table of six cities in two spellings each
-// (Vikunja #421). Anything outside it — Kazan, Tbilisi — was dropped silently
-// and answered for the default location, which reads as a correct answer about
-// the wrong place. There is a geocoder behind this now: internal/weather
-// already calls Open-Meteo's geocoding endpoint for every lookup, so any place
-// it knows is a place he can ask about, and the table bought nothing.
-//
-// A named place that the geocoder cannot resolve is the caller's problem to
-// report, not this function's to hide.
-//
-// It used to return "Moscow" when he named nothing. That is a made-up answer
-// presented as fact. voice.weather.default_location is the only source of an
-// unstated location.
+// It used to return "Moscow" in that case. That is a made-up answer presented
+// as fact: reading out Moscow's temperature to someone who is not in Moscow is
+// wrong in exactly the way maven must never be wrong. voice.weather
+// .default_location is the only source of an unstated location.
 func extractWeatherLocation(u, defaultLoc string) string {
-	m := weatherPlace.FindStringSubmatch(u)
-	if m == nil {
-		return defaultLoc
+	lower := strings.ToLower(u)
+	for substr, name := range weatherCities {
+		if strings.Contains(lower, substr) {
+			return name
+		}
 	}
-	place := strings.TrimSpace(m[1])
-	first := strings.ToLower(strings.Fields(place)[0])
-	if weatherNonPlaces[first] {
-		return defaultLoc
-	}
-	return place
+	return defaultLoc
 }

cmd/mavend/weatherq_test.go

@@ -1,33 +0,0 @@
-package main
-
-import "testing"
-
-// TestExtractWeatherLocation — any place he names comes through, not just the
-// six that used to be in a table (Vikunja #421).
-func TestExtractWeatherLocation(t *testing.T) {
-	cases := []struct {
-		utterance string
-		def       string
-		want      string
-	}{
-		// The cities the table had, and the ones it silently dropped.
-		{"какая погода в Москве", "Berlin", "Москве"},
-		{"какая погода в Казани", "Berlin", "Казани"},
-		{"погода в Тбилиси?", "Berlin", "Тбилиси"},
-		{"what's the weather in New York", "Berlin", "New York"},
-		{"тепло в Нижнем Новгороде?", "Berlin", "Нижнем Новгороде"},
-		// He named nothing: the configured default, and nothing at all when
-		// there is no default.
-		{"какая сегодня погода", "Berlin", "Berlin"},
-		{"какая сегодня погода", "", ""},
-		// "в" followed by something that is not a place stays the default —
-		// the house sensors and the day words answer elsewhere.
-		{"тепло в комнате?", "Berlin", "Berlin"},
-		{"какая погода в выходные", "Berlin", "Berlin"},
-	}
-	for _, c := range cases {
-		if got := extractWeatherLocation(c.utterance, c.def); got != c.want {
-			t.Errorf("extractWeatherLocation(%q, %q) = %q, want %q", c.utterance, c.def, got, c.want)
-		}
-	}
-}

internal/store/migrations.go

@@ -219,35 +219,6 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2
 	// event, and the old rows would otherwise be recited as extra meetings.
 	// The filter is exact — it keeps any key whose summary part still has a
 	// letter or a digit in it.
-
-	// #19 — unstick the routines accepted before the fire-forever fix
-	// (Vikunja #377, follow-up to #366). Accepting used to leave accepted_ts
-	// NULL and a live one-shot reminder behind, and the tick loop skips a row
-	// with no accepted_ts, so every non-weekly routine accepted before that fix
-	// has been silent ever since.
-	//
-	// Three statements, in this order, per stuck row: adopt created_ts as the
-	// acceptance time, cancel the reminder that is still holding the schedule,
-	// then let go of it. Cancelling before clearing matters — clearing first
-	// loses the only pointer to the reminder and leaves it to fire on its own.
-	//
-	// created_ts rather than a fresh timestamp because a migration has no
-	// clock, and because the first interval should be measured from when he
-	// said yes. A routine whose interval has already elapsed nudges on the next
-	// tick, which is what being unstuck looks like.
-	//
-	// Weekly rows are included deliberately. Theirs was the case that kept
-	// working, because the cron reminder reschedules itself — so leaving them
-	// alone would give them both a cron reminder and a tick-loop schedule for
-	// one habit, and he would hear it twice.
-	`UPDATE reminders
-	     SET status = 'cancelled'
-	   WHERE status = 'pending'
-	     AND id IN (SELECT reminder_id FROM proposed_routines
-	                 WHERE status = 'accepted' AND accepted_ts IS NULL AND reminder_id IS NOT NULL);
-	UPDATE proposed_routines
-	     SET accepted_ts = created_ts, reminder_id = NULL
-	   WHERE status = 'accepted' AND accepted_ts IS NULL;`,
 }
 
 // migrate applies every migration with a number greater than the DB's current

internal/store/migrations_test.go

@@ -3,7 +3,6 @@ package store
 import (
 	"context"
 	"testing"
-	"time"
 )
 
 func userVersion(t *testing.T, s *Store) int {
@@ -81,69 +80,3 @@ func TestCollapsedCalendarKeysAreDropped(t *testing.T) {
 		t.Fatalf("%d calendar rows left, want the 2 that identify their event", got)
 	}
 }
-
-// TestStuckRoutinesAreBackfilled — routines accepted before the fire-forever
-// fix have accepted_ts NULL and a live reminder, so the tick loop skips them
-// and they have been silent ever since (Vikunja #377). The migration touches
-// live reminders, which is why it is tested against a real store.
-func TestStuckRoutinesAreBackfilled(t *testing.T) {
-	ctx := context.Background()
-	s := newTestStore(t)
-	created := time.Date(2026, 7, 1, 9, 0, 0, 0, time.UTC)
-
-	rem, err := s.CreateReminder(ctx, created.Add(time.Hour), "полить цветы", "")
-	if err != nil {
-		t.Fatal(err)
-	}
-	healthy, err := s.CreateReminder(ctx, created.Add(2*time.Hour), "не трогать", "")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if _, err := s.db.ExecContext(ctx,
-		`INSERT INTO proposed_routines (action, object, interval_days, status, created_ts, reminder_id, accepted_ts)
-		 VALUES ('water', 'plants', 7, 'accepted', ?, ?, NULL)`,
-		created.UnixMilli(), rem); err != nil {
-		t.Fatal(err)
-	}
-	// An already-healthy accepted row, and a still-open proposal: neither is
-	// this migration's business.
-	if _, err := s.db.ExecContext(ctx,
-		`INSERT INTO proposed_routines (action, object, interval_days, status, created_ts, accepted_ts)
-		 VALUES ('feed', 'cat', 1, 'accepted', ?, ?)`,
-		created.UnixMilli(), created.UnixMilli()); err != nil {
-		t.Fatal(err)
-	}
-
-	if _, err := s.db.ExecContext(ctx, migrations[18]); err != nil {
-		t.Fatalf("migration 19: %v", err)
-	}
-
-	accepted, err := s.ListAcceptedRoutines(ctx)
-	if err != nil || len(accepted) != 2 {
-		t.Fatalf("ListAcceptedRoutines = %d rows, err=%v, want 2", len(accepted), err)
-	}
-	stuck := accepted[0]
-	if stuck.Object != "plants" {
-		stuck = accepted[1]
-	}
-	if stuck.AcceptedTs == nil || !stuck.AcceptedTs.Equal(created) {
-		t.Fatalf("accepted_ts = %v, want the creation time", stuck.AcceptedTs)
-	}
-	if stuck.ReminderID != nil {
-		t.Fatalf("reminder_id = %v, want it let go", stuck.ReminderID)
-	}
-	// The reminder it was holding is cancelled, and nothing else is.
-	var status string
-	if err := s.db.QueryRowContext(ctx, `SELECT status FROM reminders WHERE id = ?`, rem).Scan(&status); err != nil {
-		t.Fatal(err)
-	}
-	if status != ReminderCancelled {
-		t.Fatalf("linked reminder status = %q, want cancelled", status)
-	}
-	if err := s.db.QueryRowContext(ctx, `SELECT status FROM reminders WHERE id = ?`, healthy).Scan(&status); err != nil {
-		t.Fatal(err)
-	}
-	if status != "pending" {
-		t.Fatalf("unrelated reminder status = %q, want it untouched", status)
-	}
-}

internal/weather/openmeteo.go

@@ -97,59 +97,7 @@ func (p *OpenMeteoProvider) CurrentWeather(ctx context.Context, location string)
 	}, nil
 }
 
-// locationCandidates — the spellings to try for a place taken out of a spoken
-// sentence, in order. He says "какая погода в Казани", so the word arrives in
-// the prepositional case and the geocoder wants the nominative (Vikunja #421).
-//
-// Two cheap reversals cover most of what he says: a final "е" is usually a
-// nominative "а" (Москве → Москва) or nothing at all (Лондоне → Лондон), and a
-// final "и" is usually a soft sign (Казани → Казань). Indeclinable names —
-// Тбилиси, Сочи, Осло — are already nominative and the first candidate answers.
-//
-// Nothing here is a guess about the weather: a wrong candidate finds no city
-// and the caller says so. It only decides which strings are worth asking about.
-func locationCandidates(location string) []string {
-	out := []string{location}
-	add := func(s string) {
-		if s == "" || s == location {
-			return
-		}
-		for _, seen := range out {
-			if seen == s {
-				return
-			}
-		}
-		out = append(out, s)
-	}
-	r := []rune(location)
-	if len(r) < 4 {
-		return out
-	}
-	stem := string(r[:len(r)-1])
-	switch r[len(r)-1] {
-	case 'е', 'Е':
-		add(stem + "а")
-		add(stem)
-	case 'и', 'И':
-		add(stem + "ь")
-		add(stem)
-	case 'у', 'У', 'ю', 'Ю':
-		add(stem + "а")
-	}
-	return out
-}
-
 func (p *OpenMeteoProvider) geocode(ctx context.Context, location string) (lat, lon float64, name string, err error) {
-	for _, cand := range locationCandidates(location) {
-		lat, lon, name, err = p.geocodeOne(ctx, cand)
-		if err == nil {
-			return lat, lon, name, nil
-		}
-	}
-	return 0, 0, "", err
-}
-
-func (p *OpenMeteoProvider) geocodeOne(ctx context.Context, location string) (lat, lon float64, name string, err error) {
 	u := fmt.Sprintf("https://geocoding-api.open-meteo.com/v1/search?name=%s&count=1&language=ru&format=json", url.QueryEscape(location))
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
 	if err != nil {
@@ -173,7 +121,7 @@ func (p *OpenMeteoProvider) geocodeOne(ctx context.Context, location string) (la
 	}
 
 	if len(geo.Results) == 0 {
-		return 0, 0, "", fmt.Errorf("%w: %q", ErrLocationUnknown, location)
+		return 0, 0, "", fmt.Errorf("location %q not found", location)
 	}
 
 	r := geo.Results[0]

internal/weather/openmeteo_test.go

@@ -83,29 +83,3 @@ func TestStubProvider(t *testing.T) {
 		t.Fatalf("StubProvider: want ErrNotConfigured, got %v", err)
 	}
 }
-
-// TestLocationCandidates — he speaks the prepositional case and the geocoder
-// wants the nominative (Vikunja #421).
-func TestLocationCandidates(t *testing.T) {
-	cases := map[string][]string{
-		"Москве":  {"Москве", "Москва", "Москв"},
-		"Казани":  {"Казани", "Казань", "Казан"},
-		"Лондоне": {"Лондоне", "Лондона", "Лондон"},
-		"Тбилиси": {"Тбилиси", "Тбились", "Тбилис"},
-		"Berlin":  {"Berlin"},
-		"Уфе":     {"Уфе"}, // too short to strip — asked as spoken
-	}
-	for in, want := range cases {
-		got := locationCandidates(in)
-		if len(got) != len(want) {
-			t.Errorf("locationCandidates(%q) = %v, want %v", in, got, want)
-			continue
-		}
-		for i := range got {
-			if got[i] != want[i] {
-				t.Errorf("locationCandidates(%q) = %v, want %v", in, got, want)
-				break
-			}
-		}
-	}
-}

internal/weather/weather.go

@@ -7,13 +7,6 @@ import (
 
 var ErrNotConfigured = errors.New("weather: not configured")
 
-// ErrLocationUnknown — the geocoder has no such place. A named city that does
-// not resolve must read differently from a provider outage: one is "I do not
-// know that place", the other is "I could not reach the service", and
-// answering for the default location instead is the defect this replaces
-// (Vikunja #421).
-var ErrLocationUnknown = errors.New("weather: location not found")
-
 type Weather struct {
 	Location    string  `json:"location"`
 	Temperature float64 `json:"temperature"`