The same bill pair renders three different version headings depending on which surface asked

[willhea]

What's wrong

Comparing the same two bill files gives a report with a different heading depending on which
way you asked. The published example on the project's site, the same comparison run from the
command line, and the same two files uploaded to the web page each name the versions
differently.

Measured on the committed fixture pair tests/corpus/118-hr-8752, comparing
1_reported-in-house.pdf to 2_engrossed-in-house.pdf:

Surface	Report heading
scripts/render_examples.py, which builds the published examples	v1: reported-in-house → v2: engrossed-in-house
./diff_pdf.py	reported-in-house → engrossed-in-house
Uploading the two files to the web page	1_reported-in-house → 2_engrossed-in-house

The same three-way split appears inside each report's embedded diff.json, the
machine-readable copy of the comparison that the download button hands out: the two CLI
variants and the upload disagree on both the label and the version_number of each side.

Two causes, which is why they are filed together. They share a fix and touch the same call
sites.

Version ordinals are known but not passed. compare_pdfs_html in
src/deltatrack/compare/pdf.py accepts start_version_number and end_version_number, the
bill's legislative ordinals (the 1 and 2 in 1_reported-in-house.pdf, meaning first and
second published version of that bill). scripts/render_examples.py passes them.
render_pdf_diff_html in src/deltatrack/diff_pdf.py, which is what ./diff_pdf.py calls,
does not, even though its own docstring names exactly that caller:

Pass the version numbers when the caller knows the bill's legislative ordinals (rendering
a numbered corpus file, not an upload) so the report heads itself identically to the XML
report for the same pair.

Two different filename-to-label algorithms. label_from_stem in
src/deltatrack/version_stems.py strips a leading <n>_ ordinal prefix, turning
1_reported-in-house into reported-in-house. _label_from_filename in web/app.py
strips path components and the file extension only, so the prefix survives into the heading.
It is the only one of the two with no test.

Between them, resolving a source document to its version identity is implemented at six
places in production code:

Site	Derives label	Derives ordinal
compare/xml.py::compare_xml_files_html	yes	yes
diff_pdf.py::render_pdf_diff_html	yes	no
diff_bill.py::cmd_compare, HTML branch	inline	inline
diff_bill.py::cmd_compare, JSON branch	no	separate inline loop
web/app.py::_label_from_filename	different algorithm	no
scripts/render_examples.py::render_pdf_diff	inline	inline

tests/test_canonical_baseline.py and tests/test_pdf_canonical_baseline.py re-derive it
again, which is how a change to the rule can leave the baselines agreeing with a copy of
itself rather than with the code.

A contributing cause worth naming, because it is what makes the duplication load-bearing:
compare/xml.py offers a file-level entry point, compare_xml_files_html, that takes two
paths and does the whole resolution. compare/pdf.py has no equivalent, so every PDF caller
hand-rolls it, and they hand-rolled it differently.

How it surfaced

An audit of the command-line and web diff paths on develop at commit 8fb0fa16. The
headings above were read out of four rendered reports: the two committed examples in
examples/, and two rendered during the audit by driving diff_pdf.main() and by posting
to /api/compare through Starlette's TestClient. The probe is
docs/research/cli-ui-parity/probes/probe_htmldiff.py on branch
worktree-audit-cli-ui-diff-parity, and it prints the embedded version metadata for each:

cli_pdf: versions={"v1": {"label": "reported-in-house", "version_number": null, ...},
                   "v2": {"label": "engrossed-in-house", "version_number": null, ...}}
ui_pdf:  versions={"v1": {"label": "1_reported-in-house", "version_number": null, ...},
                   "v2": {"label": "2_engrossed-in-house", "version_number": null, ...}}

Aside from these two lines and the visible heading built from them, the two reports are
byte-identical.

Why it matters

Two docstrings and one README section state an invariant the code does not hold, which is
worse than the inconsistency itself because it means a reader is being told the wrong thing.

• compare_xml_files_html says deriving the labels and ordinals from filename stems "is what
  makes a rendered example identical to the report a reader would get by uploading the same
  two files." The upload keeps the 1_ prefix and carries a null ordinal, so it is not.
• README's HTML report section documents the heading as
  v1: reported-in-house → v2: engrossed-in-house, then says diff_pdf.py "writes the same
  HTML report described above." For PDF input from the command line, it does not.

The practical consequence is for anyone checking a figure in a published example against the
same comparison run locally, or against their own upload: the documents are the same but do
not present as the same, and the version identity is exactly the field they would use to
confirm they are looking at the right pair.

