Skip to content

feat(desktop): add isolated Pricing settings editor - #2218

Closed
me2seeks wants to merge 7 commits into
apache:mainfrom
me2seeks:feat/2015-pricing-settings-ui
Closed

feat(desktop): add isolated Pricing settings editor#2218
me2seeks wants to merge 7 commits into
apache:mainfrom
me2seeks:feat/2015-pricing-settings-ui

Conversation

@me2seeks

@me2seeks me2seeks commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR lands the isolated M4 Pricing renderer/editor slice on top of the Host-backed semantic adapter added by #2148 and #2024.

  • Add an effective Pricing table with Builtin/Custom provenance and add, edit, reset, and delete flows through DesktopPricingSettingsPort.
  • Preserve raw drafts and exact-key semantics, including blank optional values versus explicit zero.
  • Make revision conflicts, uncertain mutation outcomes, refresh/write races, and Host replacement explicit. Writes are never replayed automatically; the user reviews against fresh authority before retrying.
  • Add English/Chinese copy, responsive dialogs, focused model/gate/boundary tests, and nine Storybook states covering loading, errors, provenance mixes, validation, conflicts, uncertain outcomes, and narrow layouts.
  • Remove the unused UsageStats.pricing projection so Pricing has one Host-backed source of truth.

Part of #2015.

Verification

  • mise exec node@24.18.1 -- npm run lint
  • mise exec node@24.18.1 -- npm run format:check
  • mise exec node@24.18.1 -- npm --workspace @maka/desktop run typecheck
  • mise exec node@24.18.1 -- npm --workspace @maka/desktop run build:renderer
  • mise exec node@24.18.1 -- npm --workspace @maka/desktop run build-storybook
  • Desktop compiled test suite: 1,716 passed, 0 failed.
  • Core suite: 781 passed, 0 failed.
  • Storage suite: 681 passed, 14 skipped, 0 failed on the final full run. An earlier run hit the process-crash fixture timing path once; that test passed in isolation and in the next full run.
  • Storybook browser QA covered add/edit/reset/delete, conflict review, uncertain-outcome recovery, focus restoration, and a 480 x 720 viewport with no horizontal overflow or console errors.

Scope

This PR intentionally does not activate Pricing in the production SettingsModal and does not add a legacy IPC path. The copyable exact-key source from Host-backed Usage/Inspector remains a follow-up acceptance item before #2015 can close; production activation remains part of the M5 cutover.

中文说明

摘要

本 PR 在 #2148#2024 提供的 Host-backed 语义适配器之上,实现独立的 M4 Pricing renderer/editor 切片。

  • 新增有效 Pricing 表格,展示 Builtin/Custom 来源,并通过 DesktopPricingSettingsPort 支持新增、编辑、重置和删除。
  • 保留用户的原始输入与精确 key 语义,明确区分可选空值和显式 0
  • 显式处理 revision 冲突、结果不确定、刷新/写入竞态及 Host 替换。写操作不会自动重放,用户必须基于最新 authority 复核后再试。
  • 新增中英文文案、响应式对话框、聚焦的 model/gate/boundary 测试,以及覆盖加载、错误、来源组合、校验、冲突、不确定结果和窄屏的九个 Storybook 状态。
  • 移除未使用的 UsageStats.pricing 投影,使 Pricing 只保留一个 Host-backed 事实来源。

这是 #2015 的一部分。

验证

  • mise exec node@24.18.1 -- npm run lint
  • mise exec node@24.18.1 -- npm run format:check
  • mise exec node@24.18.1 -- npm --workspace @maka/desktop run typecheck
  • mise exec node@24.18.1 -- npm --workspace @maka/desktop run build:renderer
  • mise exec node@24.18.1 -- npm --workspace @maka/desktop run build-storybook
  • Desktop 编译后测试:1,716 项通过,0 项失败。
  • Core 测试:781 项通过,0 项失败。
  • Storage 最终全量运行:681 项通过、14 项跳过、0 项失败。此前一次运行触发了 process-crash fixture 的时序路径;该用例单独重跑及下一次全量运行均通过。
  • Storybook 浏览器实测覆盖新增、编辑、重置、删除、冲突复核、结果不确定恢复、焦点恢复及 480 x 720 窄屏布局,未发现横向溢出或控制台错误。

范围

