Skip to content

The diff summary reports different change categories for the same bill depending on whether it was compared from XML or PDF #706

Description

@willhea

What's wrong

Comparing the same two versions of the same bill reports a different set of change
categories depending on whether the comparison ran on the XML or the PDF of those
versions. A tool that reads the diff output to answer "what kinds of change are in here"
gets five categories from one and four from the other, for identical legislative content.

The summary object at the top of the diff JSON counts how many sections were added,
removed, modified and moved. Measured on the committed fixture pair
tests/corpus/118-hr-8752, comparing 1_reported-in-house to 2_engrossed-in-house:

XML: {"added": 18, "removed": 2, "modified": 18, "unchanged": 0, "moved": 1}
PDF: {"modified": 18, "removed": 2, "moved": 1, "added": 16}

Three places in the repository state what summary may contain, and two of the three
agree on four keys:

Where What it says
schema/canonical-diff.md, the prose contract "Keys with zero count MAY be omitted. The four canonical keys are added, removed, modified, moved."
formatters/diff_html.py, the renderer _SUMMARY_ORDER = ("modified", "added", "removed", "moved"), and it skips zero buckets
diff_bill.py::_count_changes, the XML producer {t: counts.get(t, 0) for t in ("added", "removed", "modified", "unchanged", "moved")}

The XML producer seeds five keys, always, including zeros. The PDF producer builds its
summary as a Counter over the change types actually present, so it omits zeros and has
no unchanged kind to emit at all.

unchanged counts sections that the comparison matched across the two versions and found
identical. The diff document deliberately does not carry those entries: the adapter that
builds it drops every entry whose type is unchanged before serializing. So on an XML
comparison the key is a count of entries the document does not contain, and it now reads
0 on every document any shipped entry point produces, because #693 (the command line
returned the engine's internal JSON instead of the published contract) removed the last
flag that could raise it above zero.

How it surfaced

#693 raised this as a smaller question worth answering alongside it, and it was left out
of that change's scope. The measurements below were taken while implementing #693 in
#702, on commit 6efcc02c of branch worktree-issue693-cli-canonical-json, by calling
both pipeline entry points on the same committed fixture pair:

XML keys: ['added', 'modified', 'moved', 'removed', 'unchanged']
PDF keys: ['added', 'modified', 'moved', 'removed']
XML unchanged value: 0
XML changes carrying change_type 'unchanged': 0

The two committed regression baselines record the same split, on a different bill
(113-hr-3547, introduced to engrossed in House):

tests/data/canonical_baseline.json      "summary": {"added": 0, "modified": 1, "moved": 0, "removed": 0, "unchanged": 0}
tests/data/pdf_canonical_baseline.json  "summary": {"modified": 2}

All 27 entries in the XML baseline carry "unchanged": 0. The PDF baseline contains the
string unchanged zero times.

Why it matters

ADR 0006 (the canonical diff contract) makes this document the boundary that lets the
renderer and any outside tool be built independently of the engine, and pipeline
neutrality is the property it sells: schema/canonical-diff.md opens by saying "a diff
produced from XML inputs and a diff produced from PDF inputs share this shape." The
summary does not share it. That is a break in the one guarantee the contract exists to
make, in the field most likely to be read first.

The machine-readable schema cannot catch it. summary is declared as:

"summary": { "type": "object", "additionalProperties": { "type": "integer", "minimum": 0 } }

No required list and no enumerated key names, so any integer-valued key validates. The
prose contract names four keys and the schema accepts any, which means a producer can
drift from the written spec and stay green. The top level of the same schema uses
"additionalProperties": false, so the strictness is inconsistent within one file.

Nothing is currently breaking. Checked in this repository: the renderer iterates its own
four-name tuple rather than the document's keys, the browser front-end in web/webapp/js/
never reads summary at all, and the only reads of summary["unchanged"] anywhere are
in tests, against the internal BillDiff dataclass rather than against a diff document.
So this is contract hygiene rather than a live failure. What makes it worth doing now
rather than later is that a third output is coming: #656 (give the export an artifact shaped for a
language model rather than the renderer's document) adds another consumer of this same
document, and a machine reader is the kind that
enumerates keys instead of hardcoding four names.

What to do

Three options. The measurement that separates them is that unchanged is now provably
constant at 0 on every document a shipped entry point can produce, which was not true
before #693: filter_diff drops unchanged entries unless a caller opts in, the only
production caller of the XML adapter is compare/xml.py, and it no longer has a way to
opt in.

Option Effect Cost
A. Stop emitting unchanged from the XML producer The two pipelines agree, and the document matches the prose contract. Nothing is lost, since the value cannot be anything but 0. tests/data/canonical_baseline.json needs regenerating: all 27 entries carry the key and each carries a sha256 of the document.
B. Document unchanged as a fifth key One line in schema/canonical-diff.md. No emitted bytes change, no baseline churn. Documents a key that is always 0 and counts entries the document does not contain, and leaves the pipeline divergence in place, since the PDF side still never emits it. Converts an undocumented inconsistency into a documented one.
C. Make both pipelines emit the same fixed key set The summary becomes a fixed shape, which is the friendliest thing for a consumer. Contradicts the current "Keys with zero count MAY be omitted" sentence, so that rule changes too. Costs both baselines.

Worth settling as part of the decision: whether A needs a schema major bump. The 2.0 and
3.0 entries in the changelog both bumped major for a field removal, but both removed a
field that was in a required list and carried data. unchanged is in no required
list, is always 0, and the current spec already permits omitting a zero-count key, so
one reading is that A is already allowed by the contract as written and needs no bump at
all. That reading is worth confirming rather than assuming.

Verification

For option A, the suite passing is not proof. The XML baseline would be regenerated as
part of the change, so it agrees with whatever is emitted afterwards either way. The check
that would mean something is an assertion that both pipelines produce the same summary key
set for the same bill pair, shown to be red before the change: on 6efcc02c that
comparison is ['added', 'modified', 'moved', 'removed', 'unchanged'] against
['added', 'modified', 'moved', 'removed'].

Unverified

Consumers outside this repository. This is a public repository, so an unknown third party
could be reading the summary. #693 checked the one plausible consumer, BillTrax, and found
it vendors the engine and imports it as a library, reading BillDiff dataclass attributes
in process rather than parsing a diff document, so it is not affected. No other external
consumer is known, and the project has none it is currently supporting.

Refs #691, #693, #656

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions