Add Projects panel to Usage & Spend pane - #2985
Merged
Merged
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69f9415e45
ℹ️ 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".
| for project in input.snapshot.projects { | ||
| let name = project.name.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !name.isEmpty else { continue } | ||
| let key = Key(sourceID: input.id, name: name) |
There was a problem hiding this comment.
When one Codex account contains distinct projects with the same basename, such as /work/app and /personal/app, the scanner emits separate breakdowns keyed by canonical path but names both app; this name-only key merges their spend into one row and retains only the first path. Include the canonical project path in the aggregation key and row identity so these projects remain separate.
Useful? React with 👍 / 👎.
| let key = Key(sourceID: input.id, name: name) | ||
| var aggregate = aggregates[key] ?? Accumulator( | ||
| provider: input.provider, | ||
| providerName: input.modelProviderName, |
There was a problem hiding this comment.
With multiple Codex accounts, the controller sets displayName to the account-specific label but sets modelProviderName to the generic Codex label; using the latter here therefore gives every project the same caption even though rows are attributed to separate subscriptions. Use the input's account-specific display name for project rows so users can tell which subscription incurred each cost.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed August 16, 2026, 11:11 PM ET / August 17, 2026, 03:11 UTC. ClawSweeper reviewWhat this changesAdds a Projects panel and expandable Models/Projects lists to the Usage & Spend settings pane, including localized expansion controls. Merge readiness⛔ Blocked until real behavior proof is added - 12 items remain Keep open: current main has the project-data prerequisite but not this settings UI, so the PR remains necessary. It still exposes two source-proven attribution defects, duplicates unreleased changelog content, and lacks required app-bundle proof. Likely related people: Yuxin-Qiao (project-row implementation, high confidence) and steipete (recent merged-area owner, high confidence). Priority: P2 Review scores
Verification
How this fits togetherUsage & Spend turns local provider history into per-currency provider, model, and project totals, then renders those totals in the settings pane. This PR adds the project presentation layer, so aggregation and account-attribution mistakes become visible to users. flowchart LR
A[Local usage history] --> B[Spend dashboard controller]
B --> C[Project cost aggregation]
C --> D[Currency groups]
D --> E[Usage and Spend settings]
E --> F[Projects panel]
E --> G[Expandable model list]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve distinct project paths and account-specific captions in the shared project model, retain one clean release-note entry, and verify the completed panel in a freshly built app bundle. Do we have a high-confidence way to reproduce the issue? Yes, from source: provide two distinct canonical paths with the same basename for one Codex account, or two Codex accounts with distinct display names; current aggregation and captions produce the wrong rows. Is this the best way to solve the issue? No: the panel is the appropriate surface, but it must consume path-distinct, account-labeled project rows and be proven in the built app before merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f3f3ccea2d7f. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md # Sources/CodexBar/Resources/ar.lproj/Localizable.strings # Sources/CodexBar/Resources/ca.lproj/Localizable.strings # Sources/CodexBar/Resources/de.lproj/Localizable.strings # Sources/CodexBar/Resources/en.lproj/Localizable.strings # Sources/CodexBar/Resources/es.lproj/Localizable.strings # Sources/CodexBar/Resources/fa.lproj/Localizable.strings # Sources/CodexBar/Resources/fr.lproj/Localizable.strings # Sources/CodexBar/Resources/gl.lproj/Localizable.strings # Sources/CodexBar/Resources/id.lproj/Localizable.strings # Sources/CodexBar/Resources/it.lproj/Localizable.strings # Sources/CodexBar/Resources/ja.lproj/Localizable.strings # Sources/CodexBar/Resources/ko.lproj/Localizable.strings # Sources/CodexBar/Resources/nl.lproj/Localizable.strings # Sources/CodexBar/Resources/pl.lproj/Localizable.strings # Sources/CodexBar/Resources/pt-BR.lproj/Localizable.strings # Sources/CodexBar/Resources/ru.lproj/Localizable.strings # Sources/CodexBar/Resources/sv.lproj/Localizable.strings # Sources/CodexBar/Resources/th.lproj/Localizable.strings # Sources/CodexBar/Resources/tr.lproj/Localizable.strings # Sources/CodexBar/Resources/uk.lproj/Localizable.strings # Sources/CodexBar/Resources/vi.lproj/Localizable.strings # Sources/CodexBar/Resources/zh-Hans.lproj/Localizable.strings # Sources/CodexBar/Resources/zh-Hant.lproj/Localizable.strings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Show all (N)/Show lesstoggle now reveals the full list.Show all (%d),Show less) to all 23 app locales.Commands run
swift build --build-testsswift test --filter SpendDashboardModelTests— 39 passednode Scripts/check-app-locales.mjs— 22 catalogs OKswiftformat Sources Tests/swiftlint --strict— clean./Scripts/test.sh(full sharded suite) — green, 803s, 0 failuresNotes
feat/spend-projects-model); merge that first.Projectspanel header key lands in the chart-localization PR (fix/chart-menu-localization); if this merges first, the header falls back to the English key until that PR lands.