Skip to content

fix(falkordb): skip group_id tokenized away by RedisSearch#1330

Open
bsolomon1124 wants to merge 2 commits intogetzep:mainfrom
bsolomon1124:fix/falkordb-group-id-handling
Open

fix(falkordb): skip group_id tokenized away by RedisSearch#1330
bsolomon1124 wants to merge 2 commits intogetzep:mainfrom
bsolomon1124:fix/falkordb-group-id-handling

Conversation

@bsolomon1124
Copy link
Copy Markdown

@bsolomon1124 bsolomon1124 commented Mar 19, 2026

Summary

Fixes #1319; fixes FalkorDB default group ID and handling of the ID in query construction.

uv run examples/quickstart/quickstart_falkordb.py now succeeds.

Type of Change

  • Bug fix

Objective

Fix a bug related to the default group ID for FalkorDB/RedisSearch.

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • All existing tests pass

Breaking Changes

  • This PR contains breaking changes

Luckily, this PR fixes other breaking changes that broke functionality of FalkorDB driver when used with the default group_id.

PR #914 attempted to fix RedisSearch syntax. However, in attempting to deal with the fact that _ is a separator/tokenizer character in RedisSearch (resulting in @group_id:"_" matching nothing), the PR changed get_default_group_id for FalkorDB from '_' to '\\_' while keeping validate_group_id regex as is.

Fix:

  • Revert the default group_id for FalkorDB back to '_'
  • Skip the @group_id: filter when the group_id would be tokenized away by RedisSearch

Checklist

  • Code follows project style guidelines (make lint passes)
  • Self-review completed
  • Documentation updated where necessary
  • No secrets or sensitive information committed

Related Issues

Closes #1319

@danielchalef
Copy link
Copy Markdown
Member

danielchalef commented Mar 19, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Fixes getzep#1319

PR getzep#914 attempted to fix RedisSearch syntax. However, in
attempting to deal with the fact that `_` is a separator/tokenizer
character in RedisSearch (resulting in `@group_id:"_"` matching
nothing), the PR changed get_default_group_id for FalkorDB from '_' to
'\\_' while keeping validate_group_id regex as is.

Fix:

- Revert the default group_id for FalkorDB back to `'_'`
- skip the `@group_id:` filter when the group_id would be tokenized away
  by RedisSearch
@bsolomon1124 bsolomon1124 force-pushed the fix/falkordb-group-id-handling branch from ec418cc to 350d76c Compare March 19, 2026 00:52
@bsolomon1124
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

danielchalef added a commit that referenced this pull request Mar 19, 2026
lehcode pushed a commit to lehcode/graphiti-litellm that referenced this pull request Mar 22, 2026
@bsolomon1124
Copy link
Copy Markdown
Author

One thing: if a group_id is something like "foo_bar", sanitize would replace the underscore with a space, producing "foo bar" — which would match nodes with group_id "foo" OR "bar" instead of the exact "foo_bar". The quoting handles this for the fulltext query, but worth checking that the Cypher fallback still filters correctly in that case.

Thanks for the review @themavik . I'm not sure if I'm following here, can you elaborate?

if a group_id is something like "foo_bar", sanitize would replace the underscore with a space, producing "foo bar" — which would match nodes with group_id "foo" OR "bar" instead of the exact "foo_bar".

The sanitized value is only used for a truthiness check; it's not used to construct the fulltext query.

https://github.com/getzep/graphiti/pull/1330/changes#diff-99e252c05fd20e7c1e63fdaaac51f39bb8e6cce92c97c1b9e7c4ac8de469f37eR411

In the case of group_ids=["foo", "bar"], the resulting Cypher is WHERE n.group_id IN ["foo_bar"] since the sanitized value is discarded after the check.

@bsolomon1124 bsolomon1124 changed the title fix: skip group_id tokenized away by RedisSearch fix(falkordb): skip group_id tokenized away by RedisSearch Mar 23, 2026
2b3pro pushed a commit to 2b3pro/graphiti that referenced this pull request Mar 31, 2026
Port confirmed bug fixes from upstream getzep/graphiti PRs:

- PR getzep#1356: Fix label_propagation infinite loop by updating community_map
  before break check in bounded for-loop
