mirror of
https://github.com/zvx-echo6/meshai.git
synced 2026-08-26 09:21:33 +00:00
fix(dispatcher): qualify region-cooldown key by channel type (#115)
Section 1.5 (region_routes matrix branch) armed and checked the per-region cooldown key as (toggle, category, region) with no channel-type component. _chans always inserts mesh_broadcast before meshcore_broadcast (insertion order in the per-cell append loop), so for any matched cell with BOTH mt and mc populated, the mesh_broadcast send armed the cooldown key first; the very next iteration checked that SAME key for meshcore_broadcast and saw it as freshly cooled down, dropping it every time. Net effect: meshcore_broadcast never succeeded via the matrix branch whenever cooldown_seconds > 0 (true for weather/roads/fire, all 300s), so it never armed its own dedup/cooldown state either -- a silent, permanent MC blackout for every region-routed family. Confirmed live: dispatcher_dedup had 555 rows, zero meshcore_broadcast; mesh_broadcasts_out was 120:4 MT:MC for nws_alerts and 98:8 for traffic_events over 10 days (the few MC rows that got through came from a different, non-matrix code path). fires' 98:77 near-1:1 ratio is not evidence the matrix branch worked for fire -- those MC sends are dominated by the cooldown-exempt scheduled reminder path (dispatch_scheduled_fire_broadcast); fire's own live event-driven path has the identical latent bug, just masked. Fix folds ch_type into the region string (mirrors the existing _cd_suffix convention) so mesh_broadcast and meshcore_broadcast get independent cooldown windows. Kept the cooldown key a 3-tuple (instead of widening to 4) to avoid a dispatcher_cooldowns schema migration -- _persist_cooldown() and the boot-restore SELECT are both hard-coded to (toggle, category, region). Verified in isolation (no live/deployed behavior change, no transmit): a fresh Dispatcher built from the live production config now dispatches both mesh_broadcast and meshcore_broadcast for weather/roads/fire matched cells under a 300s cooldown. Added two regression tests covering the gap that let this ship untested: no existing test combined cooldown_seconds > 0 with a cell that has BOTH mt and mc populated (test_cell_match_routes_mt_and_mc uses the cooldown_s=0 default; test_per_region_cooldown_independence uses cooldown_s=300 but with mc=None on every cell). Co-authored-by: Matt Johnson <mj@k7zvx.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
aa3e5943c4
commit
f50c2e54d8
2 changed files with 88 additions and 2 deletions
|
|
@ -501,13 +501,28 @@ class Dispatcher:
|
|||
|
||||
# Per-region cooldown check — skip channel only when EVERY
|
||||
# region feeding it is still within its cooldown window.
|
||||
# BUGFIX: _rk MUST be qualified by _ch_type. _chans iterates
|
||||
# mesh_broadcast before meshcore_broadcast (insertion order,
|
||||
# see the append loop above), so without the channel-type
|
||||
# suffix the MT send for this SAME event armed this SAME key
|
||||
# a few lines up -- MC's check then always sees a fresh
|
||||
# last_fired_at and is dropped as "cooled down" on literally
|
||||
# every event. That's why dispatcher_dedup had zero
|
||||
# meshcore_broadcast rows despite mc_enabled+mc being
|
||||
# correctly configured: MC never got far enough to succeed
|
||||
# and arm its own dedup entry. Keeping _rk a 3-tuple (folding
|
||||
# ch_type into the region string, mirroring the existing
|
||||
# _cd_suffix convention) avoids a dispatcher_cooldowns schema
|
||||
# migration -- _persist_cooldown()/the restore SELECT are
|
||||
# both hard-coded to (toggle, category, region).
|
||||
if _cooldown_s > 0:
|
||||
_all_cooled = True
|
||||
for _rg in _regions:
|
||||
_rk = (
|
||||
getattr(tog, "name", "") or fam,
|
||||
event.category,
|
||||
_rg + ("|" + _cd_suffix if _cd_suffix else ""),
|
||||
_rg + ("|" + _cd_suffix if _cd_suffix else "")
|
||||
+ "|" + _ch_type,
|
||||
)
|
||||
_last = self._toggle_cooldown.get(_rk)
|
||||
if _last is None or (_now - _last) >= _cooldown_s:
|
||||
|
|
@ -577,7 +592,8 @@ class Dispatcher:
|
|||
_rk = (
|
||||
getattr(tog, "name", "") or fam,
|
||||
event.category,
|
||||
_rg + ("|" + _cd_suffix if _cd_suffix else ""),
|
||||
_rg + ("|" + _cd_suffix if _cd_suffix else "")
|
||||
+ "|" + _ch_type,
|
||||
)
|
||||
self._toggle_cooldown[_rk] = _commit_now
|
||||
self._persist_cooldown(_rk, _commit_now, _cooldown_s)
|
||||
|
|
|
|||
|
|
@ -900,3 +900,73 @@ def test_ptx_destination_mesh_broadcast_not_double_when_matrix_owns_mt():
|
|||
assert all(r["broadcast_channel"] != 7 for r in rec), \
|
||||
"the mesh_broadcast destination (ch7) must be filtered by _matrix_handled"
|
||||
assert len(rec) == 1, "only the matrix MT send; no extra destination delivery"
|
||||
|
||||
|
||||
# ==================================================================
|
||||
# Regression: cooldown key must be transport-qualified (2026-07-11)
|
||||
# ------------------------------------------------------------------
|
||||
# The Section 1.5 per-region cooldown key was (toggle, category, region) with
|
||||
# NO channel-type component. _chans always inserts mesh_broadcast before
|
||||
# meshcore_broadcast (insertion order in the per-cell append loop), so for a
|
||||
# cell with BOTH mt and mc populated, the loop over _chans.items() processed
|
||||
# mesh_broadcast first, delivered it, and ARMED the shared cooldown key. The
|
||||
# very next iteration (meshcore_broadcast) checked that SAME key -- now fresh
|
||||
# -- and was dropped as "cooled down", every single time, for every event.
|
||||
# Net effect: meshcore_broadcast never succeeded via the matrix branch when
|
||||
# cooldown_seconds > 0, so it never armed its own dedup/cooldown state either
|
||||
# -- a permanent, silent MC blackout for every region-routed family (weather,
|
||||
# 511/roads; fire's live path too, masked because fire's MC delivery was
|
||||
# dominated by the cooldown-exempt scheduled reminder path).
|
||||
#
|
||||
# No prior test caught this because test_cell_match_routes_mt_and_mc uses the
|
||||
# _base_cfg default cooldown_s=0 (the `if _cooldown_s > 0:` gate is skipped
|
||||
# entirely), and test_per_region_cooldown_independence uses cooldown_s=300
|
||||
# but with mc=None on every cell (MT-only), so it never exercises a cell with
|
||||
# BOTH transports populated under a live cooldown window.
|
||||
|
||||
|
||||
def test_cooldown_does_not_cross_suppress_mt_and_mc():
|
||||
"""A cell with BOTH mt and mc populated, under cooldown_seconds > 0, must
|
||||
deliver BOTH transports on the same event -- MT arming its cooldown must
|
||||
NOT cause MC's cooldown check (same toggle/category/region) to see itself
|
||||
as already-cooled-down within the same dispatch call."""
|
||||
cfg = _base_cfg(fam="fire", cooldown_s=300, min_severity="routine")
|
||||
cfg.notifications.region_routes = _rr(cells={
|
||||
"fire": {
|
||||
"SCI": {"mt": 3, "mc": "sci-fires", "min_severity": "routine", "enabled": True},
|
||||
}
|
||||
})
|
||||
ev = _ev(fam="fire", region="SCI")
|
||||
_, rec = _dispatch(cfg, ev)
|
||||
|
||||
types = {r["delivery_type"] for r in rec}
|
||||
assert "mesh_broadcast" in types, "must deliver via mesh_broadcast"
|
||||
assert "meshcore_broadcast" in types, (
|
||||
"must ALSO deliver via meshcore_broadcast -- MT's cooldown arm must "
|
||||
"not cross-suppress MC on the same event/region"
|
||||
)
|
||||
assert len(rec) == 2, "exactly two deliveries (mt + mc), got %r" % (rec,)
|
||||
|
||||
|
||||
def test_cooldown_independent_per_transport_across_events():
|
||||
"""After a successful MT+MC dispatch, a SECOND event in the SAME region
|
||||
within the cooldown window must be dropped for BOTH transports (cooldown
|
||||
still functions), and each transport's cooldown key must be independently
|
||||
keyed -- not just accidentally passing because both are simply always-on
|
||||
or always-off together."""
|
||||
cfg = _base_cfg(fam="fire", cooldown_s=300, min_severity="routine")
|
||||
cfg.notifications.region_routes = _rr(cells={
|
||||
"fire": {
|
||||
"SCI": {"mt": 3, "mc": "sci-fires", "min_severity": "routine", "enabled": True},
|
||||
}
|
||||
})
|
||||
d, rec = _make_dispatcher(cfg)
|
||||
|
||||
ev1 = _ev(fam="fire", region="SCI", eid="ev-1")
|
||||
asyncio.run(d.dispatch(ev1))
|
||||
assert len(rec) == 2, "first event: both transports deliver"
|
||||
|
||||
ev2 = _ev(fam="fire", region="SCI", eid="ev-2")
|
||||
asyncio.run(d.dispatch(ev2))
|
||||
# Still within cooldown window for BOTH transports -> no new deliveries.
|
||||
assert len(rec) == 2, "second event within cooldown: no new deliveries, got %r" % (rec,)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue