fix: an unreachable machine is not a machine earning nothing (beads batch 8) - #190
Conversation
Batch 8. Two findings, one mistake: both endpoints are built only from ONLINE workers, and both then presented the resulting absence as a measured fact. CashPilot-daq — an offline worker was told to switch itself off. per_worker_gross is built from online workers only, and the endpoint defaulted a missing entry to 0.0, so a host that had merely stopped heartbeating produced a confident financial recommendation: "geiserback earns about 0.00 a month and costs about 9.49 in electricity — roughly 9.49 out of pocket. Since this machine runs only these services, turning it off would save that." Its earnings were also being silently reattributed to whatever workers were still reporting, so the rest of the fleet looked better than it was at the same moment. assess_machine now takes monthly_gross | None and returns UNKNOWN with no cost, no net and no gross when it is None. A machine that genuinely earned 0.00 is still judged — asserted, because a fix that stopped judging anything would pass the other tests. CashPilot-1qy — an unreachable worker read as "you have no services". Three minutes after a host stops heartbeating — a reboot, a network blip, a worker container restart — the table emptied and the dashboard stated as fact that the user had nothing and should start over, with a Setup Wizard button. The containers were still running and still earning. The empty state now asks /api/workers before deciding which sentence to show, says the containers keep running and earning while a worker is offline, and — when it cannot even reach that endpoint — asserts neither case rather than guessing. A genuinely empty install still gets the wizard. Negative controls: removing the unknown-gross branch fails three tests, collapsing the empty-state condition fails one.
|
Warning Review limit reached
Next review available in: 22 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 (4)
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #190 +/- ##
=======================================
Coverage 95.08% 95.08%
=======================================
Files 46 46
Lines 5880 5883 +3
=======================================
+ Hits 5591 5594 +3
Misses 289 289
🚀 New features to boost your workflow:
|
Found in my own fresh review of this PR, not by an audit agent. Making an unreachable machine report monthly_gross: None fixed the per-machine verdict but pushed the same mistake up one level. The fleet total sums that None as 0.0, so the headline gross silently shrinks by whatever the unreachable machine earns, with nothing saying so — a number that quietly gets smaller and looks like a real decline. fleet_summary already guards exactly this for cost, and its docstring says so: 'so the total never quietly understates what the fleet costs'. The guard had simply never been extended to gross, because until this PR gross was never unknown. It now counts unreadable machines and says so in the summary. Also asserted that fleet.html can render a null gross. Its money() helper already returns an em dash for null, but monthly_gross only became nullable in this PR, so a later simplification of that helper could turn it into '0.00' or NaN without any test objecting.
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Two findings, one mistake: both endpoints are built only from online workers, and both then presented the resulting absence as measured fact.
daq— an offline worker was told to switch itself off.per_worker_grosscomes from online workers only, and the endpoint defaulted a missing entry to0.0. So a host that had merely stopped heartbeating produced a confident financial recommendation:Its earnings were also silently reattributed to whatever workers were still reporting, so the rest of the fleet looked better than it was at the same moment.
assess_machinenow takesmonthly_gross | Noneand returnsUNKNOWNwith no cost, net or gross. A machine that genuinely earned0.00is still judged — asserted, because a fix that stopped judging anything would pass the other tests.1qy— an unreachable worker read as "you have no services".Three minutes after a host stops heartbeating, the table emptied and the dashboard stated as fact that the user had nothing and should start over — with a Setup Wizard button. The containers were still running and still earning.
The empty state now asks
/api/workersbefore choosing its sentence, says containers keep running and earning while a worker is offline, and when it cannot reach that endpoint either, asserts neither case rather than guessing. A genuinely empty install still gets the wizard.Verification: 2493 tests, 95.13% coverage, ruff clean, JS parses. Negative controls: removing the unknown-gross branch fails 3 tests; collapsing the empty-state condition fails 1.