The maintenance cost is the second reason. One rule with six implementations means a change
to how versions are named has six places to land and no gate that notices if it lands in five.

What to do

The three changes are separable but touch the same call sites, so doing them together avoids
re-editing the same six places three times.

1. One resolver. Extend src/deltatrack/version_stems.py with a single function that
   takes a filename and returns both the label and the ordinal, sanitized so it is safe for an
   uploaded name (stripping path components, which _label_from_filename does today and
   label_from_stem does not). Every surface calls it, and _label_from_filename is deleted.
2. Pass the ordinals from diff_pdf.py, doing what render_pdf_diff_html's docstring
   already describes.
3. Add compare_pdf_files_html to src/deltatrack/compare/pdf.py, mirroring
   compare_xml_files_html, so render_examples.py and diff_pdf.py both drop their
   hand-rolled derivation rather than being fixed to hand-roll it the same way.

One question is genuinely open and should be decided rather than assumed: whether an upload
should derive ordinals at all. Deriving them means someone who uploads the numbered corpus
filenames gets the heading the published example shows, which is the invariant
compare_xml_files_html claims. Not deriving them means the heading never asserts a
legislative ordinal for a file whose provenance the tool cannot check, since a user can name
an upload anything. The first is more consistent; the second is more honest about what is
known. The current behaviour is the second by accident rather than by decision.

Verification

Rendering the committed examples and comparing is not sufficient on its own, because
render_examples.py is already the surface that behaves correctly, so a fix that changed
nothing elsewhere would still leave them passing.

What shows the fix landed: the same fixture pair rendered through diff_pdf.main(), through
/api/compare, and through render_examples.py produces the same heading and the same
versions block. Reverting change 2 alone should make that check fail, which is worth
confirming before trusting it.

Unverified

Whether any consumer outside this repository reads the versions block and depends on the
current, surface-dependent values. It is part of the published canonical contract
(schema/canonical-diff.md), so a conforming consumer could be reading it; none was checked.

Part of #691 (the epic making every surface produce the same answer). Related to
#606 (the documented snippet for running the PDF comparison locally fails on its first line),
which is the same documentation drift in the web-compare guide.

Refs #691, #606

[willhea]

Adding the security label per the convention in AGENTS.md that anything touching the public upload surface gets one, and flagging the specific hazard so it is not discovered during implementation.

The function this issue proposes to delete, web/app.py::_label_from_filename, is doing sanitisation as well as labelling. It strips path components from a filename supplied by an anonymous uploader:

stem = name.rsplit("/", 1)[-1].rsplit("\\\\", 1)[-1]

deltatrack.version_stems.label_from_stem, the function it would be consolidated onto, does not do this. It operates on a stem that a caller has already derived from a trusted path.

So the consolidation must not be a straight substitution. The shared resolver has to keep the path-stripping for the upload caller, and the value ends up in the report heading and in the embedded canonical document, both of which are rendered. Whatever replaces it should be tested with a filename containing path separators, and with one containing markup, rather than only with the well-formed corpus names the other five call sites see.

There is no known vulnerability here today: the current implementation strips the separators, and the renderer escapes. This is a note that a defence exists at the site being refactored, so that it is carried across deliberately rather than dropped by a consolidation whose stated goal is removing duplicate implementations.

[willhea]

Recording a considered position on the open question in the body above, so it is not re-derived from scratch. This is a recommendation, not a decision; it needs a maintainer's call before implementation starts.

The question. Someone uploads 1_reported-in-house.pdf and 2_engrossed-in-house.pdf. Does the report heading read v1: reported-in-house → v2: engrossed-in-house, matching the command line and the published example, or reported-in-house → engrossed-in-house?

Recommendation: derive both the label and the ordinal, one rule on every surface.

The argument for holding the ordinal back is that the tool cannot verify an uploaded file's provenance, so printing a legislative ordinal asserts knowledge it does not have. That argument is weaker than it first appears, because the label is already taken from the same untrusted filename. Trusting it for the ordinal is not a larger leap than trusting it for the version name.

Deriving both buys three things: one rule with no upload special case, all three surfaces finally identical, and the existing claim in compare_xml_files_html's docstring becomes true rather than needing correction. A file uploaded out of order produces a heading reading v3 → v1, which is informative rather than wrong.

The alternative on record: if the preference is that the tool never prints an ordinal it cannot check, uploads get the label only. Two surfaces then agree and the third deliberately differs, and the docstring plus the README section are corrected rather than satisfied. Defensible, but it leaves a difference that has to be explained wherever it is noticed.

Either way the consolidation must preserve the path-component stripping described in the earlier comment on this issue.