Merge pull request DeusData#883 from jstar0/fix/cypher-path-semantics

fix(cypher): enforce variable-length path semantics

Author: DeusData

src/cypher/cypher.c

@@ -3180,6 +3180,7 @@ static void process_edges(cbm_store_t *store, cbm_edge_t *edges, int edge_count,
 /* 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 _Thread_local int g_cypher_trail_truncated = 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,
@@ -3202,7 +3203,11 @@ static void expand_var_length(cbm_store_t *store, cbm_rel_pattern_t *rel,
     }
     cbm_traverse_result_t tr = {0};
     const char *dir = rel->direction ? rel->direction : "outbound";
-    cbm_store_bfs(store, src->id, dir, rel->types, rel->type_count, max_depth, CBM_PERCENT, &tr);
+    cbm_store_bfs_trail(store, src->id, dir, rel->types, rel->type_count, max_depth, CBM_PERCENT,
+                        &tr);
+    if (tr.truncated) {
+        g_cypher_trail_truncated = 1;
+    }
     cbm_node_t *bound_to = binding_get(b, to_var);
     int64_t bound_to_id = bound_to ? bound_to->id : 0;
     /* Same contract as process_edges: the budget caps materialisation, never
@@ -4893,6 +4898,7 @@ int cbm_cypher_execute(cbm_store_t *store, const char *query, const char *projec
                        cbm_cypher_result_t *out) {
     memset(out, 0, sizeof(*out));
     g_cypher_depth_clamped = 0;
+    g_cypher_trail_truncated = 0;
     cypher_deadline_arm(); /* #601: start the wall-clock budget for this query */
     if (max_rows <= 0) {
         max_rows = CYPHER_RESULT_CEILING;
@@ -4963,12 +4969,22 @@ 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) {
+    if (g_cypher_depth_clamped > 0 || g_cypher_trail_truncated) {
         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);
+        if (g_cypher_depth_clamped > 0 && g_cypher_trail_truncated) {
+            snprintf(wbuf, sizeof(wbuf),
+                     "variable-length hop range clamped to the engine ceiling (%d) and "
+                     "traversal budget was exhausted — results may be partial",
+                     g_cypher_depth_clamped);
+        } else if (g_cypher_depth_clamped > 0) {
+            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);
+        } else {
+            snprintf(wbuf, sizeof(wbuf),
+                     "variable-length traversal budget was exhausted — results may be partial");
+        }
         out->warning = heap_strdup(wbuf);
     }
 

src/store/store.c

@@ -25,6 +25,7 @@ enum {
     ST_COL_7 = 7,
     ST_COL_8 = 8,
     ST_COL_9 = 9,
+    ST_COL_10 = 10,
     ST_FOUND = -1,
     ST_BUF_16 = 16,
     ST_BUF_64 = 64,
@@ -63,6 +64,8 @@ enum {
     ST_METHOD_PROP_LEN = 8,
     ST_PATH_PROP_LEN = 6,
     ST_HANDLER_PROP_LEN = 9,
+    ST_BFS_CTE_ROW_MULTIPLIER = 8,
+    ST_BFS_MAX_CTE_ROWS = 4096,
 };
 
 #define SLEN(s) (sizeof(s) - 1)
@@ -4673,8 +4676,35 @@ static void bfs_build_types_clause(int edge_type_count, char *buf, int buf_sz) {
     }
 }
 
