Skip to content

chore(gfql): shrink predicate ast helpers - #1603

Merged
lmeyerov merged 1 commit into
masterfrom
issue-1058-predicates-ast-shrink
May 22, 2026
Merged

lmeyerov merged 1 commit into
masterfrom
issue-1058-predicates-ast-shrink

Conversation

@lmeyerov

Copy link
Copy Markdown
Contributor

Summary

Closes part of #1058.

GFQL implementation shrink in the requested paired audit surface:

  • DRY repeated scalar/temporal comparison predicate dispatch in graphistry/compute/predicates/comparison.py
  • DRY regex string predicate constructor/call/validation scaffolding shared by match() and fullmatch()
  • DRY AST predicate-aware filter-dict serialization shared by node and edge wire output

No public predicate helper names, predicate JSON wire shapes, route names, compiler-plan dispatch, structured errors, schema hooks, remote hooks, or validate='autofix' semantics changed.

Audit Notes

  • graphistry/compute/ast.py and graphistry/compute/predicates/ do not contain Arrow conversion or validate='autofix' paths; feat(arrow): add opt-in mixed type coercion #1591/fix(arrow): preserve nulls in validate autofix #1598 did not create dead Arrow code in these files.
  • No is_not_in implementation exists in the target files, so there is no is_in / is_not_in duplicate body to collapse here.
  • numeric.py and comparison.py overlap by predicate name, but both are reachable/public with different ownership: numeric.py remains used by direct AST/filter validation paths, while comparison.py remains used by Cypher lowering and temporal/string comparison support. This PR does not collapse those public modules.

LOC Buckets

  • Production: +68 / -169, net -101
    • graphistry/compute/ast.py: +12 / -20, net -8
    • graphistry/compute/predicates/comparison.py: +29 / -72, net -43
    • graphistry/compute/predicates/str.py: +27 / -77, net -50
  • Tests / fixtures / baselines: +0 / -0, net 0
  • Comments / docs / changelog: +1 / -0, net +1

Compiler-plan surface touched: no. The AST edit is a private serialization helper extraction that preserves existing node/edge wire JSON.

Validation

  • python3 -m pytest -q graphistry/tests/compute/predicates/test_comparison_strings.py graphistry/tests/compute/predicates/test_comparison_conformance.py graphistry/tests/compute/predicates/test_str.py graphistry/tests/compute/predicates/test_numeric.py graphistry/tests/test_compute_chain.py
    • 161 passed, 96 skipped
  • ./bin/ruff.sh graphistry/compute/ast.py graphistry/compute/predicates/comparison.py graphistry/compute/predicates/str.py
    • passed
  • ./bin/typecheck.sh graphistry/compute/ast.py graphistry/compute/predicates/comparison.py graphistry/compute/predicates/str.py
    • passed
  • python3 -m pytest -q graphistry/tests/test_schema_artifacts.py graphistry/tests/test_viz_settings.py
    • 11 passed
  • python3 -m graphistry.devschemas.export --check
    • passed, no schema drift
  • git diff --check
    • passed
  • Local broader predicate/GFQL run:
    • 305 passed, 96 skipped
    • one cuDF-only failure at cudf.Series(...) with cudaErrorNoDevice because this host has cuDF installed without a CUDA device
  • DGX RAPIDS 26.02:
    • RAPIDS_VERSION=26.02 PROFILE=gfql TEST_FILES="graphistry/tests/compute/predicates/test_str.py graphistry/tests/compute/predicates/test_comparison_conformance.py graphistry/tests/compute/predicates/test_temporal_values.py" docker/test-rapids-official-local.sh
    • 181 passed, 4 skipped
  • DGX RAPIDS 25.02:
    • same focused predicate target set
    • 181 passed, 4 skipped, with the known 25.02 driver-version warning from the RAPIDS image

Coordination

@lmeyerov
lmeyerov merged commit 14e1ad2 into master May 22, 2026
66 checks passed
@lmeyerov
lmeyerov deleted the issue-1058-predicates-ast-shrink branch May 22, 2026 02:39
lmeyerov added a commit that referenced this pull request Aug 19, 2026
_filter_dict_to_json dropped every entry whose value was None, so a filter the
local engine evaluates as "matches nothing" serialized to no filter at all --
"matches everything". n({'x': None}).to_json() emitted {'filter_dict': {}}.

The guard dates to the original 2023 chain-serialization commit (ece0924) and
was mechanically lifted into the helper by #1603; no caller depends on the drop
and no test asserted it. maybe_filter_dict_from_json already preserved a JSON
null verbatim, so the asymmetry was entirely on the write side and no from_json
change is needed.

Blast radius is not remote-only: serialize_binding_ops is an in-process
to_json/from_json launder used by the Cypher connected-pattern and cartesian
MATCH lowering, so MATCH (a {x: null})-[r]->(b) returned 2 rows and
MATCH (a {x: null}), (b) returned 9 rows on a 3-node graph where both must
return 0, with no server involved.

Also refreshes a now-stale docstring bullet in cypher/lowering.py that cited the
drop as the reason _connected_join_pushable_value refuses to push None; the
guard itself is unchanged and still correct.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AjbKuKheqDu78oapRT5AYm
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.

1 participant