Skip to content

feat(gfql/m2): LogicalPlanner skeleton + op_id/IdGen contract#1129

Merged
lmeyerov merged 10 commits intomasterfrom
m2-pr1-logical-planner-skeleton
Apr 18, 2026
Merged

feat(gfql/m2): LogicalPlanner skeleton + op_id/IdGen contract#1129
lmeyerov merged 10 commits intomasterfrom
m2-pr1-logical-planner-skeleton

Conversation

@lmeyerov
Copy link
Copy Markdown
Contributor

Summary

  • add graphistry.compute.gfql.logical_planner with initial LogicalPlanner and monotonic IdGen
  • implement minimal pure plan(bound_ir, ctx) -> LogicalPlan skeleton for MATCH/WHERE/WITH/RETURN/UNWIND plus deterministic fallback
  • add focused planner contract tests (importability/root type, unique op_id, determinism/purity, typed schema propagation, empty fallback, UNWIND propagation, IdGen monotonicity)

Validation

  • uv run ruff check graphistry/compute/gfql/logical_planner.py graphistry/tests/compute/gfql/test_logical_planner.py
  • python -m pytest -q graphistry/tests/compute/gfql/test_logical_planner.py graphistry/tests/compute/gfql/test_ir_compilation.py graphistry/tests/compute/gfql/test_ir_m0b_conformance.py

Closes #1126

@lmeyerov
Copy link
Copy Markdown
Contributor Author

M2-PR1 audit update (spec-conformance review)

Findings blocking merge-readiness for M2 skeleton quality:

  1. UNWIND expression contract mismatch (high)
  • logical_planner.py reads BoundQueryPart.metadata["expression"] for Unwind.list_expr.
  • Current binder emits UNWIND expression in BoundQueryPart.predicates[0].expression and metadata does not carry expression text.
  • Result: real binder output can produce empty unwind expressions in plan.
  1. Multi-MATCH state overwrite (medium)
  • Planner resets current to a fresh NodeScan on each MATCH/OPTIONAL MATCH, so prior pipeline context is dropped for multi-part shapes.
  • If this is intentionally deferred, it should be explicit guardrail/error, not silent behavior.
  1. OPTIONAL MATCH semantics not represented (medium)
  • OPTIONAL is currently routed same as MATCH with NodeScan only; no optional-arm semantics in planned form.
  1. Tests are mostly synthetic for BoundQueryPart contracts (medium)
  • Need at least one binder->planner integration test to validate real part payloads (especially UNWIND expression transport).

I wrote the full local audit notes in:

  • /home/lmeyerov/Work/graphc/agents/plans/compiler-refactor/audit/pr1129-findings-2026-04-11.md

@lmeyerov lmeyerov force-pushed the m2-pr1-logical-planner-skeleton branch from f827c01 to 2108a01 Compare April 18, 2026 05:58
@lmeyerov lmeyerov merged commit 06a0854 into master Apr 18, 2026
119 checks passed
@lmeyerov lmeyerov deleted the m2-pr1-logical-planner-skeleton branch April 18, 2026 06:41
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.

M2-PR1: LogicalPlanner skeleton + op_id/IdGen contract

1 participant