Fix PDF compatibility ligature matching while preserving scientific distinctions

[cmungall]

Summary

PDF-extracted amyloid fibrils now matches amyloid fibrils through the existing public normalize_text method. An explicit table folds ff/fi/fl/ffi/ffl/ſt/st and compatibility IJ on both content and queries, including split excerpts and title validation.

Æ/æ and Œ/œ remain distinct letters. Broad NFKC is deliberately avoided so scientific subscripts, superscripts, circled digits, and the micro sign do not acquire new validation equivalences. Existing Greek spelling, punctuation, and case handling are preserved. This is the explicit Latin ligature set requested in #73, not a rule to fold every Unicode compatibility character.

Closes #73.

Validation

• TDD: 14 failures before implementation; all 36 final regression cases pass.
• Real PLOS fixture excerpt with simulated PDF typography on content, query, or both; contiguous and split excerpts; negative checks for absent parts and scientific/AE/OE distinctions.
• Cache-backed tests cover both title-validation entry points in both typography directions and reject partial titles.
• just doctest: 202 passed.
• just mypy: passed (93 files); just format: passed.
• Final local just test: 980 passed, mypy and Ruff passed on 192a03e. An earlier run had 977 passed and 1 external-service failure. test_validate_title_partial_match_real_geo could not fetch GEO:GSE67472 because Entrez esearch returned HTTP 429 Too Many Requests. The unchanged test passed on retry (1 passed in 1.81s). No assertions or test selection were weakened.
• Coordinator independently combined normalize_text does not fold ligatures, so PDF-extracted text often cannot be quoted #73 with Make _split_query public: downstream snippet reporting has to reimplement it #74: 56 matching tests, mypy, and Ruff passed before the two additional title tests.
• Final Python 3.10, 3.11, 3.12, and 3.13 CI: all passed on 192a03e; automatic review passed with no blocking findings.

Automatic review disposition

All three automatic reviews, including the final-head review, found no blockers. Added the suggested title coverage. Deferred Greek/table performance refactoring, fuzzy-suggestion normalization, repair-path expansion, and soft-hyphen/zero-width/standalone-long-s handling: these are pre-existing behavior or separate matching-policy changes outside this ligature fix. The micro-sign negative test deliberately pins existing behavior for this bounded change; it does not preclude a separately reviewed policy change later.

One review claimed NFKC leaves long-s unchanged and maps ſt to ſt; direct Python unicodedata.normalize checks instead give NFKC("ſt") == "st" and NFKC("ſ") == "s". No change was warranted on that basis. The explicit table and documentation enumerate supported characters without claiming coverage of all Unicode compatibility digraphs.

[claude]

Claude finished @cmungall's task in 3m 33s —— View job

---

Code review: PDF ligature matching

• Read PR diff (git diff origin/main...HEAD)
• Review normalize_text change in context
• Trace all matching paths that consume normalization
• Assess test coverage
• Attempt to run test suite — ⚠️ blocked (see note)
• Post review

---

Overall

Good, tightly-scoped fix. The key design decision — an explicit ligature table instead of blanket NFKC — is the right call and is well justified in both the docstring and the how-to doc. Folding at the single choke point (normalize_text) means every consumer benefits: _substring_match normalizes content and each ...-split part (supporting_text_validator.py:527-531), validate_title (:158, :271), and both repairer strategies (repairer.py:365-366, :397-398). Coverage of the Latin block is complete (U+FB00–FB06 plus U+0132/U+0133), and the negative tests for Æ/Œ and scientific notation are a genuinely useful guard against someone "simplifying" this to unicodedata.normalize("NFKC", ...) later.

Comments below, roughly by significance. Nothing here is a blocker.

---

1. The suggestion path still doesn't fold ligatures (medium)

normalize_text is folded, but the fuzzy/suggestion path is not. generate_suggested_fix (supporting_text_validator.py:589) passes raw content, and fuzzy_text_utils.normalize_whitespace only collapses whitespace and lowercases (fuzzy_text_utils.py:54).

This bites concretely in get_significant_words (fuzzy_text_utils.py:80), which tokenizes with re.findall(r'\b[a-z0-9]+\b', normalized). fi is a word character but is outside [a-z0-9], so in confirmed there is no word boundary either side of the ligature — neither con nor rmed can match, and the token is dropped entirely. So for an excerpt that fails validation for some other reason and also contains ligatures, word overlap is computed on a mangled token set.

