mirror of
https://github.com/hansjone/netx.git
synced 2026-10-14 19:50:53 +08:00
Fix the 5 failing netx-topology-mcp tests.
Stale tests (behavior changed intentionally on 2026-08-12): - Tool count is 16: suggestSinkHubs and classifyTopologyFabricNodes are documented public tools; assert them by name. - compose_views is no longer a public layout action; assert it is rejected, and cover the background-job path via orbit_sweep instead. - Dual-unit petals are nested ellipse bands with a hollow eye, not parallel lanes; assert side split, ordering inside the portals and zero crossings instead of portal-aligned lane ends. Real bug (test failed since it was added on 2026-08-09): press_hot_edges could never reel a long spoke in to its r_use target, because max_disp (r0*0.55) is smaller than r0 - r_use. Inward pulls along the spoke cannot fling a leaf across the bbox, so give them a cap of r0 - r_use. On a 1483-node dev view crossings after the op drop 25501 -> 25346; a 500-node view is unchanged; runtime unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
f301544b99
commit
ed820126df
4 changed files with 63 additions and 23 deletions
|
|
@ -284,9 +284,10 @@ def press_hot_edges(
|
|||
tuple[int, tuple[float, float], tuple[float, float]] | None
|
||||
) = None
|
||||
|
||||
def _try_cand(cand: tuple[float, float]) -> None:
|
||||
def _try_cand(cand: tuple[float, float], disp_cap: float | None = None) -> None:
|
||||
nonlocal best
|
||||
if math.hypot(cand[0] - leaf_xy[0], cand[1] - leaf_xy[1]) > max_disp:
|
||||
cap = max_disp if disp_cap is None else disp_cap
|
||||
if math.hypot(cand[0] - leaf_xy[0], cand[1] - leaf_xy[1]) > cap:
|
||||
return
|
||||
trial_pos = {**pos, leaf: cand}
|
||||
if _overlaps_any(leaf, trial_pos, names):
|
||||
|
|
@ -304,7 +305,11 @@ def press_hot_edges(
|
|||
best = (c1, cand)
|
||||
|
||||
# Reel-in along the spoke first (shorten without hunting angles).
|
||||
# Pulling straight toward the hub cannot fling the leaf across the
|
||||
# bbox, so allow it to travel all the way to the r_use target;
|
||||
# max_disp alone stops short (e.g. r0=8000 → cap 4400 < 5800 needed).
|
||||
if r0 > r_use + 50.0:
|
||||
reel_cap = max(max_disp, r0 - r_use)
|
||||
ux = (leaf_xy[0] - hub_xy[0]) / r0
|
||||
uy = (leaf_xy[1] - hub_xy[1]) / r0
|
||||
for rt in (
|
||||
|
|
@ -318,7 +323,7 @@ def press_hot_edges(
|
|||
rt = min(rt, r0 - 1.0)
|
||||
if rt < 120.0:
|
||||
continue
|
||||
_try_cand((hub_xy[0] + ux * rt, hub_xy[1] + uy * rt))
|
||||
_try_cand((hub_xy[0] + ux * rt, hub_xy[1] + uy * rt), reel_cap)
|
||||
|
||||
# Both ends walk toward the midpoint (metro bridges often need this).
|
||||
# Skip on giant E: each trial is a full O(E²) count and stalls MCP.
|
||||
|
|
|
|||
|
|
@ -80,7 +80,7 @@ def test_classify_petal_and_straight() -> None:
|
|||
assert classify_dual_unit(short) == "straight"
|
||||
|
||||
|
||||
def test_beautify_parallel_lanes() -> None:
|
||||
def test_beautify_petal_ellipse_bands() -> None:
|
||||
nodes, edges = _two_eye_graph()
|
||||
st = build_state_from_nodes_edges(nodes, edges)
|
||||
unit = DualUnit(
|
||||
|
|
@ -89,15 +89,18 @@ def test_beautify_parallel_lanes() -> None:
|
|||
paths=[["p1", "a1", "a2", "p2"], ["p1", "b1", "b2", "p2"]],
|
||||
)
|
||||
pos = beautify_dual_unit_positions(st, unit, LayoutParams())
|
||||
# Parallel H/V lanes: same y in a corridor; first/last share portal x.
|
||||
assert abs(pos["a1"][1] - pos["a2"][1]) < 1e-6
|
||||
assert abs(pos["b1"][1] - pos["b2"][1]) < 1e-6
|
||||
assert abs(pos["a1"][1]) > 1.0 and abs(pos["b1"][1]) > 1.0
|
||||
# Portals sit on the x-axis, a left of b (the eye's chord).
|
||||
assert abs(pos["p1"][1]) < 1e-6 and abs(pos["p2"][1]) < 1e-6
|
||||
assert pos["p1"][0] < pos["p2"][0]
|
||||
# Each corridor is one band on its own side of the chord.
|
||||
assert pos["a1"][1] * pos["b1"][1] < 0
|
||||
assert abs(pos["a1"][0] - pos["p1"][0]) < 1e-6
|
||||
assert abs(pos["a2"][0] - pos["p2"][0]) < 1e-6
|
||||
assert abs(pos["b1"][0] - pos["p1"][0]) < 1e-6
|
||||
assert abs(pos["b2"][0] - pos["p2"][0]) < 1e-6
|
||||
for x, y in (("a1", "a2"), ("b1", "b2")):
|
||||
assert pos[x][1] * pos[y][1] > 0
|
||||
assert abs(pos[x][1]) > 1.0
|
||||
# Runs a→b, strictly inside the portals (hollow eye, no node on the chord ends).
|
||||
assert pos["p1"][0] < pos[x][0] < pos[y][0] < pos["p2"][0]
|
||||
links = [(e["a_node_id"], e["b_node_id"]) for e in edges if e["a_node_id"] in pos and e["b_node_id"] in pos]
|
||||
assert count_edge_crossings(pos, links) == 0
|
||||
|
||||
|
||||
def test_dual_mass_refine_reports_drift() -> None:
|
||||
|
|
|
|||
|
|
@ -105,26 +105,54 @@ def test_slim_layout_payload_drops_guide() -> None:
|
|||
assert len(slim["crossing"]["top_nodes"]) <= 5
|
||||
|
||||
|
||||
def test_compose_background_returns_job_id() -> None:
|
||||
src = [f"s{i:02d}" for i in range(12)]
|
||||
def test_compose_views_is_not_public() -> None:
|
||||
# compose_* was retired from the public surface (dual-unit drain is the main path).
|
||||
out = _layout_topology_view(
|
||||
{
|
||||
"view_id": "full",
|
||||
"action": "compose_views",
|
||||
"mode": "apply",
|
||||
"params": {"source_view_ids": [f"s{i:02d}" for i in range(12)]},
|
||||
}
|
||||
)
|
||||
assert out.get("ok") is False
|
||||
assert out.get("error") == "unknown_action:compose_views"
|
||||
|
||||
|
||||
def test_orbit_background_returns_job_id() -> None:
|
||||
graph = {
|
||||
"ok": True,
|
||||
"nodes": [
|
||||
{"fabric_node_id": f"n{i}", "x": float(i * 100), "y": 0.0, "label": f"N{i}"}
|
||||
for i in range(6)
|
||||
],
|
||||
"edges": [
|
||||
{"id": f"e{i}", "a_node_id": f"n{i}", "b_node_id": f"n{i + 1}"} for i in range(5)
|
||||
],
|
||||
}
|
||||
|
||||
def fake_start(**kwargs):
|
||||
assert kwargs["action"] == "compose_views"
|
||||
assert kwargs["action"] == "orbit_sweep"
|
||||
assert kwargs["view_id"] == "full"
|
||||
assert isinstance(kwargs.get("tool_args"), dict)
|
||||
assert kwargs["tool_args"].get("action") == "compose_views"
|
||||
tool_args = kwargs.get("tool_args")
|
||||
assert isinstance(tool_args, dict)
|
||||
# The worker re-runs the same call synchronously.
|
||||
assert tool_args["params"].get("_force_sync") is True
|
||||
assert "background" not in tool_args["params"]
|
||||
return "abc123job"
|
||||
|
||||
with patch("netx_topology_mcp.http_tools.start_job", side_effect=fake_start):
|
||||
with patch("netx_topology_mcp.http_tools.http_json", return_value={"ok": True, "data": graph}), patch(
|
||||
"netx_topology_mcp.http_tools.start_job", side_effect=fake_start
|
||||
):
|
||||
out = _layout_topology_view(
|
||||
{
|
||||
"view_id": "full",
|
||||
"action": "compose_views",
|
||||
"action": "orbit_sweep",
|
||||
"mode": "apply",
|
||||
"params": {"source_view_ids": src},
|
||||
"params": {"background": True},
|
||||
}
|
||||
)
|
||||
assert out.get("ok") is True
|
||||
assert out.get("ok") is True, out
|
||||
assert out.get("status") == "running"
|
||||
assert out.get("job_id") == "abc123job"
|
||||
assert out.get("applied") is False
|
||||
|
|
|
|||
|
|
@ -18,8 +18,10 @@ from netx_topology_mcp.server import _fetch_scopes
|
|||
|
||||
def test_tool_list_has_draw_and_query_tools() -> None:
|
||||
names = {str(t.get("name") or "") for t in HTTP_MCP_TOOLS}
|
||||
assert len(names) == 14
|
||||
assert len(names) == 16
|
||||
assert "sinkTopologyDualUnits" in names
|
||||
assert "suggestSinkHubs" in names
|
||||
assert "classifyTopologyFabricNodes" in names
|
||||
assert "copyTopologyViewNodes" in names
|
||||
assert "createTopologyFolder" in names
|
||||
assert "createTopologyView" not in names
|
||||
|
|
@ -547,8 +549,10 @@ def test_stdio_initialize_and_tools_list() -> None:
|
|||
proc.stdin.flush()
|
||||
list_resp = json.loads(proc.stdout.readline())
|
||||
tools = list_resp["result"]["tools"]
|
||||
assert len(tools) == 14
|
||||
assert len(tools) == 16
|
||||
names = {t["name"] for t in tools}
|
||||
assert "suggestSinkHubs" in names
|
||||
assert "classifyTopologyFabricNodes" in names
|
||||
assert "createTopologyView" not in names
|
||||
assert "listTopologyViews" not in names
|
||||
assert "queryTopologyFabricNodes" in names
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue