test: including official opencypher tck

[lvca]

I was able to implement the TCK into the ArcadeDB pipeline, it's now on a separate branch (opencypher-tck). Current results are not great in many areas, but hopefully I can focus on this only for the next days.

Current TCK Results

  ┌───────────────────────────┬────────┬───────┬───────┐
  │         Category          │ Passed │ Total │ Rate  │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ expressions/conditional   │ 13     │ 13    │ 100%  │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ expressions/null          │ 39     │ 44    │ 88%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ clauses/remove            │ 25     │ 33    │ 75%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ expressions/quantifier    │ 449    │ 604   │ 74%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ clauses/unwind            │ 9      │ 14    │ 64%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ clauses/return-skip-limit │ 19     │ 31    │ 61%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ expressions/aggregation   │ 21     │ 35    │ 60%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ expressions/literals      │ 75     │ 131   │ 57%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ clauses/return            │ 33     │ 63    │ 52%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ clauses/return-orderby    │ 18     │ 35    │ 51%   │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ ...                       │        │       │       │
  ├───────────────────────────┼────────┼───────┼───────┤
  │ Overall                   │ 1153   │ 3897  │ 29.6% │
  └───────────────────────────┴────────┴───────┴───────┘

To Run TCK suite (from project root on the specific branch until is merged):

cd engine && mvn test -Dtest=OpenCypherTCKRunner -DfailIfNoTests=false -Dsurefire.excludeJUnit5Engines=""

[claude]

OpenCypher TCK Integration Review

This PR adds the official OpenCypher Technology Compatibility Kit (TCK) test suite to ArcadeDB. Overall, this is a solid implementation that integrates Cucumber-based testing for openCypher compliance. Here's my detailed review:

---

✅ Strengths

1. Well-structured test framework: The Cucumber integration with step definitions, value parsing, result matching, and side-effect checking is cleanly separated into logical components.

2. Proper test isolation: Each test gets a fresh database instance (tck-{counter}) and proper cleanup in @After hooks.

3. Maven configuration: The pom.xml correctly excludes TCK tests from normal runs and requires explicit execution, preventing accidental long-running test execution.

4. Expected failures mechanism: The expected-failures.txt approach allows tracking known gaps without failing the build, which is pragmatic for incremental compliance improvement.

5. Comprehensive result matching: TCKResultMatcher handles complex comparisons including nodes, edges, paths, lists, maps, and numeric type coercion.

6. Reporting: TCKReportPlugin provides clear compliance metrics by category.

---

🔍 Code Quality & Best Practices

Following CLAUDE.md guidelines:

• ✅ Uses AssertJ assertions (assertThat()) as recommended
• ✅ Proper license headers on all new files
• ✅ No System.out debug statements (only in reporting plugin, which is appropriate)
• ✅ Follows existing code style (single-line if statements, final keywords)
• ⚠️ Minor: Some methods could use more final on local variables

Specific observations:

TCKStepDefinitions.java:88-89

} catch (final Exception ignored) {
}

Empty catch blocks without comments. Consider adding a brief comment explaining why exceptions are safely ignored during cleanup.

TCKValueParser.java:89-91, 96-98
Multiple fall-through exception handling. This is acceptable but consider consolidating the number parsing logic.

TCKResultMatcher.java:179

return Math.abs(a - e) < 1e-10 || a == e;

Magic number 1e-10 for floating-point comparison. Consider extracting to a named constant like FLOAT_EPSILON.

---

⚠️ Potential Issues

1. Resource cleanup - Database drop exception handling (TCKStepDefinitions.java:86-89)

The tearDown() method silently ignores all exceptions during database drop:

try {
  database.drop();
} catch (final Exception ignored) {
}

While this may be intentional to prevent test failures during cleanup, it could mask issues. Consider at least logging exceptions or using a more specific exception type.

2. Thread safety in side-effect checking (TCKSideEffectChecker.java:141-143)

} catch (final Exception ignored) {
  // Type may have been concurrently modified
}

The comment mentions concurrent modification, but TCK tests should be single-threaded. This suggests either:

• The comment is misleading, or
• There's an actual concurrency concern that should be addressed

Recommendation: Clarify whether this is defensive programming or addressing a real issue.

3. Path matching is incomplete (TCKResultMatcher.java:257-266)

if (actual instanceof List)
  return true; // Simplified: path structure comparison is complex

This always returns true for List-based paths without validation. This will cause false positives in TCK results for path-based tests.

Impact: Medium - Could hide real bugs in path handling
Recommendation: Either implement proper validation or document in expected-failures.txt that all path-as-list tests should be skipped.

4. No validation of Cucumber dependency versions

The PR adds Cucumber 7.22.1 without checking for potential conflicts with JUnit 5. While the configuration looks correct, this should be verified with an actual test run.

---

🔒 Security Considerations

• ✅ No SQL injection risks (parameters are properly handled via database API)
• ✅ No credential exposure
• ✅ Test databases are created in ./target/databases/ which is appropriate for build artifacts
• ✅ No external network calls in test code

---

🚀 Performance Considerations

1. Database creation overhead

Each scenario creates a fresh database. With 3897 total scenarios, this could be slow:

final String dbPath = "./target/databases/tck-" + DB_COUNTER.incrementAndGet();

Recommendation: Consider transaction-based rollback for read-only tests to reduce I/O overhead. This could significantly improve TCK execution time.

2. Side-effect checking overhead (TCKSideEffectChecker.java)

The side-effect checker iterates all documents to count properties:

private long countAllProperties(final Database database) {
  // Iterates all documents in all types
}

For large graphs, this could be expensive. Consider:

• Caching counts when no side effects are expected
• Skipping property counts for read-only queries

3. String parsing in hot path

TCKValueParser.parse() is called for every cell in every expected result table. The current implementation with multiple string operations could be optimized if it becomes a bottleneck.

---

🧪 Test Coverage

• ✅ The PR adds comprehensive integration tests via TCK
• ⚠️ Missing: No unit tests for the new TCK infrastructure classes themselves
  • TCKValueParser has complex parsing logic that deserves unit tests
  • TCKResultMatcher.pathStructureMatches() should have test coverage

Recommendation: Add unit tests for:

TCKValueParserTest.java  // Test edge cases in value parsing
TCKResultMatcherTest.java  // Test matching logic for various data types

---

📝 Documentation & Maintainability

Positives:

• ✅ Good inline comments explaining TCK concepts
• ✅ PR description clearly shows current compliance (29.6%)
• ✅ Instructions for running TCK are in PR body

Suggestions:

1. Add a README in engine/src/test/java/com/arcadedb/query/opencypher/tck/ explaining:

   • How the TCK framework works
   • How to add expected failures
   • How to interpret results

2. Document the expected-failures.txt format more clearly with examples

3. Add TODO comments for known incomplete implementations (like the path matching simplification)

---

🐛 Potential Bugs

1. Label checking logic (TCKResultMatcher.java:199-207)

for (final String label : expected.labels)
  if (!typeName.equals(label) && !hasLabel(doc, label))
    return false;

This logic checks && when it should likely be ||. If any expected label doesn't match, it should fail. Current logic only fails if the type name doesn't match AND hasLabel also returns false.

Expected: if (!typeName.equals(label) && !hasLabel(doc, label))
Should be: if (!(typeName.equals(label) || hasLabel(doc, label)))

This is a HIGH priority bug that could cause false positives in node matching tests.

2. Missing null check (TCKStepDefinitions.java:199)

resultColumns = new ArrayList<>(row.getPropertyNames());

If row.getPropertyNames() returns null, this will throw NPE. Should check for null.

---

📋 Recommendations Summary

Must Fix (Before Merge):

1. ✋ Fix label checking logic in TCKResultMatcher.java:199-207 (potential false positives)
2. ✋ Complete path matching implementation or document limitation
3. ✋ Add null check for row.getPropertyNames()

Should Fix (Soon After):

1. Add unit tests for TCKValueParser and TCKResultMatcher
2. Add descriptive comments to empty catch blocks
3. Extract magic numbers to constants
4. Add TCK infrastructure documentation (README)

Nice to Have:

1. Optimize database creation strategy for better performance
2. Reduce side-effect checking overhead
3. Add more final keywords on local variables per CLAUDE.md style

---

