fix(diff): read amounts from display_text so the money diff and money tree agree (#365)

[willhea]

Summary

• A bill's money shows up in the report two ways: a leveled tree of the whole bill, and an amount-change table listing what changed between two versions. Each was reading a different rendering of the bill text, so the two could disagree about the same section.
• The tree reads display_text (the readable rendering). The amount-change table read body_text, a rendering normalized for matching that silently drops part of a section's contents. This change points amount extraction at display_text on both sides, so the two views read the same text.
• Why it matters: money that sat in the dropped part never reached the amount-change table, which is the headline output for the appropriations audience. It was invisible rather than wrong, so nothing failed and no one saw it.

What was actually wrong

Issue #365 identified the split by reading the code but flagged the divergence as unverified: no one had checked whether any real bill produced different amounts from the two sources. It does.

bill_tree._extract_section_text has a "simple lead-in" fast path that returns only a section's first <text> element when the rest of the section's contents sit in <list>, <continuation-text> or <paragraph>. Everything after the lead-in, amounts included, is dropped from body_text.

Measured on the committed test corpus:

• 267 nodes / 1,662 amount-instances present in display_text and absent from body_text
• 83 real amount changes missing from the amount-change table

The clearest instance is 118-hr-4366 (FY24 Consolidated Appropriations), House-passed to Senate-amended, Defense sec. 128: three $30,000,000 figures become $15,000,000 / $7,500,000 / $7,500,000. The report listed the section as modified with no financial change attached at all, so a $90M reallocation never reached the table.

The issue offered two options and said option 1 decides whether option 2 is needed. Option 1 (prove the two sources equivalent) is answered here: they are not equivalent, so this is option 2.

Changes

src/deltatrack/diff_bill.py — NodeDiff gains old_amount_text / new_amount_text, populated from the same display_text or body_text expression structure_tree already uses for the tree, so the two money views cannot drift apart again. bill_diff_to_dict and filter_diff read those instead of body_text. All seven NodeDiff construction sites populate them, including the moved-section path, so a section that relocates between versions keeps its amounts.

old_text / new_text deliberately stay body_text. They feed section matching, the text diff and the JSON payload, none of which this is trying to change; repointing them wholesale would churn rendered output for no gain. The two new fields default to None, and the properties that read them fall back to old_text / new_text, so any caller that builds a NodeDiff by hand behaves exactly as before.

tests/test_financial_diff.py — three unit tests for the source-selection and fallback behavior, plus a corpus regression class pinning the sec. 128 case and asserting the switch is strictly additive (it may surface an amount change, never hide one).

tests/test_diff_validation.py — two existing checks now read the same source the report reads, so they are testing the shipped path.

examples/hr4366_house_vs_senate_xml_diff.html — regenerated, since the change alters which entries carry financial data.

Test plan

Everything below was run on this branch at b919ce2.

The bug fails without the fix. Reverting only the two property bodies so they return old_text / new_text again, and re-running the new tests:

3 failed, 2 passed

E  AssertionError: sec. 128 must carry a financial change
E  assert None is not None

E  AssertionError: expected 8 newly surfaced amount changes, got 0
E  assert 0 == 8

E  - For construction, $1,000,000: Provided, $30,000,000 more.
E  + For construction, $1,000,000.

The two that still pass are the ones that do not depend on the source choice (the no-separate-source fallback, and the empty-display_text guard), which is expected. The source was restored afterwards and the tree confirmed clean.

Gates. Full suite: 1968 passed, 23 skipped, 15 xfailed, 0 failed. Every slow gate (pytest -m slow --deselect tests/test_govinfo_corpus_parity.py, the deselection being the live-network one): 556 passed, 7 skipped. ruff check: clean. ruff format --check: clean. Skip counts match the pre-change baseline; no new skip.

Effect on the rendered report, measured on the regenerated example rather than on an intermediate: 687 change entries before and after, 0 lost their financial flag, 6 gained one. Strictly additive at the output a reader actually sees.

Not run locally: the live-network corpus parity gate, which CI runs on its own schedule.

Breaking changes

None. The new fields are optional with a fallback to the previous behavior, and old_text / new_text are untouched, so the JSON payload shape and the text diff are unchanged. The only output difference is that more entries now correctly carry financial data.

Known limitations

This is a mitigation, not a cure, and the residual gap is recorded on the dataclass so the next reader does not have to rediscover it.

When the dropped payload is the only thing that changed, both versions' body_text are byte-identical. The section is therefore classified unchanged and filtered out of the report before the new fields are ever read. Its amounts are now correct, but the entry never appears. 118-hr-4366 sec. 124 and sec. 256 are live instances, and sec. 256 moves billions.

Fixing that requires completing body_text itself, i.e. repairing _extract_section_text. That was kept out of this change deliberately: body_text is the matching key, so changing it affects section matching across every bill, a much wider blast radius than the amount table. It wants its own issue and its own evidence.

Closes #365
Refs #147

---

Written with the help of Claude Code. I specified the change, reviewed the diff, ran the fault-injection repro and every gate reported above, and verified the rendered-output effect myself.

[willhea]

Review follow-ups, pushed as three commits on top of b919ce2. No behavior change: the committed example reports regenerate byte-identical, and the corpus A/B is unchanged at 0 lost / 63 gained across all 17 adjacent manifest pairs.

The residual gap is now tracked in #422 (a section whose only change is its dollar amounts can be dropped from the report entirely). Confirmed while reviewing that the limitation this PR documents is real rather than theoretical: for 118-hr-4366 v2 to v4, sec. 256 is classified unchanged because both sides' body_text are byte-identical, and filter_diff removes unchanged before any money filter runs, so it produces zero change cards in both the default and money-only views while $4.93B / $1.909B / $250M becomes $1.0B / $976M. sec. 124 behaves the same way. The dataclass comment now names #422 instead of "tracked separately".

fe0ca23 moves the strictly-additive safety property from one pinned bill pair to every corpus pair, in TestCorpusDiffSmoke, which is already parametrized over the manifest and already carries the fail-closed completeness floor. The per-pair count of newly surfaced changes stays pinned where it was, scoped to the one pair on purpose, so adding a corpus bill extends the guard instead of failing a count. Shown to fail rather than assumed green: truncating the display rendering to 80 characters turns 13 of the 17 new cases red.

5947aa0 collapses the two inline display_text or body_text expressions into bill_tree.amount_text. The comment claimed the two money views "cannot drift apart again", but nothing enforced it, so the guarantee was prose across two call sites. Both modules already depend on bill_tree, so this makes it structural at no new coupling cost.

de4ad5e records the one consumer-visible consequence in the schema doc: an amount in amount_entries need not appear in that change's text, since the two are derived from different renderings. The HTML path is unaffected (the card renders from full_text_span, and card and callout agree), but a JSON consumer searching text for a listed figure can legitimately miss it.

Gates on de4ad5e: fast suite 1406 passed, slow gates 416 passed, ruff check and ruff format --check clean. Skip counts unchanged, no new skip.

You may want to fold the #422 reference into the PR body's "Known limitations" section before merging.