feat(ai): standardize AI tool results as JSON#1876
Conversation
|
Do we need the |
|
Thanks for the suggestion. That makes sense since the tool name is already available from the tool I removed the The tool name is still available from the invocation context / trace metadata, but it is no longer |
|
Thanks for the update. After reviewing the current result mapping and trace flow, I found three remaining issues that should be addressed before merging:
Please also add behavior tests for duplicate column labels, SQL null values, truncation semantics, and trace rendering. The current tests only verify serialization of the outer envelope and do not exercise any of these transformations. |
|
Thanks for the detailed review. I updated the PR to address the remaining issues.
I also added behavior tests for duplicate column labels, SQL null/raw value preservation, preview Verification was rerun with
|
Remaining design concern: keep the service result boundary typed and transport-independentThanks for addressing the previous data-fidelity and trace-rendering issues. The updated payload is much safer for model consumption, but I think the result boundary is still placed at the wrong layer. Current flow
The caller then receives only the serialized, projected result. It cannot use the original typed execution result, and For Proposed responsibility splitThe service should return strongly typed business results and allow failures to propagate as typed exceptions: A possible service contract would be: List<WorkspaceDataSource> listAllDataSources(AiToolContextRequest request);
List<SimpleTable> listAllTables(AiListTablesRequest request);
List<Database> listAllDatabases(
Long dataSourceId,
AiToolContextRequest request);
List<Schema> listAllSchemas(
String databaseName,
Long dataSourceId,
AiToolContextRequest request);
List<ExecuteResponse> executeSql(AiExecuteSqlRequest request);
List<TableSchemaResult> getTablesSchema(
AiGetTablesSchemaRequest request);Here, "raw" means a lossless, strongly typed business result. It does not require exposing an internal driver object directly. For example, Error handlingService errors should not be converted into Suggested behavior:
The adapters can then map these exception types to the public tool protocol: try {
List<ExecuteResponse> result = aiToolService.executeSql(request);
AiToolResult<SqlToolData> output = converter.fromExecuteResult(result);
emitTrace(toolContext, toolName, output.getSummary());
return serializer.toJson(output);
} catch (InvalidArgumentException e) {
return failure("INVALID_ARGUMENT", e.getMessage());
} catch (SqlConfirmationRequiredException e) {
return failure("SQL_REQUIRES_MANUAL_CONFIRMATION", e.getMessage());
} catch (SqlExecutionException e) {
return failure("SQL_EXECUTION_FAILED", e.getMessage());
} catch (Exception e) {
log.error("AI tool execution failed", e);
return failure("TOOL_CALL_FAILED", "Tool execution failed.");
}This ensures that exceptions never leak directly to the model while keeping the error envelope at the transport boundary where it belongs. Typed output contract
public class AiToolResult<T> {
private boolean success;
private String summary;
private T data;
private String errorCode;
}Tool-specific payloads such as Test boundariesThe tests can then follow the same separation:
This keeps the goal of this PR, namely consistent structured tool results, while separating execution, projection, error mapping, and serialization. At minimum, JSON serialization should move out of |
|
Addressed. IAiToolService now returns typed business results instead of serialized AiToolResult JSON. AiToolServiceImpl is limited to resolving/binding context, validating inputs and SQL policy, executing the AI-specific projection now lives at the adapter/converter boundary. AiToolResultConverter builds the tool DTOs, summaries, SQL preview data, truncation metadata, and table schema payloads. AiToolAdapter The MCP boundary now uses the same failure envelope as the regular adapter for unexpected exceptions: TOOL_CALL_FAILED / Tool execution failed., without leaking internal exception messages. Verification run:
I also ran git diff --check successfully. Web focused tests covering the changed adapter/converter/MCP surface passed; the later attempt to run the full web module suite was interrupted after dependency- |
Related issue
Closes #1879
Summary
Standardize AI database tool results as structured JSON instead of free-form text.
Previously, AI tools returned plain strings with different formats across tools. The model had to
infer success, errors, metadata, schema details, and SQL result rows from natural language or table-
like text.
This PR introduces a unified
AiToolResultshape:{ "success": true, "summary": "...", "data": [], "errorCode": null }The tool name is not included in the result payload because it is already available from the tool
invocation / callback context.
This makes tool result consumption more predictable for the model and easier to extend for trace
rendering, MCP usage, and future frontend display.
Affected surfaces
Verification
mvn -B test -f chat2db-community-server/pom.xml \ -pl chat2db-community-domain/chat2db-community-domain-core -am \ -Dtest=AiToolServiceImplTest \ -Dmaven.test.skip=false \ -DskipTests=false \ -Dsurefire.failIfNoSpecifiedTests=falseResult:
BUILD SUCCESS,Tests run: 2, Failures: 0, Errors: 0, Skipped: 0.mvn -B test -f chat2db-community-server/pom.xml \ -pl chat2db-community-web -am \ -Dtest=WebClientParameterFilterTest \ -Dmaven.test.skip=false \ -DskipTests=false \ -Dsurefire.failIfNoSpecifiedTests=falseResult:
BUILD SUCCESS,Tests run: 3, Failures: 0, Errors: 0, Skipped: 0.Risk and compatibility
Public API or stored data:
AI tool return content changes from free-form text to JSON strings. This may affect model/tool
consumers that directly parse tool return text. No stored data format is changed.
Database or driver compatibility:
N/A. This PR does not change database drivers or SQL execution behavior.
Network, privacy, or security:
N/A. This PR does not change network access, credential handling, or data exposure rules.
Community / Local / Pro boundary:
N/A. This PR only changes community AI tool result formatting and does not change product boundary
behavior.
Backward compatibility:
Existing tool methods still return
String, but the string payload now follows a JSON contract.Prompts were updated to instruct the model to consume
success,summary,data, anderrorCode.Reviewer map
Start here:
AiToolResultandAiToolServiceImplresult wrapping:chat2db-community-domain-api/src/main/java/ai/chat2db/community/domain/api/model/ai/ AiToolResult.javachat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/ai/ AiToolServiceImpl.javaFailure condition:
AI tool consumers fail to handle JSON result strings, or model responses regress because prompts no
longer match tool output shape.
Rollback or disable path:
Revert this PR to restore the previous free-form string tool results.
Contributor declaration
code.