CAMEL-23393: Enforce agentic loop token budget and expose cumulative …#24902
CAMEL-23393: Enforce agentic loop token budget and expose cumulative …#24902atiaomar1978-hub wants to merge 5 commits into
Conversation
…usage Add maxAgenticTokens to cap cumulative prompt plus completion tokens across MCP agentic iterations, expose cumulative usage headers, and extend the OpenAI mock to return configurable usage metadata for tests.
gnodet
left a comment
There was a problem hiding this comment.
Claude Code review on behalf of gnodet — AI-generated review
CAMEL-23393: Enforce agentic loop token budget and expose cumulative token usage
Verdict: APPROVE ✅
Addresses a real operational concern — the MCP agentic loop was only capped by maxToolIterations (default 50), but each iteration could consume significant tokens, leading to unexpectedly large costs without any token-level guardrail.
What's good
-
Clean tracker design:
OpenAIAgenticTokenTrackeris a focused accumulator —final class, null-safeaddUsage(ChatCompletion)that handles optionalusage(), and computedgetTotalTokens(). Usinglongfor the counters is appropriate given that cumulative token counts across many iterations could exceed int range. -
Correct enforcement timing: The budget check runs after each API call (
tokenTracker.addUsage→setAgenticTokenHeaders→enforceAgenticTokenBudget). This is the right order — you can't know the token count until after the call completes. The exception prevents further iterations while allowing the response from the budget-exceeding call to be available on the exchange (the headers are set before the exception). -
Observability regardless of budget: The three cumulative headers (
CamelOpenAIAgenticPromptTokens,CamelOpenAIAgenticCompletionTokens,CamelOpenAIAgenticTotalTokens) are set on every agentic loop iteration whether or not a budget is configured. This enables monitoring and dashboarding without requiring a budget. -
Consistent error pattern:
IllegalStateExceptionwith a descriptive message matches the existingmaxToolIterationsenforcement pattern. The message includes the budget, iteration number, and per-category token breakdown — very useful for debugging. -
Backward-compatible mock changes: The
ResponseBuilderupdates maintain all existing method signatures and delegate to new overloads with defaults (DEFAULT_PROMPT_TOKENS = 10,DEFAULT_COMPLETION_TOKENS = 5). Existing tests are unaffected. -
Good test coverage: The unit test verifies accumulation, and the two integration tests cover both the observability path (headers set on success) and the enforcement path (exception with correct message when budget exceeded).
Well-implemented feature — clean, focused, and consistent with existing patterns. 👏
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
@atiaomar1978-hub would you mind leaving a Co-Authored-By in the commit message when you use AI coding assitance |
|
@davsclaus sure. |
Commit generated catalog, component schema, and endpoint DSL artifacts for maxAgenticTokens so CI clean-tree check passes. Co-Authored-By: Cursor <cursoragent@cursor.com>
gnodet
left a comment
There was a problem hiding this comment.
Re-review (3rd commit 4dc63fea): Regenerated OpenAI metadata for agentic token budget.
Standard generated metadata update — catalog JSON (openai.json), component JSON, and Endpoint DSL builder factory. Changes add the new token budget headers and options from the prior functional commit.
Approval stands. ✅
Claude Code on behalf of gnodet — AI-generated review
Add OpenAIAgenticPromptTokens, OpenAIAgenticCompletionTokens, and OpenAIAgenticTotalTokens header accessors to the generated endpoint DSL so CI clean-tree check passes. Co-Authored-By: Cursor <cursoragent@cursor.com>
gnodet
left a comment
There was a problem hiding this comment.
Re-review (4th commit 02e2532d): Regenerate endpoint DSL headers for agentic token usage.
Standard generated code — adds openAIAgenticPromptTokens(), openAIAgenticCompletionTokens(), and openAIAgenticTotalTokens() header constants to the Endpoint DSL builder. Consistent with the token budget feature from the prior commits.
Approval stands. ✅
Claude Code on behalf of gnodet — AI-generated review
|
@davsclaus would you please trigger the CI build ? |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 15 tested, 27 compile-only — current: 13 all testedMaveniverse Scalpel detected 42 affected modules (current approach: 13).
|
|
I will resolve conflicts after the merge of CAMEL-23394: Limit OpenAI conversation history size |
oscerd
left a comment
There was a problem hiding this comment.
Good addition — a cost ceiling on the agentic loop is a real operational need, and this is implemented cleanly. A couple of things I checked specifically that came out well:
- The tracker is correctly scoped.
OpenAIAgenticTokenTrackeris a local variable insideprocessNonStreamingAgentic(OpenAIProducer.java:484), not a field on the Producer or Endpoint, so a fresh instance exists per invocation and it is thread-confined to the calling thread. No token-count leakage between concurrent exchanges — which is the failure mode I was most worried about. It is also package-private, so no new public API surface. - It genuinely enforces, rather than only observing:
enforceAgenticTokenBudget(OpenAIProducer.java:620-630) throwsIllegalStateExceptionwith a descriptive message, mirroring the existingmaxToolIterationsbehaviour. - Headers follow the convention. The three new constants carry
@MetadatainOpenAIConstants, use theCamel+ component + feature PascalCase form perdesign/headers.adoc, and are therefore covered byDefaultHeaderFilterStrategy. - Off by default (
0), so no upgrade-guide entry is required. Generated files are all present and correct.
Two things I would like you to consider before this merges.
A successful final answer that crosses the budget is thrown away
enforceAgenticTokenBudget is called at OpenAIProducer.java:491, which is before requireFirstChoice at :493 and before the final-response branch at :495. So when the model returns a normal stop response with no tool calls, and that response happens to push cumulative usage over the budget, the producer throws instead of returning the answer.
The concrete case: maxAgenticTokens=1000, MCP tools configured, and the model answers directly on the first call using 1200 tokens. No agentic looping happened at all, the tokens are already spent and unrecoverable, a complete valid answer is sitting in choice — and the route fails with no body. The budget exists to prevent further spend, and after a final response there is no further spend to prevent, so throwing there costs the user their answer without saving anything.
Moving the check to after the final-response early-return, or gating it on isToolCallsFinishReason(choice), would keep the protection while not discarding work already paid for.
Enforcement is post-call, and the option description reads like a hard cap
The check runs after create() at :488, which is the pragmatic choice — you cannot know a call's token cost without a tokenizer. But it means real spend can exceed the configured budget by up to one full call, and since the prompt grows each iteration, that last call is typically the most expensive one. The description ("Maximum cumulative prompt plus completion tokens allowed across the MCP agentic loop") does not convey that overshoot. Worth a sentence.
Related, and the one project-rule gap I found: the PR touches no .adoc file. components/camel-ai/camel-openai/src/main/docs/openai-mcp.adoc:188 documents the sibling option in exactly the place this one belongs:
The
maxToolIterationsoption (default: 50) prevents infinite loops. If exceeded, anIllegalStateExceptionis thrown.
maxAgenticTokens is the direct analogue and should sit on the adjacent line, covering the default, the post-call overshoot, and how the cumulative headers differ from the existing per-call ones. The generated option/header tables will populate automatically via the included partials, so this is narrative documentation only — but CLAUDE.md does ask for doc updates where applicable, and this is applicable.
Smaller notes
- Confusable header pairs, with inconsistent declared types. In the agentic path both sets land on the same message:
setResponseHeaderssetsCamelOpenAIPromptTokens/CompletionTokens/TotalTokens(last call only) whilesetAgenticTokenHeaderssets the...Agentic...variants (cumulative). The pre-existing three are declaredjavaType = "Integer"atOpenAIConstants.java:71-76, the new three"Long", though both are populated from the samelong-returning SDK accessor. Your new declarations are the correct ones — the old ones are wrong. Not caused by this PR, but it puts the inconsistency in adjacent catalog rows, so it may be worth fixing opportunistically or filing a follow-up. - Test-infra mock now emits
usageunconditionally.ResponseBuilder.createBaseChatCompletionalways adds a usage block (10/5). Signatures are preserved, but the emitted JSON changed for every consumer, so existing mock-backed tests now getCamelOpenAIPromptTokensetc. populated where they previously were absent. Nothing in-repo asserts their absence and CI is green, butcamel-test-infra-openai-mockis a published artifact used bycamel-langchain4j-agentandcamel-langchain4j-tools, so downstream repos could see the change. An opt-in emission, or a brief note, would be safer. - Test coverage gaps. The budget test only covers "first response already over budget". Missing: accumulation crossing the budget across multiple iterations (the actual scenario in the JIRA), the boundary case where
total == maxAgenticTokensmust not throw, and an assertion that the loop actually stopped issuing requests. - Negative values silently disable the budget (
:623guards<= 0while the docs say "When 0"). Cosmetic, and consistent withmaxToolIterations. - Scope: only
processNonStreamingAgentichonours the budget; the simple and streaming paths ignore it. The description does say "across the MCP agentic loop", so it is stated — just worth being deliberate about.
None of this is a correctness bug, so I am leaving it as a comment rather than blocking. The final-answer case is the one I would most like to see addressed.
Reviewed with Claude Code (Opus 4.8) on behalf of Andrea Cosentino (@oscerd). This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Do not discard a final text response when cumulative usage exceeds maxAgenticTokens; enforce the budget only before continuing the tool loop. Document post-call overshoot and cumulative vs per-call headers, correct per-call token header types to Long, and expand budget tests. Co-authored-by: Cursor <noreply@cursor.com>
gnodet
left a comment
There was a problem hiding this comment.
Re-review: Follow-up commit (e3193e9) — Address agentic token budget review feedback
Good improvements across the board. The core logic change, documentation, and new tests are all well done.
Key changes reviewed
1. Enforcement logic placement (OpenAIProducer.java) — Correct ✅
Moving enforceAgenticTokenBudget() from before the stop-condition check to after it is the right call. If the model returns a final text answer (no more tool calls), there's no value in rejecting it — the user already paid for those tokens and no further spend will occur. Budget enforcement only makes sense as a gate before spending more tokens on additional tool calls.
2. Integer → Long for per-call token headers — Correct ✅
The OpenAI Java SDK returns long from CompletionUsage.promptTokens(), so the previous Integer annotation in @Metadata was actually wrong. Since camel-openai is unreleased (new in 4.22 SNAPSHOT), this carries no backward-compat risk.
3. New tests — Thorough ✅
The three new tests cover important scenarios:
finalAnswerOverBudgetShouldStillReturnResponse— validates the enforcement-position fixmaxAgenticTokensAtExactBoundaryShouldAllowLoopToContinue— boundary case (total == budget → continues)maxAgenticTokensShouldStopAfterMultipleIterations— multi-step accumulation withverify(weatherClient, times(1))confirming only one tool call before enforcement
4. Documentation (openai-mcp.adoc) — Good ✅
Clear explanation of enforcement timing, the distinction between per-call and cumulative headers, and the behavior when budget is exceeded.
One item to address
Catalog-level generated JSON may be stale: The component-level openai.json was correctly updated (Integer → Long, improved descriptions), but the catalog-level copy at catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/components/openai.json may not have been regenerated after this commit's metadata changes. The CI build checks for uncommitted generated file changes, so this should be caught — but worth running the build/regeneration step to sync.
Reviewed with Claude Code on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
CAMEL-23393 — Agentic loop token budget
Issue: https://issues.apache.org/jira/browse/CAMEL-23393
Branch:
CAMEL-23393-agentic-token-budgetProblem
The MCP agentic loop in
OpenAIProducer.processNonStreamingAgentic()was only capped bymaxToolIterations(default 50). A loop could still burn through hundreds of thousands of tokens as the full conversation grew on each iteration.Solution
maxAgenticTokens0(unlimited)prompt + completiontokens exceed the budget before further tool executionCamelOpenAIAgenticPromptTokensCamelOpenAIAgenticCompletionTokensCamelOpenAIAgenticTotalTokensHeaders are set whenever usage data is available, even if no budget is configured.
When the budget is exceeded after an API call that requests further tool execution, an
IllegalStateExceptionis thrown (similar tomaxToolIterations) with the iteration number and cumulative token counts. A final text response is still returned when cumulative usage exceeds the budget, because no further spend occurs after that point. Enforcement is post-call, so actual spend may exceed the configured budget by up to one API call.Review follow-up (oscerd)
openai-mcp.adocLongTests
OpenAIAgenticTokenTrackerTest— unit test for token accumulationOpenAIAgenticTokenBudgetTest— cumulative headers, budget enforcement, final-answer retention, boundary, multi-iteration stop (6 tests)