Skip to content

[0.62.0] Add provider usage item visibility - #581

Open
Finesssee wants to merge 12 commits into
codex/port-0.61.0-provider-detailsfrom
codex/port-0.62.0-provider-detail-visibility
Open

Finesssee wants to merge 12 commits into
codex/port-0.61.0-provider-detailsfrom
codex/port-0.62.0-provider-detail-visibility

Conversation

@Finesssee

Copy link
Copy Markdown
Collaborator

Summary

  • add stable persisted usage-item IDs for provider quota metrics and provider-emitted extra rows
  • add provider-detail visibility controls with restore defaults and legacy settings compatibility
  • filter existing metric rows at presentation boundaries while preserving raw snapshots, selected metrics, tray behavior, and provider isolation
  • keep Claude Daily Routines and other provider data in raw snapshots; the new visibility patch does not trigger a provider refresh
  • propagate hidden-item settings through the Tauri bridge, events, provider detail, menu card, and settings UI

This PR is stacked on #564 (codex/port-0.61.0-provider-details) and ports the provider-detail visibility portion of upstream 0.62.0 for the Windows surfaces.

Validation

  • full Rust tests — 1,931 passed, 1 ignored
  • full Tauri tests — 443 passed
  • Rust and Tauri clippy with -D warnings
  • focused Claude/settings/Tauri regressions passed
  • frontend tests — 370 passed
  • frontend build and locale check passed
  • fresh Tauri debug build passed
  • cargo fmt --check and git diff --check passed

Native CUA screenshot proof was unavailable in this environment because no native CUA inventory or window-control methods were available; the affected bridge, presentation, settings, and raw-data behavior has automated coverage.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 8a253408-d482-4344-b597-9ced73d69541

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Thermo-Nuclear Review: PR #581 — [0.62.0] Add provider usage item visibility

Verdict: REQUEST CHANGES

The core architectural move is right: replacing the hardcoded filter_hidden_codex_spark_rows snapshot mutation with presentation-time filtering against a persisted hidden-ID list keeps raw snapshots intact and deletes a special-case filter from the tray/events/providers paths. That is genuinely good code-judo in the macro sense. But the dual-source-of-truth shims for the two legacy flags (spark_usage_visible, claude_daily_routines_usage_visible) add a bidirectional write-sync web across Settings that the PR could have deleted entirely, the same four-way read-modify-write loop is copy-pasted twice in settings.rs, and one now-orphaned frontend component is left behind.

Structural regressions

  1. Bidirectional legacy-flag ↔ hidden-ID synchronization is a two-way dependency web in Settings.
    set_hidden_usage_item_ids writes spark_usage_visible from the list; set_spark_usage_visible writes the list from the flag — each setter mutating the other's representation, guarded by has_explicit_usage_item_visibility. hidden_usage_item_ids() then has to re-derive list-from-flag at read time for legacy configs. Three functions now participate in a dual-write protocol where one representation would do. The migration can be one-shot instead of ongoing: on settings load, materialize hidden_usage_item_ids from the legacy flags once (when None), then delete the flag-driven derivation branches from the read path — hidden_usage_item_ids() becomes a plain lookup, set_spark_usage_visible becomes a plain list edit, and both setters shrink to a fraction of their current size. The current design makes every future settings write path (imports, resets, sync) reason about both representations simultaneously.

  2. Two get_provider_detail settings loads and a redundant hidden_usage_item_ids write.
    build_provider_detail already loads Settings and sets detail.hidden_usage_item_ids = settings.hidden_usage_item_ids(id); get_provider_detail then does Settings::load() again (disk read Reduce unnecessary redraws and improve tray interactions #2), re-parses the provider id, and overwrites the same field with the same value. The extra load-and-overwrite is pure ceremony — thread the already-loaded settings/id out of build_provider_detail (it currently discards both) and delete six lines.

Missed simplification opportunities (code-judo)

  1. Copy-pasted retain/push/normalize block, twice.
    set_spark_usage_visible and set_claude_daily_routines_usage_visible are the same function with different constants:

    retain(|item| !<IDS>.contains(&item)); if !value { hidden.extend(<IDS>) }

    vs retain(|item| item != <ID>); if !value { hidden.push(<ID>) }. One helper — fn toggle_hidden_items(&mut self, id: ProviderId, target: &[&str], visible: bool) — implements both legacy setters in three lines each and deletes the duplicated read-clone-mutate-normalize-reserialize pipeline. This is the single highest-value extraction in the PR.

  2. unavailable_usage_item_title reimplements title-casing at the bridge layer.
    The extra--prefix strip, -/_ split, per-word capitalize, join — plus the six hardcoded special cases — is presentation string-shaping living in commands/bridge.rs (the wrong layer, and the special-case table will grow with every provider that emits a new extra row id). The snapshot already carries real titles; the only titles it lacks are for persisted-but-no-longer-emitted IDs. If a fallback title is genuinely needed, it belongs beside the descriptor contract (or as a Display impl on the item type), not as a match arm inside the bridge.

  3. Dead field: SettingsSnapshot.providerHiddenUsageItemIds is serialized but never read.
    grep over the frontend shows zero consumers — the visibility section reads ProviderDetail.usageItems/hiddenUsageItemIds instead. A bridge field with no reader is weightless surface area that must now stay in sync with bridge.ts and the Rust From<Settings> conversion forever; delete it or wire it up.

  4. presentation_snapshots in tray_bridge.rs is now an identity function.
    The PR reduced it to snapshots.to_vec() — a pass-through wrapper adding one copy and one level of indirection over calling .to_vec() at the two call sites (or passing the slice through). Delete it; its existence now hides the fact that no presentation filtering happens in the tray path, which is the new invariant worth stating plainly.

Spaghetti / branching complexity

  1. get_provider_detail has a hidden-invariant fallback chain.
    usage_items starts empty → gets populated from the snapshot → "if empty, rebuild with snapshot=None" → and the second call re-derives hidden_usage_item_ids via push("raw_id", "", false). An empty list now means one of two things (no snapshot, or snapshot present with zero items), disambiguated only by control flow. Computing usage_items = usage_item_descriptors(snapshot.as_ref(), …) once, unconditionally, after the merge (descriptors handle None fine — that's what the fallback is testing) collapses the chain and makes available: false the single way to say "not emitted."

  2. Cross-layer toggle granularity mismatch left implicit.
    UsageItemVisibilitySection shows only items the provider currently emits (plus persisted-hidden placeholders), while hiddenUsageItemIds may contain IDs the provider stopped emitting forever. The section's "restore defaults" clears everything including those ghosts — correct behavior, but nothing in the code states the invariant ("displayed ⊆ persisted ∪ emitted"). One comment on the descriptor contract would spare the next reader a reverse-engineering session.

Boundary / abstraction / type problems

  1. Backend descriptor IDs hardcode the frontend's own metric-key spellings.
    usage_item_descriptors pushes "primary" | "secondary" | "model-specific" | "tertiary" as raw ids, and frontend consumers re-derive metric:primary etc. through isUsageItemVisible(hiddenUsageItemIds, "primary") — matching string keys across the bridge by convention. A single shared constant module (the Rust side already owns USAGE_ITEM_METRIC_PREFIX) or a typed enum serialized into both usageItems[].id and the filter calls would make a spelling drift a compile/test error instead of a silent hidden row. Right now a typo'd "modelSpecific" in a filter call hides nothing, silently.

File-size / decomposition concerns

  1. commands/bridge.rs grows 1051 → 1182 lines — already over 1k and pushing further.
    The ~130 added lines (descriptors, unavailable-title shaping, SettingsSnapshot plumbing) are self-contained presentation logic inside the bridge megafile. Extracting usage_items.rs (descriptors + titles + ProviderUsageItemSnapshot) keeps the growth out of a file that is already the largest in the shell crate. Not a blocker on its own, but the direction is wrong: this PR adds the third distinct concern to that file in two port cycles.

Lower-priority notes

  • CodexUsageOptions.tsx is orphaned: the last render site was removed from ProviderDetailPane.tsx, but the component (and its test hookups) still exist in the tree. Dead code — delete it in this PR per clean-cutover.
  • MenuCard.tsx: visibleMetrics is computed by filtering, then compactVisibleMetrics slices it — but presence is computed from compactVisibleMetrics while the name visibleMetrics lingers unused otherwise; rename to metrics-vs-displayMetrics or inline the slice to remove the second name.
  • The ProviderDetailPane "reload on settings-changed" listener re-fetches the entire detail pane on any settings event (including unrelated toggles). Scoped to work today, but a narrower predicate (event carries the touched key) would prevent a class of full-pane flickers as more settings use the broadcast.
  • Test quality is good: the removed hiding_codex_spark_rows_preserves_other_extra_usage test asserted implementation text and was correctly deleted rather than re-pinned; the new tests assert observable bridge behavior.

* Add transient provider inventory plumbing

* Add transient provider detail carrier

* Restore inventory row classes in MenuCardDetails
* Add transient provider inventory plumbing

* Port Grok usage reset coupons

* Harden Grok reset credit fetching

* Extract Grok reset-coupon parser to billing/reset_coupons.rs

Split the SuperGrok GetRemainingResets parser out of billing/mod.rs into
billing/reset_coupons.rs with its own fail-closed framing policy. Dedupe
the length-field and Unix-seconds decode into billing/mod.rs helpers
(read_length_field, unix_seconds_timestamp, map_grpc_status) shared by
both endpoints; billing frame walking moves into a parameterized
grpc_web_frames walker so each endpoint picks its malformed-frame
policy.

* Drop duplicated ProviderInventoryItem from merge residue

* Drop duplicated inventory bridge mapping from merge residue

* Restore single inventory bridge mapping in merge residue fix
…vider-detail-visibility

# Conflicts:
#	apps/desktop-tauri/src-tauri/src/commands/bridge.rs
#	apps/desktop-tauri/src-tauri/src/commands/tests.rs
#	apps/desktop-tauri/src/components/MenuCardDetails.tsx
#	apps/desktop-tauri/src/surfaces/settings/providers/sections/UsageSection.tsx
#	apps/desktop-tauri/src/types/bridge.ts
#	rust/src/core/usage_snapshot.rs
* Add transient provider inventory plumbing

* Add transient provider detail carrier

* Port Muse Code subscription usage

* Bound Muse response buffering

* cargo fmt after merge resolution

* Address thermo-nuclear review: read env directly, pin alias boundary

* Fix compile fallout: f64 bound check, unused import

* Update muse test for plan separated from login_method

* Fix duplicate wayfinder render and inventory field

* Fix display details tests for main API
…ls' into codex/port-0.62.0-provider-detail-visibility

# Conflicts:
#	apps/desktop-tauri/src-tauri/src/commands/tests.rs
#	apps/desktop-tauri/src/components/MenuCardDetails.tsx
#	apps/desktop-tauri/src/types/bridge.ts
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.

1 participant