Two user-visible consequences:

• similarity_score reported on failure is similarity / 100.0 from this path (:542), so failing ligature excerpts get a misleadingly low score.
• suggested_fix / best_match quality degrades exactly where PDF-extracted text is involved.

Suggestion: extract the table into something like fold_ligatures(text) and call it from both normalize_text and normalize_whitespace, so the matching and the explain-the-failure paths agree on what a character is. Fix this →

2. The µ vs μ case pins behavior that arguably contradicts the PR's own goal (medium, judgment call)

The Æ/æ and Œ/œ reasoning is sound — those are orthographic choices, and equating them would accept a spelling change. But µ (U+00B5 MICRO SIGN) vs μ (U+03BC GREEK SMALL MU) is not that. Which one appears in a document is an artifact of the source font/encoding — the same failure mode as fi vs fi, which is what #73 is about.

There's also an existing asymmetry that this makes worse: the Greek map spells out μ → mu, while µ falls through untouched. So 5 μM normalizes to 5 mum and 5 µM to 5 µm — a guaranteed false failure whenever the publisher HTML and the curator's quote picked different codepoints for the same glyph.

The PR now locks that in via ("µ", "μ") in test_distinct_letters_and_scientific_symbols, so a future fix has to delete a test that reads like an intentional invariant. I'd either drop that one parametrize case, or keep it with a comment saying it records current behavior pending a decision, and open a follow-up issue. The superscript/subscript/circled-digit cases in the same parametrize list are unambiguously correct and should stay.

3. Rebuild the translation table once (minor)

str.maketrans({...}) is constructed on every call (supporting_text_validator.py:658). normalize_text is called against full article content on every excerpt check, and repairer.py:397-398 re-normalizes the entire reference content inside the per-part loop. The added translate pass is cheap next to the 24 sequential str.replace calls for Greek, so this is style more than throughput — but a module-level constant (alongside a module-level greek_map, if you want) is the idiomatic form and costs nothing.

4. Adjacent PDF artifacts, same bug family (minor / follow-up)

Worth a follow-up issue rather than scope creep here, but extractors that emit fi also emit:

• Soft hyphen U+00AD and zero-width space/joiner U+200B/200C/200D — none are \w or \s, so re.sub(r"[^\w\s]", " ", text) turns them into a space, splitting de<SHY>fined into de fined and breaking the match.
• Long s ſ U+017F — note the internal inconsistency that ſt (long s + t) folds to st, but a bare ſ does not fold to s.

The soft-hyphen one is the most likely to show up in real PLOS/PMC PDF text.

5. Test notes (minor)

• test_real_excerpt_with_ligatures with ligature_side="both" is close to a tautology — both sides carry fi, so it passes even if the fold were a no-op. The "content" and "query" cases are the ones doing real work. Harmless, just don't count it as three independent cases.
• The document-level test only exercises fi; ff/ffi/ffl/ſt/st/IJ are covered only at unit level. That's a reasonable split.
• assert normalize(normalize(x)) == expanded (idempotency) is trivially true given the fold has no ligature outputs — fine to keep as a regression guard, but it isn't testing much.
• Nice touch asserting not found for the appended ... nonexistent conclusion part; that's the check that keeps the fold from loosening the split-excerpt gate.

6. Behavioral note (not a defect)

Because validation now accepts fi, repair will no longer flag those quotes, so ligature characters will persist verbatim in curated data. That's the correct call for a validator, but if you'd rather curated text be clean ASCII, a low-risk CHARACTER_NORMALIZATION repair action would be the place for it.

---

Quality / standards

Follows CLAUDE.md: doctests added and they double as documentation, no try/except around deterministic code, test docstrings present, docs updated in the same PR. No security surface — str.translate on a fixed table, no new external input handling. Types are unchanged and mypy-clean by inspection (text: str in, str out).

⚠️ I could not run the test suite — uv run pytest, just pytest, ruff, and python3 all require approval in this environment, so the findings above are from static reading only. Please confirm against CI / the just test + mypy results the PR description says were still running.
• Branch: cmungall/p2-73-ligatures

[claude]

Claude finished @cmungall's task in 2m 58s —— View job

---

