[Data][LLM] Support OpenAI's nested image_url schema in PrepareImageStage#56584
Conversation
…tage Signed-off-by: Guy Stone <guys@spotify.com>
There was a problem hiding this comment.
Code Review
This pull request adds support for OpenAI's nested image_url schema in PrepareImageStage, which is a great improvement for consistency and user experience. The implementation is clean and correctly handles the new format, including validation for malformed inputs. A new unit test is also added to cover the happy path. My only suggestion is to also add a test case for the error path to ensure the new validation logic is fully covered.
| if image is None: | ||
| raise ValueError("image_url dict must contain 'url' key") |
There was a problem hiding this comment.
There was a problem hiding this comment.
Supporting nested image_url makes perfect sense, thanks for the contribution @GuyStone
Few quick nits
- there's an optional param called
detail(From openai schema that we're dropping here. I don't think the downstream templates / etc would currently use it, but would be good to just leave a note in the docstring that we're not passing it forward - Would be good to have at least one or two unhappy path tests - ex. missing or non-string
image_url - type-guard + clarify error message, something like
if content_item["type"] == "image_url" and isinstance(image_data, dict):
url = image_data.get("url")
if not isinstance(url, str) or not url:
raise ValueError("image_url must be an object with a non-empty 'url' string")
image = url
otherwise looks good - kicked off a release test in the meantime
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
nrghosh
left a comment
There was a problem hiding this comment.
LGTM after review
Not re-running release tests, so good to merge cc @kouroshHakha
…tage (#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com> Signed-off-by: Douglas Strodtman <douglas@anyscale.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com> Signed-off-by: Seiji Eicher <seiji@anyscale.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com> Signed-off-by: Josh Kodi <joshkodi@gmail.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
…tage (ray-project#56584) Signed-off-by: Guy Stone <guys@spotify.com> Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil Ghosh <nikhil@anyscale.com> Co-authored-by: Nikhil G <nrghosh@users.noreply.github.com> Signed-off-by: Future-Outlier <eric901201@gmail.com>
Why are these changes needed?
image_urlschema in PrepareImageStage.Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.