Improve Usage and Spend layout - #3353
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. Reviewed September 2, 2026, 1:52 AM ET / 05:52 UTC. ClawSweeper reviewWhat this changesThe PR reorganizes the Usage & Spend settings pane into provider-first detail panels, selectable trend views, a compact summary, nearby refresh feedback, and collapsed data controls. Merge readinessKeep this PR open for a maintainer UX decision: the prior proof-isolation and current-head visual-proof blockers are addressed, but the default collapsed management controls are an intentional product choice requiring approval. Priority: P2 Review scores
Verification
How this fits togetherCodexBar collects provider usage and local spend history into a settings dashboard. The dashboard aggregates those inputs into summaries, breakdowns, trends, refresh feedback, and export/privacy controls for the user. flowchart LR
A[Provider usage and local history] --> B[Usage and spend aggregation]
B --> C[Currency summary]
C --> D[Provider, project, and session details]
C --> E[Daily and hourly trends]
F[Refresh and data settings] --> D
Decision needed
Why: The implementation and current-head packaged visual evidence support both possible presentations, but the default information hierarchy is a product and discoverability choice rather than a correctness question. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the provider-first hierarchy if approved, with an explicit maintainer decision on whether management controls should remain collapsed by default. Do we have a high-confidence way to reproduce the issue? Not applicable as a layout and interaction redesign; the current-head screenshots and source review demonstrate the proposed states rather than an existing bug reproduction. Is this the best way to solve the issue? Unclear until the collapsed-controls default is approved; the hierarchy itself preserves the existing detail categories and has focused isolation and interaction coverage. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a8b9c55cd008. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
|
@clawsweeper re-review Hourly fixture is explicit now, and the PR body includes isolated native production-window interaction proof for selector changes, selected-day clearing, and expanded data controls at head d17dcb9. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: 📸 screenshot
|
@clawsweeper re-review The packaged-app proof blocker is addressed at head
Please re-review the current head and updated proof boundary. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
![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: f4e9b5c2a5
ℹ️ 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".
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
@clawsweeper re-review The P1 proof-profile isolation finding is fixed at head Before any settings mutation, proof mode now:
Regression tests now reject real/outside profile paths, mismatched homes, non-temporary proof roots, missing boundaries, and invalid isolation flags. Validation after the fix:
The PR body and final-head receipt have been updated. Please re-review the current head. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e20fb3419e
ℹ️ 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".
| let temporaryRoot = FileManager.default.temporaryDirectory | ||
| .standardizedFileURL | ||
| .resolvingSymlinksInPath() | ||
| .path |
There was a problem hiding this comment.
When bundle proof is launched with a custom TMPDIR, FileManager.default.temporaryDirectory follows that environment value, so the caller can redefine what this check considers the “system temporary directory”; for example, TMPDIR=/, a proof root of /Users, and profile paths under /Users/<user> satisfy the descendant checks and let prepareProcessIsolation() mutate the real user defaults. Fresh evidence in this revision is that the new strict-descendant hardening is anchored to this environment-influenced API, so derive the trusted temporary root independently of the proof environment or create and verify a dedicated owned directory.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
P2
|
@clawsweeper re-review Current head 7279854 addresses the two concrete blockers from the previous review:
The collapsed-controls default remains explicitly documented as a maintainer product decision, separate from the repaired technical and proof blockers. Please re-review the current head. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
1 participant
Summary
The hierarchy is informed by the compact, whitespace-led presentation at https://tokens.ci/u/missuo while preserving CodexBar's native SwiftUI components and accessibility semantics.
Provider hierarchy behavior
ACCOUNTSgroup appears only when multiple accounts exist or account provenance differs from the provider label.MODELSgroup is rendered under the provider rather than repeated beneath accounts when the source cannot prove an account/model relationship.Other models.~; unavailable, empty, and partial model history are not conflated.Review follow-up
_CS_DARWIN_USER_TEMP_DIR, independently of caller-controlledTMPDIRTMPDIR=/and verifies a real-profile-like path remains rejectedCurrent packaged-app proof
The screenshots below use the packaged production
CodexBar.appsettings window and production SwiftUI views. The data is intentionally synthetic and isolated; it is not real account data.Provider → model hierarchy
Projects detail and hourly trend
Expanded cost, source, privacy, and export controls
Validation
CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 make test— 993/993 selections, 83/83 groups passed on head727985417; 0 failures, 0 retries, and 0 timeoutsmake check— passed on head727985417; formatting and strict lint reported 0 violations across 2,088 Swift filesCODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter UsageSpendBundleProofTests— 7/7 passed, including hostileTMPDIR=/./Scripts/package_app.sh— final-head Release package passed deep signature, resource, and detached-checkout launch checks;Info.plistrecordsCodexGitCommit = 727985417git diff --check— passedProof boundary
The proof profile root must resolve as a strict child of the OS-owned per-user temporary directory.
CFFIXED_USER_HOME,HOME, andCODEX_HOMEmust remain strict children of that root, and all Keychain/file/session/testing isolation flags are mandatory. Validation happens before settings mutation. The screenshots therefore prove the current production view and interaction path without reading or changing the installed app's real profile.Remaining product decision
The implementation keeps source/privacy/export controls collapsed by default to preserve the compact information hierarchy. Whether those management controls should instead remain expanded by default is a maintainer product decision; it is intentionally not presented as a technical correctness claim.