From 7247f781664d0e5ad06125601e06fde84bd6a2b3 Mon Sep 17 00:00:00 2001 From: Mancolt <38768585+Mancolt@users.noreply.github.com> Date: Tue, 6 Oct 2026 22:55:37 -0400 Subject: [PATCH] contributors: an empty composer/artist tag no longer sends narrator "" to Audible An empty or blank composer tag became Contributor(""), getList enclosed it as `""`, and with add_narrators on Audible was searched with narrator=`""` and returned nothing (Before She Knew Him, Peter Swanson, 0062838180). Names that are empty once cleansed are now never turned into contributors (id3 tags, run-log rows, hints, parsed release names, Audible and MAM answers), and getList skips any that remain. MAMBook.ffprobe (no caller) appended a nameless Contributor(); it now builds named authors. run_replay.sh names its container. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_017Ut2SJwyL9F8zUJHeM2GLP --- CHANGELOG.md | 18 +++++ myx_audible.py | 6 +- myx_classes.py | 51 +++++++------ myx_mam.py | 6 +- myx_utilities.py | 13 ++-- tests/replay/run_replay.sh | 4 +- tests/test_contributors.py | 148 +++++++++++++++++++++++++++++++++++++ 7 files changed, 210 insertions(+), 36 deletions(-) create mode 100644 tests/test_contributors.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4097689..d8a9229 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ first release is 3.0.0 because exit codes and the cookie store changed in ways a ## Unreleased +## 3.0.5 - 2026-10-06 + +### Fixed +- A file whose `composer` (narrator) tag was present but empty was searched on Audible with narrator `""` + (with `add_narrators` on) and Audible answered with nothing. Seen 2026-10-06 with *Before She Knew Him* + (Peter Swanson): the same search without the `""` returns `0062838180` first. An empty or blank tag, or a + stray comma (`A, , B`) in `artist`/`composer`, no longer creates a nameless author or narrator, and a name + that is empty once cleansed is never sent to Audible or written to the run log (`A,,B` becomes `A,B`). Empty + names in Audible and MAM answers are dropped too, so an empty tag can no longer pass the author check + against an empty result author. Where a book is filed from its own tags, a blank `artist` tag now gives + the `Unknown` author folder, as a missing tag already did, instead of an empty one. Side effects worth + knowing: a series with an empty name (an empty `album` tag next to `SERIES`/`PART`) no longer leaves a + trailing comma in the `*-series` columns (`Saga,` becomes `Saga`, matching `seriesparts`); for a blank + `artist` tag the MAM search drops its empty `("")` author group, so its cache key changes and the search is + repeated once; the JSON run log lists such authors/narrators as `[]` instead of `[""]`. +- `MAMBook.ffprobe` appended an author without a name (upstream; it raised `TypeError`). The method is not + called by booktree, but it now builds named authors like the live parser. + ## 3.0.4 - 2026-09-22 ### Fixed diff --git a/myx_audible.py b/myx_audible.py index e6cba44..87e5970 100644 --- a/myx_audible.py +++ b/myx_audible.py @@ -133,11 +133,9 @@ def product2Book(product): if 'publisher_summary' in product: book.description=str(product["publisher_summary"]) if 'runtime_length_min' in product: book.length=product["runtime_length_min"] if 'authors' in product: - for author in product["authors"]: - book.authors.append(myx_classes.Contributor(str(author["name"]))) + book.authors.extend(myx_classes.contributors(str(author["name"]) for author in product["authors"])) if 'narrators' in product: - for narrator in product["narrators"]: - book.narrators.append(myx_classes.Contributor(str(narrator["name"]))) + book.narrators.extend(myx_classes.contributors(str(narrator["name"]) for narrator in product["narrators"])) if 'publisher_name' in product: book.publisher=str(product["publisher_name"]) if 'publication_datetime' in product: book.publishYear=str(product["publication_datetime"]) if 'issue_date' in product: book.releaseDate=str(product["issue_date"]) diff --git a/myx_classes.py b/myx_classes.py index f3fae2f..5e2d4da 100644 --- a/myx_classes.py +++ b/myx_classes.py @@ -27,6 +27,21 @@ class Contributor: name:str #books:list[int]= field(default_factory=list) + +def contributors(names, clean=None): + """Contributors for the given names, skipping any that are empty once cleansed. An empty `composer` tag, a + blank `artist` or a stray comma ("A, , B") used to become Contributor(""); getList then emitted `""` and + Audible answered a search with narrator=`""` with nothing. `clean` is applied to each name first (removeGA + for authors, say); the kept name is otherwise left as it was.""" + people = [] + for name in names: + name = "" if name is None else str(name) + if clean is not None: + name = clean(name) + if myx_utilities.cleanseAuthor(name).strip(): + people.append(Contributor(name)) + return people + #target_path template for a book in a series whose part is unknown (Config/target_path/in_series_no_part) IN_SERIES_NO_PART = "{author}/{series}/{series} - {title}" @@ -132,14 +147,12 @@ def getSeriesParts(self, delimiter=",", encloser="", stripaccents=True): def setAuthors(self, authors): #Given a csv of authors, convert it to a list if len(authors.strip()): - for author in authors.split (","): - self.authors.append(Contributor(author)) + self.authors.extend(contributors(authors.split(","))) def setNarrators(self, narrators): #Given a csv of authors, convert it to a list if len(narrators.strip()): - for narrator in narrators.split (","): - self.narrators.append(Contributor(narrator)) + self.narrators.extend(contributors(narrators.split(","))) def setSeries(self, series): #Given a csv of authors, convert it to a list @@ -279,13 +292,11 @@ def ffprobe(self, parent): if 'artist' in metadata: #remove everything in parentheses firstm before parsing artist = re.sub(r"\(.+\)", "", metadata["artist"], flags=re.IGNORECASE) - for author in artist.split(","): - book.authors.append(Contributor(myx_utilities.removeGA(author))) + book.authors.extend(contributors(artist.split(","), myx_utilities.removeGA)) #parse narrators if 'composer' in metadata: composer = re.sub(r"\(.+\)", "", metadata["composer"], flags=re.IGNORECASE) - for narrator in composer.split(","): - book.narrators.append(Contributor(narrator)) + book.narrators.extend(contributors(composer.split(","))) #duration in minutes book.duration = duration @@ -537,7 +548,7 @@ def applyParsedName(self, book, cfg): if parsed["authors"] and myx_names.isJunkAuthors(book.authors): if searchBook is book: searchBook = copy.copy(searchBook) - searchBook.authors = [Contributor(a) for a in parsed["authors"]] + searchBook.authors = contributors(parsed["authors"]) applied.append("authors") if parsed["asin"] and not (book.asin or "").strip() and myx_names.isJunkTitle(book.title, relName): # filename ASINs are only for untagged/junk-title files. A usable id3 title must not be @@ -563,7 +574,7 @@ def searchBookFromHint(self, book): if self.hint.get("title"): searchBook.title = self.hint["title"] if self.hint.get("authors"): - searchBook.authors = [Contributor(a) for a in self.hint["authors"]] + searchBook.authors = contributors(self.hint["authors"]) return searchBook def acceptPinnedAsin(self, client, cfg, book, language): @@ -636,18 +647,14 @@ def ffprobe(self, file): #parse authors if 'artist' in metadata: #remove everything in parentheses firstm before parsing - artist = metadata["artist"] - for author in re.split(",", artist): - author = re.sub(r"\([.]+\)", "", author, flags=re.IGNORECASE) - author = myx_utilities.removeGA(author) - if len(author): book.authors.append(Contributor()) + #(upstream appended a nameless Contributor() here, which raises TypeError; this method has no caller) + book.authors.extend(contributors(re.split(",", metadata["artist"]), + lambda a: myx_utilities.removeGA(re.sub(r"\([^)]*\)", "", a)))) #parse narrators if 'composer' in metadata: - composer = metadata["composer"] - for narrator in re.split(",", composer): - #remove any occurrence of (Narrator) - narrator = re.sub(r"\([.]+\)", "", narrator, flags=re.IGNORECASE) - book.narrators.append(Contributor(narrator)) + #remove any occurrence of (Narrator) + book.narrators.extend(contributors(re.split(",", metadata["composer"]), + lambda n: re.sub(r"\([^)]*\)", "", n))) #return a book object created from ffprobe self.ffprobeBook=book @@ -863,12 +870,12 @@ def _audibleAttempts(self, book, cfg, searchAsin, noAsin=False): [a.name for a in parsedBook.authors], parsedAuthors): #the tags name someone else (often the narrator in the artist tag): try the release name's authors namedBook = copy.copy(parsedBook) - namedBook.authors = [Contributor(a) for a in parsedAuthors] + namedBook.authors = contributors(parsedAuthors) attempts.append(("parsed-authors", namedBook, "", True)) alt = self.parsedName.get("alternative") if self.parsedName else None if alt and not pAsin and "title" in parsedApplied: altBook = copy.copy(parsedBook) - altBook.title, altBook.authors = alt["title"], [Contributor(a) for a in alt["authors"]] + altBook.title, altBook.authors = alt["title"], contributors(alt["authors"]) attempts.append(("swapped", altBook, "", True)) if parsedBook.authors and not pAsin: soloBook = copy.copy(parsedBook) diff --git a/myx_mam.py b/myx_mam.py index 218010c..ab547ab 100644 --- a/myx_mam.py +++ b/myx_mam.py @@ -199,14 +199,12 @@ def getMAMBook(cfg, titleFilename="", authors="", extension="", refresh=False): #format {id:author, id:author} if len(b["author_info"]): authors = json.loads(b["author_info"]) - for author in authors.values(): - book.authors.append(myx_classes.Contributor(str(author))) + book.authors.extend(myx_classes.contributors(str(author) for author in authors.values())) if 'narrator_info'in b: #format {id:narrator, id:narrator} if ((not b["narrator_info"] is None) and len(b["narrator_info"])): narrators = json.loads(b["narrator_info"]) - for narrator in narrators.values(): - book.narrators.append(myx_classes.Contributor(str(narrator))) + book.narrators.extend(myx_classes.contributors(str(narrator) for narrator in narrators.values())) if 'series_info'in b: #format {"35598": ["Kat Dubois", "5"]} if ((not b["series_info"] is None) and len(b["series_info"])): diff --git a/myx_utilities.py b/myx_utilities.py index bbd0283..8f2b799 100644 --- a/myx_utilities.py +++ b/myx_utilities.py @@ -26,15 +26,18 @@ def probe_file(filename): #Utilities def getList(items, delimiter=",", encloser="", stripaccents=True): + #an item whose name is empty once cleansed is skipped: an enclosed empty name ('""') sent to Audible as + #narrator/author matches nothing, and an unenclosed one leaves "A,,B" in the run log enclosedItems=[] for item in items: if type(item) == myx_classes.Contributor: - enclosedItems.append(f"{encloser}{cleanseAuthor(item.name)}{encloser}") + name = cleanseAuthor(item.name) + elif type(item) == myx_classes.Series: + name = cleanseSeries(item.name) else: - if type(item) == myx_classes.Series: - enclosedItems.append(f"{encloser}{cleanseSeries(item.name)}{encloser}") - else: - enclosedItems.append(f"{encloser}{item.name}{encloser}") + name = str(item.name) + if name.strip(): + enclosedItems.append(f"{encloser}{name}{encloser}") return delimiter.join(enclosedItems) diff --git a/tests/replay/run_replay.sh b/tests/replay/run_replay.sh index ccd4269..fa6187d 100755 --- a/tests/replay/run_replay.sh +++ b/tests/replay/run_replay.sh @@ -22,7 +22,9 @@ done # BOOKTREE_REPLAY_LIVE=1: uncached Audible queries are fetched from the real API and cached into the staged copy CFG_MODE=ro; LIVE=() if [ "${BOOKTREE_REPLAY_LIVE:-0}" = "1" ]; then CFG_MODE=rw; LIVE=(--audible-live); fi -docker run --rm --user root -e PYTHONDONTWRITEBYTECODE=1 -w /booktree \ +# a named container (override with BOOKTREE_REPLAY_NAME) so a hung replay can be stopped by that exact name, +# never by image: production containers on the same host share base images +docker run --rm --name "${BOOKTREE_REPLAY_NAME:-review-replay-$$-$RANDOM}" --user root -e PYTHONDONTWRITEBYTECODE=1 -w /booktree \ -v "$CODE:/booktree:ro" -v "$REPO/tests/replay:/replay:ro" -v "$LOGS:/logs:ro" -v "$STAGE/Config:/Config:$CFG_MODE" -v "$OUT:/out" \ "$IMAGE" /venv/bin/python3 /replay/replay_corpus.py \ --config pass1=/Config/config.json --config pass2=/Config/config-audible.json --out /out "${LIVE[@]}" "$@" diff --git a/tests/test_contributors.py b/tests/test_contributors.py new file mode 100644 index 0000000..4eb218a --- /dev/null +++ b/tests/test_contributors.py @@ -0,0 +1,148 @@ +"""Empty contributor names: a blank composer/artist tag or a stray comma must not become Contributor("") and must +never reach Audible as narrator/author `""` (seen with Before She Knew Him, whose empty composer tag made the +catalog search return nothing).""" +import contextlib +import io +import json +import tempfile +import unittest +from unittest.mock import patch + +import myx_audible +import myx_classes +import myx_mam +import myx_utilities +from tests.support import FakeAudible, FakeConfig, product + + +def probed(tags, duration=600.0): + """BookFile.ffprobe over canned ffprobe output (no ffprobe binary, no file).""" + bf = myx_classes.BookFile("Release/file.m4b", "/src/Release/file.m4b", "/src", "/media") + with patch.object(myx_classes.BookFile, "__probe_file__", return_value={"format": {"duration": duration, "tags": tags}}): + return bf.ffprobe("Release") + + +def names(people): + return [p.name for p in people] + + +class ContributorsHelperTest(unittest.TestCase): + def test_names_empty_once_cleansed_are_dropped(self): + # cleanseAuthor turns "." into a space and drops "'", so these are empty names too + self.assertEqual(myx_classes.contributors(["", " ", ".", " . ", "'", None]), []) + + def test_real_names_are_kept_as_given(self): + self.assertEqual(names(myx_classes.contributors(["Peter Swanson", " Val McDermid"])), ["Peter Swanson", " Val McDermid"]) + + def test_clean_function_runs_before_the_emptiness_check(self): + self.assertEqual(myx_classes.contributors(["[GraphicAudio]"], myx_utilities.removeGA), []) + + +class SettersTest(unittest.TestCase): + def test_set_narrators_skips_empty_tokens(self): + b = myx_classes.Book() + b.setNarrators(" , ,") + self.assertEqual(b.narrators, []) + b.setNarrators("Sophie Amoss, , Graham Halstead,") + self.assertEqual(names(b.narrators), ["Sophie Amoss", " Graham Halstead"]) + + def test_set_authors_skips_empty_tokens(self): + b = myx_classes.Book() + b.setAuthors("A Writer, , B Writer") + self.assertEqual(len(b.authors), 2) + self.assertEqual(b.getAuthors(), "A Writer,B Writer") + + +class GetListTest(unittest.TestCase): + def test_no_enclosed_empty_token(self): + people = [myx_classes.Contributor(""), myx_classes.Contributor("."), myx_classes.Contributor("A Writer"), + myx_classes.Contributor(" ")] + self.assertEqual(myx_utilities.getList(people, "|", '"'), '"A Writer"') + self.assertEqual(myx_utilities.getList([myx_classes.Contributor("")], "|", '"'), "") + + def test_empty_series_name_is_skipped(self): + series = [myx_classes.Series("", "1"), myx_classes.Series("Saga", "2")] + self.assertEqual(myx_utilities.getList(series, ",", ""), "Saga") + + def test_book_getters_never_return_an_enclosed_empty_name(self): + b = myx_classes.Book() + b.narrators = [myx_classes.Contributor("")] + b.authors = [myx_classes.Contributor(" "), myx_classes.Contributor("Peter Swanson")] + self.assertEqual(b.getNarrators(delimiter="|", encloser='"'), "") + self.assertEqual(b.getAuthors(delimiter="|", encloser='"'), '"Peter Swanson"') + + +class FfprobeTest(unittest.TestCase): + def test_empty_composer_gives_no_narrators(self): + book = probed({"title": "Before She Knew Him", "artist": "Peter Swanson", "composer": ""}) + self.assertEqual(book.narrators, []) + self.assertEqual(names(book.authors), ["Peter Swanson"]) + + def test_blank_artist_and_stray_commas(self): + book = probed({"title": "T", "artist": " ", "composer": "Reader One, , Reader Two (Narrator)"}) + self.assertEqual(book.authors, []) + self.assertEqual([n.strip() for n in names(book.narrators)], ["Reader One", "Reader Two"]) + + def test_empty_composer_does_not_reach_audible_as_quoted_empty_narrator(self): + """The reported case end to end: the catalog query must carry narrator '' (Audible then finds the book), + not '\"\"' (Audible answers 0 results).""" + book = probed({"title": "Before She Knew Him", "artist": "Peter Swanson", "composer": ""}, duration=615 * 60) + mb = myx_classes.MAMBook("Peter Swanson - Before She Knew Him") + bf = myx_classes.BookFile("Peter Swanson - Before She Knew Him/Peter Swanson - Before She Knew Him.m4b", + "/src/x.m4b", "/src", "/media") + bf.ffprobeBook = book + mb.files.append(bf) + mb.ffprobeBook = book + client = FakeAudible(search=[product("0062838180", "Before She Knew Him", ["Peter Swanson"], 615)]) + with tempfile.TemporaryDirectory() as tmp, contextlib.redirect_stdout(io.StringIO()) as out: + #narrators are only sent to Audible with add_narrators on (as in the config the report came from) + mb.getAudibleBooks(client, book, FakeConfig(tmp, **{"Config/flags/add_narrators": 1})) + searches = [p for (url, p) in client.calls if url.endswith("/catalog/products")] + self.assertTrue(searches) + self.assertEqual(searches[0]["narrator"], "") + for params in searches: + self.assertNotIn('""', params.get("narrator", "")) + self.assertNotIn('""', params.get("author", "")) + self.assertNotIn('narrators:""', out.getvalue()) + self.assertEqual(mb.bestAudibleMatch.asin, "0062838180") + + +class MAMBookFfprobeTest(unittest.TestCase): + """MAMBook.ffprobe is not called by booktree.py (BookFile.ffprobe is the live parser); upstream appended a + nameless Contributor() there, which raised TypeError for any file with an artist tag.""" + + def run_ffprobe(self, tags): + with patch.object(myx_utilities, "probe_file", return_value={"format": {"tags": tags}}): + return myx_classes.MAMBook("Release").ffprobe("/src/Release/file.m4b") + + def test_authors_are_named(self): + book = self.run_ffprobe({"title": "T", "artist": "Writer One, [GraphicAudio], Writer Two (Author)", "composer": ", Reader (Narrator)"}) + self.assertEqual(names(book.authors), ["Writer One", "Writer Two"]) + self.assertEqual([n.strip() for n in names(book.narrators)], ["Reader"]) + + def test_empty_tags_give_empty_lists(self): + book = self.run_ffprobe({"title": "T", "artist": "", "composer": " "}) + self.assertEqual((book.authors, book.narrators), ([], [])) + + +class RemoteNamesTest(unittest.TestCase): + """An empty name from Audible or MAM is dropped too: an empty id3 author and an empty result author would + otherwise compare equal in isThisMyAuthorsBook.""" + + def test_audible_product_with_empty_narrator(self): + p = product("B000000001", "T", ["Writer", ""], 100, narrators=[{"name": ""}, {"name": "Reader"}]) + book = myx_audible.product2Book(p) + self.assertEqual(names(book.authors), ["Writer"]) + self.assertEqual(names(book.narrators), ["Reader"]) + + def test_mam_result_with_empty_author(self): + row = {"id": 1, "title": "T", "author_info": json.dumps({"1": "", "2": "Writer"}), + "narrator_info": json.dumps({"3": " "}), "my_snatched": 1} + with patch.object(myx_mam, "searchMAM", return_value=[row]): + books = myx_mam.getMAMBook(FakeConfig("/nonexistent"), titleFilename="T.m4b") + self.assertEqual(names(books[0].authors), ["Writer"]) + self.assertEqual(books[0].narrators, []) + + +if __name__ == "__main__": + unittest.main()