Skip to content

refactor(cypher): extract bounded-reentry runtime helpers (#987 Step 3)#1331

Merged
lmeyerov merged 2 commits intomasterfrom
987-extract-reentry-execution
May 7, 2026
Merged

refactor(cypher): extract bounded-reentry runtime helpers (#987 Step 3)#1331
lmeyerov merged 2 commits intomasterfrom
987-extract-reentry-execution

Conversation

@lmeyerov
Copy link
Copy Markdown
Contributor

@lmeyerov lmeyerov commented May 7, 2026

Summary

  • Continues Cypher/GFQL: replace bounded reentry hidden-column handshake with an explicit ReentryPlan #987 (replace bounded reentry hidden-column handshake with an explicit ReentryPlan) — Step 3 (move runtime stitching into a dedicated reentry module).
  • Pure-move refactor: bounded-reentry data-frame execution helpers move out of graphistry/compute/gfql_unified.py into a new graphistry/compute/gfql/cypher/reentry/execution.py module. No semantic change.
  • gfql_unified.py shrinks by ~440 LOC, comfortably hitting the issue's "low-hundreds LOC reduction" success criterion.

What moved

To graphistry/compute/gfql/cypher/reentry/execution.py (new):

To graphistry/compute/gfql/cypher/result_postprocess.py:

  • entity_projection_meta_entry — moved next to WholeRowProjectionMeta since it is shared by the connected-OPTIONAL-MATCH (_apply_optional_null_fill) and bounded-reentry whole-row paths.

What stays

  • _execute_compiled_query_with_reentry (the dispatcher) stays in gfql_unified.py — it threads the recursive _execute_compiled_query call inside the orchestration loop, so moving it would require introducing a callback indirection. Out of scope for a pure-move slice.
  • The leading-underscore names remain accessible as gfql_unified._compiled_query_reentry_state etc. via aliased re-imports, so existing tests that reach into graphistry.compute.gfql_unified continue to work without modification.

Pointers

Test plan

  • ./bin/pytest.sh graphistry/tests/compute/test_gfql.py -k reentry — 6 passed, 3 skipped (cuDF only)
  • ./bin/pytest.sh graphistry/tests/compute/gfql/cypher/ — 1202 passed, 89 skipped, 15 xfailed
  • ./bin/pytest.sh graphistry/tests/compute/test_gfql.py graphistry/tests/test_compute_chain.py graphistry/tests/compute/gfql/ — 1963 passed, 123 skipped, 15 xfailed
  • ./bin/ruff.sh on touched files — clean
  • ./bin/mypy.sh on touched files — clean
  • Full CI suite (will run after flipping to ready)

🤖 Generated with Claude Code

@lmeyerov lmeyerov marked this pull request as ready for review May 7, 2026 01:35
lmeyerov and others added 2 commits May 6, 2026 18:47
…#987 Step 3)

Move bounded-reentry data-frame execution helpers out of `gfql_unified.py`
into a new `graphistry/compute/gfql/cypher/reentry/execution.py` module so
the bounded-reentry contract assembled at compile time (`ReentryPlan`) and
the matching data-frame stitching live next to each other.

`_entity_projection_meta_entry` moves to `result_postprocess.py` next to
`WholeRowProjectionMeta` since it is shared by the connected-OPTIONAL-MATCH
and bounded-reentry paths.

Pure-move refactor — no semantic change. `gfql_unified.py` shrinks by ~440
LOC; `gfql_unified` re-exports the moved private names via aliased imports
so existing tests reaching `gfql_unified._compiled_query_reentry_state`
continue to work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wave-2 review nit: the subpackage docstring listed naming/scope/carry/rewrite
but not the existing `runtime` (compile-time rewrites) or the new `execution`
(data-frame stitching). Adds both for grep-navigability.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lmeyerov lmeyerov force-pushed the 987-extract-reentry-execution branch from 2099b7f to c1cb0a4 Compare May 7, 2026 01:49
@lmeyerov
Copy link
Copy Markdown
Contributor Author

lmeyerov commented May 7, 2026

CI status: 1 failure inherited from master tip

After rebase onto 557acde0b (master tip), tck-gfql is failing on this PR. Same failure exists on the master commit itself:

Both fail the same scenario:

FAILED tests/cypher_tck/test_tck_runner.py::test_direct_cypher_xfail_contract[expr-pattern1-10]
AssertionError: assert 'success_wrong_rows' == 'GFQLValidationError'

This is a master-side TCK xfail-contract pin that needs updating after #1328 widened direct-Cypher varlen WHERE admission (the scenario that previously raised GFQLValidationError now succeeds with wrong rows). Not caused by this PR's pure-move refactor — AST signature parity for all 11 moved helpers was verified locally, and full cypher dir (1211 pass / 89 skip / 15 xfail) is green.

Holding merge until master is green again.

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