fix(cypher): preserve legacy clients and classify parser errors (HG-19)

[imbajin]

Purpose of the PR

HG-19 follow-up to #3241, within #3242. Keep the response envelope compatible with existing Java clients and classify actual Cypher translator failures correctly. Semantic parser errors previously fell through to execution or unsupported-feature errors, while backend unsupported predicates could be mistaken for translator limitations.

This PR retains Nguyen Quang Minh's structured-error work and authorship from #3241. Its response metadata lives in status.attributes.errors, preserving the existing top-level fields, status and message.

ExecutionError may also cover unrecognized translation failures and does not imply traversal execution started; original messages are preserved and classification does not cover every translator failure.

Main Changes

• Recognize actual translation-1.0.4 parser syntax and semantic diagnostics; match translator unsupported features narrowly so backend execution failures keep their execution classification.
• Preserve the public two-argument response factory for existing callers and keep successful and failed responses readable by legacy Java clients.
• Align docs/cypher-api.md with the parameter support already merged in feat(server): support cypher bound queries safely #3289: JSON POST bindings, legacy raw POST, GET limits, missing/null bindings and the fixed sixteen-parameter limit.

Verifying these changes

• Trivial rework / code cleanup without any test coverage. (No Need)
• Already covered by existing tests.
• Need tests and can be verified as follows:
  • Java 17 formatting, clean full reactor compilation, and parser/request/client regression tests passed.
  • Actual parser failures and backend diagnostics reproduce the old incorrect classification and pass with the fix.
  • An unchanged caller compiled against the old response factory links successfully again; actual Java Client 1.7.0 and development 1.8.0 deserialization checks pass.

Live API, Hubble and matching release-candidate acceptance remain separate validation tasks. This change preserves the existing request processor and does not add an error-code contract for missing parameters.

Does this PR potentially affect the following parts?

• Dependencies
• Modify configurations
• The public API
• Other affects
• Nope

Documentation Status

• Doc - TODO: repository documentation is complete; paired website alignment remains with its author.
• Doc - Done
• Doc - No Need

Repository contract: docs/cypher-api.md. Existing paired website work: apache/hugegraph-doc#499.

Illustrated scope

This draft illustrates the dependent Cypher response-compatibility proposal. It does not indicate current integration readiness or replace the dependency and validation notes above.

[codecov]

Codecov Report

❌ Patch coverage is 87.17949% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.31%. Comparing base (92af2f8) to head (3d44839).
⚠️ Report is 11 commits behind head on master.

Files with missing lines	Patch %	Lines
...apache/hugegraph/api/cypher/CypherErrorMapper.java	81.48%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3259      +/-   ##
============================================
- Coverage     54.31%   53.31%   -1.00%     
+ Complexity     2250        3    -2247     
============================================
  Files           770      505     -265     
  Lines         67952    48523   -19429     
  Branches       9213     7056    -2157     
============================================
- Hits          36905    25872   -11033     
+ Misses        27470    20050    -7420     
+ Partials       3577     2601     -976     

Flag	Coverage Δ
server_memory	51.82% <87.17%> (?)
server_rocksdb	42.32% <61.53%> (?)

☔ View full report in Codecov by Harness.
📢 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.

[imbajin]

P2 — Update the Cypher parameter contract documentation to match this integrated head

Reviewed commit: e3cd2e2.

docs/cypher-api.md:6–9 says the endpoint does not accept a parameter map and that unbound expressions do not raise a missing-parameter error; the later optional-fork section (around lines 79–81) also describes binding support as not implemented. Those statements contradict the code at this head: CypherAPI.post() accepts JSON {cypher, parameters}, CypherClient.createRequest() sends ARGS_BINDINGS, and CypherOpProcessor normalizes the bindings, passes them into the AST/translation, and explicitly rejects missing parameters in validateParameters().

This can lead users to avoid supported parameterized requests or expect a missing parameter to evaluate to null. Please update the document and examples to describe the actual bound-parameter endpoint. The translation-only testParametersRemainUnbound does not exercise the processor's validation and should not be used as evidence for the REST contract; add or reference an endpoint/processor test for the supported map and missing-binding behavior.

Evidence boundary: independently cross-checked source and tests at the commit above; no live REST request was executed in this review. This is a current documentation/API-contract mismatch, unrelated to preserving pre-1.8 client compatibility.

[bitflicker64]

