feat(core): add full-resolution image zoom tool - #7809
Conversation
|
|
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
PR Review: Add full-resolution image zoom tool
Summary
This PR adds a zoom_image tool allowing image-capable models to request magnified crops from full-resolution images. The implementation is solid and well-integrated with existing permission and scheduling infrastructure.
✅ Strengths
1. Excellent Security Considerations
- Path validation enforces absolute paths only
- Permission boundary correctly reuses
read_filelogic via the new sharedfile-read-permission.tsmodule - File size limits (100MB source, 9MB output) prevent memory exhaustion
limitInputPixels: trueprevents decompression bombs- Animated images are explicitly rejected
- Tool is only registered for image-capable models (conditional registration)
2. Comprehensive Test Coverage
- 480+ lines of tests covering success cases, error handling, edge cases
- Tests for: missing files, directories, non-regular files, unsupported formats, oversized files, EXIF orientation, transparency flattening, budget constraints, animated images, abort handling, validation errors, and permissions
3. Clean Architecture
- Good separation with shared
file-read-permission.tsextracted fromread-file.ts - Proper integration with existing systems: permissions, loop detection, tool scheduling, AUTO mode allowlist
- Well-documented tool schema and description
4. Budget Constraints
- Visual patch budget (1568 patches of 28px) aligns with model input limits
- Edge limit (1568px) and output byte limit (9MB) are reasonable
🔍 Issues to Address
1. Copyright Year Typo (Minor)
- * Copyright 2026 Qwen
+ * Copyright 2025 QwenThe PR was created in 2025-07, but the new files have 2026.
2. Sharp as Direct Dependency
Adding sharp (a native module) increases:
- Install time and complexity
- Bundle size
- Platform-specific considerations
Considerations:
- Is lazy loading sufficient to mitigate impact on text-only configurations?
- Should this be documented in a CHANGELOG or migration guide?
3. Platform Testing Gap
Author notes Windows was not locally tested. Given sharp's native dependencies, consider:
- Adding CI checks for Windows if not present
- Or documenting this limitation
💡 Suggestions (Non-blocking)
1. Binary Search Edge Case
The magnifiedSize function initializes best = { width: 1, height: 1 }. While this works, consider adding a comment explaining that the minimum valid size will always be found.
2. Error Handling Scope
The catch block in execute() is broad:
} catch {
signal.throwIfAborted();
return failureResult(...);
}This catches all errors. Consider checking if the caught error is an AbortError first to propagate it correctly.
3. Race Condition Documentation
Between fs.stat and image processing, the file could theoretically change. This is acceptable for a read-only tool, but could be documented.
📋 Integration Checklist ✅
- Added to
ToolNamesandToolDisplayNames - Registered in config with conditional loading for image-capable models
- Added to
SAFE_TOOL_ALLOWLISTfor AUTO mode - Added to
FS_PATH_TOOL_NAMESfor path extraction - Added to
READ_TOOLSmeta-category for permission rules - Added to
READ_LIKE_TOOLSfor loop detection - Added to
CORE_TOOLSset - Added to
TOOL_NAME_ALIASESwithZoomImageandZoomImageTool - Proper permission handling via shared
file-read-permission.ts
Conclusion
Approve with minor suggestions. The implementation is production-ready with excellent security practices and test coverage. The copyright year and Windows testing are the only actionable items before merge.
Great work on this feature! 🎉
|
Thanks for the PR! Template looks good ✓ Problem: this is a feature addition, not a bug fix — and the problem is real. Large images are downscaled before reaching the model, and small text, chart labels, and thin lines are routinely lost. No linked issue, but the gap is well-established and observable any time a high-resolution screenshot or diagram is read. Direction: aligned. A targeted crop from the original source is a natural extension of the existing image-read path. No direct CHANGELOG reference found, but the area is clearly relevant to a coding assistant that handles images. Size: 466 production logic lines (additions + deletions, excluding tests), 561 test lines. Core paths are touched ( Approach: the scope feels right. The new Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个功能新增，而非 bug 修复——问题是真实存在的。大图片在发送给模型前会被缩小，小字、图表标签和细线经常丢失。没有关联 issue，但这个差距是公认的，任何读取高分辨率截图或图表时都会遇到。 方向：对齐。从原始文件做定向裁剪是现有图片读取能力的自然延伸。CHANGELOG 中没有直接参考，但该领域与处理图片的编码助手明显相关。 规模：466 行生产逻辑代码（增删合计，不含测试），561 行测试代码。触及核心路径（ 方案：范围合理。新的 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I would add a Comparison: the PR matches this proposal closely. The implementation is clean and follows existing tool patterns faithfully. No critical blockers found. Specific observations:
Test EvidenceThis is an unattended CI run — PR code is not executed locally. Evidence comes from the PR's own CI checks on commit Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The main unit test suite ( Not verified: live TUI behavior (CI run — a maintainer can trigger the isolated 中文说明代码审查独立方案： 我会在 对比： PR 与该方案高度一致。实现干净，忠实遵循现有工具模式。 未发现关键阻塞问题。具体观察：
测试证据这是无人值守的 CI 运行——不在本地执行 PR 代码。证据来自 PR 自身的 CI 检查。主单元测试套件仍在运行中，目前无失败。作者报告本地 1,324 个测试通过——这是作者的声明，非独立验证。 未验证：实时 TUI 行为（CI 运行）。维护者可以在一次性容器中检出 PR 或手动复现行为声明。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean implementation that follows existing tool patterns faithfully, with thorough test coverage and correct integration across all touch points. The Stepping back: this is a well-scoped feature that solves a genuine problem. The code reads like it was written by someone who studied the existing tool architecture and followed it precisely — conditional registration, deferred discovery, shared permission boundary, typed error results, budget-bounded output. The The only reservation is the Approval deferred until CI lands green on 中文说明置信度：4/5 — 实现干净，忠实遵循现有工具模式，测试覆盖全面，所有集成点正确。 总体来看：这是一个范围合理的功能，解决了真实问题。代码读起来像是作者研究了现有工具架构后精确遵循的——条件注册、延迟发现、共享权限边界、类型化错误结果、有预算约束的输出。 唯一的保留意见是 审批推迟到 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
已修复。\n\n验证证据：\n- |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
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.7-max via Qwen Code /review
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] packages/core/package.json:83 — sharp is a native module with platform-specific binary addons (@img/sharp-{platform}), but it is neither added to esbuild's external list in esbuild.config.js nor to optionalDependencies in scripts/prepare-package.js. The established pattern for native modules externalizes them in esbuild and declares their platform binaries as optional dependencies. Failure scenario: In the published package, zoom_image crashes — the tool is non-functional for every user. (Already reported at packages/core/package.json:83; blocker re-checked — still stands.)
— qwen3.7-max via Qwen Code /review
Review —
|
| Place | Contents today | sharp? |
|---|---|---|
esbuild.config.js:145 external |
node-pty, @teddyzhu/clipboard, audio-capture |
❌ |
scripts/prepare-package.js:305-321 (published package.json) |
dependencies: {} + explicit optionalDependencies allowlist |
❌ |
scripts/build-standalone-release.js:162 |
@teddyzhu/clipboard |
❌ |
scripts/create-standalone-package.js:52-56, 401 |
per-target native package map | ❌ |
Because it is not external, esbuild inlines sharp's JS into the bundle (I measured 233 KB, no warnings) while the native addon stays a runtime require('@img/sharp-<platform>/sharp.node') resolved relative to the emitted bundle. Since dist/'s package.json declares neither sharp nor @img/*, that require has nothing to find. Reproduced with this repo's exact bundle settings (format: esm, packages: 'bundle', inject: scripts/esbuild-shims.js) emitted into a directory with no @img present:
Error: Could not load the "sharp" module using the darwin-arm64 runtime
Possible solutions:
- Ensure optional dependencies can be installed: npm install --include=optional sharp
...
The blast radius is larger than "the tool is unavailable", because the sharp import is static at zoom-image.ts:9 and the factory is warmed at startup:
Config.initialize() → warmAll({ strict: options?.lenientToolWarmup !== true }) (config.ts:2765) → ensureTool (tool-registry.ts:352, rethrows) → Promise.allSettled rejected + strict → throw (tool-registry.ts:393-405) → initialize() throws → the CLI does not start for any image-capable model. The factory in config.ts:7157 has no try/catch either.
Why local validation passed: sharp is already hoisted into the root node_modules as a transitive dep of packages/mobile-mcp → mobilewright → @mobilewright/core, so an in-repo node dist/cli.js resolves it fine. packages/desktop is not evidence either way — it is excluded from the root workspace ("!packages/desktop"), is a separate private Electron app, and only lists sharp in trustedDependencies. And scripts/verify-installation-release.js only checks artifact names + SHA256, it never executes the binary, so CI will not catch this.
Suggested fix, matching the existing native-module pattern (clipboardUtils.ts is the model):
sharp→optionalDependenciesinpackages/core/package.json, plusesbuild.config.jsexternaland all three release allowlists (with the@img/sharp-*+@img/sharp-libvips-*platform packages, as node-pty/clipboard do).const sharp = (await import('sharp')).defaultinsideexecute(), wrapped so a missing binding returns a boundedToolErrorTyperesult instead of propagating.
Worth a maintainer call regardless: @qwen-code/qwen-code-core is a published package, and sharp in dependencies (not optional) adds ~16 MB per platform (@img/sharp-libvips-darwin-arm64 alone is 15 MB) to every core/SDK consumer, image tool or not.
🟠 "The module is loaded lazily" isn't true today
Even with packaging fixed, registerLazy only defers registration; Config.initialize() calls warmAll() unconditionally, which runs every factory and therefore import('../tools/zoom-image.js') → top-level import sharp → libvips dlopen on every startup for image-capable models. Moving the import into execute() (fix #2 above) is what actually makes it lazy.
🟠 The registration gate reads a value that isn't always populated yet
getEffectiveInputModalities() (config.ts:3665) returns getContentGeneratorConfig()?.modalities ?? {}, and contentGeneratorConfig is assigned only in refreshAuth (config.ts:3399). The common path is fine — gemini.tsx:934 initializeApp() → performInitialAuth → refreshAuth runs before config.initialize() at gemini.tsx:1084 — but only when an authType is already persisted. Probed on main with a real Config and model: 'qwen3-vl-plus':
>>> modalities AT createToolRegistry: {}
>>> modalities AFTER initialize: {}
So with no auth yet configured the gate is false. Since createToolRegistry has exactly one caller (config.ts:2736) and nothing rebuilds the registry, the consequences are:
- first run where auth is selected in the dialog →
zoom_imageabsent for that entire session; /modelswitch into an image-capable model → absent until restart;- switch away from one → the tool lingers and will hand an image to a text-only model.
Both new config.test.ts cases mock getEffectiveInputModalities, so they can't see any of this. Consider registering unconditionally and gating at execute time — image-gen.ts:131 already does exactly that check at invocation — or re-gating from onModelChange.
🟠 The EXIF test does not pin auto-orientation (mutation-verified)
interprets coordinates in the EXIF-oriented image space is the test covering verify-item #2 of your test plan, and it survives deletion of the behavior. I reimplemented the exact pipeline (magnifiedSize + extract → resize → flatten → jpeg) and ran it with and without autoOrient: true on the crop's sharp() options:
autoOrient=true : out=1328x896 rgb=254,0,0 text="Oriented source: 40x60" => assertions PASS
autoOrient=false: out=1328x896 rgb=251,3,1 text="Oriented source: 40x60" => assertions PASS
Two reasons: the fixture is a single vertical red/green split, so the requested top strip's center pixel is red in raw space too; and Oriented source: 40x60 is built from metadata.autoOrient, which the mutant never touches. A fixture that does discriminate — 4-color quadrants, orientation 6, select the displayed top-left quadrant — verified:
autoOrient=true : displayed-top-left-quadrant center RGB = 0,2,251 (blue)
autoOrient=false: displayed-top-left-quadrant center RGB = 254,0,2 (red)
🟡 Smaller points
- Dead budget branch.
IMAGE_MAX_OUTPUT_BYTES(9 MB) is unreachable in practice: output is already capped at ≤1568px edge and ≤1568 28px patches (≈1092² for a square), so a q92 4:4:4 JPEG can't get close.bounds the returned view by edge, patch, and byte budgetsonly asserts the output is under the cap, never exercises the failure path. Fine as a backstop — just say so in a comment, or drop it. limitInputPixels: trueis sharp's 268 MP default. Paired with a 100 MB source cap, a decompression-bomb PNG can still force a very large decode buffer for what is only ever a crop. An explicit tighter cap (~100 MP) would bound it.- No telemetry.
read_fileemitsFileOperationEventvialogFileOperation;zoom_imageemits nothing, so usage of the new capability is invisible in metrics. - i18n gap.
toolDisplayName.ZoomImagelanded inen/zh/zh-TW, butca.jsalso carriestoolDisplayName.ReadFile('Llegeix fitxer') and was missed. - webui not updated.
packages/web-shell/.../toolFormatting.tsgotzoom_image, butpackages/webui/src/components/toolcalls/labelUtils.tshas the same per-name map and didn't. (routing.tsis fine — it keys onKind.) - Read-only family sets left out — all fail safe, but worth a deliberate decision:
speculationToolGate.ts:31SAFE_READ_ONLY_TOOLS(unknown tool →boundary, so a zoom aborts speculation) andsubagent-plan-tool-policy.ts:21PLAN_REQUIRED_TEAMMATE_PRE_APPROVAL_TOOLS(a read-only zoom will prompt for plan-required teammates). Microcompaction is genuinely fine — zoom results land in thenested-mediabranch, so the JPEG is evicted while the text survives; noCOMPACTABLE_TOOLSentry needed. - Discoverability. The tool is deferred and nothing in
prompts.tsor the imageread_filepath mentions it, so the model only finds it if it happens to search "zoom". The image-read path is the natural place for a one-line hint. - Description nit. "You may call it repeatedly" invites iterative zooming, but coordinates always address the original file, never the previously returned view. Worth stating explicitly so the model doesn't treat the last crop as the new coordinate space.
中文说明
在 c6b07eb 上审阅。工具本体质量很好：file-read-permission.ts 抽取是逐行等价的重构；权限规则、SAFE_TOOL_ALLOWLIST、extractToolFilePaths、循环检测只读集合、CORE_TOOLS、web-shell 展示名等接入点齐全且各有测试；所有失败路径都返回带类型的 ToolErrorType 而不是抛异常；坐标裁剪是有效的（我确认越界 extract() 会抛 extract_area: bad extract area）。sharp@0.34.5 的类型也符合代码假设（format 与 autoOrient 均为必填）。
问题不在工具本身，而在 sharp 如何进入发布产物，以及一个没有真正锁住行为的测试。
🔴 阻塞：sharp 没有接入任何打包路径，npm 安装版 CLI 会“无法启动”。 本仓库在四处维护原生模块清单，sharp 四处全缺：esbuild.config.js:145 的 external、scripts/prepare-package.js:305-321 生成的发布版 package.json、scripts/build-standalone-release.js:162、scripts/create-standalone-package.js:52-56,401。由于不是 external，esbuild 会把 sharp 的 JS 内联进 bundle（233 KB，无告警），而原生扩展仍靠运行时 require('@img/sharp-<平台>/sharp.node') 解析——发布包里既没有 sharp 也没有 @img/*，必然找不到。我用本仓库完全相同的打包配置（esm + packages:'bundle' + scripts/esbuild-shims.js）在没有 @img 的目录复现出：Error: Could not load the "sharp" module using the darwin-arm64 runtime。
影响范围不止“工具不可用”：zoom-image.ts:9 是静态 import，而 Config.initialize() → warmAll({strict})（config.ts:2765）→ ensureTool 重新抛出 → strict 直接 throw，即任何图片能力模型下 CLI 直接启动失败，工厂函数也没有 try/catch。本地验证之所以通过：sharp 已经通过 packages/mobile-mcp → mobilewright → @mobilewright/core 被提升到根 node_modules；而 packages/desktop 不能作为依据（它被 "!packages/desktop" 排除在 workspace 外，且只在 trustedDependencies 里提到 sharp）。scripts/verify-installation-release.js 只校验文件名和 SHA256，不会执行产物，因此 CI 也发现不了。建议按现有原生模块模式修复：sharp 放入 optionalDependencies，补齐 esbuild external 与三个发布清单（含 @img/sharp-* 平台包），并在 execute() 内改为 await import('sharp') 且失败时返回有界的工具错误。另外 @qwen-code/qwen-code-core 是对外发布的包，把 sharp 放进非可选 dependencies 会给每个 core/SDK 使用者增加约 16 MB（仅 @img/sharp-libvips-darwin-arm64 就 15 MB）。
🟠“按需加载”目前并不成立。 registerLazy 只延迟注册；Config.initialize() 无条件调用 warmAll()，会执行所有工厂，也就是每次启动都会动态导入 zoom-image.js 并 dlopen libvips。把 import 移入 execute() 才是真正的懒加载。
🟠 注册开关读取的值并非总已就绪。 getEffectiveInputModalities()（config.ts:3665）取自 contentGeneratorConfig.modalities，而该字段只在 refreshAuth（config.ts:3399）赋值。常规路径没问题（gemini.tsx:934 的 initializeApp 早于 1084 行的 config.initialize()），但前提是已持久化 authType。我在 main 上用真实 Config、model: 'qwen3-vl-plus' 实测：createToolRegistry 时刻与 initialize 之后都是 {}。由于 createToolRegistry 只有一个调用点（config.ts:2736）且没有重建机制，后果是：首次在对话框中选择认证的那个 session 拿不到该工具；/model 切换到图片模型需重启才生效；切离图片模型后工具仍残留。两个新增 config 测试都 mock 了 getEffectiveInputModalities，覆盖不到这一点。建议无条件注册、在执行时判定（image-gen.ts:131 已是此模式），或在 onModelChange 时重新判定。
🟠 EXIF 测试没有锁住方向纠正（已做变异验证）。 我复现了完整流水线，仅删除裁剪时的 autoOrient: true：两个断言依旧全绿（中心像素 251,3,1 vs 254,0,0；Oriented source: 40x60 来自 metadata.autoOrient，变异不影响）。原因是 fixture 只有左右红绿两半，所选顶部条带的中心像素在原始方向下也是红色。改用四象限 + orientation 6 + 取“显示后的左上象限”即可区分：开启为蓝 (0,2,251)、关闭为红 (254,0,2)，已验证。
🟡 其他小点： 9 MB 输出上限在现有 1568 边长 / 1568 patch 约束下不可达，测试也只断言未超限，属死分支（保留可加注释说明）；limitInputPixels: true 是 268 MP 默认值，配合 100 MB 源限制仍可能让解压炸弹撑出很大解码缓冲，建议显式收紧；缺少 read_file 那样的 FileOperationEvent 遥测；ca.js 有 toolDisplayName.ReadFile 但漏了 ZoomImage；packages/webui/.../labelUtils.ts 与 web-shell 同类映射未同步；speculationToolGate.ts:31 与 subagent-plan-tool-policy.ts:21 的只读集合未加入（行为安全但值得明确决策），微压缩无需改动（走 nested-media 分支，图片被清理、文本保留）；工具为 deferred 且 prompts.ts 与图片读取路径都没有提示，模型可能永远不会去搜索它；描述可明确说明坐标永远相对原图而非上一次返回的视图。
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
中文说明
未审查：反向审计——有审计 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，agent 实际被要求做的并不是本 skill 所认证的内容。
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Unresolved, please confirm: [Critical] zoom-image.ts:9 — top-level import sharp from 'sharp' makes loading eager at warmAll(), not lazy (raised by @wenshao; mechanism traced through config.ts:2765 warmAll → tool-registry.ts:352 ensureTool → import → dlopen; code unchanged since report) [Critical] config.ts:7157 — registration gate reads getEffectiveInputModalities() which may be empty when no auth persisted; tool absent for first-run session (raised by @wenshao; code unchanged since report) [Critical] zoom-image.test.ts — EXIF orientation test fixture is a red/green split that passes with autoOrient=true or false; does not pin auto-orientation behavior (raised by @wenshao; mutation-verified) Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies. Not reviewed: verification — the review posts findings, but no verifier was launched with a prompt this skill builds — they were ruled on, if at all, without the verdict bar its brief carries. [Critical] sharp is a native module not wired into esbuild external or optionalDependencies — CLI fails to start for image-capable models (existing blocker at packages/core/package.json:83, still stands after re-check of esbuild.config.js, scripts/prepare-package.js, scripts/build-standalone-release.js, scripts/create-standalone-package.js)
中文说明
已审查。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未决，请确认：[Critical] zoom-image.ts:9 — top-level import sharp from 'sharp' makes loading eager at warmAll(), not lazy (raised by @wenshao; mechanism traced through config.ts:2765 warmAll → tool-registry.ts:352 ensureTool → import → dlopen; code unchanged since report) [Critical] config.ts:7157 — registration gate reads getEffectiveInputModalities() which may be empty when no auth persisted; tool absent for first-run session (raised by @wenshao; code unchanged since report) [Critical] zoom-image.test.ts — EXIF orientation test fixture is a red/green split that passes with autoOrient=true or false; does not pin auto-orientation behavior (raised by @wenshao; mutation-verified) 未审查：反向审计——有审计 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，agent 实际被要求做的并不是本 skill 所认证的内容。 未审查：验证——本次评审发布了发现，但没有验证 agent 是用本 skill 构建的 prompt 启动的——这些发现即便被裁定过，也缺失了 brief 承载的裁定标准。 [Critical] sharp is a native module not wired into esbuild external or optionalDependencies — CLI fails to start for image-capable models (existing blocker at packages/core/package.json:83, still stands after re-check of esbuild.config.js, scripts/prepare-package.js, scripts/build-standalone-release.js, scripts/create-standalone-package.js)
— qwen3.7-max via Qwen Code /review
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
Externalize sharp in esbuild and declare it (plus the @img platform binaries) in the published package's optionalDependencies so an npm-installed CLI resolves the native binding. Import sharp dynamically inside execute() so a missing binding returns a bounded tool error instead of crashing startup during strict tool warmup, and so the module is only loaded when zoom_image actually runs. Also pin the EXIF auto-orientation test with a discriminating fixture and cover the .qwenignore and y1>=y2 validation paths.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — summaryCommit Resolved (implemented in this commit)
Escalated to a maintainer (open questions, not settled here)
Declined (with reasons)
Process notes (not code defects)The "Integration Tests skipped", "reverse audit", and "verification" review Verification
中文说明Autofix 评审轮次 — 总结提交 已解决（本次提交中实现）
升级给维护者（开放问题，本轮不裁定）
已拒绝（附原因）
流程说明（非代码缺陷）“Integration Tests skipped”“reverse audit”“verification”等评审观察描述的是评审/CI 的执行方式，而非本 PR 代码的缺陷；不适用代码改动。Integration test 被跳过属于 CI/工作流事项，由维护者处理。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Re-review —
|
| source | file | crop | wall | peak RSS |
|---|---|---|---|---|
| 16000×16000 (256 Mpx — the max the current limit allows) | 0.74 MB | bottom-edge 160×160 | 96 ms | 61 MB |
| 5750×5750 full-noise (33 Mpx — worst case still under the 100 MB byte cap) | 2.4 MB | bottom-edge 58×58 | 39 ms | 104 MB |
Bottom-edge crops specifically, since a top-left crop would let a sequential decoder stop early. No unbounded decode in either case. Please disregard that request; the current pairing of the sharp default with the 100 MB cap is fine.
Minor
validateToolParamValuescallsthis.config.getFileService()three times on the ignore path;read-file.ts:663binds it to a local first. Cosmetic, but each call constructs work the sibling avoids.- Sibling read-only sets still omit
zoom_image:followup/speculationToolGate.ts:31andagents/runtime/subagent-plan-tool-policy.ts:21(both listREAD_FILE/GREP/GLOB), pluspackages/webui/.../labelUtils.ts:86androuting.ts:40. All fail safe — the tool simply isn't speculated / isn't offered to plan-mode subagents / renders with a default label — so I'm flagging these as a deliberate decision, not a defect. - Defensive nit: the
tryaround the dynamic import wraps only the import, not the unwrap. If.defaultwere everundefined,sharpstaysundefinedand the resultingTypeErroris swallowed by the metadatacatch, surfacing as"Unsupported image. zoom_image accepts static PNG, JPEG, or WebP files"— a misleading message for a load failure. I confirmed.defaultis correct for sharp 0.34.5 (CJS, noexportsmap), so this is theoretical; atypeof sharp !== 'function'check in the load branch would make the diagnostic honest.
Verdict
The blocker is cleared and the tool itself remains well built. The registration gate is the one thing I'd want settled before merge — it's a real user-visible gap (first-run sessions and every /model switch), and the fix is smaller than the conditional it replaces.
中文说明
复审 —— feat(core): add full-resolution image zoom tool
在 head c9e0988（autofix 第 1 轮）复审。阻塞性的打包缺陷已修复 —— 我重新做了验证，而不是直接采信报告。还有 1 项遗留问题未解决、1 项新发现，另外我撤回上一轮自己提的一个要求，因为实测下来 bot 拒绝得没错。
✅ 已解决 —— sharp 打包（原阻塞项）
三个环节都改对了：esbuild.config.js 的 external、scripts/prepare-package.js 的 optionalDependencies，以及顶部 type-only 导入 + execute() 内 await import('sharp')。启动崩溃路径确实关闭了 —— zoom-image.ts 不再在模块作用域接触 sharp，因此严格 warmAll 不会在 Config.initialize() 中抛错。
我在真实 Node ESM（而非仅 vitest）下验证了 CJS 互操作解包，因为 .default 塌陷只会在那里出问题：typeof sharp = function，kernel.lanczos3 = lanczos3。
两点后续，均不阻塞：
- 那 10 条
@img/*显式 pin 是冗余的，且容易漂移。sharp@0.34.5自身的optionalDependencies已声明全部 24 个平台包，所以只写sharp: '0.34.5'就足以让 npm 安装正确的二进制 —— 包括列表遗漏的 14 个（Alpine 的@img/sharp-linuxmusl-x64/-arm64、win32-arm64、win32-ia32、linux-s390x/-ppc64/-riscv64/-arm）。因此当前覆盖没问题。代价是重复的0.34.5/1.2.4版本号需要手工维护：下次升级 sharp 时它们会静默过期，npm 就会在正确的嵌套二进制之外再装一个不匹配的顶层二进制。建议只保留sharp: '0.34.5'，删掉@img/*那一段。 - standalone 发布仍未打包 sharp —— 已确认，
scripts/build-standalone-release.js与create-standalone-package.js只暂存@teddyzhu/clipboard。有了懒加载，这现在是有界的READ_CONTENT_FAILURE而非崩溃，所以我同意作为后续项。建议在 PR 描述中写明zoom_image目前仅在 npm 安装下可用，免得后面被当成 bug 报上来。
✅ 已解决 —— EXIF 方向测试
四象限 + orientation: 6 + 选取显示后左上象限的 fixture 现在具备区分能力，报告中 autoOrient: true → false 的变异失败结果与我上一轮本地所得一致。修得很好。
❌ 仍未解决 —— 注册开关（唯一实质性遗留项）
这一项没有变化，而且我现在可以把它陈述为结构性事实而非猜测：
this.toolRegistry只在一处赋值：config.ts:2736，位于initialize()内。文件中没有任何其他地方重建它。setModel()（config.ts:3984）明确热更新contentGeneratorConfig.modalities，其注释还说明必须如此 —— "不同图片支持能力的 oauth 模型之间热切换必须更新它，否则 vision-bridge 判定和图片剥离会读到上一个模型的 modalities。" 但它从不重建工具注册表。因此/model从纯文本模型切到图片模型不会新增zoom_image，反向切换也不会移除它。 4056 行的注释恰好描述了新开关所破坏的那条不变式。getEffectiveInputModalities()是this.getContentGeneratorConfig()?.modalities ?? {}，而contentGeneratorConfig只由refreshAuth填充。gemini.tsx:476用if (authType)守卫该调用，其注释写道："新用户可能尚未选择/持久化 authType。" 因此在首次运行时 —— 以及只要设置了security.auth.useExternal（会整体跳过该代码块）——initialize()读到的是{}，即便主模型支持图片，该工具在整个 session 中都不存在。
新增的两个 config.test.ts 用例无法覆盖上述任何一种情况，因为它们 vi.spyOn(config, 'getEffectiveInputModalities').mockReturnValue(...)。它们锁住的是那个 if，而不是"在真正读取的时刻该值是否已被填充"。
建议与上一轮相同，而且现在看成本比维持现状更低：无条件注册，在执行时判定（先例：image-gen.ts:131）。这样还能与你已经加好的 "sharp 缺失" 失败分支合并为同一条错误路径，而不是两种行为。
🆕 新发现 —— 极小裁剪区域会耗尽整张图片的预算
magnifiedSize() 始终放大到符合视觉预算的最大尺寸，且对源裁剪尺寸没有下限，而 validateToolParamValues 只检查 x1 < x2 —— 没有任何地方约束 x2 - x1：
裁剪 1×1 → 视图 1092×1092 （放大 1092 倍）
裁剪 10×10 → 视图 1092×1092 （109 倍）
裁剪 100×100 → 视图 1092×1092 （10.9 倍）
裁剪 2000×500→ 视图 1568×392 （0.8 倍 —— 缩小，正常）
实测：10×10 的裁剪返回 1092×1092、126 KB 的 JPEG = 1521 个 patch —— 几乎把单张图片的全部预算，花在对 100 个源像素做 lanczos 插值上。模型只要算错归一化坐标（很容易：在 1000 像素宽的图上，1 个归一化单位 ≈ 1 px），就要为噪声付出整张图片的 token 代价。
两个低成本缓解方案，任选其一：
- 限制放大倍数（例如 ≤ 8 倍），使视图退化到裁剪自身分辨率而不是被撑大；以及/或者
- 在 text part 中报告实际像素矩形。目前 text 回显的是请求的归一化坐标，因此
Math.min/Math.max的钳制对模型不可见 —— 模型无法得知自己放大过头或矩形被裁掉了。
🔄 撤回我自己的要求：limitInputPixels
上一轮我要求收紧 limitInputPixels，理由是 100 MB 字节上限内的 268 Mpx 源图会导致约 1 GB 的解码。bot 以"调参判断"为由拒绝。我做了实测，拒绝是对的 —— 此处 libvips 是按需驱动的，开销跟随裁剪区域而非源图：
| 源图 | 文件 | 裁剪 | 耗时 | 峰值 RSS |
|---|---|---|---|---|
| 16000×16000（256 Mpx —— 当前上限允许的最大值） | 0.74 MB | 底边 160×160 | 96 ms | 61 MB |
| 5750×5750 全噪声（33 Mpx —— 仍在 100 MB 字节上限内的最坏情况） | 2.4 MB | 底边 58×58 | 39 ms | 104 MB |
特意选底边裁剪，因为左上角裁剪会让顺序解码器提前结束。两种情况都没有无界解码。请忽略该要求；当前 sharp 默认值与 100 MB 上限的搭配是合适的。
次要项
validateToolParamValues在 ignore 分支上调用了三次this.config.getFileService()；read-file.ts:663先绑定到局部变量。属于外观问题，但每次调用都做了同级工具已避免的工作。- 同类只读工具集合仍未包含
zoom_image：followup/speculationToolGate.ts:31和agents/runtime/subagent-plan-tool-policy.ts:21（两者都列了READ_FILE/GREP/GLOB），以及packages/webui/.../labelUtils.ts:86和routing.ts:40。它们都安全失败 —— 工具只是不被推测执行 / 不提供给 plan 模式子代理 / 用默认标签渲染 —— 所以我把这些标记为需要明确决策的事项，而非缺陷。 - 防御性细节：动态导入外的
try只包住了 import，没包住解包。若.default为undefined，sharp会保持undefined，随后的TypeError会被 metadata 的catch吞掉，表现为"Unsupported image. zoom_image accepts static PNG, JPEG, or WebP files"—— 对加载失败而言是误导性信息。我已确认 sharp 0.34.5（CJS，无exports映射）的.default是正确的，所以这属于理论情形；在加载分支加一个typeof sharp !== 'function'检查可以让诊断信息如实反映问题。
结论
阻塞项已清除，工具本身依然做得很扎实。注册开关是我希望在合并前敲定的唯一一点 —— 它是真实的用户可见缺口（首次运行的 session 以及每一次 /model 切换），而且修复方案比它所替换的条件判断更小。
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. Unresolved, please confirm: [Critical] config.ts:7157 — registration gate reads getEffectiveInputModalities() which may be empty when no auth is persisted; tool absent for first-run session / model switch (raised by @wenshao; escalated to maintainer as product decision; code unchanged since report — cannot determine if defect or by-design)
中文说明
已审查。 建议见行内评论。 未决，请确认：[Critical] config.ts:7157 — registration gate reads getEffectiveInputModalities() which may be empty when no auth is persisted; tool absent for first-run session / model switch (raised by @wenshao; escalated to maintainer as product decision; code unchanged since report — cannot determine if defect or by-design)
— qwen3.7-max via Qwen Code /review
…wenLM#7809) Register zoom_image unconditionally and move the image-modality check to execute time so first-run sessions and hot /model switches resolve the tool without re-running initialize(). Cap magnification at 8x so a tiny crop no longer inflates to the full image-token budget. Drop the redundant @img/* platform pins; sharp's own optionalDependencies install the matching binary for each OS/arch.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round 2 — PR #7809Addressed the feedback newer than the last evaluation. One commit Feedback points
Conflict notesNone — Verification
中文说明Autofix 审查第 2 轮 — PR #7809已处理上一轮评估之后的新反馈。一个提交（ 反馈点
冲突说明无 —— 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
…wenLM#7809) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #7809Addressed all three inline suggestions from the automated reviewer. No conflict Feedback points
Conflict notesNone. Verification
No settings source changed, so 中文说明Autofix 审查轮次 — PR #7809已处理自动审查器提出的全部三条行内建议。无需冲突合并（ 反馈点
冲突说明无。 验证
未改动任何 settings 源文件，因此无需运行 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.7-max via Qwen Code /review
Local verification (round 3) —
|
| # | Finding | Raised | Status at this head |
|---|---|---|---|
| 1 | sharp not externalized/packaged → startup crash |
R1 blocking | ✅ Fixed in c9e0988, re-confirmed here |
| 2 | EXIF-orientation test could not discriminate | R1 | ✅ Fixed in c9e0988 |
| 3 | Registration gate — tool absent on first-run sessions and never added/removed by /model |
R1, R2 | ✅ Fixed in 573b0d9 — proven by controlled experiment below |
| 4 | Tiny crops burn a full image budget (1×1 → 1092×1092) | R2 | ✅ Fixed in 573b0d9 — 8× cap, measured below |
| 5 | 10 redundant @img/* pins, drift-prone |
R2 minor | ✅ Removed in 573b0d9; sharp: '0.34.5' alone, with a comment explaining why |
| 6 | getFileService() constructed 3× on the ignore path |
R2 minor | ✅ Fixed in 573b0d9 |
| 7 | Tighter limitInputPixels |
R1 | 🔄 I retracted this in R2 after measuring; it stays retracted |
| 8 | Standalone releases don't ship sharp |
R2 follow-up | @teddyzhu/clipboard. Now a bounded error, not a crash. Follow-up. |
| 9 | Sibling read-only sets omit zoom_image |
R2 informational |
One correction to my own R2 text: I suggested the tool "report the actual pixel rect". It already did at c9e0988 — the text part carries Oriented source: …; source crop: …; returned view: …. What is still unreported is the crop's origin, which is only reachable through the degenerate-crop guard, so it isn't worth changing.
1 · The registration gate — controlled experiment
The fix registers zoom_image unconditionally and moves the modality check into execute(), which is what I asked for (precedent: image-gen.ts:131). To confirm it is load-bearing rather than incidental, I wrote a spec that uses a real Config, a real ToolRegistry, and a real image on disk — the PR's own suite drives a stub config object literal, so it pins the if inside execute() but never the registration path — and ran the identical 4-test spec twice, changing only config.ts + zoom-image.ts:
- pre-fix (
c9e0988) overlaid → 4 failed.registry.getTool('zoom_image')returnsundefined; the tool simply does not exist for a first-run or text-only session, so a later/modelswitch has nothing to call. - head (
9cccf54) → 4 passed. The tool resolves with empty modalities; a text-only model gets a boundedread_content_failure; and a hot/modelswitch works on the same registry object with no re-initialize(), returning the correct pixels.
Worth recording why the hot switch now works: handleModelChange() mutates contentGeneratorConfig.modalities in place, and getEffectiveInputModalities() reads it live at execute time — so the fix composes with the existing hot-update path rather than depending on a registry rebuild that never happens (this.toolRegistry is still assigned at exactly one place, inside initialize()).
I also mutation-tested the PR's own tests to check they pin the new behaviour rather than merely passing: 9 / 9 mutants killed, no survivors, each by a test whose name matches the mutated behaviour.
The one I'd point at is M2 — changing IMAGE_MAX_UPSCALE from 8 to 9 also dies. The cap assertion is tight, not just "smaller than before".
2 · The 8× cap, and pixel-level correctness
Measured by driving the real tool over a 1000×1000 noise source (where 1 normalized unit = exactly 1 source pixel, which is the coordinate mistake a model actually makes), once per arm:
| requested crop | before | after |
|---|---|---|
| 1×1 | 1092×1092 · 1521 patches · 14,345 B | 8×8 · 1 patch · 267 B |
| 10×10 | 1092×1092 · 1521 patches · 105,110 B | 80×80 · 9 patches · 2,705 B |
| 100×100 | 1092×1092 · 1521 patches · 350,482 B | 800×800 · 841 patches · 237,790 B |
| ≥196×196 | — | byte-for-byte unchanged |
The crossover is exactly 1568 / 8 = 196 px, so every legitimate zoom is untouched and only the pathological end is bounded. That is the right shape for this fix.
Correctness spot-checks all came back exact: each of the four quadrants returns the precise colour encoded in it; an orientation:6 JPEG (stored rotated 90°) returns green for the displayed top-right, so coordinates follow what a user sees rather than the stored bytes; and a fully transparent PNG flattens to rgb(255,255,255).
3 · End-to-end on a real screenshot
Finally I drove the real tool against a 3200×628 dashboard screenshot with ~10–11 px table text — the situation the PR is actually for. Both panels below are the tool's own returned JPEGs written straight to disk.
The whole frame reaches a model as 1568×308 (0.49×), leaving that region as 251×73 pixels where 0.0126 is indistinguishable from 0.0128 and 11,904 from 11,804. zoom_image returns 512×148 real source pixels magnified to 1568×453, and every value matches the source data exactly. This is a genuine capability gain, not a re-render of what the model already had.
Also verified: a persisted Read(...) rule covers zoom_image through the meta-category, and the rule the tool offers is identical to read_file's (parent directory) — so the permission story in the description holds.
New findings at this head — both minor, neither blocking
1 · A text-only model still reaches the approval prompt before the gate fires. build() runs validateToolParamValues at coreToolScheduler.ts:1826, well before evaluatePermissionFlow at :2362, and validation does not consult modalities. So for an image outside the workspace, getFileReadDefaultPermission returns 'ask', the user is prompted to approve a read, and then execute() returns "requires a model that accepts image inputs". I have this reproducing in the spec above. It is a direct consequence of the placement I recommended, so I'm flagging the cost rather than complaining about it. If you want it gone, moving the check into validateToolParamValues() keeps the entire hot-switch benefit — build() runs per call — while failing before any prompt.
2 · The new telemetry call is never exercised against a real Config. zoom-image.test.ts does vi.mock('../telemetry/loggers.js') for the whole module, so logFileOperation() is a spy in every test. In reality it reaches QwenLogger.getInstance(config) → config.getUsageStatisticsEnabled(); my harness hit that instantly because the stub configs in that file don't define it. I ran it with a real Config and it works — no throw, correct event — so this is not a defect, just a note that a future signature change to logFileOperation/FileOperationEvent would not be caught by these tests. Mutants M6/M7 do confirm the call site itself is pinned.
Minor bookkeeping: the description says the affected suites total 1,324 tests; at this head I measure 1,242 across the seven files, so that line predates the last two commits.
Environment and gates
Isolated worktree at 9cccf54; node_modules APFS-cloned from a lockfile-matching tree; all 20 @qwen-code/* entries verified to resolve inside the worktree (no cross-tree contamination); vite 7.0.0 / vitest 3.2.4 / sharp 0.34.5, all matching the lockfile.
| gate | result |
|---|---|
| 7 affected suites | ✅ 1,242 passed |
read-file regression |
✅ 87 passed |
tsc --noEmit (packages/core) |
✅ exit 0, 0 errors |
eslint (24 changed files) |
✅ exit 0 — positive control: planted unused import → exit 1, restored → exit 0 |
prettier --check |
✅ all match |
i18n keys en / zh / zh-TW |
✅ present in all three; the self-referential en value matches the file's convention (1555/1556 entries) |
| mutation matrix | ✅ 9/9 killed |
Verdict
Merge-ready from my side. The blocker cleared two rounds ago, and the two substantive findings I was holding out on are now genuinely fixed — I verified both by controlled experiment rather than inspection, and the tests that cover them survive mutation. The two new items are minor, and the first one is a tradeoff I asked for.
The one thing worth stating in the description before merge is that zoom_image is npm-install-only for now — standalone builds still don't stage sharp, so it degrades to a bounded error there. Better to document it than to have it reported as a bug later.
中文说明
本地验证（第 3 轮）—— feat(core): add full-resolution image zoom tool
在隔离 worktree 中基于 head 9cccf54 复验，承接第 1 轮（c6b07eb）与第 2 轮（c9e0988）。
我遗留的两项问题都已修复，并且每一项都重新做了实测，而不是采信 commit message。 下面每条结论都有针对当前 head 的实际运行支撑；两项遗留问题被当作"关于新 head 的假设"重新验证，而非直接沿用。
历史问题 → 在 9cccf54 的状态
| # | 问题 | 提出轮次 | 当前状态 |
|---|---|---|---|
| 1 | sharp 未 external/打包 → 启动崩溃 |
R1 阻塞 | ✅ c9e0988 已修，本轮再次确认 |
| 2 | EXIF 方向测试不具区分能力 | R1 | ✅ c9e0988 已修 |
| 3 | 注册开关 —— 首次运行时工具缺失，/model 切换不增不减 |
R1、R2 | ✅ 573b0d9 已修，见下方对照实验 |
| 4 | 极小裁剪耗尽整张图片预算（1×1 → 1092×1092） | R2 | ✅ 573b0d9 已修，8× 上限，实测见下 |
| 5 | 10 条冗余 @img/* pin，易漂移 |
R2 次要 | ✅ 573b0d9 已删除，仅保留 sharp: '0.34.5' 并附注释说明 |
| 6 | ignore 路径上 getFileService() 构造 3 次 |
R2 次要 | ✅ 573b0d9 已修 |
| 7 | 收紧 limitInputPixels |
R1 | 🔄 我在 R2 实测后已撤回，继续维持撤回 |
| 8 | standalone 发布未打包 sharp |
R2 后续项 | @teddyzhu/clipboard。现在是有界错误而非崩溃，可作后续项 |
| 9 | 同类只读工具集合遗漏 zoom_image |
R2 提示 |
对我自己 R2 措辞的一处更正：我曾建议"在文本部分报告实际像素矩形"，其实 c9e0988 就已经在报告 Oriented source: …; source crop: …; returned view: …。真正未报告的是裁剪区域的起点坐标，而它只能通过退化裁剪保护分支触达，因此不值得改动。
1 · 注册开关 —— 对照实验
修复方案是无条件注册 zoom_image，把 modality 判定移到 execute()，与我的建议一致（先例：image-gen.ts:131）。为确认该修复是真正起作用而非碰巧通过，我写了一份使用真实 Config、真实 ToolRegistry 和磁盘上真实图片的测试 —— PR 自带的测试驱动的是一个 stub config 字面量，因此只锁住了 execute() 里的 if，从未覆盖注册路径 —— 并用完全相同的 4 条测试跑了两遍，仅替换 config.ts 与 zoom-image.ts：
- 覆盖回 pre-fix（
c9e0988）→ 4 条全部失败。registry.getTool('zoom_image')返回undefined，该工具在首次运行/纯文本会话中根本不存在，后续/model切换也就无从调用。 - 当前 head（
9cccf54）→ 4 条全部通过。 modalities 为空时工具可解析；纯文本模型得到有界的read_content_failure；热切/model在同一个 registry 对象上、无需重新initialize()即可工作，并返回正确像素。
值得记录热切换现在为何有效：handleModelChange() 是就地修改 contentGeneratorConfig.modalities，而 getEffectiveInputModalities() 在执行时实时读取 —— 因此该修复与既有热更新路径天然契合，不再依赖那个实际从未发生的 registry 重建（this.toolRegistry 仍然只在 initialize() 内部的唯一一处赋值）。
我还对 PR 自带测试做了变异测试，确认它们确实锁住新行为而非仅仅通过：9 / 9 变异体全部被杀，无存活，且每个都由名称与被变异行为对应的测试杀死。其中最值得一提的是 M2 —— 把 IMAGE_MAX_UPSCALE 从 8 改成 9 同样被杀，说明该断言是紧的，而不只是"比原来小"。
2 · 8× 上限与像素级正确性
用真实工具在 1000×1000 噪声图上实测（此时 1 归一化单位恰好等于 1 源像素，这正是模型真实会犯的坐标错误），两个分支各跑一次：
| 请求裁剪 | 修复前 | 修复后 |
|---|---|---|
| 1×1 | 1092×1092 · 1521 patch · 14,345 B | 8×8 · 1 patch · 267 B |
| 10×10 | 1092×1092 · 1521 patch · 105,110 B | 80×80 · 9 patch · 2,705 B |
| 100×100 | 1092×1092 · 1521 patch · 350,482 B | 800×800 · 841 patch · 237,790 B |
| ≥196×196 | — | 逐字节不变 |
分界点恰好是 1568 / 8 = 196 px，因此所有正常的放大请求完全不受影响，只有病态区间被收敛 —— 这正是该修复应有的形状。
正确性抽查全部精确：四个象限各自返回其编码的确切颜色；orientation:6 的 JPEG（字节中旋转了 90°）在请求显示后的右上象限时返回绿色，说明坐标遵循用户所见而非存储字节；全透明 PNG 铺白为 rgb(255,255,255)。
3 · 真实截图上的端到端验证
最后我用一张 3200×628、表格文字约 10–11 px 的 dashboard 截图驱动真实工具 —— 这正是本 PR 的目标场景。图中两块面板都是工具自己返回、直接落盘的 JPEG。
整帧到达模型时是 1568×308（0.49×），该区域只剩 251×73 像素，此时 0.0126 与 0.0128、11,904 与 11,804 无法区分。zoom_image 返回 512×148 个真实源像素并放大到 1568×453，每个数值都与源数据完全一致。这是真实的能力增益，而不是把模型已有的信息重新渲染一遍。
另外验证：持久化的 Read(...) 规则通过 meta-category 覆盖 zoom_image，且该工具提议的规则与 read_file 完全一致（父目录），说明描述中的权限说法成立。
本轮新发现 —— 两项均为次要，都不阻塞
1 · 纯文本模型仍会先走到授权确认再触发拦截。 build() 在 coreToolScheduler.ts:1826 执行 validateToolParamValues，远早于 :2362 的 evaluatePermissionFlow，而校验并不检查 modalities。因此对工作区之外的图片，getFileReadDefaultPermission 返回 'ask'，用户被要求批准一次读取，之后 execute() 才返回"requires a model that accepts image inputs"。上述测试中已复现。这是我所建议的放置位置带来的直接代价，所以我是在标注成本而非指责。若希望消除，把该检查移入 validateToolParamValues() 可完整保留热切换收益（build() 每次调用都会执行），同时在任何确认弹窗之前失败。
2 · 新增的 telemetry 调用从未在真实 Config 下被执行。 zoom-image.test.ts 对 ../telemetry/loggers.js 整个模块做了 vi.mock，因此 logFileOperation() 在所有测试中都是 spy。实际运行时它会走到 QwenLogger.getInstance(config) → config.getUsageStatisticsEnabled()；我的验证脚手架立刻撞上了这一点，因为该文件中的 stub config 并未定义此方法。我用真实 Config 跑通了这条路径 —— 不抛错、事件正确 —— 所以这不是缺陷，只是提示：未来 logFileOperation/FileOperationEvent 的签名变更不会被这些测试捕获。变异体 M6/M7 确认调用点本身是被锁住的。
小的记录性问题：描述中写受影响套件共 1,324 个测试；我在当前 head 实测这七个文件为 1,242，说明该行早于最后两个 commit。
环境与门禁
隔离 worktree 位于 9cccf54；node_modules 由与 lockfile 一致的树 APFS 克隆而来；已验证全部 20 个 @qwen-code/* 条目解析到 worktree 内部（无跨树污染）；vite 7.0.0 / vitest 3.2.4 / sharp 0.34.5 均与 lockfile 一致。
| 门禁 | 结果 |
|---|---|
| 7 个受影响套件 | ✅ 1,242 通过 |
read-file 回归 |
✅ 87 通过 |
tsc --noEmit（packages/core） |
✅ exit 0，0 错误 |
eslint（24 个改动文件） |
✅ exit 0 —— 阳性对照：植入未使用 import → exit 1，还原后 → exit 0 |
prettier --check |
✅ 全部符合 |
i18n 键 en / zh / zh-TW |
✅ 三者齐备；en 的自引用值符合该文件既有约定（1555/1556 条） |
| 变异矩阵 | ✅ 9/9 被杀 |
结论
从我这边看可以合并。 阻塞项两轮前已清除，我一直保留意见的两项实质问题现已真正修复 —— 两项我都用对照实验而非阅读代码来验证，且覆盖它们的测试能经受变异测试。本轮两项新发现均为次要，其中第一项还是我自己要求的取舍。
合并前唯一值得在描述中写明的是：zoom_image 目前仅在 npm 安装下可用 —— standalone 构建仍未暂存 sharp，在那里会退化为有界错误。与其日后被当作 bug 报上来，不如现在写清楚。
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review triage (round 3) — no code changeThanks for the thorough round-3 verification and the merge-ready verdict. This round's feedback contains no Finding 1 — text-only model reaches the approval prompt before the modality gate · DeclinedThis is the optional suggestion to move the modality check from
The cost (an occasional spurious approval prompt for an out-of-workspace image under a text-only model) is bounded and is the direct consequence of the execute-time placement recommended earlier. If a maintainer later wants it gone, the Finding 2 — telemetry call never exercised against a real
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action neededThe newest feedback on PR #7809 (items newer than the last evaluation at
No code change is warranted this round, so no commit was made and no 中文说明Autofix 审查轮次 — 无需处理PR #7809 的最新反馈（即上次评估 2026-07-27T18:38:21Z 之后的新增项）没有任何需要处理的内容：
本轮无需任何代码改动，因此没有产生提交，也没有运行验证命令（在没有任何改动时无需运行）。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Released in v0.21.1. |
6 participants
What this PR does
This PR adds a deferred, read-only
zoom_imagecapability for image-capable primary models. Given an absolute path to a static PNG, JPEG, or WebP image and a normalized 0–1000 rectangle, it crops from the EXIF-oriented full-resolution original, magnifies the selected region within bounded edge, visual-patch, source-byte, and output-byte budgets, and returns a JPEG view to the model.The capability follows the existing file-read permission boundary and participates in persistent
Read(...)rules, the core-tool allowlist, filesystem-path attribution, AUTO-mode safety classification, and read-loop detection. Text-only models do not register it, the existing image-read behavior is unchanged, and deferred discovery avoids adding its schema to the initial tool list.Why it's needed
Large images are commonly downscaled before reaching a model, which can erase small text, chart labels, thin lines, and other important details. A targeted crop from the original source lets the model recover those details deterministically without repeatedly sending the entire full-resolution image or changing the existing read contract.
Reviewer Test Plan
How to verify
zoom_image, and select the upper-right quadrant. The returned view should contain the correct source pixels and be visibly magnified.Read(...)rules apply, and--core-toolscan explicitly enable or disable the capability.Validation performed locally: the seven affected test suites passed with 1,324 tests; the full workspace build and full workspace typecheck passed after rebasing onto the latest
main; the bundled CLI successfully cropped a real screenshot and returned an inline JPEG.Evidence (Before & After)
Before: when downscaling hid fine image detail, the model had no built-in way to request a bounded crop from the original source.
After: an image-capable model can request a normalized region from the original file and receive a magnified, budget-bounded view while preserving the existing read path.
Tested on
Environment (optional)
Node.js v24.14.1, local non-sandboxed unit/build validation.
Risk & Scope
sharpa direct Core dependency. The module is loaded lazily, input formats and dimensions are bounded, animated images are rejected, and output is constrained before it reaches the model.Linked Issues
None.
中文说明
本 PR 做了什么
本 PR 为支持图片输入的主模型增加一个延迟发现、只读的
zoom_image能力。模型传入静态 PNG、JPEG 或 WebP 的绝对路径以及 0–1000 的归一化矩形后，工具会在应用 EXIF 方向后的全分辨率原图上裁剪，并在长边、视觉 patch、源文件大小和输出大小限制内放大所选区域，最后向模型返回 JPEG 视图。该能力复用现有文件读取权限边界，并接入持久化
Read(...)规则、Core 工具白名单、文件路径归因、AUTO 模式安全分类和只读循环检测。纯文本模型不会注册该能力，现有图片读取行为保持不变，延迟发现也避免把 schema 加入初始工具列表。为什么需要
大图片在发送给模型前通常会被缩小，导致小字、图表标签、细线和其他关键细节丢失。基于原始文件做定向裁剪，可以让模型确定性地恢复这些细节，无需反复发送整张全分辨率图片，也不需要修改现有读取契约。
Reviewer 测试计划
如何验证
zoom_image后选择右上象限。返回视图应包含正确的原始像素，并且被明显放大。Read(...)规则生效，且--core-tools可以显式启停该能力。本地验证结果：7 个受影响测试套件共 1,324 个测试全部通过；rebase 到最新
main后，全 workspace build 和全 workspace typecheck 通过；最终 bundle 对真实截图完成裁剪并返回了内联 JPEG。前后对比证据
Before：图片缩小后隐藏细节时，模型没有内置方式从原始文件请求一个受预算约束的裁剪区域。
After：支持图片输入的模型可以从原始文件请求归一化区域，并在保持现有读取路径不变的前提下获得放大且有预算约束的视图。
测试平台
环境
Node.js v24.14.1，本地非 sandbox 单测和构建验证。
风险与范围
sharp成为 Core 的直接依赖。模块按需加载，输入格式和尺寸有界，动态图片会被拒绝，输出在发送给模型前受到限制。关联 Issue
无。