Croway commented on code in PR #23996:
URL: https://github.com/apache/camel/pull/23996#discussion_r3404212232
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -102,7 +121,13 @@ public void process(Exchange exchange) throws Exception {
ToolProvider toolProvider = createComposedToolProvider(tags, exchange);
String response = agent.chat(aiAgentBody, toolProvider);
- exchange.getMessage().setBody(response);
+
+ Class<?> responseType = endpoint.getConfiguration().getResponseType();
Review Comment:
**`responseType` parsing runs even when the schema was never applied to the
model.** This lookup makes `process()` parse with `responseType` regardless of
whether `resolveAndApplyStructuredOutput()` actually ran in `doInit()`. Two
reachable configurations bypass it:
1. `agent` + `agentConfiguration` both set — both options are
`autowired=true`, so a registry containing one bean of each auto-injects both.
The `agent != null` branch wins and the `ResponseFormat` is never applied, yet
the startup guard passes because `agentConfiguration != null`.
2. `agentFactory` + `agentConfiguration` + `responseType` — the
factory-created agent replaces the internal agent on every exchange and never
receives the `ResponseFormat` that `doInit()` applied to the (discarded)
internal agent.
In both cases the model gets no JSON schema, answers in free-form prose, and
`SERVICE_OUTPUT_PARSER.parseText` throws `OutputParsingException` on every
message at runtime instead of failing fast at startup. Suggest extending the
`doInit()` validation to reject `responseType` when `agent` or `agentFactory`
is configured (or fixing it at the root — see the comment on
`setResponseFormat` below).
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -102,7 +121,13 @@ public void process(Exchange exchange) throws Exception {
ToolProvider toolProvider = createComposedToolProvider(tags, exchange);
String response = agent.chat(aiAgentBody, toolProvider);
- exchange.getMessage().setBody(response);
+
+ Class<?> responseType = endpoint.getConfiguration().getResponseType();
+ if (responseType != null) {
+
exchange.getMessage().setBody(SERVICE_OUTPUT_PARSER.parseText(responseType,
response));
Review Comment:
**No error handling around parsing — raw response is lost on failure.** If
the model returns a null/blank/unparseable response (content filter, guardrail
rewrite, provider quirk), `parseText` throws a raw
`dev.langchain4j.service.output.OutputParsingException` and the original
response text is gone — `onException` handlers can't log it or route a
fallback. Consider wrapping in a Camel-friendly exception that carries the raw
response (or preserving it in a header) so routes keep an escape hatch.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -305,41 +330,55 @@ private Map<String, CamelToolSpecification>
discoverToolsByName(String tags) {
}
/**
- * Resolves the jsonSchema endpoint property: loads from
classpath/filesystem if it is a resource URI, otherwise
- * treats it as inline JSON. Converts the raw JSON string into a
langchain4j ResponseFormat and sets it on the
- * agent. Only called when the agent is created internally from
agentConfiguration.
+ * Resolves and applies structured output configuration (jsonSchema or
responseType) to the agent. Only called when
+ * the agent is created internally from agentConfiguration.
*/
- private void resolveAndApplyJsonSchema() throws Exception {
+ private void resolveAndApplyStructuredOutput() throws Exception {
String jsonSchema = endpoint.getConfiguration().getJsonSchema();
- if (ObjectHelper.isEmpty(jsonSchema)) {
+ Class<?> responseType = endpoint.getConfiguration().getResponseType();
+
+ if (ObjectHelper.isEmpty(jsonSchema) && responseType == null) {
return;
}
- String resolved =
endpoint.getCamelContext().resolvePropertyPlaceholders(jsonSchema);
+ if (responseType != null) {
+ JsonSchema schema = JsonSchemas.jsonSchemaFrom(responseType)
Review Comment:
**Non-POJO `responseType` values fail with a misleading error message.** In
langchain4j 1.16.2, `JsonSchemas.jsonSchemaFrom()` returns `Optional.empty()`
for any type whose output parser is not `PojoOutputParser` — `String`,
`Integer`, enums, `List`/`Set`, primitives. So e.g.
`responseType=java.lang.Integer` fails startup with "Ensure the class has
public fields or getters" even though `Integer` has getters; the real
restriction is POJO-only. Suggest rewording the message (e.g. "responseType
must be a POJO class; simple types, enums and collections are not supported")
so users aren't sent chasing the wrong fix.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -305,41 +330,55 @@ private Map<String, CamelToolSpecification>
discoverToolsByName(String tags) {
}
/**
- * Resolves the jsonSchema endpoint property: loads from
classpath/filesystem if it is a resource URI, otherwise
- * treats it as inline JSON. Converts the raw JSON string into a
langchain4j ResponseFormat and sets it on the
- * agent. Only called when the agent is created internally from
agentConfiguration.
+ * Resolves and applies structured output configuration (jsonSchema or
responseType) to the agent. Only called when
+ * the agent is created internally from agentConfiguration.
*/
- private void resolveAndApplyJsonSchema() throws Exception {
+ private void resolveAndApplyStructuredOutput() throws Exception {
String jsonSchema = endpoint.getConfiguration().getJsonSchema();
- if (ObjectHelper.isEmpty(jsonSchema)) {
+ Class<?> responseType = endpoint.getConfiguration().getResponseType();
+
+ if (ObjectHelper.isEmpty(jsonSchema) && responseType == null) {
return;
}
- String resolved =
endpoint.getCamelContext().resolvePropertyPlaceholders(jsonSchema);
+ if (responseType != null) {
+ JsonSchema schema = JsonSchemas.jsonSchemaFrom(responseType)
+ .orElseThrow(() -> new IllegalArgumentException(
+ "Cannot derive JSON schema from responseType: " +
responseType.getName()
+ + ".
Ensure the class has public fields or getters and is not a raw generic type."));
+ ResponseFormat derivedFormat = ResponseFormat.builder()
+ .type(ResponseFormatType.JSON)
+ .jsonSchema(schema)
Review Comment:
**Schema name diverges from the documented `camel_schema` convention.**
`JsonSchemas.jsonSchemaFrom()` names the schema `rawClass.getSimpleName()`
(verified in langchain4j 1.16.2 sources), while the `jsonSchema` branch below
deliberately uses the fixed name `camel_schema` with a comment citing
cross-component consistency with camel-openai. Migrating a route from
`jsonSchema` to `responseType` therefore silently changes the
`json_schema.name` sent to OpenAI-compatible providers from `camel_schema` to
e.g. `CarRentalRecommendation`. Consider rebuilding the derived schema with
`.name("camel_schema")` to keep both options consistent.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -56,6 +58,10 @@
public class LangChain4jAgentProducer extends DefaultProducer {
private static final Logger LOG =
LoggerFactory.getLogger(LangChain4jAgentProducer.class);
+ // ServiceOutputParser is used intentionally: beyond JSON parsing, it
strips markdown code fences
+ // (e.g. ```json ... ```) that some LLMs emit even when ResponseFormat is
explicitly set to JSON.
+ // Plain Jackson would throw a JsonParseException on such responses.
+ private static final ServiceOutputParser SERVICE_OUTPUT_PARSER = new
ServiceOutputParser();
Review Comment:
Heads-up: `ServiceOutputParser` is annotated `@Internal` in langchain4j
1.16.2 (verified via `javap`; `JsonSchemas` by contrast is public API), and
this is the only place in Camel that uses it. The code-fence-stripping
rationale in the comment is a fair tradeoff, but its signature can change
without deprecation on any langchain4j upgrade — worth keeping in mind when
bumping the dependency.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -305,41 +330,55 @@ private Map<String, CamelToolSpecification>
discoverToolsByName(String tags) {
}
/**
- * Resolves the jsonSchema endpoint property: loads from
classpath/filesystem if it is a resource URI, otherwise
- * treats it as inline JSON. Converts the raw JSON string into a
langchain4j ResponseFormat and sets it on the
- * agent. Only called when the agent is created internally from
agentConfiguration.
+ * Resolves and applies structured output configuration (jsonSchema or
responseType) to the agent. Only called when
+ * the agent is created internally from agentConfiguration.
*/
- private void resolveAndApplyJsonSchema() throws Exception {
+ private void resolveAndApplyStructuredOutput() throws Exception {
String jsonSchema = endpoint.getConfiguration().getJsonSchema();
- if (ObjectHelper.isEmpty(jsonSchema)) {
+ Class<?> responseType = endpoint.getConfiguration().getResponseType();
+
+ if (ObjectHelper.isEmpty(jsonSchema) && responseType == null) {
return;
}
- String resolved =
endpoint.getCamelContext().resolvePropertyPlaceholders(jsonSchema);
+ if (responseType != null) {
+ JsonSchema schema = JsonSchemas.jsonSchemaFrom(responseType)
+ .orElseThrow(() -> new IllegalArgumentException(
+ "Cannot derive JSON schema from responseType: " +
responseType.getName()
+ + ".
Ensure the class has public fields or getters and is not a raw generic type."));
+ ResponseFormat derivedFormat = ResponseFormat.builder()
+ .type(ResponseFormatType.JSON)
+ .jsonSchema(schema)
+ .build();
+ ((AbstractAgent<?>) agent).setResponseFormat(derivedFormat);
Review Comment:
**Design suggestion:** rather than the cast+setter, a `responseFormat` field
on `AgentConfiguration` consumed by `AbstractAgent.configureBuilder()` (which
already sources guardrails, tools, RAG and memory from the configuration) would
let factory-created agents and user-provided `AbstractAgent` subclasses receive
the format too — removing the "requires agentConfiguration" restriction at the
root and fixing the agentFactory gap noted above. The cast pattern pre-dates
this PR (CAMEL-23642 introduced it for `jsonSchema`), but this PR adds a second
cast site, so it may be the right moment to generalize.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -305,41 +330,55 @@ private Map<String, CamelToolSpecification>
discoverToolsByName(String tags) {
}
/**
- * Resolves the jsonSchema endpoint property: loads from
classpath/filesystem if it is a resource URI, otherwise
- * treats it as inline JSON. Converts the raw JSON string into a
langchain4j ResponseFormat and sets it on the
- * agent. Only called when the agent is created internally from
agentConfiguration.
+ * Resolves and applies structured output configuration (jsonSchema or
responseType) to the agent. Only called when
+ * the agent is created internally from agentConfiguration.
*/
- private void resolveAndApplyJsonSchema() throws Exception {
+ private void resolveAndApplyStructuredOutput() throws Exception {
Review Comment:
Minor: both branches of this method duplicate the
`ResponseFormat.builder().type(JSON).jsonSchema(...).build()` +
`((AbstractAgent<?>) agent).setResponseFormat(...)` epilogue, and the feature's
validation is spread over three places (top of `doInit`, inside the
`agentConfiguration` branch, and the early-return here). Deriving the
`JsonSchema` per branch and building/applying the format once — plus one
consolidated validation block — would prevent drift; the two branches have
already diverged on schema naming.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentConfiguration.java:
##########
@@ -58,10 +58,18 @@ public class LangChain4jAgentConfiguration implements
Cloneable {
@UriParam
@Metadata(description = "JSON schema for structured output validation. "
- + "This option works only when using
agentConfiguration (inline agent creation mode).",
+ + "This option works only when using
agentConfiguration (inline agent creation mode). "
+ + "Mutually exclusive with responseType.",
supportFileReference = true, largeInput = true, inputLanguage =
"json")
private String jsonSchema;
+ @UriParam
+ @Metadata(description = "Java class to use as response format for
structured outputs. "
+ + "Camel will automatically derive the JSON schema
from the class structure and unmarshal the response. "
+ + "This option works only when using
agentConfiguration (inline agent creation mode). "
+ + "Mutually exclusive with jsonSchema.")
+ private Class<?> responseType;
Review Comment:
Consistency note: camel-openai exposes the equivalent feature as
`outputClass` — a `String` resolved at request time via `ClassResolver`,
overridable per message via the `CamelOpenAIOutputClass` header, with the body
left as the raw JSON string. This option differs in name, type, dynamicity and
body semantics (auto-unmarshalled POJO), even though the PR borrows
camel-openai's `camel_schema` convention. A `Class<?>` bound at startup also
bypasses the `ClassResolver` indirection, which matters in
classloader-partitioned runtimes. Worth aligning the surface (or at least
documenting the difference) so users moving between camel-ai components aren't
surprised.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -72,15 +78,28 @@ public LangChain4jAgentProducer(LangChain4jAgentEndpoint
endpoint) {
protected void doInit() throws Exception {
super.doInit();
+ if (endpoint.getConfiguration().getResponseType() != null
+ && endpoint.getConfiguration().getAgentConfiguration() ==
null) {
+ throw new IllegalArgumentException(
+ "responseType requires agentConfiguration to be set. "
+ + "It only works when Camel
creates the agent internally (inline agent creation mode).");
+ }
+
if (endpoint.getConfiguration().getAgent() != null) {
agent = endpoint.getConfiguration().getAgent();
} else if (endpoint.getConfiguration().getAgentConfiguration() !=
null) {
+ if
(ObjectHelper.isNotEmpty(endpoint.getConfiguration().getJsonSchema())
Review Comment:
**The mutual-exclusion check is unreachable when an `agent` bean is also
configured.** Because this check is nested inside the `else if
(agentConfiguration != null)` branch, an endpoint with
`agent=#myAgent&agentConfiguration=#cfg&jsonSchema=...&responseType=...` starts
up cleanly with both "mutually exclusive" options set — `jsonSchema` is
silently dropped while `responseType` still drives response parsing. Moving the
check to the top-level validation in `doInit()` (alongside the
`responseType`-requires-`agentConfiguration` guard) would make the documented
contract hold on every path.
##########
components/camel-ai/camel-langchain4j-agent/src/main/java/org/apache/camel/component/langchain4j/agent/LangChain4jAgentProducer.java:
##########
@@ -102,7 +121,13 @@ public void process(Exchange exchange) throws Exception {
ToolProvider toolProvider = createComposedToolProvider(tags, exchange);
Review Comment:
Pre-existing issue adjacent to this change (not introduced by this PR,
noting for a follow-up JIRA): a few lines above, `agent =
agentFactory.createAgent(exchange)` writes the shared instance field from
`process()` on a singleton producer, so concurrent exchanges race — thread A
can end up chatting with thread B's factory-created agent (cross-tenant
memory/credential leakage with per-tenant factories). The fix is a local
variable: `Agent agent = agentFactory != null ?
agentFactory.createAgent(exchange) : this.agent;`.
--
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]