🎯 Overall Assessment

This is a valuable addition that will help track openCypher compliance systematically. The implementation is generally solid with good separation of concerns. The main concerns are:

1. A potential logic bug in label matching (high priority)
2. Incomplete path matching implementation (medium priority)
3. Missing test coverage for the TCK infrastructure itself (medium priority)

Recommendation: Address the label matching bug and path matching limitation before merging. The other issues can be tackled incrementally as you work on improving the TCK pass rate from 29.6%.

Great work on getting this integrated! The structured approach will make it much easier to track and improve openCypher compliance.

[gemini-code-assist]

Summary of Changes

Hello @lvca, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances ArcadeDB's testing infrastructure by integrating the official OpenCypher TCK. This integration provides a robust framework for validating Cypher query language compliance, featuring detailed reporting and precise result and side effect verification. This is a foundational step towards improving the overall compatibility and reliability of ArcadeDB's Cypher implementation.

Highlights

• OpenCypher TCK Integration: The official OpenCypher Technology Compatibility Kit (TCK) has been integrated into the ArcadeDB project, enabling comprehensive compliance testing for the Cypher query language.
• Cucumber-based Testing Framework: The TCK is implemented using Cucumber, a behavior-driven development (BDD) framework, allowing for human-readable test scenarios defined in '.feature' files.
• Custom TCK Runner and Reporting: A dedicated OpenCypherTCKRunner class is introduced to execute the TCK tests, complemented by a TCKReportPlugin that generates a detailed compliance report summarizing pass/fail rates by category.
• Advanced Result and Side Effect Verification: New utility classes (TCKResultMatcher, TCKSideEffectChecker) are added to accurately compare query results against expected outcomes and to verify database state changes (nodes, relationships, properties, labels) after query execution.
• Extensive Test Coverage: A large suite of TCK feature files has been added, covering various Cypher clauses such as CALL, CREATE, DELETE, MATCH, MERGE, REMOVE, RETURN, UNWIND, and their sub-clauses, ensuring broad test coverage.
• Maven Configuration Updates: The pom.xml file has been updated to include necessary Cucumber and JUnit Platform Suite dependencies, and the maven-surefire-plugin is configured to manage TCK test execution separately from standard unit tests.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• engine/pom.xml
  • Added cucumber.version and junit-platform-suite.version properties.
  • Included maven-surefire-plugin configuration to exclude OpenCypherTCKRunner from default test runs and prevent auto-discovery of TCK feature files.
  • Added cucumber-java, cucumber-junit-platform-engine, and junit-platform-suite as test dependencies for TCK execution.
• engine/src/test/java/com/arcadedb/query/opencypher/tck/OpenCypherTCKRunner.java
  • New file: Implements the main JUnit Platform Suite runner for the OpenCypher TCK, configuring Cucumber to discover features and use a custom report plugin.
• engine/src/test/java/com/arcadedb/query/opencypher/tck/TCKReportPlugin.java
  • New file: Provides a custom Cucumber plugin to generate a comprehensive compliance report, tracking total, passed, failed, and skipped scenarios by category.
• engine/src/test/java/com/arcadedb/query/opencypher/tck/TCKResultMatcher.java
  • New file: Contains static methods for asserting query results against expected TCK tables, supporting unordered and ordered comparisons, and handling various Cypher value types (nodes, relationships, paths, lists, maps, numbers, booleans, strings).
• engine/src/test/java/com/arcadedb/query/opencypher/tck/TCKSideEffectChecker.java
  • New file: Implements functionality to snapshot and verify database state changes (node, relationship, label, and property counts) to ensure queries have expected side effects.
• engine/src/test/java/com/arcadedb/query/opencypher/tck/TCKStepDefinitions.java
  • New file: Defines Cucumber step definitions (Given, When, Then, And) that map TCK scenarios to ArcadeDB's query execution and assertion logic, including database setup, query execution, result validation, and side effect checking.
• engine/src/test/java/com/arcadedb/query/opencypher/tck/TCKValueParser.java
  • New file: Utility class for parsing TCK value strings (e.g., nodes, relationships, paths, lists, maps, primitives) into corresponding Java objects for comparison.
• engine/src/test/resources/junit-platform.properties
  • New file: Configures JUnit Platform to prevent Cucumber from auto-discovering features during normal test runs and to quiet Cucumber publishing.
• engine/src/test/resources/opencypher/tck/expected-failures.txt
  • New file: A list of TCK scenario names or substrings that are currently expected to fail and will be skipped during test execution.
• engine/src/test/resources/opencypher/tck/features/clauses/call/Call1.feature
  • New file: TCK scenarios for basic procedure calling, including standalone and in-query calls, argument handling, and error cases.
• engine/src/test/resources/opencypher/tck/features/clauses/call/Call2.feature
  • New file: TCK scenarios focusing on explicit and implicit procedure arguments, as well as type mismatch errors.
• engine/src/test/resources/opencypher/tck/features/clauses/call/Call3.feature
  • New file: TCK scenarios for assignable-type arguments in procedures, testing type coercion for numbers.
• engine/src/test/resources/opencypher/tck/features/clauses/call/Call4.feature
  • New file: TCK scenarios for handling null arguments in procedure calls.
• engine/src/test/resources/opencypher/tck/features/clauses/call/Call5.feature
  • New file: TCK scenarios for procedure result projection, including explicit YIELD, renaming, and error cases for variable binding.
• engine/src/test/resources/opencypher/tck/features/clauses/call/Call6.feature
  • New file: TCK scenarios for interoperation of the CALL clause with other clauses like WITH.
• engine/src/test/resources/opencypher/tck/features/clauses/create/Create1.feature
  • New file: TCK scenarios for creating nodes with and without labels and properties, including error cases for bound variables.
• engine/src/test/resources/opencypher/tck/features/clauses/create/Create2.feature
  • New file: TCK scenarios for creating relationships, including self-loops, relationships between existing nodes, and error cases for invalid relationship patterns.
• engine/src/test/resources/opencypher/tck/features/clauses/create/Create3.feature
  • New file: TCK scenarios for interoperation of the CREATE clause with other clauses like MATCH, WITH, UNWIND, and MERGE.
• engine/src/test/resources/opencypher/tck/features/clauses/create/Create4.feature
  • New file: TCK scenarios for large-scale graph creation, demonstrating the creation of complex graph structures like a movie database and a school network.
• engine/src/test/resources/opencypher/tck/features/clauses/create/Create5.feature
  • New file: TCK scenarios for creating patterns with multiple hops and varying directions.
• engine/src/test/resources/opencypher/tck/features/clauses/create/Create6.feature
  • New file: TCK scenarios for testing the persistence of side effects from CREATE clauses when combined with other clauses like LIMIT, SKIP, WHERE, and aggregation.
• engine/src/test/resources/opencypher/tck/features/clauses/delete/Delete1.feature
  • New file: TCK scenarios for deleting nodes, including DETACH DELETE and handling null nodes, as well as error cases for deleting connected nodes.
• engine/src/test/resources/opencypher/tck/features/clauses/delete/Delete2.feature
  • New file: TCK scenarios for deleting relationships, including optional matches and bidirectional matching.
• engine/src/test/resources/opencypher/tck/features/clauses/delete/Delete3.feature
  • New file: TCK scenarios for deleting named paths, including DETACH DELETE.
• engine/src/test/resources/opencypher/tck/features/clauses/delete/Delete4.feature
  • New file: TCK scenarios for interoperation of the DELETE clause with other clauses like MATCH and variable-length paths.
• engine/src/test/resources/opencypher/tck/features/clauses/delete/Delete5.feature
  • New file: TCK scenarios for deleting elements from lists and maps, and error cases for invalid DELETE arguments.
• engine/src/test/resources/opencypher/tck/features/clauses/delete/Delete6.feature
  • New file: TCK scenarios for testing the persistence of side effects from DELETE clauses when combined with other clauses like LIMIT, SKIP, WHERE, and aggregation.
• engine/src/test/resources/opencypher/tck/features/clauses/match-where/MatchWhere1.feature
  • New file: TCK scenarios for filtering single variables in MATCH-WHERE clauses using label, property, and relationship type predicates.
• engine/src/test/resources/opencypher/tck/features/clauses/match-where/MatchWhere2.feature
  • New file: TCK scenarios for filtering multiple variables in MATCH-WHERE clauses using conjunctive property predicates.
• engine/src/test/resources/opencypher/tck/features/clauses/match-where/MatchWhere3.feature
  • New file: TCK scenarios for equi-joins on node identities and properties in MATCH-WHERE clauses.
• engine/src/test/resources/opencypher/tck/features/clauses/match-where/MatchWhere4.feature
  • New file: TCK scenarios for non-equi-joins on variables in MATCH-WHERE clauses.
• engine/src/test/resources/opencypher/tck/features/clauses/match-where/MatchWhere5.feature
  • New file: TCK scenarios for filtering on predicates that may result in null values in MATCH-WHERE clauses.
• engine/src/test/resources/opencypher/tck/features/clauses/match-where/MatchWhere6.feature
  • New file: TCK scenarios for filtering optional matches in MATCH-WHERE clauses, including complex join conditions.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match1.feature
  • New file: TCK scenarios for matching nodes, including non-existent nodes, all nodes, nodes with multiple labels, and inline property predicates.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match2.feature
  • New file: TCK scenarios for matching relationships, including self-loops, multiple types, and error cases for variable conflicts.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match3.feature
  • New file: TCK scenarios for matching fixed-length patterns, including directed, undirected, and cyclic patterns.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match4.feature
  • New file: TCK scenarios for matching variable-length patterns, including fixed-length, unbounded, and bounded ranges.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match5.feature
  • New file: TCK scenarios for matching variable-length patterns over pre-defined graph structures, testing various length bounds and chain combinations.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match6.feature
  • New file: TCK scenarios for matching named paths, including zero-length, simple, three-node, and variable-length paths, respecting direction.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match7.feature
  • New file: TCK scenarios for optional match, covering cases with and without existing data, bound nodes, variable-length relationships, and null handling.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match8.feature
  • New file: TCK scenarios for interoperation of the MATCH clause with other clauses like WITH, MERGE, and OPTIONAL MATCH.
• engine/src/test/resources/opencypher/tck/features/clauses/match/Match9.feature
  • New file: TCK scenarios for deprecated match features, specifically variable-length relationship variables as lists and label predicates on both sides.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge1.feature
  • New file: TCK scenarios for merging nodes, including creation, matching existing nodes, and handling properties and labels.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge2.feature
  • New file: TCK scenarios for merging nodes with ON CREATE actions, including setting labels and properties.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge3.feature
  • New file: TCK scenarios for merging nodes with ON MATCH actions, including setting labels and properties.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge4.feature
  • New file: TCK scenarios for merging nodes with both ON CREATE and ON MATCH actions.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge5.feature
  • New file: TCK scenarios for merging relationships, including creation, matching existing relationships, and handling properties and directions.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge6.feature
  • New file: TCK scenarios for merging relationships with ON CREATE actions, including setting properties and copying properties from other elements.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge7.feature
  • New file: TCK scenarios for merging relationships with ON MATCH actions, including setting properties and copying properties from other elements.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge8.feature
  • New file: TCK scenarios for merging relationships with both ON CREATE and ON MATCH actions.
• engine/src/test/resources/opencypher/tck/features/clauses/merge/Merge9.feature
  • New file: TCK scenarios for interoperation of the MERGE clause with other clauses like UNWIND and CREATE.
• engine/src/test/resources/opencypher/tck/features/clauses/remove/Remove1.feature
  • New file: TCK scenarios for removing properties from nodes and relationships, including handling missing properties and null values.
• engine/src/test/resources/opencypher/tck/features/clauses/remove/Remove2.feature
  • New file: TCK scenarios for removing labels from nodes, including single and multiple labels, and handling non-existent labels.
• engine/src/test/resources/opencypher/tck/features/clauses/remove/Remove3.feature
  • New file: TCK scenarios for testing the persistence of side effects from REMOVE clauses when combined with other clauses like LIMIT, SKIP, WHERE, and aggregation.
• engine/src/test/resources/opencypher/tck/features/clauses/return-orderby/ReturnOrderBy1.feature
  • New file: TCK scenarios for ordering by a single variable, testing the expected order for various data types (booleans, strings, integers, floats, lists, dates, times, datetimes, and mixed types).
• engine/src/test/resources/opencypher/tck/features/clauses/return-orderby/ReturnOrderBy2.feature
  • New file: TCK scenarios for ordering by a single expression, including aggregated functions, distinct values, and returning all variables.
• engine/src/test/resources/opencypher/tck/features/clauses/return-orderby/ReturnOrderBy3.feature
  • New file: TCK scenarios for ordering by multiple expressions, testing priority of sort directions.
• engine/src/test/resources/opencypher/tck/features/clauses/return-orderby/ReturnOrderBy4.feature
  • New file: TCK scenarios for ordering in combination with projection, testing how aliases and expressions affect sorting.
• engine/src/test/resources/opencypher/tck/features/clauses/return-orderby/ReturnOrderBy5.feature
  • New file: TCK scenarios for ordering in combination with column renaming.
• engine/src/test/resources/opencypher/tck/features/clauses/return-orderby/ReturnOrderBy6.feature
  • New file: TCK scenarios for aggregation expressions in ORDER BY, including constants, parameters, and error cases for undefined variables or ambiguous aggregations.
• engine/src/test/resources/opencypher/tck/features/clauses/return-skip-limit/ReturnSkipLimit1.feature
  • New file: TCK scenarios for the SKIP clause, testing various skip amounts, parameters, and error cases for non-constant or negative values.
• engine/src/test/resources/opencypher/tck/features/clauses/return-skip-limit/ReturnSkipLimit2.feature
  • New file: TCK scenarios for the LIMIT clause, testing various limit amounts, parameters, and error cases for non-constant or negative values.
• engine/src/test/resources/opencypher/tck/features/clauses/return-skip-limit/ReturnSkipLimit3.feature
  • New file: TCK scenarios for combining SKIP and LIMIT clauses.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return1.feature
  • New file: TCK scenarios for returning single variables, including list properties and error cases for undefined variables.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return2.feature
  • New file: TCK scenarios for returning single expressions, including arithmetic expressions, node/relationship properties, label predicates, and projected maps.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return3.feature
  • New file: TCK scenarios for returning multiple expressions, including node properties and projected nodes/relationships.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return4.feature
  • New file: TCK scenarios for column renaming in RETURN, including aliases for expressions and aggregations, and error cases for column name conflicts.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return5.feature
  • New file: TCK scenarios for implicit grouping with DISTINCT, including lists in maps and nullable values.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return6.feature
  • New file: TCK scenarios for implicit grouping with aggregates, including counting, arithmetic expressions with aggregation, and aggregates within functions.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return7.feature
  • New file: TCK scenarios for returning all variables (RETURN *), including cases with and without variables in scope.
• engine/src/test/resources/opencypher/tck/features/clauses/return/Return8.feature
  • New file: TCK scenarios for interoperation of the RETURN clause with other clauses like WITH and filtering.
• engine/src/test/resources/opencypher/tck/features/clauses/set/Set1.feature
  • New file: TCK scenarios for setting properties on nodes and relationships, including expressions, lists, and error cases for undefined variables or invalid property types.
• engine/src/test/resources/opencypher/tck/features/clauses/set/Set2.feature
  • New file: TCK scenarios for setting properties to null, which effectively removes them.
• engine/src/test/resources/opencypher/tck/features/clauses/set/Set3.feature
  • New file: TCK scenarios for setting labels on nodes, including single and multiple labels, and handling whitespace.
• engine/src/test/resources/opencypher/tck/features/clauses/set/Set4.feature
  • New file: TCK scenarios for setting all properties with a map, including removal of non-existent or null values.
• engine/src/test/resources/opencypher/tck/features/clauses/set/Set5.feature
  • New file: TCK scenarios for setting multiple properties with an appending map (+=), including overwriting, retaining values, and handling nulls.
• engine/src/test/resources/opencypher/tck/features/clauses/set/Set6.feature
  • New file: TCK scenarios for testing the persistence of side effects from SET clauses when combined with other clauses like LIMIT, SKIP, WHERE, and aggregation.
• engine/src/test/resources/opencypher/tck/features/clauses/union/Union1.feature
  • New file: TCK scenarios for the UNION clause, testing distinct results from multiple queries and error cases for different column names.
• engine/src/test/resources/opencypher/tck/features/clauses/union/Union2.feature
  • New file: TCK scenarios for the UNION ALL clause, testing non-distinct results from multiple queries.
• engine/src/test/resources/opencypher/tck/features/clauses/union/Union3.feature
  • New file: TCK scenarios for mixing UNION and UNION ALL clauses, demonstrating syntax errors.
• engine/src/test/resources/opencypher/tck/features/clauses/unwind/Unwind1.feature
  • New file: TCK scenarios for the UNWIND clause, including unwinding lists, ranges, concatenated lists, collected expressions, and handling empty lists or nulls.
• engine/src/test/resources/opencypher/tck/features/clauses/with-orderBy/WithOrderBy1.feature
  • New file: TCK scenarios for ordering by a single variable in the WITH clause, testing various data types and sort directions.
• engine/src/test/resources/opencypher/tck/features/clauses/with-orderBy/WithOrderBy2.feature
  • New file: TCK scenarios for ordering by a single expression in the WITH clause, including boolean, integer, float, string, list, date, time, and datetime expressions.
• engine/src/test/resources/opencypher/tck/features/clauses/with-orderBy/WithOrderBy3.feature
  • New file: TCK scenarios for ordering by multiple expressions in the WITH clause, testing various combinations of sort directions and constant expressions.
• engine/src/test/resources/opencypher/tck/features/clauses/with-orderBy/WithOrderBy4.feature
  • New file: TCK scenarios for ordering in combination with projection and aliasing in the WITH clause, including aggregate projections and error cases for undefined variables.

Activity

• The author, lvca, has successfully integrated the official OpenCypher TCK into the ArcadeDB pipeline.
• The TCK is currently running on a separate branch (opencypher-tck) to facilitate development and testing.
• Initial TCK results indicate an overall pass rate of 29.6%, with varying success rates across different query categories.
• The author has expressed intent to dedicate time in the coming days to improve the TCK pass rate.
• Instructions for running the TCK suite locally have been provided for easy reproduction and further development.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces the official OpenCypher TCK, which is a great step towards ensuring compatibility. The implementation is comprehensive, including Maven configuration, a test runner, and various helper classes for test execution and reporting. The code is well-structured. I have a couple of suggestions to improve the robustness of the test infrastructure by adding logging for ignored exceptions, which will aid in future debugging.

[gemini-code-assist]

Catching and ignoring a generic Exception can hide important issues. While the comment suggests this is for handling concurrent modification, it's better to catch a more specific exception if possible. If not, at least log the ignored exception at a WARN or DEBUG level. This will help in debugging if unexpected errors occur during test runs.

        } catch (final Exception e) {
          // Type may have been concurrently modified
          System.err.println("Warning: could not count properties for type " + type.getName() + " during side effect check: " + e.getMessage());
        }

[gemini-code-assist]

Ignoring exceptions silently, even in a teardown method, can make debugging difficult. It's better to log the exception to be aware of any cleanup issues that might affect subsequent test runs or leave artifacts behind.

Suggested change

	} catch (final Exception ignored) {
	}
	} catch (final Exception e) {
	System.err.println("Warning: failed to drop database in TCK teardown: " + e.getMessage());
	}

[claude]

OpenCypher TCK Integration - Code Review

Overview: This PR adds the official OpenCypher TCK test suite, achieving 29.6% compliance (1153/3897 tests). Excellent foundation for improving OpenCypher compatibility.

STRENGTHS:

1. Solid Architecture: Clean separation with distinct classes for parsing, matching, side-effects, and reporting
2. Code Quality: Follows project conventions (final, AssertJ, proper imports, Apache 2.0 headers)
3. Test Infrastructure: Comprehensive TCKValueParser, robust TCKResultMatcher, TCKSideEffectChecker, TCKReportPlugin
4. Maven Config: Proper test scope, smart exclusion from normal runs, clear documentation

ISSUES AND RECOMMENDATIONS:

Critical:

1. Database Cleanup (TCKStepDefinitions:84-90): Silent exception swallowing may cause flaky tests. Add logging.
2. Database Creation Race (TCKStepDefinitions:290-302): Potential race condition. Add defensive error handling.

Major:
3. Path Matching (TCKResultMatcher:263-266): Always returns true for List paths. False positives risk. Implement or document.
4. Property Counting Performance (TCKSideEffectChecker:131-147): O(N) iteration creates overhead. Consider caching.
5. Multi-label Support (TCKResultMatcher:199-207): Incomplete, likely contributes to low pass rate. Verify strategy.

Minor:
6. Float tolerance hardcoded to 1e-10
7. All procedure tests skipped
8. isNode() dead code (always false)
9. expected-failures.txt empty

TEST COVERAGE: 29.6% (1153/3897)
Focus on: expressions/null (88%), clauses/remove (75%), expressions/quantifier (74%, 604 tests)

SECURITY: No concerns identified

SUMMARY:
Solid foundational work. Well-structured with excellent visibility.

Before Merging: Fix critical cleanup/race issues, address path matching, consider performance
After Merging: Improve multi-label support, reduce expected-failures, monitor execution time

Recommendation: Approve with minor fixes - Address critical issues 1-4, then merge.

[mergify]

🧪 CI Insights

Here's what we observed from your CI run for 6993ad5.

🟢 All jobs passed!

But CI Insights is watching 👀

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -8.71%	✅ 57.50%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (bc8cf92)	101211	66680	65.88%
Head commit (811d818)	130889 (+29678)	74826 (+8146)	57.17% (-8.71%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3358)	160	92	57.50%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[codecov]

Codecov Report

❌ Patch coverage is 47.61236% with 373 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.26%. Comparing base (92d276f) to head (6993ad5).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...ery/opencypher/parser/CypherSemanticValidator.java	50.95%	195 Missing and 36 partials ⚠️
...query/opencypher/executor/CypherExecutionPlan.java	37.83%	23 Missing ⚠️
...ery/opencypher/parser/CypherExpressionBuilder.java	23.33%	14 Missing and 9 partials ⚠️
...query/opencypher/executor/ExpressionEvaluator.java	52.38%	10 Missing and 10 partials ⚠️
...ery/opencypher/executor/CypherFunctionFactory.java	0.00%	18 Missing ⚠️
...cadedb/query/opencypher/ast/LogicalExpression.java	54.05%	12 Missing and 5 partials ⚠️
...om/arcadedb/query/opencypher/ast/InExpression.java	37.50%	4 Missing and 6 partials ⚠️
...b/query/opencypher/executor/steps/OrderByStep.java	33.33%	6 Missing and 2 partials ⚠️
...db/query/opencypher/executor/steps/CreateStep.java	14.28%	4 Missing and 2 partials ⚠️
...dedb/query/opencypher/executor/steps/WithStep.java	53.84%	3 Missing and 3 partials ⚠️
... and 5 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3358      +/-   ##
==========================================
- Coverage   57.36%   57.26%   -0.10%     
==========================================
  Files        1373     1375       +2     
  Lines      101260   101906     +646     
  Branches    20656    20892     +236     
==========================================
+ Hits        58090    58360     +270     
- Misses      34024    34339     +315     
- Partials     9146     9207      +61     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

PR Review: OpenCypher TCK Integration

Overview

This PR adds the official OpenCypher Technology Compatibility Kit (TCK) test suite to ArcadeDB, including ~3,900 scenarios with a current pass rate of 29.6%. This is excellent work for bringing standardized compliance testing to the Cypher implementation.

Positive Aspects

Excellent Test Infrastructure

• Well-structured TCK integration using Cucumber/JUnit Platform Suite
• Custom report generation (TCKReportPlugin) with categorized results
• Comprehensive test utilities: TCKResultMatcher, TCKValueParser, TCKSideEffectChecker
• Proper test isolation: Excluded from normal test runs via maven-surefire configuration
• Clear documentation: Instructions in PR description for running TCK suite

Production Code Improvements

• Enhanced OpenCypher compliance with proper ternary logic (null handling)
• XOR operator support added to LogicalExpression
• Improved comparison semantics in ComparisonExpression with null propagation
• Better expression evaluation pathways

Issues and Recommendations

1. Code Quality Issues

Exception Handling Anti-Pattern

TCKStepDefinitions.java:86-89 and TCKSideEffectChecker.java:141
Silent exception swallowing without logging. Recommend adding comments explaining why exceptions can be safely ignored or add logging.

Inefficient Property Counting

TCKSideEffectChecker.java:131-147 performs full table scans for every side effect check (O(n) complexity). With large databases, this could be slow. Consider caching or using database statistics, or at minimum document the performance implications.

Complex Path Matching Logic

TCKResultMatcher.java:257-332 - The pathStructureMatches method has deeply nested loops with manual parsing. Consider extracting to a dedicated PathPatternParser class with unit tests for better maintainability.

2. Potential Bugs

Boolean Coercion Logic

LogicalExpression.java:105-110 - The toBoolean() method returns true for all non-null, non-boolean values. Verify this matches OpenCypher truthiness rules (especially for empty collections, zero values). Add documentation/tests.

Unchecked Type Casts

Multiple @SuppressWarnings("unchecked") throughout TCKResultMatcher could throw ClassCastException. Add defensive type checks or document assumptions clearly.

3. Test Coverage Concerns

Missing Tests for TCK Infrastructure

The TCK infrastructure classes (TCKValueParser, TCKResultMatcher, TCKSideEffectChecker) lack unit tests. Given their complexity, bugs here could cause false positives/negatives in TCK results.

Strongly recommend adding unit tests for:

• TCKValueParser.parse() for all value types
• TCKResultMatcher.valuesMatch() edge cases
• TCKSideEffectChecker counting logic

4. Documentation

Missing Javadoc

Complex methods lack documentation:

• TCKValueParser.parseList() - complex parsing logic
• TCKResultMatcher.pathStructureMatches() - intricate pattern matching
• TCKSideEffectChecker.assertSideEffects() - delta calculations

Expected Failures Tracking

The expected-failures.txt file is a good pattern! As you fix TCK failures, document WHY each test was failing (in git history or comments) to prevent regressions.

5. CLAUDE.md Compliance

✅ Apache 2.0 license headers present
✅ Using assertThat() syntax per guidelines
✅ No leftover System.out debug statements
⚠️ Some code doesn't follow "final on variables" guideline consistently

Recommendations Summary

High Priority

1. Add unit tests for TCK infrastructure classes - prevent false test results
2. Document/verify boolean coercion logic - ensure OpenCypher compliance
3. Add defensive type checks before unchecked casts

Medium Priority

4. Improve exception handling - log or document why ignoring is safe
5. Document performance characteristics of property counting
6. Add Javadoc to complex methods

Low Priority

7. Extract path matching to separate, testable class
8. Track expected failures with reason comments
9. Enforce final keyword per project conventions

Conclusion

This is a high-quality PR that significantly improves ArcadeDB's OpenCypher compliance verification. The test infrastructure is well-designed and the integration is clean. The 29.6% pass rate provides a clear baseline for future improvements.

Recommendation: Approve with minor revisions. Address high-priority items (especially unit tests for TCK infrastructure) before merging to ensure the test framework itself is reliable.

Great work on bringing standardized compliance testing to ArcadeDB! 🚀

---

Review generated with analysis of: pom.xml changes, all Java source files, test infrastructure, and CLAUDE.md guidelines

[claude]

Pull Request Review: OpenCypher TCK Integration

Overview

This PR integrates the official OpenCypher Technology Compatibility Kit (TCK) into ArcadeDB test suite. This is an excellent initiative that will significantly improve OpenCypher compliance. Current pass rate is 29.6% (1153/3897 tests).

---

✅ Strengths

1. Test Infrastructure Quality

The TCK integration is well-architected with clear separation of concerns:

• TCKStepDefinitions: Clean Cucumber step definitions
• TCKValueParser: Robust parser for complex TCK formats
• TCKResultMatcher: Comprehensive comparison with 3-valued logic
• TCKSideEffectChecker: Smart side-effect verification

2. Proper Three-Valued Logic (3VL)

Correctly handles Cypher ternary logic throughout:

• LogicalExpression.java:58-103: Excellent AND/OR/NOT/XOR with null propagation
• ComparisonExpression.java:95-97: Returns null when operands are null
• InExpression.java:78-95: Sophisticated IN operator with proper null semantics

3. OpenCypher Compliance Fixes

• Enhanced null handling in logical expressions
• Fixed ORDER BY sorting with better type coercion
• Improved WITH clause processing for aggregations

4. Code Quality

• Follows project conventions (no System.out, proper exception handling)
• Good use of AssertJ assertions with descriptive messages
• Clean resource management with proper database cleanup

---

🔍 Areas for Improvement

1. Performance Concerns

Issue: Inefficient Property Counting
Location: TCKSideEffectChecker.java:131-146

Problem: Iterates through ALL documents for every test that checks side effects. O(n*m) complexity.

Recommendation:

• Cache property counts at snapshot level
• Skip property verification for tests that do not modify properties
• Add configuration flag to disable for performance-critical suites

---

2. Test Isolation

Issue: Database Path Collision Risk
Location: TCKStepDefinitions.java:297

Problem: AtomicInteger may not be sufficient for parallel test execution.

Recommendation: Add thread ID or UUID for guaranteed uniqueness

---

3. Error Handling

Issue: System.err usage
Location: TCKStepDefinitions.java:367

Problem: Violates CLAUDE.md guidelines and silently continues if expected-failures.txt is missing.

Recommendation: Use proper logging and consider failing fast

---

🐛 Potential Bugs

1. String Escaping Edge Case

Location: TCKValueParser.java:246-248

Problem: Replacement order matters. Should replace backslash-backslash before backslash-quote

2. Null Comparison Semantics

Location: InExpression.java:99-100

Verify null=null returns null (not true) per OpenCypher semantics

---

🔒 Security

✅ No security issues found

• Proper transaction boundaries
• Parameterized queries
• Database cleanup in teardown

---

📊 Test Coverage

Current: 29.6% pass rate (1153/3897)
Strong: Conditional expressions (100%), null handling (88%)
Weak: Graph patterns (31%)

Missing per CLAUDE.md:

• Unit tests for TCK infrastructure classes
• TCKValueParser edge case tests
• TCKResultMatcher comparison logic tests

---

🎯 Priority Recommendations

High Priority

1. Fix TCKSideEffectChecker performance
2. Add unit tests for TCK infrastructure (required per CLAUDE.md)
3. Fix string unescape order

Medium Priority

4. Improve test isolation for parallel execution
5. Replace System.err with proper logging
6. Add defensive null checks

---

🏁 Conclusion

Verdict: Approve with Minor Changes ✅

This is high-quality work that adds significant value. The TCK infrastructure is well-designed and the 3VL fixes are excellent. The 29.6% pass rate is a solid foundation.

Recommendations:

1. Address performance concern in TCKSideEffectChecker
2. Add unit tests for TCK infrastructure
3. Fix string unescape order
4. Create follow-up issues for other items

Great work on this integration! The systematic approach to improving Cypher compliance via TCK will pay dividends.

---

Reviewed by: Claude Sonnet 4.5
Review Date: 2026-02-06

[claude]

OpenCypher TCK Integration Review

This PR introduces comprehensive TCK (Technology Compatibility Kit) testing for OpenCypher, which is an excellent step toward improving Cypher compatibility. Current pass rate of ~30% provides a clear baseline for future improvements.

✅ Strengths

1. Test Infrastructure (Excellent)

• Well-structured Cucumber integration with proper step definitions
• TCKValueParser: Robust parsing of TCK value formats (nodes, relationships, paths, primitives)
• TCKResultMatcher: Comprehensive value comparison with proper handling of numeric types, ternary logic, and graph elements
• TCKSideEffectChecker: Validates database mutations (nodes, edges, properties)
• TCKReportPlugin: Generates useful compliance reports by category
• Clean separation of concerns across test infrastructure files

2. Core Logic Improvements

• LogicalExpression.java: Proper ternary (three-valued) logic implementation for AND/OR/NOT/XOR with null handling - critical for Cypher spec compliance
• Expression builder enhancements for better operator precedence

3. Test Organization

