Fix regression introduced by Apply-to-Oldest optimization

[dcenic]

What & why

Fix regression introduced by Apply-to-Oldest optimization. Detailed explanation of the regression is in #9529

Linked work

Fixes AB#646691 and its GitHub counterpart #9529

How I validated this

• I read the full diff and it contains only changes I intended.
• I built the affected app(s) locally with no new analyzer warnings.
• I ran the change in Business Central and confirmed it behaves as expected.
• I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)
Added new automated tests:

• 4 tests that guard against the regression added by the partner, taken from this PR Restore the full-sum sign decision in Apply-to-Oldest application #9881
• • one extra tests that confirms the performance optimization works (that early exit is done when summing is sufficient)

Risk & compatibility

There is test coverage for both the regression and the performance optimization, so the risk is low.

[Franco111000]

Nice solution dcenic, the same-sign-first ordering is elegant. I re-traced it against the scenarios behind #9881 and I agree it restores the correct sign decision while keeping the optimization: the sum cannot cross zero inside the same-sign block, and once it strictly crosses in the opposite-sign block, every remaining entry only pushes it further the same way, so the truncated decision matches the full one. The < 0 tightening also handles the exact-offset case correctly.

One residual population worth a thought for the ICM, since it survives any early exit whose proof relies on an entry's Positive flag matching the sign of its current remaining amount: the flag is stamped at creation and never recomputed, and the uncapped application path can leave overshot entries open with a flipped remaining sign. The 28.2 regression itself has been producing exactly such entries, so ledgers already damaged by it can still feed the truncated decision a wrong-sign contribution. The same holds for the rarer case of a payment discount exceeding a partially credited invoice's remaining amount, and for employee subscribers mutating the amount via OnPrepareTempEmplLedgEntryOnBeforeUpdateRemainingAmount. If that is judged acceptable as a data-quality edge, it may still be worth a line in the ICM for support scenarios on affected tenants.

Happy to see the tests carried over. I'll close #9881 in favor of this PR once it merges.

[dcenic]

Franco111000 Thanks, as for the concern about the corrupt remaining amounts that may have already been caused by the regression - we have no way of fixing that with a code fix. Customers will need to un-apply the erroneously applied entries, then re-apply them with the fixed 'Apply-to-Oldest' strategy.

[Franco111000]

I dug deeper to make sure my earlier concern was concrete rather than theoretical. It is, with a happy ending: your remediation guidance covers it.

On 28.2 the truncated sum can land on exactly zero, the <= 0 trip fires, and because FindNextOldCustLedgEntryToApply only advances within the filtered set without re-checking the new entry, the application can walk through the transient zero and terminate with the new entry open, remaining sign flipped, and Positive still at its creation-time value (nothing ever rewrites it; reversal flips amount and flag together). Example: an invoice of +1000 against open credit memos of -1000 and -5000 plus an invoice of +100 ends as an open entry with remaining -4900 and Positive = true. Entries like that would keep confusing block membership for future Apply-to-Oldest decisions even after this fix, since the iteration order relies on the flag.

Two good things follow. Your < 0 tightening closes that mint going forward. And un-apply / re-apply incidentally repairs the flag inconsistency as well, because unapply restores the remaining amount and the flag was never touched. So your stated remediation is complete as-is; it just seems worth a line in the ICM that affected accounts should be repaired before Apply-to-Oldest is relied on again.

One minor note: OnPrepareTempEmplLedgEntryOnBeforeUpdateRemainingAmount hands subscribers a var entry inside the reordered loop, so a subscriber replacing "Remaining Amount" with an opposite-sign value would void the same-sign-first reasoning. No such subscriber exists in the repo today, but it may deserve a comment on the event.

Still looks good to me overall, closing #9881 once this merges.

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Accept

What this PR does

The new commits since round 1 clean up the analyzer issues from the added tests and add exact-offset coverage for vendor and employee Apply-to-Oldest scenarios. The full PR still fixes the posting regression by weighing same-sign entries first, using a strict < 0 early-exit boundary, and then applying the final sign filter. The added customer, vendor, and employee tests now cover the reported same-sign application regression, the retained early exit, and the exact-zero boundary that could otherwise choose the wrong sign bucket.

Status of previous suggestions

ID	Title	Status	Author response
S1	Fix employee test key warnings	Addressed	The Analyze checks are now green after the employee test warning cleanup.
S2	Move StrSubstNo text to labels	Addressed	The Analyze checks are now green after the customer test warning cleanup.

New observations (commits since round 1)

None - the new commits address the prior validation findings and add boundary coverage.

Risk assessment and necessity

Risk: This remains a high-sensitivity posting and data-integrity path in codeunit 12 across customer, vendor, and employee application logic. The change does not alter public signatures, and the current PR checks are green. The new tests reduce the main regression risk by covering both the reported same-sign failure and the exact-offset boundary for the optimized loop.

Necessity: The change is required because normal Apply-to-Oldest posting can otherwise close the wrong ledger entries without an error. The scope is right for the bug: it fixes the shared customer, vendor, and employee pattern while keeping the performance optimization and adding regression coverage.

---

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10227 round=2 by=alexei-dobriansky at=2026-08-14T10:12:24.0127633Z lastSha=e31dac2c26edc5ee3547b549f598d52400f87c0f reviewKey=na suggestions=S1@e05d32ec:addressed,S2@30aaf953:addressed parentRound=1

[dcenic]

Franco111000 This is being backported to 28.2 and higher release branches too. That means it will be deployed to live customers within the next 10-12 days.

[Franco111000]

Great news, thanks for carrying it through to the release branches, that closes the loop on the 28.2 regression for live tenants. Pleasure collaborating on this one.