fix: normalize image_url blocks to OpenAI-compliant dict format

[nabinchha]

📋 Summary

ImageContext.get_contexts() produced bare-string and non-standard dict shapes for image_url content blocks, which broke the native OpenAI adapter and only worked with Anthropic by accident. This is a regression from the litellm removal in v0.5.4 — litellm was normalizing the shape internally before forwarding to provider APIs.

🔗 Related Issue

Fixes #576

🔄 Changes

🐛 Fixed

• models.py — data_type=URL path now wraps value in {"url": ...} instead of passing a bare string
• models.py — Auto-detect URL path returns {"url": ...} dict instead of bare string
• models.py — Removed non-standard "format" key from base64 dicts (media type is already encoded in the data URI)

🔧 Changed

• anthropic_translation.py — translate_image_url_block now raises TypeError when image_url is not a dict (e.g. a bare string). Since all image_url blocks are constructed internally by DataDesigner, we control the shape — a malformed block indicates an internal bug and should fail loudly rather than be silently dropped.

🧪 Testing

• uv run pytest passes (136 tests across 5 test files)
• Unit tests updated to match new canonical format
• test_completion_rejects_bare_string_image_blocks — verifies Anthropic client raises TypeError on bare-string image_url
• test_translate_content_blocks_rejects_malformed_image_url_block — verifies translation layer raises TypeError on malformed blocks
• E2E tests — N/A, no live API calls in this change

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• Architecture docs updated — N/A, no architectural change

Made with Cursor

[greptile-apps]

Greptile Summary

This PR normalizes all image_url content blocks to OpenAI-compliant {"url": ...} dict format, removes the non-standard "format" key from base64 dicts, and hardens translate_image_url_block to raise TypeError on bare-string inputs rather than silently handling them. The fix correctly restores the normalization behavior that was previously provided by litellm before its removal in v0.5.4.

Confidence Score: 5/5

Safe to merge — fix is well-scoped, tests are comprehensive, and the previous P1 test structure issue has been resolved.

No P0 or P1 issues found. The previous P1 (bare-string rejection case inside a parametrize with expected=None) has been correctly addressed by extracting it into a dedicated test_translate_image_url_block_rejects_bare_strings test that uses pytest.raises(TypeError). All changed code paths are covered by updated tests.

No files require special attention.

Important Files Changed

Filename	Overview
packages/data-designer-config/src/data_designer/config/models.py	URL path now wraps bare strings in {"url": ...} dict; "format" key removed from base64 dicts since media type is already in the data URI. Return type annotation updated to match.
packages/data-designer-engine/src/data_designer/engine/models/clients/adapters/anthropic_translation.py	translate_image_url_block now raises TypeError for non-dict image_url values instead of silently falling back; translate_content_blocks simplified to remove the now-redundant None guard.
packages/data-designer-engine/tests/engine/models/clients/test_anthropic_translation.py	Bare-string rejection cases properly extracted into test_translate_image_url_block_rejects_bare_strings using pytest.raises(TypeError), resolving the previous P1 where they were inside a parametrize with expected=None.
packages/data-designer-config/tests/config/test_models.py	All URL assertions updated from bare strings to {"url": ...} dicts; "format" key assertions removed. Coverage is thorough across all data_type/auto-detect paths.
packages/data-designer-engine/tests/engine/models/clients/test_anthropic.py	Renamed and split tests to clearly distinguish dict-format (valid) from bare-string (invalid) cases; new pytest.raises(TypeError) assertions align with the updated translation layer.
packages/data-designer-engine/tests/engine/models/clients/test_openai_compatible.py	Two new passthrough tests verify that OpenAI-compatible client forwards {"url": ...} dicts unchanged for both remote URL and base64 data-URI cases.
packages/data-designer-engine/tests/engine/column_generators/generators/test_image.py	Two assertions updated to expect {"url": "https://..."} dict format instead of bare strings, consistent with the rest of the test suite changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ImageContext.get_contexts] --> B{data_type set?}
    B -- URL --> C["image_url = {'url': value}"]
    B -- BASE64 --> D["image_url = {'url': 'data:image/...;base64,...'}"]
    B -- None auto-detect --> E[_auto_resolve_context_value]
    E --> F{is file path?}
    F -- Yes --> G[load to base64 → _format_base64_context]
    F -- No --> H{is URL?}
    H -- Yes --> I["return {'url': value}"]
    H -- No --> J[_format_base64_context]
    G --> K["{'url': 'data:image/...;base64,...'}"]
    J --> K
    C --> L[OpenAI-compliant block]
    D --> L
    I --> L
    K --> L
    L --> M{Adapter?}
    M -- OpenAI --> N[Pass through as-is]
    M -- Anthropic --> O[translate_image_url_block]
    O --> P{image_url is dict?}
    P -- No --> Q[raise TypeError]
    P -- Yes --> R{data URI?}
    R -- Yes --> S[source type=base64]
    R -- No --> T[source type=url]

Loading

_{Reviews (3): Last reviewed commit: "address review: tighten return type, add..." | Re-trigger Greptile}

[github-actions]

Review: PR #577 — fix: normalize image_url blocks to OpenAI-compliant dict format

Summary

This PR fixes a regression introduced when litellm was removed in v0.5.4. ImageContext.get_contexts() was emitting image_url content blocks in three inconsistent shapes:

• URL + explicit data_type: bare string (context["image_url"] = context_value)
• URL + auto-detect: bare string
• Base64: {"url": ..., "format": ...} with a non-standard format key

