fix: net earnings subtracted a EUR cost from a USD gross (beads batch 13) - #196
Merged
Conversation
CashPilot-dlr. gross comes from get_earned_by_platform, which is USD by contract. cost is computed from a tariff the user entered in power_currency. Subtracting one from the other produced a number in neither, labelled with the tariff currency. At roughly 1.08 USD/EUR that is an ~8% error, in the direction that flatters the result, on the single figure that decides whether a machine is worth keeping powered on. Gross is now converted into the tariff currency first, so the subtraction is like-for-like. Measured on a seeded request: 100 USD gross with a 0.92 rate reports 92.00 EUR against 3.97 EUR of electricity. exchange_rates gains from_usd, deliberately next to to_usd because the direction is the easy thing to get wrong: _fiat_rates holds USD->X rates, so from_usd MULTIPLIES where to_usd divides. Inverting it would report 109 EUR for 100 USD and flatter the number further — worse than the bug. A round-trip assertion pins it. Fiat only. A USD figure has no meaningful expression in a provider's token, and inventing one would put a crypto amount where a currency belongs. With no rate available the response stays in USD and says so via fx_unavailable and tariff_currency, rather than quietly mixing the two — the same rule the module already follows when no tariff is set at all. Negative control: removing the conversion fails four tests.
CashPilot-dlr. gross comes from get_earned_by_platform, which is USD by contract; cost is computed from a tariff the user entered in power_currency. Subtracting one from the other produced a number in neither, labelled with the tariff currency — at ~1.08 USD/EUR an 8% error, in the direction that flatters the result, on the figure that decides whether a machine is worth keeping powered on. The TARIFF is converted into USD, not the gross into the tariff currency. My first attempt did the latter and it was the wrong call: every other money figure in this API is canonical USD and the frontend has a display-currency layer, so converting the gross would have made this one endpoint the exception. Converting the price keeps the convention and needs no new rate direction. With no rate the endpoint reports cost_known: false and says why, rather than producing a net from two currencies. That mirrors what the module already does with no tariff at all: "a zero cost would render gross as net and quietly overstate earnings." Passing "USD" as a label while the price stayed EUR-denominated — which I tried first — only moves the mixing rather than removing it. conftest now seeds the fiat rates a RUNNING system always has. exchange_rates.refresh() populates them at startup and every 15 minutes, but the test process never calls it, so _fiat_rates was empty everywhere. Ten tests configure a EUR tariff and expect a cost; with no rate the endpoint correctly refuses, and they failed for a reason unrelated to what they test. Seeded rather than mocked per test, because "a rate exists" is the normal state. Two existing assertions changed, both of which encoded the defect: test_power expected currency EUR on a figure whose gross was USD. Negative control: restoring the mixing fails four tests.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesCurrency normalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 #196 +/- ##
==========================================
- Coverage 95.09% 95.07% -0.02%
==========================================
Files 46 46
Lines 5889 5908 +19
==========================================
+ Hits 5600 5617 +17
- Misses 289 291 +2
🚀 New features to boost your workflow:
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
GeiserX
added a commit
that referenced
this pull request
Aug 4, 2026
Every gross in /api/fleet/economics comes from get_earned_by_platform, which is USD by contract. The electricity price came straight from the tariff config, in whatever currency the user set, and machine_economics does net = gross-cost. Live that rendered gross 5.00 (USD) minus cost 14.24 (EUR, 65 W at EUR 0.30/kWh) = net -9.24, verdict "losing money", with "turning it off would save that" — advice to switch off hardware, wrong by the whole FX spread. The true net was about -$11.35, not -9.24 of any currency. With a weaker tariff currency the error is not 15% but orders of magnitude. The payload also carried no currency field at all, and fleet.html rendered Number(v).toFixed(2), so the user saw bare numbers they could not attribute to a unit — while those numbers silently mixed two. Fixed the way /api/earnings/net was in #196: convert the TARIFF to USD, so the endpoint stays canonical USD like the rest of the API and the frontend's display-currency layer renders it in whatever the viewer reads in. When no rate is available the cost is None, not zero — a zero cost would render gross as net and overstate earnings, which is the mistake this module already guards against when no tariff is set at all. formatCurrency is now exported on CP so fleet.html can reach the one function that knows the viewer's display currency. null still renders as an em dash, so "not reporting" stays distinct from "earns nothing". Negative control, each part separately: not converting the tariff fails 2 tests; treating a missing rate as usable fails 1; dropping the currency label fails 1; restoring the bare-number renderer fails 1. Nothing overlaps. Closes CashPilot-dlr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dlr—grossis USD by contract;costcomes from a tariff inpower_currency. Subtracting one from the other produced a number in neither, labelled with the tariff currency.At ~1.08 USD/EUR that's an 8% error, in the direction that flatters the result, on the single figure that decides whether a machine is worth keeping powered on.
The tariff is converted into USD, not the gross into the tariff currency. My first attempt did the latter and it was wrong: every other money figure in this API is canonical USD and the frontend has a display-currency layer, so converting the gross would have made this endpoint the exception. Converting the price keeps the convention and needs no new rate direction.
With no rate available it reports
cost_known: falseand says why, rather than inventing a net. That mirrors what the module already does with no tariff at all — "a zero cost would render gross as net and quietly overstate earnings." Passing"USD"as a label while the price stayed EUR-denominated (my second attempt) only moves the mixing.conftestnow seeds the fiat rates a running system always has.exchange_rates.refresh()populates them at startup and every 15 minutes, but the test process never calls it, so_fiat_rateswas empty everywhere. Ten tests configure a EUR tariff and expect a cost; with no rate the endpoint correctly refuses, so they failed for reasons unrelated to what they test.Two existing assertions changed, both encoding the defect —
test_powerexpectedcurrency == "EUR"on a figure whose gross was USD.Verification: 2533 tests, 95.12% coverage, both ruff gates, JS parses. Negative control: restoring the mixing fails four tests.
Summary by CodeRabbit