k-krawczyk commented on PR #25410: URL: https://github.com/apache/camel/pull/25410#issuecomment-5230550586
Thanks @atiaomar1978-hub — a genuinely useful review. All three "should fix" items and the nits are addressed in 5cdf24538 and 1c8641a5f. **1. Empty results fail open.** You were right, and this was the serious one: for an operation whose only job is gating untrusted content, silently passing the message through is the worst possible failure mode. A result count that does not match the input count now raises a `CamelExchangeException`. I did not take the "treat as flagged" option: marking content as violating when no verdict came back asserts something we do not know, and would show up as an unexplainable false positive. Failing the exchange states the truth — there is no verdict. To prove the behaviour rather than just claim it, the mock can now omit a result (`replyWithoutModerationResult()`), and two tests cover it: one through a plain route, one through the guard route asserting the message is not accepted. **2. Singleton `List` vs documented shape.** Also right. The header shape now follows the **body** shape rather than the input count: a `String` body yields `Map` headers, any `List` body yields `List` headers, including a single-element one. Tests assert the runtime type for both shapes, and the batch docs state the contract explicitly. **3. `storeFullResponse` metadata.** Extended with `CamelOpenAIModerationResponse`. **Nits.** - A `null` element in the input list is now rejected instead of moderating the literal `"null"`. - The error handling docs no longer list a missing moderation model next to the required audio models, since `moderationModel` defaults to `omni-moderation-latest`. The defensive check stays for an explicitly empty value. - The empty-list case now raises the same `IllegalArgumentException` as a missing body, and the non-`String` body test asserts the body is passed through untouched. - **Test infra compile break** — good catch, calling it out here: `OpenAIMockExpectations` gained a component and the `OpenAIMockBuilder` constructor gained a parameter. Nothing in the repository calls either directly (`camel-langchain4j-agent` and `camel-langchain4j-tools` use the fluent API), and both modules are built and tested as part of this change, but a downstream project constructing them directly would need a one-line update. - JIRA is now assigned and In Progress. **Separately, I verified the operation against the live moderation API** (5cdf24538), which corrected two things a mock alone could never have caught: - Every real response carries `category_applied_input_types`, which the mock did not emit. `Moderation.categoryAppliedInputTypes()` is non-optional, so a stored response coming from the mock would have thrown where the real one works. The mock now emits it, and the test asserts `isValid()` on the stored response so the two cannot drift apart again. - The legacy `text-moderation-*` models no longer exist — the API rejects them with a 400 — so the tests use the `omni-moderation` models that are actually served, and the note about the optional `illicit` categories now attributes them to OpenAI-compatible providers rather than older OpenAI models. The live run also confirmed the batch ordering, the 13 category names (independently cross-checked against the constants in the SDK deserialiser) and the guard route end to end. That test was local and temporary — it is not in this PR, because the component's ITs run against Ollama, which does not expose `/v1/moderations`. Current state: 16 test cases for the operation; `mvn install -Psourcecheck -DskipITs` green across `camel-openai`, `camel-test-infra-openai-mock`, `camel-langchain4j-agent` and `camel-langchain4j-tools` (202 + 49 + 42 tests). _Reported by Claude Code on behalf of Karol Krawczyk_ -- 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]
