Skip to content

fix(arcadedb): wire up fulltext/similarity search (BM25 currently crashes)#2

Open
agc-63 wants to merge 2 commits into
ArcadeData:feat/arcadedb-backendfrom
agc-63:fix/arcadedb-search-interface
Open

fix(arcadedb): wire up fulltext/similarity search (BM25 currently crashes)#2
agc-63 wants to merge 2 commits into
ArcadeData:feat/arcadedb-backendfrom
agc-63:fix/arcadedb-search-interface

Conversation

@agc-63

@agc-63 agc-63 commented Jul 5, 2026

Copy link
Copy Markdown

Hey — while stress-testing the driver end-to-end for #1, ran into another one: ArcadeDBSearchOperations already has fulltext/similarity search implemented correctly (CONTAINS-based, sidesteps the Neo4j-only db.index.fulltext.query* procedures ArcadeDB doesn't support) — it just was never connected via search_interface. Same story for the other 4 providers' equivalent search_ops classes, per the driver-operations-architecture-redesign spec — this migration is still in progress project-wide, not an ArcadeDB-specific gap. In the meantime, search_utils.py checks driver.search_interface before falling back to the generic Neo4j-assuming path, and ArcadeDBDriver never set it, so every fulltext search crashed today.

Fix: a thin ArcadeDBSearchInterface adapter delegating to the existing (correct!) ArcadeDBSearchOperations, wired in ArcadeDBDriver.__init__. Wired exactly the 6 methods that search_utils.py calls without a try/except NotImplementedError safety net — the rest (6 more) keep using their already-working generic path, so this can't regress anything.

Also found and fixed 2 pre-existing bugs while finally exercising this end-to-end for the first time:

  1. node_similarity_search / edge_similarity_search both built an invalid double-WHERE when a search filter was present.
  2. lucene_sanitize() was used to build the CONTAINS search terms — it Lucene-escapes special characters with a literal backslash ('Alice''\Alice'), which broke almost every fulltext query silently (returned empty instead of matching). Verified live with real queries after the fix ('Alice' now correctly finds 'Alice Smith').

Known, pre-existing follow-ups not fixed here (neither used by us today): list_has_all() isn't implemented by ArcadeDB's Cypher engine (crashes if SearchFilters.node_labels is used against ArcadeDB); edge_bfs_search's generic fallback silently returns [] for one specific MATCH pattern shape.

Verified against a real 26.8.1-SNAPSHOT instance: full add_episode → retrieve_episodes → search cycle now returns correct BM25 results instead of crashing.

…ze breaking CONTAINS

ArcadeDBSearchOperations already implements fulltext/similarity search
correctly (CONTAINS-based, avoiding the Neo4j-only
db.index.fulltext.query{Nodes,Relationships} procedures ArcadeDB doesn't
support) -- it just was never connected via search_interface. The same is
true for the equivalent search_ops classes in the other 4 providers: per
the driver-operations-architecture-redesign spec, search_utils.py's
dispatchers are meant to gradually migrate to driver.search_ops, and that
migration is still in progress project-wide, not something specific to
ArcadeDB. In the meantime, search_utils.py's dispatchers check
driver.search_interface before falling back to the generic Neo4j-assuming
path, and ArcadeDBDriver never set it, so every fulltext search (edge, node,
episode, community) crashed.

- graphiti_core/driver/arcadedb/search_interface.py (new): ArcadeDBSearchInterface
  adapter delegating to ArcadeDBSearchOperations. Wires exactly the 6 methods
  search_utils.py calls WITHOUT a try/except NotImplementedError guard around
  the search_interface dispatch (edge_fulltext_search, edge_similarity_search,
  node_fulltext_search, node_similarity_search, episode_fulltext_search,
  community_fulltext_search) -- leaving any of those unwired would turn
  "search_interface not set" (falls through to the generic path) into
  "search_interface set but incomplete" (crashes with an uncaught
  NotImplementedError) for functionality that already worked via the generic
  path. The remaining 6 methods (edge_bfs_search, node_bfs_search,
  community_similarity_search, node_distance_reranker,
  episode_mentions_reranker, get_embeddings_for_communities) are left
  unwired on purpose: their generic paths use provider-agnostic Cypher and
  already work on ArcadeDB, and all 6 call sites DO guard with except
  NotImplementedError, so they safely keep using that path.

- graphiti_core/driver/arcadedb_driver.py: self.search_interface =
  ArcadeDBSearchInterface() in __init__.

- graphiti_core/driver/arcadedb/operations/search_ops.py: two bugs found and
  fixed while exercising this code end-to-end for the first time:
  1. node_similarity_search / edge_similarity_search built an invalid
     double-WHERE clause when a search filter was present (filter_query
     already started with WHERE, then an unconditional second WHERE was
     appended) -- merged into a single filter_queries list instead.
  2. lucene_sanitize(query) was used to build the CONTAINS search terms in
     all 4 fulltext methods (node/edge/episode/community). That function
     Lucene-escapes special/reserved characters with a literal backslash
     (e.g. 'Alice' -> '\Alice'), which is correct for building a real Lucene
     query string but breaks a literal CONTAINS substring match against
     unescaped stored text -- verified live that 'Alice' never matched
     toLower(n.name) CONTAINS '\alice'. Switched to using the raw query
     terms for the CONTAINS clauses; lucene_sanitize is still used via
     _build_arcadedb_fulltext_query purely as an empty/oversized-query gate,
     not for the actual match text.

Known follow-ups, not fixed here (pre-existing, orthogonal to this PR,
neither used by us today): list_has_all() is not implemented by ArcadeDB's
Cypher engine (crashes if a caller sets SearchFilters.node_labels against
ArcadeDB); edge_bfs_search's generic fallback returns [] silently for one
specific inline-map-literal-with-outer-scope-variable MATCH pattern.

Verified: 21/21 unit tests, full test suite (337 passed, same pre-existing
unrelated failures as before this change), and a live add_episode ->
retrieve_episodes -> search cycle against a real ArcadeDB instance now
returns correct BM25 results (e.g. searching 'Alice' correctly finds
'Alice Smith') instead of crashing or silently returning nothing.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request integrates ArcadeDB-specific search operations by implementing the ArcadeDBSearchInterface, resolving crashes caused by unsupported Neo4j fulltext procedures. It also fixes fulltext search matching by using raw query terms instead of Lucene-sanitized strings, and prevents engine-level datetime comparison errors in ArcadeDB by serializing datetime parameters to ISO strings. The review feedback points out a critical issue where ArcadeDBSearchInterface overrides the constructor of a Pydantic BaseModel without calling super().__init__(), which can bypass Pydantic's internal initialization and should be corrected.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +56 to +57
def __init__(self):
self._ops = ArcadeDBSearchOperations()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Overriding __init__ on a Pydantic BaseModel (which SearchInterface inherits from) without calling super().__init__() bypasses Pydantic's internal initialization process. This can cause runtime errors or unexpected behavior when Pydantic's internal features (such as validation, serialization, or field tracking) are utilized.

To fix this, ensure super().__init__() is called within the constructor, accepting and forwarding any keyword arguments (**data).

Suggested change
def __init__(self):
self._ops = ArcadeDBSearchOperations()
def __init__(self, **data: Any):
super().__init__(**data)
self._ops = ArcadeDBSearchOperations()

…erride

ArcadeDBSearchInterface's custom __init__ set self._ops directly without
calling super().__init__(), since SearchInterface extends pydantic's
BaseModel. This left the instance missing pydantic-internal attributes
(__pydantic_fields_set__, __pydantic_extra__), which doesn't break any
current call path (nothing serializes or deep-copies this object today)
but is fragile and not idiomatic pydantic v2.

Declare _ops as a PrivateAttr with a default_factory and drop the
custom __init__ entirely, which lets BaseModel's own __init__ run
normally.

Thanks to the gemini-code-assist review on this PR for catching it.
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.

1 participant