fix(cli): OpenTUI transcript visibility for commands, /clear and steers - #10947
Conversation
The U-26 section still read "Registered as U-26" after that number was retired and folded into U-12 the same day, so it pointed at a record that does not exist. The U-27 section said ink's warning names the formats the model cannot read; the shared warning text names the formats the pipeline supports. The second error mattered more than the wording: porting U-27 faithfully would have read as a behaviour change against a claim that was never true.
The renderer kept two histories and painted one. Everything a slash command reported was recorded in a store no view reads, so a command ran, changed real state, and left the screen untouched. A recorded item now becomes a transcript row at the moment it is recorded, through one mapping that decides every kind the command layer can write and is checked for exhaustiveness at compile time; the kinds the live model turn already renders are explicit no-ops with the reason beside each, so nothing paints twice. Clearing the conversation emptied the recorded history and left the visible transcript standing, which the session-switch contract already described as clearing both. Clearing also exposed a crash hiding behind the invisible transcript: the command context handed out one host method detached from its receiver, so the command threw before it cleared anything and the error was the only thing on screen. A message typed while the model is still working now appears as its own row once the turn accepts it, with its own read cards immediately before it, in the order the existing renderer produces. Previously the queue counter dropped and the text disappeared without a trace. Submitting an image the pipeline cannot read now discloses the supported formats on both the fresh-submit hop and the mid-turn one; such an image previously reached the model with no notice at all. Four kinds the existing renderer draws with dedicated components are written under this renderer too and stay invisible, exactly as before: each needs a row shape this transcript does not have, and the replay path never handled them either, so leaving them out keeps live and replay consistent. The design doc names every producer line and registers them, with four further findings, as follow-ups. Closes U-12 (visible half), U-27, U-28, and the two gaps this change's own smoke turned up. Part of #8662.
|
|
|
Thanks for the PR — this is a clean, well-documented batch. Template ✓ — every required heading is present, including the bilingual section. Problem — observed, not theoretical. #10905 is a filed P1 ( Direction — aligned. This is part of the OpenTUI renderer migration (#8662), whose whole promise is that switching renderers doesn't change what the user sees; a command that runs silently is the most visible break of that promise. It stays inside the renderer seam and touches no auth/sandbox/model/telemetry/release/public contract, so no direction escalation. Size — not a core change. All seven production files live under Approach — the scope feels right, and it matches what I'd have proposed independently: one total Risk — no elevated risk signals; none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献——这是一批干净、文档完善的改动。 模板 ✓ —— 所有必需小标题齐全,含中英双语部分。 问题 —— 已观测,非理论性。#10905 是一个带具体复现的 P1( 方向 —— 对齐。这属于 OpenTUI 渲染器迁移(#8662),其核心承诺就是「切换渲染器不改变用户所见」;命令静默运行是对该承诺最显眼的破坏。改动留在渲染器接缝内,不触碰 auth/sandbox/模型/遥测/发布/公共契约,因此无需方向升级。 规模 —— 非核心改动。七个生产文件都在 方案 —— 范围合理,与我独立设想的做法一致:一个带 风险 —— 无升级风险信号;改动文件均未命中与回滚相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewNo blocking issues. I read the production diff against the base and verified the load-bearing claims rather than taking the design doc's word for them:
Non-blocking notes, all already documented by the author and worth a maintainer's glance:
The change is minimal for its goal: ~254 production lines, the rest tests and the decision record. No scope creep. How a recorded item reaches the screensequenceDiagram
participant P1 as User
participant P2 as Command dispatcher
participant P3 as OpenTuiAppHost.addItem
participant P4 as projectItemToStreamEvent
participant P5 as transcript.append
participant P6 as live turn applyEvent
P1->>P2: run slash command
P2->>P3: addItem (recorded item)
P3->>P4: project item to stream event
P4-->>P3: event, or null for no-op kinds
P3->>P5: append event when non-null
P5->>P6: onTranscriptEvent folds into the live transcript
P6-->>P1: the row reaches the screen
Files changed (production + the new spec; tests/docs grouped)
Test evidence — the PR's own CIThis is an unattended CI run, so I did not build or execute any PR code; the evidence below is the PR's own check-runs on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the rest: 中文说明代码审查无阻断问题。我对照基线阅读了生产代码 diff,并亲自核实了关键论断,而非仅采信设计文档:
非阻断备注(作者均已记录,值得维护者一瞥):
改动相对目标是最小的:约 254 行生产代码,其余为测试与决策记录。无范围蔓延。 测试证据——PR 自身 CI这是无人值守的 CI 运行,因此我未构建或执行任何 PR 代码;下方证据是 PR 自身在 (CI 表格见上方英文区的机器可读区域,此处不重复。) 沙箱验证可落实其余部分: — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — solid, ship-worthy parity fix; the only reservations are divergences the author already named and filed, not defects I found. Stepping back: this does the unglamorous-but-correct thing. The renderer migration's whole promise is "switching renderers doesn't change what you see", and a command that runs, mutates real state, and says nothing is the most visible break of it — a real P1 with a reproduction, not a hypothesis. The fix matches what I'd have proposed independently, and arguably picks the better of the two seams the design doc weighed: project-on-write at The things that would have made this dangerous all check out under scrutiny rather than on faith: the new memo dependency doesn't churn the host (verified What keeps it at 4 and not 5 is honest incompleteness the author flagged rather than hid: CI is still running on the legs that would compile the exhaustiveness check and drive the new spec, so I'm not approving into a void — approval is deferred until CI lands green on 中文说明信心:4/5 —— 扎实、值得发布的 parity 修复;我仅有的保留是作者已点名并登记的差异,而非我发现的缺陷。 退一步看:这件不显眼但正确的事做到了位。渲染器迁移的全部承诺就是「切换渲染器不改变你所见」,而一个运行了、改了真实状态、却什么都不说的命令,是对它最显眼的破坏——这是带复现的真实 P1,不是假设。修复与我独立设想的做法一致,甚至在设计文档权衡的两条接缝里选了更好的那条:在 会让这类改动变危险的几点,都经得起推敲而非仅凭信任:新的 memo 依赖不会让 host 抖动(已核实 让它停在 4 而非 5 的,是作者坦白而非掩盖的未完成之处:本批之后 CI 仍在跑那几个会编译穷尽性检查、驱动新测试的腿,所以我不会批准进一个真空——批准延迟到 CI 在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
E2E test reportBundle: local Interactive specs, both renderer legs
Both legs ran the two spec files together: ink 5/5 tests in 19.65 s, OpenTUI 5/5 in 17.40 s. The OpenTUI leg carries The new spec's signal is 1:1 with the fix (measured)Neutering the transcript append the projection writes through (
The failure is not a generic timeout. It is thrown from the spec's own wait helper, with its own message — — and the attached screen dump is the OpenTUI composer ( Dropping the receiver bind behind Restoring the grouped echo order (all read cards, then all steered rows) turns exactly one unit test red — Moving the image-format disclosure from before the send loop to just after A boot race the spec had to absorb — and its recovery path, measuredThe first verification run failed the ink leg 3/3 attempts with The spec therefore re-sends Two things about that retry were measured rather than assumed:
Unit suite, typecheck, lint
That file is untouched here — It is reported rather than fixed here on purpose. The fix is one line in ink's composer, and this PR's non-causation argument below rests on its production changes being confined to The targeted run — all 67 files under The full suite (the workspace-local Two are inherited, with CI-side proof on main itself at the exact base commit. Same file, same two test names, same two line numbers as this machine's run — and as this PR's own CI run, quoted in the next section — on ubuntu/Node 22, on main, before this branch existed. #10929 touched exactly two files, Re-running all ten red files together in isolation (151.06 s) splits them into four that still fail here and six that pass:
The six that pass in isolation carry 8 of the 9 recovered failures — The locale hypothesis was re-tested at this base and rejected again: pinning Non-causation is structural, not inferred. None of the ten files contains the string CI on this PRRun 33779976843 at head Green:
Same file count and same two line numbers as main's run at
The same attempt re-ran the other two gates and changed nothing. What that leaves for the deferred approval. The triage stage-3 review withheld its commit-pinned approval pending CI green on Update after those attempts (2026-09-04)main fixed the two inherited gates, and the branch now carries the fix. The new spec has run in CI, under both renderers. Dispatch 33830499451 ran The OpenTUI leg's intermittency is main's, and it is not resolved by #10986. The same leg was green end to end at our base (job 100731177955, U-35 closed upstream, not here. The Enter-resolution race this batch registered as a suspicion — "shape matches the ink bug #10926 fixed, not reproduced" — is exactly what Diff shape at the synced head, superseding the file counts above. The review rounds grew this PR from 15 files to 19 (measured against A third gate went red on the synced head — preceded by npm's own Coverage boundary — what this green does not show
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1b": none — no Budget gap: lines, as nothing was cut short..
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 1b":none — no Budget gap: lines, as nothing was cut short.。
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
@qwen-code /tmux |
|
tmux real-user testing: cancelled - workflow run The testing job was cancelled before producing a verdict. See the workflow run for details. — Qwen Code · tmux real-user testing |
Project-on-write made the recorded invocation visible, which exposed a second user row behind every submit_prompt outcome: the live turn echoed the expanded content the user never typed, where ink adds only the invocation row. Suppress that echo at the submit seam. Steering the same text twice also rendered two rows where ink renders one, because stream echoes never pass through either addItem implementation — collapse them at the fold, the one chokepoint both echoes share. Adds the instruments review round 1 asked for: the fileData image disclosure and its per-message attribution in the steering drain, ink's blocked-prompt text and redaction at the projection site, the host state forwarded into the projector context, and the host and seam-callback identities the transcript memo preserves.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/opentui/live-session.ts:331 — [review] D2-1 hasUnsupportedImageFormat is a line-for-line re-implementation of ink's private checkImageFormatsSupport; the two could share one helperpackages/cli/src/ui/opentui/live-session.ts:339 — [probe] D2-2 the startsWith('image/') scoping guard of hasUnsupportedImageFormat has no negative testintegration-tests/interactive/command-output-visibility.test.ts:52 — [probe] D2-3 the retry predicate scans the whole xterm buffer including scrollback, so a stale ABOUT_UNKNOWN keeps it permanently true
中文说明
已审查。 建议见行内评论。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.23.0)
The design doc recorded the new interactive spec as collected only by the opentui e2e job. The ink lanes pass the whole integration root with two excludes, so they collect it under ink as well — a wrong map that would send a maintainer bisecting a red ink shard to the wrong job.
Local end-to-end verificationI built a real verification rig for this branch rather than reading the diff, and drove the bundled CLI through a PTY under both renderer legs. Short version: the four headline behaviours all reproduce, the fix is real and my rig discriminates it — and while probing around the edges I found two parity divergences this PR itself introduces, one of which is user-visible on an ordinary path. I'd fix the first before merge; everything else is non-blocking. Setup
Both worktrees were built and bundled from source ( Results
B1/B1b were run with the model never called ( Negative controlsGreen on its own proves little, so I broke the fix three ways and re-ran:
Gates
FindingsF1 — a prompt-expanding command run twice in a row loses its second invocation row (I'd fix before merge)Repro.
The second reply lands under no prompt at all, which reads worse than the pre-PR expansion row. Mechanism. F2 — a resumed session collapses two adjacent identical user turns into one rowThe new collapse lives in Repro. Two identical prompts whose turns record nothing between them (the "previous session appears to have stopped after user input" shape), then
Pinned to the mechanism at unit level: One change fixes both, and I verified itGate the collapse behind an option only the append path sets, and let --- a/packages/cli/src/ui/opentui/live-session-model.ts
+++ b/packages/cli/src/ui/opentui/live-session-model.ts
export function foldLiveEvent(
prev: readonly LiveHistoryItem[],
ev: OpenTuiStreamEvent,
+ options?: { collapseRepeatedUser?: boolean },
): readonly LiveHistoryItem[] {
...
- if (last?.kind === 'user' && last.text === ev.text) return prev;
+ if (
+ options?.collapseRepeatedUser &&
+ last?.kind === 'user' &&
+ last.text === ev.text
+ )
+ return prev;
--- a/packages/cli/src/ui/opentui/live-turn.ts
+++ b/packages/cli/src/ui/opentui/live-turn.ts
- setItems((prev) => foldLiveEvent(prev, ev));
+ setItems((prev) =>
+ foldLiveEvent(prev, ev, { collapseRepeatedUser: true }),
+ );
--- a/packages/cli/src/ui/opentui/opentui-host.ts
+++ b/packages/cli/src/ui/opentui/opentui-host.ts
// (move the projection out of the dedup guard: project every item and
// let the visible fold collapse a genuine repeat)Measured on that patched tree: F1 → 2 rows (matches ink), F2 → 2 replayed rows (matches ink and the base), and nothing else moves — B3 still collapses the double steer to one row while both copies still ride to the model, B6 still renders exactly one row, B1/B2/B2b/B5 unchanged. Cost is two unit tests that encode the current behaviour ( F3 — Reviewer Test Plan behaviour 4 is not reproducible (non-blocking, description only)I could not make the format disclosure appear on any arm, including ink, and I think it cannot appear:
So U-27 is faithful ink parity of code that is currently dead on both entry points — harmless, but the test plan asks a reviewer to observe something they cannot. Relatedly, "the image is still forwarded, as in the existing renderer" is not accurate for non-pipeline formats: core omits the data and substitutes an in-band notice. F4 —
|
| PR head | 3c54fc8646 |
| Base | d4e3e4fc87(与 main 的 merge-base) |
| 对照臂 | main + OpenTUI、PR + OpenTUI、PR + ink(parity 基准),外加两条变异臂 |
| 驱动 | 仓库自带的 InteractiveSession(node-pty → @xterm/headless)+ startFakeOpenAIServer,100×40,QWEN_CODE_LANG=en |
| 渲染器腿 | QWEN_E2E_RENDERER=opentui(bun + QWEN_TUI_RENDERER_STRICT=1)与 =ink(node) |
| 模型 | 脚本化的假 OpenAI 服务器——不接真 provider,因此屏幕上每一行的来源都可追溯 |
| 主机 | Linux x64、node v24.3.0、bun 1.3.14、retry=0、单 fork |
两个 worktree 都从源码 npm run build && npm run bundle,下面所有画面均来自 dist/cli.js。
结果
main = base OpenTUI,PR = PR OpenTUI,ink = PR 在 ink 渲染器下。
| # | 行为 | main | PR | ink |
|---|---|---|---|---|
| B1 | /about 状态块上屏 |
✗ | ✓ | ✓ |
| B1b | /approval-mode plan 回显调用并汇报新模式 |
✗ / ✗ | ✓ / ✓ | ✓ / ✓ |
| B2b | 屏幕上已有一轮内容时执行 /clear,转录真的清空 |
✗(内容还在) | ✓ | ✓ |
| B3 | 中途投喂成为独立的一行 | 0 行 | 1 行 | 1 行 |
| B3 | 同一条中途消息投两次 → 一行,但两份都发给模型 | — | 1 行 / 已发 ✓ | 1 行 / 已发 ✓ |
| B5 | 会展开成提示词的命令显示调用而非展开结果 | 显示展开 | 显示 /probe |
显示 /probe |
| B6 | 普通提示词恰好渲染一行用户行(无重复绘制) | 1 | 1 | 1 |
| B7 | 8 条命令扫描(/about /tools /mcp /extensions /context /memory show /approval-mode /model)全部可见、无错误行 |
4/8(仅弹窗类) | 8/8 | 8/8 |
| B9 | clearItems 学会清转录之后,/resume 仍能正常重放 |
✓ | ✓ | ✓ |
B1/B1b 全程模型零调用(requests.length === 0),因此这些行只可能来自命令本身。
(截图见英文部分。)
反向对照
只看到绿色说明不了什么,所以我用三种方式把修复破坏掉再跑:
- 只回退
.bind(host)这一处 →/clear抛出undefined is not an object (evaluating 'this.deps.startNewSession'),并且什么都没清。这正是 OpenTUI: slash command output never reaches the screen (invocation echo + result messages) #10905 里的崩溃,从 PR 树上仅移除一个 hunk 就复现了。 - 把
projectItemToStreamEvent掐成return null→/about重新变得不可见。说明 B1 测的是投影,而不是启动输出。 - 你自己的那个测试(
command-output-visibility.test.ts)跑在main上:OpenTUI 腿失败、ink 腿通过;跑在 PR head 上两条腿都通过。这正是描述里声称的判别力,独立复核成立。
门禁
- 对全部 17 个改动的
.ts/.tsx文件跑eslint --max-warnings 0—— 干净。 packages/cli的tsc --noEmit—— 干净。- 8 个改动的测试文件:249 通过。整个
src/ui/opentui/目录加上useHistoryManager/historyUtils:1170 通过。 - 穷尽性检查是真的:往
HistoryItemWithoutId联合类型里加一个成员,会得到src/ui/opentui/item-projection.ts(1196,13): error TS2322: Type 'HistoryItemMutantProbe' is not assignable to type 'never'。新增类型确实会让构建失败,与文档一致。
发现
F1 —— 连续两次执行同一个「展开成提示词」的命令,第二次的调用行丢失(建议合并前修)
复现。 建 .qwen/commands/probe.md(提示词内容随意);执行 /probe,等回复,再执行一次 /probe。
第二次 /probe 之后 |
|
|---|---|
| ink | > /probe · PROBE_REPLY_1 · > /probe · PROBE_REPLY_2 |
| PR,OpenTUI | > /probe · PROBE_REPLY_1 · PROBE_REPLY_2 —— 没有第二条用户行 |
| main,OpenTUI | 两行(内容是展开结果——正是本 PR 要修的问题) |
第二条回复底下没有任何提示词,观感比修之前的「展开行」还差。
机理。 OpenTuiAppHost.addItem 是对着 this.history 去重的,而与 ink 的单一存储不同,这个数组永远收不到模型的回复——里面只有命令写入的项。于是第一次 /probe 之后,this.history 的末尾仍然是那条 user "/probe";第二次调用被判定为连续重复,既没被记录、也就从未被投影。此时 invocationEchoed: true 又(正确地)压掉了 live turn 自己的那一行,最后什么都不剩。这段去重逐行忠实于 useHistoryManager——不等价的是存储本身。
F2 —— 恢复会话时,两条相邻且相同的用户回合被折叠成一行
新的折叠写在 foldLiveEvent 里,而 foldBatch——resetTranscript 背后的重放 fold——也会走它。于是它作用到了录制历史上,而不只是实时的中途投喂。
复现。 两条相同的提示词,且两轮之间没有任何东西被录进去(也就是「上一次会话在用户输入后停住」的形态),然后用一个全新的 CLI 执行 /resume:
| 重放出的行数 | |
|---|---|
| ink | 2 |
| main,OpenTUI | 2 |
| PR,OpenTUI | 1 |
在单测层面钉死了机理:foldBatch([user 'x', user 'x']) 在本分支得到 1 项,在 base 上得到 2 项。
一处改动同时修掉两个,并且我实测过
把折叠放到只有 append 路径才开启的开关后面,并让 addItem 无条件投影——由可见的 fold(而不是只装命令的 history)来判定某一行是不是重复。这也正是 ink 的实际语义,因为 ink 是对着它自己渲染的那个存储去重的。
(补丁见英文部分。)
在打了该补丁的树上实测:F1 → 2 行(与 ink 一致),F2 → 重放 2 行(与 ink 和 base 一致),而其他一切不动——B3 仍然把两次相同投喂折叠成一行、两份仍然都发给模型,B6 仍然恰好一行,B1/B2/B2b/B5 均不变。代价是两个编码了当前行为的单测(live-session-model.test.ts 的 "collapses the same user text folded twice in a row" 与 opentui-host.test.ts 的 "does not append for a consecutive-duplicate user message"),该目录下其余全绿(1120 中的 1118)。具体怎么改由你定——我只声明这一种方案是我实测过的。
F3 —— 评审测试计划的行为 4 无法复现(不阻塞,属描述问题)
我在任何一条臂上都没能让格式披露出现,包括 ink;而且我认为它根本出不来:
@path路径——core 的读取路径门在图片能变成 image part 之前,就把PIPELINE_IMAGE_MIME_TYPES之外的一切剔除了(PROVIDER_SAFE_IMAGE_MIME_TYPES,fileUtils.ts:1635)。在支持视觉的模型上实测(--model kimi-k3):ok.png→ 以image_url正常转发、无披露;odd.tiff→Image format image/tiff … cannot be safely sent to the model、无披露;bad.avif→ 同上、无披露。既然只有 PIPELINE 集合的 mime 能活下来,而它们全部落在SUPPORTED_IMAGE_MIME_TYPES之内,hasUnsupportedImageFormat在这里永远为假。- 输入框附件路径——
IMAGE_MIME_BY_EXTENSION(live-turn.ts:49)只映射了本来就在SUPPORTED_IMAGE_MIME_TYPES里的扩展名;其余会走它自己的Unsupported image type:提示。
所以 U-27 是对一段目前在两个入口上都已死亡的代码做的忠实 ink parity——无害,但测试计划要求评审者去观察一个观察不到的现象。相关地,「图片本身仍会照常转发,与既有渲染器一致」对非 pipeline 格式并不准确:core 会丢掉数据并替换成一条 in-band 提示。
F4 —— OpenTUI 下 /branch、/fork、/cd 会自我阻塞(既有问题;本 PR 只是让它显形)
OpenTuiAppHost.isIdle() 是 !this.processing && !this.streaming,而 OpenTuiSlashDispatcher 在它正在执行的那条命令的整个生命周期里都把 processing 置为 true(commands-dispatch.ts:459 → :1001)。于是在每一个读取 isIdleRef 的命令内部,context.ui.isIdleRef.current 都是 false。ink 那边是 isIdleRef.current = streamingState === StreamingState.Idle(AppContainer.tsx:2407)——只反映模型回合状态。
实测:一轮完成后在空闲提示符下执行 /branch verifybranch → ✖︎ Cannot branch while a response or tool call is in progress.,本分支和 base 都一样,而同一条命令在 ink 下会正常打开 branch 弹窗。branchCommand.ts:29、forkCommand.ts:74、cdCommand.ts:71、advisor-command.ts:200 都读这个 ref。值得单开一个 issue——不是本 PR 的活,但正因为本 PR,这条错误行才不再被吞掉。
F5 —— sentToModel 的翻转到不了转录行(前瞻性)
updateItem 刻意不投影,因此 dispatcher 的 updateItem(invocationItemId, { sentToModel: true })(commands-dispatch.ts:609 / :825)会让投影出的那一行永远停在 sentToModel: false。今天这没有影响——我查过,session-rewind-model.ts 是唯一的消费者,而 session-rewind.tsx 还没有接进任何 shell——但等 rewind 接上以后,submit_prompt 命令的调用会在 OpenTUI 的回退列表里被过滤掉,而 ink 会保留它。大概值得在 U-34 后续项里再加一行。
另外观察到的,不算缺陷
- 消息还待在投喂队列里时,ink 会把它们渲染在 spinner 下方(
⏳ 2 queued加上原文);OpenTUI 目前仍只显示计数。本 PR 修的是「被接受之后」的那一行,与它声称的一致;队列预览仍是另一个独立缺口。 - 两条相同的中途投喂折叠成一行,但两份都会到达模型(请求体里是
[{text: STEER}, {text: "\n\n"}, {text: STEER}])。这与 ink 的accept()一致,属预期——只是提一句,免得「一行」被读成「一条消息」。
未覆盖
仅 Linux。/quit 摘要渲染、设计文档登记为 U-34 的 advisor/arena/recap 几类、以及 Windows/macOS 均未验证。
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Reviewed at head 7dab0662.
- Threads: all 10 resolved, including ci-bot's round-2 inline suggestions and the author fix-notes; the last formal review (round 2, at
3c54fc86) posted no blockers. Everything since3c54fc86is two merges of main plus docs — I checked the PR's semantic markers still stand at this head after the merges folded main's own OpenTUI changes (#10986's input-prompt rework) into the same directory: the no-echo collapse at the shared fold chokepoint,invocationEchoedon the submit path,transcript.clear/appendwiring, the U-27 checks on both fresh-query and steering hops, and the per-message cards-then-row ordering all read as claimed against the checked-out head. - My own pass found no new Criticals. The riskiest semantic is deliberately ink's: two adjacent identical user items collapse to one row — it is placed at the single chokepoint both echoes share, matches the addItem parity the body documents, and costs only a transcript row (the second submit still runs); the same for the
sentToModel: falsesteer echo (rewind-list exclusion, as the existing renderer records). The projection map is compile-time exhaustive with the four invisible kinds named in the design doc rather than silently swallowed, and the detached-thisfix forstartNewSessionis the minimal bind at the context's construction site. - Evidence quality: real terminal captures of
/aboutand/clearat the base commit, the mutation-measured discriminating window for the new interactive spec, and an honestly disclosed limitation (that spec runs post-merge/schedule, not onpull_request) — with the retry-window semantics against the ink boot race reasoned through rather than papered over. - CI facts on this head: 13 checks pass, zero fail; Test/Lint/CVE/review-pr still in flight (the focused suites for these files run there); the earlier withheld deferred approval was against the pre-merge head and re-ran. Per the channel convention the call is on the review itself.
qqqys
left a comment
There was a problem hiding this comment.
Reviewed at head 7dab0662. No historical blocking issues exist on this PR (both automated rounds were Suggestion-only COMMENTED reviews, and every item was addressed with verification), and my independent Critical-only pass over the production hunks finds no blocking defects.
Critical-only pass — verified at this head:
- Prompt redaction preserved on both paths: the
user_prompt_submit_blockedrendering now goes through one shared formatter (formatUserPromptSubmitBlockedin event-adapter.ts,sanitizeSensitiveTextapplied) used by both the live stream mapper and the new item projector — the second call site can no longer drift from the redaction. projectItemToStreamEventis an exhaustive switch over every history kind with explicit, documented decisions for the no-op kinds; blocked prompts become the redacted warning row.- U-27 image-format disclosure is a faithful ink parity port (
hasUnsupportedImageFormat, image/-scoped, core's acceptance set) applied once on the fresh hop and per surviving message on the steering hop, each emitting the INFO row without dropping the part. - U-12 steer echoes ride the tool boundary as
sentToModel: falserows in inkaccept()order, and the restored-hop case yields none (aborted hops never reach accept) — pinned by tests. - Consecutive-duplicate suppression now exists at both chokepoints, matching ink's
addItem: the fold'suserarm returnsprevfor an identical consecutive text (verified in live-session-model.ts), and the host'saddItemskips history write and projection alike (verified at head lines 199-224). submit_promptoutcomes setinvocationEchoed, sosubmit()skips the duplicate user row for generated (never-typed) content — ink parity, with the typed-prompt echo preserved./clearcontract:startNewSessionis bound in the command context (the bare-reference throw that left the transcript standing is closed),clearItemscompletes the contract by clearing the visible transcript, and the entry wiresonTranscriptEventinto the shell. Host/dispatcher identity across re-renders is pinned by tests.
Historical items: round 1's nine Suggestions (U-30 doc traceability, fileData coverage, blocked-prompt projection pin, ctx-dependent host projection, per-message steering granularity, the two-rows submit_prompt echo, steer-echo dedupe, host-identity pin, spec-collection disclosure) are all addressed per the thread replies and present at this head; round 2's single doc-lane item was fixed in 7dab0662 after verification against e2e.yml. The deferred round-2 probes (shared image-format helper, startsWith scoping) are explicitly non-blocking.
CI at this head: the only failing check is Dependency CVE audit, which is not introduced by this PR — the diff touches no dependency manifest, and the job log shows both completed scans reporting 0 vulnerabilities; the exit-1 comes from the registry's legacy audit endpoint returning 400 Bad Request on the request itself (the endpoint's own retirement notice is in the log). Test (ubuntu-latest, Node 22.x), Lint & Static and review-pr are still pending, which does not gate this review per policy; the OpenTUI no-flicker gate, TUI parity snapshots and the no-AK integration lane pass.
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings. (Approval not recorded from this account — I'm the PR author.)
Scope: all 19 files reviewed at head 7dab0662 (the 407-line design doc skimmed for claims; specs read at assertion level). NOT covered — the PR's own mutation claim (neutering the projection fails the OpenTUI leg, ink stays green) not re-executed (needs a re-bundle) · Windows/Linux locally · no e2e-leg run exists at this final head (see below).
Checked at head:
- Exhaustiveness by construction:
projectItemToStreamEventis a total switch with aneverdefault; every no-op kind carries a stated reason (live-stream duplicates would double-render;helprenders through the dialog overlay; advisor/arena/recap kinds = registered U-34 gap, unchanged visibility-wise). A new history kind fails the build rather than vanishing silently — matches the design claim. - No double render / no dropped row across the two stores:
host.addItemcollapses an identical consecutive user item in history and skips the transcript append in the same branch;foldLiveEventcollapses identical consecutive user rows in the transcript — so two identical steers = one row on both stores.invocationEchoedsuppresses the live-turn user row exactly forsubmit_promptoutcomes (whose typed invocation is echoed via host history), and never for plain prompts. Traced all three row-producing paths (live stream, host projection, steer echo) through the fold. - Steer echo shape matches ink: per-message read cards immediately before that message's own row (ink
accept()order),sentToModel: falsekeeps it out of the rewind list, and a restored hop discards cards+rows as a unit. /clear: the crash was a real receiver-detachment (host.startNewSessionpassed bare into a context that/clearcalls beforeui.clear()) — now bound at the construction site;transcript.clearroutes through the live-turn reset with an empty batch, which also drops a stray steering queue. Both histories clear, matching the SessionSwitchHost contract the code cites.- U-27 disclosure: fresh hop checks resolved parts once before the send loop (tool continuations never checked, as ink); steering hop checks per surviving message; warning names the narrow set while the check uses the wide set — the ported quirk matches core's helper.
- Leg evidence, honestly read: in dispatched run 33830499451 the new spec passed (its 11 files: 1 failed | 9 passed | 1 skipped; the failure is
context-compress … forward /compress instructions, 289 s with retries, at pre-head1cdb70d7). Neighbouring main runs show the flake pattern is pre-existing and moves around (33834473606 failed on different interactive tests; 33839728395 fully green). Two evidence nits, not code defects: (a) the run is red overall and the body cites it without saying so; (b) no leg run exists at the final head — worth one more dispatch before merge since the leg is exactly this feature's proof surface.
Ran (linux, head 7dab0662): vitest src/ui/opentui → 67 files, 1125/1125 passed.
Cross-check: the ci-bot's two rounds are Suggestion-only (doc disposition of U-30, untested fileData branch, projection-test assertions, spec-collection notes) and nothing on record contradicts the pass above; @qqqys and the dev-bot approved this head with the suggestion items addressed.
Reviewed with AI assistance.
|
Released in v0.23.1. |





What this PR does
The OpenTUI renderer kept two histories and painted only one of them. Everything a slash command reported — an approval-mode change, a status block, a rejected submission, a hook failure, a quit summary — was recorded in a store no view reads, so the command ran, changed real state, and left the screen untouched. This makes a recorded item become a transcript row at the moment it is recorded, through one mapping that decides every kind of item the command layer can write and is checked for exhaustiveness at compile time. Kinds the live model turn already renders are deliberate no-ops so nothing paints twice. Four kinds the existing renderer draws with components this transcript has no row shape for — an advisor review, two arena completion cards, a session recap — are written here too and stay invisible, exactly as before; the design doc names each producer and registers them as one follow-up.
Clearing the conversation emptied the recorded history but left the visible transcript standing, which the session-switch contract already described as clearing both. It now clears both. Clearing also surfaced a crash that had been hiding behind the invisible transcript: the command context handed out one host method detached from its receiver, so the clear command threw before it cleared anything, and the only thing the user saw was the error. The receiver is now bound where the context is built.
A message typed while the model is still working now appears as its own row once the turn accepts it. Previously the queue counter dropped and the text disappeared without a trace. Each steered message renders with its own read cards immediately before its own row, matching the order the existing renderer produces. The row is recorded as not-a-standalone-turn, the way the existing renderer records it. Neither renderer's row drawing reads that flag, so the row renders the same whichever way it is set; what the flag decides is whether the steer counts as a turn — it stays out of the rewind list. Making the recorded invocation visible exposed two row-count differences in the other direction, now matched to the existing renderer: a command that expands into a prompt of its own shows only the invocation the user typed, never the expansion, and two identical messages steered back to back are one row.
Submitting an image the pipeline cannot read now discloses the supported formats, on both the fresh-submit path and the mid-turn path. Previously such an image reached the model with no notice at all.
Why it's needed
The renderer migration's promise is that switching renderers does not change what the user sees. Silent command output is the most visible break of that promise: a user runs a command, the command works, and the terminal says nothing. It survived every batch so far because no test asserted on a command's rendered output — the assertions looked at internal stores, which were always correct. This PR adds the missing bridge and, with it, the first interactive spec that waits for a command's output to actually reach the screen.
Reviewer Test Plan
How to verify
Run the CLI under the OpenTUI renderer and check four behaviours:
/about. Expect the full status block in the transcript — version, runtime, auth, model, session id, memory usage. Before this PR: nothing at all appears. The same holds for/approval-mode plan, which should echo the invocation and then report the new mode./clear. Expect an empty transcript and no error row. Before this PR: an error row about an undefined object, and the transcript still standing.Behaviour 1 is the first thing this spec covers, but it is not covered by the PR's own check run: the workflow that carries the spec triggers on pushes to
main, on the nightly schedule, and on demand — never onpull_request. So a reviewer sees either the local runs quoted below, a manual dispatch against this branch (run 33830499451), or the run that lands onmainafter merge. What the spec asserts: the boot screen does not contain a field label only/aboutrenders, then it waits for that label after the command and asserts the model was never called — so the row can only have come from the command. The wait accepts either that field label or ink's unknown-command text and re-sends on the latter, inside a bounded window: ink loads its slash-command registry in an async effect that gates nothing, so a command sent the moment the prompt appears can land on an empty registry — a boot race unrelated to this change, which otherwise turns the ink leg flaky as soon as two spec files boot CLIs concurrently. The OpenTUI leg does not need the retry because it reloads the registry and re-parses whenever nothing matched. The window stays discriminating: a projection regression produces neither string, so the wait throws instead of spinning — measured by neutering the projection, which fails the OpenTUI leg and leaves the ink leg green.Evidence (Before & After)
Before (from the filed report):
/approval-mode planchanges the approval mode and changes nothing on screen.After, OpenTUI (bun + strict) at the PR's base commit, in a 100-column tmux pane:
/aboutrenders the projected status block, then/clearempties the transcript, leaving only the composer and no error row. TheBase URLfield is elided below because it names a private endpoint; every other line is verbatim.Tested on
Windows and Linux are left to CI.
Environment (optional)
Local bundle under both renderer legs (
QWEN_TUI_RENDERER=inkon node,=opentuiwith strict mode on bun), plus the tmux smoke quoted above.Risk & Scope
Linked Issues
Fixes #10905. Part of the renderer migration tracked in #8662.
中文说明
这个 PR 做了什么
OpenTUI 渲染器一直维护两份历史,却只渲染其中一份。斜杠命令汇报的所有内容——审批模式变更、状态块、被拒绝的提交、hook 失败、退出摘要——都被写进一个没有视图读取的存储里,于是命令确实执行了、确实改变了真实状态,屏幕上却什么都不动。本 PR 让「记录一条历史项」的同一时刻就把它变成一条转录行,走的是一张对命令层能写出的每一种历史项类型都做出明确判定的映射表,并在编译期做穷尽性检查。模型回合自身已经渲染的那些类型被刻意留成空操作,避免同一行画两遍。另有四种类型——advisor 的评审、arena 的两张完成卡片、会话回顾——旧渲染器是用专门的组件画的,这边的转录还没有对应的行形状:它们在本渲染器下同样会被写入,也同样仍然不可见(与本 PR 之前一致)。设计文档逐条写明了它们的生产者,并登记为一个后续项。
清空会话此前只清空了记录用的历史,可见的转录仍然留在屏幕上——而会话切换的契约本来就写明两者都要清。现在两者都清。清空还暴露出一个一直藏在「不可见转录」背后的崩溃:命令上下文把宿主的某个方法以脱离接收者的形式交了出去,于是清空命令在清任何东西之前就抛错,用户唯一看到的就是那条错误。现在接收者在构建上下文处绑定。
模型仍在工作时输入的消息,现在会在回合接受它之后显示为独立的一行。此前队列计数消失、文本无痕无踪。每条中途消息都会先渲染自己的读取卡片、紧跟着自己的那一行,与既有渲染器的顺序一致。这一行按既有渲染器同样的方式被记为「不是独立的用户回合」。两边渲染用户行的代码都不读这个标记,所以这一行无论标记取何值渲染结果都相同;这个标记决定的是中途消息算不算一个回合——它因此不会进入回退(rewind)列表。让「被记录的调用」可见之后,也暴露出两个反方向的行数差异,现已与既有渲染器对齐:会展开成自己那段提示词的命令只显示用户真正敲下的调用、不显示展开结果;连续两次中途投喂同一条文本只渲染一行。
提交流水线读不了的图片时,现在会披露受支持的格式,新提交与中途两条路径都覆盖。此前这种图片会毫无提示地直达模型。
为什么需要
渲染器迁移的承诺是:换渲染器不改变用户看到的东西。命令输出静默是对这个承诺最显眼的破坏——用户敲了命令,命令生效了,终端却一声不响。它能一直存活到今天,是因为没有任何测试断言过命令的渲染输出:断言看的都是内部存储,而内部存储一直是对的。本 PR 补上缺失的这座桥,并带来第一个「等命令输出真正上屏」的交互测试。
评审测试计划
如何验证
在 OpenTUI 渲染器下运行 CLI,确认四个行为:
/about,预期转录里出现完整状态块——版本、运行时、鉴权、模型、会话 ID、内存占用。本 PR 之前:什么都不出现。/approval-mode plan同理,应先回显调用、再汇报新模式。/clear,预期转录为空且没有错误行。本 PR 之前:出现一条「undefined is not an object」错误行,转录原封不动。行为 1 是这个测试覆盖的第一项,但它并不在本 PR 自己的门禁检查里:承载该测试的 workflow 只在推送到
main、每晚定时任务、以及手动触发时运行,从不因pull_request运行。所以评审者能看到的证据有三种:下面引用的本地运行、针对本分支的手动触发、或合并后落在main上的那次运行。测试断言的内容是:启动画面不包含只有/about才会渲染的字段标签,然后在命令之后等待该标签出现,并断言模型从未被调用——因此这一行只可能来自命令本身。这个等待接受两种文本之一:该字段标签,或 ink 的「未知命令」提示;命中后者就在一个有界时间窗内重发。原因是 ink 在一个不受任何门控的异步 effect 里加载斜杠命令注册表,所以提示符一出现就发出的命令可能落在空注册表上——这是与本次改动无关的启动竞态,一旦有两个测试文件并发起 CLI,ink 腿就会因此变抖。OpenTUI 腿不需要重试,因为它在没有匹配到命令时会重新加载注册表并重新解析。这个时间窗仍然具备判别力:投影回归时两种文本都不会出现,于是等待会抛错而不是空转——这一点已用变异实测:把投影掐断后 OpenTUI 腿失败、ink 腿保持绿。证据(前后对比)
之前(取自提交的 issue 报告):
/approval-mode plan改变了审批模式,屏幕上没有任何变化。之后,OpenTUI 下先
/about再/clear——状态块渲染出来,随后转录为空,只剩输入区,没有错误行(见英文部分的终端截取)。测试环境
Windows 与 Linux 交给 CI。
本地在两条渲染器腿上跑打包产物(node 上
QWEN_TUI_RENDERER=ink,bun 上=opentui且开启 strict),外加上面引用的 tmux 冒烟。风险与范围
关联 Issue
Fixes #10905。属于 #8662 跟踪的渲染器迁移工作。