diff --git a/cmd/mavend/actions_query.go b/cmd/mavend/actions_query.go index c349d2d..9629303 100644 --- a/cmd/mavend/actions_query.go +++ b/cmd/mavend/actions_query.go @@ -390,6 +390,11 @@ 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 diff --git a/cmd/mavend/weatherq.go b/cmd/mavend/weatherq.go index 344e2fa..04f67f9 100644 --- a/cmd/mavend/weatherq.go +++ b/cmd/mavend/weatherq.go @@ -1,10 +1,13 @@ // Package main — weatherq.go holds the weather-query keyword helpers: does -// 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. +// 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. package main -import "strings" +import ( + "regexp" + "strings" +) // isWeatherQuery returns true if the utterance is about weather. func isWeatherQuery(u string) bool { @@ -19,40 +22,49 @@ func isWeatherQuery(u string) bool { strings.Contains(lower, "temperature") } -// 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", +// 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, } -// extractWeatherLocation returns the city he named, or the configured default +// extractWeatherLocation returns the place 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 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. +// 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. func extractWeatherLocation(u, defaultLoc string) string { - lower := strings.ToLower(u) - for substr, name := range weatherCities { - if strings.Contains(lower, substr) { - return name - } + m := weatherPlace.FindStringSubmatch(u) + if m == nil { + return defaultLoc } - return defaultLoc + place := strings.TrimSpace(m[1]) + first := strings.ToLower(strings.Fields(place)[0]) + if weatherNonPlaces[first] { + return defaultLoc + } + return place } diff --git a/cmd/mavend/weatherq_test.go b/cmd/mavend/weatherq_test.go new file mode 100644 index 0000000..b41ca9e --- /dev/null +++ b/cmd/mavend/weatherq_test.go @@ -0,0 +1,33 @@ +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) + } + } +} diff --git a/internal/weather/openmeteo.go b/internal/weather/openmeteo.go index 2bc246e..4eedf2d 100644 --- a/internal/weather/openmeteo.go +++ b/internal/weather/openmeteo.go @@ -97,7 +97,59 @@ 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 { @@ -121,7 +173,7 @@ func (p *OpenMeteoProvider) geocode(ctx context.Context, location string) (lat, } if len(geo.Results) == 0 { - return 0, 0, "", fmt.Errorf("location %q not found", location) + return 0, 0, "", fmt.Errorf("%w: %q", ErrLocationUnknown, location) } r := geo.Results[0] diff --git a/internal/weather/openmeteo_test.go b/internal/weather/openmeteo_test.go index e2c1917..b51aae3 100644 --- a/internal/weather/openmeteo_test.go +++ b/internal/weather/openmeteo_test.go @@ -83,3 +83,29 @@ 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 + } + } + } +} diff --git a/internal/weather/weather.go b/internal/weather/weather.go index 71a2f45..b10e83f 100644 --- a/internal/weather/weather.go +++ b/internal/weather/weather.go @@ -7,6 +7,13 @@ 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"`