CAMEL-24538: camel-openai - fix operator precedence so an empty userMessage does not drop the body prompt - #25939
Open
oscerd wants to merge 1 commit into
Open
CAMEL-24538: camel-openai - fix operator precedence so an empty userMessage does not drop the body prompt#25939oscerd wants to merge 1 commit into
oscerd wants to merge 1 commit into
Conversation
…essage does not drop the body prompt buildUserMessage used `userPrompt == null || userPrompt.isEmpty() && isNotEmpty(config.getUserMessage())`. Because && binds tighter than ||, this is `userPrompt == null || (userPrompt.isEmpty() && ...)`, so when the USER_MESSAGE header is absent and the configured userMessage is an empty string, userPrompt was set to that empty string and buildTextMessage then used "" instead of the message body, failing with "No input provided". Parenthesize as `(userPrompt == null || userPrompt.isEmpty()) && isNotEmpty(...)` so the configured message is only used when it is actually set, otherwise the body is used. The same precedence is corrected for the (benign) system-message twin for consistency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
reviewed
Aug 31, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Review
Verified beyond the diff:
- Checked out the PR branch and ran
OpenAIEmptyUserMessageBodyPromptTest— passes. - Reverted just the
userPromptprecedence fix locally and re-ran the same test — it fails with exactly the reported symptom (IllegalArgumentException: No input provided to LLM), confirming the test genuinely exercises the bug rather than passing vacuously. Restored the fix afterward. - Confirmed JIRA CAMEL-24538 is
Bug,In Progress, correctly assigned to the author. - Checked git blame on
OpenAIProducer.java— the buggy precedence has no intentional design history; it's leftover boilerplate from several unrelated feature commits, not a deliberate choice, so this fix doesn't revert anything intentional. - Confirmed the
systemPromptcompanion fix is behavior-neutral today (masked by the downstreamObjectHelper.isNotEmpty(systemPrompt)guard before the message is added) — the PR's own reasoning for touching it "for consistency" checks out.
Suggestion (non-blocking)
The identical precedence bug still exists a few lines below the systemPrompt fix, in the developerPrompt branch (outside this diff's hunk, so I can't anchor an inline suggestion to it):
// components/camel-ai/camel-openai/.../OpenAIProducer.java, buildMessages()
if (developerPrompt == null
|| developerPrompt.isEmpty() && ObjectHelper.isNotEmpty(config.getDeveloperMessage())) {
developerPrompt = config.getDeveloperMessage();
}should become:
if ((developerPrompt == null || developerPrompt.isEmpty()) && ObjectHelper.isNotEmpty(config.getDeveloperMessage())) {
developerPrompt = config.getDeveloperMessage();
}It's currently harmless (masked by the downstream ObjectHelper.isNotEmpty(developerPrompt) guard before the message is added), same as the systemPrompt case this PR already fixed "for consistency." Since two of three identical instances are being cleaned up here, it seems like an oversight to leave this third one — worth completing in the same pass, or in a fast follow-up.
Good fix, well tested, no blocking concerns.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Contributor
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
CAMEL-24538
Problem
OpenAIProducer.buildUserMessageresolves the user prompt with:&&binds tighter than||, so this parses asuserPrompt == null || (userPrompt.isEmpty() && configHasMessage).When the
CamelOpenAIUserMessageheader is absent (userPrompt == null) and the configureduserMessageoption is an empty string, the first branch is already true, so
userPromptis set to that emptystring.
buildTextMessagethen doesuserPrompt != null ? userPrompt : body, picks the empty string overthe message body, and the request fails with "No input provided to LLM" — the body prompt is silently
dropped.
Fix
Parenthesise as
(userPrompt == null || userPrompt.isEmpty()) && ObjectHelper.isNotEmpty(config.getUserMessage())so the configured message is only substituted when it is actually set; otherwise
userPromptstays null andthe body is used. The identical precedence in the system-message branch is corrected too for consistency
(that one was benign because its downstream check uses
isNotEmpty).Testing
OpenAIEmptyUserMessageBodyPromptTestconfigures the endpoint with an emptyuserMessageand assertsthe body prompt still reaches the model. Verified it fails with "No input provided" against the
unpatched code and passes with the fix.
mvn -Psourcecheck validategreen.Claude Code on behalf of oscerd