fix: connected-node fast path drops valid neighbors (0.0.11)

CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.11] - 2026-07-02
+
+### Fixed
+
+- **Single-hop `find_connected_nodes` fast path dropped valid neighbors** (`jvspatial/core/entities/node.py`). Two regressions introduced with the 0.0.10 fast path, both invisible to the existing suite because `JsonDB` exposes no `find_connected_nodes` (so tests fell back to the slow edge scan):
+  - *Limit-before-filter:* `limit` was pushed into 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, even though matching neighbors existed. `limit` is now applied to the DB scan only when there is no node filter; otherwise it is applied after filtering.
+  - *Subtype resolution under an entity-name collision:* rows were deserialized with the base `Node` class, so when two `Node` subclasses persisted in one database shared 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 silently dropped the valid neighbor. The fast path now hydrates using the caller's requested concrete type as the resolution hint.
+- Regression coverage: `tests/core/test_connected_nodes_fast_path.py` installs a `find_connected_nodes` shim so the fast path is exercised (the stock `JsonDB` never did).
+
 ## [0.0.10] - 2026-07-02
 
 ### Added

jvspatial/core/entities/node.py

@@ -677,18 +677,46 @@ async def _node_query(
                 edge_coll = context._get_collection_name(
                     context._get_entity_type_code(edge_cls)
                 )
+                # Only push ``limit`` into the DB scan when there is no node
+                # filter. With a node filter the type match happens in Python
+                # below, so a DB-side limit would truncate the candidate set
+                # BEFORE filtering (e.g. limit=1 fetches one neighbor that may
+                # not match the type, yielding an empty result even though
+                # matching neighbors exist). Fetch all, filter, then slice.
+                db_limit = limit if node_filter is None else None
                 records = await find_connected(
                     node_coll,
                     edge_coll,
                     self.id,
                     direction=direction,
                     edge_entity=edge_entity,
-                    limit=limit,
+                    limit=db_limit,
                 )
+                # Deserialize with the caller's requested concrete type as the
+                # hint (not the base ``Node``) when the node filter names one.
+                # ``_deserialize_entity`` resolves the stored ``entity`` name
+                # against this class's subtree first, so when two Node
+                # subclasses across embedded graphs share an entity name
+                # (e.g. an app ``User`` and an embedded-agent ``User``), the
+                # row hydrates as the class the caller asked for instead of the
+                # first global name match — otherwise ``_matches_node_filter``'s
+                # ``isinstance`` check silently drops a valid neighbor.
+                deser_class: type = NodeClass
+                if isinstance(node_filter, type):
+                    deser_class = node_filter
+                elif (
+                    isinstance(node_filter, list)
+                    and len(node_filter) == 1
+                    and isinstance(node_filter[0], type)
+                ):
+                    deser_class = node_filter[0]
+
                 connected_nodes: List["Node"] = []
                 for data in records:
                     try:
-                        node_obj = await context._deserialize_entity(NodeClass, data)
+                        node_obj: Optional["Node"] = await context._deserialize_entity(
+                            deser_class, data
+                        )
                         if node_obj:
                             connected_nodes.append(node_obj)
                             await context._add_to_cache(node_obj.id, node_obj)

jvspatial/version.py

@@ -9,4 +9,4 @@
 # - MAJOR: Breaking changes
 # - MINOR: New features, backward compatible
 # - PATCH: Bug fixes, backward compatible
-__version__ = "0.0.10"
+__version__ = "0.0.11"

tests/core/test_connected_nodes_fast_path.py

@@ -0,0 +1,145 @@
+"""Regression tests for the single-hop ``find_connected_nodes`` fast path.
+
+Both bugs below shipped in 0.0.10 because the stock ``JsonDB`` used by the
+rest of the suite exposes no ``find_connected_nodes`` method, so ``_node_query``
+silently fell back to the slow edge-scan path and the fast path went untested.
+These tests install a minimal ``find_connected_nodes`` shim to exercise it.
+
+Bug 1 — limit-before-filter: ``limit`` was pushed into the DB scan *before* the
+Python node-type filter, so ``nodes(node=T, limit=1)`` could return nothing when
+the first raw neighbor was a different type, even though matching neighbors
+existed.
+
+Bug 2 — subtype resolution under an entity-name collision: the fast path
+deserialized every row with the base ``Node`` class, so when two ``Node``
+subclasses shared an entity name (e.g. an app ``User`` and an embedded-agent
+``User`` persisted in one database) ``find_subclass_by_name`` returned the first
+global match and ``_matches_node_filter``'s ``isinstance`` check dropped the
+valid neighbor.
+"""
+
+from __future__ import annotations
+
+from typing import Any, Dict, List, Optional
+
+import pytest
+
+from jvspatial.core.context import GraphContext, set_default_context
+from jvspatial.core.entities.node import Node
+from jvspatial.db.jsondb import JsonDB
+
+
+class _FastJsonDB(JsonDB):
+    """JsonDB plus a ``find_connected_nodes`` so the fast path is exercised.
+
+    Mirrors the Postgres backend contract: strict source/target endpoints per
+    ``direction`` and DB-side ``limit`` applied to the raw (unfiltered) rows.
+    """
+
+    async def find_connected_nodes(
+        self,
+        node_collection: str,
+        edge_collection: str,
+        node_id: str,
+        *,
+        direction: str = "out",
+        edge_entity: Optional[str] = None,
+        limit: Optional[int] = None,
+    ) -> List[Dict[str, Any]]:
+        edges = await self.find(edge_collection, {})
+        rows: List[Dict[str, Any]] = []
+        for e in edges:
+            src, tgt = e.get("source"), e.get("target")
+            if direction == "out" and src == node_id:
+                other = tgt
+            elif direction == "in" and tgt == node_id:
+                other = src
+            else:
+                continue
+            if edge_entity is not None and e.get("entity") != edge_entity:
+                continue
+            n = await self.get(node_collection, other)
+            if n is not None:
+                rows.append(n)
+        # Deterministic order (Postgres has no implicit ORDER BY, so the bug
+        # surfaces whenever a non-matching neighbor happens to sort first).
+        rows.sort(key=lambda r: r["id"])
+        if limit is not None:
+            rows = rows[:limit]
+        return rows
+
+
+class Widget(Node):
+    """A distinct child type used to prove type-filtered fetches."""
+
+    label: str = ""
+
+
+class Gadget(Node):
+    """A different child type sharing the parent but not the filter."""
+
+    label: str = ""
+
+
+# Two subclasses that intentionally share one entity name — the collision that
+# broke subtype resolution on the fast path.
+class AlphaAccount(Node):
+    __entity_name__ = "CollidingAccount"
+    label: str = ""
+
+
+class BetaAccount(Node):
+    __entity_name__ = "CollidingAccount"
+    label: str = ""
+
+
+@pytest.fixture
+async def ctx(tmp_path):
+    context = GraphContext(database=_FastJsonDB(base_path=str(tmp_path)))
+    set_default_context(context)
+    yield context
+
+
+@pytest.mark.asyncio
+async def test_type_filter_with_limit_is_not_truncated_before_filtering(ctx):
+    """``nodes(node=Widget, limit=1)`` returns a Widget even when a non-Widget
+    neighbor sorts first — limit must apply after the type filter."""
+    parent = Node()
+    # ids chosen so the non-matching Gadget sorts first — a DB-side limit of 1
+    # would grab only the Gadget and then filter it out (the bug).
+    gadget = Gadget(id="n.Gadget.0000", label="g")
+    w1 = Widget(id="n.Widget.1111", label="w1")
+    w2 = Widget(id="n.Widget.2222", label="w2")
+    for n in (parent, gadget, w1, w2):
+        await ctx.save(n)
+    await parent.connect(gadget)
+    await parent.connect(w1)
+    await parent.connect(w2)
+
+    one = await parent.nodes(node=Widget, direction="out", limit=1)
+    assert len(one) == 1
+    assert isinstance(one[0], Widget)
+
+    got = await parent.node(node=Widget, direction="out")
+    assert got is not None and isinstance(got, Widget)
+
+
+@pytest.mark.asyncio
+async def test_subtype_resolved_under_entity_name_collision(ctx):
+    """When two Node subclasses share an entity name, the fast path hydrates the
+    concrete class the caller filtered on (not the first global name match)."""
+    parent = Node()
+    beta = BetaAccount(label="beta")
+    for n in (parent, beta):
+        await ctx.save(n)
+    await parent.connect(beta)
+
+    # AlphaAccount is defined first, so an unhinted global name lookup for
+    # "CollidingAccount" resolves to Alpha — the old bug dropped the Beta row.
+    got = await parent.node(node=BetaAccount, direction="out")
+    assert got is not None, "subtype filter dropped a valid neighbor (bug 2)"
+    assert isinstance(got, BetaAccount)
+    assert got.id == beta.id
+
+    many = await parent.nodes(node=BetaAccount, direction="out")
+    assert [n.id for n in many] == [beta.id]