Skip to content

Commit ae37106

Browse files
committed
fix(client): add type guards for direction and max_depth in traverse()
Passing direction=None raised AttributeError; passing max_depth='2' or max_depth=True raised TypeError. Both now raise ValueError consistently, matching the documented intent of the validation block. - isinstance(direction, str) check before .lower() call - isinstance(max_depth, int) and not isinstance(max_depth, bool) before < 1 - Three new unit tests cover None direction, str max_depth, bool max_depth Addresses review thread #29.
1 parent cd3f3d7 commit ae37106

2 files changed

Lines changed: 37 additions & 3 deletions

File tree

coordinode/coordinode/client.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,12 +385,16 @@ async def traverse(
385385
"""
386386
# Validate pure string/int inputs before importing proto stubs — ensures ValueError
387387
# is raised even when proto stubs have not been generated yet.
388+
# Type guards come first so that wrong types raise ValueError, not AttributeError/TypeError.
389+
if not isinstance(direction, str):
390+
raise ValueError(f"direction must be a str, got {type(direction).__name__!r}.")
388391
_valid_directions = {"outbound", "inbound", "both"}
389392
key = direction.lower()
390393
if key not in _valid_directions:
391394
raise ValueError(f"Invalid direction {direction!r}. Must be one of: 'outbound', 'inbound', 'both'.")
392-
if max_depth < 1:
393-
raise ValueError(f"max_depth must be >= 1, got {max_depth!r}.")
395+
# bool is a subclass of int in Python, so `isinstance(True, int)` is True — exclude it.
396+
if not isinstance(max_depth, int) or isinstance(max_depth, bool) or max_depth < 1:
397+
raise ValueError(f"max_depth must be an integer >= 1, got {max_depth!r}.")
394398

395399
from coordinode._proto.coordinode.v1.graph.graph_pb2 import ( # type: ignore[import]
396400
TraversalDirection,

tests/unit/test_schema_crud.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,37 @@ def test_max_depth_below_one_raises(self):
221221

222222
async def _inner() -> None:
223223
client = AsyncCoordinodeClient("localhost:0")
224-
with pytest.raises(ValueError, match="max_depth must be >= 1"):
224+
with pytest.raises(ValueError, match="max_depth must be"):
225225
await client.traverse(1, "KNOWS", max_depth=0)
226226

227227
asyncio.run(_inner())
228+
229+
def test_direction_none_raises_value_error(self):
230+
"""traverse() raises ValueError (not AttributeError) when direction is None."""
231+
232+
async def _inner() -> None:
233+
client = AsyncCoordinodeClient("localhost:0")
234+
with pytest.raises(ValueError, match="direction must be a str"):
235+
await client.traverse(1, "KNOWS", direction=None) # type: ignore[arg-type]
236+
237+
asyncio.run(_inner())
238+
239+
def test_max_depth_string_raises_value_error(self):
240+
"""traverse() raises ValueError (not TypeError) when max_depth is a string."""
241+
242+
async def _inner() -> None:
243+
client = AsyncCoordinodeClient("localhost:0")
244+
with pytest.raises(ValueError, match="max_depth must be an integer"):
245+
await client.traverse(1, "KNOWS", max_depth="2") # type: ignore[arg-type]
246+
247+
asyncio.run(_inner())
248+
249+
def test_max_depth_bool_raises_value_error(self):
250+
"""traverse() raises ValueError for bool max_depth (bool is a subclass of int in Python)."""
251+
252+
async def _inner() -> None:
253+
client = AsyncCoordinodeClient("localhost:0")
254+
with pytest.raises(ValueError, match="max_depth must be an integer"):
255+
await client.traverse(1, "KNOWS", max_depth=True) # type: ignore[arg-type]
256+
257+
asyncio.run(_inner())

0 commit comments

Comments
 (0)