The OpenAI Chat Completions spec requires image_url to be an object {"url": "..."} in all cases, with no format field. Previously the code only happened to work with Anthropic because the adapter silently coerced bare strings.

The fix:

1. Always wrap URLs in {"url": ...} in models.py (both explicit-type and auto-detect paths).
2. Drop the non-standard "format" key from base64 dicts (the media type is already encoded in the data URI).
3. Tighten translate_image_url_block in the Anthropic adapter to require a dict; bare strings are now rejected (block dropped).
4. Update all affected tests and add a bare-string-rejected case.

Findings

Correctness — LGTM

• The new shape matches the OpenAI spec ({"type": "image_url", "image_url": {"url": "..."}}), which is the canonical format DataDesigner targets.
• Removing "format" is correct: it was never part of the OpenAI spec, and the data URI already carries image/{format}. The Anthropic adapter re-parses media type from the URI via _DATA_URI_RE, so nothing downstream depends on this key.
• parsing.py:165-166 (resolve_image_payload) still works with the new shape — it recursively resolves raw_image["image_url"], which now lands on the "url" in raw_image dict branch at line 167.

Concern — Silent drop of legacy bare-string blocks (medium)

translate_image_url_block at packages/data-designer-engine/src/data_designer/engine/models/clients/adapters/anthropic_translation.py:321-324 now returns None for any non-dict image_url, and translate_content_blocks (line 197) drops None blocks without logging. The test test_completion_drops_bare_string_image_blocks documents this as intentional.

If a user (or external code) still constructs messages with bare-string image_url values — e.g., hand-rolled prompts, older cached datasets, or a plugin predating this PR — the image will silently disappear from the Anthropic request. Consider emitting a logger.warning(...) when an image block is dropped so the regression is diagnosable from logs rather than from missing model context. This is consistent with the logger.warning on image parse failures in parsing.py:142.

Minor — Stale return-type annotation

_auto_resolve_context_value at packages/data-designer-config/src/data_designer/config/models.py:137 is annotated -> str | dict[str, str], but after this PR every branch returns dict[str, str] (URL branch at line 151 now wraps in a dict; other two branches already returned dicts). Tighten to -> dict[str, str].

Minor — Test name/description drift

In tests/engine/models/clients/test_anthropic.py, the renamed test_completion_drops_bare_string_image_blocks replaces what used to be test_completion_translates_data_uri_string_image_blocks. The replacement keeps a bare-string URL (https://example.com/cat.png) but the old test was about a data-URI bare string. Consider keeping both scenarios (URL bare string and data-URI bare string) to confirm both legacy shapes are rejected uniformly. Today only one is covered.

Style / conventions — LGTM

• Follows project absolute-import and typing conventions.
• Diff is tightly scoped to the bug; no drive-by refactors.
• No new files needing SPDX headers.
• Removed inline comments (# Explicit data_type: use existing behavior, # Should treat the entire JSON string as a single image URL) are appropriate — they restated what the code already says.

Tests — adequate

• All touched tests updated; new bare-string-rejected parametrize case locks in the Anthropic behavior.
• test_image_cell_generator_auto_detect_passes_through_urls and test_image_cell_generator_with_multi_modal_context exercise the engine path end-to-end.
• Not covered: an OpenAI client happy-path test that verifies the new shape flows through unchanged to the OpenAI request payload. The PR description claims this was "broken with the native OpenAI adapter"; an assertion-level regression test would prevent re-breaking it.

Security — no concerns

No changes to auth, input validation at trust boundaries, or credential handling. Data-URI parsing is regex-based and unchanged.

Performance — no concerns

Wrapping a string in a one-key dict is negligible. No new I/O paths.

Verdict

Approve with minor follow-ups. The core fix is correct and well-tested. Before merging, please consider:

1. (Recommended) Add a logger.warning when translate_image_url_block drops a non-dict block — silent data loss is the kind of bug that re-emerges as an issue months later.
2. (Nit) Tighten _auto_resolve_context_value return type to dict[str, str].
3. (Nit) Add an explicit OpenAI-client regression test asserting the new image_url shape is forwarded unchanged, since OpenAI-compatibility is the stated motivation.

None are blocking.

[nabinchha]

Thanks for the thorough review! All three follow-ups addressed in 7e7b26e:

Concern — Silent drop → now a hard TypeError (done in prior commit ee73e99)
translate_image_url_block raises TypeError instead of returning None. Since all image_url blocks are constructed internally by DataDesigner, a bare string indicates an internal bug and should fail loudly. This is stricter than logger.warning — we'd rather catch the invariant violation immediately than let it surface as missing model context.

Nit — Stale return-type annotation
_auto_resolve_context_value return type narrowed from str | dict[str, str] to dict[str, str].

Nit — OpenAI-client regression test
Added test_completion_forwards_image_url_dict_unchanged and test_completion_forwards_base64_image_url_dict_unchanged in test_openai_compatible.py — verifies both URL and data-URI image blocks pass through to the OpenAI payload unmodified.

Bonus — data-URI bare string coverage
The prior bare-string-rejected case only tested a URL string. Split it into a dedicated test_translate_image_url_block_rejects_bare_strings with both bare-url-string and bare-data-uri-string parametrized cases. Also added test_completion_rejects_bare_data_uri_image_blocks at the Anthropic client level.

[andreatgretel]

oops sorry reviewed stale checkout! approved now