From 4f0ebc9b19d7bdac4adb874dbef133fca79e0888 Mon Sep 17 00:00:00 2001 From: Manfred Cheung Date: Fri, 26 Jun 2026 18:35:11 -0400 Subject: [PATCH 01/12] switch WHERE parser from Earley to LALR; preserve filter_dict/where_rows routing via post-parse lift --- graphistry/compute/gfql/cypher/ast.py | 11 +- graphistry/compute/gfql/cypher/parser.py | 124 +++++++++++++----- .../tests/compute/gfql/cypher/test_parser.py | 96 ++++++++++++++ 3 files changed, 196 insertions(+), 35 deletions(-) diff --git a/graphistry/compute/gfql/cypher/ast.py b/graphistry/compute/gfql/cypher/ast.py index 0b9524fe52..3a26dbd24a 100644 --- a/graphistry/compute/gfql/cypher/ast.py +++ b/graphistry/compute/gfql/cypher/ast.py @@ -192,11 +192,12 @@ class WhereClause: populated by the parser; consumers MUST handle all three: - **Structured path**: ``predicates`` populated, ``expr_tree is None``. - ``predicates`` carries either ``WherePredicate`` entries (pure AND of - comparable / has-labels predicates routed via the ``where_predicates`` - grammar rule, or AND-joined bare label predicates lifted by - ``generic_where_clause`` via label narrowing) or a single - ``WherePatternPredicate`` (pattern-only WHERE: ``WHERE (n)-[]->(m)``). + ``predicates`` carries either ``WherePredicate`` entries (a flat top-level + AND of simple predicates -- cmp / IS NULL / CONTAINS / STARTS/ENDS WITH / + has-labels -- lifted post-parse from the generic ``expr_tree`` by + ``generic_where_clause`` via ``_lift_label_only_and_spine`` / + ``_lift_and_spine_predicates``) or a single ``WherePatternPredicate`` + (pattern-only WHERE: ``WHERE (n)-[]->(m)``). - **Tree path**: ``predicates == ()``, ``expr_tree`` populated. Fires when ``generic_where_clause`` cannot lift to structured predicates (OR / XOR / NOT / parenthesized boolean / non-label atoms); consumers diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index 5f02788051..e8f48aed5e 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -120,9 +120,11 @@ properties: "{" [property_entry ("," property_entry)*] "}" property_entry: NAME ":" expr -where_clause: "WHERE"i where_predicates - | "WHERE"i expr -> generic_where_clause -where_predicates: where_predicate ("AND"i where_predicate)* +// Unified: every WHERE parses as a generic boolean ``expr`` (so LALR(1) accepts +// OR/XOR/NOT/parenthesized clauses, no Earley). ``generic_where_clause`` lifts the +// structured (filter_dict) predicates back out of the parsed tree. ``where_predicate`` +// is retained as the start symbol for the per-atom lift parser. +where_clause: "WHERE"i expr -> generic_where_clause where_predicate: property_ref COMP_OP where_rhs -> cmp_where | property_ref "IS"i "NULL"i -> is_null_where | property_ref "IS"i "NOT"i "NULL"i -> is_not_null_where @@ -533,15 +535,15 @@ class _BoundPattern: @lru_cache(maxsize=1) -def _parser() -> _ParserLike: +def _parser_lalr() -> _ParserLike: Lark, _, _, _ = _lark_imports() - # Earley required: LALR(1) cannot resolve `comparable AND WHERE_PATTERN` - # where WHERE_PATTERN is a leaf in `?primary`. Ambiguity defaults to - # Lark's "resolve" mode (highest-priority derivation). + # Sole whole-query parser. The unified WHERE grammar is LALR(1)-parseable, so + # this accepts every supported query (~80x faster than the former Earley + # parser); generic_where_clause lifts the structured predicates back out. parser = Lark( _GRAMMAR, start="start", - parser="earley", + parser="lalr", maybe_placeholders=False, propagate_positions=True, ) @@ -551,16 +553,84 @@ def _parser() -> _ParserLike: @lru_cache(maxsize=1) def _pattern_parser() -> _ParserLike: Lark, _, _, _ = _lark_imports() + # Parses a single pattern fragment (e.g. ``(n)-[:R]->()``) for WHERE pattern + # predicates. LALR(1) accepts the pattern grammar, so no Earley is needed here. parser = Lark( _GRAMMAR, start="pattern", - parser="earley", + parser="lalr", maybe_placeholders=False, propagate_positions=True, ) return cast(_ParserLike, parser) +@lru_cache(maxsize=1) +def _where_predicate_parser() -> _ParserLike: + Lark, _, _, _ = _lark_imports() + # Parses a single ``where_predicate`` (cmp / IS NULL / CONTAINS / STARTS / + # ENDS / label). Used to lift a flat-AND conjunct out of the generic expr + # tree into the structured (filter_dict) form, reusing the existing predicate + # builders. A non-predicate atom (arithmetic, OR-group, function) fails to + # parse and is kept in the residual ``where_rows`` expression. + parser = Lark( + _GRAMMAR, + start="where_predicate", + parser="lalr", + maybe_placeholders=False, + propagate_positions=True, + ) + return cast(_ParserLike, parser) + + +def _lift_atom_as_where_predicate(atom_text: str) -> Optional["WherePredicate"]: + """Re-parse one WHERE atom as a structured ``where_predicate``; ``None`` if it + is not a simple predicate. A ``None`` makes the caller drop the whole clause to + its ``expr_tree`` -> ``where_rows`` (full-lift-only, no partial residual).""" + _, _, LarkError, _ = _lark_imports() + try: + tree = _where_predicate_parser().parse(atom_text) + pred = _build_transformer(atom_text).transform(tree) + except (LarkError, GFQLSyntaxError, GFQLValidationError): + return None + return pred if isinstance(pred, WherePredicate) else None + + +def _lift_and_spine_predicates( + expr_tree: BooleanExpr, +) -> Optional[List["WherePredicate"]]: + """Lift a *whole* top-level AND spine to structured predicates (-> ``filter_dict``). + Returns the predicate list only when **every** conjunct lifts to a simple + ``where_predicate`` (cmp / IS NULL / CONTAINS / label); otherwise ``None`` (leave + the entire tree as ``expr_tree`` -> ``where_rows``). + + This is parity-plus machinery, not a perf optimization. For a *flat* AND it + reproduces the routing the former dual ``where_predicates | expr`` grammar did + in-grammar. It additionally lifts *nested/parenthesized* ANDs (e.g. + ``a AND (b AND c)``) that the old flat ``where_predicate ("AND" where_predicate)*`` + rule could not -- safe by AND-associativity (identical result, just the + ``filter_dict`` fast path instead of ``where_rows``). Full-lift-only -- never a + partial split -- so the result stays within the documented WhereClause shapes + (``structured`` or ``tree``). Routing optimizations beyond this (OR->is_in, + partial pushdown) are deferred.""" + stack = [expr_tree] + conjuncts: List[BooleanExpr] = [] + while stack: + cur = stack.pop() + if cur.op == "and" and cur.left is not None and cur.right is not None: + stack.append(cur.right) + stack.append(cur.left) + else: + conjuncts.append(cur) + lifted: List["WherePredicate"] = [] + for conjunct in conjuncts: + pred = _lift_atom_as_where_predicate(conjunct.atom_text) if conjunct.op == "atom" and conjunct.atom_text is not None else None + if pred is None: + return None # a non-liftable conjunct -> keep the whole clause as expr_tree + lifted.append(pred) + return lifted + + def _to_syntax_error(message: str, *, line: Optional[int] = None, column: Optional[int] = None, **extra: Any) -> GFQLSyntaxError: return GFQLSyntaxError( ErrorCode.E107, @@ -938,9 +1008,6 @@ def has_labels_where(self, meta: Any, items: Sequence[Any]) -> WherePredicate: span=_span_from_meta(meta), ) - def where_predicates(self, _meta: Any, items: Sequence[Any]) -> Tuple[WherePredicate, ...]: - return tuple(items) - def _parse_single_where_pattern_predicate_text(self, pattern_item_text: str, span: SourceSpan) -> WherePatternPredicate: """Parse one bare-pattern-item text (no AND-chain) into a WherePatternPredicate. @@ -1101,14 +1168,6 @@ def not_op(self, meta: Any, items: Sequence[Any]) -> BooleanExpr: left=self._wrap_as_boolean_atom(items[0], meta), ) - def where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: - if len(items) != 1: - raise _to_syntax_error("WHERE clause cannot be empty", line=meta.line, column=meta.column) - predicates = cast(Tuple[WherePredicate, ...], items[0]) - if len(predicates) == 0: - raise _to_syntax_error("WHERE clause cannot be empty", line=meta.line, column=meta.column) - return WhereClause(predicates=cast(Any, predicates), span=_span_from_meta(meta)) - def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: span = _span_from_meta(meta) expr_text = self._slice(span)[len("WHERE"):].strip() @@ -1123,14 +1182,12 @@ def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: if items and isinstance(items[0], BooleanExpr) else BooleanExpr(op="atom", span=span, atom_text=expr_text, atom_span=span) ) - # Lark's ambiguity resolution prefers the generic `expr` path over - # `where_predicates` (#1194), so bare label predicates and AND - # chains of them land here as a ``BooleanExpr``. Walk the parsed - # tree (single source of truth) to lift them to structured - # predicates instead of re-splitting the WHERE body text on - # top-level ``AND``. Any non-AND boolean op, non-atom node, or - # non-bare-label atom causes a conservative fallback to raw expr - # (preserved on ``expr_tree`` for downstream consumers). + # The unified grammar parses every WHERE as a generic `expr`, so bare + # label predicates and AND chains of them arrive here as a ``BooleanExpr``. + # Walk the parsed tree (single source of truth) to lift them to structured + # predicates instead of re-splitting the WHERE body text on top-level + # ``AND``. Any non-AND boolean op, non-atom node, or non-bare-label atom + # causes a conservative fallback to raw expr (preserved on ``expr_tree``). lifted = _lift_label_only_and_spine(expr_tree) if lifted: predicates = tuple( @@ -1161,6 +1218,14 @@ def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: expr_text=expr_text, span=span, ) + # Parity lift: a whole top-level AND spine of simple predicates + # (cmp / IS NULL / CONTAINS / label) becomes structured ``predicates`` + # -> ``filter_dict``. If any conjunct is non-liftable (OR/XOR/NOT, + # arithmetic, cross-expr) the whole clause stays on ``expr_tree`` -> + # ``where_rows``. + lifted_preds = _lift_and_spine_predicates(expr_tree) + if lifted_preds is not None: + return WhereClause(predicates=tuple(lifted_preds), span=span) return WhereClause( predicates=(), expr_tree=expr_tree, @@ -1873,10 +1938,9 @@ def _parse_cypher_cached(query: str) -> Union[CypherQuery, CypherUnionQuery, Cyp # Pre-parse detection of known-but-unsupported Cypher forms _check_unsupported_syntax_patterns(query) - parser = _parser() transformer = _build_transformer(query) try: - tree = parser.parse(query) + tree = _parser_lalr().parse(query) node = transformer.transform(tree) except Exception as exc: _, _, LarkError, _ = _lark_imports() diff --git a/graphistry/tests/compute/gfql/cypher/test_parser.py b/graphistry/tests/compute/gfql/cypher/test_parser.py index e8f45bf24b..452983f059 100644 --- a/graphistry/tests/compute/gfql/cypher/test_parser.py +++ b/graphistry/tests/compute/gfql/cypher/test_parser.py @@ -1206,3 +1206,99 @@ def test_parse_cypher_invalid_input_not_cached() -> None: parse_cypher("") with pytest.raises(GFQLSyntaxError): parse_cypher(" ") + + +# --- Single LALR(1) path (Earley removed) ------------------------------------- +# After WHERE-grammar unification, parse_cypher uses one LALR(1) parser for every +# supported query -- including OR/XOR/NOT-in-WHERE and post-WITH WHERE, which the +# former dual grammar could only parse under Earley. These smoke-test that the +# whole corpus parses to a valid query on the sole LALR path. + +_PARITY_QUERIES = [ + "MATCH (n) WHERE n.x = 50 RETURN n.__rid__", + "MATCH (n) WHERE n.active = true AND n.x = 5 RETURN n.id", + "MATCH (a)-[r:R]->(b) RETURN a.id, r.weight, b.id", + "MATCH (n {a: 1, b: 2}) RETURN n", + "MATCH (n) WHERE n.x IN [1, 2, 3] RETURN count(n)", + "OPTIONAL MATCH (n)-[r]->(m) RETURN n, m", + "MATCH (n) WHERE NOT (n)-[:R]->() RETURN n", + "MATCH (n) RETURN n.first_name AS fn ORDER BY fn", + # Formerly Earley-only constructs, now on the LALR path: + "MATCH (n) WHERE n:Admin AND n.active = true RETURN n", # label + property + "MATCH (n) WHERE n.a > 3 OR n.b = 1 RETURN n", # OR in WHERE + "MATCH (n) WHERE n.x = 1 XOR n.y = 2 RETURN n", # XOR in WHERE + "MATCH (n)-[rel]->(x) WITH n, x WHERE n.animal = x.animal RETURN n, x", # WITH-WHERE +] + + +@pytest.mark.parametrize("query", _PARITY_QUERIES) +def test_unified_lalr_parses_corpus(query: str) -> None: + assert isinstance(parse_cypher(query), CypherQuery) + + +def test_lalr_is_the_only_parser() -> None: + # The Earley parser and its fallback hook are gone. + from graphistry.compute.gfql.cypher import parser as _p + + assert not hasattr(_p, "_parser") + assert not hasattr(_p, "_lalr_tree_needs_earley") + assert _p._parser_lalr().parse("MATCH (n) WHERE n.x = 1 OR n.y = 2 RETURN n") is not None + + +def test_unified_where_lifts_label_and_property_to_structured() -> None: + # Post grammar-unification: every WHERE parses as the generic expr under the + # sole LALR parser, and generic_where_clause lifts label + property back to + # structured predicates. + label = "MATCH (n) WHERE n:Admin AND n.active = true RETURN n" + parsed = cast(CypherQuery, parse_cypher(label)) + assert parsed.where is not None + assert parsed.where.expr_tree is None # structured via the lift + assert len(parsed.where.predicates) == 2 + + +# --- Flat-AND spine lift in generic_where_clause ------------------------------- +# A parenthesized / nested pure-AND of simple predicates lifts wholly to the +# structured (filter_dict) form instead of dropping to the where_rows engine. +# Anything with a non-liftable conjunct (OR/XOR/NOT, arithmetic) keeps the WHOLE +# clause on expr_tree -- full-lift-only, never a partial split. + +def test_nested_pure_and_lifts_to_structured() -> None: + w = cast(CypherQuery, parse_cypher( + "MATCH (n) WHERE n.x = 1 AND (n.y = 2 AND n.z = 3) RETURN n" + )).where + assert w is not None + assert w.expr_tree is None # structured, not a tree + assert len(w.predicates) == 3 # x, y, z all lifted + + +def test_and_with_or_stays_on_expr_tree() -> None: + w = cast(CypherQuery, parse_cypher( + "MATCH (n) WHERE n.x = 1 AND (n.y = 2 OR n.z = 3) RETURN n" + )).where + assert w is not None + assert w.predicates == () # no partial split + assert w.expr_tree is not None # whole clause stays a tree + + +def test_not_in_and_spine_stays_on_expr_tree() -> None: + w = cast(CypherQuery, parse_cypher( + "MATCH (n) WHERE NOT n.x = 1 AND n.y = 2 RETURN n" + )).where + assert w is not None + assert w.predicates == () + assert w.expr_tree is not None + + +# --- OR stays on the row engine (no OR->is_in optimization in this PR) ---------- +# OR-of-equalities collapse to is_in is a deferred routing optimization; the +# parser-switch lift reproduces the old grammar's routing, under which any OR +# clause stays on expr_tree -> where_rows. +def test_or_clause_stays_on_expr_tree() -> None: + for q in ( + "MATCH (n) WHERE n.x = 1 OR n.x = 2 OR n.x = 3 RETURN n", # same column + "MATCH (n) WHERE n.x = 1 OR n.y = 2 RETURN n", # cross column + ): + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None + assert w.predicates == () + assert w.expr_tree is not None From 305f2fb5be2fc7d5d35b44735c5a089b31f7b8b5 Mon Sep 17 00:00:00 2001 From: Manfred Cheung Date: Fri, 26 Jun 2026 19:27:36 -0400 Subject: [PATCH 02/12] fix predicate span source character offsets --- graphistry/compute/gfql/cypher/parser.py | 63 ++++++++++++++++--- .../tests/compute/gfql/cypher/test_parser.py | 30 +++++++++ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index e8f48aed5e..abfa630cb7 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -583,17 +583,50 @@ def _where_predicate_parser() -> _ParserLike: return cast(_ParserLike, parser) -def _lift_atom_as_where_predicate(atom_text: str) -> Optional["WherePredicate"]: +def _retarget_span(s: SourceSpan, base: SourceSpan) -> SourceSpan: + """Shift an atom-relative span (from re-parsing an isolated atom substring) into + absolute query coordinates, given the atom's absolute ``base`` span. Exact for + single-line atoms (the normal case); best-effort across embedded newlines.""" + return SourceSpan( + line=base.line + (s.line - 1), + column=(base.column + s.column - 1) if s.line == 1 else s.column, + end_line=base.line + (s.end_line - 1), + end_column=(base.column + s.end_column - 1) if s.end_line == 1 else s.end_column, + start_pos=base.start_pos + s.start_pos, + end_pos=base.start_pos + s.end_pos, + ) + + +def _retarget_predicate_spans(pred: "WherePredicate", base: SourceSpan) -> "WherePredicate": + """Rebuild a lifted predicate with every span shifted from atom-relative to + absolute (see ``_retarget_span``), so downstream errors (e.g. E108 in lowering) + point at the predicate's real position in the query instead of column 1.""" + left = replace(pred.left, span=_retarget_span(pred.left.span, base)) + right = pred.right + if isinstance(right, (PropertyRef, ParameterRef)): + right = replace(right, span=_retarget_span(right.span, base)) + return replace(pred, left=left, right=right, span=_retarget_span(pred.span, base)) + + +def _lift_atom_as_where_predicate( + atom_text: str, base_span: Optional[SourceSpan] = None +) -> Optional["WherePredicate"]: """Re-parse one WHERE atom as a structured ``where_predicate``; ``None`` if it is not a simple predicate. A ``None`` makes the caller drop the whole clause to - its ``expr_tree`` -> ``where_rows`` (full-lift-only, no partial residual).""" + its ``expr_tree`` -> ``where_rows`` (full-lift-only, no partial residual). + + The atom is parsed in isolation, so Lark reports spans relative to the substring; + ``base_span`` (the atom's absolute position in the original query) shifts them back + to absolute coordinates so error messages locate the predicate correctly.""" _, _, LarkError, _ = _lark_imports() try: tree = _where_predicate_parser().parse(atom_text) pred = _build_transformer(atom_text).transform(tree) except (LarkError, GFQLSyntaxError, GFQLValidationError): return None - return pred if isinstance(pred, WherePredicate) else None + if not isinstance(pred, WherePredicate): + return None + return _retarget_predicate_spans(pred, base_span) if base_span is not None else pred def _lift_and_spine_predicates( @@ -624,7 +657,7 @@ def _lift_and_spine_predicates( conjuncts.append(cur) lifted: List["WherePredicate"] = [] for conjunct in conjuncts: - pred = _lift_atom_as_where_predicate(conjunct.atom_text) if conjunct.op == "atom" and conjunct.atom_text is not None else None + pred = _lift_atom_as_where_predicate(conjunct.atom_text, conjunct.atom_span) if conjunct.op == "atom" and conjunct.atom_text is not None else None if pred is None: return None # a non-liftable conjunct -> keep the whole clause as expr_tree lifted.append(pred) @@ -1177,11 +1210,23 @@ def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: # non-BooleanExpr operand and are wrapped as a single-atom tree # so the invariant ``(expr is None) == (expr_tree is None)`` # holds (#1213 sub-PR A / #1214). - expr_tree: BooleanExpr = ( - cast(BooleanExpr, items[0]) - if items and isinstance(items[0], BooleanExpr) - else BooleanExpr(op="atom", span=span, atom_text=expr_text, atom_span=span) - ) + if items and isinstance(items[0], BooleanExpr): + expr_tree: BooleanExpr = cast(BooleanExpr, items[0]) + else: + # Align the synthesized atom's ``atom_span`` with the stripped + # ``expr_text`` (which drops "WHERE" + whitespace) so the span matches + # the atom text; otherwise a lifted single-predicate's error position + # would point at the WHERE keyword instead of the predicate. + body = self._slice(span)[len("WHERE"):] + start_off = len("WHERE") + (len(body) - len(body.lstrip())) + atom_span = replace( + span, + column=span.column + start_off, + start_pos=span.start_pos + start_off, + end_column=span.column + start_off + len(expr_text), + end_pos=span.start_pos + start_off + len(expr_text), + ) + expr_tree = BooleanExpr(op="atom", span=atom_span, atom_text=expr_text, atom_span=atom_span) # The unified grammar parses every WHERE as a generic `expr`, so bare # label predicates and AND chains of them arrive here as a ``BooleanExpr``. # Walk the parsed tree (single source of truth) to lift them to structured diff --git a/graphistry/tests/compute/gfql/cypher/test_parser.py b/graphistry/tests/compute/gfql/cypher/test_parser.py index 452983f059..267e781f4a 100644 --- a/graphistry/tests/compute/gfql/cypher/test_parser.py +++ b/graphistry/tests/compute/gfql/cypher/test_parser.py @@ -1302,3 +1302,33 @@ def test_or_clause_stays_on_expr_tree() -> None: assert w is not None assert w.predicates == () assert w.expr_tree is not None + + +# --- Lifted predicates keep absolute source spans ------------------------------ +# Re-parsing each conjunct in isolation would collapse its span to column 1; the +# lift shifts spans back to absolute query coordinates (via the conjunct's +# atom_span) so downstream errors (e.g. E108) point at the real predicate, not +# the start of the query. Regression guard for the column-1 span bug. +def test_lifted_predicate_spans_are_absolute() -> None: + q = "MATCH (n) WHERE n.x = 1 AND m.y = 2 RETURN n" + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None and w.expr_tree is None # structured via the lift + by_alias = {cast(PropertyRef, p.left).alias: p for p in w.predicates} + # `n.x` / `m.y` sit at their true offsets in the query, not column 1. + assert q[by_alias["n"].left.span.start_pos:].startswith("n.x") + assert by_alias["n"].left.span.column == q.index("n.x") + 1 + assert q[by_alias["m"].left.span.start_pos:].startswith("m.y") + assert by_alias["m"].left.span.column == q.index("m.y") + 1 + + +def test_lifted_single_predicate_span_skips_where_keyword() -> None: + # Single-predicate WHERE: the synthesized atom span must align with the + # predicate text, not the WHERE keyword (the +len("WHERE ") off-by-six case). + q = "MATCH (a)-[]->(b) WHERE a.x = b.y RETURN a" + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None and len(w.predicates) == 1 + left = cast(PropertyRef, w.predicates[0].left) + right = w.predicates[0].right + assert left.span.column == q.index("a.x") + 1 + assert isinstance(right, PropertyRef) + assert right.span.column == q.index("b.y") + 1 From 79715fc160b72bbfda50655619459cd4662168f5 Mon Sep 17 00:00:00 2001 From: Manfred Cheung Date: Tue, 30 Jun 2026 11:38:37 -0400 Subject: [PATCH 03/12] restrict WHERE filter_dict lift to flat AND-chains; parenthesized ANDs stay on where_rows --- graphistry/compute/gfql/cypher/parser.py | 130 +++++++----------- .../compute/gfql/cypher/test_lowering.py | 14 ++ .../tests/compute/gfql/cypher/test_parser.py | 27 +++- 3 files changed, 87 insertions(+), 84 deletions(-) diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index abfa630cb7..65214483be 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -123,7 +123,7 @@ // Unified: every WHERE parses as a generic boolean ``expr`` (so LALR(1) accepts // OR/XOR/NOT/parenthesized clauses, no Earley). ``generic_where_clause`` lifts the // structured (filter_dict) predicates back out of the parsed tree. ``where_predicate`` -// is retained as the start symbol for the per-atom lift parser. +// is retained as the building block for the ``where_predicate_chain`` lift parser. where_clause: "WHERE"i expr -> generic_where_clause where_predicate: property_ref COMP_OP where_rhs -> cmp_where | property_ref "IS"i "NULL"i -> is_null_where @@ -132,6 +132,10 @@ | property_ref "STARTS"i "WITH"i where_rhs -> starts_with_where | property_ref "ENDS"i "WITH"i where_rhs -> ends_with_where | variable labels -> has_labels_where +// Flat AND-chain of bare predicates; the start symbol for the lift parser. Parens +// / OR / XOR / NOT are not ``where_predicate``s, so such clauses fail this parse and +// stay on the row-filter path (which treats a missing property as null). +where_predicate_chain: where_predicate ("AND"i where_predicate)* where_rhs: property_ref | value @@ -566,16 +570,15 @@ def _pattern_parser() -> _ParserLike: @lru_cache(maxsize=1) -def _where_predicate_parser() -> _ParserLike: +def _where_predicate_chain_parser() -> _ParserLike: Lark, _, _, _ = _lark_imports() - # Parses a single ``where_predicate`` (cmp / IS NULL / CONTAINS / STARTS / - # ENDS / label). Used to lift a flat-AND conjunct out of the generic expr - # tree into the structured (filter_dict) form, reusing the existing predicate - # builders. A non-predicate atom (arithmetic, OR-group, function) fails to - # parse and is kept in the residual ``where_rows`` expression. + # Parses a flat ``where_predicate ("AND" where_predicate)*`` chain to lift a WHERE + # body into structured (filter_dict) form. Parens / OR / XOR / NOT / arithmetic make + # the body NOT a flat chain, so this parse fails and the clause stays on the + # ``where_rows`` row-filter path (see _lift_and_spine_predicates for why). parser = Lark( _GRAMMAR, - start="where_predicate", + start="where_predicate_chain", parser="lalr", maybe_placeholders=False, propagate_positions=True, @@ -608,60 +611,32 @@ def _retarget_predicate_spans(pred: "WherePredicate", base: SourceSpan) -> "Wher return replace(pred, left=left, right=right, span=_retarget_span(pred.span, base)) -def _lift_atom_as_where_predicate( - atom_text: str, base_span: Optional[SourceSpan] = None -) -> Optional["WherePredicate"]: - """Re-parse one WHERE atom as a structured ``where_predicate``; ``None`` if it - is not a simple predicate. A ``None`` makes the caller drop the whole clause to - its ``expr_tree`` -> ``where_rows`` (full-lift-only, no partial residual). - - The atom is parsed in isolation, so Lark reports spans relative to the substring; - ``base_span`` (the atom's absolute position in the original query) shifts them back - to absolute coordinates so error messages locate the predicate correctly.""" +def _lift_and_spine_predicates( + expr_text: str, base_span: Optional[SourceSpan] = None +) -> Optional[List["WherePredicate"]]: + """Lift the WHERE body to structured ``filter_dict`` predicates iff it parses as a + flat ``where_predicate ("AND" where_predicate)*`` chain (cmp / IS NULL / CONTAINS / + STARTS|ENDS WITH / label); else ``None``, leaving it on ``expr_tree`` -> ``where_rows``. + + Only flat chains lift; parentheses, OR / XOR / NOT and arithmetic stay on + ``where_rows``. This is a correctness boundary, not just routing: a parenthesized + predicate over a *missing* property (e.g. ``a IS NULL AND (b = 1)`` where ``b`` is + absent) must stay on ``where_rows``, which treats an absent property as null + (Cypher semantics); ``filter_dict`` requires the column to exist and would raise. + + ``base_span`` is the WHERE body's absolute position; the body is parsed in isolation + (spans relative to it), so predicate spans are shifted back to absolute.""" _, _, LarkError, _ = _lark_imports() try: - tree = _where_predicate_parser().parse(atom_text) - pred = _build_transformer(atom_text).transform(tree) + tree = _where_predicate_chain_parser().parse(expr_text) + preds = _build_transformer(expr_text).transform(tree) except (LarkError, GFQLSyntaxError, GFQLValidationError): return None - if not isinstance(pred, WherePredicate): + if not (isinstance(preds, tuple) and preds and all(isinstance(p, WherePredicate) for p in preds)): return None - return _retarget_predicate_spans(pred, base_span) if base_span is not None else pred - - -def _lift_and_spine_predicates( - expr_tree: BooleanExpr, -) -> Optional[List["WherePredicate"]]: - """Lift a *whole* top-level AND spine to structured predicates (-> ``filter_dict``). - Returns the predicate list only when **every** conjunct lifts to a simple - ``where_predicate`` (cmp / IS NULL / CONTAINS / label); otherwise ``None`` (leave - the entire tree as ``expr_tree`` -> ``where_rows``). - - This is parity-plus machinery, not a perf optimization. For a *flat* AND it - reproduces the routing the former dual ``where_predicates | expr`` grammar did - in-grammar. It additionally lifts *nested/parenthesized* ANDs (e.g. - ``a AND (b AND c)``) that the old flat ``where_predicate ("AND" where_predicate)*`` - rule could not -- safe by AND-associativity (identical result, just the - ``filter_dict`` fast path instead of ``where_rows``). Full-lift-only -- never a - partial split -- so the result stays within the documented WhereClause shapes - (``structured`` or ``tree``). Routing optimizations beyond this (OR->is_in, - partial pushdown) are deferred.""" - stack = [expr_tree] - conjuncts: List[BooleanExpr] = [] - while stack: - cur = stack.pop() - if cur.op == "and" and cur.left is not None and cur.right is not None: - stack.append(cur.right) - stack.append(cur.left) - else: - conjuncts.append(cur) - lifted: List["WherePredicate"] = [] - for conjunct in conjuncts: - pred = _lift_atom_as_where_predicate(conjunct.atom_text, conjunct.atom_span) if conjunct.op == "atom" and conjunct.atom_text is not None else None - if pred is None: - return None # a non-liftable conjunct -> keep the whole clause as expr_tree - lifted.append(pred) - return lifted + if base_span is None: + return list(preds) + return [_retarget_predicate_spans(p, base_span) for p in preds] def _to_syntax_error(message: str, *, line: Optional[int] = None, column: Optional[int] = None, **extra: Any) -> GFQLSyntaxError: @@ -979,6 +954,10 @@ def where_rhs(self, _meta: Any, items: Sequence[Any]) -> object: raise _to_syntax_error("Invalid WHERE right-hand side") return _unwrap_primitive_literal(items[0]) + def where_predicate_chain(self, _meta: Any, items: Sequence[Any]) -> Tuple[WherePredicate, ...]: + # Flat AND-chain of bare predicates (the lift parser's start symbol). + return tuple(cast(WherePredicate, p) for p in items) + def cmp_where(self, meta: Any, items: Sequence[Any]) -> WherePredicate: if len(items) != 3: raise _to_syntax_error("Invalid WHERE comparison", line=meta.line, column=meta.column) @@ -1204,6 +1183,18 @@ def not_op(self, meta: Any, items: Sequence[Any]) -> BooleanExpr: def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: span = _span_from_meta(meta) expr_text = self._slice(span)[len("WHERE"):].strip() + # Absolute span of the WHERE body (drops "WHERE" + whitespace): aligns the + # synth single-atom node and is the lift's span base (the lift parses + # ``expr_text`` in isolation, so its spans need shifting to absolute). + _body = self._slice(span)[len("WHERE"):] + _start_off = len("WHERE") + (len(_body) - len(_body.lstrip())) + body_span = replace( + span, + column=span.column + _start_off, + start_pos=span.start_pos + _start_off, + end_column=span.column + _start_off + len(expr_text), + end_pos=span.start_pos + _start_off + len(expr_text), + ) # Capture Lark's structural expression tree. Only ``and_op`` / # ``or_op`` / ``xor_op`` / ``not_op`` produce ``BooleanExpr``; # atomic WHERE expressions (single predicate) route here with a @@ -1213,20 +1204,7 @@ def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: if items and isinstance(items[0], BooleanExpr): expr_tree: BooleanExpr = cast(BooleanExpr, items[0]) else: - # Align the synthesized atom's ``atom_span`` with the stripped - # ``expr_text`` (which drops "WHERE" + whitespace) so the span matches - # the atom text; otherwise a lifted single-predicate's error position - # would point at the WHERE keyword instead of the predicate. - body = self._slice(span)[len("WHERE"):] - start_off = len("WHERE") + (len(body) - len(body.lstrip())) - atom_span = replace( - span, - column=span.column + start_off, - start_pos=span.start_pos + start_off, - end_column=span.column + start_off + len(expr_text), - end_pos=span.start_pos + start_off + len(expr_text), - ) - expr_tree = BooleanExpr(op="atom", span=atom_span, atom_text=expr_text, atom_span=atom_span) + expr_tree = BooleanExpr(op="atom", span=body_span, atom_text=expr_text, atom_span=body_span) # The unified grammar parses every WHERE as a generic `expr`, so bare # label predicates and AND chains of them arrive here as a ``BooleanExpr``. # Walk the parsed tree (single source of truth) to lift them to structured @@ -1263,12 +1241,10 @@ def generic_where_clause(self, meta: Any, items: Sequence[Any]) -> WhereClause: expr_text=expr_text, span=span, ) - # Parity lift: a whole top-level AND spine of simple predicates - # (cmp / IS NULL / CONTAINS / label) becomes structured ``predicates`` - # -> ``filter_dict``. If any conjunct is non-liftable (OR/XOR/NOT, - # arithmetic, cross-expr) the whole clause stays on ``expr_tree`` -> - # ``where_rows``. - lifted_preds = _lift_and_spine_predicates(expr_tree) + # Lift a flat AND chain of bare predicates to structured ``filter_dict`` + # predicates; parens / OR / XOR / NOT keep the clause on ``expr_tree`` -> + # ``where_rows`` (see _lift_and_spine_predicates for why parens matter). + lifted_preds = _lift_and_spine_predicates(expr_text, body_span) if lifted_preds is not None: return WhereClause(predicates=tuple(lifted_preds), span=span) return WhereClause( diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 07eb7f841d..0ad2b73cd5 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -16952,3 +16952,17 @@ def test_count_table_frame_op_error_and_empty_paths() -> None: assert ctx3._nodes is None and ctx3._edges is None out3 = frame_ops.count_table(ctx3, table="nodes", alias="c") assert out3._nodes.to_dict(orient="records") == [{"c": 0}] + +def test_string_cypher_parenthesized_and_preserves_missing_property_as_null() -> None: + # A parenthesized AND must stay on the row-filter path, not the filter_dict path: + # the row engine treats an absent property as null (Cypher semantics), whereas + # filter_dict requires the column to exist and would raise. + g = _mk_graph( + pd.DataFrame({"id": ["a", "b", "c", "d"], "x": [1, 2, 1, 1]}), # no 'missing' col + pd.DataFrame({"s": ["a", "b", "c"], "d": ["b", "c", "d"]}), + ) + got = g.gfql("MATCH (n) WHERE n.missing IS NULL AND (n.x = 1) RETURN n.id AS id") + assert sorted(got._nodes["id"].tolist()) == ["a", "c", "d"] + # IS NOT NULL on the same absent property yields no rows (absent -> null) + none = g.gfql("MATCH (n) WHERE n.missing IS NOT NULL AND (n.x = 1) RETURN n.id AS id") + assert none._nodes["id"].tolist() == [] if "id" in none._nodes.columns else len(none._nodes) == 0 diff --git a/graphistry/tests/compute/gfql/cypher/test_parser.py b/graphistry/tests/compute/gfql/cypher/test_parser.py index 267e781f4a..3f78c096a0 100644 --- a/graphistry/tests/compute/gfql/cypher/test_parser.py +++ b/graphistry/tests/compute/gfql/cypher/test_parser.py @@ -1256,21 +1256,34 @@ def test_unified_where_lifts_label_and_property_to_structured() -> None: assert len(parsed.where.predicates) == 2 -# --- Flat-AND spine lift in generic_where_clause ------------------------------- -# A parenthesized / nested pure-AND of simple predicates lifts wholly to the -# structured (filter_dict) form instead of dropping to the where_rows engine. -# Anything with a non-liftable conjunct (OR/XOR/NOT, arithmetic) keeps the WHOLE -# clause on expr_tree -- full-lift-only, never a partial split. +# --- Flat-AND-chain lift in generic_where_clause ------------------------------- +# Only a FLAT ``where_predicate ("AND" where_predicate)*`` chain lifts to the +# structured (filter_dict) form. Parens / OR / XOR / NOT keep the WHOLE clause on +# expr_tree -> where_rows. -def test_nested_pure_and_lifts_to_structured() -> None: +def test_flat_and_chain_lifts_to_structured() -> None: w = cast(CypherQuery, parse_cypher( - "MATCH (n) WHERE n.x = 1 AND (n.y = 2 AND n.z = 3) RETURN n" + "MATCH (n) WHERE n.x = 1 AND n.y = 2 AND n.z = 3 RETURN n" )).where assert w is not None assert w.expr_tree is None # structured, not a tree assert len(w.predicates) == 3 # x, y, z all lifted +def test_parenthesized_and_stays_on_expr_tree() -> None: + # Parens make the body NOT a flat predicate chain, so it stays on where_rows, + # which (unlike filter_dict) treats an absent property as null. + for q in ( + "MATCH (n) WHERE n.x = 1 AND (n.y = 2 AND n.z = 3) RETURN n", + "MATCH (n) WHERE (n.x = 1 AND n.y = 2) AND n.z = 3 RETURN n", + "MATCH (n) WHERE n.missing IS NULL AND (n.x = 1) RETURN n", + ): + w = cast(CypherQuery, parse_cypher(q)).where + assert w is not None + assert w.predicates == () + assert w.expr_tree is not None + + def test_and_with_or_stays_on_expr_tree() -> None: w = cast(CypherQuery, parse_cypher( "MATCH (n) WHERE n.x = 1 AND (n.y = 2 OR n.z = 3) RETURN n" From 22849fa7d958c0c7824e79aca17f444de756b68d Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 09:13:09 -0700 Subject: [PATCH 04/12] test(gfql/cypher): machine-checked grammar invariants for the LALR parser Three grammar-level guards (test_grammar_invariants.py) so the correctness argument lives in the grammar, not the Python: 1. Pinned LALR conflict profile: 8 shift/reduce (DOT x5, IN, WHERE, RPAR), all resolved as shift, ZERO reduce/reduce. R/R is the genuine-ambiguity line a grammar edit must never cross silently. 2. Semantic ambiguity = 0: every Earley derivation (ambiguity='explicit' + CollapseAmbiguities) of every corpus query transforms to the same AST -- except the one characterized WITH..WHERE attachment ambiguity, pinned as exactly binary. (Raw _ambig-node counting is NOT usable: Earley reports harmless alternative derivations even for trivial queries; the invariant that matters is that they collapse to one AST.) 3. LALR == Earley differential: the production pipeline run under both parsers yields byte-identical ASTs and identical rejections across a corpus covering every grammar construct family. Also: honest CHANGELOG entry for the Earley->LALR switch (~70x parse, full WHERE language preserved, parse-equivalence machine-checked). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 3 + .../gfql/cypher/test_grammar_invariants.py | 272 ++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 08a8d80ab0..c0cdff0bc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL Cypher `count(*)` short-circuit — O(1) instead of O(N) materialize**: a lone `RETURN count(*)` over a single node or edge pattern (`MATCH (n) RETURN count(*)`, `MATCH ()-[r]->() RETURN count(*)`) previously materialized the entire matched frame and ran a constant-key `group_by` just to count its rows. The lowering now emits a new `count_table` row op that reads the scanned table's height directly (or, when the pattern applies a filter, sums the boolean alias-mask column) in a single reduction — no full-frame copy, no group_by. Applies only to the provably-equivalent shapes: exactly one non-DISTINCT `count(*)`, no group keys / post-aggregate exprs / row-level `WHERE` / `UNWIND` / paging / multi-relationship binding, and either a pure node scan (`relationship_count == 0`) or a single relationship counted on its edge alias — every other shape (node-alias-over-relationship, multi-hop paths, `count(DISTINCT …)`, grouped counts) falls through to the general aggregate path unchanged. Engine-polymorphic across pandas/cuDF/polars/polars-gpu; differential parity verified on all four engines (`count_all_nodes`/`count_all_edges` cypher conformance cases + a `count_table` row-op subject case, chain and DAG surfaces). No change to any result value — only the execution path. - **GFQL `engine='polars'`/`'polars-gpu'` can now run off-engine analytic `call()`s (umap/hypergraph/compute_cugraph/…) — `call_mode='auto'` (default)**: a GFQL `call()` that invokes a Plottable-method analytic with **no native polars implementation** (and never will — `umap`, `hypergraph`, `compute_cugraph`, `compute_igraph`, `layout_*`, `collapse`, `get_topological_levels`, …) previously raised `NotImplementedError` under a polars engine, forcing users off polars for the whole pipeline. It now runs as a **mode-gated, warned modality switch**: `call_mode='auto'` (default) bridges the graph off-engine (`polars`→pandas, `polars-gpu`→cuDF **on-device**), runs the analytic, and coerces the result back to polars **losslessly via Arrow**, warning once per (process, function). `call_mode='strict'` keeps the honest `NotImplementedError` decline (for benchmark integrity / a hard memory ceiling). `polars-gpu` is **GPU-or-error**: it bridges to cuDF and declines rather than silently dropping a GPU analytic to host pandas. This is **deliberately narrower than CHAIN traversal/filter/row ops**, which stay parity-or-NIE (a bridge there would hide a missing impl and cheat a benchmark) — the split is mechanical (`is_row_pipeline_call`), and the chain and DAG surfaces bridge consistently. Configurable via `graphistry.compute.gfql.lazy.set_call_mode('auto'|'strict')` (Python) or `GFQL_POLARS_CALL_MODE` (env), resolving Python override > env > default('auto'), read live. Verified on dgx: `compute_cugraph` PageRank is byte-parity with the pandas oracle under both `polars` and `polars-gpu`. +### Changed +- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. **Parse-equivalence, machine-checked** (`test_grammar_invariants.py`): (1) the grammar's LALR conflict profile is pinned (8 shift/reduce, all resolved as shift, **0 reduce/reduce**) so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is zero — every Earley derivation of every corpus query transforms to the same AST — up to one characterized binary WITH..WHERE attachment case that both parsers resolve identically (WHERE attaches to the WITH stage); (3) a differential test runs the production pipeline under both parsers and asserts byte-identical ASTs and identical rejections across a corpus covering every grammar construct family. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. + ### Fixed - **CI: `compute_networkx` HITS test was scipy-version flaky**: `test_compute_networkx_hits_outputs_hubs_and_authorities` used a pure directed 3-cycle (`a→b→c→a`), whose adjacency is a permutation matrix (all singular values equal). networkx HITS computes scores via scipy `svds(k=1)`, which then returns an arbitrary vector from that fully-degenerate singular space; when its components sum to ~0, HITS's `h /= h.sum()` normalization blows up to `inf`/`nan`. Which vector `svds` returns is scipy/LAPACK-version dependent, so the test passed on the pinned scipy but failed on the `nx-upper-scipy` CI profile's newer scipy — a latent flake, not a code regression. Switched to a graph with a well-defined hub/authority structure (`a→b`, `a→c`, `b→c`), whose dominant singular vector is unique (Perron-Frobenius) and therefore version-stable. Test-only change. - **GFQL `materialize_nodes()` on an edges-only graph under `engine='polars'`**: crashed with `AttributeError: 'Series' object has no attribute 'drop_duplicates'` — the node-id derivation used pandas-only `.drop_duplicates()` / `.reset_index()` / `.rename(columns=)` on a polars Series/DataFrame. Now polars-aware (`.unique(maintain_order=True)`, `.select().rename({...})`), matching the pandas oracle. Surfaced by any `let()` DAG on an edges-only graph under Polars (the DAG pre-materializes nodes). pandas/cuDF paths unchanged. diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py new file mode 100644 index 0000000000..8df0297865 --- /dev/null +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -0,0 +1,272 @@ +"""Machine-checkable grammar invariants for the GFQL Cypher LALR parser. + +The parser's correctness argument is grammar-level, not implementation-level: + +1. **Conflict profile is pinned** (``test_lalr_conflict_profile_pinned``): the + unified grammar builds under LALR(1) with a fixed, reviewed set of + shift/reduce conflicts and ZERO reduce/reduce conflicts. Reduce/reduce is + the line that must never be crossed: it means two rules derive the same + string (genuine ambiguity), which LALR resolves arbitrarily. A grammar edit + that changes the profile fails this test and forces a review. + +2. **Semantic ambiguity is zero, up to one characterized case** + (``test_semantic_ambiguity_zero_up_to_known_cases``): expanding EVERY + Earley derivation of every corpus query (``ambiguity='explicit'`` + + ``CollapseAmbiguities``), all derivations transform to the SAME AST — + except the known WITH..WHERE attachment ambiguity, where the production + resolution (WHERE attaches to the WITH stage) is asserted explicitly. + Note plain ``_ambig`` node counting is NOT a usable invariant: Earley + reports harmless alternative derivations even for ``MATCH (a) RETURN a.val``; + what matters is that they collapse to one AST. + +3. **Parser-choice neutrality** (``test_lalr_ast_equals_earley_ast_differential``): + running the production pipeline with an Earley parser over the same grammar + yields byte-identical ASTs to the LALR parser across the corpus, and both + reject the same invalid inputs. LALR is a pure speed change (~70x). + +Together: the grammar (a declarative artifact) carries the correctness +argument, and these tests make its safety properties machine-checked so +syntactic extensions cannot silently regress them. +""" +from __future__ import annotations + +import logging +import re +from typing import Any, Dict, List, Tuple + +import pytest + +from graphistry.compute.exceptions import GFQLSyntaxError +from graphistry.compute.gfql.cypher import parse_cypher +from graphistry.compute.gfql.cypher import parser as parser_mod + +lark = pytest.importorskip("lark") + + +# --- Corpus -------------------------------------------------------------------- +# One query per grammar construct family. Every query must parse in production. + +DIFFERENTIAL_CORPUS = [ + # patterns + "MATCH (n) RETURN n", + "MATCH (n:Person) RETURN n", + "MATCH (n:Person {id: 1, name: 'x'}) RETURN n", + "MATCH (a)-[r]->(b) RETURN a, r, b", + "MATCH (a)<-[r:KNOWS]-(b) RETURN a", + "MATCH (a)-[r:A|B|C]-(b) RETURN r", + "MATCH (a)-[r:KNOWS*1..3]->(b) RETURN b", + "MATCH (a)-[:R*]->(b) RETURN b", + "MATCH p = shortestPath((a:X)-[:R*1..4]->(b:Y)) RETURN p", + "MATCH (a)-[:R*2..]->(b) RETURN b", + "MATCH (a)-[:R]->(b), (b)-[:S]->(c) RETURN a, c", + "OPTIONAL MATCH (n)-[r]->(m) RETURN n, m", + "MATCH (n) MATCH (m) RETURN n, m", + # WHERE: structured predicate forms (flat AND chain lifts) + "MATCH (n) WHERE n.x = 50 RETURN n", + "MATCH (n) WHERE n.x <> 1 AND n.y >= 2 AND n.z < 3 RETURN n", + "MATCH (n) WHERE n.name CONTAINS 'ab' RETURN n", + "MATCH (n) WHERE n.name STARTS WITH 'a' AND n.name ENDS WITH 'z' RETURN n", + "MATCH (n) WHERE n.x IS NULL AND n.y IS NOT NULL RETURN n", + "MATCH (n) WHERE n:Admin AND n.active = true RETURN n", + "MATCH (a), (b) WHERE a.id = b.id RETURN a", + "MATCH (n) WHERE n.x = $param RETURN n", + # WHERE: generic expression forms (stay on expr_tree) + "MATCH (n) WHERE n.a > 3 OR n.b = 1 RETURN n", + "MATCH (n) WHERE n.x = 1 XOR n.y = 2 RETURN n", + "MATCH (n) WHERE NOT n.deleted = true RETURN n", + "MATCH (n) WHERE (n.x = 1 AND n.y = 2) OR n.z = 3 RETURN n", + "MATCH (n) WHERE n.x = 1 AND (n.y = 2 AND n.z = 3) RETURN n", + "MATCH (n) WHERE n.x IN [1, 2, 3] RETURN n", + "MATCH (n) WHERE n.x + 1 > n.y * 2 RETURN n", + "MATCH (n) WHERE NOT (n)-[:BLOCKED]->() RETURN n", + # RETURN / projection + "MATCH (n) RETURN DISTINCT n.city AS city", + "MATCH (n) RETURN count(n) AS c, sum(n.x) AS s", + "MATCH (n) RETURN n.a, n.b ORDER BY n.a ASC, n.b DESC SKIP 2 LIMIT 5", + "MATCH (n) RETURN CASE WHEN n.x > 1 THEN 'hi' ELSE 'lo' END AS bucket", + "MATCH (n) RETURN CASE n.x WHEN 1 THEN 'a' ELSE 'b' END AS k", + "RETURN 1 AS one", + # WITH pipelines + "MATCH (n) WITH n.city AS city, count(n) AS c WHERE c > 1 RETURN city, c", + "MATCH (n) WITH n ORDER BY n.x LIMIT 3 RETURN n.id", + "MATCH (n:Person) WITH n AS m WHERE m.age > 1 RETURN m", + "MATCH (a)-[r]->(x) WITH a, x WHERE a.animal = x.animal RETURN a, x", + "MATCH (a:A)-[:R]->(b) WITH collect(b) AS bs UNWIND bs AS b2 MATCH (b2)-[:S]->(c) RETURN c", + "MATCH (n) WITH n WHERE n.x = 1 MATCH (m) WHERE m.y = n.x RETURN m", + # UNWIND / CALL / UNION / params + "UNWIND [1, 2, 3] AS x RETURN x", + "MATCH (n) UNWIND n.tags AS t RETURN t", + "CALL gds.pageRank({maxIter: 10}) YIELD nodeId, score RETURN nodeId, score", + "MATCH (a:X) RETURN a.id AS id UNION MATCH (b:Y) RETURN b.id AS id", + "MATCH (a:X) RETURN a.id AS id UNION ALL MATCH (b:Y) RETURN b.id AS id", + # graph constructor / USE + "GRAPH g1 = GRAPH { MATCH (a)-[r]->(b) WHERE a.id = 'x' } USE g1 MATCH (n) RETURN n", +] + +# Invalid inputs: both parsers must reject (rejection parity, not message parity). +REJECTED_CORPUS = [ + "MATCH (n) WHERE RETURN n", + "MATCH (n RETURN n", + "MATCH (n) RETURN", + "MATCH (n)-[r->-(m) RETURN n", + "WHERE n.x = 1 RETURN n", # WHERE without MATCH (rejected post-parse) + "MATCH (a)-[:R*..4]->(b) RETURN b", # open LOWER hop bound is not in the grammar +] + +# The ONE known semantic ambiguity: WHERE after a WITH stage can attach to the +# WITH stage or (in an alternative derivation) to the MATCH clause. Both Earley +# (rule priority ``with_where_clause.2``) and LALR (shift preference) resolve it +# to the WITH-stage attachment; the differential test proves they agree. +KNOWN_AMBIGUOUS = { + q for q in DIFFERENTIAL_CORPUS if re.search(r"\bWITH\b.*\bWHERE\b", q) +} + + +def _fresh_lalr_with_log() -> List[logging.LogRecord]: + """Build a fresh (uncached) LALR parser with debug on, capturing lark's + grammar-analysis log records (conflicts are reported at WARNING).""" + records: List[logging.LogRecord] = [] + + class _Capture(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + records.append(record) + + lark_logger = logging.getLogger("lark") + handler = _Capture() + old_level = lark_logger.level + lark_logger.addHandler(handler) + lark_logger.setLevel(logging.DEBUG) # lark.utils defaults it to CRITICAL + try: + lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="lalr", + debug=True, + maybe_placeholders=False, + propagate_positions=True, + ) + finally: + lark_logger.removeHandler(handler) + lark_logger.setLevel(old_level) + return records + + +def _earley_parser(ambiguity: str = "resolve") -> Any: + return lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="earley", + ambiguity=ambiguity, + maybe_placeholders=False, + propagate_positions=True, + ) + + +# --- 1. Conflict profile ------------------------------------------------------- + +def test_lalr_conflict_profile_pinned() -> None: + """The grammar's LALR conflict profile is pinned: 0 reduce/reduce (genuine + ambiguity — never acceptable), and exactly these shift/reduce conflicts, + all resolved as shift. A grammar edit that changes this set must update + the pin AND justify each new conflict here: + + - DOT x5, IN x1 after ``qualified_name : NAME``: greedy-extend of dotted + names / IN-expressions; shift = longest match, the intended parse. + - WHERE x1 at ``with_stage : with_clause``: attach WHERE to the WITH stage + (the documented resolution of the one known attachment ambiguity). + - RPAR x1 at ``bare_label_predicate_expr : NAME labels``: greedy label + list inside parens; shift = longest match. + """ + conflict_re = re.compile( + r"(Shift/Reduce|Reduce/Reduce) conflict for terminal (\w+): \(resolving as (\w+)\)" + ) + profile: Dict[Tuple[str, str, str], int] = {} + for record in _fresh_lalr_with_log(): + m = conflict_re.match(record.getMessage()) + if m: + key = (m.group(1), m.group(2), m.group(3)) + profile[key] = profile.get(key, 0) + 1 + + assert not any(kind == "Reduce/Reduce" for kind, _, _ in profile), ( + f"Reduce/reduce conflict introduced (genuine grammar ambiguity): {profile}" + ) + assert profile == { + ("Shift/Reduce", "DOT", "shift"): 5, + ("Shift/Reduce", "IN", "shift"): 1, + ("Shift/Reduce", "WHERE", "shift"): 1, + ("Shift/Reduce", "RPAR", "shift"): 1, + }, f"LALR conflict profile changed: {profile}" + + +# --- 2. Semantic ambiguity ----------------------------------------------------- + +def test_semantic_ambiguity_zero_up_to_known_cases() -> None: + """Every Earley derivation of every corpus query transforms to the same AST + (semantic ambiguity = 0), except the known WITH..WHERE attachment case, + which must stay exactly binary (2 distinct ASTs: WHERE on the WITH stage + vs WHERE on the MATCH clause). Which side production picks is pinned + elsewhere: the LALR==Earley differential below plus the WITH..WHERE + parser tests both fix it to the WITH-stage attachment. + + Note: ``CollapseAmbiguities`` rebuilds trees without ``meta`` positions, so + derivation ASTs have degraded spans/text and CANNOT be compared against the + production AST — only against each other (all equally degraded, so any + structural divergence between derivations still shows).""" + from lark.visitors import CollapseAmbiguities + + explicit = _earley_parser(ambiguity="explicit") + unexpected: List[str] = [] + for query in DIFFERENTIAL_CORPUS: + trees = CollapseAmbiguities().transform(explicit.parse(query)) + asts = { + repr(parser_mod._build_transformer(query).transform(t)) for t in trees + } + if query in KNOWN_AMBIGUOUS: + assert len(asts) == 2, ( + f"WITH..WHERE attachment ambiguity should be exactly binary, " + f"got {len(asts)} distinct ASTs for: {query!r}" + ) + elif len(asts) != 1: + unexpected.append(f"{query!r}: {len(trees)} derivations, {len(asts)} ASTs") + assert not unexpected, ( + "New semantic ambiguity introduced (derivations disagree on AST):\n" + + "\n".join(unexpected) + ) + + +# --- 3. LALR == Earley differential --------------------------------------------- + +def test_lalr_ast_equals_earley_ast_differential( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Running the production pipeline with Earley instead of LALR yields the + identical AST for every corpus query: the parser swap is AST-neutral.""" + earley = _earley_parser() + + def _with_parser(parser: Any, query: str) -> str: + monkeypatch.setattr(parser_mod, "_parser_lalr", lambda: parser) + parser_mod._parse_cypher_cached.cache_clear() + try: + return repr(parse_cypher(query)) + finally: + monkeypatch.undo() + parser_mod._parse_cypher_cached.cache_clear() + + for query in DIFFERENTIAL_CORPUS: + lalr_ast = _with_parser(parser_mod._parser_lalr(), query) + earley_ast = _with_parser(earley, query) + assert lalr_ast == earley_ast, f"AST divergence on: {query}" + + +def test_lalr_and_earley_reject_the_same_inputs( + monkeypatch: pytest.MonkeyPatch, +) -> None: + earley = _earley_parser() + for query in REJECTED_CORPUS: + for parser in (parser_mod._parser_lalr(), earley): + monkeypatch.setattr(parser_mod, "_parser_lalr", lambda p=parser: p) + parser_mod._parse_cypher_cached.cache_clear() + with pytest.raises(GFQLSyntaxError): + parse_cypher(query) + monkeypatch.undo() + parser_mod._parse_cypher_cached.cache_clear() From cc2fa4b783bb6c13a775edffc45c4ebc9f93ef41 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 09:18:07 -0700 Subject: [PATCH 05/12] test(gfql/cypher): pin the one deliberate language fix found by the full differential Full repo-corpus differential on dgx-spark (1839 scraped queries, production pipeline under LALR vs Earley): 0 AST divergences, 0 error-class differences, 1 language divergence -- 'MATCH (n) RETURN DISTINCT' (no items). Earley's dynamic lexer re-lexed DISTINCT (absent from the NAME reserved lookahead) as a NAME and accepted it as returning a variable named DISTINCT with distinct=False: a lexing accident, not Cypher. LALR's contextual lexer keeps it a keyword, so it's now a syntax error. Pinned as a rejection test. Full cypher suite on dgx-spark: 1671 passed, 5 skipped, 15 xfailed. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- .../compute/gfql/cypher/test_grammar_invariants.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0cdff0bc7..bdf1a1b895 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL `engine='polars'`/`'polars-gpu'` can now run off-engine analytic `call()`s (umap/hypergraph/compute_cugraph/…) — `call_mode='auto'` (default)**: a GFQL `call()` that invokes a Plottable-method analytic with **no native polars implementation** (and never will — `umap`, `hypergraph`, `compute_cugraph`, `compute_igraph`, `layout_*`, `collapse`, `get_topological_levels`, …) previously raised `NotImplementedError` under a polars engine, forcing users off polars for the whole pipeline. It now runs as a **mode-gated, warned modality switch**: `call_mode='auto'` (default) bridges the graph off-engine (`polars`→pandas, `polars-gpu`→cuDF **on-device**), runs the analytic, and coerces the result back to polars **losslessly via Arrow**, warning once per (process, function). `call_mode='strict'` keeps the honest `NotImplementedError` decline (for benchmark integrity / a hard memory ceiling). `polars-gpu` is **GPU-or-error**: it bridges to cuDF and declines rather than silently dropping a GPU analytic to host pandas. This is **deliberately narrower than CHAIN traversal/filter/row ops**, which stay parity-or-NIE (a bridge there would hide a missing impl and cheat a benchmark) — the split is mechanical (`is_row_pipeline_call`), and the chain and DAG surfaces bridge consistently. Configurable via `graphistry.compute.gfql.lazy.set_call_mode('auto'|'strict')` (Python) or `GFQL_POLARS_CALL_MODE` (env), resolving Python override > env > default('auto'), read live. Verified on dgx: `compute_cugraph` PageRank is byte-parity with the pandas oracle under both `polars` and `polars-gpu`. ### Changed -- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. **Parse-equivalence, machine-checked** (`test_grammar_invariants.py`): (1) the grammar's LALR conflict profile is pinned (8 shift/reduce, all resolved as shift, **0 reduce/reduce**) so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is zero — every Earley derivation of every corpus query transforms to the same AST — up to one characterized binary WITH..WHERE attachment case that both parsers resolve identically (WHERE attaches to the WITH stage); (3) a differential test runs the production pipeline under both parsers and asserts byte-identical ASTs and identical rejections across a corpus covering every grammar construct family. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. +- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. **Parse-equivalence, machine-checked** (`test_grammar_invariants.py`): (1) the grammar's LALR conflict profile is pinned (8 shift/reduce, all resolved as shift, **0 reduce/reduce**) so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is zero — every Earley derivation of every corpus query transforms to the same AST — up to one characterized binary WITH..WHERE attachment case that both parsers resolve identically (WHERE attaches to the WITH stage); (3) a differential test runs the production pipeline under both parsers and asserts byte-identical ASTs and identical rejections across a corpus covering every grammar construct family. A full repo-corpus differential (1839 scraped queries) found **zero AST divergences** and exactly one deliberate language fix: `MATCH (n) RETURN DISTINCT` (no items) is now a syntax error — Earley's dynamic lexer had re-lexed `DISTINCT` as a NAME and accepted it as returning a *variable named* DISTINCT (with `distinct=False`), a lexing accident, now pinned as a rejection test. Full cypher suite (1671 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. ### Fixed - **CI: `compute_networkx` HITS test was scipy-version flaky**: `test_compute_networkx_hits_outputs_hubs_and_authorities` used a pure directed 3-cycle (`a→b→c→a`), whose adjacency is a permutation matrix (all singular values equal). networkx HITS computes scores via scipy `svds(k=1)`, which then returns an arbitrary vector from that fully-degenerate singular space; when its components sum to ~0, HITS's `h /= h.sum()` normalization blows up to `inf`/`nan`. Which vector `svds` returns is scipy/LAPACK-version dependent, so the test passed on the pinned scipy but failed on the `nx-upper-scipy` CI profile's newer scipy — a latent flake, not a code regression. Switched to a graph with a well-defined hub/authority structure (`a→b`, `a→c`, `b→c`), whose dominant singular vector is unique (Perron-Frobenius) and therefore version-stable. Test-only change. diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py index 8df0297865..91620ff4da 100644 --- a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -270,3 +270,17 @@ def test_lalr_and_earley_reject_the_same_inputs( parse_cypher(query) monkeypatch.undo() parser_mod._parse_cypher_cached.cache_clear() + + +def test_return_distinct_without_items_is_rejected() -> None: + """The ONE deliberate language difference from the old Earley parser, + found by a full repo-corpus differential (1839 queries; everything else + is accept/reject- and AST-identical). + + ``DISTINCT`` is not in the NAME terminal's reserved-word lookahead, so + Earley's dynamic lexer re-lexed it as a NAME and accepted + ``MATCH (n) RETURN DISTINCT`` as returning a *variable named* DISTINCT + (with ``distinct=False``!) — a lexing accident, not Cypher. The LALR + contextual lexer keeps it a keyword, so the query is now a syntax error.""" + with pytest.raises(GFQLSyntaxError): + parse_cypher("MATCH (n) RETURN DISTINCT") From 38e1de322186cef5a5d9c13dcbaabc2fb7c2fcd5 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 09:38:55 -0700 Subject: [PATCH 06/12] =?UTF-8?q?refactor(gfql/cypher):=20purify=20the=20g?= =?UTF-8?q?rammar=20to=20(near-)unambiguous=20=E2=80=94=208=20S/R=20confli?= =?UTF-8?q?cts=20->=202?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two structural fixes so the grammar, not Python, carries clause structure: 1. WHERE binds to its preceding clause IN THE GRAMMAR: bundled into match_clause (with_stage already owned WHERE-after-WITH); the standalone where_clause query item is gone. This ELIMINATES the WITH..WHERE attachment ambiguity (previously tie-broken by a rule priority + shift preference) rather than resolving it. The clause-sequence assembler is unchanged: bundled MATCH..WHERE is re-flattened to the exact item sequence the old state machine was written against, and the MatchClause span is reconstructed to end at the last pre-WHERE character, so ASTs are byte-identical (old-vs-new production differential: 0 AST diffs). 2. Name-rooted dot chains derive ONLY via qualified_name; property_access applies only to composite roots (f(x).y, (e).y, xs[0].y). Kills the property_access/qualified_name derivation redundancy and all five DOT shift/reduce conflicts. Result: 2 shift/reduce conflicts (IN, RPAR), 0 reduce/reduce, each mapped 1:1 to a characterized binary AST-NEUTRAL derivation ambiguity ([x IN xs] comprehension-vs-list, (n:Admin) grouped-label), pinned as witnesses. Semantic ambiguity is now ZERO unconditionally (every Earley derivation of every corpus query -> same AST, no exceptions). The obsolete with_where_clause priority is dropped. Language fixes (accept-by-accident shapes with ill-defined attachment, now honest syntax errors, all pinned): double WHERE (old code kept BOTH predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, WHERE before MATCH in a graph constructor. Verified on dgx-spark: full cypher suite 1677 passed; LALR-vs-Earley full-repo differential (1846 queries) 0 AST divergences; old-vs-new production differential 0 AST diffs, exactly the 4 pinned language fixes. Raw parse speed unchanged (0.57ms). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- graphistry/compute/gfql/cypher/parser.py | 71 +++++++-- .../gfql/cypher/test_grammar_invariants.py | 144 ++++++++++++------ 3 files changed, 158 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdf1a1b895..e8ab40c156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL `engine='polars'`/`'polars-gpu'` can now run off-engine analytic `call()`s (umap/hypergraph/compute_cugraph/…) — `call_mode='auto'` (default)**: a GFQL `call()` that invokes a Plottable-method analytic with **no native polars implementation** (and never will — `umap`, `hypergraph`, `compute_cugraph`, `compute_igraph`, `layout_*`, `collapse`, `get_topological_levels`, …) previously raised `NotImplementedError` under a polars engine, forcing users off polars for the whole pipeline. It now runs as a **mode-gated, warned modality switch**: `call_mode='auto'` (default) bridges the graph off-engine (`polars`→pandas, `polars-gpu`→cuDF **on-device**), runs the analytic, and coerces the result back to polars **losslessly via Arrow**, warning once per (process, function). `call_mode='strict'` keeps the honest `NotImplementedError` decline (for benchmark integrity / a hard memory ceiling). `polars-gpu` is **GPU-or-error**: it bridges to cuDF and declines rather than silently dropping a GPU analytic to host pandas. This is **deliberately narrower than CHAIN traversal/filter/row ops**, which stay parity-or-NIE (a bridge there would hide a missing impl and cheat a benchmark) — the split is mechanical (`is_row_pipeline_call`), and the chain and DAG surfaces bridge consistently. Configurable via `graphistry.compute.gfql.lazy.set_call_mode('auto'|'strict')` (Python) or `GFQL_POLARS_CALL_MODE` (env), resolving Python override > env > default('auto'), read live. Verified on dgx: `compute_cugraph` PageRank is byte-parity with the pandas oracle under both `polars` and `polars-gpu`. ### Changed -- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. **Parse-equivalence, machine-checked** (`test_grammar_invariants.py`): (1) the grammar's LALR conflict profile is pinned (8 shift/reduce, all resolved as shift, **0 reduce/reduce**) so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is zero — every Earley derivation of every corpus query transforms to the same AST — up to one characterized binary WITH..WHERE attachment case that both parsers resolve identically (WHERE attaches to the WITH stage); (3) a differential test runs the production pipeline under both parsers and asserts byte-identical ASTs and identical rejections across a corpus covering every grammar construct family. A full repo-corpus differential (1839 scraped queries) found **zero AST divergences** and exactly one deliberate language fix: `MATCH (n) RETURN DISTINCT` (no items) is now a syntax error — Earley's dynamic lexer had re-lexed `DISTINCT` as a NAME and accepted it as returning a *variable named* DISTINCT (with `distinct=False`), a lexing accident, now pinned as a rejection test. Full cypher suite (1671 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. +- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the LALR conflict profile from 8 shift/reduce to **2 shift/reduce, 0 reduce/reduce**, each remaining conflict mapped 1:1 to a characterized, AST-neutral derivation ambiguity (`[x IN xs]`, `(n:Admin)`). **Machine-checked invariants** (`test_grammar_invariants.py`): (1) the conflict profile is pinned so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is ZERO — every Earley derivation of every corpus query transforms to the same AST, no exceptions — with the two residual binary derivation ambiguities pinned as witnesses; (3) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and exactly five deliberate language fixes, all accept-by-accident shapes with ill-defined semantics, now syntax errors and pinned as tests: `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, and WHERE before MATCH in a graph constructor. Full cypher suite (1,673 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. ### Fixed - **CI: `compute_networkx` HITS test was scipy-version flaky**: `test_compute_networkx_hits_outputs_hubs_and_authorities` used a pure directed 3-cycle (`a→b→c→a`), whose adjacency is a permutation matrix (all singular values equal). networkx HITS computes scores via scipy `svds(k=1)`, which then returns an arbitrary vector from that fully-degenerate singular space; when its components sum to ~0, HITS's `h /= h.sum()` normalization blows up to `inf`/`nan`. Which vector `svds` returns is scipy/LAPACK-version dependent, so the test passed on the pinned scipy but failed on the `nx-upper-scipy` CI profile's newer scipy — a latent flake, not a code regression. Switched to a graph with a well-defined hub/authority structure (`a→b`, `a→c`, `b→c`), whose dominant singular vector is unique (Perron-Frobenius) and therefore version-stable. Test-only change. diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index 65214483be..3ffbea256f 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -57,7 +57,6 @@ graph_constructor_body: graph_constructor_item+ graph_constructor_item: match_clause - | where_clause | call_clause | use_clause @@ -69,7 +68,6 @@ | "UNION"i -> union_distinct query_item: match_clause - | where_clause | call_clause | unwind_clause | use_clause @@ -81,8 +79,12 @@ with_stage: with_clause with_where_clause? order_by_clause? skip_clause? limit_clause? return_stage: return_clause order_by_clause? skip_clause? limit_clause? -match_clause: "MATCH"i match_item ("," match_item)* -> match_clause - | "OPTIONAL"i "MATCH"i match_item ("," match_item)* -> optional_match_clause +// WHERE binds to its preceding clause IN THE GRAMMAR (openCypher scoping): +// after MATCH it is part of match_clause; after WITH it is with_where_clause. +// A standalone where_clause item would make `MATCH .. WHERE ..` derivable two +// ways (bundled vs standalone) -- a genuine ambiguity -- so it does not exist. +match_clause: "MATCH"i match_item ("," match_item)* where_clause? -> match_clause + | "OPTIONAL"i "MATCH"i match_item ("," match_item)* where_clause? -> optional_match_clause match_item: pattern | NAME "=" pattern -> bound_pattern | NAME "=" "shortestPath"i "(" pattern ")" -> shortest_path_pattern @@ -148,7 +150,7 @@ yield_item: NAME alias? with_clause: "WITH"i distinct? return_item ("," return_item)* -with_where_clause.2: "WHERE"i expr +with_where_clause: "WHERE"i expr return_clause: "RETURN"i distinct? return_item ("," return_item)* distinct: "DISTINCT"i return_item: return_expr alias? @@ -215,14 +217,20 @@ | MINUS unary -> uminus | postfix -?postfix: primary +// Dotted chains rooted at a bare NAME derive ONLY via qualified_name (a.b.c); +// property_access applies ONLY to composite roots (calls, groups, subscripts: +// f(x).y, (e).y, xs[0].y). Splitting the two keeps the grammar unambiguous -- +// a single derivation for every dotted expression -- and LALR(1) conflict-free +// at DOT (no reduce-qualified_name vs shift-extend race). +?postfix: qualified_name + | postfix_composite +?postfix_composite: primary_composite | postfix "[" subscript_key "]" -> subscript - | postfix "." NAME -> property_access + | postfix_composite "." NAME -> property_access -?primary: parameter +?primary_composite: parameter | literal | function_call - | qualified_name | case_expr | quantifier_expr | list_comprehension @@ -916,6 +924,13 @@ def optional_match_clause(self, meta: Any, items: Sequence[Any]) -> MatchClause: return self._match_clause(meta, items, optional=True) def _match_clause(self, meta: Any, items: Sequence[Any], *, optional: bool) -> MatchClause: + # The grammar bundles a trailing WHERE into the match_clause (WHERE + # binds to its preceding clause declaratively). Split it back off; + # query_body's assembly consumes it as if it were a standalone item. + where: Optional[WhereClause] = None + if items and isinstance(items[-1], WhereClause): + where = items[-1] + items = items[:-1] if len(items) < 1: clause_name = "OPTIONAL MATCH" if optional else "MATCH" raise _to_syntax_error(f"Cypher {clause_name} clause cannot be empty", line=meta.line, column=meta.column) @@ -931,12 +946,26 @@ def _match_clause(self, meta: Any, items: Sequence[Any], *, optional: bool) -> M patterns.append(cast(Tuple[PatternElement, ...], item)) pattern_aliases.append(None) pattern_alias_kinds.append("pattern") + # Span covers the MATCH..patterns text only (not the bundled WHERE), + # preserving the clause span the AST has always carried: end at the + # last non-whitespace character before the WHERE keyword. + span = _span_from_meta(meta) + if where is not None: + end_pos = span.start_pos + len(source[span.start_pos:where.span.start_pos].rstrip()) + line_start = source.rfind("\n", 0, end_pos) + 1 + span = replace( + span, + end_pos=end_pos, + end_line=source.count("\n", 0, end_pos) + 1, + end_column=end_pos - line_start + 1, + ) return MatchClause( patterns=tuple(patterns), - span=_span_from_meta(meta), + span=span, optional=optional, pattern_aliases=tuple(pattern_aliases), pattern_alias_kinds=tuple(pattern_alias_kinds), + where=where, ) def distinct(self, _meta: Any, _items: Sequence[Any]) -> bool: @@ -1463,6 +1492,18 @@ def query_item(self, meta: Any, items: Sequence[Any]) -> Any: return items[0] def query_body(self, meta: Any, items: Sequence[Any]) -> CypherQuery: + # The grammar bundles WHERE into its MATCH clause. Re-emit it as a + # follow-on item so the clause-sequence state machine below (which + # predates the bundling and encodes all ordering/support rules) + # processes the exact sequence the source text spells. + flat: List[Any] = [] + for item in items: + if isinstance(item, MatchClause) and item.where is not None: + flat.append(replace(item, where=None)) + flat.append(item.where) + else: + flat.append(item) + items = flat trailing_semicolon = any(str(item) == ";" for item in items) match_clauses: List[MatchClause] = [] reentry_match_clauses: List[MatchClause] = [] @@ -1742,6 +1783,16 @@ def graph_constructor_body(self, meta: Any, items: Sequence[Any]) -> Any: def graph_constructor(self, meta: Any, items: Sequence[Any]) -> GraphConstructor: span = _span_from_meta(meta) body_items = items[0] if items and isinstance(items[0], list) else list(items) + # Split grammar-bundled MATCH..WHERE back into the item sequence the + # constructor-body rules below were written against (see query_body). + flat: List[Any] = [] + for item in body_items: + if isinstance(item, MatchClause) and item.where is not None: + flat.append(replace(item, where=None)) + flat.append(item.where) + else: + flat.append(item) + body_items = flat matches: List[MatchClause] = [] where: Optional[WhereClause] = None use: Optional[UseClause] = None diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py index 91620ff4da..8b17d4d230 100644 --- a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -9,15 +9,16 @@ string (genuine ambiguity), which LALR resolves arbitrarily. A grammar edit that changes the profile fails this test and forces a review. -2. **Semantic ambiguity is zero, up to one characterized case** - (``test_semantic_ambiguity_zero_up_to_known_cases``): expanding EVERY - Earley derivation of every corpus query (``ambiguity='explicit'`` + - ``CollapseAmbiguities``), all derivations transform to the SAME AST — - except the known WITH..WHERE attachment ambiguity, where the production - resolution (WHERE attaches to the WITH stage) is asserted explicitly. - Note plain ``_ambig`` node counting is NOT a usable invariant: Earley - reports harmless alternative derivations even for ``MATCH (a) RETURN a.val``; - what matters is that they collapse to one AST. +2. **Semantic ambiguity is ZERO** (``test_semantic_ambiguity_zero``): + expanding EVERY Earley derivation of every corpus query + (``ambiguity='explicit'`` + ``CollapseAmbiguities``), all derivations + transform to the SAME AST, unconditionally. Two residual DERIVATION + ambiguities exist (each exactly binary, each AST-identical, matching the + two pinned shift/reduce conflicts) and are machine-surfaced as pinned + witnesses in ``test_residual_derivation_ambiguity_witnesses``. The former + WITH..WHERE attachment ambiguity was ELIMINATED at grammar level (WHERE is + bundled into its preceding clause), as was the dotted-name redundancy + (name-rooted dot chains derive only via ``qualified_name``). 3. **Parser-choice neutrality** (``test_lalr_ast_equals_earley_ast_differential``): running the production pipeline with an Earley parser over the same grammar @@ -101,6 +102,12 @@ "MATCH (a:X) RETURN a.id AS id UNION ALL MATCH (b:Y) RETURN b.id AS id", # graph constructor / USE "GRAPH g1 = GRAPH { MATCH (a)-[r]->(b) WHERE a.id = 'x' } USE g1 MATCH (n) RETURN n", + # comprehensions / quantifiers / subscripts / composite property access + "MATCH (n) RETURN [x IN n.tags WHERE x > 1 | x + 1] AS t", + "MATCH (n) WHERE any(x IN n.tags WHERE x = 'a') RETURN n", + "MATCH (n) RETURN n.tags[0] AS first, head(n.tags) AS h", + "MATCH (n) RETURN (n:Admin) AS is_admin", + "MATCH (n) RETURN [x IN n.tags] AS t", ] # Invalid inputs: both parsers must reject (rejection parity, not message parity). @@ -113,13 +120,18 @@ "MATCH (a)-[:R*..4]->(b) RETURN b", # open LOWER hop bound is not in the grammar ] -# The ONE known semantic ambiguity: WHERE after a WITH stage can attach to the -# WITH stage or (in an alternative derivation) to the MATCH clause. Both Earley -# (rule priority ``with_where_clause.2``) and LALR (shift preference) resolve it -# to the WITH-stage attachment; the differential test proves they agree. -KNOWN_AMBIGUOUS = { - q for q in DIFFERENTIAL_CORPUS if re.search(r"\bWITH\b.*\bWHERE\b", q) -} +# Residual DERIVATION ambiguities: strings with exactly TWO parse trees that +# transform to ONE identical AST (harmless, but machine-surfaced so a grammar +# edit can't silently grow them). Each corresponds 1:1 to a pinned +# shift/reduce conflict: +# - `[x IN xs]`: list_comprehension (no body) vs list_literal of an in_op +# element — the IN conflict; both mean the identity comprehension text. +# - `(n:Admin)` in RETURN: grouped_label_predicate vs grouped_expr over a +# bare_label_predicate — the RPAR conflict; both yield the same item. +RESIDUAL_DERIVATION_AMBIGUITY = [ + "MATCH (n) RETURN [x IN n.tags] AS t", + "MATCH (n) RETURN (n:Admin) AS is_admin", +] def _fresh_lalr_with_log() -> List[logging.LogRecord]: @@ -170,12 +182,19 @@ def test_lalr_conflict_profile_pinned() -> None: all resolved as shift. A grammar edit that changes this set must update the pin AND justify each new conflict here: - - DOT x5, IN x1 after ``qualified_name : NAME``: greedy-extend of dotted - names / IN-expressions; shift = longest match, the intended parse. - - WHERE x1 at ``with_stage : with_clause``: attach WHERE to the WITH stage - (the documented resolution of the one known attachment ambiguity). - - RPAR x1 at ``bare_label_predicate_expr : NAME labels``: greedy label - list inside parens; shift = longest match. + - IN x1 at ``qualified_name : NAME``: inside ``[``, a NAME followed by IN + is a comprehension head (shift) rather than an expr atom (reduce). The + residual-witness test pins that both readings of the overlapping string + ``[x IN xs]`` produce the same AST. + - RPAR x1 at ``bare_label_predicate_expr : NAME labels``: ``(n:Admin)`` as + a return item is a grouped_label_predicate (shift) rather than a + grouped_expr over the bare label predicate (reduce); same AST either + way (residual-witness test). + + Historical note: DOT x5 (dotted-name redundancy) and WHERE x1 (WITH..WHERE + attachment) were eliminated at grammar level — name-rooted dot chains + derive only via ``qualified_name``, and WHERE is bundled into its + preceding clause — so they must never reappear. """ conflict_re = re.compile( r"(Shift/Reduce|Reduce/Reduce) conflict for terminal (\w+): \(resolving as (\w+)\)" @@ -191,22 +210,18 @@ def test_lalr_conflict_profile_pinned() -> None: f"Reduce/reduce conflict introduced (genuine grammar ambiguity): {profile}" ) assert profile == { - ("Shift/Reduce", "DOT", "shift"): 5, ("Shift/Reduce", "IN", "shift"): 1, - ("Shift/Reduce", "WHERE", "shift"): 1, ("Shift/Reduce", "RPAR", "shift"): 1, }, f"LALR conflict profile changed: {profile}" # --- 2. Semantic ambiguity ----------------------------------------------------- -def test_semantic_ambiguity_zero_up_to_known_cases() -> None: - """Every Earley derivation of every corpus query transforms to the same AST - (semantic ambiguity = 0), except the known WITH..WHERE attachment case, - which must stay exactly binary (2 distinct ASTs: WHERE on the WITH stage - vs WHERE on the MATCH clause). Which side production picks is pinned - elsewhere: the LALR==Earley differential below plus the WITH..WHERE - parser tests both fix it to the WITH-stage attachment. +def test_semantic_ambiguity_zero() -> None: + """Every Earley derivation of every corpus query transforms to the SAME + AST — semantic ambiguity is zero, no exceptions. (The former WITH..WHERE + attachment ambiguity was eliminated by bundling WHERE into its preceding + clause in the grammar.) Note: ``CollapseAmbiguities`` rebuilds trees without ``meta`` positions, so derivation ASTs have degraded spans/text and CANNOT be compared against the @@ -221,19 +236,40 @@ def test_semantic_ambiguity_zero_up_to_known_cases() -> None: asts = { repr(parser_mod._build_transformer(query).transform(t)) for t in trees } - if query in KNOWN_AMBIGUOUS: - assert len(asts) == 2, ( - f"WITH..WHERE attachment ambiguity should be exactly binary, " - f"got {len(asts)} distinct ASTs for: {query!r}" - ) - elif len(asts) != 1: + if len(asts) != 1: unexpected.append(f"{query!r}: {len(trees)} derivations, {len(asts)} ASTs") assert not unexpected, ( - "New semantic ambiguity introduced (derivations disagree on AST):\n" + "Semantic ambiguity introduced (derivations disagree on AST):\n" + "\n".join(unexpected) ) +def test_residual_derivation_ambiguity_witnesses() -> None: + """Machine-surfaced witnesses for the ONLY residual derivation + ambiguities: each is exactly binary and AST-identical (see + RESIDUAL_DERIVATION_AMBIGUITY for the 1:1 mapping to the two pinned + shift/reduce conflicts). Every other corpus query has exactly ONE + derivation. A grammar edit that grows either count fails here and must + characterize the new ambiguity before landing.""" + from lark.visitors import CollapseAmbiguities + + explicit = _earley_parser(ambiguity="explicit") + for query in RESIDUAL_DERIVATION_AMBIGUITY: + trees = CollapseAmbiguities().transform(explicit.parse(query)) + asts = { + repr(parser_mod._build_transformer(query).transform(t)) for t in trees + } + assert len(trees) == 2, f"expected binary derivation ambiguity: {query!r}" + assert len(asts) == 1, f"derivations must agree on the AST: {query!r}" + for query in DIFFERENTIAL_CORPUS: + if query in RESIDUAL_DERIVATION_AMBIGUITY: + continue + trees = CollapseAmbiguities().transform(explicit.parse(query)) + assert len(trees) == 1, ( + f"unexpected derivation ambiguity ({len(trees)} trees): {query!r}" + ) + + # --- 3. LALR == Earley differential --------------------------------------------- def test_lalr_ast_equals_earley_ast_differential( @@ -272,15 +308,27 @@ def test_lalr_and_earley_reject_the_same_inputs( parser_mod._parse_cypher_cached.cache_clear() -def test_return_distinct_without_items_is_rejected() -> None: - """The ONE deliberate language difference from the old Earley parser, - found by a full repo-corpus differential (1839 queries; everything else - is accept/reject- and AST-identical). +# The complete set of DELIBERATE language fixes vs the old Earley parser, +# found by full repo-corpus differentials (1800+ queries; everything else is +# accept/reject- and AST-identical). Each was an accept-by-accident with +# ill-defined semantics; all are now honest syntax errors. +DELIBERATE_LANGUAGE_FIXES = [ + # Earley's dynamic lexer re-lexed DISTINCT (absent from the NAME reserved + # lookahead) as a NAME and accepted this as returning a *variable named* + # DISTINCT with distinct=False. The LALR contextual lexer keeps it a keyword. + "MATCH (n) RETURN DISTINCT", + # The old flat clause-item grammar accepted WHERE anywhere and attached it + # positionally in Python; these shapes had ill-defined attachment (a double + # WHERE kept BOTH predicates in different AST fields). WHERE now binds to + # its preceding clause in the grammar, so they are syntax errors: + "MATCH (a) WHERE a.x = 1 WHERE a.y = 2 RETURN a", # double WHERE + "MATCH (n) WITH n WHERE n.x = 1 WHERE n.y = 2 RETURN n", # double post-WITH WHERE + "MATCH (n) UNWIND n.tags AS t WHERE t = 'a' RETURN t", # WHERE after UNWIND (not openCypher) + "GRAPH g1 = GRAPH { WHERE a.x = 1 MATCH (a) } USE g1 MATCH (n) RETURN n", # WHERE before MATCH +] + - ``DISTINCT`` is not in the NAME terminal's reserved-word lookahead, so - Earley's dynamic lexer re-lexed it as a NAME and accepted - ``MATCH (n) RETURN DISTINCT`` as returning a *variable named* DISTINCT - (with ``distinct=False``!) — a lexing accident, not Cypher. The LALR - contextual lexer keeps it a keyword, so the query is now a syntax error.""" +@pytest.mark.parametrize("query", DELIBERATE_LANGUAGE_FIXES) +def test_deliberate_language_fixes_are_rejected(query: str) -> None: with pytest.raises(GFQLSyntaxError): - parse_cypher("MATCH (n) RETURN DISTINCT") + parse_cypher(query) From 701d75c327751c753487e09c9346f27d9e54c67c Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 12:07:13 -0700 Subject: [PATCH 07/12] test(gfql/cypher): full-repo LALR==Earley differential as a CI gate test_differential_grammar_parity.py scrapes every cypher-looking string in the package (1,800+) and runs each through the production pipeline under both parsers: byte-identical ASTs, rejection parity, error-class parity, with the 5 pinned deliberate language fixes as the only allowed divergences. Named to match the test_differential*.py glob of the existing cypher-frontend-differential-parity CI job (zero workflow changes); the grammar invariants file is already auto-discovered by test-gfql-core. Verified on dgx-spark: 11 grammar tests pass in 30s. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test_differential_grammar_parity.py | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 graphistry/tests/compute/gfql/cypher/test_differential_grammar_parity.py diff --git a/graphistry/tests/compute/gfql/cypher/test_differential_grammar_parity.py b/graphistry/tests/compute/gfql/cypher/test_differential_grammar_parity.py new file mode 100644 index 0000000000..4cc25188f3 --- /dev/null +++ b/graphistry/tests/compute/gfql/cypher/test_differential_grammar_parity.py @@ -0,0 +1,134 @@ +"""Full-repo LALR==Earley grammar-parity differential (CI gate). + +Scrapes every cypher-looking string literal from the graphistry package +(python ``ast`` over all ``.py`` files — tests, source, corpora) and runs each +through the PRODUCTION pipeline twice: once with the production LALR parser +and once with an Earley parser built from the SAME grammar. Asserts: + +- every query both parsers accept produces a byte-identical AST; +- both parsers reject the same queries (rejection parity), with the same + error class — EXCEPT the pinned deliberate language fixes (see + ``DELIBERATE_LANGUAGE_FIXES`` in test_grammar_invariants.py), where + Earley's dynamic lexer accepts a lexing accident that LALR correctly + rejects. + +This is the broad-corpus complement to the constructed corpus in +``test_grammar_invariants.py``: it sweeps everything the repo actually says, +so a grammar edit that shifts parser behavior on ANY in-repo query fails +here. The file name matches the ``test_differential*.py`` glob of the +``cypher-frontend-differential-parity`` CI job. +""" +from __future__ import annotations + +import ast as pyast +import os +from typing import Any, List, Set, Tuple + +import pytest + +from graphistry.compute.gfql.cypher import parse_cypher +from graphistry.compute.gfql.cypher import parser as parser_mod + +lark = pytest.importorskip("lark") + +# Queries where LALR (correctly) rejects what Earley's dynamic lexer +# accepted by accident — the pinned language fixes. Loaded from the sibling +# invariants module by path (the tests tree is not a package). +import importlib.util as _ilu # noqa: E402 + +_spec = _ilu.spec_from_file_location( + "_tgi", os.path.join(os.path.dirname(__file__), "test_grammar_invariants.py") +) +assert _spec is not None and _spec.loader is not None +_tgi = _ilu.module_from_spec(_spec) +_spec.loader.exec_module(_tgi) +DELIBERATE_LANGUAGE_FIXES = _tgi.DELIBERATE_LANGUAGE_FIXES + +_PKG_ROOT = os.path.abspath( + os.path.join(os.path.dirname(parser_mod.__file__), "..", "..", "..") +) + + +def _looks_cypher(s: str) -> bool: + if not (10 <= len(s) <= 4000) or "\x00" in s: + return False + return "RETURN" in s or "MATCH" in s + + +def _scrape_corpus() -> List[str]: + corpus: Set[str] = set() + for dirpath, dirnames, filenames in os.walk(_PKG_ROOT): + dirnames[:] = [d for d in dirnames if d != "__pycache__"] + for fn in filenames: + if not fn.endswith(".py"): + continue + try: + with open(os.path.join(dirpath, fn), encoding="utf-8") as f: + tree = pyast.parse(f.read()) + except (SyntaxError, OSError): + continue + for node in pyast.walk(tree): + if isinstance(node, pyast.Constant) and isinstance(node.value, str): + s = node.value.strip() + if _looks_cypher(s): + corpus.add(s) + return sorted(corpus) + + +def _run_with(parser: Any, query: str, monkeypatch: pytest.MonkeyPatch) -> Tuple[str, str]: + monkeypatch.setattr(parser_mod, "_parser_lalr", lambda: parser) + parser_mod._parse_cypher_cached.cache_clear() + try: + return ("OK", repr(parse_cypher(query))) + except Exception as exc: # both syntax + validation rejections count + return ("REJECT", type(exc).__name__) + finally: + monkeypatch.undo() + parser_mod._parse_cypher_cached.cache_clear() + + +def test_full_repo_lalr_earley_differential(monkeypatch: pytest.MonkeyPatch) -> None: + earley = lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="earley", + maybe_placeholders=False, + propagate_positions=True, + ) + lalr = parser_mod._parser_lalr() + corpus = _scrape_corpus() + assert len(corpus) > 500, "corpus scrape looks broken (too few queries)" + + known_fixes = set(DELIBERATE_LANGUAGE_FIXES) + ast_diverge: List[str] = [] + lang_diverge: List[str] = [] + err_diverge: List[str] = [] + n_ok = n_rej = 0 + for query in corpus: + lalr_status, lalr_val = _run_with(lalr, query, monkeypatch) + earley_status, earley_val = _run_with(earley, query, monkeypatch) + if lalr_status == earley_status == "OK": + n_ok += 1 + if lalr_val != earley_val: + ast_diverge.append(query) + elif lalr_status == earley_status == "REJECT": + n_rej += 1 + if lalr_val != earley_val: + err_diverge.append(f"{query!r}: LALR={lalr_val} Earley={earley_val}") + elif query in known_fixes: + assert lalr_status == "REJECT", ( + f"pinned language fix should be LALR-rejected: {query!r}" + ) + else: + lang_diverge.append(f"{query!r}: LALR={lalr_status} Earley={earley_status}") + + assert not ast_diverge, ( + f"AST divergence on {len(ast_diverge)} queries, e.g.:\n" + + "\n".join(repr(q) for q in ast_diverge[:10]) + ) + assert not lang_diverge, ( + "unpinned accept/reject divergence (add to DELIBERATE_LANGUAGE_FIXES " + "only if deliberate):\n" + "\n".join(lang_diverge[:10]) + ) + assert not err_diverge, "error-class divergence:\n" + "\n".join(err_diverge[:10]) + assert n_ok > 500 and n_rej > 50, f"corpus balance unexpected: ok={n_ok} rej={n_rej}" From 847def4933d3780e2c8d353607323539e29465cd Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 12:18:37 -0700 Subject: [PATCH 08/12] =?UTF-8?q?test(gfql/cypher):=20grammar-rule=20cover?= =?UTF-8?q?age=20gate=20=E2=80=94=20extensions=20can't=20skip=20the=20safe?= =?UTF-8?q?ty=20net?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_every_grammar_rule_is_exercised_by_the_corpus asserts every tree-producing grammar rule (aliases + non-inlined origins, across all three entry points) appears in some corpus parse. Adding a grammar rule without a DIFFERENTIAL_CORPUS query now FAILS with the rule's name — which forces the new construct through every other invariant (ambiguity probe, LALR==Earley differential) and the full-repo differential. Closes the gap where a new rule got only conflict-profile coverage until someone remembered to write queries. Backfilled the corpus to 100% rule coverage (arithmetic/comparison operators, unary +/- on non-literal operands, composite-root property access, list-slice subscripts, quantifiers, map/list literals, DISTINCT-agg, simple relationship arrows, path binding, count(*)). Documented the safe-by-construction edit flow at the top of _GRAMMAR in parser.py. Verified on dgx-spark: 182 grammar+parser tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphistry/compute/gfql/cypher/parser.py | 25 ++++++ .../gfql/cypher/test_grammar_invariants.py | 88 +++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index 3ffbea256f..0d379db391 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -44,6 +44,31 @@ ) +# --------------------------------------------------------------------------- +# GRAMMAR — the declarative source of truth for Cypher syntax. +# +# EDITING THIS GRAMMAR (the safe-by-construction extension flow): +# 1. Add / change a rule here. +# 2. Add at least one query exercising the new syntax to +# DIFFERENTIAL_CORPUS in tests/.../test_grammar_invariants.py (or, if the +# construct is grammatical-but-intentionally-unsupported, to +# GRAMMAR_ONLY_COVERAGE). test_every_grammar_rule_is_exercised_by_the_corpus +# FAILS with the new rule's name until you do — you cannot land a rule +# with no coverage. +# 3. Run test_grammar_invariants.py. The machine checks that guard you: +# - conflict profile is PINNED (0 reduce/reduce always; the exact +# shift/reduce set is asserted) — a new ambiguity fails the build; +# - semantic ambiguity is ZERO (every Earley derivation -> same AST); +# a new binary-but-AST-neutral ambiguity must be added to +# RESIDUAL_DERIVATION_AMBIGUITY with a justification, anything worse +# fails hard; +# - LALR == Earley over the corpus + full-repo scrape (AST-identical). +# 4. If the edit deliberately changes accept/reject, pin it in +# DELIBERATE_LANGUAGE_FIXES. Otherwise a language change fails the +# differential. +# The grammar carries the correctness argument; the tests make its properties +# machine-checked. Do not resolve ambiguity in Python — fix it in the grammar. +# --------------------------------------------------------------------------- _GRAMMAR = r""" ?start: graph_query diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py index 8b17d4d230..9358c782df 100644 --- a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -104,10 +104,46 @@ "GRAPH g1 = GRAPH { MATCH (a)-[r]->(b) WHERE a.id = 'x' } USE g1 MATCH (n) RETURN n", # comprehensions / quantifiers / subscripts / composite property access "MATCH (n) RETURN [x IN n.tags WHERE x > 1 | x + 1] AS t", + "MATCH (n) RETURN [x IN n.tags WHERE x > 1] AS t", + "MATCH (n) RETURN [x IN n.tags | x + 1] AS t", "MATCH (n) WHERE any(x IN n.tags WHERE x = 'a') RETURN n", + "MATCH (n) WHERE all(x IN n.tags WHERE x > 0) RETURN n", + "MATCH (n) WHERE none(x IN n.tags WHERE x < 0) RETURN n", + "MATCH (n) WHERE single(x IN n.tags WHERE x = 1) RETURN n", "MATCH (n) RETURN n.tags[0] AS first, head(n.tags) AS h", "MATCH (n) RETURN (n:Admin) AS is_admin", "MATCH (n) RETURN [x IN n.tags] AS t", + # remaining construct families (rule-coverage: every grammar rule must be + # exercised -- see test_every_grammar_rule_is_exercised_by_the_corpus) + "MATCH p = (a)-[:R]->(b) RETURN p", + "MATCH (n) RETURN count(DISTINCT n.city) AS c", + "MATCH (n) WHERE n.flag = false AND n.gone = null RETURN n", + "GRAPH { MATCH (a)-[r]->(b) }", + "MATCH (n) RETURN {name: n.x, 'k': 1} AS m", + "MATCH (n) RETURN *", + "MATCH ()-[r]->() RETURN count(*)", + "MATCH (a)-->(b) RETURN a, b", + "MATCH (a)<--(b) RETURN a, b", + "MATCH (a)--(b) RETURN a, b", + "MATCH (a)<-->(b) RETURN a, b", + "MATCH (a)-[:R*2]->(b) RETURN b", + # arithmetic / comparison operators + unary + "MATCH (n) WHERE 1 < n.x < 10 RETURN n", + "MATCH (n) WHERE n.x / 2 > n.y % 3 RETURN n", + "MATCH (n) WHERE n.x - 1 > 0 RETURN n", + # unary +/- on a NON-literal operand: a signed literal (-1) is an + # ambiguous literal-vs-uminus form, but -n.x is an unambiguous unary node + "MATCH (n) RETURN -n.x AS v, +n.y AS w", + # composite-root property access + list subscript slices + "MATCH (n) RETURN head(n.tags).name AS v", + "MATCH (n) RETURN n.tags[1..3] AS a, n.tags[1..] AS b, n.tags[..3] AS c, n.tags[..] AS d", +] + +# Queries that are GRAMMATICAL but rejected by the transformer by design +# (known-unsupported features that must still parse to give a good error). +# They contribute to grammar-rule coverage via raw parse only. +GRAMMAR_ONLY_COVERAGE = [ + "MATCH p = allShortestPaths((a)-[:R]->(b)) RETURN p", # raises unsupported at transform ] # Invalid inputs: both parsers must reject (rejection parity, not message parity). @@ -270,6 +306,58 @@ def test_residual_derivation_ambiguity_witnesses() -> None: ) +# --- Rule coverage: the corpus must exercise EVERY grammar rule ------------------ + +def test_every_grammar_rule_is_exercised_by_the_corpus() -> None: + """Every tree-producing grammar rule must appear in some corpus parse. + + This is what makes syntactic EXTENSIONS safe-by-construction: adding a + grammar rule without a corpus query fails HERE with the rule's name, and + adding the query automatically subjects the new construct to every other + invariant in this file (ambiguity probe, LALR==Earley differential) plus + the full-repo differential. There is no way to extend the grammar and + silently skip the safety net. + + Mechanics: 'tree-producing' = rule aliases plus non-inlined rule names + (lark's ``?rule:`` single-child inlining and ``_``-prefixed helpers never + appear as tree nodes, so they are not coverable and not counted). + Coverage counts raw parses of DIFFERENTIAL_CORPUS + GRAMMAR_ONLY_COVERAGE + (grammatical-but-unsupported constructs) on all three grammar entry + points (whole query, WHERE-lift chain, pattern fragment).""" + lalr = parser_mod._parser_lalr() + + expected = set() + for rule in lalr.rules: # type: ignore[attr-defined] # concrete lark.Lark + if rule.alias: + expected.add(rule.alias) + elif not rule.options.expand1 and not rule.origin.name.startswith("_"): + expected.add(rule.origin.name) + + observed = set() + + def walk(tree: Any) -> None: + if isinstance(tree, lark.Tree): + observed.add(tree.data) + for child in tree.children: + walk(child) + + for query in DIFFERENTIAL_CORPUS + GRAMMAR_ONLY_COVERAGE: + walk(lalr.parse(query)) + # the two sub-grammar entry points (their rules are unused from start=) + walk(parser_mod._where_predicate_chain_parser().parse( + "n.x = 1 AND n.y IS NULL AND n.z IS NOT NULL AND n.s CONTAINS 'a' " + "AND n.t STARTS WITH 'b' AND n.u ENDS WITH 'c' AND n:Admin" + )) + walk(parser_mod._pattern_parser().parse("(a:X {k: 1})-[r:R*1..2]->(b)")) + + uncovered = sorted(expected - observed) + assert not uncovered, ( + "Grammar rules with NO corpus coverage (add a DIFFERENTIAL_CORPUS " + "query exercising each, or GRAMMAR_ONLY_COVERAGE if the construct is " + f"grammatical-but-unsupported): {uncovered}" + ) + + # --- 3. LALR == Earley differential --------------------------------------------- def test_lalr_ast_equals_earley_ast_differential( From 6902f367e2ba1fea57940519d3931134bf64b776 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 13:13:59 -0700 Subject: [PATCH 09/12] =?UTF-8?q?refactor(gfql/cypher):=20eliminate=20the?= =?UTF-8?q?=20RPAR=20conflict=20=E2=80=94=202=20S/R=20->=201=20S/R?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dropped the redundant `label_predicate_expr: "(" NAME labels ")"` rule from return_expr. A parenthesized label predicate `(n:Admin)` already parses via `expr -> grouped_expr` over `bare_label_predicate_expr` with a byte-identical AST, so the dedicated rule was pure derivation redundancy — exactly the RPAR shift/reduce conflict. Verified 0 AST diffs on the probe corpus + full suite. Conflict profile is now **1 S/R (IN), 0 R/R**. The IN conflict is inherent to Cypher (`[NAME IN ...` is a comprehension or a list whose first element is an in_op — indistinguishable at `[ NAME . IN` with one lookahead); empirically unremovable without duplicating the expr hierarchy (dropping lc_source does NOT remove it and breaks `[x IN xs]`), so it stays pinned as a characterized, AST-neutral witness `[x IN xs]`. Renamed the transformer method grouped_label_predicate -> bare_label_predicate (its only remaining binding) now that the "grouped" rule is gone. Verified on dgx-spark: full cypher suite 1679 passed / 0 failed; grammar invariants + full-repo differential green. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- graphistry/compute/gfql/cypher/parser.py | 10 +++---- .../gfql/cypher/test_grammar_invariants.py | 27 +++++++++---------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8ab40c156..2cffc7a2c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL `engine='polars'`/`'polars-gpu'` can now run off-engine analytic `call()`s (umap/hypergraph/compute_cugraph/…) — `call_mode='auto'` (default)**: a GFQL `call()` that invokes a Plottable-method analytic with **no native polars implementation** (and never will — `umap`, `hypergraph`, `compute_cugraph`, `compute_igraph`, `layout_*`, `collapse`, `get_topological_levels`, …) previously raised `NotImplementedError` under a polars engine, forcing users off polars for the whole pipeline. It now runs as a **mode-gated, warned modality switch**: `call_mode='auto'` (default) bridges the graph off-engine (`polars`→pandas, `polars-gpu`→cuDF **on-device**), runs the analytic, and coerces the result back to polars **losslessly via Arrow**, warning once per (process, function). `call_mode='strict'` keeps the honest `NotImplementedError` decline (for benchmark integrity / a hard memory ceiling). `polars-gpu` is **GPU-or-error**: it bridges to cuDF and declines rather than silently dropping a GPU analytic to host pandas. This is **deliberately narrower than CHAIN traversal/filter/row ops**, which stay parity-or-NIE (a bridge there would hide a missing impl and cheat a benchmark) — the split is mechanical (`is_row_pipeline_call`), and the chain and DAG surfaces bridge consistently. Configurable via `graphistry.compute.gfql.lazy.set_call_mode('auto'|'strict')` (Python) or `GFQL_POLARS_CALL_MODE` (env), resolving Python override > env > default('auto'), read live. Verified on dgx: `compute_cugraph` PageRank is byte-parity with the pandas oracle under both `polars` and `polars-gpu`. ### Changed -- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the LALR conflict profile from 8 shift/reduce to **2 shift/reduce, 0 reduce/reduce**, each remaining conflict mapped 1:1 to a characterized, AST-neutral derivation ambiguity (`[x IN xs]`, `(n:Admin)`). **Machine-checked invariants** (`test_grammar_invariants.py`): (1) the conflict profile is pinned so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is ZERO — every Earley derivation of every corpus query transforms to the same AST, no exceptions — with the two residual binary derivation ambiguities pinned as witnesses; (3) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and exactly five deliberate language fixes, all accept-by-accident shapes with ill-defined semantics, now syntax errors and pinned as tests: `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, and WHERE before MATCH in a graph constructor. Full cypher suite (1,673 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. +- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — and dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), dropping the LALR conflict profile from 8 shift/reduce to **1 shift/reduce, 0 reduce/reduce** — the single remaining conflict (`[x IN xs]`, inherent to Cypher's comprehension-vs-list overlap) mapped to a characterized, AST-neutral derivation ambiguity and pinned as a witness. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) the conflict profile is pinned so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is ZERO — every Earley derivation of every corpus query transforms to the same AST, no exceptions — with the two residual binary derivation ambiguities pinned as witnesses; (3) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and exactly five deliberate language fixes, all accept-by-accident shapes with ill-defined semantics, now syntax errors and pinned as tests: `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, and WHERE before MATCH in a graph constructor. Full cypher suite (1,673 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. ### Fixed - **CI: `compute_networkx` HITS test was scipy-version flaky**: `test_compute_networkx_hits_outputs_hubs_and_authorities` used a pure directed 3-cycle (`a→b→c→a`), whose adjacency is a permutation matrix (all singular values equal). networkx HITS computes scores via scipy `svds(k=1)`, which then returns an arbitrary vector from that fully-degenerate singular space; when its components sum to ~0, HITS's `h /= h.sum()` normalization blows up to `inf`/`nan`. Which vector `svds` returns is scipy/LAPACK-version dependent, so the test passed on the pinned scipy but failed on the `nx-upper-scipy` CI profile's newer scipy — a latent flake, not a code regression. Switched to a graph with a well-defined hub/authority structure (`a→b`, `a→c`, `b→c`), whose dominant singular vector is unique (Perron-Frobenius) and therefore version-stable. Test-only change. diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index 0d379db391..8d6f0cbdd3 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -179,10 +179,12 @@ return_clause: "RETURN"i distinct? return_item ("," return_item)* distinct: "DISTINCT"i return_item: return_expr alias? +// A parenthesized label predicate ``(n:Admin)`` parses via ``expr`` -> +// ``grouped_expr`` over ``bare_label_predicate_expr`` (identical AST), so a +// dedicated ``label_predicate_expr`` rule here would be a pure derivation +// redundancy -- exactly the RPAR shift/reduce conflict. Omitted on purpose. return_expr: "*" -> projection_star - | label_predicate_expr | expr -label_predicate_expr: "(" NAME labels ")" -> grouped_label_predicate bare_label_predicate_expr: NAME labels -> bare_label_predicate alias: "AS"i NAME @@ -1385,7 +1387,7 @@ def projection_star(self, meta: Any, _items: Sequence[Any]) -> _ExpressionSlice: span = _span_from_meta(meta) return _ExpressionSlice(text="*", span=span) - def grouped_label_predicate(self, meta: Any, items: Sequence[Any]) -> _ExpressionSlice: + def bare_label_predicate(self, meta: Any, items: Sequence[Any]) -> _ExpressionSlice: if len(items) != 2: raise _to_syntax_error("Invalid label predicate expression", line=meta.line, column=meta.column) labels = cast(Sequence[str], items[1]) @@ -1394,8 +1396,6 @@ def grouped_label_predicate(self, meta: Any, items: Sequence[Any]) -> _Expressio span = _span_from_meta(meta) return _ExpressionSlice(text=self._slice(span), span=span) - bare_label_predicate = grouped_label_predicate - def _projection_clause( self, *, diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py index 9358c782df..5312986570 100644 --- a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -156,17 +156,16 @@ "MATCH (a)-[:R*..4]->(b) RETURN b", # open LOWER hop bound is not in the grammar ] -# Residual DERIVATION ambiguities: strings with exactly TWO parse trees that +# Residual DERIVATION ambiguity: strings with exactly TWO parse trees that # transform to ONE identical AST (harmless, but machine-surfaced so a grammar -# edit can't silently grow them). Each corresponds 1:1 to a pinned -# shift/reduce conflict: +# edit can't silently grow it). Maps 1:1 to the single pinned shift/reduce +# conflict: # - `[x IN xs]`: list_comprehension (no body) vs list_literal of an in_op -# element — the IN conflict; both mean the identity comprehension text. -# - `(n:Admin)` in RETURN: grouped_label_predicate vs grouped_expr over a -# bare_label_predicate — the RPAR conflict; both yield the same item. +# element — the IN conflict. Both mean the identity comprehension text. +# Inherent to Cypher's `[NAME IN ...` overlap; unremovable under LALR(1) +# without duplicating the whole expr hierarchy (zero semantic gain). RESIDUAL_DERIVATION_AMBIGUITY = [ "MATCH (n) RETURN [x IN n.tags] AS t", - "MATCH (n) RETURN (n:Admin) AS is_admin", ] @@ -221,16 +220,17 @@ def test_lalr_conflict_profile_pinned() -> None: - IN x1 at ``qualified_name : NAME``: inside ``[``, a NAME followed by IN is a comprehension head (shift) rather than an expr atom (reduce). The residual-witness test pins that both readings of the overlapping string - ``[x IN xs]`` produce the same AST. - - RPAR x1 at ``bare_label_predicate_expr : NAME labels``: ``(n:Admin)`` as - a return item is a grouped_label_predicate (shift) rather than a - grouped_expr over the bare label predicate (reduce); same AST either - way (residual-witness test). + ``[x IN xs]`` produce the same AST. This is the ONE conflict inherent to + Cypher's grammar (``[NAME IN ...`` is a comprehension or a list whose + first element is an ``in_op``); removing it needs a duplicated expr + hierarchy for zero semantic gain, so it stays — pinned and characterized. Historical note: DOT x5 (dotted-name redundancy) and WHERE x1 (WITH..WHERE attachment) were eliminated at grammar level — name-rooted dot chains derive only via ``qualified_name``, and WHERE is bundled into its - preceding clause — so they must never reappear. + preceding clause. RPAR x1 (``(n:Admin)`` label predicate) was eliminated + by dropping the redundant ``label_predicate_expr`` rule (it parses via + ``grouped_expr`` over ``bare_label_predicate_expr``). None must reappear. """ conflict_re = re.compile( r"(Shift/Reduce|Reduce/Reduce) conflict for terminal (\w+): \(resolving as (\w+)\)" @@ -247,7 +247,6 @@ def test_lalr_conflict_profile_pinned() -> None: ) assert profile == { ("Shift/Reduce", "IN", "shift"): 1, - ("Shift/Reduce", "RPAR", "shift"): 1, }, f"LALR conflict profile changed: {profile}" From 3e09bceb4108f539b2f8880dc4f4e82d3703aca9 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 13:24:17 -0700 Subject: [PATCH 10/12] test(gfql/cypher): pin the IN-conflict sibling gap (list-of-IN-booleans) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating whether the single residual IN shift/reduce could be closed surfaced a narrow latent divergence: `[x IN xs, y IN ys]` — a list literal whose FIRST element is a bare-name membership test with 2+ elements — is accepted by same-grammar Earley but rejected by LALR (the IN conflict resolves as shift toward a comprehension, so the parser commits at `[ x IN` and chokes on the comma). Deliberately NOT closing it: resolution needs an out-of-grammar bracket scan (scan for `|`/WHERE before the matching `]`) or a duplicated expr hierarchy, and the construct is non-executable in GFQL regardless — `[1 IN a, 2 IN b]` already raises downstream. LALR's parse-time rejection is cleaner than Earley's accept-then-cryptic-runtime-error. Parenthesize for a real list of booleans: `[(x IN xs), y IN ys]` parses; int-/dotted-first lists are unaffected. Pinned in DELIBERATE_LANGUAGE_FIXES so the full-repo differential stays honest if such a query ever enters the corpus. Test-only; no parser change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../gfql/cypher/test_grammar_invariants.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py index 5312986570..8de56c38ea 100644 --- a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -395,10 +395,11 @@ def test_lalr_and_earley_reject_the_same_inputs( parser_mod._parse_cypher_cached.cache_clear() -# The complete set of DELIBERATE language fixes vs the old Earley parser, -# found by full repo-corpus differentials (1800+ queries; everything else is -# accept/reject- and AST-identical). Each was an accept-by-accident with -# ill-defined semantics; all are now honest syntax errors. +# The complete set of DELIBERATE accept/reject differences the LALR parser +# introduces, found by full repo-corpus differentials (1800+ queries; +# everything else is accept/reject- and AST-identical). Two kinds: shapes the +# OLD Earley parser accepted by accident, and one shape same-grammar Earley +# accepts but LALR(1) cannot. All are now honest syntax errors. DELIBERATE_LANGUAGE_FIXES = [ # Earley's dynamic lexer re-lexed DISTINCT (absent from the NAME reserved # lookahead) as a NAME and accepted this as returning a *variable named* @@ -412,6 +413,17 @@ def test_lalr_and_earley_reject_the_same_inputs( "MATCH (n) WITH n WHERE n.x = 1 WHERE n.y = 2 RETURN n", # double post-WITH WHERE "MATCH (n) UNWIND n.tags AS t WHERE t = 'a' RETURN t", # WHERE after UNWIND (not openCypher) "GRAPH g1 = GRAPH { WHERE a.x = 1 MATCH (a) } USE g1 MATCH (n) RETURN n", # WHERE before MATCH + # Sibling of the single residual IN conflict (see RESIDUAL_DERIVATION_AMBIGUITY): + # a list literal whose FIRST element is a bare-name membership test with 2+ + # elements -- `[x IN xs, y IN ys]`. The IN conflict resolves as shift (toward + # a comprehension), so LALR commits at `[ x IN` and rejects the comma; + # same-grammar Earley accepts it. Deliberately NOT worth closing: resolving + # it needs an out-of-grammar bracket scan or a duplicated expr hierarchy, and + # the construct (a list of IN booleans) is non-executable in GFQL anyway + # (`[1 IN a, 2 IN b]` already raises downstream). Use parens if you mean a + # list of booleans: `[(x IN xs), y IN ys]` parses. int-/dotted-first lists + # (`[1 IN a, ...]`, `[n.x IN a, ...]`) are unaffected. + "RETURN [x IN xs, y IN ys] AS t", ] From c28ad6a8cac41c35ad2f5e6f9b20aa479fa6d53a Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 13:39:03 -0700 Subject: [PATCH 11/12] =?UTF-8?q?refactor(gfql/cypher):=20ZERO=20conflicts?= =?UTF-8?q?=20=E2=80=94=20grammar=20is=20now=20provably=20unambiguous=20LA?= =?UTF-8?q?LR(1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Your question ("if it's invalid, can we just eliminate it from the grammar?") was right. The last conflict (IN) existed because a list-literal element could be a bare top-level `IN` expression, overlapping list-comprehension syntax `[x IN xs ...`. Excluding top-level IN from list elements removes it: list_literal: "[" [list_element_list] "]" list_element: Net: **8 shift/reduce conflicts -> 0**. The grammar builds under Lark `strict=True` — a build-time PROOF of unambiguity (single derivation for every input). This is the "ambiguity is machine-checkable" property the design was aiming for, now literally true rather than approximated by a pinned conflict set. Bonus: fixes an inconsistency. `[1 IN a, 2]` and `[n.x IN a, y]` used to parse (then fail downstream with GFQLTypeError) while `[x IN xs, y]` was rejected — now the invalid "list of IN-booleans" is rejected UNIFORMLY at grammar level. A genuine list of membership booleans stays expressible with parens: `[(x IN xs), y]`. Tests: replaced the pinned-conflict-profile + residual-witness tests with test_grammar_has_zero_lalr_conflicts (dependency-free, always-on CI guard) and test_grammar_builds_under_strict_mode (skips without the optional interegular dep). test_semantic_ambiguity_zero now asserts exactly one derivation per query, no residual exceptions. The 3 list-of-IN-booleans forms pinned in DELIBERATE_LANGUAGE_FIXES. Verified on dgx-spark: full cypher suite 1681 passed / 0 failed; zero-conflict + strict build + full-repo differential all green. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- graphistry/compute/gfql/cypher/parser.py | 41 +++- .../gfql/cypher/test_grammar_invariants.py | 176 +++++++----------- 3 files changed, 105 insertions(+), 114 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cffc7a2c4..d0d0cbcbb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL `engine='polars'`/`'polars-gpu'` can now run off-engine analytic `call()`s (umap/hypergraph/compute_cugraph/…) — `call_mode='auto'` (default)**: a GFQL `call()` that invokes a Plottable-method analytic with **no native polars implementation** (and never will — `umap`, `hypergraph`, `compute_cugraph`, `compute_igraph`, `layout_*`, `collapse`, `get_topological_levels`, …) previously raised `NotImplementedError` under a polars engine, forcing users off polars for the whole pipeline. It now runs as a **mode-gated, warned modality switch**: `call_mode='auto'` (default) bridges the graph off-engine (`polars`→pandas, `polars-gpu`→cuDF **on-device**), runs the analytic, and coerces the result back to polars **losslessly via Arrow**, warning once per (process, function). `call_mode='strict'` keeps the honest `NotImplementedError` decline (for benchmark integrity / a hard memory ceiling). `polars-gpu` is **GPU-or-error**: it bridges to cuDF and declines rather than silently dropping a GPU analytic to host pandas. This is **deliberately narrower than CHAIN traversal/filter/row ops**, which stay parity-or-NIE (a bridge there would hide a missing impl and cheat a benchmark) — the split is mechanical (`is_row_pipeline_call`), and the chain and DAG surfaces bridge consistently. Configurable via `graphistry.compute.gfql.lazy.set_call_mode('auto'|'strict')` (Python) or `GFQL_POLARS_CALL_MODE` (env), resolving Python override > env > default('auto'), read live. Verified on dgx: `compute_cugraph` PageRank is byte-parity with the pandas oracle under both `polars` and `polars-gpu`. ### Changed -- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — and dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), dropping the LALR conflict profile from 8 shift/reduce to **1 shift/reduce, 0 reduce/reduce** — the single remaining conflict (`[x IN xs]`, inherent to Cypher's comprehension-vs-list overlap) mapped to a characterized, AST-neutral derivation ambiguity and pinned as a witness. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) the conflict profile is pinned so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is ZERO — every Earley derivation of every corpus query transforms to the same AST, no exceptions — with the two residual binary derivation ambiguities pinned as witnesses; (3) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and exactly five deliberate language fixes, all accept-by-accident shapes with ill-defined semantics, now syntax errors and pinned as tests: `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, and WHERE before MATCH in a graph constructor. Full cypher suite (1,673 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. +- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), and excluding a top-level `IN` from list-literal elements (`[x IN xs ...` is comprehension syntax; allowing a bare `IN` list element made it overlap). Net: the LALR conflict profile goes from 8 shift/reduce to **ZERO conflicts** — the grammar is now **provably unambiguous LALR(1)** and builds under Lark's `strict=True` (a build-time proof, machine-checked in CI as zero conflicts, plus a strict-mode build test where the optional `interegular` dep is present). Every input has a single derivation. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) the conflict profile is pinned so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is ZERO — every Earley derivation of every corpus query transforms to the same AST, no exceptions — with the two residual binary derivation ambiguities pinned as witnesses; (3) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and exactly five deliberate language fixes, all accept-by-accident shapes with ill-defined semantics, now syntax errors and pinned as tests: `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, and WHERE before MATCH in a graph constructor. Full cypher suite (1,673 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. ### Fixed - **CI: `compute_networkx` HITS test was scipy-version flaky**: `test_compute_networkx_hits_outputs_hubs_and_authorities` used a pure directed 3-cycle (`a→b→c→a`), whose adjacency is a permutation matrix (all singular values equal). networkx HITS computes scores via scipy `svds(k=1)`, which then returns an arbitrary vector from that fully-degenerate singular space; when its components sum to ~0, HITS's `h /= h.sum()` normalization blows up to `inf`/`nan`. Which vector `svds` returns is scipy/LAPACK-version dependent, so the test passed on the pinned scipy but failed on the `nx-upper-scipy` CI profile's newer scipy — a latent flake, not a code regression. Switched to a graph with a well-defined hub/authority structure (`a→b`, `a→c`, `b→c`), whose dominant singular vector is unique (Perron-Frobenius) and therefore version-stable. Test-only change. diff --git a/graphistry/compute/gfql/cypher/parser.py b/graphistry/compute/gfql/cypher/parser.py index 8d6f0cbdd3..53138e3589 100644 --- a/graphistry/compute/gfql/cypher/parser.py +++ b/graphistry/compute/gfql/cypher/parser.py @@ -56,12 +56,12 @@ # FAILS with the new rule's name until you do — you cannot land a rule # with no coverage. # 3. Run test_grammar_invariants.py. The machine checks that guard you: -# - conflict profile is PINNED (0 reduce/reduce always; the exact -# shift/reduce set is asserted) — a new ambiguity fails the build; +# - the grammar has ZERO LALR conflicts and builds under strict=True — +# provably unambiguous; a new ambiguity introduces a conflict and +# fails the build. Fix it IN THE GRAMMAR (as WITH..WHERE bundling, +# the dotted-name split, and no-top-level-IN list elements did) — +# never by resolving a conflict in Python; # - semantic ambiguity is ZERO (every Earley derivation -> same AST); -# a new binary-but-AST-neutral ambiguity must be added to -# RESIDUAL_DERIVATION_AMBIGUITY with a justification, anything worse -# fails hard; # - LALR == Earley over the corpus + full-repo scrape (AST-identical). # 4. If the edit deliberately changes accept/reject, pin it in # DELIBERATE_LANGUAGE_FIXES. Otherwise a language change fails the @@ -287,8 +287,35 @@ case_when: "WHEN"i expr "THEN"i expr case_else: "ELSE"i expr -list_literal: "[" [expr_list] "]" -expr_list: expr ("," expr)* +// A list literal's elements are expressions with NO top-level ``IN`` operator. +// Rationale: ``[x IN xs ...`` is list-comprehension syntax; if a list element +// could also be a bare ``in_op`` (``[x IN xs, y]``), the two overlap and the +// grammar is ambiguous at ``[ NAME . IN`` (this was the last shift/reduce +// conflict). Excluding top-level ``IN`` from list elements makes the grammar +// provably unambiguous (builds under ``strict=True``) AND rejects the invalid +// "list of IN-booleans" uniformly (``[1 IN a, 2]``, ``[n.x IN a, y]`` too, not +// just the bare-name case) — a construct GFQL cannot execute anyway. A genuine +// list of membership booleans is still expressible with parens: ``[(x IN xs)]``. +// ``list_element`` mirrors the ``expr`` hierarchy exactly, minus the ``in_op`` +// alternative at the comparable level; ``additive`` and below are shared. +list_literal: "[" [list_element_list] "]" +list_element_list: list_element ("," list_element)* +?list_element: or_expr_no_in +?or_expr_no_in: xor_expr_no_in | or_expr_no_in "OR"i xor_expr_no_in -> or_op +?xor_expr_no_in: and_expr_no_in | xor_expr_no_in "XOR"i and_expr_no_in -> xor_op +?and_expr_no_in: not_expr_no_in | and_expr_no_in "AND"i not_expr_no_in -> and_op +?not_expr_no_in: "NOT"i not_expr_no_in -> not_op + | predicate_no_in +?predicate_no_in: comparable_no_in + | comparable_no_in COMP_OP comparable_no_in COMP_OP comparable_no_in -> chained_cmp + | comparable_no_in COMP_OP comparable_no_in -> cmp_op + | bare_label_predicate_expr +?comparable_no_in: additive + | additive "IS"i "NULL"i -> expr_is_null + | additive "IS"i "NOT"i "NULL"i -> expr_is_not_null + | additive "CONTAINS"i additive -> contains_op + | additive "STARTS"i "WITH"i additive -> starts_with_op + | additive "ENDS"i "WITH"i additive -> ends_with_op map_literal: "{" [map_entries] "}" map_entries: map_entry ("," map_entry)* diff --git a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py index 8de56c38ea..05c1049c57 100644 --- a/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py +++ b/graphistry/tests/compute/gfql/cypher/test_grammar_invariants.py @@ -2,23 +2,24 @@ The parser's correctness argument is grammar-level, not implementation-level: -1. **Conflict profile is pinned** (``test_lalr_conflict_profile_pinned``): the - unified grammar builds under LALR(1) with a fixed, reviewed set of - shift/reduce conflicts and ZERO reduce/reduce conflicts. Reduce/reduce is - the line that must never be crossed: it means two rules derive the same - string (genuine ambiguity), which LALR resolves arbitrarily. A grammar edit - that changes the profile fails this test and forces a review. +1. **The grammar is PROVABLY UNAMBIGUOUS LALR(1)** — zero conflicts. + ``test_grammar_has_zero_lalr_conflicts`` asserts the LALR build emits no + shift/reduce and no reduce/reduce conflicts (dependency-free), and + ``test_grammar_builds_under_strict_mode`` confirms it builds under Lark's + ``strict=True`` (also checks lexer-terminal collisions; needs the optional + ``interegular`` dep, skipped if absent). This is the strongest form of the + "ambiguity is machine-checkable" invariant: a single derivation for every + input, verified at build time. A grammar edit that introduces ANY conflict + fails here. 2. **Semantic ambiguity is ZERO** (``test_semantic_ambiguity_zero``): expanding EVERY Earley derivation of every corpus query - (``ambiguity='explicit'`` + ``CollapseAmbiguities``), all derivations - transform to the SAME AST, unconditionally. Two residual DERIVATION - ambiguities exist (each exactly binary, each AST-identical, matching the - two pinned shift/reduce conflicts) and are machine-surfaced as pinned - witnesses in ``test_residual_derivation_ambiguity_witnesses``. The former - WITH..WHERE attachment ambiguity was ELIMINATED at grammar level (WHERE is - bundled into its preceding clause), as was the dotted-name redundancy - (name-rooted dot chains derive only via ``qualified_name``). + (``ambiguity='explicit'`` + ``CollapseAmbiguities``), each query has + exactly one derivation and they all transform to the SAME AST — a + redundant but independent confirmation of (1) at the AST level. The + WITH..WHERE attachment ambiguity, the dotted-name redundancy, the + ``(n:Admin)`` label predicate, and the ``[x IN xs]`` comprehension overlap + were all eliminated at grammar level (see the grammar's inline comments). 3. **Parser-choice neutrality** (``test_lalr_ast_equals_earley_ast_differential``): running the production pipeline with an Earley parser over the same grammar @@ -33,7 +34,7 @@ import logging import re -from typing import Any, Dict, List, Tuple +from typing import Any, List import pytest @@ -156,18 +157,6 @@ "MATCH (a)-[:R*..4]->(b) RETURN b", # open LOWER hop bound is not in the grammar ] -# Residual DERIVATION ambiguity: strings with exactly TWO parse trees that -# transform to ONE identical AST (harmless, but machine-surfaced so a grammar -# edit can't silently grow it). Maps 1:1 to the single pinned shift/reduce -# conflict: -# - `[x IN xs]`: list_comprehension (no body) vs list_literal of an in_op -# element — the IN conflict. Both mean the identity comprehension text. -# Inherent to Cypher's `[NAME IN ...` overlap; unremovable under LALR(1) -# without duplicating the whole expr hierarchy (zero semantic gain). -RESIDUAL_DERIVATION_AMBIGUITY = [ - "MATCH (n) RETURN [x IN n.tags] AS t", -] - def _fresh_lalr_with_log() -> List[logging.LogRecord]: """Build a fresh (uncached) LALR parser with debug on, capturing lark's @@ -209,54 +198,57 @@ def _earley_parser(ambiguity: str = "resolve") -> Any: ) -# --- 1. Conflict profile ------------------------------------------------------- - -def test_lalr_conflict_profile_pinned() -> None: - """The grammar's LALR conflict profile is pinned: 0 reduce/reduce (genuine - ambiguity — never acceptable), and exactly these shift/reduce conflicts, - all resolved as shift. A grammar edit that changes this set must update - the pin AND justify each new conflict here: - - - IN x1 at ``qualified_name : NAME``: inside ``[``, a NAME followed by IN - is a comprehension head (shift) rather than an expr atom (reduce). The - residual-witness test pins that both readings of the overlapping string - ``[x IN xs]`` produce the same AST. This is the ONE conflict inherent to - Cypher's grammar (``[NAME IN ...`` is a comprehension or a list whose - first element is an ``in_op``); removing it needs a duplicated expr - hierarchy for zero semantic gain, so it stays — pinned and characterized. - - Historical note: DOT x5 (dotted-name redundancy) and WHERE x1 (WITH..WHERE - attachment) were eliminated at grammar level — name-rooted dot chains - derive only via ``qualified_name``, and WHERE is bundled into its - preceding clause. RPAR x1 (``(n:Admin)`` label predicate) was eliminated - by dropping the redundant ``label_predicate_expr`` rule (it parses via - ``grouped_expr`` over ``bare_label_predicate_expr``). None must reappear. - """ +# --- 1. Provable unambiguity: zero conflicts ----------------------------------- + +def test_grammar_has_zero_lalr_conflicts() -> None: + """The grammar is unambiguous LALR(1): the build emits ZERO conflicts — + no shift/reduce, no reduce/reduce. Every input has a single derivation. + + Dependency-free (parses lark's debug log). This is the machine-checkable + "ambiguity is decidable" invariant the whole design is built around: any + grammar edit that introduces an ambiguity produces a conflict and fails + here, naming the terminal. Historically the grammar had 8 shift/reduce + conflicts; each was eliminated at grammar level (WITH..WHERE bundling, + dotted-name split via ``qualified_name``, dropping the redundant + ``label_predicate_expr``, and excluding top-level ``IN`` from list + elements) — see the grammar's inline comments.""" conflict_re = re.compile( - r"(Shift/Reduce|Reduce/Reduce) conflict for terminal (\w+): \(resolving as (\w+)\)" + r"(Shift/Reduce|Reduce/Reduce) conflict for terminal (\w+)" ) - profile: Dict[Tuple[str, str, str], int] = {} - for record in _fresh_lalr_with_log(): - m = conflict_re.match(record.getMessage()) - if m: - key = (m.group(1), m.group(2), m.group(3)) - profile[key] = profile.get(key, 0) + 1 - - assert not any(kind == "Reduce/Reduce" for kind, _, _ in profile), ( - f"Reduce/reduce conflict introduced (genuine grammar ambiguity): {profile}" + conflicts = [ + m.group(0) + for record in _fresh_lalr_with_log() + if (m := conflict_re.match(record.getMessage())) + ] + assert conflicts == [], f"grammar is no longer conflict-free: {conflicts}" + + +def test_grammar_builds_under_strict_mode() -> None: + """The grammar builds under Lark ``strict=True`` — a build-time PROOF of + unambiguity that also checks lexer-terminal collisions. Stronger than the + conflict-log check, but needs the optional ``interegular`` dependency, so + it skips where that isn't installed (the log check above is the always-on + guard).""" + pytest.importorskip("interegular") + lark.Lark( + parser_mod._GRAMMAR, + start="start", + parser="lalr", + strict=True, + maybe_placeholders=False, + propagate_positions=True, ) - assert profile == { - ("Shift/Reduce", "IN", "shift"): 1, - }, f"LALR conflict profile changed: {profile}" # --- 2. Semantic ambiguity ----------------------------------------------------- def test_semantic_ambiguity_zero() -> None: - """Every Earley derivation of every corpus query transforms to the SAME - AST — semantic ambiguity is zero, no exceptions. (The former WITH..WHERE - attachment ambiguity was eliminated by bundling WHERE into its preceding - clause in the grammar.) + """Every corpus query has EXACTLY ONE Earley derivation, and it transforms + to a single AST — semantic ambiguity is zero, no exceptions and no residual + witnesses (an independent AST-level confirmation of the zero-conflict + invariant). Every ambiguity the grammar once had — WITH..WHERE attachment, + dotted-name redundancy, the ``(n:Admin)`` label predicate, and the + ``[x IN xs]`` comprehension overlap — was eliminated at grammar level. Note: ``CollapseAmbiguities`` rebuilds trees without ``meta`` positions, so derivation ASTs have degraded spans/text and CANNOT be compared against the @@ -271,40 +263,14 @@ def test_semantic_ambiguity_zero() -> None: asts = { repr(parser_mod._build_transformer(query).transform(t)) for t in trees } - if len(asts) != 1: + if len(trees) != 1 or len(asts) != 1: unexpected.append(f"{query!r}: {len(trees)} derivations, {len(asts)} ASTs") assert not unexpected, ( - "Semantic ambiguity introduced (derivations disagree on AST):\n" + "Derivation ambiguity introduced (expected exactly 1 derivation each):\n" + "\n".join(unexpected) ) -def test_residual_derivation_ambiguity_witnesses() -> None: - """Machine-surfaced witnesses for the ONLY residual derivation - ambiguities: each is exactly binary and AST-identical (see - RESIDUAL_DERIVATION_AMBIGUITY for the 1:1 mapping to the two pinned - shift/reduce conflicts). Every other corpus query has exactly ONE - derivation. A grammar edit that grows either count fails here and must - characterize the new ambiguity before landing.""" - from lark.visitors import CollapseAmbiguities - - explicit = _earley_parser(ambiguity="explicit") - for query in RESIDUAL_DERIVATION_AMBIGUITY: - trees = CollapseAmbiguities().transform(explicit.parse(query)) - asts = { - repr(parser_mod._build_transformer(query).transform(t)) for t in trees - } - assert len(trees) == 2, f"expected binary derivation ambiguity: {query!r}" - assert len(asts) == 1, f"derivations must agree on the AST: {query!r}" - for query in DIFFERENTIAL_CORPUS: - if query in RESIDUAL_DERIVATION_AMBIGUITY: - continue - trees = CollapseAmbiguities().transform(explicit.parse(query)) - assert len(trees) == 1, ( - f"unexpected derivation ambiguity ({len(trees)} trees): {query!r}" - ) - - # --- Rule coverage: the corpus must exercise EVERY grammar rule ------------------ def test_every_grammar_rule_is_exercised_by_the_corpus() -> None: @@ -413,17 +379,15 @@ def test_lalr_and_earley_reject_the_same_inputs( "MATCH (n) WITH n WHERE n.x = 1 WHERE n.y = 2 RETURN n", # double post-WITH WHERE "MATCH (n) UNWIND n.tags AS t WHERE t = 'a' RETURN t", # WHERE after UNWIND (not openCypher) "GRAPH g1 = GRAPH { WHERE a.x = 1 MATCH (a) } USE g1 MATCH (n) RETURN n", # WHERE before MATCH - # Sibling of the single residual IN conflict (see RESIDUAL_DERIVATION_AMBIGUITY): - # a list literal whose FIRST element is a bare-name membership test with 2+ - # elements -- `[x IN xs, y IN ys]`. The IN conflict resolves as shift (toward - # a comprehension), so LALR commits at `[ x IN` and rejects the comma; - # same-grammar Earley accepts it. Deliberately NOT worth closing: resolving - # it needs an out-of-grammar bracket scan or a duplicated expr hierarchy, and - # the construct (a list of IN booleans) is non-executable in GFQL anyway - # (`[1 IN a, 2 IN b]` already raises downstream). Use parens if you mean a - # list of booleans: `[(x IN xs), y IN ys]` parses. int-/dotted-first lists - # (`[1 IN a, ...]`, `[n.x IN a, ...]`) are unaffected. - "RETURN [x IN xs, y IN ys] AS t", + # A list literal element cannot be a top-level `IN` expression -- that + # syntax is reserved for list comprehensions (`[x IN xs | ...]`), and + # allowing both made the grammar ambiguous. So a "list of IN-booleans" is + # rejected UNIFORMLY at grammar level -- the old parser accepted some of + # these (then failed downstream: `[1 IN a, 2 IN b]` raises a GFQLTypeError). + # Parenthesize for a genuine list of membership booleans: `[(x IN xs), y]`. + "RETURN [x IN xs, y IN ys] AS t", # bare-name first + "RETURN [1 IN a, 2 IN b] AS t", # int first (old parser accepted this) + "RETURN [n.x IN a, y] AS t", # dotted first (old parser accepted this) ] From 136ba28d1b0d55ade8380c397a2f07ff2e118b94 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 5 Jul 2026 13:45:36 -0700 Subject: [PATCH 12/12] =?UTF-8?q?docs(changelog):=20accurate=20parser=20en?= =?UTF-8?q?try=20=E2=80=94=20zero=20conflicts,=20full=20fix=20list,=201681?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correct stale bits in the Earley->LALR entry: conflict profile is now ZERO conflicts + strict-build (not a pinned nonzero set), no residual witnesses, the full deliberate-fix list including the list-of-IN-booleans, the lift correctness boundary, 1681-test count, and the #1683 3VL cross-reference. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0d0cbcbb6..024c070559 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL `engine='polars'`/`'polars-gpu'` can now run off-engine analytic `call()`s (umap/hypergraph/compute_cugraph/…) — `call_mode='auto'` (default)**: a GFQL `call()` that invokes a Plottable-method analytic with **no native polars implementation** (and never will — `umap`, `hypergraph`, `compute_cugraph`, `compute_igraph`, `layout_*`, `collapse`, `get_topological_levels`, …) previously raised `NotImplementedError` under a polars engine, forcing users off polars for the whole pipeline. It now runs as a **mode-gated, warned modality switch**: `call_mode='auto'` (default) bridges the graph off-engine (`polars`→pandas, `polars-gpu`→cuDF **on-device**), runs the analytic, and coerces the result back to polars **losslessly via Arrow**, warning once per (process, function). `call_mode='strict'` keeps the honest `NotImplementedError` decline (for benchmark integrity / a hard memory ceiling). `polars-gpu` is **GPU-or-error**: it bridges to cuDF and declines rather than silently dropping a GPU analytic to host pandas. This is **deliberately narrower than CHAIN traversal/filter/row ops**, which stay parity-or-NIE (a bridge there would hide a missing impl and cheat a benchmark) — the split is mechanical (`is_row_pipeline_call`), and the chain and DAG surfaces bridge consistently. Configurable via `graphistry.compute.gfql.lazy.set_call_mode('auto'|'strict')` (Python) or `GFQL_POLARS_CALL_MODE` (env), resolving Python override > env > default('auto'), read live. Verified on dgx: `compute_cugraph` PageRank is byte-parity with the pandas oracle under both `polars` and `polars-gpu`. ### Changed -- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), and excluding a top-level `IN` from list-literal elements (`[x IN xs ...` is comprehension syntax; allowing a bare `IN` list element made it overlap). Net: the LALR conflict profile goes from 8 shift/reduce to **ZERO conflicts** — the grammar is now **provably unambiguous LALR(1)** and builds under Lark's `strict=True` (a build-time proof, machine-checked in CI as zero conflicts, plus a strict-mode build test where the optional `interegular` dep is present). Every input has a single derivation. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) the conflict profile is pinned so grammar edits can't silently introduce ambiguity; (2) semantic ambiguity is ZERO — every Earley derivation of every corpus query transforms to the same AST, no exceptions — with the two residual binary derivation ambiguities pinned as witnesses; (3) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and exactly five deliberate language fixes, all accept-by-accident shapes with ill-defined semantics, now syntax errors and pinned as tests: `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, and WHERE before MATCH in a graph constructor. Full cypher suite (1,673 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. +- **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), and excluding a top-level `IN` from list-literal elements (`[x IN xs ...` is comprehension syntax; allowing a bare `IN` list element made it overlap). Net: the LALR conflict profile goes from 8 shift/reduce to **ZERO conflicts** — the grammar is now **provably unambiguous LALR(1)** and builds under Lark's `strict=True` (a build-time proof, machine-checked in CI as zero conflicts, plus a strict-mode build test where the optional `interegular` dep is present). Every input has a single derivation. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) **zero LALR conflicts** (dependency-free, always-on in CI) plus a `strict=True` build test (skipped where the optional `interegular` dep is absent) — a grammar edit that introduces any ambiguity fails CI; (2) semantic ambiguity is ZERO — every corpus query has exactly one Earley derivation and it transforms to a single AST, no exceptions; (3) a rule-coverage gate (`test_every_grammar_rule_is_exercised_by_the_corpus`) forces every new grammar rule through the invariants; (4) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and a small set of deliberate language fixes — accept-by-accident shapes with ill-defined semantics, now honest syntax errors and pinned as tests (`DELIBERATE_LANGUAGE_FIXES`): `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, WHERE before MATCH in a graph constructor, and the invalid "list of IN-booleans" (`[x IN xs, y]`; use `[(x IN xs), y]` for a genuine bool list). The lift stays an internal optimization: only a flat `AND` chain over present columns lifts to `filter_dict`; parens / OR / XOR / NOT and any absent-column case stay on `where_rows` (a correctness boundary, since `where_rows` treats an absent property as null while `filter_dict` would raise). Full cypher suite (1,681 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. A pre-existing `<>`-over-null 3VL divergence between the two execution paths (surfaced by #1653's metamorphic test) is tracked separately in #1683. ### Fixed - **CI: `compute_networkx` HITS test was scipy-version flaky**: `test_compute_networkx_hits_outputs_hubs_and_authorities` used a pure directed 3-cycle (`a→b→c→a`), whose adjacency is a permutation matrix (all singular values equal). networkx HITS computes scores via scipy `svds(k=1)`, which then returns an arbitrary vector from that fully-degenerate singular space; when its components sum to ~0, HITS's `h /= h.sum()` normalization blows up to `inf`/`nan`. Which vector `svds` returns is scipy/LAPACK-version dependent, so the test passed on the pinned scipy but failed on the `nx-upper-scipy` CI profile's newer scipy — a latent flake, not a code regression. Switched to a graph with a well-defined hub/authority structure (`a→b`, `a→c`, `b→c`), whose dominant singular vector is unique (Perron-Frobenius) and therefore version-stable. Test-only change.