diff --git a/changelog.md b/changelog.md
index 86779d3..64b349d 100644
--- a/changelog.md
+++ b/changelog.md
@@ -7,6 +7,11 @@
## Bug fixes
* Fixed a stored cross-site scripting (XSS) vulnerability in `VG.render()`. Graph data was injected into an executable `` could break out and run arbitrary code in the browser of anyone opening a saved visualization. Data is now delivered as an inert `` can appear literally. The `render_widget` was unaffected.
+* Fixed `widget.remove_data` leaving dangling relationships when only nodes were removed.
+* Fixed `widget.remove_data` silently doing nothing when the id type differed (e.g. `Node(id=1)` vs `remove_data(nodes="1")`).
+* Fixed `VisualizationGraph.resize_nodes` and `color_nodes` crashing with `ValueError: min() iterable argument is empty` when no node has the requested property. They now raise a `ValueError` that names the missing property.
+* Fixed `VisualizationGraph.color_nodes` with `color_space=ColorSpace.CONTINUOUS` raising an unhelpful `TypeError` when colouring non-numeric (text) values. It now raises a `ValueError` suggesting `ColorSpace.DISCRETE`.
+* Fixed the `max_allowed_nodes` limit only being enforced at draw time. `GraphWidget.add_data` now rejects additions that would exceed the limit set when the widget was created (via `render_widget`/`from_graph_data`), so the graph can no longer be grown past it afterwards.
## Improvements
diff --git a/python-wrapper/src/neo4j_viz/_graph_entity_operations.py b/python-wrapper/src/neo4j_viz/_graph_entity_operations.py
index 2fa8343..f9d2ac7 100644
--- a/python-wrapper/src/neo4j_viz/_graph_entity_operations.py
+++ b/python-wrapper/src/neo4j_viz/_graph_entity_operations.py
@@ -132,6 +132,12 @@ def resize_nodes(
if node.size is not None:
all_sizes[node.id] = node.size
+ if property is not None and not all_sizes:
+ raise ValueError(
+ f"No node has the property {property!r}, so sizes cannot be computed. "
+ "Check the spelling, or pass explicit `sizes`."
+ )
+
# Validate node sizes
for id, size in all_sizes.items():
if size is None:
@@ -207,6 +213,9 @@ def resize_relationships(
def _normalize_values(
node_map: dict[NodeIdType, RealNumber], min_max: tuple[float, float] = (0, 1)
) -> dict[NodeIdType, RealNumber]:
+ if not node_map:
+ return {}
+
unscaled_min_size = min(node_map.values())
unscaled_max_size = max(node_map.values())
unscaled_size_range = float(unscaled_max_size - unscaled_min_size)
@@ -262,6 +271,21 @@ def node_to_attr(node: Node) -> Any:
colors = NEO4J_COLORS_DISCRETE
else:
node_map = {node.id: node_to_attr(node) for node in self.nodes if node_to_attr(node) is not None}
+
+ if not node_map:
+ raise ValueError(
+ f"No node has a value for {attribute!r}, so continuous coloring cannot be computed. "
+ "Check the spelling, or use `color_space=ColorSpace.DISCRETE` for categorical values."
+ )
+
+ for node_id, value in node_map.items():
+ if isinstance(value, bool) or not isinstance(value, (int, float)):
+ raise ValueError(
+ f"Continuous coloring needs numeric values, but node {node_id!r} has "
+ f"{attribute}={value!r} ({type(value).__name__}). "
+ "Use `color_space=ColorSpace.DISCRETE` for non-numeric values."
+ )
+
normalized_map = self._normalize_values(node_map)
if colors is None:
diff --git a/python-wrapper/src/neo4j_viz/visualization_graph.py b/python-wrapper/src/neo4j_viz/visualization_graph.py
index c7b0034..e063552 100644
--- a/python-wrapper/src/neo4j_viz/visualization_graph.py
+++ b/python-wrapper/src/neo4j_viz/visualization_graph.py
@@ -572,4 +572,5 @@ def render_widget(
options=render_options,
theme=theme,
legend=self.legend,
+ max_allowed_nodes=max_allowed_nodes,
)
diff --git a/python-wrapper/src/neo4j_viz/widget.py b/python-wrapper/src/neo4j_viz/widget.py
index 2791e60..5d0b511 100644
--- a/python-wrapper/src/neo4j_viz/widget.py
+++ b/python-wrapper/src/neo4j_viz/widget.py
@@ -125,6 +125,8 @@ class GraphWidget(anywidget.AnyWidget):
from_json=lambda value, widget: Legend.model_validate(value),
)
+ _max_allowed_nodes: int = 10_000
+
def on_selection_change(self, callback: Callable[[GraphSelection], None]) -> Callable[[dict[str, Any]], None]:
"""
Register a callback that fires whenever the widget's `selected` trait changes.
@@ -173,9 +175,10 @@ def from_graph_data(
options: RenderOptions | None = None,
theme: str = "auto",
legend: Legend | None = None,
+ max_allowed_nodes: int = 10_000,
) -> GraphWidget:
"""Create a GraphWidget from Node and Relationship lists."""
- return cls(
+ widget = cls(
nodes=nodes,
relationships=relationships,
width=width,
@@ -184,6 +187,8 @@ def from_graph_data(
theme=theme,
legend=legend if legend is not None else Legend(),
)
+ widget._max_allowed_nodes = max_allowed_nodes
+ return widget
def __str__(self) -> str:
return f"GraphWidget(nodes={len(self.nodes)}, relationships={len(self.relationships)}, options={self.options}, theme={self.theme}, width={self.width}, height={self.height})"
@@ -622,6 +627,14 @@ def add_data(
if isinstance(relationships, Relationship):
relationships = [relationships]
+ if nodes and len(self.nodes) + len(nodes) > self._max_allowed_nodes:
+ raise ValueError(
+ f"Adding {len(nodes)} nodes would result in {len(self.nodes) + len(nodes)} nodes, "
+ f"which exceeds the maximum of {self._max_allowed_nodes} nodes set when this widget "
+ "was created. It can be increased by overriding `max_allowed_nodes` in "
+ "`render_widget`, but rendering could then take a long time."
+ )
+
if nodes:
self.nodes = self.nodes + nodes
if relationships:
@@ -644,33 +657,39 @@ def remove_data(
relationships:
Relationships to remove from the graph widget.
"""
+ # Compare ids as strings on both sides, matching how ids are serialized for the
+ # frontend (see _validation.check_dangling_relationships): ``Node(id=1)`` and a
+ # request to remove ``"1"`` refer to the same node.
if isinstance(nodes, Node):
- node_ids_to_remove = {nodes.id}
+ node_ids_to_remove = {str(nodes.id)}
elif isinstance(nodes, NodeIdType):
- node_ids_to_remove = {nodes}
+ node_ids_to_remove = {str(nodes)}
elif nodes is None:
node_ids_to_remove = set()
else:
- node_ids_to_remove = {n.id if isinstance(n, Node) else n for n in nodes}
+ node_ids_to_remove = {str(n.id) if isinstance(n, Node) else str(n) for n in nodes}
if isinstance(relationships, Relationship):
- rel_ids_to_remove = {relationships.id}
+ rel_ids_to_remove = {str(relationships.id)}
elif isinstance(relationships, RelationshipIdType):
- rel_ids_to_remove = {relationships}
+ rel_ids_to_remove = {str(relationships)}
elif relationships is None:
rel_ids_to_remove = set()
else:
- rel_ids_to_remove = {r.id if isinstance(r, Relationship) else r for r in relationships}
+ rel_ids_to_remove = {str(r.id) if isinstance(r, Relationship) else str(r) for r in relationships}
if node_ids_to_remove:
- self.nodes = [n for n in self.nodes if n.id not in node_ids_to_remove]
+ self.nodes = [n for n in self.nodes if str(n.id) not in node_ids_to_remove]
def keep_rel(r: Relationship) -> bool:
return (
- r.id not in rel_ids_to_remove
- and r.source not in node_ids_to_remove
- and r.target not in node_ids_to_remove
+ str(r.id) not in rel_ids_to_remove
+ and str(r.source) not in node_ids_to_remove
+ and str(r.target) not in node_ids_to_remove
)
- if rel_ids_to_remove:
+ # Run the cleanup whenever anything is being removed. A node-only delete must also
+ # drop the relationships that pointed at it, otherwise the frontend silently renders
+ # an empty graph (see _validation.check_dangling_relationships).
+ if node_ids_to_remove or rel_ids_to_remove:
self.relationships = [r for r in self.relationships if keep_rel(r)]
diff --git a/python-wrapper/tests/test_color_nodes.py b/python-wrapper/tests/test_color_nodes.py
index dff4d2c..5903884 100644
--- a/python-wrapper/tests/test_color_nodes.py
+++ b/python-wrapper/tests/test_color_nodes.py
@@ -290,3 +290,21 @@ def test_color_nodes_override_false() -> None:
assert VG.nodes[0].color == Color("#ff0000") # Should keep existing color
assert VG.nodes[1].color == Color("#ff0000") # Should keep existing color
assert VG.nodes[2].color == Color("#00ff00") # Should get new color (no existing color)
+
+
+def test_color_nodes_continuous_missing_property_raises() -> None:
+ VG = VisualizationGraph(nodes=[Node(id="1"), Node(id="2")], relationships=[])
+
+ with pytest.raises(ValueError, match="No node has a value for 'name'"):
+ VG.color_nodes(property="name", color_space=ColorSpace.CONTINUOUS)
+
+
+def test_color_nodes_continuous_non_numeric_raises() -> None:
+ nodes = [
+ Node(id="1", properties={"name": "Alice"}),
+ Node(id="2", properties={"name": "Bob"}),
+ ]
+ VG = VisualizationGraph(nodes=nodes, relationships=[])
+
+ with pytest.raises(ValueError, match="Continuous coloring needs numeric values"):
+ VG.color_nodes(property="name", color_space=ColorSpace.CONTINUOUS)
diff --git a/python-wrapper/tests/test_resize_nodes.py b/python-wrapper/tests/test_resize_nodes.py
index 251fd35..156115d 100644
--- a/python-wrapper/tests/test_resize_nodes.py
+++ b/python-wrapper/tests/test_resize_nodes.py
@@ -158,3 +158,10 @@ def test_resize_nodes_no_args_failure() -> None:
with pytest.raises(ValueError, match="At least one of `sizes`, `property` or `node_radius_min_max` must be given"):
VG.resize_nodes(node_radius_min_max=None)
+
+
+def test_resize_nodes_missing_property_raises() -> None:
+ VG = VisualizationGraph(nodes=[Node(id="1"), Node(id="2")], relationships=[])
+
+ with pytest.raises(ValueError, match=re.escape("No node has the property 'pagerank'")):
+ VG.resize_nodes(property="pagerank")
diff --git a/python-wrapper/tests/test_widget.py b/python-wrapper/tests/test_widget.py
index c0623e9..37951be 100644
--- a/python-wrapper/tests/test_widget.py
+++ b/python-wrapper/tests/test_widget.py
@@ -213,6 +213,57 @@ def test_remove_data(self) -> None:
assert {n.id for n in widget.nodes} == {"n3"}
assert {r.id for r in widget.relationships} == {43}
+ def test_remove_data_nodes_only_deletes_dangling_relationships(self) -> None:
+ nodes = [Node(id="n1"), Node(id="n2")]
+ rels = [
+ Relationship(source="n1", target="n2"),
+ Relationship(source="n2", target="n2"),
+ ]
+ widget = GraphWidget.from_graph_data(nodes, rels)
+
+ widget.remove_data(nodes=["n1"])
+ assert {n.id for n in widget.nodes} == {"n2"}
+ # The relationship that pointed at the removed node is deleted, not left dangling.
+ assert {(r.source, r.target) for r in widget.relationships} == {("n2", "n2")}
+
+ def test_remove_data_relationships_only(self) -> None:
+ nodes = [Node(id="n1"), Node(id="n2")]
+ rels = [Relationship(id="r1", source="n1", target="n2"), Relationship(id="r2", source="n2", target="n1")]
+ widget = GraphWidget.from_graph_data(nodes, rels)
+
+ widget.remove_data(relationships=["r1"])
+ assert {n.id for n in widget.nodes} == {"n1", "n2"}
+ assert {r.id for r in widget.relationships} == {"r2"}
+
+ def test_remove_data_id_type_mismatch(self) -> None:
+ widget = GraphWidget.from_graph_data([Node(id=1), Node(id=2)], [Relationship(source=1, target=2)])
+
+ widget.remove_data(nodes="1")
+ assert {n.id for n in widget.nodes} == {2}
+ # Relationship pointing at the removed node is also deleted.
+ assert widget.relationships == []
+
+ def test_add_data_exceeds_max_allowed_nodes(self) -> None:
+ widget = GraphWidget.from_graph_data([Node(id="n1")], [], max_allowed_nodes=10)
+
+ with pytest.raises(ValueError, match="exceeds the maximum of 10 nodes"):
+ widget.add_data(nodes=[Node(id=f"x{i}") for i in range(10)])
+
+ # The graph must be left unchanged when the limit would be exceeded.
+ assert {n.id for n in widget.nodes} == {"n1"}
+
+ def test_add_data_max_allowed_nodes_threaded_from_render_widget(self) -> None:
+ """A custom max_allowed_nodes passed to render_widget is honored by add_data (L-03)."""
+ vg = VisualizationGraph(nodes=[Node(id="n1")], relationships=[])
+ widget = vg.render_widget(max_allowed_nodes=3)
+
+ # Up to the limit is fine.
+ widget.add_data(nodes=[Node(id="n2")])
+ assert len(widget.nodes) == 2
+
+ with pytest.raises(ValueError, match="exceeds the maximum of 3 nodes"):
+ widget.add_data(nodes=[Node(id=f"x{i}") for i in range(3)])
+
def test_add_data_dangling_warns_by_default(self) -> None:
widget = GraphWidget.from_graph_data([Node(id="n1")], [])
with pytest.warns(UserWarning, match=re.escape("reference node ids that are not in the graph")):