fix(cli): optimize repeated inline image rendering - #8789
Conversation
Verification / 验证报告English
中文
|
|
Thanks for the PR! Template looks good ✓ Problem: observed and documented. This is the follow-up spun out of #8305's fifth review round — issue #8608 is open with explicit acceptance criteria naming exactly the three inefficiencies addressed here: invalid multi-megabyte payloads re-decoded and re-validated on every re-render, image base64 payloads walked by the recursive ANSI sanitizer, and image-only rows reserving a text share they never use. Not theoretical. Direction: aligned — terminal-rendering performance in the CLI, squarely in scope. No direct CHANGELOG reference in claude-code for this area, but the linked issue itself (with its acceptance criteria) is the direction signal here. Size: no core paths touched — all six files live in Approach: scope matches the issue 1:1 — one small mechanism per acceptance criterion, each with a test. The SHA-256-keyed negative cache mirrors the existing bounded-LRU Risk: no elevated risk signals — none of the revert-correlated paths are touched. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测且有记录。这是 #8305 第五轮评审拆出的后续工作——issue #8608 仍然 open，验收标准明确列出了本 PR 处理的三个低效点：无效的大尺寸 payload 每次重渲染都重新解码和校验、图片 base64 数据被递归 ANSI 清洗遍历、纯图片消息预留了从未使用的文本高度份额。不是理论性问题。 方向：对齐——CLI 终端渲染性能优化，完全在项目范围内。claude-code CHANGELOG 中无此方向的直接引用，但关联 issue 本身（含明确验收标准）就是方向信号。 规模：未触及核心路径——六个文件全部位于 方案：范围与 issue 一一对应——每条验收标准一个小机制，且各配测试。SHA-256 负缓存复用了现有 风险：无升级风险信号——未触及任何与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8789 optimizes repeated inline image rendering in the...: could not execute the test suites to confirm they pass (no node_modules in the review worktree); both findings rest on exact code-path tracing rather than a l…; PR #8789 optimizes repeated inline image rendering in the...: full npm run typecheck / build not run — this fresh worktree has no generated packages/core/dist , and vitest's transform covers syntax/imports but not full ….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
未探索到全部深度（达到工具调用预算）：PR #8789 optimizes repeated inline image rendering in the...：could not execute the test suites to confirm they pass (no node_modules in the review worktree); both findings rest on exact code-path tracing rather than a l…；PR #8789 optimizes repeated inline image rendering in the...：full npm run typecheck / build not run — this fresh worktree has no generated packages/core/dist , and vitest's transform covers syntax/imports but not full …。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| if ( | ||
| !Array.isArray(obj) && | ||
| typeof (obj as Record<string, unknown>)['data'] === 'string' && | ||
| typeof (obj as Record<string, unknown>)['mimeType'] === 'string' && | ||
| (obj as Record<string, unknown>)['mimeType'] | ||
| .toString() | ||
| .toLowerCase() | ||
| .startsWith('image/') | ||
| ) { | ||
| return obj; | ||
| } |
There was a problem hiding this comment.
[Critical] packages/cli does not compile at this commit: tsc --build fails with TS2571: Object is of type 'unknown' on this block. The typeof narrowing from the preceding conjunct does not carry through the repeated (obj as Record<string, unknown>) assertion expressions, so .toString() is invoked on a value of type unknown.
Failure scenario: any build of this PR fails — npm run build --workspace=packages/cli exits 1 with this single error (the other 14 workspaces in the build set compile clean). The CLI cannot emit dist/, npm run bundle cannot run, and the CI build job fails; because the build fails, the workspace's test suite cannot run either.
| if ( | |
| !Array.isArray(obj) && | |
| typeof (obj as Record<string, unknown>)['data'] === 'string' && | |
| typeof (obj as Record<string, unknown>)['mimeType'] === 'string' && | |
| (obj as Record<string, unknown>)['mimeType'] | |
| .toString() | |
| .toLowerCase() | |
| .startsWith('image/') | |
| ) { | |
| return obj; | |
| } | |
| const record = obj as Record<string, unknown>; | |
| if ( | |
| !Array.isArray(obj) && | |
| typeof record['data'] === 'string' && | |
| typeof record['mimeType'] === 'string' && | |
| record['mimeType'].toLowerCase().startsWith('image/') | |
| ) { | |
| return obj; | |
| } |
Hoisting the cast once lets element-access narrowing work inside the && chain, and the redundant .toString() drops with it (the typeof check already established a string). Verified to compile under the repo's tsc 5.8.3 --strict.
中文说明
packages/cli 在此提交无法编译：tsc --build 在该代码块报 TS2571: Object is of type 'unknown'。前一个条件的 typeof 收窄无法穿透后续重复的 (obj as Record<string, unknown>) 断言表达式，导致 .toString() 被调用在 unknown 类型的值上。
失败场景：该 PR 的任何构建都会失败 —— npm run build --workspace=packages/cli 以这唯一的错误退出（退出码 1），构集中其余 14 个 workspace 均编译通过。CLI 无法产出 dist/，npm run bundle 无法运行，CI 构建任务失败；构建失败也导致该 workspace 的测试无法运行。
修复：将类型断言提升为一个局部变量，使元素访问收窄在 && 链中生效，同时去掉多余的 .toString()（typeof 检查已确定其为字符串）。已验证在仓库的 tsc 5.8.3 --strict 下编译通过。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Fixed in 9cea67e0a7c0f5d9d13791289edf1f89d6b6619f.
The guard now reuses one Record<string, unknown> reference, so the existing typeof check narrows mimeType before toLowerCase() is called. I also removed the redundant toString() call. This is a type-safety-only change and preserves the existing runtime behavior; adjacent Suggestions were intentionally left out of this Critical-only round.
Verified with Node.js 22.23.1: CLI tsc --noEmit passed; the three focused test files passed 97/97 tests; repository build, bundle, and workspace typecheck passed; targeted Prettier, ESLint, and git diff --check passed. An independent verifier also returned VERIFIED_FIXED.
已在 9cea67e0a7c0f5d9d13791289edf1f89d6b6619f 修复。
现在复用同一个 Record<string, unknown> 引用，使现有的 typeof 检查能够在调用 toLowerCase() 前正确收窄 mimeType，同时删除多余的 toString()。这次只修类型安全问题，运行时行为不变；相邻的 Suggestion 刻意不纳入本轮 Critical 修复。
在 Node.js 22.23.1 下验证：CLI tsc --noEmit 通过；3 个聚焦测试文件共 97/97 通过；仓库 build、bundle、全 workspace typecheck 通过；目标文件 Prettier、ESLint 和 git diff --check 通过。独立验证结论也是 VERIFIED_FIXED。
| prepareInlineTerminalImage({ | ||
| data: invalidPayloads[0], | ||
| mimeType: 'image/png', | ||
| contentWidth: 24, | ||
| env: { TERM: 'xterm-kitty' }, | ||
| stdoutIsTTY: true, | ||
| }); | ||
|
|
||
| expect(bufferFrom).toHaveBeenCalledTimes( | ||
| INLINE_DECODE_NEGATIVE_CACHE_LIMIT + 2, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This test never exercises a cache hit: the re-submitted payload (invalidPayloads[0]) is exactly the entry evicted when the 65th payload was inserted (capacity 64, oldest first), so the re-submission is a guaranteed miss. The Buffer.from count is LIMIT + 2 = 66 whether or not the negative cache exists — a mutant with the hit path and both cacheInvalidInlineImage calls deleted still passes (measured by probe). Only eviction is pinned; issue (1) (#8608) AC4 asks for focused eviction/negative-hit tests.
Failure scenario: a future refactor that removes or breaks the invalidInlineImageCache.has(...) early return ships with this test green, reintroducing per-render base64 decode + PNG validation of repeated invalid payloads — the exact regression this PR exists to fix.
| prepareInlineTerminalImage({ | |
| data: invalidPayloads[0], | |
| mimeType: 'image/png', | |
| contentWidth: 24, | |
| env: { TERM: 'xterm-kitty' }, | |
| stdoutIsTTY: true, | |
| }); | |
| expect(bufferFrom).toHaveBeenCalledTimes( | |
| INLINE_DECODE_NEGATIVE_CACHE_LIMIT + 2, | |
| ); | |
| prepareInlineTerminalImage({ | |
| data: invalidPayloads[invalidPayloads.length - 1], | |
| mimeType: 'image/png', | |
| contentWidth: 24, | |
| env: { TERM: 'xterm-kitty' }, | |
| stdoutIsTTY: true, | |
| }); | |
| expect(bufferFrom).toHaveBeenCalledTimes( | |
| INLINE_DECODE_NEGATIVE_CACHE_LIMIT + 1, | |
| ); | |
| prepareInlineTerminalImage({ | |
| data: invalidPayloads[0], | |
| mimeType: 'image/png', | |
| contentWidth: 24, | |
| env: { TERM: 'xterm-kitty' }, | |
| stdoutIsTTY: true, | |
| }); | |
| expect(bufferFrom).toHaveBeenCalledTimes( | |
| INLINE_DECODE_NEGATIVE_CACHE_LIMIT + 2, | |
| ); |
Re-submitting the most recently inserted payload must add no decode (cache hit); the strengthened probe passes on the PR code and fails under the no-cache mutation.
中文说明
该测试从未覆盖缓存命中路径：重新提交的 payload（invalidPayloads[0]）恰好是插入第 65 个 payload 时被逐出的条目（容量 64，逐出最早条目），因此重新提交必然未命中。无论负缓存是否存在，Buffer.from 的调用次数都是 LIMIT + 2 = 66 —— 即使删除命中路径和两处 cacheInvalidInlineImage 调用，测试仍然通过（已用探针实测）。当前只验证了逐出；issue #8608 的验收标准 AC4 要求的是逐出/负命中两项定向测试。
失败场景：未来若有重构移除或破坏 invalidInlineImageCache.has(...) 提前返回，该测试仍会绿灯通过，导致重复的无效 payload 在每次渲染时重新进行 base64 解码 + PNG 校验 —— 正是本 PR 要消除的回归。
修复：先重新提交最近插入、仍在缓存中的 payload，断言没有新增解码（缓存命中）；再提交被逐出的条目验证逐出。加强后的探针在 PR 代码上通过，在无缓存变体上失败。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| }); | ||
|
|
||
| it('preserves inline image data while sanitizing surrounding text', () => { | ||
| const imageData = 'payload\u001bwith-control-code'; |
There was a problem hiding this comment.
[Suggestion] This test is inert: 'payload\u001bwith-control-code' contains a bare ESC followed by w, which ansi-regex does not match (verified: search returns -1), so with the image-skip block in escapeAnsiCtrlCodes reverted, all 38 tests still pass (confirmed by the test-efficacy probe with a validated harness).
Failure scenario: if a later change deletes the image skip or breaks its mimeType condition, every test in this diff stays green and the regression silently ships — image payloads are again scanned/escaped recursively (the O(n) cost on large base64 blobs this PR removes), with nothing in the suite to flag it.
| const imageData = 'payload\u001bwith-control-code'; | |
| const imageData = 'payload\u001b[31mwith-control-code'; |
With a sequence ansi-regex actually matches, the assertion discriminates: skip present → data preserved (pass); skip reverted → data escaped (fail).
中文说明
该测试是无效的（inert）：'payload\u001bwith-control-code' 包含一个裸 ESC 后跟 w，ansi-regex 无法匹配（已验证：search 返回 -1），因此即使还原 escapeAnsiCtrlCodes 中的图片跳过逻辑，全部 38 项测试仍然通过（测试效力探针确认，harness 已通过阳性对照验证）。
失败场景：若后续改动删除图片跳过逻辑或破坏其 mimeType 条件，本 diff 的所有测试仍为绿灯，回归将悄悄合入 —— 图片 payload 会再次被递归扫描/转义（即本 PR 要消除的大体积 base64 O(n) 开销），而测试套件无法发现。
修复：使用 ansi-regex 确实能匹配的序列，断言才具备区分能力：跳过逻辑存在 → 数据原样保留（通过）；跳过逻辑被还原 → 数据被转义（失败）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| availableTerminalHeight / | ||
| (images.length + (text.length > 0 ? 1 : 0)), |
There was a problem hiding this comment.
[Suggestion] The imageHeightBudget formula is duplicated verbatim in PrefixedMarkdownMessage (here) and ContinuationMarkdownMessage (the copy at ~line 238); this PR had to apply the identical edit to both.
Failure scenario: the next formula change applied to only one of the two components makes first-segment assistant messages (AssistantMessage) and continuation segments (AssistantMessageContent) split the same terminal height differently — the same image renders at different heights between segments of one multi-segment reply, and no test renders both segments together, so the divergence ships silently.
Extract one helper used by both, e.g.:
function getImageHeightBudget(
availableTerminalHeight: number | undefined,
imageCount: number,
hasText: boolean,
): number | undefined {
if (availableTerminalHeight === undefined || imageCount === 0) {
return availableTerminalHeight;
}
return Math.max(
1,
Math.floor(availableTerminalHeight / (imageCount + (hasText ? 1 : 0))),
);
}(2 locations: this one and the ContinuationMarkdownMessage copy)
中文说明
imageHeightBudget 公式在 PrefixedMarkdownMessage（此处）与 ContinuationMarkdownMessage（约第 238 行的副本）中逐字重复；本 PR 不得不对两处做完全相同的修改。
失败场景：下次修改公式时若只改了其中一个组件，首段 assistant 消息（AssistantMessage）与续段（AssistantMessageContent）将以不同方式分配同一终端高度 —— 同一条多段回复中的相同图片在不同段落渲染出不同高度，而没有任何测试同时渲染两段，因此这种分歧会悄悄合入。
修复：提取一个两处共用的辅助函数（见英文部分代码示例）。（共 2 处：此处与 ContinuationMarkdownMessage 副本）
— qwen3.8-max via Qwen Code /review (v0.21.8)
| availableTerminalHeight / | ||
| (images.length + (text.length > 0 ? 1 : 0)), |
There was a problem hiding this comment.
[Suggestion] Same duplicated-formula pattern as the PrefixedMarkdownMessage copy (~line 179): imageHeightBudget exists verbatim at both depths, and this PR had to write the identical edit twice.
Failure scenario: a future budget-policy change applied to only one of the two components makes first-segment assistant messages and continuation segments split the same terminal height differently — visibly inconsistent image heights within one multi-segment reply, and no test renders both segments together to catch it.
Fix: extract the shared helper suggested in the other comment and call it from both components.
中文说明
与 PrefixedMarkdownMessage 副本（约第 179 行）相同的公式重复问题：imageHeightBudget 在两处逐字存在，本 PR 不得不将同一修改写两遍。
失败场景：未来修改预算策略时若只改其中一个组件，首段 assistant 消息与续段将以不同方式分配同一终端高度 —— 同一条多段回复内图片高度明显不一致，且没有测试同时渲染两段来发现问题。
修复：提取另一条评论中建议的共享辅助函数，并在两个组件中调用。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| (obj as Record<string, unknown>)['mimeType'] | ||
| .toString() | ||
| .toLowerCase() |
There was a problem hiding this comment.
[Suggestion] This guard omits .trim() on mimeType, while the producer (getInlineImageData in inline-image-parts.ts) checks inlineData.mimeType.trim().toLowerCase().startsWith('image/') but stores the original untrimmed string, and the renderer's getImageFormat also trims.
Failure scenario: an image with mimeType: ' image/png' (padded — a shape the producer plainly anticipates) is collected and rendered normally but fails this skip check, so its multi-megabyte data keeps paying the O(n) escape scan on every sanitize pass — the exact cost this PR removes. Probe-verified: the guard fires for 'image/png' but not ' image/png'; adding .trim() flips the padded case.
Combined with the Critical fix above this becomes record['mimeType'].trim().toLowerCase().startsWith('image/').
中文说明
该判断对 mimeType 缺少 .trim()，而生产方（inline-image-parts.ts 的 getInlineImageData）使用 inlineData.mimeType.trim().toLowerCase().startsWith('image/') 判断、但保存的是未去空白的原始字符串，渲染方的 getImageFormat 同样会先 trim。
失败场景：mimeType: ' image/png'（带空白 —— 生产方显然预期这种形态）的图片会被正常收集并渲染，但无法通过此跳过判断，其数 MB 的 data 在每次清洗时仍要付出 O(n) 扫描开销 —— 正是本 PR 要消除的成本。已用探针验证：判断对 'image/png' 生效、对 ' image/png' 不生效；加上 .trim() 后带空白的情况也会生效。
与上方 Critical 修复合并后为 record['mimeType'].trim().toLowerCase().startsWith('image/')。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const prefixWidth = getPrefixWidth(basePrefix); | ||
| const imageHeightBudget = | ||
| availableTerminalHeight !== undefined && images?.length |
There was a problem hiding this comment.
[Suggestion] The identical budget-formula change in ContinuationMarkdownMessage is pinned by no test: the only <AssistantMessageContent /> image test omits availableTerminalHeight, so the budget branch never evaluates. Reverting this hunk's formula to images.length + 1 leaves all 23 existing tests green (measured by probe). The path is production-reachable — HistoryItemDisplay renders gemini_content items through it with images and availableTerminalHeight.
Failure scenario: a reverted or mis-applied continuation formula makes an image-only continuation chunk render with the shrunk n+1 budget — with 2 images and height 20 each image gets 6 rows instead of 10 (observed in the probe), the exact bug this PR fixes for first chunks.
Mirror the new AssistantMessage test:
it('gives image-only continuation messages the full image height budget', () => {
const { lastFrame } = render(
<AssistantMessageContent
text=""
images={[
{ data: 'Zmlyc3Q=', mimeType: 'image/png' },
{ data: 'c2Vjb25k', mimeType: 'image/png' },
]}
isPending={false}
availableTerminalHeight={20}
contentWidth={80}
/>,
);
expect(lastFrame()).toContain('MockTerminalImage:image/png:height=10');
});中文说明
ContinuationMarkdownMessage 中相同的预算公式修改没有任何测试覆盖：唯一的 <AssistantMessageContent /> 图片测试未传 availableTerminalHeight，预算分支从不执行。即使把本 hunk 的公式还原为 images.length + 1，现有 23 项测试仍全部通过（探针实测）。该路径在生产中可达 —— HistoryItemDisplay 会携带 images 与 availableTerminalHeight 通过它渲染 gemini_content 项。
失败场景：续段公式被还原或改错时，纯图片续段将按缩小的 n+1 预算渲染 —— 2 张图、高度 20 时每张只得 6 行而非 10 行（探针实测观察值），正是本 PR 为首段修复的 bug。
修复：仿照新增的 AssistantMessage 测试补一个 AssistantMessageContent 测试（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| availableTerminalHeight / | ||
| (images.length + (text.length > 0 ? 1 : 0)), |
There was a problem hiding this comment.
[Suggestion] When output.showTimestamps is enabled, HistoryItemDisplay renders a [HH:MM:SS] timestamp row above every gemini item, but the image-only budget now hands the full availableTerminalHeight to the images and never counts that sibling row — a pending image-only message exceeds the budget by one row. This fires with a single image for every budget value and was introduced by this diff's arithmetic: pre-diff, / (images.length + 1) left the slack that absorbed the timestamp row. Probe-measured: budget=20 → 21 content rows. Pending-render only (committed items receive MAX_GEMINI_MESSAGE_LINES).
Failure scenario: with output.showTimestamps: true (real opt-in setting, default false) and constrainHeight on (the streaming default), a pending image-only item at height 20 renders 20 image rows + 1 timestamp row = 21, pushing the constrained pending area one row past the viewport budget whose stated purpose is keeping pending content within it.
Suggested fix (probe-verified to restore an exact fit): in HistoryItemDisplay, pass availableTerminalHeight - 1 to AssistantMessage when showTimestamps && timestamp != null and the height is defined.
中文说明
启用 output.showTimestamps 时，HistoryItemDisplay 会在每个 gemini 项上方渲染一行 [HH:MM:SS] 时间戳，但纯图片预算现在把完整的 availableTerminalHeight 全部分给图片，没有计入这条同级行 —— pending 的纯图片消息会超出预算一行。单张图片、任意预算值都会触发，且由本 diff 的算式引入：改动前 / (images.length + 1) 留出的余量恰好吸收了时间戳行。探针实测：预算 20 → 实际内容 21 行。仅影响 pending 渲染（已提交项使用 MAX_GEMINI_MESSAGE_LINES）。
失败场景：开启 output.showTimestamps（真实的可选设置，默认关闭）且流式渲染默认开启 constrainHeight 时，高度 20 的 pending 纯图片项渲染出 20 行图片 + 1 行时间戳 = 21 行，使受约束的 pending 区域超出视口预算一行 —— 该预算的既定目的就是让 pending 内容保持在视口内。
建议修复（探针验证可精确贴合）：在 HistoryItemDisplay 中，当 showTimestamps && timestamp != null 且高度有定义时，向 AssistantMessage 传入 availableTerminalHeight - 1。
— qwen3.8-max via Qwen Code /review (v0.21.8)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): Context: PR #8789 (round 2 review) optimizes repeated inl...: none — all planned checks completed within budget..
[Critical] R1-1 (round-1 Critical) still stands at this head: packages/cli does not compile — npm run build --workspace=packages/cli exits 1 with src/ui/utils/textUtils.ts(347,5): error TS2571: Object is of type 'unknown'. The typeof narrowing does not carry through the repeated (obj as Record<string, unknown>) assertion expressions, so .toString() is invoked on unknown. The fix is in the open round-1 thread at textUtils.ts:353 (hoist the cast once into a local record variable and drop the redundant .toString()); this round re-measured the identical failure at head 34996f5.
中文说明
未探索到全部深度（达到工具调用预算）：Context: PR #8789 (round 2 review) optimizes repeated inl...：none — all planned checks completed within budget.。
[Critical] R1-1 (round-1 Critical) still stands at this head: packages/cli does not compile — npm run build --workspace=packages/cli exits 1 with src/ui/utils/textUtils.ts(347,5): error TS2571: Object is of type 'unknown'. The typeof narrowing does not carry through the repeated (obj as Record<string, unknown>) assertion expressions, so .toString() is invoked on unknown. The fix is in the open round-1 thread at textUtils.ts:353 (hoist the cast once into a local record variable and drop the redundant .toString()); this round re-measured the identical failure at head 34996f5.
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const prefixWidth = getPrefixWidth(prefix); | ||
| const imageHeightBudget = | ||
| availableTerminalHeight !== undefined && images?.length | ||
| ? Math.max(1, Math.floor(availableTerminalHeight / (images.length + 1))) | ||
| ? Math.max( | ||
| 1, | ||
| Math.floor( | ||
| availableTerminalHeight / |
There was a problem hiding this comment.
[Suggestion] The image-only full-height-budget fix is applied to the two assistant-message components, but not to the third copy of the same formula at ToolMessage.tsx:783-784 — it still divides by (images.length + 1) and reserves one height share for result text even when none renders. — Failure scenario: a tool row whose response carries inline image parts but no display text (useReactToolScheduler attaches images independently of resultDisplay, and an empty resultDisplay skips the result box entirely) renders each image one terminal row shorter than the budget allows (e.g. 1 image with 14 rows of budget gets 7 instead of 14): the same wasted-row artifact this PR removes for assistant messages persists on the tool path.
// ToolMessage.tsx (~line 783) — mirror the same conditional:
Math.max(
1,
Math.floor(
availableHeight / (images.length + (hasResultText ? 1 : 0)),
),
)
// where hasResultText reflects whether the result box actually renders中文说明
纯图片全高预算修复只应用于两个 assistant 消息组件，同一公式在 ToolMessage.tsx:783-784 的第三份副本仍除以 (images.length + 1)，即使没有结果文本也会预留一份高度。失败场景：某工具行的响应携带内联图片部分但没有展示文本（useReactToolScheduler 挂载 images 与 resultDisplay 相互独立，resultDisplay 为空时结果框被整体跳过），每张图片会比可用预算少一行（例如预算 14 行、1 张图片时得到 7 行而不是 14 行）：本 PR 为 assistant 消息消除的“浪费一行”问题在工具路径上依然存在。修复：在 ToolMessage 中镜像同样的条件（见上方代码块），其中 hasResultText 反映结果框是否实际渲染。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| function cacheInvalidInlineImage(key: string): void { | ||
| invalidInlineImageCache.add(key); | ||
| while (invalidInlineImageCache.size > INLINE_DECODE_NEGATIVE_CACHE_LIMIT) { | ||
| const oldest = invalidInlineImageCache.values().next().value; | ||
| if (oldest === undefined) break; | ||
| invalidInlineImageCache.delete(oldest); | ||
| } | ||
| } |
There was a problem hiding this comment.
[Suggestion] cacheInvalidInlineImage adds the fourth hand-rolled bounded-cache eviction loop in this one file (renderCache, inlineDecodeCache above, transmittedKeys), and mermaidImageRenderer.ts holds two more; the copies have already drifted (if vs while eviction, .keys() vs .values()). A generic LruCache exists at packages/core/src/utils/LruCache.ts but is not exported from the core index and has zero importers. — Failure scenario: any fix or policy change to eviction semantics must now be applied to 4 sites in this file (6 across the two renderer files), and the next cache copies a fifth — each copy is a fresh chance to fork the semantics again. Consolidate before the pattern spreads: a small local bounded-eviction helper in this file, or export and reuse LruCache (its get/set already implement the same delete-and-reinsert LRU semantics).
中文说明
cacheInvalidInlineImage 是本文件中第四个手写的有界缓存淘汰循环（renderCache、上方的 inlineDecodeCache、transmittedKeys），mermaidImageRenderer.ts 中还有两个；这些副本已经出现漂移（if 与 while 淘汰、.keys() 与 .values()）。packages/core/src/utils/LruCache.ts 已有一个通用 LruCache，但未从 core 导出且没有任何使用方。失败场景：任何针对淘汰语义的修复或策略变更现在都必须应用到本文件的 4 处（两个 renderer 文件共 6 处），下一个缓存还会复制出第 5 份——每多一份副本就多一次语义再次分叉的机会。建议在模式继续扩散前收敛：在本文件内提供一个小的有界淘汰辅助函数，或导出并复用 LruCache（其 get/set 已实现相同的“删除再插入” LRU 语义）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| if ( | ||
| !Array.isArray(obj) && | ||
| typeof (obj as Record<string, unknown>)['data'] === 'string' && | ||
| typeof (obj as Record<string, unknown>)['mimeType'] === 'string' && |
There was a problem hiding this comment.
[Suggestion] The image exemption is a shape-based duck check that returns the entire object subtree untouched rather than exempting only the large data payload. No path is reachable today (verified: InlineImageData is exactly {data, mimeType}, no display union matches the duck type, and neither field is ever rendered raw), but the function's documented contract — "escapes all ANSI control characters found in any string values" — silently gains an undocumented shape-based exception. — Failure scenario: the first future history field or extended image shape that matches the duck type while carrying an additional rendered string — e.g. a caption/fileName added to InlineImageData, or an MCP-derived node with attacker-controllable sibling text beside data + an image-looking mimeType — silently skips ANSI sanitization for those fields (OSC 52 clipboard writes, alt-screen escapes reach the terminal), with no type-system signal and no test to catch the change. Exempt only the data key — keep walking the object's other keys (copy-on-write, preserving data identity, which keeps the perf win) — or lift the exemption to the caller that knows the shape; if the whole-object bypass is kept, state the exception in the docblock.
中文说明
该图片豁免是一个基于形状的类型检查，它原样返回整个对象子树，而不是只豁免大体积的 data 负载。当前没有可触达路径（已验证：InlineImageData 恰好只有 {data, mimeType} 两个字段，没有任何 display 联合体匹配该形状，且这两个字段都不会以原始文本渲染），但函数的文档契约——“转义所有字符串值中发现的 ANSI 控制字符”——悄悄多了一个未记录的形状化例外。失败场景：未来第一个匹配该形状、同时携带其他渲染字符串的历史字段或扩展图片形状——例如给 InlineImageData 增加 caption/fileName，或某个 MCP 派生节点在 data + 图片 mimeType 之外带有攻击者可控的兄弟文本——会悄悄跳过这些字段的 ANSI 清洗（OSC 52 剪贴板写入、alt 屏转义可到达终端），且没有编译期信号、也没有测试能捕获该变更。建议只豁免 data 键——继续遍历对象的其他键（copy-on-write，保持 data 引用不变，性能收益仍在），或把豁免提升到知道形状的调用方；若保留整对象跳过，请在 docblock 中写明该例外。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
Verification report / 验证报告Result: PASS English
中文
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
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 via Qwen Code /review (v0.21.15)
| ? Math.max( | ||
| 1, | ||
| Math.floor( | ||
| availableTerminalHeight / |
There was a problem hiding this comment.
[Suggestion] R2-1: Still stands at this head (re-checked at 9cea67e0): the image-only full-height-budget fix is applied to the two assistant-message components in this file, but not to the third copy of the same formula at ToolMessage.tsx:783-784, which still divides by (images.length + 1) and reserves a text share that an image-only tool result never uses. An image-only tool result forfeits one height share, so its images render shorter than an equivalent assistant message, and any future budget change must again be applied to three sites in lockstep — this PR itself had to write the identical edit twice inside this file. Extract one shared imageHeightBudget(availableHeight, imageCount, hasText) helper used by all three sites, or decide explicitly that tool results keep reserving the text share.
中文说明
R2-1：在当前提交仍然存在（已在 9cea67e0 复查确认）：纯图片消息使用完整高度预算的修复应用于本文件中的两个 assistant 消息组件，但同一公式在 ToolMessage.tsx:783-784 的第三处副本未更新，仍除以 (images.length + 1)，为纯图片工具结果永远不会使用的文本预留了一份高度。纯图片的工具结果因此损失一份高度，其图片渲染得比等效的 assistant 消息更矮；未来任何预算调整都必须再次同步修改三处——本 PR 自己就不得不在本文件中将同一编辑写两遍。建议抽取一个共享的 imageHeightBudget(availableHeight, imageCount, hasText) 辅助函数供三处调用，或明确决定工具结果继续预留文本份额。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| invalidInlineImageCache.delete(oldest); | ||
| } | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-2: Still stands at this head (re-checked at 9cea67e0): cacheInvalidInlineImage adds the fourth hand-rolled bounded-cache eviction loop in this one file (renderCache, inlineDecodeCache above, transmittedKeys), and mermaidImageRenderer.ts holds two more; the copies have already drifted (markKittyImageWritten evicts with a single if and does not refresh recency on re-add). Any future fix or policy change to eviction must be applied to six sites independently, and readers must cross-check each copy to learn the actual policy. Extract a small shared evictOldest(entries, limit) helper colocated with these renderers, or consciously document the local convention.
中文说明
R2-2：在当前提交仍然存在（已在 9cea67e0 复查确认）：cacheInvalidInlineImage 在本文件中新增了第四个手写有界缓存淘汰循环（renderCache、上方的 inlineDecodeCache、transmittedKeys），此外 mermaidImageRenderer.ts 中还有两个；这些副本已经出现差异（markKittyImageWritten 用单个 if 淘汰，且重新加入时不刷新新旧顺序）。未来任何针对淘汰逻辑的修复或策略变更都必须分别应用到六处，读者也必须逐一对照每个副本才能了解实际策略。建议抽取一个与这些渲染器放在一起的共享 evictOldest(entries, limit) 辅助函数，或有意识地记录这一本地约定。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const record = obj as Record<string, unknown>; | ||
| if ( | ||
| !Array.isArray(obj) && |
There was a problem hiding this comment.
[Suggestion] R2-3: Still stands at this head; the anchor moved because the fix commit rewrote this guard for type safety. The image exemption remains a shape-based duck check that returns the entire object subtree untouched rather than exempting only the large data payload. Today this is coextensive with InlineImageData (re-verified: exactly {data, mimeType}), but if that shape — or another {data, mimeType: 'image/…'} history shape — gains a rendered text field such as a caption, that field flows through HistoryItemDisplay's whole-item escapeAnsiCtrlCodes pass into <Text> unescaped, with no compiler or test signal. Exempt only the data payload (recurse into the object, passing data through unchanged), or restrict the exemption to objects whose own keys are exactly data and mimeType.
中文说明
R2-3：在当前提交仍然存在；由于修复提交为类型安全重写了该守卫，锚点位置有所移动。图片豁免仍然是一个基于形状的鸭子类型检查，会原样返回整个对象子树，而不是只豁免大体积的 data 负载。目前它与 InlineImageData 完全重合（已复查确认恰好为 {data, mimeType}），但如果该形状——或其他 {data, mimeType: 'image/…'} 历史记录形状——新增了 caption 等需要渲染的文本字段，该字段将经由 HistoryItemDisplay 对整个条目的 escapeAnsiCtrlCodes 处理原样进入 <Text>，且没有任何编译器或测试信号。建议只豁免 data 负载（递归进入对象、仅对 data 原样透传），或将豁免限定为自身键恰好是 data 与 mimeType 的对象。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| record['mimeType'].toLowerCase().startsWith('image/') | ||
| ) { | ||
| return obj; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-1: The image-object exemption silently contradicts escapeAnsiCtrlCodes's doc comment ("escapes all ANSI control characters found in any string values") and carries no comment explaining why image objects are exempted. This function is the shared terminal-sanitization boundary, and its documented contract now misstates the behavior: a maintainer extending InlineImageData with a display-facing text field would trust the contract, but the whole-object return obj skip means that field reaches the terminal unescaped. Verified by probe at this commit: escapeAnsiCtrlCodes({data:'aGVsbG8=', mimeType:'image/png', caption:'\u001b[31mcaption\u001b[0m'}) returns the same object reference with caption still raw; removing the exemption flips the assertions. Update the doc comment to state the exemption and add a one-line why-comment here (base64 data is never rendered as text, and escaping would rescan megabyte payloads on every re-render); narrowing the skip to data also closes the drift hazard.
中文说明
R3-1：该图片对象豁免与 escapeAnsiCtrlCodes 的文档注释（"转义所有字符串值中发现的全部 ANSI 控制字符"）悄悄矛盾，且没有任何注释说明为何豁免图片对象。该函数是共享的终端内容清洗边界，其文档约定现已与真实行为不符：未来维护者为 InlineImageData 添加面向显示的文本字段时会信任该约定，但整个对象 return obj 的跳过会让该字段未经转义直达终端。已在本提交用探针验证：escapeAnsiCtrlCodes({data:'aGVsbG8=', mimeType:'image/png', caption:'\u001b[31mcaption\u001b[0m']}) 返回同一对象引用且 caption 保持原样；移除豁免后断言翻转。请更新文档注释说明该豁免，并在此处加一行注释说明原因（base64 data 从不会被渲染为文本，转义只会在每次重新渲染时重复扫描兆字节级的负载）；将跳过范围收窄到 data 也能同时消除这一漂移隐患。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| if (invalidInlineImageCache.has(negativeKey)) { | ||
| invalidInlineImageCache.delete(negativeKey); | ||
| invalidInlineImageCache.add(negativeKey); |
There was a problem hiding this comment.
[Suggestion] R3-2: The LRU recency refresh on the negative-cache hit path (delete before add) is gated by no test — a test-efficacy mutant that deleted the delete statement survived with every affected test still green (the harness's positive control passed, so the suite can catch changes here). Set iteration order is insertion order and add on an existing key is a no-op, so the delete is what moves a hit key to the newest position; without it the cache silently degrades from LRU to FIFO — with more than 64 distinct invalid images, recently re-rendered payloads get evicted and re-decoded on every render, the exact CPU cost this PR exists to eliminate. Extend the negative-cache test: insert more than the limit of payloads, re-hit one early payload, insert one more, and assert the re-hit payload is served from the cache (no fresh Buffer.from) while the true oldest is evicted.
中文说明
R3-2：负缓存命中路径上的 LRU 新旧刷新（先 delete 再 add）没有任何测试守护——测试有效性探针删除该 delete 语句后，所有受影响的测试仍然全绿（harness 正向对照通过，说明测试套件有能力在此捕获变更）。由于 Set 的迭代顺序即插入顺序、对已存在键的 add 是空操作，正是这个 delete 把命中键移到最新位置；没有它，缓存会从 LRU 悄悄退化为 FIFO——当无效图片超过 64 个时，最近仍在重复渲染的负载会被淘汰、每次渲染都重新解码，这正是本 PR 要消除的 CPU 开销。请扩展负缓存测试：插入超过上限数量的负载、重新命中其中一个较早的负载、再插入一个新负载，然后断言被重新命中的负载仍由缓存提供服务（没有新的 Buffer.from），且真正被淘汰的是最旧的条目。
— qwen3.8-max via Qwen Code /review (v0.21.15)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- negative-cache test never exercises the hit path — already reported (comment 3743160742)
- ContinuationMarkdownMessage budget change pinned by no test — already reported (comment 3743160755)
- imageHeightBudget formula duplicated across the two assistant-message components — already reported (comments 3743160748, 3743160750)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): 38 tests passed — this review observed 23509 passed.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：38 tests passed — this review observed 23509 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const prefixWidth = getPrefixWidth(prefix); | ||
| const imageHeightBudget = | ||
| availableTerminalHeight !== undefined && images?.length |
There was a problem hiding this comment.
[Suggestion] R2-1: Still stands at this head (re-checked at 222e31f8): the image-only full-height-budget fix is applied to the two assistant-message components in this file, but not to the third copy of the same formula at ToolMessage.tsx:796-798 (shifted from 783-784 by the main merge), which still divides by (images.length + 1) and reserves a text share that an image-only tool result never uses. A tool row whose resultDisplay renders as type: 'none' skips the result block and renders its images alone, so nothing consumes the reserved share — an image-only tool result forfeits one height share and its images render shorter than an equivalent assistant message. Any future budget change must again be applied to three sites in lockstep; this PR itself had to write the identical edit twice inside this file. Extract one shared imageHeightBudget(availableHeight, imageCount, hasText) helper used by all three sites, or decide explicitly that tool results keep reserving the text share.
中文说明
R2-1：在当前提交仍然存在（已在 222e31f8 复查确认）：纯图片消息使用完整高度预算的修复应用于本文件中的两个 assistant 消息组件，但同一公式在 ToolMessage.tsx:796-798 的第三处副本未更新（因 main 合并，行号由 783-784 移动至此），仍除以 (images.length + 1)，为纯图片工具结果永远不会使用的文本预留了一份高度。当工具行的 resultDisplay 渲染为 type: 'none' 时会跳过结果块、只渲染图片，预留的份额无人消费——纯图片的工具结果因此损失一份高度，其图片渲染得比等效的 assistant 消息更矮。未来任何预算调整都必须再次同步修改三处；本 PR 自己就不得不在本文件中将同一编辑写两遍。建议抽取一个共享的 imageHeightBudget(availableHeight, imageCount, hasText) 辅助函数供三处调用，或明确决定工具结果继续预留文本份额。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| function cacheInvalidInlineImage(key: string): void { | ||
| invalidInlineImageCache.add(key); | ||
| while (invalidInlineImageCache.size > INLINE_DECODE_NEGATIVE_CACHE_LIMIT) { |
There was a problem hiding this comment.
[Suggestion] R2-2: Still stands at this head (re-checked at 222e31f8): cacheInvalidInlineImage adds the fourth hand-rolled bounded-cache eviction loop in this one file (renderCache, inlineDecodeCache above, transmittedKeys), and mermaidImageRenderer.ts holds two more; the copies have already drifted (markKittyImageWritten evicts with a single if and does not refresh recency on re-add). Any future fix or policy change to eviction must be applied to six sites independently, and readers must cross-check each copy to learn the actual policy. Extract a small shared evictOldest(entries, limit) helper colocated with these renderers, or consciously document the local convention.
中文说明
R2-2：在当前提交仍然存在（已在 222e31f8 复查确认）：cacheInvalidInlineImage 在本文件中新增了第四个手写有界缓存淘汰循环（renderCache、上方的 inlineDecodeCache、transmittedKeys），此外 mermaidImageRenderer.ts 中还有两个；这些副本已经出现差异（markKittyImageWritten 用单个 if 淘汰，且重新加入时不刷新新旧顺序）。未来任何针对淘汰逻辑的修复或策略变更都必须分别应用到六处，读者也必须逐一对照每个副本才能了解实际策略。建议抽取一个与这些渲染器放在一起的共享 evictOldest(entries, limit) 辅助函数，或有意识地记录这一本地约定。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const record = obj as Record<string, unknown>; | ||
| if ( | ||
| !Array.isArray(obj) && | ||
| typeof record['data'] === 'string' && |
There was a problem hiding this comment.
[Suggestion] R2-3: Still stands at this head (re-checked at 222e31f8): the image exemption remains a shape-based duck check that returns the entire object subtree untouched rather than exempting only the large data payload. Today this is coextensive with InlineImageData (verified again: exactly {data, mimeType}), but if that shape — or another {data, mimeType: 'image/…'} history shape — gains a rendered text field such as a caption, that field flows through HistoryItemDisplay's whole-item escapeAnsiCtrlCodes pass into <Text> unescaped, with no compiler or test signal. Session resume restores resultDisplay verbatim from persisted JSON, and sinks like StringResultRenderer render strings with no sanitization of their own — the recursion IS the protection. Probe at this head: escapeAnsiCtrlCodes({data:'', mimeType:'image/x', caption:'<ESC>[31mcaption<ESC>[0m'}) returned the same object reference with caption still raw; removing the exemption flipped the assertions. Exempt only the data payload (recurse into the object, passing data through unchanged), or restrict the exemption to objects whose own keys are exactly data and mimeType.
中文说明
R2-3：在当前提交仍然存在（已在 222e31f8 复查确认）：图片豁免仍然是一个基于形状的鸭子类型检查，会原样返回整个对象子树，而不是只豁免大体积的 data 负载。目前它与 InlineImageData 完全重合（再次复查确认恰好为 {data, mimeType}），但如果该形状——或其他 {data, mimeType: 'image/…'} 历史记录形状——新增了 caption 等需要渲染的文本字段，该字段将经由 HistoryItemDisplay 对整个条目的 escapeAnsiCtrlCodes 处理原样进入 <Text>，且没有任何编译器或测试信号。会话恢复会从持久化 JSON 原样恢复 resultDisplay，而 StringResultRenderer 等渲染端自身不做任何清洗——这层递归就是唯一的防护。已在本提交用探针验证：escapeAnsiCtrlCodes({data:'', mimeType:'image/x', caption:'<ESC>[31mcaption<ESC>[0m'}) 返回同一对象引用且 caption 保持原样；移除豁免后断言翻转。建议只豁免 data 负载（递归进入对象、仅对 data 原样透传），或将豁免限定为自身键恰好是 data 与 mimeType 的对象。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| record['mimeType'].toLowerCase().startsWith('image/') | ||
| ) { | ||
| return obj; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-1: Still stands at this head (re-checked at 222e31f8): the image-object exemption silently contradicts escapeAnsiCtrlCodes's doc comment ("escapes all ANSI control characters found in any string values") and carries no comment explaining why image objects are exempted. This function is the shared terminal-sanitization boundary, and its documented contract now misstates the behavior: a maintainer extending InlineImageData with a display-facing text field would trust the contract, but the whole-object return obj skip means that field reaches the terminal unescaped. Probe re-verified at this head: the caption in escapeAnsiCtrlCodes({data:'aGVsbG8=', mimeType:'image/png', caption:'<ESC>[31mcaption<ESC>[0m'}) comes back raw on the same object reference, and removing the exemption flips the assertions; the PR's own new test also still passes with the exemption removed, so no test gates the exemption's existence. Update the doc comment to state the exemption and add a one-line why-comment here (base64 data is never rendered as text, and escaping would rescan megabyte payloads on every re-render); narrowing the skip to data also closes the drift hazard.
中文说明
R3-1：在当前提交仍然存在（已在 222e31f8 复查确认）：该图片对象豁免与 escapeAnsiCtrlCodes 的文档注释（"转义所有字符串值中发现的全部 ANSI 控制字符"）悄悄矛盾，且没有任何注释说明为何豁免图片对象。该函数是共享的终端内容清洗边界，其文档约定现已与真实行为不符：未来维护者为 InlineImageData 添加面向显示的文本字段时会信任该约定，但整个对象 return obj 的跳过会让该字段未经转义直达终端。已在本提交再次用探针验证：escapeAnsiCtrlCodes({data:'aGVsbG8=', mimeType:'image/png', caption:'<ESC>[31mcaption<ESC>[0m'}) 中的 caption 在同一对象引用上保持原样；移除豁免后断言翻转。本 PR 新增的测试在移除豁免后同样通过，因此没有任何测试守护该豁免的存在。请更新文档注释说明该豁免，并在此处加一行注释说明原因（base64 data 从不会被渲染为文本，转义只会在每次重新渲染时重复扫描兆字节级的负载）；将跳过范围收窄到 data 也能同时消除这一漂移隐患。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (invalidInlineImageCache.has(negativeKey)) { | ||
| invalidInlineImageCache.delete(negativeKey); | ||
| invalidInlineImageCache.add(negativeKey); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-2: Still stands at this head (re-checked at 222e31f8): the LRU recency refresh on the negative-cache hit path (delete before add) is gated by no test. Mutant verification at this head: deleting the delete statement leaves all 31 renderer tests green (a recency probe fails expected 66 to be 65, proving what the suite cannot see), and deleting the entire hit branch also leaves them green — no test exercises the cache-hit path at all, because the only repeated payload in the eviction test is exactly the entry evicted by the 65th insert (that gap is the open comment on terminal-image-renderer.test.ts). Set iteration order is insertion order and add on an existing key is a no-op, so the delete is what moves a hit key to the newest position; without it the cache silently degrades from LRU to FIFO — with more than 64 distinct invalid images, recently re-rendered payloads get evicted and re-decoded on every render, the exact CPU cost this PR exists to eliminate. Extend the negative-cache test: insert more than the limit of payloads, re-hit one early payload, insert one more, and assert the re-hit payload is served from the cache (no fresh Buffer.from) while the true oldest is evicted.
中文说明
R3-2：在当前提交仍然存在（已在 222e31f8 复查确认）：负缓存命中路径上的 LRU 新旧刷新（先 delete 再 add）没有任何测试守护。已在本提交做变异验证：删除该 delete 语句后渲染器全部 31 项测试仍然全绿（新旧探针以 expected 66 to be 65 失败，证明测试套件看不到这一点）；删除整个命中分支后测试同样全绿——没有任何测试执行过缓存命中路径，因为淘汰测试中唯一重复提交的负载恰好是被第 65 次插入淘汰的那一条（该缺口即 terminal-image-renderer.test.ts 上的开放评论）。由于 Set 的迭代顺序即插入顺序、对已存在键的 add 是空操作，正是这个 delete 把命中键移到最新位置；没有它，缓存会从 LRU 悄悄退化为 FIFO——当无效图片超过 64 个时，最近仍在重复渲染的负载会被淘汰、每次渲染都重新解码，这正是本 PR 要消除的 CPU 开销。请扩展负缓存测试：插入超过上限数量的负载、重新命中其中一个较早的负载、再插入一个新负载，然后断言被重新命中的负载仍由缓存提供服务（没有新的 Buffer.from），且真正被淘汰的是最旧的条目。
— qwen3.8-max via Qwen Code /review (v0.22.0)
3 participants
What this PR does
images[].dataout of recursive ANSI control-code escaping while still sanitizing surrounding text.Why it’s needed
Issue #8608 reports repeated expensive work and incorrect height allocation when inline images are rendered repeatedly. Invalid multi-megabyte payloads could be decoded and validated again on every render, image payloads were unnecessarily traversed as text, and image-only messages reserved a text share that was never used.
Reviewer Test Plan
How to verify
Evidence
textUtils.test.ts: 38 tests passed.git diff --checkpassed.@opentelemetry/apiand AJV v8 dependency links.packages/core/distdeclarations before this check can run.Tested on
Environment
Risk & Scope
Linked Issues
Closes #8608
中文翻译
本 PR 做了什么
images[].data，但继续清理周围文本。为什么需要
Issue #8608 指出 inline 图片重复渲染会产生重复开销并错误分配高度。无效的大尺寸 payload 可能在每次渲染时重复解码和校验，图片 payload 也会被不必要地当作文本遍历；纯图片消息还会预留实际未使用的文本高度。
Reviewer Test Plan
如何验证
证据
textUtils.test.ts：38 项测试通过。git diff --check通过。@opentelemetry/api与 AJV v8 依赖链接。packages/core/dist声明文件。测试平台
环境
风险与范围
关联 Issue
Closes #8608