feat(client): add create_label/create_edge_type with SchemaMode#36
Closed
feat(client): add create_label/create_edge_type with SchemaMode#36
Conversation
…te proto - Update proto submodule to 381dbcd: adds SchemaMode enum (STRICT/VALIDATED/ FLEXIBLE), ComputedPropertyDefinition, schema_mode field on CreateLabelRequest and Label messages - Add AsyncCoordinodeClient.create_label(name, properties, *, schema_mode) → LabelInfo with full _type_map / _mode_map conversion; schema_mode defaults to 'strict' - Add AsyncCoordinodeClient.create_edge_type(name, properties) → EdgeTypeInfo (proto does not expose schema_mode for edge types) - Add sync wrappers CoordinodeClient.create_label / create_edge_type - Expose schema_mode: int on LabelInfo and EdgeTypeInfo (getattr-safe: defaults to 0 for older server images that omit the field) - Revert refresh_schema() labels(a)[0] shortcut to labels(a) AS src_labels + Python _first_label() helper; published Docker image does not yet support subscript-on-function syntax (TODO comment added) - Add 11 unit tests (test_schema_crud.py): schema_mode propagation on LabelInfo and EdgeTypeInfo, missing-field default, repr coverage - Add 6 integration tests (test_sdk.py): create_label / create_edge_type round- trips, schema_mode=flexible acceptance, invalid schema_mode ValueError, appears-in-list verification (with documented workaround for G009) - Mark test_traverse_inbound as xfail: INBOUND direction not supported by server Known gaps recorded in GAPS.md: G008 - SchemaMode enforcement not active in published image G009 - ListLabels/ListEdgeTypes omits schema-only objects (no nodes yet) G010 - labels(n)[0] subscript not in published image (workaround: _first_label) G011 - traverse INBOUND direction unimplemented on server
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds schema-registration and full-text/hybrid search APIs and types to the SDK; injects optional client support into adapters; tightens schema parsing and Cypher identifier validation; adds tests, demo notebooks/docker, and updates dependency/submodule pins and public exports. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Client as AsyncCoordinodeClient
participant Schema as SchemaServiceStub
participant Text as TextServiceStub
participant Server as CoordiNode Server
rect rgba(200,150,100,0.5)
Note over App,Server: Schema Registration Flow
App->>Client: create_label(name, properties, schema_mode)
Client->>Client: _validate_property_dict / _validate_cypher_identifier
Client->>Schema: CreateLabel RPC
Schema->>Server: RPC CreateLabel
Server-->>Schema: LabelInfo proto
Schema-->>Client: proto.LabelInfo
Client-->>App: LabelInfo(name, schema_mode)
end
rect rgba(100,200,150,0.5)
Note over App,Server: Text Search Flow
App->>Client: text_search(label, query, ...)
Client->>Text: TextSearch RPC
Text->>Server: RPC TextSearch
Server-->>Text: TextSearch response
Text-->>Client: proto results
Client-->>App: list[TextResult]
end
rect rgba(150,100,200,0.5)
Note over App,Server: Hybrid Search Flow
App->>Client: hybrid_text_vector_search(label, text_query, vector, ...)
Client->>Client: normalize vector -> floats
Client->>Text: HybridTextVectorSearch RPC
Text->>Server: RPC HybridTextVectorSearch
Server-->>Text: Hybrid response
Text-->>Client: proto results
Client-->>App: list[HybridResult]
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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: 3
🤖 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 418-425: The current loop building PropertyDefinition uses
_type_map.get(..., PropertyType.PROPERTY_TYPE_STRING) which silently coerces
unknown types to STRING; change this to validate the resolved type by checking
if type_str exists in _type_map and if not raise a ValueError (include the
unknown type_str and p["name"] in the message) before constructing
PropertyDefinition, using _type_map[type_str] when present; apply the same
validation and error behavior to the other occurrence that builds
PropertyDefinition (the block around the second instance noted in the comment).
- Around line 398-408: Extract the duplicated _type_map into a single shared
constant or helper (e.g., PROPERTY_TYPE_MAP or get_property_type) so both create
methods reference that single source; remove the local _type_map dicts and
import/use the shared symbol instead; ensure the mapping keys and values remain
the same (mapping to PropertyType.PROPERTY_TYPE_*) and update both places where
_type_map was used so behavior is unchanged.
In `@langchain-coordinode/langchain_coordinode/graph.py`:
- Around line 270-273: The docstring/comment claims CoordiNode nodes have
exactly one label but the API (create_node(labels: list[str]) and
NodeResult.labels: list[str]) allows multiple labels; clarify intent and enforce
or document accordingly: either enforce the single-label constraint by
validating in create_node (raise on len(labels) != 1 and store/return a
single-label invariant) and update uses that call labels()[0] (or keep min()
deterministic) OR change the comment to state "nodes typically have one label in
this application; code uses min() to deterministically pick one when multiple
labels exist" and leave create_node/NodeResult.labels as list[str]; update the
docstring in graph.py near labels() and any comment referencing min() to match
the chosen approach.
🪄 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: 8f002175-5aaf-4cab-8d74-37209a075a63
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
coordinode/coordinode/client.pylangchain-coordinode/langchain_coordinode/graph.pyprototests/integration/test_sdk.pytests/unit/test_schema_crud.py
…upport
- Add client= parameter to CoordinodeGraph.__init__ and
CoordinodePropertyGraphStore.__init__: accepts any object with .cypher()
instead of creating a gRPC CoordinodeClient from addr. Enables passing
LocalClient from coordinode-embedded.
- Rewrite all 4 demo notebooks for Google Colab:
* 00_seed_data.ipynb — seeds tech-industry knowledge graph; Colab uses
LocalClient(":memory:") directly (same .cypher() API)
* 01_llama_index_property_graph.ipynb — PropertyGraphStore via
_EmbeddedAdapter wrapping LocalClient; get_schema_text() via Cypher fallback
* 02_langchain_graph_chain.ipynb — CoordinodeGraph with client= adapter;
OpenAI GraphCypherQAChain section is optional (auto-skips without key)
* 03_langgraph_agent.ipynb — LocalClient direct; LangGraph agent with
mock demo first (no LLM key needed), LLM section optional
- Each notebook: IN_COLAB detection, installs coordinode-embedded from GitHub
source (Rust build, ~5 min first run), Open in Colab badge in first cell
- Add demo/README.md with Colab badge table for all 4 notebooks
Colab links:
https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/00_seed_data.ipynb
https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/01_llama_index_property_graph.ipynb
https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/02_langchain_graph_chain.ipynb
https://colab.research.google.com/github/structured-world/coordinode-python/blob/main/demo/notebooks/03_langgraph_agent.ipynb
…pe validation - Extract shared _build_property_definitions() static method: both create_label and create_edge_type now call it, eliminating duplicated _type_map and property-building loop (addresses CodeRabbit #1) - Raise ValueError for unknown property type in both methods instead of silently coercing to STRING; error includes property name and valid options (addresses CodeRabbit #2, already in previous commit) - Clarify _first_label() docstring: nodes "typically have one label in this application" rather than asserting system-level single-label constraint (addresses CodeRabbit #3)
Member
Author
|
CodeRabbit review addressed (3 threads): #1 (shared _type_map): Extracted into #2 (unknown type coercion): Now raises #3 (_first_label docstring): Updated to say "nodes typically have one label in this application" rather than asserting a system-level constraint. The |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demo/notebooks/01_llama_index_property_graph.ipynb`:
- Line 128: The adapter's no-op close() prevents LocalClient resources from
being released; update the adapter's def close(self): pass to delegate to the
embedded client by calling the LocalClient.close() (or the stored client
instance's close method) so that store.close() triggers proper cleanup of
temporary dirs and other resources; locate the adapter class with the close
method and replace the no-op with a call to the underlying client's close()
(e.g., self._client.close() or whichever attribute holds the LocalClient
instance).
In `@demo/notebooks/02_langchain_graph_chain.ipynb`:
- Line 129: The adapter's close() method currently no-ops and must forward to
the embedded LocalClient to avoid leaking resources; update the adapter class
(the method named close) to call the wrapped LocalClient instance's close() (the
same object created in Colab mode and stored on the adapter) and propagate any
exceptions or suppress them consistently with existing shutdown semantics so
that graph.close() triggers LocalClient.close() and cleans up DB resources and
temp dirs.
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 192-198: In find_related(entity_name: str, depth: int = 1) -> str,
depth is only clamped with min(depth, 3) which allows 0 or negative values and
leads to an invalid '*1..N' range; sanitize depth by computing a bounded_depth =
max(1, min(depth, 3)) (or equivalent) before building the Cypher fragment and
use bounded_depth in the string passed to client.cypher so the relationship
range is always between 1 and 3.
- Around line 173-181: The save_fact function currently interpolates the
tool-provided relation into the Cypher identifier (in client.cypher), allowing
injection; before building the query in save_fact, validate and sanitize
relation: normalize to upper(), replace spaces with underscores, then enforce a
strict whitelist regex (e.g., only [A-Z0-9_]+) and reject or map any value that
fails (raise an exception or use a safe fallback like DEFAULT_RELATION). Use the
sanitized token when concatenating into the Cypher relationship name and keep
all other values as params (subject, obj, SESSION) passed to client.cypher to
avoid further injection risks.
In `@langchain-coordinode/langchain_coordinode/graph.py`:
- Around line 48-54: The graph wrapper currently always closes self._client in
close(), which will close caller-owned clients passed via the client parameter;
change the constructor (where client: Any = None and self._client = client if
client is not None else CoordinodeClient(...)) to record ownership (e.g.
self._owns_client = client is None) and update the close() method to only call
close on self._client when self._owns_client is True, leaving externally
provided clients open.
In `@llama-index-coordinode/llama_index/graph_stores/coordinode/base.py`:
- Around line 71-75: The constructor currently takes a `client` parameter and
always assigns it to `self._client`, but `close()` unconditionally calls
`self._client.close()`, which can close externally managed clients; modify the
initializer (the function that sets `self._client`) to record ownership (e.g.,
set `self._owns_client = True` when you create `CoordinodeClient(addr,
timeout=timeout)` and `False` when a client is injected), and update the
`close()` method to only call `self._client.close()` if `self._owns_client` is
True; reference the initializer where `self._client` is set and the `close()`
method to implement this ownership check.
🪄 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: ca1cd5d3-f7f2-4ed9-994a-740d5bd812a8
📒 Files selected for processing (8)
coordinode/coordinode/client.pydemo/README.mddemo/notebooks/00_seed_data.ipynbdemo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynblangchain-coordinode/langchain_coordinode/graph.pyllama-index-coordinode/llama_index/graph_stores/coordinode/base.py
- ruff.toml: exclude coordinode-rs/ (submodule) and **/.ipynb_checkpoints/ from lint+format; add per-file-ignores for *.ipynb (E401/E402/I001 — normal notebook patterns: multi-import on one line, import after pip install cell) - Auto-format demo notebooks and coordinode-embedded .pyi stub - Fix UP037 in _coordinode_embedded.pyi: remove quotes from type annotation - Add .ipynb_checkpoints/ to .gitignore
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
demo/notebooks/02_langchain_graph_chain.ipynb (1)
132-133:⚠️ Potential issue | 🟡 MinorDelegate
close()to the underlyingLocalClientto release resources.Same issue as in notebook 01 — the adapter's
close()is a no-op whileLocalClient.close()should be called to release database resources.Suggested fix
def close(self): - pass + self._lc.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo/notebooks/02_langchain_graph_chain.ipynb` around lines 132 - 133, The adapter's close() method is currently a no-op; change it to delegate to the underlying LocalClient to release resources by calling the client's close method (e.g., invoke LocalClient.close() on the adapter's stored client instance from within close()). Locate the adapter class with def close(self): pass and replace the pass with a call to the underlying client (the attribute holding the LocalClient instance) so the adapter's close() forwards cleanup to LocalClient.close().demo/notebooks/01_llama_index_property_graph.ipynb (1)
131-132:⚠️ Potential issue | 🟡 MinorDelegate
close()to the underlyingLocalClientto release resources.The adapter's
close()is a no-op, butLocalClient.close()releases database resources. Whenstore.close()is called at the end of the notebook, resources remain uncleaned.Suggested fix
def close(self): - pass + self._lc.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo/notebooks/01_llama_index_property_graph.ipynb` around lines 131 - 132, The adapter's close() currently does nothing; update its close() implementation to call and await (if async) the underlying LocalClient.close() so resources are released when store.close() is invoked; locate the adapter class that defines def close(self): pass and replace it to delegate to the LocalClient instance (e.g., self._local_client.close() or await self._local_client.close()) ensuring correct sync/async semantics to mirror LocalClient.close().demo/notebooks/03_langgraph_agent.ipynb (2)
203-212:⚠️ Potential issue | 🟡 MinorClamp
depthto a valid lower bound.
min(depth, 3)allows0or negative values, producing an invalid*1..0or*1..-1range that will fail.Suggested fix
`@tool` def find_related(entity_name: str, depth: int = 1) -> str: """Find all entities reachable from entity_name within the given number of hops (max 3).""" + safe_depth = max(1, min(depth, 3)) rows = client.cypher( - "MATCH (n:Entity {name: $name, session: $sess})-[r*1.." + str(min(depth, 3)) + "]->(m) " + "MATCH (n:Entity {name: $name, session: $sess})-[r*1.." + str(safe_depth) + "]->(m) " "RETURN m.name AS related, type(last(r)) AS via LIMIT 20", params={"name": entity_name, "sess": SESSION}, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo/notebooks/03_langgraph_agent.ipynb` around lines 203 - 212, The function find_related constructs a Cypher range using min(depth, 3) which can be zero or negative; clamp depth to a valid lower bound by computing hops = max(1, min(depth, 3)) (or equivalent) and use hops when building the pattern (e.g., "[r*1..{hops}]") so the range is always at least 1 and no larger than 3; update the client.cypher call in find_related to use this hops variable when concatenating the query string.
182-191:⚠️ Potential issue | 🟠 MajorSanitize relationship type before interpolating into Cypher.
The
relationparameter from tool input is concatenated directly into the Cypher query. A crafted value could inject malicious Cypher (e.g.,RELATED]->(x) DELETE (a) //).Suggested fix
+import re + `@tool` def save_fact(subject: str, relation: str, obj: str) -> str: """Save a fact (subject → relation → object) into the knowledge graph. Example: save_fact('Alice', 'WORKS_AT', 'Acme Corp')""" + rel_type = relation.upper().replace(" ", "_") + if not re.fullmatch(r'[A-Z_][A-Z0-9_]*', rel_type): + return f"Invalid relation type: {relation}" client.cypher( "MERGE (a:Entity {name: $s, session: $sess}) " "MERGE (b:Entity {name: $o, session: $sess}) " - "MERGE (a)-[r:" + relation.upper().replace(" ", "_") + "]->(b)", + f"MERGE (a)-[r:{rel_type}]->(b)", params={"s": subject, "o": obj, "sess": SESSION}, ) return f"Saved: {subject} -[{relation}]-> {obj}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo/notebooks/03_langgraph_agent.ipynb` around lines 182 - 191, The save_fact function concatenates the tool-provided relation directly into the Cypher string (see save_fact, relation, client.cypher, SESSION), which allows Cypher injection; sanitize and validate relation before interpolation by normalizing (e.g., relation.upper().replace(" ", "_")) and enforcing a tight whitelist regex such as only A-Z, 0-9 and underscores (e.g., r'^[A-Z0-9_]+$'); if validation fails, raise an error and do not call client.cypher—only then interpolate the validated relation into the MERGE relationship token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demo/notebooks/01_llama_index_property_graph.ipynb`:
- Around line 108-139: Update the _EmbeddedAdapter class to document the stubbed
methods: add a brief note in the class docstring and/or inline comments inside
vector_search, get_labels, and get_edge_types stating these return empty results
because embedded mode does not implement vector search or label/edge
introspection; mention expected behavior (e.g., returns empty list) and point
users to the full CoordinodeClient implementation for production features so
it's clear these are intentional stubs.
---
Duplicate comments:
In `@demo/notebooks/01_llama_index_property_graph.ipynb`:
- Around line 131-132: The adapter's close() currently does nothing; update its
close() implementation to call and await (if async) the underlying
LocalClient.close() so resources are released when store.close() is invoked;
locate the adapter class that defines def close(self): pass and replace it to
delegate to the LocalClient instance (e.g., self._local_client.close() or await
self._local_client.close()) ensuring correct sync/async semantics to mirror
LocalClient.close().
In `@demo/notebooks/02_langchain_graph_chain.ipynb`:
- Around line 132-133: The adapter's close() method is currently a no-op; change
it to delegate to the underlying LocalClient to release resources by calling the
client's close method (e.g., invoke LocalClient.close() on the adapter's stored
client instance from within close()). Locate the adapter class with def
close(self): pass and replace the pass with a call to the underlying client (the
attribute holding the LocalClient instance) so the adapter's close() forwards
cleanup to LocalClient.close().
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 203-212: The function find_related constructs a Cypher range using
min(depth, 3) which can be zero or negative; clamp depth to a valid lower bound
by computing hops = max(1, min(depth, 3)) (or equivalent) and use hops when
building the pattern (e.g., "[r*1..{hops}]") so the range is always at least 1
and no larger than 3; update the client.cypher call in find_related to use this
hops variable when concatenating the query string.
- Around line 182-191: The save_fact function concatenates the tool-provided
relation directly into the Cypher string (see save_fact, relation,
client.cypher, SESSION), which allows Cypher injection; sanitize and validate
relation before interpolation by normalizing (e.g., relation.upper().replace("
", "_")) and enforcing a tight whitelist regex such as only A-Z, 0-9 and
underscores (e.g., r'^[A-Z0-9_]+$'); if validation fails, raise an error and do
not call client.cypher—only then interpolate the validated relation into the
MERGE relationship token.
🪄 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: 62214ae0-ea5f-4ac7-bc46-36478c863db6
📒 Files selected for processing (7)
.gitignorecoordinode-embedded/python/coordinode_embedded/_coordinode_embedded.pyidemo/notebooks/00_seed_data.ipynbdemo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynbruff.toml
…ebooks - graph.py, base.py: add _owns_client flag — close() skips shutdown for externally-injected clients (client= kwarg); only closes clients it created - notebooks 01/02: _EmbeddedAdapter.close() delegates to self._lc.close() instead of being a no-op - notebook 03: validate rel_type against _REL_TYPE_RE before Cypher interpolation; clamp find_related depth to max(1, min(int(depth), 3))
vector_search(), get_labels(), get_edge_types() return empty results in embedded mode — add inline comments explaining this limitation so users understand the behavior without reading server-side code
Member
Author
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 `@demo/notebooks/01_llama_index_property_graph.ipynb`:
- Line 228: The notebook creates a CoordinodePropertyGraphStore with an injected
client (CoordinodePropertyGraphStore(client=client)), but store.close() won’t
close that injected client under the current ownership model; update the cleanup
to explicitly call client.close() after store.close() (or instead of relying on
store.close()) so the underlying connection is properly closed—ensure you modify
the cleanup block that calls store.close() (around the store creation/cleanup
code) to add client.close() as well.
In `@demo/notebooks/02_langchain_graph_chain.ipynb`:
- Line 231: Because the notebook injects an external client into CoordinodeGraph
(graph = CoordinodeGraph(client=client)), graph.close() will not close that
injected client; update the cleanup to explicitly call client.close() in
addition to graph.close() (e.g., after using graph, call graph.close() then
client.close()) so the injected client's resources are released; reference
CoordinodeGraph, graph, client, graph.close(), and client.close() when making
the change.
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 134-150: The notebook starts a detached Docker container (proc ->
container_id, COORDINODE_ADDR) but never guarantees it is stopped on errors or
when re-running; wrap the docker run + port-wait logic in a try/finally (or
register an atexit handler) and in the finally call
subprocess.run(["docker","stop", container_id]) (checking container_id is set
and not empty) so the container is always stopped on exit or error; also update
the existing cleanup at the later block that attempted to stop the container
(lines referencing container_id/COORDINODE_ADDR) to reuse the same container_id
variable or guard it and call docker stop there if the try/finally path didn’t
run.
- Around line 201-205: query_facts currently executes arbitrary Cypher
(client.cypher) which allows destructive or cross-session operations; change it
to reject or sanitize input by enforcing read-only queries and session scoping:
validate the cypher string in query_facts to disallow dangerous
keywords/constructs (e.g., CREATE, DELETE, MERGE, CALL, LOAD, DROP, SET, ';')
and require that the query includes or is transformed to include a session
filter (params={"sess": SESSION} and WHERE n.session = $sess) or only allow a
small whitelist of safe patterns (e.g., STARTS WITH "MATCH" and no mutation
tokens); if validation fails, return an error instead of executing the query.
Ensure you reference and update the query_facts function and its use of
client.cypher to implement this validation/sanitization and fail-safe behavior.
In `@langchain-coordinode/langchain_coordinode/graph.py`:
- Around line 80-83: Replace the inline TODO at the comment above rows =
self._client.cypher with a non-TODO note or a tracked-issue reference: update
the comment that currently reads "TODO: simplify to labels(a)[0] once
subscript-on-function is in published image." to either reference the issue/PR
number (e.g., "see ISSUE-1234") or rewrite as a descriptive note (e.g., "Note:
can simplify to labels(a)[0] after subscript-on-function support is available")
so static analysis no longer flags a TODO; locate this comment adjacent to the
rows variable and self._client.cypher call in the Graph code to make the 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: 6ffce079-c347-4845-a1ce-6a32f536ec9a
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
demo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynblangchain-coordinode/langchain_coordinode/graph.pyllama-index-coordinode/llama_index/graph_stores/coordinode/base.py
There was a problem hiding this comment.
Pull request overview
Adds schema-creation APIs to the CoordiNode Python client (including SchemaMode support) and updates downstream integrations/demos to support injecting a pre-built client (e.g., embedded LocalClient) for easier Colab usage.
Changes:
- Add
create_label(..., schema_mode=...)andcreate_edge_type(...)to async + sync clients, plusschema_modesurfaced onLabelInfo/EdgeTypeInfo. - Allow LangChain/LlamaIndex graph stores to accept an externally-provided
client=and only close internally-owned clients. - Refresh demo notebooks/README for Colab + embedded mode; update lint config and expand unit/integration tests.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
coordinode/coordinode/client.py |
New schema CRUD APIs + schema_mode plumbing on info objects. |
langchain-coordinode/langchain_coordinode/graph.py |
Accept injected client and adjust schema refresh notes/docs. |
llama-index-coordinode/llama_index/graph_stores/coordinode/base.py |
Accept injected client and avoid closing caller-owned clients. |
tests/unit/test_schema_crud.py |
Unit coverage for schema_mode defaults/propagation and repr. |
tests/integration/test_sdk.py |
Integration coverage for create_label/create_edge_type and schema_mode validation. |
demo/notebooks/00_seed_data.ipynb |
Colab-ready seeding notebook using embedded mode when available. |
demo/notebooks/01_llama_index_property_graph.ipynb |
Colab-ready LlamaIndex demo using injected client adapter. |
demo/notebooks/02_langchain_graph_chain.ipynb |
Colab-ready LangChain demo using injected client adapter. |
demo/notebooks/03_langgraph_agent.ipynb |
Colab-ready LangGraph agent demo with embedded/local modes. |
demo/README.md |
Colab badge index and local Docker Compose instructions. |
ruff.toml |
Exclusions for submodules/checkpoints and notebook per-file ignores. |
coordinode-embedded/python/coordinode_embedded/_coordinode_embedded.pyi |
Minor stub typing tweak for __enter__. |
.gitignore |
Add ignore entry for .ipynb_checkpoints. |
uv.lock |
Workspace version bump in lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…fix CONTAINER_ID scope - notebooks 01/02: add explicit client.close() after store/graph.close() in cleanup cells — _owns_client prevents store from closing injected clients - notebook 03: promote container_id to module-level CONTAINER_ID = None so cleanup cell can stop auto-started Docker containers on re-runs - notebook 03: add write-keyword guard and $sess requirement to query_facts to prevent destructive or cross-session Cypher from agent-facing tool - graph.py:82: replace TODO comment with descriptive note referencing G010/GAPS.md to silence SonarCloud warning
polaz
commented
Apr 13, 2026
…ync docstring types - _build_property_definitions: raise ValueError with index when property entry is not a dict with a non-empty 'name' key (was KeyError) - LabelInfo.schema_mode inline annotation: 0=unspecified (not 0=unspecified/strict) - create_label docstring: add "list" and "map" to the type strings list - _first_label docstring: clarify min() is intentional for determinism; labels(n)[0] is unstable due to non-guaranteed label ordering
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 371-411: Rename the parameters in _build_property_definitions from
PropertyType and PropertyDefinition to snake_case names (e.g., property_type_cls
and property_definition_cls) to satisfy linters and clarify they are class
references; update all uses inside the function (type_map values and the call to
PropertyDefinition(...)) to refer to the new parameter names and adjust any call
sites that pass the proto classes into _build_property_definitions to use the
same positional or keyword names if necessary.
In `@demo/notebooks/01_llama_index_property_graph.ipynb`:
- Around line 397-402: The cleanup cell contains a duplicated/concatenated token
"callerclient.close()" — fix the typo by replacing the malformed trailing text
with a single call to client.close() and its comment; specifically update the
lines around store.delete(...), store.close(), and client.close() so the final
line reads a single client.close() and retains the comment "injected client —
owned by caller" (remove the extra "caller" concatenation and duplicate
client.close()).
In `@demo/notebooks/02_langchain_graph_chain.ipynb`:
- Around line 395-400: The cleanup cell contains duplicated text where the
client.close() call and its comment were concatenated; update the final lines in
the cell so the Neo4j calls are correct by leaving graph.query(...),
print("Cleaned up"), graph.close(), then a single client.close() with the
correct comment ("# injected client — owned by caller")—remove the duplicated
"client.close() # injected client — owned by callerclient.close() # injected
client — owned by caller" fragment.
🪄 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: 3c8ff839-a7b7-41aa-88c0-c9831cf19dc9
📒 Files selected for processing (5)
coordinode/coordinode/client.pydemo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynblangchain-coordinode/langchain_coordinode/graph.py
…bel visibility test Server now enforces strict schema for labels registered via create_label(). The workaround node must include only declared properties, so add tag:string to the schema alongside x:int64.
…case; fix duplicate client.close() in notebooks - _build_property_definitions: PropertyType → property_type_cls, PropertyDefinition → property_definition_cls (SonarCloud naming rule) - notebooks 01/02: remove duplicated client.close() line in cleanup cells
polaz
commented
Apr 13, 2026
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… dep to >=0.6.0 - CoordinodeGraph.refresh_schema() now uses get_labels() + get_edge_types() when available (coordinode>=0.6.0), bypassing the regex text parser; injected clients without these methods fall back to _parse_schema() - Add _PROPERTY_TYPE_NAME dict mapping PropertyType proto ints to strings (INT64, FLOAT64, STRING, BOOL, BYTES, TIMESTAMP, VECTOR, LIST, MAP) - Bump coordinode requirement to >=0.6.0 in both langchain-coordinode and llama-index-graph-stores-coordinode
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 25: Replace the single-root .ipynb_checkpoints/ ignore entry with a
recursive pattern so checkpoint directories are ignored at any depth; update the
.gitignore entry `.ipynb_checkpoints/` to use a recursive glob (e.g.,
`**/.ipynb_checkpoints/`) so Jupyter checkpoints alongside notebooks in
subdirectories are also excluded.
In `@coordinode/coordinode/client.py`:
- Around line 394-409: The loop that builds property_definition_cls currently
coerces 'required' and 'unique' with bool(...), allowing non-boolean values like
"false" or 1 to be treated as True; change the validation so that for each
property dict (in the properties loop inside the code that constructs
property_definition_cls) you explicitly check if 'required' or 'unique' keys are
present and, if so, assert they are instances of bool (raise ValueError with a
clear message referencing the property name/index), otherwise default them to
False; then pass the validated boolean values into
property_definition_cls(name=..., type=..., required=..., unique=...).
In `@demo/notebooks/01_llama_index_property_graph.ipynb`:
- Around line 332-337: The query passed to store.structured_query uses a
truncated tag in param_map ("prefix": f"Alice-{tag[:4]}) which can cause
cross-run collisions; update the f-string in the param_map for
store.structured_query to use the full tag (e.g., f"Alice-{tag}") so the WHERE
p.name STARTS WITH parameter is unique per run and preserves isolation.
- Around line 163-207: The notebook may leave a detached Docker container
running when you start CoordiNode (container_id created after subprocess.run)
because there is no teardown; modify the startup logic around the branch that
runs docker (where IMAGE, proc, container_id, and COORDINODE_ADDR are set) to
register a cleanup that stops the container (e.g., atry/finally or
atexit.register that calls subprocess.run(["docker","stop", container_id]))
and/or ensure any exception path (including the timeout else) always stops the
created container; ensure the cleanup uses the captured container_id and only
attempts to stop it if proc.returncode == 0 and container_id is non-empty so you
don’t try to stop a non-created container.
In `@demo/notebooks/02_langchain_graph_chain.ipynb`:
- Around line 288-293: The demo query uses a truncated prefix when calling
graph.query (params={"prefix": f"Turing-{tag[:4]}"}), which can collide with
other runs; change the params value to use the full tag (e.g., params={"prefix":
f"Turing-{tag}"} or otherwise include the entire unique tag variable) so the
MATCH filter on s.name is fully namespaced by the run's tag and prevents
cross-run matches.
- Around line 164-208: The cell can leave a started Docker container running
because when falling back to docker run you set container_id =
proc.stdout.strip() but never record or stop it on teardown; ensure you record
the started container id (e.g., in a module/global name like
STARTED_CONTAINER_ID) and update your cleanup logic to stop it
(subprocess.run(["docker","stop", STARTED_CONTAINER_ID])) only if that variable
is set and the container was created by this notebook; alternatively register an
atexit handler or try/finally around the startup sequence to call
subprocess.run(["docker","stop", container_id]) so
COORDINODE_ADDR/GRPC_PORT/_port_open logic still works and you avoid leaked
containers.
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 207-217: The current check in query_facts only looks for the
literal "$sess" token but doesn't ensure it's used to restrict node access;
update query_facts to validate that $sess is used in a WHERE clause that
constrains a node property (e.g. pattern matching "WHERE <alias>.session =
$sess" or equivalent) rather than merely present in the query or returned;
implement a robust check (regex or simple parsing) on q to confirm presence of a
WHERE condition binding a node/alias session property to $sess and reject
queries that only return $sess or embed it outside a WHERE predicate; keep the
existing read-only keyword check (_WRITE_KEYWORDS) and still call
client.cypher(q, params={"sess": SESSION}) only after this stricter
session-scoping validation.
In `@langchain-coordinode/langchain_coordinode/graph.py`:
- Around line 50-55: The constructor currently accepts a prebuilt client but
assumes it has get_schema_text() and vector_search(), causing refresh_schema()
and similarity_search() to fail when passed the simple LocalClient (which only
exposes cypher() and close()); update __init__ of the Graph class to either
validate the passed client implements the required protocol (cypher,
get_schema_text, vector_search) and raise a clear error, or automatically wrap
the passed LocalClient in a small adapter that implements get_schema_text()
(calling cypher under the hood) and vector_search() (delegating to an embedded
vector search implementation), and update usages of
get_triplets()/relation_names handling to accept relation_names=None and extract
relationship type via type(r) instead of r.__type__ so callers that omit
relation_names still work with CoordiNode v0.3.2; reference Graph.__init__,
refresh_schema, similarity_search, LocalClient, CoordinodeClient, and
get_triplets when making the changes.
In `@llama-index-coordinode/llama_index/graph_stores/coordinode/base.py`:
- Around line 73-76: The constructor currently accepts an injected client but
doesn't validate its API, causing later calls in get_schema() and vector_query()
to raise AttributeError when a bare coordinode-embedded LocalClient (which only
implements cypher() and close()) is passed; update the constructor that sets
self._client / self._owns_client to validate that the injected client implements
the richer protocol expected by get_schema() and vector_query() (e.g., has
get_schema_text and vector_search methods) and raise a clear TypeError if not,
or alternatively accept an adapter parameter and wrap a LocalClient into an
adapter implementing get_schema_text and vector_search before assigning
self._client (referencing CoordinodeClient, LocalClient, get_schema(),
vector_query(), get_schema_text, and vector_search to locate the code).
In `@proto`:
- Line 1: Verify the proto submodule update points to commit 381dbcd7 and
contains the expected schema-mode additions by updating/fetching the submodule
(git submodule update --init --recursive / git fetch && git checkout 381dbcd7),
then inspect the .proto files for the new SchemaMode enum, the
ComputedPropertyDefinition message, and schema_mode fields in relevant messages,
and confirm the create_label and create_edge_type RPCs reference schema_mode; if
any piece is missing, update the submodule to the correct commit or add the
missing proto definitions and re-run the proto compilation to ensure generated
clients reflect the changes.
🪄 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: a5821a64-2c69-4542-80fc-5c813932a1a1
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.gitignorecoordinode-embedded/python/coordinode_embedded/_coordinode_embedded.pyicoordinode/coordinode/client.pydemo/README.mddemo/notebooks/00_seed_data.ipynbdemo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynblangchain-coordinode/langchain_coordinode/graph.pyllama-index-coordinode/llama_index/graph_stores/coordinode/base.pyprotoruff.tomltests/integration/test_sdk.pytests/unit/test_schema_crud.py
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 `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 257-263: The current logic using _LIMIT_RE and _cap_limit modifies
any LIMIT token (including inner clauses) and therefore can miss capping the
outer query; update the regex _LIMIT_RE to only match a trailing end-of-query
LIMIT (e.g., match LIMIT <number> optionally followed by whitespace and an
optional semicolon at the end of the string) so you only substitute the final
LIMIT, then if that end-of-query LIMIT exists cap it via _cap_limit and replace
it, otherwise strip trailing semicolons/spaces from q and append " LIMIT 20" to
ensure the outer result set is always capped.
🪄 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: 1d4ea575-d2f7-43f7-9416-943c5f643b5e
📒 Files selected for processing (3)
demo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynb
…IT in query_facts
- client.py: _CYPHER_IDENT_RE.fullmatch() instead of .match() —
pattern is already anchored but fullmatch is explicit for DDL safety
- base.py, graph.py: add comments explaining SET r += {} is intentional
(no-op supported since coordinode v0.3.12, removes conditional branch)
- 03_langgraph_agent.ipynb: fix LIMIT bypass in query_facts sandbox guard;
_LIMIT_RE matched inner clauses (WITH ... LIMIT 1), leaving outer result
unbounded; replaced with _LIMIT_AT_END_RE anchored to end of query
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
demo/notebooks/03_langgraph_agent.ipynb (1)
319-322: 🧹 Nitpick | 🔵 Trivial
"Acme Corp"duplication remains in examples.Line 319/Line 321/Line 322 and Line 339 still repeat the same literal. This was already called out earlier; extracting a small constant would keep examples easier to maintain and silence repeat static-analysis noise.
♻️ Proposed refactor
+ACME_CORP = "Acme Corp" + print("=== Saving facts ===") -print(save_fact.invoke({"subject": "Alice", "relation": "WORKS_AT", "obj": "Acme Corp"})) +print(save_fact.invoke({"subject": "Alice", "relation": "WORKS_AT", "obj": ACME_CORP})) print(save_fact.invoke({"subject": "Alice", "relation": "MANAGES", "obj": "Bob"})) -print(save_fact.invoke({"subject": "Bob", "relation": "WORKS_AT", "obj": "Acme Corp"})) -print(save_fact.invoke({"subject": "Acme Corp", "relation": "LOCATED_IN", "obj": "Berlin"})) +print(save_fact.invoke({"subject": "Bob", "relation": "WORKS_AT", "obj": ACME_CORP})) +print(save_fact.invoke({"subject": ACME_CORP, "relation": "LOCATED_IN", "obj": "Berlin"})) @@ - "cypher": 'MATCH (p:Entity {session: $sess})-[:WORKS_AT]->(c:Entity {name: "Acme Corp", session: $sess}) RETURN p.name AS employee' + "cypher": f'MATCH (p:Entity {{session: $sess}})-[:WORKS_AT]->(c:Entity {{name: "{ACME_CORP}", session: $sess}}) RETURN p.name AS employee'Also applies to: 339-340
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo/notebooks/03_langgraph_agent.ipynb` around lines 319 - 322, Replace the repeated literal "Acme Corp" used in the example save_fact.invoke calls with a single named constant to avoid duplication: introduce a small constant (e.g., company = "Acme Corp") near the top of the notebook cell and update all occurrences where save_fact.invoke({..., "obj": "Acme Corp"}) appears (including the instances around save_fact.invoke in the shown block and the later examples around lines 339-340) to use that constant (e.g., "obj": company); keep the constant local to the example cell and preserve existing call structure and ordering.
🤖 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 613-616: The fallback branch that constructs TextIndexInfo drops
the caller-supplied language; update the return in that branch to include the
original default_language value (e.g., add "default_language": default_language)
so TextIndexInfo receives the explicit language when rows is empty; locate the
fallback return in the method that calls self.cypher (the block that returns
TextIndexInfo({"index": name, "label": label, "properties": ",
".join(prop_list)})) and add the default_language key to that dict.
In `@langchain-coordinode/langchain_coordinode/graph.py`:
- Around line 114-149: When the structured path populates node_props/rel_props
(in the try block that builds structured), also backfill self._schema with a
text representation so both views remain consistent for clients that only expose
structured APIs; after the line that constructs structured = {"node_props":
node_props, "rel_props": rel_props, "relationships": []} (and only if
self._schema is empty), generate a textual schema from node_props/rel_props
(using the same formatting that _parse_schema expects or a small helper) and
assign it to self._schema before leaving the try block so get_schema_text() will
return a consistent text form.
---
Duplicate comments:
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 319-322: Replace the repeated literal "Acme Corp" used in the
example save_fact.invoke calls with a single named constant to avoid
duplication: introduce a small constant (e.g., company = "Acme Corp") near the
top of the notebook cell and update all occurrences where save_fact.invoke({...,
"obj": "Acme Corp"}) appears (including the instances around save_fact.invoke in
the shown block and the later examples around lines 339-340) to use that
constant (e.g., "obj": company); keep the constant local to the example cell and
preserve existing call structure and ordering.
🪄 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: af64da1a-4152-4022-871d-0f0c60a7caee
📒 Files selected for processing (4)
coordinode/coordinode/client.pydemo/notebooks/03_langgraph_agent.ipynblangchain-coordinode/langchain_coordinode/graph.pyllama-index-coordinode/llama_index/graph_stores/coordinode/base.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…backfill schema text; add Dockerfile.jupyter - client.py: fallback TextIndexInfo (empty CREATE TEXT INDEX rows) now includes default_language so callers always get the explicit language back - client.py: fix isinstance tuple → union type (ruff UP038) - graph.py: add _structured_to_text(); backfill self._schema when only get_labels()/get_edge_types() are available, keeping graph.schema consistent with graph.structured_schema - demo/Dockerfile.jupyter: add missing file referenced by docker-compose.yml - demo/notebooks: extract ACME_CORP constant (03); fix rustup flags (00)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 541-563: The create_edge_type async method (create_edge_type)
currently omits a schema_mode parameter so the CreateEdgeTypeRequest uses server
default; add a schema_mode: str | None (or enum) parameter to create_edge_type
and its sync counterpart, validate/convert it to the proto enum the same way as
properties (reuse _build_property_definitions pattern if needed), and pass
schema_mode into CreateEdgeTypeRequest so the request includes the chosen mode;
ensure EdgeTypeInfo remains the return type and that the sync API variant at the
other location (the synchronous create_edge_type wrapper around the stub)
receives and forwards the new schema_mode argument as well.
- Around line 586-588: The zero-row fallback metadata currently preserves
TextIndexInfo.default_language as an empty string, which contradicts the
docstring that empty means the server default ("english"); update the fallback
path that constructs the TextIndexInfo metadata so that when default_language ==
"" it uses the effective default language ("english") instead of leaving it
empty. Locate the fallback branch that returns the zero-row TextIndexInfo
(references to TextIndexInfo.default_language in the client.py code around the
fallback logic) and set default_language = "english" when the stored value is
empty (do the same fix for the other identical branch at the second occurrence).
In `@demo/Dockerfile.jupyter`:
- Line 1: The Dockerfile currently uses the floating image tag
"jupyter/scipy-notebook:latest", which makes builds non-reproducible; update the
FROM line to pin a specific tested release tag or, preferably, an immutable
digest (e.g., replace "jupyter/scipy-notebook:latest" with a known tag like
"jupyter/scipy-notebook:2024-01-01" or "jupyter/scipy-notebook@sha256:...") so
the demo environment is stable and reproducible across rebuilds.
In `@demo/notebooks/00_seed_data.ipynb`:
- Around line 149-166: The code prints "Connected to ..." immediately after
creating CoordinodeClient but the constructor doesn't verify the server; update
both branches where CoordinodeClient(COORDINODE_ADDR) is instantiated to perform
a lightweight probe (e.g., call client.health() or the client's equivalent
health/ping method) and only print success after that probe succeeds; if the
probe fails, raise a RuntimeError (including the COORDINODE_ADDR and the probe
error) so the notebook fails fast; reference the CoordinodeClient constructor,
the client variable, and the _port_open branch so you apply this change in both
places.
🪄 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: 83e57d67-2de8-4f88-b016-17709034eefd
📒 Files selected for processing (8)
.gitignorecoordinode/coordinode/client.pydemo/Dockerfile.jupyterdemo/notebooks/00_seed_data.ipynbdemo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynblangchain-coordinode/langchain_coordinode/graph.py
…lth checks - create_edge_type(): add schema_mode parameter (strict/validated/flexible) matching create_label() — both async and sync variants - create_text_index(): use `language or "english"` in fallback TextIndexInfo so empty-string language resolves to the server default correctly - demo/Dockerfile.jupyter: pin base image from :latest to :python-3.11.6 - demo/install-sdk.sh: track untracked install script - notebooks 00, 02: add client.health() after CoordinodeClient() construction to surface connection errors early before running queries
polaz
commented
Apr 15, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 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: 4
🤖 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 755-788: Update the docstring for hybrid_text_vector_search to
explicitly state that a full-text index must exist on the target label (same
prerequisite as text_search), e.g. created via client.create_text_index(...) or
a CREATE TEXT INDEX command; note that calling this method on a label without a
text index will yield empty results, and mention that the index must cover the
fields used by text_query and coexist with the embedding property
(vector_property) used for vector search.
- Around line 624-629: The code currently checks isinstance(properties, str)
then does prop_list = list(properties) which can raise a TypeError for invalid
inputs (e.g., None or ints); update the validation to detect and reject
non-iterable/non-sequence inputs before coercion: explicitly check that
properties is a str or an iterable (e.g., list/tuple/set) or attempt to coerce
inside a try/except and on TypeError raise ValueError("'properties' must be an
iterable of property names"); ensure prop_list is set to [properties] for
strings or list(properties) for other iterables and keep the existing
empty-check that raises ValueError if prop_list is empty so callers consistently
get ValueError instead of raw TypeError (refer to the properties variable,
prop_list assignment, and the existing ValueError message).
In `@demo/notebooks/00_seed_data.ipynb`:
- Around line 149-168: The health probe CoordinodeClient.health() returns a
boolean but the code ignores False and still prints "Connected to ..."; update
both places where client = CoordinodeClient(COORDINODE_ADDR) and client.health()
is called to check the boolean result and fail fast: if health() returns False
raise a RuntimeError (or call sys.exit(1)) with a message that includes
COORDINODE_ADDR so downstream cells don't proceed with a dead endpoint.
In `@demo/notebooks/02_langchain_graph_chain.ipynb`:
- Around line 173-174: Check the boolean returned by client.health() before
printing "Connected": replace the unconditional calls to client.health()
followed by print(...) with logic in the notebook cells that captures the return
value (e.g., result = client.health()) and only prints "Connected to
{COORDINODE_ADDR}" when result is truthy; if health() is False, log or raise an
error and stop further execution to avoid continuing with an unhealthy client
(apply the same change for both occurrences around client.health() at the two
locations).
🪄 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: df2d11f2-9e37-4eef-abea-8ff03f420f4f
📒 Files selected for processing (7)
coordinode/coordinode/client.pydemo/Dockerfile.jupyterdemo/install-sdk.shdemo/notebooks/00_seed_data.ipynbdemo/notebooks/01_llama_index_property_graph.ipynbdemo/notebooks/02_langchain_graph_chain.ipynbdemo/notebooks/03_langgraph_agent.ipynb
… document hybrid index prerequisite; guard health() return value in notebooks
|
Member
Author
|
Closing to reopen with squashed history. |
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
create_label(name, properties, *, schema_mode)andcreate_edge_type(name, properties, *, schema_mode)toAsyncCoordinodeClient+ sync wrapperscreate_text_index()/drop_text_index()methods withTextIndexInforesult type; validate Cypher identifiers before DDL interpolationtext_search()andhybrid_text_vector_search()viaTextServicegRPC; clarify that aCREATE TEXT INDEXDDL is required before searchingprotosubmodule:SchemaModeenum,ComputedPropertyDefinition,TextService,DecommissionNodeRPCcoordinode-rssubmodule to v0.3.15 (5777554); regenerate proto stubs; pin Colab notebook install to matching coordinode-python commitschema_mode: intonLabelInfoandEdgeTypeInfoIN_COLABguard + pinnedcoordinode-embeddedinstall; addclient.health()after gRPC client construction for early failure detectionclient=parameter toCoordinodeGraphandCoordinodePropertyGraphStore;callable(getattr())guards for optional methodsrefresh_schema()inCoordinodeGraphto use structuredget_labels()/get_edge_types()API; fall back to_parse_schema(); backfillschematext from structured data whenget_schema_text()unavailablegraph.pyandbase.py—SET r += {}supported since v0.3.12_validate_property_dicthelper to reduce_build_property_definitionscognitive complexity below SonarCloud threshold-s --flags (stdin mode, wrong for file-path execution)LangGraph StatefulGraph→LangGraph StateGraph(correct class name)find_relatedexplaining path variable limitation (MATCH p=...) not yet supported by CoordiNodeTextIndexInfofallback: preservedefault_language; uselanguage or "english"so empty string resolves to server defaultre.fullmatch()for DDL identifier validator; fix LIMIT-bypass in demo query sandbox_structured_to_text()helper to keepgraph.schemaconsistent withgraph.structured_schema.gitignore: version files,DEVLOG*.md; adddemo/Dockerfile.jupyterisinstanceunion syntax (UP038) in_types.pyfor Python >=3.11Changes
coordinode/coordinode/client.pycreate_label,create_edge_type(both withschema_mode),create_text_index,drop_text_index,text_search,hybrid_text_vector_search,TextIndexInfotype;_validate_property_dicthelper;_validate_cypher_identifierfor DDL safety; fallback useslanguage or "english"for correct server-default resolutioncoordinode/coordinode/_types.pyisinstanceunion type syntax to PEP 604 style (UP038, Python >=3.11)langchain-coordinode/langchain_coordinode/graph.pyclient=param,callable(getattr())guards, structured schema API,_PROPERTY_TYPE_NAMEmapping, fallback to_parse_schema(); remove MERGE split workaround; add_structured_to_text()for schema backfillllama-index-coordinode/.../base.pyclient=param,callable(getattr())guards; remove MERGE split inupsert_relations()protocoordinode-rsdemo/notebooks/find_relateddocuments path variable limitation;ACME_CORPconstant extracted;client.health()added in notebooks 00 and 02; pinned commit updateddemo/README.mddemo/Dockerfile.jupyterjupyter/scipy-notebook:python-3.11.6demo/install-sdk.shdemo/docker-compose.ymlcoordinode:0.3.15; HTTP/healthreadiness probe; Jupyter on127.0.0.1:38888docker-compose.ymlcoordinode:0.3.15ruff.toml**/*.ipynb.gitignorecoordinode/_version.py,DEVLOG*.md);**/.ipynb_checkpoints/tests/unit/test_schema_crud.pytests/integration/test_sdk.pyDETACH DELETEin cleanupTest plan
uv run pytest tests/unit/— 54 passed, 0 failedCOORDINODE_ADDR=localhost:7082 uv run pytest tests/integration/againstghcr.io/structured-world/coordinode:0.3.15— 41 passed, 0 failedCloses #29