Cypher AST Work and Cleanup#89
Conversation
WalkthroughThis PR hardens Cypher AST tooling by implementing deterministic MapLiteral key ordering, adding XOR operator translation support, refactoring the walker's cursor negotiation for clarity and correctness, and introducing comprehensive test coverage for semantic versus structural traversal semantics. ChangesCypher AST Tooling Test-Hardening
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cypher/models/pgsql/translate/predicate_test.go (1)
82-86: ⚡ Quick winExpand XOR coverage to include n-ary operands.
Line 82 currently validates only binary XOR, while translator logic handles expression lists. Add one 3-operand case to guard operand-folding/order regressions.
🧪 Suggested test extension
func TestExclusiveDisjunctionTranslates(t *testing.T) { formatted := translatePredicateQuery(t, `MATCH (n:NodeKind1) WHERE true XOR false RETURN n`, nil) require.Contains(t, formatted, "true != false") + + formatted = translatePredicateQuery(t, `MATCH (n:NodeKind1) WHERE n.a XOR n.b XOR n.c RETURN n`, nil) + require.GreaterOrEqual(t, strings.Count(formatted, "!="), 2) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cypher/models/pgsql/translate/predicate_test.go` around lines 82 - 86, The test TestExclusiveDisjunctionTranslates only covers binary XOR; add a 3-operand case to guard folding/order regressions by calling translatePredicateQuery with a WHERE expression like `true XOR false XOR true` (using the same helper translatePredicateQuery) and assert the formatted output contains the expected n-ary translation (e.g., the folded/parenthesized inequality form produced by the translator, such as an expression with two != operators or a parenthesized left-associative !=). Place this assertion alongside the existing binary XOR assertion in TestExclusiveDisjunctionTranslates so both cases are validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cypher/models/pgsql/translate/predicate_test.go`:
- Around line 82-86: The test TestExclusiveDisjunctionTranslates only covers
binary XOR; add a 3-operand case to guard folding/order regressions by calling
translatePredicateQuery with a WHERE expression like `true XOR false XOR true`
(using the same helper translatePredicateQuery) and assert the formatted output
contains the expected n-ary translation (e.g., the folded/parenthesized
inequality form produced by the translator, such as an expression with two !=
operators or a parenthesized left-associative !=). Place this assertion
alongside the existing binary XOR assertion in
TestExclusiveDisjunctionTranslates so both cases are validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 330744ff-0320-475f-8431-722dce3c3120
📒 Files selected for processing (15)
cypher/models/cypher/format/format.gocypher/models/cypher/format/format_test.gocypher/models/cypher/model.gocypher/models/cypher/model_test.gocypher/models/pgsql/translate/predicate_test.gocypher/models/pgsql/translate/references_test.gocypher/models/pgsql/translate/translator.gocypher/models/walk/walk.gocypher/models/walk/walk_benchmark_test.gocypher/models/walk/walk_cypher.gocypher/models/walk/walk_pgsql.gocypher/models/walk/walk_test.godocs/cypher_ast_tooling_validation.mddocs/cypher_support_v4.mddocs/cypher_walker_semantics.md
Description
Harden Cypher AST traversal and formatting support:
walk.CypherStructuralfor full AST inspection while preservingwalk.Cypheras the semantic walker used by translation and optimizer code.walk.NewSimpleVisitorWithOrder,walk.OrderInfix, andwalk.OrderPostfix.cypher.MapLiteraliteration deterministic throughForEachItemand ascending lexical key ordering.XORpredicates by lowering exclusive disjunctions to boolean inequality.Resolves: N/A
Type of Change
Testing
make test_allwithCONNECTION_STRINGset)Validation recorded for this branch:
go test ./cypher/models/walkgo test ./cypher/models/walk ./cypher/models/cypher ./cypher/models/cypher/formatgo test -covermode=count -coverpkg=./cypher/models/walk,./cypher/models/cypher,./cypher/models/cypher/format -coverprofile=/tmp/cypher_ast_tooling_validation.cover ./cypher/models/walk ./cypher/models/cypher ./cypher/models/cypher/formatmake testPATH="/home/zinic/codex/config/go/bin:$PATH" make formatCONNECTION_STRING=<neo4j connection string> make test_neo4jCONNECTION_STRING=<postgres connection string> make test_pgmake test_allwas not recorded as a single command; unit and backend integration validation were run separately.Screenshots (if appropriate):
N/A
Driver Impact
drivers/pg)drivers/neo4j)PostgreSQL Cypher translation now handles
XORpredicates. Shared Cypher AST walker behavior was also updated and separately validated against Neo4j.Checklist
go.mod/go.sumare up to date if dependencies changedSummary by CodeRabbit
Release Notes
New Features
Tests
Documentation