本 PR 不会在生产 SettingsModal 中提前激活 Pricing,也不会新增 legacy IPC 路径。来自 Host-backed Usage/Inspector 的可复制 exact-key 来源仍是 #2015 关闭前的后续验收项;生产激活仍属于 M5 cutover。

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for this — I reviewed it from four angles (design/boundaries, implementation, UI, tests) and the core is genuinely solid: authority stays with the Host (the renderer snapshot is a pure mirror, base is sent up unchanged, uncertain outcomes discard the snapshot entirely), the never-replay contract is the right call for a mutation surface, the exact-key semantics (blank vs explicit 0) are pinned by tests, the UsageStats.pricing removal is verified dead code (never persisted since #1266, hardcoded []), and the two "unrelated" test-file changes are just the type-removal ripple. All 12 new tests pass and would fail on the parent. Non-blocking notes below, plus one merge blocker:

Merge blocker — the branch conflicts with main on runtime-host-client.ts, and the resolution has a trap. git merge-tree shows the only conflict is that file. In the second hunk the PR side is empty (the pricing types moved to shared/runtime-host-pricing.ts), so resolving by "taking the PR side" would silently delete DesktopSkillCatalogSnapshot and the skill-catalog additions that landed on main (#2216). A rebase onto latest main that keeps main's additions and the PR's shared-type migration (the re-export is compatible — verified by typechecking runtime-host-client-pricing.test.ts against the PR tree) is all that's needed; after rebase app-shell.tsx's branchPicker drift should also clear.

P2 — PricingNarrow never renders narrow. parameters.viewport.defaultViewport does nothing in Storybook 10 (FIDELITY.md documents this exact pattern as inert), so the story renders PricingMixed's pixels at the default 1280px width. The panel's 480px behavior is entirely unverified. Fix options: register a floor-viewport surface in product-smoke-manifest.json (the dailyReviewSettingsBounds pattern), or have the play assert the iframe width — or drop the story and say so.

P2 — the destructive/recovery branches have zero automated coverage. PricingDeleteDialog (delete/reset confirm + confirmAgain re-confirm), the ready empty state, and the saved/saved_refresh_failed/synchronized/stale-snapshot outcomes have no story and no component test, and the story fake can't produce most of them (it never throws pricing_snapshot_stale, never returns saved_refresh_failed or synchronized). A scripted-port component test or a couple of extra stories would close the riskiest surface of the panel.

P2 — the // Real path: 设置 → 使用统计 → 定价配置 annotations describe a state production can't reach yet. Production passes no pricingPort until M5, so the real user sees the legacy empty table (with the hardcoded 0 badge and the empty-state copy promising "在此可为特定模型登记自定义价格"). The M4 slice itself is coherent — but the annotations should say "M5-preview, story-only today" so reviewers and future measurement don't mistake the panel for the production state (two of us independently misread it).

P2 (cheap) — PricingSettingsOperationGate is a third parallel sync primitive. action-guard.ts's header promises the re-entrancy machinery exists "once", and this is a new seam rather than an extension (the gate genuinely serializes more than the ticket pattern — the refresh button isn't disabled during refreshing, and the completion handlers share refs — but that rationale isn't in the code). A one-line comment tying it to the existing guards (or a shared primitive) would settle it.

P3 (optional): the host/renderer copies of pricingTargetMatchesSnapshot can drift (no_override on the host side only); cancel-after-review-delete leaves a warning banner with no action surface; edit-mode dialog focuses a disabled input; editor.saving copy key is dead; rate formatting (String(rate)$1e-8) diverges from the sibling page's .toFixed(2) and no EN-locale story exercises the long strings.

None of this blocks the design — happy to approve once the rebase lands (and the cheap P2s are handled or explicitly deferred).

@me2seeks

me2seeks commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and addressed the review notes. Could you take another look when you have a chance?

@me2seeks
me2seeks force-pushed the feat/2015-pricing-settings-ui branch from ab42d1d to 3483e93 Compare August 8, 2026 16:40
@me2seeks

me2seeks commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Current head 3483e93 is rebased onto c2d09090, includes the review follow-ups, and all CI checks are green. Could you take another look when convenient?

@me2seeks
me2seeks force-pushed the feat/2015-pricing-settings-ui branch from 3483e93 to 7b5c82e Compare August 9, 2026 04:01
@me2seeks

me2seeks commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main. While resolving the Usage overlap I also aligned Pricing with Visual System 2.0: predictable loading now reserves the table geometry, the extra empty-state Card is gone, and the panel uses the shared icon scale. I also made the confirmation stories wait for their portal content instead of racing it.

The Pricing stories now pass at wide and floor widths. The only local smoke failure is the icon catalog already tracked by #2541/#2544. Ready for another look.

@me2seeks

me2seeks commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

The Storybook failure is outside Pricing: every Pricing manifest and catalog state passed. The icon error is #2541/#2544. The two Daily Review bounds failures are also outside this diff; a fresh production Storybook build from current main passed both floor runs locally. I don’t see a Pricing patch to make for this CI result.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thank you again for patiently working through all of the earlier feedback. Rechecking this after #2329 landed, I think the premise underneath this UI has materially changed rather than the implementation itself being at fault.

The built-in pricing set has grown from a small hand-maintained list to 1,239 generated models.dev entries. The current adapter assembles the complete set and the panel renders it as one table, while the stories exercise at most three rows. We could add filtering, pagination, and large-dataset coverage, but I’m no longer sure that rescuing the full-table design is the simplest product.

Pricing overrides now look like a low-frequency advanced escape hatch. My preference would be to keep models.dev as the read-only built-in authority and, if overrides are still needed, include pricing fields in the unified models.json-style override layer proposed in #2330 alongside context-window and capability overrides. That would avoid introducing a dedicated 1,239-row management surface and its associated adapter/UI state machine before we have evidence that users need it.

If real demand later justifies a UI, it could be much smaller: show only user overrides and use a catalog-backed picker when adding one, rather than presenting the complete built-in catalog.

I don’t think this calls for another incremental patch here; it would be worth revisiting the product decision first. This is a change in the surrounding facts after #2329, not a reflection on the care put into this PR.

@me2seeks

me2seeks commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

That makes sense. #2329 changed the product shape enough that I don't think adding pagination and filtering to the 1,239-row table is the right next step.

I'll stop iterating on this branch and close the PR. I agree with keeping models.dev read-only and moving pricing overrides into the unified model-facts layer discussed in #2330. If a dedicated UI is justified later, an overrides-only list with a catalog-backed picker would be a much smaller surface.

Thanks for taking another look and for separating the changed product facts from the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants