chore(deps): bump gremlin.version from 3.7.4 to 3.8.0

[robfrank]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @robfrank, 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 focuses on upgrading the core Gremlin dependency to version 3.8.0. This update involves necessary code adjustments to accommodate changes introduced in the new TinkerPop version, particularly concerning error handling for type comparisons and the behavior of arithmetic operations like sum() when dealing with long overflows. The changes ensure compatibility and leverage the latest features and fixes from the Gremlin framework.

Highlights

• Dependency Upgrade: The Gremlin dependency has been updated from version 3.7.4 to 3.8.0 in the pom.xml.
• GremlinValueComparator Adaptations: The GremlinValueComparator class was modified to align with changes in TinkerPop 3.8.0. This includes replacing GremlinTypeErrorException with IllegalStateException for type comparison errors and making the comparable utility method public to allow access from TinkerPop's Compare class.
• Test Case Update for Long Overflow Behavior: The GremlinTest.java file was updated to reflect a behavioral change in TinkerPop 3.8.0. Specifically, the longOverflow test now asserts that Long.MAX_VALUE + 1 in sum() operations throws an ArithmeticException (due to overflow) instead of implicitly promoting to BigInteger.

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

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

[mergify]

🧪 CI Insights

Here's what we observed from your CI run for 9f593e7.

🟢 All jobs passed!

But CI Insights is watching 👀

[gemini-code-assist]

Code Review

This pull request updates the Gremlin dependency from version 3.7.4 to 3.8.0. The changes in the codebase are adaptations to breaking changes introduced in the new version, such as replacing GremlinTypeErrorException with IllegalStateException and updating a test for a change in long overflow behavior. The modifications appear correct and necessary for the dependency upgrade. I have one suggestion to improve code clarity in an exception handler.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 28533811	✅ 81.82%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (2853381)	Report Missing	Report Missing	Report Missing
Head commit (9f593e7)	114574	69008	60.23%

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 (#3209)	11	9	81.82%

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

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[claude]

Pull Request Review: Bump Gremlin from 3.7.4 to 3.8.0

✅ Overall Assessment

This is a well-executed dependency upgrade with appropriate compatibility fixes. The changes are minimal, focused, and properly documented.

---

📋 Code Quality & Best Practices

Strengths:

• ✅ Clean migration from deprecated GremlinTypeErrorException to IllegalStateException
• ✅ Proper visibility change (comparable method made public) to support TinkerPop 3.8.0's Compare class
• ✅ Excellent test adaptation that documents the behavioral change in TinkerPop 3.8.0
• ✅ Code follows project conventions (removed wildcard imports, properly formatted)
• ✅ Good inline documentation explaining the TinkerPop 3.8.0 behavior change

Minor observations:

• The comment at line 432 in GremlinTest.java is helpful and aligns with CLAUDE.md guidance to document changes

---

🐛 Potential Issues

None identified. The changes correctly handle:

1. Exception type migration (GremlinTypeErrorException → IllegalStateException)
2. Test expectations updated to match new overflow behavior
3. No orphaned references to the removed exception class (verified via grep)

---

⚡ Performance Considerations

Positive impact:

• The change from throwing GremlinTypeErrorException to IllegalStateException maintains the performance optimization noted in GremlinValueComparator.java:33-34 ("Throwing exceptions in Java is expensive and should be avoided")
• TinkerPop 3.8.0's stricter arithmetic overflow handling (throws ArithmeticException instead of silently promoting to BigInteger) is actually safer and more predictable

No concerns.

---

🔒 Security Concerns

No issues identified.

• TinkerPop is Apache 2.0 licensed (compatible per CLAUDE.md)
• Stricter overflow handling in 3.8.0 is actually a security improvement (prevents silent type promotion bugs)

---

🧪 Test Coverage

Excellent:

• ✅ The longOverflow() test properly validates the new behavior
• ✅ Test now expects ArithmeticException on overflow (correct for 3.8.0)
• ✅ Test verifies BigInteger explicit usage still works
• ✅ Comments explain why Groovy engine is required for this test

Recommendation:
Consider running the full Gremlin test suite to ensure no other edge cases are affected by the 3.8.0 upgrade:

mvn test -pl gremlin

---

📝 Documentation Updates Needed

ACTION REQUIRED: Update ATTRIBUTIONS.md

The file currently lists TinkerPop as version 3.7.x (lines 159-161):

| org.apache.tinkerpop | gremlin-core | 3.7.x | Apache 2.0 | https://tinkerpop.apache.org/ |
| org.apache.tinkerpop | gremlin-driver | 3.7.x | Apache 2.0 | https://tinkerpop.apache.org/ |
| org.apache.tinkerpop | gremlin-groovy | 3.7.x | Apache 2.0 | https://tinkerpop.apache.org/ |

Please update to:

| org.apache.tinkerpop | gremlin-core | 3.8.x | Apache 2.0 | https://tinkerpop.apache.org/ |
| org.apache.tinkerpop | gremlin-driver | 3.8.x | Apache 2.0 | https://tinkerpop.apache.org/ |
| org.apache.tinkerpop | gremlin-groovy | 3.8.x | Apache 2.0 | https://tinkerpop.apache.org/ |

---

✨ Summary

This is a solid upgrade that:

1. Properly handles TinkerPop API changes
2. Updates tests to match new behavioral expectations
3. Maintains code quality and project conventions

Recommendation: ✅ Approve after updating ATTRIBUTIONS.md

The only blocker is the documentation update. Once that's addressed, this PR is ready to merge.

---

🔍 Files Changed Review

File	Changes	Assessment
gremlin/pom.xml	Version bump 3.7.4 → 3.8.0	✅ Correct
GremlinValueComparator.java	Exception & visibility changes	✅ Proper API migration
GremlinTest.java	Test adapted for new behavior	✅ Well documented
ATTRIBUTIONS.md	Not updated	⚠️ Needs update

[claude]

PR Review: TinkerPop 3.8.0 Upgrade

I've reviewed this PR which upgrades TinkerPop from 3.7.4 to 3.8.0. Overall, this is a well-executed dependency upgrade with thoughtful compatibility fixes. Here's my detailed feedback:

---

✅ Strengths

1. Excellent Problem Analysis: The engine fallback logic in ArcadeGremlin.java correctly addresses TinkerPop 3.8.0's stricter parameter placement restrictions in the gremlin-lang grammar.

2. Smart Architecture: Extracting getEffectiveEngine() as a protected method allows ArcadeCypher to cleanly override it - good OOP design.

3. Clear Documentation: The commit messages thoroughly explain the breaking changes and rationale for each fix.

4. Appropriate Fallback Strategy: The logic to try Java engine first, then fallback to Groovy for parameterized queries is pragmatic given TinkerPop's grammar limitations.

---

🔍 Code Quality Observations

1. Field Ordering Issue (Minor Style)

In ArcadeCypher.java, the getEffectiveEngine() override appears before the field declarations. Methods should come after fields per Java conventions.

Recommendation: Move the method after field declarations to match existing codebase style.

2. Fallback Logic Could Be Clearer (Readability)

The condition in ArcadeGremlin.java:206 implements this behavior:

• engine=java + no parameters → throw exception (no fallback)
• engine=java + parameters → fallback to Groovy
• engine=auto → always fallback on error

Recommendation: Consider adding a comment explaining why parameterless java-mode queries shouldn't fallback (to catch actual syntax errors early).

3. Security Warning on Every Groovy Query (Performance)

The Groovy engine logs a security warning at Level.WARNING on every query execution (line 236-239). For Cypher queries (which always use Groovy), this could generate substantial log spam.

Recommendations:

• Consider logging this warning only once per database/session using a static flag
• Or reduce to Level.INFO for explicit user opt-in (when gremlinEngine=groovy), keeping WARNING only for auto-fallback cases
• Document in CLAUDE.md that Cypher queries inherently use Groovy and this is expected

---

🧪 Test Coverage Concerns

Missing Test Cases

I couldn't find regression tests for the new fallback logic:

1. Java Engine with Parameters: Test that parameterized queries correctly fallback from Java → Groovy
2. Java Engine Strict Mode: Test that non-parameterized queries in engine=java mode properly throw exceptions on syntax errors
3. Auto Mode Fallback: Test that engine=auto tries Java first, then falls back
4. Cypher Always Uses Groovy: Test that ArcadeCypher correctly overrides to use Groovy engine

Recommendation: Add regression tests to ensure this critical logic doesn't break in future refactors. This follows the CLAUDE.md requirement: "all new server-side code must be tested with a test case".

---

🔒 Security Considerations

✅ Acceptable: The continued use of Groovy for Cypher is justified since the opencypher-gremlin translator generates Groovy-style Gremlin. However:

• Document the risk: Add a note to the documentation that Cypher queries have the same RCE risk as Groovy Gremlin
• Consider sandboxing: Long-term, investigate if the opencypher-gremlin library can target the Java engine instead

---

⚡ Performance Considerations

1. Double Parsing on Fallback: When a parameterized query fails in Java engine, it's parsed twice (once fails, once succeeds in Groovy). This is acceptable for now but could be optimized with a heuristic check for parameters before attempting Java engine.

2. No Regression Risk: The fallback logic only triggers on parse exceptions, so successfully parsing queries have no performance impact.

---

📝 Suggestions

1. Update ATTRIBUTIONS.md: Verify that TinkerPop 3.8.0 license info is current
2. Update Documentation: Add a note about the Java/Groovy engine behavior with parameters
3. Remove Debug Output: Ensure no System.out statements were added (couldn't find any in the diff)

---

✅ Approval Recommendation

This PR is approved pending minor improvements:

Required:

• Add regression tests for engine fallback logic
• Fix field ordering in ArcadeCypher.java

Recommended:

• Reduce log spam from Groovy security warnings (throttle or reduce level)
• Add inline comment explaining java-mode no-fallback behavior

Great work on handling a non-trivial dependency upgrade! The fallback strategy is pragmatic and the code changes are minimal and focused.

---

Review conducted per CLAUDE.md guidelines. Build status: CI checks still in progress at time of review.

[claude]

PR Review: Gremlin 3.7.4 → 3.8.0 Upgrade

Summary

This PR upgrades Apache TinkerPop Gremlin from version 3.7.4 to 3.8.0 and introduces smart engine selection logic to handle compatibility issues with the new version.

---

✅ Strengths

1. Proper Attribution Updates: ATTRIBUTIONS.md correctly updated with new version numbers and added missing dependencies (gremlin-server, gremlin-util)

2. Smart Fallback Logic: The enhanced engine selection in ArcadeGremlin.executeStatement() handles TinkerPop 3.8.0's stricter parameter placement rules gracefully:

   • Tries Java engine first when configured as "auto" or "java"
   • Falls back to Groovy only when necessary (parameterized queries in auto mode)
   • Maintains strict Java-only mode when explicitly configured without parameters

3. Cypher Compatibility: The ArcadeCypher.getEffectiveEngine() override correctly forces Groovy engine since Cypher-to-Gremlin translation produces Groovy-style code

4. Good Documentation: Comments explain the rationale for changes (grammar restrictions, translation requirements)

---

🔍 Issues & Concerns

1. Security Warning May Fire Incorrectly ⚠️

Location: ArcadeGremlin.java:235-239

The security warning about Groovy RCE vulnerabilities will now fire for ALL Cypher queries, even in production environments where Cypher is a legitimate use case. This is because ArcadeCypher always uses Groovy engine.

Recommendation: Refactor the warning to distinguish between:

• Direct Groovy usage (security risk)
• Necessary Groovy usage for Cypher translation (expected behavior)

// In ArcadeGremlin.executeStatement(String):
} else if ("groovy".equals(gremlinEngine)) {
  if (!(this instanceof ArcadeCypher)) {  // Only warn for direct Gremlin queries
    LogManager.instance().log(this, Level.WARNING,
        "SECURITY WARNING: Using insecure Groovy Gremlin engine...");
  }
  // ... rest of code

2. Method Ordering Inconsistency

Location: ArcadeCypher.java:53-58

The getEffectiveEngine() override appears BEFORE the static field declarations. This breaks the typical Java code organization pattern.

Recommendation: Move the override method after the constructor, following standard Java conventions:

- static fields
- instance fields  
- constructors
- methods

3. Test Coverage Questions

Missing verification:

• Are there tests covering the new fallback behavior (auto mode → Java fails → Groovy succeeds)?
• Are there tests ensuring strict Java mode properly rejects incompatible queries?
• Have existing Gremlin and Cypher tests been run to verify no regressions?

Recommendation: Run the test suite and confirm:

mvn test -pl gremlin -Dtest="GremlinTest,CypherTest,CypherQueryEngineTest"

4. Parameter Placement Grammar Changes 📚

The comment mentions "TinkerPop 3.8.0 restricted parameter placement in gremlin-lang grammar" but doesn't document what these restrictions are.

Recommendation: Add a comment with an example of what types of queries will fail in Java engine vs. Groovy engine to help future maintainers understand when fallback occurs.

---

🎯 Performance Considerations

Positive: The new logic attempts Java engine first, which is more secure and potentially faster than Groovy.

Concern: The try-catch fallback for parameterized queries adds overhead. However, this is acceptable since:

• It only affects the first parse attempt
• The alternative (always using Groovy for safety) would be worse
• The fallback path is well-documented

---

🔒 Security Analysis

Good:

• Maintains existing security warnings for Groovy engine
• Prefers secure Java engine when possible
• Explicit handling of different modes

Needs attention:

• Cypher queries will spam security warnings (see issue build support #1 above)
• Consider documenting security implications in CLAUDE.md or project docs

---

📋 Testing Checklist

Before merging, please verify:

• All existing Gremlin tests pass
• All existing Cypher tests pass
• Test parameterized queries in both Java and Groovy modes
• Test strict Java mode behavior
• Verify no security warning spam for legitimate Cypher usage
• Integration tests with the new Gremlin version
• Performance testing (if applicable)

---

💡 Recommendations

1. High Priority: Fix security warning for Cypher queries (Issue build support #1)
2. Medium Priority: Reorder method in ArcadeCypher for code consistency
3. Low Priority: Add documentation about parameter placement restrictions
4. Required: Confirm all tests pass before merging

---

Verdict

Conditional Approval: The core upgrade logic is sound and well-reasoned, but the security warning issue for Cypher needs to be addressed before merging to avoid log pollution and confusion in production environments.

Once the Cypher warning issue is fixed and tests are confirmed passing, this would be ready to merge.

[codecov]

Codecov Report

❌ Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.43%. Comparing base (9bf6260) to head (9f593e7).
⚠️ Report is 26 commits behind head on main.

Files with missing lines	Patch %	Lines
.../main/java/com/arcadedb/gremlin/ArcadeGremlin.java	50.00%	2 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3209      +/-   ##
==========================================
- Coverage   62.44%   62.43%   -0.02%     
==========================================
  Files        1271     1271              
  Lines       85013    85016       +3     
  Branches    17479    17479              
==========================================
- Hits        53084    53077       -7     
- Misses      24415    24425      +10     
  Partials     7514     7514              

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