simonmichael / hledger

Robust, fast, intuitive plain text accounting tool with CLI, TUI and web interfaces.
https://hledger.org
GNU General Public License v3.0
3.02k stars 321 forks source link

close command shows wrong transaction price for multi-priced balances #1035

Closed lestephane closed 5 years ago

lestephane commented 5 years ago

I'm unable to explain the output I'm getting from hledger close

hledger prices correctly calculates the prices for a specific commodity whose purchase was done using total cost (ie @@) postings. But hledger close does not produce a total cost in line with that price.

I have journal entries where BTCs are purchased using total price (ie @@) postings

finance $ grep -r -A2 "12/31 To BTC Savings" 
2018-EUR.journal:2018/12/31 To BTC Savings    ; category:general
2018-EUR.journal-    Personal:Assets:Girokonto:Revolut:EUR                   -0,400000000 EUR = EUR78,71
2018-EUR.journal-    Personal:Assets:Savings:Revolut:BTC      0,00011812 BTC @@ EUR0,40000000
--
2018-EUR.journal:2018/12/31 To BTC Savings    ; category:general
2018-EUR.journal-    Personal:Assets:Girokonto:Revolut:EUR                   -7,200000000 EUR = EUR66,31
2018-EUR.journal-    Personal:Assets:Savings:Revolut:BTC      0,00213894 BTC @@ EUR7,20000000

The hledger-derived unit prices of said BTC commodity look fine too:

finance $ hledger prices --costs BTC | grep 2018-12-31
P 2018-12-31 BTC EUR3386,38673
P 2018-12-31 BTC EUR3366,153328

I do not have any 'P' directives anywhere in my journals, and am simply relying on transaction prices for the commodity prices to be determined by hledger automatically.

It's when I run hledger close command that I get what is (to me) unexpected.

finance $ hledger close -e 2019 --closing BTC 
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,2140556 BTC @@ EUR0,10000000 = 0 BTC
    equity:closing balances

finance $ hledger-ui --version
hledger-ui 1.14.2

Expected behaviour

2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,2140556 BTC @@  EUR720,543970317 = 0 BTC
    equity:closing balances

EUR720,543970317 is 0,2140556 * 3366,153328(last price of 2018)

the manual section for hledger close does not say anything about how transaction prices are determined in hledger close postings, so my expectation may well be off the mark.

