diff --git a/.agents/skills/shared/metric-alarms.toml b/.agents/skills/shared/metric-alarms.toml index b567163b..f188457f 100644 --- a/.agents/skills/shared/metric-alarms.toml +++ b/.agents/skills/shared/metric-alarms.toml @@ -753,7 +753,17 @@ ratio_threshold = 0.50 min_volume = 100 streak_threshold = 3 severity = "WARN" -gates = ["synced-only", "validator-only"] +# NO synced-only gate (#3826): the denominator stellar_herder_pending_received_total +# only accrues while the node buffers future-slot SCP envelopes (crates/herder/src/ +# pending.rs) — i.e. at/near sync, the exact regime `synced-only` suppresses. With +# the gate on, the only condition that feeds the denominator was the one that +# disabled the alarm, making it structurally unfireable (0 over 13 days of +# archives). `min_volume = 100` plus eval_counter_ratio's own fresh_start / +# ledger_age>30s / uptime<600s guards already suppress catchup noise, so dropping +# the gate makes the alarm reachable without reintroducing that noise. +gates = ["validator-only"] +baseline_version = 2 +semantic_change_date = "2026-08-25T00:00:00Z" # gate removal via #3826 # pending counters are optional — if missing, only this check skips optional_counters = true cooldown_key = "pending_too_old_ratio" diff --git a/.claude/skills/shared/metric-alarms.toml b/.claude/skills/shared/metric-alarms.toml index b567163b..f188457f 100644 --- a/.claude/skills/shared/metric-alarms.toml +++ b/.claude/skills/shared/metric-alarms.toml @@ -753,7 +753,17 @@ ratio_threshold = 0.50 min_volume = 100 streak_threshold = 3 severity = "WARN" -gates = ["synced-only", "validator-only"] +# NO synced-only gate (#3826): the denominator stellar_herder_pending_received_total +# only accrues while the node buffers future-slot SCP envelopes (crates/herder/src/ +# pending.rs) — i.e. at/near sync, the exact regime `synced-only` suppresses. With +# the gate on, the only condition that feeds the denominator was the one that +# disabled the alarm, making it structurally unfireable (0 over 13 days of +# archives). `min_volume = 100` plus eval_counter_ratio's own fresh_start / +# ledger_age>30s / uptime<600s guards already suppress catchup noise, so dropping +# the gate makes the alarm reachable without reintroducing that noise. +gates = ["validator-only"] +baseline_version = 2 +semantic_change_date = "2026-08-25T00:00:00Z" # gate removal via #3826 # pending counters are optional — if missing, only this check skips optional_counters = true cooldown_key = "pending_too_old_ratio" diff --git a/scripts/lib/eval-alarms.py b/scripts/lib/eval-alarms.py index 5f206778..32c067d0 100644 --- a/scripts/lib/eval-alarms.py +++ b/scripts/lib/eval-alarms.py @@ -68,6 +68,29 @@ # a still-running tick). SKIP_INTERVAL_TOO_SHORT = "interval too short" +# Structural-inertness marker for counter-ratio alarms (#3826). When a ratio's +# denominator delta is 0 for INERT_ZERO_DEN_TICKS consecutive evaluation ticks, +# the alarm is not merely "quiet this tick" — its denominator is structurally +# empty, so the ratio can never be computed. eval_counter_ratio flips the +# skip_reason from the generic `low volume (delta=0 < …)` to +# `inert (denominator 0 for N ticks)` at that point, and render_aggregate +# surfaces it as ` inert (…)` so a permanently-dead ratio is visible in +# the tick report instead of masquerading as a routine low-volume skip. +INERT_ZERO_DEN_TICKS = 20 +SKIP_INERT_PREFIX = "inert" + +# Startup gate/metric contradiction lint (#3826). Maps a gate to the metric-name +# prefixes whose data-bearing regime that gate excludes: an alarm carrying the +# gate AND drawing an input from a mapped family is structurally unfireable (the +# gate suppresses the only regime that feeds the metric). Seeded with the single +# proven case — `synced-only` ⊥ the `stellar_herder_pending_*` family, whose +# counters accrue only while the node buffers future-slot envelopes (i.e. at/near +# sync, the regime synced-only suppresses). Keep this narrow; if it grows to +# model many gate⊥metric relationships, split it into a dedicated schema check. +GATE_METRIC_CONTRADICTIONS = { + "synced-only": ["stellar_herder_pending_"], +} + # Skip reasons that must NOT trigger a counter-snapshot reset (#3758). These are # monitoring-side caller errors (e.g. an abbreviated tick that failed to export # PID/START_TICKS) that carry ZERO information about the node — treating them as @@ -1396,6 +1419,11 @@ def eval_counter_ratio( prev_num_key = f"{alarm_name}_numerator" prev_den_key = f"{alarm_name}_denominator" streak_key = f"{alarm_name}_streak" + # Structural-inertness streak (#3826): consecutive evaluation ticks with + # den_delta == 0. Reset in every re-baseline branch (collecting_baseline, + # counter reset, gap_stale) so a fresh baseline never inherits a stale + # inert count, exactly like {name}_streak. + zero_den_key = f"{alarm_name}_zero_den_streak" if not snapshot or prev_num_key not in snapshot: # Collecting baseline — write current values @@ -1405,18 +1433,21 @@ def eval_counter_ratio( snapshot[prev_num_key] = str(int(cur_num)) snapshot[prev_den_key] = str(int(cur_den)) snapshot[streak_key] = "0" + snapshot[zero_den_key] = "0" write_snapshot(snapshot_path, snapshot) return make_result(alarm, "collecting_baseline", extra_values=ev_default) prev_num = int(snapshot[prev_num_key]) prev_den = int(snapshot[prev_den_key]) streak = int(snapshot.get(streak_key, "0")) + zero_den_streak = int(snapshot.get(zero_den_key, "0")) # Counter reset check if cur_num < prev_num or cur_den < prev_den: snapshot[prev_num_key] = str(int(cur_num)) snapshot[prev_den_key] = str(int(cur_den)) snapshot[streak_key] = "0" + snapshot[zero_den_key] = "0" write_snapshot(snapshot_path, snapshot) return make_result(alarm, "collecting_baseline", extra_values=ev_default) @@ -1434,6 +1465,7 @@ def eval_counter_ratio( snapshot[prev_num_key] = str(int(cur_num)) snapshot[prev_den_key] = str(int(cur_den)) snapshot[streak_key] = "0" + snapshot[zero_den_key] = "0" write_snapshot(snapshot_path, snapshot) return make_result(alarm, "collecting_baseline", extra_values=ev_default) @@ -1460,17 +1492,45 @@ def eval_counter_ratio( min_volume = alarm.get("min_volume", 0) if den_delta < min_volume: snapshot[streak_key] = "0" + if den_delta == 0: + # Structural inertness (#3826): the denominator did not move AT ALL + # this tick. Track how many consecutive ticks that has held; once it + # reaches INERT_ZERO_DEN_TICKS, flip the skip_reason so a permanently + # dead ratio is distinguishable from a merely quiet-but-working one. + zero_den_streak += 1 + snapshot[zero_den_key] = str(zero_den_streak) + write_snapshot(snapshot_path, snapshot) + if zero_den_streak >= INERT_ZERO_DEN_TICKS: + return make_result( + alarm, "skipped", + skip_reason=f"{SKIP_INERT_PREFIX} (denominator 0 for {zero_den_streak} ticks)", + extra_values=ev_default) + return make_result( + alarm, "skipped", + skip_reason=f"low volume (delta={int(den_delta)} < {min_volume})", + extra_values=ev_default) + # 0 < den_delta < min_volume — real (if small) denominator activity, so + # the ratio is NOT structurally inert; reset the zero-den streak. + snapshot[zero_den_key] = "0" write_snapshot(snapshot_path, snapshot) return make_result(alarm, "skipped", skip_reason=f"low volume (delta={int(den_delta)} < {min_volume})", extra_values=ev_default) # Compute ratio if den_delta == 0: + # Reachable only when min_volume == 0 (den_delta >= min_volume passed + # above with min_volume 0). Still a zero-denominator tick — advance the + # inert streak for consistency, though the result renders as ok. snapshot[streak_key] = "0" + zero_den_streak += 1 + snapshot[zero_den_key] = str(zero_den_streak) write_snapshot(snapshot_path, snapshot) return make_result(alarm, "ok", value=0, threshold=alarm["ratio_threshold"], extra_values=ev_default) + # Denominator moved — not inert; reset the zero-den streak. + snapshot[zero_den_key] = "0" + ratio = num_delta / den_delta ratio_op = alarm.get("ratio_op", ">") ratio_threshold = alarm["ratio_threshold"] @@ -1824,7 +1884,15 @@ def render_aggregate(results: list[dict], watcher_mode: bool) -> dict: elif r["state"] == "breach": parts.append(f"{short} breach ({r['details']})") elif r["state"] == "skipped": - parts.append(f"{short} skipped ({r.get('skip_reason', '')})") + sr = r.get("skip_reason", "") + # Structural inertness (#3826): render ` inert (…)` + # directly rather than ` skipped (…)`, so a ratio whose + # denominator has been 0 for many ticks reads as a visible + # permanent no-op instead of a routine low-volume skip. + if sr.startswith(SKIP_INERT_PREFIX): + parts.append(f"{short} {sr}") + else: + parts.append(f"{short} skipped ({sr})") elif r["state"] == "collecting_baseline": parts.append(f"{short} collecting baseline") else: @@ -2016,6 +2084,52 @@ def validate_catalog(catalog: dict) -> list[str]: return errors +def _alarm_input_metrics(alarm: dict) -> list[str]: + """Collect every metric name an alarm draws an input value from. + + Covers the scalar (`metric`, `numerator`, `denominator`) and list-sum + (`metric_sum`, `numerator_sum`, `denominator_sum`) forms. + """ + metrics: list[str] = [] + for key in ("metric", "numerator", "denominator"): + val = alarm.get(key) + if isinstance(val, str) and val: + metrics.append(val) + for key in ("metric_sum", "numerator_sum", "denominator_sum"): + val = alarm.get(key) + if isinstance(val, list): + metrics.extend(m for m in val if isinstance(m, str) and m) + return metrics + + +def lint_gate_metric_contradictions(catalog: dict) -> list[str]: + """Flag alarms whose gate set excludes the regime their inputs come from. + + A gate/metric contradiction (#3826) is an alarm carrying a gate whose + suppressed regime is the ONLY regime that feeds one of the alarm's input + metrics — the alarm is present, well-formed, and structurally unfireable. + Returns a list of non-fatal warning strings (empty when clean). Driven by + GATE_METRIC_CONTRADICTIONS; kept intentionally narrow (see that constant). + """ + warnings: list[str] = [] + for alarm in catalog.get("alarm", []): + gates = alarm.get("gates", []) + if not gates: + continue + name = alarm.get("name", "") + metrics = _alarm_input_metrics(alarm) + for gate in gates: + for prefix in GATE_METRIC_CONTRADICTIONS.get(gate, []): + hit = next((m for m in metrics if m.startswith(prefix)), None) + if hit is not None: + warnings.append( + f"alarm '{name}': gate '{gate}' excludes the regime that " + f"feeds input metric '{hit}' (prefix '{prefix}') — the " + f"alarm is structurally unfireable" + ) + return warnings + + # ── Main ───────────────────────────────────────────────────────────────────── def main() -> int: @@ -2073,6 +2187,13 @@ def main() -> int: print(f"SCHEMA ERROR: {e}", file=sys.stderr) return 1 + # Authoring-time gate/metric contradiction lint (#3826). NON-fatal: these + # surface a structurally-unfireable alarm (a gate excluding the only regime + # its inputs come from) as a loud stderr warning without failing the catalog + # or the tick, so a future author sees it at the point the alarm is written. + for w in lint_gate_metric_contradictions(catalog): + print(f"SCHEMA WARNING: {w}", file=sys.stderr) + if args.validate_only: print(json.dumps({"schema_version": SCHEMA_VERSION, "valid": True, "alarm_count": len(catalog.get("alarm", []))})) return 0 diff --git a/scripts/lib/test_eval_alarms_pending_inert.py b/scripts/lib/test_eval_alarms_pending_inert.py new file mode 100644 index 00000000..c9adc493 --- /dev/null +++ b/scripts/lib/test_eval_alarms_pending_inert.py @@ -0,0 +1,310 @@ +#!/usr/bin/env python3 +"""Regression + coverage tests for pending-too-old-ratio reachability (#3826). + +Three coupled fixes: + 1. The `synced-only` gate is removed from `pending-too-old-ratio` so the alarm + is reachable — the denominator only accrues at/near sync, the exact regime + `synced-only` suppressed. + 2. `eval_counter_ratio` tracks a persistent `{name}_zero_den_streak`; after + INERT_ZERO_DEN_TICKS consecutive `den_delta == 0` ticks the skip_reason + flips from the generic `low volume (…)` to `inert (denominator 0 for N + ticks)`, and `render_aggregate` surfaces it as ` inert (…)`. + 3. `lint_gate_metric_contradictions` flags any alarm whose gate set excludes + the regime its input metrics come from, so this dead-alarm shape is caught + at authoring time. +""" + +import importlib.util +import sys +import tempfile +from pathlib import Path + +# eval-alarms.py uses a hyphen, so we need importlib. +_spec = importlib.util.spec_from_file_location( + "eval_alarms", + Path(__file__).parent / "eval-alarms.py", +) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) + +read_snapshot = _mod.read_snapshot +write_snapshot = _mod.write_snapshot +eval_counter_ratio = _mod.eval_counter_ratio +render_aggregate = _mod.render_aggregate +gates_pass = _mod.gates_pass + +try: + import tomllib +except ModuleNotFoundError: + import tomli as tomllib # type: ignore[no-redef] + +CATALOG_PATH = ( + Path(__file__).parent.parent.parent + / ".claude" / "skills" / "shared" / "metric-alarms.toml" +) + + +# ── Helpers ────────────────────────────────────────────────────────────────── + +def _load_catalog() -> dict: + with open(CATALOG_PATH, "rb") as f: + return tomllib.load(f) + + +def _find_alarm(catalog: dict, name: str) -> dict: + for a in catalog.get("alarm", []): + if a.get("name") == name: + return a + raise AssertionError(f"alarm {name!r} not found in catalog") + + +def _ratio_alarm(**kwargs) -> dict: + alarm = { + "name": "test-ratio", + "kind": "counter-ratio", + "numerator": "num", + "denominator": "den", + "ratio_op": ">", + "ratio_threshold": 0.5, + "min_volume": 100, + "streak_threshold": 3, + "severity": "WARN", + } + alarm.update(kwargs) + return alarm + + +def _scrape(num: float, den: float, ledger_age: float = 5.0) -> dict: + return { + "num": [({}, num)], + "den": [({}, den)], + "stellar_ledger_age_current_seconds": [({}, ledger_age)], + } + + +def _eval(alarm, current, state_dir, **kw): + kwargs = dict( + pid="123", start_ticks="456", + fresh_start=False, crash_recovery=False, uptime=3600, + ) + kwargs.update(kw) + return eval_counter_ratio( + alarm, current, {}, state_dir, + kwargs.pop("pid"), kwargs.pop("start_ticks"), + fresh_start=kwargs.pop("fresh_start"), + crash_recovery=kwargs.pop("crash_recovery"), + uptime=kwargs.pop("uptime"), + **kwargs, + ) + + +# ── Fix 1: gate removed / reachable in synced regime ───────────────────────── + +def test_pending_too_old_ratio_gate_removed(): + """The catalog's pending-too-old-ratio no longer carries `synced-only`. + + Fails on main: gates == ["synced-only", "validator-only"]. + """ + alarm = _find_alarm(_load_catalog(), "pending-too-old-ratio") + assert alarm["gates"] == ["validator-only"], \ + f"expected [validator-only], got {alarm['gates']}" + + +def test_ratio_evaluates_in_synced_regime_after_gate_removal(): + """With the real catalog gate set, a synced-but-<15m node (uptime=700) is + NOT gated out, and a two-tick eval reaches a real breach/ok/firing state. + + Fails on main: catalog gates include `synced-only`, so gates_pass at + uptime=700 returns (False, "not synced (synced-only gate)"). + """ + gates = _find_alarm(_load_catalog(), "pending-too-old-ratio")["gates"] + ok, reason = gates_pass( + gates, warmup_remaining=0, fresh_start=False, + crash_recovery=False, uptime=700, monitor_mode="validator", + ) + assert ok is True, f"gate must pass at uptime=700, got ({ok}, {reason!r})" + + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + alarm = _ratio_alarm() + # Tick 1 — collecting baseline. + r1 = _eval(alarm, _scrape(0, 0), state_dir, uptime=700) + assert r1["state"] == "collecting_baseline", r1 + # Tick 2 — breach: 90 too-old of 100 received = 0.9 > 0.5. + r2 = _eval(alarm, _scrape(90, 100), state_dir, uptime=700) + assert r2["state"] in ("breach", "firing", "ok"), \ + f"second tick must evaluate, got {r2['state']} ({r2.get('skip_reason')})" + assert r2["state"] != "skipped", r2 + + +# ── Fix 2: inert rendering ─────────────────────────────────────────────────── + +def test_zero_denominator_streak_renders_inert(): + """After INERT_ZERO_DEN_TICKS consecutive den_delta==0 ticks the skip_reason + flips to `inert (…)` and render_aggregate surfaces `pending inert (…)`. + + Fails on main: always `low volume (delta=0 < 100)`; no inert concept. + """ + n = _mod.INERT_ZERO_DEN_TICKS + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + alarm = _ratio_alarm(name="pending-too-old-ratio") + # Baseline. + _eval(alarm, _scrape(0, 0), state_dir) + result = None + for _ in range(n): + result = _eval(alarm, _scrape(0, 0), state_dir) + assert result["state"] == "skipped", result + assert result["skip_reason"].startswith("inert (denominator 0 for"), \ + f"expected inert skip_reason, got {result['skip_reason']!r}" + + # Production always has three ratio alarms (scp / apply / pending); the + # per-alarm parts renderer runs whenever they are not ALL skipped. Give + # the inert `pending` result a healthy `scp` sibling so the realistic + # per-alarm path is exercised. + scp_ok = { + "name": "scp-accept-rate-low", "state": "ok", + "contributes_to": "metrics_ratio", "value": 0.02, + } + out = render_aggregate([scp_ok, result], watcher_mode=False) + line = out["metrics_ratio_line"] + assert "pending inert (denominator 0 for" in line, \ + f"expected inert render, got {line!r}" + assert "pending skipped" not in line, line + + +def test_inert_threshold_boundary(): + """Streak N-1 is still `low volume`; streak N flips to `inert`.""" + n = _mod.INERT_ZERO_DEN_TICKS + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + alarm = _ratio_alarm() + _eval(alarm, _scrape(0, 0), state_dir) # baseline + last = None + for i in range(1, n): # ticks 1..n-1 + last = _eval(alarm, _scrape(0, 0), state_dir) + assert last["skip_reason"].startswith("low volume"), \ + f"streak {n - 1} must still be low volume, got {last['skip_reason']!r}" + # tick n → inert + final = _eval(alarm, _scrape(0, 0), state_dir) + assert final["skip_reason"].startswith("inert (denominator 0 for"), \ + f"streak {n} must be inert, got {final['skip_reason']!r}" + + +def test_zero_denominator_streak_resets_on_volume(): + """A 0 < den_delta < min_volume tick resets the inert streak; a following + den_delta==0 tick restarts the streak from 1 (not from where it was).""" + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + alarm = _ratio_alarm(name="pending-too-old-ratio") + _eval(alarm, _scrape(0, 0), state_dir) # baseline num=0 den=0 + # A few zero-den ticks accumulate the streak. + for _ in range(3): + _eval(alarm, _scrape(0, 0), state_dir) + snap = read_snapshot(state_dir / "ratio_snapshot") + assert snap["pending-too-old-ratio_zero_den_streak"] == "3", snap + + # Small-but-nonzero denominator activity: den 0 → 40 (delta 40 < 100). + r = _eval(alarm, _scrape(0, 40), state_dir) + assert r["skip_reason"].startswith("low volume"), r + snap = read_snapshot(state_dir / "ratio_snapshot") + assert snap["pending-too-old-ratio_zero_den_streak"] == "0", \ + f"streak must reset on volume, got {snap.get('pending-too-old-ratio_zero_den_streak')!r}" + + # Next zero-den tick (den stays 40) restarts the streak at 1. + _eval(alarm, _scrape(0, 40), state_dir) + snap = read_snapshot(state_dir / "ratio_snapshot") + assert snap["pending-too-old-ratio_zero_den_streak"] == "1", snap + + +def test_zero_den_streak_resets_on_rebaseline(): + """A counter-reset re-baseline and a gap_stale re-baseline both zero the + `{name}_zero_den_streak` key (Critic A item).""" + # Counter reset (cur_den < prev_den). + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + snap_path = state_dir / "ratio_snapshot" + write_snapshot(snap_path, { + "version": "1", "pid": "123", "start_ticks": "456", + "myalarm_numerator": "10", "myalarm_denominator": "500", + "myalarm_streak": "0", "myalarm_zero_den_streak": "15", + }) + alarm = _ratio_alarm(name="myalarm") + r = _eval(alarm, _scrape(10, 100), state_dir) # den 500 → 100 = reset + assert r["state"] == "collecting_baseline", r + snap = read_snapshot(snap_path) + assert snap["myalarm_zero_den_streak"] == "0", \ + f"counter reset must zero zero_den_streak, got {snap.get('myalarm_zero_den_streak')!r}" + + # gap_stale re-baseline. + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + snap_path = state_dir / "ratio_snapshot" + write_snapshot(snap_path, { + "version": "1", "pid": "123", "start_ticks": "456", + "myalarm_numerator": "10", "myalarm_denominator": "50", + "myalarm_streak": "0", "myalarm_zero_den_streak": "15", + }) + alarm = _ratio_alarm(name="myalarm") + r = _eval(alarm, _scrape(20, 200), state_dir, gap_stale=True) + assert r["state"] == "collecting_baseline", r + snap = read_snapshot(snap_path) + assert snap["myalarm_zero_den_streak"] == "0", \ + f"gap_stale must zero zero_den_streak, got {snap.get('myalarm_zero_den_streak')!r}" + + +# ── Fix 3: startup contradiction lint ──────────────────────────────────────── + +def test_gate_metric_lint_flags_synced_only_pending(): + """lint_gate_metric_contradictions flags a synced-only alarm whose + denominator is a pending-family metric. + + Fails on main: function does not exist. + """ + catalog = {"alarm": [{ + "name": "synthetic-pending", + "kind": "counter-ratio", + "gates": ["synced-only"], + "numerator": "stellar_herder_pending_too_old_total", + "denominator": "stellar_herder_pending_received_total", + }]} + warnings = _mod.lint_gate_metric_contradictions(catalog) + assert warnings, "expected a contradiction warning, got none" + assert any("synthetic-pending" in w and "synced-only" in w for w in warnings), \ + f"warning must name alarm + gate, got {warnings!r}" + + +def test_lint_no_warning_for_sound_alarm(): + """A synced-only alarm over a metric that accrues DURING sync is sound.""" + catalog = {"alarm": [{ + "name": "scp-accept-rate-low", + "kind": "counter-ratio", + "gates": ["synced-only"], + "numerator": "henyey_scp_post_verify_total", + "denominator": "henyey_scp_post_verify_total", + }]} + assert _mod.lint_gate_metric_contradictions(catalog) == [], \ + "sound synced-only alarm must not warn" + + +def test_live_catalog_lint_clean(): + """The real post-fix catalog has no gate/metric contradictions.""" + warnings = _mod.lint_gate_metric_contradictions(_load_catalog()) + assert warnings == [], f"live catalog must be lint-clean, got {warnings!r}" + + +# ── Run tests ───────────────────────────────────────────────────────────────── + +if __name__ == "__main__": + tests = [v for k, v in sorted(globals().items()) if k.startswith("test_")] + passed = failed = 0 + for t in tests: + try: + t() + passed += 1 + print(f" PASS {t.__name__}") + except Exception as e: + failed += 1 + print(f" FAIL {t.__name__}: {e}") + print(f"\n{passed} passed, {failed} failed") + sys.exit(1 if failed else 0)