feat: support full-turn multimodal routing for image prompts - #7045
Conversation
Phase 1 local acceptance reportThe focused automated acceptance matrix passed:
Evidence:
A real ACP + local mock-provider black-box attempt was made, but the existing ACP bootstrap resolved its default Per the requested local workflow, |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
5afc0ed to
c754cdb
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
Final local review updateThis supersedes the earlier local acceptance comment after the branch was rebuilt through the final local
Per the requested workflow, |
c754cdb to
c009350
Compare
c009350 to
5d57f87
Compare
Review — full-turn multimodal routing (at
|
| # | Endpoint hit | stream | image | tools | tool-result | Response |
|---|---|---|---|---|---|---|
| 1 | vision-agent :9102 |
✓ | ✓ (data-URL) | ✓ full agent tool list | — | glob tool call |
| 2 | vision-agent :9102 |
✓ | ✓ | ✓ | ✓ | final answer — continuation stayed on the exact route |
| 3 | vision-agent :9102 |
side query | ✓ | — | ✓ | suggestion side-query (this target supports images) |
| — | text-model :9101 |
ZERO requests during the image turn |
The routing notice renders (◎ Routing this image turn to openai:vision-agent (127.0.0.1:9102); retries and tool continuations will stay on that model until the turn ends.), the real tool loop runs on the vision model under YOLO approvals, and the next text-only turn returns to the primary — whose requests include the image turn in history with imageBytes=0 in every request: the per-egress media filter demonstrably strips historical image parts for the text-only target.
2. Gate control — same model with {vision: true} only: exactly one non-streaming transcription request to vision-agent (image ✓, no tools), the turn is answered by the primary, notice is the existing "Converted 1 image(s) to text via …" — today's Vision Bridge behavior byte-for-byte, and zero image bytes on the primary across all 5 of its requests.
3. Fail-closed — {vision, agent} pointing at an unreachable endpoint (127.0.0.1:9): the image turn shows the routing notice, then errors (✕ [API Error: Connection error]) — during it zero requests reached any endpoint: no primary fallback, no ordinary fallback chain (getModelFallbacks disabled on exact routes), no raw image egress. The next text turn recovers cleanly on the primary.
Tests
packages/core— all PR-touched suites (geminiChat, client, baseLlmClient, coreToolScheduler, compactionInputSlimming, modelRegistry, config, visionBridge): 1396 passed.packages/cli— useGeminiStream, useToolScheduler, ACP Session: 451 passed (the PR body's exact focused count).
Pre-merge requirement: rebase over #7028 (one small conflict, resolution verified)
git merge-tree origin/main HEAD conflicts only in packages/core/src/models/modelRegistry.ts — #7028 (merged) tightened the explicit-baseUrl lookup to the exact composite key (if (baseUrl !== undefined) return models.get(modelRegistryKey(modelId, baseUrl ?? undefined))), while this PR adds a fallback to a plain-keyed entry whose own default baseUrl equals the requested one. The semantics compose — the fallback only matches the identical route registered under a plain key, so #7028's disambiguation is preserved:
if (baseUrl !== undefined) {
const exact = models.get(modelRegistryKey(modelId, baseUrl ?? undefined));
if (exact) return exact;
const plain = models.get(modelId);
return plain?.baseUrl === baseUrl ? plain : undefined;
}I ran a trial merge with exactly that resolution: modelRegistry.test.ts 75/75 (both PRs' cases) and merged ACP Session.test.ts 271/271. So this is mechanical — please rebase and fold it in.
Non-blocking observations
- Side queries during a full turn bill to the vision model — observed on the wire: the prompt-suggestion side-query ran on
vision-agent(request 如何自定义密钥文件 .env可能与其他文件冲突 #3 above). Consistent with the design ("every outgoing request gets its exact target's modalities"), just worth knowing that a pricier vision model also serves aux calls for that turn whensuggestionModelis unset. - Pipeline-side of the design doc's compression tradeoff is real: with
exactRoute, both proactive hard-tier rescue and reactive overflow compression are skipped (geminiChat.ts), so an oversized image turn fails on the selected model. Documented and reasonable for Phase 1; long agentic image turns will hit it first. - The
\0selector encoding is sound where I probed it:resolveForModel(model.slice(0, -1), {failClosed: true})still parsesmodel\0baseUrl; theperModelGeneratorCachekey now including:open/:closedprevents a cached fail-open main-generator fallback from ever serving a fail-closed call — nice catch;useReactToolSchedulerfails tool calls closed (errorfunctionResponses, recorded to chat recording) when route resolution dies mid-turn, andCoreToolSchedulerre-enters the runtime view on queued/manually-approved confirmations so late approvals keep the route. - Coverage boundary, stated plainly: my E2E covers the TUI end-to-end; the ACP entry point is verified at unit level here (271 Session tests incl. the new full-turn cases) — the same core engine (
resolveForModel+runWithRuntimeContentGenerator+sendMessageStream(modelOverride)) drives both. The author's "credentialed two-provider manual run" gap is now closed by the mock-based matrix above. - Environmental noise in my screenshots (
[crawler] rg ENOENTfallback lines) is my sandbox lacking ripgrep, unrelated to this PR.
Repro sketch (two-endpoint mock harness)
mocks two servers: :9101 role=primary, :9102 role=vision — both serve /v1/chat/completions
stream requests → SSE; vision first-call answers a `glob` tool call, tool-result call answers text+finish
non-stream requests (side queries / transcription) → JSON completion
every request logged: {model, stream, hasImage, imageBytes, hasTools, hasToolResult, msgCount}
config isolated HOME: security.auth.selectedType=openai · tools.approvalMode=yolo · visionModel="openai:vision-agent"
modelProviders.openai=[ {id:text-model, baseUrl::9101}, {id:vision-agent, baseUrl::9102, capabilities:{vision,agent}} ]
env: OPENAI_MODEL=text-model, OPENAI_BASE_URL=http://127.0.0.1:9101/v1, QWEN_CODE_NO_RELAUNCH=1
drive PTY: "@tiny.png what is in this image?" → wait turn → "say hi in plain text" → /quit
scenarios: fullturn {vision,agent} · bridge {vision} · failclosed {vision,agent}@127.0.0.1:9
中文版本(完整翻译)
Review — 整轮多模态路由(基于重建后的 5d57f875)
结论:行为层面 LGTM —— Phase 1 的全部路由声明都在真实 bundle 上端到端验证通过,包括 PR 描述中标记为受阻的黑盒 E2E。 有一项机械性的合并前要求(与 #7028 在 modelRegistry.ts 的小冲突,下文已给出经过测试的解决方案),以及若干不阻塞的观察项。
由于 PR 说明 ACP mock 尝试被鉴权引导卡住,我以维护者身份补齐了缺失的黑盒 E2E:真实打包 CLI 跑在 PTY 中,隔离 $HOME(security.auth.selectedType=openai 绕过鉴权对话框),两个本地 mock OpenAI 端点(文本主模型 text-model 在 :9101,vision-agent 在 :9102),固定 visionModel: "openai:vision-agent",通过 @tiny.png 提交图片,服务端记录每个请求的模型、图片字节数与工具标志。
E2E 矩阵(真实 bundle,线上断言)
1. 整轮路由 — capabilities: {vision: true, agent: true}:图片轮的全部 3 个请求都命中 vision-agent(初始请求带完整 agent 工具列表 → 返回 glob 工具调用;工具结果继续请求留在同一精确路由;轮内建议侧查询同样在该模型);期间 text-model 零请求。路由通知正确渲染,真实工具循环在 vision 模型上执行,下一个纯文本轮回到主模型——且其请求历史虽包含图片轮,每个请求的 imageBytes=0:按出口过滤媒体的声明在线上成立。
2. 能力门控 — 仅 {vision: true}:恰好一个非流式转写请求发往 vision-agent(带图、无工具),轮次由主模型作答,通知为现有的 "Converted 1 image(s) to text via …" —— 与现状行为一致,主模型全部 5 个请求零图片字节。
3. Fail-closed — {vision, agent} 指向不可达端点(127.0.0.1:9):图片轮显示路由通知后报错(✕ [API Error: Connection error])——期间任何端点都收到零请求:不回退主模型、不走普通 fallback 链、无原始图片外发;下一个文本轮在主模型上正常恢复。
测试
packages/core全部 PR 触及套件:1396 通过;packages/cli(useGeminiStream、useToolScheduler、ACP Session):451 通过(与 PR 正文的聚焦计数一致)。
合并前要求:基于 #7028 变基(一处小冲突,解决方案已验证)
git merge-tree origin/main HEAD 仅在 packages/core/src/models/modelRegistry.ts 冲突——#7028(已合并)将显式 baseUrl 查找收紧为精确组合键,本 PR 则增加"回退到默认 baseUrl 恰好匹配的 plain 条目"。两者语义可叠加(回退只匹配以 plain 键注册的同一路由,不破坏 #7028 的消歧),建议解法见英文部分代码块。按该解法试验合并后:modelRegistry.test.ts 75/75(两个 PR 的用例都在)、合并后的 ACP Session.test.ts 271/271。属机械性变基,请作者处理。
不阻塞的观察项
- 整轮期间的侧查询计费在 vision 模型上(线上观察到建议侧查询命中
vision-agent)——与设计一致,但当suggestionModel未设置时,较贵的 vision 模型也会承担该轮的辅助调用,值得知晓。 - 压缩取舍真实存在:精确路由下主动救援与被动溢出压缩都被跳过(
geminiChat.ts),超长图片轮会在所选模型上直接失败。设计文档已如实记录,Phase 1 合理。 \0选择器编码在我探查的范围内是健全的:slice(0, -1)后仍正确解析model\0baseUrl;perModelGeneratorCache键加入:open/:closed防止 fail-open 的主生成器回退结果污染 fail-closed 调用——处理得很细;useReactToolScheduler在路由解析失败时对工具调用 fail-closed(错误functionResponse并写入会话记录);CoreToolScheduler对排队/人工批准的确认重新进入 runtime view,晚批准也不丢路由。- 覆盖边界如实说明:我的 E2E 端到端覆盖 TUI;ACP 入口在单测层面验证(271 个 Session 用例含新增整轮用例)——两者共用同一核心引擎。作者所缺的"双真实供应商手动验证"由上述 mock 矩阵补齐。
- 截图中的
[crawler] rg ENOENT为我沙箱缺少 ripgrep 的环境噪音,与本 PR 无关。
Reviewed & verified on macOS arm64 · Node v22.23.1 · isolated worktree at 5d57f875 · full npm ci + bundle · assets on pr-assets/pr-7045-verify.
ACP multi-turn black-box follow-upI rebuilt the bundle and exercised the actual ACP JSON-RPC process against two local OpenAI-compatible HTTP providers. The same six-turn conversation was run with the vision model first configured without
The complete before/after scenario passed once while building the harness and then passed three consecutive clean repetitions with identical wire-level results.
The extra vision request after the change is the tool-result continuation. No primary request occurs inside the tool loop, and each subsequent text-only turn returns to the primary. This ACP evidence is consistent with the earlier TUI black-box review. One known Phase 1 limitation is now explicit: canonical history retains prior image parts, so later image-agent requests replay historical media. In this scenario the image payload grew from 1 image on the first image turn, to 2 on the next image turn, then 4 on the final multi-image turn. Primary-model egress still strips those images. Selective historical-media cleanup is intentionally deferred because it changes conversation semantics and is outside this PR's routing scope. An independent test-engineer cross-check also started the real The independent negative matrix also passed:
Both runs validate the actual ACP process, HTTP routing, request payloads, and tool continuation. They do not claim visual-answer quality against a credentialed production model pair; no such credentials were available to these isolated test processes.
|
Real tmux/TUI verificationRan the built Full-turn image agentThe image turn routes to Wire evidence:
Existing Vision Bridge controlWith the same vision model configured as Wire evidence:
The deterministic local endpoints validate the real TUI, routing, image egress, and tool continuation. They do not claim production-model visual-answer quality. |
…mal-conflict-fix # Conflicts: # packages/core/src/models/modelRegistry.ts
|
Thanks for the PR! Template looks good ✓ — all required headings present, bilingual, reviewer test plan with evidence and matrix. Problem: real capability gap, not theoretical hardening. Current Vision Bridge transcribes images through a vision model then hands text to the primary. When the vision model is also agent-capable, that's both lossy and redundant. The PR demonstrates this with concrete before/after wire-level evidence (multiple independent E2E runs, maintainer-verified bundle). Related #6988 triaged as Phase 1. Direction: aligned with qwen-code's multi-provider model-routing mission. The explicit Size: production logic ~728 lines (Core ~428 + CLI ~300), tests ~1296 lines (Core ~878 + CLI ~418), docs 53 lines. Touches core modules (models, services, core, config) and cross-package (Core → CLI). Since this is a Approach: the scope is disciplined — Phase 1 only (TUI + ACP), explicit carve-outs for persistence, session recovery, historical media cleanup, and non-interactive routing. The Moving on to code review and live testing. 🔍 中文说明感谢贡献! 模板完整 ✓ —— 所有必需标题齐全,双语,包含带证据和矩阵的 reviewer test plan。 问题:真实的能力缺口,不是理论性加固。当前 Vision Bridge 先用视觉模型转写图片,再把文本交给主模型。当视觉模型同时具备 agent 能力时,这既损失信息又冗余。PR 用具体 before/after 线上证据证明了这点(多次独立 E2E 运行,维护者在 bundle 上已验证)。关联 #6988 已分诊为 Phase 1。 方向:与 qwen-code 的多供应商模型路由定位一致。显式 规模:生产代码约 728 行(Core 约 428 + CLI 约 300),测试约 1296 行(Core 约 878 + CLI 约 418),文档 53 行。触及 core 模块(models、services、core、config)和跨包(Core → CLI)。作为 方案:范围克制 —— 仅 Phase 1(TUI + ACP),明确排除持久化、会话恢复、历史媒体清理和非交互路由。 进入代码审查和实机测试。🔍 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: verification — its prompt was built, but no agent was launched with it that opened its brief, so the posted findings were not verified.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built.
Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built.
Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.
— qwen3.7-max via Qwen Code /review
|
本轮处理:
验证:
|
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] useReactToolScheduler.ts:289 — existing Critical blocker about scheduler.schedule outside inner try/catch; this review's verification confirmed the structure at low confidence (narrow trigger window) Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 9 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 5 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 8 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 2 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 3 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 7 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Re-verification at head
|
| turn | PRIMARY (primary-text) |
VISION (vision-agent) |
result |
|---|---|---|---|
| T1 text‑only | stream · img=0 |
— | → primary ✓ |
| T2 text+image | — | stream · image_url=1 (69B) |
→ vision ✓ |
| ↳ model tool‑call → continuation | — | stream · image_url=1 · toolResult |
route sticky ✓ |
| T3 text‑only (next turn) | stream · img=0 |
— | → back to primary ✓ |
Notice emitted on the image turn: “Routing this image turn to vision‑agent (127.0.0.1); retries and tool continuations will stay on that model until the turn ends.”
Key invariant (raw request bodies): the image (image_url, 69 B) is present only on the vision-agent endpoint — on both the initial turn and the tool continuation — and appears 0 times on the primary-text endpoint across all three turns, including T3 where the image turn is already in history. The complete image‑bearing turn stays on the exact provider/model/endpoint across the tool call, and the next independent turn returns to the primary.
S2 · Transcription fallback — vision-agent { vision:true } (no agent)
| turn | PRIMARY | VISION | result |
|---|---|---|---|
| T1 text+image | stream · img=0 (text only) |
stream=false · image_url=1 (69B) |
Vision Bridge transcription ✓ |
Dropping agent cleanly reverts to the existing transcription path: the image goes to the vision model as a non‑stream side query, and the primary receives only the transcription text. Notice: “Converted 1 image(s) to text via vision‑agent (…). Your image and prompt/context were sent to that model.”
S3 · Fail‑closed exact route — vision endpoint unreachable (127.0.0.1:9)
| turn | PRIMARY | VISION | result |
|---|---|---|---|
| T1 text+image | (none) | (none) | image not leaked to primary ✓ |
| T2 text‑only (recovery) | stream · img=0 |
— | session recovers ✓ |
When the exact pinned route is down, the image turn produces zero requests on either endpoint — the image is never silently downgraded onto the image‑blind primary. The turn surfaces a hard error, and the next turn recovers normally on the primary.
Controlled A/B — this is genuinely new behavior
Same settings (vision-agent { vision:true, agent:true }, pinned, one text+image turn) run against the PR's merge‑base (0ecba4b, pre‑PR) and the PR head:
merge‑base 0ecba4b (pre‑PR) |
PR head 9ab4240 |
|
|---|---|---|
| routing decision | TRANSCRIPTION | FULL‑TURN AGENT |
| vision request | stream=false (side query) |
stream=true (real turn) |
| image reaching vision | dropped → [Unsupported image] text |
image_url 69 B preserved |
| tool continuation route | n/a | stays on vision-agent |
agent:true honored? |
✗ ignored (capability unknown) | ✓ routes the full turn |
Without the PR, the identical agent:true config silently falls back to the Vision Bridge transcription — confirming the full‑turn path is new behavior correctly gated on capabilities.agent.
Regression — PR‑touched suites at head 9ab4240
vitest run (worktree build, real bundle):
- ✓ core
vision-bridge-service.test.ts— 61 - ✓ core
modelRegistry.test.ts— 75 - ✓ core
coreToolScheduler.test.ts— 279 - ✓ cli
useToolScheduler.test.ts— 23 - ✓ cli
acp-integration/session/Session.test.ts— 274
5 files, 712 tests, all passing.
The mechanism behind S1 stickiness is also code‑reviewed: coreToolScheduler/useReactToolScheduler thread a RuntimeContentGeneratorView so tool continuations run inside the vision route's content generator; the ACP #runWithFullTurnModel resolves the null‑terminated model sentinel via resolveForModel({ failClosed:true }) and wraps the tool run in runWithRuntimeContentGenerator; compression is skipped under a model override; and hasAmbiguousRoute disambiguation + displayVisionModelId (clean notice) are in place.
Non‑blocking observations
- Fail‑closed UX asymmetry (S3). On the full‑turn path an unreachable exact route surfaces as a hard turn error (correct — no silent image leak, and the next turn recovers), whereas the transcription path degrades gracefully (“bridge failed, image not interpreted” + a text reply). The fail‑closed choice is the safer default; a friendlier “vision endpoint unavailable” message for the full‑turn case could be a Phase‑2 polish.
- Side queries (Phase‑2, unchanged from my prior note). With
approvalMode: yolothere were no aux/classifier side queries in these runs; whensuggestionModelis unset, suggestion/aux queries during a full turn would run on the (pricier) vision route. Worth a Phase‑2 look, not a blocker here.
Nothing above blocks merge. LGTM at 9ab4240.
中文版本(点击展开)
在 head 9ab4240 的复核 —— 全回合多模态路由
结论:LGTM(可合并)。 我在当前 head(9ab42401b)用重新构建的产物做了独立的端到端复核。所有 Phase‑1 全回合路由的声明在网络请求层面都成立;自我上次评审(5d57f875)以来补齐 review gap 的若干提交是正确的;我当时提的唯一合并前事项 —— 与 #7028 的 modelRegistry.ts 冲突 —— 现已解决:#7028 已合并,且本 head 已经带上了我建议的 精确匹配 → 默认 baseUrl 兜底 的解决方案。对当前 origin/main(620effef)做 git merge-tree 干净、无冲突。
验证方式
在 9ab4240 构建 dist/cli.js,通过 ACP(--acp)驱动它,对接两个进程内的 OpenAI 兼容 mock 端点 —— 纯文本主模型 primary-text 与 vision-agent,并在网络层记录每次 /chat/completions 调用:命中哪个端点、是否 stream、image_url 部件数量、是否包含 role:"tool" 结果。判据是黑盒的:哪个端点收到哪个请求,以及图片字节是否会流到纯文本主模型。
S1 · 全回合 agent 路由(vision-agent { vision:true, agent:true },已 pin):文本回合→主模型;文本+图片回合→vision 端点(stream,携带 image_url 69B);模型发起工具调用后的续回合仍留在 vision 端点(携带 image_url 且带工具结果)——路由粘滞成立;下一个独立文本回合→回到主模型。会话提示:“Routing this image turn to vision‑agent …; retries and tool continuations will stay on that model until the turn ends.” 关键不变量: 图片(image_url,69B)只出现在 vision-agent 端点(首回合与工具续回合都在),在 primary-text 端点的三个回合中出现 0 次(包括图片回合已进入历史的 T3)。
S2 · 转写回退(仅 { vision:true },无 agent):去掉 agent 干净地回到既有的 Vision Bridge 转写路径 —— 图片作为非流式 side query 发给 vision 模型,主模型只收到转写文本。
S3 · 精确路由 fail‑closed(vision 端点不可达 127.0.0.1:9):图片回合在两个端点上都产生零请求 —— 图片绝不会被静默降级发给不支持图像的主模型;该回合报错,而下一个回合在主模型上正常恢复。
对照 A/B —— 这是真正的新行为
相同配置分别跑 PR 的 merge‑base(0ecba4b,PR 之前)与 PR head:merge‑base 走转写路径、agent:true 被忽略(识别不了该能力);PR head 走全回合 agent 路径、图片被完整保留、工具续回合粘滞在 vision-agent。这证明全回合路径是由 capabilities.agent 正确门控的新增行为。
回归测试(head 9ab4240,真实产物)
core:vision-bridge-service(61)、modelRegistry(75)、coreToolScheduler(279);cli:useToolScheduler(23)、Session(274)。5 个文件,712 用例全部通过。 粘滞机制也做了代码走查:调度器透传 RuntimeContentGeneratorView,#runWithFullTurnModel 用 resolveForModel({failClosed:true}) 解析空字符结尾的 model sentinel 并在 runWithRuntimeContentGenerator 中执行工具,model override 下跳过压缩;hasAmbiguousRoute 去歧义与 displayVisionModelId(提示清理)均已就位。
非阻塞观察
- fail‑closed 的 UX 不对称(S3): 全回合路径下,精确路由不可达会以硬错误暴露(这是对的——不静默泄漏图片,且下一回合可恢复);而转写路径是优雅降级。fail‑closed 是更安全的默认;给全回合场景加一条更友好的“vision 端点不可用”提示可作为 Phase‑2 打磨。
- side query(Phase‑2,与上次结论一致):
yolo模式下本次未观察到 aux/classifier side query;当suggestionModel未设置时,全回合中的 suggestion/aux 查询会走到(更贵的)vision 路由,值得 Phase‑2 关注,但不阻塞本 PR。
以上均不阻塞合并。9ab4240 处 LGTM。
Verified on a freshly built bundle at 9ab42401b in an isolated worktree; two‑endpoint ACP E2E harness + merge‑base A/B; screenshots are rendered from the harness's own request logs.
|
@qwen-code /triage |
Code reviewThe implementation is clean and well-structured. Independent proposal before reading the diff: I'd add a capability gate on the vision model, route the full turn through it when agent-capable, thread the route's content generator through tool continuations via the existing AsyncLocalStorage mechanism, and filter unsupported media at each model egress. The PR's approach matches this and exceeds it in several respects — the ambiguous-route detection in Key observations:
No critical correctness bugs, security holes, or regressions found. No over-abstraction or duplication — the changes follow existing patterns. The Unit testsAll 1736 tests across 10 changed-file test suites pass:
Real-scenario testingThe feature requires a text-only primary model + an agent-capable multimodal vision fallback with real API credentials. This environment doesn't have those credentials, so the multimodal routing path can't be exercised end-to-end in tmux. The PR author acknowledges the same limitation. Basic text-path smoke test on the bundled build confirms no regression: Before/After (text-only path, installed build vs PR build)The text-only path works identically — the feature is opt-in and doesn't affect existing configurations. Build passes cleanly ( 中文说明代码审查实现干净且结构良好。在看 diff 之前我的独立方案是:给视觉模型加能力门控,agent-capable 时整轮路由,通过现有 AsyncLocalStorage 机制传递路由的 content generator,在每个模型出口过滤不支持的媒体。PR 的方案与此匹配并在多个方面超越: 关键观察:
未发现关键正确性缺陷、安全漏洞或回归。没有过度抽象或重复——改动遵循现有模式。 单元测试10 个变更文件测试套件共 1736 个测试全部通过。 实机测试该功能需要文本主模型 + agent-capable 多模态视觉 fallback 及真实 API 凭证。此环境没有这些凭证,因此无法在 tmux 中端到端测试多模态路由路径。PR 作者也承认同样的限制。 基础文本路径冒烟测试确认无回归:bundle 构建正常(v0.19.11),简单文本 prompt 返回正确响应。功能为 opt-in,不影响现有配置。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 4/5 — solid implementation of a real capability gap, clean code, comprehensive tests; only reservation is that E2E multimodal routing couldn't be verified in this environment (same limitation the author acknowledges). The PR does what it says: when a vision model is explicitly agent-capable, the complete image turn stays on that exact provider/model/endpoint across retries, tool calls, and continuations. Existing configurations are completely unaffected — the opt-in gate ( What I liked:
What I'd flag for awareness (non-blocking):
Tests all pass, CI is green, build is clean. The maintainer ( 中文说明置信度:4/5 —— 对真实能力缺口的扎实实现,代码干净,测试全面;唯一的保留意见是无法在此环境中端到端验证多模态路由(与作者承认的限制相同)。 PR 做了它承诺的事:当视觉模型明确声明 agent-capable 时,完整图片回合在重试、工具调用和续传中始终留在精确 provider/model/endpoint 上。现有配置完全不受影响——opt-in 门控( 亮点:
非阻塞的提醒:
测试全部通过,CI 绿色,构建干净。维护者( — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code review summary (qwen3.7-max)Incremental re-review of PR #7045 after the latest commit. 0 new findings from 11 dimension agents + 2 rounds of reverse audit.
|







What this PR does
When the primary model is text-only and the configured vision fallback explicitly declares both
capabilities.vision: trueandcapabilities.agent: true, this change routes the complete image-bearing turn through that model's exact provider, model, and endpoint.The selected route remains fixed across provider retries, output continuations, tool execution, tool results, queued or manually approved tool calls, and blocking ACP Stop Hook continuations. The next independent user turn returns to the primary model. Models without the explicit agent capability continue to use the existing Vision Bridge transcription flow.
Every outgoing model request, including side queries and fallback attempts, now receives only the media modalities supported by its exact resolved target. Exact-route resolution failures fail closed instead of sending raw image data to another model.
The implementation is intentionally limited to Phase 1 in the interactive TUI and ACP. It does not add route persistence, restart or session recovery, durable visual summaries, stable image references, historical media cleanup, later image reinspection, or non-interactive routing.
Why it's needed
Vision Bridge currently asks a vision model to transcribe an image and then asks the text-only primary model to produce the final answer. That remains the safe default for OCR- or description-only fallbacks, but it is unnecessarily lossy and adds a second generation when the configured multimodal model can already follow the normal agent policy and use tools.
An explicit capability gate lets those models own the complete image turn without changing existing configurations or silently sending raw image data to an unresolved primary, side-query, or fallback route.
Reviewer Test Plan
How to verify
capabilities.agent; submit an image and confirm the existing Vision Bridge transcription notice and behavior remain unchanged.capabilities.agent: true; submit an image and confirm the original image reaches the selected multimodal model, a full-turn routing notice is shown, and no transcription is performed.Automated validation completed locally:
maincutoff660ae9f712, 17 critical Core routing/runtime cases and 4 exact CLI lifecycle cases passed again. An earlier post-rebase pass also covered 438 Core cases and the full 451 CLI cases.maincutoff it is blocked by two errors already present in unchanged upstream code: an unsupportedparentContextoption in ACP session registration and an extra telemetry argument in the serve package.git diff --checkpassed./btwmedia-filtering layer and its test.main: 26 files, production+578/-99, tests+1074/-34, docs+53/-0; no route persistence or restart/session-recovery logic is included.Per the requested local workflow,
npm run verify:prwas not run.Evidence (Before & After)
Before: a text-only primary always uses the vision fallback as a transcription service, even when that model can run the complete agent loop.
After: an explicitly opted-in agent-capable multimodal model owns the complete image-bearing turn and its tool runtime, while the next independent text turn returns to the primary. Existing configurations keep the transcription path, and unsupported media is filtered at each exact model egress.
Credentialed manual TUI execution with two real providers was not available locally. A local ACP + mock-provider black-box attempt was also blocked before routing by the existing ACP bootstrap resolving its default model and requiring authentication, so it is not presented as E2E evidence. This PR remains a draft and uses focused automated routing coverage as acceptance evidence.
Tested on
Environment (optional)
Node.js v22.22.0 on macOS arm64, using package-level Vitest, TypeScript, ESLint, Prettier, and the repository build.
Risk & Scope
capabilities.agent: trueopt-in; existing model configurations retain Vision Bridge behavior.Linked Issues
Related to #6988. Implements the triaged Phase 1 scope only.
中文说明
此 PR 做了什么
当主模型不支持图片,且配置的视觉 fallback 明确声明
capabilities.vision: true和capabilities.agent: true时,本改动会把包含图片的完整回合路由到该模型的精确 provider、model 和 endpoint。所选路由会在 provider 重试、输出续写、工具执行、工具结果、排队或手工批准的工具调用以及阻塞式 ACP Stop Hook 续传中保持不变。下一个独立用户回合会恢复使用主模型。没有显式 agent capability 的模型继续使用现有 Vision Bridge 转写流程。
每个发出的模型请求(包括 side query 和 fallback 尝试)现在只会收到其精确目标支持的媒体类型。精确路由解析失败时会安全失败,不会把原始图片发送给其他模型。
实现范围被刻意限制在交互式 TUI 和 ACP 的 Phase 1。它不包含路由持久化、重启或会话恢复、持久视觉摘要、稳定图片引用、历史媒体清理、后续图片重检或非交互路由。
为什么需要它
Vision Bridge 当前会先让视觉模型转写图片,再让文本主模型生成最终回答。对于只适合 OCR 或图片描述的 fallback,这仍然是安全默认行为;但当配置的多模态模型已经能遵循正常 agent policy 并使用工具时,这种方式会产生不必要的信息损失,并额外增加一次生成。
显式能力门控允许这些模型处理完整图片回合,同时不改变现有配置,也不会把原始图片数据静默发送给无法解析的主模型、side query 或 fallback 路由。
Reviewer Test Plan
如何验证
capabilities.agent的图片 fallback,提交图片,确认现有 Vision Bridge 转写提示和行为保持不变。capabilities.agent: true,提交图片,确认原始图片发送到所选多模态模型,显示整轮路由提示,并且不执行转写。已完成的本地自动验证:
main截止点660ae9f712后,再次通过 17 个关键 Core 路由/运行时 case 和 4 个精确 CLI 生命周期 case;此前一轮 rebase 后还通过了 438 个 Core case 和全部 451 个 CLI case。main截止点上,被未修改的上游代码中已有的两个错误阻断:ACP session registration 使用了不受支持的parentContext选项,以及 serve telemetry 多传了一个参数。git diff --check通过。/btw媒体过滤层及对应测试。main的最终范围审计:26 个文件,生产代码+578/-99、测试+1074/-34、文档+53/-0;不包含路由持久化或重启/会话恢复逻辑。按照本次约定的本地流程,没有运行
npm run verify:pr。证据(Before & After)
Before:即使视觉 fallback 能运行完整 agent loop,文本主模型仍然只把它当作转写服务。
After:显式 opt-in 的 agent-capable 多模态模型处理完整图片回合及其工具运行时,下一个独立文本回合恢复主模型。现有配置继续使用转写路径,并且每个精确模型出口都会过滤不支持的媒体。
本地没有可用的双真实 provider 凭证来执行手工 TUI 验收。本地 ACP + mock provider 黑盒尝试也在进入路由前被现有 ACP bootstrap 的默认模型解析和认证要求阻断,因此不把它作为 E2E 证据。此 PR 保持 Draft,并使用定向自动路由覆盖作为验收证据。
测试平台
环境(可选)
macOS arm64,Node.js v22.22.0,使用 package-level Vitest、TypeScript、ESLint、Prettier 和仓库构建。
风险与范围
capabilities.agent: trueopt-in;现有模型配置保留 Vision Bridge 行为。关联 Issue
关联 #6988,仅实现已分诊的 Phase 1 范围。