feat(llm): deliver model-generated images via outbound pipeline - #253
Conversation
- Responses: tolerate builtin image_generation_call progress events (relay backends inject the tool server-side, even when undeclared) and extract result payloads into a protocol-neutral generated_images response attachment; items are stripped from native_blocks so base64 never enters replay - Gemini: response-side inlineData image parts feed the same attachment and are excluded from native replay parts (functionCall default ids renumbered contiguously) - Tool loop collects attachments into the outbound image channel at each response arrival: same cap and same images-survive-later-failures semantics as tool-produced images, plus an svg_render-style rate limiter (10/min global, 2/min per user) - Fixes the production crash on unknown semantic event response.image_generation_call.in_progress
- mirror of the deployed openai_family addition: hedge uncertainty the way group members do, no liability-style riders after conclusions
![khpilot[bot]](https://avatars.githubusercontent.com/u/178400596?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bot Review — ✅ Approve
This PR adds first-class support for model-generated images (Responses image_generation_call events and Gemini inlineData image parts) into the outbound pipeline, following the established LLMResponse.web_search pattern. Complete coverage (13/13 both reviewers) with no blocking findings. Three should-fix issues remain: (1) no byte-limit/media-type guard on collected images, risking platform-size rejections or non-image payloads forwarded as images; (2) image-only responses get a confusing placeholder text under the default config due to uncoordinated finalize_reply_text; (3) an unused provider_id parameter in _extract_generated_images. Four nit-level findings address test isolation gaps, missing format-coverage tests, terminology inconsistency ("native" vs "generated"), duplicated rate-limit rules, scope creep in the example config, and DRY violations in tests. These are addressable post-merge without blocking the feature.
Findings
🟡 SHOULD-FIX: 图片数据无字节上限或媒体类型校验即进入 outbound_images 通道：draw_svg 设了 MAX_OUTPUT_PNG_BYTES = 4MB 以防平台单条消息超限，但 collect_native_images 对 Gemini 和 Responses 产出的大图没有同口径上限；且 Gemini _inline_image_payload 不校验 mimeType，非图片 inlineData（如音频）会被当成图片剥除并外发。
File: src/quickquip/llm/native_images.py
在 collect_native_images 内加入 bytes 上限检查（复用 MAX_OUTPUT_PNG_BYTES 或 MAX_IMAGE_BYTES），超限丢弃并 logger.warning；Gemini 侧只在 media_type.startswith("image/") 时才提取，其余 inlineData 保留在原生批次中。
🟡 SHOULD-FIX: 只有图片、无正文/工具调用的响应被标记为合法，但下游 finalize_reply_text 在正文为空时会补占位文本"模型没有返回可显示的文本。"，导致用户看到占位文本+图片的组合，与本 PR 放开该语义的意图相悖；逐 Turn 交付开启时行为又不同（只发图），两种配置不一致。
File: src/quickquip/llm/provider/openai_responses/response.py
给 finalize_reply_text 增加 has_images/allow_empty 参数，图片非空且正文为空时不补占位文本；或在 service 组装 reply_result 时对"无正文但有 images"跳过占位。
🟡 SHOULD-FIX: 函数签名声明 provider_id 参数但函数体内从未引用；同文件其他校验函数都利用 provider_id 调用 _malformed 做错误归因，此参数存在却未使用会误导调用方。
File: src/quickquip/llm/provider/openai_responses/response.py
删除 provider_id 参数，调用改为 _extract_generated_images(body["output"])；若需诊断信息则将其注入为真实 logger.warning 中的 provider_id。
🟡 SHOULD-FIX: 术语不统一：类型/字段/文档用"generated image"（LLMGeneratedImage、generated_images、source），新模块内部却用"native image"（native_images.py、collect_native_images、native_image_allowed）。代码库中"native"已有协议原生 replay blocks 的含义，native_images 容易被误解为"协议原生 replay 图片"。
File: src/quickquip/llm/native_images.py
统一使用 generated_image 作为领域名词：将模块改名为 generated_images.py，函数改为 collect_generated_images/generated_image_allowed，限流规则 key 改为"generated_image"，日志同步更新；保留 LLMGeneratedImage/generated_images 为对外 API。
🟡 SHOULD-FIX: base64 解码失败时静默返回空数据的策略在 Gemini 和 Responses 中各写一遍且实现略有差异；此外 Gemini 的图片提取、id 重编号、解码逻辑全内联在 _parse_candidate 中，耦合过紧。
File: src/quickquip/llm/provider/gemini.py
在 provider/base.py 的 LLMGeneratedImage 上新增 classmethod from_base64(data_b64, *, media_type, source) -> LLMGeneratedImage | None，将策略和类型定义归一处；Gemini 侧提取 _extract_inline_images(parts) -> tuple[list[LLMGeneratedImage], list[dict]] 辅助函数，职责单一化。
Review provenance
- General reviewer: complete — 13/13 units completed
- Style reviewer: complete — 13/13 units completed
- Coverage: complete
- Reviewed head:
8a7d7d6(based02de33)
Automated review by @KHPilot. Reply with @KHPilot to ask follow-up questions.
| context.user_id, | ||
| ) | ||
| break | ||
| context.outbound_images.append( |
There was a problem hiding this comment.
SHOULD-FIX: 图片数据无字节上限或媒体类型校验即进入 outbound_images 通道：draw_svg 设了 MAX_OUTPUT_PNG_BYTES = 4MB 以防平台单条消息超限，但 collect_native_images 对 Gemini 和 Responses 产出的大图没有同口径上限；且 Gemini _inline_image_payload 不校验 mimeType，非图片 inlineData（如音频）会被当成图片剥除并外发。
在 collect_native_images 内加入 bytes 上限检查（复用 MAX_OUTPUT_PNG_BYTES 或 MAX_IMAGE_BYTES），超限丢弃并 logger.warning；Gemini 侧只在 media_type.startswith("image/") 时才提取，其余 inlineData 保留在原生批次中。
| ) | ||
| finish_reason = "length" if reason == "max_output_tokens" else reason | ||
| elif not text and not tool_calls: | ||
| elif not text and not tool_calls and not generated_images: |
There was a problem hiding this comment.
SHOULD-FIX: 只有图片、无正文/工具调用的响应被标记为合法，但下游 finalize_reply_text 在正文为空时会补占位文本"模型没有返回可显示的文本。"，导致用户看到占位文本+图片的组合，与本 PR 放开该语义的意图相悖；逐 Turn 交付开启时行为又不同（只发图），两种配置不一致。
给 finalize_reply_text 增加 has_images/allow_empty 参数，图片非空且正文为空时不补占位文本；或在 service 组装 reply_result 时对"无正文但有 images"跳过占位。
|
|
||
|
|
||
| def _extract_generated_images( | ||
| items: list[Any], provider_id: str |
There was a problem hiding this comment.
SHOULD-FIX: 函数签名声明 provider_id 参数但函数体内从未引用；同文件其他校验函数都利用 provider_id 调用 _malformed 做错误归因，此参数存在却未使用会误导调用方。
删除 provider_id 参数，调用改为 _extract_generated_images(body["output"])；若需诊断信息则将其注入为真实 logger.warning 中的 provider_id。
| ) | ||
|
|
||
|
|
||
| def collect_native_images(response: LLMResponse, context: ToolExecutionContext) -> int: |
There was a problem hiding this comment.
SHOULD-FIX: 术语不统一：类型/字段/文档用"generated image"（LLMGeneratedImage、generated_images、source），新模块内部却用"native image"（native_images.py、collect_native_images、native_image_allowed）。代码库中"native"已有协议原生 replay blocks 的含义，native_images 容易被误解为"协议原生 replay 图片"。
统一使用 generated_image 作为领域名词：将模块改名为 generated_images.py，函数改为 collect_generated_images/generated_image_allowed，限流规则 key 改为"generated_image"，日志同步更新；保留 LLMGeneratedImage/generated_images 为对外 API。
| if inline is not None: | ||
| data_b64, media_type = inline | ||
| try: | ||
| data = base64.b64decode(data_b64, validate=True) |
There was a problem hiding this comment.
SHOULD-FIX: base64 解码失败时静默返回空数据的策略在 Gemini 和 Responses 中各写一遍且实现略有差异；此外 Gemini 的图片提取、id 重编号、解码逻辑全内联在 _parse_candidate 中，耦合过紧。
在 provider/base.py 的 LLMGeneratedImage 上新增 classmethod from_base64(data_b64, *, media_type, source) -> LLMGeneratedImage | None，将策略和类型定义归一处；Gemini 侧提取 _extract_inline_images(parts) -> tuple[list[LLMGeneratedImage], list[dict]] 辅助函数，职责单一化。
- byte cap (MAX_OUTPUT_PNG_BYTES, same as draw_svg) and image/* media-type gate before outbound collection; non-image inlineData stays in native parts - empty-text placeholder suppressed for image-only replies, keyed on images actually surviving collection so dropped images still get a visible notice - shared LLMGeneratedImage.from_base64 decode policy; gemini extraction factored into _extract_inline_images; unused provider_id param dropped - terminology unified: generated_images module/collectors/limiter key (native was already taken by replay blocks)
|
All five SHOULD-FIX findings addressed in ccba340:
Tests: +4 (oversize drop, non-image inlineData preserved, placeholder on/off); full suite 2441 passed, ruff clean. |
1 participant
Problem
A relay Responses backend injects its builtin
image_generationtool server-side (visible in theresponse.createdtools echo, even though the request never declares it). When the model uses it, the SSE stream emitsresponse.image_generation_call.*progress events and the terminal output contains animage_generation_callitem — both unknown to our stream folder and item validator, so the whole reply failed withResponses 流畸形：未知的语义事件 response.image_generation_call.in_progresswhile the image had actually been generated successfully.Design: protocol-neutral, no backend favoritism
Model-produced images become a first-class response-side capability, mirroring two existing precedents:
image_url/inline_data/input_image→ media_guard). This is the output-side mirror.LLMResponse.web_search(builtin grounding) is the existing pattern for provider-native capabilities — a normalized attachment field filled by per-protocol adapters.generated_imagesfollows it.Any protocol whose backend yields images gets the same path; the codex-style relay is merely the first producer.
Changes
LLMGeneratedImage+LLMResponse.generated_images(provider/base, exported).image_generation_call.*events tolerated; item payloads (resultbase64) extracted to attachments and stripped fromnative_blocks— base64 never enters replay. Decode failures degrade to no-image without killing the text; image-only responses are no longer malformed. The strict validator is untouched: persisted blocks containing image items would failblocks_replay_validand degrade to structured history (fail-safe for free).inlineDataimage parts feed the same attachment and are excluded from native replay parts (functionCall default ids renumber contiguously).MAX_OUTBOUND_TOOL_IMAGES), same "images survive later provider failures" semantics, plus an svg_render-style rate limiter (10/min global, 2/min per user) so both drawing paths stay product-level peers.Tests
9 new: extraction (valid/bad base64/image-only), stream folding with the production event sequence, gemini inlineData extraction + id renumbering, collector cap/rate-limit/noop. Full suite 2437 passed, ruff clean. One pre-push run hit the known flaky from #246 (passes in isolation).
Out of scope (noted)
web_search_callitems remain fail-closed; builtin search for Responses is a tracked roadmap item.