[Bug]: Scheduled transaction preview with splits shows split error

[joel-jeremy]

Verified issue does not already exist?

• I have searched and found no existing issue
• I will be providing steps how to reproduce the bug (in most cases this will also mean uploading a demo budget file)

What happened?

Steps to reproduce:

1. Create a schedule with some splits
   
   
   

2. Set the schedule so that it shows up as Upcoming
   

3. The split error is shown even though the amounts are balanced

Where are you hosting Actual?

Fly.io

What browsers are you seeing the problem on?

Chrome, Other

Operating System

Windows 11

[joel-jeremy]

For visibility - @jfdoming

[jfdoming]

For visibility - @jfdoming

Yes thanks for triaging! I took a look yesterday and was struggling to root cause. But I'll keep at it

[alter3d]

If it helps with root cause, I can confirm that this also happens under the following scenarios:

• Firefox on Ubuntu 24.04 (syncing to self-hosted AWS EC2 instance)
• Firefox on Win10 (same sync server as above)
• Native Windows application (same sync server as above)
• Native Windows application (no sync server)

All running current 24.9.0 build.

[sjones512]

Noticed this today.

Windows 10
Firefox 130.0.1
Actual Budget v24.10.1

Not sure if it helps but I also noticed:
If I change the last split by 0.01 and save it, actual show an additional split in the transaction register that is not categorized, and the floating box will go away.

If I post the transaction it will go away, deleted the posted transaction brings it back.

If I add another split with "100% of remaining", which equals 0, it gets rid of the error, but I have an extra split for 0 in my transaction.

It also shows when the split scheduled transaction is collapsed:

Probably irrelevant that the errant transaction split doesn't obfuscate the amount.

[sjones512]

@jfdoming I took a look at it and considered attempting a PR but don't feel familiar enough with the code to know I wont break things.

As a kludge it seems like you can just add update = recalculateSplit(update); to the end of the rules.ts execActions function.
rules.ts:723

I believe the root cause lies in this block of code in the rules.ts execActions function

for (const action of childActions) {
    const splitIndex = action.options?.splitIndex ?? 0;
    if (splitIndex >= update.subtransactions.length) {
      const { data, newTransaction } = addSplitTransaction(
        newTransactions,
        transaction.id,
      );
      update = recalculateSplit(newTransaction);
      data[0] = update;
      newTransactions = data;
    }
    action.exec(update.subtransactions[splitIndex]);
  }

In this block the amounts of the split subtransactions don't appear to be populated until after action.exec is called, but we are doing the recalculateSplit before that, in the loop.

I'm not very familiar with the code, but it seems wasteful to recalculateSplit on every iteration of the loop. I would think it could just be recalculated at the end after all of the subtransactions have been added.

[jfdoming]

@jfdoming I took a look at it and considered attempting a PR but don't feel familiar enough with the code to know I wont break things.

As a kludge it seems like you can just add update = recalculateSplit(update); to the end of the rules.ts execActions function. rules.ts:723

I believe the root cause lies in this block of code in the rules.ts execActions function

for (const action of childActions) {
    const splitIndex = action.options?.splitIndex ?? 0;
    if (splitIndex >= update.subtransactions.length) {
      const { data, newTransaction } = addSplitTransaction(
        newTransactions,
        transaction.id,
      );
      update = recalculateSplit(newTransaction);
      data[0] = update;
      newTransactions = data;
    }
    action.exec(update.subtransactions[splitIndex]);
  }

In this block the amounts of the split subtransactions don't appear to be populated until after action.exec is called, but we are doing the recalculateSplit before that, in the loop.

I'm not very familiar with the code, but it seems wasteful to recalculateSplit on every iteration of the loop. I would think it could just be recalculated at the end after all of the subtransactions have been added.

@sjones512 Yeah sorry I dropped the ball on this one. I want to do a larger refactor to avoid these problems and add some unit tests. Are you okay to wait for that? I will get to it soon but work is killing me right now 😞

[sjones512]

@sjones512 Yeah sorry I dropped the ball on this one. I want to do a larger refactor to avoid these problems and add some unit tests. Are you okay to wait for that? I will get to it soon but work is killing me right now 😞

Totally your call. It is a fairly intrusive UI bug when it happens, but I think it can be worked around by setting the last split rule as "a portion of the remainder" instead of "fixed-amount".

I'm not familiar with Actual's release process. If you think you'll be able to include the refactor in the next release, there is no reason to merge this. If not, this could be a band-aid to resolve the immediate issue, and a separate issue could be raised for the refactor.

Either way, good luck with work! 💪

[jfdoming]

@sjones512 Yeah sorry I dropped the ball on this one. I want to do a larger refactor to avoid these problems and add some unit tests. Are you okay to wait for that? I will get to it soon but work is killing me right now 😞

Totally your call. It is a fairly intrusive UI bug when it happens, but I think it can be worked around by setting the last split rule as "a portion of the remainder" instead of "fixed-amount".

I'm not familiar with Actual's release process. If you think you'll be able to include the refactor in the next release, there is no reason to merge this. If not, this could be a band-aid to resolve the immediate issue, and a separate issue could be raised for the refactor.

Either way, good luck with work! 💪

Thanks! I'll try to get to it this weekend. Releases are cut around the 20th IIRC, and I'm a maintainer so I can force merge it 😈 jk, but I'll make sure it's the next release in some form.