atiaomar1978-hub commented on PR #26144: URL: https://github.com/apache/camel/pull/26144#issuecomment-5560252742
## Review summary (security, tests, architecture) _AI-generated review on behalf of [atiaomar1978-hub]_ Overall this is a solid UX improvement for CAMEL-24631 — the docs, F8 setup guide, and `LLM_BASE_URL` support are valuable. A few items should be addressed before merge. ### CI (blocking) Build is **red** on Java 17/25 (`regen.sh` / failsafe): `camel-spring-ai-image` and `camel-docling` IT failures. The new `ci-env-name` profile is the right idea (propagate `ci.env.name` to forked surefire/failsafe JVMs so `@DisabledIfSystemProperty` works), but CI still fails — please confirm the profile activates during the regen build and that those ITs are skipped as intended. Consider splitting the `parent/pom.xml` + `SpringAiImageOllamaIT` CI fix into a separate PR so CAMEL-24631 stays focused. ### Tests (blocking for Camel conventions) No automated tests were added for: - `checkOllama` / `parseOllamaModels` in `Doctor` and `DoctorPopup` - `isSmallModel()` heuristic - `LLM_BASE_URL` / `OPENAI_BASE_URL` in `tryOpenAi()` - F8 setup guide rendering path in `AiPanel` The PR test-plan checklist is entirely unchecked. Please add unit tests (see existing patterns: `DoctorTest`, `LlmClientAzureTest`, `AiPanelTest`). ### Architecture / maintainability Ollama probe + JSON parsing is duplicated in `Doctor.java` and `DoctorPopup.java`, and does not reuse existing `LlmClient.detectEndpoint()` / `listModels()` or `tryInfraOllama()` (camel infra PID files). A shared helper would avoid drift and the double localhost probe in `DoctorPopup` (`checkOllama` + `isOllamaRunning()`). ### Security **Low risk, acceptable:** localhost read-only probe; `LLM_BASE_URL` is operator-controlled config (not a framework SSRF issue). No security blockers. See inline comments for specifics. -- 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]
