feat: report net profit, not just gross earnings (CashPilot-f5u) - #161
Conversation
Every dashboard in this space reports what a service PAID. None report what it COST to run. For a large share of users the honest number is negative: a service earning EUR 2/month on hardware drawing 15W at EUR 0.30/kWh is losing EUR 1.29 a month, and we presented that as income. GET /api/earnings/net reports gross, estimated electricity cost and net per service, and names the services whose trailing net is below zero. Three rules are built into the module and tested directly, because the failure mode here is dishonesty rather than a crash: - An estimate is labelled as one. The CPU-share model is crude; it is directionally right and far better than pretending cost is zero, but every figure carries its quality so a UI cannot render it with false precision. - Net is reported ALONGSIDE gross, never instead of it. - With no tariff configured, the cost is reported as UNKNOWN rather than as zero. A zero would render gross as net and overstate earnings, which is the exact thing this bead exists to stop. Two modelling decisions worth stating: a host's idle draw is shared across its containers rather than billed to each one, because a machine kept running for ten containers costs its idle draw once; and a host the user does not pay power for - a VPS, whose bill does not move with CPU - is charged nothing, since inventing a cost is as dishonest as ignoring one. Scope: this is the calculation and the API. Surfacing it in the dashboard belongs with the app.js work (CashPilot-cyc), and real measurement from RAPL or a smart plug can replace the estimate later without changing the contract, since cost_quality already distinguishes them.
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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)
📝 WalkthroughWalkthroughChangesNet-profit reporting
🚥 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #161 +/- ##
==========================================
+ Coverage 94.23% 94.29% +0.06%
==========================================
Files 36 37 +1
Lines 4475 4595 +120
==========================================
+ Hits 4217 4333 +116
- Misses 258 262 +4
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@app/main.py`:
- Around line 1811-1816: Update the running-container filter in the power
allocation logic to include only containers with a service and status equal to
"running". Ensure both count and cpu_by_service are derived from this filtered
collection, and add a regression test covering a stopped container so it does
not affect allocation or fleet cost.
- Around line 1818-1835: Update the earnings aggregation around
get_earnings_per_service and the rows construction to use each platform’s
earnings within the requested days window rather than its latest balance.
Convert each gross amount from its platform currency into the configured
power-cost currency before power.summarise, and ensure the fallback does not mix
unconverted balances with cost totals.
- Line 1811: Update the /api/power/current flow around
_get_all_worker_containers() so malformed or missing per-worker
heartbeat/container data is handled independently rather than aborting the
endpoint. Ensure one worker’s status failure does not prevent database-sourced
gross earnings from being returned, and represent costs as unknown when CPU
status cannot be obtained.
- Around line 1811-1826: Update api_earnings_net to retain the worker grouping
returned by _get_all_worker_containers through power calculation instead of
deriving one fleet-wide count, host TDP, and CPU map. For each worker, apply its
worker_meta settings and power.is_metered(worker_meta), zero unmetered platforms
on their source platform, calculate watts, then aggregate the resulting watts by
platform before earnings allocation.
In `@app/power.py`:
- Around line 130-153: Update the service-row construction in the loop around
net_for_service so an unconfigured tariff does not expose gross as per-service
net: return cost and net as None, or use the established explicit unknown-state
representation, while preserving calculated values when configured is true. Add
a regression test asserting out["services"][0] reports the unknown cost/net
state when price_per_kwh=0.0.
🪄 Autofix (Beta)
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: f2babcf1-c9a4-4c03-9898-d0665ff3956e
📒 Files selected for processing (4)
CHANGELOG.mdapp/main.pyapp/power.pytests/test_power.py
|
Holding this PR rather than merging it. Five findings, and two of them mean the numbers it reports would be wrong — in a change whose whole purpose is reporting numbers honestly.
The other three are real too: stopped containers inflate Fixing these properly needs a window-scoped earnings query and per-worker power attribution — more than a patch on top of this diff. Not merging a feature that would mislead about money. |
Four of the five review findings on this PR. Two of them meant the numbers were wrong, which disqualifies a feature about reporting money honestly. Per-service net no longer equals gross when the tariff is unknown. The totals already said cost_known false / total_net None, but every service row still reported net == gross - presenting earnings as profit, the exact dishonesty this module exists to prevent, in my own code against my own stated rule. cost and net are now None per service, with cost_quality "unknown", and nothing is flagged negative on an unknown cost. Net is now computed from earnings OVER THE WINDOW. The endpoint used get_earnings_per_service(), which returns the latest BALANCE - a running total - so a 30-day electricity cost was charged against a lifetime of earnings. get_earned_by_platform(days) sums per-platform deltas across the window, clamped per platform the same way the dashboard does (CashPilot-glc) so a payout does not read as negative earnings. Also: only RUNNING containers are counted, since a stopped one draws nothing and counting it inflates container_count, shrinking every running service's share of the idle floor and understating the fleet's cost. And a worker-status failure no longer takes out the earnings figures, which come from the database and are still perfectly reportable. STILL OPEN, deliberately: per-worker power attribution. Every worker is still collapsed into one count and one host TDP, so a multi-host fleet is charged a single idle floor and power.is_metered cannot be applied per worker. That needs the worker grouping carried through to the watt calculation and is a larger change than this one.
|
Reworked. Four of the five findings are fixed and verified; the fifth is called out below rather than quietly left. Fixed — per-service net was gross when the tariff is unknown. This was the feature's own rule broken in its own code: totals said Fixed — a window's cost charged against a lifetime balance. Fixed — stopped containers inflated Fixed — no error boundary. A worker-status failure no longer takes out the earnings figures, which come from the database and are still reportable. Still open, deliberately: per-worker power attribution. Every worker is still collapsed into one count and one host TDP, so a multi-host fleet is charged a single idle floor and Verification: ruff clean, 1520 passed, coverage 93.67%. |
codecov flagged the patch: the module was well covered but the endpoint around it was not - and that is where config parsing, the running-only filter and the error boundary live. Covers: net reported when a tariff is set, unknown (not gross-as-net) when it is not, a stopped container not diluting the idle floor, a worker-status failure still returning the database-sourced gross, and a malformed tariff treated as unset rather than crashing.
Every dashboard in this space reports what a service paid. None report what it cost to run.
For a large share of users the honest number is negative. A service earning EUR 2/month on hardware drawing 15W at EUR 0.30/kWh costs EUR 3.29 in electricity — a EUR 1.29 monthly loss that we were presenting as income.
GET /api/earnings/netnow reports gross, estimated cost and net per service, and names the ones whose trailing net is below zero.Three honesty rules, tested directly
The failure mode here is dishonesty rather than a crash, so these are asserted, not assumed:
cost_qualityand a UI cannot render it with false precision.unknown, not zero. A zero would render gross as net and overstate earnings, which is the exact thing this bead exists to stop.Two modelling decisions worth stating
Scope
This is the calculation and the API. Surfacing it in the dashboard belongs with the
app.jswork (cyc). Real measurement from RAPL or a smart plug can replace the estimate later without changing the contract, sincecost_qualityalready distinguishesestimatedfrommeasured.Verification
ruff check . && ruff format --check .— cleanpytest --cov=app --cov-fail-under=90— 1515 passed, coverage 93.70%Summary by CodeRabbit
New Features
Documentation