Skip to content

fix(gfql/binder): replace regex WHERE label narrowing with AST-based analysis (#1125) - #1193

Merged
lmeyerov merged 5 commits into
masterfrom
issue-1125-ast-label-narrowing
Apr 23, 2026
Merged

lmeyerov merged 5 commits into
masterfrom
issue-1125-ast-label-narrowing

Conversation

@lmeyerov

Copy link
Copy Markdown
Contributor

Summary

Closes #1125

Replaces the fragile regex-based label narrowing fallback in _apply_where_label_narrowing with purely AST-based analysis.

  • binder.py: Removed _WHERE_LABEL_RE, _WHERE_NON_CONJUNCTIVE_RE constants and the 4-line regex fallback block from _apply_where_label_narrowing(). The function now derives label narrowing exclusively from WhereClause.predicates (structured AST).
  • parser.py: Added _WHERE_AND_SPLIT_RE. Extended generic_where_clause to recognize AND-joined bare label predicates (n:Admin AND n:Active) and produce structured WherePredicate(op="has_labels") entries. Non-label parts cause conservative fallback to raw where.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 new
  • pytest graphistry/tests/compute/gfql/ -q — full GFQL suite clean
  • ruff check — no lint errors

🤖 Generated with Claude Code

lmeyerov and others added 4 commits April 22, 2026 19:18
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Binder follow-up: replace regex WHERE label narrowing with AST-based analysis

1 participant