feat: report skipped duplicates in the upload tray

A book whose files were all already stored settles as skipped rather than done,
naming the book that holds them and offering to add it anyway.
This commit is contained in:
2026-08-13 17:23:57 -04:00
parent d78b21c27f
commit b124a65d6e
7 changed files with 302 additions and 58 deletions
+6 -4
View File
@@ -157,10 +157,12 @@ but take a baseline first, because neither is clean (see below).
Observed in the current tree — don't mistake these for intentional patterns to copy: Observed in the current tree — don't mistake these for intentional patterns to copy:
- `src/lib/schema/openapi/schema.d.ts` is **stale** — it predates the `BookProgress` rework and has - `pnpm check` is not clean. **Baseline as of 2026-08-13: 30 errors, 1 warning, 8 files**, most of
no `BookProgressRead`, so `book.progress` types as `{}`. It is the source of most of the errors them in `src/routes/api/[...path]/+server.ts` (see below). Get your own baseline before assuming
`pnpm check` reports; regenerating it should clear them. **Baseline as of 2026-08-11: 37 errors, an error is yours. `src/lib/schema/openapi/schema.d.ts` was regenerated on that date and is
1 warning, 11 files.** Get your own baseline before assuming an error is yours. current; regenerate it again after any backend API change, with
`pnpm exec openapi-typescript http://localhost:8000/schema/openapi.json -o src/lib/schema/openapi/schema.d.ts`
against a backend running **your** branch — a stale server silently writes a stale file.
- `pnpm lint` does not pass either — 131 pre-existing ESLint errors, 35 of them - `pnpm lint` does not pass either — 131 pre-existing ESLint errors, 35 of them
`svelte/no-navigation-without-resolve` on plain `href`s, plus ~59 files Prettier would rewrite `svelte/no-navigation-without-resolve` on plain `href`s, plus ~59 files Prettier would rewrite
(mostly vendored shadcn components). Check the files you touched, not the whole tree. (mostly vendored shadcn components). Check the files you touched, not the whole tree.
+24 -4
View File
@@ -9,11 +9,27 @@ import {
editBookMetadataSchema, editBookMetadataSchema,
updateBookProgressSchema, updateBookProgressSchema,
type Book, type Book,
type BooksUploadResult,
bookFilesUpload bookFilesUpload
} from '$lib/schema/index'; } from '$lib/schema/index';
import { stringCoerce, type PaginatedResponse } from '$lib/schema/common'; import { stringCoerce, type PaginatedResponse } from '$lib/schema/common';
import { createQueryParams } from '$lib/utils'; import { createQueryParams } from '$lib/utils';
/**
* The backend's own message for a failed response, rather than its JSON envelope.
*
* A refused duplicate answers 409 with a `detail` worth reading and the offending
* files in `extra`; passing the body through whole puts JSON in front of the reader.
*/
function detailOf(body: string): string {
try {
const parsed = JSON.parse(body);
return typeof parsed?.detail === 'string' ? parsed.detail : body;
} catch {
return body;
}
}
export const getBook = query(stringCoerce, async (id): Promise<Book> => { export const getBook = query(stringCoerce, async (id): Promise<Book> => {
const { locals } = getRequestEvent(); const { locals } = getRequestEvent();
@@ -68,7 +84,9 @@ export const updateBookCover = form(bookCoverUpload, async ({ book_id, file }) =
return await response.json(); return await response.json();
}); });
export const uploadBooks = form(booksUpload, async ({ library_id, files }) => { export const uploadBooks = form(
booksUpload,
async ({ library_id, files }): Promise<BooksUploadResult> => {
const { locals } = getRequestEvent(); const { locals } = getRequestEvent();
const formData = new FormData(); const formData = new FormData();
@@ -87,7 +105,8 @@ export const uploadBooks = form(booksUpload, async ({ library_id, files }) => {
} }
return await response.json(); return await response.json();
}); }
);
export const uploadBookFiles = form(bookFilesUpload, async ({ book_id, files }) => { export const uploadBookFiles = form(bookFilesUpload, async ({ book_id, files }) => {
const { locals } = getRequestEvent(); const { locals } = getRequestEvent();
@@ -100,8 +119,9 @@ export const uploadBookFiles = form(bookFilesUpload, async ({ book_id, files })
const response = await locals.api.postMultipart(`/books/${book_id}/files`, formData); const response = await locals.api.postMultipart(`/books/${book_id}/files`, formData);
if (!response.ok) { if (!response.ok) {
const message = await response.text(); // 409 here means the file is already stored under a different book, which is
error(response.status, message); // something the reader can act on — so the message has to survive the trip.
error(response.status, detailOf(await response.text()));
} }
return await response.json(); return await response.json();
@@ -50,6 +50,22 @@
toast.error(`${file.name} was not added`, { description: reason }); toast.error(`${file.name} was not added`, { description: reason });
}; };
/**
* The API's own words, when it has any.
*
* Adding a file the library already holds under another book is refused with a
* 409 naming it — far more use than "failed to add files". SvelteKit hands an
* `error()` back as an HttpError on the client, so the message sits on `body`.
*/
function apiMessage(error: unknown): string | undefined {
if (typeof error !== 'object' || error === null) return undefined;
const body = (error as { body?: { message?: string } }).body;
if (typeof body?.message === 'string') return body.message;
return error instanceof Error ? error.message : undefined;
}
function confirmDelete(file: BookFile) { function confirmDelete(file: BookFile) {
fileToDelete = file; fileToDelete = file;
confirmOpen = true; confirmOpen = true;
@@ -148,7 +164,8 @@
toast.success('Files added'); toast.success('Files added');
} catch (error) { } catch (error) {
console.error('Failed to add files: ', error); console.error('Failed to add files: ', error);
toast.error('Failed to add files'); toast.error(apiMessage(error) ?? 'Failed to add files');
uploadBookFiles.fields.files.set([]);
} }
})} })}
enctype="multipart/form-data" enctype="multipart/form-data"
@@ -1,11 +1,13 @@
<script lang="ts"> <script lang="ts">
import { fly } from 'svelte/transition'; import { fly } from 'svelte/transition';
import { prefersReducedMotion } from 'svelte/motion'; import { prefersReducedMotion } from 'svelte/motion';
import { ChevronDown, CircleAlert, RotateCcw, X } from '@lucide/svelte'; import { resolve } from '$app/paths';
import { ChevronDown, CircleAlert, Copy, RotateCcw, X } from '@lucide/svelte';
import { Button } from '$lib/components/ui/button/index.js'; import { Button } from '$lib/components/ui/button/index.js';
import { Spinner } from '$lib/components/ui/spinner/index'; import { Spinner } from '$lib/components/ui/spinner/index';
import { getUploadQueueState } from '$lib/state/upload-queue.svelte'; import { getUploadQueueState } from '$lib/state/upload-queue.svelte';
import type { DuplicateFile } from '$lib/schema';
import { formatFileSize } from '$lib/utils'; import { formatFileSize } from '$lib/utils';
const queue = getUploadQueueState(); const queue = getUploadQueueState();
@@ -19,10 +21,34 @@
const heading = $derived.by(() => { const heading = $derived.by(() => {
const noun = queue.total === 1 ? 'book' : 'books'; const noun = queue.total === 1 ? 'book' : 'books';
if (queue.active) return `Adding ${queue.settled} of ${queue.total} ${noun}`; if (queue.active) return `Adding ${queue.settled} of ${queue.total} ${noun}`;
if (queue.failed === 0) return `Added ${queue.done} ${noun}`; if (queue.failed === 0 && queue.skipped === 0) return `Added ${queue.done} ${noun}`;
if (queue.done === 0) return `Couldn't add ${queue.failed} ${noun}`; if (queue.done === 0 && queue.skipped === 0) return `Couldn't add ${queue.failed} ${noun}`;
return `Added ${queue.done}, ${queue.failed} failed`;
const parts = [];
if (queue.done > 0) parts.push(`Added ${queue.done}`);
if (queue.skipped > 0) parts.push(`${queue.skipped} already here`);
if (queue.failed > 0) parts.push(`${queue.failed} failed`);
return parts.join(', ');
}); });
/**
* What the reader needs to know about files that were not stored.
*
* One duplicate can name where its bytes already live; several would be a list,
* and the row has no room for one, so they collapse to a count.
*/
function duplicateNote(duplicates: DuplicateFile[]) {
if (duplicates.length > 1) return `${duplicates.length} files are already in your library`;
const [only] = duplicates;
// No book to name: it matched another file in this same upload.
return only.book_title ? `Already in ${only.book_title}` : 'Already added by this upload';
}
/** The book a note can link to, when a single duplicate points at exactly one. */
function noteTarget(duplicates: DuplicateFile[]) {
return duplicates.length === 1 ? duplicates[0].book_id : null;
}
</script> </script>
{#if queue.total > 0} {#if queue.total > 0}
@@ -44,6 +70,9 @@
<Spinner class="size-4 shrink-0" /> <Spinner class="size-4 shrink-0" />
{:else if queue.failed > 0} {:else if queue.failed > 0}
<CircleAlert class="size-4 shrink-0 text-destructive" /> <CircleAlert class="size-4 shrink-0 text-destructive" />
{:else if queue.skipped > 0}
<!-- Muted, not alarming: nothing went wrong, the books were already here. -->
<Copy class="size-4 shrink-0 text-muted-foreground" />
{/if} {/if}
<span class="min-w-0 flex-1 truncate text-sm font-medium">{heading}</span> <span class="min-w-0 flex-1 truncate text-sm font-medium">{heading}</span>
@@ -78,12 +107,51 @@
{#if !queue.collapsed} {#if !queue.collapsed}
<ul class="flex max-h-64 flex-col divide-y overflow-y-auto"> <ul class="flex max-h-64 flex-col divide-y overflow-y-auto">
{#each queue.jobs as job (job.id)} {#each queue.jobs as job (job.id)}
{@const duplicates = job.duplicates ?? []}
{@const target = noteTarget(duplicates)}
<li class="flex min-w-0 items-center gap-2 px-3 py-2"> <li class="flex min-w-0 items-center gap-2 px-3 py-2">
<span class="min-w-0 flex-1"> <span class="min-w-0 flex-1">
<span class="block truncate text-sm" title={job.label}>{job.label}</span> <span class="block truncate text-sm" title={job.label}>{job.label}</span>
{#if job.error}
<span class="block font-mono text-[10px] text-muted-foreground tabular-nums"> <span class="block font-mono text-[10px] text-muted-foreground tabular-nums">
{job.error ?? formatFileSize(job.size)} {job.error}
</span> </span>
{:else if duplicates.length > 0}
<span class="flex min-w-0 items-baseline gap-2 text-[10px] text-muted-foreground">
{#if target}
<a
href={resolve('/(root)/(library)/book/[bookId]', { bookId: String(target) })}
class="truncate underline underline-offset-2"
>
{duplicateNote(duplicates)}
</a>
{:else}
<span class="truncate">{duplicateNote(duplicates)}</span>
{/if}
<!-- The hash reads a sample of the file, so a match is strong
evidence rather than proof. Someone who knows better needs a
way to overrule it.
Only for a book that was skipped whole: sending a partly-new
folder again would build a second book holding both the file
that just went in and the one that was already here. -->
{#if job.status === 'skipped' && !queue.active}
<button
type="button"
class="shrink-0 underline underline-offset-2 hover:text-foreground"
onclick={() => queue.addAnyway(job.id)}
>
Add anyway
</button>
{/if}
</span>
{:else}
<span class="block font-mono text-[10px] text-muted-foreground tabular-nums">
{formatFileSize(job.size)}
</span>
{/if}
</span> </span>
<span <span
@@ -96,6 +164,8 @@
Adding Adding
{:else if job.status === 'done'} {:else if job.status === 'done'}
Done Done
{:else if job.status === 'skipped'}
Skipped
{:else if job.status === 'failed'} {:else if job.status === 'failed'}
Failed Failed
{:else} {:else}
+9
View File
@@ -6,6 +6,15 @@ export type Book = components['schemas']['BookRead'];
export type BookFile = components['schemas']['FileMetadataRead']; export type BookFile = components['schemas']['FileMetadataRead'];
export type BookProgress = components['schemas']['BookProgressRead']; export type BookProgress = components['schemas']['BookProgressRead'];
/**
* A file the library already held, so the upload did not store it again.
*
* `book_id` is null when the match was another file in the same upload — there is
* no book to point at yet.
*/
export type DuplicateFile = components['schemas']['DuplicateFileRead'];
export type BooksUploadResult = components['schemas']['BooksUploadResult'];
export const bookQuerySchema = commonQuerySchema.extend({ export const bookQuerySchema = commonQuerySchema.extend({
libraries: stringArrayCoerce, libraries: stringArrayCoerce,
authors: stringArrayCoerce, authors: stringArrayCoerce,
+83 -9
View File
@@ -22,6 +22,23 @@ export interface paths {
patch?: never; patch?: never;
trace?: never; trace?: never;
}; };
"/books/duplicates": {
parameters: {
query?: never;
header?: never;
path?: never;
cookie?: never;
};
get?: never;
put?: never;
/** CheckDuplicates */
post: operations["BooksDuplicatesCheckDuplicates"];
delete?: never;
options?: never;
head?: never;
patch?: never;
trace?: never;
};
"/books": { "/books": {
parameters: { parameters: {
query?: never; query?: never;
@@ -710,6 +727,27 @@ export interface components {
BooksCreateFromFiles: { BooksCreateFromFiles: {
files?: string[]; files?: string[];
}; };
/** BooksUploadResult */
BooksUploadResult: {
created: components["schemas"]["BookRead"][];
skipped: components["schemas"]["DuplicateFileRead"][];
};
/** DuplicateFileRead */
DuplicateFileRead: {
filename: string;
hash: string;
size: number;
library_id: number;
book_id?: number | null;
book_title?: string | null;
};
/** FileFingerprint */
FileFingerprint: {
hash: string;
size: number;
/** @default */
filename: string;
};
/** FileMetadataRead */ /** FileMetadataRead */
FileMetadataRead: { FileMetadataRead: {
id: number; id: number;
@@ -828,6 +866,7 @@ export interface operations {
parameters: { parameters: {
query?: { query?: {
library_id?: number | null; library_id?: number | null;
allow_duplicates?: boolean;
}; };
header?: never; header?: never;
path: { path: {
@@ -908,6 +947,47 @@ export interface operations {
}; };
}; };
}; };
BooksDuplicatesCheckDuplicates: {
parameters: {
query?: {
library_id?: number | null;
};
header?: never;
path?: never;
cookie?: never;
};
requestBody: {
content: {
"application/json": components["schemas"]["FileFingerprint"][];
};
};
responses: {
/** @description Request fulfilled, document follows */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["DuplicateFileRead"][];
};
};
/** @description Bad request syntax or unsupported method */
400: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": {
status_code: number;
detail: string;
extra?: null | {
[key: string]: unknown;
} | unknown[];
};
};
};
};
};
BooksListBooks: { BooksListBooks: {
parameters: { parameters: {
query?: { query?: {
@@ -969,6 +1049,7 @@ export interface operations {
parameters: { parameters: {
query?: { query?: {
library_id?: number | null; library_id?: number | null;
allow_duplicates?: boolean;
}; };
header?: never; header?: never;
path?: never; path?: never;
@@ -1047,6 +1128,7 @@ export interface operations {
parameters: { parameters: {
query?: { query?: {
library_id?: number | null; library_id?: number | null;
allow_duplicates?: boolean;
}; };
header?: never; header?: never;
path?: never; path?: never;
@@ -1064,15 +1146,7 @@ export interface operations {
[name: string]: unknown; [name: string]: unknown;
}; };
content: { content: {
"application/json": { "application/json": components["schemas"]["BooksUploadResult"];
items?: components["schemas"]["BookRead"][];
/** @description Maximal number of items to send. */
limit?: number;
/** @description Offset from the beginning of the query. */
offset?: number;
/** @description Total number of items. */
total?: number;
};
}; };
}; };
/** @description Bad request syntax or unsupported method */ /** @description Bad request syntax or unsupported method */
+71 -19
View File
@@ -1,9 +1,9 @@
import { getContext, setContext } from 'svelte'; import { getContext, setContext } from 'svelte';
import { invalidate } from '$app/navigation'; import { invalidate } from '$app/navigation';
import type { Book, PaginatedResponse } from '$lib/schema'; import type { Book, BooksUploadResult, DuplicateFile } from '$lib/schema';
export type UploadStatus = 'queued' | 'uploading' | 'done' | 'failed'; export type UploadStatus = 'queued' | 'uploading' | 'done' | 'skipped' | 'failed';
export interface UploadJob { export interface UploadJob {
id: string; id: string;
@@ -15,10 +15,19 @@ export interface UploadJob {
status: UploadStatus; status: UploadStatus;
error?: string; error?: string;
book?: Book; book?: Book;
/**
* Files the library already held, which were not stored again. A job with
* nothing left over lands as `skipped`; one that had something new is `done`
* and still carries these, since the reader asked for those files too.
*/
duplicates?: DuplicateFile[];
/** Send this one again with `allow_duplicates`, from "Add anyway". */
force?: boolean;
} }
export interface UploadSummary { export interface UploadSummary {
created: number; created: number;
skipped: number;
failed: number; failed: number;
firstBook?: Book; firstBook?: Book;
} }
@@ -70,12 +79,23 @@ export class UploadQueueState {
readonly total = $derived(this.jobs.length); readonly total = $derived(this.jobs.length);
readonly done = $derived(this.jobs.filter((job) => job.status === 'done').length); readonly done = $derived(this.jobs.filter((job) => job.status === 'done').length);
readonly skipped = $derived(this.jobs.filter((job) => job.status === 'skipped').length);
readonly failed = $derived(this.jobs.filter((job) => job.status === 'failed').length); readonly failed = $derived(this.jobs.filter((job) => job.status === 'failed').length);
readonly active = $derived( readonly active = $derived(
this.jobs.some((job) => job.status === 'queued' || job.status === 'uploading') this.jobs.some((job) => job.status === 'queued' || job.status === 'uploading')
); );
readonly current = $derived(this.jobs.find((job) => job.status === 'uploading')); readonly current = $derived(this.jobs.find((job) => job.status === 'uploading'));
readonly settled = $derived(this.done + this.failed); readonly settled = $derived(this.done + this.skipped + this.failed);
/**
* Whether the run left something the reader still has to see.
*
* A skipped book is not a failure, but it is the only place that says the file
* was already here — and the only place to override it from.
*/
readonly needsAttention = $derived(
this.jobs.some((job) => job.status === 'failed' || (job.duplicates?.length ?? 0) > 0)
);
#running = false; #running = false;
#dismissTimer: ReturnType<typeof setTimeout> | undefined; #dismissTimer: ReturnType<typeof setTimeout> | undefined;
@@ -129,20 +149,20 @@ export class UploadQueueState {
release() { release() {
this.#held = false; this.#held = false;
if (!this.active && this.failed === 0 && this.total > 0) this.#scheduleDismiss(); if (!this.active && !this.needsAttention && this.total > 0) this.#scheduleDismiss();
} }
/** /**
* Clears itself only when everything worked. A run with failures stays until * Clears itself only when everything went in cleanly. A run with failures or
* dismissed — it is the only record of what did not make it in, and the only * skipped files stays until dismissed — it is the only record of what did not
* place to retry from. * make it in, and the only place to retry or override from.
*/ */
#scheduleDismiss() { #scheduleDismiss() {
clearTimeout(this.#dismissTimer); clearTimeout(this.#dismissTimer);
if (this.#held) return; if (this.#held) return;
this.#dismissTimer = setTimeout(() => { this.#dismissTimer = setTimeout(() => {
if (!this.active && this.failed === 0) this.jobs = []; if (!this.active && !this.needsAttention) this.jobs = [];
}, DISMISS_AFTER_MS); }, DISMISS_AFTER_MS);
} }
@@ -154,6 +174,27 @@ export class UploadQueueState {
void this.#run(); void this.#run();
} }
/**
* Sends a job again, telling the server to store it even though it matched.
*
* The hash samples a small part of the file, so a match is strong evidence
* rather than proof; this is how someone who knows better overrules it.
*/
addAnyway(id: string) {
this.jobs = this.jobs.map((job) =>
job.id === id
? {
...job,
status: 'queued' as UploadStatus,
force: true,
duplicates: undefined,
error: undefined
}
: job
);
void this.#run();
}
async #run(onFinished?: (summary: UploadSummary) => void) { async #run(onFinished?: (summary: UploadSummary) => void) {
if (this.#running) return; if (this.#running) return;
this.#running = true; this.#running = true;
@@ -169,26 +210,37 @@ export class UploadQueueState {
const index = this.jobs.findIndex((job) => job.status === 'queued'); const index = this.jobs.findIndex((job) => job.status === 'queued');
if (index === -1) break; if (index === -1) break;
const job = this.jobs[index];
this.#patch(index, { status: 'uploading' }); this.#patch(index, { status: 'uploading' });
try { try {
const body = new FormData(); const body = new FormData();
for (const file of this.jobs[index].files) body.append('files', file); for (const file of job.files) body.append('files', file);
const response = await fetch( const query =
`/api/books/fromFiles?library_id=${encodeURIComponent(String(this.jobs[index].libraryId))}`, `library_id=${encodeURIComponent(String(job.libraryId))}` +
{ method: 'POST', body } (job.force ? '&allow_duplicates=true' : '');
);
const response = await fetch(`/api/books/fromFiles?${query}`, {
method: 'POST',
body
});
if (!response.ok) throw new Error(`The server returned ${response.status}`); if (!response.ok) throw new Error(`The server returned ${response.status}`);
const result: PaginatedResponse<Book> = await response.json(); const result: BooksUploadResult = await response.json();
const book = result.items[0]; const book = result.created[0];
created += result.total ?? result.items.length; created += result.created.length;
firstBook ??= book; firstBook ??= book;
this.#patch(index, { status: 'done', book }); // Nothing created means every file in this folder was already here.
// That is not a failure, and it is not something to hide either.
this.#patch(index, {
status: result.created.length === 0 ? 'skipped' : 'done',
book,
duplicates: result.skipped
});
} catch (error) { } catch (error) {
// One bad book must not take the rest of the queue with it. // One bad book must not take the rest of the queue with it.
console.error(`Failed to upload ${this.jobs[index].label}`, error); console.error(`Failed to upload ${this.jobs[index].label}`, error);
@@ -205,9 +257,9 @@ export class UploadQueueState {
if (created > 0) await invalidate('app:books'); if (created > 0) await invalidate('app:books');
if (this.failed === 0) this.#scheduleDismiss(); if (!this.needsAttention) this.#scheduleDismiss();
onFinished?.({ created, failed: this.failed, firstBook }); onFinished?.({ created, skipped: this.skipped, failed: this.failed, firstBook });
} }
#patch(index: number, changes: Partial<UploadJob>) { #patch(index: number, changes: Partial<UploadJob>) {