From f6bb06ac6e150a967d00ed125df002059a6c4874 Mon Sep 17 00:00:00 2001 From: patrick Date: Sat, 15 Aug 2026 21:57:01 -0400 Subject: [PATCH] fix: merge identifiers across a book's formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Identifiers are a collection, but the whole dict was replaced per format, so the last file to report won outright — an EPUB declaring an ASIN, a Google id and a Calibre id kept none of them once a PDF contributed one ISBN. Accumulate instead, letting a declared identifier outrank one scraped off a page. --- .../src/chitai/services/metadata_extractor.py | 25 +++++++- backend/tests/unit/test_metadata_extractor.py | 61 ++++++++++++++++++- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/backend/src/chitai/services/metadata_extractor.py b/backend/src/chitai/services/metadata_extractor.py index 409b6da..e757d91 100644 --- a/backend/src/chitai/services/metadata_extractor.py +++ b/backend/src/chitai/services/metadata_extractor.py @@ -124,15 +124,36 @@ class Extractor: # EPUB tends to give better metadata results over pdf sorted_files = sorted(files, key=lambda f: Extractor._get_file_priority(f)) + # Identifiers accumulate across formats instead of replacing each other. Every + # other field is a single value where the later, better-trusted format simply + # wins, but identifiers are a *collection*: a book holding an EPUB and a PDF + # genuinely carries what both of them declare, and merging the dict wholesale + # threw away everything the earlier format found. An EPUB that declares an + # ASIN, a Google volume id and a Calibre id kept none of them once a PDF + # contributed a single ISBN. + identifiers: dict[str, str] = {} + for file in sorted_files: match get_file_extension(file): case "epub": - metadata = metadata | await EpubExtractor.extract_metadata(file) + extracted = await EpubExtractor.extract_metadata(file) case "pdf": - metadata = metadata | await PdfExtractor.extract_metadata(file) + extracted = await PdfExtractor.extract_metadata(file) case _: break + # First writer wins per name, and the files are already ordered by how + # far their metadata can be trusted. A `dc:identifier` the publisher + # declared outranks an ISBN scraped out of a PDF's copyright page, which + # routinely prints the ISBNs of other formats and older editions too. + for name, value in (extracted.pop("identifiers", None) or {}).items(): + identifiers.setdefault(name, value) + + metadata = metadata | extracted + + if identifiers: + metadata["identifiers"] = identifiers + # Get metadata from file names for file in files: metadata = FilenameExtractor.extract_metadata(file) | metadata diff --git a/backend/tests/unit/test_metadata_extractor.py b/backend/tests/unit/test_metadata_extractor.py index d8335cc..35f17d6 100644 --- a/backend/tests/unit/test_metadata_extractor.py +++ b/backend/tests/unit/test_metadata_extractor.py @@ -1,7 +1,7 @@ import pytest from pathlib import Path from datetime import date -from chitai.services.metadata_extractor import EpubExtractor +from chitai.services.metadata_extractor import EpubExtractor, Extractor, PdfExtractor @pytest.mark.asyncio() @@ -15,3 +15,62 @@ class TestEpubExtractor: assert metadata["authors"] == ["Herman Melville"] assert metadata["published_date"] == date(year=2001, month=7, day=1) + + +EPUB = Path("tests/data_files/Metamorphosis - Franz Kafka.epub") +PDF = Path("tests/data_files/Calculus Made Easy - Silvanus Thompson.pdf") + + +@pytest.mark.asyncio() +class TestIdentifierMerging: + """A book's formats each contribute identifiers; none of them replaces the rest.""" + + async def test_every_format_contributes(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + Identifiers are a collection, not a single value. + + Merging the whole dict meant the last format to report won outright: an EPUB + declaring an ASIN, a Google volume id and a Calibre id kept none of them once + a PDF contributed one ISBN. + """ + + async def epub(_file): + return { + "title": "How Linux Works", + "identifiers": {"isbn-13": "9781718500419", "asin": "1718500408"}, + } + + async def pdf(_file): + return {"identifiers": {"isbn-13": "9781718500402", "isbn-10": "1593270356"}} + + monkeypatch.setattr(EpubExtractor, "extract_metadata", epub) + monkeypatch.setattr(PdfExtractor, "extract_metadata", pdf) + + metadata = await Extractor.extract_metadata( + [Path("How Linux Works.epub"), Path("How Linux Works.pdf")] + ) + + assert metadata["identifiers"] == { + # Declared by the publisher's toolchain, so it outranks the PDF's, which + # was scraped off a copyright page that also prints the print edition's. + "isbn-13": "9781718500419", + "asin": "1718500408", + "isbn-10": "1593270356", + } + + async def test_a_second_format_that_finds_nothing_erases_nothing(self) -> None: + """The PDF fixture carries no ISBN, so it must leave the EPUB's alone.""" + metadata = await Extractor.extract_metadata([EPUB, PDF]) + + assert metadata["identifiers"] == {"id": "http://www.gutenberg.org/5200"} + + async def test_one_format_on_its_own_is_unaffected(self) -> None: + metadata = await Extractor.extract_metadata([EPUB]) + + assert metadata["identifiers"] == {"id": "http://www.gutenberg.org/5200"} + + async def test_no_identifiers_anywhere_leaves_the_field_absent(self) -> None: + """An empty dict would count as extracted metadata and overwrite nothing.""" + metadata = await Extractor.extract_metadata([PDF]) + + assert "identifiers" not in metadata