fix(model): add DataBlock handling in OpenAIMessageConverter by Sparkle6979 · Pull Request #1744 · agentscope-ai/agentscope-java

Sparkle6979 · 2026-06-13T18:04:38Z

Background

DataBlock was introduced in 2.0 as the forward-looking multimodal container (image/audio/video), replacing ImageBlock/AudioBlock/VideoBlock. Its Javadoc states:

New code should prefer DataBlock over the legacy subclasses.

However, none of the four provider converters process DataBlock — it is silently dropped during message formatting. It is unclear whether this is a bug (the converter side was simply forgotten) or an intentional work-in-progress (the feature was merged before it was fully wired up).

Reported as #1693.

What this PR does

Adds DataBlock handling to OpenAIMessageConverter, which covers all providers that extend OpenAIBaseFormatter (OpenAI, DeepSeek, GLM, and any OpenAI-compatible API):

hasMediaContent(): recognize DataBlock as media content
convertContentBlocks(): infer MIME type from source (Base64Source.getMediaType() / URLSource.getUrl() + MediaUtils.determineMediaType()), then route to the appropriate image_url / input_audio / video_url content part
Extracted shared helper methods (addTextPart / addImagePart / addAudioPart / addVideoPart / addDataPart) to avoid duplicating logic between legacy blocks and the new DataBlock path

Not covered

Converters for Gemini, DashScope, Anthropic, and Ollama have the same gap — they also need DataBlock support. This PR only addresses the OpenAI-compatible path.

Note

DataBlock appears to be a half-finished feature — introduced in the message model but never wired up on the converter side. I am not sure whether the maintainers are already working on completing this internally. If so, or if a different approach is preferred, I am happy to adjust or close this PR.

DataBlock is the 2.0-forward multimodal container (image/audio/video), but convertContentBlocks() never handled it — DataBlock media was silently dropped from API requests. - Add DataBlock branch in convertContentBlocks(): infer MIME type from source (Base64Source.getMediaType / URLSource.getUrl + MediaUtils), then route to image_url / input_audio / video_url content parts - Add DataBlock to hasMediaContent() so multimodal formatting path is used - Extract addTextPart/addImagePart/addAudioPart/addVideoPart to eliminate duplication between legacy blocks and the new DataBlock path - Add DataBlockConverterTest: URL and Base64 source coverage Fixes agentscope-ai#1693

codecov · 2026-06-13T18:21:49Z

Codecov Report

❌ Patch coverage is 50.00000% with 40 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../core/formatter/openai/OpenAIMessageConverter.java	50.00%	30 Missing and 10 partials ⚠️

📢 Thoughts on this report? Let us know!

AgentScopeJavaBot

🤖 AI Review

This PR addresses issue #1693 by adding DataBlock support to OpenAIMessageConverter, filling a gap where the new polymorphic media container was silently dropped during message formatting. The approach is sound: infer the MIME type from the source (explicit mediaType on Base64Source, extension-based detection on URLSource), then route to the appropriate image_url / input_audio / video_url content part via extracted helper methods. The refactoring of legacy ImageBlock/AudioBlock/VideoBlock handling into dedicated addImagePart / addAudioPart / addVideoPart helpers is clean and behavior-preserving. Test coverage is reasonable for the happy paths, though a few edge cases are missing.

AgentScopeJavaBot · 2026-06-14T01:39:38Z

+        }
+    }
+
+    private void addImagePart(Source source, List<OpenAIContentPart> parts) {


[major] When MediaUtils.determineMediaType() returns "application/octet-stream" (e.g., a URLSource whose URL has no recognizable file extension like https://cdn.example.com/stream), the DataBlock is silently dropped with only a log.warn. This is inconsistent with how legacy blocks handle failures — they add a text placeholder (e.g., [Image - processing failed: ...]). Users migrating to DataBlock (as the Javadoc recommends) may not notice the warning and will see their content silently vanish from the message. Consider adding a text placeholder in the else branch:

} else { log.warn("DataBlock has unrecognized MIME type '{}', skipping", mimeType); addTextPart("[Data - unrecognized type: " + mimeType + "]", parts); }

Fixed: unrecognized MIME types now produce a text placeholder like [Data - unrecognized type: ...] instead of being silently dropped, consistent with how ImageBlock/AudioBlock failures are handled.

✅ Verified as addressed in 0af6b61: addDataPart else-branch now calls addTextPart with '[Data - unrecognized type: ...]' placeholder, exactly matching the bot's suggestion.

AgentScopeJavaBot · 2026-06-14T01:39:38Z

+        } else {
+            log.warn("DataBlock has unknown source type: {}", source.getClass().getSimpleName());
+            return;
+        }


