From 119c54604c361a58d85ccb3c35031d73f8f80fd7 Mon Sep 17 00:00:00 2001 From: Emi Griffith Date: Fri, 24 Jul 2026 14:47:19 -0700 Subject: [PATCH] discord: resolve the city people actually typed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from #general: "@Thermograph Vilnius, Lithuania" came back "I don't track a city called 'Vilnius, Lithuania' yet." Vilnius has been in cities.json the whole time, with country "Lithuania" sitting right there in the record. The resolver matched the query against `name` alone, exact-or-prefix, so the comma form could never match anything and the country field was never consulted. The same message also showed the two neighbouring failures: "Vilinus" (one transposition) fell through to the same refusal, which reads as "we've never heard of Vilnius" rather than "you typed it wrong"; and "Tell me about weather in lithuania" was quoted back verbatim as a city we don't track, which is what made it look broken rather than merely unmatched. So the resolver now goes, in descending order of confidence: exact name honouring a "City, Country/Region" qualifier; the same ignoring an unrecognised qualifier (better to grade Paris and name it in the reply than refuse over "Paris, Wherever"); prefix; close-enough name for typos; and finally a city or country named somewhere inside a sentence, so "weather in lithuania" lands on Vilnius. Accents fold both ways, since people type Zurich and Sao Paulo far more often than Zürich and São Paulo. Free-text matching is whole-word and ignores names under four characters, so ordinary chatter doesn't turn into a weather report -- "how are you today" still resolves to nothing. And the unmatched reply no longer reads a whole sentence back as a place name. This fixes /grade identically; it shared the resolver and the same bug. Claude-Session: https://claude.ai/code/session_015Z1ebLbhUxeZ9ozpNrVTCP --- backend/notifications/discord_interactions.py | 122 ++++++++++++++++-- .../test_discord_interactions.py | 59 +++++++++ 2 files changed, 169 insertions(+), 12 deletions(-) diff --git a/backend/notifications/discord_interactions.py b/backend/notifications/discord_interactions.py index a4935df..a01ed1f 100644 --- a/backend/notifications/discord_interactions.py +++ b/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 - prefix match, preferring the largest (cities.json is population-sorted).""" - q = (query or "").strip().casefold() + """Best curated-city match for whatever someone actually typed. + + 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 - name = c["name"].casefold() - if name == q: - exact = exact or c - elif prefix is None and name.startswith(q): + + # "Vilnius, Lithuania" -> head "vilnius", tail "lithuania". Only the first + # comma splits: "Washington, DC, USA" keeps "dc, usa" as the qualifier, and + # the ISO/admin1 check below still matches it on "dc". + head, _, tail = q.partition(",") + 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. " - "Try a major city name.", "flags": _EPHEMERAL} + # Don't read a whole sentence back as if it were a city name — quoting + # "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. " diff --git a/backend/tests/notifications/test_discord_interactions.py b/backend/tests/notifications/test_discord_interactions.py index d420b41..02305a0 100644 --- a/backend/tests/notifications/test_discord_interactions.py +++ b/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):