Repository navigation
[pull] main from vavallee:main - #17
Merged
Merged
Conversation
) (#2678) * feat(author): surface duplicate-title candidates for human review (#1970) The same book often reaches a catalogue twice under slightly different titles ("The Martian" / "Martian", "Dune" / "Dune (Unabridged)"), and there was no way to see the collisions. This adds a read-only report that groups an author's titles that look like the same book, and a review modal whose only action is the existing exclude route. Detection lives in internal/duplicates, a package nothing else may import (a source-scan test pins the boundary): an aggressive title fold (case, punctuation, diacritics; & expanded to "and") feeds four rules — alnum-equal, article-strip, edition-suffix, and a guarded substring — and matches are linked transitively into groups. Each group and member reports which rule(s) matched, so the UI can explain every group in plain language. The aggressive fold is separate from the dedup key on purpose: it may be aggressive, the dedup key may not. GET /author/{id}/duplicate-candidates writes nothing; groups whose members are all already excluded are omitted. The author page's More menu gains "Review duplicates…", and the modal lists each group with its rules and an Exclude button per row, which calls PUT /book/{id}/exclude and re-fetches. Excluded members are shown struck through, and a group disappears once every member is excluded. API.md documents the endpoint and response; the user guide explains the review flow. Signed-off-by: gch ahcg <gchahcg@proton.me> * fix(web): address review findings on duplicate-candidates (#2678) The modal tracked one busy book id at a time, so starting a second row's toggle re-enabled the first row's button; a second click could fire an overlapping flip that cancels the first out silently. Track in-flight rows per book instead. The Scan godoc claimed maxPairwiseBooks caps the O(n^2) pairwise pass, but only the substring rule is gated above 2000 books. State what the loop actually does. Signed-off-by: gch ahcg <gchahcg@proton.me> * fix(duplicates): suppress substring across different series positions (#1970) The substring rule alone grouped separate books in a series whenever a long series-opener title doubled as a prefix of a sequel's title ("Foundation" vs "Foundation and Empire", "Second Foundation") — found in review by running real titles through duplicates.Scan. DuplicateCandidates now also loads the author's series memberships and passes them to Scan, which drops a substring-only match between two books that are known, different positions in the same series — by definition different works. Same-position pairs (a genuine subtitle variant, e.g. Mistborn / Mistborn: The Final Empire) are unaffected. Missing series data (h.series nil, or no membership recorded) falls back to today's behaviour, per the review's accepted fallback. Signed-off-by: gch ahcg <gchahcg@proton.me> * fix(duplicates): only match a subtitle at a separator boundary (#1970) The series position guard only helps when series links exist, and they are often missing. Without them the substring rule still grouped a series opener with every sequel carrying its name. Real catalogues with no series data: Asimov "Foundation" pulled in nine other titles as one group, King's "The Dark Tower" merged true and false pairs into a single transitive group, and "The Science of Discworld" grouped with its two sequels, as did "Carrie" with "Carrie Soto Is Back". The shorter title must now equal a whole part of the longer one as it is punctuated (split at a colon, semicolon, bracket, spaced hyphen, or en or em dash), compared with and without a leading article. Mistborn, The Gunslinger, Hogfather (Discworld, #20) and the other real subtitle cases still group; the series guard still covers "Mistborn" against "Mistborn: The Well of Ascension". The aggressive key also drops Latin diacritics after the umlaut expansion, so "Les Misérables" meets "Les Miserables", which is what the alnum-equal rule text in the UI already claimed. The handler series test gains a position 2 sequel that the separator rule matches, so removing the guard now fails it; with the old Foundation fixture it passed either way. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 * docs(changelog): rewrite the duplicate review fragment (#1970) Drop the dashes, describe the separator rule, and credit gchahcg. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 --------- Signed-off-by: gch ahcg <gchahcg@proton.me> Signed-off-by: vavallee <vavallee@protonmail.com> Co-authored-by: gch ahcg <gchahcg@proton.me> Co-authored-by: vavallee <vavallee@protonmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* fix(import): skip files already tracked in library Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * test(import): cover tracked-file scan failures Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * fix(import): detect tracked hardlinked files Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * fix(api): isolate manual import path validation Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * fix(import): skip fully tracked directory units Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * fix(import): suppress already imported scan matches Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * fix(import): skip copied sources for tracked books Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * test(import): cover manual scan tracking helpers Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * perf(db): batch book_files lookups and expose a change counter The manual-import scan looked up book_files once per confident catalogue match (N+1) to check whether a match's format was already imported. ListByBooks/ListFilesForBooks replace that with one query. BookFileRepo also gets an atomic version counter bumped on every mutation, so a derived cache (the scan's tracked-file index) can tell cheaply whether it needs to rebuild instead of re-querying on every request. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fn38ejN3ZtW9HNAjcS4p8m Signed-off-by: Claude <noreply@anthropic.com> * fix(import): filter already-tracked scan units before the cap, not after enumerateImportUnits capped at 1000 raw units before the already-tracked filter ran, so a folder where the first 1000 units were all already imported came back empty with truncated=true even though untracked files existed further down the tree. The tracked/hardlink check now runs inside the walk via a skip predicate, so the cap counts only surfaced units; when the separate post-lookup "book already has this format" check still empties part of a page, Scan re-walks (bounded by maxScanRounds) to refill it instead of returning a short, misleadingly non-empty-looking truncated page. Also adds includeImported (default off) to disable both filters and label each unit's AlreadyImported status instead of dropping it, and skips the tracked-file hardlink stat/walk for tracked paths confirmed to be on a different device than the scan root (the common NFS/ multi-mount case) — both feeding the new tracked-file index cache that rebuilds only when book_files actually changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fn38ejN3ZtW9HNAjcS4p8m Signed-off-by: Claude <noreply@anthropic.com> * feat(web): add a show-already-imported toggle to manual import Filtering already-imported files out of the bulk scan made them unreachable, with no way to spot a corrupt file or one needing a relink. Both scan UIs (Settings > Import > Bulk folder import, and the /import page) get a "Show already imported" checkbox (default off, mirroring the wanted-list includeExcluded pattern) that re-scans with includeImported and labels surfaced units instead of pre-selecting them. The truncation banner also now renders whenever the scan reports truncated, not only when the item list happens to be non-empty. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fn38ejN3ZtW9HNAjcS4p8m Signed-off-by: Claude <noreply@anthropic.com> * refactor(api): reuse importer.SameDevice instead of a third copy confirmedCrossDevice duplicated the same device-comparison logic already sitting in hardlinkableReason's own copy of importer.sameDevice -- justified there as avoiding an api->importer dependency, but that dependency already exists throughout this package (manual_import.go, scan_walk.go, books.go, ...). Export sameDevice as SameDevice and call it directly instead of adding a fourth copy of the same comparison. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fn38ejN3ZtW9HNAjcS4p8m Signed-off-by: Claude <noreply@anthropic.com> * refactor(web): extract useFolderScan, fix stale truncated banner FolderScanSection and ManualImportPage had near-duplicated scan state and logic (path/scanning/items/truncated/showImported plus the run/ toggle functions), which had already started drifting between the two copies. useFolderScan consolidates it, following the existing usePolling/useView hook convention. Consolidating surfaced a real bug shared by both copies: neither reset `truncated` when a new scan started, so toggling "show already imported" (or, on the settings page, editing the path) after a truncated scan could leave that banner showing over a later, untruncated result. Fixed once in the hook instead of twice. Also tightens two related correctness gaps the "show already imported" toggle introduced in ManualImportPage: "Select all matched" no longer sweeps up an already-imported unit the toggle deliberately left unselected, and the per-row Import button is now disabled to match what clicking it actually does, instead of silently no-op'ing on an unselected row. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fn38ejN3ZtW9HNAjcS4p8m Signed-off-by: Claude <noreply@anthropic.com> * fix(web): align manual import's show-already-imported checkbox top-right Move it into the header row next to the "Manual Import" title, matching the Wanted page's "Show excluded" placement, instead of sitting on its own line below the scan input. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016BZJo7qdcVP6K8vRiF42C8 Signed-off-by: Claude <noreply@anthropic.com> * fix(import): rebuild the tracked-file cache from book_files itself, cut redundant stats Two issues from review on #2480: The cache was keyed on BookFileRepo's own atomic mutation counter, which only advances when BookFileRepo's own methods run. book_files rows also disappear through the books(id) ON DELETE CASCADE FK when a book or author is deleted (BookRepo.Delete), and through BookRepo.UntrackFilePath's calibre-rollback DELETE — neither touches BookFileRepo, so neither bumped the counter. Reproduced: seed a tracked epub, scan (hidden), delete the book keeping the file, scan again — still hidden. BookFileRepo.Fingerprint replaces the counter with a (count, maxID) snapshot read straight off the table, which catches every mutation regardless of which code path made it. The cold rebuild also cost far more than it should: confirmedCrossDevice restatted the scan root AND re-stat'd every tracked path (once directly, once inside importer.SameDevice) before the loop's own os.Stat ran a third time on the same path — up to 5 stats per tracked row, versus main's 8 stats for a whole scan. trackedFileIndex now resolves the root's device once per call and reads each tracked path's device off the FileInfo the loop already has, down to 1 stat per row. Separately, isAlreadyTracked ran once in roundSkip and again in the results loop for the same unit; roundSkip already filters every already-tracked unit out of cands when includeImported is off, so the second call only ever matters when includeImported is on, and can be skipped otherwise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3vXCo9PwmffPWUik6Gwvh Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * i18n(web): add manualImport already-imported and show-imported keys Both strings existed only as inline t() defaults in FolderImportView and FolderImportRow, flagged in review on #2480. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3vXCo9PwmffPWUik6Gwvh Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * chore(changelog): add fragment for the skip-tracked manual import feature Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> * fix(import): rebuild the tracked-file cache after a reorganize move The (COUNT, MAX(id)) fingerprint catches inserts and deletes, including the FK cascade, but UpdatePath rewrites book_files.path in place and changes neither number. After Reorganize Library moved a tracked file, the next scan served the old index: the moved file was offered for import again and a new file landing at the old path stayed hidden. UpdatePath now bumps a per database epoch, and the cache keys on it alongside the fingerprint. It is per *sql.DB rather than per repo because reorganize and the scan reach book_files through different repo instances. The epoch is read before the fingerprint so a rename racing a rebuild only causes another rebuild. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com> * fix(import): do not hide a new file that recycled a tracked inode The tracked-file cache keeps each tracked file's os.FileInfo and matches hardlinks with os.SameFile, which compares device and inode only. When a tracked file is deleted outside Bindery its book_files row stays, the cache is not rebuilt, and ext4 gives the freed inode to the next file created. Deleting a corrupt library copy and downloading a fresh one is enough: the fresh download matched the stale FileInfo and was hidden as already imported until something else touched book_files. A real hardlink shares the inode's size and mtime as well, so require those to match too. A tracked file edited in place since the cache was built now shows as importable, which is the safe direction. Also adds a test that a tracked path missing from disk when the index was built stays hidden by its exact path once it comes back, which is the only thing that covered the path set lookup: hardlink matching masked its removal in every existing test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com> * fix(web): say already imported files are hidden when a folder scan is empty With the already imported filter on, an empty scan usually means every file in the folder is already in the library, but the page still said "No book files or folders found here." Point at the Show already imported toggle instead, and keep the plain empty message for when the toggle is on and the folder really has nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com> * chore(changelog): file the skip tracked fragment under Fixed Moves it from Added to Fixed, drops the dashes, and credits @trevorswanson in the usual form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee <vavallee@protonmail.com> --------- Signed-off-by: Trevor Swanson <83826109+trevorswanson@users.noreply.github.com> Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: vavallee <vavallee@protonmail.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: vavallee <vavallee@protonmail.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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )