Skip to content

feat(cli): render inline terminal images - #8305

Merged
wenshao merged 15 commits into
QwenLM:mainfrom
tlysanhuo:tlysanhuo/inline-terminal-images
Aug 6, 2026
Merged

feat(cli): render inline terminal images#8305
wenshao merged 15 commits into
QwenLM:mainfrom
tlysanhuo:tlysanhuo/inline-terminal-images

Conversation

@tlysanhuo

@tlysanhuo tlysanhuo commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

What this PR does

This PR extends the terminal-image infrastructure merged in #8217 from workspace-file previews to model and tool inlineData in the interactive CLI.

  • Preserves ordered text/image parts on ServerGeminiContentEvent while keeping the existing concatenated value contract and text-only event shape unchanged.
  • Adds an in-memory PNG entry point to feat(cli): add TUI image display tool #8217's TerminalImage and terminal-image-renderer: validated and bounded to 8 MiB and 64 million pixels, rendered through native Kitty/Ghostty placement or chafa stdin, with deterministic placeholders, a bounded decode/render cache, and screen-reader-safe output.
  • Renders live assistant images plus top-level or nested images from successful tool responses. Resume parsing supports persisted image parts; the current Core recorder flattens assistant output to text, so assistant images are not restored by --continue. Failed and cancelled tool records do not currently carry inline image parts from Core.
  • Preserves text -> image -> text order across fresh/continuation retry, model fallback, cancellation, finished/status boundaries, displayed Goal-state events, and later content chunks.
  • Clears transient failed-turn output before clear/resume/branch/restore/rewind history replacement, and shares the pending height budget across multiple images.
  • Keeps image-bearing tools expanded and clears old assistant/tool image payloads during UI history compaction.

The earlier duplicate renderer and direct iTerm2 placement implementation were removed. #8217 is now the single terminal rendering source of truth.

Why it's needed

Qwen Code currently drops model inlineData at the Core-to-TUI event boundary and reduces tool media to text in the display. Image-generating models, screenshots, and chart-producing tools therefore cannot show their output in the conversation. The current Core recorder also flattens assistant output when writing session history; assistant-image persistence is tracked in #8521. Issue #8090 explicitly recommends a focused render-and-forget first slice.

Reviewer Test Plan

How to verify

  1. In direct Kitty or Ghostty, return a PNG inlineData part between two text parts. Expect native placement between the two text runs. Repeat with PNG media in a successful tool's functionResponse.parts and after session resume. Assistant output resumes its persisted text; assistant inline images are not persisted by the current Core recorder.
  2. In iTerm2, Warp, tmux/SSH, or another non-native environment with chafa installed, repeat the cases. Expect ANSI symbol rows produced from PNG bytes supplied over stdin.
  3. Without chafa, with screen-reader mode enabled, or with invalid/non-PNG data, expect a deterministic placeholder such as [image: 1024x768 png] and no raw image sequence. Payloads above the encoded-length limit are dropped before UI history and do not produce a placeholder.
  4. Exercise fresh retry, continuation retry, fallback, cancellation, a displayed Goal-state event followed by more content, and session resume. Expect the original ordering with no duplicated or stale runs.

Automated evidence on follow-up head 1e1a6e8a4

  • Focused CLI regression suite: 11 files, 579 tests passed. This includes history replacement cleanup, mixed-content/tool ordering, shown citations, multi-event image caps, retry/fallback cap resets, total-pixel rejection, shared image-height budgets, resume parsing, and tool image mapping.
  • npm run lint:ci: passed.
  • npm run typecheck: passed for every workspace.
  • Prettier and git diff --check: passed.
  • Chrome extension packaging and scan:artifacts: passed after synchronizing the fork head with current main.
  • Earlier implementation heads also passed npm run build and npm run check:serve-fast-path-bundle (Startup bundle closure checks passed).

The local environment is not a real Kitty/Ghostty session, so this update does not claim a new hardware screenshot. #8217 already carries manual Kitty, Ghostty, cmux, and Warp evidence for the reused protocol layer; this PR's new in-memory entry point is covered through forced-protocol and chafa-stdin tests.

Tested on

OS Status
macOS ✅ automated
Windows ⚠️ not hardware-tested
Linux ⚠️ not hardware-tested

Risk & Scope

  • This remains the render-and-forget slice recommended by issue triage. Kitty deletion, resize-driven replacement, terminal cell pixel queries, and global scroll lifecycle ownership remain out of scope and are tracked in feat(cli): manage inline Kitty images across resize and scroll #8520.
  • The shared feat(cli): add TUI image display tool #8217 renderer intentionally renders validated PNG data only. Other image MIME types stay visible as deterministic placeholders.
  • No temporary files are created, image data is never used as a command argument, and chafa receives the existing allowlisted child environment.
  • Breaking changes / migration notes: none. The Core event addition is optional and emitted only for image-bearing chunks; existing text consumers continue reading value.

Linked Issues

Fixes #8090

Follow-ups: #8520, #8521

中文说明

本 PR 做了什么

本 PR 基于已合并的 #8217,将其工作区 PNG 预览能力扩展到交互式 CLI 中的模型与工具 inlineData

  • 内容事件新增可选的有序 parts,现有拼接文本 value 与纯文本事件形状保持不变。
  • 复用 feat(cli): add TUI image display tool #8217TerminalImage 与 renderer,内存 PNG 经 8 MiB 上限、Base64、签名和尺寸校验后,走 Kitty/Ghostty 原生放置或 chafa stdin;不可渲染时显示确定性占位文本。
  • 支持实时助手图片,以及成功工具响应中的顶层或嵌套图片。恢复解析支持已有持久化图片部件;当前 Core recorder 会将助手输出压平为文本,因此 --continue 不会恢复助手图片。失败、取消工具记录目前不会携带 Core 生成的图片部件。
  • 在重试、模型回退、取消、完成/状态边界、Goal 状态插入以及后续内容 chunk 中保持 文本 -> 图片 -> 文本 顺序且不重复。
  • 带图片的工具不会被折叠;UI 历史压缩会清理旧图片 payload。

此前 PR 内自研的重复 renderer 和直接 iTerm2 放置路径已删除,#8217 成为唯一的终端渲染实现。

验证结果

后续 HEAD 1e1a6e8a4:CLI 聚焦回归共 11 个文件、579 项测试通过,覆盖历史替换清理、内容/工具边界顺序、引用显示、跨事件图片上限、retry/fallback 重置、总像素上限与多图高度预算。lint:ci、全 workspace typecheck、Prettier 与 diff check 均通过;同步当前 main 后,Chrome 扩展打包与 scan:artifacts 也已通过。

风险与范围

本次仍是 issue triage 建议的“渲染即忘”首版;Kitty 删除、resize 重放、终端 cell 像素查询和全局滚动生命周期由 #8520 跟踪,助手图片持久化由 #8521 跟踪。复用的 #8217 renderer 当前只实际渲染 PNG,其他图片格式保留确定性占位文本。无破坏性变更。

@tlysanhuo

Copy link
Copy Markdown
Contributor Author

E2E test report

Result: automated changed-path and build-gate coverage passed on commit 89071efda; real graphics-terminal verification is pending because this environment exposes only TERM=dumb and has no Kitty, Ghostty, iTerm2, WezTerm, or Warp installation.

Passed on the submitted commit

  • Focused CLI regression suite: 12 files, 467 tests passed. Coverage includes protocol detection/encoding, bounded image parsing, Kitty and iTerm2 component behavior, off-screen re-entry, screen-reader and main-screen fallbacks, live mixed-stream ordering, retry/fallback/error transitions, tool images, resume, memory compaction, and the extracted Mermaid helpers.
  • Core Turn suite: 36 tests passed, including ordered text -> image -> text events, image-only events, unchanged value, and unchanged text-only event shape.
  • npm run lint:ci: passed.
  • npm run typecheck: passed for every workspace.
  • npm run build: passed.
  • npm run check:serve-fast-path-bundle: passed.

Broader regression evidence

Earlier full-suite runs on the same implementation passed all 566 core files (18,940 tests, 11 skipped) and all 31 SDK files (1,442 tests). The full CLI run passed 725 files and 16,261 tests with 6 skipped; its 11 failures were reproduced as unrelated baseline/concurrency cases (10 AuthDialog keyboard-timing assertions and one socket race). The socket case passed in isolated serial execution; the auth file reproduced its existing MiniMax timing failure in isolation.

Hardware/manual status

  • Kitty/Ghostty and iTerm2/WezTerm/Warp protocol bytes, row reservation, cursor preservation, viewport re-entry, invalid/oversized payload handling, and fallbacks are asserted through isolated forced-protocol tests.
  • No real-terminal screenshot is attached or claimed. Reviewer hardware steps and expected outcomes are listed in the PR body.
  • Resize replacement, Kitty deletion, global scroll lifecycle ownership, and terminal cell pixel queries are intentionally outside this render-and-forget slice, matching the issue triage direction.

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Re-run triggered by the maintainer — same head b9ebfddf as the last pass, so the gate findings stand; recording what changed since.

Template looks good ✓ — all required sections present, bilingual body, and a concrete reviewer test plan.

Problem: this is an observed, maintainer-acknowledged feature gap, not theoretical hardening. Issue #8090 documents that qwen-code drops model inlineData images and tool media at the core→TUI boundary even on capable terminals. Solid grounding.

Direction: aligned. Inline image rendering is squarely within the terminal-UX roadmap the issue is labeled under, and the PR follows the issue's explicit recommendation of a focused render-and-forget first slice — Kitty deletion, resize replacement, and global scroll lifecycle stay deferred to #8520.

Size: 1,200 production-logic lines across 19 files, 2,655 test lines across 17, and 235 lines of design doc + E2E plan. The core touch is small and additive (packages/core/src/core/turn.ts, ~58 production lines). A feat touching core above 500 production lines is flagged for maintainer awareness per the core-module gate (informational, not a block), and the 1000+ large-PR advisory applies — but the awareness flag is now satisfied: @wenshao reviewed this in depth (including a local real-stack TUI run) and approved the exact commit under review.

Approach: the scope feels right for a first slice. It threads ordered text/image parts through the existing content event (leaving the value text contract untouched) and reuses #8217's bounded renderer via a new in-memory entry point. I don't see a materially simpler route that still preserves text/image/text ordering across retry, fallback, and resume. The diff stays focused on the one feature — no drive-by refactors.

Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

由维护者触发的 re-run——head 仍为上次的 b9ebfddf,门禁结论不变;记录此后的变化。

模板完整 ✓ —— 所有必需章节齐全,含双语正文与具体的评审者测试计划。

问题:这是一个已观测、且经维护者确认的功能缺口,而非理论性加固。Issue #8090 记录了 qwen-code 即使在支持图形协议的终端上,也会在核心→TUI 边界丢弃模型 inlineData 图片与工具媒体。依据充分。

方向:对齐。内联图片渲染完全落在该 issue 所属的 terminal-UX 路线图内,且 PR 遵循 issue 明确建议的聚焦"渲染即忘"首版——Kitty 删除、resize 替换、全局滚动生命周期仍延后至 #8520

规模:1,200 行生产逻辑(19 个文件)、2,655 行测试(17 个文件)、235 行设计文档与 E2E 计划。核心改动很小且为增量(packages/core/src/core/turn.ts,约 58 行生产代码)。触及核心且超过 500 行生产代码的 feat 按核心模块门禁需提请维护者关注(仅知会,非阻塞),同时适用 1000+ 大 PR 建议——该知会现已满足:@wenshao 已深度评审(含本地真实 TUI 实测)并批准了正在评审的这一提交。

方案:对首版而言范围合理。它把有序的文本/图片 parts 接入现有内容事件(value 文本契约保持不变),并通过一个新的内存入口复用 #8217 的有界渲染器。我没有找到能在重试、回退与恢复中保持文本/图片/文本顺序的更简路径。diff 聚焦于单一功能——无顺手重构。

风险:无升级风险信号——改动文件均未命中与回滚相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at b9ebfddface455cc4060215691176b581033e334 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Code review

Reviewed at b9ebfddf — the same commit as the last pass, so the code findings stand; restated for the record. I wrote my independent proposal first and the PR landed on essentially the same shape, so my job was to check whether it cut corners — it didn't, and the restructure onto the merged #8217 renderer actually tightened it. No critical blockers, no AGENTS.md violations.

The core addition is the right shape and I verified every downstream consumer of the content event tolerates it: turn.ts adds an optional ordered parts field emitted only for image-bearing chunks, leaving value untouched. The new case is an image-only chunk that now emits value: '' plus parts, where previously no event fired — I checked all five other consumers (client messageDisplay, loop-detection, the non-interactive CLI preview, the JSON output adapter, and the subagent runtime) and each treats an empty value as a no-op, so the additive event can't regress them. The sandboxed A/B run below independently confirmed the same four-consumer inertness on the real built code.

The renderer entry point (terminal-image-renderer.ts) is the strongest part. PNG-only: base64 is validated with a canonical round-trip, the IHDR signature and dimensions are bounds-checked before every read, and inputs are capped at 8 MiB decoded, 1,000,000 px per side and 64,000,000 px total. There's no escape-sequence or command-injection surface — image bytes only ever enter Kitty sequences base64-encoded (a safe alphabet) and reach chafa over stdin (never as a path argument), and the control fields are bounded integers. Invalid, oversized, or non-canonical payloads fall back to a deterministic [image: WxH png] placeholder. Decode and render caches are both size- and byte-bounded.

Ordering across retry / fallback / cancel / resume is handled by a staging region in useGeminiStream that keeps completed text/image runs live until a normal commit boundary — so a fresh retry discards the failed attempt's partial images while a new turn commits them first. useHistoryManager now clears old assistant and tool image payloads during compaction (a real memory fix, extending the existing keep-recent pattern), and useReactToolScheduler surfaces successful tool-response images without dropping resultDisplay in the merged error/cancelled case. Each transition has a dedicated test in the 1,631 added useGeminiStream test lines.

The one thing a maintainer should weigh (not a blocker — it's the documented first-slice scope): this is render-and-forget, so Kitty images are never deleted or re-placed on scroll/resize. That's the part most likely to need a follow-up (#8520), and the PR says so plainly.

The runtime flow, for orientation:

sequenceDiagram
    participant P1 as Model stream
    participant P2 as Turn core
    participant P3 as useGeminiStream
    participant P4 as inline-image-parts
    participant P5 as terminal-image-renderer
    participant P6 as Terminal
    P1->>P2: chunk with text and inlineData parts
    P2->>P3: Content event, value plus optional ordered parts
    P3->>P3: stage runs to keep order across retry and resume
    P3->>P4: extract ordered runs, cap images per item
    P4->>P5: validated PNG data and mimeType
    P5->>P5: validate base64, read IHDR, fit to cells
    P5->>P6: kitty placement or chafa stdin, else placeholder
Loading
Files changed (30 of 38 shown)
File What changed
packages/core/src/core/turn.ts Adds optional ordered parts to the content event
packages/core/src/core/turn.test.ts Tests ordered, image-only, and thought-excluded parts
packages/cli/src/ui/types.ts Adds InlineImageData and the gemini_content item type
packages/cli/src/ui/utils/inline-image-parts.ts Ordered run extraction, per-item image cap, overflow marker
packages/cli/src/ui/utils/terminal-image-renderer.ts New in-memory PNG entry point: validation, sizing, Kitty/chafa
packages/cli/src/ui/hooks/useGeminiStream.ts Staging region preserving text and image order
packages/cli/src/ui/hooks/useHistoryManager.ts Clears image payloads during memory compaction
packages/cli/src/ui/hooks/useReactToolScheduler.ts Surfaces tool-response images
packages/cli/src/ui/utils/resumeHistoryUtils.ts Restores ordered runs and nested tool images on resume
packages/cli/src/ui/components/TerminalImage.tsx Renders a prepared image or its placeholder
packages/cli/src/ui/components/messages/ConversationMessages.tsx Renders assistant images
packages/cli/src/ui/components/messages/ToolMessage.tsx Renders tool images
packages/cli/src/ui/components/messages/ToolGroupMessage.tsx Renders tool-group images
packages/cli/src/ui/components/HistoryItemDisplay.tsx Routes image-bearing items to the renderer
packages/cli/src/ui/AppContainer.tsx Clears pending image state on history replacement
packages/cli/src/ui/hooks/slashCommandProcessor.ts Clears pending images on relevant commands
packages/cli/src/ui/hooks/useBranchCommand.ts Clears pending images on branch
packages/cli/src/ui/hooks/useResumeCommand.ts Clears pending images on resume
packages/cli/src/ui/commands/restoreCommand.ts Clears pending images on restore
docs/design/terminal-inline-images.md Design doc for the pipeline
.qwen/e2e-tests/terminal-inline-images.md E2E test plan
packages/cli/src/ui/hooks/useGeminiStream.test.tsx Ordering across retry, fallback, cancel, resume (1631 lines)
packages/cli/src/ui/utils/terminal-image-renderer.test.ts Renderer validation and fallback tests
packages/cli/src/ui/utils/inline-image-parts.test.ts Run-extraction and cap tests
packages/cli/src/ui/utils/resumeHistoryUtils.test.ts Resume ordering and nested-tool-image tests
packages/cli/src/ui/hooks/useHistoryManager.test.ts Bounded-compaction tests
packages/cli/src/ui/hooks/useReactToolScheduler.test.tsx Tool-image surfacing tests
packages/cli/src/ui/components/TerminalImage.test.tsx Component render and placeholder tests
packages/cli/src/ui/components/messages/ConversationMessages.test.tsx Assistant-image render tests
packages/cli/src/ui/components/messages/ToolMessage.test.tsx Tool-image render tests
…and 8 more files Remaining test coverage

Testing

This is an unattended CI run, so I did not build or execute any PR code — the signal below is the PR's own CI plus the sandboxed verification runs, read through the API.

CI has settled on this head. Qwen Code CI (the only pull_request-event run on b9ebfddf) completed green; nothing red, nothing pending:

Final CI results for b9ebfdd (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The skipped checks are by design, not a gap: Test (macos-latest), Test (windows-latest), and Integration Tests (CLI, No Sandbox) are gated on event_name == 'merge_group' in ci.yml — they never run at PR stage and report their exact names as skipped to satisfy required checks, then run for real in the merge queue.

Sandboxed verification (this head): the sponsored @qwen-code /verify run on b9ebfddf reported ✅ merge-ready — 55/55 scripted assertions. The central claim was A/B-proven load-bearing against the base build (base drops the image and emits 3 events; head emits 4 with ordered parts, flip-probe fails on base), the 33-check PNG validation matrix and the fake-chafa stdin wire oracle passed, and the mutation matrix shows the new tests go red when the change is reverted (core: 1 failed on base; TUI pipeline: 6/7 red under coarse mutation; the oversized-cap test pinned by a fine mutation). A second verify run re-triggered with this triage covers the same head and is redundant confirmation. That run is advisory evidence, not a substitute for review — but it is exactly the evidence the earlier "not verified" line asked for.

Real-terminal rendering: covered by the maintainer's own local real-stack verification — a real interactive TUI driven against a real HTTP model endpoint and a real stdio MCP server, on Ghostty (native Kitty placement) and tmux+chafa (fallback), with base-vs-head screenshots and raw TTY byte captures showing exactly two APC transmissions for two images. That verification ran on head ff4284b7a; I compared it to the current head and the PR-scoped delta since is documentation and E2E-plan wording only — every runtime file in this PR is byte-identical between the two heads.

Not verified: assistant-image persistence through --continue — a declared gap tracked in #8521, not a claim this slice makes.

中文说明

代码审查

审查于 b9ebfddf——与上次相同的提交,代码结论维持不变;此处为存档复述。我先独立写出方案,PR 的形态与之基本一致——所以我的工作是检查它是否偷工减料;它没有,而且重构到已合并的 #8217 渲染器之后反而更精炼。无关键阻塞项,无 AGENTS.md 违规。

核心增量形态正确,且我核对了内容事件的每一个下游消费者都能容忍它:turn.ts 新增仅在含图片 chunk 中发出的可选有序 partsvalue 保持不变。新情形是纯图片 chunk 现在会发出 value: ''parts(此前不发出事件)——我检查了其余五个消费者(client messageDisplay、循环检测、非交互 CLI 预览、JSON 输出适配器、子代理运行时),它们均把空 value 视为无操作,因此这一增量事件不会造成回归。下方的沙箱 A/B 运行在真实构建产物上独立确认了同样的消费者无碍结论。

渲染器入口(terminal-image-renderer.ts)是最强的部分。仅支持 PNG:base64 经规范化往返校验,IHDR 签名与尺寸在每次读取前都有边界检查,输入上限为解码后 8 MiB、单边 1,000,000 像素、总计 64,000,000 像素。无转义序列或命令注入面——图片字节只以 base64(安全字母表)进入 Kitty 序列,并经 stdin 传给 chafa(绝不作为路径参数),控制字段均为有界整数。无效、超大或非规范负载回退为确定性的 [image: WxH png] 占位符。解码与渲染缓存均有大小与字节上限。

重试/回退/取消/恢复间的顺序由 useGeminiStream 中的暂存区处理,它把已完成的文本/图片段保留在实时区直到正常提交边界——因此新的重试会丢弃失败尝试的部分图片,而新的用户轮次会先提交它们。useHistoryManager 现在在压缩时清除旧的助手与工具图片负载(一项真实的内存修复,扩展了现有 keep-recent 模式),useReactToolScheduler 呈现成功工具响应中的图片,且在合并后的 error/cancelled 分支中不丢失 resultDisplay。每个转换在新增的 1631 行 useGeminiStream 测试中都有专门用例。

维护者应权衡的一点(非阻塞——属文档化的首版范围):这是"渲染即忘",因此 Kitty 图片在滚动/resize 时从不被删除或重新放置。这是最可能需要后续跟进的部分(#8520),PR 也坦率说明了。

测试

这是无人值守的 CI 运行,因此我未构建或执行任何 PR 代码——下方信号是通过 API 读取的 PR 自身 CI 与沙箱验证运行。

CI 已在该 head 上收敛。Qwen Code CIb9ebfddf 上唯一的 pull_request 事件运行)以绿色完成;无红色项、无 pending(见上表)。skipped 的检查是设计使然而非缺口:Test (macos-latest)Test (windows-latest)Integration Tests (CLI, No Sandbox)ci.yml 中以 event_name == 'merge_group' 为条件——它们从不在 PR 阶段运行,而是以精确名称报 skipped 来满足必选检查,随后在合并队列中真正执行。

沙箱验证(本 head)b9ebfddf 上赞助的 @qwen-code /verify 运行报告 ✅ 可合入——55/55 脚本断言。核心主张经与 base 构建的 A/B 对照证明为真实有效(base 丢弃图片且只发 3 个事件;head 发 4 个且含有序 parts,翻转探针在 base 上失败),33 项 PNG 校验矩阵与 fake-chafa stdin 线协议预言全部通过,突变矩阵显示撤销改动后新测试变红(core:base 上 1 失败;TUI 链路:粗突变 6/7 红;超大上限测试由细突变钉住)。随本次 triage 重新触发的第二次 verify 覆盖同一 head,属冗余确认。该运行是评审的参考证据而非评审本身——但它正是此前"未验证"一行所要求的那份证据。

真实终端渲染:已由维护者本人的本地真实链路验证覆盖——在真实交互 TUI 中驱动真实 HTTP 模型端点与真实 stdio MCP 服务器,覆盖 Ghostty(原生 Kitty 放置)与 tmux+chafa(回退),附 base 对 head 的截图以及原始 TTY 字节捕获(两张图片恰好两次 APC 传输)。该验证运行于 head ff4284b7a;我将其与当前 head 对比,此后 PR 范围内的差异仅为文档与 E2E 计划的措辞——本 PR 的每个运行时文件在两个 head 之间逐字节一致。

未验证:助手图片经 --continue 的持久化——这是已声明的缺口,由 #8521 跟踪,并非本首版的主张。

Qwen Code · qwen3.8-max

Reviewed at b9ebfddface455cc4060215691176b581033e334 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean across every stage; the remaining point is the honest residue of things no PR stage can settle here (platform coverage rides the merge queue, follow-ups #8520/#8521 are deliberately deferred), not doubt about the code.

Stepping back: this is the right first slice of a maintainer-blessed feature, and it's built carefully. My independent read of the problem landed on essentially the same shape the PR took — an additive, optional parts field on the content event rather than a new event type, reuse of the existing renderer rather than a parallel implementation, and a staging region to keep text/image/text ordered across retry, fallback, and resume. Six review rounds in, the author restructured onto the merged #8217 renderer, addressed every blocking finding, and deferred the rest to tracked issues per the five-round rule. If I had to maintain this in six months I'd thank the author, not curse them.

The three reasons the last pass deferred are all settled, each on this exact commit:

  • Policy (maintainer sign-off): @wenshao — who this escalation was addressed to — reviewed the PR in depth, ran a local real-stack verification (real interactive TUI, real model endpoint, real MCP server, Ghostty native Kitty plus tmux/chafa fallback, with base-vs-head captures), and approved b9ebfdd explicitly. The awareness gate is satisfied.
  • CI: Qwen Code CI is green on this head — nothing red, nothing pending. The skipped matrix checks are merge-queue-only by design and will run there.
  • Behaviour evidence: the sponsored sandboxed verification on this head reported merge-ready (55/55 assertions, A/B proof that the change is load-bearing, mutation matrix showing the tests pin it), and the maintainer's real-terminal run covers the hardware step. The only delta between the verified head and this one is documentation wording — I compared them.

Acting on the verdict: approving, pinned to the reviewed commit. I'm also dismissing this bot's two stale CHANGES_REQUESTED reviews (on e5f892ed and 29298b9f): the first gated on the integration suite, which by current ci.yml never runs at PR stage; the second's Critical was fixed in ac7d564f and its Suggestions were deferred to #8608#8611 per the five-round rule, with later reviews on newer heads finding no blockers. Neither reflects open concerns on the code as it stands.

Merge is the maintainer's call — with the stale reviews dismissed and this approval in place, nothing on the bot's side blocks it anymore.

中文说明

置信度:4/5 —— 各阶段均干净;扣掉的一分是 PR 阶段在此无法收口的剩余项(平台覆盖由合并队列承担,后续 #8520/#8521 为有意延后),而非对代码的疑虑。

退一步看:这是经维护者认可的功能的正确首版,且构建得很用心。我对问题的独立判断与 PR 采取的形态基本一致——在内容事件上新增可选的 parts 字段而非新事件类型,复用现有渲染器而非另起一套实现,以及用一个暂存区在重试、回退与恢复间保持文本/图片/文本的顺序。六轮评审下来,作者重构到已合并的 #8217 渲染器之上,处理了每一个阻塞项,其余按五轮规则转交已立项的 issue。若六个月后由我维护,我会感谢作者而非责怪。

上次暂缓的三个原因都已在这一提交上解决:

  • 策略(维护者签字):本次升级所面向的 @wenshao 已深度评审该 PR,完成本地真实链路验证(真实交互 TUI、真实模型端点、真实 MCP 服务器,Ghostty 原生 Kitty 加 tmux/chafa 回退,附 base 对 head 的捕获),并明确批准了 b9ebfdd。知会门禁已满足。
  • CIQwen Code CI 在该 head 上为绿色——无红色项、无 pending。skipped 的矩阵检查按设计仅在合并队列运行,届时会真正执行。
  • 行为证据:该 head 上的赞助沙箱验证报告可合入(55/55 断言,A/B 证明改动真实有效,突变矩阵证明测试钉住了改动),维护者的真实终端运行覆盖了硬件步骤。经验证 head 与当前 head 之间唯一的差异是文档措辞——我已逐一对比。

按结论执行:批准,并钉在已审查的提交上。同时 dismiss 本机器人的两个过期 CHANGES_REQUESTED 评审(位于 e5f892ed29298b9f):第一个以集成套件为门槛,而按当前 ci.yml 它从不在 PR 阶段运行;第二个的 Critical 已在 ac7d564f 修复,其 Suggestions 按五轮规则转交 #8608#8611,且后续在更新 head 上的评审均未发现阻塞项。二者均不反映当前代码上的未决问题。

合并由维护者决定——过期评审被 dismiss、本批准就位后,机器人一侧不再有任何阻塞。

Qwen Code · qwen3.8-max

Reviewed at b9ebfddface455cc4060215691176b581033e334 · re-run with @qwen-code /triage

@tlysanhuo
tlysanhuo force-pushed the tlysanhuo/inline-terminal-images branch from 89071ef to 7b772c9 Compare August 1, 2026 12:33
@tlysanhuo

Copy link
Copy Markdown
Contributor Author

Rebase update

Rebased onto current main at 36698c566 and force-pushed commit 7b772c965.

The two initial conflicts were resolved by preserving the new Goal v3 flow while retaining ordered assistant image staging. A final hook-dependency audit exposed one additional integration edge: displayed GoalState events still committed only the current assistant row. That could place a goal card before earlier staged text/image runs. The handler now uses commitItemInOrder, and the GoalState regression test asserts the full text → image → text → goal_state order.

Validation on the new commit:

  • Focused CLI suite: 9 files, 422 tests passed.
  • Core Turn plus the newly landed workflow integration suite: 4 files, 138 tests passed.
  • npm run lint:ci: passed.
  • npm run typecheck: passed for every workspace.
  • npm run build: passed.
  • npm run check:serve-fast-path-bundle: passed.

The branch is now one commit ahead of and zero commits behind main.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

…rminal-images

# Conflicts:
#	packages/cli/src/ui/components/TerminalImage.test.tsx
#	packages/cli/src/ui/components/TerminalImage.tsx
#	packages/cli/src/ui/components/messages/ToolMessage.test.tsx
@tlysanhuo

Copy link
Copy Markdown
Contributor Author

Upstream integration update

Merged current main normally (no rebase or force-push) through e5f892ed2; the branch is now 0 commits behind upstream.

The important change is architectural: #8217 landed while this PR was under review, so this branch now reuses its accepted TerminalImage / terminal-image-renderer implementation instead of carrying a second renderer. The old direct iTerm2/multi-format path and duplicate tests were removed. This PR is now limited to the missing integration layer:

  • ordered model inlineData events;
  • live/resumed assistant and tool images;
  • in-memory validated PNG input for the feat(cli): add TUI image display tool #8217 renderer (chafa receives bytes over stdin);
  • retry/fallback/cancel/status-boundary ordering;
  • bounded UI-history image compaction.

The integration audit also fixed two boundary bugs: truncation/session-limit notices now follow the final assistant runs, and a displayed Goal-state event followed by another content chunk no longer duplicates the pre-Goal text buffer.

Final validation on e5f892ed2:

  • CLI affected suite: 443/443 passed (9 files)
  • Core affected suite: 873/873 passed (5 files)
  • latest upstream Web Shell suite: 30/30 passed
  • npm run lint:ci
  • npm run typecheck
  • npm run build
  • npm run check:serve-fast-path-bundle

The final PR diff is 25 files with 825 production additions / 132 deletions, down substantially from the earlier parallel-renderer version. The PR body and reviewer steps have been rewritten to match the new scope.

@wenshao

wenshao commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Review

Overview

This PR extends the #8217 terminal-image infrastructure from workspace-file previews to model/tool inlineData: Turn now emits an optional ordered parts field on content events (only when a chunk carries an image), the TUI stages mixed text/image runs as separate history rows to preserve text -> image -> text order across retry/fallback/cancel/boundary events, tool responses surface nested images on IndividualToolCallDisplay, resume reconstructs ordered runs, and UI compaction clears old image payloads. A new in-memory PNG entry point (prepareInlineTerminalImage) validates bounded base64 + PNG signature/IHDR before rendering via Kitty placement or chafa stdin.

Overall this is a careful, well-tested change. I verified the main correctness invariants and found no blocking bugs:

  • getDisplayContentParts (core) and getResponseText filter thoughts and join text identically, so value stays consistent with the concatenation of parts text runs — the text-only event shape is genuinely unchanged for SDK/ACP/daemon consumers.
  • readPngSize (via mermaidImageRenderer) does check the 8-byte PNG signature, so the signature + IHDR-length + dimension validation chain in readValidatedInlinePngSize is complete.
  • The staging invariant holds: stagePendingAssistantItem always immediately installs a new pending item at both call sites, so pendingAssistantItems is never non-empty while pendingHistoryItemRef is null — every commit/cancel path that checks pendingHistoryItemRef.current before commitItemInOrder therefore flushes staged runs correctly.
  • Resume's extractInlineContentRuns(parts, '\n') matches the old extractTextFromParts join separator, so text-only resumed messages render identically.

Security posture is good: size bounded before decode, canonical base64 round-trip check, no temp files, PNG bytes to chafa via stdin with - (no model-controlled argv), allowlisted child env (with a test asserting no env leak), and the placeholder format label is regex-restricted so no escape-sequence injection through mimeType.

Suggestions (non-blocking)

  1. Sync chafa spawn inside React render. prepareInlineTerminalImage runs in InlineTerminalImage's useMemo and can call execFileSync with an 8s timeout, blocking the Ink render loop. mermaidImageRenderer.ts explicitly labels its sync renderer as test-oriented ("the interactive TUI uses the async renderer to keep external processes outside React render"). This follows the feat(cli): add TUI image display tool #8217 file-path precedent so it's not a regression, but with N inline images a resize changes contentWidth, invalidates every cache key, and re-spawns chafa N times synchronously in one render pass. Worth a follow-up to move inline (and file) rendering onto the async path.

  2. unavailable results are never cached (rememberRenderResult is skipped), so a missing/broken chafa re-spawns on every remount of every image row. Caching negative results (or the findExecutable miss) would avoid repeated sync spawn attempts.

  3. Memory ceiling of compaction. UI_COMPACT_KEEP_RECENT = 20 now also applies to assistant image items, which at the 8 MiB payload cap means up to ~213 MB of base64 (UTF-16 doubles that in-heap) legitimately retained in React history, separate from the tool-group budget. Consider a smaller keep-recent for image-bearing items or a byte-based budget.

  4. No size cap at the core boundary. getDisplayContentParts forwards any image/* inlineData of any size; an oversized payload rides through the event stream and React state and is only rejected at render time, then lingers until compaction. Cheap length check in turn.ts (or truncation to a placeholder part) would bound this earlier.

  5. Duplicated image-part validation. getInlineImageData (packages/cli/src/ui/utils/inline-image-parts.ts) and getDisplayContentParts (packages/core/src/core/turn.ts) implement the same mime/data checks in two packages — drift risk if the accepted shape ever changes. Since ServerGeminiContentPart already lives in core, the predicate could too.

  6. Whitespace between images is silently dropped — intentional per the "does not overwrite an image with whitespace before the next image" test, but the early return in handleContentEvent deserves a one-line comment stating that whitespace-only text adjacent to an image is deliberately not rendered, since the buffer/pending interplay there is subtle.

  7. Base64url payloads become placeholders. The strict canonical round-trip in decodeInlineImage rejects -/_ alphabets and non-canonical padding. That's a safe default, but if any provider emits base64url inlineData, users will see [image: png] with no hint why; normalizing base64url before the check would be a small robustness win.

Test coverage

Strong: Turn ordering (incl. thought filtering and non-PNG mime passthrough), stream lifecycle (fresh/continuation retry, fallback, cancel, MAX_TOKENS/MaxSessionTurns/SessionTokenLimit/Goal-state boundaries, thrown streams), whitespace-adjacency edge cases, resume ordering for assistant and nested tool images, compaction for both assistant and tool payloads, renderer validation (oversize, malformed base64, bad IHDR, screen-reader, env-leak, stdin bytes). The chafa stdin test is skipped on Windows, so the .cmd shim + stdin combination is untested there — low risk since no model-controlled argv is involved, but worth covering in the Windows hardware pass along with the real Kitty/Ghostty verification the PR already defers to reviewers.

The buffer resets added to the ChatCompressed/Citation/StopHook branches fix a latent cumulative-buffer inconsistency beyond the image scope — good catch, and the MaxSessionTurns/SessionTokenLimit variants are covered by tests.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

中文说明

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/cli/src/ui/hooks/useGeminiStream.ts
Comment thread packages/cli/src/ui/hooks/useGeminiStream.ts
Comment thread packages/cli/src/ui/utils/inline-image-parts.ts Outdated
Comment thread packages/cli/src/ui/components/messages/ConversationMessages.tsx
@tlysanhuo

Copy link
Copy Markdown
Contributor Author

Review follow-up

Addressed all four findings in f74d9548f:

  • fixed the hidden-citation boundary so geminiMessageBuffer is cleared only when citations are enabled and the pending item is actually committed;
  • added the reproducing Content → hidden Citation → Content regression;
  • added ModelFallback mixed-run discard coverage;
  • added direct top-level tool inlineData extraction coverage;
  • added independent AssistantMessageContent / continuation image rendering coverage.

The citation regression failed before the fix with expected Hello world more, received more, and passes after the change. All four inline threads have been replied to and resolved.

Validation on f74d9548f:

  • affected CLI suite: 447/447 passed (10 files)
  • npm run lint:ci
  • npm run typecheck
  • npm run build

The skipped Integration Tests (CLI, No Sandbox) job is configured to run only for merge_group events in .github/workflows/ci.yml, so a normal PR run cannot execute it. If pre-merge deep verification is desired, a maintainer can sponsor @qwen-code /verify against this head.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.8-max-preview via Qwen Code /review (v0.21.3)

Comment thread packages/cli/src/ui/utils/inline-image-parts.test.ts Outdated
Comment thread packages/cli/src/ui/utils/terminal-image-renderer.test.ts
Comment thread packages/cli/src/ui/hooks/useGeminiStream.ts
Comment thread packages/cli/src/ui/hooks/useGeminiStream.test.tsx
Comment thread packages/cli/src/ui/components/TerminalImage.tsx Outdated

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.

— qwen3.7-max via Qwen Code /review

@tlysanhuo

Copy link
Copy Markdown
Contributor Author

Latest review follow-up

Addressed all five suggestion threads in 447bccb94:

  • /clear now discards staged assistant runs, the current pending item, and the pending retry error before committed history is cleared;
  • continuation retries have direct mixed text → image → text preservation coverage;
  • inline image helpers now have direct nested-response, non-image rejection, ordered-run, and thought-filtering tests;
  • the 1,000,001-pixel PNG IHDR limit has a regression test;
  • unavailable inline renderers now include the sanitized reason, matching file-backed images.

Validation on this head:

  • affected CLI suite: 685/685 passed (12 files)
  • npm run lint:ci
  • npm run typecheck
  • npm run build

All five new review threads have been replied to and resolved. The latest automated review reported no blockers, but the earlier CHANGES_REQUESTED review on the superseded head still controls the GitHub review decision.

@wenshao, when convenient, could you dismiss the stale changes-requested review or submit a formal approval if the current head looks good?

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: @qwen-code /review --timeout=240. See workflow logs.

@tlysanhuo

Copy link
Copy Markdown
Contributor Author

@qwen-code /review --timeout=240

@wenshao

wenshao commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Review: feat(cli): render inline terminal images

Reviewed the full 29-file diff against pr8305-head (d253fd7), plus the surrounding code it touches (turn.ts, useGeminiStream.ts, mermaidImageRenderer.ts, loopDetectionService.ts, the non-interactive/JSON adapters).

Overview

Extends #8217's terminal-image infrastructure from workspace file previews to in-memory inlineData:

  • ServerGeminiContentEvent gains an optional ordered parts field, emitted only when a chunk carries an image; value stays the concatenated text.
  • terminal-image-renderer gains prepareInlineTerminalImage() — a bounded base64 → PNG-validation → Kitty/chafa-over-stdin entry point sharing feat(cli): add TUI image display tool #8217's sizing, cache and transmitted-key set.
  • The TUI stages ordered text/image runs (pendingAssistantItems) so text → image → text survives retry, fallback, cancel, boundary events and resume; tool functionResponse.parts images are attached to IndividualToolCallDisplay.
  • UI history compaction drops old image payloads.

What's good

  • Reuse over reinvention. Dropping the in-PR renderer and the direct iTerm2 path in favour of the feat(cli): add TUI image display tool #8217 layer is the right call, and renderWithChafa was widened via a ChafaImageSource union rather than duplicated.
  • The value / parts contract can't drift. getDisplayContentParts filters on !part.thought && typeof part.text === 'string' && length > 0, which is exactly getResponseText's part.text && !part.thought. Verified the four other GeminiEventType.Content consumers (client.ts messageDisplay.addChunk, BaseJsonOutputAdapter.appendText, nonInteractiveCli preview, loopDetectionService.checkContentLoop) all tolerate the new value: '' image-only event — appendText early-returns on empty, and checkContentLoop('') is idempotent (isDivider is +-quantified so '' doesn't trip the divider reset).
  • Security posture is solid. Image bytes never reach argv ('-' + input:), the cmd-metacharacter guard is correctly narrowed to the filePath arm, createRendererChildEnv is preserved, base64 is validated for canonicality and bounded before Buffer.from, PNG signature comes via readPngSize, and both the placeholder (getImageFormat's [a-z0-9.+-] regex) and the unavailable reason (sanitizeTerminalText) are sanitized. Screen-reader mode short-circuits to text with disabled: true.
  • Two genuine adjacent bug fixes. Several handlers (handleChatCompressionEvent, handleCitationEvent, handleUserPromptSubmitBlockedEvent, handleStopHookLoopEvent, MaxSessionTurns, SessionTokenLimitExceeded, displayed GoalState) commit + null the pending item but previously left geminiMessageBuffer intact, so the next content chunk re-rendered all prior text into a fresh block — visible duplication. Resetting the buffer fixes that. Likewise, moving handleFinishedEvent after the commit stops a MAX_TOKENS warning from jumping above the answer it describes. The Citation reset is correctly gated on showCitations(settings) to mirror the handler's early return — nice attention to detail.

Issues

1. (High) Unbounded image count × synchronous execFileSync inside React render

extractInlineImages collects every image in a tool response with no cap, and an assistant chunk can carry many inlineData parts. Each TerminalImage calls prepareInlineTerminalImage from a useMemo during render, and the non-Kitty path runs execFileSync(chafa, …) with CHAFA_TIMEOUT_MS = 8000. A tool or model returning 20 images therefore performs 20 sequential blocking spawns in one render pass; a stalled chafa costs 8 s each.

#8217's justification for sync rendering — "Completed tool rows enter Ink's append-only Static region immediately, so rendering must stay synchronous" (terminal-image-renderer.ts:38) — no longer holds: assistant images now render in the live/dynamic region while the stream is still running. The sibling renderer draws the opposite conclusion explicitly: "the interactive TUI uses the async renderer to keep external processes outside React render" (mermaidImageRenderer.ts:370-373).

Suggested: cap images per assistant item / tool row (render the first N, collapse the rest to [+K more images]), and consider the async render path for the non-Static case. A cap alone would remove the worst of it.

2. (Medium) Payloads that can never render are still retained in UI history

getInlineImageData accepts any non-empty base64 string; the 8 MiB limit is only applied later in prepareInlineTerminalImage. So a 50 MB base64 blob is copied into HistoryItem.images / IndividualToolCallDisplay.images and held for the session while rendering only ever as [image: png]. Cheap fix: apply the same encoded-length bound in getInlineImageData so unrenderable payloads are never mirrored into UI state.

Related: UI compaction is count-based (UI_COMPACT_KEEP_RECENT = 20), so the worst case retains ~20 × 8 MiB of base64 in UI history on top of provider/session history and the 32 MiB render cache. A byte budget for image-bearing items would bound this properly.

3. (Medium) Unannounced behaviour change for text-only turns

assistantOutputStarted is reset only by Finished / non-continuation Retry / ModelFallback — not by the status-row handlers. Combined with the new geminiMessageBuffer = '', text resuming after a mid-stream info row (chat compression, citation, max-session-turns, goal state, stop-hook message) now starts as gemini_content instead of gemini, i.e. it loses the prefix and its marginTop: 1.

That's plausibly the nicer rendering, but it changes non-image turns and nothing in the test suite pins it. Please confirm it's intended and add a text-only regression test (content → ChatCompressed → content) asserting both the absence of duplication and the resulting item type.

4. (Low) displayName is plumbed end-to-end but never rendered

It's extracted in getDisplayContentParts, again in getInlineImageData, stored on history items and asserted in three test files — yet no component reads it. Either surface it (it would make a far better placeholder / screen-reader label than [image: 1024x768 png]) or drop it rather than retain and persist a dead field.

5. (Low) clearPendingState clears 3 of 5 pending slots

It resets pendingAssistantItems, pendingHistoryItem and pendingRetryErrorItem, but leaves pendingThoughtItem and pendingRetryCountdownItem, so /clear mid-stream can still leave a stray thought or countdown row in the dynamic region. If the intent is "discard the dynamic region", include those two; otherwise the name overstates the scope.

6. (Low) Structurally duplicated type across the package boundary

ServerGeminiContentPart['inlineData'] (core) and InlineImageData (cli) are identical shapes kept in sync by hand — the CLI already imports core types in these very files. Exporting one shape from core would remove the drift risk.

7. (Low) showCitations(settings) is now evaluated in two places

processGeminiStreamEvents re-derives it to decide whether to reset the buffer, mirroring handleCitationEvent's early return; if either side changes, they drift silently. Having the handler report whether it committed would be more robust. This also adds settings to processGeminiStreamEvents' dependency array — worth a sanity check that a mid-stream settings change recreating the callback is harmless (it looks fine, since it's only invoked at turn start).

8. (Nit) decodeInlineImage allocates ~3× the payload per validation

data.replace(/\s/g, '') plus decoded.toString('base64') for the canonicality round-trip, at up to 8 MiB. Acceptable at current caps, but the round-trip's purpose (rejecting non-canonical base64 such as AB==) is non-obvious — worth a one-line comment.

9. Test coverage gaps

The new suite is thorough on the image lifecycle (fresh/continuation retry, fallback, thrown-stream retry, cancellation, goal state, whitespace-between-images). Missing:

  • Buffer-reset coverage for ChatCompressed, StopHookSystemMessage, UserPromptSubmitBlocked, StopHookLoop (Citation, MaxSessionTurns and GoalState are covered).
  • The text-only regression from 如何自定义密钥文件 .env可能与其他文件冲突 #3.
  • An inline-path equivalent of the file-path "remount does not re-transmit the Kitty payload" test — the inline test asserts the first write but not that a remount skips it.
  • ToolGroupMessage: the image-bearing-collapsible case is covered, but not that countToolCallsWithResults height accounting now includes an image-only tool (resultDisplay === '').

10. (Nit) Docs

docs/design/terminal-inline-images.md says the entry point "verifies the PNG signature and IHDR dimensions" — accurate, but the signature check lives inside readPngSize while readValidatedInlinePngSize only re-checks the IHDR length/tag. A one-line comment there would stop a future reader from concluding the signature check is missing.


Verdict

The design is sound, the core event change is genuinely backward-compatible, and the security handling of model-controlled bytes is careful. I'd like #1 (cap the image count, and reconsider sync spawning in the live region) and #3 (confirm + pin the text-only behaviour change) addressed before merge; #2 is a cheap win worth taking with them. The rest are fine as follow-ups.

Note that the manual Kitty/Ghostty/chafa verification is still outstanding per the PR description — the automated evidence covers the forced-protocol and stdin paths, but the ordering behaviour in a real terminal (particularly a mid-stream image followed by more text, which is the new live-region case) hasn't been observed on hardware.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): 443 tests passed — this review observed 16885, 18976 passed; 873 tests passed — this review observed 16885, 18976 passed; 30 tests passed — this review observed 16885, 18976 passed.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):443 tests passed — this review observed 16885, 18976 passed; 873 tests passed — this review observed 16885, 18976 passed; 30 tests passed — this review observed 16885, 18976 passed

— qwen3.8-max-preview via Qwen Code /review (v0.21.3)

Comment thread packages/cli/src/ui/components/HistoryItemDisplay.tsx
Comment thread packages/cli/src/ui/hooks/useReactToolScheduler.ts Outdated
@tlysanhuo

Copy link
Copy Markdown
Contributor Author

@wenshao The synchronized-head CI run now needs fork approval: https://github.com/QwenLM/qwen-code/actions/runs/30914560077. The exact previously failing Chrome-extension package + artifact scan passes locally on 1e1a6e8a4, along with 579 focused CLI tests and workspace typecheck.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs.

@tlysanhuo

Copy link
Copy Markdown
Contributor Author

@wenshao Thanks for syncing main. The refreshed automatic review timed out again after its 360-minute limit without producing new findings: https://github.com/QwenLM/qwen-code/actions/runs/30967660010. All review threads are resolved, but the review decision still reflects the old bot CHANGES_REQUESTED on 29298b9. The refreshed fork CI also still needs maintainer approval: https://github.com/QwenLM/qwen-code/actions/runs/30967659861. Could you please approve the CI run and clear/re-review the stale decision when convenient?

@tlysanhuo

Copy link
Copy Markdown
Contributor Author

@wenshao The approved CI run 30967659861 failed only at the head-SHA guard: it expected fcd0eb7d, but checkout already contained the new b9ebfdd head after the latest main merge. No code tests ran; this is a stale-run race, not a PR failure. Current-head CI is https://github.com/QwenLM/qwen-code/actions/runs/31024524332 and needs approval.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline. Not reviewed: reverse audit — stopped before round 3 by the review time budget. Test Plan (not a blocker): 579 tests passed — this review observed 17831, 19162, 1067, 1455, 475, 2825, 444 passed.

中文说明

已审查。 建议见行内评论。 未审查:反向审计——评审时间预算不足,未能开始第 3 轮。 Test Plan(非阻断):579 tests passed — this review observed 17831, 19162, 1067, 1455, 475, 2825, 444 passed

— qwen3.8-max via Qwen Code /review (v0.21.6)

Comment thread .qwen/e2e-tests/terminal-inline-images.md
Comment thread packages/cli/src/ui/hooks/useGeminiStream.test.tsx
Comment thread packages/cli/src/ui/utils/terminal-image-renderer.ts
Comment thread packages/cli/src/ui/hooks/useGeminiStream.ts
Comment thread packages/cli/src/ui/hooks/useGeminiStream.ts
Comment thread packages/cli/src/ui/components/messages/ConversationMessages.tsx
Comment thread packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx
Comment thread packages/cli/src/ui/hooks/useGeminiStream.test.tsx
Comment thread packages/cli/src/ui/hooks/useHistoryManager.test.ts
Comment thread packages/cli/src/ui/hooks/useGeminiStream.test.tsx
@tlysanhuo

Copy link
Copy Markdown
Contributor Author

@wenshao The latest automatic review completed successfully on b9ebfdd and reported Suggestions only, with no new Critical blocker. Per the repository rule for PRs past roughly five review rounds, I tracked them in #8608, #8609, #8610, and #8611 instead of expanding this diff; every thread has been replied to and resolved (0 unresolved). Remaining gates are maintainer-only: approve current-head CI https://github.com/QwenLM/qwen-code/actions/runs/31024524332, then clear the stale bot CHANGES_REQUESTED from 29298b9 and merge.

@wenshao

wenshao commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 55 passed · 0 failed · 55 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:55 通过 · 0 失败 · 55 总计

Verification report

PR 8305 — feat(cli): render inline terminal images

Verdict: merge-ready — 55/55 scripted assertions passed, 0 unexpected failures. Verified head b9ebfddface455cc4060215691176b581033e334 (merge d799d715 over base 2c514b50). First verification round (no previous-report.md present).

中文摘要
  • 结论:merge-ready。55/55 脚本化断言通过,0 个意外失败;无阻塞性发现。
  • A/B 结论(核心主张):在真实构建的 Turn 上驱动 [text, image/png, thought, text] → [image/webp only] → [text] 流:head 发出 4 个事件(含有序 parts 与仅图片事件),base 只发 3 个且完全丢弃图片(见下表与 01-ab-core-turn-parts.png)。拼接文本 value 契约两臂一致(beforeafter tail)。
  • 验证矩阵:内存 PNG 入口 33/33(8 MiB 编码上限、64 MP 像素上限的 8000×8000 边界接受 / 8000×8001 拒绝、非规范 base64 拒绝、非 PNG MIME 占位符、screen-reader 占位符、无 chafa 时 unavailable);fake-chafa wire oracle 5/5(PNG 字节经 stdin 逐字节送达、argv 以 - 结尾、ANSI 行回传)。
  • 测试非空性:head 的 turn.test.ts 跑在 base 代码上 = 1 failed | 36 passed(恰好是新测试);粗突变(删图片缓冲)6/7 红;细突变(删编码长度上限)使 oversized 测试红;恢复后树干净。
  • 门禁:core turn 37/37;CLI 14 文件 451/451;useGeminiStream+AppContainer 345/345;全仓 typecheck 退出 0。
  • 未覆盖:真实 Kitty/Ghostty/iTerm2 硬件渲染(容器无真实终端,用强制协议 + fake chafa 代替);逐 commit 归因(shallow checkout,15 个 commit 本地仅可达 1 个);助手图片的 --continue 持久化(PR 已声明的缺口,feat(core): persist assistant inline images for session resume #8521)。

Central claim and A/B

Central claim: model inlineData, previously dropped at the Core→TUI event boundary, now flows as ordered parts on ServerGeminiContentEvent while the concatenated value contract and the text-only event shape stay unchanged.

Harness ab-turn.mjs drives the real built Turn (dist) of each tree with a mock chat whose stream carries chunk1 [text 'before' | image/png | thought | text 'after'], chunk2 [image/webp only], chunk3 [text ' tail']. Witness: evidence/01-ab-core-turn-parts.png.

cell build oracle result
1 (control) base 2c514b5 base expectations 5/5 pass — 3 events; image-only chunk emits nothing; no parts key anywhere
2 head b9ebfdd head expectations 7/7 pass — 4 events; chunk1 parts = [text, image, text] in order; chunk2 emits {value:'', parts:[webp]}; chunk3 keeps old shape (no parts key)
3 (flip probe) base head expectations 4 head-only checks fail on base (event count, ordered parts, image-only event, shape) — the flip that proves the change load-bearing
both arms value contract concat of Content values = beforeafter tail on both arms

Base-side build note: packages/core was compiled in a scratch worktree at HEAD^1 with the shared root node_modules (the PR touches no package.json/lockfile, so third-party deps are a clean control; core has no @qwen-code/* runtime deps, so the workspace-symlink trap does not exist — verified by grep; the only nesting fix needed was linking the pre-existing packages/core/node_modules (ajv v8 etc.) into the worktree).

Secondary evidence (mock-free harnesses)

In-memory PNG entry pointpng-validation.mjs, 33/33, witness evidence/02-png-validation-matrix.png. Highlights:

  • accept path under forced Kitty env: kind:'kitty', sequence starts \x1b_G, fallback [image: 8x8 png];
  • pixel cap boundary: 8000×8000 (= 64,000,000) accepted, 8000×8001 and 9000×9000 rejected → [image: png]; dimension cap 1,200,000 rejected;
  • non-canonical base64 (AB==, lenient-decodable but re-encode-mismatch) rejected; truncated PNG rejected; image/jpeg[image: jpeg], image/svg+xml[image: svg+xml], text/plain[image];
  • screen-reader (disabled:true) → sized placeholder, no sequence;
  • stripped env (container ships no chafa) → kind:'unavailable' with chafa-named reason;
  • pre-history gate getInlineImageData: oversized encoded length (> ceil(8MiB·4/3)+4) dropped before UI state; cap/overflow: 6 images → 4 kept + omitted_images count 2; nested functionResponse.parts collected; text→image→text runs preserved; thought parts skipped.

chafa stdin wire oraclechafa-stdin-oracle.mjs, 5/5, witness evidence/04-chafa-stdin-oracle.png. A fake chafa on PATH records argv and stdin; the real renderer (non-Kitty env) invoked it with --format=symbols --size=WxH and final arg - (never the payload as an argument), delivered the PNG byte-identical over stdin, and the printed rows round-tripped as kind:'ansi'.

ReDoS ladder on the three new regexes (image/... format, base64 charset, whitespace strip) at 20 k hostile chars: all < 1 ms — linear, no catastrophic backtracking.

Vacuity / mutation matrix

Witness evidence/03-vacuity-mutations-red.png; scripted re-run vacuity-assert.sh 5/5.

mutation suite outcome classification
none (head tests on base code) core turn.test.ts 1 failed | 36 passed — the failing test is exactly should preserve ordered image parts in content events, failing on the intended deep-equal of events core test load-bearing
coarse: Content handler reverted to base form (push event.value only) useGeminiStream.test.tsx -t "inline image" 6 failed | 1 passed (ordering, caps, resets all red) TUI pipeline load-bearing
coarse survivor does not retain an oversized inline image payload same passes under coarse mutation because it asserts an absence the coarse mutation also removes escalated, not a gap
fine: drop encoded-length cap in getInlineImageData same, -t "oversized inline image" 1 failedexpected [ { type: 'gemini', … images } ] to deeply equal [] oversized test pins the cap
restores git diff clean (0 changed files)

Reviewer Test Plan walk-through

step result
1. Kitty/Ghostty native placement for model + tool + resumed images not runnable on hardware here (container, no real terminal). Substituted: forced-protocol harness proves the Kitty sequence/placeholder path on the real renderer; ordering/resume covered by the green suites below
2. chafa environments partially runnable: fake-chafa oracle proves stdin delivery and ANSI round-trip; real chafa not installed in this container
3. placeholders without chafa / screen-reader / invalid data; oversized dropped pre-history verified in the 33-check matrix
4. retry / fallback / cancellation / Goal-state / resume ordering verified by executed tests: the 7 -t "inline image" tests (incl. retry/fallback/finished cap resets) plus resumeHistoryUtils, useHistoryManager suites — and proven non-vacuous by the mutation matrix

Targeted gates (head)

  • core: turn.test.ts 37/37 (logs/gate-core.log)
  • cli batch (14 files: inline-image-parts, terminal-image-renderer, TerminalImage, resumeHistoryUtils, useHistoryManager, ConversationMessages, ToolMessage, ToolGroupMessage, HistoryItemDisplay, slashCommandProcessor, useBranchCommand, useResumeCommand, restoreCommand, useReactToolScheduler): 451/451 (logs/gate-cli-units.log)
  • cli large (useGeminiStream.test.tsx 204 + AppContainer.test.tsx): 345/345 (logs/gate-cli-large.log)
  • npm run typecheck (all workspaces): exit 0 (logs/typecheck.log)

Findings

No blocking findings. Non-blocking observations, each bounded:

  1. New empty-value Content events reach non-TUI consumers (an image-only chunk emits {value:''} where base emitted nothing). Traced all four consumers and verified inert: BaseJsonOutputAdapter.appendText('') returns early; loopDetectionService.checkContentLoop('') appends nothing to history; nonInteractiveCli preview concat of '' is a no-op; messageDisplay.addChunk('') is a no-op. Informational, no action needed.
  2. Coarse-mutation survivor on the oversized-payload test is adjudicated by the fine mutation (table above) — a coverage property of an absence-assertion, not a dead guard.

Not covered

  • Real-hardware Kitty/Ghostty/iTerm2/Warp rendering and real chafa decoding (environmental: container has no TTY terminal and no chafa; base and head share this limitation — the substituted oracles drive the real renderer code paths).
  • Per-commit attribution: checkout is shallow (--is-shallow-repository true; 1 commit reachable in HEAD^1..HEAD^2 vs 15 listed in the metadata snapshot), so only the aggregate diff was verified.
  • Assistant-image persistence through --continue — declared gap in the PR, tracked upstream as feat(core): persist assistant inline images for session resume #8521; resume of tool images and of already-persisted assistant parts is what this slice claims and what the resume suite covers.
  • Interactive TUI visual rendering (no tmux capture run); component behavior covered by unit/component suites only.
  • Windows .cmd/.bat chafa shim path (Linux container).
  • Full-repo test suite (targeted suites + repo-wide typecheck only; the PR's own CI covers the rest).

Methodology

Environment: CI verify container (node:22-bookworm), merge-ref checkout (HEAD merge d799d715, HEAD^1 base, HEAD^2 verified head), npm ci+npm run build pre-provided. A/B drove the compiled dist Turn of each tree via ab-turn.mjs (mock chat, real event loop); base core was compiled in a scratch worktree (removed afterwards) reusing root node_modules — clean control since the PR changes no dependency manifests. Validation/chafa harnesses imported the real built CLI modules (packages/cli/dist/...) with no stubs of the code under test; the chafa peer was a recording fake on PATH. Mutations were applied to scratch copies of two source files, executed through vitest, and restored (tree verified clean). Raw logs in logs/, harnesses at the artifact root, captures in evidence/ (01 A/B, 02 validation matrix, 03 vacuity reds, 04 chafa oracle).

Evidence images

01-ab-core-turn-parts

02-png-validation-matrix

03-vacuity-mutations-red

04-chafa-stdin-oracle

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@tlysanhuo

Copy link
Copy Markdown
Contributor Author

@wenshao Thanks for approving and running verification. Current-head CI is fully green and the sandbox report says merge-ready. GitHub still shows CHANGES_REQUESTED from the old bot review on 29298b9, so the PR remains BLOCKED despite your approval. Could you please dismiss that stale review and merge?

@wenshao

wenshao commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 55 passed · 0 failed · 55 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:55 通过 · 0 失败 · 55 总计

Verification report

PR 8305 — feat(cli): render inline terminal images (follow-up round)

Verdict: merge-ready — 55/55 scripted assertions passed, 0 unexpected failures. Verified head b9ebfddface455cc4060215691176b581033e334 (merge d799d715 over base 2c514b50). Follow-up round: the verified head, merge commit, and base are byte-identical to the previous round's (HEAD^2, HEAD, HEAD^1 all match the previous report's cited OIDs), so the input closure is proven identical at the commit level; every carried measurement was nonetheless re-run at this head per the follow-up contract, not diffed from the old report.

中文摘要
  • 结论:merge-ready(跟进轮)。本轮验证的 head b9ebfdd、merge d799d715、base 2c514b50 与上一轮完全相同(输入闭包在 commit 级别可证一致),但所有关键测量均按跟进轮规则重新执行,而非沿用旧报告数字。55/55 脚本断言通过,0 个意外失败。
  • A/B 结论(核心主张):真实构建的 Turn 上驱动 [text, image/png(+displayName), thought, text] → [image/webp only] → [text] 流:head 发 5 个事件(含有序 parts、仅图片事件 {value:'', parts:[webp]}、纯文本事件保持旧形状),base 发 4 个且完全丢弃图片;flip probe 中 4 项 head 专有检查在 base 上全红(见下表与 01-ab-core-turn-parts.png)。拼接 value 契约两臂一致。
  • 验证矩阵:内存 PNG 入口 25/25(64 MP 边界 8000×8000 接受 / 8000×8001 与 9000×9000 拒绝、1,200,000 维上限拒绝、>8 MiB 噪声 PNG 拒绝、非规范 base64 拒绝、空白 base64 容忍、截断 PNG 拒绝、非 PNG MIME 占位符、screen-reader 占位符、无 chafa 时 unavailable、编码长度上限边界);fake-chafa wire oracle 5/5(stdin 逐字节一致、argv 以 - 结尾、ANSI 行回传)。
  • 测试非空性:head 的 turn.test.ts 跑在 base 代码上 = 1 failed | 36 passed(恰为新测试);粗突变 6 failed | 1 passed(存活者恰为 oversized 测试);细突变(删编码长度上限)使 oversized 测试以预期断言红;恢复后树干净。
  • 上轮发现状态:两条非阻塞观察均 stands(空 value 事件对四个非 TUI 消费者惰性,本轮以脚本化 stepMessageDisplay 检查 + 代码复核重新测量;coarse 存活者仍由 fine 突变裁决)。新增一条信息级观察:解码后字节上限几乎被编码长度上限包含(仅 ~4 字节窗口可达),像素展开实际由 64 MP 上限约束——属纵深防御冗余,非缺陷。
  • 未覆盖:真实 Kitty/Ghostty/iTerm2 硬件渲染与真实 chafa(容器无终端/chafa,两臂同限);逐 commit 归因(shallow,15 个 commit 仅 1 个本地可达);助手图片 --continue 持久化(PR 已声明,feat(core): persist assistant inline images for session resume #8521)。

Previous-finding status table (follow-up round)

# previous finding severity status at b9ebfdd (unchanged head) re-measurement
1 New empty-value Content events ({value:''} for image-only chunks) reach non-TUI consumers; all four traced consumers inert non-blocking / informational stands (still inert) empty-value-consumers.mjs 3/3 against the built stepMessageDisplay (no flush on '', no text injection); re-read BaseJsonOutputAdapter.appendText (if (fragment.length === 0) return; at BaseJsonOutputAdapter.ts:819), loopDetectionService.checkContentLoop (streamContentHistory += '' no-op at loopDetectionService.ts:573), nonInteractiveCli preview (+= String('').slice(…) no-op at nonInteractiveCli.ts:2217)
2 Coarse-mutation survivor on does not retain an oversized inline image payload (absence-assertion also removed by the coarse mutation) non-blocking / adjudicated stands coarse mutation re-run: 6 failed | 1 passed, survivor is exactly that test; fine mutation (drop encoded-length cap) re-run: 1 failed with expected [ { type: 'gemini', … } ] to deeply equal [] — the cap is pinned
prior not-covered list (hardware terminals, per-commit attribution, assistant-image persistence) n/a unchanged same environmental constraints this round (see Not covered)

No declined or deferred rows existed in the previous report; nothing to re-measure beyond the above.

Central claim and A/B

Central claim: model inlineData, previously dropped at the Core→TUI event boundary, now flows as ordered parts on ServerGeminiContentEvent while the concatenated value contract and the text-only event shape stay unchanged.

Harness ab-turn.mjs drives the real built Turn (each tree's dist, imported by absolute path so each arm can only load its own code) with a mock chat whose stream carries chunk1 [text 'before' | image/png (+displayName) | thought | text 'after'], chunk2 [image/webp only], chunk3 [text ' tail', STOP]. Witness: evidence/01-ab-core-turn-parts.png.

cell build oracle result
1 (control) base 2c514b5 base expectations 4/4 pass — events thought, content, content, finished; image-only chunk emits nothing; no parts key anywhere
2 head b9ebfdd head expectations 5/5 pass — events thought, content(parts), content(parts), content, finished; chunk1 parts = [text, image/png(+displayName), text] in order with thought skipped; chunk2 {value:'', parts:[webp]}; chunk3 old shape (no parts key)
3 (flip probe) base head expectations 4 head-only checks fail on base (event count, ordered parts, image-only event, text-only shape) — the flip proving the change load-bearing; the arm-agnostic value contract still passes
both arms value contract concat of Content values = beforeafter tail on both arms

Base-side build note (methodology correction to the previous round's note): compiling base core in the scratch worktree required two links, not one — packages/core/node_modules (nested deps) and the repo-root node_modules at the worktree root. The second is needed because packages/core/tsconfig.json works around @lydell/node-pty's missing type exports with paths: {"@lydell/node-pty": ["../../node_modules/@lydell/node-pty/node-pty.d.ts"]}, which resolves relative to the package and therefore only exists when a root node_modules sits two levels up. Without it the base build fails with TS7016 while the identical head build passes — a worktree-layout artifact, not a code difference (the file and tsconfig are byte-identical across the PR diff; verified by sha256). With both links in place the control is clean: the PR touches no package.json/lockfile and core's dist/turn.js imports no @qwen-code/* workspace (grep-verified), so the shared third-party deps cannot confound the A/B.

Secondary evidence (mock-free harnesses)

In-memory PNG entry pointpng-validation.mjs, 25/25, witness evidence/02-png-validation-matrix.png. Highlights: forced-Kitty accept (kind:'kitty', sequence \x1b_G, [image: 8x8 png]); pixel-cap boundary 8000×8000 accepted / 8000×8001 and 9000×9000 rejected; 1,200,000×1 dimension cap rejected; >8 MiB incompressible-noise PNG rejected while a small-file 3000×3000 (9 MP) is accepted (expansion bounded by the 64 MP pixel cap); non-canonical base64 rejected, whitespace-tolerant canonical base64 accepted, truncated PNG rejected; image/jpeg[image: jpeg], image/svg+xml[image: svg+xml], text/plain[image], case/whitespace mime normalized; screen-reader disabled → sized placeholder without sequence; stripped env → kind:'unavailable' naming chafa; encoded-length gate boundary at MAX_INLINE_IMAGE_ENCODED_LENGTH (cap passes, cap+1 dropped); 6 images → 4 kept + omitted 2; nested functionResponse.parts collected; text→image→text runs preserved with thought skipped; 20 k hostile mime/data inputs < 1 ms (linear, no ReDoS).

chafa stdin wire oraclechafa-stdin-oracle.mjs, 5/5, witness evidence/03-chafa-stdin-oracle.png. A fake chafa on PATH (found via the hardened findExecutable, which skips node_modules/.bin) recorded argv and stdin; the real renderer invoked it with --format=symbols --size=WxH and final arg -, delivered the PNG byte-identical over stdin (never as an argument), and the printed rows round-tripped as kind:'ansi' with the sized placeholder.

Vacuity / mutation matrix — witness evidence/04-mutation-matrix-red.png; raw logs logs/vacuity-head-tests-on-base.log, logs/mutation-coarse.log, logs/mutation-fine.log.

mutation suite outcome classification
none (head turn.test.ts on base code) core turn.test.ts 1 failed | 36 passed — failing test is exactly should preserve ordered image parts in content events, failing on the intended deep-equal core test load-bearing
coarse: Content handler reverted to base form (push {kind:'content', value: event.value}) useGeminiStream.test.tsx -t "inline image" 6 failed | 1 passed — ordering, caps, and all three reset tests red; survivor is the oversized-payload test TUI pipeline load-bearing; survivor adjudicated by fine mutation
fine: drop encoded-length cap in getInlineImageData same, -t "oversized inline image" 1 failedexpected [ { type: 'gemini', … } ] to deeply equal [] oversized test pins the cap
restores git checkout both files tree clean (only the env-locked .qwen doc absent, see Methodology)

Re-measurement of carried finding #1 (empty-value consumers)

Scripted: empty-value-consumers.mjs 3/3 against the built stepMessageDisplay'' on a fresh state produces no flush and no text; '' after an actually-flushed chunk has hasNewText=false (no spurious flush); '' interleaved with text never injects text into the cumulative display. By code re-inspection at this head: appendText('') returns early (BaseJsonOutputAdapter.ts:819); checkContentLoop('') appends nothing to the repetition history (loopDetectionService.ts:573); the non-interactive preview concatenates '' (nonInteractiveCli.ts:2217). Status: stands, informational, no action needed.

Targeted gates (head)

  • core turn.test.ts: 37/37 (logs/gate-core.log)
  • CLI focused batch, 14 files (inline-image-parts, terminal-image-renderer, TerminalImage, resumeHistoryUtils, useHistoryManager, ConversationMessages, ToolMessage, ToolGroupMessage, HistoryItemDisplay, slashCommandProcessor, useBranchCommand, useResumeCommand, restoreCommand, useReactToolScheduler): 451/451 (logs/gate-cli-units.log)
  • CLI large (useGeminiStream.test.tsx + AppContainer.test.tsx): 345/345 (logs/gate-cli-large.log)
  • npm run typecheck (all workspaces): exit 0 (logs/typecheck.log)

The first parallel launch of the unit batch died with an ENOENT on packages/cli/coverage/.tmp — two concurrent vitest processes racing on the shared coverage dir (the large gate ran in the same package). Re-run alone: green. Infrastructure race in my harness scheduling, not a PR defect; an A/A check was unnecessary because the error is a reporter-path ENOENT with zero test results, unrelated to PR code.

Findings

No blocking findings. Non-blocking observations, each bounded:

  1. Carried pre-release: fix ci #1 stands — empty-value Content events reach non-TUI consumers but are inert at all four (table above). Informational, no action needed.
  2. New, informational: the decoded-byte cap is almost entirely subsumed by the encoded-length cap. decodeInlineImage re-checks decoded.length > MAX_TERMINAL_IMAGE_BYTES after base64 decode, but any payload whose decoded length exceeds 8 MiB already exceeds MAX_INLINE_IMAGE_ENCODED_LENGTH = ceil(8MiB·4/3)+4 in base64 form (leaving at most a ~4-byte padding window where the second check can fire). The check is therefore defense-in-depth, and raw-pixel expansion of a small valid PNG is what the 64 MP cap bounds (measured: 3000×3000 zero PNG accepted, 9 MP; 8000×8001 rejected). Not a defect — the two caps together bound both stored bytes and expansion; noted so a future reader does not mistake the decoded check for the primary byte guard.

Not covered

  • Real-hardware Kitty/Ghostty/iTerm2/Warp rendering and real chafa decoding (container has no TTY terminal and no chafa; base and head share the limitation — the substituted oracles drive the real renderer code paths, forced-protocol and fake-chafa respectively).
  • Per-commit attribution: checkout is shallow (--is-shallow-repository true; git rev-list HEAD^1..HEAD^2 = 1 commit vs 15 listed in the metadata snapshot), so only the aggregate HEAD^1..HEAD diff was verified.
  • Assistant-image persistence through --continue — declared gap in the PR, tracked upstream as feat(core): persist assistant inline images for session resume #8521; resume of tool images and already-persisted assistant parts is what this slice claims and what the resume suite covers.
  • Interactive TUI visual rendering (no tmux capture this round; component behavior covered by the unit/component suites).
  • Windows .cmd/.bat chafa shim path (Linux container).
  • Full-repo test suite (targeted suites + repo-wide typecheck only; the PR's own CI covers the rest).
  • .qwen/e2e-tests/terminal-inline-images.md could not be materialized in the working tree: the repo's .qwen/ overlay is root-owned mode 555 and I run as uid 1000, so the checkout reports it deleted and git restore is permission-denied. Its content was reviewed from the git object (git show HEAD:.qwen/...); no gate or harness reads it from disk, so this environmental gap does not affect any measurement above.

Methodology

Environment: CI verify container (node:22-bookworm), merge-ref checkout (HEAD merge d799d715, HEAD^1 base 2c514b50, HEAD^2 verified head b9ebfdd), npm ci+npm run build pre-provided. This is a follow-up round whose head/merge/base OIDs are identical to the previous round's, so the input closure is proven identical at the commit level; all measurements were nonetheless re-executed. A/B drove the compiled dist Turn of each tree via ab-turn.mjs (mock chat, real event loop); base core was compiled in a scratch worktree (removed afterwards) with the root node_modules and packages/core/node_modules linked in (see the build note above). Validation/chafa/consumer harnesses imported the real built CLI/core modules with no stubs of the code under test; the chafa peer was a recording fake on PATH. Mutations were applied to the two source files, executed through vitest, and restored (tree verified clean apart from the env-locked .qwen doc). Assertion encoding: expected-fail cells (flip probe reds, mutation reds) count as passes of the assertion that predicted them; fail: 0 means zero unexpected outcomes. Raw logs in logs/, harnesses at the artifact root, captures in evidence/ (01 A/B cells, 02 validation matrix, 03 chafa oracle, 04 mutation matrix).

Evidence images

01-ab-core-turn-parts

02-png-validation-matrix

03-chafa-stdin-oracle

04-mutation-matrix-red

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅ CI green on this head, sandboxed verification merge-ready (55/55), and the maintainer has verified it on real hardware and approved. Dismissing the two stale bot reviews from superseded heads alongside this.

@qwen-code-ci-bot
qwen-code-ci-bot dismissed stale reviews from themself August 6, 2026 12:32

Superseded: this gated on Integration Tests (CLI, No Sandbox), which current ci.yml only runs in the merge queue (event_name == merge_group) — it never runs at PR stage. Qwen Code CI is green on the current head b9ebfdd and the maintainer has approved it.

@wenshao
wenshao added this pull request to the merge queue Aug 6, 2026
Merged via the queue into QwenLM:main with commit 95655da Aug 6, 2026
307 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): support inline terminal image rendering for kitty/iTerm2/WezTerm/Ghostty/Warp

4 participants