Skip to content

docs(cli): correct inline image reviewer steps - #8746

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
zjunothing:fix/issue-8611-inline-image-docs
Aug 11, 2026
Merged

docs(cli): correct inline image reviewer steps#8746
wenshao merged 1 commit into
QwenLM:mainfrom
zjunothing:fix/issue-8611-inline-image-docs

Conversation

@zjunothing

Copy link
Copy Markdown
Collaborator

What this PR does

Fixes #8611 by correcting two reviewer steps in .qwen/e2e-tests/terminal-inline-images.md: session resume now uses /resume or --continue, and screen-reader verification uses --screen-reader or ui.accessibility.screenReader.

Why it's needed

Ctrl+O toggles expanded thinking/tool details; it does not resume a session. INK_SCREEN_READER is not a Qwen Code control and the CLI passes an explicit screen-reader option, so the old steps could not exercise the intended behaviors.

Reviewer Test Plan

How to verify

Read the two updated steps and follow them against the current CLI controls. /resume opens session selection, --continue resumes the latest session, and --screen-reader enables the screen-reader rendering branch.

Evidence (Before & After)

Before: the plan instructed reviewers to use Ctrl+O and INK_SCREEN_READER=true. After: it names the supported resume and accessibility controls. This is documentation-only; no screenshot applies.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Static source verification on macOS, Node.js 22.

Risk & Scope

  • Main risk or tradeoff: none; only reviewer instructions change.
  • Not validated / out of scope: running the full image E2E flow; this PR does not change product code.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #8611

中文说明

本 PR 做了什么

修复 #8611:更正 .qwen/e2e-tests/terminal-inline-images.md 中两处评审步骤:会话恢复使用 /resume--continue,屏幕阅读器验证使用 --screen-readerui.accessibility.screenReader

为什么需要它

Ctrl+O 用于切换思考/工具详情展开,不会恢复会话。INK_SCREEN_READER 不是 Qwen Code 支持的控制项,而 CLI 会传入显式屏幕阅读器选项,因此旧步骤无法测试目标行为。

评审测试计划

如何验证

阅读更新后的两处步骤,并按当前 CLI 控制项执行。/resume 打开会话选择,--continue 恢复最近会话,--screen-reader 开启屏幕阅读器渲染分支。

证据(修复前与修复后)

修复前计划要求使用 Ctrl+O 和 INK_SCREEN_READER=true;修复后改为支持的会话恢复与无障碍控制项。本 PR 仅修改文档,不适用截图。

测试平台

操作系统 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境

macOS、Node.js 22,静态源码核验。

风险与范围

  • 主要风险或取舍:无,仅修改评审说明。
  • 未验证/不在范围内:完整 inline image E2E 流程;本 PR 不修改产品代码。
  • 破坏性变更/迁移说明:无。

关联 Issue

Fixes #8611

@zjunothing

Copy link
Copy Markdown
Collaborator Author

Verification report

Environment

macOS, Node.js 22; static source verification against latest upstream/main (d91c66119).

Reproduction and result

The old instructions were verified against current source: Ctrl+O is the thinking/detail expansion toggle, while /resume and --continue are the supported resume paths; INK_SCREEN_READER is absent from the repository, while --screen-reader and ui.accessibility.screenReader are supported. The two E2E steps now name those actual controls.

Tests executed

  • npx prettier --check .qwen/e2e-tests/terminal-inline-images.md — PASS
  • git diff --check — PASS

Evidence

Documentation-only change with a two-line diff. No screenshot is applicable.

中文验证报告

验证报告

环境

macOS、Node.js 22;基于最新 upstream/maind91c66119)完成静态源码核验。

复现与结果

已对照当前源码核实旧说明:Ctrl+O 是思考/详情展开开关,/resume--continue 才是支持的会话恢复路径;仓库中不存在 INK_SCREEN_READER,支持的是 --screen-readerui.accessibility.screenReader。两处 E2E 步骤现已改为这些真实控制项。

已执行测试

  • npx prettier --check .qwen/e2e-tests/terminal-inline-images.md —— 通过
  • git diff --check —— 通过

证据

仅修改文档两行,不适用截图。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 8, 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 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

  • Template: complete ✓
  • Problem: observed, not theoretical. Linked issue docs(cli): correct terminal inline-image reviewer steps #8611 (opened by a different user) reports exactly these two stale reviewer steps, and I verified both against the source: Ctrl+O is the "expand details" transcript key (KeyboardShortcuts.tsx), not a session-resume control, and INK_SCREEN_READER is referenced nowhere in this codebase — the CLI passes an explicit screen-reader option to Ink instead, which overrides the env var. The old steps could not exercise the intended behaviors.
  • Direction: aligned — docs-only correction to the terminal inline-images E2E plan so reviewers follow controls that actually exist. No CHANGELOG signal expected for an internal test-plan doc; the area is clearly relevant.
  • Size: not applicable — no core paths touched; 2 changed lines in one docs file.
  • Approach: minimal, and it maps 1:1 onto the issue's two scope items. Nothing to cut, no unrelated changes.
  • Risk: no elevated risk signals — no high-risk paths matched.

Moving on to code review. 🔍

中文说明

感谢贡献!

  • 模板:完整 ✓
  • 问题:已观测到,不是理论问题。关联 issue docs(cli): correct terminal inline-image reviewer steps #8611(由另一位用户提出)明确指出了这两处过时的评审步骤,且两处均已对照源码核实:Ctrl+O 是"展开详情"转录快捷键(KeyboardShortcuts.tsx),不是会话恢复控制项;INK_SCREEN_READER 在本代码库中没有任何引用——CLI 会向 Ink 传入显式的屏幕阅读器选项并覆盖该环境变量。旧步骤无法测试目标行为。
  • 方向:对齐——仅修改文档,纠正 terminal inline-images E2E 计划,使评审者按照实际存在的控制项操作。内部测试计划文档不涉及 CHANGELOG 信号;该领域明显相关。
  • 规模:不适用——未触及核心路径;单个文档文件改动 2 行。
  • 方案:最小化改动,与 issue 的两个范围项一一对应。没有可砍掉的内容,也没有无关改动。
  • 风险:无升级风险信号——未命中任何高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code review

Before reading the diff, my own fix for #8611 would have been exactly two line edits: name a real session-resume control in the resume step, and replace the nonexistent INK_SCREEN_READER env var with the supported flag/setting. That is precisely what this PR does — no simpler version exists.

Every control the updated steps name checks out against the source:

  • /resume with no argument opens the session picker dialog, and continue is its registered altName (resumeCommand.ts).
  • --continue resumes the latest session — the CLI's own validation message says so: "Use --continue to resume the latest session, or --resume to resume a specific session" (config.ts).
  • --screen-reader is a real CLI option; it takes precedence over the ui.accessibility.screenReader setting, which also exists (config.ts, settingsSchema.ts).
  • The screen-reader branch is genuinely exercised by the corrected step: the resolved value is passed to Ink as an explicit isScreenReaderEnabled render option (startInteractiveUI.tsx) — which is also why the old INK_SCREEN_READER env var had no effect here. TerminalImage disables native inline-image output in that mode and falls back to placeholder text, exactly what the plan's step asserts ("only the placeholder is emitted").
  • The negative claims check out too: INK_SCREEN_READER appears nowhere in the repo, and Ctrl+O is bound to expanding details / transcript mode (KeyboardShortcuts.tsx), not session resume.

Also checked the one formatting risk — the replacement lines run ~100 chars, but Prettier's default proseWrap: preserve (the repo config sets no override) means no format job will rewrap them. No blockers found.

Test evidence

Evidence carried here: static source verification (above) plus the PR's own CI checks read via the API — no PR code was executed; the review is static. This is a docs-only change with nothing user-visible, so real-scenario tmux testing is N/A. Not verified: runtime execution of the plan steps themselves — out of scope for a docs fix, and no behavioural claim is being settled here.

CI has settled on the reviewed commit: the ubuntu suite — the only platform that ran tests, since macOS/Windows jobs were skipped per the docs-only classification — is green, alongside precheck, Desktop Shell, and the web-shell smoke test. The one cancelled entry is the triage bot orchestration job from the earlier run being superseded by this re-trigger, not PR CI.

Check Conclusion
precheck-pr / precheck success
Classify PR success
authorize success
label success
route success
Desktop Shell (ubuntu-22.04) success
Desktop Shell (windows-2022) success
Test (ubuntu-latest, Node 22.x) success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) success
Test (macos-latest, Node 22.x) skipped (docs-only)
Test (windows-latest, Node 22.x) skipped (docs-only)
Integration Tests (CLI, No Sandbox) skipped
verify skipped
tmux-testing skipped
triage cancelled (superseded bot run)
中文说明

代码审查

在读 diff 之前,我对 #8611 的独立修复方案也正是这两行编辑:在恢复步骤中使用真实存在的会话恢复控制项,并用受支持的 flag/配置替换不存在的 INK_SCREEN_READER 环境变量。本 PR 正是这样做的——不存在更简单的版本。

更新后步骤引用的每个控制项都经过源码核实:

  • /resume 不带参数时打开会话选择对话框,且 continue 是其注册的别名(resumeCommand.ts)。
  • --continue 恢复最近会话——CLI 自身的校验提示即为:"Use --continue to resume the latest session, or --resume to resume a specific session"(config.ts)。
  • --screen-reader 是真实存在的 CLI 选项;它优先于同样存在的 ui.accessibility.screenReader 配置(config.tssettingsSchema.ts)。
  • 修正后的步骤确实会走到屏幕阅读器分支:解析后的值会作为显式的 isScreenReaderEnabled 渲染选项传给 Ink(startInteractiveUI.tsx)——这也正是旧的 INK_SCREEN_READER 环境变量在此无效的原因。TerminalImage 在该模式下禁用原生内联图片输出并回退为占位文本,与计划断言一致("只输出占位符")。
  • 否定性论断同样成立:INK_SCREEN_READER 在代码库中没有任何引用;Ctrl+O 绑定的是展开详情/转录模式(KeyboardShortcuts.tsx),不是会话恢复。

也检查了唯一的格式风险——替换后的行约 100 字符,但 Prettier 默认 proseWrap: preserve(仓库配置未覆盖),格式化任务不会重排它们。未发现阻塞问题。

测试证据

本节证据:上述静态源码核验,加上通过 API 读取的 PR 自身 CI 结果——未执行任何 PR 代码,审查为静态审查。本 PR 仅改文档、无用户可见行为变化,真实场景 tmux 测试为 N/A。未验证项:计划步骤本身的运行时执行——超出文档修复的范围,此处也不涉及需要证实的行为性论断。

CI 已在被审查提交上跑完:ubuntu 套件(唯一实际执行测试的平台——macOS/Windows 任务因 docs-only 分类被跳过)全绿,precheck、Desktop Shell 与 web-shell 冒烟测试同样通过。唯一的 cancelled 条目是 triage 机器人编排任务(上一轮运行被本次重新触发取代),并非 PR CI。

Qwen Code · qwen3.8-max

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

@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.

⚠️ Downgraded from Approve to Comment: CI failing: triage. Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI failing: triage。 已审查。

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

@wenshao

wenshao commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 11, 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: 42 passed · 0 failed · 42 total

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

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

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

Verification report

PR 8746 — docs(cli): correct inline image reviewer steps

Verdict: merge-ready — 42/42 scripted assertions passed (0 unexpected failures), verified head c16bc841cc5dde4c02afeab7364e72357845d4f6 (= HEAD^2, matches headRefOid; single PR commit, matches the metadata commits array, so per-commit attribution holds).

中文摘要
  • 结论:merge-ready。42/42 条脚本化断言全部通过,0 个意外失败。
  • A/B 结论(见下文 "Central claim" 表):PR 修改的两条评审步骤均被真实 CLI 行为证实——--continue 端到端恢复最近会话(同一 session_id、历史在 wire 上重放、模型据此作答,对照会话无泄漏);/resume 打开 "Resume Session" 会话选择器;--screen-readerui.accessibility.screenReader: true 均进入屏幕阅读器渲染分支。旧步骤均被证伪:INK_SCREEN_READER=true 不触发该分支(Qwen Code 总是向 Ink 传显式 isScreenReaderEnabled 选项,?? 使环境变量失效);Ctrl+O 是明细展开开关("(ctrl+o to expand)" → "(ctrl+o to collapse)"),不打开任何恢复会话的 UI。
  • Findings:仅一条 nit——两条新行(96/108 字符)是该文件仅有的超 80 列行,文件自身换行约定为 ≤80;prettier(proseWrap preserve)接受,不阻塞。
  • 未覆盖范围:文档其余步骤(完整 inline-image E2E 流程,PR 自述 out of scope);macOS/Windows 终端;--resume <id>/--fork-session 路径;仓库级测试套件(docs-only,无产品代码变更)。

Scope

Docs-only PR: two reviewer steps in .qwen/e2e-tests/terminal-inline-images.md. Since no product code changes, a build-vs-build A/B has no discriminating power by construction; the load-bearing proof is a truth table of the four control mentions (two old, two new) against the live CLI, with a wire-oracle mock OpenAI peer for the resume claim.

Central claim: the corrected steps name real controls that perform the described behavior; the old steps did not.

Cell Instruction under test Oracle Result
A run1→run2 new: --continue resumes most recent session session_id equality + mock peer saw run1's user+assistant history on the wire + end-to-end answer derived from that history PASS (13/13 in 01-resume-ab-wire-oracle.png)
A run3 control: no flag fresh session_id, no leaked history on wire, answer NO-HISTORY PASS
B1 old: INK_SCREEN_READER=true real-pty frame: default layout (banner >_ Qwen Code, Tips:, header info panel) rendered → SR branch NOT engaged old step wrong (12/12 in 02-screen-reader-cells.png)
B2 new: --screen-reader SR layout: banner/tips/header suppressed, composer reached new step works
B3 new: ui.accessibility.screenReader: true same as B2 new step works
C old old: Ctrl+O resumes session real-pty: collapsed thinking shows "(ctrl+o to expand)"; after Ctrl+O "(ctrl+o to collapse)" + thinking text visible (toggle fired); no "Resume Session" UI; conversation intact old step wrong (10/10 in 03-ctrlo-not-resume.png)
C new new: /resume opens session selection "Resume Session" picker dialog rendered new step works

The app's own startup Tips corroborate: "You can resume a previous conversation by running qwen --continue or qwen --resume."

Findings

  1. Nit — line-length convention. The two changed lines are the only lines in the file exceeding 80 columns (line 35: 96 chars, line 69: 108 chars; all other lines ≤80). Prettier with proseWrap: preserve accepts them (byte-identical format output on both arms, 04-prettier-gate-live-control.png), so this is not a gate failure — only an inconsistency with the file's own wrap style. Non-blocking.

  2. Informational — why the old env var is inert (mechanism). INK_SCREEN_READER is a genuine Ink feature (node_modules/ink/build/ink.js: options.isScreenReaderEnabled ?? process.env['INK_SCREEN_READER'] === 'true'), but Qwen Code always passes an explicit option (startInteractiveUI.tsx: isScreenReaderEnabled: config.getScreenReader()), so the ?? fallback never applies. The PR's rationale ("not a Qwen Code control") is behaviorally accurate; this note is for precision, not a code change request.

  3. Informational — checkout anomaly (methodology). The working tree held the BASE version of the changed file (index/HEAD = PR text). All harnesses extracted file versions via git show, never from the worktree. This did not affect any cell.

