Skip to content

Clarify CodexBar spend overview and share card - #3892

Closed
Chipagosfinest wants to merge 18 commits into
steipete:mainfrom
Chipagosfinest:codex/share-card-widget-redesign
Closed

Chipagosfinest wants to merge 18 commits into
steipete:mainfrom
Chipagosfinest:codex/share-card-widget-redesign

Conversation

@Chipagosfinest

@Chipagosfinest Chipagosfinest commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Status

Reviewable source and synthetic visual proof. A debug app package built and passed signature/integrity checks at d12fc80e5, but image export and warm/cold widget-link behavior remain unverified. This PR is not merge-ready yet.

Current changes

  • Overview shows aggregate spend with provider, history, and pricing coverage. Provider rows keep configured notes and detail while replacing a repeated inline usage dashboard with a compact activity readout.
  • The 1200 × 630 local share image leads with tracked tokens and a proportional model-family composition. Identically named models aggregate across serving providers in this model-family view. Cost rows show currency, covered days, and basis; two rows fit in the image and Copy Stats carries the complete currency list.
  • The share preview continues to offer Copy Image, Copy Stats, and Save Image. Medium and large widget share controls route to the aggregate preview; the small widget omits the control to keep long provider names readable. All-time cost coverage says the number of covered days instead of 12/all.

Visual proof

Both images below are native SwiftUI renders with synthetic data from before the d12fc80e5 footer copy change. They demonstrate the composition and bounded currency rows, but are not after-fix exports or packaged-app proof.

Share image with synthetic model mix

Share image with four synthetic currencies

The design note records the hierarchy, reference boundary, and critique. The consumption research covers the menu, Spend & Usage, widget, and share preview.

Earlier Overview flow mock and native summary render predate the latest note, detail, and pricing-coverage fixes and should not be treated as after-fix proof.

Verification and gates

  • swift build --jobs 4, targeted SwiftFormat and strict SwiftLint, and git diff --check passed on the latest local source. The exact SwiftUI card view was rendered at 1200 × 630 with one and four synthetic currencies and inspected at social preview size.
  • ./Scripts/package_app.sh debug completed at d12fc80e5; the separate com.steipete.codexbar.debug bundle and embedded widget passed the packaging integrity/signature checks. The packaged binary SHA-256 is 121d7dc603225562fd80cfcdcee4dd4b46a50ff7fbe49d3bee59252f7091b75a. The installed app was left running.
  • With the debug app closed, open -a CodexBar.app codexbar://share/overview started its process (PID 71742); issuing the same URL while it ran kept that PID. This confirms OS-level cold launch and warm delivery requests only. Native UI capture timed out (also for Preview), so the preview, saved PNG, and actual widget click remain unverified. No private usage image is posted.
  • GitHub CI on d12fc80e5 is pending. The preceding head had one macOS shard fail in an unchanged provider-plugin timeout test; the remaining build/lint/shard checks passed. No local test suite or live account probe was run in this pass.
  • Before merge: capture a redacted export from the packaged app and verify widget-link activation with the app both running and closed. Maintainer review must settle whether this compact export retains enough provider/subscription context for existing users.

@clawsweeper

clawsweeper Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added 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. labels Sep 23, 2026
@clawsweeper

clawsweeper Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 24, 2026, 12:49 AM ET / 04:49 UTC (Revision 12).

ClawSweeper review

What this changes

Redesigns the Overview spend summary and locally generated share image, adds a widget link to the share preview, and updates packaging, tests, and design documentation.

Merge readiness

⛔ Blocked before merge - 5 items remain

The PR adds a distinct share-card design and widget entry point that current main does not provide. Keep it open: a source-level overflow remains, the available visuals do not prove the latest packaged export or widget click, and the loss of provider context needs an owner decision.

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

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The design and focused tests provide useful signal, but real-path proof, a small arithmetic repair, and the information tradeoff remain unresolved.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: The inspected synthetic SwiftUI images demonstrate the share-card composition but predate the latest footer change and do not show a packaged-app image export. The reported cold and warm launch requests use a URL different from the widget’s exact route, and no actual widget click or preview result is shown. A redacted exported image plus warm and cold widget activation evidence is needed; private details should be removed before posting. Updating the PR body should trigger a fresh review, or a maintainer can request @clawsweeper re-review. No stored-data contract changes were found. 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) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The inspected synthetic SwiftUI images demonstrate the share-card composition but predate the latest footer change and do not show a packaged-app image export. The reported cold and warm launch requests use a URL different from the widget’s exact route, and no actual widget click or preview result is shown. A redacted exported image plus warm and cold widget activation evidence is needed; private details should be removed before posting. Updating the PR body should trigger a fresh review, or a maintainer can request @clawsweeper re-review. No stored-data contract changes were found. 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 7 items Introduced surface: The pinned main-to-head diff changes the share card, Overview presentation, widget control, URL route, packaging, and focused tests.
Remaining overflow: Each family uses a checked integer sum, but the folded tail is summed as Double and converted back with Int(remainder). Five distinct families of Int.max tokens can leave a positive remainder rounded beyond Int.max, causing a trap.
Prior finding continuity: The share-card blob at the prior reviewed head and current head has the same object ID, e062af1eb99c873a49d047f00ac75b9181644cce; the folded-total concern was already visible.
Findings 1 actionable finding [P3] Guard the folded model total before converting to Int
Security None None.

How this fits together

CodexBar turns local usage and spend history into the Overview menu and a shareable image. The PR also lets a widget open that image’s preview in the app.

flowchart LR
A[Local usage history] --> B[Spend summary]
B --> C[Overview menu]
B --> D[Share payload]
D --> E[Share preview]
E --> F[Copied or saved image]
G[Widget share control] --> E
Loading

Decision needed

Question Recommendation
Should the compact share image retain visible provider or subscription identification for existing users? Retain concise provider context: Add a bounded provider or subscription identifier area while preserving the compact composition.

Why: The branch intentionally replaces the existing provider and subscription rows with a model-family composition; source inspection cannot settle the intended information tradeoff.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The inspected synthetic SwiftUI images demonstrate the share-card composition but predate the latest footer change and do not show a packaged-app image export. The reported cold and warm launch requests use a URL different from the widget’s exact route, and no actual widget click or preview result is shown. A redacted exported image plus warm and cold widget activation evidence is needed; private details should be removed before posting. Updating the PR body should trigger a fresh review, or a maintainer can request @clawsweeper re-review. No stored-data contract changes were found. 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.
  • Guard the folded model total before converting to Int (P3) - With five distinct families each holding Int.max tokens, the fifth remains in remainder; Double(Int.max) rounds above the Int range, so Int(remainder) traps while building the share image. Use checked integer accumulation or format the tail without an unsafe conversion. This was visible in the prior reviewed head and remains unfixed.
  • Resolve merge risk (P1) - The new image removes the existing subscription and provider rows. Existing users may lose the context needed to interpret or identify a shared snapshot; the owner must decide whether the compact card retains enough context.
  • Complete next step (P2) - Guard the folded model total, provide a redacted packaged-app export and actual warm/cold widget-click evidence, and obtain an owner decision on provider context before merge.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P3] Guard the folded model total before converting to Int — Sources/CodexBar/ShareStatsCardView.swift:229
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Line changes by surface production +184 net, tests +132 net, text docs +553 net The production growth supports the new widget route and UI; the substantial design and research material should be reviewed with the product direction.

Merge-risk options

Maintainer options:

  1. Keep concise source context (recommended)
    Add bounded provider or subscription identification to the image and verify it remains readable in a real export.
  2. Approve the reduced context
    Explicitly accept that the image omits provider detail while copied statistics retain the complete breakdown.

Technical review

Best possible solution:

Ship a compact share card that keeps its amounts, coverage, and model mix interpretable, with safe arithmetic and a verified packaged-app export and widget handoff.

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