- PR getzep#1362/getzep#1291: Strip markdown code fences from OpenAI generic client
  JSON responses before parsing
- PR getzep#1357: CJK character support in MinHash fuzzy dedup - use Unicode
  \w instead of [a-z0-9], detect CJK for 2-gram shingles vs 3-gram
- PR getzep#1332: Guard against null/invalid embeddings in similarity search
  by adding size() checks to Neo4j and FalkorDB vector queries
- PR getzep#1303: Search both edge directions during dedup resolution using
  bidirectional RELATES_TO match
- PR getzep#1330: Fix FalkorDB default_group_id from escaped '\\_' to '_'

Not applicable (TS port doesn't have the code):
- PR getzep#1281: No Gemini LLM client in TS port
- PR getzep#1276: TS resolver uses client-side scoring, not LLM context
- PR getzep#1289: TS reranker already returns 0 for empty logprobs
- PR getzep#1351: TS episode_mentions_reranker already sorts DESC
- PR getzep#1312: TS already validates node labels
- PR getzep#1212: TS addTripletFull already checks UUID collision
- PRs getzep#1326,1305,1295,1270,1222,1249,1272: FalkorDB RediSearch query
  building not present in TS port (uses Cypher CONTAINS)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ehfazrezwan added a commit to ehfazrezwan/neuralscape that referenced this pull request Apr 2, 2026
feafc42 Bump the uv group across 2 directories with 2 updates (getzep#1363)
c4e6923 Upstream Zep internal improvements (getzep#1361)
e88c09c @VictorECDSA has signed the CLA in getzep#1356
91fe7e0 @majiayu000 has signed the CLA in getzep#1351
c52786d @dudo has signed the CLA in getzep#1350
d631437 @Ker102 has signed the CLA in getzep#1339
73cff2c @chengjon has signed the CLA in getzep#1340
8c61763 @rhlsthrm has signed the CLA in getzep#1335
e6424ba @pratyush618 has signed the CLA in getzep#1332
6f05647 @bsolomon1124 has signed the CLA in getzep#1330
10d9139 @spencer2211 has signed the CLA in getzep#1326
1ca1468 Add hiring promotion section to README (getzep#1323)
19e44a9 Bump mcp-server to 1.0.2 and require graphiti-core>=0.28.2 (getzep#1317)
77b1609 Bump graphiti-core version to 0.28.2 (getzep#1315)
7d65d5e Harden search filters against Cypher injection (getzep#1312)
b10b488 Restore README title and subtitle (getzep#1314)
a9065fa Refresh README content and fix image refs (getzep#1313)
5a334ec @lvca has signed the CLA in getzep#1310
45c8040 @jawherkh has signed the CLA in getzep#1309
9eb2c9e @kraft87 has signed the CLA in getzep#1305
334c8fa @adsharma has signed the CLA in getzep#1296
b6f9d87 @StephenBadger has signed the CLA in getzep#1295
4b91076 feat: Add GLiNER2 hybrid LLM client (getzep#1284)
db54ce0 chore: update Docker images to graphiti-core 0.28.1 (getzep#1292)
edc71e8 @devmao has signed the CLA in getzep#1289
b4ddc55 @carlos-alm has signed the CLA in getzep#1288
aa8e81e @giulio-leone has signed the CLA in getzep#1280
6fdb352 @aelhajj has signed the CLA in getzep#1281
2099603 @avianion has signed the CLA in getzep#1278
9eb59f7 @themavik has signed the CLA in getzep#1214
98f5b5f fix: replace edge name with uuid in debug log (getzep#1261)
510bd50 @hanxiao has signed the CLA in getzep#1257
17a8ea9 @sprotasovitsky has signed the CLA in getzep#1254
9d509a2 @Yifan-233-max has signed the CLA in getzep#1245
ef52a2a chore: regenerate lockfiles to drop diskcache (getzep#1244)
7605303 chore: bump version to 0.28.1 (getzep#1243)
bde2f79 fix: replace diskcache with sqlite-based cache to resolve CVE (getzep#1238)

git-subtree-dir: graphiti
git-subtree-split: feafc42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Basic FalkorDB usage fails with GroupIdValidationError

2 participants