wenjin272 commented on code in PR #1178: URL: https://github.com/apache/flink-agents/pull/1178#discussion_r4152017398
########## python/flink_agents/integrations/chat_models/tests/test_ollama_multimodal_live.py: ########## @@ -0,0 +1,83 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################# +"""A real image through the Ollama connection. + +Checks that the server accepts what test_ollama_multimodal.py pins. Skipped +unless OLLAMA_VISION_MODEL names a vision model (for example qwen2.5vl:3b); +the model is pulled if the server lacks it. Mirrors the Java +OllamaMultimodalLiveTest. +""" + +import base64 +import os +import struct +import zlib + +import pytest + +from flink_agents.api.chat_message import ChatMessage, ImageBlock, TextBlock +from flink_agents.integrations.chat_models.ollama_chat_model import ( + OllamaChatModelConnection, +) + +pytestmark = pytest.mark.integration + +VISION_MODEL = os.environ.get("OLLAMA_VISION_MODEL") + + +def _client_ready() -> bool: + if not VISION_MODEL: + return False + from flink_agents.e2e_tests.test_utils import pull_model + + return pull_model(VISION_MODEL) is not None + + +def _red_square_png() -> str: + """A 16x16 red PNG.""" + rows = b"".join(b"\x00" + b"\xff\x00\x00" * 16 for _ in range(16)) + + def chunk(kind: bytes, data: bytes) -> bytes: + body = kind + data + return struct.pack(">I", len(data)) + body + struct.pack(">I", zlib.crc32(body)) + + png = ( + b"\x89PNG\r\n\x1a\n" + + chunk(b"IHDR", struct.pack(">IIBBBBB", 16, 16, 8, 2, 0, 0, 0)) + + chunk(b"IDAT", zlib.compress(rows)) + + chunk(b"IEND", b"") + ) + return base64.b64encode(png).decode() + + [email protected](not _client_ready(), reason="OLLAMA_VISION_MODEL is not set") Review Comment: Could we add multimodal tests to the existing **Java and Python E2E modules**, so they run in the `it-java` and `it-python` CI jobs? Unlike the OpenAI tests, these do not require external API credentials: both IT jobs already install and start an Ollama server. With a small vision model provisioned, we can exercise the full agent execution path with real image input in CI, rather than leave this coverage behind an opt-in environment variable. ########## integrations/chat-models/ollama/src/main/java/org/apache/flink/agents/integrations/chatmodels/ollama/OllamaChatModelConnection.java: ########## @@ -181,6 +187,10 @@ private OllamaChatMessage convertToOllamaChatMessages(ChatMessage message) { // Without the calls, the history shows tool results the model never requested. ollamaMessage.setToolCalls(toOllamaToolCalls(toolCalls)); } + final List<byte[]> images = toOllamaImages(message); Review Comment: Could we preserve the media validation exception types through the public `chat()` API? `toOllamaImages()` throws `UnsupportedContentBlockException` or `IllegalArgumentException`, but `doChat()` wraps both in a generic `RuntimeException`. I reproduced this through `chat()`, so callers cannot catch the exceptions documented by this PR, unlike the Python implementation. Please narrow the catch or rethrow runtime exceptions unchanged, and cover these cases through `chat()` rather than only `buildRequest()`. ########## integrations/chat-models/ollama/src/test/java/org/apache/flink/agents/integrations/chatmodels/ollama/OllamaMultimodalLiveTest.java: ########## @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.integrations.chatmodels.ollama; + +import org.apache.flink.agents.api.chat.messages.ChatMessage; +import org.apache.flink.agents.api.chat.messages.ImageBlock; +import org.apache.flink.agents.api.chat.messages.TextBlock; +import org.apache.flink.agents.api.resource.ResourceContext; +import org.apache.flink.agents.api.resource.ResourceDescriptor; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import javax.imageio.ImageIO; + +import java.awt.Color; +import java.awt.image.BufferedImage; +import java.io.ByteArrayOutputStream; +import java.util.Base64; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Sends a real image through the Ollama connection, to check that the server accepts what {@link + * OllamaMultimodalTest} pins. + * + * <p>Skipped unless {@code OLLAMA_VISION_MODEL} names a vision model the server already has (for + * example {@code qwen2.5vl:3b}); {@code OLLAMA_ENDPOINT} defaults to {@code + * http://localhost:11434}. + */ +class OllamaMultimodalLiveTest { + + private static final String MODEL = System.getenv("OLLAMA_VISION_MODEL"); + + @BeforeAll + static void requireModel() { + assumeTrue(MODEL != null && !MODEL.isBlank(), "OLLAMA_VISION_MODEL is not set"); + } + + @Test + @DisplayName("A base64 image is accepted") + void testImage() throws Exception { + String endpoint = System.getenv("OLLAMA_ENDPOINT"); + ResourceDescriptor descriptor = + ResourceDescriptor.Builder.newBuilder(OllamaChatModelConnection.class.getName()) + .addInitialArgument( + "endpoint", + endpoint == null || endpoint.isBlank() + ? "http://localhost:11434"; + : endpoint) + .build(); + OllamaChatModelConnection connection = + new OllamaChatModelConnection( + descriptor, ResourceContext.fromGetResource((name, type) -> null)); + Map<String, Object> params = new HashMap<>(); + params.put("model", MODEL); + + ChatMessage response = + connection.chat( + List.of( + ChatMessage.user( + List.of( + TextBlock.of( + "What color is this image? Answer in one" + + " word."), + ImageBlock.fromBase64( + "image/png", redSquarePng())))), + List.of(), + params); + + assertThat(response.getText()).isNotBlank(); + } + + private static String redSquarePng() throws Exception { Review Comment: Both Java and Python live tests failed locally with Ollama 0.17.5 and `qwen3.5:2b`: the image processor rejects the 16×16 fixture with `height:16 or width:16 must be larger than factor:32`. Changing only the image dimensions to 64×64 made both tests pass. Could we use a larger fixture? Also, both tests inherit `think=true`, while the suggested `qwen2.5vl:3b` does not support thinking. Please explicitly configure `think=false` for that model so the tests exercise image handling rather than fail on an unrelated parameter. -- 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]