[minor] MediaUtils.determineMediaType() relies solely on file-extension matching. A DataBlock with a URLSource pointing to a URL without a recognizable extension (e.g., https://api.example.com/media/12345) will be routed to the application/octet-stream path and dropped. For Base64Source, the explicit mediaType field handles this correctly. Consider documenting this limitation in the addDataPart Javadoc or adding a comment, so callers understand that URLSource-based DataBlocks require URLs with standard media extensions.

Acknowledged. This is an inherent limitation of extension-based MIME detection. Added a comment in addDataPart noting that URLSource-based DataBlocks require URLs with recognizable file extensions.

AgentScopeJavaBot · 2026-06-14T01:39:38Z

+                                                        .contains("https://example.com/song.mp3"));
+        assertTrue(hasAudioText, "URL-based audio should produce a text placeholder");
+    }
+}


[minor] Missing test coverage for two edge cases:

Unrecognized MIME type: A DataBlock with a URLSource whose URL has no recognizable extension (e.g., https://example.com/data). This exercises the application/octet-stream / else branch in addDataPart.

Base64Source audio via DataBlock: A DataBlock with a Base64Source of type audio/wav. This exercises the Base64Source → addAudioPart → input_audio path, which is the most important happy path for audio.

Adding these two tests would significantly improve confidence in the new code paths.

Good catch — these edge cases are already covered in the latest commit (testDataBlockUnknownMimeType and testDataBlockBase64Audio).

✅ Verified as addressed in 0af6b61: Null-source check is now at the top of addDataPart (lines 373-375) with early return and log.warn, cleanly encapsulating the guard instead of using continue in convertContentBlocks.

AgentScopeJavaBot · 2026-06-14T01:39:38Z

+        parts.add(OpenAIContentPart.text(text));
+    }
+
+    private void addDataPart(Source source, List<OpenAIContentPart> parts) {


[nit] addTextPart is a trivial one-liner wrapper (parts.add(OpenAIContentPart.text(text))) that adds indirection without the null-check / try-catch / error-handling value that the other addXxxPart methods provide. It's not wrong, but it's asymmetric. Consider either inlining it back (as it was before), or adding a null-check on text to make it consistent with the defensive style of the other helpers.

Fair point. Kept as-is for now — the wrapper provides consistency with the other addXxxPart methods at the call site (every branch uses the same pattern).

✅ Verified as addressed in 0af6b61: Nit explicitly acknowledged and deferred by developer; bot itself stated the current if-chain works fine, so conscious deferral is a valid resolution.

AgentScopeJavaBot · 2026-06-14T01:39:38Z

+
+class DataBlockConverterTest {
+
+    @Test


[praise] Good test design — the tests directly exercise the OpenAIChatFormatter end-to-end rather than mocking internals. The assertions verify both the structure (List) and the content (correct type, correct URL/data) of the output. The testDataBlockAudioUrlFallback test correctly verifies the fallback-to-text-placeholder behavior for URL-based audio.

✅ Verified as addressed in 0af6b61: This is a praise comment requiring no code change; developer acknowledged the feedback.

AgentScopeJavaBot

🤖 AI Review

This PR addresses issue #1693 by adding DataBlock support to OpenAIMessageConverter, filling a gap where the new polymorphic media container was silently dropped during message formatting. The approach is sound: infer the MIME type from the source (explicit mediaType on Base64Source, extension-based detection on URLSource), then route to the appropriate image_url / input_audio / video_url content part via extracted helper methods. The refactoring of legacy ImageBlock/AudioBlock/VideoBlock handling into dedicated addImagePart / addAudioPart / addVideoPart helpers is clean and behavior-preserving. Test coverage is reasonable for the happy paths, though a few edge cases are missing.

AgentScopeJavaBot · 2026-06-14T01:40:13Z

+        }
+    }
+
+    private void addImagePart(Source source, List<OpenAIContentPart> parts) {


[major] When MediaUtils.determineMediaType() returns "application/octet-stream" (e.g., a URLSource whose URL has no recognizable file extension like https://cdn.example.com/stream), the DataBlock is silently dropped with only a log.warn. This is inconsistent with how legacy blocks handle failures — they add a text placeholder (e.g., [Image - processing failed: ...]). Users migrating to DataBlock (as the Javadoc recommends) may not notice the warning and will see their content silently vanish from the message. Consider adding a text placeholder in the else branch:

} else { log.warn("DataBlock has unrecognized MIME type '{}', skipping", mimeType); addTextPart("[Data - unrecognized type: " + mimeType + "]", parts); }

✅ Verified as addressed in 0af6b61: Same issue as earlier thread; addDataPart else-branch now adds '[Data - unrecognized type: ...]' text placeholder instead of silently dropping.

AgentScopeJavaBot · 2026-06-14T01:40:13Z

+        } else {
+            log.warn("DataBlock has unknown source type: {}", source.getClass().getSimpleName());
+            return;
+        }


