fix: connected-node fast path drops valid neighbors (0.0.11)#25
Merged
Conversation
The 0.0.10 single-hop find_connected_nodes fast path had two regressions, both invisible to the suite because JsonDB exposes no find_connected_nodes (tests fell back to the slow edge scan): 1. Limit-before-filter: `limit` was pushed to the DB scan before the Python node-type filter, so node(node=T) / nodes(node=T, limit=n) could return nothing when a non-matching neighbor sorted first. Push `limit` to the DB only when there is no node filter; otherwise apply it after filtering. 2. Subtype resolution under an entity-name collision: rows were deserialized with the base Node class, so when two Node subclasses in one database share an entity name (e.g. an app User and an embedded-agent User), find_subclass_by_name returned the first global match and isinstance-based filtering dropped the valid neighbor. Deserialize using the caller's requested concrete type as the resolution hint. Adds tests/core/test_connected_nodes_fast_path.py, which installs a find_connected_nodes shim so the fast path is actually exercised; both tests fail on 0.0.10 and pass here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 tasks
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
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.
Summary
Fixes two regressions in the 0.0.10 single-hop
find_connected_nodesfast path that made.node(node=T)/.nodes(node=T, limit=n)silently drop valid neighbors. Found via an end-to-end failure of the embedded jvagent agent in integral (agent turns died withno_memorythensession_resolution_error); root-caused to graph traversal, not agent code.Bug 1 — limit-before-filter.
limitwas pushed into the DB scan before the Python node-type filter, sonodes(node=T, limit=1)returned nothing when a non-matching neighbor sorted first, even though matching neighbors existed. (In integral: an agent with 20Memoryneighbors →node(node="Memory")returnedNone→get_memory()failed.) Fix: pushlimitto the DB only when there is no node filter; otherwise apply after filtering.Bug 2 — subtype resolution under an entity-name collision. The fast path deserialized every row with the base
Nodeclass, so when twoNodesubclasses persisted in one database share an entity name — e.g. an appUserand an embedded-agentUser—find_subclass_by_name(Node, "User")returned the first global match and_matches_node_filter'sisinstancecheck dropped the valid neighbor. (In integral:conversation.node(direction="in", node=User)returnedNone→ the session looked "foreign".) Fix: hydrate using the caller's requested concrete type as the resolution hint.Why the suite missed it
The stock
JsonDBexposes nofind_connected_nodes, so every existing test fell back to the slow edge-scan path and never exercised the fast path. The new test installs afind_connected_nodesshim.Test plan
tests/core/test_connected_nodes_fast_path.py— 2 tests; both fail on 0.0.10, pass here.Bumps version to 0.0.11 + CHANGELOG.
🤖 Generated with Claude Code