feat(client,adapters,demo): schema DDL API, full-text search, Colab notebooks#37
Merged
feat(client,adapters,demo): schema DDL API, full-text search, Colab notebooks#37
Conversation
…otebooks
## Client (coordinode)
- create_label(name, properties, *, schema_mode) — node label DDL (strict/validated/flexible)
- create_edge_type(name, properties, *, schema_mode) — edge type DDL
- create_text_index(name, label, properties, *, language) / drop_text_index(name)
with Cypher identifier validation and consistent ValueError surface
- text_search(label, query, *, limit) — BM25 full-text search via TextService gRPC
- hybrid_text_vector_search(label, query, vector, ...) — BM25 + cosine RRF fusion
- LabelInfo, EdgeTypeInfo, TextIndexInfo, HybridResult result types
- Fix isinstance union syntax (UP038) in _types.py for Python >=3.11
## Adapters (langchain, llama-index)
- client= parameter on CoordinodeGraph and CoordinodePropertyGraphStore
- callable(getattr()) guards for optional methods (vector_search, get_schema_text)
- refresh_schema() uses get_labels() / get_edge_types() structured API
with _parse_schema() fallback and _structured_to_text() backfill
- Remove MERGE split workarounds — SET r += {} supported since v0.3.12
## Demo
- 4 Google Colab-ready notebooks with IN_COLAB guard and pinned SDK install
- demo/Dockerfile.jupyter pinned to jupyter/scipy-notebook:python-3.11.6
- demo/docker-compose.yml with /health readiness probe and Jupyter on 127.0.0.1:38888
- Notebook health checks: if not client.health(): raise RuntimeError(...)
## Infrastructure
- Bump coordinode-rs submodule to v0.3.15
- Update proto submodule (SchemaMode, ComputedPropertyDefinition, TextService, DecommissionNode)
- Regenerate proto stubs
- ruff.toml: exclude submodule, .ipynb_checkpoints, per-file ignores for notebooks
- .gitignore: version files, DEVLOG*.md, .ipynb_checkpoints
## Tests
- tests/unit/test_schema_crud.py: +8 tests (property validation, bool enforcement, schema_mode)
- tests/integration/test_sdk.py: +12 tests (FTS with explicit index lifecycle, schema_mode,
Cypher identifier validation, hybrid search, DETACH DELETE cleanup)
Related to #22
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds schema-mutating APIs, full-text and hybrid text-vector search with new result/metadata types and a text gRPC stub; injectible client support for LangChain/LlamaIndex adapters; demo Jupyter/Docker stack and notebooks; PEP 604 typing tweaks; lint/.gitignore updates; and submodule/image pins. No public API removals. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as CoordinodeClient
participant TextStub as TextSearch gRPC Stub
participant Server as CoordiNode Server
rect rgba(100, 200, 150, 0.5)
Note over Client,Server: Text Search Flow
Client->>TextStub: text_search(label, query, limit, fuzzy, language)
TextStub->>Server: TextSearch RPC
Server->>Server: BM25 index lookup
Server-->>TextStub: TextResult[] (node_id, score, snippet)
TextStub-->>Client: list[TextResult]
end
sequenceDiagram
participant Client as CoordinodeClient
participant HybridStub as Hybrid gRPC Stub
participant Server as CoordiNode Server
rect rgba(150, 150, 200, 0.5)
Note over Client,Server: Hybrid Text-Vector Search Flow
Client->>HybridStub: hybrid_text_vector_search(label, text_query, vector, weights)
HybridStub->>Server: HybridTextVectorSearch RPC
Server->>Server: BM25 + vector similarity
Server->>Server: Reciprocal Rank Fusion (RRF)
Server-->>HybridStub: HybridResult[] (node_id, combined_score)
HybridStub-->>Client: list[HybridResult]
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds schema DDL and full-text search APIs to the core coordinode Python client, updates LangChain/LlamaIndex adapters to support injected clients and structured schema retrieval, and introduces Colab/local demo notebooks + docker compose tooling to showcase the new capabilities.
Changes:
- Core client: schema CRUD (
create_label,create_edge_type), text index lifecycle (create_text_index,drop_text_index), and search APIs (text_search,hybrid_text_vector_search) plus new result/info types. - Adapters: optional
client=injection, safer capability detection (callable(getattr(...))), and structured schema refresh with text fallback/backfill. - Demos + tooling: notebooks, docker-compose + Jupyter image, updated lint ignores, and expanded unit/integration tests.
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
coordinode/coordinode/client.py |
Adds schema DDL + full-text search APIs, identifier/property validation helpers, and new TextResult/HybridResult/TextIndexInfo types. |
coordinode/coordinode/_types.py |
Fixes isinstance union syntax to PEP 604 form for Python ≥3.11. |
coordinode/coordinode/__init__.py |
Exposes new client result/info types at package top-level. |
langchain-coordinode/langchain_coordinode/graph.py |
Adds client= injection, structured schema refresh with fallback/backfill, capability guards, and removes MERGE split workaround. |
llama-index-coordinode/llama_index/graph_stores/coordinode/base.py |
Adds client= injection, ownership-aware close, vector capability detection, and removes MERGE split workaround. |
tests/unit/test_schema_crud.py |
Adds unit coverage for schema_mode propagation/defaulting and property-definition validation. |
tests/integration/test_sdk.py |
Adds integration coverage for schema CRUD + full-text index/search lifecycle (with UNIMPLEMENTED xfail wrapper). |
langchain-coordinode/pyproject.toml |
Bumps minimum coordinode dependency to >=0.6.0. |
llama-index-coordinode/pyproject.toml |
Bumps minimum coordinode dependency to >=0.6.0. |
docker-compose.yml |
Pins CoordiNode server image to 0.3.15. |
demo/docker-compose.yml |
Adds local demo stack (CoordiNode + Jupyter) with healthcheck and SDK source mount. |
demo/Dockerfile.jupyter |
Adds pinned Jupyter base image + dependencies for running notebooks. |
demo/install-sdk.sh |
Adds helper script to install SDK packages editable from mounted /sdk. |
demo/README.md |
Documents Colab launch links and local Docker Compose workflow/ports. |
demo/notebooks/00_seed_data.ipynb |
Adds seed-data notebook with embedded/gRPC autodetection and health fast-fail. |
demo/notebooks/01_llama_index_property_graph.ipynb |
Adds LlamaIndex property graph demo notebook (embedded/gRPC autodetection). |
demo/notebooks/02_langchain_graph_chain.ipynb |
Adds LangChain graph-chain demo notebook and embedded adapter example. |
demo/notebooks/03_langgraph_agent.ipynb |
Adds LangGraph “agent with graph memory” demo notebook. |
ruff.toml |
Excludes submodule + notebook checkpoint dirs and adds notebook per-file ignores. |
.gitignore |
Expands ignored generated version files and notebook checkpoint dirs. |
uv.lock |
Updates workspace lock version metadata. |
coordinode-embedded/python/coordinode_embedded/_coordinode_embedded.pyi |
Minor typing cleanup for LocalClient.__enter__ return annotation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…run install-sdk.sh at Jupyter startup
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 525-545: Extract the duplicated schema_mode handling into a shared
static helper (e.g., ClientClass._normalize_schema_mode(schema_mode: str | int,
mode_map: dict[str,int]) -> int) that implements the integer/str validation and
normalization logic shown (use the passed mode_map, check ints against
set(mode_map.values()), normalize strings with .strip().lower(), and raise the
same ValueError messages on invalid input), then replace the duplicated blocks
in the methods that build proto_schema_mode (the current block in the shown
function and the analogous block in create_edge_type) to call
_normalize_schema_mode(schema_mode, _mode_map) and assign its return to
proto_schema_mode.
In `@demo/docker-compose.yml`:
- Around line 42-43: The install script /tmp/install-sdk.sh is missing
installation of the coordinode-embedded package referenced by the notebooks;
update the script to add a pip installation step for coordinode-embedded by
inserting a line that runs pip install --no-cache-dir -e
/sdk/coordinode-embedded (same pattern used for coordinode,
llama-index-coordinode, and langchain-coordinode) so that coordinode-embedded is
available at runtime for the notebooks (e.g., ensure the pip install call is
executed before start-notebook.sh is invoked in the command that runs
"/tmp/install-sdk.sh && start-notebook.sh").
🪄 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: 9f54a57b-0409-4f85-ba92-f02fecd99673
📒 Files selected for processing (2)
coordinode/coordinode/client.pydemo/docker-compose.yml
- coordinode-rs submodule: v0.3.15 → v0.3.17 - proto submodule: advance to e1ab91d (write_concern/causal fields) - docker-compose.yml, demo/docker-compose.yml: image 0.3.15 → 0.3.17 - demo/README.md: update version references to v0.3.17 - regenerate _proto stubs (tracked separately via make proto)
Deduplicate schema_mode validation from create_label and create_edge_type into a shared AsyncCoordinodeClient._normalize_schema_mode() staticmethod. Both callers now pass their local _mode_map and receive the proto int back. Also remove unused langchain-coordinode from 03_langgraph_agent install cell — the notebook uses client.cypher() directly and never imports langchain_coordinode.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 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-rs`:
- Line 1: PR summary incorrectly states "bump coordinode-rs to v0.3.15" despite
the submodule commit 6df09c87a94eebd4c63dde08df420396df480487 and docker-compose
updates pointing to coordinode:0.3.17; update the PR title/body and any
changelog or commit message that mentions "v0.3.15" to "v0.3.17" so the PR
summary, commit messages, and docker-compose/submodule references are consistent
(search for the string "v0.3.15" in the PR description, title, and related
metadata and replace it with "v0.3.17").
In `@demo/docker-compose.yml`:
- Around line 42-43: The container command currently runs bash -c
"/tmp/install-sdk.sh && start-notebook.sh" which leaves bash as PID 1; update
the command so after running /tmp/install-sdk.sh it uses exec to replace the
shell with the Jupyter process (start-notebook.sh) — e.g., change the bash
invocation to run "/tmp/install-sdk.sh && exec start-notebook.sh" so
start-notebook.sh becomes PID 1 and improves signal handling.
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 83-84: The same pinned coordinode-embedded install spec string
("git+https://github.com/structured-world/coordinode-python.git@8da94d694ecaabee6f8380147d02f08220061bfa#subdirectory=coordinode-embedded")
is duplicated; extract it into a single named constant (e.g.,
COORDINODE_EMBEDDED_INSTALL_SPEC) near the notebook's top and replace both
occurrences (the install list and the error/guidance message) to reference that
constant so future bumps only need one change.
- Around line 256-263: In query_facts, avoid appending a second LIMIT by
changing the logic that handles q and _LIMIT_AT_END_RE: keep the existing
terminal-numeric LIMIT capping via _cap_limit when _LIMIT_AT_END_RE.search(q) is
true, but if any other LIMIT exists (detect with a case-insensitive
re.search(r"\\bLIMIT\\b", q)), do not append " LIMIT 20" — leave q unchanged;
only append or replace when there is no existing LIMIT at all. Update the code
around _LIMIT_AT_END_RE, _cap_limit, and the q reassignment to implement this
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: 909c428a-a185-4af5-a7c5-bb5f3ed0009f
📒 Files selected for processing (7)
coordinode-rscoordinode/coordinode/client.pydemo/README.mddemo/docker-compose.ymldemo/notebooks/03_langgraph_agent.ipynbdocker-compose.ymlproto
…; notebook fixes - Update _build_property_definitions annotation to include tuple[dict, ...] (matches isinstance check) - Update create_text_index annotation to include tuple[str, ...] in async and sync signatures - Add `exec` before start-notebook.sh so Jupyter becomes PID 1 (proper signal handling) - Extract _EMBEDDED_PIP_SPEC constant in notebook 03 to avoid drift on future bumps - Fix query_facts LIMIT guard: skip appending LIMIT 20 when query already has any LIMIT clause (prevents invalid double-LIMIT with parameterized LIMIT $n)
Member
Author
Member
Author
|
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 527-533: The public methods (e.g., create_label) advertise
properties: list[dict[str, Any]] | None but the internal helper
_build_property_definitions accepts tuples as well; update the type hints to
accept both lists and tuples of property definitions (e.g., list[dict[str, Any]]
| tuple[dict[str, Any], ...] | None or more concisely Sequence[dict[str, Any]] |
None) so static checkers allow tuple inputs; apply the same change to the other
affected methods (the ones around lines referenced: the method at ~570-576 and
the block at ~963-979) and ensure any import for typing (Sequence or Tuple) is
added if necessary.
- Around line 116-125: HybridResult currently only stores node_id and score, but
callers of hybrid_text_vector_search() expect a full node accessible as r.node;
update the HybridResult class (constructor __init__) to also set self.node from
proto_result.node (or otherwise resolve the node object corresponding to
proto_result.node_id) so consumers can inspect r.node.properties without an
extra lookup, and adjust __repr__ if needed to include or safely display the
node (while preserving existing node_id and score behavior).
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 245-272: The query_facts function currently allows parameter
placeholders like $n to pass the LIMIT handling but then calls client.cypher
with only params={"sess": SESSION}, causing unbound-parameter errors; fix by
scanning q for any $placeholders (e.g. with regex r"\$([A-Za-z_][A-Za-z0-9_]*)")
and if any placeholder name other than "sess" is present return an error message
rejecting non-$sess placeholders (so only $sess is permitted), then proceed to
run client.cypher(q, params={"sess": SESSION}); update the validation logic
inside query_facts (around where q is built and before rows =
client.cypher(...)) to enforce this 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: 7b9f0608-9345-4ec0-b71f-faea9cfe4267
📒 Files selected for processing (3)
coordinode/coordinode/client.pydemo/docker-compose.ymldemo/notebooks/03_langgraph_agent.ipynb
…bsent CreateEdgeTypeRequest does not have a schema_mode field in the current proto (only CreateLabelRequest supports it). Passing schema_mode caused "Protocol message has no field" at runtime. Remove the parameter entirely; add a docstring note that schema_mode for edge types is planned for a future server release.
- create_label (async + sync): widen properties annotation to list[dict] | tuple[dict, ...] | None (matches _build_property_definitions) - HybridResult: add doc comment explaining why .node is absent — proto HybridResult carries only node_id + score by design - query_facts (notebook): reject queries with parameters other than $sess to prevent unbound-parameter runtime errors
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: 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 770-771: Validate the `limit` argument before building the
TextSearchRequest and before the similar block at 818-827: ensure `limit` is an
int but not a bool (reject using isinstance(limit, bool)), and that it is
non-negative (or meets the same positive/zero constraint used elsewhere in this
class); if the check fails raise ValueError with a clear message. Apply this
same guard to the other request path referenced (the block around 818-827) so
both TextSearchRequest creation and the other RPC won't receive booleans or
negative values.
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 160-167: The implicit localhost branch currently raises a
RuntimeError if a process is listening on grpc_port but not serving CoordiNode;
change this so that after creating CoordinodeClient(COORDINODE_ADDR) and
checking client.health() you only raise the hard error when COORDINODE_ADDR was
explicitly provided by the user, otherwise call client.close() and fall through
to the embedded fallback (e.g., construct/use LocalClient); keep the _port_open
check and ensure COORDINODE_ADDR remains set for explicit mode but is treated as
non-fatal when auto-detected.
- Around line 250-256: The current session-scope check (around q =
cypher.strip() using _SESSION_WHERE_SCOPE_RE and _SESSION_NODE_SCOPE_RE) can be
bypassed by queries that include at least one scoped pattern while returning or
matching other unscoped nodes (e.g., MATCH (n), (m {session: $sess}) RETURN n),
leaking other sessions; update the query_facts validation to require that every
node identifier used in MATCH/RETURN is session-scoped or reject the query:
parse the cypher (or use regex) to extract all node aliases from MATCH clauses
and all aliases referenced in RETURN, then ensure each alias either appears in a
node pattern that includes {session: $sess} or is constrained by an explicit
WHERE alias.session = $sess; if any alias is unscoped, return an error (or
alternatively narrow the allowed shape to a single MATCH with all nodes scoped
or remove arbitrary Cypher from query_facts). Ensure you modify the guard around
_SESSION_WHERE_SCOPE_RE/_SESSION_NODE_SCOPE_RE in query_facts to implement this
per-alias 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: 0f77da55-b1a2-4cf7-83b9-f273a6ccdf56
📒 Files selected for processing (2)
coordinode/coordinode/client.pydemo/notebooks/03_langgraph_agent.ipynb
…llthrough - text_search, hybrid_text_vector_search: validate limit is int >= 1 (reject bool and negative/zero to avoid opaque server-side failures) - HybridResult: fix doc comment — point callers to client.get_node() instead of invalid MATCH (n) WHERE id(n) = ... Cypher - notebook 03: when auto-detected localhost port is open but not serving CoordiNode, close and fall through to LocalClient instead of raising; only raise hard error when COORDINODE_ADDR was explicitly provided
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.
♻️ Duplicate comments (4)
coordinode/coordinode/client.py (2)
774-828:⚠️ Potential issue | 🟡 MinorSame
limitvalidation needed here.Apply the same guard as in
text_searchfor consistency.🛡️ Proposed guard
async def hybrid_text_vector_search( self, label: str, text_query: str, vector: Sequence[float], *, limit: int = 10, text_weight: float = 0.5, vector_weight: float = 0.5, vector_property: str = "embedding", ) -> list[HybridResult]: """Fuse BM25 text search and cosine vector search using Reciprocal Rank Fusion (RRF). ... """ + if not isinstance(limit, int) or isinstance(limit, bool) or limit < 1: + raise ValueError(f"limit must be an integer >= 1, got {limit!r}.") from coordinode._proto.coordinode.v1.query.text_pb2 import ( # type: ignore[import] HybridTextVectorSearchRequest, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coordinode/coordinode/client.py` around lines 774 - 828, Add the same input validation guard used in text_search to hybrid_text_vector_search: at the start of CoordinatorNodeClient.hybrid_text_vector_search validate the limit parameter exactly as in text_search (check positive and within the same upper bound and raise the same ValueError/message), so the method enforces the same limit constraints before building HybridTextVectorSearchRequest and calling self._text_stub.HybridTextVectorSearch.
732-772:⚠️ Potential issue | 🟡 MinorValidate
limitbefore issuing the RPC (outstanding from prior review).The
limitparameter is passed through without validation. Sinceboolis a subclass ofintin Python,True/Falsesilently become1/0, and negative values pass through to opaque server-side failures. This is inconsistent with the validation pattern used elsewhere in this class (e.g.,traversevalidatesmax_depth).🛡️ Proposed guard
async def text_search( self, label: str, query: str, *, limit: int = 10, fuzzy: bool = False, language: str = "", ) -> list[TextResult]: """Run a full-text BM25 search over all indexed text properties for *label*. ... """ + if not isinstance(limit, int) or isinstance(limit, bool) or limit < 1: + raise ValueError(f"limit must be an integer >= 1, got {limit!r}.") from coordinode._proto.coordinode.v1.query.text_pb2 import TextSearchRequest # type: ignore[import] req = TextSearchRequest(label=label, query=query, limit=limit, fuzzy=fuzzy, language=language)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coordinode/coordinode/client.py` around lines 732 - 772, The text_search implementation currently forwards limit without validation; add the same guards used elsewhere (e.g., traverse's max_depth) to ensure limit is an int (reject bool) and non-negative before constructing TextSearchRequest: if limit is not an int or is a bool raise TypeError, and if limit < 0 raise ValueError; only then create TextSearchRequest(label=..., query=..., limit=limit, ... ) and call self._text_stub.TextSearch.demo/notebooks/03_langgraph_agent.ipynb (2)
213-217:⚠️ Potential issue | 🟠 MajorFail closed on partially scoped
MATCHpatterns.The current guard only proves that one pattern or
WHEREclause mentions$sess. Queries likeMATCH (n), (m {session: $sess}) RETURN nstill pass and can leak rows from other notebook sessions. For an agent-facing tool, this needs a per-alias scope check or a narrower allowed query shape that requires every matched node pattern to carry{session: $sess}.Also applies to: 250-256
🤖 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 213 - 217, The current notebook guard that only verifies "AT LEAST ONE" pattern or WHERE mentions $sess is insufficient (it allows queries like MATCH (n), (m {session: $sess}) RETURN n); update the guard logic that lives alongside the commented note to either (A) parse the Cypher AST and enforce a per-alias check so every node pattern identifier (each match alias) contains {session: $sess}, or (B) restrict allowed query shapes to a single MATCH with all node patterns explicitly bearing {session: $sess}; implement the chosen fix where the guard is evaluated (the code referenced by the comment block about the Cartesian-product query and $sess) and reject/raise on queries that do not satisfy the per-alias session scoping requirement.
155-181:⚠️ Potential issue | 🟠 MajorFall back to embedded mode when implicit localhost health checks fail.
In the auto-detect branch, any unrelated listener on
COORDINODE_PORTturns the notebook into a hard failure instead of continuing toLocalClient. That breaks the advertised “use gRPC if available, otherwise embedded” behavior; only an explicitCOORDINODE_ADDRshould be fatal.♻️ Proposed fix
else: try: grpc_port = int(os.environ.get("COORDINODE_PORT", "7080")) except ValueError as exc: raise RuntimeError("COORDINODE_PORT must be an integer") from exc + client = None if _port_open(grpc_port): COORDINODE_ADDR = f"localhost:{grpc_port}" from coordinode import CoordinodeClient client = CoordinodeClient(COORDINODE_ADDR) - if not client.health(): + if not client.health(): client.close() - raise RuntimeError(f"CoordiNode at {COORDINODE_ADDR} is not serving health checks") - print(f"Connected to {COORDINODE_ADDR}") - else: + client = None + else: + print(f"Connected to {COORDINODE_ADDR}") + + if client is None: # No server available — use the embedded in-process engine. try: from coordinode_embedded import LocalClient except ImportError as exc: raise RuntimeError(🤖 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 155 - 181, The auto-detect logic treats any open listener on COORDINODE_PORT as a hard failure when health() fails; change it so only an explicit COORDINODE_ADDR environment override is fatal. In the block that constructs CoordinodeClient(COORDINODE_ADDR) and calls client.health(), detect whether os.environ.get("COORDINODE_ADDR") was set: if it was set and health() is false, keep raising the RuntimeError; if it was not set (implicit localhost auto-detect), close the client and fall back to creating LocalClient(":memory:") and printing the embedded message instead of raising. Ensure you still close the CoordinodeClient before falling back.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@coordinode/coordinode/client.py`:
- Around line 774-828: Add the same input validation guard used in text_search
to hybrid_text_vector_search: at the start of
CoordinatorNodeClient.hybrid_text_vector_search validate the limit parameter
exactly as in text_search (check positive and within the same upper bound and
raise the same ValueError/message), so the method enforces the same limit
constraints before building HybridTextVectorSearchRequest and calling
self._text_stub.HybridTextVectorSearch.
- Around line 732-772: The text_search implementation currently forwards limit
without validation; add the same guards used elsewhere (e.g., traverse's
max_depth) to ensure limit is an int (reject bool) and non-negative before
constructing TextSearchRequest: if limit is not an int or is a bool raise
TypeError, and if limit < 0 raise ValueError; only then create
TextSearchRequest(label=..., query=..., limit=limit, ... ) and call
self._text_stub.TextSearch.
In `@demo/notebooks/03_langgraph_agent.ipynb`:
- Around line 213-217: The current notebook guard that only verifies "AT LEAST
ONE" pattern or WHERE mentions $sess is insufficient (it allows queries like
MATCH (n), (m {session: $sess}) RETURN n); update the guard logic that lives
alongside the commented note to either (A) parse the Cypher AST and enforce a
per-alias check so every node pattern identifier (each match alias) contains
{session: $sess}, or (B) restrict allowed query shapes to a single MATCH with
all node patterns explicitly bearing {session: $sess}; implement the chosen fix
where the guard is evaluated (the code referenced by the comment block about the
Cartesian-product query and $sess) and reject/raise on queries that do not
satisfy the per-alias session scoping requirement.
- Around line 155-181: The auto-detect logic treats any open listener on
COORDINODE_PORT as a hard failure when health() fails; change it so only an
explicit COORDINODE_ADDR environment override is fatal. In the block that
constructs CoordinodeClient(COORDINODE_ADDR) and calls client.health(), detect
whether os.environ.get("COORDINODE_ADDR") was set: if it was set and health() is
false, keep raising the RuntimeError; if it was not set (implicit localhost
auto-detect), close the client and fall back to creating LocalClient(":memory:")
and printing the embedded message instead of raising. Ensure you still close the
CoordinodeClient before falling back.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8440b939-4bbb-40bb-b94e-3c9fa6345d1e
📒 Files selected for processing (2)
coordinode/coordinode/client.pydemo/notebooks/03_langgraph_agent.ipynb
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 176-185: The import-failure handler in the "if _use_embedded"
block references _EMBEDDED_PIP_SPEC which may not be defined if the cell runs
standalone; fix by ensuring a local fallback pip spec string is used or by
defining a local constant before the try/except and then use that in the
RuntimeError message (refer to the LocalClient import/except block and
_EMBEDDED_PIP_SPEC symbol), so the exception message always contains a valid pip
install suggestion and never raises NameError.
🪄 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: 11d4a04c-a579-46c8-ad60-d35c38ee5b7d
📒 Files selected for processing (3)
coordinode/coordinode/client.pydemo/notebooks/03_langgraph_agent.ipynblangchain-coordinode/langchain_coordinode/graph.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eference
- _normalize_schema_mode: explicitly reject bool before int branch —
True/False would previously be accepted as int 1/0 silently
- notebook 03 connect cell: use globals().get('_EMBEDDED_PIP_SPEC', fallback)
so the ImportError message is actionable even when the cell runs standalone
Strengthen the existing trust-model comment with an explicit 'by design' note to reduce future review noise.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 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
create_label(name, properties, *, schema_mode)andcreate_edge_type(name, properties)toAsyncCoordinodeClient+ sync wrappers (schema_modenot yet supported by server for edge types)create_text_index()/drop_text_index()with Cypher identifier validation;language or "english"default resolution in fallbacktext_search()andhybrid_text_vector_search()viaTextServicegRPC; text index must exist before searchingschema_mode: intonLabelInfoandEdgeTypeInfo; newTextIndexInfo,HybridResulttypesclient=parameter toCoordinodeGraphandCoordinodePropertyGraphStore;callable(getattr())guards for optional methodsrefresh_schema()to useget_labels()/get_edge_types()structured API with_parse_schema()fallback and_structured_to_text()backfillSET r += {}on empty dict supported since v0.3.12coordinode-rsto v0.3.17; updateprotosubmodule (SchemaMode, ComputedPropertyDefinition, TextService, DecommissionNode)client.health()fast-fail, pinned SDK installdemo/Dockerfile.jupyterpinned tojupyter/scipy-notebook:python-3.11.6isinstanceunion syntax (UP038) in_types.pyfor Python ≥3.11Changes
coordinode/coordinode/client.pycreate_label(withschema_mode),create_edge_type,create_text_index,drop_text_index,text_search,hybrid_text_vector_search,TextIndexInfotype;_validate_property_dicthelper;_validate_cypher_identifierfor DDL safety; properties type guard; hybrid docstring prerequisite notecoordinode/coordinode/_types.pyisinstanceunion syntax to PEP 604 (UP038, Python ≥3.11)langchain-coordinode/langchain_coordinode/graph.pyclient=param,callable(getattr())guards, structured schema API,_PROPERTY_TYPE_NAMEmapping,_parse_schema()fallback,_structured_to_text()backfill; remove MERGE split workaroundllama-index-coordinode/.../base.pyclient=param,callable(getattr())guards; remove MERGE split inupsert_relations()protocoordinode-rsdemo/notebooks/if not client.health(): raise RuntimeError(...)in gRPC branches; per-line source format; pinned commitdemo/README.mddemo/Dockerfile.jupyterjupyter/scipy-notebook:python-3.11.6demo/install-sdk.shdemo/docker-compose.ymlcoordinode:0.3.17; HTTP/healthreadiness probe; Jupyter on127.0.0.1:38888docker-compose.ymlcoordinode:0.3.17ruff.toml.ipynb_checkpoints; per-file ignores for notebooks.gitignoreDEVLOG*.md,**/.ipynb_checkpoints/tests/unit/test_schema_crud.pytests/integration/test_sdk.pyTest 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.17— 41 passed, 0 failedRelated to #22