-int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const char **edge_types,
-                  int edge_type_count, int max_depth, int max_results, cbm_traverse_result_t *out) {
+static int bfs_cte_row_limit(int max_results) {
+    int result_budget = max_results > 0 ? max_results : ST_INIT_CAP_16;
+    long row_budget = (long)result_budget * ST_BFS_CTE_ROW_MULTIPLIER + SKIP_ONE;
+    if (row_budget > ST_BFS_MAX_CTE_ROWS) {
+        return ST_BFS_MAX_CTE_ROWS;
+    }
+    if (row_budget < ST_INIT_CAP_16) {
+        return ST_INIT_CAP_16;
+    }
+    return (int)row_budget;
+}
+
+static int bfs_cte_row_limit_for_depth(int max_results, int max_depth) {
+    int base = bfs_cte_row_limit(max_results);
+    /* Reserve rows across the requested depth range.  A result-sized cap can
+     * be exhausted by a wide first hop before any deeper match is generated;
+     * scaling by depth keeps the bounded traversal useful for *N..N queries
+     * while the hard ceiling still protects the SQLite worker. */
+    long depth = max_depth > 0 ? max_depth : ST_INIT_CAP_16;
+    long scaled = (long)base * depth;
+    if (scaled > ST_BFS_MAX_CTE_ROWS) {
+        return ST_BFS_MAX_CTE_ROWS;
+    }
+    return (int)scaled;
+}
+
+static int store_bfs(cbm_store_t *s, int64_t start_id, const char *direction,
+                     const char **edge_types, int edge_type_count, int max_depth, int max_results,
+                     bool trail, cbm_traverse_result_t *out) {
     memset(out, 0, sizeof(*out));
 
     cbm_node_t root = {0};
@@ -4701,32 +4731,59 @@ int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const
         next_id = "e.target_id";
     }
 
-    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"
-             "  SELECT %s, bfs.hop + 1"
-             "  FROM bfs"
-             "  JOIN edges e ON %s"
-             "  WHERE e.type IN (%s) AND bfs.hop < %d"
-             ")"
-             "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 at hop 0 (self via a loop still appears) */
-             "GROUP BY n.id "
-             /* (hop, id) is a unique total order — deterministic pagination
-              * watermarks and reproducible trace output depend on it. */
-             "ORDER BY hop, n.id "
-             "LIMIT %d;",
-             (long long)start_id, next_id, join_cond, types_clause, max_depth, max_results);
+    int cte_row_limit = 0;
+    if (trail) {
+        cte_row_limit = bfs_cte_row_limit_for_depth(max_results, max_depth);
+        snprintf(sql, sizeof(sql),
+                 "WITH RECURSIVE bfs(node_id, hop, edge_path) AS ("
+                 "  SELECT %lld, 0, ''"
+                 "  UNION"
+                 "  SELECT %s, bfs.hop + 1,"
+                 "         CASE WHEN bfs.edge_path = '' THEN CAST(e.id AS TEXT)"
+                 "              ELSE bfs.edge_path || ',' || e.id END"
+                 "  FROM bfs"
+                 "  JOIN edges e ON %s"
+                 "  WHERE e.type IN (%s) AND bfs.hop < %d"
+                 "    AND instr(',' || bfs.edge_path || ',', ',' || e.id || ',') = 0"
+                 /* SQLite's recursive queue is breadth-first by default.  A
+                  * bounded trail budget must reach the requested depth before
+                  * a wide shallow layer consumes every row, so prioritize the
+                  * recursive hop (depth-first) while retaining deterministic
+                  * ordering in the outer result query. */
+                 "  ORDER BY 2 DESC, 1 ASC, 3 ASC"
+                 "  LIMIT %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 count(*) FROM bfs) "
+                 "FROM bfs JOIN nodes n ON n.id = bfs.node_id "
+                 "WHERE bfs.hop > 0 ORDER BY bfs.hop, n.id LIMIT %d;",
+                 (long long)start_id, next_id, join_cond, types_clause, max_depth,
+                 cte_row_limit + SKIP_ONE, max_results);
+    } else {
+        snprintf(sql, sizeof(sql),
+                 /* SHORTEST-PATH semantics: the UNION dedupes (node, hop) pairs.
+                  * MIN(hop) GROUP BY node returns each node once at its minimal
+                  * distance, while relationship-trail state remains scoped to
+                  * Cypher variable-length expansion. */
+                 "WITH RECURSIVE bfs(node_id, hop) AS ("
+                 "  SELECT %lld, 0"
+                 "  UNION"
+                 "  SELECT %s, bfs.hop + 1"
+                 "  FROM bfs"
+                 "  JOIN edges e ON %s"
+                 "  WHERE e.type IN (%s) AND bfs.hop < %d"
+                 ")"
+                 "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 "
+                 "GROUP BY n.id "
+                 "ORDER BY hop, n.id "
+                 "LIMIT %d;",
+                 (long long)start_id, next_id, join_cond, types_clause, max_depth, max_results);
+    }
 
     sqlite3_stmt *stmt = NULL;
     rc = sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL);
