I suppose your October 2008 claim comes from https://github.com/Gnucash/gnucash/commit/425ec8eb68254948cb03b2c152ebab05b20acb0e that negates shares and value if they’re reversed in the pair because the commodity isn’t the transaction currency.
That remained essentially unchanged until https://github.com/Gnucash/gnucash/commit/ce675eaac6b52e85cd9fe8602002f7b9d55e63ba on 3 May 2019 substantially reordered the function and introduced the clause that you propose to change. That would be “the culprit” I asked you for yesterday morning. That commit is titled “Refactor” but it would be more correct to call it “Complicate”. It turns two conditional branches—split commodity is or isn’t the transaction currency—into 5. I agree with you that the amounts in the last case should be negated: The commodity isn’t the transaction currency. What I’m less sure about is that it needs to be a separate case at all—or rather that the `(assoc acc-comm sublist)` test needs to exist. Changing that to a plain `else` and deleting the current `else` clause should accomplish the same thing and removes the logic change buried in what’s claimed to be a refactor. Regards, John Ralls > On Aug 24, 2025, at 13:20, Sherlock Holmes <[email protected]> wrote: > > John, > > Yes. I have. And I do think I understand both algorithms and their > evolution. > > Again, assuming the sign swap was necessary for "average cost", when the sign > swap was first effectuated 17 years ago, it does not appear to have been > applied on the first entry. This code has been rewritten multiple times > since then and the issue appears to have been replicated time after time. I > know it seems unlikely but have a look at the code. What am I missing? > > Regards, > > Sherlock > > > > On 8/24/25 12:43 PM, John Ralls wrote: >> Sherlock, >> >> Seems unlikely. Have you read through >> https://bugs.gnucash.org/show_bug.cgi?id=797796, >> https://bugs.gnucash.org/show_bug.cgi?id=775368, and >> https://bugs.gnucash.org/show_bug.cgi?id=775368 (the latter two are >> referenced in https://bugs.gnucash.org/show_bug.cgi?id=797796#c8) so that >> you thoroughly understand the problem and the design of the algorithm? >> >> Regards, >> John Ralls >> >> >>> On Aug 24, 2025, at 12:13, Sherlock Holmes <[email protected]> >>> <mailto:[email protected]> wrote: >>> >>> John, >>> >>> Here's where I'm at: I suspect the initial "account" commodity list entry >>> in the "average cost" algorithm has been a bit off since Oct 16, 2008. It >>> appears the share and value signs are supposed to be swapped on assignment >>> to the "account" commodity list, but are not when the list is first >>> created. See the attached patch file. >>> >>> Thoughts? >>> >>> Regards, >>> >>> Sherlock >>> >>> >>> >>> On 8/24/25 10:21 AM, Sherlock Holmes wrote: >>>> John, >>>> >>>> I'm still working it. It appears to me that both algorithms may have gone >>>> a bit awry sometime ago although the "average price" may be okay. >>>> Specifically, the initial comm-list entry became an else case which meant >>>> it needed to perform the assignments and, in the "average cost", the sign >>>> handling doesn't >>>> >>>> My WAG was just that. Just a quick check to see if I was in the right >>>> area. >>>> >>>> >>>> >>>> Regards, >>>> >>>> Sherlock >>>> >>>> >>>> On 8/24/25 9:16 AM, John Ralls wrote: >>>>> Sherlock, >>>>> >>>>> What’s this part for? It seems irrelevant to Chang Wang’s example as that >>>>> doesn’t use trading accounts: >>>>> - >>>>> - ;; However skip splits in trading accounts as these counterbalance >>>>> - ;; the actual value and share amounts back to zero >>>>> - ((eqv? (xaccAccountGetType (xaccSplitGetAccount (car >>>>> comm-splits))) >>>>> - ACCT-TYPE-TRADING) >>>>> - (loop (cdr comm-splits) >>>>> - sumlist)) >>>>> >>>>> I guess you did a bisect to arrive at the 2019 change date. There were 8 >>>>> changes to gnc-commodity-utils.scm that day. Which one was the culprit? >>>>> >>>>> Regards, >>>>> John Ralls >>>>> >>>>> >>>>>> On Aug 23, 2025, at 20:18, Sherlock Holmes <[email protected]> >>>>>> <mailto:[email protected]> wrote: >>>>>> >>>>>> I concur. >>>>>> >>>>>> There are significant differences in the implementation between >>>>>> gnc:get-exchange-totals and gnc:get-exchange-cost-totals that I believe >>>>>> are the root cause of the issue. These differences appear to date back >>>>>> to May 3, 2019. As a WAG, I modified gnc:get-exchange-cost-totals to >>>>>> match gnc:get-exchange-totals in the attached patch and the issue you've >>>>>> raised appears to be resolved. I have not done any further testing, >>>>>> >>>>>> Regards, >>>>>> >>>>>> Sherlock >>>>>> >>>>>> >>>>>> >>>>>> On 8/23/25 2:20 PM, Chang Wang wrote: >>>>>>> Thanks for the reminder. I'll post to the user list in the future. >>>>>>> However, in the above example, there is no gain or loss due to currency >>>>>>> exchange as the exchange rates are set to 1 so no currency gain/loss >>>>>>> needs to be booked. And the price source is set to be Last up through >>>>>>> report date instead of average cost. Therefore, I think these are >>>>>>> different issues. >>>>>>> >>>>>>> On Sat, Aug 23, 2025 at 3:51 PM John Ralls <[email protected] >>>>>>> <mailto:[email protected]>> wrote: >>>>>>>> Oddly I just told somebody on IRC the same answer: >>>>>>>> https://bugs.gnucash.org/show_bug.cgi?id=797796 >>>>>>>> >>>>>>>> Unless you’re willing to submit a PR, this is a user-list topic, so in >>>>>>>> the future please use gnucash-user instead of gnucash-devel. >>>>>>>> >>>>>>>> Regards, >>>>>>>> John Ralls >>>>>>>> >>>>>>>> > On Aug 23, 2025, at 1:43 PM, Chang Wang <[email protected] >>>>>>>> > <mailto:[email protected]>> wrote: >>>>>>>> > >>>>>>>> > Hi all, >>>>>>>> > I noticed an issue with the Trial Balance report when using stock >>>>>>>> > trading and multiple currencies. Even when transactions are >>>>>>>> > balanced, the Trial Balance report appears to break due to incorrect >>>>>>>> > calculation of unrealized gains. >>>>>>>> > >>>>>>>> > I've attached an uncompressed minimal example to illustrate the >>>>>>>> > problem. >>>>>>>> > >>>>>>>> > Steps to reproduce: >>>>>>>> > Open the attached book. >>>>>>>> > Generate a Trial Balance report with reporting currency set to USD, >>>>>>>> > price source set to Last up through report date, and enable Show >>>>>>>> > Foreign Currencies and Exchange Rates. >>>>>>>> > >>>>>>>> > Observed behavior: >>>>>>>> > The report shows an Unrealized Gain of $20,800, which is incorrect. >>>>>>>> > >>>>>>>> > Expected behavior: >>>>>>>> > The Unrealized Gain should be $200. >>>>>>>> > >>>>>>>> > Explanation: >>>>>>>> > The example contains three transactions: >>>>>>>> > 1) 08/02/2025 - Buy one stock for 10,200 JPY. >>>>>>>> > 2) 08/03/2025 - Exchange 100,000 JPY for 100,000 USD. >>>>>>>> > 3) 08/04/2025 - Buy one stock for 10,400 JPY. >>>>>>>> > >>>>>>>> > The JPY/USD rate is fixed at 1 on all days, so there should be no >>>>>>>> > realized or unrealized currency gains. Stock prices are set at 10X00 >>>>>>>> > JPY on 08/0X/2025, where X = 1, 2, 3, 4. >>>>>>>> > >>>>>>>> > Therefore, in USD reporting currency, the Trial Balance should show >>>>>>>> > unrealized gains as: >>>>>>>> > (10,400 * 2) - 10,200 - 10,400 = 200 JPY = 200 USD >>>>>>>> > >>>>>>>> > Notably, the Balance Sheet report does display the correct >>>>>>>> > unrealized gain. And if transaction 3) or transaction 2) is removed, >>>>>>>> > the Trial Balance turns out to be correct. >>>>>>>> > >>>>>>>> > I'm not familiar with Scheme, so I wasn't able to locate the problem >>>>>>>> > in the source code. I also wasn't able to file a bug on Bugzilla, >>>>>>>> > since the registration function appears to be broken. >>>>>>>> > >>>>>>>> > Thanks for your attention, >>>>>>>> > Chang >>>>>>>> > <tb.gnucash>_______________________________________________ >>>>>>> >>> <commodity-utilities.scm.patch> >>
_______________________________________________ gnucash-devel mailing list [email protected] https://lists.gnucash.org/mailman/listinfo/gnucash-devel