Not covered

  • The doc's other steps (full terminal-inline-image E2E flow) — the PR explicitly scopes itself to the two corrected lines; unchanged text was not re-verified against the product.
  • --resume <id> and --fork-session headless paths (doc does not change them).
  • macOS/Windows terminals; this round ran on the lane's Linux container (80×24 pty via util-linux script).
  • Repo-wide test/typecheck suites: not run — the diff touches no product code; targeted gates were prettier (both arms + live control), the stale-control census, and the four behavioral harnesses.
  • Whether the repo-wide CI prettier run traverses .qwen/e2e-tests/: prettier 3.6.1 here honors gitignore scope (a planted violation under gitignored tmp/ was silently skipped by --check from the repo root — proven by paired control), and .qwen/* is gitignored though this file is tracked. I therefore verified formatting by byte-comparing prettier's output against both versions instead of relying on --check's reporting.
  • The alternate-screen marker (?1049h) could not discriminate cells here because the container's CI env vars disable the virtual viewport on both arms (symmetric); the synchronized-output sequence (?2026h) is gated on kitty/iTerm/WezTerm detection and never fires under xterm-256color — both dropped from the assertion set, replaced by the banner/tips/header-panel markers.

Methodology

Environment: CI verify container (node:22-bookworm), merge-ref checkout (HEAD eafac81 merge, HEAD^1 base tip, HEAD^2 PR head); npm ci/npm run build pre-run, npm run bundle produced dist/cli.js for all harness runs. Harness A drove three headless runs against a zero-dependency mock OpenAI peer (specialized from .qwen/skills/e2e-testing/scripts/mock-openai-server.js) that recorded every request body to a wire JSONL and answered the codeword question from request history, so the end-to-end result text is itself a history oracle. Harnesses B/C ran the real TUI under a real pty (script -qfc), parsed frames with @xterm/headless, and staged key input (\x0f for Ctrl+O, /resume + Enter); the container's QWEN_HOME env override was pinned per cell to a scratch dir. Harness C's mock streams reasoning_content so the collapsed/expanded thinking hints ("(ctrl+o to expand)" / "(ctrl+o to collapse)") serve as a deterministic toggle oracle. Assertion counts come from the four result JSONs in logs/ (13+12+10+7 = 42). Raw frames, wire logs, and per-run stdout/stderr live in logs/ and scratch/; evidence PNGs were produced by scripts/verify-capture.mjs re-running each harness.

Evidence images

01-resume-ab-wire-oracle

02-screen-reader-cells

03-ctrlo-not-resume

04-prettier-gate-live-control

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

Qwen Code · sandboxed verification

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — docs-only, two lines, every named control verified in source, CI green; would merge without hesitation.

Stepping back: this is exactly the kind of PR the gate should wave through quickly. The problem is real and independently reported (#8611), the fix maps 1:1 onto the issue's two scope items, and there is no simpler version of it — my independent proposal before reading the diff was the same two line edits. Both negative claims hold under grep: INK_SCREEN_READER exists nowhere in the repo, and Ctrl+O expands details rather than resuming sessions. Both positive claims hold in code: /resume opens the session picker (--continue resumes the latest session), and --screen-reader / ui.accessibility.screenReader genuinely reach Ink's screen-reader branch, which is what makes TerminalImage emit only the placeholder. CI settled green on the reviewed commit. Nothing unrelated in the diff, nothing to maintain later — in six months this reads as a two-line correction, because that is all it is.

Approving, pinned to the reviewed commit.

中文说明

置信度:5/5 —— 纯文档、两行改动,步骤中引用的每个控制项都已在源码中核实,CI 全绿;可以毫不犹豫地合并。

退一步看:这正是门禁应当快速放行的 PR。问题真实存在且由他人独立报告(#8611),修复与 issue 的两个范围项一一对应,也不存在更简单的版本——我在读 diff 之前的独立方案就是同样的两行编辑。两个否定性论断经得起 grep 检验:INK_SCREEN_READER 在代码库中不存在;Ctrl+O 是展开详情而非恢复会话。两个肯定性论断在代码中成立:/resume 打开会话选择器(--continue 恢复最近会话),且 --screen-reader / ui.accessibility.screenReader 确实会进入 Ink 的屏幕阅读器分支,使 TerminalImage 只输出占位符。CI 已在被审查提交上全绿。diff 中没有无关内容,也没有后续维护负担——六个月后它读起来就是两行修正,因为它确实只是两行修正。

批准,并固定到被审查的提交。

Qwen Code · qwen3.8-max

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

@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. ✅

@wenshao
wenshao added this pull request to the merge queue Aug 11, 2026
Merged via the queue into QwenLM:main with commit e8182a3 Aug 11, 2026
74 of 75 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.10.

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.

docs(cli): correct terminal inline-image reviewer steps

3 participants