Skip to content

feat(desktop): activate Pricing settings editor - #2291

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

feat(desktop): activate Pricing settings editor#2291
Thinkya1 wants to merge 7 commits into
apache:mainfrom
Thinkya1:feat/2015-pricing-settings-activation

Conversation

@Thinkya1

Copy link
Copy Markdown
Contributor

Summary

Adds an editable Pricing override editor to Desktop Settings → Usage → Pricing.

  • Expose a narrow settings.pricing preload contract for listing, upserting, resetting, and change notifications.
  • Connect the existing usage:pricing:* main-process handlers to the renderer.
  • Add add, edit, and delete flows for input, output, and optional cache rates.
  • Preserve exact model keys, omitted cache rates, and explicit zero values.
  • Update the Pricing table, English/Chinese copy, and responsive layout.
  • Update the IPC surface contract so the Pricing channels are recognized as renderer-consumed.

Why

Issue #2175 reports that the Usage → Pricing tab promises custom pricing overrides but provides no way to create, edit, or remove them. The backend handlers already exist, but the renderer could not reach them.

Current implementation note

This draft uses the existing main-side usage:pricing:* handlers through a narrow preload bridge. PR #2218 defines the longer-term Host-backed DesktopPricingSettingsPort path and intentionally avoids the legacy renderer bridge. That integration choice is left visible for review; this PR should be merged only if the maintainers accept this as the activation path. Otherwise, the renderer changes should be reshaped as a follow-up to #2218 before merge.

Verification

  • git diff --check
  • npm --workspace @maka/desktop run typecheck
  • npm --workspace @maka/desktop run test:checks
  • npm --workspace @maka/desktop run build:renderer
  • Targeted Desktop Pricing and IPC tests: 23 passed
  • Astryx component behavior tests: 8 passed

The full Desktop suite is not fully green on this worktree: three existing xAI OAuth tests fail with ECONNRESET in the network path. Storybook browser interaction coverage for the new CRUD flow has not been run yet.

Review focus

  • Confirm whether the legacy Pricing handlers are acceptable for this activation or must be replaced by the Host-backed port from feat(desktop): add isolated Pricing settings editor #2218.
  • Add the Pricing bridge to Storybook fixtures before relying on browser-rendered Usage stories.
  • Add revision/conflict protection if this path remains responsible for concurrent Pricing edits.

Part of #2015
Related to #2218
Closes#2175

@Thinkya1
Thinkya1 marked this pull request as ready for review August 5, 2026 20:13
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for this — the UI work is genuinely solid: the table, the add/edit form with the provider:model placeholder guidance, the responsive folding, and the delete confirmation dialog are all well done, and the explicit activation-only framing in the description is appreciated.

On the routing question, we'd like to keep the Host-backed port from #2218 as the activation path rather than the legacy usage:pricing:* bridge. Two concrete reasons:

  1. Both PRs touch the same file (usage-settings-page.tsx) — merging the legacy bridge now would conflict with feat(desktop): add isolated Pricing settings editor #2218's panel and fork the wiring in two directions. The legacy bridge is exactly the path feat(desktop): add isolated Pricing settings editor #2218 intentionally avoids.
  2. The builtin-pricing dimension is real, and the legacy UI can't show it. The runtime has a genuine builtin price table (packages/runtime/src/telemetry/builtin-pricing.ts, 17 hand-maintained entries) and the billing lookup is override ?? builtin (buildPricingLookup). Your delete dialog says "restore builtin pricing (if exists)" — but the legacy UI never shows builtin prices or a source column, so the "if exists" part is unverifiable in the UI. For the majority of models (only 17 have builtin entries), deleting an override actually leaves the model unpriced, not back on builtin — exactly the restore_builtin vs become_unpriced distinction feat(desktop): add isolated Pricing settings editor #2218 surfaces with a source column and reset-effect semantics. We think that's the correct behavior to ship.

Suggested path, matching what you proposed: reshape the renderer work as a follow-up to #2218 (or rebase on top once #2218 lands), and drop the preload / bridge-contract / ipc-surface pieces. The UI itself is worth keeping — it covers the add/edit/delete flows cleanly.

