Repository navigation
fix(gfql/binder): replace regex WHERE label narrowing with AST-based analysis (#1125) - #1193
Merged
Merged
Conversation
…analysis (#1125) The binder's _apply_where_label_narrowing() previously fell back to regex (_WHERE_LABEL_RE / _WHERE_NON_CONJUNCTIVE_RE) to extract label narrowing from raw WHERE expression text. This was fragile: a quoted string such as WHERE n.name = 'n:Admin' could produce false-positive label assignments, and the OR/NOT/XOR guard was text-level and sensitive to unusual expressions. Root cause: the Lark grammar resolves the ambiguity between `where_predicates` and `expr` in favour of `expr` for AND-joined label predicates like `WHERE n:Admin AND n:Active`, so they reached generic_where_clause as raw text rather than as structured AST. Fix (two-part): - parser.py: extend generic_where_clause to recognise AND-joined bare label predicates via _BARE_LABEL_PREDICATE_RE and produce structured WhereClause.predicates, eliminating the need for any regex in the binder. Non-label parts (property comparisons, OR/NOT/XOR) fall through to the raw-expr path and are conservatively not narrowed. - binder.py: remove _WHERE_LABEL_RE, _WHERE_NON_CONJUNCTIVE_RE, and the regex fallback block; _apply_where_label_narrowing now derives label narrowing exclusively from WhereClause.predicates (AST). New tests: triple AND conjunction, XOR (no narrowing), quoted label pattern (no false positive), string-with-colon (no false positive), nested OR-AND expression (no narrowing). All 1416 GFQL tests pass. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
… Lark ambiguity Fixes mypy [var-annotated] error on `predicates = []` in generic_where_clause. Adds comment explaining why AND-joined label predicates are handled here rather than in the structured where_predicates grammar rule (Lark ambiguity resolution prefers the generic expr path). Adds multi-alias narrowing test: WHERE n:Admin AND m:User narrows both aliases. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…ERE clauses Verifies that generic_where_clause produces structured WhereClause.predicates (not raw expr) for single and AND-joined bare label predicates, and that XOR/non-label expressions correctly land in WhereClause.expr. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…ion tests Wave 3 review findings: - DRY: Replace _WHERE_AND_SPLIT_RE with existing quote/bracket/paren/backtick-aware _split_top_level_and_terms helper (parser.py:477). Removes naive duplicate splitter. - Document the fullmatch security invariant that protects against string-literal false-matches (motivation for issue #1125). - Add regression tests: mixed label + property predicate (locks all-or-nothing narrowing invariant); multi-label-per-alias in AND; lowercase `and`. - Tighten overly permissive assertion in test_parse_where_non_label_expression_produces_raw_expr. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…ntry Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
This was referenced Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1125
Replaces the fragile regex-based label narrowing fallback in
_apply_where_label_narrowingwith purely AST-based analysis.binder.py: Removed_WHERE_LABEL_RE,_WHERE_NON_CONJUNCTIVE_REconstants and the 4-line regex fallback block from_apply_where_label_narrowing(). The function now derives label narrowing exclusively fromWhereClause.predicates(structured AST).parser.py: Added_WHERE_AND_SPLIT_RE. Extendedgeneric_where_clauseto recognize AND-joined bare label predicates (n:Admin AND n:Active) and produce structuredWherePredicate(op="has_labels")entries. Non-label parts cause conservative fallback to rawwhere.expr(no narrowing).test_binder.py: 5 targeted regression tests covering triple-AND conjunction, XOR, quoted strings, colon-containing strings, and nested OR — verifying no false positives and correct narrowing.Test plan
pytest graphistry/tests/compute/gfql/cypher/test_binder.py— all 39 tests pass including 5 newpytest graphistry/tests/compute/gfql/ -q— full GFQL suite cleanruff check— no lint errors🤖 Generated with Claude Code