Skip to content

feat(desktop): add host-scoped pricing override editor - #5557

Open
liuxiaocs7 wants to merge 4 commits into
apache:mainfrom
liuxiaocs7:feat/pricing-overrides-only
Open

liuxiaocs7 wants to merge 4 commits into
apache:mainfrom
liuxiaocs7:feat/pricing-overrides-only

Conversation

@liuxiaocs7

@liuxiaocs7 liuxiaocs7 commented Sep 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Settings → Usage → Pricing currently shows a disconnected read-only table. This replaces it with a Host-scoped editor that lists user overrides and offers a searchable built-in catalog picker or exact-key manual entry. Users can edit input, output, cache-read, and cache-write rates, reset a built-in override, or delete a custom-only price. Blank optional rates remain distinct from zero.

Writes use the snapshot the user reviewed. Host changes preserve raw drafts while requiring fresh review, and conflicts or lost responses reconcile without replaying mutations. The Pricing tab has its own refresh and hides time-range summaries.

Rebuilt from main at acab16537639cffd3cbd3b2652368f3679b1f245; supersedes #4164. The diff is limited to Pricing implementation, wiring, tests, and UI evidence. Usage pagination and the wire schema are preserved. The unrelated ACP, Side Chat, and dependency patch changes from the original PR are excluded.

Refs #2015 #2218 #4164

Verification

Verified on b5d83e239024a4f370f79eda9c3c8ffe088e18e4:

  • Clean root build, workspace typechecks, lint/format, both Knip workspaces, renderer architecture (112 tests), protocol epoch guard, and Astryx inventory: passed.
  • Pricing/Usage controller, view model, protocol, client, and IPC suites: 105 passed.
  • Pricing/Usage browser smoke: 26 renders passed, covering 1280px/480px layouts, validation, keyboard submit, recovery, Host switching, and Usage pagination. Browser smoke/AX contracts: 24 passed.
  • Real Electron Pricing write → renderer reload → authoritative read → delete journey: passed.
  • Removing the Pricing-only catalog accessibility binding makes the existing browser story fail at its required-combobox assertion.
  • CI for this commit: running.

Before (main) and after:

Before After
Read-only Pricing Editable Pricing

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code authored the initial Pricing editor; Codex rebuilt it on current main, removed unrelated changes, localized catalog accessibility, and verified behavior. The commit includes Generated-by: trailers.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XXL Over 2500 readable lines label Sep 21, 2026
Replace the disconnected Pricing table with an overrides-only editor and searchable built-in catalog picker. Preserve reviewed CAS snapshots, drafts across Host changes, and explicit reconciliation without replaying uncertain writes.

Rebuild the Pricing changes from apache#4164 on current main, keeping Usage pagination and wire compatibility. Localize catalog validation semantics to the editor and include focused, browser, IPC and Electron coverage.

Generated-by: Claude Code
Generated-by: Codex
@liuxiaocs7
liuxiaocs7 force-pushed the feat/pricing-overrides-only branch from add5d0d to b5d83e2 Compare September 21, 2026 05:45
Preserve pricing drafts and Host fencing alongside Usage pagination and capacity feedback. Regenerate the surface inventory, align compatible protocol declarations with epoch 183, and normalize imported plugin license headers.

Generated-by: Codex

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a Host-scoped pricing override editor backed by a complete effective-pricing snapshot, CAS mutation, and reconnect reconciliation. I checked the selected-Host bridge and Main IPC path, input validation, controller fencing around Host changes and in-flight reloads, conflict/reset behavior, and the new regression cases. I found no substantiated P0–P3 issue in those paths. I also inspected the two before/after screenshots; the Pricing tab visibly removes the usage-range summary and adds the override editor entry point.

The current-head test check passed, but I did not independently run the full Desktop E2E suite or an authenticated multi-Host desktop session. This PR currently conflicts with main in docs/astryx-surface-file-inventory.md; resolve that conflict and re-run the checks before a human merge decision. No database schema migration is included.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

Preserve Pricing services and smoke coverage alongside the new Plan and Storage composition. Update compatibility declarations to epoch 199 and the Pricing DOM fixture for Astryx 0.6.3.

Generated-by: Codex
Add regressions for same-Host disposal, refresh ordering, malformed pagination, reset reconciliation and native Host-review focus. Remove three locally redundant bookkeeping steps validated independently and together; archive the reproducible 33-variant experiment.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 17e0d3db. The pricing override editor looks solid: rate parsing rejects malformed/negative/underflow input and keeps an unset cache rate distinct from 0, main strictly validates renderer snapshots and mutations, stale-Host snapshots are rejected before sending, and the reconcile path only re-reads rather than resending writes. The protocol change is helper-only and correctly declared compatible at the current epoch (the epoch guard passes against main).

No P0–P2 found. Three P3s:

  1. Merge resolution rewrote unrelated compatibility declarations (P3). Besides the new pricing-reconciliation-helpers.json, six existing files under packages/runtime-host/protocol-compatible-changes/ change their epoch: recall-query-operation.json 169→183, session-model-limit-authority.json 175→183, executor-catalog-browser-safe.json and issue-4032-composition-identity.json 189→199 (reason text now says "through epoch 199"), mechanical-candidate-sweep.json and turn-snapshot-optional-fields.json 198→199. These appear to come from the merge commits and the epoch guard only checks newly added declarations, so CI won't flag it. Please restore those six files to main.
  2. False conflict after changing the model key mid-save (P3) — inline below.
  3. Experiment scaffolding in the tree (P3). scripts/experiments/pricing-ablation*.mjs, the *.probes.ts files, and docs/archive/pricing-ablation-2026-09-30.json (+2.7k lines, pinned to an earlier tree) aren't wired into CI. Worth confirming with maintainers whether they belong in the repo or in the PR description.

Not verified: tests were not run locally; whether every Host reconnect bumps the Settings lifecycle epoch (if not, the editor could keep showing pricing_snapshot_stale until refresh).

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

switch (outcome.kind) {
case 'saved':
setWriteState({ kind: 'idle' });
if (ownsDialog) finishReconciledIntent(attempt, outcome.snapshot);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Fields stay editable while a save is pending, and changing the model key bumps dialogGenerationRef (line 200). So when this save settles as saved/synchronized, ownsDialog is false and the open dialog's base snapshot stays at the pre-save revision. The next save then fails the revision check and shows a "changed elsewhere" conflict whose only other writer was the user; the duplicate-key check also reads that stale base.

Repro: Add → manual → key A → Save → change key to B while pending → Save. The existing test around pricing-editor.test.ts:696 covers only review_required. Suggest either locking the key field while saving, or rebasing the open dialog onto outcome.snapshot even when it no longer owns the attempt.

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 17e0d3db35aa01ee3f66bcbc5d8c03ae37668dd1. The PR replaces the disconnected Usage pricing table with a Host-scoped override editor. The UI keeps a reviewed snapshot as its write base, preserves drafts across Host changes, and fences older reloads (apps/desktop/src/renderer/features/usage/controller/pricing-controller.ts:127-179,211-259,497-530). Main IPC validates the renderer-supplied snapshot and mutation (apps/desktop/src/main/runtime-host-pricing-ipc-main.ts:82-124); the client checks Host identity and uses revision-CAS plus read-only reconciliation rather than replaying an uncertain write (apps/desktop/src/main/runtime-host-client.ts:635-709). This head also adds focused regression and ablation material.

I found no additional substantiated P0-P3 issue in the reviewed paths. The three P3 observations already published in the current-head review by Astro-Han remain for the author/maintainers; I am not duplicating those inline comments. In particular, the unrelated protocol-compatible-change declaration edits and experiment files are still present in this head. This is not a merge approval.

Node 24 clean npm ci and build:test passed; focused Pricing/Usage Desktop/Host tests passed (115/115). This head's hosted test passed. Diff check and static merge against current main ed38ccbb were clean; the branch is 2 commits behind main. I inspected the checked-in Pricing screenshot, but did not run a fresh native Electron write journey or a real multi-Host interactive session. Those behaviors remain outside my local verification.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

This branch has not been deployed

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

Labels

effort/XXL Over 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants