Skip to content

fix: slashes in destinationtype raise cypher error #2374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nocyphr
Copy link

@nocyphr nocyphr commented Mar 13, 2025

Description

llm generated destinationtype with slashes (eg Software/Model)
This raises an error because neo4j cypher does not allow slashes in entity names.
Extended the existing sanitizing method (replace spaces) to include replacing / with : and renamed it to _remove_invalid_chars_from_entities to reflect the more general functionality.
using : in cyphr allows us to define more than one label (which is pretty much what the llm was trying to do)
Fixes # (issue)
I did not report the issue, I just went and fixed it.

Type of change

Please delete options that are not relevant.

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

  • Unit Test

Checklist:

  • My code follows the style guidelines of this project (precommit)
  • [x ] I have performed a self-review of my own code (it's just one line and a rename, i considered refactoring it to just use re.sub instead and define more than one char but decided to skip it for now)
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

Copy link

vercel bot commented Mar 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
multimodal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 13, 2025 11:56am

@CLAassistant
Copy link

CLAassistant commented Mar 13, 2025

CLA assistant check
All committers have signed the CLA.

@nocyphr
Copy link
Author

nocyphr commented Mar 13, 2025

had to adjust the entity_type_map as well to avoid "/" from sneaking into the labels

@nocyphr
Copy link
Author

nocyphr commented Mar 13, 2025

@taranjeet @Dev-Khant any way to get this reviewed? I hotfixed it in my deployment but I would prefer to not have to redo it on each docker redeployment 😄

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.

2 participants