feat(ui): show the account-wide picture while paired, and only then - #116
Conversation
The other half of CashPilot-Desktop-xjr. Pairing already hands the server the history this machine collected alone; this shows the result. The heartbeat response has always carried the server's account-level figures for the platforms this machine runs. Nothing read them. The dashboard now renders them as 'Across your CashPilot account', below the local numbers rather than replacing them. Unlinking removes the panel and leaves exactly the local numbers as before. That is the behaviour the whole design exists for, and it needs no restore step: the history was COPIED upstream, never moved, so there is only a view to switch back. Two independent barriers enforce it -- stopUpstream drops the cached figures, and fleetView refuses when no server is configured. Two rules the rendering is careful about: * A platform with no reading renders as an em dash, NEVER 0.00. No reading usually means a collector that does not exist yet, or credentials nobody entered. Showing zero reports a loss that did not happen, and it does so most convincingly to the user whose collector is broken -- exactly the person who must not be told all is well. The platforms with no reading are named underneath, so a total that looks low is explicable rather than mysterious. * A platform on more than one machine is marked shared. Earnings are collected per PLATFORM from the provider; if two machines run the same service the provider reports one balance and nothing can split it. The panel is therefore labelled as the account's, not this machine's. A heartbeat that reports nothing leaves the last figures alone rather than blanking them: one bad minute on the server is not evidence the account earned nothing. Refs: CashPilot-Desktop-xjr
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe application now parses fleet earnings from heartbeat responses, caches valid reports, exposes them through ChangesFleet earnings
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
+ Coverage 74.24% 74.45% +0.20%
==========================================
Files 18 18
Lines 3747 3785 +38
==========================================
+ Hits 2782 2818 +36
+ Misses 762 761 -1
- Partials 203 206 +3
🚀 New features to boost your workflow:
|
…ns it They lived in package main, exercised the function perfectly, and counted for nothing: go test measures coverage per-package, so a call from another package left internal/upstream reading 0% for ParseEarnings. codecov/patch failed on it, which is the gate working.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@upstream_client.go`:
- Around line 136-142: Update stopUpstream so it first cancels the old upstream
loop and waits for done, then acquires a.upstream.mu before clearing
fleetEarnings and fleetEarningsAt; preserve the existing shutdown flow while
ensuring sendUpstream cannot repopulate the cache afterward. Add an interleaving
test covering a response completing during unlinking and verify the cache
remains cleared.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 80138b13-089d-425b-b362-10bc1513c8e0
📒 Files selected for processing (9)
CHANGELOG.mdapp.gofrontend/src/main.tsfrontend/src/style.cssfrontend/src/wails.d.tsinternal/upstream/earnings_test.gointernal/upstream/upstream.goupstream_client.goupstream_fleetview_test.go
stopUpstream cleared the cached fleet figures and THEN cancelled the loop. A heartbeat already returned from the server -- merely not yet holding the mutex -- writes the old server's figures after the clear. Re-pair to a DIFFERENT server and fleetView then presents those figures under the new server's URL. That is worse than showing nothing: the label makes the wrong number look authoritative. Cancel and wait for the loop to exit first, then clear under the lock. The clear now runs on both branches, including the commonest one of all -- no loop running, which is a Desktop that was never paired or one being stopped twice. Driven deterministically rather than by racing and hoping: the test's stand-in loop writes figures only once it OBSERVES the cancel, so the write is guaranteed to land in the window the old ordering left open. A test that merely raced would have passed on the broken code most of the time. Proven by control -- restoring the old ordering fails it with the old server's 99.0 still cached. Reported by CodeRabbit on PR #116.
1 participant
The other half of
CashPilot-Desktop-xjr. #115 hands the server the history this machine collected alone; this shows the result.What
The heartbeat response has always carried the server's account-level figures for the platforms this machine runs. Nothing read them. The dashboard now renders them as "Across your CashPilot account", below the local numbers rather than replacing them.
Unlinking
Removing the pairing removes the panel and leaves exactly the local numbers as before. That is the behaviour the whole design exists for, and it needs no restore step — the history was copied upstream, never moved, so there is only a view to switch back.
Two independent barriers enforce it:
stopUpstreamdrops the cached figures, andfleetViewrefuses when no server is configured. Both are tested separately, and the second one exists because a negative control found it untested — deleting the config check left every other test passing, since clearing the cache already covered them. A barrier nothing exercises is a barrier the next person deletes as dead code.Two rules the rendering is careful about
—, never0.00. No reading usually means a collector that does not exist yet, or credentials nobody entered. Showing zero reports a loss that did not happen — most convincingly to the user whose collector is broken, who is exactly the person who must not be told everything is fine. The platforms with no reading are named underneath, so a total that looks low is explicable rather than mysterious.shared. Earnings are collected per platform from the provider; if two machines run the same service the provider reports one balance and nothing can split it. So the panel is labelled as the account's, not this machine's.A heartbeat that reports nothing leaves the last figures alone rather than blanking them — one bad minute on the server is not evidence the account earned nothing.
Verification
go build,go vet,gofmtclean;go test -race ./...green.tsc5.9.3 in Docker (npx tscsilently installs a bogustsc@2.0.4that checks nothing and exits 0). Proven by control: injectingconst _bad: number = fleet.serverUrlgivesTS2322and exit 2, so the checker is genuinely running.vite buildsucceeds.fleetserialises asnull(not omitted, not{}) when there is nothing to show — the frontend branches on it.Known gap, filed rather than hidden
frontend/has no test runner at all, so the null-vs-zero rule is proven up to the JSON boundary and no further. Filed as its own bead with the concrete first step (main.tsis a single 1700-line file with import-time side effects, so nothing in it can be imported by a harness today) and an explicit warning not to write tests that assert on the source text.Summary by CodeRabbit
New Features
—.Bug Fixes