perf(cli): avoid hashing invalid inline image cache hits - #10626
perf(cli): avoid hashing invalid inline image cache hits#10626zjunothing wants to merge 2 commits into
Conversation
Verification reportResult: PASS ( Environment: macOS, Node.js 22.23.1. Baseline: Reproduction and result
Automated checks
There is no visual output change, so screenshots are not applicable. The full workspace build stopped at an unchanged SDK browser-bundle size gate (221,287 bytes versus 221,184 allowed); all three currently red CI jobs stop at this same gate during dependency installation, before their test steps. The adjacent component test is also blocked before collection by the existing 中文验证报告验证报告结果：PASS（ 环境：macOS，Node.js 22.23.1。基线： 复现与结果
自动化检查
本次没有可视输出变化，因此不适用截图。全 workspace build 在未改动的 SDK 浏览器 bundle 大小门禁处停止（221,287 字节，限制为 221,184 字节）；当前三项红色 CI job 也都在依赖安装阶段遇到同一门禁，尚未进入测试步骤。相邻组件测试还会在收集前被既有的 |
|
Thanks for the follow-up! Template looks good ✓ Problem: observed, not theoretical. I checked the code on Direction: aligned. #8608 asked for exactly this class of fix (avoiding repeated work on inline images), and this finishes the job on the invalid-payload path the merged PR already introduced. Size: not core infrastructure — both files live in Approach: the scope feels exactly right. Keying the negative cache by the raw payload string mirrors the positive decode cache in the same file, which is already keyed by raw string; shrinking the bound from 64 hashes to 8 strings compensates for the larger entries; and rejecting empty/oversized payloads before consulting either cache is an O(1) short-circuit that also keeps oversized payloads out of the cache entirely. No unrelated changes in the diff. Risk: no elevated risk signals — neither file matches the revert-correlated paths. Moving on to code review. 🔍 中文说明感谢这个 follow-up！ 模板完整 ✓ 问题：是已观测到的问题，不是理论性的。我核对了 方向：对齐。#8608 要求的正是这类修复（避免内联图片的重复开销），本 PR 在已合并 PR 引入的无效 payload 路径上完成了收尾。 规模：不涉及核心基础设施——两个文件都在 方案：范围恰当。负缓存改用原始 payload 字符串作 key，与同文件中已用原始字符串作 key 的正向解码缓存保持一致；上限从 64 个哈希降为 8 个字符串以匹配更大的条目；在查询任何缓存前拒绝空/超限 payload，是 O(1) 的短路，也让超限数据完全不进缓存。diff 中没有无关改动。 风险：无升级风险信号——两个文件均不命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — no blockers. I formed my own proposal from the base code before reading the diff (key the negative cache by the raw payload string like the positive cache already does, shrink the bound, reject empty/oversized payloads before consulting anything), and this PR lands exactly on it:
One non-blocking observation: the valid-image path still hashes the decoded buffer on every render in Testing — the PR's own CI, quoted from the API (unattended run; no PR code executed here): CI on the reviewed commit — all three red checks fail at the same pre-existing build gate, before any test executes:
The red checks are pre-existing infra noise, not this PR: all three die in the build step with The consequence, stated plainly: no CI test results exist for this commit. Every test lane was blocked by that gate. The "focused renderer suite passes 31/31" line above is the author's local claim from the PR description — attributed as such, not verified evidence. Not verified: unit/integration/E2E results on the PR code (reason: pre-existing build gate on Sandboxed verification would settle this: 中文说明代码审查——无阻塞问题。我在读 diff 前先基于 base 代码给出了自己的方案（负缓存改用原始 payload 字符串作 key、与正向缓存保持一致，收紧上限，在查询任何缓存前拒绝空/超限数据），本 PR 与该方案完全一致：
非阻塞观察：合法图片路径的 测试——引用 PR 自身 CI（无人值守运行，此处未执行任何 PR 代码）：三个红色检查全部在 build 步骤的既有门禁处失败（ 因此直说：本提交没有任何 CI 测试结果，所有测试通道都被该门禁挡住。描述中"定向测试 31/31 通过"是作者的本地声明，仅转述、不作为证据。未验证项：PR 代码的单测/集成/E2E 结果（原因： 可用沙箱验证收口： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — minimal, correct change with a test that pins the behavior; one point withheld because CI never produced a test run for this commit (blocked by the pre-existing Stepping back: this is what a good follow-up PR looks like. It finishes the job #8608 asked for and #8789 started, deletes more code than it adds (the hash, the single-use helper), and its test would fail against the pre-PR implementation — The honest reservation: my confidence is a static trace, not an observed green run. The paths are short and I verified every consumer of the changed surface, and the red CI is conclusively not this PR's doing (byte-identical SDK bundle-gate failure on the base commit, untouched package) — so none of it counts against the author. But it does mean merging waits on Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 改动最小且正确，测试能够钉住行为；扣一分是因为 CI 未能为本提交产出任何测试运行（被 整体来看：这是一个标准的高质量 follow-up。它完成了 #8608 提出、#8789 开始的收尾工作，删除的代码比新增的多（哈希计算、只用一次的辅助函数），且其测试在旧实现上必然失败—— 如实保留一点：我的信心来自静态走查，而非一次观察到的绿色运行。路径很短、改动面的所有消费方均已核查，红色 CI 确定与本 PR 无关（基线提交上字节级相同的 SDK bundle 门禁失败、未触及该包），因此这些都不应记在作者头上。但这意味着合并需要先修复 批准，并固定到所审查的提交。✅ — 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.
LGTM, looks ready to ship. ✅
E2E review report (no Critical found) — head
|
| Probe | Result |
|---|---|
| Valid 1×1 PNG, kitty env | ✅ rendered (kind=kitty, [image: 1x1 png]) |
| Invalid payload, first miss | ✅ fallback [image: png]; exactly 1 decode, 0 full-payload hashes |
| Same payload ×50 cached hits | ✅ 0 extra decodes, 0 full-payload hashes, median ≈ 2.7 µs/hit |
| LRU bound (limit 8): 9 inserts, re-hit evicted entry, re-hit resident entry | ✅ 9 decodes for 9 payloads; evicted entry decodes again (10); resident entry does not (stays 10) |
| Oversized payload (limit+1 chars) | ✅ rejected before decode and before hashing (0/0) |
tmux interactive-TUI smoke (bundled dist/cli.js, hermetic HOME + runtime dir, --yolo, real model qwen3.8-max): boots clean; a WriteFile → Read → Shell verification turn completes and renders correctly; /quit exits cleanly with a resumable session UUID and no orphaned processes.
Verdict
No Critical; the perf fix is real in the shipped bundle (cached invalid payloads no longer pay any full-payload hashing or decoding) and the LRU semantics are preserved. Not approving from this account — CI is red (pre-existing main gate, not this PR) and no bot/maintainer approval is on record; the 🧐 review lane was still running at the time of this check with no comments posted. Once main's bundle gate is green again this should re-CI cleanly.
— automated e2e pass by qqqys (probe + tmux on the built head); no code changes.
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 — Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x) and Integration Tests (CLI, No Sandbox) were skipped in CI, and the full packages/cli suite never reached its test phase locally because the pre-existing packages/sdk-typescript bundle-size gate failure stops the dependency build; the changed renderer suite itself was run directly and passed 30/31 with one pre-existing environmental failure (fake-chafa ESM shim).
Not explored to full depth (tool budget reached): "agent 3a": none — no checks were cut short..
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x) and Integration Tests (CLI, No Sandbox) were skipped in CI, and the full packages/cli suite never reached its test phase locally because the pre-existing packages/sdk-typescript bundle-size gate failure stops the dependency build; the changed renderer suite itself was run directly and passed 30/31 with one pre-existing environmental failure (fake-chafa ESM shim)。
未探索到全部深度（达到工具调用预算）："agent 3a"：none — no checks were cut short.。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| ).toEqual({ fallbackText: '[image: png]', result: null }); | ||
| expect(bufferFrom).not.toHaveBeenCalled(); | ||
| } finally { |
There was a problem hiding this comment.
[Suggestion] R1-1: The expect(bufferFrom).not.toHaveBeenCalled() assertion added here passes identically against the pre-change code — the old size guard inside decodeInlineImage already ran before any Buffer.from call — so it pins nothing about the actual behavioral change of this hunk: oversized payloads now return before the negative-cache lookup and are never hashed or negatively cached. Pre-change, getDecodedInlinePng computed crypto.createHash('sha256') over every payload — oversized ones included — before the size check ran, and negatively cached the result. If a future edit moves the length check back below the cache lookups, every test in this file stays green while every oversized call again hashes an up-to-~11 MB string and occupies one of the 8 negative-cache slots, evicting useful entries — silently resurrecting the exact cost this PR removes. Spy on createHash here (as the LRU test above already does) and assert it is not called.
Witness:
BASE (pre-change source + this PR's test incl. the bufferFrom assertion): "Tests 1 passed" — the mutation survives the shipped assertion
Pre-change source + proposed expect(createHash).not.toHaveBeenCalled(): "1 failed" at that assertion
PR source + proposed assertion: "1 passed"
The fix spans three spots (spy setup, assertion, restore):
it('rejects inline payloads above the shared image limit before decoding', () => {
const oversizedBase64 = 'A'.repeat(MAX_INLINE_IMAGE_ENCODED_LENGTH + 1);
const bufferFrom = vi.spyOn(Buffer, 'from');
const createHash = vi.spyOn(crypto, 'createHash');
try {
expect(
prepareInlineTerminalImage({
data: oversizedBase64,
mimeType: 'image/png',
contentWidth: 24,
env: { TERM: 'xterm-kitty' },
stdoutIsTTY: true,
}),
).toEqual({ fallbackText: '[image: png]', result: null });
expect(bufferFrom).not.toHaveBeenCalled();
expect(createHash).not.toHaveBeenCalled();
} finally {
bufferFrom.mockRestore();
createHash.mockRestore();
}
});Keep the assertion scoped to this oversized/invalid flow: crypto.createHash is still legitimately invoked on the successful-render path — createInlineRenderCacheKey (terminal-image-renderer.ts:334) and createImageId (:541) — which this test never reaches. Once the fix is in, removing it must turn rejects inline payloads above the shared image limit before decoding red — verify by moving the length check back into decodeInlineImage and confirming the new assertion fails while the existing bufferFrom one still passes.
中文说明
此处新增的 expect(bufferFrom).not.toHaveBeenCalled() 断言在改动前的代码上同样通过——旧的大小检查位于 decodeInlineImage 内部、任何 Buffer.from 调用之前——因此它并没有钉住本处真正的行为变化：超限 payload 现在会在查询负缓存之前直接返回，既不会被哈希，也不会进入负缓存。改动前，getDecodedInlinePng 会在大小检查运行前对每个 payload（包括超限的）计算 crypto.createHash('sha256')，并将哈希结果存入负缓存。如果未来的修改把长度检查移回缓存查询之后，本文件所有测试仍会全绿，而每次超限调用又会重新哈希一个最长约 11 MB 的字符串并占用 8 个负缓存槽位之一、挤掉有用条目——悄悄复活本 PR 要消除的开销。请在此处对 createHash 打桩（上方 LRU 测试已有同样写法）并断言它未被调用。
探针证据（临时树中四组对照）：改动前源码 + 本 PR 测试（含 bufferFrom 断言）："Tests 1 passed"——该 mutation 在现有断言下存活；改动前源码 + 建议新增的 expect(createHash).not.toHaveBeenCalled()："1 failed"，正失败在该断言上；本 PR 源码 + 建议断言："1 passed"。建议的断言在本次改动前后发生红/绿翻转，而现有断言不翻转。
修复涉及三处（打桩、断言、恢复），完整代码见上方英文部分的代码块。
约束：该断言须保持在超限/无效流程内——crypto.createHash 在渲染成功路径上仍被正常使用（createInlineRenderCacheKey，terminal-image-renderer.ts:334；createImageId，:541），本测试不会走到这些路径。修复落地后，若将其移除，rejects inline payloads above the shared image limit before decoding 必须变红——可将长度检查移回 decodeInlineImage 验证：新断言失败，而现有 bufferFrom 断言仍通过。
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- The oversized test's unpinned guard-ordering assertion (agent 5, probe-confirmed this round) — same defect as the still-standing R1-1, merged into its re-post — already reported (comment 3894525255)
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/utils/terminal-image-renderer.ts:393 — [probe] byte-bound the raw-keyed negative cache (~89 MB worst-case retention)packages/cli/src/ui/utils/terminal-image-renderer.ts:54 — [review] document the coupling between the 8-entry limit and raw-key size
中文说明
已审查。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| expect(bufferFrom).not.toHaveBeenCalled(); | ||
| } finally { |
There was a problem hiding this comment.
[Suggestion] R1-1: Still stands — re-reported from round 1 (the branch code is unchanged since; only a merge from main landed). The expect(bufferFrom).not.toHaveBeenCalled() assertion added here passes identically against the pre-change code — the old size guard inside decodeInlineImage already ran before any Buffer.from call — so it pins nothing about the actual behavioral change of this hunk: oversized payloads now return before the negative-cache lookup and are never hashed or negatively cached. If a future edit moves the length check back below the cache lookups, every test in this file stays green while every oversized call again hashes an up-to-~11 MB string and occupies one of the 8 negative-cache slots, evicting useful entries — silently resurrecting the exact cost this PR removes. This round's probe re-confirmed the mechanism at the reviewed commit: moving the guard back into decodeInlineImage keeps the full suite green while the oversized payload again flows through both cache lookups and evicts a resident entry.
Witness:
MUTANT (guard moved back below the cache lookups): Tests 31 passed (31)
PROBE-GUARD-ORDER callsAfterFill=8 callsAfterOversized=8 callsAfterResubmit=9 junk0-redecoded=true
INTACT (PR code): PROBE-GUARD-ORDER callsAfterResubmit=8 junk0-redecoded=false
Suggested fix (unchanged from round 1): spy on createHash here (as the LRU test above already does) and assert it is not called — that assertion flips red/green across the change, unlike the bufferFrom one:
it('rejects inline payloads above the shared image limit before decoding', () => {
const oversizedBase64 = 'A'.repeat(MAX_INLINE_IMAGE_ENCODED_LENGTH + 1);
const bufferFrom = vi.spyOn(Buffer, 'from');
const createHash = vi.spyOn(crypto, 'createHash');
try {
expect(
prepareInlineTerminalImage({
data: oversizedBase64,
mimeType: 'image/png',
contentWidth: 24,
env: { TERM: 'xterm-kitty' },
stdoutIsTTY: true,
}),
).toEqual({ fallbackText: '[image: png]', result: null });
expect(bufferFrom).not.toHaveBeenCalled();
expect(createHash).not.toHaveBeenCalled();
} finally {
bufferFrom.mockRestore();
createHash.mockRestore();
}
});Keep the assertion scoped to this oversized/invalid flow — crypto.createHash is still legitimately invoked on the successful-render path by createInlineRenderCacheKey (terminal-image-renderer.ts:334) and createImageId (:541), which this test never reaches. Once the fix is in, removing it must turn this test red: move the length check back into decodeInlineImage and confirm the new createHash assertion fails while the existing bufferFrom one still passes.
中文说明
（仍然成立 — 自第 1 轮起重新报告；此后分支代码未变，仅合并了 main。）此处新增的 expect(bufferFrom).not.toHaveBeenCalled() 断言在改动前的代码上同样通过——旧的大小检查位于 decodeInlineImage 内部、任何 Buffer.from 调用之前——因此它并没有钉住本处真正的行为变化：超限 payload 现在会在查询负缓存之前直接返回，既不会被哈希，也不会进入负缓存。如果未来的修改把长度检查移回缓存查询之后，本文件所有测试仍会全绿，而每次超限调用又会重新哈希一个最长约 11 MB 的字符串并占用 8 个负缓存槽位之一，挤掉有用条目——悄悄复活本 PR 要消除的开销。本轮探针在受审提交上复证了该机制：把长度检查移回 decodeInlineImage 后，整个测试套件仍然全绿，而超限 payload 又会流经两处缓存查询并挤掉驻留条目。
探针证据：MUTANT（长度检查移回缓存查询之后）：Tests 31 passed (31)，callsAfterResubmit=9 junk0-redecoded=true；INTACT（本 PR 代码）：callsAfterResubmit=8 junk0-redecoded=false。
建议的修复（与第 1 轮相同）：在此处对 createHash 打桩（上方 LRU 测试已有同样写法）并断言它未被调用——该断言会在改动前后红/绿翻转，而现有 bufferFrom 断言不会。完整代码见上方英文部分的代码块。
约束：该断言须保持在超限/无效流程内——crypto.createHash 在渲染成功路径上仍被 createInlineRenderCacheKey（terminal-image-renderer.ts:334）和 createImageId（:541）正常使用，本测试不会走到这些路径。修复落地后，若将其移除，本测试必须变红：可将长度检查移回 decodeInlineImage 验证——新的 createHash 断言失败，而现有 bufferFrom 断言仍通过。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Local verification — the headline claim reproduces; one trade-off worth an explicit decisionVerdict: the perf win is real and larger than claimed on this machine. No correctness defect found. But the EnvironmentmacOS (darwin 25.6.0), Node v24.18.1. 1. The author's claim reproduces — and is understated here
One 8 MiB Base64-valid non-PNG payload, 26 calls:
Also confirmed for the oversized path, which the PR body does not quantify: on 2. New finding — the
|
| N | main per-pass |
PR per-pass | main decodes |
PR decodes |
|---|---|---|---|---|
| 1 | 0.62 ms | 0.00 ms | 0 | 0 |
| 4 | 2.55 ms | 0.01 ms | 0 | 0 |
| 8 | 5.14 ms | 0.01 ms | 0 | 0 |
| 9 | 5.73 ms | 66.32 ms | 0 | 54 |
| 12 | 7.57 ms | 87.20 ms | 0 | 72 |
| 16 | 10.69 ms | 117.56 ms | 0 | 96 |
| 32 | 20.11 ms | 232.70 ms | 0 | 192 |
| 64 | 40.46 ms | 465.37 ms | 0 | 384 |
| 65 | 510.72 ms | 473.40 ms | 390 | 390 |
| 80 | 635.19 ms | 580.31 ms | 480 | 480 |
For 9 ≤ N ≤ 64 this PR is ~11.5× slower per image than main, because main still hits its 64-entry cache (paying one hash) while this PR misses and pays a full replace(/\s/g,'') + Buffer.from + toString('base64') round-trip. At N ≥ 65 both thrash and the PR is marginally ahead again.
3. Is N ≥ 9 reachable? Yes — measured through the real component
Driving the actual <TerminalImage> React component with the actual renderer (ink-testing-library), changing contentWidth 80 → 72 — i.e. one terminal resize, which is exactly what the comment at the top of this file describes ("A terminal resize or a restored session re-renders every visible image"):
| images on screen | main resize |
PR resize | main decodes |
PR decodes |
|---|---|---|---|---|
| 4 (256 KiB each) | 7.27 ms | 5.36 ms | 0 | 0 |
| 8 | 7.71 ms | 5.85 ms | 0 | 0 |
| 9 | 7.38 ms | 12.30 ms | 0 | 9 |
| 12 | 8.77 ms | 14.17 ms | 0 | 12 |
| 16 | 10.50 ms | 16.86 ms | 0 | 16 |
| 8 (2 MiB each) | 18.09 ms | 6.32 ms | 0 | 0 |
| 12 (2 MiB each) | 21.69 ms | 64.63 ms | 0 | 12 |
The useMemo in InlineTerminalImage is keyed on contentWidth, so every visible image is re-prepared on a resize. MAX_INLINE_IMAGES_PER_ITEM = 4, so three tool results carrying invalid images already exceed the new bound of 8, and there is no cap on how many such history items a session accumulates. At 2 MiB payloads that is +43 ms on every resize at N=12, against −12 ms at N=8.
I am not claiming this is common — a session with ≥9 distinct invalid inline images is a corner. But so is the ≤8 case the PR optimises, and the bound change is what decides which corner you land in.
4. Retention — the disclosed trade-off, measured
Worth flagging how it must be measured: Buffer.toString('base64') at these sizes yields an external string, so it does not appear in process.memoryUsage().heapUsed or in any v8.getHeapSpaceStatistics() space at all — a heap-based measurement reads 0 MB and is simply wrong. Using steady-state RSS over 40 distinct 8 MiB invalid payloads (caller references dropped each iteration, 3 forced GCs), 3 runs each:
main |
this PR | |
|---|---|---|
| steady-state RSS growth | 171.4 / 190.1 / 171.5 MB | 235.3 / 235.4 / 235.4 MB |
≈ +45–64 MB for the PR, consistent with retaining 8 × ~10.7 MiB of raw payload minus allocator noise, and with the ~85.3 M-char bound the PR discloses. One mitigating fact the description does not mention: those strings are usually the same objects the conversation history already holds, so the marginal retention only materialises after the owning history item is dropped.
5. Test strength
- PR code + PR tests: 31 passed.
- Counterfactual (
maincode + PR tests): 1 failed / 30 passed. The single failing assertion isexpect(createHash).not.toHaveBeenCalled()(68 calls onmain) — that one assertion carries the whole change. - Mutation matrix (6 mutants, PR tests):
| mutant | result |
|---|---|
| N1 drop the hit-path recency refresh | killed |
| N2 move the empty/oversize guard back below the cache lookups | SURVIVED |
| N3 never evict (unbounded negative cache) | killed |
| N4 never populate the negative cache | killed |
| N5 key the negative cache by a 64-char prefix | SURVIVED |
| N6 restore the sha256 negative key (undo the perf change) | killed |
N2 confirms the bot's standing R1-1 with a mutation. The hunk that hoists the empty/oversize guard above the cache lookups can be reverted and the whole suite stays green, because expect(bufferFrom).not.toHaveBeenCalled() in the oversize test also passes on main (the old size check inside decodeInlineImage already ran before any Buffer.from). A createHash spy in that test — the same assertion that makes the LRU test load-bearing — closes it, and section 1 shows exactly what it would pin (11,184,816 chars hashed per call).
N5 is lower-stakes but shows the suite never pins the key identity: a 64-char-prefix key (which would collide across distinct payloads sharing a prefix) passes unchanged.
Suggestion for the bound
The file already has the pattern for this: renderCache is bounded by bytes (RENDER_CACHE_BYTE_LIMIT = 32 MiB), not by count, precisely because its entries vary hugely in size. Now that the negative cache holds raw payload strings instead of fixed 64-byte digests, the same treatment fits it better than any single count:
- a byte budget keeps the worst-case retention explicit (the thing the PR flags as its main risk), and
- it lets the small-payload case keep many more entries than 8, which is what removes the N ≥ 9 cliff — a 32 MiB budget holds ~90 payloads of 256 KiB but only 3 at the 10.7 MiB cap.
If a count is preferred for simplicity, keeping it at 64 costs nothing in the ≤8 case measured above (that case never evicts) and preserves main's cliff position; only the retention bound moves. Either way this is a design call for the maintainer, not a defect in the diff.
Not covered
- macOS only; no Linux/Windows run. Nothing in the change is platform-specific.
- I drove the renderer module and the
<TerminalImage>component directly. I did not drive a full TUI session end-to-end with a model producing invalid inline images, so "≥9 distinct invalid images in one session" is argued fromMAX_INLINE_IMAGES_PER_ITEM = 4plus unbounded history items, not observed in a live session. - Absolute timings are one machine; the ratios and the decode/hash counts are the load-bearing numbers.
中文说明
本地验证 —— 主张成立；但有一个取舍值得先明确决定
结论：性能收益真实，在本机上甚至比声称的更大。未发现正确性缺陷。但 64 → 8 这个上限把缓存悬崖从「65 张不同的无效图」拉到了「9 张」，越过悬崖后本 PR 每张图比 main 慢约 11 倍，而且这一区间通过真实组件是可达的。我建议在合并前把这个取舍定下来；改成按字节限额（本文件里已有先例）可以两头都拿到。
环境
macOS（darwin 25.6.0），Node v24.18.1。main@61697df9b0 对比 PR head 141e2fbe47，两棵独立 worktree，均完整构建（npm run build）。测量针对真实的 renderer 模块，用 esbuild 直接从 TypeScript 源打包（真实的 MAX_TERMINAL_IMAGE_BYTES 和 MAX_INLINE_IMAGE_ENCODED_LENGTH），并在模块加载前对 crypto.createHash 与 Buffer.from(…, 'base64') 插桩。
1. 作者的主张复现 —— 而且这里比声称的还要好
一个 8 MiB、Base64 合法但非 PNG 的 payload，26 次调用：
main |
本 PR | |
|---|---|---|
| 缓存命中中位耗时 | 5.0990 ms | 0.0015 ms |
| 完整 payload 的 SHA-256 update 次数 | 26 | 0 |
| 26 次调用累计哈希字节数 | 290,805,112 | 0 |
| base64 解码次数 | 1 | 1 |
另外还确认了 PR 正文未量化的超限路径：在 main 上，一个超限 payload 每次调用都要对 11,184,816 个字符做一次完整 SHA-256（5.137 ms）；本 PR 则 0.7 µs 返回、零哈希。这是实实在在的第二项收益。
2. 新发现 —— 64 → 8 把抖动悬崖从 N=65 挪到了 N=9
负缓存是按插入顺序淘汰的 LRU，因此一轮渲染若触及 N 个不同的无效 payload 且 N > 上限，就会每一项都未命中（典型的顺序扫描抖动）。main 的 64 让这个悬崖离得很远；8 把它拉进了可达范围。
N 个不同的 1 MiB 无效 payload，6 轮重渲染，每个 N 用全新进程：
| N | main 每轮 |
本 PR 每轮 | main 解码 |
PR 解码 |
|---|---|---|---|---|
| 1 | 0.62 ms | 0.00 ms | 0 | 0 |
| 4 | 2.55 ms | 0.01 ms | 0 | 0 |
| 8 | 5.14 ms | 0.01 ms | 0 | 0 |
| 9 | 5.73 ms | 66.32 ms | 0 | 54 |
| 12 | 7.57 ms | 87.20 ms | 0 | 72 |
| 16 | 10.69 ms | 117.56 ms | 0 | 96 |
| 32 | 20.11 ms | 232.70 ms | 0 | 192 |
| 64 | 40.46 ms | 465.37 ms | 0 | 384 |
| 65 | 510.72 ms | 473.40 ms | 390 | 390 |
| 80 | 635.19 ms | 580.31 ms | 480 | 480 |
在 9 ≤ N ≤ 64 区间，本 PR 每张图比 main 慢约 11.5 倍：main 仍命中它 64 项的缓存（只付一次哈希），而本 PR 未命中，要付完整的 replace(/\s/g,'') + Buffer.from + toString('base64') 往返。N ≥ 65 时两边都抖动，本 PR 略微领先。
3. N ≥ 9 可达吗？可达 —— 通过真实组件实测
用 ink-testing-library 驱动真实的 <TerminalImage> React 组件与真实 renderer，把 contentWidth 从 80 改到 72 —— 也就是一次终端 resize，正是本文件顶部注释所描述的场景（"终端 resize 或恢复会话会重新渲染每一张可见图片"）：
| 屏上图片数 | main resize |
本 PR resize | main 解码 |
PR 解码 |
|---|---|---|---|---|
| 4（各 256 KiB） | 7.27 ms | 5.36 ms | 0 | 0 |
| 8 | 7.71 ms | 5.85 ms | 0 | 0 |
| 9 | 7.38 ms | 12.30 ms | 0 | 9 |
| 12 | 8.77 ms | 14.17 ms | 0 | 12 |
| 16 | 10.50 ms | 16.86 ms | 0 | 16 |
| 8（各 2 MiB） | 18.09 ms | 6.32 ms | 0 | 0 |
| 12（各 2 MiB） | 21.69 ms | 64.63 ms | 0 | 12 |
InlineTerminalImage 的 useMemo 依赖 contentWidth，所以一次 resize 会让每张可见图片重新走一遍 prepare。MAX_INLINE_IMAGES_PER_ITEM = 4，因此三条携带无效图片的工具结果就已经超过新的上限 8，而会话能累积多少这样的历史项并没有上限。在 2 MiB payload 下，N=12 时每次 resize 多花 43 ms，而 N=8 时省下 12 ms。
我并不主张这很常见 —— 一个会话里出现 ≥9 张不同的无效内联图片属于边角情形。但本 PR 优化的 ≤8 情形同样是边角，而这次上限改动决定了你落在哪个边角里。
4. 驻留内存 —— 已披露的取舍，实测
先说测量方法上的坑：这种尺寸下 Buffer.toString('base64') 产生的是外部字符串，它既不出现在 process.memoryUsage().heapUsed，也不出现在 v8.getHeapSpaceStatistics() 的任何一个 space 里 —— 基于堆的测量会读到 0 MB，纯属错误。改用稳态 RSS：40 个不同的 8 MiB 无效 payload（每轮丢弃调用方引用，3 次强制 GC），每臂 3 次运行：
main |
本 PR | |
|---|---|---|
| 稳态 RSS 增长 | 171.4 / 190.1 / 171.5 MB | 235.3 / 235.4 / 235.4 MB |
本 PR 约多 45–64 MB，与「驻留 8 × ~10.7 MiB 原始 payload 再减去分配器噪声」吻合，也与 PR 自己披露的约 8530 万字符上限吻合。有一点描述里没提，对本 PR 有利：这些字符串通常与会话历史持有的是同一批对象，所以边际驻留只有在对应历史项被丢弃之后才真正产生。
5. 测试强度
- PR 代码 + PR 测试：31 passed。
- 反事实（
main代码 + PR 测试）：1 failed / 30 passed。唯一失败的断言是expect(createHash).not.toHaveBeenCalled()（在main上被调用 68 次）—— 整个改动靠这一条断言承重。 - 变异矩阵（6 个变异体，跑 PR 测试）：
| 变异体 | 结果 |
|---|---|
| N1 删掉命中路径的新近度刷新 | 杀死 |
| N2 把空值/超限守卫移回缓存查询之下 | 存活 |
| N3 永不淘汰（负缓存无界） | 杀死 |
| N4 从不写入负缓存 | 杀死 |
| N5 用 64 字符前缀作为负缓存 key | 存活 |
| N6 恢复 sha256 负缓存 key（撤销性能改动） | 杀死 |
N2 用变异实证了 bot 那条未决的 R1-1。 把空值/超限守卫提升到缓存查询之上的那个 hunk 可以整个还原，而整套测试依然全绿 —— 因为超限测试里的 expect(bufferFrom).not.toHaveBeenCalled() 在 main 上同样通过（旧的长度检查在 decodeInlineImage 内部，本来就跑在任何 Buffer.from 之前）。在那个测试里加一个 createHash spy（和让 LRU 测试承重的是同一类断言）即可闭合，而第 1 节正好给出了它会钉住什么（每次调用哈希 11,184,816 个字符）。
N5 影响较小，但说明测试从未钉住 key 的身份：换成 64 字符前缀 key（不同 payload 若共享前缀就会碰撞）依旧全绿。
关于上限的建议
本文件里已有现成的做法：renderCache 是按字节限额的（RENDER_CACHE_BYTE_LIMIT = 32 MiB）而不是按条数，正是因为它的条目大小差异极大。既然负缓存现在存的是原始 payload 字符串而不再是固定 64 字节的摘要，同样的处理比任何单一条数都更合适：
- 字节预算把最坏情况的驻留量写死（这正是 PR 自己标记的主要风险），并且
- 它让小 payload 的场景能保留远多于 8 条的条目，从而消除 N ≥ 9 的悬崖 —— 32 MiB 预算能装约 90 个 256 KiB 的 payload，但在 10.7 MiB 上限时只能装 3 个。
如果为了简单更倾向于用条数，那么维持 64 在上文实测的 ≤8 场景里没有任何代价（该场景根本不会触发淘汰），同时保住 main 的悬崖位置；变化的只有驻留上限。无论哪种，这都是维护者的设计决策，而不是 diff 里的缺陷。
未覆盖
- 仅 macOS；未跑 Linux/Windows。改动本身没有平台相关内容。
- 我直接驱动的是 renderer 模块和
<TerminalImage>组件，没有端到端驱动一个由模型产出无效内联图片的完整 TUI 会话；因此"一个会话里 ≥9 张不同的无效图片"是由MAX_INLINE_IMAGES_PER_ITEM = 4加上历史项无上限推导出来的，不是在真实会话里观察到的。 - 绝对耗时来自单台机器；真正承重的是比值以及解码/哈希次数。
4 participants
What this PR does
This follow-up stores invalid inline-image payloads directly in a small negative LRU cache, rejects empty or oversized payloads before consulting either decode cache, and adds regression coverage that distinguishes resident hits, true eviction, and hit-based recency refresh.
Why it's needed
PR #8789 avoided repeated Base64 decoding for invalid inline images, but its negative-cache lookup still calculated SHA-256 over the entire payload on every render. On current
main, an 8 MiB invalid payload took a median 2.758 ms per cached hit and performed 26 full-payload hash updates across 26 calls. This follow-up removes that repeated linear work while keeping invalid-payload retention explicitly bounded.Reviewer Test Plan
How to verify
The focused renderer suite passes 31/31. A mutation check that removes the hit-path recency refresh makes the LRU test fail, confirming that the assertions observe the intended behavior rather than only the total insertion count.
Evidence (Before & After)
There is no UI output change. An independent probe using the same 8 MiB invalid payload measured the following:
mainTested on
Environment (optional)
Node.js 22.23.1 on macOS. The focused renderer suite, CLI build, CLI typecheck, full workspace and integration typecheck, ESLint, Prettier, and diff whitespace checks passed.
Risk & Scope
cli-boxesimport-attribute parser error; the changed renderer suite and CLI build pass.Linked Issues
Follow-up to #8789 for #8608.
中文说明
这个 PR 做了什么
这个 follow-up 将无效的内联图片 payload 直接存入一个较小的负向 LRU 缓存，在查询任一解码缓存前拒绝空数据或超限数据，并补充了能够区分驻留命中、真实淘汰和命中后新近度刷新的回归测试。
为什么需要这个 PR
PR #8789 避免了无效内联图片被重复执行 Base64 解码，但它的负缓存查询仍会在每次渲染时对整个 payload 计算 SHA-256。在当前
main上，一个 8 MiB 的无效 payload 每次缓存命中的中位耗时为 2.758 ms，26 次调用共执行 26 次完整 payload 哈希更新。这个 follow-up 去掉了这段重复的线性开销，同时继续明确限制无效 payload 的驻留数量。Reviewer 测试计划
如何验证
renderer 定向测试 31/31 通过。删除命中路径中新近度刷新的 mutation 检查会使 LRU 测试失败，证明这些断言确实观察到了目标行为，而不只是统计总插入次数。
证据（Before & After）
本次没有 UI 输出变化。独立探针使用同一个 8 MiB 无效 payload 得到以下结果：
main测试平台
环境（可选）
macOS，Node.js 22.23.1。renderer 定向测试、CLI build、CLI typecheck、全 workspace 与 integration typecheck、ESLint、Prettier 和 diff 空白检查均通过。
风险与范围
cli-boxesimport attribute 解析错误阻塞，但本次改动的 renderer 测试和 CLI build 均通过。关联 Issue
这是 #8789 针对 #8608 的 follow-up。