Skip to content

feat(#115): rule-based financial classifier for bill nodes - #616

Open
ruggbk wants to merge 8 commits into
AgoraDMV:developfrom
ruggbk:feature/115-financial-classifier
Open

feat(#115): rule-based financial classifier for bill nodes#616
ruggbk wants to merge 8 commits into
AgoraDMV:developfrom
ruggbk:feature/115-financial-classifier

Conversation

@ruggbk

@ruggbk ruggbk commented Aug 11, 2026

Copy link
Copy Markdown

Related issue

Relates to #115

What does this change?

Adds a rule-based text classifier (financial_classifier/classify_bill.py) that labels dollar-bearing bill nodes by financial type. This is the foundation for meaningful financial summaries and per-account rollups described in #115.

Classifier: 22 regex patterns → 11 labels: appropriation, authorization, transfer, rescission, restriction, directive, cap, fee, earmark, availability, sub_allocation. Also includes build_financial_df() for DataFrame extraction and split_clauses() for Provided-That / of-which / in-addition clause splitting.

Validated across 7 bills (2 appropriations, 2 reconciliation, 3 authorization) — zero false positives in all authorization bills. CJS FY2024 drove most pattern refinements; went from 10 unknowns → 2.

Also adds:

  • financial_classifier/stress_test_analysis.py — multi-bill unknown/FP report
  • financial_classifier/stress_test_detail.py — per-bill full unknown-node inspector
  • financial_classifier/classifier_notes.md — pattern rationale and intentionally-unknown categories
  • financial_classifier/README.md — directory overview and run instructions
  • tests/test_classify_bill.py — 37 unit tests for all classify_text() outcomes

Open questions documented in classifier_notes.md for team input:

  • IRA reservation set-asides (the Administrator shall reserve $X) — ~10–15 nodes, semantically sub_allocation but don't start with "of which"
  • Auth-adjacent unknowns in authorization bills (is authorized, are authorized) — ~27 nodes; may warrant broadening the AUTHORIZATION pattern

How to test

uv run pytest tests/test_classify_bill.py -v        # 37 tests, all passing
uv run python financial_classifier/stress_test_analysis.py  # requires bills/ downloaded

For stress_test_analysis.py you'll need the 7 bill XMLs in bills/. Summary: 7 bills, zero false positives, unknown counts documented in classifier_notes.md.

Checklist

  • Linked the issue above (Relates to #115)
  • Ran the CI gates locally and they pass
  • New or changed behavior has tests
  • For a bug fix: the test fails without the fix, and I ran it both ways to check
  • Disclosed AI assistance below

AI assistance

Developed with Claude Code (claude-sonnet-4-6).


The test_similarity_labels.py collection errors (Windows UTF-8 encoding) are pre-existing and unrelated to this PR — happy to fix in a separate branch if useful.

ruggbk and others added 8 commits August 3, 2026 22:46
Refactor: classifier logic moves from 02_ into a standalone module so it
can be imported by both the report generator and future test scripts.

02_ is now a pure report generator; 03_ stress-tests the classifier across
all downloaded bills. Additions from testing on 119-hr-1: APPROP_ALT and
FEE patterns, build_financial_df now covers all dollar-amount nodes (not
just PRIMARY_LABELS), check_coverage added to verify no nodes are dropped.
Legend flattened to single list with per-type counts; fee label added.
New pattern:
- AUTHORIZATION: "authorized to be appropriated" -> authorization; fires
  after APPROP_ALT so hybrid nodes ("authorized to be appropriated, and
  there is hereby appropriated") correctly stay appropriation

Pattern fixes (CJS FY2024 surfaced all five gaps):
- RESCISSION: catches plural "are hereby rescinded"
- APPROP_ALT: catches "there is hereby appropriated" (hereby insertion)
- RESTRICT: handles subsection-prefixed opening -- (b)None of the funds
- APPROP: handles subsection-prefixed opening -- (a) For an additional amount
- RESTRICT_NOTWITHSTANDING: new, catches Notwithstanding ... none of the funds

Tested across 7 bills (2 approp, 2 reconciliation, 3 authorization).
Zero false positives in authorization bills (NDAA, Farm Bill, IIJA).
CJS unknowns: 10 -> 2; auth bills now surface authorization label correctly.

Also adds:
- notebooks/stress_test_analysis.py: multi-bill stress-test runner
- notebooks/stress_test_detail.py: per-bill unknown-node inspector
- notebooks/classifier_notes.md: pattern rationale and intentional unknowns

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
37 tests covering every label path in classify_text(), including
subsection-prefix edge cases fixed in this branch, AUTHORIZATION
ordering (fires after APPROP_ALT), and APPROP/RESCISSION interactions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…istency

- Add authorization to TYPE_COLORS/TYPE_DESCRIPTIONS/ALL_TYPES in 02_financial_report.ipynb
  (was silently falling back to gray for any authorization node)
- Fix two relic section headers in stress_test_analysis.py: auth-hint unknowns
  no longer say "should stay unknown"; FP section no longer implies authorization
  is not a primary label
- Update classifier_notes.md: post-fix unknown counts, pattern table now includes
  AUTHORIZATION and RESTRICT_NOTWITHSTANDING, section 1 updated to reflect that
  "authorized to be appropriated" is now classified
- Add notebooks/README.md: file inventory, run instructions, pandas note
- Regenerate financial_118_hr_4366.html with current classifier

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The directory contains a classifier module, analysis scripts, docs, and
tests — not primarily notebooks. financial_classifier/ is more accurate
and distinguishes it from the financial diff code elsewhere in the repo.

Update all import paths: notebooks.classify_bill -> financial_classifier.classify_bill

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@willhea
willhea self-requested a review August 11, 2026 22:37

@willhea willhea left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruggbk, thanks for putting this together! I've reviewed with Claude Opus and Chatgpt Sol. Full recommendations below. The fast summary from me:

  1. Let's move this to Docs/Research as a research spike rather than changing the core deltatrack product. Once we're confident in the methodology, we can move it over.
  2. Let's let PR #620 handle the UTF issue. Review comment on that PR is there.
  3. There's some minor challenges with the code, detailed below.

Overall, I really love this line of research and the presentation of different appropriations types.

Requested scope/layout changes

Please move this work under docs/research/financial-semantics/ (or very similar), consistent with the other research streams in the repo.

For example, the notebooks, classifier/probe scripts, notes, sample results, and HTML visualization can all live there. I'd also reframe the PR as something like:

research(#115): explore rule-based financial semantics

rather than feat(#115).

This isn't only a tidiness preference. On current develop, a top-level financial_classifier/ containing Python is deliberately rejected by the repository's surface-boundary gate:

tests/test_surface_boundary.py::test_a_namespace_package_cannot_hide_from_the_product_scan

top-level directories hold Python but are neither a discovered product package
nor a listed consumer tree: ['financial_classifier']

That gate exists so new Python surfaces have to be deliberately classified as product, tooling, delivery, etc.

Please remove the diff_bill.py change from this PR. It's already covered in #620

Please keep the classifier explicitly exploratory for now. I don't want to integrate it into the deliverable or treat the current labels as a finalized financial model until we've investigated further.

I'd also soften the README/PR language around "validated across 7 bills." The results are quite strong on the two appropriations bills, while the three authorization bills deliberately leave roughly half or more of dollar-bearing nodes unknown. That's completely reasonable for an appropriations-focused classifier; I just want the research artifact to describe exactly what has been established.

Methodology/code findings to address in the spike

These are the result-bearing issues I think are worth fixing now. I don't want to turn this into production-grade classifier review.

1. TRANSFER = ^Of (the )?amounts is too broad

The pattern only checks the opening words and never establishes that a transfer actually occurs.

These currently all classify as transfer:

Of the amounts made available under this heading, $10,000,000 shall be available for grants

Of the amounts appropriated under this heading, $5,000,000 shall remain available until expended

Of amounts made available in this Act, $2,000,000 is for administrative expenses

Of the amounts provided, not to exceed $1,000,000 may be used for official reception

Of the amounts made available under this heading, $10,000,000 shall be transferred to the Secretary

Only the last is actually a transfer.

The current corpus impact is small but real: across the seven analyzed bills, 4 nodes are labeled transfer; 3 contain actual transfer language and 1 does not.

Please tighten the rule and add a negative control demonstrating that an "Of the amounts..." availability/sub-allocation clause does not become a transfer.

2. The "zero false positives" claim is broader than the evidence

stress_test_analysis.py currently sends a predicted appropriation / rescission / transfer node to the false-positive review population only when that same node also matches AUTH_HINTS.

Across the seven-bill run:

  • 619 nodes were predicted appropriation, rescission, or transfer
  • 12 reached the FP-review population
  • 607 primary predictions were therefore not examined by that false-positive check

The 12 reviewed cases do appear correct, which is useful evidence. I'd just state the narrower result:

No false positives were found among the auth-hint-flagged primary predictions that were manually reviewed.

Broader precision should remain an open research question rather than being reported as zero false positives.

3. check_coverage() can pass when an occurrence was dropped

The current check compares sets of body_text.

ADR 0019 specifically rejected body text as observation identity because different parsed observations can have identical text, causing coverage/validation checks to fail optimistically.

This occurs in the current research population. In 119-hr-1, 206 dollar-bearing node occurrences collapse to 199 unique body_text values. One repeated text occurs five times.

That means the existing coverage output can say:

All 199 dollar-amount nodes represented

even though there were actually 206 observations, and losing one of the duplicate occurrences could remain invisible.

Please make coverage occurrence-aware using the ADR 0019 observation identity approach, and add a negative control showing that if two observations have identical text and one occurrence is dropped, coverage fails.

4. Pin the seven source versions

find_xml() currently effectively selects:

sorted(directory.glob("*.xml"))[0]

so the version analyzed depends on which files happen to exist locally.

This is not theoretical: a second reproduction initially disagreed with the recorded table for two bills because the local directories were populated differently. Re-fetching the intended seven bills reproduced the PR's recorded results exactly.

Please pin the source/version for each analyzed bill so the table is reproducible. Since ADR 0019 already gives us the vocabulary, I'd record enough provenance to identify:

  • source/version
  • source_sha256
  • parser_revision

and use node ordinal where individual observations are persisted or manually adjudicated.

This also matters because parser evolution changes the emitted research population. Running the same source material through current develop can surface additional dollar-bearing nodes even though the underlying methodology hasn't necessarily regressed. The research artifact should make clear which parser snapshot produced its reported counts.

Smaller cleanup items

A few additional things I would clean up while moving the spike:

  • The scripts/notebooks still import bill_tree as a top-level module. After the engine move they should use deltatrack.bill_tree; otherwise the stress scripts don't run on current develop.
  • primary_amount() appears unused and can return a cap/ceiling as though it were the primary amount in some clauses. Since build_financial_df() doesn't use it, I'd remove it rather than leave a misleading helper around for later #115 work.
  • The existing 37 tests for classify_text() are real and useful. I don't think every helper needs production-level testing here, but split_clauses() is result-bearing and sufficiently intricate that I'd add focused tests for its main split forms. The duplicate-occurrence negative control above should cover check_coverage().
  • pandas currently lives behind build_financial_df() inside the classifier module. Moving the whole thing under docs/research/ cleanly avoids turning that analysis dependency into part of the core product surface.
  • I'd strip committed notebook execution outputs before merging. There are thousands of lines of generated output/full bill text, which makes future notebook diffs difficult to review. I don't think we need to introduce a repo-wide notebook hook in this PR unless we decide notebooks will become a common research format.
  • The README's download example still points to root fetch_bills.py; the current path is tools/fetch_bills.py.
  • 01_bbi_eda.ipynb is included in the PR but missing from the README's file table.

Thanks again — I think this is a good foundation and would like to see it land as a research spike we can build on.

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.

2 participants