Review: PDF ligature matching (#81)

• Read PR diff against origin/main
• Review normalize_text implementation and call sites
• Check interaction with repairer / fuzzy-suggestion paths
• Assess test coverage
• Post review

---

Verdict

Solid, well-scoped change. The core decision — an explicit fold table instead of blanket NFKC — is the right one for this codebase, and the reasoning is documented where a future reader will actually hit it. Matching is purely normalized-substring (_substring_match, supporting_text_validator.py:527-533), with no offsets mapped back into the original string, so the length-changing expansions (ffi → ffi) are safe. The NFKC avoidance is also consistent with the existing division of labour: subscript equivalence (H2O → H₂O) is deliberately a repair action via RepairConfig.character_mappings (repairer.py:116-138), not a validation equivalence — so the new negative tests reinforce an existing contract rather than inventing one.

Tests follow CLAUDE.md properly: real PLOS fixture, no mocks, ligatures injected on content/query/both × contiguous/split, negatives in both directions, plus an idempotency assertion (normalize(normalize(x)) == normalize(x)) that is easy to forget and genuinely valuable here.

Comments below are non-blocking.

---

1. normalize_text is now 25 sequential passes over full-document text (performance)

supporting_text_validator.py:658-694 rebuilds both tables on every call and then runs 24 str.replace() passes for Greek plus one translate:

ligature_map = {...}                      # rebuilt per call
text = text.translate({ord(l): e ...})    # rebuilt per call
for greek, spelled in greek_map.items():  # 24 full-string passes
    text = text.replace(greek, spelled)

This function is called on entire reference documents, and repairer.py:397-398 re-normalizes the whole reference content once per query part inside a loop. str.translate supports multi-character replacements, so the Greek map can fold into the same table and the whole thing becomes one pass over a table built once at import:

_NORMALIZATION_TABLE = str.maketrans({**_LIGATURES, **_GREEK})

Pre-existing for Greek, but this PR is the natural moment to fix it. Fix this →

2. The stated rationale doesn't quite match the table

The docs justify inclusions by compatibility decomposition ("IJ is included because Unicode gives it a compatibility decomposition into I and J"). Two places where the table doesn't follow that rule:

• ſt → st is more aggressive than NFKC. U+FB05 decomposes to ſt (long s + t), and NFKC does not map ſ → s — that only happens under NFKC_Casefold. The behavior is right for matching PDF text, but the principle as written doesn't produce it. A consequence: ſtudy now matches study, while a bare long s (ſtudy) still does not.
• Conversely, the digraphs DŽ/Dž/dž, LJ/Lj/lj, NJ/Nj/nj (U+01C4–U+01CC) have exactly the same kind of compatibility decomposition as IJ and are omitted. Correct call practically (they're irrelevant to biomedical PDFs) — but then the honest rationale is "these are what PDF extractors emit," not "these are what Unicode decomposes."

Worth one sentence in the docstring and docs/how-to/repair-validation-errors.md so the table's boundary is defensible on its actual grounds.

3. Soft hyphens and zero-width characters are the same bug, still unfixed (follow-up)

supporting_text_validator.py:697 maps anything that is neither \w nor \s to a space. U+00AD (soft hyphen), U+200B/200C/200D, and U+FEFF are category Cf — not word characters — so they become spaces inside words:

"fibrils"  →  "fi brils"   ≠  "fibrils"

These are at least as common as ligatures in PDF- and HTML-derived text (soft hyphens survive copy-paste from hyphenated line breaks; zero-width chars come from HTML wrapping hints). Same failure mode, same class of fix, and the new test file is the natural home for it. Suggest a follow-up issue rather than growing this PR. Fix this →

4. Test coverage gaps (small)

• normalize_text also backs title validation (supporting_text_validator.py:158-159, :271), which is exact-match-after-normalization rather than substring. Ligature folding now silently changes title comparison too. One case pinning IJ/fi in a title would lock that in.
• No test covers the repairer interaction. Previously a ligature mismatch could reach attempt_repair; now it validates upstream. A case asserting that a ligature-only difference yields was_valid/no CHARACTER_NORMALIZATION action would document the new division of responsibility between the two layers.
• generate_suggested_fix → find_fuzzy_match_in_text operates on raw text, so when a quote fails for an unrelated reason and the content has ligatures, suggestion similarity is still depressed. Not a validation bug — worth a note, or normalizing in the fuzzy path too.

5. Nits

• supporting_text_validator.py:662 is ~108 chars (the rest of the file sits well under that) and the comprehension is just str.maketrans(ligature_map).
• ligature_map uses double quotes and packed multi-pair lines; the adjacent greek_map uses single quotes, one pair per line. ruff check passes either way (E501 isn't in the default rule set), but matching the neighbour reads better.

---

Note on verification

I could not execute just test / uv run pytest in this run — the sandbox declined those commands, so I reviewed statically. The PR body reports 202 doctests passing and ruff check clean; please confirm the full just test and mypy results before merge, as the description says is still pending.
• Branch: cmungall/p2-73-ligatures

[claude]

Claude finished @cmungall's task in 3m 26s —— View job

---

Review: PDF ligature matching (#81)

• Read PR diff against origin/main (3 commits)
• Review normalize_text and all 6 call sites
• Check title-validation and repairer interactions
• Assess test coverage
• Attempt to run test suite — ⚠️ blocked (see note at bottom)
• Post review

---

Verdict

Good change, and the two new commits since the last round improved it. The central design call — an explicit fold table rather than blanket NFKC — is the right one for a validator, where every new equivalence is a new class of quote you silently accept. The rationale is written down where someone about to "simplify" this to unicodedata.normalize("NFKC", ...) will actually read it, and the negative tests back it up.

I traced every consumer of normalize_text to check the length-changing expansions (ffi → ffi) are safe:

Call site	Use
supporting_text_validator.py:158-159, :271	title equality after normalization
:527, :531	_substring_match — content and each ... part
repairer.py:365-366, :397-398	both repair strategies

All six are equality/containment on the normalized string; none maps an index back into the original text, so expansion changing string length is safe. Folding at the single choke point means all of them benefit at once.

test_cached_title_ligatures (192a03e) closes the title-path gap from the prior round, and it's network-safe for a non-obvious reason worth noting — see point 5.

Nothing below is a blocker.

---

1. The suggestion path still doesn't fold, and ligatures delete whole words there (medium)

_substring_match normalizes for the match, but on failure hands raw content to generate_suggested_fix (supporting_text_validator.py:537-539), which goes through fuzzy_text_utils.normalize_whitespace — whitespace + lowercase only (fuzzy_text_utils.py:54).

The bite is in get_significant_words (fuzzy_text_utils.py:80):

words = re.findall(r'\b[a-z0-9]+\b', normalized)

fi is a word character but is outside [a-z0-9], so in confirmed there is no \b on either side of it — con can't match (next char is a word char) and rmed can't match (previous char is a word char). The token vanishes entirely rather than degrading.

Two user-visible effects, both on the failure path:

• similarity_score returned to the caller is similarity / 100.0 from this path (:542), so an excerpt that fails for an unrelated reason and happens to contain ligatures reports a misleadingly low score.
• suggested_fix / best_match degrade exactly where PDF-derived text is involved — i.e. the population this PR is about.

Pre-existing, but this PR is what makes the matching layer and the explain-the-failure layer disagree about what a character is. Extracting a shared fold_ligatures() and calling it from both would keep them aligned. Fix this →

2. µ vs μ is now pinned as an invariant, and it's the same bug class as fi (medium, judgment call)

The Æ/Œ reasoning is sound — those are orthographic choices and equating them accepts a spelling change. But U+00B5 MICRO SIGN vs U+03BC GREEK SMALL MU is not that: which codepoint a document carries is an artifact of the source font/encoding, exactly like fi vs fi.

There is also a pre-existing asymmetry that this now freezes. greek_map spells out μ → mu (:677) while µ falls straight through to the \w-preserving substitution, so:

"5 μM"  →  "5 mum"
"5 µM"  →  "5 µm"

A guaranteed false failure whenever the publisher HTML and the curator's quote picked different codepoints for the same glyph — plausibly common in concentration units.

("µ", "μ") in test_distinct_letters_and_scientific_symbols and the sentence in docs/how-to/repair-validation-errors.md:29 now read as a deliberate invariant, so fixing it later means deleting a test that looks intentional. I'd either drop that parametrize case and the doc clause, or keep them with a comment saying they record current behaviour pending #73 follow-up. The superscript / subscript / circled-digit cases beside it are unambiguously right and should stay.

3. normalize_text is 25 passes over full-document text, with both tables rebuilt per call (minor, perf)

:658-662 builds the dict and the translation table on every invocation, then :693-694 runs 24 separate str.replace() passes for Greek. str.translate handles multi-character replacements, so the whole thing collapses to one pass over a table built once at import:

_NORMALIZATION_TABLE = str.maketrans({**_LIGATURES, **_GREEK})

The Greek part is pre-existing, but repairer.py:397-398 makes it matter: content_norm is loop-invariant yet the entire reference content is re-normalized once per query part. Hoisting that one line out of the loop is the bigger win and is independent of the table change. Fix this →

4. The stated rationale doesn't quite produce the table you shipped (minor, docs)

Both the docstring (:627-628) and the doc justify membership by compatibility decomposition. Two places that rule doesn't hold:

• ſt → st is more aggressive than NFKC. U+FB05 decomposes to ſt (long s + t); ſ → s only happens under NFKC_Casefold. The behaviour is right for PDF text, but the principle as written doesn't yield it — and it leaves an internal oddity where ſtudy now matches study while a bare ſtudy still doesn't.
• Conversely DŽ/Dž/dž, LJ/Lj/lj, NJ/Nj/nj (U+01C4–U+01CC) have the same kind of decomposition as IJ and are omitted — correct practically, since no biomedical PDF emits them.

The honest and more defensible rule is "these are what PDF extractors emit," not "these are what Unicode decomposes." One sentence in each place.

5. Test notes (minor)

• test_cached_title_ligatures is network-safe only by accident. ReferenceContent defaults content_type="unknown" (models.py:781), which is not in NEEDS_FULL_TEXT_TYPES (reference_fetcher.py:35-41), so _maybe_retry_full_text short-circuits and fetch never reaches a source. If "unknown" were ever added to that set, this test would start making live calls in CI. Passing content_type="abstract_only"… no — passing an explicit non-refetching type, or asserting it, makes the guarantee visible rather than incidental.
• test_real_excerpt_with_ligatures with ligature_side="both" is close to a tautology: both sides carry fi, so it passes even if the fold were a no-op. "content" and "query" are the two doing real work — 4 real cases, not 6.
• assert normalize(normalize(x)) == expanded is trivially true given no fold output contains a ligature. Fine as a regression guard, just not load-bearing.
• Good call asserting not found for the appended ... nonexistent conclusion part — that's the check that stops the fold from loosening the split-excerpt gate.

6. Soft hyphens and zero-width chars — same bug family, follow-up (not this PR)

:697 maps anything that is neither \w nor \s to a space. U+00AD (soft hyphen), U+200B/200C/200D and U+FEFF are category Cf — so defined normalizes to de fined and fails to match defined. Soft hyphens survive copy-paste from hyphenated line breaks in exactly the PLOS/PMC PDFs this PR targets. Pre-existing, same class of fix, and tests/test_ligature_normalization.py is the natural home — but a separate issue rather than scope creep here.

7. Nits

• :662 is ~108 chars where the rest of the file sits well under that, and the comprehension is exactly str.maketrans(ligature_map) (typeshed types that as dict[int, str], so it stays mypy-clean — the explicit ord() form isn't buying type safety over it).
• ligature_map uses double quotes and packed pairs; the adjacent greek_map uses single quotes, one pair per line. ruff check accepts both (E501 isn't in the default rule set and just format doesn't run ruff format), but matching the neighbour reads better.
• IJ → "IJ" vs ij → "ij" is a distinction without a difference given the later .lower() — harmless, and it does keep the table readable as a decomposition.

8. Behavioural note (not a defect)

Ligature quotes now validate, so repair will no longer flag them and those characters persist verbatim in curated data. Correct for a validator. If clean ASCII in curated files is wanted, RepairConfig.character_mappings (models.py:325-335) is the right home for it, not normalize_text.

---

Standards

Follows CLAUDE.md: doctests added and doubling as docs, no try/except around deterministic code, real PLOS fixture instead of mocks, test docstrings present, docs updated in the same PR. No security surface — str.translate over a fixed table, no new external input handling. Types unchanged (str → str).

⚠️ I could not execute the test suite in this run — uv run pytest, just pytest, ruff and python3 all require approval in this sandbox, so everything above is static analysis. The PR body reports 202 doctests and ruff check clean with just test + mypy still running; please confirm those against CI before merge.
• Branch: cmungall/p2-73-ligatures