Fix stacked bar corner radius - #3439
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P3
|
Codex review: blocked before merge. Reviewed September 5, 2026, 5:39 PM ET / 21:39 UTC. ClawSweeper reviewWhat this changesThe PR makes daily and hourly spend bars square at provider boundaries and baselines while retaining rounded tops, with shared selection logic and regression coverage. Merge readiness⛔ Blocked before merge - 1 item remains The chart fix remains useful: neither current main nor v0.56.6 contains it. No actionable defect was found, and the owner’s native verification supports the rendering correction. Repository policy also prohibits automatic cleanup closure. Priority: P3 Review scores
Verification
How this fits togetherCodexBar’s Usage & Spend dashboard turns provider cost history into daily and hourly stacked charts. This change adjusts how those segments are drawn without changing their amounts or offsets. flowchart LR
A[Provider cost history] --> B[Spend dashboard model]
B --> C[Daily and hourly segments]
C --> D[Select highest segment per bucket]
D --> E[Square native marks and clip tops]
E --> F[Usage and Spend charts]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Use the shared top-segment selector and top-only clipping in both charts while preserving the existing spend model and accessibility output. Do we have a high-confidence way to reproduce the issue? Yes: current main still draws independently rounded provider marks, and the supplied native baseline records the resulting seams. This reviewer inspected the source and harness but did not execute the app. Is this the best way to solve the issue? Yes: disabling native rounding before top-only clipping directly addresses the lost corner pixels, with one selector shared by both charts and explicit trailing-zero coverage. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 34ac0879c30f. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
Retain the API request region when formatting Moonshot balances and deficits, and preserve formatted currency symbols in CLI presentation. China accounts display CNY; international accounts retain USD. Fixes steipete#3434. Co-authored-by: Som Samantray <som.samantray@gmail.com>
Preserve the last confirmed Command Code allowance during optional subscription timeouts and calculate monthly usage from fresh credits. Leave unknown monthly windows unavailable instead of displaying an untouched paid grant. Keep bounded credential-scoped observations, including invalidations, so older responses cannot overwrite newer plans or resurrect cleared allowances. Limit the synthetic endpoint override to debug loopback origins; release builds use the official endpoint. Include regression coverage, synthetic CLI transport proof, documentation, and an Unreleased changelog entry. Thanks @enieuwy for the original diagnosis and contribution. Co-authored-by: enieuwy <121954036+enieuwy@users.noreply.github.com>
Retain contributor stack-top selection and clipping, disable built-in mark rounding so internal seams become square, and add signed native proof with realistic zero-height coverage. Co-authored-by: Elijah Friedman <efriedman810@gmail.com>
|
Signed native before/after proof using the production Usage & Spend currency section and synthetic data. The fixture includes three providers, trailing zero-cost segments, and very thin segments. Both daily and hourly charts retain their totals, series, and geometry. Before (chart/model source identical to main b921ed2):
After (6ecb6f8):
Native proof exposed why clipping alone was insufficient: Swift Charts had already rounded the underlying marks. The final change sets the native corner radius to zero before applying the top-only clipping shape. Both captures were inspected in full and contain synthetic fixture data only. |
|
Merged as 211781b. Daily and hourly stacked spend bars now have square internal seams and baselines, with rounded outer tops. Native proof exposed the missing step in the original proposal: disabling built-in mark rounding before applying the top-only clip. Validation: The signed native fixture hosts the actual production currency section and passed before and after. Inspected synthetic before/after captures cover three providers, zero-height and thin segments; both attachments are anonymously downloadable with hashes matching the inspected files. Fixture totals and offsets stayed unchanged. The final tree combines cleanly with the independent ElevenLabs fix. The Unreleased changelog is updated, and @elijahfriedman's contribution and commit credit are preserved. |
4 participants
The daily and hourly spend charts round each provider segment independently, leaving notches at internal stack boundaries and rounded baselines. This fix makes the internal seams and baseline square while preserving rounded outer tops.
The shared selector identifies the highest visible segment per day/hour. Native verification found that clipping alone did not fix the seams: the original rounded mark had already lost those corner pixels. Both charts now disable native mark rounding before applying the per-segment top-only clip. A realistic trailing-zero regression ensures an invisible segment cannot replace the visible top.
Thanks @elijahfriedman for the original contribution. Contributor commits and human credit are preserved, and the Unreleased changelog is updated.
Native behavior proof
SpendStackedChartsNativeProofTestshosts the existing productionSpendDashboardCurrencySectionin a signed Developer ID XCTest window. It builds the same data throughSpendDashboardModel.buildfor both captures; it does not recreate the chart drawing code. Before uses chart/model source identical to main b921ed2, and after uses the final candidate.The fixture covers three providers, normal stacks, single visible providers, all-zero buckets, zero-height trailing/middle segments, and very thin segments. Assertions verify all 18 daily and 18 hourly points, contiguous offsets, $137.15 daily total, and $74.40 hourly total. Captures contain synthetic data only.
Validation
swift test --no-parallel --filter 'SpendStackedBarChartTests|SpendDashboardModelTests': 50 tests in two suites passed.make check: passed, zero violations across 2,128 files.make test: all 1,024 selections across 86 groups passed on the first attempt, with no retries or timeouts.