Skip to content

Add cost summary display style#1741

Open
Zihao-Qi wants to merge 1 commit into
steipete:mainfrom
Zihao-Qi:cost-summary-display-style
Open

Add cost summary display style#1741
Zihao-Qi wants to merge 1 commit into
steipete:mainfrom
Zihao-Qi:cost-summary-display-style

Conversation

@Zihao-Qi

Copy link
Copy Markdown
Contributor

Summary

Adds a user preference for how CodexBar displays cost summaries when “Show cost summary” is enabled.

Users can now choose:

  • Inline
  • Submenu
  • Both

Changes

  • Adds CostSummaryDisplayStyle to settings and persists it in UserDefaults.
  • Adds a Display style picker under Settings → Usage → Show cost summary.
  • Splits cost-summary rendering into separate inline-dashboard and submenu gates.
  • Applies the selected style consistently across:
    • main provider menu cards
    • overview row submenus
    • provider preview cards in Preferences
    • OpenAI/Mistral primary cost-history inline dashboards
  • Adds localized strings for the new setting.
  • Adds focused tests for settings persistence and menu presentation.

Why

When cost summaries were enabled, CodexBar could show both the inline cost dashboard and the Cost submenu at the same time. This adds an explicit choice so users can keep the style they prefer and avoid redundant UI.

Validation

  • swift test --filter StatusMenuTests
  • swift test --filter ProviderInlineDashboardModelTests
  • swift test --filter OpenAIAPIMenuCardModelTests
  • swift test --filter StatusMenuPersistentRefreshTests
  • make check
  • git diff --check

Screenshot

Before:
Screenshot 2026-06-24 at 09 54 51

After:
when inline selected
Screenshot 2026-06-24 at 09 54 16

when submenu selected
Screenshot 2026-06-24 at 09 54 32

Display style in setting
Screenshot 2026-06-23 at 17 45 28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 230ddc587a

ℹ️ 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".

Comment thread Sources/CodexBar/StatusItemController+Menu.swift
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 1:03 PM ET / 17:03 UTC.

Summary
The branch adds a CostSummaryDisplayStyle UserDefaults preference, a Settings picker, inline/submenu cost-summary gates, localization strings, and focused settings/menu tests.

Reproducibility: not applicable. This is a feature PR, and the inspected screenshots plus source diff demonstrate intended UI states rather than a bug reproduction path.

Review metrics: 2 noteworthy metrics.

  • Persistent Setting: 1 new UserDefaults key. The new key controls user-visible upgrade and fresh-install behavior, so maintainers should explicitly accept the setting surface.
  • Diff Scope: 47 files changed, 395 added, 19 deleted. The change spans settings, menu rendering, localization, and tests rather than a single isolated code path.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] This adds a new persisted UserDefaults key, Settings picker, and fresh-install display default; maintainer product acceptance is still required by VISION.md.
  • [P1] This read-only review did not run tests; the PR body lists focused Swift tests and make check, while several GitHub checks were still pending at inspection time.

Maintainer options:

  1. Decide the mitigation before merge
    Land this only if maintainers accept the new display-style preference and default behavior; otherwise narrow or close the feature direction before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] The remaining action is maintainer product approval for the new persisted preference and default behavior, not an automated code repair.

Security
Cleared: The diff changes Swift settings/menu logic, localization strings, and tests; I found no concrete security or supply-chain concern.

Review details

Best possible solution:

Land this only if maintainers accept the new display-style preference and default behavior; otherwise narrow or close the feature direction before merge.

Do we have a high-confidence way to reproduce the issue?

Not applicable. This is a feature PR, and the inspected screenshots plus source diff demonstrate intended UI states rather than a bug reproduction path.

Is this the best way to solve the issue?

Yes, implementation-wise: the enum, centralized display gates, upgrade-preserving migration, and focused tests are a maintainable shape. Product acceptance of the new persisted preference remains the maintainer decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ada3660e9d61.

Label changes

Label justifications:

  • P2: This is a bounded user-facing settings improvement with tests and proof, not an urgent production regression.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The inspected screenshots show the new Settings picker and the after-change Inline and Submenu menu states, which is sufficient visual proof for this UI change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The inspected screenshots show the new Settings picker and the after-change Inline and Submenu menu states, which is sufficient visual proof for this UI change.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The inspected screenshots show the new Settings picker and the after-change Inline and Submenu menu states, which is sufficient visual proof for this UI change.
Evidence reviewed

What I checked:

  • Repository policy read: Read the target AGENTS.md fully; its guidance to prefer focused settings/menu tests and avoid live Keychain/provider validation shaped this read-only review. (AGENTS.md:1, ada3660e9d61)
  • VISION feature sign-off: VISION.md places new features in the needs-sign-off bucket, which applies because this PR adds a new user-facing display preference. (VISION.md:13, ada3660e9d61)
  • Not implemented on current main: Current main has no CostSummaryDisplayStyle, costSummaryDisplayStyle, or costSummaryShows symbols, so the central feature is not already on main. (ada3660e9d61)
  • New persisted setting and migration: PR head defines CostSummaryDisplayStyle and migrates users who already had cost summary enabled to Both, preserving the previous combined display behavior for that enabled state. (Sources/CodexBar/SettingsStore.swift:115, 1917d1a2ab1c)
  • Central display gates: PR head centralizes inline and submenu decisions in SettingsStore helpers and gates OpenAI API usage submenu creation on the submenu style. (Sources/CodexBar/SettingsStore+TokenCost.swift:5, 1917d1a2ab1c)
  • OpenAI submenu review note addressed: A Codex review comment identified missing OpenAI submenu gating, and the author replied that it was fixed with makeOpenAIAPIUsageSubmenu gating plus a regression test; the current head shows that guard. (Sources/CodexBar/StatusItemController+Menu.swift:1530, 1917d1a2ab1c)

Likely related people:

  • steipete: git blame shows the existing token-cost settings, menu-card cost section, OpenAI cost submenu, and inline cost-dashboard baseline on current main are mostly from f380287. (role: introduced current baseline; confidence: high; commits: f380287041b8; files: Sources/CodexBar/SettingsStore.swift, Sources/CodexBar/MenuCardView.swift, Sources/CodexBar/InlineUsageDashboardContent.swift)
  • Zihao-Qi: Recent current-main commits touched adjacent inline dashboard and menu refresh behavior that overlaps this PR's cost-summary display surface. (role: recent adjacent contributor; confidence: medium; commits: 929d55aaf1d7, ada3660e9d61; files: Sources/CodexBar/InlineUsageDashboardContent.swift, Sources/CodexBar/StatusItemController+Menu.swift, Sources/CodexBar/StatusItemController+MenuRefreshScheduling.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@Zihao-Qi Zihao-Qi force-pushed the cost-summary-display-style branch from 230ddc5 to ff7840b Compare June 24, 2026 14:44
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 24, 2026
@Zihao-Qi Zihao-Qi force-pushed the cost-summary-display-style branch from ff7840b to 87edff1 Compare June 24, 2026 15:17
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 24, 2026
@Zihao-Qi Zihao-Qi force-pushed the cost-summary-display-style branch from 87edff1 to 0ff82c7 Compare June 24, 2026 16:01
@Zihao-Qi Zihao-Qi force-pushed the cost-summary-display-style branch from 0ff82c7 to 1917d1a Compare June 24, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant