fix(isolation): distinguish accident links from same-numbered nodes

Prefer link endpoints for legacy accident IDs and allow explicit node selection. Read authoritative initial statuses and sources, verify source isolation, and separate direct isolation from newly disconnected supply nodes. Return validation errors as HTTP 422.

The node-first lookup misrouted adjacent Fengyang pipes 19025 and 19026 into unrelated regions. Regression cases fail against the previous algorithm and pass after this change.

tests/api/test_openapi_contract.py

@@ -254,14 +254,16 @@ def test_committed_write_refresh_failure_has_explicit_http_contract() -> None:
     assert response.json()["code"] == "materialized_view_refresh_failed_after_commit"
 
 
-def test_valve_isolation_runtime_accepts_frontend_query(monkeypatch) -> None:
+@pytest.mark.parametrize("element_type", ["auto", "link", "node"])
+def test_valve_isolation_runtime_accepts_frontend_query(monkeypatch, element_type) -> None:
     captured: dict[str, object] = {}
 
-    def fake_analyze_valve_isolation(network, accident_element, disabled_valves):
+    def fake_analyze_valve_isolation(network, accident_element, disabled_valves, accident_element_type):
         captured.update(
             network=network,
             accident_element=accident_element,
             disabled_valves=disabled_valves,
+            accident_element_type=accident_element_type,
             business_dsn=get_project_pgconn_string(network),
             timescale_dsn=get_project_timescale_pgconn_string(network),
         )
@@ -288,6 +290,7 @@ def test_valve_isolation_runtime_accepts_frontend_query(monkeypatch) -> None:
             ("accident_element", "P-1"),
             ("accident_element", "P-2"),
             ("disabled_valves", "V-9"),
+            *([] if element_type == "auto" else [("accident_element_type", element_type)]),
         ],
     )
 
@@ -297,11 +300,39 @@ def test_valve_isolation_runtime_accepts_frontend_query(monkeypatch) -> None:
         "network": "fengyang",
         "accident_element": ["P-1", "P-2"],
         "disabled_valves": ["V-9"],
+        "accident_element_type": element_type,
         "business_dsn": "postgresql://user:password@biz/fengyang",
         "timescale_dsn": "postgresql://user:password@timescale/fengyang",
     }
 
 