[minor] MediaUtils.determineMediaType() relies solely on file-extension matching. A DataBlock with a URLSource pointing to a URL without a recognizable extension (e.g., https://api.example.com/media/12345) will be routed to the application/octet-stream path and dropped. For Base64Source, the explicit mediaType field handles this correctly. Consider documenting this limitation in the addDataPart Javadoc or adding a comment, so callers understand that URLSource-based DataBlocks require URLs with standard media extensions.

AgentScopeJavaBot · 2026-06-14T01:40:13Z

+                                                        .contains("https://example.com/song.mp3"));
+        assertTrue(hasAudioText, "URL-based audio should produce a text placeholder");
+    }
+}


[minor] Missing test coverage for two edge cases:

Unrecognized MIME type: A DataBlock with a URLSource whose URL has no recognizable extension (e.g., https://example.com/data). This exercises the application/octet-stream / else branch in addDataPart.

Base64Source audio via DataBlock: A DataBlock with a Base64Source of type audio/wav. This exercises the Base64Source → addAudioPart → input_audio path, which is the most important happy path for audio.

Adding these two tests would significantly improve confidence in the new code paths.

✅ Verified as addressed in 0af6b61: Same fix as earlier thread; addDataPart handles null source with early return and log.warn at lines 373-375.

AgentScopeJavaBot · 2026-06-14T01:40:13Z

+        parts.add(OpenAIContentPart.text(text));
+    }
+
+    private void addDataPart(Source source, List<OpenAIContentPart> parts) {


[nit] addTextPart is a trivial one-liner wrapper (parts.add(OpenAIContentPart.text(text))) that adds indirection without the null-check / try-catch / error-handling value that the other addXxxPart methods provide. It's not wrong, but it's asymmetric. Consider either inlining it back (as it was before), or adding a null-check on text to make it consistent with the defensive style of the other helpers.

AgentScopeJavaBot · 2026-06-14T01:40:13Z

+
+class DataBlockConverterTest {
+
+    @Test


[praise] Good test design — the tests directly exercise the OpenAIChatFormatter end-to-end rather than mocking internals. The assertions verify both the structure (List) and the content (correct type, correct URL/data) of the output. The testDataBlockAudioUrlFallback test correctly verifies the fallback-to-text-placeholder behavior for URL-based audio.

✅ Verified as addressed in 0af6b61: Praise comment requiring no code change; DataBlock is correctly included in hasMediaContent at line 360.

Instead of silently dropping DataBlock with unknown MIME type, add a text placeholder consistent with how ImageBlock/AudioBlock handles processing failures. This matches the legacy block behavior and gives users a visible hint when their DataBlock is not formatted correctly.

AgentScopeJavaBot

All blocking review comments have been addressed. 3 non-blocking suggestion(s) (minor/nit) remain open and may be addressed in a follow-up.

Sparkle6979 requested a review from a team June 13, 2026 18:04

chore: add missing license header to test file

6af54c7

test(model): add video and audio routing tests for DataBlock

5782e49

AgentScopeJavaBot added bug Something isn't working area/core/model Model providers and formatters labels Jun 14, 2026

AgentScopeJavaBot reviewed Jun 14, 2026

View reviewed changes

Sparkle6979 added 2 commits June 14, 2026 09:41

test(model): add unknown MIME type and Base64 audio tests for DataBlock

bf1fc7a

AgentScopeJavaBot approved these changes Jun 14, 2026

View reviewed changes

Conversation

Sparkle6979 commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

What this PR does

Not covered

Note

Uh oh!

codecov Bot commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AgentScopeJavaBot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AgentScopeJavaBot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AgentScopeJavaBot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Sparkle6979 commented Jun 13, 2026 •

edited

Loading

codecov Bot commented Jun 13, 2026 •

edited

Loading