Skip to content

fix: round compact stat badges to nearest instead of truncating#81

Merged
steipete merged 2 commits into
steipete:mainfrom
LeoLin990405:fix/compact-stat-rounding
Jun 13, 2026
Merged

fix: round compact stat badges to nearest instead of truncating#81
steipete merged 2 commits into
steipete:mainfrom
LeoLin990405:fix/compact-stat-rounding

Conversation

@LeoLin990405

@LeoLin990405 LeoLin990405 commented Jun 12, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Summary

StatValueFormatter.compact rounded counts below 10K, but its 10K-1M and 10M-1B branches used integer division and therefore truncated. This patch applies nearest-value rounding consistently, carries 999,500 to 1M, preserves the existing 999M cap, and uses remainder-based arithmetic so Int.max cannot overflow.

value before after
10,500 10K 11K
19,999 19K 20K
99,500 99K 100K
567,890 567K 568K
999,500 999K 1M
10,500,000 10M 11M

The shared formatter covers compact issue, pull request, star, fork, comment, and release download badges.

Tests

Adds seven StatValueFormatterTests cases covering pass-through values, sub-10K decimals, thousand and million rounding, carry into millions, the oversized cap, and Int.max overflow safety.

Verification

  • Built and launched exact head 596f438 via pnpm restart from this repository checkout.
  • Live GitHub count for steipete/CodexBar: 14,711 stars. The built RepoBar menu rendered 15K; the previous truncating branch would render 14K.
  • Redacted native-app screenshot and maintainer proof.
  • pnpm check: SwiftFormat clean, SwiftLint clean, 598 tests across 101 suites passed.
  • Branch autoreview reported no accepted or actionable findings.
  • Public Model Identifier Gate: PASS.

Risk

Low. The change is isolated to the shared compact-number formatter, has boundary regression coverage, and is reproduced in the exact built macOS app.

@LeoLin990405 LeoLin990405 force-pushed the fix/compact-stat-rounding branch from f20fc8b to 426de5e Compare June 12, 2026 12:29
@clawsweeper

clawsweeper Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 13, 2026, 5:06 AM ET / 09:06 UTC.

Summary
This PR rounds compact K/M stat badges in StatValueFormatter to nearest values, adds boundary tests, and updates the unreleased changelog.

Reproducibility: yes. Current-main source uses integer division in the affected formatter ranges, which deterministically truncates examples such as 10,500 to 10K; this read-only review did not relaunch the app locally.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 3 files changed, +67/-8. The change is confined to one shared formatter, its focused tests, and an unreleased changelog entry.
  • Regression coverage: 7 tests added. The new suite covers pass-through values, decimal ranges, K/M rounding, carry behavior, the existing cap, and Int.max safety.
  • Current checks: 2 successful checks. The latest head has successful macOS CI and GitGuardian results in GitHub's status rollup.

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

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

Next step before merge

  • [P2] No repair lane is needed; there are no actionable code findings, and the remaining action is ordinary maintainer merge review.

Security
Cleared: Cleared: the diff only changes local formatter logic, Swift tests, and an unreleased changelog entry, with no dependency, secret, permission, script, or supply-chain change.

Review details

Best possible solution:

Merge the centralized formatter rounding fix with its boundary tests once maintainers are satisfied with exact-head CI and review state.

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

Yes. Current-main source uses integer division in the affected formatter ranges, which deterministically truncates examples such as 10,500 to 10K; this read-only review did not relaunch the app locally.

Is this the best way to solve the issue?

Yes. Updating the shared formatter with overflow-safe nearest rounding and boundary tests is the narrow maintainable fix, rather than changing individual badge call sites.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: a maintainer built and launched exact head 596f438, reported a live count rendering as 15K, and attached a redacted native RepoBar menu screenshot showing that badge.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Sufficient: a maintainer built and launched exact head 596f438, reported a live count rendering as 15K, and attached a redacted native RepoBar menu screenshot showing that badge.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): Sufficient: a maintainer built and launched exact head 596f438, reported a live count rendering as 15K, and attached a redacted native RepoBar menu screenshot showing that badge.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This fixes inaccurate user-visible compact statistics with a limited display-only blast radius.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): Sufficient: a maintainer built and launched exact head 596f438, reported a live count rendering as 15K, and attached a redacted native RepoBar menu screenshot showing that badge.
  • proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: a maintainer built and launched exact head 596f438, reported a live count rendering as 15K, and attached a redacted native RepoBar menu screenshot showing that badge.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Sufficient: a maintainer built and launched exact head 596f438, reported a live count rendering as 15K, and attached a redacted native RepoBar menu screenshot showing that badge.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Git blame attributes the current formatter to the v0.8.2 release commit, follow history shows the formatter extraction, shortlog shows him as the central-file contributor, and he provided exact-head app verification on this PR. (role: feature history owner and recent verifier; confidence: high; commits: 5244d0634ff4, 4610221b51c4, 3488f65e3409; files: Sources/RepoBar/Support/StatValueFormatter.swift, Sources/RepoBar/Views/MenuItemViews.swift, Sources/RepoBar/StatusBar/RepoSubmenuBuilder.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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 12, 2026
@LeoLin990405 LeoLin990405 force-pushed the fix/compact-stat-rounding branch from 426de5e to d6a73d5 Compare June 12, 2026 12:39
@LeoLin990405 LeoLin990405 force-pushed the fix/compact-stat-rounding branch from d6a73d5 to 2c17efc Compare June 12, 2026 17:36
@LeoLin990405

LeoLin990405 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review's two rank-up moves on StatValueFormatter:

  1. Overflow-safe arithmetic — replaced (value + divisor/2) / divisor with value / divisor + (value % divisor * 2 >= divisor ? 1 : 0), removing the intermediate value + 500_000 that could overflow near Int.max.
  2. Int.max test — added caps Int.max without integer overflow (compact(Int.max) == "999M", no trap).

On real-behavior proof: StatValueFormatter is an internal SwiftUI badge helper with no CLI/headless surface, so the closest-to-real evidence is the suite exercising the shipped function directly — 7/7 pass, e.g. 19_999 → "20K", 10_500 → "11K", 999_500 → "1M", Int.max → "999M". swiftformat --lint + swiftlint --strict both clean. Happy to attach a menu-bar screenshot of an affected badge if you'd prefer GUI evidence.

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. label Jun 12, 2026
@steipete

steipete commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Maintainer verification on exact head 596f438:

  • Built and launched the branch via pnpm restart; running binary confirmed at the repository .build path.
  • Live RepoBar menu proof: GitHub reported 14,711 stars for steipete/CodexBar, and the exact branch rendered the compact badge as 15K. The previous truncating branch would render 14K.
  • pnpm check: SwiftFormat clean, SwiftLint clean, 598 tests across 101 suites passed.
  • Focused formatter suite: 7/7 passed, including carry boundaries and Int.max overflow safety.
  • Branch autoreview: no accepted or actionable findings.
  • Public Model Identifier Gate: PASS. The exact diff and public proof contain no model-bearing identifiers or generated model metadata.
  • Remaining risk: low; behavior is centralized in the shared formatter and verified both at boundaries and in the built app.

Recommendation: land after exact-head CI is green.

@clawsweeper re-review
pr81-ui-proof-crop

@clawsweeper

clawsweeper Bot commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦞 diamond lobster Very strong PR readiness with only minor 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: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 13, 2026
@steipete steipete merged commit 393f19e into steipete:main Jun 13, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@LeoLin990405 @steipete