-
Notifications
You must be signed in to change notification settings - Fork 567
[codex] Support raw image offload in v1 train client #1746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
173a518
de37650
e6b13dc
9430999
4a7b37a
0b1d73f
2d4969b
7ade0b2
0dc57a1
18b0fbe
22c7cf4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,90 @@ | ||||||||||||||||||||||
| """Multimodal ingress helpers for renderer-backed training.""" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| from importlib import import_module | ||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _offload_image_url(url: object, image_dir: Path | None) -> str | None: | ||||||||||||||||||||||
| try: | ||||||||||||||||||||||
| offload_image_to_run_assets = getattr( | ||||||||||||||||||||||
| import_module("renderers.mm_store"), | ||||||||||||||||||||||
| "offload_image_to_run_assets", | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| except ( | ||||||||||||||||||||||
| ImportError, | ||||||||||||||||||||||
| AttributeError, | ||||||||||||||||||||||
| ) as exc: # pragma: no cover - dependency-version guard | ||||||||||||||||||||||
| raise RuntimeError( | ||||||||||||||||||||||
| "Multimodal training requires a renderers version with raw image " | ||||||||||||||||||||||
| "asset offload support." | ||||||||||||||||||||||
| ) from exc | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return offload_image_to_run_assets(url, image_dir=image_dir) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _image_source_url(source: Any) -> object: | ||||||||||||||||||||||
| if isinstance(source, dict): | ||||||||||||||||||||||
| return source.get("url") | ||||||||||||||||||||||
| return getattr(source, "url", None) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _set_image_source_url(source: Any, url: str) -> None: | ||||||||||||||||||||||
| if isinstance(source, dict): | ||||||||||||||||||||||
| source["url"] = url | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| source.url = url | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _require_file_image_url(source: Any) -> None: | ||||||||||||||||||||||
| url = _image_source_url(source) | ||||||||||||||||||||||
| if not isinstance(url, str) or not url.startswith("file://"): | ||||||||||||||||||||||
| raise RuntimeError( | ||||||||||||||||||||||
| "multimodal training requires image_url entries to be offloaded " | ||||||||||||||||||||||
| "to file:// run image assets" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _prepare_image_source(source: Any, *, image_dir: Path | None) -> None: | ||||||||||||||||||||||
| result = _offload_image_url(_image_source_url(source), image_dir) | ||||||||||||||||||||||
| if result is not None: | ||||||||||||||||||||||
| _set_image_source_url(source, result) | ||||||||||||||||||||||
| _require_file_image_url(source) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def prepare_images_inplace(value: Any, *, image_dir: Path | None = None) -> None: | ||||||||||||||||||||||
| """Offload image URLs reachable from ``value`` to run image assets. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Handles OpenAI wire dicts/lists and the pydantic v0/v1 message/content-part | ||||||||||||||||||||||
| models used by trajectories and traces. | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| if isinstance(value, dict): | ||||||||||||||||||||||
| if value.get("type") == "image_url": | ||||||||||||||||||||||
| source = value.get("image_url") | ||||||||||||||||||||||
| if source is not None: | ||||||||||||||||||||||
| _prepare_image_source(source, image_dir=image_dir) | ||||||||||||||||||||||
|
Comment on lines
+64
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium
Suggested change
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||||||||||||||||||||||
| for child in value.values(): | ||||||||||||||||||||||
| prepare_images_inplace(child, image_dir=image_dir) | ||||||||||||||||||||||
| return | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if isinstance(value, list): | ||||||||||||||||||||||
| for child in value: | ||||||||||||||||||||||
| prepare_images_inplace(child, image_dir=image_dir) | ||||||||||||||||||||||
| return | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if isinstance(value, tuple): | ||||||||||||||||||||||
| for child in value: | ||||||||||||||||||||||
| prepare_images_inplace(child, image_dir=image_dir) | ||||||||||||||||||||||
| return | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if getattr(value, "type", None) == "image_url": | ||||||||||||||||||||||
| source = getattr(value, "image_url", None) | ||||||||||||||||||||||
| if source is not None: | ||||||||||||||||||||||
| _prepare_image_source(source, image_dir=image_dir) | ||||||||||||||||||||||
| return | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| content = getattr(value, "content", None) | ||||||||||||||||||||||
| if isinstance(content, (list, tuple)): | ||||||||||||||||||||||
| prepare_images_inplace(content, image_dir=image_dir) | ||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inline image URLs always rejected
High Severity
The v1 train path always requires every
image_urlto become afile://run asset after preparation, even when offload leaves adata:image/...;base64,...URL unchanged. That conflicts with the intended inline multimodal storage mode where base64 image URLs stay in the message payload, so inline training rollouts fail at request preparation instead of validating in place.Additional Locations (1)
verifiers/v1/clients/train.py#L208-L216Reviewed by Cursor Bugbot for commit 0b1d73f. Configure here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed inline mode so this is irrelevant