Skip to content

Update Neo4jVectorGraphStore params BaseModel #350

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.

Sign up for GitHub

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

Jump to bottom
Merged
edwinyyyu merged 4 commits into from
Oct 28, 2025
Merged

Update Neo4jVectorGraphStore params BaseModel #350

edwinyyyu merged 4 commits into from
Oct 28, 2025

Conversation

@edwinyyyu
Copy link
Contributor

@edwinyyyu edwinyyyu commented Oct 20, 2025

Purpose of the change

Refactor parameters for Neo4jVectorGraphStore to match decided standard.

Description

Instead of allowing arbitrary types, use InstanceOf. This should be safer.
Rename to match discussed (in-person) format.

Type of change

Breaking for consumers using Neo4jVectorGraphStoreConfig directly.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g., code style improvements, linting)
  • Documentation update
  • Project Maintenance (updates to build scripts, CI, etc., that do not affect the main project)
  • Security (improves security without changing functionality)

How Has This Been Tested?

Modified integration tests pass.

  • Unit Test
  • Integration Test
  • End-to-end Test
  • Test Script (please provide)
  • Manual verification (list step-by-step instructions)

Checklist

[Please delete options that are not relevant.]

  • I have signed the commit(s) within this pull request
  • My code follows the style guidelines of this project (See STYLE_GUIDE.md)
  • I have performed a self-review of my own code
  • I have commented my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Maintainer Checklist

  • Confirmed all checks passed
  • Contributor has signed the commit(s)
  • Reviewed the code
  • Run, Tested, and Verified the change(s) work as expected

@edwinyyyu
Update Neo4jVectorGraphStore params BaseModel
b3465fa 
Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>
o-love
o-love reviewed Oct 20, 2025
View reviewed changes
src/memmachine/common/vector_graph_store/neo4j_vector_graph_store.py Outdated
Comment on lines 97 to 99
uri = cast(str, config.uri)
username = cast(str, config.username)
password = cast(SecretStr, config.password).get_secret_value()
uri = cast(str, params.uri)
username = cast(str, params.username)
password = cast(SecretStr, params.password).get_secret_value()
Copy link
Contributor

@o-love o-love Oct 20, 2025

Choose a reason for hiding this comment

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

The idea would be more to go towards to only accepting AsyncDriver in the Params and the user is responsible always for building.

@edwinyyyu edwinyyyu requested a review from jealous October 21, 2025 00:11
@edwinyyyu
Take driver as dependency
aaa2573 
Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>
@edwinyyyu edwinyyyu requested a review from o-love October 21, 2025 00:31
@edwinyyyu
Emphasize required fields
c535eb0 
Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>
@edwinyyyu
Update docstring
eb02706 
Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>
svetly-todorov
svetly-todorov approved these changes Oct 27, 2025
View reviewed changes
Copy link
Contributor

@svetly-todorov svetly-todorov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Built & tested with a few posts and queries to profile memory.

@edwinyyyu edwinyyyu merged commit 631f77e into MemMachine:main Oct 28, 2025
24 checks passed
ChristianKniep pushed a commit to ChristianKniep/MemMachine that referenced this pull request Oct 29, 2025
@edwinyyyu @ChristianKniep
Update Neo4jVectorGraphStore params BaseModel (MemMachine#350)
dc104f4 
* Update Neo4jVectorGraphStore params BaseModel

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

* Take driver as dependency

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

* Emphasize required fields

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

* Update docstring

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

---------

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>
SarahScargall pushed a commit to SarahScargall/MemMachine that referenced this pull request Oct 30, 2025
@edwinyyyu @SarahScargall
Update Neo4jVectorGraphStore params BaseModel (MemMachine#350)
6b0a1db 
* Update Neo4jVectorGraphStore params BaseModel

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

* Take driver as dependency

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

* Emphasize required fields

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

* Update docstring

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>

---------

Signed-off-by: Edwin Yu <edwinyyyu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jealous jealous jealous approved these changes

@o-love o-love o-love approved these changes

@svetly-todorov svetly-todorov svetly-todorov approved these changes

@malatewang malatewang Awaiting requested review from malatewang

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@edwinyyyu @jealous @o-love @svetly-todorov