Skip to content

Cypher AST Work and Cleanup#89

Merged
zinic merged 29 commits into
SpecterOps:mainfrom
zinic:cypher-ast-work
May 28, 2026
Merged

Cypher AST Work and Cleanup#89
zinic merged 29 commits into
SpecterOps:mainfrom
zinic:cypher-ast-work

Conversation

@zinic
Copy link
Copy Markdown
Contributor

@zinic zinic commented May 28, 2026

Description

Harden Cypher AST traversal and formatting support:

  • Adds walk.CypherStructural for full AST inspection while preserving walk.Cypher as the semantic walker used by translation and optimizer code.
  • Adds configurable simple visitor ordering with walk.NewSimpleVisitorWithOrder, walk.OrderInfix, and walk.OrderPostfix.
  • Restores and documents semantic traversal contracts for consume/cancellation, nil-root and nil-branch handling, declaration-only fields, and map literal traversal.
  • Makes cypher.MapLiteral iteration deterministic through ForEachItem and ascending lexical key ordering.
  • Fixes traversal and PostgreSQL translation for Cypher XOR predicates by lowering exclusive disjunctions to boolean inequality.
  • Expands walker, map literal, formatter, and PostgreSQL translation tests, plus records validation and walker semantics documentation.

Resolves: N/A

Type of Change

  • Chore (a change that does not modify the application functionality)
  • Bug fix (a change that fixes an issue)
  • New feature / enhancement (a change that adds new functionality)
  • Refactor (no behaviour change)
  • Test coverage
  • Build / CI / tooling
  • Documentation

Testing

  • Unit tests added / updated
  • Integration tests added / updated
  • Full test suite run (make test_all with CONNECTION_STRING set)

Validation recorded for this branch:

  • go test ./cypher/models/walk
  • go test ./cypher/models/walk ./cypher/models/cypher ./cypher/models/cypher/format
  • go 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/format
  • make test
  • PATH="/home/zinic/codex/config/go/bin:$PATH" make format
  • CONNECTION_STRING=<neo4j connection string> make test_neo4j
  • CONNECTION_STRING=<postgres connection string> make test_pg

make test_all was not recorded as a single command; unit and backend integration validation were run separately.

Screenshots (if appropriate):

N/A

Driver Impact

  • PostgreSQL driver (drivers/pg)
  • Neo4j driver (drivers/neo4j)

PostgreSQL Cypher translation now handles XOR predicates. Shared Cypher AST walker behavior was also updated and separately validated against Neo4j.

Checklist

  • Code is formatted
  • All existing tests pass
  • go.mod / go.sum are up to date if dependencies changed

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for exclusive disjunction (XOR) operator in Cypher.
    • Map literals now iterate in deterministic sorted-key order.
    • Added structural AST traversal mode for comprehensive syntax tree inspection.
  • Tests

    • Expanded test coverage for map literal behavior and XOR operator translation.
    • Added performance benchmarks for large AST traversals.
  • Documentation

    • Added Cypher translation support roadmap and AST traversal semantics guides.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Walkthrough

This 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.

Changes

Cypher AST Tooling Test-Hardening

Layer / File(s) Summary
MapLiteral Deterministic Key Ordering
cypher/models/cypher/model.go, cypher/models/cypher/format/format.go, cypher/models/cypher/format/format_test.go, cypher/models/cypher/model_test.go
MapLiteral adds sortedKeys() helper; Items(), ForEachItem(), and Keys() now return entries in deterministic lexicographic order. Formatting switches to ForEachItem callback to emit key-value pairs without Go map randomness. Three new model tests verify sorted iteration and error propagation; three new emitter tests verify key ordering and writer error handling.
ExclusiveDisjunction (XOR) Translation Support
cypher/models/pgsql/translate/translator.go, cypher/models/pgsql/translate/predicate_test.go, cypher/models/pgsql/translate/references_test.go
Translator.Enter and Translator.Exit now handle *cypher.ExclusiveDisjunction by translating operand pairs to OperatorNotEquals (SQL !=) expressions. Tests verify XOR-to-inequality translation and identifier collection from XOR operands in WHERE clauses.
Walker Core Refactoring and Nil Handling
cypher/models/walk/walk.go, cypher/models/walk/walk_pgsql.go
walk.go changes error handling from error slices to single errors.Join(), adds isNilNode reflection helper for nil-safety, and rewires Generic traversal state machine to correctly handle Done(), consumed nodes, and exit unwinding. walk_pgsql.go adds nil-node guard at entry.
Cypher Cursor Negotiation Refactoring
cypher/models/walk/walk_cypher.go
Refactors from monolithic type-switch into layered structural and categorical dispatchers. Adds newCypherStructuralWalkCursor for complete AST inspection; generic branch/map-item cursor helpers; and specialized negotiators for leaf, value, predicate, operator, projection, query, clause, updating, and pattern node categories.
Walker Behavior Validation and Semantic Traversal Tests
cypher/models/walk/walk_test.go
Adds 20\+ test functions validating Consume() semantics, bare MapLiteral leaf treatment, properties map key ordering, nil branch/root handling, ordered visitor modes, generic walker Done/Error aggregation, cursor construction error propagation, semantic vs. structural field filtering, node-type-driven child visitation, and traversal sequencing for expressions and query clauses.
Benchmarks and Walker Semantics Documentation
cypher/models/walk/walk_benchmark_test.go, docs/cypher_walker_semantics.md, docs/cypher_ast_tooling_validation.md, docs/cypher_support_v4.md
Adds three benchmarks (large projection/map literal, long pattern) for walker performance; documents semantic vs. structural traversal rules, nil handling contracts, visitor cancellation behavior; records validation pass including benchmark comparisons, coverage deltas, and behavior change notes; provides Cypher Translation Support V4 implementation roadmap.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

enhancement, go

Suggested reviewers

  • kpom-specter
  • urangel

Poem

🐰 Ordered Keys and Walks Refined

MapLiterals now sort their keys with care,
XOR blooms as inequality fair,
The walker's state machine takes new shape—
Structural paths and semantic escape,
With tests and benchmarks guiding the way,
This tooling is hardened for a brighter day!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is vague and overly broad, using generic terms like 'Work and Cleanup' that don't clearly convey the primary changes. Replace with a more specific title that highlights the main change, such as 'Add structural walker and fix XOR translation' or 'Harden Cypher AST traversal and formatting'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description is comprehensive, covering objectives, type of changes, testing performed, driver impact, and checklist items aligned with the template.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
cypher/models/pgsql/translate/predicate_test.go (1)

82-86: ⚡ Quick win

Expand 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9fe7797 and 5838b44.

📒 Files selected for processing (15)
  • cypher/models/cypher/format/format.go
  • cypher/models/cypher/format/format_test.go
  • cypher/models/cypher/model.go
  • cypher/models/cypher/model_test.go
  • cypher/models/pgsql/translate/predicate_test.go
  • cypher/models/pgsql/translate/references_test.go
  • cypher/models/pgsql/translate/translator.go
  • cypher/models/walk/walk.go
  • cypher/models/walk/walk_benchmark_test.go
  • cypher/models/walk/walk_cypher.go
  • cypher/models/walk/walk_pgsql.go
  • cypher/models/walk/walk_test.go
  • docs/cypher_ast_tooling_validation.md
  • docs/cypher_support_v4.md
  • docs/cypher_walker_semantics.md

@zinic zinic merged commit 5efb0a7 into SpecterOps:main May 28, 2026
7 of 9 checks passed
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