~ BNodeResolver: harden recursive neighbor identification

Sibling bnode neighbors reached through the same link predicate used to
all take the FIRST candidate of the recursive resolution, collapsing
distinct neighbors (and thus their holes) onto one identity. Now, within
one interrogation response, neighbor labels resolve once (labels are
response-consistent) and siblings sharing a link pin draw successive
candidates from a single recursive resolution - a bijection, correct up
to automorphism. The neighbor pin is also narrowed by the arcs already
accumulated for the hole, discriminating it where the outer pin alone
does not, and pooling drops the duplicate recursion round trips.

test: siblings distinguishable only through a second bnode hop must get
distinct proxies whose mentions cover both leaf values (fails on the
pre-hardening code).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: Eric Prud'hommeaux

lib/BNodeResolver.cpp

@@ -192,6 +192,15 @@ namespace bnr {
 	 * us there are N nodes - collapsing to one witness would lose the
 	 * others. Distinct candidates get distinct proxies; structurally
 	 * identical ones dedup onto one witness via the fragment key. */
+	/* neighbor identification is shared across candidates: labels are
+	 * consistent within this one response, so a neighbor reached from two
+	 * candidates resolves once (nbrAssigned), and sibling neighbors
+	 * sharing a link pin draw successive candidates from one recursive
+	 * resolution (nbrPools) - a bijection, correct up to automorphism,
+	 * instead of collapsing them all onto the first candidate */
+	std::map<const TTerm*, const TTerm*> nbrAssigned;
+	std::map<std::string, std::pair<std::vector<const TTerm*>, size_t> > nbrPools;
+
 	std::vector<const TTerm*> proxies;
 	for (std::vector<const TTerm*>::const_iterator cand = order.begin();
 	     cand != order.end(); ++cand) {
@@ -204,19 +213,37 @@ namespace bnr {
 	    for (std::vector<Arc>::const_iterator a = arcs.begin(); a != arcs.end(); ++a) {
 		Arc arc = *a;
 		if (dynamic_cast<const BNode*>(arc.other) != NULL) {
-		    /* neighbor bnode: identify it through its link to the hole */
-		    std::string nbrPin = pinWhere
-			+ (arc.holeAtSubject
-			   ? "?bnrH9 " + sparqlTerm(arc.p) + " ?bnrHn9 . "
-			   : "?bnrHn9 " + sparqlTerm(arc.p) + " ?bnrH9 . ");
-		    /* re-pin with the neighbor as the new hole */
-		    std::string renamed;
-		    for (size_t i = 0; i < nbrPin.size(); ) {
-			if (nbrPin.compare(i, 7, "?bnrHn9") == 0) { renamed += "?bnrH9"; i += 7; }
-			else if (nbrPin.compare(i, 6, "?bnrH9") == 0) { renamed += "?bnrPrev9"; i += 6; }
-			else renamed += nbrPin[i++];
+		    std::map<const TTerm*, const TTerm*>::const_iterator known
+			= nbrAssigned.find(arc.other);
+		    if (known != nbrAssigned.end()) {
+			arc.other = known->second;
+		    } else {
+			/* pin the neighbor through its link to the hole,
+			 * narrowed by the arcs already accumulated for the
+			 * hole (they discriminate it where the outer pin
+			 * alone does not) */
+			std::string nbrPin = pinWhere;
+			size_t nctr = (depth + 1) * 100; // mention vars disjoint per level
+			nbrPin += fragmentWhere(frag, "bnrH9", nctr);
+			nbrPin += (arc.holeAtSubject
+				   ? "?bnrH9 " + sparqlTerm(arc.p) + " ?bnrHn9 . "
+				   : "?bnrHn9 " + sparqlTerm(arc.p) + " ?bnrH9 . ");
+			/* re-pin with the neighbor as the new hole */
+			std::string renamed;
+			for (size_t i = 0; i < nbrPin.size(); ) {
+			    if (nbrPin.compare(i, 7, "?bnrHn9") == 0) { renamed += "?bnrH9"; i += 7; }
+			    else if (nbrPin.compare(i, 6, "?bnrH9") == 0) { renamed += "?bnrPrev9"; i += 6; }
+			    else renamed += nbrPin[i++];
+			}
+			if (nbrPools.find(renamed) == nbrPools.end())
+			    nbrPools[renamed] = std::make_pair(
+				resolveBNode(renamed, inFlight, depth + 1), (size_t)0);
+			std::pair<std::vector<const TTerm*>, size_t>& pool = nbrPools[renamed];
+			const TTerm* nbr = pool.first[std::min(pool.second, pool.first.size() - 1)];
+			++pool.second;
+			nbrAssigned[arc.other] = nbr;
+			arc.other = nbr;
 		    }
-		    arc.other = resolveBNode(renamed, inFlight, depth + 1)[0];
 		}
 		frag.arcs.push_back(arc);
 		if (uniquelyIdentifies(frag)) {

tests/test_BNodeResolver.cpp

@@ -120,6 +120,47 @@ BOOST_AUTO_TEST_CASE( mention_rejoins_the_right_bnode ) {
     }
 }
 
+BOOST_AUTO_TEST_CASE( sibling_neighbors_distinguished_by_recursion ) {
+    /* two anonymous neighbors of x:s via the SAME predicate are themselves
+     * only distinguishable through THEIR neighbors (a second bnode hop):
+     * identification must recurse, and sibling neighbors must draw distinct
+     * candidates from the recursive resolution instead of all collapsing
+     * onto its first candidate */
+    sw::RdfDB db;
+    {
+	sw::TurtleDriver parser("", &F);
+	std::string ttl =
+	    "@prefix x: <http://a.example/x#> .\n"
+	    "x:s x:p _:n1, _:n2 .\n"
+	    "_:n1 x:q _:c1 . _:c1 x:r x:v1 .\n"
+	    "_:n2 x:q _:c2 . _:c2 x:r x:v2 .\n";
+	sw::IStreamContext istr(ttl, sw::IStreamContext::STRING);
+	parser.parse(istr, db.ensureGraph(sw::DefaultGraph));
+    }
+    sw::bnr::RelabelingLocalClient client(&F, &db);
+    sw::bnr::BNodeResolver resolver(&F, &client);
+
+    std::string where = "<http://a.example/x#s> <http://a.example/x#p> ?n . ";
+    sw::bnr::Table t = resolver.select("SELECT ?n WHERE { " + where + "}", where);
+    BOOST_REQUIRE_EQUAL(t.size(), 2u);
+    const sw::TTerm* ns[] = { t[0]["n"], t[1]["n"] };
+    BOOST_CHECK(ns[0] != ns[1]); // siblings must not collapse
+
+    /* each proxy re-queries to exactly one node, and between them the two
+     * reach both x:r values - the bijection covers both siblings */
+    std::set<std::string> vs;
+    for (int i = 0; i < 2; ++i) {
+	std::string w2;
+	size_t ctr = 0;
+	std::string m = resolver.mention(ns[i], w2, ctr);
+	w2 += m + " <http://a.example/x#q> ?c . ?c <http://a.example/x#r> ?v . ";
+	sw::bnr::Table t2 = resolver.select("SELECT ?v WHERE { " + w2 + "}", w2);
+	BOOST_REQUIRE_EQUAL(t2.size(), 1u);
+	vs.insert(t2[0].find("v")->second->getLexicalValue());
+    }
+    BOOST_CHECK_EQUAL(vs.size(), 2u);
+}
+
 BOOST_AUTO_TEST_CASE( provider_caches_and_subsumes ) {
     Endpoint ep;
     sw::RdfDB cacheDB;