+@pytest.mark.parametrize(
+    "target,disabled,detail",
+    [("P", "typo", "Disabled valve"), ("missing", "V", "Accident element")],
+)
+def test_valve_isolation_invalid_ids_return_validation_error(monkeypatch, target, disabled, detail):
+    from app.services import valve_isolation as service
+
+    monkeypatch.setattr(
+        service, "get_valve_isolation_topology",
+        lambda _network: (["P:pipe:A:B", "V:valve:B:R"], ["R"], []),
+    )
+    monkeypatch.setattr(simulation_endpoint, "analyze_valve_isolation", service.analyze_valve_isolation)
+    app = FastAPI()
+    app.include_router(api_router, prefix="/api/v1")
+    _override_project_routing(app, ProjectContext(
+        project_id=uuid4(), project_code="project", user_id=uuid4(), project_role="member",
+    ))
+
+    response = TestClient(app, raise_server_exceptions=False).post(
+        "/api/v1/valve-isolation-analyses",
+        params={"accident_element": target, "disabled_valves": disabled},
+    )
+
+    assert response.status_code == 422
+    assert detail in response.json()["detail"]
+
+
 def test_scada_cleaning_runs_are_post() -> None:
     methods_by_path = {
         route.path: route.methods

tests/unit/test_valve_isolation.py

@@ -1,3 +1,5 @@
+import pytest
+
 from app.algorithms.valve_isolation import topology_search as valve
 
 
@@ -23,6 +25,183 @@ def test_isolatable_keeps_affected_node_ids_and_count():
     assert result["must_close_valves"] == ["V-close"]
 
 
+@pytest.fixture
+def colliding_topology():
+    # The reported adjacent pipes and their same-numbered nodes are in
+    # different valve-bounded regions. Keep this fixture independent of a DB.
+    return [
+        "19025:pipe:17038:17037",
+        "19026:pipe:17038:17039",
+        "AUTO_TCV_4:valve:17037:D",
+        "AUTO_TCV_5:valve:17039:E",
+        "AUTO_TCV_546:valve:17038:R",
+        "32252:pipe:19025:X",
+        "15383:pipe:19026:Y",
+        "V-wrong-25:valve:X:R",
+        "V-wrong-26:valve:Y:R",
+    ]
+
+
+@pytest.mark.parametrize("target", ["19025", "19026", ["19025", "19026"]])
+def test_adjacent_pipes_take_priority_over_same_numbered_nodes(colliding_topology, target):
+    result = valve.valve_isolation_analysis(colliding_topology, target)
+
+    assert result["affected_nodes"] == ["17037", "17038", "17039"]
+    assert result["must_close_valves"] == ["AUTO_TCV_4", "AUTO_TCV_5", "AUTO_TCV_546"]
+    assert result["optional_valves"] == []
+
+
+def test_explicit_node_type_resolves_the_other_side_of_an_id_collision(colliding_topology):
+    result = valve.valve_isolation_analysis(
+        colliding_topology, "19025", accident_element_type="node",
+    )
+
+    assert result["affected_nodes"] == ["19025", "X"]
+    assert result["must_close_valves"] == ["V-wrong-25"]
+
+
+def test_unambiguous_legacy_node_input_still_works():
+    result = valve.valve_isolation_analysis(["P:pipe:A:B", "V:valve:B:R"], "A")
+    assert result["affected_nodes"] == ["A", "B"]
+
+
+def test_explicit_link_type_does_not_fall_back_to_a_node():
+    with pytest.raises(ValueError, match="not found"):
+        valve.valve_isolation_analysis(
+            ["P:pipe:A:B"], "A", accident_element_type="link",
+        )
+
+
+def test_a_boundary_valve_does_not_prove_source_isolation():
+    result = valve.valve_isolation_analysis(
+        ["P:pipe:R:A", "V:valve:A:B"], "P", water_source_nodes=["R"],
+    )
+
+    assert result["isolatable"] is False
+    assert result["source_check_performed"] is True
+    assert result["isolation_failure_reason"] == "connected_to_water_source"
+    assert result["affected_nodes"] == []
+    assert result["affected_node_count"] == 2
+    assert result["supply_affected_nodes"] is None
+
+
+def test_supply_impact_includes_downstream_regions_but_not_preexisting_outages():
+    links = [
+        "P:pipe:A:B", "V-source:valve:A:R", "V-downstream:valve:B:C",
+        "P-downstream:pipe:C:D", "P-already-off:pipe:X:Y",
+    ]
+    result = valve.valve_isolation_analysis(links, "P", water_source_nodes=["R"])
+
+    assert result["isolatable"] is True
+    assert result["source_check_performed"] is True
+    assert result["affected_nodes"] == ["A", "B"]
+    assert result["affected_node_count"] == 2
+    assert result["supply_affected_nodes"] == ["A", "B", "C", "D"]
+    assert result["supply_affected_node_count"] == 4
+
+
+def test_closed_links_are_not_traversed_or_recommended_for_closure():
+    result = valve.valve_isolation_analysis(
+        ["P:pipe:A:B", "P-closed:pipe:B:R", "V:valve:A:R", "V-closed:valve:B:C"],
+        "P", water_source_nodes=["R"], closed_links=["P-closed", "V-closed"],
+    )
+
+    assert result["isolatable"] is True
+    assert result["affected_nodes"] == ["A", "B"]
+    assert result["must_close_valves"] == ["V"]
+    assert result["supply_affected_nodes"] == ["A", "B"]
+
+
+def test_disabled_valve_cannot_cut_the_source_path():
+    result = valve.valve_isolation_analysis(
+        ["P:pipe:A:B", "V-disabled:valve:A:R", "V-other:valve:B:C"],
+        "P", disabled_valves=["V-disabled"], water_source_nodes=["R"],
+    )
+
+    assert result["isolatable"] is False
+    assert result["isolation_failure_reason"] == "connected_to_water_source"
+
+
+def test_joint_accidents_fail_if_any_region_remains_connected_to_a_source():
+    result = valve.valve_isolation_analysis(
+        ["P1:pipe:A:B", "V1:valve:B:R", "P2:pipe:R:C", "V2:valve:C:D"],
+        ["P1", "P2"], water_source_nodes=["R"],
+    )
+    assert result["isolatable"] is False
+
+
+def test_missing_sources_do_not_claim_verified_isolation():
+    result = valve.valve_isolation_analysis(
+        ["P:pipe:A:B", "V:valve:B:C"], "P", water_source_nodes=[],
+    )
+    assert result["isolatable"] is False
+    assert result["source_check_performed"] is False
+    assert result["isolation_failure_reason"] == "water_sources_unavailable"
+    assert result["supply_affected_node_count"] is None
+
+
+@pytest.mark.parametrize("sources", [["R1"], ["R1", "R2"]])
+def test_all_water_sources_are_considered(sources):
+    result = valve.valve_isolation_analysis(
+        ["P:pipe:A:B", "V:valve:A:R1", "P2:pipe:B:R2"],
+        "P", water_source_nodes=sources,
+    )
+    assert result["isolatable"] is (len(sources) == 1)
+
+
+def test_empty_targets_are_rejected():
+    with pytest.raises(ValueError, match="At least one"):
+        valve.valve_isolation_analysis(["P:pipe:A:B"], [])
+
+
+def test_unknown_disabled_valve_is_rejected():
+    with pytest.raises(ValueError, match="Disabled valve"):
+        valve.valve_isolation_analysis(["P:pipe:A:B"], "P", disabled_valves=["typo"])
+
+
+def test_service_passes_authoritative_sources_statuses_and_element_type(monkeypatch):
+    from app.services import valve_isolation as service
+
+    projects = []
+
+    def snapshot(network):
+        projects.append(network)
+        return (
+            ["P:pipe:A:B", "P-closed:pipe:B:R", "V:valve:A:R"],
+            ["R"], ["P-closed"],
+        )
+
+    monkeypatch.setattr(service, "get_valve_isolation_topology", snapshot)
+    result = service.analyze_valve_isolation(
+        "trusted-project", ["A"], accident_element_type="node",
+    )
+
+    assert projects == ["trusted-project"]
+    assert result["isolatable"] is True
+    assert result["source_check_performed"] is True
+    assert result["must_close_valves"] == ["V"]
+    assert result["supply_affected_nodes"] == ["A", "B"]
+
+
+def test_parallel_links_preserve_an_open_bypass():
+    result = valve.valve_isolation_analysis(
+        ["P:pipe:A:B", "V:valve:B:R", "P-bypass:pipe:B:R"],
+        "P", water_source_nodes=["R"],
+    )
+    assert result["isolatable"] is False
+    assert result["isolation_failure_reason"] == "connected_to_water_source"
+
+
+def test_closed_disabled_valve_does_not_reopen_a_known_closed_link():
+    result = valve.valve_isolation_analysis(
+        ["P:pipe:A:B", "V:valve:B:R", "V-disabled:valve:A:R"],
+        "P", disabled_valves=["V-disabled"], closed_links=["V-disabled"],
+        water_source_nodes=["R"],
+    )
+    assert result["isolatable"] is True
+    assert result["must_close_valves"] == ["V"]
+
+
 def test_disabled_valve_expands_affected_area_before_counting():
     links = [
         "P-1:pipe:A:B",

tests/unit/test_wndb_network_views.py

@@ -52,6 +52,36 @@ def test_network_link_nodes_use_one_unified_view_query(monkeypatch) -> None:
     assert "FROM gis.network_links" in calls[0][1]
 
 
+def test_isolation_snapshot_reads_authoritative_links_statuses_and_sources_once(monkeypatch):
+    calls = []
+
+    def fake_read_all(name, statement, params=None):
+        calls.append((name, statement, params))
+        return [
+            {"id": "P", "element_type": "pipe", "start_node_id": "A", "end_node_id": "B", "status": None},
+            {"id": "P-closed", "element_type": "pipe", "start_node_id": "A", "end_node_id": "R", "status": "CLOSED"},
+            {"id": "V", "element_type": "valve", "start_node_id": "B", "end_node_id": "R", "status": "CLOSED"},
+            {"id": "PU", "element_type": "pump", "start_node_id": "T", "end_node_id": "A", "status": "CLOSED"},
+            {"id": "R", "element_type": "reservoir", "start_node_id": None, "end_node_id": None, "status": None},
+            {"id": "T", "element_type": "tank", "start_node_id": None, "end_node_id": None, "status": None},
+        ]
+
+    monkeypatch.setattr(network_views, "read_all", fake_read_all)
+
+    assert network_views.get_valve_isolation_topology("project_a") == (
+        ["P:pipe:A:B", "P-closed:pipe:A:R", "V:valve:B:R", "PU:pump:T:A"],
+        ["R", "T"], ["P-closed", "V", "PU"],
+    )
+    assert len(calls) == 1
+    name, statement, _params = calls[0]
+    assert name == "project_a"
+    assert "FROM network.links" in statement
+    assert "network.link_initial_settings" in statement
+    assert "COALESCE(s.status::text, p.status::text)" in statement
+    assert "FROM network.nodes" in statement
+    assert "UNION ALL" in statement
+
+
 def test_topology_rows_are_loaded_in_two_batch_queries(monkeypatch) -> None:
     calls = []
     responses = [