Re-evaluate incident severity on link, stamp resolved_at at every resolution site
#17: severity was written once at _create_incident and never touched again, so an incident that opened routine and escalated to a working fire stayed routine forever. _update_incident now merges call_severity into the incident via _max_severity() on every link. Severity is monotonic: it only ever rises, never falls. An incident briefly assessed "major" genuinely was major at that moment; a later, calmer-sounding call is evidence the situation is winding down, not that the earlier read was wrong. status/resolved_at exist to retire an incident — severity should stay as the high-water mark so the worst-first rail, "Major only" filter, and map colouring never bury a call that was genuinely major. See _max_severity's docstring in incident_correlator.py for the full argument. #18: none of the resolution sites wrote resolved_at, so an incident's lifespan couldn't be reconstructed for the history-scrub feature. Added resolved_at alongside status="resolved" at all six sites that flip it: - incident_correlator.py _update_incident (signal-based: units all cleared) - incident_correlator.py maybe_resolve_parent (master auto-resolve) - summarizer.py _stale_sweep (90-minute auto-resolve) - upload.py, both scene-resolution loops (single- and multi-scene) - calls.py reprocess/correction path (_update_incident's signal-resolve and maybe_resolve_parent's master-resolve weren't named in the issue's four call sites, but they set status the same way and were missing resolved_at too.) No backfill: existing resolved incidents keep resolved_at = null, which means "resolved before this field existed," not "never resolved." Backfilling from updated_at would be a guess dressed up as data. Tests: added to tests/test_correlator_gate.py, which needs no Firestore for the pure _max_severity cases and patches fstore for the _update_incident/ maybe_resolve_parent writes. Covers the escalation case (routine -> major), the no-downgrade case, and resolved_at on both the signal-resolve and master-resolve paths. 52/52 passing in that file; 83 passed / 10 pre-existing failures for drb-c2-core overall (baseline was 69/10 — the +14 is exactly the new tests, no regressions). Fixes #17, #18.
This commit is contained in:
@@ -18,6 +18,7 @@ from datetime import datetime, timedelta, timezone
|
||||
from unittest.mock import AsyncMock, patch
|
||||
from app.internal.incident_correlator import (
|
||||
_run_decision, _update_incident, _normalize_unit, _matching_units,
|
||||
_max_severity, maybe_resolve_parent,
|
||||
)
|
||||
|
||||
NOW = datetime(2026, 8, 16, 21, 0, 0, tzinfo=timezone.utc)
|
||||
@@ -247,3 +248,117 @@ def test_normalised_units_link_a_call_that_exact_match_would_orphan():
|
||||
call_units=["K-9A2"], is_thin_call=False, call_severity="routine",
|
||||
))
|
||||
assert decision["action"] == "link"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Issue #17 — severity re-evaluation as calls attach (monotonic ladder)
|
||||
#
|
||||
# Decision: severity only ever rises, never falls, as more calls link (see
|
||||
# _max_severity's docstring in incident_correlator.py for the full argument).
|
||||
# An incident briefly assessed "major" genuinely was major at that moment;
|
||||
# resolution (status/resolved_at), not a later calmer-sounding call, is what
|
||||
# retires it. These tests lock in both halves of that: escalation raises the
|
||||
# stored severity, and a later lower-severity call does not undo it.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("current,new,expected", [
|
||||
("routine", "major", "major"), # escalation — the motivating case
|
||||
("routine", "minor", "minor"),
|
||||
("minor", "moderate", "moderate"),
|
||||
("major", "routine", "major"), # calmer call does NOT downgrade
|
||||
("major", "minor", "major"),
|
||||
("moderate", "moderate", "moderate"), # tie
|
||||
(None, "moderate", "moderate"), # incident with no prior severity
|
||||
("major", None, "major"),
|
||||
("major", "bogus", "major"), # malformed value ranks as routine
|
||||
("bogus", "minor", "minor"),
|
||||
])
|
||||
def test_max_severity_is_monotonic(current, new, expected):
|
||||
assert _max_severity(current, new) == expected
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_escalating_call_raises_stored_incident_severity():
|
||||
"""The #17 motivating case: an incident opened routine, a later call is a
|
||||
working structure fire — the incident's severity must reflect it."""
|
||||
inc = _incident(2.0)
|
||||
inc["severity"] = "routine"
|
||||
with patch("app.internal.incident_correlator.fstore") as mock_fstore:
|
||||
mock_fstore.doc_set = AsyncMock()
|
||||
await _update_incident(
|
||||
inc, "call-2", 9048, "sys-1", [], None, None, [], [], None, NOW,
|
||||
call_severity="major",
|
||||
)
|
||||
updates = mock_fstore.doc_set.await_args.args[2]
|
||||
assert updates["severity"] == "major"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_calmer_followup_call_does_not_downgrade_severity():
|
||||
inc = _incident(2.0)
|
||||
inc["severity"] = "major"
|
||||
with patch("app.internal.incident_correlator.fstore") as mock_fstore:
|
||||
mock_fstore.doc_set = AsyncMock()
|
||||
await _update_incident(
|
||||
inc, "call-2", 9048, "sys-1", [], None, None, [], [], None, NOW,
|
||||
call_severity="routine",
|
||||
)
|
||||
updates = mock_fstore.doc_set.await_args.args[2]
|
||||
assert updates["severity"] == "major"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Issue #18 — every resolution site stamps resolved_at
|
||||
#
|
||||
# updated_at is not a substitute (thin/ack calls deliberately don't move it,
|
||||
# unrelated field updates do) and existing rows are left null, not backfilled
|
||||
# — null means "resolved before this field existed", not "never resolved".
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_signal_resolve_stamps_resolved_at():
|
||||
"""All tracked units clear -> _update_incident's own auto-resolve path."""
|
||||
inc = _incident(2.0)
|
||||
inc["units_active"] = ["6-Adam"]
|
||||
with patch("app.internal.incident_correlator.fstore") as mock_fstore:
|
||||
mock_fstore.doc_set = AsyncMock()
|
||||
# standalone incident — maybe_resolve_parent's own doc_get short-circuits on None
|
||||
mock_fstore.doc_get = AsyncMock(return_value=None)
|
||||
await _update_incident(
|
||||
inc, "call-2", 9048, "sys-1", [], None, None, [], [], None, NOW,
|
||||
cleared_units=["6-Adam"],
|
||||
)
|
||||
updates = mock_fstore.doc_set.await_args.args[2]
|
||||
assert updates["status"] == "resolved"
|
||||
assert updates["resolved_at"] == NOW.isoformat()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_master_auto_resolve_stamps_resolved_at():
|
||||
"""maybe_resolve_parent closes a master once every child has resolved."""
|
||||
child_a = {"incident_id": "child-a", "parent_incident_id": "master-1"}
|
||||
master = {
|
||||
"incident_id": "master-1",
|
||||
"status": "active",
|
||||
"child_incident_ids": ["child-a", "child-b"],
|
||||
}
|
||||
child_b_resolved = {"incident_id": "child-b", "status": "resolved"}
|
||||
|
||||
async def fake_doc_get(collection, doc_id):
|
||||
return {
|
||||
"child-a": child_a,
|
||||
"master-1": master,
|
||||
"child-b": child_b_resolved,
|
||||
}.get(doc_id)
|
||||
|
||||
with patch("app.internal.incident_correlator.fstore") as mock_fstore:
|
||||
mock_fstore.doc_get = AsyncMock(side_effect=fake_doc_get)
|
||||
mock_fstore.doc_set = AsyncMock()
|
||||
await maybe_resolve_parent("child-a")
|
||||
|
||||
mock_fstore.doc_set.assert_awaited_once()
|
||||
args = mock_fstore.doc_set.await_args.args
|
||||
assert args[0] == "incidents"
|
||||
assert args[1] == "master-1"
|
||||
assert args[2]["status"] == "resolved"
|
||||
assert "resolved_at" in args[2] and args[2]["resolved_at"]
|
||||
|
||||
Reference in New Issue
Block a user