"Skip Parts and Sets" never skips split volumes

[JordanFromIT]

Description

Skip Parts and Sets decides whether a record is a slice of a work by looking only at the position on its SeriesBookLink rows. Those rows are frequently sparse, and where they do exist they often hold a tidied-up number, while the book itself still carries the position the metadata provider actually returned — 3, Part 1 of 2, 2A, 4 חלק א'. So the option leaves split volumes in the library with the setting switched on, and because they sit next to the real book under the same author with a near-identical title, grabs import onto them instead of the book you monitored. This makes the filter consult Book.SeriesPosition as well: a position that is populated but does not parse as a number means the record is one slice of a work, not the work. One if block and one extracted helper, plus eight regression tests.

There is no existing issue for this — happy to file one if you'd rather have it tracked separately.

A concrete example. Harry Potter has a Japanese split-volume children's edition whose halves are separate records: position 2A for the first half of Chamber of Secrets, 3, Part 1 of 2 for Prisoner of Azkaban. With Skip Parts and Sets enabled they all survive the filter. In my library a grab for Chamber of Secrets then imported onto the 2A record rather than the monitored book — the files land in a correctly named folder, so nothing looks wrong on disk, but the monitored book stays empty and the series metadata written into the files is the split-volume series. That happened three times in one day before I traced it here.

What already works, and what doesn't. The plumbing is all present; only the last row is missing.

Step	State
skipPartsAndSets exposed in the metadata profile UI and API	✅
Persisted on MetadataProfile, read by FilterBooks	✅
IsPartOrSet recognises a non-numeric SeriesBookLink.Position	✅
IsPartOrSet recognises a non-numeric Book.SeriesPosition	❌ — what this PR adds

Technical detail

MetadataProfileService.FilterBooks calls:

FilterByPredicate(hash, GetBookKey, localHash, profile,
    (x, p) => !p.SkipPartsAndSets || !IsPartOrSet(x, seriesLinks.GetValueOrDefault(x), titles),
    "book is part of set");

seriesLinks is built from input.Series → LinkItems, so a book reaches IsPartOrSet with null links whenever the refresh did not populate series link items for it. The first check in IsPartOrSet is then skipped entirely:

if (seriesLinks != null &&
    seriesLinks.Any(x => x.Position.IsNotNullOrWhiteSpace()) &&
    !seriesLinks.Any(s => double.TryParse(s.Position, out _)))

Note the second clause as well: a single link that parses to a number is enough to clear the whole book, so a book with links ["3", "3, Part 1 of 2"] is treated as a normal entry. The remaining checks are Title1 / Title2 splitting and PartOrSetRegex against book.Title, and a split volume whose title is just the ordinary book title passes both.

Book.SeriesPosition is the denormalised per-book position and survives all of this. It is already populated by the metadata refresh and already used elsewhere for display, so this adds a read, not a new source of data.

The change:

// SeriesBookLink rows are often sparse, or hold a tidied-up position, while the book itself
// still carries what the metadata provider returned - "3, Part 1 of 2", "2A", "2 Part B".
// A position that is not a number means this record is one slice of a work, not the work.
if (book.SeriesPosition.IsNotNullOrWhiteSpace() && !IsNumericSeriesPosition(book.SeriesPosition))
{
    return true;
}

IsNumericSeriesPosition is a one-line extraction of the double.TryParse already used on the link path, so both checks stay in step.

Deliberate choices worth flagging for review

1. When the two sources disagree, the book wins. A split volume can carry a clean SeriesBookLink.Position of 3 and a Book.SeriesPosition of 3, Part 1 of 2; this treats it as a part. That is the judgement call most worth challenging here — the opposite reading is that the link rows are the curated value and should override. I went with the book because the link rows are what is missing or flattened in every case I could reproduce, and because the filter is opt-in and exempts anything already on disk, so a false positive costs a record you did not want rather than a file you did.
2. I reused the existing double.TryParse call rather than pinning CultureInfo.InvariantCulture. Keeping both checks identical seemed better than having the new path disagree with the old one. Happy to switch both to invariant if you'd rather — see the known gaps below.
3. The new check sits after the link check, not before. Both return true, so ordering is cosmetic; this way the existing comment stays attached to the code it describes.

Blast radius. IsPartOrSet has exactly one caller, the predicate above. The whole path is behind SkipPartsAndSets, which defaults to false on both seeded profiles in 001_chaptarr_complete_schema, so nothing changes for anyone who has not enabled it. FilterByPredicate only removes items absent from localItems, so books that already have files, or were added manually, are never dropped by this.

Measured on my own library, 587 books, 333 of which carry a series position:

newly filtered: 13 books, 7 distinct values
    2  '2 - Heavy Metal '      2  '2 חלק ב''      2  '3 חלק ב''
    2  '3, Part 1 of 2'        2  '4 חלק א''      2  '4 חלק ב''
    1  '2A'

unaffected, parses as a number: 320 books
    0.4, 0.5, 1, 1.5, 2, 2.1, 3, 3.5, 4, 4.1, 4.2, 5, 6, 7, 8, 9, 10, 11, 12,
    13, 14, 15, 16, 17, 18, 20, 21, 22, 23, 25, 26, 28, 29, 30, 31, 32, 33, 37

Every newly filtered value is a genuine split-part or split-edition designation. Fractional positions are the case I was most worried about breaking — legitimate novellas like The Hedge Knight at 0.5 and the 4.1 / 4.2 entries all parse as numbers and are untouched. There is a regression test pinning that.

Known gaps I deliberately did not fix

• Culture-sensitive parsing. double.TryParse without an explicit culture means 3,5 parses on a de-DE host and 3.5 may be read as 35. That is pre-existing on the link path and I did not want to change existing behaviour inside a bug fix, but it is a real latent issue and I'll fold an invariant-culture change into this PR if you want it.
• The pollution itself. This filters the symptom. SeriesBookLink.Position being flattened or missing where Book.SeriesPosition is populated is a refresh-side problem and a much larger change.
• Records already in the database. This only affects filtering during refresh, so split volumes already added stay until removed by hand. A cleanup path felt out of scope for a filter fix.

While tracing this I also noticed that a single numeric link position clears a book even when its other link positions are non-numeric (the !seriesLinks.Any(...) clause quoted above). It looks intentional — "at least one real slot" — so I left it alone rather than fold an unrelated behaviour change in here. Flagging it so it isn't mistaken for something this PR should have covered.

Database Migration

NO. No schema change of any kind. Book.SeriesPosition and MetadataProfile.SkipPartsAndSets are both existing columns created in 001_chaptarr_complete_schema — this only reads a field that was already populated. No new migration file, no change to VersionInfo, and the branch runs against an existing database with no upgrade step.

How was this tested?

Native dotnet on Linux (.NET 10.0.111), SQLite backend.

Tests first, watched failing before the fix existed. New fixture MetadataProfileServicePartsAndSetsFixture, 8 tests, driving the real MetadataProfileService.FilterBooks end to end rather than the private method:

Test	Asserts
should_skip_book_whose_series_position_is_a_split_volume	3, Part 1 of 2 is filtered
should_skip_book_whose_series_position_is_a_lettered_part	2A is filtered
should_skip_split_volume_even_when_the_series_link_position_is_clean	book beats a contradicting link position of 3
should_keep_book_whose_series_position_is_a_whole_number	2 survives
should_keep_book_whose_series_position_is_fractional	0.5 survives — the novella guard
should_keep_book_that_has_no_series_position	null survives
should_keep_split_volume_when_parts_and_sets_filter_is_disabled	the setting still gates everything
should_keep_split_volume_that_is_already_on_disk	localItems exemption holds

The five "keep" tests passed before the fix as well — they exist to pin behaviour I did not want to change, and the first run proves they were green beforehand rather than being written to match new code.

Counts, base commit develop (5713d83): 2838 → 2846 (+8), 0 failing, both Debug and Release.

The tests do catch the bug. Reverting only the production hunk and leaving the fixture untouched:

Failed should_skip_book_whose_series_position_is_a_lettered_part
  Expected: <empty>
  But was:  < <[0][Harry Potter und die Kammer des Schreckens]> >
Failed should_skip_book_whose_series_position_is_a_split_volume
  Expected: <empty>
  But was:  < <[0][Harry Potter and the Prisoner of Azkaban]> >
Failed should_skip_split_volume_even_when_the_series_link_position_is_clean
  Expected: <empty>
  But was:  < <[0][Harry Potter and the Prisoner of Azkaban]> >

Failed!  - Failed: 3, Passed: 5, Total: 8

CI commands from .github/workflows/build.yml, run locally:

Step	Result
Guard (repo) — no merge-conflict markers, package.json parses	pass
version_guard.py sync	pass — 0.9.929
version_guard.py monotonic --compare-ref origin/develop	pass — 0.9.929 vs 0.9.929
version_guard.py commit-hygiene --compare-ref origin/develop	pass
dotnet build src/Chaptarr.NoTests.sln --configuration Release	0 warnings, 0 errors
dotnet test src/Chaptarr.Core.Test/Chaptarr.Core.Test.csproj --configuration Release	2846 passed, 0 failed
dotnet publish (Console + Update)	pass

No frontend files are touched, so the yarn steps aren't applicable.

Against real data. The blast-radius figures above come from querying the Books table of a live instance (587 books, two authors' worth of series data from Hardcover and Goodreads) and evaluating each distinct SeriesPosition with the same whole-string numeric semantics as double.TryParse.

What I did not test. I have not run this against the PostgreSQL backend — the change is a pure in-memory predicate with no SQL, but I can't claim I exercised it there. I also have not tested under a non-English locale, which is exactly where the culture gap above would show up. And I have not let a full refresh run to completion on a library with these split volumes present to watch the records disappear; the evidence here is unit-level plus the query above. Happy to run any of those if you'd like them before merging.

Screenshots (UI changes only)

None — no UI changes. The setting and its help text are unchanged; only its behaviour is.