• 242 feature files organized by category (clauses/match, clauses/create, expressions/*, etc.)
• Expected failures tracked in expected-failures.txt for managing known issues

⚠️ Issues & Recommendations

1. Code Quality & Best Practices

TCKStepDefinitions.java:

• Line 73: Using assumeTrue(false, ...) to skip tests is unconventional. Consider using @Disabled annotation or JUnit 5 assumptions more idiomatically
• Lines 84-89: Empty catch block silently ignores database cleanup errors. At minimum, log the exception
• Line 159: Hardcoded assumption that custom procedures aren't supported. Document this limitation or make it configurable

TCKResultMatcher.java:

• Line 110: Method signature uses raw type toBoolean() converting any non-null value to true. This might mask bugs - document this behavior explicitly
• Line 266: Path comparison returns true for all list-based paths (simplified). This could lead to false positives

TCKSideEffectChecker.java:

• Lines 141-143: Empty catch block silently ignoring errors during type iteration could hide concurrency issues. Add logging

LogicalExpression.java:

• Line 110: toBoolean() treats any non-null, non-Boolean value as true. Per Cypher spec, only Boolean values should be in boolean contexts. Consider adding validation

2. Potential Bugs

TCKValueParser.java:

• Line 86: Float detection regex contains("e") could match the letter 'e' in strings. Should validate it's actually scientific notation
• Line 247: String unescaping only handles \' and \\. Missing: \n, \t, \r, \b, \f, Unicode escapes. TCK tests may fail on these

TCKResultMatcher.java:

• Lines 270-332: pathStructureMatches() has complex parsing logic with nested loops and depth tracking. Edge cases with malformed patterns could cause array out of bounds. Add defensive checks
• Line 339: valuesMatch(..., ignoreListOrder: true) is recursive. Deep nested lists could cause stack overflow. Consider iterative approach or depth limit

CypherExecutionPlan.java:

• Lines 168-179: The check for hasUnwindPrecedingMatch() disables the optimizer. This is a workaround, not a fix. The root cause (optimizer doesn't handle clause ordering) should be tracked in a TODO/FIXME with issue number

3. Performance Considerations

TCKSideEffectChecker.java (Lines 131-147):

private long countAllProperties(final Database database) {
    long count = 0;
    for (final DocumentType type : database.getSchema().getTypes()) {
        if (type instanceof VertexType || type instanceof EdgeType) {
            final Iterator<? extends Document> it = database.iterateType(type.getName(), false);
            while (it.hasNext()) {
                count += it.next().getPropertyNames().size();
            }
        }
    }
    return count;
}

This iterates through all documents in the database to count properties. For large databases, this is O(n) and very slow. Consider:

• Caching property counts
• Only checking property deltas when explicitly tested
• Warning in comments about performance implications

TCKResultMatcher (Line 49):

for (final Map<String, Object> expected : expectedRows) {
    boolean found = false;
    for (int i = 0; i < remaining.size(); i++) {
        if (rowMatches(...)) { ... }
    }
}

This is O(n²) for unordered comparison. For large result sets (TCK has some tests with hundreds of rows), consider using a hash-based approach or at least document the complexity

4. Security Concerns

✅ No major security issues identified. Good practices:

• No SQL injection vectors (using parameterized queries)
• No arbitrary code execution
• Database cleanup handled properly in @After hook
• Test isolation via unique database paths

5. Test Coverage

Current: 29.6% (1153/3897 scenarios passing)

Strong areas:

• ✅ expressions/conditional: 100%
• ✅ expressions/null: 88%
• ✅ clauses/remove: 75%
• ✅ expressions/quantifier: 74%

Weak areas:

• ⚠️ Many categories below 50%
• Focus areas for improvement: expressions/aggregation, clauses/match, clauses/create

Missing test coverage:

• No unit tests for the TCK infrastructure itself (TCKValueParser, TCKResultMatcher, etc.)
• Consider adding tests for edge cases in parsing and matching logic

🔧 Actionable Recommendations

High Priority:

1. Fix empty catch blocks - add logging at minimum
2. Document the LogicalExpression.toBoolean() behavior that treats non-boolean values as true
3. Add comprehensive string escape handling in TCKValueParser.unescapeString()
4. Add issue tracking comment for the optimizer/UNWIND workaround

Medium Priority:
5. Optimize countAllProperties() or document performance implications
6. Add defensive checks for path pattern parsing edge cases
7. Consider iterative approach for deep nested list comparisons
8. Add unit tests for TCK infrastructure classes

Low Priority:
9. Replace assumeTrue(false) with more idiomatic test skipping
10. Make custom procedure support configurable rather than hardcoded
11. Document O(n²) complexity in unordered result matching

📊 Overall Assessment

Score: 8/10

This is high-quality work that significantly advances ArcadeDB's Cypher compatibility. The test infrastructure is well-designed, and the implementation demonstrates good understanding of the Cypher specification. The 30% pass rate is a solid foundation to build on.

The issues identified are mostly minor quality improvements and edge case handling. No blocking bugs were found. The main concern is performance of side-effect checking for large databases, which should be documented.

Recommendation: Approve with minor revisions. Address the high-priority items before merge.

Great work on this complex integration! 🎉

[claude]

Pull Request Review: OpenCypher TCK Integration

This PR integrates the official OpenCypher TCK into ArcadeDB. Currently achieving 29.6% pass rate (1153/3897 tests).

Strengths

• Comprehensive test coverage with 242 Gherkin feature files
• Clean architecture: TCKStepDefinitions, TCKResultMatcher, TCKValueParser, TCKSideEffectChecker
• Proper test isolation with fresh database per scenario
• Correct three-valued logic (3VL) implementation in LogicalExpression, ComparisonExpression, InExpression
• Useful TCKReportPlugin for categorized results

Code Quality Issues

1. Empty Catch Blocks (TCKStepDefinitions.java:87-88, 294-295; TCKSideEffectChecker.java:141-143)

• Add comments explaining why exceptions are ignored

2. Magic Numbers (CypherExecutionPlan.java:188, 230, 295)

• Define constant: DEFAULT_BATCH_SIZE = 100

3. Potential NullPointerException (TCKStepDefinitions.java:343-350)

• Add null check for pattern in isExpectedFailure method

4. Inefficient Property Counting (TCKSideEffectChecker.java:131-147)

• O(n) iteration acknowledged as best-effort, consider caching

Security: No critical issues found

Performance Considerations

• Creates 3897 databases (one per test) - consider pooling for CI
• Optimizer disabled for UNWIND before MATCH - add TODO comment
• Result materialization for write ops could impact memory

Potential Bugs

• Path matching parser may fail on nested patterns
• Float tolerance 1e-10 may be too strict - verify against OpenCypher spec
• Test skip strategy could mask regressions

Recommendations

High Priority:

1. Add comments to empty catch blocks
2. Add null check in isExpectedFailure
3. Document magic number constant

Medium Priority:
4. Review float comparison tolerance
5. Add TODO for optimizer issue
6. Reconsider test skip vs expected-fail

Verdict: Approve with Minor Changes

Excellent foundational work! Issues are minor and can be addressed in follow-ups.

Before merging:

• Add comments to empty catch blocks
• Add null check in isExpectedFailure
• Document magic number

Post-merge:

• Create issues for 70.4% failing tests
• Document expected-failure strategy
• Add CI job to track TCK pass rate

Great work! 29.6% provides clear baseline for improving Cypher compatibility.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -8.99%	✅ 58.15%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (92d276f)	101260	67271	66.43%
Head commit (6993ad5)	131470 (+30210)	75524 (+8253)	57.45% (-8.99%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3358)	712	414	58.15%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[claude]

Pull Request Review: OpenCypher TCK Integration

Overview

This PR integrates the official OpenCypher Technology Compatibility Kit (TCK) into ArcadeDB's test pipeline. This is an excellent addition that will significantly improve OpenCypher compliance and quality. Current pass rate: 29.6% (1153/3897 tests), with strong performance in specific areas.

---

Positive Aspects ✅

1. Excellent Foundation

• Strong pass rates in key areas:
  • 100% on conditional expressions
  • 88% on null handling
  • 75% on REMOVE clause
  • 74% on quantifiers
• Comprehensive test infrastructure with proper Cucumber/JUnit 5 integration
• Well-structured TCK support classes (TCKStepDefinitions, TCKResultMatcher, TCKValueParser, TCKSideEffectChecker)

2. Code Quality Improvements

The semantic validation additions are particularly strong:

CypherSemanticValidator.java (new file, 763 lines):

• Comprehensive multi-phase validation (variable types, bindings, scope, boolean types, aggregations)
• Well-documented with clear phase separation
• Good error messages that will help users debug queries
• Smart heuristics for detecting synthetic variable names (isValidVariableName, hasBoundaryKeyword methods)

LogicalExpression.java enhancements:

• Proper ternary logic implementation for AND/OR/NOT/XOR
• Follows Cypher's null-handling semantics correctly
• Clean switch expression usage (Java 21 feature)

3. Testing Infrastructure

• Proper use of JUnit 5 Platform Suite with Cucumber engine
• Expected failures tracking mechanism allows incremental progress
• Good test isolation with fresh database per scenario
• Comprehensive result matching with proper null/list handling

---

Issues & Recommendations 🔍

1. Critical: Test Execution Scope ⚠️

The TCK tests are currently excluded by default in pom.xml configuration. This means:

• CI/CD won't run these tests automatically
• Regression detection will be manual
• Easy to break OpenCypher compliance without noticing

Recommendation:

• Enable TCK tests in CI pipeline (perhaps as a separate job due to runtime)
• Add a Maven profile for TCK testing: mvn test -PTCK
• Document the expected pass rate and alert on regressions

2. Code Duplication in CypherExecutionPlan.java

There's significant code duplication between:

• buildExecutionStepsWithOrder() (lines 613-818)
• buildExecutionStepsLegacy() (lines 1124-1616)

Both methods handle MATCH/WITH/UNWIND/CREATE/SET/DELETE/REMOVE clauses with nearly identical logic.

Recommendation:

• Extract common clause-building logic into helper methods
• Consider a strategy pattern for clause execution step builders
• This will improve maintainability and reduce bug risk

3. Security: Incomplete DELETE Validation

In CypherSemanticValidator.java:698-704:

private void validateDeleteTargets(final DeleteClause deleteClause) {
  // DELETE targets from the parser are variable names (possibly with map access).
  // The TCK's InvalidDelete tests involve deleting labels (DELETE n:Person)
  // which would need AST-level detection, not simple string checks.
  // For now, skip this validation to avoid false positives on map access patterns.
}

This is a placeholder that doesn't validate anything. Invalid DELETE patterns could pass through.

Recommendation:

• Implement proper AST-level validation for DELETE targets
• Reject syntactically invalid patterns like DELETE n:Label
• Add test coverage for invalid DELETE cases

4. Performance: Semantic Validation on Every Query

The semantic validator runs comprehensive multi-phase validation on every query execution. While thorough, this adds overhead.

Recommendation:

• Cache validation results keyed by query text (pre-parameter substitution)
• Skip validation if query plan is cached
• Profile validation overhead to ensure it's acceptable

5. Code Style: Inconsistent Use of final

Per CLAUDE.md guidelines, the project prefers final on variables/parameters. The new TCK classes are inconsistent:

• TCKStepDefinitions.java: uses final consistently ✅
• TCKResultMatcher.java: missing final on some local variables
• CypherSemanticValidator.java: mostly consistent but some misses

Minor issue, but worth addressing for consistency.

6. Missing Test Coverage Documentation

The PR description shows current TCK results, but doesn't document:

• Which categories are priority for improvement
• Known issues preventing test passage
• Roadmap for reaching higher compliance

Recommendation:

• Add a TCK.md file documenting:
  • Current pass rates by category
  • Known limitations (e.g., procedure support)
  • Improvement roadmap
  • How to run TCK tests locally

7. Potential NPE in Variable Scope Validation

In CypherSemanticValidator.java:156-164 (WITH clause handling):

for (final ReturnClause.ReturnItem item : withClause.getItems()) {
  final String alias = item.getAlias();
  if (alias != null)
    boundVars.add(alias);
  else if (item.getExpression() instanceof VariableExpression)
    boundVars.add(((VariableExpression) item.getExpression()).getVariableName());
}

If getVariableName() returns null, this could add null to the set. Consider adding a null check.

---

Testing Recommendations 🧪

1. Build & Test Verification

Before merging, verify:

mvn clean install -DskipTests=false
cd engine && mvn test -Dtest=OpenCypherTCKRunner

2. Regression Testing

Run existing OpenCypher tests to ensure no regressions:

mvn test -Dtest="*Cypher*Test"

3. Performance Baseline

Establish performance baseline for:

• Query parsing time (with semantic validation)
• Simple query execution time
• TCK test suite runtime

---

Architecture Considerations 🏗️

Positive:

• Clean separation: TCK infrastructure is isolated in test scope
• No production code pollution with test-specific logic
• Semantic validator is reusable beyond TCK

Concern:

• The semantic validator creates tight coupling between parser/AST and validation logic
• Changes to AST structure will require validator updates
• Consider visitor pattern for more flexible validation

---

Summary

This is a high-quality PR that adds significant value to OpenCypher compliance. The implementation is thorough, well-structured, and demonstrates good understanding of OpenCypher semantics.

Required Before Merge:

1. ✅ Enable TCK tests in CI pipeline (or document why not)
2. ✅ Implement DELETE validation or add TODO ticket
3. ✅ Add TCK documentation file

Recommended Improvements:

1. Reduce code duplication in CypherExecutionPlan
2. Add validation result caching for performance
3. Add null checks in scope validation
4. Complete code style consistency

Pass Rate Target:

Current 29.6% is a solid start. I recommend setting a goal of 50% pass rate before GA, focusing on:

• Match patterns (currently weak)
• Expression handling
• Path operations

Great work on this foundational improvement! 🎉

---

Note: This review was conducted on the opencypher-tck branch. Some context may be missing from the PR diff due to the large size (20000+ lines with test feature files).

[claude]

Pull Request Review: OpenCypher TCK Integration

Thank you for implementing the official OpenCypher Technology Compatibility Kit (TCK)! This is a significant addition that will help ensure ArcadeDB's Cypher implementation meets the standard.

Positive Aspects

Excellent Testing Framework

• Well-structured Cucumber integration with clear step definitions
• Good separation of concerns: TCKStepDefinitions, TCKResultMatcher, TCKValueParser, TCKSideEffectChecker
• Proper use of JUnit 5 Platform Suite with Cucumber engine
• Smart exclusion of TCK tests from normal test runs to prevent auto-discovery

Strong Semantic Validation

The new CypherSemanticValidator (763 lines) is comprehensive and well-organized:

• Variable type conflict detection (node vs relationship vs path)
• Variable binding validation for CREATE/MERGE
• Boolean operand type checking
• Aggregation validation (nested, in WHERE)
• CREATE/MERGE/DELETE structural constraints

Code Quality

• Follows project conventions (final keywords, coding style)
• Proper error messages with context
• Good javadoc comments
• Comprehensive test infrastructure

Issues and Recommendations

1. CRITICAL - Missing Validation Call (Potential Bug)

CypherSemanticValidator.java lines 221-302 and line 44 - The validateVariableScope method is defined but NEVER called in the validate method. This means undefined variable checks are not running. Either add it to the validation chain or remove it if causing false positives.

2. Security Consideration

CypherSemanticValidator.java line 122 - When constructing error messages that include user input, ensure no risk of injection attacks if messages are logged or displayed in web interfaces. Consider sanitizing or limiting the length of user-provided identifiers.

3. Code Duplication

CypherSemanticValidator.java lines 66-104 and 133-184 - Significant duplication in traversing clauses for different validation phases. Consider extracting a common clause visitor pattern.

4. Performance Concern

CypherSemanticValidator.java lines 712-762 - The hasBoundaryKeyword method performs multiple string searches for each variable name validation. Consider caching validation results, using compiled regex, or pre-filtering.

5. Magic Numbers

TCKResultMatcher.java line 179 - The epsilon value 1e-10 for floating-point comparison should be a named constant with documentation.

6. Exception Handling

TCKStepDefinitions.java lines 86-88 - Silent exception swallowing during database cleanup could hide issues. Consider logging at DEBUG level.

7. Resource Management

TCKValueParser.java - Parser makes many substring allocations. For large result sets, consider reusing StringBuilder instances and using start/end indices.

8. Path Matching Complexity

TCKResultMatcher.java lines 269-332 - The pathStructureMatches method has high cyclomatic complexity. Consider extracting a dedicated path pattern parser and simplifying logic.

Testing Notes

Per CLAUDE.md requirements:

• Tests are included (comprehensive TCK suite)
• Should verify compilation works
• Should run TCK to verify functionality
• Should verify no existing test regressions

Dependency Management

Adds Cucumber dependencies for TCK testing (all test-scoped, MIT licensed):

• io.cucumber:cucumber-java (7.22.1)
• io.cucumber:cucumber-junit-platform-engine (7.22.1)
• org.junit.platform:junit-platform-suite (6.0.2)

Conclusion

This is a valuable addition to ArcadeDB. The code quality is good overall with well-structured test infrastructure. Main concerns:

1. Missing scope validation call (potential bug) - highest priority
2. Performance optimizations for production use
3. Code duplication that could be refactored

The 29.6% pass rate is a good starting point providing a clear roadmap for improving Cypher compliance.

Recommendation: Approve with minor changes - investigate validateVariableScope and address exception swallowing.

[claude]

Code Review: PR #3358 - OpenCypher TCK Integration

Great work integrating the official OpenCypher TCK test suite! This is a significant step toward improving ArcadeDB's Cypher compatibility. Here's my comprehensive review:

---

📊 Overview

• Lines Changed: +40,879 / -152
• Current TCK Results: 29.6% (1,153 / 3,897 tests passing)
• Verdict: ✅ APPROVE WITH MINOR CHANGES

---

🔴 Critical Issues (Must Fix Before Merge)

1. Missing Null/Bounds Checks in CypherFunctionFactory.java

Multiple functions access array indices without validation:

Lines 656-658, 698-699, 720-722 - RangeFunction, LeftFunction, RightFunction:

// Current code
final Object start = args[0];  // No bounds check!

// Should be:
if (args == null || args.length < 2)
    throw new CommandExecutionException("Function requires exactly 2 arguments");
final Object start = args[0];

Risk: ArrayIndexOutOfBoundsException if caller provides wrong number of arguments.

2. Code Style Violations in LogicalExpression.java

Per CLAUDE.md: "if statements with only one child sub-statement don't require curly braces"

Lines 71-75, 82-83, 84-85, 91-92 - Remove unnecessary braces:

// Current (lines 71-72):
if (Boolean.FALSE.equals(leftBool) || Boolean.FALSE.equals(rightBool)) {
    return false;
}

// Should be:
if (Boolean.FALSE.equals(leftBool) || Boolean.FALSE.equals(rightBool))
    return false;

---

⚠️ Medium Priority Issues

3. Error Handling in CypherExecutionPlan.java

Line 1832-1860 - evaluateIdValue() returns null silently for unsupported expression types:

default:
    return null;  // Consider logging warning or throwing exception

Recommendation: Log at WARN level or throw descriptive exception.

4. Memory Usage Documentation

Lines 205-213 - Materializes entire ResultSet for write operations:

final List<Result> buffer = new ArrayList<>();
while (internal.hasNext())
    buffer.add(internal.next());  // Loads all results into memory

Assessment: Acceptable for write operations (required for transaction safety), but should be documented with a comment explaining the intentional materialization.

---

✅ Strengths

1. Security: No issues found

   • No System.out debug statements left in code
   • No credential logging
   • Proper parameter handling

2. Test Infrastructure: Excellent design

   • Clean database isolation per scenario (TCKStepDefinitions.java:297)
   • Proper cleanup in @After hooks
   • Expected failures mechanism via assumption skipping
   • Transaction-based execution

3. CLAUDE.md Compliance:

   • ✅ Test-scoped dependencies only
   • ✅ TDD approach with tests
   • ✅ No new production dependencies
   • ✅ AssertJ syntax used
   • ✅ No Claude authorship tags

4. Semantic Validation: Comprehensive multi-phase validation in CypherSemanticValidator.java

   • Variable type checking
   • Scope management
   • Aggregation nesting detection
   • Smart AST artifact filtering

---

💡 Recommendations

Before Merge (High Priority):

1. Add null/bounds checks in CypherFunctionFactory function implementations
2. Fix brace style in LogicalExpression.java to match project conventions
3. Run mvn clean install -DskipTests to verify compilation
4. Run cd engine && mvn test -Dtest=OpenCypherTCKRunner to verify TCK execution

Post-Merge (Medium Priority):

5. Add unit tests for new semantic validation rules
6. Document memory materialization behavior in CypherExecutionPlan
7. Consider extracting CypherSemanticValidator (763 lines) into separate validator classes
8. Address TODO comments systematically

Future Improvements:

9. Performance profiling of multi-pass semantic validation
10. Continue improving TCK pass rate (target: 50%+)

---

📈 Test Coverage Assessment

Test Infrastructure Files:

• ✅ OpenCypherTCKRunner.java - Clean JUnit Platform Suite setup
• ✅ TCKStepDefinitions.java - Comprehensive Cucumber step definitions
• ✅ TCKResultMatcher.java - Result comparison logic
• ✅ TCKSideEffectChecker.java - Side effect verification
• ✅ TCKValueParser.java - Test data parsing

TCK Feature Files: 3,897 test scenarios across all Cypher clauses (MATCH, CREATE, MERGE, etc.)

---

🎯 Overall Assessment

This PR represents significant progress in OpenCypher compatibility. The test infrastructure is well-designed with proper isolation and cleanup. The production code changes are defensive enhancements to pass more TCK tests.

Risk Level: LOW - Changes are primarily test infrastructure with localized production code improvements.

The issues identified are minor and easily addressable. Once the critical null checks and style issues are fixed, this is ready to merge.

Great work on achieving 29.6% TCK compliance in the initial integration! 🚀

[lvca]

I kept TCK build separated, once all tests pass @robfrank will run it on our CI. Now it takes 47s on my laptop to execute the whole suite. This is the latest results:

=======================================================
         OpenCypher TCK Compliance Report
=======================================================
Total scenarios:  3897
Passed:           1558 (39%)
Failed:           2289 (58%)
Skipped:          50 (1%)
-------------------------------------------------------
By category:
  clauses/call                               2/ 52 passed (3%)
  clauses/create                            62/ 78 passed (79%)
  clauses/delete                            24/ 41 passed (58%)
  clauses/match                            278/381 passed (72%)
  clauses/match-where                       14/ 34 passed (41%)
  clauses/merge                             46/ 75 passed (61%)
  clauses/remove                            28/ 33 passed (84%)
  clauses/return                            32/ 63 passed (50%)
  clauses/return-orderby                    20/ 35 passed (57%)
  clauses/return-skip-limit                 19/ 31 passed (61%)
  clauses/set                               26/ 53 passed (49%)
  clauses/union                              6/ 12 passed (50%)
  clauses/unwind                            10/ 14 passed (71%)
  clauses/with                              12/ 29 passed (41%)
  clauses/with-orderBy                      61/292 passed (20%)
  clauses/with-skip-limit                    3/  9 passed (33%)
  clauses/with-where                         4/ 19 passed (21%)
  expressions/aggregation                   21/ 35 passed (60%)
  expressions/boolean                       70/150 passed (46%)
  expressions/comparison                    29/ 72 passed (40%)
  expressions/conditional                   13/ 13 passed (100%)
  expressions/existentialSubqueries          2/ 10 passed (20%)
  expressions/graph                         25/ 61 passed (40%)
  expressions/list                          79/185 passed (42%)
  expressions/literals                      75/131 passed (57%)
  expressions/map                           18/ 44 passed (40%)
  expressions/mathematical                   3/  6 passed (50%)
  expressions/null                          44/ 44 passed (100%)
  expressions/path                           0/  7 passed (0%)
  expressions/pattern                        7/ 50 passed (14%)
  expressions/precedence                    18/121 passed (14%)
  expressions/quantifier                   470/604 passed (77%)
  expressions/string                        11/ 32 passed (34%)
  expressions/temporal                       0/1004 passed (0%)
  expressions/typeConversion                19/ 47 passed (40%)
  useCases/countingSubgraphMatches           6/ 11 passed (54%)
  useCases/triadicSelection                  1/ 19 passed (5%)
=======================================================