Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 797824 - Inconsistent behavior in Loan Review #2060

Open
wants to merge 4 commits into
base: stable
Choose a base branch
from

Conversation

Genie-Liu
Copy link

fix the shift issue in loan review. More discussion is https://bugs.gnucash.org/show_bug.cgi?id=797824

fix the shift issue in loan review
@jralls
Copy link
Member

jralls commented Jan 24, 2025

Hmm, still not right. 6 Months of loan but only 5 in the summary. Whole loan has the same problem.
Screenshot 2025-01-24 at 15 16 28

Screenshot 2025-01-24 at 15 15 57

It's still an improvement: Before your change it only went for 4 months.

@Genie-Liu
Copy link
Author

Hmm, interesting. Thank you for the feedback, It seems that after the change, the first payments is correct now. But I have a question. In my current version, the number of loan payments is accurate, yet you mentioned there are only four? Could there be a difference in settings? Here are my parameters:

Screenshot 2025-01-25 at 15 20 57 Screenshot 2025-01-25 at 15 21 14 Screenshot 2025-01-25 at 15 21 21

@jralls
Copy link
Member

jralls commented Jan 25, 2025

Interesting. The difference is the start date: You're starting the loan and the payments on Feb. 1, I'm leaving the loan start date as today, the default, and changing the payment start to Feb 1. Your screenshot also shows the bad first payment meaning it's without your change. Here's what it looks like with the change:
image

So it's correct as long as the loan start and payment start are the same.

@Genie-Liu
Copy link
Author

Genie-Liu commented Jan 26, 2025

I see. I think my fix has resolved the issue with the first incorrect payment.

The missing payment in your previous figure is due to the start date configuration. The current mechanism calculates the end date based on the start date and the number of periods. In your earlier setting, the start date was set to the default, let’s assume it was 01/25/2025, with a 6-month duration. The end date would then be calculated as 06/25/2025. However, if we start the payment from 02/01/2025 and aim for 6 payments, the last payment would fall on 07/01/2025, which is after the calculated end date and it will be filtered during calculating the review.

A propose solution is to calculate the payments review using the remain months. This also aligns with the final created schedules.

@Genie-Liu
Copy link
Author

Genie-Liu commented Jan 26, 2025

If we aim to create a loan schedule starting in the middle of the loan term, we will still have issues. For example, suppose the loan begins on 12/01/2024 with a 6-month period, and we intend to start payments on 02/01/2025.

The current result is incorrect cause the first payment is wrong and the last payment is missing:
image

If we apply this change the fixed version should produce the payments starting at 02/01/2025 and end at 05/01/2025. The payments' date is correct. But the interest is still not the right one.

@jralls
Copy link
Member

jralls commented Jan 26, 2025

The missing payment in your previous figure is due to the start date configuration. The current mechanism calculates the end date based on the start date and the number of periods.

Fair enough, but a partial payment on the first day of the loan is called a down payment and technically isn't part of the loan. For house mortgages the first payment is usually near the beginning of the following month; auto loans may defer payments for up to a year.

If we aim to create a loan schedule starting in the middle of the loan term, we will still have issues. For example, suppose the loan begins on 12/01/2024 with a 6-month period, and we intend to start payments on 02/01/2025.

If we apply this change the fixed version should produce the payments starting at 02/01/2025 and end at 05/01/2025. The payments' date is correct. But the interest is still not the right one.

A 6-month loan beginning on 12/1 is due on 5/31. Strictly speaking if we start monthly payments on 2/1 then we would expect to have 5 payments of roughly $210 with the last one on 5/31. But we have a problem in the Months Remaining box: If we leave it to itself it changes the number to 4 and assumes that we've already made two payments; if we push it to 6 it decides that we have until 7/1 to pay. Fixing that's out of scope for the bug and is deeper in the logic of the assistant. I'm satisfied that your current change fixes the date issues sufficiently to satisfy the bug. I'll do a code review tomorrow (it's already past my bedtime), but assuming it all looks good I'm willing to merge this as-is and I'll leave it up to you whether to work on getting the interest calculations right on a separate PR.

@Genie-Liu
Copy link
Author

Fixing that's out of scope for the bug and is deeper in the logic of the assistant. I'm satisfied that your current change fixes the date issues sufficiently to satisfy the bug.

Yeah, I agree. The generation logic need more discussion. I will revert the repayments generation code that based on remain month.

I'll leave it up to you whether to work on getting the interest calculations right on a separate PR.

The interest fix is also included in this pr. It depends on the remain month. Maybe you can help take a look.

@jralls
Copy link
Member

jralls commented Jan 26, 2025

You locked in on monthly payments but the payments option has several options. For example setting payments to semi-monthly produces:
Screenshot 2025-01-26 at 08 57 43

Screenshot 2025-01-26 at 08 57 56 Screenshot 2025-01-26 at 08 58 23

Edit:
Without your changes the result is more correct though it of course ignores the start of payments date:
Without your changes the result has the right number of payments but doesn't adjust the payment amount so it overpays by 2x:

Screenshot 2025-01-26 at 09 04 12

@jralls
Copy link
Member

jralls commented Jan 26, 2025

Notice that the formula (labeled Amount) is unaffected by the payments schedule. That explains a lot of the problem. I wondered if it had been noticed and found a lot of bugs documenting problems with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants