Repository navigation
fix(desktop): 图片输入三态声明贯通投影与设置入口（#6243 #6275） - #6333
Battleplus wants to merge 11 commits into
Conversation
- 目录投影只透传 supportsImageInput === true，显式声明的 false 被丢成 undefined：设置里选的「不支持」对发图本地拦截、原生请求体与徽标判定 全部失效（makecindy#6243 诉求 1） - 模型高级抽屉对自定义供应商新增「支持图片输入」三态下拉（支持/不支持/ 未声明），写回该模型所属的每个引擎；未声明时删除键恢复「缺省不猜」 的历史口径；内置供应商保持只读 - 拦截文案引用的「支持图片输入」设置自此真实存在（makecindy#6243/makecindy#6275） 测试：model-providers 1298 passed；maker-core pi 1379 passed/113 skipped； desktop Drawer 50 passed（新增 3 条）+ 主进程 7 套件 360 passed；tsc 双 0 错 Fixes makecindy#6243 Fixes makecindy#6275 Signed-off-by: Battleplus <3559424769@qq.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 [Medium impact] The change reaches custom-provider image-input settings and their catalog behavior. 合并前仍需修复连续修改设置时相互覆盖的问题。 Findings
Fix with agent prompt### Issue 1
apps/desktop/src/renderer/components/settings/ModelAdvancedDrawer.tsx:405-407
图片输入保存只检查 `imageInputSaving`，不会等待协议保存完成。用户修改协议后，如果在保存完成、目录刷新前再修改图片输入，两次请求会从同一个旧 `provider` 复制并提交整份配置。后一次写入会撤销刚修改的协议，主进程串行保存也无法修复请求中已经带上的旧值。
请让这些整份配置的保存共用互斥状态，并在新快照到达后才允许下一次修改，或改为在主进程只更新目标字段。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0a61cd69f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!model) continue; | ||
| if (value === 'supported') model.supportsImageInput = true; | ||
| else if (value === 'unsupported') model.supportsImageInput = false; | ||
| else delete model.supportsImageInput; |
There was a problem hiding this comment.
For a preset-backed Pi custom provider that still has piCatalogProviderId, first choosing supported/unsupported makes invalidateEditedProviderMetadata drop that marker because supportsImageInput is one of the compared catalog fields. After the refreshed, markerless view arrives, choosing undeclared reaches this deletion but has no source from which to restore the marker, so user-provider and pi-host no longer use the matching official Pi catalog even though the control claims to return to the inherited/default state. Rehydrate the marker from the matching preset/original baseline when clearing this override.
AGENTS.md reference: AGENTS.md:L180-L181
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
这条本次不处理，想先请维护者定个方向：清空声明时回填 piCatalogProviderId 需要预设基线来源，而 invalidateEditedProviderMetadata 把 supportsImageInput 也算进目录字段比较——所以先选 supported/unsupported、再选 undeclared 时标记已经被摘掉，undeclared 分支没有来源可以取回。可选方向：① 清空时从匹配的预设重新解析并回填该标记；② 让 supportsImageInput 不参与这个标记的失效判定。确认走哪条我另开 PR 做。
There was a problem hiding this comment.
看过了 invalidateEditedProviderMetadata() / preservesPiCatalogModels()，确认现象存在，但我倾向认为「清空声明时回填 marker」与上游现在的 marker 契约冲突，需要维护者拍板，所以本轮先不改。
具体链路：marker 的落盘条件写在 custom-provider-store.ts:705-709——「官方目录标记只适用于用户尚未改写的预设快照」；preservesPiCatalogModels()（packages/model-providers/src/pi-catalog-marker.ts:93）比对的投影字段里包含 supportsImageInput（projectedPiCatalogFields，同文件 :74）。所以：
- 先选 supported/unsupported → 与预设快照的投影字段不再一致 → main 按契约删掉 marker。这一步是预期行为（用户改写了目录字段）。
- 再选 undeclared → 本 PR 只是
delete model.supportsImageInput，而此时 provider 视图里 pi runtime 已经没有piCatalogProviderId，回填没有来源。
要修就得重新定义「清空覆盖 = 回到目录快照吗」。我担心的点是：marker 一旦恢复，Pi 启动时会用官方模型整条覆盖用户配置（invalidateEditedProviderMetadata 的注释正是这个顾虑），而此时用户可能还留着别的目录字段覆盖（reasoning / contextWindow 等）只是没走这个抽屉入口。也就是「回填 marker」会把第 1 步已经判定为『用户已改写』的状态又翻回『未改写』。
如果维护者的口径是：
- (a) 「显式清空即等于回到未改写」→ 我来实现：清空时先按
preservesPiCatalogModels()校验当前配置与预设快照是否只剩这一项差异，是则恢复 marker； - (b) 维持现状（清空只恢复「缺省推断」，marker 不复活）→ 那是 UX 文案问题，我把 undeclared 的文案从「跟随目录/名称推断」改成更准确的说法（不再暗示回到官方目录快照），然后关闭这条。
请二选一，我再按结论收尾，在此之前保持 open。
There was a problem hiding this comment.
On current HEAD b11571e, clearing the explicit image-input declaration should restore the inherited Pi catalog state. Rehydrate piCatalogProviderId from the matching preset/original baseline when clearing; keep explicit supported/unsupported changes subject to normal catalog invalidation. Please cover supported -> undeclared in a regression. This thread remains open until the behavior is implemented.
新增的「支持图片输入」三态下拉触发器与同抽屉既有 wireProtocol / imageCapability / videoCapability 触发器同款 rounded-full pill （DESIGN.md §4 Select & Dropdown 触发器），按 pnpm design:inventory 重新生成 GENERATED 区块，修复 design-inventory CLI --check。 验证：node --test scripts/__tests__/design-inventory.test.mjs 52/52 passed； node scripts/design-inventory.mjs --check 通过。 Signed-off-by: Battleplus <3559424769@qq.com>
MagicLizi
added
touches:product-ui
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
PR 格式门未通过（formatPass=false）：Description 缺少模板要求的段落
本次为格式门自动打回，绑定本轮 head 8b26d55b734a70a3d9edb71d944caacfcb33b590。打回只针对 PR 描述格式，不涉及代码评审结论。
问题（事实）
.github/PULL_REQUEST_TEMPLATE.md 要求的三个段落，在本 PR Description 中缺失或未按模板组织：
- 这次改了什么（摘要 / 变更类型 / 范围）——正文现有「问题 / 根因 / 改动」内容基本覆盖，但未按模板段落组织；变更类型勾选、范围（明确不包含 / 用户可见变化 / 是否存在 breaking change）未填。
- 怎么验证的（自动验证 / 手工验证 / 未执行的验证）——正文「测试」小节只覆盖自动验证；缺「手工验证」与「未执行的验证」。
- 风险（风险分类 / 影响与回滚）——完全缺失。本次改动涉及设置界面与
packages/model-providers投影语义（supportsImageInput === false由丢弃改为透传），请写清影响范围与回滚 / 降级方式；确认无风险也要明确写「无已知风险」。
怎么修
不需要新 commit：在 PR 页面 Edit 描述，按模板补齐上述三段（现有「问题 / 根因 / 改动 / UI 变化 / 测试」内容可直接搬入对应段落；「UI 变化」中引用 DESIGN.md §4 的说明请保留在模板的「UI 变化」小节）。改完后回复本 review 或请重新审查即可。
补充提醒（非阻断，不构成本次打回原因）
- 本 PR 命中 UI 路径（
apps/desktop/src/renderer/components/settings/ModelAdvancedDrawer.tsx）但描述未附界面效果证据：建议补改动后效果截图 / 录屏（注明平台），或改动后界面的 HTML，便于核对是否符合docs/design-rules/DESIGN.md。 - 信息位：PR 上仍有 2 条未解决的 bot thread（greptile P1、Codex P2，同一主题：整份
CustomProviderConfig保存与并发修改互相覆盖的竞态），需要处理并在对应讨论上点 Resolve conversation，否则合并前仍会被未解决 thread 拦住：
…-input-declaration
…ge-input tri-state The model advanced drawer had three controls that each copied the whole CustomProviderConfig, mutated one field and wrote the whole thing back: the wire protocol, the image/video capability and (added in this PR) the image-input declaration. Each had its own saving flag, so changing two of them before the provider snapshot came back sent two requests built from the same stale provider; the second one silently restored the field the first had just changed. The main process only serializes those writes, it does not merge them, so the fix has to happen before the request. All three now go through one lock (a ref, so two clicks in the same tick cannot both get through) and the config written back by a successful save becomes the baseline for the next one until a fresh provider snapshot arrives. Also drop the `supportsImageInput !== undefined` spread from toCatalogModel: `runtimeUserModelMetadata` already copies an explicit `false` and `applyModelMetadata` lands it on the catalog model, so the spread changed nothing. Measured across three input shapes (plain, registry v4, discovered metadata) the output was byte-identical with and without it, which made the test that asserted it a no-op. The test now pins the path that really carries the value, plus the case where an explicit false strips `image` from discovered modalities. Fixes makecindy#6243 Fixes makecindy#6275 Signed-off-by: Battleplus <3559424769@qq.com>
…nput-declaration Signed-off-by: Battleplus <3559424769@qq.com> # Conflicts: # docs/design-rules/design-inventory.md
Windows CI 分片 4/4 上 "0 ms child delay" 用例偶发失败：coordinator 自身 启动与收尾不计入预算，慢 runner 上嵌套 powershell.exe 冷启动超过 9.5s 的 子超时，导致 D/F 行未产出、只剩 timeout 诊断。将正例子层预算 10s→30s、 外层进程超时 30s→60s（须大于 coordinator 预算，否则外层先杀）；断言与 反例、生产默认预算（1250ms/3000ms）均不变。 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com> Signed-off-by: Battleplus <3559424769@qq.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3bb570ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| const result = await updateCustomProvider(config, {}, { source: 'manual-settings' }); | ||
| if (!result.ok) { toast.error(t('settings.providers.custom.toast.saveFailed')); return; } | ||
| pendingConfigRef.current = { base: provider, config }; |
There was a problem hiding this comment.
When a protocol save changes a model route that has discoveredCost, main's invalidateEditedProviderMetadata deliberately removes that route-bound quote (apps/desktop/src/main/maker-host/custom-provider-store.ts:708-725), but this line caches the unnormalized request as the next baseline. If the user changes another drawer field before the refreshed provider prop arrives, the second request re-sends the old quote; because the stored model now has no prior quote, the main guard accepts it, attaching pricing from the old endpoint to the new route. Rebase on the canonical persisted config (or return it from the IPC) rather than the request.
AGENTS.md reference: AGENTS.md:L29-L32
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
这条还没改，说明一下卡点：主进程 updateCustomProvider 目前只回 { ok: true }，不返回归一化后的 config，renderer 拿不到 main 真正落库的结果（invalidateEditedProviderMetadata 摘掉 discoveredCost 之后的值）。按建议做需要扩 CustomProviderUpdateResult 回传 canonical config，跨 main / preload / renderer 三层，想单独一个 commit 提，避免和这次合并上游的冲突解决混在一起。如果更倾向「main 只更新目标字段」那个方向，我按那个改。
There was a problem hiding this comment.
确认仍然成立，本轮没有改 —— 但先说清楚为什么它不能在这个 PR 里随手修掉，请维护者定一下口径。
复述一下链路：saveProviderConfig() 在成功写回后把这次的请求体记成下一次的基线（pendingConfigRef），目的是「provider 快照异步回来之前的连续修改要叠加而不是互相覆盖」。而 main 的 invalidateEditedProviderMetadata() 会按 route 变化主动删掉 discoveredCost（custom-provider-store.ts:708-725，注释写的是 “Discovery quotes belong to a route. Editing an endpoint must not relabel its old price.”）。于是基线里还留着已被 main 判为失效的旧报价，第二次保存把它重新发回去，此时库里的 model 已经没有报价，验收通过 → 旧端点的价格挂到了新路由上。
要修，只有两条路，都不是这个 PR 能单方面决定的：
- 让 IPC 回带规范化后的落盘 config（
CustomProviderUpdateResult目前只有{ ok: true }，shared/customProviderUpdate.ts:14）—— 基线直接用权威落盘值，最干净，但动了 IPC 契约，影响所有updateCustomProvider调用方。 - 在 renderer 复刻一份 main 的失效规则（route key 比对 + 删
discoveredCost）—— 改动小，但把 main 的裁决逻辑复制到 renderer，两边以后容易漂，我不认为这是好方案。
另外还有一条更简单的「不叠加、直接等快照」路线，但那会退化成上一版的问题（快照回来前的连续修改互相覆盖），正是本 PR 要解决的。
倾向方案 1。请维护者确认后我再动；在那之前这条保持 open。
|
@Battleplus 这个 PR 因 review conversation 未 Resolve，已等待约 49 小时，当前仍有 6 条未解决讨论。 如果已按意见修改并验证，请在对应讨论中说明处理结果，再点击 Resolve conversation；如果问题还未解决，请先处理。仅回复“已修复”不会关闭讨论。 全部解决后，下一轮自动审查会重新检查 PR；仍需满足代码审查、CI 和其他合并条件。 |
|
本 PR 需要维护者确认（自动审查流程）。 命中 product 触发： 本次改动概述：
请维护者直接在本 PR 上 Approve 确认；需要修改则 Request Changes 写明要求，作者改完后重新 Approve。 讨论 issue:#6890 |
…-input-declaration 上游在协议下拉上新增了「接口配置」概念（availableInterfaces / interfaceId / profileId），与本地分支为抽屉三处整份写回加的统一锁落在同一处，冲突按语义并集解： - setModelApi 保留上游的接口优先分支（api === 'auto' 或命中 profileId 时写 interfaceId 并清掉 api / piApi / route），其余分支仍写 api / piApi + 全新 route；两条路径都改为走 saveProviderConfig，与协议/能力/图片输入三态共用同一 把锁，连续修改不再互相覆盖。上游那句独立 setProtocolSaving 由该锁取代。 - ModelAdvancedDrawer.test.tsx 两侧均为新增用例（本分支的图片输入三态与并发 保存，上游的报价摘要/折扣），全部保留。 - design-inventory.md 为生成文件，按 scripts/design-inventory.mjs 重新生成 （54 个 surface），--check 通过。 Signed-off-by: Battleplus <3559424769@qq.com>
|
已合并 冲突解决（3 处）
本轮修复的 review 意见 新增 commit： 验证
仍开着 2 条讨论
其余 4 条已按处理结果说明并 resolve。 |
MagicLizi
removed
touches:product-ui
MagicLizi
added
touches:product-ui
…nput-declaration Resolve generated design-inventory.md by regenerating (54 surfaces); ModelAdvancedDrawer.tsx and its test auto-merged. ModelAdvancedDrawer.test.tsx: 77/77. Signed-off-by: Battleplus <3559424769@qq.com>
…3-6275-image-input-declaration 冲突只有一处生成文件 docs/design-rules/design-inventory.md：本分支与上游都改了同一批 surface 行，按仓库约定取上游版本（本 PR 不新增/删除任何 design surface： ModelAdvancedDrawer 与 ComposerQuoteNode 都已在既有行里，改动只涉组件内部， 不影响 surface 统计）。 apps/desktop/src/renderer/components/settings/ModelAdvancedDrawer.tsx 两侧都改过但可 三路自动合并（本 PR 的图片输入三态声明 + 上游新增的 ModelCompactionSettings 区块）， 未丢弃任一侧内容。 验证：apps/desktop/src/renderer/components/settings/__tests__/ModelAdvancedDrawer.test.tsx 77 passed（本机 worktree，含上游新增的 ModelCompactionSettings 真实文件）。 Signed-off-by: Battleplus <3559424769@qq.com>
MagicLizi
added
touches:core
上游推进导致的冲突处理（语义并集，未丢弃任一侧改动）： - ModelAdvancedDrawer.test.tsx：上游新增「报价组件命名」用例（labels primary quotes without inferring a category）与本 PR 的图片输入三态 / 并发保存用例 落在同一处，两侧全部保留（83 用例）。 - 行尾规范化：本分支此前经 API 推送时把 4 个文件写成了 CRLF（上游为 LF）， 每次合并都会整文件冲突。本次把 user-provider.ts、user-provider.test.ts、 maker-core 的 windows-git-path 测试、ModelAdvancedDrawer.test.tsx 统一回 LF，避免后续合并再出现整文件冲突。 - design-inventory.md：生成文件，按 scripts/design-inventory.mjs 重新生成 （59 个 surface），--check 通过。 验证：ModelAdvancedDrawer.test.tsx 83/83、user-provider.test.ts 78/78、 maker-core windows-git-path 56/56、tsc -p apps/desktop 无 error TS。 Signed-off-by: Battleplus <3559424769@qq.com>
|
已合并 本轮冲突解决（1 处 + 1 个历史隐患）
另外修掉一个会反复造成冲突的历史问题：本分支此前通过 API 推送时把 4 个文件写成了 CRLF（上游是 LF），导致每次合并这 4 个文件都整文件冲突、无法自动合并。本次把它们统一回 LF（
验证
Windows CI本 PR 的 |
MagicLizi
added
touches:rules
ModelAdvancedDrawer 的「支持图片输入」一行两侧都重写了：本 PR 给用户自定义 供应商加三态声明入口（写进每个 runtime 的 supportsImageInput），上游 a215440/c71830e 另起 model-catalog-overrides.json 加了 per-model override。 两者落地位置不同（provider config vs 独立 override 存档），按语义并集保留两条 路径、同一时刻只显示其一： - provider.source === 'user' 且非 native/sidecar：本 PR 的三态下拉（含 undeclared 即删除键回缺省），沿用 makecindy#6243 的拦截提示文案引用这条设置。 - 其余 conversational 且非企业托管供应商：上游新增的 override 下拉（default / supported / unsupported，含 mixed 态与保存失败提示），行列 label 与报错 alert 一并保留。 - 都不是：既有的只读 CapabilityValue。 design-inventory.md 用 scripts/design-inventory.mjs 重算并过 --check。 Signed-off-by: Battleplus <3559424769@qq.com>
|
已 rebase 到上游 并集的处理
两者落盘位置不同（provider config vs 独立 override 存档），所以保留两条路径、同一时刻只显示其一：自定义供应商走本 PR 的三态；其余
验证
未处理的旧讨论（上一轮已写明，等维护者在 |
问题
根因
buildUserProvider的目录投影只透传supportsImageInput === true，显式声明的false被丢成undefined：发图本地拦截、原生请求体与徽标全部退回「未知/放行」，用户声明的「不支持」实际不生效。改动
packages/model-providers/src/user-provider.ts：投影改为!== undefined即透传，true/false原样进目录层，缺省仍不猜（保留防 Pi 静默降级截图的口径）。ModelAdvancedDrawer：自定义供应商新增「支持图片输入」三态下拉（支持 / 不支持 / 未声明），写回该模型所属的每个引擎的模型配置；选「未声明」时删除键恢复缺省；内置供应商保持只读。modelSupportsImageInput*、advanced.supported/unsupported/undeclared，5 个语言包均已存在）。UI 变化
rounded-fullpill、text-secondary、hover surface、ChevronDown 12px），面板复用共享DropdownMenuContent（12px 面板 +--shadow-menu，§4「Menu and popover panels」）；选项文案复用既有advanced.supported/unsupported/undeclared，无新增字号与颜色（§3 桌面 UI 字号白名单）。新增 pill 触发器使desktop.settingsradii 计数 553→554，已按pnpm design:inventory重新生成docs/design-rules/design-inventory.md并通过design-inventory --check（非视觉新样式，为既有登记值的复用）。测试
model-providers全量 1298 passed（新增投影三态透传用例）maker-corepi 全量 1379 passed / 113 skippedModelAdvancedDrawer50 passed（新增 3 条：三态写回全引擎、未声明还原、内置只读）+ 主进程相关 7 套件 360 passednode --test scripts/__tests__/design-inventory.test.mjs52/52 passed；node scripts/design-inventory.mjs --check通过tsc --noEmit：desktop 与 model-providers 均 0 错误upstream/main（91f1c5d6b5）上验证Fixes #6243
Fixes #6275