diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cc0ce91..927f88d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -298,9 +298,9 @@ Each `Playbook` (the field-guide entries in `catalog.py`, keyed by `detector_key | Key | Signature (compressed) | Sev | |---|---|---| -| `wired.bad_cable` | rx_errors delta rate > 10/min sustained or > 0.001% of packets; OR gigabit-capable peer negotiated at 10/100 (broken-pair downshift). Confounders: known-100Mbps device classes, counter age, unmanaged-switch hop | P2, P1 on uplink | +| `wired.bad_cable` | rx_errors delta rate > 10/min sustained or > 0.001% of packets; OR a broken-pair speed downshift on either arm — **rated** (gigabit-capable peer negotiated at 10/100) or **observed** (a port running below a speed it held itself in the last 7 days). The observed arm carries the check past gigabit: on a 2.5G/10G port a fall to 1000 is the same symptom, but 1000 is unremarkable for a 2.5G port carrying a 1G device, so only the port's *own* history proves the peer can do better. The ceiling is measured as **time-held, not row count**: `record_state_change` writes only on change, so a port that ran a week at 2500 carries one 2500 row dated a week ago, and the value in effect *entering* the window has to seed the timeline or the commonest shape of all — damaged once, renegotiated down once, sitting there since — reports nothing. (`prune` preserves that boundary row for exactly this read.) Rows are fetched **by window, never by row count** — a count-limited fetch drops the oldest first, which is where the seed lives, so the harder a link flapped the more certainly its ceiling would be truncated away. Confounders: known-100Mbps device classes, counter age, unmanaged-switch hop, and a peer newer than the speed it would be credited with — compared against **when that speed was actually held**, not against the window start, since the latter both rejects peers that demonstrably ran fast mid-window and silently disables the whole arm on any store younger than the lookback. Where several peers claim one port the **newest** wins, so a departed device's lingering entity cannot vouch for its replacement. An observed ceiling overrules the device-class list only when it **dominates** — held longer than the current speed — because a 10/100 camera that blipped to 1000 for twelve minutes during a cabling event is not a cable fault | P2, P1 on uplink | | `wired.duplex_mismatch` | `full_duplex=false` on modern link | P2 | -| `wired.port_flapping` | ≥5 link transitions/10 min or ≥10/h from events; weight infra ports higher; correlate PoE draw 0 between flaps (reboot loop) | P2, P1 for AP/uplink | +| `wired.port_flapping` | ≥5 link transitions/10 min, ≥10/h, or ≥12/24 h; weight infra ports higher; correlate PoE draw 0 between flaps (reboot loop). The 24 h tier exists because the first two only see a link failing *fast* — a port dropping once or twice an hour around the clock never puts 5 transitions in any 10-minute window, and that slow-burn shape is what a marginal cable or a power-managed NIC actually produces. The title names the tightest tier that tripped, so a wide-window finding never renders as "0 transitions/10m" | P2, P1 for AP/uplink | | `wired.uplink_saturation` | uplink bps > 80%/95% negotiated speed 5 min+ with rising tx_dropped; hour-of-day baseline first | P2 | | `wired.poe_budget` | Σ poe_power > 80%/90% budget; `EVT_SW_PoeOverload` | P2/P1 | | `wired.stp_loop` | `EVT_SW_StpPortBlocking`, stp_state churn | P1 active | diff --git a/netadmin/detect/catalog.py b/netadmin/detect/catalog.py index fe3412f..a023004 100644 --- a/netadmin/detect/catalog.py +++ b/netadmin/detect/catalog.py @@ -292,9 +292,13 @@ def _channel_plan_spread_note(per_channel: Any) -> str: ), "wired.bad_cable": Playbook( signature="rx_errors delta rate > 10/min sustained or > 0.001% of packets; OR a " - "gigabit-capable peer negotiated at 10/100 (broken-pair downshift).", - confounders="Known 100 Mbps device classes; counter age (a stale cumulative counter); " - "an unmanaged-switch hop hiding the real port.", + "gigabit-capable peer negotiated at 10/100; OR a port running below a speed it " + "held itself in the last 7 days (broken-pair downshift, rated or observed).", + confounders="Known 100 Mbps device classes (overruled only by an observed ceiling that " + "outlasts the current speed, so a brief blip cannot condemn a 10/100 device); counter " + "age (a stale cumulative counter); an unmanaged-switch hop hiding the real port; a " + "wired peer newer than the speed it would be credited with (a faster device swapped " + "out for a slower one).", fix_guidance="Reseat then replace the patch cable; re-test the run. On an uplink port " "this is P1: the whole segment rides it.", evidence_fields=( @@ -303,6 +307,7 @@ def _channel_plan_spread_note(per_channel: Any) -> str: EvidenceField("error_packet_fraction", "Errors, share of packets", percent=True), EvidenceField("negotiated_speed", "Negotiated speed", "Mbps"), EvidenceField("port_capable_speed", "Port's rated speed", "Mbps"), + EvidenceField("observed_speed_max", "Speed this link has held", "Mbps"), ), confounder_notes={ "coverage_gated": _coverage_note, @@ -328,6 +333,22 @@ def _channel_plan_spread_note(per_channel: Any) -> str: if ev.get("negotiated_speed") is not None else None ), + "peer_predates_observed_speed": lambda ev: ( + "Peer age checked: the wired device on this port was already here before the " + f"link last held {_n(ev.get('observed_speed_max'), 0)} Mbps, so that speed is " + "this device's own history — not a faster machine that used to sit here." + if ev.get("observed_speed_max") is not None + else None + ), + "observed_speed_regression": lambda ev: ( + f"Measured against this link's own history: it has held " + f"{_n(ev.get('observed_speed_max'), 0)} Mbps recently and is now at " + f"{_n(ev.get('negotiated_speed'), 0)} Mbps, so the peer is provably capable of " + "the higher speed and something on the run is holding it back. A device that " + "simply cannot go faster would never have linked faster." + if ev.get("observed_speed_max") is not None + else None + ), }, ), "wired.duplex_mismatch": Playbook( @@ -354,26 +375,44 @@ def _channel_plan_spread_note(per_channel: Any) -> str: }, ), "wired.port_flapping": Playbook( - signature="≥5 link transitions/10 min or ≥10/h from events; infra ports weighted higher; " + signature="≥5 link transitions/10 min, ≥10/h, or ≥12/24 h; infra ports weighted higher; " "PoE draw dropping to 0 between flaps signals a reboot loop.", confounders="A laptop docking/undocking; scheduled device reboots.", fix_guidance="Reseat cable/SFP; on a PoE reboot loop check the PoE budget and power-" - "cycle the port; replace the cable if errors persist.", + "cycle the port; replace the cable if errors persist. A port that only trips the 24 h " + "tier — dropping steadily around the clock rather than in bursts — is more often the " + "device end than the run: check NIC/adapter power management before re-cabling.", evidence_fields=( EvidenceField("transitions_short", "Transitions, short window"), EvidenceField("window_short_s", "Short window", duration=True), EvidenceField("transitions_long", "Transitions, long window"), EvidenceField("window_long_s", "Long window", duration=True), + EvidenceField("transitions_sustained", "Transitions, sustained window"), + EvidenceField("window_sustained_s", "Sustained window", duration=True), EvidenceField("poe_reboot_loop", "PoE reboot loop"), EvidenceField("poe_min_w", "PoE draw, min", "W"), EvidenceField("poe_max_w", "PoE draw, max", "W"), ), confounder_notes={ "coverage_gated": _coverage_note, + # The sustained clause is guarded because issues predating that tier + # carry no such evidence: a resolved port_flapping issue never gets + # its evidence refreshed, so an unguarded f-string renders the old + # shape as "unknown in unknown" forever. Same contract as every other + # note here -- a missing key falls back, never fabricates. "sustained_transition_count": lambda ev: ( f"Sustained, not a blip: {_n(ev.get('transitions_short'), 0)} transitions in the " - f"last {_dur(ev.get('window_short_s'))} ({_n(ev.get('transitions_long'), 0)} in " - f"{_dur(ev.get('window_long_s'))})." + f"last {_dur(ev.get('window_short_s'))}, {_n(ev.get('transitions_long'), 0)} in " + f"{_dur(ev.get('window_long_s'))}" + + ( + f", {_n(ev.get('transitions_sustained'), 0)} in " + f"{_dur(ev.get('window_sustained_s'))}. A link that drops steadily all day " + "trips the widest window even when no single burst is fast enough for the " + "others." + if ev.get("transitions_sustained") is not None + and ev.get("window_sustained_s") is not None + else "." + ) ), # The evidence, not the confounder key, carries the verdict: this key only # means PoE data existed to check, not that a reboot loop was confirmed diff --git a/netadmin/detect/detectors/wired.py b/netadmin/detect/detectors/wired.py index 680fdf3..3f71fe9 100644 --- a/netadmin/detect/detectors/wired.py +++ b/netadmin/detect/detectors/wired.py @@ -6,11 +6,13 @@ into confounder-checked :class:`~netadmin.domain.entities.Finding` objects: * :class:`BadCableDetector` (``wired.bad_cable``) — rx/tx error-rate deltas, or a - gigabit-capable port negotiated down to 10/100 (broken-pair downshift). + broken-pair downshift on either arm: **rated** (a gigabit-capable port negotiated + down to 10/100) or **observed** (a port running below a speed it held itself, + which is what carries the check past gigabit onto 2.5G/10G links). * :class:`DuplexMismatchDetector` (``wired.duplex_mismatch``) — half-duplex on a modern (>=100 Mbps) up link. * :class:`PortFlappingDetector` (``wired.port_flapping``) — link transitions above - a short/long tier; PoE-draw-to-zero between flaps flags a reboot loop; infra + a short/long/daily tier; PoE-draw-to-zero between flaps flags a reboot loop; infra (uplink) ports escalate to P1. * :class:`UplinkSaturationDetector` (``wired.uplink_saturation``) — uplink utilisation past a % of negotiated speed with rising ``tx_dropped``, checked @@ -37,6 +39,7 @@ from __future__ import annotations +import math from functools import lru_cache from typing import Any, Iterable, Optional @@ -150,21 +153,33 @@ def _known_100mbps_patterns() -> tuple[str, ...]: # Shared helpers # ---------------------------------------------------------------------- # def _as_int(value: Any) -> Optional[int]: + # OverflowError, not just TypeError/ValueError: json.loads accepts `Infinity` + # by default and port.meta comes straight from stored controller JSON, so + # int(float("inf")) is reachable from one malformed field. Uncaught it takes + # down the whole detector pass -- every port on the site goes unjudged + # because one port reported nonsense. if value is None: return None try: - return int(float(value)) + f = float(value) except (TypeError, ValueError): return None + if not math.isfinite(f): + return None + try: + return int(f) + except (OverflowError, ValueError): + return None def _as_float(value: Any) -> Optional[float]: if value is None: return None try: - return float(value) + f = float(value) except (TypeError, ValueError): return None + return f if math.isfinite(f) else None def _as_bool(value: Any) -> Optional[bool]: @@ -433,39 +448,234 @@ def _downshift( switches: dict[int, Entity], confounders: list[str], ) -> Optional[dict[str, Any]]: - """Gigabit-capable port negotiated at 10/100, peer not a known 10/100 class.""" + """A link running below the speed it has been proven able to reach. + + Two arms, because there are two ways to prove the peer can do better: + + * **Rated** — a gigabit-capable port negotiated at 10/100, peer not a + known 10/100 class. The port's advertised ceiling is the proof. + * **Observed** — a port sitting below a speed it has *itself* linked at + recently. The port's own history is the proof, and it is what extends + this check past gigabit: on a 2.5G/10G port a fall to 1000 is the same + broken-pair symptom, but the rated arm can never see it, since 1000 is + a perfectly ordinary speed for a 2.5G port carrying a 1G device. Only + a port that *has* run at 2500 tells us 1000 is a regression. + """ cap = _as_int(port.meta.get("max_speed")) or _speed_caps_max(port.meta.get("speed_caps")) - if cap is None or cap < 1000: - return None # cannot assert the port is gigabit-capable neg = _as_int(ctx.repo.current_state(port.entity_id, "speed")) - if neg is None or neg >= 1000 or neg <= 0: - return None - # Confounder: a peer that is 10/100 by design is not a bad cable. Only a - # 100 Mbps link can be explained that way -- a 10/100 device sitting at - # *10* is 100BASE-TX falling back, which is the broken-pair signature this - # arm exists to catch, so it is never explained away by device class. + if neg is None or neg <= 0: + return None # 0/absent is "link down" — the flapping detector's beat + + # Measure the port's own history FIRST, because it outranks every other + # signal here. A speed this link demonstrably held is proof about the peer; + # the rated ceiling is proof only about the switch, and the 10/100 device + # class list is a guess from a device *name*. Deciding arm 1 before looking + # would let the name-based guess veto the measurement -- a port that held + # 1000 Mbps for a month would be dismissed as "a known 10/100 class" + # because its peer happens to be called "printer". + observed, observed_ts, held = self._observed_max_speed(ctx, port) + proven_faster = ( + observed is not None + and observed > neg + and self._peer_predates(ctx, port, switches, observed_ts) + ) + # Overriding the device-class list needs more than *any* sighting of a + # faster link: it needs the faster speed to be what this link normally + # runs at. A real G6 Turret -- 10/100 by Ubiquiti's own spec, working + # correctly -- blipped to 1000 six times inside one 20-minute cabling + # event, then sat at 100 for five days. Twelve minutes of anomaly must + # not outvote five days of steady, correct operation and send someone to + # replace a healthy camera's cable. Thirty days at 1000 against two hours + # at 100 is a different claim entirely, and still wins. + dominates = proven_faster and held.get(observed, 0) > held.get(neg, 0) + + # Arm 1: rated ceiling. Gigabit-capable port down at 10/100. + if cap is not None and cap >= 1000 and neg < 1000: + if not dominates and self._explained_by_peer_class( + ctx, port, switches, neg, cap, confounders + ): + return None + confounders.append("port_gigabit_capable") + evidence: dict[str, Any] = {"negotiated_speed": neg, "port_capable_speed": cap} + if proven_faster: + # The strongest evidence available, and on the severest downshift: + # this very link, with this very peer, has run faster than this. + confounders.append("peer_predates_observed_speed") + confounders.append("observed_speed_regression") + evidence["observed_speed_max"] = observed + return evidence + + # Arm 2: observed ceiling. Below a speed this very port has held recently. + if observed is not None and observed >= 1000 and neg < observed: + if self._explained_by_peer_class(ctx, port, switches, neg, observed, confounders): + return None + if not proven_faster: + _log.info( + "bad_cable: %s negotiated %s below an observed %s Mbps, downshift not " + "reported -- the wired peer on this port is newer than that speed", + port.native_id, + neg, + observed, + ) + return None + confounders.append("peer_predates_observed_speed") + confounders.append("observed_speed_regression") + evidence = {"negotiated_speed": neg, "observed_speed_max": observed} + if cap is not None: + evidence["port_capable_speed"] = cap + return evidence + return None + + def _observed_max_speed( + self, ctx: Any, port: Entity + ) -> tuple[Optional[int], Optional[int], dict[int, int]]: + """Highest speed this port actually *held*, when it last held it, and the + full held-seconds map (callers weigh one speed's dwell against another's). + + Measured as time-held, not as a count of rows, and that distinction is the + whole detector on a cleanly-degrading link. ``record_state_change`` writes + only on change, so a port that ran at 2500 for a week carries exactly ONE + row saying 2500 -- dated a week ago. Counting rows inside the window finds + nothing and the degradation that follows goes unreported, which is the + commonest shape of all: a cable damaged once, renegotiated down once, and + sitting there ever since. So the value in effect *entering* the window + (the last row at or before its start) seeds the timeline -- and ``prune`` + deliberately preserves that row for exactly this read. + + Held time is summed across the window rather than measured as one + contiguous run, because a flapping link earns its ceiling in fragments: + the port this detector was written for reached 2.5G in bursts between + drops, and demanding one unbroken stretch would dismiss it. The floor + (``downshift_min_hold_s``) is therefore a total-exposure guard against a + stray poll, not a proof of stability. + + Bounded by ``downshift_lookback_s``: past that horizon a link that has run + slow for longer is simply this link's normal, and we have no standing to + call it a regression. Rows are fetched BY WINDOW, never by row count -- a + count-limited fetch drops the oldest rows first, which is precisely where + the seed lives, so the harder a link flapped the more certainly its + ceiling would be truncated away. + """ + lookback_s = int(ctx.threshold(self.key, "downshift_lookback_s", 7 * 86400)) + min_hold_s = int(ctx.threshold(self.key, "downshift_min_hold_s", 300)) + start = ctx.now_ts - lookback_s + + rows = ctx.repo.list_state_changes( + start, ctx.now_ts + 1, entity_id=port.entity_id, attr="speed", limit=100_000 + ) + timeline: list[tuple[int, Optional[int]]] = [ + (int(r["ts"]), _as_int(r["new_value"])) for r in rows + ] + timeline.sort(key=lambda x: x[0]) + + # Seed with whatever was in effect entering the window (ts <= start). + prior = ctx.repo.list_state_changes( + 0, start + 1, entity_id=port.entity_id, attr="speed", limit=1 + ) + if prior: + timeline.insert(0, (start, _as_int(prior[0]["new_value"]))) + if not timeline: + return None, None, {} + + held: dict[int, int] = {} + last_at: dict[int, int] = {} + for i, (ts, val) in enumerate(timeline): + end = timeline[i + 1][0] if i + 1 < len(timeline) else ctx.now_ts + if val is None or val <= 0: + continue # 0/absent is link-down, which holds no speed at all + held[val] = held.get(val, 0) + max(0, end - ts) + last_at[val] = max(last_at.get(val, end), end) + + qualified = [s for s, dur in held.items() if dur >= min_hold_s] + if not qualified: + return None, None, held + best = max(qualified) + return best, last_at.get(best), held + + def _peer_predates( + self, + ctx: Any, + port: Entity, + switches: dict[int, Entity], + observed_ts: Optional[int], + ) -> bool: + """True when the wired peer on this port is old enough to own the history. + + The observed arm reads a speed the *port* once held and attributes it to + the device on that port now. Swap a 2.5G workstation for a 1G printer and + that inference inverts: the printer looks like a broken pair for as long + as the lookback runs. So a peer first seen *after the link last held that + speed* cannot be credited with it. + + The comparison is against ``observed_ts`` -- when the speed was actually + held -- and not against the start of the lookback window. Those differ in + both directions and both are wrong: a peer present for the whole fast + stretch but first seen mid-window would be rejected despite having + demonstrably run at that speed, and on a store younger than the lookback + *every* peer would read as a newcomer, silently disabling this arm across + the entire site until the database aged past a week. + + Where several peers claim one port, the NEWEST wins. A departed device's + entity lingers with its old ``sw_port``, so taking the oldest would let a + ghost vouch for the newcomer that replaced it -- the exact swap this + guard exists to catch. + + Unknown peers (no port map, no wired client, no first-seen) return True: + this guard only ever *suppresses* on positive evidence of a newcomer, + never on absence. Note this leaves it inert on infra ports, whose peer is + an AP or switch rather than a client. + """ + if observed_ts is None: + return True switch = switches.get(port.parent_id) if port.parent_id is not None else None - if switch is not None and neg == 100: - candidates = _peers_on_port(ctx, switch.entity_id, port) - if candidates: - confounders.append("known_100mbps_device_class") - match = next((c for c in candidates if _matches_known_100mbps(c)), None) - if match is not None: - # Say so. A suppressed finding is never constructed, so the - # confounder list dies with it and the operator is left with - # an absence they cannot explain -- exactly the silence that - # made the un-shipped device KB invisible for so long. - _log.info( - "bad_cable: %s negotiated %s of %s Mbps, downshift not reported " - "-- peer %r is a known 10/100-by-design class", - port.native_id, - neg, - cap, - match.name, - ) - return None - confounders.append("port_gigabit_capable") - return {"negotiated_speed": neg, "port_capable_speed": cap} + if switch is None: + return True + peers = _peers_on_port(ctx, switch.entity_id, port) + first_seen = [p.first_seen_ts for p in peers if p.first_seen_ts is not None] + if not first_seen: + return True + return max(int(f) for f in first_seen) <= int(observed_ts) + + def _explained_by_peer_class( + self, + ctx: Any, + port: Entity, + switches: dict[int, Entity], + neg: int, + ceiling: int, + confounders: list[str], + ) -> bool: + """True when a peer that is 10/100 by design explains the negotiated speed. + + Only a 100 Mbps link can be explained that way -- a 10/100 device sitting + at *10* is 100BASE-TX falling back, which is the broken-pair signature this + check exists to catch, so it is never explained away by device class. + """ + if neg != 100: + return False + switch = switches.get(port.parent_id) if port.parent_id is not None else None + if switch is None: + return False + candidates = _peers_on_port(ctx, switch.entity_id, port) + if not candidates: + return False + confounders.append("known_100mbps_device_class") + match = next((c for c in candidates if _matches_known_100mbps(c)), None) + if match is None: + return False + # Say so. A suppressed finding is never constructed, so the confounder + # list dies with it and the operator is left with an absence they cannot + # explain -- exactly the silence that made the un-shipped device KB + # invisible for so long. + _log.info( + "bad_cable: %s negotiated %s of %s Mbps, downshift not reported " + "-- peer %r is a known 10/100-by-design class", + port.native_id, + neg, + ceiling, + match.name, + ) + return True def _speed_caps_max(caps: Any) -> Optional[int]: @@ -557,9 +767,16 @@ def evaluate(self, ctx: Any) -> EvalResult: # wired.port_flapping # ====================================================================== # class PortFlappingDetector: - """``wired.port_flapping`` — link transitions above a short/long tier. + """``wired.port_flapping`` — link transitions above a short/long/daily tier. + + Counts recorded ``up`` transitions in a short (10 min), long (1 h) and + sustained (24 h) window. The sustained tier exists because the first two only + see a link that is failing *fast*: a port that drops once or twice an hour, + all day and all night, never puts 5 transitions in any 10-minute window and + so stayed invisible. That slow-burn shape is what a marginal cable, a + power-managed NIC or a dying USB adapter actually produces, and it is just as + disruptive to the person using the port — so it carries the same severity. - Counts recorded ``up`` transitions in a short (10 min) and long (1 h) window. A PoE port whose draw falls to ~0 between transitions is a powered-device reboot loop (recorded in evidence). Infra/uplink ports escalate to P1. """ @@ -573,24 +790,44 @@ def evaluate(self, ctx: Any) -> EvalResult: return UNKNOWN short_s = int(ctx.threshold(self.key, "window_short_s", 600)) long_s = int(ctx.threshold(self.key, "window_long_s", 3600)) + sustained_s = int(ctx.threshold(self.key, "window_sustained_s", 86400)) n_short = int(ctx.threshold(self.key, "transitions_short", 5)) n_long = int(ctx.threshold(self.key, "transitions_long", 10)) + n_sustained = int(ctx.threshold(self.key, "transitions_sustained", 12)) poe_floor = float(ctx.threshold(self.key, "poe_reboot_floor_w", 0.5)) switches = _switches_by_id(ctx) findings: list[Finding] = [] for port in _ports(ctx): - history = ctx.repo.state_history(port.entity_id, "up", limit=500) + # A day of transitions on a badly flapping port is still only a few + # hundred rows, but ask for enough that the daily count is not capped. + history = ctx.repo.state_history(port.entity_id, "up", limit=2000) short_ct = sum(1 for r in history if int(r["ts"]) >= ctx.now_ts - short_s) long_ct = sum(1 for r in history if int(r["ts"]) >= ctx.now_ts - long_s) - if short_ct < n_short and long_ct < n_long: + sustained_ct = sum(1 for r in history if int(r["ts"]) >= ctx.now_ts - sustained_s) + + # Tightest tier that tripped wins the headline: a port doing 6 in ten + # minutes is a different story from one doing 16 across a day, and the + # title must not quote a window that did not fire (a real stored issue + # reads "(0 transitions/10m)", which makes a true finding look broken). + tier = None + if short_ct >= n_short: + tier = (short_ct, "10m") + elif long_ct >= n_long: + tier = (long_ct, "1h") + elif sustained_ct >= n_sustained: + tier = (sustained_ct, "24h") + if tier is None: continue + tier_ct, tier_label = tier evidence: dict[str, Any] = { "transitions_short": short_ct, "transitions_long": long_ct, + "transitions_sustained": sustained_ct, "window_short_s": short_s, "window_long_s": long_s, + "window_sustained_s": sustained_s, } confounders = ["coverage_gated", "sustained_transition_count"] @@ -612,7 +849,7 @@ def evaluate(self, ctx: Any) -> EvalResult: self.key, port, sev, - f"Port flapping: {label} ({short_ct} transitions/10m)", + f"Port flapping: {label} ({tier_ct} transitions/{tier_label})", evidence, confounders, ) diff --git a/netadmin/store/repository.py b/netadmin/store/repository.py index 3610284..9d37976 100644 --- a/netadmin/store/repository.py +++ b/netadmin/store/repository.py @@ -1133,17 +1133,32 @@ def prune( "DELETE FROM poll_runs WHERE ts < ?", (raw_before,) ).rowcount events_deleted = conn.execute("DELETE FROM events WHERE ts < ?", (raw_before,)).rowcount - # state_changes is pruned past the raw window too, but the *latest* - # row per (entity, attr) is always kept even when it is older than the - # window: it is the current value current_state() reads back, so - # dropping it would erase the entity's present state (e.g. a firmware - # set once a year ago and never changed since). Only superseded - # history past the window is discarded. + # state_changes is pruned past the raw window too, but TWO rows per + # (entity, attr) are always kept even when older than the window, + # because both answer "what was the value at time T" for a T we still + # serve: + # + # * the *latest* row -- the current value current_state() reads back, + # so dropping it would erase the entity's present state (e.g. a + # firmware set once a year ago and never changed since). + # * the last row at or before the cutoff -- the value in effect + # *entering* the retained window. Without it, any consumer + # reconstructing a timeline sees an attribute that appears to + # spring into existence mid-window. A port that linked at 2500 + # once, a year ago, and held it until last week records exactly + # one 2500 row, dated a year back: prune it and the whole retained + # window looks like the port has only ever run at its degraded + # speed. wired.bad_cable's observed-ceiling arm reads precisely + # this, and silently stops reporting without it. + # + # Both are one row per (entity, attr), so the bound stays trivial. state_changes_deleted = conn.execute( "DELETE FROM state_changes WHERE ts < ? AND id NOT IN (" " SELECT MAX(id) FROM state_changes GROUP BY entity_id, attr" + ") AND id NOT IN (" + " SELECT MAX(id) FROM state_changes WHERE ts < ? GROUP BY entity_id, attr" ")", - (raw_before,), + (raw_before, raw_before), ).rowcount return { "raw": int(raw_deleted), diff --git a/tests/netadmin/detect/detectors/test_wired.py b/tests/netadmin/detect/detectors/test_wired.py index f18e7f5..a601f3a 100644 --- a/tests/netadmin/detect/detectors/test_wired.py +++ b/tests/netadmin/detect/detectors/test_wired.py @@ -306,6 +306,420 @@ def test_bad_cable_downshift_ignored_when_not_gigabit_capable(repo: Repository) assert BadCableDetector().evaluate(_ctx(repo)) == [] +# ---------------------------------------------------------------------- # +# Observed-speed regression: the multi-gig arm of the downshift check. +# +# The absolute arm only ever fires below 1000 Mbps, which was right when gigabit +# was the ceiling. On a 2.5G/10G port a fall to 1000 is the SAME broken-pair +# symptom and went unreported — confirmed on a real site where a 2.5G port sat +# at 1000 for hours between flaps and wired.bad_cable stayed silent throughout. +# +# The peer's own history is what makes this safe: a port that has linked at 2500 +# proves its peer can do 2500, so sitting below that is a regression. A 1G device +# on a 2.5G port never linked at 2500, so it never trips this arm. +# ---------------------------------------------------------------------- # +def _seed_speeds(repo: Repository, pid: int, speeds, *, end_ts: int = NOW, step: int = 60) -> None: + """Walk a port's negotiated speed through ``speeds``, oldest first.""" + start = end_ts - (len(speeds) - 1) * step + for i, s in enumerate(speeds): + repo.record_state_change(pid, "speed", s, ts=start + i * step) + + +def test_bad_cable_downshift_fires_when_multigig_port_falls_to_gigabit(repo: Repository) -> None: + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + # The real flapping shape: hours at 2.5G broken by short drops, now at 1000. + # Time-scaled like the site it came from — a link holds its speed for stretches + # between flaps, so 2500 accumulates far past the minimum-hold floor. + _seed_speeds(repo, pid, [2500, 0, 2500, 0, 1000], step=3600) + seed_counter(repo, pid, "rx_errors", step=0) # clean errors; the regression is the signal + + findings = BadCableDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + f = findings[0] + assert "speed_downshift" in f.evidence["signals"] + assert f.evidence["negotiated_speed"] == 1000 + assert f.evidence["observed_speed_max"] == 2500 + assert "observed_speed_regression" in f.confounders_checked + + +def test_bad_cable_downshift_ignores_gigabit_peer_that_never_linked_faster( + repo: Repository, +) -> None: + """The false-positive guard that matters: most 2.5G ports carry 1G devices. + + The 1000 must be held for a long, *qualifying* stretch — seeded as a brief + blip this passes for the wrong reason (the minimum-hold floor rejects it + before the comparison is ever reached) and would keep passing even if + ``neg < observed`` were mutated to ``neg <= observed``. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 30 * 86400) + seed_counter(repo, pid, "rx_errors", step=0) + + det = BadCableDetector() + ctx = _ctx(repo) + # The ceiling really is observed at 1000 — this is not a min-hold rejection. + port = next(p for p in ctx.entities(EntityType.PORT) if p.entity_id == pid) + assert det._observed_max_speed(ctx, port)[0] == 1000 + assert det.evaluate(ctx) == [] + + +def test_bad_cable_downshift_observed_arm_stays_above_gigabit(repo: Repository) -> None: + # The `observed >= 1000` floor. A 100 Mbps-capable port that held 100 and now + # sits at 10 must not be reported through the observed arm: arm 1 cannot see + # it (cap < 1000) and arm 2 has no business inventing a sub-gigabit ceiling. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"max_speed": 100}, speed=None) + repo.record_state_change(pid, "speed", 100, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 10, ts=NOW - 2 * 3600) + seed_counter(repo, pid, "rx_errors", step=0) + assert BadCableDetector().evaluate(_ctx(repo)) == [] + + +def test_bad_cable_observed_history_overrides_the_10_100_device_class( + repo: Repository, +) -> None: + """A measurement beats a name. + + A port that held 1000 Mbps for a month proves its peer is not 10/100-by-design, + whatever the device is called. Suppressing on the name here would dismiss the + exact fault this detector exists to catch — and log a reason the evidence + disproves. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"max_speed": 1000}, speed=None) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 100, ts=NOW - 2 * 3600) + # Resident since long before the port held 1000, so that speed is its own. + repo.upsert_entity( + Entity( + entity_type=EntityType.CLIENT, + native_id="cli:printer", + site_id="default", + name="Brother HL-L2350DW printer", + parent_id=sw, + meta={"oui": "", "is_wired": True, "sw_port": 1}, + ), + ts=NOW - 40 * 86400, + ) + seed_counter(repo, pid, "rx_errors", step=0) + + findings = BadCableDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + f = findings[0] + assert f.evidence["negotiated_speed"] == 100 + assert f.evidence["observed_speed_max"] == 1000 + assert "observed_speed_regression" in f.confounders_checked + + +def test_bad_cable_10_100_class_still_suppresses_without_faster_history( + repo: Repository, +) -> None: + # The matched negative: same printer, but the port has never run above 100. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"max_speed": 1000}, speed=None) + repo.record_state_change(pid, "speed", 100, ts=NOW - 30 * 86400) + make_client(repo, sw_id=sw, name="Brother HL-L2350DW printer", sw_port=1) + seed_counter(repo, pid, "rx_errors", step=0) + assert BadCableDetector().evaluate(_ctx(repo)) == [] + + +def test_bad_cable_brief_fast_blip_does_not_overrule_the_device_class( + repo: Repository, +) -> None: + """From a real site, and the reason the override requires *dominance*. + + A G6 Turret — 10/100 by Ubiquiti's own spec, working perfectly — sat on a + gigabit port that blipped to 1000 six times inside one 20-minute cabling + event, then returned to 100 and stayed there for five days. Letting any + sighting of a faster link overrule the curated device-class list turns that + into "Cable/link fault" and sends someone to re-run cable to a healthy + camera. Twelve minutes of anomaly must not outvote five days of correct + operation; thirty days at gigabit against two hours at 100 still wins. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048623}, speed=None) + repo.record_state_change(pid, "speed", 100, ts=NOW - 6 * 86400) + # The 20-minute event: six short stretches at 1000, ~12 minutes in total — + # comfortably past the minimum-hold floor on its own. + t = NOW - 5 * 86400 + for i in range(6): + repo.record_state_change(pid, "speed", 1000, ts=t + i * 300) + repo.record_state_change(pid, "speed", 0, ts=t + i * 300 + 120) + repo.record_state_change(pid, "speed", 100, ts=NOW - 5 * 86400 + 3600) + make_client(repo, sw_id=sw, name="g6-turret---mailbox", sw_port=1) + seed_counter(repo, pid, "rx_errors", step=0) + + det = BadCableDetector() + ctx = _ctx(repo) + port = next(p for p in ctx.entities(EntityType.PORT) if p.entity_id == pid) + observed, _ts, held = det._observed_max_speed(ctx, port) + # The blip really does qualify as an observed ceiling — it is dominance, + # not the min-hold floor, that has to do the work here. + assert observed == 1000 + assert held[100] > held[1000] + assert det.evaluate(ctx) == [] + + +def test_bad_cable_error_rate_arm_does_not_inherit_the_peer_age_confounder( + repo: Repository, +) -> None: + """A suppressed arm must not leave its confounder on someone else's finding. + + ``confounders`` is the shared list handed to every finding ``_assess`` + produces. If the peer-age guard appends before it decides, an error-rate + finding ships claiming the peer predated a speed history that was in fact + rejected — and the LLM dossier prints every confounder key unconditionally, + so it reads as a trap the detector "tested and rejected". + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 2 * 3600) + make_client(repo, sw_id=sw, name="New Workstation", sw_port=1) # first seen just now + seed_counter(repo, pid, "rx_errors", step=20) # error arm fires on its own + + findings = BadCableDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + f = findings[0] + assert f.evidence["signals"] == ["error_rate"] # downshift correctly suppressed + assert "peer_predates_observed_speed" not in f.confounders_checked + assert "observed_speed_regression" not in f.confounders_checked + + +def test_bad_cable_reports_both_arms_together(repo: Repository) -> None: + # Errors AND a downshift on one port merge into a single finding. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 2 * 3600) + seed_counter(repo, pid, "rx_errors", step=20) + + f = BadCableDetector().evaluate(_ctx(repo))[0] + assert f.evidence["signals"] == ["error_rate", "speed_downshift"] + assert f.evidence["errors_per_min"] > 0 + assert f.evidence["observed_speed_max"] == 2500 + + +def test_bad_cable_downshift_fires_on_a_clean_degradation_with_no_flapping( + repo: Repository, +) -> None: + """The commonest shape of all, and the one a row-count check cannot see. + + ``record_state_change`` writes only on change, so a port that ran at 2500 for + a week carries exactly ONE row saying 2500 — dated a week ago. A cable damaged + once, renegotiated down once, and left there produces no further 2500 rows at + all. Only the speed *in effect entering the window* reveals the regression. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 2 * 3600) + seed_counter(repo, pid, "rx_errors", step=0) + + findings = BadCableDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + assert findings[0].evidence["observed_speed_max"] == 2500 + assert findings[0].evidence["negotiated_speed"] == 1000 + + +def test_bad_cable_downshift_ignores_speed_history_beyond_lookback(repo: Repository) -> None: + # Past the horizon, a link that has run slow for longer is simply this link's + # normal: it entered the window at 1000 and never held anything faster inside it. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 60 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 30 * 86400) + seed_counter(repo, pid, "rx_errors", step=0) + assert BadCableDetector().evaluate(_ctx(repo)) == [] + + +def test_bad_cable_downshift_ignores_a_speed_held_only_for_a_moment( + repo: Repository, +) -> None: + # A single garbled poll reading 2500 for one interval is not a ceiling. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 3600 - 60) # held 60 s + repo.record_state_change(pid, "speed", 1000, ts=NOW - 3600) + seed_counter(repo, pid, "rx_errors", step=0) + assert BadCableDetector().evaluate(_ctx(repo)) == [] + + +def test_bad_cable_downshift_suppressed_when_the_peer_is_newer_than_the_history( + repo: Repository, +) -> None: + """Swap a 2.5G workstation for a 1G printer and the inference inverts. + + The port really did hold 2500 — but not for *this* device, so crediting the + newcomer with it would report a perfectly healthy printer as a broken pair + for as long as the lookback runs. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 2 * 3600) + # The peer on the port today first appeared an hour ago: it cannot own the 2500. + repo.upsert_entity( + Entity( + entity_type=EntityType.CLIENT, + native_id="cli:new-printer", + site_id="default", + name="Office Printer", + parent_id=sw, + meta={"oui": "", "is_wired": True, "sw_port": 1}, + ), + ts=NOW - 3600, + ) + seed_counter(repo, pid, "rx_errors", step=0) + assert BadCableDetector().evaluate(_ctx(repo)) == [] + + +def test_bad_cable_downshift_suppressed_by_the_newest_claimant_not_the_oldest( + repo: Repository, +) -> None: + """A departed device's entity lingers on its old port with its old first-seen. + + Taking the oldest peer would let that ghost vouch for the newcomer that + replaced it — precisely the swap the guard exists to catch, and it would ship + a confidently-worded note asserting the printer had held 2500 itself. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 2 * 3600) + for native, name, ts in ( + ("cli:gone-workstation", "Workstation", NOW - 300 * 86400), # unplugged ghost + ("cli:new-printer", "Office Printer", NOW - 3600), # today's occupant + ): + repo.upsert_entity( + Entity( + entity_type=EntityType.CLIENT, + native_id=native, + site_id="default", + name=name, + parent_id=sw, + meta={"oui": "", "is_wired": True, "sw_port": 1}, + ), + ts=ts, + ) + seed_counter(repo, pid, "rx_errors", step=0) + assert BadCableDetector().evaluate(_ctx(repo)) == [] + + +def test_bad_cable_downshift_survives_a_store_younger_than_the_lookback( + repo: Repository, +) -> None: + """first_seen_ts is the ingest timestamp, not the controller's own first-seen. + + On a fresh install every client is younger than a 7-day lookback. Comparing + peer age against the window START would read every peer as a newcomer and + silently disable this arm across the whole site until the database aged past + a week. Comparing against when the speed was actually held is immune. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + # A four-day-old store: the port held 2500 for three days, then degraded. + repo.record_state_change(pid, "speed", 2500, ts=NOW - 4 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 3600) + repo.upsert_entity( + Entity( + entity_type=EntityType.CLIENT, + native_id="cli:workstation", + site_id="default", + name="Workstation", + parent_id=sw, + meta={"oui": "", "is_wired": True, "sw_port": 1}, + ), + ts=NOW - 4 * 86400, # first seen when the store was created + ) + seed_counter(repo, pid, "rx_errors", step=0) + + findings = BadCableDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + assert findings[0].evidence["observed_speed_max"] == 2500 + + +def test_bad_cable_downshift_ceiling_survives_a_hard_flapping_port( + repo: Repository, +) -> None: + """The ceiling must not be truncated away by sheer flap volume. + + A count-limited fetch drops the OLDEST rows first, which is exactly where the + seed lives — so the harder a link flapped, the more certainly its ceiling + would vanish. Fetching by window instead removes the coupling. + """ + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 6 * 86400) + # Thousands of flaps between the ceiling and now. + ts = NOW - 3 * 86400 + for i in range(6000): + repo.record_state_change(pid, "speed", 0 if i % 2 else 1000, ts=ts + i * 20) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 60) + seed_counter(repo, pid, "rx_errors", step=0) + + findings = BadCableDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + assert findings[0].evidence["observed_speed_max"] == 2500 + + +def test_bad_cable_downshift_fires_when_the_peer_predates_the_history( + repo: Repository, +) -> None: + # The matched positive: same port, same degradation, but a long-resident peer. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + repo.record_state_change(pid, "speed", 2500, ts=NOW - 30 * 86400) + repo.record_state_change(pid, "speed", 1000, ts=NOW - 2 * 3600) + repo.upsert_entity( + Entity( + entity_type=EntityType.CLIENT, + native_id="cli:workstation", + site_id="default", + name="Workstation", + parent_id=sw, + meta={"oui": "", "is_wired": True, "sw_port": 1}, + ), + ts=NOW - 40 * 86400, + ) + seed_counter(repo, pid, "rx_errors", step=0) + + findings = BadCableDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + assert "peer_predates_observed_speed" in findings[0].confounders_checked + assert "observed_speed_regression" in findings[0].confounders_checked + + +def test_bad_cable_downshift_skips_a_port_that_is_currently_down(repo: Repository) -> None: + # speed 0 is "link down", not "negotiated slow" — the flapping detector's job. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, meta={"speed_caps": 1048687}, speed=None) + _seed_speeds(repo, pid, [2500, 1000, 2500, 0]) + seed_counter(repo, pid, "rx_errors", step=0) + assert BadCableDetector().evaluate(_ctx(repo)) == [] + + def test_bad_cable_unknown_on_low_coverage(repo: Repository) -> None: low_coverage(repo) sw = make_switch(repo) @@ -403,6 +817,87 @@ def test_port_flapping_unknown_on_low_coverage(repo: Repository) -> None: assert PortFlappingDetector().evaluate(_ctx(repo)) is UNKNOWN +# ---------------------------------------------------------------------- # +# The sustained (daily) tier. +# +# The 10-minute and 1-hour tiers only see a port that is failing *fast*. A port +# that drops twice an hour, all day, never trips either — and that is the shape +# real marginal links take. Confirmed on a real site: a port dropped 55 times in +# two weeks (16 in one day) and wired.port_flapping never fired once, because no +# single 10-minute window ever held 5 transitions. +# ---------------------------------------------------------------------- # +def test_port_flapping_fires_on_sustained_daily_transitions(repo: Repository) -> None: + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, up=None) + # 16 transitions spread evenly across 24 h: ~1 per 90 min, so neither the + # 10-minute (>=5) nor the 1-hour (>=10) tier can possibly trip. + _seed_flaps(repo, pid, 16, span=86_000) + + findings = PortFlappingDetector().evaluate(_ctx(repo)) + assert len(findings) == 1 + f = findings[0] + assert f.detector_key == KEY_PORT_FLAPPING + assert f.evidence["transitions_short"] < 5 + assert f.evidence["transitions_long"] < 10 + assert f.evidence["transitions_sustained"] == 16 + assert f.evidence["window_sustained_s"] == 86_400 + assert f.severity is Severity.P2 + + +def test_port_flapping_quiet_below_sustained_threshold(repo: Repository) -> None: + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, up=None) + # 8 transitions = 4 drops in a day. Real links do this; it is not a fault. + _seed_flaps(repo, pid, 8, span=86_000) + assert PortFlappingDetector().evaluate(_ctx(repo)) == [] + + +def test_port_flapping_sustained_infra_port_is_p1(repo: Repository) -> None: + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, is_uplink=True, up=None) + _seed_flaps(repo, pid, 16, span=86_000) + assert PortFlappingDetector().evaluate(_ctx(repo))[0].severity is Severity.P1 + + +def test_port_flapping_title_names_the_window_that_actually_tripped(repo: Repository) -> None: + # The old title always quoted the 10-minute count, so a long- or sustained-tier + # trip rendered as "(0 transitions/10m)" — a real issue in the store reads + # exactly that way, and it makes a true finding look like a bug. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, up=None) + _seed_flaps(repo, pid, 16, span=86_000) + + title = PortFlappingDetector().evaluate(_ctx(repo))[0].title + assert "0 transitions" not in title + assert "16 transitions/24h" in title + + +def test_port_flapping_short_tier_still_titles_in_minutes(repo: Repository) -> None: + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, up=None) + _seed_flaps(repo, pid, 6) # 6 transitions in 10 min -> tightest tier wins + assert "6 transitions/10m" in PortFlappingDetector().evaluate(_ctx(repo))[0].title + + +def test_port_flapping_middle_tier_titles_in_hours(repo: Repository) -> None: + # The 1 h tier is the one that most often produced the "(0 transitions/10m)" + # title bug: short_ct=0 while long_ct clears its threshold. + full_coverage(repo) + sw = make_switch(repo) + pid = make_port(repo, sw_id=sw, idx=1, up=None) + _seed_flaps(repo, pid, 10, span=3500) # 10 in an hour, none in the last 10 min + + f = PortFlappingDetector().evaluate(_ctx(repo))[0] + assert f.evidence["transitions_short"] < 5 # short tier did not trip + assert f.evidence["transitions_long"] == 10 + assert "10 transitions/1h" in f.title + + # ====================================================================== # # wired.uplink_saturation # ====================================================================== # diff --git a/tests/netadmin/detect/test_catalog.py b/tests/netadmin/detect/test_catalog.py index b60017f..bbe2874 100644 --- a/tests/netadmin/detect/test_catalog.py +++ b/tests/netadmin/detect/test_catalog.py @@ -103,3 +103,74 @@ def test_catalog_is_iterable_and_sized() -> None: cat = build_catalog(list(DEFAULT_CATALOG.entries)) assert isinstance(cat, Catalog) assert len(list(cat)) == len(cat) == CATALOG_V1_SIZE + + +# ---------------------------------------------------------------------- # +# Presentation coverage for the wired downshift/flapping evidence. +# +# The issue page renders an evidence field only if the catalog declares it, and a +# confounder note only if the playbook has a closure for that key — both look up +# by string and skip silently on a miss. So a detector can emit new evidence and +# have it vanish from the UI with every test still green. These lock the keys the +# multi-gig downshift arm and the sustained-flapping tier actually emit. +# ---------------------------------------------------------------------- # +def _playbook(key: str): + return DEFAULT_CATALOG.get(key).playbook + + +def test_bad_cable_playbook_renders_observed_speed_regression() -> None: + pb = _playbook("wired.bad_cable") + assert "observed_speed_max" in {f.key for f in pb.evidence_fields} + + evidence = {"negotiated_speed": 1000, "observed_speed_max": 2500, "port_capable_speed": 2500} + note = pb.confounder_notes["observed_speed_regression"](evidence) + assert note and "2500" in note and "1000" in note + + +def test_bad_cable_observed_note_is_silent_without_its_evidence() -> None: + # The rated arm emits no observed_speed_max; the notes must not render a blank. + pb = _playbook("wired.bad_cable") + assert pb.confounder_notes["observed_speed_regression"]({"negotiated_speed": 100}) is None + assert pb.confounder_notes["peer_predates_observed_speed"]({"negotiated_speed": 100}) is None + + +def test_bad_cable_playbook_renders_the_peer_age_guard() -> None: + pb = _playbook("wired.bad_cable") + note = pb.confounder_notes["peer_predates_observed_speed"]({"observed_speed_max": 2500}) + assert note and "2500" in note + + +def test_port_flapping_playbook_renders_the_sustained_tier() -> None: + pb = _playbook("wired.port_flapping") + keys = {f.key for f in pb.evidence_fields} + assert {"transitions_sustained", "window_sustained_s"} <= keys + + evidence = { + "transitions_short": 0, + "transitions_long": 2, + "transitions_sustained": 38, + "window_short_s": 600, + "window_long_s": 3600, + "window_sustained_s": 86400, + } + note = pb.confounder_notes["sustained_transition_count"](evidence) + assert note and "38 in 24 h" in note + + +def test_port_flapping_note_omits_the_sustained_clause_on_older_evidence() -> None: + """Issues predating the sustained tier carry no such keys. + + A resolved port_flapping issue never gets its evidence refreshed, and the + demo seed writes the pre-tier shape too, so an unguarded f-string renders + "unknown in unknown" on the public demo forever. + """ + pb = _playbook("wired.port_flapping") + old = { + "transitions_short": 6, + "transitions_long": 9, + "window_short_s": 600, + "window_long_s": 3600, + } + note = pb.confounder_notes["sustained_transition_count"](old) + assert note and "unknown" not in note + assert note.endswith("9 in 1 h.") diff --git a/tests/netadmin/detect/test_unit_convention.py b/tests/netadmin/detect/test_unit_convention.py index 14d2233..5c3480c 100644 --- a/tests/netadmin/detect/test_unit_convention.py +++ b/tests/netadmin/detect/test_unit_convention.py @@ -99,6 +99,7 @@ "min_baseline_delta", # dimensionless baseline delta "transitions_long", # transition count (long window); count, no unit "transitions_short", # transition count (short window); count, no unit + "transitions_sustained", # transition count (24 h window); count, no unit } ) diff --git a/tests/netadmin/llm/fixtures/dossier_bad_cable.golden.md b/tests/netadmin/llm/fixtures/dossier_bad_cable.golden.md index a728006..a01a8c6 100644 --- a/tests/netadmin/llm/fixtures/dossier_bad_cable.golden.md +++ b/tests/netadmin/llm/fixtures/dossier_bad_cable.golden.md @@ -37,7 +37,7 @@ The detector tested and rejected these false-positive traps: - Counter age - Unmanaged switch hop -**Traps this class of problem is known for:** Known 100 Mbps device classes; counter age (a stale cumulative counter); an unmanaged-switch hop hiding the real port. +**Traps this class of problem is known for:** Known 100 Mbps device classes (overruled only by an observed ceiling that outlasts the current speed, so a brief blip cannot condemn a 10/100 device); counter age (a stale cumulative counter); an unmanaged-switch hop hiding the real port; a wired peer newer than the speed it would be credited with (a faster device swapped out for a slower one). ## Related issues @@ -78,8 +78,8 @@ Inventory: 1 ap, 1 port, 1 switch. ## Detector playbook — `wired.bad_cable` -- **Signature:** rx_errors delta rate > 10/min sustained or > 0.001% of packets; OR a gigabit-capable peer negotiated at 10/100 (broken-pair downshift). -- **Confounders to rule out:** Known 100 Mbps device classes; counter age (a stale cumulative counter); an unmanaged-switch hop hiding the real port. +- **Signature:** rx_errors delta rate > 10/min sustained or > 0.001% of packets; OR a gigabit-capable peer negotiated at 10/100; OR a port running below a speed it held itself in the last 7 days (broken-pair downshift, rated or observed). +- **Confounders to rule out:** Known 100 Mbps device classes (overruled only by an observed ceiling that outlasts the current speed, so a brief blip cannot condemn a 10/100 device); counter age (a stale cumulative counter); an unmanaged-switch hop hiding the real port; a wired peer newer than the speed it would be credited with (a faster device swapped out for a slower one). - **Fix guidance:** Reseat then replace the patch cable; re-test the run. On an uplink port this is P1: the whole segment rides it. ## STRUCTURED QUESTIONS diff --git a/tests/netadmin/store/test_repository_findings.py b/tests/netadmin/store/test_repository_findings.py index bbe7f66..bb5b42b 100644 --- a/tests/netadmin/store/test_repository_findings.py +++ b/tests/netadmin/store/test_repository_findings.py @@ -83,11 +83,18 @@ def test_prune_covers_poll_runs_events_and_state_changes( repo.record_event(ts=old, key="EVT_OLD", native_id="e-old") repo.record_event(ts=recent, key="EVT_NEW", native_id="e-new") + ancient = now - 90 * DAY_SECONDS + eid = switch_entity_id # firmware set once long ago and never changed: its single old row must survive # (it is the current value), so it must NOT be counted/pruned. repo.record_state_change(eid, "firmware", "v1", ts=old) - # speed changed: the old superseded row is prunable, the recent one is kept. + # speed changed twice. Two rows past the window are protected for different + # reasons and one is genuinely disposable: + # 10 @ ancient -- superseded AND older than the boundary: prunable. + # 100 @ old -- the value in effect ENTERING the retained window. + # 1000 @ recent -- the current value. + repo.record_state_change(eid, "speed", "10", ts=ancient) repo.record_state_change(eid, "speed", "100", ts=old) repo.record_state_change(eid, "speed", "1000", ts=recent) @@ -95,7 +102,7 @@ def test_prune_covers_poll_runs_events_and_state_changes( assert deleted["poll_runs"] == 1 assert deleted["events"] == 1 - assert deleted["state_changes"] == 1 # only the superseded old speed row + assert deleted["state_changes"] == 1 # only the doubly-superseded 10 Mbps row assert len(repo.read_poll_runs("device", 0, now + 1)) == 1 assert len(repo.read_events(0, now + 1)) == 1 @@ -103,6 +110,14 @@ def test_prune_covers_poll_runs_events_and_state_changes( assert repo.current_state(eid, "firmware") == "v1" assert repo.current_state(eid, "speed") == "1000" + # The boundary row survives too, so a timeline reconstructed over the retained + # window still knows the port entered it at 100 Mbps rather than appearing to + # spring into existence at 1000. wired.bad_cable's observed-ceiling arm reads + # exactly this row; without it a link that degraded last week but had been + # stable for months reports nothing at all. + kept = [r["new_value"] for r in repo.state_history(eid, "speed", limit=10)] + assert kept == ["1000", "100"] + # --------------------------------------------------------------------------- # Finding 8: WindowResult.rate() divides deltas by actual elapsed time