From 315149e8a23affaa595fe5c0d6f91da0ff59f0b2 Mon Sep 17 00:00:00 2001 From: patrick Date: Sun, 16 Aug 2026 20:21:25 -0400 Subject: [PATCH] docs: brief for moving duplicates into library settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Option B — libraries expand in the settings nav, duplicates becomes a section within a library's pane. --- docs/duplicates-in-library-settings.md | 146 +++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 docs/duplicates-in-library-settings.md diff --git a/docs/duplicates-in-library-settings.md b/docs/duplicates-in-library-settings.md new file mode 100644 index 0000000..640053c --- /dev/null +++ b/docs/duplicates-in-library-settings.md @@ -0,0 +1,146 @@ +# Implementation brief: move Duplicates into library settings + +Written for an agent picking this up cold. Read the repo-root `AGENTS.md` and +`frontend/AGENTS.md` first — this brief assumes both. + +**This is a frontend-only change.** The backend already scopes everything by library +(`GET /books/duplicate-books?library_id=`), so no endpoint, schema or migration is +involved. + +## Where this starts from + +The duplicates review screen exists and works. It currently lives at + +``` +frontend/src/routes/(root)/(library)/library/[libraryId]/duplicates/ + +page.server.ts loads the groups, plus the full Book records merge needs + +page.svelte group cards, "Not duplicates", "Merge…" +``` + +and is reached from a **Duplicates entry in the main sidebar** +(`frontend/src/lib/components/layout/nav-main.svelte`), which is what this change +removes. + +Settings today is a flat, entirely global four-item nav +(`frontend/src/routes/(root)/settings/+layout.svelte`): Account, Appearance, Libraries, +Devices. `settings/libraries/+page.svelte` is a single table of every library whose rows +link *out* to the library itself. **There is nowhere that means "settings for this +library"** — that is the gap this change fills. + +## What to build — option B + +Libraries expands in the settings nav. Every library is a sub-item; selecting one swaps +the pane; Duplicates is a section inside that pane. All libraries and all their sections +end up one click apart. + +``` +/settings/libraries the existing table (leave it as the index) +/settings/libraries/[libraryId] redirects to the first section +/settings/libraries/[libraryId]/duplicates the review screen, moved +``` + +Suggested files: + +| Path | What | +| --- | --- | +| `settings/libraries/[libraryId]/+layout.svelte` | Library name, and the section tabs | +| `settings/libraries/[libraryId]/+page.ts` | `redirect(303, …/duplicates)` | +| `settings/libraries/[libraryId]/duplicates/+page.server.ts` | Moved verbatim | +| `settings/libraries/[libraryId]/duplicates/+page.svelte` | Moved verbatim | + +Duplicates is the **only** real section today. Build the tab strip so General and Danger +zone have somewhere obvious to land, but do not invent them now — an empty tab is worse +than no tab. + +## The nav + +In `settings/+layout.svelte`, `items` is a flat `as const` array matched on +`page.route.id`. Libraries needs to render its children beneath it: + +```svelte +{#each libraryState.libraries as library (library.id)} + … +{/each} +``` + +`getLibraryState()` **is** available under `/settings` — it is set in +`(root)/+layout.svelte`, above the settings group, and `settings/libraries/+page.svelte` +already uses it. No new load function is needed to list the libraries. + +**Active state is matched on route id, not pathname.** There is a comment in +`settings/+layout.svelte` explaining why: `resolve()` returns an absolute path on the +client and a relative one during SSR, so a pathname comparison is false on the server and +true after hydration, and the highlight flashes in. A nested library item is active when +the route id matches **and** `page.params.libraryId === String(library.id)` — both, or +every library lights up at once. + +## Things that will bite + +1. **Remove the sidebar entry in the same change.** `nav-main.svelte` gained a + `Duplicates` item and a `CopyCheck` import when the screen was built. Delete both, and + delete the old route directory. Doing the removal and the move together is the point — + split across two commits the screen is unreachable in between. + +2. **Delete the old route, do not leave it.** Two live copies of a screen that both write + is how they drift. + +3. **`setBookSelectionState` is not available under `/settings`.** It is set in + `(root)/(library)/+layout.svelte`, which the settings group is not inside. This is + fine — the duplicates page uses `BookImage` directly, not `book-thumbnail.svelte`, and + `MergeBooks` takes its `libraryId` as a prop. **Verify this stays true** if you touch + either component; a `getBookSelectionState()` under settings returns `undefined` and + fails at the first access, not at import. + +4. **Keep `depends('app:duplicate-books')`.** Both the dismiss action and `MergeBooks` + call `invalidate('app:duplicate-books')` to make a resolved group leave the screen. + Drop it and the page silently stops refreshing. `MergeBooks` also invalidates + `app:books`, which is a no-op under settings and should stay that way. + +5. **The settings shell is height-constrained.** `settings/+layout.svelte` is + `h-[calc(100vh-var(--header-height)-2rem)]` with `overflow-auto` on the content pane. + The review screen is a long list of cards — it must scroll *inside* that pane. Its + current `mx-auto max-w-5xl` wrapper will want revisiting. + +6. **Three levels of nav is option B's known cost.** Nav → library → section, and the + pane is narrower than the full-width route the screen was designed against. The group + cards are `w-36` covers in a wrapping flex row, so they reflow, but check a group of + four at a narrow window before calling it done. + +7. **`resolve()` must be a direct call in markup** for `svelte/no-navigation-without-resolve`. + Where `nav-main.svelte` computes a url through a variable it carries an + `eslint-disable-next-line`; prefer the direct call over inheriting that. + +8. **The loader depends on the `?ids=` fix.** `+page.server.ts` fetches full `Book` + records with `listBooks({ ids, pageSize })` because the merge workbench needs + identifiers, description and publisher, which `DuplicateBookRead` does not carry. + advanced_alchemy's stock id filter types that parameter as `list[str]` regardless of + config, which made Postgres refuse `bigint = character varying`; the override lives in + `backend/src/chitai/services/dependencies.py` (`create_book_filter_dependencies`). + If `GET /books?ids=1&ids=2` 500s, that override is missing — do not work around it in + the loader. + +## Out of scope + +The **General** and **Danger zone** sections (rename, path template, read-only, consume +directory, delete), and any change to the merge workbench itself. The toolbar entry point +for merge — select 2+ books in the library view — is unrelated and stays where it is. + +## Verification + +```bash +cd frontend +pnpm check # baseline: 30 errors, 1 warning, 8 files — none of them yours +pnpm lint # not clean either; check the files you touched, not the tree +pnpm build +``` + +By hand, with a library that has a duplicate group: + +- Settings → Libraries lists every library beneath it; clicking one opens its pane. +- Duplicates shows the same groups the old route did, and scrolls inside the settings pane. +- **Not duplicates** removes the group and it stays gone after a reload. +- **Merge…** opens the workbench, merges, and the group leaves the screen. +- The main sidebar no longer has a Duplicates entry, and + `/library//duplicates` no longer resolves. +- A library with no duplicates shows the empty state, not a blank pane.