fix(daemon): Route ACP images through the vision bridge - #6111
Conversation
|
Local daemon smoke report:
I did not send a live image prompt to the remote model during smoke testing to avoid consuming provider quota; the regression test covers the ACP vision bridge routing by mocking |
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, with evidence. On direction: this is a straightforward parity fix. The interactive CLI already runs the vision bridge for text-only models, but the daemon/ACP path skips it — images silently degrade to placeholders. Real user problem, clearly described in #6110. No direction concerns. On approach: the scope is tight — 2 files, focused entirely on wiring the existing Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必要部分齐全,中英双语,附带证据。 方向:这是一个明确的对等性修复。交互式 CLI 已经会对文本模型调用 vision bridge,但 daemon/ACP 路径跳过了这一步——图片被静默降级为占位符。问题真实存在,#6110 描述清晰,无方向问题。 方案:范围紧凑——2 个文件,专注于将已有的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe implementation closely mirrors the CLI path in Correctness: All return paths in Error handling: Independent try/catch around Prior review comments: All 5 critical issues from the initial review were addressed — error boundaries, abort handling, fallback paths, Minor note: Regression Tests (7/7 pass)All branches covered: happy path, cancelled bridge, oversized images, bridge exception, bridge failure status, negative gate (model supports images), and @-file resolution. Tmux Smoke Test (daemon startup)Daemon starts and responds. Full ACP image-prompt routing through the vision bridge requires a configured text-only primary model with an available vision bridge model — not available in this CI environment. The unit tests cover the code path comprehensively with mocked bridge calls. 中文说明代码审查实现紧贴 正确性: 错误处理: 先前审查意见: 初始审查的 5 个关键问题已全部解决——错误边界、中断处理、回退路径、 小建议: 回归测试(7/7 通过)所有分支均已覆盖:正常路径、bridge 取消、大尺寸图片、bridge 异常、bridge 失败状态、负向门控(模型支持图片)、@-文件解析。 Tmux 烟测(daemon 启动)Daemon 正常启动并响应。完整的 ACP 图片 prompt 经 vision bridge 路由需要配置文本主模型和可用的 vision bridge 模型——当前 CI 环境不具备此条件。单元测试通过 mock bridge 调用全面覆盖了代码路径。 — Qwen Code · qwen3.7-max |
|
This PR is a clean, focused parity fix. The daemon/ACP path now matches the interactive CLI's vision bridge behavior — text-only primary models get image-to-text conversion instead of silent placeholder degradation. The implementation mirrors the proven CLI pattern, handles errors gracefully (independent try/catch for bridge and notice, abort checks, text-only fallback), and passes all 7 regression tests covering every meaningful branch. The prior review's 5 critical issues were all addressed before this re-run. The one duplication — Two files changed, no scope creep, no new abstractions. Ships the fix, moves on. Approving. ✅ 中文说明这个 PR 是一个干净、专注的对等性修复。daemon/ACP 路径现在与交互式 CLI 的 vision bridge 行为一致——文本主模型会获得图片转文本,而非静默降级为占位符。 实现复刻了已验证的 CLI 模式,错误处理稳健(bridge 和通知各自有独立的 try/catch、中断检查、纯文本回退),7 个回归测试覆盖了所有有意义的分支。先前审查的 5 个关键问题在本次重审前已全部解决。唯一的重复—— 2 个文件改动,无范围蔓延,无新抽象。完成修复,继续前进。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
DragonnZhang
left a comment
There was a problem hiding this comment.
Review summary (automated): Reviewed the full diff (2 files, +117/-3) covering correctness, security, and code quality.
Correctness: No issues found. All three return paths in #resolvePrompt correctly route through #applyVisionBridgeIfNeeded. The gating (hasImageParts + shouldRunVisionBridge) matches the CLI path in useGeminiStream.ts. The failure helper in runVisionBridge sets applied: true with image-free replacement parts, so the bridge failure case is handled correctly without needing the explicit image-stripping fallback the CLI uses. The preserveUnsupportedImageForBridge flag is correctly passed to readManyFiles only when the bridge is active.
Security: No issues. Egress disclosure is properly surfaced in all relevant cases (success, failure with egress, skipped with egress). The system instruction remains injection-aware.
Code quality (minor, non-blocking): #formatVisionBridgeNotice in Session.ts duplicates formatVisionBridgeNotice from useGeminiStream.ts with minor wording differences (e.g., CLI says "cancelled" for skipped status, ACP says "skipped"; success-case egress note wording differs). Consider extracting a shared formatter to core to prevent future drift.
Test: The regression test properly mocks the bridge and verifies routing + inlineData removal. LGTM.
|
@qwen-code /triage |
doudouOUC
left a comment
There was a problem hiding this comment.
All review comments have been addressed in subsequent commits (harden fallback + test coverage). The final implementation:
- Wraps runVisionBridge and emitAgentMessage in independent try/catch blocks (best-effort enhancement)
- Checks abortSignal.aborted after bridge returns, falls back to text-only parts
- Returns splitImageParts(parts).nonImageParts in all non-applied/failed/cancelled paths (never leaks raw inlineData to text-only models)
- formatVisionBridgeNotice wording aligned with CLI behavior
- preserveUnsupportedImageForBridge correctly bypasses clampInlineMediaPart when bridge is active
- 7 test cases cover all critical branches including error, cancel, gate-negative, oversized, and @file paths
LGTM. One follow-up suggestion: extract formatVisionBridgeNotice into packages/core to eliminate the remaining duplication between Session.ts and useGeminiStream.ts.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
What this PR does
Routes daemon/ACP image prompts through the existing vision bridge before they are sent to the primary model. When the bridge is enabled for a text-only primary model, ACP now converts image parts to text with the configured vision model instead of letting the downstream converter replace images with placeholders. It also preserves unsupported image file parts while resolving
@paths when the bridge can handle them.Why it's needed
The interactive CLI already applies the vision bridge, but the daemon/ACP prompt path resolved image parts and sent them directly to the primary model. With a config such as a text-only
qwen3.7-maxprimary model plus a same-providerqwen3.7-plusvision-capable model, image prompts still reachedqwen3.7-maxand were degraded to text placeholders. This makes the daemon behavior match the CLI behavior and lets existing provider/model discovery work for ACP image prompts.Reviewer Test Plan
How to verify
Configure a text-only primary model with an available vision bridge model, then send an ACP image prompt through
qwen serve. The prompt should be converted by the bridge before the primary model call; the downstream message should contain the image transcript and should not contain rawinlineDataimage parts for the text-only primary model. The targeted regression test mocks the bridge and verifies that ACP calls it and forwards converted text.Evidence (Before & After)
Before: the reported daemon logs showed
Model 'qwen3.7-max' does not support image input. Replacing with text placeholder: image/png, and no vision bridge conversion logs appeared. After: the regression test verifiesrunVisionBridgeis invoked for an ACP image prompt and the primary chat receives the converted transcript withoutinlineData. Local daemon smoke also verifiedqwen servestartup,/health, ACPinitialize, and ACPsession/newwithmcpServers: []. Manual Web UI verification also passed:Tested on
Environment (optional)
Local macOS worktree with Node.js 22. Verified
qwen serveon127.0.0.1:4171using the dev entrypoint and--no-web.Risk & Scope
Linked Issues
Resolves #6110
Related #6086
中文说明
这个 PR 做了什么
将 daemon/ACP 图片 prompt 在发送给主模型之前接入现有 vision bridge。当主模型不支持图片但配置了可用的视觉模型时,ACP 现在会先用 bridge 把图片转换成文本,而不是让下游转换器把图片替换成占位文本。对于
@路径解析出来的不受主模型支持的图片文件,在 bridge 可处理时也会保留下来供转换使用。为什么需要
交互式 CLI 已经会应用 vision bridge,但 daemon/ACP prompt 路径之前只解析图片 part 后直接发给主模型。像主模型是文本模型
qwen3.7-max,同 provider 下还有支持视觉的qwen3.7-plus这种配置,图片仍然会到达qwen3.7-max,然后被降级成文本占位符。这个改动让 daemon 行为和 CLI 行为保持一致,也让现有 provider/model 自动发现能用于 ACP 图片 prompt。Reviewer Test Plan
如何验证
配置一个文本主模型和可用的 vision bridge 模型,然后通过
qwen serve发送 ACP 图片 prompt。预期 prompt 会先被 bridge 转换,再调用主模型;下游消息应包含图片转写文本,并且对于文本主模型不应再包含原始inlineData图片 part。新增回归测试通过 mock bridge 验证 ACP 会调用 bridge,并把转换后的文本传给主模型。证据(Before & After)
Before:用户提供的 daemon 日志显示
Model 'qwen3.7-max' does not support image input. Replacing with text placeholder: image/png,且没有 vision bridge 转换日志。After:回归测试验证 ACP 图片 prompt 会调用runVisionBridge,主 chat 收到转换后的文本且不包含inlineData。本地 daemon 烟测也验证了qwen serve启动、/health、ACPinitialize、以及带mcpServers: []的 ACPsession/new。 人工 Web UI 验证也已通过:Tested on
环境(可选)
本地 macOS worktree,Node.js 22。使用 dev 入口在
127.0.0.1:4171启动qwen serve --no-web完成验证。风险和范围
关联 Issue
Resolves #6110
Related #6086