feat(tool_falkordb): query FalkorDB graphs with Cypher as an agent tool#1251
Merged
Conversation
New tool node exposing query, list_graphs and get_schema. Queries run read-only via GRAPH.RO_QUERY (the server itself rejects write clauses) unless allow_writes is enabled in config; values go through Cypher parameters so user data is never spliced into query text. Rows are capped at a configurable max with a truncated flag; nodes/edges/paths serialize to plain JSON; redis errors come back as error dicts. memory_persistent moves to redis>=7.1,<8: falkordb requires redis>=7.1 (older falkordb requires <6), so the previous >=6.4,<7 pin made the engine-wide uv constraints solve unsatisfiable. The redis-py surface memory_persistent uses (get/exists/hget/incrby/lrange/pexpire/pipeline/ pttl/rpush/sadd/scard/sismember/smembers/srem) is identical across 6.x and 7.x. Full-tree solve verified with uv pip compile: redis==7.4.1, falkordb==1.6.1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kwit75
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
Contributor
|
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:
📝 WalkthroughWalkthroughAdds a FalkorDB Cypher tool node: dependency updates, global lifecycle (IGlobal), instance exposing query/list_graphs/get_schema with serialization and structured error handling, service/config wiring, docs, and unit tests. ChangesFalkorDB Tool Node
Sequence Diagram(s)sequenceDiagram
participant User
participant IInstance
participant IGlobal
participant Graph
participant FalkorDB
User->>IInstance: query(cypher, params)
IInstance->>IInstance: validate cypher and params
IInstance->>IGlobal: select graph (or use default)
IGlobal-->>IInstance: Graph client
alt allow_writes is false
IInstance->>Graph: ro_query(cypher, params, timeout)
else allow_writes is true
IInstance->>Graph: query(cypher, params, timeout)
end
Graph->>FalkorDB: execute command
FalkorDB-->>Graph: rows, headers, optional stats
Graph-->>IInstance: result
IInstance->>IInstance: serialize rows, truncate to max_rows
IInstance-->>User: {columns, rows, truncated, stats?, error?}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID. Do not edit or delete. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nodes/test/test_tool_falkordb.py (1)
275-282: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen
get_schemacontract coverage.This test only checks
labels. Add assertions forrelationship_typesandproperty_keysto lock the documented/runtime response shape and prevent partial regressions.Proposed test assertion extension
def test_get_schema_shapes_columns(): graph = _FakeGraph(_FakeResult(result_set=[['Person'], ['City']], header=[[1, 'label']])) inst = _instance(_FakeGlobal(graph)) out = inst.get_schema({}) assert out['labels'] == ['Person', 'City'] + assert 'relationship_types' in out + assert 'property_keys' in out # All three procedures run read-only. assert all(call[0] == 'ro_query' for call in graph.calls)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/test/test_tool_falkordb.py` around lines 275 - 282, Extend the test_get_schema_shapes_columns to assert that inst.get_schema({}) returns the full schema shape: verify out contains 'relationship_types' and 'property_keys' keys and that their values match the expected lists (or at minimum are lists of the expected length/contents) in addition to 'labels'; locate the test using test_get_schema_shapes_columns and the helper fixtures (_FakeGraph, _FakeResult, _instance, get_schema) and add assertions after the labels check, and keep the existing check that all graph.calls are 'ro_query'.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/tool_falkordb/doc.md`:
- Line 9: Fix the typo in the intro sentence by replacing "Lets agents query a
FalkorDB..." with a grammatically correct form such as "Let's agents query..."
or better "Allows agents to query a FalkorDB..." in the documentation line that
mentions FalkorDB and GRAPH.RO_QUERY; ensure surrounding references like
"GRAPH.RO_QUERY" and the UI label "Allow Writes" remain unchanged.
- Line 27: Update the documented return contract in the "Returns" section to
explicitly include the error response shape used on Redis failures: document
that responses may include an "error" string and that in error cases "rows" and
"columns" are empty, "row_count" equals 0, and "truncated" is false; mention
that write "stats" will be absent or zero when an error occurs so consumers can
handle success vs error branches predictably.
---
Outside diff comments:
In `@nodes/test/test_tool_falkordb.py`:
- Around line 275-282: Extend the test_get_schema_shapes_columns to assert that
inst.get_schema({}) returns the full schema shape: verify out contains
'relationship_types' and 'property_keys' keys and that their values match the
expected lists (or at minimum are lists of the expected length/contents) in
addition to 'labels'; locate the test using test_get_schema_shapes_columns and
the helper fixtures (_FakeGraph, _FakeResult, _instance, get_schema) and add
assertions after the labels check, and keep the existing check that all
graph.calls are 'ro_query'.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 20f72337-6ade-433a-932f-57511b5c99fd
⛔ Files ignored due to path filters (1)
nodes/src/nodes/tool_falkordb/falkordb.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
nodes/src/nodes/memory_persistent/requirements.txtnodes/src/nodes/tool_falkordb/IGlobal.pynodes/src/nodes/tool_falkordb/IInstance.pynodes/src/nodes/tool_falkordb/README.mdnodes/src/nodes/tool_falkordb/__init__.pynodes/src/nodes/tool_falkordb/doc.mdnodes/src/nodes/tool_falkordb/requirements.txtnodes/src/nodes/tool_falkordb/services.jsonnodes/test/mocks/falkordb/__init__.pynodes/test/test_tool_falkordb.py
…omment, README mirror The error-path test raised a file-local stub exception while the module under test binds RedisError at import — with real redis already in sys.modules (the nodes:test-full condition) the except clause no longer caught it. The fake now raises mod.RedisError, which is correct under both stubbed and real imports (verified by running with real redis preimported). memory_persistent's requirements comment describes the current constraint only; README mirrors doc.md per the tool_apify convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/tool_falkordb/doc.md`:
- Line 27: The Returns sections for falkordb.list_graphs and falkordb.get_schema
omit the error response shapes; update the docs for those two tools to
explicitly state the failure shape: for falkordb.list_graphs add a sentence like
"On failure, returns `error` instead (with empty `graphs`)." and for
falkordb.get_schema add "On failure, returns `error` instead (with empty
`labels`, `relationship_types`, and `property_keys`)." so they match the query
error documentation.
In `@nodes/src/nodes/tool_falkordb/README.md`:
- Line 27: Update the README entries for falkordb.list_graphs and
falkordb.get_schema to explicitly document their error response shapes (as
implemented in IInstance.py): in the falkordb.list_graphs "Returns" section add
"On failure, returns `error` instead (with empty `graphs`)." and in the
falkordb.get_schema "Returns" section add "On failure, returns `error` instead
(with empty `labels`, `relationship_types`, and `property_keys`)." Ensure the
wording matches the query error description style and references the same field
names used by the implementation.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 551e7c49-77b4-430a-ba4c-878ab752bc49
📒 Files selected for processing (2)
nodes/src/nodes/tool_falkordb/README.mdnodes/src/nodes/tool_falkordb/doc.md
| | `params` | no | Values for the `$name` placeholders (injection-safe) | | ||
| | `graph` | no | Graph to query (default from config) | | ||
|
|
||
| Returns `columns`, `rows` (nodes/edges serialized to objects, capped at *Max Rows* with a `truncated` flag) and, when writes are enabled, non-zero write `stats`. On a query failure all three tools return `error` instead (with empty `rows`/`columns`, `row_count: 0`, `truncated: false` for `query`). |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Explicitly document error response shapes for falkordb.list_graphs and falkordb.get_schema.
Lines 31 and 39 document success returns but omit error cases, while line 27 clarifies the query error shape. Per the implementation, both list_graphs and get_schema return error plus their respective empty fields on failure:
list_graphserror:{error, graphs: []}get_schemaerror:{error, labels: [], relationship_types: [], property_keys: []}
Add explicit error documentation to the "Returns" sections for those two tools, matching the clarity provided for query.
✏️ Suggested updates
After line 31, add:
On failure, returns `error` instead (with empty `graphs`).After line 39, add:
On failure, returns `error` instead (with empty `labels`, `relationship_types`, and `property_keys`).Also applies to: 31-31, 39-39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nodes/src/nodes/tool_falkordb/doc.md` at line 27, The Returns sections for
falkordb.list_graphs and falkordb.get_schema omit the error response shapes;
update the docs for those two tools to explicitly state the failure shape: for
falkordb.list_graphs add a sentence like "On failure, returns `error` instead
(with empty `graphs`)." and for falkordb.get_schema add "On failure, returns
`error` instead (with empty `labels`, `relationship_types`, and
`property_keys`)." so they match the query error documentation.
| | `params` | no | Values for the `$name` placeholders (injection-safe) | | ||
| | `graph` | no | Graph to query (default from config) | | ||
|
|
||
| Returns `columns`, `rows` (nodes/edges serialized to objects, capped at *Max Rows* with a `truncated` flag) and, when writes are enabled, non-zero write `stats`. On a query failure all three tools return `error` instead (with empty `rows`/`columns`, `row_count: 0`, `truncated: false` for `query`). |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Explicitly document error response shapes for falkordb.list_graphs and falkordb.get_schema.
Line 27 clarifies the query error shape but leaves the error contracts for list_graphs and get_schema implicit. Per the implementation in IInstance.py, both tools return error plus their respective empty fields on failure:
list_graphserror:{error, graphs: []}get_schemaerror:{error, labels: [], relationship_types: [], property_keys: []}
Expand the "Returns" sections for those tools to document their error cases explicitly, matching the clarity provided for query.
✏️ Suggested update
After line 31 (in the falkordb.list_graphs section), add:
On failure, returns `error` instead (with empty `graphs`).After line 39 (in the falkordb.get_schema section), add:
On failure, returns `error` instead (with empty `labels`, `relationship_types`, and `property_keys`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nodes/src/nodes/tool_falkordb/README.md` at line 27, Update the README
entries for falkordb.list_graphs and falkordb.get_schema to explicitly document
their error response shapes (as implemented in IInstance.py): in the
falkordb.list_graphs "Returns" section add "On failure, returns `error` instead
(with empty `graphs`)." and in the falkordb.get_schema "Returns" section add "On
failure, returns `error` instead (with empty `labels`, `relationship_types`, and
`property_keys`)." Ensure the wording matches the query error description style
and references the same field names used by the implementation.
IInstance catches redis.exceptions.RedisError (falkordb-py re-raises redis-py exceptions verbatim and exports no error hierarchy of its own), so the import gets an explicit requirement instead of leaning on falkordb's transitive dependency. Same range the solve already pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
asclearuc
approved these changes
Jun 12, 2026
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
2 participants
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.
New
tool_falkordbnode — agents query a FalkorDB graph database directly with Cypher (Joe's integration list, #6). Deliberately a lightweighttool_*node (agent writes its own Cypher), not adb_*NL→Cypher pipeline component like db_neo4j — that heavier shape can come later if needed.Tools
falkordb.queryparams→$name, injection-safe), rows serialized + cappedfalkordb.list_graphsfalkordb.get_schemaSafety
GRAPH.RO_QUERYunlessallow_writesis on — write clauses are rejected by FalkorDB itself, not by a client-side regex.max_rows, default 250, max 25000 per repo convention) with atruncatedflag; server-side per-query timeout (ms).Live smoke (real server)
Ran the node code against
falkordb/falkordb:latestin Docker — 6 scenarios, all green:RO write rejection (server-side) ✓, parameterized CREATE + write stats ✓, Node/Edge serialization ✓, row cap + truncated ✓, schema + list_graphs ✓, bad-Cypher error dict ✓.
Dependency note
falkordb>=1.6,<2requiresredis>=7.1(and pre-1.4 falkordb requires<6), so no falkordb version coexists with memory_persistent'sredis>=6.4,<7in the engine-wide constraints solve. This PR bumps memory_persistent toredis>=7.1,<8: the redis-py surface it uses (get/exists/hget/incrby/lrange/pexpire/pipeline/pttl/rpush/sadd/scard/sismember/smembers/srem) is identical across 6.x/7.x, and the<7cap came from a conservative dependabot range (#949), not a compat need. Full-treeuv pip compileverified:redis==7.4.1,falkordb==1.6.1.Tests
nodes/test/test_tool_falkordb.py— 10 network-free unit tests (serialization incl. temporals, header shaping, ro/write routing, row cap, error dicts, graph override, validation-before-call).nodes/test/mocks/falkordb/→ keyless dynamic smoke runs;services.jsontest block; contracts 238 passed locally; ruff clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
Tests