feat(weather): romanise non-Latin place names before geocoding
Open-Meteo's geocoder only matches Latin/English spellings, so a Korean city
name like "서울" returns zero results even though the place exists. With
OUTPUT_LANGUAGE locked to Korean the tool-calling model naturally fills
`location` with the Korean name, which dead-ended on every weather request
("could not find location").
When the first geocode is empty and the name is non-ASCII, ask the warm small
model for the common English exonym ("서울" -> "Seoul") and retry once. ASCII
names skip the round-trip entirely.
Live-verified: "서울 날씨 알려줘" now returns real Seoul weather. Tests cover the
romanise-and-retry path and the ASCII short-circuit.
This commit is contained in:
javis-bot
@@ -83,6 +83,65 @@ def _extract_place_from_user_text(text: str, cfg) -> Optional[str]:
|
||||
return place
|
||||
|
||||
|
||||
def _romanise_place(name: str, cfg) -> Optional[str]:
|
||||
"""Romanise a non-Latin place name for the geocoder.
|
||||
|
||||
Open-Meteo's geocoding API only matches Latin/English spellings, so a
|
||||
Korean (or other non-Latin) city name like ``서울`` returns zero results
|
||||
even though the place plainly exists. When OUTPUT_LANGUAGE locks replies to
|
||||
Korean the tool-calling model naturally fills ``location`` with the Korean
|
||||
name, which would otherwise dead-end. Ask the (already warm) small model for
|
||||
the common English exonym so geocoding can succeed on a retry.
|
||||
|
||||
Returns the romanised name, or ``None`` if it is unavailable, unchanged, or
|
||||
doesn't look like a place.
|
||||
"""
|
||||
if not isinstance(name, str) or not name.strip() or name.isascii():
|
||||
return None
|
||||
if cfg is None:
|
||||
return None
|
||||
|
||||
model = (
|
||||
getattr(cfg, "tool_router_model", "")
|
||||
or getattr(cfg, "intent_judge_model", "")
|
||||
or getattr(cfg, "ollama_chat_model", "")
|
||||
)
|
||||
base_url = getattr(cfg, "ollama_base_url", "")
|
||||
if not model or not base_url:
|
||||
return None
|
||||
|
||||
try:
|
||||
from ...llm import call_llm_direct
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
sys_prompt = (
|
||||
"You romanise place names for a geocoding API that only understands "
|
||||
"English/Latin spellings. Reply with ONLY the common English name of "
|
||||
"the place, no punctuation, quotes, or explanation. "
|
||||
"Examples: 서울 -> Seoul, 도쿄 -> Tokyo, 뮌헨 -> Munich."
|
||||
)
|
||||
user_prompt = f"Place: {name}\n\nEnglish name:"
|
||||
|
||||
try:
|
||||
resp = call_llm_direct(
|
||||
base_url, model, sys_prompt, user_prompt,
|
||||
timeout_sec=float(getattr(cfg, "llm_tools_timeout_sec", 8.0)),
|
||||
)
|
||||
except Exception as e:
|
||||
debug_log(f" ⚠️ place romanisation failed: {e}", "tools")
|
||||
return None
|
||||
|
||||
if not resp or not isinstance(resp, str):
|
||||
return None
|
||||
out = resp.strip().strip("'\"`*.,:;!?()[]{}<>").split("\n", 1)[0].strip()
|
||||
if not out or out.lower() in _NO_PLACE_SENTINELS:
|
||||
return None
|
||||
if len(out) > 60 or len(out.split()) > 5 or out == name:
|
||||
return None
|
||||
return out
|
||||
|
||||
|
||||
# WMO Weather interpretation codes
|
||||
# https://open-meteo.com/en/docs
|
||||
WMO_CODES = {
|
||||
@@ -274,6 +333,23 @@ class WeatherTool(Tool):
|
||||
geo_response.raise_for_status()
|
||||
geo_data = geo_response.json()
|
||||
|
||||
# Open-Meteo only matches Latin spellings, so a non-Latin name
|
||||
# (e.g. Korean "서울") returns nothing. Retry once with an
|
||||
# LLM-romanised name before giving up.
|
||||
if not geo_data.get("results"):
|
||||
romanised = _romanise_place(location_str, getattr(context, "cfg", None))
|
||||
if romanised:
|
||||
debug_log(
|
||||
f" 🌤️ geocode empty for '{location_str}'; retrying romanised '{romanised}'",
|
||||
"tools",
|
||||
)
|
||||
geocode_params["name"] = romanised
|
||||
geo_response = requests.get(geocode_url, params=geocode_params, timeout=10)
|
||||
geo_response.raise_for_status()
|
||||
geo_data = geo_response.json()
|
||||
if geo_data.get("results"):
|
||||
location_str = romanised
|
||||
|
||||
if not geo_data.get("results"):
|
||||
return ToolExecutionResult(
|
||||
success=False,
|
||||
|
||||
@@ -8,6 +8,7 @@ from src.jarvis.tools.builtin.weather import (
|
||||
WeatherTool,
|
||||
WMO_CODES,
|
||||
_extract_place_from_user_text,
|
||||
_romanise_place,
|
||||
)
|
||||
from src.jarvis.tools.base import ToolContext
|
||||
from src.jarvis.tools.types import ToolExecutionResult
|
||||
@@ -100,6 +101,77 @@ class TestWeatherTool:
|
||||
assert "7-day forecast" in result.reply_text
|
||||
self.context.user_print.assert_called()
|
||||
|
||||
@patch('src.jarvis.tools.builtin.weather._romanise_place')
|
||||
@patch('requests.get')
|
||||
def test_run_non_latin_location_romanises_and_retries(self, mock_get, mock_romanise):
|
||||
"""A Korean city name Open-Meteo can't geocode is romanised and retried.
|
||||
|
||||
Open-Meteo's geocoder only matches Latin spellings, so ``서울`` returns
|
||||
no results. The tool must romanise to ``Seoul`` and retry rather than
|
||||
dead-ending with "could not find location" (the failure the Korean-
|
||||
locked deployment hit on every weather request)."""
|
||||
mock_romanise.return_value = "Seoul"
|
||||
|
||||
geo_empty = Mock()
|
||||
geo_empty.raise_for_status = Mock()
|
||||
geo_empty.json.return_value = {} # no "results"
|
||||
|
||||
geo_seoul = Mock()
|
||||
geo_seoul.raise_for_status = Mock()
|
||||
geo_seoul.json.return_value = {
|
||||
"results": [{
|
||||
"latitude": 37.566,
|
||||
"longitude": 126.9784,
|
||||
"name": "Seoul",
|
||||
"country": "South Korea",
|
||||
"admin1": "Seoul",
|
||||
}]
|
||||
}
|
||||
|
||||
weather_response = Mock()
|
||||
weather_response.raise_for_status = Mock()
|
||||
weather_response.json.return_value = {
|
||||
"current": {
|
||||
"time": "2026-06-12T14:00",
|
||||
"temperature_2m": 24.0,
|
||||
"apparent_temperature": 25.0,
|
||||
"relative_humidity_2m": 55,
|
||||
"weather_code": 0,
|
||||
"wind_speed_10m": 6.0,
|
||||
"wind_gusts_10m": 10.0,
|
||||
},
|
||||
"hourly": {
|
||||
"time": [f"2026-06-12T{h:02d}:00" for h in range(24)],
|
||||
"temperature_2m": [20 + h * 0.2 for h in range(24)],
|
||||
"weather_code": [0] * 24,
|
||||
},
|
||||
"daily": {
|
||||
"time": [f"2026-06-{12+d:02d}" for d in range(7)],
|
||||
"weather_code": [0, 1, 2, 3, 1, 0, 2],
|
||||
"temperature_2m_max": [25, 26, 24, 23, 27, 28, 25],
|
||||
"temperature_2m_min": [16, 17, 15, 14, 18, 19, 16],
|
||||
},
|
||||
}
|
||||
|
||||
mock_get.side_effect = [geo_empty, geo_seoul, weather_response]
|
||||
|
||||
result = self.tool.run({"location": "서울"}, self.context)
|
||||
|
||||
assert result.success is True
|
||||
assert "Seoul" in result.reply_text
|
||||
mock_romanise.assert_called_once()
|
||||
# The romanised name must be what the second geocode actually queried.
|
||||
assert mock_get.call_args_list[1].kwargs["params"]["name"] == "Seoul"
|
||||
|
||||
def test_romanise_place_skips_ascii_names(self):
|
||||
"""ASCII names already geocode, so no LLM round-trip is spent on them."""
|
||||
cfg = Mock()
|
||||
cfg.ollama_base_url = "http://x"
|
||||
cfg.ollama_chat_model = "qwen2.5:3b"
|
||||
cfg.tool_router_model = ""
|
||||
cfg.intent_judge_model = ""
|
||||
assert _romanise_place("Seoul", cfg) is None
|
||||
|
||||
@patch('requests.get')
|
||||
def test_run_location_not_found(self, mock_get):
|
||||
"""Test weather with unknown location."""
|
||||
|
||||
Reference in New Issue
Block a user