feat(client): add get_labels, get_edge_types, traverse#29
Merged
Conversation
- LabelInfo, EdgeTypeInfo, PropertyDefinitionInfo, TraverseResult result types
- AsyncCoordinodeClient: get_labels(), get_edge_types(), traverse()
- CoordinodeClient: sync wrappers for all three methods
- traverse() maps direction strings ("outbound"/"inbound"/"both") to TraversalDirection enum
- Export new types from coordinode.__init__
- 15 mock-based unit tests in tests/unit/test_schema_crud.py (no Docker)
All 33 unit tests pass.
…ve server 5 integration tests: LabelInfo list non-empty, EdgeTypeInfo list includes created type, TraverseResult returns outbound and inbound neighbours.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds schema-introspection and traversal support: four new result types ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Schema as SchemaService
participant Graph as GraphService
Note over Client,Schema: List schema entities
Client->>Schema: ListLabels / ListEdgeTypes RPC
Schema-->>Client: proto list of labels / edge types
Client->>Client: wrap protos into LabelInfo / EdgeTypeInfo
Note over Client,Graph: Perform traversal
Client->>Graph: Traverse(start_node_id, edge_type, direction, max_depth)
Graph-->>Client: TraverseResponse (nodes, edges)
Client->>Client: wrap response into TraverseResult (NodeResult / EdgeResult)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coordinode/coordinode/client.py`:
- Around line 398-405: The code currently uses _direction_map.get(...) to
silently default invalid direction strings to
TraversalDirection.TRAVERSAL_DIRECTION_OUTBOUND; change this to validate the
input and raise an error instead: compute key = direction.lower(), attempt a
lookup in _direction_map, and if the lookup yields None raise a ValueError (or
custom exception) stating the invalid direction and listing allowed values
("outbound", "inbound", "both"); then set direction_value to the looked-up
TraversalDirection. Update references around _direction_map and direction_value
to use this validated lookup.
In `@tests/integration/test_sdk.py`:
- Around line 222-223: The test uses an ambiguous single-letter loop variable
'l' in assertions and the list comprehension (assert all(isinstance(l,
LabelInfo) for l in labels) and names = [l.name for l in labels]) which triggers
Ruff E741; rename the variable to a descriptive name like 'label' in both places
(and the similar occurrence at the other location around line 234) so the checks
become assert all(isinstance(label, LabelInfo) for label in labels) and names =
[label.name for label in labels], updating any other same-scope uses
accordingly.
- Line 15: The import list in tests/integration/test_sdk.py is not
alphabetically sorted and triggers Ruff I001; reorder the imported symbols in
the from coordinode import ... line into alphabetical order
(AsyncCoordinodeClient, CoordinodeClient, EdgeTypeInfo, LabelInfo,
TraverseResult) so the import statement matches the project's configured import
ordering.
- Around line 267-271: Move the setup Cypher calls (the client.cypher ID lookup
and similar setup queries for the inbound traversal test) inside the try block
so the finally cleanup always runs even if the lookup/query raises;
specifically, place the rows = client.cypher(...) call that uses tag and any
preceding setup for the traversal tests into the same try that contains the test
assertions and keep the deletion/cleanup logic in the finally block so cleanup
executes on setup or query failures (apply the same change for the inbound
traversal test block).
In `@tests/unit/test_schema_crud.py`:
- Around line 8-15: The import block in tests/unit/test_schema_crud.py is not
formatted to the repo's import style causing Ruff I001; reformat the import list
from coordinode.client to match the project's isort/ruff configuration (grouping
and line-wrapping) — e.g., adjust the multi-line import that currently lists
EdgeResult, EdgeTypeInfo, LabelInfo, NodeResult, PropertyDefinitionInfo,
TraverseResult to the canonical ordering/line breaks used by the repo and run
`ruff check --fix` (or the configured isort) so the import statement for
coordinode.client is properly normalized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c669001-61b1-4f57-a2fa-cc1b91588351
📒 Files selected for processing (4)
coordinode/coordinode/__init__.pycoordinode/coordinode/client.pytests/integration/test_sdk.pytests/unit/test_schema_crud.py
…leanup - Raise ValueError for unknown direction strings instead of silently falling back to outbound — masks caller bugs and returns wrong results - Fix Ruff E741: rename loop variable l → lbl in get_labels tests - Fix Ruff I001: sort imports in test_sdk.py and test_schema_crud.py - Move ID-lookup cypher calls inside try blocks so finally cleanup always runs even if the lookup query raises - Skip test_traverse_inbound_direction: CoordiNode Traverse RPC does not yet support inbound direction (server returns empty result set)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/test_sdk.py`:
- Around line 280-283: Replace the pytest.mark.skip decorator on
test_traverse_inbound_direction with pytest.mark.xfail (e.g.,
pytest.mark.xfail(reason="CoordiNode Traverse RPC does not yet support inbound
direction", strict=False)) so the test remains visible and fails when the
behavior is implemented; keep the test function name
test_traverse_inbound_direction and its body unchanged.
- Around line 231-239: The test uses a global probe node and plain DELETE which
can collide or fail; update the setup/cleanup around client.cypher and
get_labels to use a per-test unique label/name (e.g., include a UUID or
test-specific suffix) when creating the node via client.cypher("MERGE
(n:PropLabel {name: 'probe'})") and then remove the node with DETACH DELETE
using that same unique identifier in the finally block; ensure the same unique
value is used when searching labels via client.get_labels()/found to keep the
test isolated and allow cleanup to succeed even if relationships exist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2727e81c-3818-41b1-9849-40a0622f2ca8
📒 Files selected for processing (3)
coordinode/coordinode/client.pytests/integration/test_sdk.pytests/unit/test_schema_crud.py
There was a problem hiding this comment.
Pull request overview
Adds new schema helper APIs and a traversal API to the CoordiNode Python SDK, along with unit + integration coverage to validate the new result wrapper types and client methods.
Changes:
- Add
LabelInfo,EdgeTypeInfo,PropertyDefinitionInfo, andTraverseResultwrapper types. - Add
get_labels(),get_edge_types(), andtraverse()to both async and sync clients. - Add new unit tests for wrappers and new integration tests for label/edge type listing and traversal.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
coordinode/coordinode/client.py |
Implements new wrapper types and adds schema listing + traverse client methods. |
coordinode/coordinode/__init__.py |
Re-exports new public types from the package root. |
tests/unit/test_schema_crud.py |
Adds mock-based unit tests validating wrapper field mapping and repr behavior. |
tests/integration/test_sdk.py |
Adds integration coverage for get_labels, get_edge_types, and traverse (with inbound currently skipped). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- PropertyDefinitionInfo: class name typo in string form corrected - LabelInfo, EdgeTypeInfo: include version field; summarise property count - Integration test: unique tag for label node; use DETACH DELETE cleanup - Integration test: replace skip with xfail for unsupported inbound direction
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coordinode/coordinode/client.py`:
- Around line 368-406: The traverse() method currently sends invalid max_depth
values to the server; add a local guard in the traverse function (before
constructing TraverseRequest) that checks the max_depth parameter is an integer
>= 1 and raise a ValueError with a clear message if not; modify the traverse
function (referencing the max_depth parameter, the local variable
key/direction_value logic, and the TraverseRequest creation) to perform this
validation so invalid depths are rejected locally.
In `@tests/integration/test_sdk.py`:
- Around line 229-237: The test test_get_labels_has_property_definitions is too
weak—only checking that LabelInfo.properties is a list; strengthen it by seeding
a concrete property on the created node and asserting that client.get_labels()
surfaces that property in the LabelInfo.properties for "PropLabel".
Specifically, when creating the node (the client.cypher("CREATE (n:PropLabel
{tag: $tag})", ...)), include an additional property (e.g., "pname": "value" or
a numeric property) and then after calling client.get_labels() locate the
LabelInfo for "PropLabel" and assert that its properties list contains an entry
for that property name (and optionally check property metadata/type if LabelInfo
exposes it), failing the test if the property is missing or malformed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26617e4e-34d1-4587-b7c3-986b925140dd
📒 Files selected for processing (2)
coordinode/coordinode/client.pytests/integration/test_sdk.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- traverse(): raise ValueError locally when max_depth < 1 before RPC - test_traverse_inbound_direction: strict=True so CI catches unexpected XPASS when server adds inbound direction support - test_get_labels_has_property_definitions: document why the assertion is intentionally weak (schema-free labels may have empty properties)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/test_sdk.py`:
- Around line 283-286: The xfail decorator on the failing integration test
currently uses strict=True which will cause CI to fail when the server starts
returning inbound traversal results; update the pytest.mark.xfail(...) decorator
to use strict=False instead of strict=True (keep the existing reason text and
other parameters the same) so the test is allowed to XPASS without failing the
suite.
- Around line 214-241: The tests test_get_labels_returns_list and
test_get_labels_has_property_definitions currently use hard-coded label names
which can produce false positives from global schema state; change them to
create and assert against a unique label name generated by uid() (e.g., label =
f"GetLabelsTest_{uid()}") and use that variable in the CREATE, MATCH/DELETE and
in the assertion that label exists in the returned names or properties list;
apply the same pattern to any get_edge_types() tests (generate a unique
edge-type name via uid(), use it in the creation and cleanup cypher and assert
against that generated name) so each test checks for its own unique schema token
rather than a hard-coded value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8c8ed435-dca2-478b-92f8-929d351129ce
📒 Files selected for processing (2)
coordinode/coordinode/client.pytests/integration/test_sdk.py
…ives - Generate unique label and edge type names per test run so that stale schema entries from previous runs cannot cause spurious assertion hits - Change xfail marker to strict=False for inbound traverse test so CI reports XPASS as a warning instead of a hard failure when server eventually adds inbound support
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/integration/test_sdk.py (1)
230-243:⚠️ Potential issue | 🟠 MajorUse a unique label token in
test_get_labels_has_property_definitions.Line 233 and Line 236 hard-code
"PropLabel", which can let this test pass against stale global schema entries even if this test’s setup fails. Use a per-testlabel_name(fromuid()) for create, lookup, and cleanup.Suggested fix
def test_get_labels_has_property_definitions(client): """LabelInfo.properties is a list (may be empty for schema-free labels).""" tag = uid() - client.cypher("CREATE (n:PropLabel {tag: $tag})", params={"tag": tag}) + label_name = f"PropLabel{uid()}" + client.cypher(f"CREATE (n:{label_name} {{tag: $tag}})", params={"tag": tag}) try: labels = client.get_labels() - found = next((lbl for lbl in labels if lbl.name == "PropLabel"), None) - assert found is not None, "PropLabel not returned by get_labels()" + found = next((lbl for lbl in labels if lbl.name == label_name), None) + assert found is not None, f"{label_name} not returned by get_labels()" # Intentionally only check the type — CoordiNode is schema-free and may return # an empty properties list even when the node was created with properties. assert isinstance(found.properties, list) finally: - client.cypher("MATCH (n:PropLabel {tag: $tag}) DETACH DELETE n", params={"tag": tag}) + client.cypher(f"MATCH (n:{label_name} {{tag: $tag}}) DETACH DELETE n", params={"tag": tag})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/test_sdk.py` around lines 230 - 243, The test_get_labels_has_property_definitions currently hardcodes "PropLabel", which can pass spuriously; generate a unique label_name via uid() (e.g., label_name = uid()) and use that variable instead of the string literal in the client.cypher CREATE, in the lookup that finds the label (replace lbl.name == "PropLabel" with lbl.name == label_name), and in the cleanup MATCH/DELETE call so all three places (creation, assertion lookup, and finally block cleanup) reference the same per-test label_name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/integration/test_sdk.py`:
- Around line 230-243: The test_get_labels_has_property_definitions currently
hardcodes "PropLabel", which can pass spuriously; generate a unique label_name
via uid() (e.g., label_name = uid()) and use that variable instead of the string
literal in the client.cypher CREATE, in the lookup that finds the label (replace
lbl.name == "PropLabel" with lbl.name == label_name), and in the cleanup
MATCH/DELETE call so all three places (creation, assertion lookup, and finally
block cleanup) reference the same per-test label_name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3274195-681e-4397-ba0f-4004b7f2b4a9
📒 Files selected for processing (1)
tests/integration/test_sdk.py
…test Same stale-schema fix as adjacent tests: generate a unique label per run so accumulated schema state cannot produce a false assertion pass.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/integration/test_sdk.py (1)
230-242:⚠️ Potential issue | 🟠 MajorUse a unique label token in this test to avoid stale-schema false positives.
At Line 233 and Line 236, the hard-coded
PropLabelcan match pre-existing global schema state, so this test may pass without validating this run’s insertion path.Suggested fix
def test_get_labels_has_property_definitions(client): """LabelInfo.properties is a list (may be empty for schema-free labels).""" tag = uid() - client.cypher("CREATE (n:PropLabel {tag: $tag})", params={"tag": tag}) + label_name = f"PropLabel{uid()}" + client.cypher(f"CREATE (n:{label_name} {{tag: $tag}})", params={"tag": tag}) try: labels = client.get_labels() - found = next((lbl for lbl in labels if lbl.name == "PropLabel"), None) - assert found is not None, "PropLabel not returned by get_labels()" + found = next((lbl for lbl in labels if lbl.name == label_name), None) + assert found is not None, f"{label_name} not returned by get_labels()" # Intentionally only check the type — CoordiNode is schema-free and may return # an empty properties list even when the node was created with properties. assert isinstance(found.properties, list) finally: - client.cypher("MATCH (n:PropLabel {tag: $tag}) DETACH DELETE n", params={"tag": tag}) + client.cypher(f"MATCH (n:{label_name} {{tag: $tag}}) DETACH DELETE n", params={"tag": tag})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/test_sdk.py` around lines 230 - 242, The test test_get_labels_has_property_definitions uses a non-unique hard-coded label "PropLabel" which can match pre-existing schema; change the test to create and use a unique label name (e.g., append the uid tag to the label: label_name = f"PropLabel_{tag}") and update the Cypher CREATE, the search for the label in client.get_labels() (match lbl.name == label_name) and the cleanup MATCH/DELETE query to reference that dynamic label_name so the test validates this run's insertion path only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/integration/test_sdk.py`:
- Around line 230-242: The test test_get_labels_has_property_definitions uses a
non-unique hard-coded label "PropLabel" which can match pre-existing schema;
change the test to create and use a unique label name (e.g., append the uid tag
to the label: label_name = f"PropLabel_{tag}") and update the Cypher CREATE, the
search for the label in client.get_labels() (match lbl.name == label_name) and
the cleanup MATCH/DELETE query to reference that dynamic label_name so the test
validates this run's insertion path only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7aee88c4-f964-4edb-8191-4b44ad6dfa2d
📒 Files selected for processing (1)
tests/integration/test_sdk.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…raverse XPASS means the server gained inbound support — that is good news, not a CI failure. strict=True would break CI at exactly the moment the server improves. The XPASS entry in pytest output is the signal to remove the marker.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/test_schema_crud.py`:
- Line 137: Replace the inline enum-value comment by introducing a local named
constant (e.g. TIMESTAMP = 6) near the test or top of the test function and use
that constant in the _FakePropDef call (change props = [_FakePropDef("since",
6)] to props = [_FakePropDef("since", TIMESTAMP)]), so the intent is explicit
and the static analyzer no longer treats it as commented-out code; reference the
_FakePropDef usage in test_schema_crud.py when making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9b68606b-f79b-46ef-bea2-c1a4c4e4b3c1
📒 Files selected for processing (1)
tests/unit/test_schema_crud.py
Introduce PROPERTY_TYPE_TIMESTAMP = 6 as a class-level constant to replace the inline magic number that SonarCloud flagged as commented-out code.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/test_schema_crud.py`:
- Around line 166-174: The test test_edges_are_wrapped_as_edge_results is
missing an assertion that the original edge's edge_id is mapped to
EdgeResult.id; update the test in tests/unit/test_schema_crud.py (inside
test_edges_are_wrapped_as_edge_results) to assert that result.edges[0].id equals
the source fake edge's edge_id (the _FakeEdge instance used to build the
_FakeTraverseResponse), keeping the existing checks for type, source_id and
target_id using EdgeResult and TraverseResult to fully validate the wrapper
contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f318e633-0706-4c28-b9d2-6ac759517b87
📒 Files selected for processing (1)
tests/unit/test_schema_crud.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…results The edge_id → EdgeResult.id mapping was part of the wrapper contract but was not verified. Node mapping was tested (result.nodes[0].id == 1) but the equivalent edge check was missing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- test_traverse_returns_neighbours: assert the specific leaf1 node ID is in result.nodes, not just that the count is non-zero - test_traverse_inbound_direction: capture src_id alongside dst_id so that when the server gains inbound support (XPASS), the assertion validates the correct node was returned - TestTraverseValidation: new unit test class for ValueError cases in AsyncCoordinodeClient.traverse() — invalid direction and max_depth < 1; validation fires before any RPC so no running server is required
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
raises=AssertionError limits the xfail coverage to the known failure mode (server returns empty result → assertion fails). Unexpected errors such as gRPC RpcError or wrong enum mapping propagate as CI failures instead of being silently swallowed by the broad xfail marker. Addresses review thread #27.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… traverse() Pure string/int validation now fires before the deferred proto import. Callers get ValueError for invalid inputs even when proto stubs have not been generated, keeping validation independent of the build step. Addresses review thread #28.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coordinode/coordinode/client.py`:
- Around line 389-393: Validate types before performing operations: ensure
`direction` is a str and `max_depth` is an int (and not a bool) before calling
`direction.lower()` or comparing `max_depth < 1`. In the block that currently
references `direction`, `_valid_directions`, and `max_depth`, add explicit type
checks (e.g., isinstance(direction, str) and isinstance(max_depth, int) and not
isinstance(max_depth, bool)) and raise ValueError with clear messages (e.g.,
"direction must be a str" and "max_depth must be an int") if checks fail; then
proceed to call `direction.lower()` and the existing bounds check against
`_valid_directions` and `max_depth < 1`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e9ed5790-3508-4a5a-a1e7-0fe8f627e6e9
📒 Files selected for processing (1)
coordinode/coordinode/client.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test Plan
Closes #28