What's wrong
The two comparison pipelines reach the same output by different routes. The PDF pipeline turns its
diff result straight into the canonical diff document, the versioned JSON contract in
schema/canonical-diff.md. The XML pipeline first converts its diff result into an intermediate
Python dictionary, then converts that dictionary into the same canonical document.
XML: diff_bills() → BillDiff → bill_diff_to_dict() → dict → xml_diff_to_canonical(dict) → canonical
PDF: diff_pdfs() → PdfDiff ─────────────────────────────→ pdf_diff_to_canonical(PdfDiff) → canonical
xml_diff_to_canonical takes a dictionary; pdf_diff_to_canonical takes a dataclass. The XML side
carries one more representation of the same information than the PDF side needs.
The intermediate is a survival from the tool's original design rather than a considered stage. When
DeltaTrack was created (d9478330, 2026-04-13) that dictionary was the product: the whole output was
"structured JSON diffs". The canonical contract arrived a month later (d7c42802, 2026-05-08) and was
built, in its own commit message, "from the existing XML diff dict and PdfDiff dataclass" — layered on
top of what was there rather than replacing it.
Why it matters
Three costs, none of them urgent.
An extra representation is an extra place for the two pipelines to diverge. The dictionary uses its own
field vocabulary (match_path, element_id_old, new_text) which is neither the engine's internal
naming nor the contract's, so a change to what a comparison produces has to be carried through one more
translation on the XML side only.
It is also load-bearing in a way that is invisible. Facts the dictionary carries are read by
xml_diff_to_canonical and then dropped rather than emitted: element_id_old and element_id_new are
used at src/deltatrack/formatters/canonical.py:91-92 to resolve each change's position in the bill
text, and do not appear in the resulting document. That is the pattern #653 (make the view a consumer of
the diff, not a second producer of it) names as the defect one seam further out, and it is the reason
the canonical document currently cannot say which structural node a change belongs to.
Finally it explains an asymmetry that reads as a decision but is not one: ./diff_pdf.py has no JSON
output because the PDF branch never builds a dictionary, so there was nothing to serialize. Nobody chose
that.
What to do
Change xml_diff_to_canonical to take a BillDiff directly, mirroring pdf_diff_to_canonical, and
delete bill_diff_to_dict.
After #693 (the command line returns the engine's internal JSON instead of the published canonical
contract) lands, bill_diff_to_dict has exactly one caller left in production code:
src/deltatrack/compare/xml.py:68. That is what makes this a contained change rather than a sweep.
Sequencing. This should follow #693 and should not precede #653. #653 is already reshaping what the
canonical document carries and how it is produced, including whether the discarded element_id values
should be emitted rather than dropped. Rebuilding the XML adapter before that settles means rebuilding it
twice.
Cost. Five test modules construct dictionaries and feed them to the adapter:
tests/test_format_html.py, tests/test_financial_callout_whole_item.py,
tests/test_formatters_canonical.py, tests/test_canonical_tree.py, and
tests/test_formatters_adapters_xml.py. Several exist specifically to test the dictionary-to-canonical
conversion, so they are rewritten against the dataclass or removed as testing a stage that no longer
exists, rather than adjusted.
Not in scope. Whether the canonical document should carry node identity, so a consumer can tell which
structural node a change belongs to. That is a contract question with its own issue; this one is about
removing a redundant representation, and should not quietly change what the document contains.
Verification
A pure internal refactor must change no output. The canonical document produced for a committed corpus
pair should be byte-identical before and after, on both pipelines, and the committed example reports
should regenerate unchanged.
"The suite still passes" is not sufficient on its own here: the baseline files
tests/data/canonical_baseline.json and tests/data/pdf_canonical_baseline.json are regenerable, so a
change that altered the output and the baseline together would pass. The comparison should be against the
pre-change output captured before the refactor starts.
Unverified
Whether every field the dictionary currently carries has a home on BillDiff, or whether some are
computed during the dictionary conversion and would need to move. bill_diff_to_dict computes the
financial enrichment inline (compute_financial_change per change), so at least that step needs a
destination; the rest was not audited field by field.
Part of #691 (the epic making every surface produce the same answer). Follows #693 (the command line
returns internal JSON). Should not precede #653 (make the view a consumer of the diff), which is
reshaping the same boundary.
Refs #691, #693, #653
What's wrong
The two comparison pipelines reach the same output by different routes. The PDF pipeline turns its
diff result straight into the canonical diff document, the versioned JSON contract in
schema/canonical-diff.md. The XML pipeline first converts its diff result into an intermediatePython dictionary, then converts that dictionary into the same canonical document.
xml_diff_to_canonicaltakes a dictionary;pdf_diff_to_canonicaltakes a dataclass. The XML sidecarries one more representation of the same information than the PDF side needs.
The intermediate is a survival from the tool's original design rather than a considered stage. When
DeltaTrack was created (
d9478330, 2026-04-13) that dictionary was the product: the whole output was"structured JSON diffs". The canonical contract arrived a month later (
d7c42802, 2026-05-08) and wasbuilt, in its own commit message, "from the existing XML diff dict and PdfDiff dataclass" — layered on
top of what was there rather than replacing it.
Why it matters
Three costs, none of them urgent.
An extra representation is an extra place for the two pipelines to diverge. The dictionary uses its own
field vocabulary (
match_path,element_id_old,new_text) which is neither the engine's internalnaming nor the contract's, so a change to what a comparison produces has to be carried through one more
translation on the XML side only.
It is also load-bearing in a way that is invisible. Facts the dictionary carries are read by
xml_diff_to_canonicaland then dropped rather than emitted:element_id_oldandelement_id_newareused at
src/deltatrack/formatters/canonical.py:91-92to resolve each change's position in the billtext, and do not appear in the resulting document. That is the pattern #653 (make the view a consumer of
the diff, not a second producer of it) names as the defect one seam further out, and it is the reason
the canonical document currently cannot say which structural node a change belongs to.
Finally it explains an asymmetry that reads as a decision but is not one:
./diff_pdf.pyhas no JSONoutput because the PDF branch never builds a dictionary, so there was nothing to serialize. Nobody chose
that.
What to do
Change
xml_diff_to_canonicalto take aBillDiffdirectly, mirroringpdf_diff_to_canonical, anddelete
bill_diff_to_dict.After #693 (the command line returns the engine's internal JSON instead of the published canonical
contract) lands,
bill_diff_to_dicthas exactly one caller left in production code:src/deltatrack/compare/xml.py:68. That is what makes this a contained change rather than a sweep.Sequencing. This should follow #693 and should not precede #653. #653 is already reshaping what the
canonical document carries and how it is produced, including whether the discarded
element_idvaluesshould be emitted rather than dropped. Rebuilding the XML adapter before that settles means rebuilding it
twice.
Cost. Five test modules construct dictionaries and feed them to the adapter:
tests/test_format_html.py,tests/test_financial_callout_whole_item.py,tests/test_formatters_canonical.py,tests/test_canonical_tree.py, andtests/test_formatters_adapters_xml.py. Several exist specifically to test the dictionary-to-canonicalconversion, so they are rewritten against the dataclass or removed as testing a stage that no longer
exists, rather than adjusted.
Not in scope. Whether the canonical document should carry node identity, so a consumer can tell which
structural node a change belongs to. That is a contract question with its own issue; this one is about
removing a redundant representation, and should not quietly change what the document contains.
Verification
A pure internal refactor must change no output. The canonical document produced for a committed corpus
pair should be byte-identical before and after, on both pipelines, and the committed example reports
should regenerate unchanged.
"The suite still passes" is not sufficient on its own here: the baseline files
tests/data/canonical_baseline.jsonandtests/data/pdf_canonical_baseline.jsonare regenerable, so achange that altered the output and the baseline together would pass. The comparison should be against the
pre-change output captured before the refactor starts.
Unverified
Whether every field the dictionary currently carries has a home on
BillDiff, or whether some arecomputed during the dictionary conversion and would need to move.
bill_diff_to_dictcomputes thefinancial enrichment inline (
compute_financial_changeper change), so at least that step needs adestination; the rest was not audited field by field.
Part of #691 (the epic making every surface produce the same answer). Follows #693 (the command line
returns internal JSON). Should not precede #653 (make the view a consumer of the diff), which is
reshaping the same boundary.
Refs #691, #693, #653