terrymanu commented on PR #39301: URL: https://github.com/apache/shardingsphere/pull/39301#issuecomment-5150427339
Result Review Result: Mergeable Based on the latest head of PR #39301, no code correctness blockers passed the Finding Proof Gate. The algorithm catalog source, strict validation, call-chain adjustments, and test changes are consistent with the PR’s objectives. Evidence Public PR: [apache/shardingsphere#39301](https://github.com/apache/shardingsphere/pull/39301) Reviewed head: 9ebea682f374fd10ccf7cdaf2a132fe313c5b8fc GitHub authoritative change scope: 48 files, exactly matching the local diff from merge base 7c34e8571640da5c1a6098733e6f780c03ba35b0 to the reviewed head. Key changes: [PR files](https://github.com/apache/shardingsphere/pull/39301/files)Removed WorkflowQueryResult and the compatibility fallback catalog. All six algorithm categories now query the current Proxy plugin catalog directly. User-specified algorithms are strictly validated for visibility. Recommendation sources are now distinguished from Encrypt capability risk descriptions. The Completion, Resource, and Planning call chains have been updated accordingly. The DistSQL compatibility logic for rule queries remains intact; adjacent functionality was not mistakenly removed. Local verification:All 1,302 tests across the six MCP modules passed. All 6 tests in the Pipeline E2E test module passed. Spotless, Checkstyle, and git diff --check all passed. Coverage Behavior clusters reviewed:Replacing the compatibility fallback with the live Proxy plugin catalog. Algorithm visibility validation and clarification of recommendation sources and risk descriptions. Removing the shared wrapper model and updating all callers. Updating MCP and Pipeline E2E tests. Completed four review rounds covering root cause, blast radius, tests/runtime behavior, and convergence. Reviewed production code, tests, shared models, call chains, E2E paths, and indirect references. This was a Code Correctness Review limited to the code scope; GitHub Actions and CI status were not reviewed. The PR intentionally changes the behavior when plugin catalog queries are unsupported: instead of silently falling back, the query error is now surfaced directly. This is an explicitly stated design goal of the PR and does not constitute an additional blocker. -- 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]
