From 5381975e8338f67d882b7bb3abdadb4181dffd60 Mon Sep 17 00:00:00 2001 From: Matt Johnson Date: Fri, 3 Jul 2026 21:20:37 +0000 Subject: [PATCH] =?UTF-8?q?fix(nws):=20tighten=20hazard=20wording=20across?= =?UTF-8?q?=20all=20product=20types;=20kill=20dangling=20'=E2=80=94=20?= =?UTF-8?q?=E2=80=A6'=20in=20L4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Special Weather Statements (and other SPS/WSW/FFW/FLW products) rendered a verbose raw hazard sentence on L3 that ate the packet budget, collapsing L4 to a dangling 'Moving SW 24 mph —…' with every town lost (seen live in the Activity Log). - Add _tighten_hazard(): compacts free-form NWS hazard text into the terse SVR idiom for ALL branches — 'Wind gusts in excess of 45 mph' -> '45mph gusts', 'in excess of' -> '>', '45 mph' -> '45mph', 'pea size hail' -> '0.25" hail'. Applied to the FFW/FLW and SPS/WSW/else branches (SVR already terse). - Rework L4 assembly to be budget-aware BEFORE the final hard cap: try location forms richest->poorest (full list -> first->mid->last -> first->last -> first-only -> none) and only attach '— {locs}' when the whole message fits. If no location fits, degrade to motion-only; if even that overflows, drop L4. A dangling '— …' / trailing '—' is now structurally impossible. - Tests: SPS worst-case (tightened + no dangling), WSW, pathological motion-only degrade, SVR no-dangling re-verify; shared dangling-separator assertion. Co-Authored-By: Claude Opus 4.8 (1M context) --- work/meshai/central/nws_handler.py | 110 ++++++++++++++----- work/tests/test_nws_handler.py | 163 +++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 26 deletions(-) diff --git a/work/meshai/central/nws_handler.py b/work/meshai/central/nws_handler.py index 89cca16..e571e6a 100644 --- a/work/meshai/central/nws_handler.py +++ b/work/meshai/central/nws_handler.py @@ -374,6 +374,45 @@ def _collapse_certainty(text: str) -> str: return (text or "").strip() +def _tighten_hazard(text: str) -> str: + """Compact a free-form NWS hazard sentence into the terse mesh idiom the + SVR branch already uses, so no product type (SPS/WSW/FFW/FLW/else) carries a + bloated line 3. Drops filler ('in excess of' -> '>'), collapses '45 mph' -> + '45mph', rewrites wind-gust phrasings to 'Nmph gusts', and converts hail + descriptors ('pea size hail') to numeric inches ('0.25" hail').""" + if not text: + return "" + t = text.strip().rstrip(".") + # Hail: ' size hail' -> 'N" hail' (keep any leading connector). + low = t.lower() + for k in sorted(_HAIL_DESCRIPTORS, key=len, reverse=True): + for variant in (f"{k} size hail", f"{k}-size hail", f"{k} sized hail"): + idx = low.find(variant) + if idx != -1: + repl = _fmt_hail(k) + if repl: + t = t[:idx] + repl + t[idx + len(variant):] + low = t.lower() + break + # Numeric hail: 'N inch hail' / 'N-inch hail' -> 'N" hail'. + t = re.sub(r"(\d+(?:\.\d+)?)[\- ]inch(?:es)?\s+hail", + lambda m: _fmt_hail(m.group(1)) or m.group(0), t, + flags=re.IGNORECASE) + # Wind-gust phrasings -> 'Nmph gusts'. + t = re.sub(r"wind gusts?\s+(?:in excess of|up to|to|of|reaching|near|around)" + r"\s+(\d+)\s*mph", r"\1mph gusts", t, flags=re.IGNORECASE) + t = re.sub(r"winds?\s+gusting\s+(?:up\s+)?to\s+(\d+)\s*mph", + r"\1mph gusts", t, flags=re.IGNORECASE) + # Sustained winds -> 'Nmph winds'. + t = re.sub(r"(?:damaging\s+)?winds?\s+(?:in excess of|up to|to|of)\s+" + r"(\d+)\s*mph", r"\1mph winds", t, flags=re.IGNORECASE) + # Remaining generic filler + spacing. + t = re.sub(r"\bin excess of\b", ">", t, flags=re.IGNORECASE) + t = re.sub(r"(\d+)\s*mph", r"\1mph", t, flags=re.IGNORECASE) + t = re.sub(r"\s+", " ", t).strip() + return t + + def _render(*, event_type, area_desc, geocoder_city, county, state, expires_epoch, lat, lon, now, prefix: str = "", d: dict = None) -> str: d = d or {} @@ -443,9 +482,10 @@ def _render(*, event_type, area_desc, geocoder_city, county, state, line3 = f"{hazard} · {confirm}" if hazard else confirm elif same_code in ("FFW", "FLW"): hazard_text = desc.get("hazard") or "" - # First sentence only + # First sentence only, then tighten to the terse SVR idiom. if ". " in hazard_text: hazard_text = hazard_text.split(". ")[0] + hazard_text = _tighten_hazard(hazard_text) # Infer flood cause from description desc_lower = (d.get("description") or "").lower() flood_cause = "" @@ -459,10 +499,12 @@ def _render(*, event_type, area_desc, geocoder_city, county, state, cause_seg = f" · {flood_cause}" if flood_cause else "" line3 = f"{hazard_text}{cause_seg}" if hazard_text else flood_cause else: - # SPS, WSW, etc.: first hazard sentence + certainty if Observed/Likely + # SPS, WSW, etc.: first hazard sentence (tightened) + certainty if + # Observed/Likely. hazard_text = desc.get("hazard") or "" if ". " in hazard_text: hazard_text = hazard_text.split(". ")[0] + hazard_text = _tighten_hazard(hazard_text) cert_seg = "" if certainty in ("Observed", "Likely"): cert_seg = f" · {_collapse_certainty(certainty)}" @@ -481,6 +523,15 @@ def _render(*, event_type, area_desc, geocoder_city, county, state, towns[-1] = re.sub(r"^and\s+", "", towns[-1], flags=re.IGNORECASE).strip() towns = [t for t in towns if t] + def _dedup(seq): + """Drop consecutive repeats (a short list can make first == middle), + so we never render 'Buhl → Buhl → Shoshone'.""" + out = [] + for t in seq: + if not out or out[-1] != t: + out.append(t) + return out + def _line4(locs: str) -> str: if motion and locs: return f"{motion} — {locs}" @@ -490,32 +541,39 @@ def _render(*, event_type, area_desc, geocoder_city, county, state, PACKET_LIMIT = budget_for("nws") - # Prefer the FULL town list; use it verbatim if the whole message fits. - full_locs = ", ".join(towns) - line4 = _line4(full_locs) - msg = "\n".join(l for l in (line1, line2, line3, line4) if l) + # Location representations from richest to poorest: full comma list -> + # first→middle→last path sample -> first→last -> first-only -> none. The + # WHOLE message is measured against the budget for each and the first form + # that fits wins. Crucially the "— {locs}" segment is only ever attached + # when the full message fits, so we can never emit a dangling "— …": if no + # location form fits we fall through to motion-only, then (if even that + # overflows) drop line 4 entirely. + loc_options = [", ".join(towns)] # full comma list + if len(towns) >= 3: + loc_options.append(" → ".join( + _dedup([towns[0], towns[len(towns) // 2], towns[-1]]))) + if len(towns) >= 2: + loc_options.append(" → ".join(_dedup([towns[0], towns[-1]]))) + if towns: + loc_options.append(towns[0]) + loc_options.append("") # motion only / empty - if len(msg) > PACKET_LIMIT: - # Won't fit: collapse locations to a path sample that never loses the - # endpoints -> soonest-impact -> midway -> farthest-along. - if len(towns) >= 3: - sampled = [towns[0], towns[len(towns) // 2], towns[-1]] - elif len(towns) == 2: - sampled = [towns[0], towns[-1]] - else: - sampled = list(towns) - # De-dup consecutive repeats (a short list can make first == middle), - # so we never render "Buhl → Buhl → Shoshone". - deduped = [] - for t in sampled: - if not deduped or deduped[-1] != t: - deduped.append(t) - path = " → ".join(deduped) - line4 = _line4(path) - msg = "\n".join(l for l in (line1, line2, line3, line4) if l) + base_lines = [l for l in (line1, line2, line3) if l] + msg = None + for locs in loc_options: + cand4 = _line4(locs) + lines = base_lines + ([cand4] if cand4 else []) + candidate = "\n".join(lines) + if len(candidate) <= PACKET_LIMIT: + msg = candidate + break + if msg is None: + # Even motion-only line 4 overflows: drop line 4 entirely. + msg = "\n".join(base_lines) - # Final hard-cap safety net for the pathological case (extremely long town - # names overflow even the first->middle->last sample plus the other lines). + # Final hard-cap safety net for the pathological case where lines 1-3 alone + # overflow. Line 4 is already budget-fitted above, so this only ever trims + # the leading lines — it can never manufacture a dangling "— …". return fit_to_budget(msg, PACKET_LIMIT) diff --git a/work/tests/test_nws_handler.py b/work/tests/test_nws_handler.py index 162bd6d..26ce80d 100644 --- a/work/tests/test_nws_handler.py +++ b/work/tests/test_nws_handler.py @@ -378,3 +378,166 @@ def test_svr_worst_case_fits_140(mem_db): assert "70mph winds" in rendered # tightened hazard (wind) assert '1.75" hail' in rendered # golf ball -> 1.75" assert "Buhl" in rendered # >=1 town present + + +# ---- no dangling "— …" across ALL product types ---- + + +def _assert_no_dangling_separator(rendered: str): + """The L4 motion/locations line must never end in a stray separator: + no trailing '—…', '— …', or a bare '—'. Either a real location list + follows the em-dash, or the em-dash (and its locations) are absent.""" + for line in rendered.splitlines(): + stripped = line.rstrip() + assert not stripped.endswith("—…"), f"dangling '—…': {line!r}" + assert not stripped.endswith("— …"), f"dangling '— …': {line!r}" + assert not stripped.endswith("—"), f"bare trailing '—': {line!r}" + # And the "— …" fragment must not appear mid-line either. + assert "—…" not in stripped, f"'—…' fragment: {line!r}" + assert "— …" not in stripped, f"'— …' fragment: {line!r}" + + +def test_sps_worst_case_tightened_and_no_dangling(mem_db): + """The reported live-log bug: a Special Weather Statement (SPS) with wind + gusts + motion + a long town list previously collapsed L4 to + 'Moving SW 24 mph —…' (all towns lost, dangling separator). After the fix: + hazard is tightened, output fits 140, and L4 is either + 'Moving … — ' or 'Moving …' — never a trailing '—…'.""" + long_locations = ( + "Twin Falls, Kimberly, Filer, Buhl, Hansen, Murtaugh, Hollister, " + "Eden, Hazelton, and Rogerson" + ) + description = ( + "HAZARD...Wind gusts in excess of 45 mph and pea size hail.\n\n" + "SOURCE...Radar indicated.\n\n" + f"Locations impacted include...{long_locations}" + ) + d = { + "eventCode": {"SAME": ["SPS"]}, + "certainty": "Observed", + "parameters": { + # 225 DEG 21 KT -> "Moving SW 24 mph" + "eventMotionDescription": ["2200000T225DEG...21KT 42.5,-114.5"], + }, + "description": description, + } + rendered = _render( + event_type="Special Weather Statement", area_desc="Twin Falls County", + geocoder_city=None, county="Twin Falls", state="ID", + expires_epoch=1_751_400_000, lat=42.5, lon=-114.46, + now=1_751_400_000, d=d, + ) + assert len(rendered) <= 140, f"{len(rendered)} chars:\n{rendered!r}" + assert "Special Weather Statement" in rendered + # (a) hazard tightened: "Wind gusts in excess of 45 mph" -> "45mph gusts", + # "pea size hail" -> '0.25" hail'; filler dropped. + assert "45mph gusts" in rendered, f"wind not tightened:\n{rendered!r}" + assert '0.25" hail' in rendered, f"hail not numeric:\n{rendered!r}" + assert "in excess of" not in rendered, "filler 'in excess of' survived" + assert "· observed" in rendered, "certainty not collapsed" + # (b) NEVER a dangling separator. + _assert_no_dangling_separator(rendered) + # (c) the motion line, when present, either carries a town or stands alone. + last = rendered.splitlines()[-1] + if last.startswith("Moving"): + assert last == "Moving SW 24 mph" or " — " in last, ( + f"L4 neither motion-only nor motion+towns:\n{last!r}") + if " — " in last: + # A real town must follow the em-dash. + tail = last.split(" — ", 1)[1].strip() + assert tail and tail != "…", f"empty tail after em-dash:\n{last!r}" + + +def test_wsw_hazard_tightened_and_no_dangling(mem_db): + """Winter Weather product (WSW SAME code): wind-gust hazard is tightened and + no dangling '—…' can appear.""" + long_locations = ( + "Sun Valley, Ketchum, Hailey, Bellevue, Carey, Picabo, Fairfield, " + "and Gooding" + ) + description = ( + "HAZARD...Wind gusts up to 45 mph and heavy snow.\n\n" + f"Locations impacted include...{long_locations}" + ) + d = { + "eventCode": {"SAME": ["WSW"]}, + "certainty": "Observed", + "parameters": { + "eventMotionDescription": ["2200000T225DEG...21KT 42.5,-114.5"], + }, + "description": description, + } + rendered = _render( + event_type="Winter Weather Advisory", area_desc="Blaine County", + geocoder_city=None, county="Blaine", state="ID", + expires_epoch=1_751_400_000, lat=43.5, lon=-114.3, + now=1_751_400_000, d=d, + ) + assert len(rendered) <= 140, f"{len(rendered)} chars:\n{rendered!r}" + assert "45mph gusts" in rendered, f"WSW wind not tightened:\n{rendered!r}" + assert "heavy snow" in rendered + assert "up to" not in rendered, "filler 'up to' survived" + _assert_no_dangling_separator(rendered) + + +def test_sps_pathological_towns_degrade_to_motion_only(mem_db): + """When even a single sampled town cannot fit the remaining budget, L4 must + degrade to motion-only ('Moving …') with NO trailing separator — never + 'Moving … —…'.""" + # One absurdly long town name that cannot coexist with the em-dash + motion + # in the leftover budget. + long_town = "Averyverylongimpossibletownnamethatwillnotfitthebudgetatall" * 2 + description = ( + "HAZARD...Wind gusts in excess of 45 mph.\n\n" + f"Locations impacted include...{long_town}" + ) + d = { + "eventCode": {"SAME": ["SPS"]}, + "certainty": "Observed", + "parameters": { + "eventMotionDescription": ["2200000T225DEG...21KT 42.5,-114.5"], + }, + "description": description, + } + rendered = _render( + event_type="Special Weather Statement", area_desc="Twin Falls County", + geocoder_city=None, county="Twin Falls", state="ID", + expires_epoch=1_751_400_000, lat=42.5, lon=-114.46, + now=1_751_400_000, d=d, + ) + assert len(rendered) <= 140 + _assert_no_dangling_separator(rendered) + last = rendered.splitlines()[-1] + # The town can't fit, so L4 (if present) is bare motion. + if last.startswith("Moving"): + assert " — " not in last, f"expected motion-only, got:\n{last!r}" + + +def test_svr_no_dangling_separator(mem_db): + """Re-verify SVR (the branch tightened earlier) still never dangles.""" + long_locations = ( + "Buhl, Eden, Hazelton, Murtaugh, Richfield, Dietrich, Gooding, " + "Hagerman, Wendell, Jerome, Kimberly, Hansen, Filer, and Shoshone" + ) + description = ( + "HAZARD...Damaging winds to 70 mph and golf ball size hail.\n\n" + f"Locations impacted include...{long_locations}" + ) + d = { + "eventCode": {"SAME": ["SVR"]}, + "certainty": "Observed", + "parameters": { + "maxWindGust": ["70 MPH"], "maxHailSize": ["1.75"], + "eventMotionDescription": ["2200000T254DEG...35KT 42.5,-114.5"], + }, + "description": description, + } + rendered = _render( + event_type="Severe Thunderstorm Warning", area_desc="Twin Falls County", + geocoder_city=None, county="Twin Falls", state="ID", + expires_epoch=1_751_400_000, lat=42.5, lon=-114.46, + now=1_751_400_000, d=d, + ) + assert len(rendered) <= 140 + assert "70mph winds" in rendered + _assert_no_dangling_separator(rendered)