CAMEL-24223: camel-jbang-mcp - Locale.ROOT cost formatting and model-specific disclaimer (follow-up) - #25167
Conversation
…tighter tests Addresses the remaining review points: - formatCost now uses Locale.ROOT so the decimal separator is always '.' (a comma separator under e.g. a French locale would break MCP clients that parse the returned strings). - The top-level disclaimer now states explicitly that token-based (LLM) components are priced as a specific model, not a provider-wide figure, and that Nova Lite / Haiku / local models (Ollama) cost far less, often ~zero — making clear these are specific-model estimates. - shouldExtractSchemesCorrectly now uses containsExactlyInAnyOrder(file, docling, aws-bedrock, aws2-s3), catching both a false "uri" capture and a dropped "file" scheme; shouldEstimateCostForAiPipeline pins the breakdown to exactly 3 priced components and asserts the computed aws-bedrock per-execution cost rather than only the count. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-targeted follow-up that addresses three review points from the predecessor PR (#25052): Locale.ROOT in formatCost prevents locale-dependent decimal separators breaking MCP clients, the disclaimer now clarifies model-specific pricing to avoid phantom-cost confusion, and test assertions are tightened to catch regressions more precisely.
Well-scoped follow-up — each change traces directly to a specific review comment on PR #25052. The expanded disclaimer text is a thoughtful balance: it gives concrete examples (Nova Lite, Haiku, Ollama) so users can calibrate, without becoming a pricing reference document.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 1 all testedMaveniverse Scalpel detected 1 affected modules (current approach: 1). Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules) Modules Scalpel would test (1)
|
Motivation
Follow-up to CAMEL-24223 / #25052, which merged before these review refinements landed. Picks up the remaining points from that PR's review.
Changes
formatCostnow usesLocale.ROOTfor both branches. PreviouslyString.format("$%.6f", cost)used the JVM default locale, so under a comma-decimal locale (e.g. French) the tool emitted$0,003000— MCP clients that parse these strings would break. This is the substantive fix.shouldExtractSchemesCorrectlynow usescontainsExactlyInAnyOrder("file", "docling", "aws-bedrock", "aws2-s3")(catches both a falseuricapture and a droppedfilescheme), andshouldEstimateCostForAiPipelinepins the breakdown to exactly 3 priced components and asserts the computedaws-bedrockper-execution cost rather than only the count.Testing
RouteCostEstimateToolsTest— 11 tests, all passing against currentmain.Main-only (4.22.0), additive — no backport.
Claude Code on behalf of Andrea Cosentino (@oscerd).
🤖 Generated with Claude Code