CAMEL-24333: camel-ai-tool structured tool output (outputSchema / structuredContent) - #25398
Conversation
Automated review summaryAI-generated on behalf of atiaomar1978-hub Bugbot and Grok reviews were run on this PR. Key findings and disposition: Addressed in latest commit
Acknowledged / deferred
CI checks pending on the updated branch. |
54f70d7 to
dca0c12
Compare
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for the contribution — the structured output feature itself is well-designed and follows the existing input-side patterns nicely. Good test coverage across unit, lifecycle, executor, bridge, and end-to-end MCP levels.
However, the PR carries two stale catalog regeneration artifacts that would cause serious regressions. Please rebase on current main and regenerate the catalog before this can be merged.
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Rebase on current main and restore camel-spring.xsd allowedSchemes (CAMEL-24298) and camel-xml-io.xsd targetNamespace from upstream main. Regenerated ai-tool catalog metadata without touching unrelated schemas. Addresses review feedback from davsclaus on PR apache#25398. Co-authored-by: Cursor Agent <noreply@cursor.com>
dca0c12 to
f544c52
Compare
Review feedback addressedAI-generated on behalf of atiaomar1978-hub Thanks @davsclaus for the review — both critical catalog issues are fixed in commit
What we did
Please re-review when CI is green. |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
There are uncommitted changes |
davsclaus
left a comment
There was a problem hiding this comment.
PR #25398 Review: CAMEL-24333 — Structured tool output (outputSchema / structuredContent)
Clean, well-structured PR that mirrors the existing input-side parameter.* / argSchema pattern for outputs. Backwards compatibility is properly handled via convenience constructors on records and a default method on McpServerTool.
Minor suggestions (non-blocking)
-
toPlainJsonValueinAiToolParameterHelper—Jsoner.deserializealready returnsJsonObject/JsonArray/primitives, so this method performs identity type checks without any transformation. Consider removing it and using the parsed result directly inparseStructuredOutput. -
Undocumented
outputParameter.*sub-options (inai-tool-component.adoc) — The docs describeoutputParameter.NAME=TYPEandoutputParameter.NAME.description=TEXT, but the sharedparseParameterMetadatamethod also supports.requiredand.enum. Consider either documenting these for output params or noting they're not applicable.
Positive observations
- Proper backwards compatibility:
AiToolResult.Success,McpToolCallResultrecords gain fields with convenience constructors;McpServerTool.outputSchemaJson()is a default method returningnull AiToolSpecconstructor change is acceptable —@since 4.22and 4.22 hasn't shipped; upgrade guide documents the SPI extensions- Comprehensive test coverage across parameter helper, endpoint lifecycle, executor, MCP bridge, and end-to-end Vert.x MCP client
- Good documentation in component adoc and upgrade guide
- LangChain4j and Spring AI adapters verified unaffected
- All generated artifacts (catalog, configurers, DSL) properly regenerated
- Project conventions followed (branch naming, commit format, PR body)
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 17 tested, 25 compile-only — current: 13 all testedMaveniverse Scalpel detected 42 affected modules (current approach: 13).
|
…tent) Extend ai-tool with optional outputParameter.* and outputSchema metadata, parse JSON route bodies into AiToolResult.Success.structuredContent, and map through the MCP bridge to Tool.outputSchema and CallToolResult.structuredContent. Co-authored-by: Cursor <cursoragent@cursor.com>
Reject null/blank route bodies when output schema is declared, regenerate catalog and endpoint DSL metadata, expand upgrade guide SPI notes, and add null-body test coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Rebase on current main and restore camel-spring.xsd allowedSchemes (CAMEL-24298) and camel-xml-io.xsd targetNamespace from upstream main. Regenerated ai-tool catalog metadata without touching unrelated schemas. Addresses review feedback from davsclaus on PR apache#25398. Co-authored-by: Cursor Agent <noreply@cursor.com>
Commit missing AiToolComponentBuilderFactory generated outputParameters and outputSchema builder methods so CI regen check passes. Co-authored-by: Cursor Agent <noreply@cursor.com>
…, document outputParameter sub-options - Remove redundant toPlainJsonValue() wrapper; Jsoner.deserialize already returns plain types - Document outputParameter.* sub-options (.description, .required, .enum) in component docs - Revert spurious regen of camel-spring.xsd and jbang metadata after rebase onto main Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
1e8f732 to
d65babb
Compare
|
Addressed remaining review feedback in commit d65babb:
The two critical XSD review threads (stale All related unit tests pass locally:
AI-generated on behalf of atiaomar1978-hub |
Summary
AI-generated on behalf of atiaomar1978-hub
Implements CAMEL-24333: optional structured tool output for
ai-toolroutes, mapped through the MCP server toTool.outputSchemaandCallToolResult.structuredContent.Changes
outputParameter.*(flat field syntax) andoutputSchema(raw JSON Schema), mutually exclusive — mirrors the input-sideparameter.*/argSchemapattern.AiToolResult.Success: extended with optionalstructuredContent; populated when an output schema is declared and the route body is JSON (String, Map, or List).AiToolExecutor: parses JSON bodies into structured content; null/blank/invalid JSON returnsExecutionErrorwhen output schema is declared.AiToolComponent: extractsoutputParameter.*URI params (same pattern as existingparameter.*handling).McpToolCallResult.structuredContent,McpServerTool.outputSchemaJson()default method; bridge and Vert.x engine forward both to the MCP Java SDK.value()only (typed adapter support deferred as follow-up per JIRA scope note).Testing
AiToolParameterHelperTest— output schema resolution, structured parsing, mutual exclusivityAiToolEndpointLifecycleTest— endpoint registration with output schemaAiToolExecutorTest— structured content from JSON string/Map, null body and invalid JSON errorsMcpServerBridgeTest— output schema and structured content passthroughVertxMcpServerOutputSchemaTest— end-to-end MCP client verificationDocumentation
ai-tool-component.adoc— new Structured Tool Output sectioncamel-4x-upgrade-guide-4_22.adoc— SPI extension notes (AiToolResult,McpToolCallResult,McpServerTool,AiToolSpec)Review follow-ups (addressed)
mainand fixed stale catalog regeneration artifacts (camel-xml-io.xsdnamespace,camel-spring.xsdallowedSchemesfrom CAMEL-24298)outputSchema/outputParametersExecutionError(MCP structuredContent contract)McpToolCallResult,McpServerTool,AiToolSpecnotes