Repository navigation
Book the gain between a lot basis and its cost exactly - #4
Merged
Merged
Conversation
This was referenced Oct 20, 2026
The journal engine took the whole remainder of a `commodity` line as the symbol it declares, so `commodity USD dollars` declared `USD dollars` and `commodity N1 ; foo` declared `N1 ; foo`. The C++ reader has only ever taken the first token there, in commodity_t::parse_symbol(char*&), and the fix for ledger/ledger#3313 also ends a bare symbol at a `;`, which begins a comment, as a `;` does after an amount in a posting. The two readings differed only when a header carried trailing text and its block declared an alias, which then named a different commodity on each side. The same issue has the C++ side strip a `;` comment from the argument of a commodity block's `alias`, which the oracle kept as part of the alias. `headerSymbol` reads a quoted symbol up to its closing quote, or else the leading run up to whitespace or a `;`, where a backslash puts the next character into the run, as in an amount and as C++ now does, so `A\;B` names `A;B`. `stripCommodityComment` cuts an alias argument at the first `;` that is neither inside a quoted symbol nor escaped. The account block is unchanged, because an account name may contain a `;`, and the `format` branch is unchanged, because `parseAmount` already leaves a trailing comment in the unread tail. The witnesses are regress/3313 and regress/3313_2 on the ledger branch johnw/fix-3313. With this oracle and that branch, test/semantic/check_all.py reported 3802 PASS and 0 FAIL over 4373 test files, with both ratchet bins empty. With the oracle at f5d891e, the same branch failed exactly those two files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mbne6AZZDzKMQcQV6Cp9Sn
4 tasks done
The C++ implementation rounded the gain or loss between a posting's
lot basis and its cost at the cost commodity's display precision plus
the quantity's decimals, and this rule recorded that convention. A
commodity that appears only in lot prices and costs has learned no
display precision, so the gain was rounded to whole units, and
`10 GAL {$2.00} @@ $25.30` completed its elided posting to $-20.30,
which is neither the basis nor the amount paid (ledger/ledger#3258).
Ledger now books the gain exactly, so the posting balances at its lot
basis, as the manual documents for the journal of regress/1773 and as
release 3.4.1 did. It still drops the residue that a per-unit price
leaves in a rebuilt basis when Ledger itself rounded that price out of
a total (regress/2975), but `{{TOTAL}}` divides exactly in this model,
which therefore has no residue to drop.
Absorbing a nonzero gain also widens the C++ cost to the decimals of
the lot basis, and the COST_PREC tagging of an elided posting reads
the widened cost; regress/1773 depends on it, rendering $-25.289 for a
commodity with no learned precision. The lot price therefore carries
the decimals of its literal, so that the cost decimals can follow.
The README's example of a convention that departs from exact rounding
was this rule; it now names the per-unit cost tolerance of
regress/1125 instead.
`lake build` completed successfully. Against the ledger branch
johnw/fix-3258, test/semantic/check_all.py reported 3800 PASS and
0 FAIL over 4369 test files, and a second run over regress/1773, 2975,
3013, 3258, and 3258_computed reported 5 PASS. The same C++ build
against the previous revision of this repository failed exactly
regress/1773 and regress/3258.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cz3H32XazWJeNtvPbWHD6n
Member
Author
|
ledger/ledger#3308 now pins |
jwiegley
force-pushed
the
johnw/exact-lot-gain
branch
from
November 4, 2026 18:00
e7b84e9 to
5376d29
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
costValis the basis andgainValisbasis - cost. The rule used to round the gain at the cost commodity's learned display precision plus the amount's decimals, which recorded the C++ behavior added by ledger/ledger fa324c876 (#2975) and 30cb08d75 (#3013).10 GAL {$2.00} @@ $25.30completed its elided posting to$-20.30, and the manual's11 GAL {=$2.299} @@ $25.30completed to$25.300instead of the$25.289it documents. C++ now drops only the residue that a per-unit price leaves in a rebuilt basis when Ledger itself rounded that price out of a total (regress/2975).{{TOTAL}}divides exactly in this model, so there is no residue to drop here.$-25.289for a commodity with no learned precision), so the lot price now carries its literal's decimals andcostDecsfollows them.test/regress/3258.test,test/regress/3258_computed.test, andtest/regress/1773.test. The rule's comments cite them, together withxact.cc.ledger/ledger#3308, the ledger PR for #3258, moves both pins, the flake input and the
lean/submodule, to this branch's commite7b84e9. Please merge with a merge commit, as #1 and #2 were, so that commit stays reachable frommain.#3 also edits
Ledger/Journal.lean. The two branches merge cleanly (git merge-tree), but each ledger PR pins its own branch, so whichever ledger PR lands second will need its pins moved to amaincommit that contains both changes.Test plan
nix build .#oracle: the build completes, so every proof checkstest/semantic/check_all.py): 3800 PASS, 0 FAIL, 0 out-of-scope, 0 cpp-parse (490 negative, 77 no-journal, 2 python)f5d891e, the same C++ build fails on exactly1773.test(only_cpp: $|-25.289|Assets:Checking) and3258.test🤖 Generated with Claude Code
https://claude.ai/code/session_01Cz3H32XazWJeNtvPbWHD6n