feat: Add support for investment accounts with current value and XIRR

[npv12]

Important

Add support for investment accounts with current value and XIRR calculations, including frontend, backend, and database changes.

• Frontend:
  • Add current_value field to AccountForm, AddAccountModal, and UpdateAccountModal.
  • Update AccountAnalytics.tsx to display current_value and calculate percentage_increase and xirr for investment accounts.
  • Modify NetWorth.tsx to adjust Y-axis domain calculation.
• Backend:
  • Add current_value to CreateAccountInput and Account in models/account.ts.
  • Update AccountBalanceAnalytics in models/analytics.ts to include current_value, percentage_increase, and xirr.
  • Implement GetAccountCashFlows in analytics_repository.go to retrieve cash flows for XIRR calculation.
  • Modify account_repository.go to handle current_value for investment accounts.
• Database:
  • Create investment_account_value table with migration 00010_create_investment_account_value_table.sql.
• Tests:
  • Add tests for investment account creation, update, and analytics in account_controller_test.go and analytics_controller_test.go.
  • Extend account_service_test.go and analytics_service_test.go to cover new investment account features.

^{This description was created by for dc49702. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 0fb00d3 in 2 minutes and 2 seconds. Click for details.

• Reviewed 2022 lines of code in 22 files
• Skipped 0 files when reviewing.
• Skipped posting 7 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/service/analytics_service.go:116

• Draft comment:
  Be sure that adding each account’s stored balance to the initial balance (lines 117–120) is intentional. This may double‐count funds if the repository’s transaction sum already reflects the account’s starting balance.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. server/internal/service/analytics_service.go:149

• Draft comment:
  The condition to skip adding a time series data point when runningBalance equals totalAccountBalance (line 149) may omit days with no change. Consider whether a flat value should be recorded for a consistent time series.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. server/internal/service/analytics_service.go:203

• Draft comment:
  Review the investment metrics logic: cashAmount is set to -flow.Amount, so the 'if cashAmount < 0' block may never trigger as expected. Verify that totalInvested is computed correctly.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a concern about the logic, but upon closer inspection, the code seems intentional. The negation on line 204 converts the sign convention (likely flow.Amount is positive for investments), then line 209 checks if it's negative (meaning it was an investment), and line 210 negates it again to add the positive amount to totalInvested. Without seeing the models.AccountCashFlow definition or understanding the sign convention used in the data model, I cannot definitively say this is wrong. The comment is speculative ("may never trigger") and asks to "verify" rather than stating a definite issue. This violates the rule about not asking the author to verify or confirm. I don't have visibility into what flow.Amount represents or its sign convention. The double negation could be intentional to handle a specific data model convention. The comment might be highlighting a real issue if the sign convention is different than what I'm assuming. While I lack full context about the data model, the comment is phrased as "may never trigger" and asks to "verify", which are speculative and verification-requesting language that violate the rules. If this were a definite bug, the comment should state it definitively. The lack of certainty in the comment's phrasing suggests even the automated tool wasn't sure. The comment uses speculative language ("may never trigger") and asks the author to "verify" rather than stating a definite issue. This violates the rules against speculative comments and asking for verification. Without strong evidence of a bug, this comment should be deleted.

4. server/internal/service/analytics_service.go:252

• Draft comment:
  The Newton–Raphson loop in calculateXIRR looks standard; consider adding a safeguard for when the derivative is near zero to avoid unstable iterations.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

5. server/internal/repository/analytics_repository.go:44

• Draft comment:
  The net worth query multiplies SUM(amount) by -1 to generate initial balance. Ensure this negation is aligned with the intended business logic for frontend presentation.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. server/internal/service/account_service_test.go:140

• Draft comment:
  Test cases for investment account updates look solid. Consider clarifying in comments how switching bank type (investment to non-investment) should reset current_value to nil.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. server/internal/service/analytics_service_test.go:566

• Draft comment:
  The tests for net worth series expect negated values (e.g. initial balance -1000 for a 1000 debit). Please double-check that the negation across repository and service layers is consistently documented.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_af1pmvlpJY8txxZs

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed a369101 in 2 minutes and 25 seconds. Click for details.

• Reviewed 2396 lines of code in 22 files
• Skipped 0 files when reviewing.
• Skipped posting 9 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/service/analytics_service.go:247

• Draft comment:
  In GetMonthlyAnalytics, totalAmount is computed as totalIncome - totalExpenses, but test cases expect totalAmount to equal the sum of income and expenses (e.g. 1500+1200=2700). Please verify the intended formula.
• Reason this comment was not posted:
  Comment was on unchanged code.

2. server/internal/service/analytics_service.go:118

• Draft comment:
  The initial balance is augmented by summing each account’s Balance from ListAccounts. Verify if this isn’t double‐counting – should the transaction‐derived initial balance already include account balances?
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. server/internal/service/analytics_service.go:210

• Draft comment:
  In calculateInvestmentMetrics, the special-case for a single negative flow assumes len(cashFlows)==2; please double-check if this logic covers all one-off investment scenarios and whether rounding issues might affect the ‘years’ calculation.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 1% The comment asks the PR author to double-check the logic and consider potential rounding issues. This violates the rule against asking the author to double-check things or ensure behavior is intended. The comment does not provide a specific suggestion or ask for a specific test to be written.

4. server/internal/service/analytics_service.go:281

• Draft comment:
  In the fallback methods of calculateXIRR, multiple iterative loops (Newton, bisection, and secant) are used. Consider refactoring to simplify or document edge-case expectations.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is suggesting a refactor or additional documentation for complex code. However, the rules state that I should only keep comments that clearly require a code change. This comment uses "Consider refactoring" which is a soft suggestion, not a clear requirement. The code already has inline comments explaining what each section does (Newton method, bisection fallback, secant fallback). The comment doesn't point to a specific bug or issue - it's more of a code quality suggestion about complexity. According to the rules, "Comments that suggest code quality refactors are good! But only if they are actionable and clear." This comment is somewhat vague - it doesn't specify what exactly should be refactored or what edge-case expectations should be documented. It's more of a general observation about complexity rather than a specific, actionable suggestion. The code is implementing a complex numerical algorithm (XIRR calculation) which inherently requires multiple fallback methods for robustness. The comment might be valid in that the code is complex, but without specific guidance on what to refactor or document, it may not be actionable enough. The existing inline comments do explain the purpose of each section. While the comment has some merit about code complexity, it lacks specificity about what should be changed. The phrase "Consider refactoring to simplify or document edge-case expectations" is too vague to be actionable. The code already has comments explaining each fallback method. Without a concrete suggestion for improvement, this falls into the category of general observations rather than actionable feedback. This comment should be deleted because it's a vague suggestion to "consider refactoring" without providing specific, actionable guidance. The code already has inline documentation explaining each fallback method, and the comment doesn't identify a specific bug or required change.

5. server/internal/service/analytics_service.go:240

• Draft comment:
  The daily networth time series generation in GetNetworthTimeSeries skips adding points when runningBalance equals totalAccountBalance. Confirm that skipping these points is intended and won’t lead to gaps in the frontend chart.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 1% The comment is asking the author to confirm their intention regarding skipping points in a time series generation. This falls under asking the author to confirm their intention, which is not allowed according to the rules. Therefore, this comment should be removed.

6. server/internal/models/analytics.go:6

• Draft comment:
  Consider adding brief documentation to MonthlyAnalyticsResponse and clarifying the meaning of TotalAmount, to avoid confusion with the income/expenses calculation.
• Reason this comment was not posted:
  Comment was on unchanged code.

7. server/internal/repository/analytics_repository.go:231

• Draft comment:
  Ensure that the SQL query in GetMonthlyAnalytics correctly reflects the intended business logic for income/expenses. The query computes total_expenses from positive amounts and total_income from negative amounts; double-check that this aligns with financial conventions used in tests.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. server/internal/service/analytics_service_test.go:1250

• Draft comment:
  Test cases for monthly analytics expect TotalAmount values that don’t match a subtraction. Review test expectations for consistency with business logic in GetMonthlyAnalytics.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

9. server/internal/service/analytics_service.go:198

• Draft comment:
  In calculateInvestmentMetrics, returning nil for XIRR when math.Abs(rate) < 1e-12 may hide extremely small but non-zero rates. Confirm if this is acceptable in performance benchmarks.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is asking the author to "confirm if this is acceptable in performance benchmarks" which violates the rule about not asking the PR author to confirm their intention or double-check things. The comment is also speculative - it says "may hide" rather than definitively identifying a problem. The threshold of 1e-12 is extremely small (0.000000000001) and treating such values as undefined is a reasonable engineering decision for floating-point calculations. The comment doesn't provide strong evidence that this is wrong, it's just questioning the design choice. This type of comment should be deleted according to the rules. Could there be a legitimate concern about the threshold value being too aggressive? Perhaps in some financial scenarios, rates smaller than 1e-12 are meaningful and should be preserved rather than treated as undefined. Even if there's a theoretical concern about the threshold, the comment doesn't provide concrete evidence of a problem. It asks the author to "confirm" rather than pointing out a specific issue. The threshold of 1e-12 is well within typical floating-point precision limits, and treating such small values as undefined is a common practice in numerical computing. Without specific evidence that this causes issues, this is just a speculative question. This comment should be deleted. It asks the PR author to confirm/verify the design choice rather than pointing out a clear issue, and it's speculative ("may hide") without providing concrete evidence of a problem. The rules explicitly state not to ask authors to confirm intentions or double-check things.

Workflow ID: wflow_gTZ9JQgesywmHDTa

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[codecov]

Codecov Report

❌ Patch coverage is 84.64164% with 45 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
server/internal/service/analytics_service.go	80.59%	15 Missing and 11 partials ⚠️
server/internal/repository/account_repository.go	91.59%	5 Missing and 5 partials ⚠️
server/internal/repository/analytics_repository.go	66.66%	6 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed dc49702 in 1 minute and 3 seconds. Click for details.

• Reviewed 100 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/service/analytics_service.go:267

• Draft comment:
  The same-day check now uses 'if len(cashFlows) > 2' and iterates up to len(cashFlows)-1 to exclude the current value. Ensure this accurately mirrors the original intent (i.e. more than one original flow) and handles edge cases correctly.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 1% The comment is asking the author to ensure that the change accurately mirrors the original intent and handles edge cases correctly. This violates the rule against asking the author to confirm their intention or ensure behavior. The comment does not provide a specific suggestion or point out a clear issue with the code.

2. server/internal/service/analytics_service.go:360

• Draft comment:
  The fallback grid and secant method logic for XIRR calculation has been removed. Confirm that relying solely on Newton's method with multiple initial guesses is robust for all expected cash flow patterns.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 1% The comment is asking the PR author to confirm the robustness of the change, which violates the rule against asking for confirmation or ensuring behavior. It does not provide a specific suggestion or point out a clear issue with the code.

3. server/internal/service/analytics_service.go:289

• Draft comment:
  Ensure comprehensive test coverage for edge cases in XIRR computation, especially when all flows (excluding current value) occur on the same day or when Newton’s method fails to converge.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_NqyrPLMiL3trB5MZ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}