Merge pull request DeusData#1036 from DeusData/fix/797-varlength-paths

fix(cypher): shortest-path var-length semantics + advertised depth clamp

Author: DeusData

src/cypher/cypher.c

@@ -2944,6 +2944,13 @@ static void process_edges(cbm_store_t *store, cbm_edge_t *edges, int edge_count,
 }
 
 /* Expand variable-length relationship via BFS */
+/* Set when a variable-length hop range is clamped to the engine ceiling
+ * during the CURRENT execution; cbm_cypher_execute turns it into
+ * result->warning so callers can tell "clamped" from "no such path" (#797). */
+/* C11 _Thread_local directly: cypher.c stays windows.h-free (compat.h pulls
+ * in windows.h, whose legacy `far` macro breaks this file's identifiers). */
+static _Thread_local int g_cypher_depth_clamped = 0;
+
 static void expand_var_length(cbm_store_t *store, cbm_rel_pattern_t *rel,
                               cbm_node_pattern_t *target_node, binding_t *b, cbm_node_t *src,
                               const char *to_var, binding_t *new_bindings, int *new_count,
@@ -2960,6 +2967,7 @@ static void expand_var_length(cbm_store_t *store, cbm_rel_pattern_t *rel,
         snprintf(req_buf, sizeof(req_buf), "%d", max_depth);
         snprintf(cap_buf, sizeof(cap_buf), "%d", depth_cap);
         cbm_log_warn("cypher.depth_capped", "requested", req_buf, "cap", cap_buf);
+        g_cypher_depth_clamped = depth_cap; /* surfaced as result->warning (#797) */
         max_depth = depth_cap;
     }
     cbm_traverse_result_t tr = {0};
@@ -4518,6 +4526,7 @@ static int execute_single(cbm_store_t *store, cbm_query_t *q, const char *projec
 int cbm_cypher_execute(cbm_store_t *store, const char *query, const char *project, int max_rows,
                        cbm_cypher_result_t *out) {
     memset(out, 0, sizeof(*out));
+    g_cypher_depth_clamped = 0;
     if (max_rows <= 0) {
         max_rows = CYPHER_RESULT_CEILING;
     }
@@ -4573,6 +4582,14 @@ int cbm_cypher_execute(cbm_store_t *store, const char *query, const char *projec
     out->col_count = rb.col_count;
     out->rows = rb.rows;
     out->row_count = rb.row_count;
+    if (g_cypher_depth_clamped > 0) {
+        char wbuf[CBM_SZ_256];
+        snprintf(wbuf, sizeof(wbuf),
+                 "variable-length hop range clamped to the engine ceiling (%d) — an empty "
+                 "result may mean \"clamped\", not \"no such path\"",
+                 g_cypher_depth_clamped);
+        out->warning = heap_strdup(wbuf);
+    }
 
     cbm_query_free(q);
     return 0;
@@ -4582,6 +4599,8 @@ void cbm_cypher_result_free(cbm_cypher_result_t *r) {
     if (!r) {
         return;
     }
+    free(r->warning);
+    r->warning = NULL;
     for (int i = 0; i < r->col_count; i++) {
         safe_str_free(&r->columns[i]);
     }

src/cypher/cypher.h

@@ -310,6 +310,10 @@ typedef struct {
     int row_count;
     /* Non-NULL when the query was rejected (e.g. result too large) */
     char *error;
+    /* Non-NULL advisory (caller-visible, not an error): e.g. a variable-
+     * length hop range was clamped to the engine ceiling (#797) — without
+     * this, a clamped expansion is indistinguishable from "no such path". */
+    char *warning;
 } cbm_cypher_result_t;
 
 /* Execute a Cypher query against a store.

src/mcp/mcp.c

@@ -2486,6 +2486,9 @@ static char *handle_query_graph(cbm_mcp_server_t *srv, const char *args) {
             cbm_toon_row_end(&sb);
         }
         cbm_toon_scalar_int(&sb, "total", result.row_count);
+        if (result.warning) {
+            cbm_toon_scalar_str(&sb, "warning", result.warning);
+        }
         if (result.row_count == 0) {
             cbm_toon_scalar_str(&sb, "hint",
                                 "Query returned no results. Use get_graph_schema() to see "
@@ -2515,6 +2518,9 @@ static char *handle_query_graph(cbm_mcp_server_t *srv, const char *args) {
         }
         yyjson_mut_obj_add_val(doc, root, "rows", rows);
         yyjson_mut_obj_add_int(doc, root, "total", result.row_count);
+        if (result.warning) {
+            yyjson_mut_obj_add_str(doc, root, "warning", result.warning);
+        }
 
         if (result.row_count == 0) {
             yyjson_mut_obj_add_str(

src/store/store.c

@@ -3274,6 +3274,12 @@ int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const
     }
 
     snprintf(sql, sizeof(sql),
+             /* SHORTEST-PATH semantics: the UNION dedupes (node, hop) PAIRS,
+              * so a single self-loop minted every hop level for every node it
+              * could reach — walk-padding that fabricated *k..k Cypher matches
+              * of arbitrary length and exploded the row set to nodes x depth
+              * (#797). MIN(hop) GROUP BY node returns each node once at its
+              * minimal distance. */
              "WITH RECURSIVE bfs(node_id, hop) AS ("
              "  SELECT %lld, 0"
              "  UNION"
@@ -3282,12 +3288,13 @@ int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const
              "  JOIN edges e ON %s"
              "  WHERE e.type IN (%s) AND bfs.hop < %d"
              ")"
-             "SELECT DISTINCT n.id, n.project, n.label, n.name, n.qualified_name, "
-             "n.file_path, n.start_line, n.end_line, n.properties, bfs.hop "
+             "SELECT n.id, n.project, n.label, n.name, n.qualified_name, "
+             "n.file_path, n.start_line, n.end_line, n.properties, MIN(bfs.hop) AS hop "
              "FROM bfs "
              "JOIN nodes n ON n.id = bfs.node_id "
-             "WHERE bfs.hop > 0 " /* exclude root */
-             "ORDER BY bfs.hop "
+             "WHERE bfs.hop > 0 " /* exclude root at hop 0 (self via a loop still appears) */
+             "GROUP BY n.id "
+             "ORDER BY hop "
              "LIMIT %d;",
              (long long)start_id, next_id, join_cond, types_clause, max_depth, max_results);
 

tests/test_cypher.c

@@ -508,6 +508,92 @@ TEST(cypher_exec_where_eq) {
  * call outright ("unexpected operator"); RETURN-side coalesce already
  * worked, so only the WHERE leaf needs it. Semantics: when the property is
  * missing/empty, the literal default is compared instead. */
+/* #797: variable-length / repeated-variable path semantics. Fixture:
+ * loopy has a SELF-LOOP as one of its outbound CALLS edges plus a real
+ * 2-chain loopy->mid->leaf. Correct openCypher semantics:
+ *  - a repeated node variable must unify: (a)-[:CALLS]->(a) matches ONLY
+ *    the self-loop, not every edge;
+ *  - relationship uniqueness within a path: the self-loop cannot be
+ *    traversed repeatedly, so no *k..k path exists beyond the real chain;
+ *  - the engine hop cap must not fabricate or silently truncate results. */
+TEST(cypher_exec_varlength_path_semantics_issue797) {
+    cbm_store_t *s = cbm_store_open_memory();
+    cbm_store_upsert_project(s, "test", "/tmp/test");
+    cbm_node_t loopy = {.project = "test",
+                        .label = "Function",
+                        .name = "loopy",
+                        .qualified_name = "test.mod.loopy",
+                        .file_path = "mod.go",
+                        .start_line = 1,
+                        .end_line = 2};
+    cbm_node_t mid = {.project = "test",
+                      .label = "Function",
+                      .name = "mid",
+                      .qualified_name = "test.mod.mid",
+                      .file_path = "mod.go",
+                      .start_line = 3,
+                      .end_line = 4};
+    cbm_node_t leaf = {.project = "test",
+                       .label = "Function",
+                       .name = "leaf",
+                       .qualified_name = "test.mod.leaf",
+                       .file_path = "mod.go",
+                       .start_line = 5,
+                       .end_line = 6};
+    int64_t id_loopy = cbm_store_upsert_node(s, &loopy);
+    int64_t id_mid = cbm_store_upsert_node(s, &mid);
+    int64_t id_leaf = cbm_store_upsert_node(s, &leaf);
+    ASSERT_GT(id_loopy, 0);
+    cbm_edge_t self_loop = {
+        .project = "test", .source_id = id_loopy, .target_id = id_loopy, .type = "CALLS"};
+    cbm_edge_t e1 = {
+        .project = "test", .source_id = id_loopy, .target_id = id_mid, .type = "CALLS"};
+    cbm_edge_t e2 = {.project = "test", .source_id = id_mid, .target_id = id_leaf, .type = "CALLS"};
+    cbm_store_insert_edge(s, &self_loop);
+    cbm_store_insert_edge(s, &e1);
+    cbm_store_insert_edge(s, &e2);
+
+    /* Bug 1: repeated variable must unify — only the self-loop matches. */
+    cbm_cypher_result_t r1 = {0};
+    ASSERT_EQ(cbm_cypher_execute(s, "MATCH (a)-[:CALLS]->(a) RETURN a.name", "test", 0, &r1), 0);
+    ASSERT_EQ(r1.row_count, 1);
+    cbm_cypher_result_free(&r1);
+
+    /* Bug 2: *2..2 from loopy — only the REAL 2-chain (leaf); the self-loop
+     * must not be reused to pad paths (relationship uniqueness). */
+    cbm_cypher_result_t r2 = {0};
+    ASSERT_EQ(cbm_cypher_execute(s,
+                                 "MATCH (a {name: \"loopy\"})-[:CALLS*2..2]->(b) "
+                                 "RETURN DISTINCT b.name",
+                                 "test", 0, &r2),
+              0);
+    ASSERT_EQ(r2.row_count, 1); /* leaf only */
+    cbm_cypher_result_free(&r2);
+
+    /* Bug 2 amplifier: no directed path of length 5 exists at all. */
+    cbm_cypher_result_t r3 = {0};
+    ASSERT_EQ(cbm_cypher_execute(s,
+                                 "MATCH (a {name: \"loopy\"})-[:CALLS*5..5]->(b) "
+                                 "RETURN b.name",
+                                 "test", 0, &r3),
+              0);
+    ASSERT_EQ(r3.row_count, 0);
+    cbm_cypher_result_free(&r3);
+
+    /* Bug 3: a hop range beyond the engine ceiling must be an ADVERTISED
+     * clamp, not silently indistinguishable from "no such path". */
+    cbm_cypher_result_t r4 = {0};
+    ASSERT_EQ(
+        cbm_cypher_execute(s, "MATCH (a)-[:CALLS*150..150]->(b) RETURN b.name", "test", 0, &r4), 0);
+    ASSERT_EQ(r4.row_count, 0);
+    ASSERT_NOT_NULL(r4.warning);
+    ASSERT_NOT_NULL(strstr(r4.warning, "clamped"));
+    cbm_cypher_result_free(&r4);
+
+    cbm_store_close(s);
+    PASS();
+}
+
 TEST(cypher_exec_where_coalesce_issue874) {
     cbm_store_t *s = cbm_store_open_memory();
     cbm_store_upsert_project(s, "test", "/tmp/test");
@@ -2782,6 +2868,7 @@ SUITE(cypher) {
     RUN_TEST(cypher_issue252_tointeger);
     RUN_TEST(cypher_issue305_count_star_alias);
     RUN_TEST(cypher_exec_where_eq);
+    RUN_TEST(cypher_exec_varlength_path_semantics_issue797);
     RUN_TEST(cypher_exec_where_coalesce_issue874);
     RUN_TEST(cypher_exec_where_regex);
     RUN_TEST(cypher_exec_where_contains);