discord: resolve the city people actually typed
All checks were successful
PR build (required check) / changes (pull_request) Successful in 6s
secrets-guard / encrypted (pull_request) Successful in 5s
PR build (required check) / build-frontend (pull_request) Has been skipped
shell-lint / shellcheck (pull_request) Successful in 7s
PR build (required check) / validate-observability (pull_request) Successful in 38s
PR build (required check) / build-backend (pull_request) Successful in 1m19s
PR build (required check) / gate (pull_request) Successful in 2s
All checks were successful
PR build (required check) / changes (pull_request) Successful in 6s
secrets-guard / encrypted (pull_request) Successful in 5s
PR build (required check) / build-frontend (pull_request) Has been skipped
shell-lint / shellcheck (pull_request) Successful in 7s
PR build (required check) / validate-observability (pull_request) Successful in 38s
PR build (required check) / build-backend (pull_request) Successful in 1m19s
PR build (required check) / gate (pull_request) Successful in 2s
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
This commit is contained in:
parent
519541c378
commit
1b862aa049
2 changed files with 169 additions and 12 deletions
|
|
@ -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. "
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in a new issue