discord: resolve the city people actually typed (#62)

Co-authored-by: admin_emi <admin_emi@noreply.dev.jinemi.com>
Co-committed-by: admin_emi <admin_emi@noreply.dev.jinemi.com>

backend/notifications/discord_interactions.py

@@ -15,8 +15,11 @@ Register the command definitions with scripts/register_discord_commands.py.
 from __future__ import annotations
 
 import datetime
+import difflib
 import json
 import os
+import re
+import unicodedata
 
 from nacl.exceptions import BadSignatureError
 from nacl.signing import VerifyKey
@@ -53,20 +56,109 @@ def verify(signature: str, timestamp: str, body: bytes, public_key: str | None =
         return False
 
 
+def _fold(s: str) -> str:
+    """Casefold and strip accents, so `Zurich` matches `Zürich` and `Sao Paulo`
+    matches `São Paulo`. People type city names without the diacritics far more
+    often than with them."""
+    s = unicodedata.normalize("NFKD", s or "")
+    return "".join(ch for ch in s if not unicodedata.combining(ch)).casefold().strip()
+
+
+def _qualifier_matches(city: dict, tail: str) -> bool:
+    """Does the bit after the comma in "Vilnius, Lithuania" describe this city?
+    Accepts the country, its ISO code, or the admin1 region, since all three are
+    things people write there ("Paris, FR", "Springfield, Illinois")."""
+    if not tail:
+        return True
+    return tail in {
+        _fold(city.get("country", "")),
+        _fold(city.get("country_code", "")),
+        _fold(city.get("admin1", "")),
+    }
+
+
+# Below this length, a name is too collision-prone to look for inside a
+# sentence — real entries like "Of" or "Ube" would match ordinary English.
+_MIN_FREE_TEXT_NAME = 4
+
+
 def _resolve_city(query: str) -> dict | None:
-    """Best curated-city match for a free-text name: exact name first, then a
+    """Best curated-city match for whatever someone actually typed.
-    prefix match, preferring the largest (cities.json is population-sorted)."""
+
-    q = (query or "").strip().casefold()
+    The old version matched the query against `name` alone, exact-or-prefix.
+    That failed the two most natural things to write. "Vilnius, Lithuania" never
+    matched, because the comma form was compared verbatim against a bare name —
+    the country was in the record all along and simply never consulted. And a
+    single transposed letter ("Vilinus") fell straight through to "I don't track
+    a city called …", which reads as *we've never heard of Vilnius* rather than
+    *you typed it wrong*.
+
+    So, in order of how confident each step is:
+      1. exact name, honouring a "City, Country/Region" qualifier;
+      2. the same, ignoring an unrecognised qualifier — better to grade Paris
+         and say so in the reply than to refuse over "Paris, Wherever";
+      3. prefix ("san fran");
+      4. a close-enough name, for typos;
+      5. a city or country named somewhere inside a sentence, so
+         "tell me about weather in lithuania" lands on Vilnius.
+
+    All of it prefers the largest city, since cities.json is population-sorted
+    and the biggest is the likeliest thing meant by an ambiguous name.
+    """
+    q = _fold(query)
     if not q:
         return None
-    exact, prefix = None, None
+
-    for c in cities.all_cities():          # already sorted by population, desc
+    # "Vilnius, Lithuania" -> head "vilnius", tail "lithuania". Only the first
-        name = c["name"].casefold()
+    # comma splits: "Washington, DC, USA" keeps "dc, usa" as the qualifier, and
-        if name == q:
+    # the ISO/admin1 check below still matches it on "dc".
-            exact = exact or c
+    head, _, tail = q.partition(",")
-        elif prefix is None and name.startswith(q):
+    head, tail = head.strip(), tail.strip()
+
+    all_cities = cities.all_cities()       # already sorted by population, desc
+
+    named = None                           # name matched, qualifier did not
+    prefix = None
+    for c in all_cities:
+        name = _fold(c["name"])
+        if name == head:
+            if _qualifier_matches(c, tail):
+                return c
+            named = named or c
+        elif prefix is None and head and name.startswith(head):
             prefix = c
-    return exact or prefix
+    if named:
+        return named
+    if prefix:
+        return prefix
+
+    # Typos. 0.82 is tight enough that "vilinus" reaches "vilnius" while
+    # genuinely different names stay apart; get_close_matches ranks by ratio,
+    # and ties resolve to the more populous city because we scan in order.
+    names = [_fold(c["name"]) for c in all_cities]
+    close = difflib.get_close_matches(head, names, n=1, cutoff=0.82)
+    if close:
+        return all_cities[names.index(close[0])]
+
+    # A sentence, or a country on its own. Look for the longest city name
+    # mentioned in it, then fall back to the largest city of a country named in
+    # it — "weather in lithuania" is a perfectly reasonable thing to ask.
+    words = set(re.findall(r"[a-z0-9]+", q))
+    best, best_len = None, 0
+    for c in all_cities:
+        name = _fold(c["name"])
+        if len(name) < _MIN_FREE_TEXT_NAME or len(name) <= best_len:
+            continue
+        # Whole-word containment, so "Nice" does not fire on "nicely".
+        if set(re.findall(r"[a-z0-9]+", name)) <= words:
+            best, best_len = c, len(name)
+    if best:
+        return best
+    for c in all_cities:
+        country = _fold(c.get("country", ""))
+        if len(country) >= _MIN_FREE_TEXT_NAME and set(re.findall(r"[a-z0-9]+", country)) <= words:
+            return c                       # population-sorted: the largest
+    return None
 
 
 def _grade_message(query: str) -> dict:
@@ -74,8 +166,14 @@ def _grade_message(query: str) -> dict:
     plain ephemeral note when the city is unknown or not yet warmed."""
     city = _resolve_city(query)
     if city is None:
-        return {"content": f"I don't track a city called '{query}' yet. "
+        # Don't read a whole sentence back as if it were a city name — quoting
-                           "Try a major city name.", "flags": _EPHEMERAL}
+        # "Tell me about weather in lithuania" as a place we don't track is what
+        # makes this look broken rather than merely unmatched.
+        looks_like_a_name = len(query) <= 40 and len(query.split()) <= 4
+        detail = f"I don't track a city called '{query}' yet." if looks_like_a_name \
+            else "I couldn't find a city in that."
+        return {"content": f"{detail} Try a city name — \"Vilnius\" or "
+                           "\"Vilnius, Lithuania\" both work.", "flags": _EPHEMERAL}
     card = homepage._grade_city(city, datetime.date.today())
     if card is None:
         return {"content": f"No recent reading is cached for {city['name']} yet. "

backend/tests/notifications/test_discord_interactions.py

@@ -100,6 +100,65 @@ def test_resolve_city_prefers_exact_then_population():
     assert di._resolve_city("Zzznotacity") is None
 
 
+# --- what people actually type -----------------------------------------------
+#
+# Every case below came back "I don't track a city called …" in #general.
+# Vilnius was in cities.json the whole time; the resolver never looked past
+# `name`.
+
+def test_resolve_city_accepts_city_comma_country():
+    # The reported bug: the country sat in the record and was never consulted,
+    # so the most natural way to disambiguate a city always failed.
+    for query in ("Vilnius, Lithuania", "Vilnius, LT", "vilnius,lithuania"):
+        city = di._resolve_city(query)
+        assert city is not None and city["name"] == "Vilnius", query
+
+
+def test_resolve_city_tolerates_a_typo():
+    # "Vilinus" is one transposition from Vilnius. Refusing it reads as "we've
+    # never heard of Vilnius", which is both wrong and off-putting.
+    for query in ("Vilinus", "Vilinus, Lithuania"):
+        city = di._resolve_city(query)
+        assert city is not None and city["name"] == "Vilnius", query
+
+
+def test_resolve_city_finds_a_place_inside_a_sentence():
+    # Mentioning the bot is conversational by nature, so the whole message
+    # arrives as the "city name".
+    city = di._resolve_city("Tell me about weather in lithuania")
+    assert city is not None and city["country"] == "Lithuania"
+    city = di._resolve_city("what's it looking like in Tokyo today")
+    assert city is not None and city["name"] == "Tokyo"
+
+
+def test_resolve_city_ignores_diacritics_either_way():
+    for query in ("Zurich", "Sao Paulo"):
+        assert di._resolve_city(query) is not None, query
+
+
+def test_resolve_city_falls_back_when_the_qualifier_is_unknown():
+    # Better to grade Paris and name it in the reply than refuse over a
+    # qualifier we don't carry.
+    city = di._resolve_city("Paris, Wherever")
+    assert city is not None and city["name"] == "Paris"
+
+
+def test_resolve_city_does_not_match_ordinary_chatter():
+    # The free-text pass must not turn small talk into a weather report.
+    for query in ("how are you today", "what is the weather", "thanks!"):
+        assert di._resolve_city(query) is None, query
+
+
+def test_grade_does_not_quote_a_whole_sentence_back():
+    # Reading a full sentence back as a city we don't track is what made this
+    # look broken rather than merely unmatched.
+    sentence = "please tell me something about the climate of Atlantis the lost place"
+    data = di._grade_message(sentence)
+    assert data["flags"] == di._EPHEMERAL
+    assert sentence not in data["content"]
+    assert "couldn't find a city" in data["content"]
+
+
 # --- the wired FastAPI route -------------------------------------------------
 
 def test_interactions_route_pings_and_rejects(monkeypatch):