terrymanu commented on PR #38777:
URL: https://github.com/apache/shardingsphere/pull/38777#issuecomment-4602495664
### Decision
- **Merge Verdict: Mergeable**
### Basis
Reviewed Scope:
- Latest PR head: `caf5814371f21a36d3a2a9d513351ac968ceef6f`
- Merge-base used: `18aea739e4b8a0b7bcc00e22eaacac2cfe2d5aeb`
- The local reviewed file list matches GitHub `/pulls/38777/files`.
- Reviewed changed files: MCP workflow consolidation under
`.github/workflows/`, MCP release/docs updates, and LLM E2E test stability
changes under `test/e2e/mcp`.
The PR addresses the workflow responsibility split cleanly:
- `jdk21-subchain-ci.yml` is now scoped to MCP unit tests instead of
carrying MCP E2E responsibility.
- `mcp-e2e.yml` centralizes the MySQL runtime E2E path, including HTTP
contract/runtime/proxy workflow coverage and `LLMUsabilitySuiteE2ETest`.
- `mcp-release.yml` keeps the distribution and publish validation path
separate from runtime E2E.
- Obsolete LLM smoke workflow/docs references were removed or redirected to
the consolidated runtime/usability E2E path.
The Java-side stabilization is consistent with the current LLM E2E harness:
- `LLMUsabilityMetricCalculator` now ignores side-effect execution
next-actions when scoring machine-action follow-up behavior.
- This matches the runner-side intent that side-effect execution
next-actions should not be executed during the scoring path.
- Tests cover the intended counterexamples: normal tool calls still count,
while `execute` and `review-then-execute` side-effect actions are ignored.
- Scenario catalog changes also make the expected recovery/category behavior
more precise without touching production runtime code.
No blocking regression risk found. The change is limited to workflows,
documentation, and test code; no production API, SPI, SQL parser behavior, or
runtime compatibility contract is changed.
Not Reviewed Scope:
- I did not review GitHub CI/check-run results.
- I did not execute real release publishing or external registry publication.
- No dedicated security/parser/concurrency expert review appears required
for this PR; normal MCP/workflow maintainer review should be sufficient.
### Verification
Local verification completed:
- `ruby -e 'require "yaml"; ARGV.each { |file| YAML.load_file(file); puts
file }' .github/workflows/jdk21-subchain-ci.yml .github/workflows/mcp-e2e.yml
.github/workflows/mcp-release.yml`
Result: passed, exit code `0`.
- `rg` scan for obsolete workflow/test references including
`LLMSmokeE2ETest`, old MCP LLM workflow names, and obsolete job names under
`.github` and `docs`
Result: passed, exit code `0`.
- `./mvnw -pl test/e2e/mcp -am -DskipITs -Dspotless.skip=true
-Dtest=LLMUsabilityMetricCalculatorTest,LLMUsabilityScenarioCatalogTest
-Dsurefire.failIfNoSpecifiedTests=false test -B -ntp`
Result: passed, exit code `0`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