Blocking: yes. Score: 5.5/10 (10 - 2 Important new - 2 Important already raised - 0.5 Minor = 5.5; cap 6 for blocking). Summary: moving the metadata into status.attributes.errors keeps the existing Gremlin-style envelope that the Java client accepts, but the classifier mislabels common front-end semantic errors, and the doc still describes the pre-#3289 parameter contract. Evidence: CypherErrorTest at e3cd2e2 (mvn -o test -pl hugegraph-server/hugegraph-test -am -P unit-test -Dtest=CypherErrorTest): 8 run, 0 failures. A standalone probe through CypherAst.parse and CypherErrorMapper.map reproduced the inline misclassifications. Live CypherApiTest was not run locally; CI server_memory and server_rocksdb pass. codecov/project fails, but no status check is required on master.

Already raised by @imbajin: docs/cypher-api.md says the endpoint takes no parameter map and that unbound $name becomes null, but CypherAPI.post accepts {cypher, parameters} and CypherOpProcessor.validateParameters throws "Missing Cypher parameter". testParametersRemainUnbound only covers the translator, and testMissingParameterHasFallbackError fixes that error as ExecutionError with an empty hint.

[imbajin]

The request documentation now follows the parameter support already merged in #3289: JSON POST accepts a parameters object, GET has no parameter map, explicitly null values are valid, and missing referenced names use the existing execution-error envelope. The fixed sixteen-top-level-parameter limit is documented and linked to the runtime contract. Parser/request regression checks, legacy factory binary compatibility, actual Java Client 1.7.0 and development 1.8.0 response checks, formatting and full reactor compilation passed. Live API, Hubble and matching release-candidate acceptance remain separate gates.

[bitflicker64]

Blocking: no. Score: 7.5/10 (10 - 2 Important - 0.5 Minor). Summary: ebcd88e fixes the parser-position semantic errors, the runtime "Unsupported predicate" case and the parameter docs. The new UnsupportedFeature allow-list follows the requested prefix approach but covers only a few of the translator's own unsupported forms, so common gaps such as reduce() and SET n:Foo:Bar now return ExecutionError with an empty hint. Evidence: CypherErrorTest at e360d5e (mvn -o test -pl hugegraph-server/hugegraph-test -am -P unit-test -Dtest=CypherErrorTest): 12 run, 0 failures. A standalone probe through CypherAst.parse, translate and TranslationWriter.write with the server's default translator definition, then CypherErrorMapper.map, reproduced both inline findings. Live CypherApiTest was not run locally; CI server_memory and server_rocksdb pass, codecov/project fails but no check is required on master.

Earlier points, now fixed in ebcd88e:

• Semantic parser errors (type mismatch, redeclared variable, unaliased WITH, UNION column mismatch, duplicate columns) now map to SyntaxError.
• "Unsupported predicate" and "CONTAINS query ... is not supported" now stay ExecutionError.
• @imbajin's parameter-contract point: docs/cypher-api.md now matches CypherAPI.post and CypherOpProcessor.validateParameters.

[bitflicker64]

Important: The allow-list covers only a few of the translator's own unsupported forms, so common gaps now map to ExecutionError with an empty hint.

translation-1.0.4 throws "Unsupported : " from WalkerContext.unsupported() for about twenty descriptions; the list matches clause, match pattern and create pattern. Probe at e360d5e through the evalCypher translation path: reduce(s = 0, x IN [1,2] | s + x) gives "Unsupported value expression", SET n:Foo:Bar and REMOVE n:a:b give "Unsupported set clause", DELETE n DELETE n gives "Unsupported query. Multiple delete clauses", gremlin('g.V()') gives "Unsupported gremlin function". All are ExecutionError; at e3cd2e2 they were UnsupportedFeature. The doc disclaims completeness, but these are the most common translator gaps.

Add the remaining descriptions (value expression, expression, set clause, projection, query, gremlin function and the other unsupported() forms) and a reduce() test.

[imbajin]

Confirmed with a small probe through the actual CypherOpProcessor, native Gremlin response handlers and driver ResponseException. The remote exception types are preserved, but reduce/SET/DELETE translation gaps use the same generic UnsupportedOperationException and native response status as the backend negative control; the mapper cannot reliably infer the stage from that type alone.

The documentation now states that ExecutionError can also be a translation fallback and does not prove execution began. This does not implement complete UnsupportedFeature classification. I am keeping this thread open while the scope of using the existing translation boundary is decided, rather than extending the message allow-list or misclassifying backend errors.

[imbajin]

The user has now explicitly accepted the documented limitation for this PR, with complete translation-stage classification deferred to a separate follow-up. The current documentation states that unrecognized translation failures, including reduce and semantic rejections without a recognized position suffix, can return ExecutionError with an empty hint; that code does not prove traversal execution began, and the original message is preserved.

This is an accepted scope boundary, not an implementation fix. The mapper and processor remain unchanged, and the requested additional UnsupportedFeature coverage has not been implemented. The full-classification work is recorded separately, and I am leaving this Important thread open.