If this issue is a known greyzone due to hledger not supporting proper priced lot tracking (#1022), feel free to close this one, and I'll revisit when #1022 is resolved.

Current workaround

I pipe the output of the hledger close command to sed, in order to clear closing transaction prices on problematic priced lots.

$ hledger close -e 2019 BTC | sed -e "s#BTC @@ EUR.* = \(.*\) BTC#BTC = \1 BTC#g"
2018/12/31 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,02269997 BTC = 0 BTC
    equity:closing balances

2019/01/01 opening balances
    Personal:Assets:Savings:Revolut:BTC     0,02269997 BTC = 0,02269997 BTC
    equity:opening balances
simonmichael commented 5 years ago

Great bug report and workaround - thanks. As you point out, the close command's handling of transaction prices is sloppy. If the sum contained multiple different transaction prices, it shows only the first and discards the rest.

What's the best behaviour I wonder ?

  1. combine differently priced amounts, and don't show any transaction price in the closing transaction.
  2. keep differently priced amounts separate, as if they were separate commodities.

2 is what we do by default everywhere else, so I guess that's the answer.

simonmichael commented 5 years ago

This gives output like:

ghci> :main  -f e.j close BTC --closing
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000 = 0 BTC
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC
    equity:closing balances

ghci> :main  -f e.j close BTC --closing -B
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC           -7,6 EUR = 0 EUR
    equity:closing balances
simonmichael commented 5 years ago

Oh. Which has a wrong balance assertion. Maybe if I leave the transaction price in the asserted amounts it'll all work out..

simonmichael commented 5 years ago

Related: https://github.com/simonmichael/hledger/issues/824

simonmichael commented 5 years ago
; Four possible variants of closing transaction, if we keep differently priced amounts in separate postings:

; no assertions
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000
    equity:closing balances

; assertion on last posting only
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC
    equity:closing balances

; assertion on each posting
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000 = 0.00213894 BTC
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC
    equity:closing balances

; The above are all valid, giving register output:
; 2018/12/31 To BTC S..  ..a:Re:BTC                    0,00011812 BTC                   0,00011812 BTC
; 2018/12/31 To BTC S..  ..a:Re:BTC                    0,00213894 BTC                   0,00225706 BTC
; 2019/06/03 closing ..  ..a:Re:BTC                   -0,00011812 BTC                   0,00213894 BTC
;                        ..a:Re:BTC                   -0,00213894 BTC                                0

; priced assertion on each posting
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000 = 0 BTC @@ EUR0,40000000
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC @@ EUR7,20000000
    equity:closing balances

; Not valid currently, as assertion checking ignores assertion amount prices (cf #824)
; calculated: -0.00011812, asserted:   0
simonmichael commented 5 years ago

Stepping back, the first question is do we want to preserve the "lots" formed by transaction prices (and the more sophisticated lots we hope to support in future) across close/open transactions, and eg from one year's file to the next ? I think the answer must be yes.

(Note for #1022: this demonstrates how posting date and a lot's "acquisition date" can be different. Eg I acquire a lot on 2018-06-01, and start a new file in 2019, that lot appears in the 2019 journal via a 2019-01-01 opening balances posting.)

lestephane commented 5 years ago

I agree that preserving the lots across close/open transactions is what makes the most sense. There's no telling when I'll sell the earlier lots of BTCs, how many years later, and when that happens it will be crucial to know how much the oldest lot cost, to sell in FIFO order (if my tax jurisdiction so requires).

It seems to me that whatever format one comes up with in #1022 for lot acquisition/deposit is what will eventually be used in hledger close --opening lot positions, and the present issue is just it's mirror inverse, that is, the format for lot disposal (hledger close --closing). Solve one and the other is solved as well. Maybe I should close this issue, and follow or join the discussion in #1022?

simonmichael commented 5 years ago

Let's leave this one open, the close command really is showing bogus information here. An interim fix might be needed. Eg: when an account has a multi-price balance, show one posting per price and don't add balance assertions.

simonmichael commented 5 years ago

Here's an example of more correctly closing a multi-priced balance. assets balance consists of 1A priced at 1B, and another 1A priced at 2B. Note:

2019/01/01
    (assets)                        1A @ 1B

2019/01/02
    (assets)                        1A @ 2B
$ hledger close assets -p 2019
2019/12/31 closing balances
    assets                         -1A @ 1B
    assets                         -1A @ 2B = 0A
    equity:closing balances         1A @ 1B
    equity:closing balances         1A @ 2B

2020/01/01 opening balances
    assets                          1A @ 1B
    assets                          1A @ 2B = 2A
    equity:opening balances        -1A @ 1B
    equity:opening balances        -1A @ 2B

Until now, we have left the equity posting's balance blank, so that only one equity posting was needed. I tried adding the -x/--explicit flag to make this optional. But I think it's best to just always show it explicitly. If someone prefers the implicit form it's an easy manual edit.

Here's a more complex case that I found in the tests, updated for the new behaviour:

# 8. Closing a multi-priced balance, a more complex example.
<
2016/01/31
    liabilities:employer                      $5,000.00
    income:salary

2016/02/05
    liabilities:employer                     $-5,000.00 @ 0.95 EUR
    expenses:tax                               1,852.50 EUR
    assets:bank                                2,897.00 EUR
    liabilities:employer

2016/02/29
    liabilities:employer                      $5,000.00
    income:salary

2016/03/04
    liabilities:employer                     $-5,000.00 @ 0.93 EUR
    expenses:tax                               1,813.50 EUR
    assets:bank                                2,836.00 EUR
    liabilities:employer

; Without these declarations, the closing/opening entries below will
; cause decimal marks to be misparsed. (How ?)
;commodity $1,000.00
;commodity 1,000.00 EUR

$ hledger -f- close -p 2016 assets liabilities
2016/12/31 closing balances
    assets:bank                       -5,733 EUR = 0 EUR
    liabilities:employer                $-10,000
    liabilities:employer       $5,000 @ 0.93 EUR
    liabilities:employer       $5,000 @ 0.95 EUR = $0
    liabilities:employer                  -1 EUR = 0 EUR
    equity:closing balances           $10,000.00
    equity:closing balances    $-5,000.00 @ 0.93 EUR
    equity:closing balances    $-5,000.00 @ 0.95 EUR
    equity:closing balances         5,734.00 EUR

2017/01/01 opening balances
    assets:bank                         5,733 EUR = 5,733 EUR
    liabilities:employer                  $10,000
    liabilities:employer       $-5,000 @ 0.93 EUR
    liabilities:employer       $-5,000 @ 0.95 EUR = $0
    liabilities:employer                    1 EUR = 1 EUR
    equity:opening balances           $-10,000.00
    equity:opening balances    $5,000.00 @ 0.93 EUR
    equity:opening balances    $5,000.00 @ 0.95 EUR
    equity:opening balances         -5,734.00 EUR

>=0

Let me know what you think.

lestephane commented 5 years ago

Looks fine to me, thanks!