Skip to content

Make spend dashboard sessions, hourly chart, and daily counts readable - #3997

Closed
Yuxin-Qiao wants to merge 4 commits into
steipete:mainfrom
Yuxin-Qiao:fix/spend-session-rows
Closed

Yuxin-Qiao wants to merge 4 commits into
steipete:mainfrom
Yuxin-Qiao:fix/spend-session-rows

Conversation

@Yuxin-Qiao

@Yuxin-Qiao Yuxin-Qiao commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Three places in Usage & Spend rendered data that was technically correct but useless in practice.

1. Sessions tab rows were indistinguishable

Every row showed the source name (e.g. Codex · #1) plus the dominant model, and hid the timestamp whenever a model was known. With one account, all rows looked identical apart from cost.

  • CostUsageSessionBreakdown gains optional title / projectPath (default nil; OpenCodex and other producers unchanged). Codex session breakdowns fill them from the cached rollout cwd / title, which were previously dropped.
  • SessionRow.headline = title → project folder name → source name; contextLabels add project/source/model. Subtitle is now project · source · model · date time. Only the last path component is shown.

2. Hourly chart squeezed the whole range into one plot

Without a selected day, the hourly chart plotted every hour of the requested range (~1,400 hairline bars for 60 days).

  • With no selection, it now focuses the newest day that has hourly spend (same 0–24h domain as a selected day). CurrencyGroup.hourlyDay exposes that day and the panel header shows it next to the title. Selecting a day still wins.

3. Daily ledger showed — for tokens/requests on every mixed day

Daily token and request totals used completeIntSum, so any cost-only provider on a day (Cursor, OpenCode Go, …) blanked the whole row, even when Codex reported tens of millions of tokens.

  • Daily totals now sum known providers (matching how daily cost already works) and DailySummary.hasPartialTokens / hasPartialRequests mark the value as a floor (≥) when some active providers don't report the metric. Single-provider behavior is unchanged.

Tests

  • SpendDashboardSessionAndHourlyTests (new suite, keeps SpendDashboardModelTests under the file-length limit): session rows prefer title and project over the source name, hourly chart focuses the newest day with spend when no day is selected.

  • SpendDashboardDailyLedgerTests: cost only providers keep known token and request counts as a floor.

  • CostUsageFetcherTests: session fixture gains a cwd and asserts projectPath flows through the store/cache path.

  • Regenerated CodexParserHash.generated.swift (the session builder lives in the hashed Codex parser directory) and refreshed the ProviderArchitectureGatekeeperTests line anchor for SpendDashboardModel.swift.

Commands run

  • swift build --product CodexBarCLI (passes)
  • App/test targets and make check could not run locally: this machine only has Command Line Tools (no Xcode, so actool, SwiftUI macro plugins, and sourcekitd are unavailable). CI (build, both macOS test shards, lint) is green.

Yuxin-Qiao and others added 2 commits September 26, 2026 00:51
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@Yuxin-Qiao Yuxin-Qiao changed the title Show project and title in spend dashboard session rows Make spend dashboard sessions, hourly chart, and daily counts readable Sep 25, 2026
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 25, 2026
@clawsweeper

clawsweeper Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 25, 2026, 2:33 PM ET / 18:33 UTC (Revision 2).

ClawSweeper review

What this changes

The branch adds titles and project names to spend session rows, focuses the hourly chart on the newest active day, and displays known daily token and request counts as lower bounds when other providers omit them.

Merge readiness

⛔ Blocked before merge - 6 items remain

This PR still addresses three real readability problems in Usage & Spend, and current main has not adopted them. The previous privacy finding remains at the current head: session titles and project names can appear while Hide Personal Info is enabled. The change to the hourly chart also needs an explicit product choice, and the PR has no after-fix native dashboard proof.

Priority: P2
Reviewed head: ede78afa034b0155101c0ad99f657f4603352429
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) Focused code and tests provide useful signal, but the privacy defect, hourly contract choice, and missing native behavior proof block readiness.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The changed scanner and dashboard model are covered by focused tests, and the PR reports a CLI build, but the captured evidence does not show the native Settings session, hourly, and daily views after the change in either privacy state. A redacted screenshot or recording of those views would help; redact private paths, prompts, account details, and endpoints. Updating the PR body should trigger a fresh review; if it does not, a maintainer can comment @clawsweeper re-review. The in-memory session fields do not change the stored cache schema. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The changed scanner and dashboard model are covered by focused tests, and the PR reports a CLI build, but the captured evidence does not show the native Settings session, hourly, and daily views after the change in either privacy state. A redacted screenshot or recording of those views would help; redact private paths, prompts, account details, and endpoints. Updating the PR body should trigger a fresh review; if it does not, a maintainer can comment @clawsweeper re-review. The in-memory session fields do not change the stored cache schema. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Introduced privacy exposure: The PR renders session headlines, subtitles, and tooltips containing titles or project names; the existing detail panel passes Hide Personal Info to project rows but not session rows.
Prior finding remains: The session view and model files are unchanged since the previously reviewed head f289858; its Hide Personal Info finding therefore remains applicable.
Hourly behavior changes: With no selected day, the model chooses the newest day with positive hourly spend and passes that day to the chart, replacing the previous full-range hourly view.
Findings 1 actionable finding [P1] Honor Hide Personal Info in session rows
Security Needs attention Private session identity remains visible: Local session titles and project folder names reach row text and tooltips without the privacy mask used by project rows, exposing potentially sensitive context during screen sharing.

How this fits together

CodexBar scans local provider usage and builds spend summaries for its macOS Settings dashboard. The dashboard turns those summaries into session rows, daily counts, and hourly charts.

flowchart LR
  A[Local usage logs] --> B[Usage scanner]
  B --> C[Provider snapshots]
  C --> D[Spend dashboard model]
  D --> E[Session rows]
  D --> F[Daily ledger]
  D --> G[Hourly chart]
Loading

Decision needed

Question Recommendation
Should the unselected hourly chart show only the latest active day, or must users retain a full-range hourly view? Keep both views: Use the latest active day for a readable default and provide an explicit way to inspect the full-range hourly history.

Why: The branch deliberately changes an existing chart view, and the related owner-authored PR explicitly declined to adopt that behavior; code inspection cannot establish the intended product contract.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The changed scanner and dashboard model are covered by focused tests, and the PR reports a CLI build, but the captured evidence does not show the native Settings session, hourly, and daily views after the change in either privacy state. A redacted screenshot or recording of those views would help; redact private paths, prompts, account details, and endpoints. Updating the PR body should trigger a fresh review; if it does not, a maintainer can comment @clawsweeper re-review. The in-memory session fields do not change the stored cache schema. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Honor Hide Personal Info in session rows (P1) - The detail panel passes hidePersonalInfo to project rows but not session rows. With that setting on, the new headline, subtitle, and tooltips can expose a session title or project folder. Pass the setting through and mask every session identity surface. This finding remains from the prior review; the affected files are unchanged since that head.
  • Resolve security concern: Private session identity remains visible - Local session titles and project folder names reach row text and tooltips without the privacy mask used by project rows, exposing potentially sensitive context during screen sharing.
  • Resolve merge risk (P1) - The hourly chart silently replaces its full-range view with the latest active day when no day is selected. Maintainers need to decide whether users should retain an explicit way to inspect the full range.
  • Complete next step (P2) - Mask session identities under Hide Personal Info, resolve the hourly history-access choice, and provide redacted after-fix native dashboard proof before merge.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P1] Honor Hide Personal Info in session rows — Sources/CodexBar/PreferencesSpendDashboardPane.swift:1312-1319
  • [medium] Private session identity remains visible — Sources/CodexBar/PreferencesSpendDashboardPane.swift:1312
Agent review details

Security

Needs attention: The new session identity text bypasses the dashboard's Hide Personal Info setting.

Review metrics

Metric Value Why it matters
Code growth production +102 lines, tests +157 lines The production growth spans three dashboard presentation changes and has focused model and scanner coverage.

Merge-risk options

Maintainer options:

  1. Preserve hourly history access (recommended)
    Add a clear full-range or day-navigation option while keeping the new readable default.
  2. Accept the one-day contract
    Approve the changed chart behavior explicitly and make older-day navigation clear to users.

Technical review

Best possible solution:

Mask session titles, project names, subtitles, and tooltips under Hide Personal Info; then choose a discoverable hourly view that keeps the chart readable while preserving the intended history access, backed by native dashboard proof.

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

Yes for the privacy finding: source shows the setting reaches the detail panel but is omitted from session rows, which render the newly added identifiers. The chart and count changes have focused model tests, but no native after-fix run is supplied.

Is this the best way to solve the issue?

Unclear. Session and daily count presentation follow existing dashboard patterns, but the session view must honor privacy and the hourly default needs an explicit history-access decision.

Full review comments:

  • [P1] Honor Hide Personal Info in session rows — Sources/CodexBar/PreferencesSpendDashboardPane.swift:1312-1319
    The detail panel passes hidePersonalInfo to project rows but not session rows. With that setting on, the new headline, subtitle, and tooltips can expose a session title or project folder. Pass the setting through and mask every session identity surface. This finding remains from the prior review; the affected files are unchanged since that head.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against d8c2af958176.

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This is a bounded dashboard usability improvement with a privacy defect to fix before merge.
  • merge-risk: 🚨 security-boundary: Session titles and project names can remain visible even when Hide Personal Info is enabled.
  • merge-risk: 🚨 compatibility: The unselected hourly chart changes from full-range history to one day without an explicit range view.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed scanner and dashboard model are covered by focused tests, and the PR reports a CLI build, but the captured evidence does not show the native Settings session, hourly, and daily views after the change in either privacy state. A redacted screenshot or recording of those views would help; redact private paths, prompts, account details, and endpoints. Updating the PR body should trigger a fresh review; if it does not, a maintainer can comment @clawsweeper re-review. The in-memory session fields do not change the stored cache schema. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

  • [medium] Private session identity remains visible — Sources/CodexBar/PreferencesSpendDashboardPane.swift:1312
    Local session titles and project folder names reach row text and tooltips without the privacy mask used by project rows, exposing potentially sensitive context during screen sharing.
    Confidence: 0.98

What I checked:

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Yuxin-Qiao: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Sahil Imtiaz: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Mask all session identity text and tooltips when Hide Personal Info is enabled, with focused coverage.
  • Resolve whether full-range hourly history remains accessible.
  • Add redacted after-fix native dashboard proof for the session, hourly, and daily views.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-25T17:02:29.733Z sha f289858 :: needs real behavior proof before merge. :: [P1] Honor Hide Personal Info in session rows

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Sep 25, 2026
@steipete

Copy link
Copy Markdown
Owner

Thanks @Yuxin-Qiao. Closing this as written: session titles and project labels, including tooltips, remain visible with Hide personal info enabled. The hourly change also replaces full-range history with the latest positive-spend day while summary totals retain the full range. No code was adopted. A maintainer-owned split can preserve privacy and handle partial counts separately after deciding the hourly-chart scope.

@steipete steipete closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants