Skip to content

test: regenerate snapshots for engine hash, metadata and interpolation - #271

Open
robcohen wants to merge 2 commits into
mainfrom
fix/meta-hash-snapshots
Open

test: regenerate snapshots for engine hash, metadata and interpolation#271
robcohen wants to merge 2 commits into
mainfrom
fix/meta-hash-snapshots

Conversation

@robcohen

@robcohen robcohen commented Aug 5, 2026

Copy link
Copy Markdown
Member

Regenerates the four snapshots that rustledger main has moved past. Tracked as
rustledger/rustledger#1967.

Only one of the four is the meta.hash change that prompted this. The other
three are engine fixes rustfava had not caught up with, and two of them are the
failures that have been quietly red on every rustledger PR for a while — they
were never separate bugs.

I compared every JSON leaf path old-vs-new rather than reading the diff, because
these snapshots are a single line each and git diff says nothing useful.

1–2. The hash change (rustledger/rustledger#1961)

snapshot what moved
test_get_ledger_data 62 changed leaves, all hash
account_report_off_by_one_journal 1 leaf (journal HTML); its own diff is 16 lines, all hash: values

beancount#1961 length-prefixes the fields fed into compute_directive_hash, hashes each
collection's length, and presence-tags the optionals. It fixed real collisions —
a transaction with a payee hashed identically to one without, and
2 HOOL {100 USD} hashed identically to 2 HOOL {200 USD}. Every digest
changes as a result.

3. Metadata arithmetic

number-value: 10 + 10 was captured as '10'; it is now '20'. Not a hash
change — the engine evaluates arithmetic in metadata values now. Checked against
Python beancount, which returns Decimal('20'), so the old snapshot was
pinning a divergence.

4. An error disappearing — the one worth reading twice

test_api_errors loses:

interpolation failed: multiple postings missing amounts or with
unresolved cost specs for currency USD (2 unknowns) (2018-07-07, "b")

An error vanishing is what a regression looks like, so I checked it against
beancount rather than accepting the update. For this fixture:

2018-07-07 * "a" "b"
  Assets:Cash      -14.00 USD
  Expenses:Stuff     4.14
  Expenses:Stuff

the engine used to give up with "2 unknowns". Python beancount solves it —
the bare 4.14 takes USD from the balancing currency, leaving 9.86 USD for
the empty posting — and reports no error at all:

Assets:Cash        units=-14.00 USD
Expenses:Stuff     units=4.14 USD
Expenses:Stuff     units=9.86 USD

rustledger now produces exactly those three postings. So the error was the
divergence, and dropping it moved toward compatibility.

Verification

Full suite against a component built from rustledger main at 058293c7a:

666 passed, 1 skipped, 100% coverage

rustledger/rustledger#1967. Four snapshots move, and only one of the
four is the meta.hash change that prompted this - the other three are
engine fixes rustfava had not caught up with.

Verified each structurally (compare every JSON leaf path old vs new)
rather than eyeballing a single-line diff, because these files are one
line each and `git diff` says nothing useful about them.

  test_internal_api-test_get_ledger_data
    62 changed leaves, ALL of them `hash`.

  test_json_api-test_api-account_report_off_by_one_journal
    1 changed leaf, the rendered journal HTML; its own diff is 16
    lines, all of them `hash:` values.

Both are rustledger/rustledger#1961, which length-prefixes the fields
fed into compute_directive_hash, counts collection lengths and
presence-tags optionals. It fixed real collisions - most importantly a
transaction WITH a payee hashing identically to one WITHOUT, and
2 HOOL {100 USD} hashing identically to 2 HOOL {200 USD}. Every digest
changes as a result.

  test_serialisation-test_serialise_entry_types
    `number-value: 10 + 10` was captured as '10', now '20'.

Not a hash change. The engine now evaluates arithmetic in metadata
values. Checked against Python beancount, which gives Decimal('20') -
so the old snapshot was pinning a divergence.

  test_json_api-test_api_errors
    loses "interpolation failed: multiple postings missing amounts or
    with unresolved cost specs for currency USD (2 unknowns)".

Also not a hash change, and the one worth reading twice: an error
DISAPPEARING is what a regression looks like. It is not one. For

  2018-07-07 * "a" "b"
    Assets:Cash      -14.00 USD
    Expenses:Stuff     4.14
    Expenses:Stuff

the engine used to give up with "2 unknowns". Python beancount solves
it - the bare 4.14 takes USD from the balancing currency, leaving
9.86 USD for the empty posting - and reports no error. rustledger now
produces exactly the same three postings, so dropping the error moved
it toward compatibility rather than away.

Full suite: 666 passed, 1 skipped, 100% coverage, against a component
built from rustledger main at 058293c7a.
@robcohen

robcohen commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

CI red here is expected, and it exposes a constraint worth recording: rustfava@main cannot satisfy both its own CI and rustledger's downstream check at the same time right now.

The two run against different engines:

check component it tests against
rustfava test-py (this repo) the released artifact — .github/workflows/test.yml downloads rustledger-ffi-component-$TAG.wasm for RUSTLEDGER_VERSION, currently v0.21.0 (2026-07-11)
rustledger Downstream (rustfava) a component built from the rustledger PR, via RUSTLEDGER_COMPONENT_WASM

All four engine changes these snapshots encode landed after v0.21.0 — the hash change on 2026-08-05, plus the metadata-arithmetic and interpolation fixes. So one set of snapshots cannot match both engines:

  • snapshots as on main today → rustfava CI green, rustledger's downstream check red (status quo, red on every rustledger PR)
  • snapshots in this PR → rustledger's downstream check green, rustfava CI red (the 4 failures above, which are exactly the 4 files changed here)

Verified rather than inferred: the failing tests on test-py (ubuntu-latest, 3.13) are precisely test_get_ledger_data, account_report_off_by_one_journal, test_api_errors, test_serialise_entry_types.

So this PR is correct against rustledger main and cannot go green until RUSTLEDGER_VERSION advances to a version containing 2f707c6e7. It should either wait for that pin to move, or land together with the bump.

Nothing about the snapshot content is in doubt — the full suite passes locally (666 passed, 100% coverage) against a component built from rustledger main at 058293c7a, and each of the four changes was checked against Python beancount.

@robcohen

robcohen commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Overlap to resolve: #270 already changes test_api_errors, and its version of the file is byte-identical to this one. I did not see it before opening this — my mistake.

#270 (opened 2026-08-02, companion to rustledger/rustledger#1921) does that one file with the fuller rationale for why the interpolation error was wrong. This PR does the same file plus the three the meta.hash change moved.

So this is a strict superset with identical content on the shared file, and both are blocked by the same thing — the released-component pin described above, which is why #270 has been sitting red since 2026-08-02 rather than because anything is wrong with it.

Suggest closing #270 in favour of this one so the four snapshots move together, since a partial update leaves rustfava red either way. Happy to do the reverse instead — drop test_api_errors from here and let #270 own it — if you would rather keep the interpolation change on its own PR with its own rationale.

@robcohen

robcohen commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Review — the snapshots are pinning something they mask everywhere else

Two things, one of which would stop beancount#1967 from recurring rather than fixing this instance of it.

1. These snapshots need regenerating again, after rustledger#1984

rustledger#1984 (the beancount#1968 decision — metadata becomes part of a directive's identity, plus four fields that reached no digest at all) changes every digest again. The literal hashes pinned here, e.g.

"meta":{"filename":"TEST_DATA_DIR/long-example.beancount","hash":"3f43843fea1277977ca64674e0255cf3ac68b8e5900c57a2636d48a197f5569d","lineno":5809}

all move. So this PR wants to be regenerated after beancount#1984 lands, not before — it was already waiting on the RUSTLEDGER_VERSION pin, so the ordering is: pin → beancount#1984 → regenerate.

2. meta.hash is the only hash in these files that isn't masked

tests/conftest.py:195 already masks digests out of snapshots:

out = re.sub(r'_hash": ?"[0-9a-f]+', '_hash":"ENTRY_HASH', out)

The pattern requires a literal _ before hash, so it catches "entry_hash": "..." — visible in these very files as "last_entry":{"date":"2000-01-03","entry_hash":"ENTRY_HASH"} — and misses "hash": "..." inside meta, because there the preceding character is a quote.

So the same value is masked in one field and pinned in another, in the same JSON object. That asymmetry is what makes beancount#1967 a recurring chore: every hash-format change churns 100+ digests across the snapshot corpus, and each regeneration is a diff nobody can meaningfully review.

Widening the regex to [a-z_]*hash would fix it — but not on its own, and this is the part worth deciding deliberately: those pinned digests are currently the only thing in either repo that catches an unintended hash change. rustledger#1984 is a breaking behavioral change to a wire field with no type change; neither cargo-semver-checks nor the WIT gate can see it. These snapshots can.

So the shape I'd suggest is mask in bulk, pin in one place:

  • widen the mask so the corpus stops churning, and
  • add a single small test that pins a handful of known digests for known directives, deliberately, with a comment saying it is the tripwire.

Then an unintended hash change still fails loudly — in one test with a readable diff — and a deliberate one is a five-line update instead of a hundred-file regeneration. That closes beancount#1967 as a class rather than as an instance.

Resolves the conflict in test_get_ledger_data.json by taking main's
version. This branch's update to that file is obsolete: #280 stopped
pinning rustledger's directive hash in snapshots at all, so the 62
digests this branch was refreshing no longer appear in the file, and
that test now passes against both the pinned release asset and a
component built from rustledger main.

What remains here is the part #280 could not fix - three snapshots whose
expected values genuinely differ between the pinned component and main,
so no single committed value is green on both sides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018bGRsKA42peqSnz4VMreBG
@robcohen

Copy link
Copy Markdown
Member Author

Merged main in and dropped this PR's update to test_get_ledger_data.json — that part is now obsolete.

#280 stopped pinning rustledger's directive hash in snapshots altogether: the redaction the snapshot machinery already performed required an underscore (_hash":), so it caught entry_hash and missed the bare hash key inside meta, leaking 62 literal digests into that file. With that fixed, the file has no digests left to refresh, and the test passes against both the pinned v0.21.0 asset and a component built from rustledger main.

What remains here is the part #280 could not fix — three snapshots whose expected values genuinely differ between the pinned component and main:

snapshot change why it is correct
test_api_errors spurious interpolation failed error removed beancount reports no such error for that fixture — I ran it directly; main matches beancount and the released component was wrong
test_serialise_entry_types number-value "10""20" fixture is number-value: 10 + 10, and beancount evaluates metadata arithmetic
account_report_off_by_one_journal, test_serialise_entry_types 101.0101, -1.0-1 every source literal involved is an integer (1 COM {100 USD}, price 101 USD), so the trailing zero was spurious

These cannot be green on both sides at once — regenerating them against main makes the same tests fail against v0.21.0, just inverted. So this stays parked until RUSTLEDGER_VERSION advances, at which point it should merge as-is.

Note CI here will fail until then, by construction: it runs against the pinned release asset, which still produces the old values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant