Fix memory_update: forward item_id by keyword in memU SQLite monkeypatch - #119
Merged
Conversation
memu-py 1.4.0's SQLiteMemoryItemRepo.update_item is keyword-only (def update_item(self, *, item_id, ...)), but the _indexed_update_item vector-index wrapper forwarded item_id positionally. That raised "takes 1 positional argument but 2 were given", caught inside the bridge, so every memory_update silently returned "Failed to update memory". delete_item(self, item_id) is not keyword-only, which is why deletes kept working and masked the bug. Forward args verbatim (matching the _numpy_create_item wrapper) and derive item_id from kwargs/args for the index hook. Add regression tests.
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
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.
Problem
The
memory_updatetool (memU item update) silently failed for every update, returningFailed to update memory <id>instead of surfacing an error.Root cause: the vector-index monkeypatch in
nerve/memory/memu_bridge.pywrapsSQLiteMemoryItemRepo.update_itemand forwardeditem_idpositionally:But
memu-py==1.4.0declaresupdate_item(self, *, item_id, ...)—item_idis keyword-only. The wrapper therefore called the original withself+item_idas two positionals, raising:update_item()in the bridge catches thatTypeErrorand returnsFalse, so the tool reported a silent failure rather than raising.delete_item(self, item_id)is not keyword-only, so the identical wrapper pattern worked there — which is why deletes succeeded and masked the bug. This has been latent since thememu-py==1.4.0pin.Fix
Forward args verbatim (matching the sibling
_numpy_create_itemwrapper) and deriveitem_idfrom kwargs/args for the index hook:Tests
Added
TestIndexedUpdateItemForwardingintests/test_memu_bridge.py:update_itemis keyword-only (catches future signature drift)._patch_sqlite_bugswrapper with a keyword-only spy original and assertsitem_idis forwarded by keyword without aTypeError(restores global state afterward).Full suite: 1158 passed, 2 skipped.