Happy to review either way. One more relevant piece: we just filed #2329 — the builtin price table should sync from models.dev instead of the hand-maintained snapshot. It makes the restore_builtin semantics in #2218 meaningful at scale, and would have been the natural home for your usage:pricing:list if the legacy path had carried builtin prices.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for moving the pricing editor onto the Host-backed pricing authority. The snapshot and mutation outcomes are a better fit than the old renderer bridge, but the production path is not wired yet.

P1: The production Settings entry never receives pricingPort

SettingsSurface makes pricingPort optional:

https://github.com/maka-agent/maka-agent/blob/352d34d15be5ad8ae4a58dd1b2f5cac78435be1d/apps/desktop/src/renderer/settings/settings-surface.tsx#L80-L81

The production SettingsModal does not pass it:

https://github.com/maka-agent/maka-agent/blob/352d34d15be5ad8ae4a58dd1b2f5cac78435be1d/apps/desktop/src/renderer/settings/SettingsModal.tsx#L95-L116

As a result, the normal Desktop build always renders the unavailable fallback. The editor is not activated, and existing overrides that were previously visible are replaced by an empty table saying there are no pricing overrides.

A production-level test needs to open Settings through the real entry and prove that an existing override is loaded and editable.

P2: Pricing load failures have no recovery path

The initial pricing load runs only when the component mounts or the port identity changes. The page-level Refresh button reloads Usage stats but does not retry pricing.

After an initial load failure, Add remains available, but saving cannot proceed because there is no snapshot base. The same problem occurs after saved_refresh_failed or reconciliation_unavailable: the page keeps an old or missing snapshot and offers no way to reacquire the authority state.

The page needs an explicit pricing retry/reload. Mutations should remain disabled until a valid snapshot is available, and an uncertain write should block further edits until the state is reconciled.

P3: A successful write followed by a refresh failure is presented as both success and failure

saved_refresh_failed is converted into an exception, so the toast title says “Save failed” or “Delete failed.” The detail then says that the pricing change was already saved:

https://github.com/maka-agent/maka-agent/blob/352d34d15be5ad8ae4a58dd1b2f5cac78435be1d/apps/desktop/src/renderer/locales/settings-usage-copy.ts#L54-L55

That combination can prompt the user to repeat a mutation that already succeeded. This outcome needs its own message and a direct retry action.

There is also a larger ownership question before adding more UI behavior. #2218 implements another pricing editor on the same Host-backed seam, with its own model, conflict handling, and Storybook states. Keeping both would leave two renderer authorities for the same feature. Picking one implementation and making the other PR a small production-wiring follow-up would be easier to maintain and test.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thank you for all the work you’ve put into this, especially for carrying the production wiring, reconciliation states, retry path, Storybook coverage, and the previous P1/P2/P3 feedback through several rounds. I rechecked the latest head, and those earlier issues have been addressed; CI is green as well.

Unfortunately, I think the surrounding product facts have now changed enough that we should stop rather than ask you for another patch. After #2329, the built-in pricing catalog grew from a small hand-maintained set to 1,239 generated models.dev entries. This PR’s Host snapshot loads the complete effective catalog and the table renders every entry, so it now has the same product-shape problem that led us to close #2218.

More importantly, pricing overrides look like one dimension of the broader model-facts problem in #2330, alongside context-window and capability overrides. A unified models.json-style layer would give advanced users one place to override those facts while keeping models.dev read-only, without maintaining a dedicated Pricing UI, IPC bridge, snapshot reconciliation state machine, and full-catalog table before we have evidence that this surface is needed.

I therefore don’t think there is another actionable code-review round here. My recommendation is to close this PR and align #2175/#2330 with the unified override direction. If real demand later justifies a UI, the smaller shape would be an overrides-only list with a catalog-backed picker, rather than the complete built-in catalog.

I’m sorry this conclusion comes after you invested so much care in addressing the earlier reviews. This is not a reflection on the implementation—the implementation responded well to the feedback. The premise changed underneath it after #2329, and continuing to patch the branch would ask you to spend more effort on a product path we no longer think should ship.

@Thinkya1Thinkya1 closed this Aug 9, 2026
Sign up for freeto 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.

Pricing tab shows "No pricing overrides" but provides no UI to add custom model prices

2 participants

@Thinkya1@Astro-Han