Skip to content

fix: keep split chapter anchors on their first fragment - #52

Open
brunosabenca wants to merge 1 commit into
crosspoint-reader:masterfrom
brunosabenca:fix/split-anchor-last-fragment
Open

fix: keep split chapter anchors on their first fragment#52
brunosabenca wants to merge 1 commit into
crosspoint-reader:masterfrom
brunosabenca:fix/split-anchor-last-fragment

Conversation

@brunosabenca

Copy link
Copy Markdown

Summary

Fixes #51.

textsplit.py duplicates a chapter's wrapping element (and its id) into every fragment produced when a large spine file is split. The anchor-map assignment in the split loop was last-write-wins, so it always ended up pointing the chapter's TOC/NCX anchor at its last fragment instead of its first.

CrossPoint's (and YACP's) spine→TOC lookup falls back to the previous entry for any spine item with no direct TOC match, by design — reasonable for genuinely un-tocced continuation files, but it means every fragment before the mis-pointed last one now shows the previous chapter's title, correcting only right before the next chapter begins.

Fix

One-line change: setdefault instead of assignment, so the first fragment containing the id wins.

Verification

Regenerated the epub for the book that surfaced this and confirmed the resulting toc.ncx now points every split chapter at its first fragment instead of its last. Also reproduced and re-verified against a minimal synthetic fixture (attached on #51) with no copyrighted content — same before/after behavior.

textsplit.py duplicates a chapter's wrapping element (and its id) into
every fragment produced when a large spine file is split, so the anchor
map's last-write-wins assignment always ended up pointing the TOC/NCX
anchor at the chapter's last fragment instead of its first. Readers whose
spine->TOC lookup falls back to the previous entry for un-tocced spine
items (CrossPoint and YACP both do) then show the previous chapter's
title for nearly the entire chapter.

Use setdefault so the first fragment containing the id wins instead.
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d158e3f-13c6-4f13-8272-afd677c73789

📥 Commits

Reviewing files that changed from the base of the PR and between 460c530 and a6c980f.

📒 Files selected for processing (1)
  • crosspoint_reader/textsplit.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

📜 Recent review details
🔇 Additional comments (1)
crosspoint_reader/textsplit.py (1)

403-403: LGTM!


📝 Walkthrough

Walkthrough

The split process now keeps the first chunk mapped to a duplicated fragment ID. Subsequent chunks no longer overwrite that mapping during fragment link remapping.

Changes

Split anchor mapping

Layer / File(s) Summary
Preserve first anchor occurrence
crosspoint_reader/textsplit.py
anchor_map now uses setdefault, so duplicated fragment IDs map to the first split chunk instead of the last one.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to a6c98

The change makes split chapter links resolve to the chapter’s first fragment, with no actionable merge-blocking risk remaining after normal checks and review.

Suggested reviewers: ituri

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: retaining split chapter anchors on the first fragment.
Description check ✅ Passed The description directly explains the bug, the setdefault fix, and the verification results.
Linked Issues check ✅ Passed The change satisfies issue #51 by making split chapter anchor resolution first-fragment-wins, which corrects TOC/NCX targets and chapter-title lookup.
Out of Scope Changes check ✅ Passed The one-line anchor-map change is directly related to issue #51. No unrelated changes are present.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brunosabenca

Copy link
Copy Markdown
Author

Regression check against a real library (578 books)

Ran both textsplit.py versions (pre-fix vs. this patch) against every epub in a real Calibre library, comparing:

  • the split_epub_text result dict (paragraph splits, file splits, extracted images/fonts) — must be identical, since the fix only changes anchor-map bookkeeping, not splitting decisions
  • any SPLIT-ERR log lines (reverts/exceptions)
  • for books with split chapters, every TOC/NCX anchor target before vs. after
Total epubs examined:        578
Processed OK (both sides):   578
Books with >=1 file split:   555
Books that hit the bug:      29
  ...fully fixed by patch:   29
Regressions detected:        0
New-only split errors:       0
Open/parse failures:         0

Zero regressions: no book split differently, no new errors, and every anchor target that changed moved to an earlier (more correct) fragment, never a later one.

29 books actually hit the bug pre-fix — this isn't a one-book edge case. A few examples (title — chapters affected):

  • Mastering Bitcoin — 68
  • Laravel: Up and Running — 124 (and 109 in the other edition on file)
  • The Scar (China Miéville) — 48
  • Dune: The Gateway Collection — 49
  • Iron Council (China Miéville) — 34
  • The Gunslinger / The Drawing of the Three / End of Watch (Stephen King) — 7 / 18 / 5

All 29 were fully corrected by the one-line setdefault fix, with no side effects on the other 549 books.

@brunosabenca

Copy link
Copy Markdown
Author

Disclosure

The root-cause investigation (issue #51), this fix, and the regression test across a 578-book library were carried out with Claude Code. I did not have attribution enabled in that session, so it isn't reflected in the commit metadata — flagging it here for transparency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split chapter TOC anchors point to the last fragment instead of the first (textsplit.py)

1 participant