@@ -4746,6 +4803,7 @@ int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const
 
     int cap = ST_INIT_CAP_16;
     int n = 0;
+    int cte_rows = 0;
     cbm_node_hop_t *visited = malloc(cap * sizeof(cbm_node_hop_t));
 
     int scan_rc15;
@@ -4756,6 +4814,9 @@ int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const
         }
         scan_node(stmt, &visited[n].node);
         visited[n].hop = sqlite3_column_int(stmt, ST_COL_9);
+        if (trail) {
+            cte_rows = sqlite3_column_int(stmt, ST_COL_10);
+        }
         n++;
     }
     if (scan_rc15 != SQLITE_DONE) { /* SCANCHK:15:stmt */
@@ -4768,6 +4829,13 @@ int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const
 
     sqlite3_finalize(stmt);
 
+    if (trail && cte_rows > cte_row_limit) {
+        out->truncated = true;
+        char limit_buf[16];
+        snprintf(limit_buf, sizeof(limit_buf), "%d", cte_row_limit);
+        cbm_log_warn("cypher.trail_truncated", "cte_rows", limit_buf, "result", "partial");
+    }
+
     out->visited = visited;
     out->visited_count = n;
 
@@ -4783,6 +4851,19 @@ int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const
     return CBM_STORE_OK;
 }
 
+int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const char **edge_types,
+                  int edge_type_count, int max_depth, int max_results, cbm_traverse_result_t *out) {
+    return store_bfs(s, start_id, direction, edge_types, edge_type_count, max_depth, max_results,
+                     false, out);
+}
+
+int cbm_store_bfs_trail(cbm_store_t *s, int64_t start_id, const char *direction,
+                        const char **edge_types, int edge_type_count, int max_depth,
+                        int max_results, cbm_traverse_result_t *out) {
+    return store_bfs(s, start_id, direction, edge_types, edge_type_count, max_depth, max_results,
+                     true, out);
+}
+
 /* Multi-source BFS: one recursive CTE anchored on ALL seeds (via a temp
  * table — never a per-seed loop, which re-walks overlapping hub subgraphs
  * seed_count times). Semantics for impact analysis:

src/store/store.h

@@ -211,6 +211,8 @@ typedef struct {
     int visited_count;
     cbm_edge_info_t *edges;
     int edge_count;
+    /* True when trail expansion hit its recursive-row safety budget. */
+    bool truncated;
 } cbm_traverse_result_t;
 
 /* ── Schema introspection ───────────────────────────────────────── */
@@ -603,6 +605,11 @@ void cbm_store_search_free(cbm_search_output_t *out);
 int cbm_store_bfs(cbm_store_t *s, int64_t start_id, const char *direction, const char **edge_types,
                   int edge_type_count, int max_depth, int max_results, cbm_traverse_result_t *out);
 
+/* Variable-length Cypher traversal with relationship-trail semantics. */
+int cbm_store_bfs_trail(cbm_store_t *s, int64_t start_id, const char *direction,
+                        const char **edge_types, int edge_type_count, int max_depth,
+                        int max_results, cbm_traverse_result_t *out);
+
 /* Multi-source BFS from ALL seed ids at once (one CTE, temp-table anchored).
  * Seeds are EXCLUDED from the result (impact semantics); MIN(hop) across the
  * seed set; canonical (hop,id) order; *truncated set when the max_results

tests/test_cypher.c

@@ -1171,15 +1171,24 @@ TEST(cypher_exec_varlength_path_semantics_issue797) {
     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). */
+    /* Bug 2: *2..2 from loopy has two relationship-unique trails: the
+     * self-loop followed by e1 reaches mid, and e1 followed by e2 reaches leaf.
+     * Reusing the self-loop within one trail remains forbidden. */
     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 */
