feat(gfql): fix gfql_remote WHERE loss + add Let/Cypher support#962
Merged
feat(gfql): fix gfql_remote WHERE loss + add Let/Cypher support#962
Conversation
P0: Fix silent WHERE clause loss in gfql_remote(). chain_remote.py was
unwrapping the Chain envelope and discarding the `where` field, causing
queries with same-path constraints to return wrong results remotely.
Now sends both `gfql_operations` (flat array for old servers) and
`gfql_query` (full Chain/Let envelope for new servers).
P1: Accept ASTLet and Let dict input for DAG queries. Serialized as
{"type": "Let", ...} in gfql_query. Old servers receive empty
gfql_operations (safe no-op).
P2: Accept Cypher strings. Compiled locally via compile_cypher(), then
serialized as Chain or Let wire format. Supports MATCH...RETURN,
GRAPH { }, and GRAPH g = ... USE g ... forms.
10 new unit tests covering all input types and backward compat.
759 total tests pass (including full GFQL/Cypher regression suite).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_remote Audit fixes: - DRY: Consolidate two near-identical Let serializers into _binding_to_json/_final_to_json/_compiled_to_let_json - Fix: Update chain_remote_generic type signature to include str/ASTLet - Fix: Replace unreachable else with TypeError raise - Add: UserWarning when sending Let queries (old servers get no-op) - Add: Tests for invalid Cypher string, invalid type, empty chain, Let warning emission - Remove: Dead _capture_request_body helper 14 unit tests pass, 763 total pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix: CALL procedure params now serialized (was hardcoded to {})
via new _procedure_call_to_json helper
- Fix: gfql_remote docstring :param name matches actual parameter
- Add: Tests for GRAPH { CALL .write() } and GRAPH { USE g1 CALL ... }
via gfql_remote (the motivating pipeline use case)
- Add: Test for standalone CALL .write() in GRAPH constructor
- Clean: Remove unused Dict import in tests
16 unit tests pass, 24 total remote tests pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verifies that procedure arguments like {damping: 0.85} survive
through the _procedure_call_to_json serialization path and appear
in the wire format. This was the P0-class bug from audit round 2
where params were hardcoded to {}.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er WHERE e2e - Fix: _binding_to_json now uses getattr for defensive attribute access, consistent with _final_to_json - Add: End-to-end test for Cypher string with cross-step WHERE (MATCH ... WHERE a.team = b.team RETURN ...) — the exact P0 scenario proving WHERE survives through compile_cypher -> chain.to_json -> request body - Fix: Strengthen test_graph_constructor_sends_chain to verify gfql_operations has content - Fix: Strengthen test_standalone_call_write_sends_chain to verify gfql_query type is Chain 18 unit tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace all Any-typed parameters and getattr defensive access with concrete types from the lowering module: - _procedure_call_to_json: Any -> CompiledCypherProcedureCall - _binding_to_json: Any -> CompiledGraphBinding - _final_to_json: Any -> Union[CompiledCypherQuery, CompiledCypherGraphQuery] - _compiled_to_let_json: same Union type Move imports from lazy (inside function body) to module-level since there are no circular import issues. Direct attribute access replaces getattr now that types guarantee the attributes exist. 18 unit tests pass, 767 total pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…h (-23 lines) - _procedure_call_to_json + _binding_to_json + _final_to_json → single _step_to_json(chain, procedure_call, use_ref) that handles all cases - _compiled_to_let_json now uses dict comprehension + _step_to_json - CompiledCypherGraphQuery and CompiledCypherQuery Cypher dispatch branches collapsed (both check graph_bindings/use_ref identically) - Removed unused CompiledGraphBinding import 26 insertions, 49 deletions. 18 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…(-165 lines) Extract mock setup and request body capture into a shared _send(query) helper. Every test now calls _send() and asserts on the returned body dict, eliminating repeated mock_post/mock_plottable/body extraction boilerplate. 339 -> 174 lines (49% reduction), same 18 tests, same coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ruff F821 flagged 'ASTLet' as undefined name in type annotation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicate type annotation on request_body else branch - Widen chain_remote/chain_remote_shape type sigs to match chain_remote_generic (include ASTLet, str) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_remote Show that gfql_remote() now accepts Cypher strings, GRAPH constructors, and multi-stage pipelines alongside native chain syntax. Update method reference from deprecated chain_remote to gfql_remote. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gfql_remote("MATCH (n) WHERE n.x > \$cutoff RETURN n", params={"cutoff": 10})
now works — params are passed to compile_cypher() during local compilation.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The mock_server_hypergraph function didn't accept the new params keyword added to chain_remote, causing test failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 17, 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
1 participant
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.
Fix gfql_remote() WHERE loss (P0 correctness bug), add Let/Cypher support (P1/P2), dual-field backward compat.
P0: chain_remote.py silently dropped WHERE clauses — wrong results for cross-step filters. Fixed by adding gfql_query field with full Chain envelope.
P1: Accept ASTLet/Let dict, serialize as {"type":"Let"} in new gfql_query field. Old servers get empty gfql_operations (safe no-op).
P2: Accept Cypher strings, compile locally via compile_cypher(), send as Chain or Let wire format. Supports MATCH...RETURN, GRAPH { }, GRAPH g = ... USE g ..., CALL .write().
Backward compat: Always sends both gfql_operations (flat array for old servers) and gfql_query (full typed envelope for new servers).
Files: chain_remote.py (+95/-20), ComputeMixin.py (+35/-15), test_chain_remote_v2.py (+174 new, 18 tests), CHANGELOG.md (+7).
Tests: 18 new unit tests, 767 total pass.