fix(series): cover split edition parts listed in another series by the same author (#3324)

[gchahcg]

Summary

ListCoveredSplitEditionParts now also covers a split edition part listed only in another series by the same author. The scheduled sweep, the series page badges, the series Unmonitor N split parts button and series fill all read this query, so all four pick the case up with no change on their side. There's no migration, no toggle, no new UI and no sync-time write.

Closes #3324.

What changed

The query is now a UNION ALL of two branches:

Branch	Pairs	Rule
Same series (unchanged)	part and whole on one series_id, any author	SplitEditionPartOf: both signals
Across series (new)	same author; the whole is in some series; the part's own series holds no book with the whole's title	SplitEditionTitleOf: title signal only

Both branches keep the same part filter (not imported), the same whole gate (not excluded; imported or monitored) and the owner rule (bw.owner_user_id IS bp.owner_user_id). SeriesID is still the part's series. Ordering puts the same-series match first, so a part matched both ways reports its same-series whole.

SplitEditionTitleOf is the title half of SplitEditionPartOf, extracted unchanged. SplitEditionPartOf now calls it, with identical behaviour.

Why title-only across series (please check this)

This is the one place the PR goes beyond the review. In the live case (details in #3324), Part 1 of The Eye of the World sits only in hc-series:248644 "The Wheel of Time (Finnish Numbering)", where every part is its own volume, at the integer position 1. Widening the join but keeping signal 1 (fractional position under the whole's) still rejects it.

Positions from two different series aren't comparable, so across series the title signal is the only one that means anything. The loosening is bounded:

• It applies only when the part's own series holds no book with the whole's title. Otherwise only the two-signal rule decides, so a part numbered as its own volume inside the whole's own series is left alone. That still holds when the library has a second copy of the whole somewhere else (both cases tested).
• The whole must itself be in a series.
• It requires the same author and owner, and an eligible whole.
• The "a series made only of split parts has nothing to cover it" case stays safe: an owned whole still has to exist.

If you'd rather not loosen signal 1, the fallback is to drop the UNION branch's title-only gate and require a fractional position there too. That keeps the shape but doesn't fix #3324.

Tests

• AcrossSeries is seeded from the live rows in Split edition part listed only in another series (alternate numbering) is never covered by its owned whole #3324. It fails on main and passes here.

• AcrossSeriesGuards covers what must not pair:

  • a different author or a different owner
  • a part sharing a series row with the whole at an integer position, including with a second copy of the whole elsewhere, a copy that differs only by case, or one that differs only by normalisation
  • a part in two series, one of them shared with the whole
  • the whole in no series
  • a title that isn't the whole plus a marker ("…Companion, Part 2")
  • the whole excluded, or wanted and unmonitored
  • the part already imported

  It also has a positive control.

• GuardIgnoresOtherOwners: another user's copy of the whole beside the part doesn't block it.

• ReportsOneWholePerPart: same-series whole preferred, and one row per part when two copies of the whole exist elsewhere.

• PlanStartsFromThePart: EXPLAIN QUERY PLAN, with and without a series filter. No step reaches the whole through idx_books_owner, and the cross-series branch reaches it by author. It also runs under -race.

• LargeLibraryIsFast: 3,000 books, fails past 2 s (the old query fails it), asserts the result. Skipped under -race.

• TestSplitEditionTitleOf, TestSplitEditionBase.

• The existing consumer tests pass untouched. TestListCoveredSplitEditionParts gives each case its own author.

Each regression from the review was re-run as a mutation and fails a test.

Performance

The cross-series branch starts from the part, or from the one series' rows under a series filter. It reaches the whole by author (idx_books_author_id). A unary + on the owner comparison keeps idx_books_owner out of both branches. Given the choice, SQLite used it, which on a single-user library is every book against every book: 5.2 s on 3,000 books before, about 150 to 270 ms now on an adversarial seed with ~44k candidate pairs.

The shared-series guard runs in Go, only for parts that have already title-matched. It compares with CleanTitle, as the match does, and only counts books of the part's own owner. Each part title is parsed once, through seriesmatch.SplitEditionBase.

Known limitation (not in this PR)

The Hardcover catalogue diff (series.go, presentWholes/eligibleSplitEditionWhole) still pairs only within one series. On the Finnish Numbering series page, a catalogue part whose whole sits in the main series still shows as missing, and Fill can create it. Once it's stored, this query covers it: the sweep skips it, and it gets the badge and the button. Taking the diff across series would mean looking up same-author wholes outside the series being diffed. I left that out to keep this to the one query you asked for, but I can follow up if you want it.

Checklist

• Tests added or updated
• Doc-update gate cleared (docs/Hardcover-Series-Wiki.md, one sentence in the split-edition paragraph)
• Wiki pages updated if user-facing behaviour changed (the in-repo wiki doc above; nothing else covers this)

Test plan

• New across-series test fails on upstream/main, passes on this branch
• go test ./internal/db/... ./internal/seriesmatch/... ./internal/scheduler/... ./internal/api/...
• golangci-lint run --concurrency 2 ./...

🤖 Generated with Claude Code

https://claude.ai/code/session_01WA2CUNg5o4wUBaX6yWXNNk

[codecov]

Codecov Report

❌ Patch coverage is 85.21127% with 21 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/db/split_edition_parts.go	84.32%	11 Missing and 10 partials ⚠️

📢 Thoughts on this report? Let us know!

[gchahcg]

Heads up on scope: this PR fixes the symptom we hit live (split-edition parts getting created/kept monitored forever when their whole is linked to a different series record than the part), not the underlying cause.

The actual root cause is that nothing in Bindery detects or merges duplicate series records a provider returns for what is conceptually the same series (e.g. two separate Hardcover-sourced rows both representing "The Wheel of Time"). ListCoveredSplitEditionParts's series_id-scoped join (from #3109, predates this PR) silently misses a part/whole pair split across two such rows — that's the gap this PR closes, by widening the pairing to "same author + same book owner" instead of "same series_books.series_id".

That widening is independent of and strictly more robust than series-id scoping either way, so it's worth keeping regardless. But duplicate series rows still cause other, unaddressed symptoms once they exist:

• The Series page shows two separate entries for what's really one series, with no automatic consolidation.
• Each duplicate's completion/missing-count is computed only off the books linked to that row, so neither reflects the true picture.
• A Fill/"Add All" run against each duplicate independently could still want/create books under both.

There's already a real fix for the actual cause — POST /series/{id}/merge (internal/db/series_merge.go / internal/api/series_merge.go, #2554) merges duplicate series rows in one transaction. It's manual only though: nothing detects candidate duplicates and prompts a merge, so a user has to notice the duplicate themselves.

Happy to open a follow-up issue for automatic duplicate-series detection (candidate pairs per author by title/provider overlap, surfaced as a "merge?" prompt) if that seems worth doing — didn't want to scope-creep this PR to include it.

[vavallee]

Thanks for digging this out. The gap is real: ListCoveredSplitEditionParts self joins series_books on series_id, so a whole on one Hardcover series record and its parts on a duplicate record never pair, and the sweep keeps searching the parts. I'd like to fix it, but in a smaller shape.

Why I don't want this version

1. It fixes one consumer out of three. ListCoveredSplitEditionParts feeds the scheduled sweep (scheduler.go), the series page badges and the series Unmonitor button. This PR leaves that query alone and adds a second detector that only the author sync and the two new author routes use. After a sync the author page says the part is covered while the series page still counts it as missing. The comment at the top of split_edition_parts.go already asks for the SQL rule and eligibleSplitEditionWhole to stay in step; a third copy makes that harder.

2. It turns read time suppression into a write. #3109 deliberately skips covered parts in the sweep without touching monitored, and leaves unmonitoring as a button the user presses. Here every full author sync writes monitored = 0, on by default for every install through a migration. Someone who re monitors a part on purpose (they own the whole as an ebook and want the split audiobook) gets it undone on the next sync, with only a count in the sync summary to say so.

3. Size for the gain: a migration, a profile column, two routes, a checkbox, badges and a button, for what is one query change.

What I'd take instead

Widen the join in ListCoveredSplitEditionParts so the whole and the part pair when they share a series row or share an author (bw.author_id = bp.author_id), keeping the owner rule and SplitEditionPartOf as the gate. That fixes the sweep, the series page and the existing button in one place, with no migration, no toggle and no new UI. If you'd rather keep the series scope as the first choice, apply the author wide pairing only when no shared series row exists. The series page button then covers the manual cleanup you were adding to the author page.

Alongside that:

• Please open an issue first with the author and the two Hardcover series ids you hit, so the case is on record and the test can be built from it.
• A test that fails on main with a whole and a part on two different series rows.
• A changelog.d/ fragment.

Mechanical bits

• The DCO check needs Signed-off-by on the commit (git commit -s).
• tsc fails on MetadataTab.languages.test.tsx:29, which came in with #3213 after your branch was cut, so lint, validate (frontend) and smoke are all red on that one error. Moot if the profile column goes away.
• Migration 108 is the next free number today, but it will collide with anything else that claims it. Another reason to avoid a migration here.

[gchahcg]

Thanks, agreed on all three points. I've rewritten the branch to your shape: one signed-off commit that only changes ListCoveredSplitEditionParts, plus a changelog fragment and one wiki sentence. The migration, profile column, author routes, UI and sync-time write are all gone, so the sweep, the series page, the series button and fill's re-queue of stored books all get the fix in one place.

I opened #3324 with the author and both Hardcover series ids first, as you asked. Building the test from the real rows turned up something I'd missed before, and it corrects my earlier comment on this PR:

• It isn't a duplicate series record. The Eye of the World, Part 1 of 2 sits only in hc-series:248644 "The Wheel of Time (Finnish Numbering)", an alternate numbering where every part is its own volume, at the integer position 1. The whole is at 1 in hc-series:1097, and Part 2 is at 1.2 beside it (already caught today). So the auto-merge follow-up I floated is withdrawn: merging these two series would be wrong.
• That means widening the join while keeping signal 1 still misses it, and so did my original version. Its tests used a fractional position I'd made up, which is on me.

So the new branch pairs across series by the title signal alone, but only when the part's own series holds no book with the whole's title, and only when the whole is itself in a series. Positions from two series aren't comparable, so signal 1 can't be evaluated there. Inside a shared series, only the two-signal rule decides. The details and the guard tests are in the PR body. This is the one place I went beyond what you described, so tell me if you'd rather not loosen signal 1. In that case I'll drop it, but then #3324 stays open.

There's one known gap I didn't take on: the catalogue diff still pairs within one series only, so Fill on the Finnish series can still create the part (see the PR body).

On the mechanical points: the commit is signed off, and the branch is rebased on current main, so the MetadataTab.languages.test.tsx error no longer applies (no web changes). Migration 108 is gone too.

[vavallee]

Thanks, the rewrite is the shape I asked for: one commit, only ListCoveredSplitEditionParts, no migration, and CI is green. I read it and ran the new tests, and found one blocker and a few test gaps.

Blocker: the new branch of the query is too slow on a real library

The second branch of the UNION ALL in internal/db/split_edition_parts.go (around lines 83 to 96) gets this plan from SQLite: SCAN bw, then SEARCH bp USING idx_books_owner. It never uses idx_books_author_id. On a seeded library of 10k books, 400 series and 100 authors, that branch alone took about 30 s, against under 1 ms for the old branch. The sweep and the three series page callers in internal/api/series.go all run it.

Driving the join from the part fixes it. Writing the branch as FROM books bp CROSS JOIN series_books sp ON sp.book_id = bp.id CROSS JOIN books bw ON bw.author_id = bp.author_id AND bw.id <> bp.id took about 1.3 ms on the same data, with the rest of the branch unchanged. A test that seeds a few thousand books and fails past a generous time limit would keep it from creeping back.

Tests that would not catch a regression

I broke each of these in turn and everything stayed green:

• Removing the Go side SplitEditionTitleOf check (split_edition_parts.go:129). Only the SQL GLOB stands between you and a same author book such as "The Eye of the World Companion, Part 2". A case for that would pin it.
• Narrowing the NOT EXISTS to x.series_id = sp.series_id. A part that sits in two series, one of them shared with the whole at an integer position, is correctly left uncovered today, and nothing tests that.
• Making the title comparison case sensitive, flipping same_series DESC to ASC, and removing the dedup. Two copies of the whole in another series is reachable, and today it returns one row.

The owner scoping, author join, eligibility gate and imported part filter are all covered, nice.

One small thing

changelog.d/3324-split-parts-across-series.md has an em dash on line 2 (after "(#3324)"). Release notes avoid dashes, so a comma or a full stop works.

[gchahcg]

Thanks for running it. Everything is addressed in a new signed-off commit on top, not a force-push.

Performance

I reproduced it first. On a seeded library of 3,000 books (50 authors, 150 series), with a quarter of them "Part N" titles so there are about 44k candidate pairs, the old branch took 5.2 s.

• CROSS JOIN from the part (your suggestion), with a unary + on bw.owner_user_id. CROSS JOIN on its own got it to 1.39 s, but SQLite still reached the whole through idx_books_owner. With every book under one owner, that's every book against every book. The + takes the owner index out of the choice, so the plan is SEARCH bw USING INDEX idx_books_author_id (author_id=?). I tried INDEXED BY first, but it makes the query error outright if the index is ever renamed, so I switched.
• With a series filter (the series page calls), the cross-series branch starts from that series' rows instead of scanning every book.
• The same-series branch had the same owner-index plan on main. With a series filter, it reached the whole through idx_books_owner, so every same-owner book was checked for each series row, instead of reaching it by primary key through sw. The + sits in the shared eligibility check, so it fixes that too. The plan test pins both branches.
• The shared-series guard moved from SQL to Go. The correlated NOT EXISTS was most of what remained (about 470 ms). It now runs only for parts that have already title-matched a whole in another series: one indexed query fetches the titles that share any of those parts' series. It also compares with CleanTitle, as the match does. Before, it used lower(), so "Eye of the World" sitting beside the part didn't block it. It now only counts books with the part's own owner. Series rows carry no owner, so before this another user's copy of the whole in a shared series could keep this user's part searched. GuardIgnoresOtherOwners covers that case.
• Each title is parsed once. SplitEditionBase (the CleanTitle before the marker) runs once per part and CleanTitle once per whole, instead of once per pair. SplitEditionTitleOf is now defined as exactly that comparison.

The result is about 150 to 270 ms on that seed, measured with a load average around 5.

• TestListCoveredSplitEditionParts_LargeLibraryIsFast seeds those 3,000 books, fails past 2 s (the old query fails it) and asserts the result: zero, since the guard has to hold for every part. It's skipped under -race, using the raceEnabled pattern from internal/duplicates, because it takes about 7 s there and would turn the race shard red on every PR.
• TestListCoveredSplitEditionParts_PlanStartsFromThePart is the deterministic guard. It runs EXPLAIN QUERY PLAN on the real query, with and without a series filter, and fails if the whole isn't reached through idx_books_author_id or if idx_books_owner shows up. That works under -race too.

Test gaps

I re-ran each regression you described, plus a few more, against the new code:

Regression	Caught by
Drop the Go title check	AcrossSeriesGuards ("The Eye of the World Companion, Part 2", same author, passes the GLOB)
Guard only the reported row's series	Can't happen any more: the guard takes a part id and looks at every series it's in. AcrossSeriesGuards has the two-series case (one series shared with the whole at an integer position)
Case-sensitive or un-normalised guard	AcrossSeriesGuards (shared series holds "the eye of the world", or "Eye of the World")
Drop the guard	AcrossSeriesGuards, LargeLibraryIsFast
Drop "whole is in a series"	AcrossSeriesGuards
Drop the dedup	ReportsOneWholePerPart (two copies of the whole in another series; one row)
Let idx_books_owner back in	PlanStartsFromThePart
Guard counts other owners' books	GuardIgnoresOtherOwners
Drop the owner rule on the whole	AcrossSeriesGuards

One honest exception: flipping same_series DESC to ASC no longer fails anything, and that's now correct rather than a gap. Since the guard compares by CleanTitle, a part matched by the same-series rule always has its own whole in its series with the same CleanTitle. So the cross-series branch is always blocked for it, and the two rules can't both match one part. I kept the ordering as a backstop and said so in the test.

Changelog

Fixed: the dash is now a full stop.

[vavallee]

Re-checked on 905cbaf: the join now starts from the part and uses idx_books_author_id (about 1 s on 10k books, down from about 59 s), the plan test and the guard tests fail when each guard is broken, and the fragment is clean. Thanks for the quick turnaround.