+    ASSERT_EQ(r2.row_count, 2);
+    bool saw_mid = false;
+    bool saw_leaf = false;
+    for (int i = 0; i < r2.row_count; i++) {
+        saw_mid |= strcmp(r2.rows[i][0], "mid") == 0;
+        saw_leaf |= strcmp(r2.rows[i][0], "leaf") == 0;
+    }
+    ASSERT_TRUE(saw_mid);
+    ASSERT_TRUE(saw_leaf);
     cbm_cypher_result_free(&r2);
 
     /* Bug 2 amplifier: no directed path of length 5 exists at all. */
@@ -1798,6 +1807,74 @@ TEST(cypher_exec_var_length_explicit_bound_capped) {
     PASS();
 }
 
+/* Pin the relationship-trail contract: a self-loop edge cannot be reused
+ * within one variable-length trail, so *2..2 yields no fabricated match. */
+TEST(cypher_exec_var_length_no_reuse_self_loop) {
+    cbm_store_t *s = cbm_store_open_memory();
+    cbm_store_upsert_project(s, "test", "/tmp/test");
+
+    cbm_node_t n = {.project = "test",
+                    .label = "Function",
+                    .name = "Recursive",
+                    .qualified_name = "test.Recursive",
+                    .file_path = "recursive.go"};
+    int64_t id = cbm_store_upsert_node(s, &n);
+    cbm_edge_t e = {.project = "test", .source_id = id, .target_id = id, .type = "CALLS"};
+    cbm_store_insert_edge(s, &e);
+
+    cbm_cypher_result_t r = {0};
+    int rc = cbm_cypher_execute(s,
+                                "MATCH (f:Function {name: \"Recursive\"})-[:CALLS*2..2]"
+                                "->(g:Function) RETURN g.name",
+                                "test", 0, &r);
+    ASSERT_EQ(rc, 0);
+    ASSERT_EQ(r.row_count, 0);
+
+    cbm_cypher_result_free(&r);
+    cbm_store_close(s);
+    PASS();
+}
+
+TEST(cypher_exec_var_length_truncation_surfaces_warning) {
+    cbm_store_t *s = cbm_store_open_memory();
+    cbm_store_upsert_project(s, "test", "/tmp/test");
+
+    int64_t ids[18];
+    for (int i = 0; i < 18; i++) {
+        char name[16];
+        snprintf(name, sizeof(name), "node-%d", i);
+        cbm_node_t node = {.project = "test",
+                           .label = "Function",
+                           .name = name,
+                           .qualified_name = name,
+                           .file_path = "graph.c"};
+        ids[i] = cbm_store_upsert_node(s, &node);
+    }
+    for (int source = 0; source < 17; source++) {
+        for (int target = source + 1; target < 18; target++) {
+            cbm_edge_t edge = {.project = "test",
+                               .source_id = ids[source],
+                               .target_id = ids[target],
+                               .type = "CALLS"};
+            cbm_store_insert_edge(s, &edge);
+        }
+    }
+
+    cbm_cypher_result_t result = {0};
+    int rc = cbm_cypher_execute(s,
+                                "MATCH (a:Function {name: \"node-0\"})-[:CALLS*10..10]->"
+                                "(b:Function) RETURN b.name",
+                                "test", 0, &result);
+    ASSERT_EQ(rc, 0);
+    ASSERT_NOT_NULL(result.warning);
+    ASSERT_TRUE(strstr(result.warning, "traversal budget") != NULL);
+    ASSERT_TRUE(result.row_count > 0);
+
+    cbm_cypher_result_free(&result);
+    cbm_store_close(s);
+    PASS();
+}
+
 TEST(cypher_exec_defines_edge) {
     cbm_store_t *s = setup_cypher_store();
     cbm_cypher_result_t r = {0};
@@ -4037,6 +4114,8 @@ SUITE(cypher) {
     RUN_TEST(cypher_exec_variable_length);
     RUN_TEST(cypher_exec_variable_length_repeated_node_var_unifies);
     RUN_TEST(cypher_exec_var_length_explicit_bound_capped);
+    RUN_TEST(cypher_exec_var_length_no_reuse_self_loop);
+    RUN_TEST(cypher_exec_var_length_truncation_surfaces_warning);
     RUN_TEST(cypher_exec_defines_edge);
     RUN_TEST(cypher_exec_no_results);
     RUN_TEST(cypher_exec_where_numeric);