mirror of
https://github.com/zvx-echo6/meshai.git
synced 2026-08-26 17:31:34 +00:00
fix(config): warn on unknown config keys; delete 7 phantom keys from the example (#146)
* fix(config): warn on unknown config keys instead of silently dropping them
_dict_to_dataclass() silently continue'd past any key not in the target
dataclass's field set -- an operator could set a config key, restart, and
have it vanish with zero feedback (config.example.yaml's phantom
mesh_intelligence keys are exactly this bug, fixed separately).
Now logs a WARNING naming the key and the dataclass, hinting at a typo or
a renamed/removed field, via the module's existing _config_logger.
Traced every dynamic/free-form config path to rule out false positives:
notifications.toggles, notifications.destinations, generic_sources,
mesh_sources, and notifications.rules all route through explicit
dict-of-dataclass or verbatim-passthrough handling and never spuriously
warn. Two legitimate legacy shapes DO hit the strict field-check path with
keys that were never (and will never be) dataclass fields:
- notifications.channels (pre-v0.5 channel list), consumed directly from
the raw dict by _migrate_legacy_channels
- notifications.region_routes.enabled (pre mt/mc-split master switch),
read directly by the explicit region_routes handler
Both are allowlisted in _KNOWN_LEGACY_DROP_KEYS so users mid-migration
don't get spurious noise on every load.
Added tests/test_config_loader.py coverage: unknown key warns and does
not raise, both legacy shapes stay silent, and the free-form/dynamic
sections never warn for keys valid on their real target shape.
* fix(config): remove phantom config.example.yaml keys, add missing live ones
The new unknown-key warning (previous commit) caught config.example.yaml
loading with SEVEN warnings, all real drift -- none were false positives
of the warning itself:
mesh_intelligence block shipped three keys with no MeshIntelligenceConfig
field and no implementation anywhere (git log -S confirms they were never
built, not leftovers from a removal):
- region_radius_miles, infra_overrides, region_labels
These only made sense under an older auto-clustering design; what
actually exists is explicit region anchors (regions: list[RegionAnchor]).
Deleted from both the live block and the commented-out example above it,
and added the four fields that DO exist and are live but were missing
from the example: regions, critical_nodes, alert_channel, alert_rules.
Also fixed the now-misleading comment at
dashboard/api/mesh_routes.py:281, which referenced region_labels --
comment only, no code change.
notifications block shipped a whole quiet-hours subsystem that was
deliberately ripped out (commit b948ed77, "silent is better than ugly")
and never implemented as override_quiet -- confirmed by zero readers and
zero dataclass fields anywhere in meshai/:
- quiet_hours_enabled, quiet_hours_start, quiet_hours_end
- override_quiet (on 4 rule entries, including "Emergency Broadcast",
which falsely implied emergency alerts bypass quiet hours)
Deleted; no quiet-hours feature implemented (out of scope -- product
decision for the owner).
config.example.yaml now loads with exactly zero warnings: the loader and
the example finally agree.
---------
Co-authored-by: Matt Johnson <mj@k7zvx.com>
This commit is contained in:
parent
b0d1a76e70
commit
2543dfa8d1
4 changed files with 139 additions and 18 deletions
|
|
@ -114,22 +114,31 @@ mesh_sources: []
|
||||||
#
|
#
|
||||||
# mesh_intelligence:
|
# mesh_intelligence:
|
||||||
# enabled: true
|
# enabled: true
|
||||||
# region_radius_miles: 40.0 # Radius for region clustering
|
# regions: # Fixed region anchors (explicit, not auto-clustered)
|
||||||
# locality_radius_miles: 8.0 # Radius for locality clustering
|
# - name: "magic_valley"
|
||||||
|
# lat: 42.56
|
||||||
|
# lon: -114.47
|
||||||
|
# local_name: "Magic Valley"
|
||||||
|
# description: "Twin Falls, Burley, Jerome along I-84/US-93"
|
||||||
|
# aliases: ["southern Idaho"]
|
||||||
|
# cities: ["Twin Falls", "Burley", "Jerome"]
|
||||||
|
# locality_radius_miles: 8.0 # Radius for locality clustering within regions
|
||||||
# offline_threshold_hours: 2 # Hours before node considered offline
|
# offline_threshold_hours: 2 # Hours before node considered offline
|
||||||
# packet_threshold: 500 # Non-text packets per 24h to flag
|
# packet_threshold: 500 # Non-text packets per 24h to flag
|
||||||
# battery_warning_percent: 30 # Battery level for warnings
|
# battery_warning_percent: 30 # Battery level for warnings
|
||||||
# infra_overrides: [] # Node IDs to exclude from infrastructure
|
# critical_nodes: [] # Short names of critical nodes (e.g., ["MHR", "HPR"])
|
||||||
# region_labels: {} # Override auto-names: {"Twin Falls": "Magic Valley"}
|
# alert_channel: -1 # Channel to broadcast alerts on. -1 = disabled, 0+ = channel index
|
||||||
|
# alert_rules: {} # Per-condition alert toggles/thresholds (see AlertRulesConfig)
|
||||||
mesh_intelligence:
|
mesh_intelligence:
|
||||||
enabled: false
|
enabled: false
|
||||||
region_radius_miles: 40.0
|
regions: []
|
||||||
locality_radius_miles: 8.0
|
locality_radius_miles: 8.0
|
||||||
offline_threshold_hours: 2
|
offline_threshold_hours: 2
|
||||||
packet_threshold: 500
|
packet_threshold: 500
|
||||||
battery_warning_percent: 30
|
battery_warning_percent: 30
|
||||||
infra_overrides: []
|
critical_nodes: []
|
||||||
region_labels: {}
|
alert_channel: -1
|
||||||
|
alert_rules: {}
|
||||||
|
|
||||||
# === ENVIRONMENTAL FEEDS ===
|
# === ENVIRONMENTAL FEEDS ===
|
||||||
# Live situational awareness from NWS, NOAA Space Weather, and Open-Meteo.
|
# Live situational awareness from NWS, NOAA Space Weather, and Open-Meteo.
|
||||||
|
|
@ -223,9 +232,6 @@ environmental:
|
||||||
# Categories match alert types from alert_engine.py.
|
# Categories match alert types from alert_engine.py.
|
||||||
notifications:
|
notifications:
|
||||||
enabled: false
|
enabled: false
|
||||||
quiet_hours_enabled: true # Master toggle for quiet hours feature
|
|
||||||
quiet_hours_start: "22:00" # Suppress non-emergency alerts during quiet hours
|
|
||||||
quiet_hours_end: "06:00"
|
|
||||||
|
|
||||||
# Digest scheduler settings
|
# Digest scheduler settings
|
||||||
# The digest collects priority/routine events and delivers a summary
|
# The digest collects priority/routine events and delivers a summary
|
||||||
|
|
@ -250,7 +256,6 @@ notifications:
|
||||||
delivery_type: mesh_broadcast
|
delivery_type: mesh_broadcast
|
||||||
broadcast_channel: 0
|
broadcast_channel: 0
|
||||||
cooldown_minutes: 5
|
cooldown_minutes: 5
|
||||||
override_quiet: true # Send even during quiet hours
|
|
||||||
|
|
||||||
# Infrastructure Down - critical node and infrastructure offline alerts
|
# Infrastructure Down - critical node and infrastructure offline alerts
|
||||||
- name: "Infrastructure Down"
|
- name: "Infrastructure Down"
|
||||||
|
|
@ -261,7 +266,6 @@ notifications:
|
||||||
delivery_type: mesh_broadcast
|
delivery_type: mesh_broadcast
|
||||||
broadcast_channel: 0
|
broadcast_channel: 0
|
||||||
cooldown_minutes: 30
|
cooldown_minutes: 30
|
||||||
override_quiet: false
|
|
||||||
|
|
||||||
# Fire Alert - wildfire proximity and new ignition
|
# Fire Alert - wildfire proximity and new ignition
|
||||||
- name: "Fire Alert"
|
- name: "Fire Alert"
|
||||||
|
|
@ -272,7 +276,6 @@ notifications:
|
||||||
delivery_type: mesh_broadcast
|
delivery_type: mesh_broadcast
|
||||||
broadcast_channel: 0
|
broadcast_channel: 0
|
||||||
cooldown_minutes: 60
|
cooldown_minutes: 60
|
||||||
override_quiet: false
|
|
||||||
|
|
||||||
# Severe Weather - weather warnings
|
# Severe Weather - weather warnings
|
||||||
- name: "Severe Weather"
|
- name: "Severe Weather"
|
||||||
|
|
@ -283,7 +286,6 @@ notifications:
|
||||||
delivery_type: mesh_broadcast
|
delivery_type: mesh_broadcast
|
||||||
broadcast_channel: 0
|
broadcast_channel: 0
|
||||||
cooldown_minutes: 30
|
cooldown_minutes: 30
|
||||||
override_quiet: false
|
|
||||||
|
|
||||||
# Example: Morning Digest -> mesh broadcast
|
# Example: Morning Digest -> mesh broadcast
|
||||||
# Delivers the accumulated digest at the configured schedule time
|
# Delivers the accumulated digest at the configured schedule time
|
||||||
|
|
|
||||||
|
|
@ -1226,6 +1226,29 @@ def _migrate_legacy_channels(notifications, data: dict):
|
||||||
_config_logger.info("Migrated to %d self-contained rules", len(notifications.rules))
|
_config_logger.info("Migrated to %d self-contained rules", len(notifications.rules))
|
||||||
|
|
||||||
|
|
||||||
|
# Keys that are legitimately present in raw config dicts but intentionally have
|
||||||
|
# NO matching dataclass field on the target class -- they're consumed by
|
||||||
|
# special-case logic elsewhere in _dict_to_dataclass (e.g. legacy-format
|
||||||
|
# migration) rather than becoming a field. Warning about these would be a
|
||||||
|
# false positive: the key isn't a typo, it's a known, still-supported legacy
|
||||||
|
# shape. Keyed by (dataclass, key name).
|
||||||
|
_KNOWN_LEGACY_DROP_KEYS = {
|
||||||
|
# Pre-v0.5 notifications.channels list; migrated into self-contained
|
||||||
|
# rules by _migrate_legacy_channels (reads straight from the raw dict,
|
||||||
|
# not from the coerced NotificationsConfig).
|
||||||
|
(NotificationsConfig, "channels"): (
|
||||||
|
"legacy notifications.channels format, handled by _migrate_legacy_channels"
|
||||||
|
),
|
||||||
|
# Pre-region-routing-split master switch. The explicit region_routes
|
||||||
|
# handler (in the "notifications" branch below) reads this directly via
|
||||||
|
# rr.get("enabled", ...) as the default for mt_enabled; it never becomes
|
||||||
|
# a RegionRouteMatrix field.
|
||||||
|
(RegionRouteMatrix, "enabled"): (
|
||||||
|
"legacy region_routes.enabled (pre-mt/mc split), mapped to mt_enabled"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def _dict_to_dataclass(cls, data: dict):
|
def _dict_to_dataclass(cls, data: dict):
|
||||||
"""Recursively convert dict to dataclass, handling nested structures."""
|
"""Recursively convert dict to dataclass, handling nested structures."""
|
||||||
if data is None:
|
if data is None:
|
||||||
|
|
@ -1238,6 +1261,13 @@ def _dict_to_dataclass(cls, data: dict):
|
||||||
if key.startswith("_"):
|
if key.startswith("_"):
|
||||||
continue
|
continue
|
||||||
if key not in field_types:
|
if key not in field_types:
|
||||||
|
if (cls, key) not in _KNOWN_LEGACY_DROP_KEYS:
|
||||||
|
_config_logger.warning(
|
||||||
|
"Config key '%s' is not a recognized field on %s -- it will "
|
||||||
|
"be IGNORED (dropped) on load. Check for a typo, or a "
|
||||||
|
"renamed/removed field.",
|
||||||
|
key, cls.__name__,
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
field_type = field_types[key]
|
field_type = field_types[key]
|
||||||
|
|
|
||||||
|
|
@ -297,7 +297,7 @@ async def get_regions(request: Request):
|
||||||
|
|
||||||
regions.append({
|
regions.append({
|
||||||
"name": region.name,
|
"name": region.name,
|
||||||
"local_name": region.name, # Could be overridden by region_labels
|
"local_name": region.name, # TODO: surface region.local_name (real field, unused here)
|
||||||
"node_count": len(region.node_ids),
|
"node_count": len(region.node_ids),
|
||||||
"infra_count": infra_total,
|
"infra_count": infra_total,
|
||||||
"infra_online": infra_online,
|
"infra_online": infra_online,
|
||||||
|
|
|
||||||
|
|
@ -6,10 +6,14 @@ cfg.notifications.rules as raw dicts (which crashed Dispatcher._matching_rules
|
||||||
on rule.enabled). config_loader.load_config uses this same _dict_to_dataclass.
|
on rule.enabled). config_loader.load_config uses this same _dict_to_dataclass.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import logging
|
||||||
|
|
||||||
from meshai.config import (
|
from meshai.config import (
|
||||||
Config,
|
Config,
|
||||||
|
MeshIntelligenceConfig,
|
||||||
NotificationRuleConfig,
|
NotificationRuleConfig,
|
||||||
NotificationToggle,
|
NotificationToggle,
|
||||||
|
RegionRouteMatrix,
|
||||||
_dataclass_to_dict,
|
_dataclass_to_dict,
|
||||||
_dict_to_dataclass,
|
_dict_to_dataclass,
|
||||||
)
|
)
|
||||||
|
|
@ -84,3 +88,88 @@ def test_toggle_meshcore_channel_name_round_trips():
|
||||||
# Default stays None when unset.
|
# Default stays None when unset.
|
||||||
default = _dict_to_dataclass(NotificationToggle, {"name": "weather"})
|
default = _dict_to_dataclass(NotificationToggle, {"name": "weather"})
|
||||||
assert default.meshcore_channel is None
|
assert default.meshcore_channel is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_unknown_key_warns_but_does_not_raise(caplog):
|
||||||
|
"""An unrecognized config key (typo, or a renamed/removed field) logs a
|
||||||
|
WARNING naming the key and the dataclass, and the key is silently dropped
|
||||||
|
(never raises). Regression guard for the silent-drop bug: previously an
|
||||||
|
operator could set a bogus/typo'd key, restart, and get zero feedback."""
|
||||||
|
data = {
|
||||||
|
"mesh_intelligence": {
|
||||||
|
"enabled": True,
|
||||||
|
"region_radius_miles": 40.0, # never a real field -- see config.example.yaml fix
|
||||||
|
}
|
||||||
|
}
|
||||||
|
with caplog.at_level(logging.WARNING, logger="meshai.config"):
|
||||||
|
cfg = _dict_to_dataclass(Config, data)
|
||||||
|
|
||||||
|
assert cfg.mesh_intelligence.enabled is True
|
||||||
|
assert not hasattr(cfg.mesh_intelligence, "region_radius_miles")
|
||||||
|
warnings = [r for r in caplog.records if r.levelno == logging.WARNING]
|
||||||
|
assert len(warnings) == 1
|
||||||
|
msg = warnings[0].getMessage()
|
||||||
|
assert "region_radius_miles" in msg
|
||||||
|
assert "MeshIntelligenceConfig" in msg
|
||||||
|
|
||||||
|
|
||||||
|
def test_legacy_region_routes_enabled_does_not_warn(caplog):
|
||||||
|
"""region_routes.enabled is a pre-mt/mc-split legacy key, still read
|
||||||
|
directly by the explicit region_routes handler in _dict_to_dataclass.
|
||||||
|
It intentionally has no RegionRouteMatrix field and must NOT warn --
|
||||||
|
warning here would be a false positive for every config still using the
|
||||||
|
pre-split single master switch."""
|
||||||
|
data = {"notifications": {"region_routes": {"enabled": True, "cells": {}}}}
|
||||||
|
with caplog.at_level(logging.WARNING, logger="meshai.config"):
|
||||||
|
cfg = _dict_to_dataclass(Config, data)
|
||||||
|
|
||||||
|
assert cfg.notifications.region_routes.mt_enabled is True
|
||||||
|
assert not any(
|
||||||
|
"region_routes" in r.getMessage() or "'enabled'" in r.getMessage()
|
||||||
|
for r in caplog.records
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_legacy_notifications_channels_does_not_warn(caplog):
|
||||||
|
"""notifications.channels (pre-v0.5 format) has no NotificationsConfig
|
||||||
|
field -- it's consumed directly from the raw dict by
|
||||||
|
_migrate_legacy_channels. Must not warn for users mid-migration."""
|
||||||
|
data = {
|
||||||
|
"notifications": {
|
||||||
|
"channels": [{"id": "c1", "type": "mesh_broadcast", "channel_index": 0}],
|
||||||
|
"rules": [{"name": "r1", "channel_ids": ["c1"], "categories": ["fire"]}],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
with caplog.at_level(logging.WARNING, logger="meshai.config"):
|
||||||
|
cfg = _dict_to_dataclass(Config, data)
|
||||||
|
|
||||||
|
assert len(cfg.notifications.rules) == 1
|
||||||
|
assert cfg.notifications.rules[0].broadcast_channel == 0
|
||||||
|
assert not any("channels" in r.getMessage() for r in caplog.records)
|
||||||
|
|
||||||
|
|
||||||
|
def test_dynamic_sections_do_not_warn(caplog):
|
||||||
|
"""Sections that are intentionally free-form/passthrough (generic_sources)
|
||||||
|
or use explicit dict-of-dataclass coercion (toggles, destinations) must
|
||||||
|
never warn for keys that ARE valid on their actual target shape."""
|
||||||
|
data = {
|
||||||
|
"notifications": {
|
||||||
|
"toggles": {
|
||||||
|
"weather": {"name": "weather", "enabled": True, "min_severity": "priority"},
|
||||||
|
},
|
||||||
|
"destinations": {
|
||||||
|
"d1": {"name": "d1", "type": "mesh_broadcast", "broadcast_channel": 0},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
"mesh_sources": [
|
||||||
|
{"name": "mv", "type": "meshview", "url": "http://x", "enabled": True},
|
||||||
|
],
|
||||||
|
"generic_sources": [
|
||||||
|
{"name": "gs1", "enabled": True, "url": "http://x", "not_a_dataclass_field": "fine"},
|
||||||
|
],
|
||||||
|
}
|
||||||
|
with caplog.at_level(logging.WARNING, logger="meshai.config"):
|
||||||
|
cfg = _dict_to_dataclass(Config, data)
|
||||||
|
|
||||||
|
assert caplog.records == []
|
||||||
|
assert cfg.generic_sources[0]["not_a_dataclass_field"] == "fine"
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue