fix: count billing cycles instead of pricing them in days (1000/month showed 12,167/year) - #180
Merged
Merged
Conversation
1000 a month reported 12166.67 a year. BurnRateCalculator normalised every subscription to a daily rate (cost / cycle_days) with MonthlyDays = 30 and YearlyDays = 365, then projected that rate back out. Those two constants disagree: 365/30 is 12.17, so a "year" was 12.17 months long and every annual figure was inflated by 1.4%. 1000/30 = 33.33/day, x365 = 12166.67, which the dashboard's N0 format then showed as 12,167. A calendar month is a count, not a span of days. Each subscription's annual cost is now cost x charges_per_year (Weekly 52, Monthly 12, Quarterly 4, Yearly 1); Monthly and Weekly are derived from that sum rather than computed independently, so the three horizons agree with each other and the category/payment-source breakdowns add back up to the headline. The quarterly divisor added for #171 - 91 days, chosen to make the daily rate come out at roughly four charges a year - is gone with the rest of it; four is now exactly four. A monthly subscription reads back as its own cost per month, and a weekly one as its own cost per week, which is the first thing anyone checks. Weekly is still rounded to the cent before it leaves the server, so Weekly x 52 lands within about 26 cents of Yearly rather than exactly on it. The test asserts that tolerance rather than equality. Updates the architectural rule in CLAUDE.md, which specified the daily rate, and TECHNICAL_REQUIREMENTS.md §6. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Reported: a single ₹1000/month subscription showed 12,167 a year instead of 12,000.
Reproduced exactly.
Root cause
BurnRateCalculatornormalised every subscription to a daily rate and projected that back out:Those two constants disagree with each other: 365 / 30 = 12.17, so the calculator's "year" was 12.17 months long. Every annual figure was inflated by ~1.4%.
12166.67then rendered as12,167because the dashboard's Yearly tile formats with{0:N0}.The bug hid well: the monthly figure was always exactly right (30 × cost/30 cancels), so only the yearly number was wrong, and only by an amount that looks like rounding until the totals get large.
The fix
A calendar month is a count, not a span of days. Each subscription's annual cost is now its price times how often it is billed:
Monthly and Weekly are now derived from the annual sum rather than computed independently, so the three horizons agree with each other, and the
ByCategory/ByPaymentSourcebreakdowns add back up to the headline instead of being a parallel projection.That last row is the fix, not a regression: 30/month is 360/year, and 360/52 = 6.92. The old 7.00 came from the 30-day-month fiction.
Also gone: the
QuarterlyDays = 91constant added in #171, which existed only to make a quarterly subscription's daily rate project to roughly four charges a year. Four is now exactly four.Rounding, stated honestly
Weeklyis rounded to the cent before it leaves the server, so multiplying it back by 52 amplifies that rounding by up to ~26 cents.Monthly × 12 == Yearlyexactly;Weekly × 52lands within half a unit. The test asserts that tolerance rather than false equality — I had it as exact equality first and it correctly failed.Architectural rule updated
CLAUDE.mdpreviously specified the broken approach:Rewritten to describe cycle counting, with an explicit "do not reintroduce a daily rate" and the reason.
TECHNICAL_REQUIREMENTS.md§6 item 7 updated to match. Without this, the next person follows the doc straight back into the bug.Tests
SubVora.Application.TestsSubVora.Infrastructure.TestsSubVora.Api.TestsSubVora.Mobile.TestsNew coverage:
[Theory]pinning cost × charges for all four cadences, including 1000/month → 12000 as the regression case.Two existing assertions encoded the old math and were updated with the reason inline — the
7.00/week one above, and the API-level quarterly test (1604.40→1600).Scope
Backend only —
BurnRateCalculatorplus tests and docs. No API contract, schema or client change; the mobile app displays whatever the endpoint returns, which is the point of the server-side rule.Independent of #179 (mobile-only), so the two can merge in either order.