Conversation
Six places build a book's naming variables. The token regex in
FileNamingService.ApplyNamingPattern carries RegexOptions.IgnoreCase, so a
pattern written {author} reaches the dictionary lookup as "author". Only
RenameService keyed its dictionary with StringComparer.OrdinalIgnoreCase, so a
lowercase pattern resolved under rename and missed everywhere else: the lookup
failed, the not-found path emitted the empty sentinel, and the cleanup removed
the segment along with its separators.
What a user sees is a library renamed successfully with {author}/{series}, and
then every new download importing as "Unknown Title.m4b" at the root of the
library, because DownloadImportService builds its own table and that one missed.
Four tables here: both FileNamingService overloads, DownloadImportService and
LibraryPathPlanner. The fifth, ManualImportPathPlanner, is Listenarrs#816 and is fixed by
PR 869; this branch deliberately leaves it alone so the two do not overlap.
Ordinal rather than culture-aware, because under tr-TR 'I' and 'i' are different
letters and CurrentCultureIgnoreCase would break {TITLE} against the key "Title".
Readarr's FileNameBuilderTokenEqualityComparer calls the culture-sensitive
ToLower() for this and has that trap open.
Twelve cases, one casing theory per table, three of them through a real import.
Eight fail with the four comparers reverted. DownloadImportService.cs is at the
500 line architecture cap, so its change is line neutral.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #976.
Six places in the codebase build a book's naming-variable dictionary.
ApplyNamingPattern's token regex matches case-insensitively, so{author}reaches the lookup as"author". Five of the six dictionaries key on the default case-sensitive comparer, so a lowercase-written pattern that works fine at rename time (RenameServiceis the one table already usingOrdinalIgnoreCase) fails silently on import: the token misses, takes the not-found path, and gets stripped along with its separators. A file imports asUnknown Title.m4bat the library root, with nothing logged above a per-file warning that reads like ordinary noise.This branch is
StringComparer.OrdinalIgnoreCaseon the four remaining dictionaries not already covered by #869 (ManualImportPathPlanner, fixed separately for #816):FileNamingService.Helpers.cs(both metadata overloads),DownloadImportService.cs, andLibraryPathPlanner.cs. Ordinal, not culture-aware, sinceCurrentCultureIgnoreCasebreaks under tr-TR whereIandiare different letters, and it matches whatRenameServicealready does.Twelve test cases, one casing theory per table, three of them through a real import path. Reverting the four comparer changes fails eight of the twelve.
Suite run against current canary (
upstream/canaryata630572e9, which this branch is built on): targeted filter acrossImportNamingTableCasingTestsandNamingTableCasingParityTests, 12 passed, 0 failed.DownloadImportService.cssits exactly at the 500-line architecture cap after this change, so it is line-neutral. The issue notes the longer-term fix is consolidating six copies of one table into one, which crosses an assembly boundary and is a separate decision.Disclosure: drafted with Claude Code at my direction; I read the cited code at commit a630572 and reviewed this before posting.