refactor(finance): split finance utils into focused modules - #331
Merged
Conversation
Split the 2,347-line investor_app/finance/utils.py into mortgage, taxes, scoring, and strategies submodules; utils.py now holds only core KPI primitives and the Django-coupled analysis. Update all 17 importers to canonical locations. Resolve LIMIT-20: delete the calculate_* aliases, the dead score_listing_v1 chain, the duplicate pure score_listing_v2, and the unused service-layer calculate_noi in property_service.py. Mark LIMIT-20 resolved in docs/KNOWN_LIMITATIONS.md. Full suite: 1803 passed. Ruff + mypy clean on touched files.
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.
Summary
Split the 2,347-line
investor_app/finance/utils.pyinto focused submodules and resolve LIMIT-20 (duplicate/divergent finance implementations).Changes
investor_app/finance/{mortgage,taxes,scoring,strategies}.pymortgage.py— monthly mortgage, carrying costs, break-even rent, paydown, appreciation, ROI componentstaxes.py— depreciation, tax benefits, after-tax IRR/CF, hold-period projections, sale proceeds, recapturescoring.py— 1% rule, GRM, price-to-rent, market normalization helpersstrategies.py— flip, buy-and-hold, vacation rental, BRRRR calculatorsutils.pynow holds only core KPI primitives (noi,cap_rate,cash_on_cash,dscr,irr,build_cashflows) plus Django-coupled analysisutils.py)calculate_noi/calculate_cap_rate/calculate_cash_on_cash/calculate_irraliasesscore_listing_v1chain and deprecatedscore_listing_v1_deprecatedscore_listing_v2(production version stays incore/services/scoring.py)calculate_noiincore/services/property_service.py(+ its test file)docs/KNOWN_LIMITATIONS.mdVerification
tests/acceptance/conftest.py:69unrelated, file unmodified)Notes
estimate_rehab_costnow takes acost_per_sqftdict parameter instead of importingsettings.REHAB_COST_PER_SQFT— keeps the strategies module Django-free. Callers updated.total_return_summaryinitially dropped thepurchase_pricereturn key; restored to match original contract (caught bytest_dict_keys_present).preivs Django) investigation documented in the build report: Django is load-bearing; pydantic state machine removal deferred as a separate reviewed change.