feat(cypher): string functions + fail loudly on unsupported syntax

Add single-argument string functions to projections — size, length, trim,
ltrim, rtrim, reverse — via the generic function recognizer.

More importantly, change the engine to FAIL LOUDLY on unsupported syntax instead
of silently projecting an empty column. An unknown function call (e.g.
split(...), coalesce(...)) or list indexing/slicing in RETURN/WITH now returns a
clear "unsupported function '<name>' (supported: ...)" error rather than a
valid-looking but blank result — the same silent-empty failure mode that hid the
labels() bug. This supersedes the #373 graceful-resync behaviour (its test is
flipped to assert the error; a new test covers an unknown function in RETURN).

Decision: full Cypher Tier 3 (lists/maps/paths/comprehensions/params — a value
data-model rewrite) is intentionally NOT implemented; clear errors on
unsupported features are the higher-value, lower-risk completeness win for the
read subset agents actually use.

Refs: Cypher read suite (tiers 1-2a + unsupported-syntax hardening)

Author: DeusData

scripts/smoke-test.sh

@@ -248,6 +248,13 @@ case "$TIV" in
   *) echo "OK: query_graph toInteger(f.start_line) = $TIV" ;;
 esac
 
+# size() string-length function in projection
+SZV=$(cyp_first_cell 'MATCH (f:Function) RETURN size(f.name) AS s LIMIT 1')
+case "$SZV" in
+  ''|*[!0-9]*) echo "FAIL: query_graph size(f.name) returned non-integer '$SZV'"; exit 1 ;;
+  *) echo "OK: query_graph size(f.name) = $SZV" ;;
+esac
+
 # =~ regex match in WHERE
 CYPHER_RX=$(cli query_graph "{\"project\":\"$PROJECT\",\"query\":\"MATCH (f:Function) WHERE f.name =~ \\\".+\\\" RETURN f.name\"}")
 RX_ROWS=$(echo "$CYPHER_RX" | python3 -c "import json,sys; d=json.loads(sys.stdin.read()); print(len(d.get('rows',[])))" 2>/dev/null || echo "0")

src/cypher/cypher.c

@@ -1264,8 +1264,9 @@ static bool cyp_ci_eq(const char *a, const char *b) {
  * casts toInteger/toFloat/toBoolean — or NULL if unrecognised (case-insensitive).
  * toLower/toUpper/toString are separate keyword tokens handled elsewhere. */
 static const char *scalar_func_canonical(const char *s) {
-    static const char *const names[] = {"labels",    "type",    "id",        "keys", "properties",
-                                        "toInteger", "toFloat", "toBoolean", NULL};
+    static const char *const names[] = {
+        "labels", "type",   "id",   "keys",  "properties", "toInteger", "toFloat", "toBoolean",
+        "size",   "length", "trim", "ltrim", "rtrim",      "reverse",   NULL};
     for (int i = 0; names[i]; i++) {
         if (cyp_ci_eq(s, names[i])) {
             return names[i];
@@ -1274,6 +1275,16 @@ static const char *scalar_func_canonical(const char *s) {
     return NULL;
 }
 
+/* True for single-argument functions that transform a scalar string value
+ * (vs. entity-introspection funcs that act on the bound node/edge). */
+static bool is_scalar_value_func(const char *f) {
+    return f && (strcmp(f, "toLower") == 0 || strcmp(f, "toUpper") == 0 ||
+                 strcmp(f, "toString") == 0 || strcmp(f, "toInteger") == 0 ||
+                 strcmp(f, "toFloat") == 0 || strcmp(f, "toBoolean") == 0 ||
+                 strcmp(f, "size") == 0 || strcmp(f, "length") == 0 || strcmp(f, "trim") == 0 ||
+                 strcmp(f, "ltrim") == 0 || strcmp(f, "rtrim") == 0 || strcmp(f, "reverse") == 0);
+}
+
 static int parse_var_dot_prop(parser_t *p, cbm_return_item_t *item) {
     const cbm_token_t *var = expect(p, TOK_IDENT);
     if (!var) {
@@ -1365,28 +1376,26 @@ static int parse_return_item(parser_t *p, cbm_return_item_t *item) {
     if (rc < 0) {
         return CBM_NOT_FOUND;
     }
-    /* An unknown function call / indexed expression after a bare identifier
-     * (e.g. split(f.path,'/')[0]) is not evaluated, but its balanced
-     * parentheses/brackets MUST be consumed so the parser stays in sync. Left
-     * unconsumed, the trailing tokens desynced the parser into a misleading
-     * default star projection (wrong columns, blank rows) (#373). The column
-     * keeps its alias and simply projects empty. */
-    if (!item->func && !item->kase) {
-        while (check(p, TOK_LPAREN) || check(p, TOK_LBRACKET)) {
-            cbm_token_type_t open = peek(p)->type;
-            cbm_token_type_t close = (open == TOK_LPAREN) ? TOK_RPAREN : TOK_RBRACKET;
-            advance(p);
-            int depth = 1;
-            while (depth > 0 && !check(p, TOK_EOF)) {
-                cbm_token_type_t t = peek(p)->type;
-                if (t == open) {
-                    depth++;
-                } else if (t == close) {
-                    depth--;
-                }
-                advance(p);
-            }
+    /* A bare identifier followed by '(' is a function we don't recognise
+     * (recognised aggregates / string funcs / scalar funcs are handled above),
+     * and '[' begins list indexing/slicing we don't support. Rather than
+     * silently projecting an empty column — which looks like a valid but blank
+     * result and hides the real problem — fail loudly with a clear message so
+     * the caller knows the query used an unsupported feature (#373). */
+    if (!item->func && !item->kase && (check(p, TOK_LPAREN) || check(p, TOK_LBRACKET))) {
+        if (check(p, TOK_LPAREN)) {
+            snprintf(p->error, sizeof(p->error),
+                     "unsupported function '%s' (supported: count, sum, avg, min, max, collect, "
+                     "toLower, toUpper, toString, toInteger, toFloat, toBoolean, size, length, "
+                     "trim, ltrim, rtrim, reverse, labels, type, id, keys, properties)",
+                     item->variable ? item->variable : "?");
+        } else {
+            snprintf(p->error, sizeof(p->error),
+                     "unsupported expression: list indexing/slicing '[...]' is not supported");
         }
+        safe_str_free(&item->variable);
+        safe_str_free(&item->property);
+        return CBM_NOT_FOUND;
     }
     /* Optional AS alias */
     if (match(p, TOK_AS)) {
@@ -2427,6 +2436,41 @@ static const char *apply_string_func(const char *func, const char *val, char *bu
         }
         return ""; /* not a boolean → null */
     }
+    if (strcmp(func, "size") == 0 || strcmp(func, "length") == 0) {
+        snprintf(buf, buf_sz, "%zu", strlen(val));
+        return buf;
+    }
+    if (strcmp(func, "trim") == 0 || strcmp(func, "ltrim") == 0 || strcmp(func, "rtrim") == 0) {
+        bool do_left = (strcmp(func, "trim") == 0 || strcmp(func, "ltrim") == 0);
+        bool do_right = (strcmp(func, "trim") == 0 || strcmp(func, "rtrim") == 0);
+        const char *start = val;
+        const char *end = val + strlen(val);
+        while (do_left && (*start == ' ' || *start == '\t' || *start == '\n' || *start == '\r')) {
+            start++;
+        }
+        while (do_right && end > start &&
+               (end[-1] == ' ' || end[-1] == '\t' || end[-1] == '\n' || end[-1] == '\r')) {
+            end--;
+        }
+        size_t n = (size_t)(end - start);
+        if (n >= buf_sz) {
+            n = buf_sz - SKIP_ONE;
+        }
+        memcpy(buf, start, n);
+        buf[n] = '\0';
+        return buf;
+    }
+    if (strcmp(func, "reverse") == 0) {
+        size_t len = strlen(val);
+        if (len >= buf_sz) {
+            len = buf_sz - SKIP_ONE;
+        }
+        for (size_t i = 0; i < len; i++) {
+            buf[i] = val[len - SKIP_ONE - i];
+        }
+        buf[len] = '\0';
+        return buf;
+    }
     return val;
 }
 
@@ -2952,10 +2996,7 @@ static const char *project_item(binding_t *b, cbm_return_item_t *item, char *fun
         }
     }
     const char *raw = binding_get_virtual(b, item->variable, item->property);
-    if (item->func &&
-        (strcmp(item->func, "toLower") == 0 || strcmp(item->func, "toUpper") == 0 ||
-         strcmp(item->func, "toString") == 0 || strcmp(item->func, "toInteger") == 0 ||
-         strcmp(item->func, "toFloat") == 0 || strcmp(item->func, "toBoolean") == 0)) {
+    if (is_scalar_value_func(item->func)) {
         return apply_string_func(item->func, raw, func_buf, buf_sz);
     }
     return raw;

tests/test_cypher.c

@@ -646,6 +646,23 @@ TEST(cypher_func_tointeger_tofloat) {
     PASS();
 }
 
+TEST(cypher_func_size_reverse) {
+    cbm_store_t *s = setup_cypher_store();
+    cbm_cypher_result_t r = {0};
+    int rc = cbm_cypher_execute(s,
+                                "MATCH (f:Function) WHERE f.name = \"LogError\" "
+                                "RETURN size(f.name), length(f.name), reverse(f.name)",
+                                "test", 0, &r);
+    ASSERT_EQ(rc, 0);
+    ASSERT_EQ(r.row_count, 1);
+    ASSERT_STR_EQ(r.rows[0][0], "8"); /* "LogError" has 8 chars */
+    ASSERT_STR_EQ(r.rows[0][1], "8");
+    ASSERT_STR_EQ(r.rows[0][2], "rorrEgoL");
+    cbm_cypher_result_free(&r);
+    cbm_store_close(s);
+    PASS();
+}
+
 TEST(cypher_exec_calls_relationship) {
     cbm_store_t *s = setup_cypher_store();
     cbm_cypher_result_t r = {0};
@@ -918,25 +935,36 @@ TEST(cypher_exec_count_distinct_issue239) {
     PASS();
 }
 
-/* issue #373: an unsupported computed expression in WITH/RETURN (function call
- * like split(...) or list indexing [..]) must not desync the parser into a
- * misleading default star projection (wrong columns f.name/.../.label, blank
- * rows). The requested column shape is preserved; the unsupported expression
- * projects empty while sibling aggregates still compute. */
-TEST(cypher_exec_unsupported_with_expr_resync_issue373) {
+/* issue #373: an unsupported computed expression in WITH/RETURN (an unknown
+ * function like split(...) or list indexing [..]) must FAIL LOUDLY with a clear
+ * "unsupported function" error rather than silently projecting an empty column
+ * (which looks like a valid-but-blank result and hides the real problem). */
+TEST(cypher_exec_unsupported_func_errors_issue373) {
     cbm_store_t *s = setup_cypher_store();
 
     cbm_cypher_result_t r = {0};
     int rc = cbm_cypher_execute(
         s, "MATCH (f:Function) WITH split(f.name)[0] AS top, count(*) AS c RETURN top, c", "test",
         0, &r);
-    ASSERT_EQ(rc, 0);
-    ASSERT_NULL(r.error);
-    /* Columns are the requested aliases (top, c) — NOT a 3-column star
-     * projection of f.name/f.qualified_name/f.label. */
-    ASSERT_EQ(r.col_count, 2);
-    ASSERT_STR_EQ(r.columns[0], "top");
-    ASSERT_STR_EQ(r.columns[1], "c");
+    ASSERT_TRUE(rc != 0); /* unsupported function now fails loudly */
+    ASSERT_NOT_NULL(r.error);
+    ASSERT_TRUE(strstr(r.error, "unsupported") != NULL);
+    ASSERT_TRUE(strstr(r.error, "split") != NULL);
+    cbm_cypher_result_free(&r);
+
+    cbm_store_close(s);
+    PASS();
+}
+
+/* A recognised function still works, and an unknown one in plain RETURN errors. */
+TEST(cypher_exec_unknown_func_return_errors) {
+    cbm_store_t *s = setup_cypher_store();
+
+    cbm_cypher_result_t r = {0};
+    int rc = cbm_cypher_execute(s, "MATCH (f:Function) RETURN nosuchfunc(f.name)", "test", 0, &r);
+    ASSERT_TRUE(rc != 0);
+    ASSERT_NOT_NULL(r.error);
+    ASSERT_TRUE(strstr(r.error, "unsupported function") != NULL);
     cbm_cypher_result_free(&r);
 
     cbm_store_close(s);
@@ -2430,6 +2458,7 @@ SUITE(cypher) {
     RUN_TEST(cypher_func_keys);
     RUN_TEST(cypher_func_properties);
     RUN_TEST(cypher_func_tointeger_tofloat);
+    RUN_TEST(cypher_func_size_reverse);
     RUN_TEST(cypher_exec_calls_relationship);
     RUN_TEST(cypher_exec_calls_with_where);
     RUN_TEST(cypher_exec_inbound);
@@ -2446,7 +2475,8 @@ SUITE(cypher) {
     RUN_TEST(cypher_exec_where_label_test_issue241);
     RUN_TEST(cypher_exec_label_alternation_issue242);
     RUN_TEST(cypher_exec_count_distinct_issue239);
-    RUN_TEST(cypher_exec_unsupported_with_expr_resync_issue373);
+    RUN_TEST(cypher_exec_unsupported_func_errors_issue373);
+    RUN_TEST(cypher_exec_unknown_func_return_errors);
     RUN_TEST(cypher_exec_inline_props);
     RUN_TEST(cypher_parse_where_starts_with);
     RUN_TEST(cypher_parse_where_contains);