Repository navigation
1.19-1.22: balancesheet --tree --no-elide --begin with certain account directives can show wrong balances #1698
Copy link
Copy link
Closed
Labels
A-BUGSomething wrong, confusing or sub-standard in the software, docs, or user experience.Something wrong, confusing or sub-standard in the software, docs, or user experience.balancesheetbountyThar's some kind o' loot on offer..Thar's some kind o' loot on offer..regressionA backwards step, indicating a weakness in our QA. We don't like these.A backwards step, indicating a weakness in our QA. We don't like these.
Description
I noticed a difference in the final line between these two commands, even though the only difference is the --tree flag. Here is an example (anonymised):
$ hledger bs --no-elide --begin=2019-01-01
|| 2021-09-14
=======================================++============================================================
Assets ||
---------------------------------------++------------------------------------------------------------
assets:parent:child1 || GBP 90.00
assets:parent:child2 || GBP 2731.02
---------------------------------------++------------------------------------------------------------
|| GBP 2821.02
=======================================++============================================================
Liabilities ||
---------------------------------------++------------------------------------------------------------
---------------------------------------++------------------------------------------------------------
||
=======================================++============================================================
Net: || GBP 2821.02
$ hledger bs --no-elide --begin=2019-01-01 --tree
Balance Sheet 2021-09-14
|| 2021-09-14
=========================++===========================================================
Assets ||
-------------------------++-----------------------------------------------------------
assets || GBP 4121.98
parent || GBP 4121.98
child1 || GBP 90.00
child2 || GBP 2731.02
-------------------------++-----------------------------------------------------------
|| GBP 4121.98
=========================++===========================================================
Liabilities ||
-------------------------++-----------------------------------------------------------
-------------------------++-----------------------------------------------------------
=========================++===========================================================
Net: || GBP 4121.98
I can't explain why the the two Net numbers differ, even though the only difference is the --tree flag. This change in calculation doesn't seem to happen when I don't pass --begin=2019-01-01.
I'm afraid I can't figure out a small reproducible example, that's why I shared an anonymous sample output, in which I removed most of the lines.
I'm using the latest hledger version on Ubuntu:
$ hledger --version
hledger 1.21
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
A-BUGSomething wrong, confusing or sub-standard in the software, docs, or user experience.Something wrong, confusing or sub-standard in the software, docs, or user experience.balancesheetbountyThar's some kind o' loot on offer..Thar's some kind o' loot on offer..regressionA backwards step, indicating a weakness in our QA. We don't like these.A backwards step, indicating a weakness in our QA. We don't like these.
Activity
simonmichael commentedon Sep 15, 2021
Thanks for the report @Flimm. Would you be able to test with the current release ? That's 1.22.2, available from https://hledger.org/download.html
Flimm commentedon Sep 15, 2021
Thanks! I thought I had upgraded, but I was mistaken. I did not experience this issue in 1.22.2. My apologies.
Xitian9 commentedon Sep 15, 2021
I'd still be interested in knowing what exactly caused this issue in an old version. That's a rather alarming mistake, and I'd like there to be a test for it.
simonmichael commentedon Sep 16, 2021
Agreed, for such a bad issue we should know the versions affected and advertise it.
Xitian9 commentedon Sep 16, 2021
@Flimm
Any information you could give us to help reproduce this would be great. How big is your journal, when does it start and end, what is the narrowest date range you can choose which still shows this error (and what postings does that encompass), are there any multicommodity amounts or prices, and so on.
It would be even better if you could do a git bisect and identify the precise commit which fixes it.
simonmichael commentedon Sep 16, 2021
Usually a smaller repro is possible - keep removing things until it stops happening. There's also hledger print --anon, and secure 1-to-1 matrix chat if that helps.
Flimm commentedon Sep 16, 2021
Thanks for encouraging me. I did what you said, and I found a reproducible example. Here is
rep.journal:It turns out the
--no-elideis required for the bug to manifest itself.The two commands should have identical
Nettotals, but they don't.I also tried it with hledger 1.22.2, and it seems to be in that version too.
Xitian9 commentedon Sep 16, 2021
Great work! I can reproduce on master.
This as a weird one, as it seems to require all of:
Let me dig into this.
Tentative hypothesis: historical postings are not being included in subtotals for boring parents in compound balance reports, for some reason. But only when you manually declare account types?
Xitian9 commentedon Sep 16, 2021
Aha! I've got it.
By declaring account types manually, you've overridden the default rules. This means that the following accouns are not considered asset accounts, and so are not included in the balancesheet:
assets,assets:foobar. Since these accounts have no postings in them this would not normally be a problem, except that since you've asked for--no-elidein--treemode, it goes ahead and puts them in.CompoundBalanceReport will filter historical postings based on whether they match the subreport query: in this case whether it's an Asset or Cash account. Since the parent accounts don't match this, it throws out their historical postings.
A temporary fix is to declare all your parent accounts:
fix: bal: Make sure boring parent accounts in compound balance reports
Xitian9 commentedon Sep 16, 2021
You found a knotty one here @Flimm. Thanks so much for your work cutting this down to size!
fix: bal: Make sure boring parent accounts in compound balance reports
simonmichael commentedon Sep 18, 2021
Excellent! This is indeed quite hard to wrap one's head around; I'm glad you two did. This shows it in another way:
hledger 1.22 bal, showing expected output:
hledger 1.22 bs, showing incorrect numbers for parents in first columns:
$ hledger-1.22.2 -f c.j bs --begin=2019-01-01 -D --tree --no-elide Balance Sheet 2019-01-01..2019-01-04 || 2019-01-01 2019-01-02 2019-01-03 2019-01-04 =============++================================================= Assets || -------------++------------------------------------------------- assets || 0 0 0 GBP -63.49 foobar || 0 0 0 GBP -63.49 current || GBP -63.49 GBP -63.49 GBP -63.49 GBP -126.98 -------------++------------------------------------------------- || 0 0 0 GBP -63.49 =============++================================================= Liabilities || -------------++------------------------------------------------- -------------++------------------------------------------------- || =============++================================================= Net: || 0 0 0 GBP -63.49With the #1699 fix, numbers look correct again, similar to bal:
simonmichael commentedon Sep 18, 2021
It seems this was one of the casualties in 1.19, ie a regression.
So @Flimm, thank you and please claim your https://hledger.org/regressionbounty !
$ hledger-1.18 -f c.j bs --begin=2019-01-01 -D --tree --no-elide Balance Sheet 2019-01-01..2019-01-04 || 2019-01-01 2019-01-02 2019-01-03 2019-01-04 =============++================================================= Assets || -------------++------------------------------------------------- assets || GBP -63.49 GBP -63.49 GBP -63.49 GBP -126.98 foobar || GBP -63.49 GBP -63.49 GBP -63.49 GBP -126.98 current || GBP -63.49 GBP -63.49 GBP -63.49 GBP -126.98 -------------++------------------------------------------------- || GBP -63.49 GBP -63.49 GBP -63.49 GBP -126.98 =============++================================================= Liabilities || -------------++------------------------------------------------- -------------++------------------------------------------------- || =============++================================================= Net: || GBP -63.49 GBP -63.49 GBP -63.49 GBP -126.98 $ hledger-1.19 -f c.j bs --begin=2019-01-01 -D --tree --no-elide Balance Sheet 2019-01-01..2019-01-04 || 2019-01-01 2019-01-02 2019-01-03 2019-01-04 =============++================================================================================= Assets || -------------++--------------------------------------------------------------------------------- assets || 0 0 0 GBP -63.49 foobar || 0 0 0 GBP -63.49 current || GBP -63.49 GBP -63.49 GBP -63.49 GBP -126.98 -------------++--------------------------------------------------------------------------------- || 0 0 0 GBP -63.49 =============++================================================================================= Liabilities || -------------++--------------------------------------------------------------------------------- -------------++--------------------------------------------------------------------------------- || =============++================================================================================= Net: || 0 0 0 GBP -63.49simonmichael commentedon Sep 18, 2021
For each major release, our usual policy is to make bugfix releases until it is free of major bugs. In 1.19.1 (2020/09/07), we fixed most of them, but we missed this one. Do we need to backport this fix to 1.19, 1.20, 1.21, and 1.22 ? I think, and hope, this one is obscure enough that we can just fix master and move on.
fix: bal: Make sure boring parent accounts in compound balance reports
[-]Combining --tree with --begin for balancesheet causes miscalculation[/-][+]balancesheet --tree --no-elide --begin with certain account directives can show wrong balances (1.19-1.22)[/+]Xitian9 commentedon Sep 18, 2021
I think we're pretty safe here. The combination of factors that trigger it are unlikely.
;cln: clarify #1698 test
[-]balancesheet --tree --no-elide --begin with certain account directives can show wrong balances (1.19-1.22)[/-][+]1.19-1.22: balancesheet --tree --no-elide --begin with certain account directives can show wrong balances[/+]