Yes for the overflow path: five distinct positive model families at Int.max reach an out-of-range Double-to-Int conversion in the folded segment. The actual packaged export and widget click have not been observed.

Is this the best way to solve the issue?

Unclear. The compact presentation is coherent, but safe tail arithmetic, real export proof, and an owner decision on provider context are needed before it can be judged the best replacement.

Full review comments:

  • [P3] Guard the folded model total before converting to Int — Sources/CodexBar/ShareStatsCardView.swift:229
    With five distinct families each holding Int.max tokens, the fifth remains in remainder; Double(Int.max) rounds above the Int range, so Int(remainder) traps while building the share image. Use checked integer accumulation or format the tail without an unsafe conversion. This was visible in the prior reviewed head and remains unfixed.
    Confidence: 0.95

Overall correctness: patch is correct
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This is a bounded user-facing sharing improvement with an unresolved edge-case defect and proof gap.
  • merge-risk: 🚨 compatibility: The redesigned image removes provider and subscription rows that existing shared images show.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The inspected synthetic SwiftUI images demonstrate the share-card composition but predate the latest footer change and do not show a packaged-app image export. The reported cold and warm launch requests use a URL different from the widget’s exact route, and no actual widget click or preview result is shown. A redacted exported image plus warm and cold widget activation evidence is needed; private details should be removed before posting. Updating the PR body should trigger a fresh review, or a maintainer can request @clawsweeper re-review. No stored-data contract changes were found. 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.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The inspected synthetic SwiftUI images demonstrate the share-card composition but predate the latest footer change and do not show a packaged-app image export. The reported cold and warm launch requests use a URL different from the widget’s exact route, and no actual widget click or preview result is shown. A redacted exported image plus warm and cold widget activation evidence is needed; private details should be removed before posting. Updating the PR body should trigger a fresh review, or a maintainer can request @clawsweeper re-review. No stored-data contract changes were found.

Evidence

What I checked:

  • Introduced surface: The pinned main-to-head diff changes the share card, Overview presentation, widget control, URL route, packaging, and focused tests. (Sources/CodexBar/ShareStatsCardView.swift:203, 29cc0662fdba)
  • Remaining overflow: Each family uses a checked integer sum, but the folded tail is summed as Double and converted back with Int(remainder). Five distinct families of Int.max tokens can leave a positive remainder rounded beyond Int.max, causing a trap. (Sources/CodexBar/ShareStatsCardView.swift:229, 29cc0662fdba)
  • Prior finding continuity: The share-card blob at the prior reviewed head and current head has the same object ID, e062af1eb99c873a49d047f00ac75b9181644cce; the folded-total concern was already visible. (Sources/CodexBar/ShareStatsCardView.swift:229, 29cc0662fdba)
  • Widget route: The widget emits codexbar://share-stats?version=1. The PR body reports cold and warm OS delivery for codexbar://share/overview, a different URL, and explicitly leaves the preview and widget click unverified. (Sources/CodexBarCore/ShareStatsRoute.swift:7, 29cc0662fdba)
  • Visual proof scope: The prepared 1200 × 630 images visibly show the model mix and one- and four-currency layouts. The PR body identifies them as synthetic native renders predating its latest footer change, not packaged-app exports. The earlier Overview images were inspected from the checked-out branch and also predate later changes. (docs/screenshots/share-stats-og-multicurrency-synthetic.png, 29cc0662fdba)
  • Existing share behavior: Current main already provides an Overview share preview and local image export, but its share image includes subscription rows; the new widget route is absent. The proposed redesign remains distinct work. (docs/ui.md:24, 5f70bfc33607)

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)
  • Chipagosfinest: 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.

  • Capture a redacted image exported by the packaged app and the result of actual widget clicks with the app running and closed.
  • Guard the folded model token total against out-of-range conversion.
  • Obtain the owner’s decision on provider and subscription context in the image.

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 (11 earlier review cycles; latest 8 shown)
  • reviewed 2026-09-23T19:15:07.525Z sha b293223 :: needs real behavior proof before merge. :: [P2] Bound currency rows within the exported image | [P2] Preserve coverage beside exported cost totals | [P2] Disambiguate model segments before showing percentages | [P2] Count distinct providers in the footer
  • reviewed 2026-09-23T19:33:31.991Z sha 767065b :: needs real behavior proof before merge. :: [P2] Bound currency rows within the exported image | [P2] Keep coverage beside exported cost totals | [P2] Disambiguate model segments before showing percentages | [P2] Count distinct providers in the footer | [P2] Preserve partial-history warnings in overview activity
  • reviewed 2026-09-23T20:10:01.989Z sha c5532dd :: needs real behavior proof before merge. :: [P2] Bound currency rows within the exported image | [P2] Keep coverage beside exported cost totals | [P2] Disambiguate model segments before showing percentages | [P2] Count distinct providers in the footer | [P2] Preserve partial-history warnings in overview activity | [P2] Restore pricing coverage in the overview summary
  • reviewed 2026-09-23T20:41:05.806Z sha 1e10aa2 :: needs real behavior proof before merge. :: [P1] Keep configured provider details in Overview | [P2] Bound currency rows within the exported image | [P2] Keep coverage beside exported cost totals | [P2] Disambiguate model segments before showing percentages | [P2] Count distinct providers in the footer | [P2] Preserve partial-history warnings in overview activity
  • reviewed 2026-09-23T21:22:47.664Z sha 6615fb7 :: needs real behavior proof before merge. :: [P1] Keep configured provider details in Overview | [P2] Bound currency rows within the exported image | [P2] Keep coverage beside exported cost totals | [P2] Disambiguate model segments before showing percentages | [P2] Count distinct providers in the footer | [P2] Preserve partial-history warnings in Overview activity
  • reviewed 2026-09-24T01:26:44.459Z sha 648b7d1 :: needs real behavior proof before merge. :: [P2] Label the share image’s mixed data sources accurately
  • reviewed 2026-09-24T01:44:40.999Z sha 67234a9 :: needs real behavior proof before merge. :: [P2] Label the share image’s mixed data sources accurately
  • reviewed 2026-09-24T03:25:43.052Z sha d12fc80 :: needs real behavior proof before merge. :: [P3] Guard the folded model total before converting to Int

@clawsweeper clawsweeper Bot added the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label Sep 23, 2026
@Chipagosfinest
Chipagosfinest marked this pull request as draft September 23, 2026 05:50
@Chipagosfinest Chipagosfinest changed the title Polish share usage card and widget action Draft: Rework share usage card and consumption language Sep 23, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Sep 23, 2026
@Chipagosfinest Chipagosfinest changed the title Draft: Rework share usage card and consumption language Clarify CodexBar spend overview and share card Sep 23, 2026
@clawsweeper clawsweeper Bot removed the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label Sep 23, 2026
@Chipagosfinest
Chipagosfinest marked this pull request as ready for review September 23, 2026 20:34
@clawsweeper clawsweeper Bot added the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label Sep 23, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Sep 24, 2026
Chipagosfinest and others added 2 commits September 23, 2026 18:38
showsSupplementalContent was always true at its only call site after
notes and provider details were restored, so only the inline dashboard
flag remains. ShareStatsRoute.parse already required an exact URL match,
which made the component checks after it unreachable.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Replace the circular provenance test with one that drives the builder,
and cover exact-URL route parsing, pending-route delivery before the
status controller exists, model-family merging and tail folding,
overflow and empty mixes, and per-currency Overview spend amounts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@steipete

Copy link
Copy Markdown
Owner

Thanks @Chipagosfinest for the research and implementation. Closing this proposal: the current head includes a share-card and Overview redesign, widget controls, and a new URL route, beyond the bounded wording change under review. The new “LOCAL USAGE LOGS” footer is not valid for provider-reported or mixed inputs, and list-price provenance does not establish that activity was “not billed.” The complete-provider/partial-model behavior from #3817 is already retained on main. We are keeping the current presentation and not adopting this patch.

@steipete steipete closed this Sep 24, 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. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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