Match review: best match first, and Match by hand over the review

CLAUDE.md

@@ -1076,6 +1076,13 @@ future metadata site can attach enrichment without reshaping the schema. The
   repick records only books with a new ASIN), so a clean library leaves no items. A refresh
   proposal names the identifier it was found by (`found_by`, "asin:..."/"isbn:..."), and
   applying it is a no-op for a book that no longer has it (matched again since).
+  Likewise a match run's book that has an ASIN or ISBN now (`matchedSince`: matched by hand
+  from the review or anywhere, or a rescan found one) has no changes, reads `matched_since`
+  in the review and is skipped with detail `matched_since`, under any scope; a match whose
+  fields went in before a stop cut its cover short stays to apply, detail `cover_pending`, its
+  ASIN the run's own rather than one matched since, so the next apply takes the cover. The review's
+  items page best match first (`score DESC, id`), the cursor still an item id whose score
+  the next page reads on from.
   Each scope's change carries `series` {from, to} (main first, position 0 = none) when it
   changes the book's series, which the review shows as "old -> new". A retired work slug
   is a redirect the client follows, so the names written are the merged record's.

admin-ui/src/api/types.ts

@@ -1994,12 +1994,17 @@ export interface MatchRunItem {
   proposal: MatchProposal;
   /** What applying did: '' (not yet) | applied | skipped | failed. */
   applied: '' | 'applied' | 'skipped' | 'failed';
-  /** Why: book_gone | nothing_to_change | edit_failed | cover_failed | metadata_unavailable | identifier_unknown. */
+  /** Why: book_gone | nothing_to_change | matched_since | edit_failed | cover_failed | metadata_unavailable | identifier_unknown; cover_pending while still to apply (a stop cut its cover short). */
   detail?: string;
   /** The book as it is now. */
   book: { title: string; author: string };
   /** Nothing is indexed at the path any more. */
   gone?: boolean;
+  /**
+   * A match run's book matched since the run (it has an ASIN or ISBN now: by hand,
+   * or elsewhere): an apply leaves it alone, and it has no changes.
+   */
+  matched_since?: boolean;
   /** What each scope would write: fields (in display order) and the cover. */
   changes: Partial<Record<MatchScope, MatchChange>>;
 }

admin-ui/src/features/book/book-page.tsx

@@ -196,7 +196,12 @@ function BookView({ detail, matchOnOpen }: { detail: AdminBookDetail; matchOnOpe
         onSaved={reset}
         onRefused={(field, message) => setRefused((r) => ({ ...r, [field]: message }))}
       />
-      <MatchDialog open={matching} onOpenChange={setMatching} detail={detail} />
+      <MatchDialog
+        open={matching}
+        onOpenChange={setMatching}
+        libraryId={detail.book.library_id}
+        path={detail.book.path}
+      />
       <AddToShareDialog
         open={sharing}
         onOpenChange={setSharing}

admin-ui/src/features/book/match-dialog.tsx

@@ -18,12 +18,14 @@ import {
   cachedCommunityCover,
   invalidateBookCover,
   settleBookEdit,
+  useAdminBook,
   useCommunityCover,
   useMatchCandidates,
 } from '@/api/hooks';
 import type { AdminBookDetail, MatchCandidate, MatchRecording, OverrideField } from '@/api/types';
 import { BookCover, CoverArt } from '@/components/book-cover';
 import { ProvenanceMarker } from '@/components/provenance';
+import { QueryError } from '@/components/query-error';
 import { Badge } from '@/components/ui/badge';
 import { Button } from '@/components/ui/button';
 import { Checkbox } from '@/components/ui/checkbox';
@@ -64,15 +66,24 @@ const clip = (s: string, n: number) => (s.length > n ? `${s.slice(0, n - 3)}...`
  * unticked, and so does the community cover for a book with art of its own.
  * Covers are fetched by the server (the console's CSP loads no other host's
  * images). Each opening starts afresh, at the search.
+ *
+ * It reads the book itself, only while open (on the book page that's the page's
+ * own query, already loaded); opened elsewhere (the bulk match's review) it waits
+ * for it, or says why it didn't load. `onMatched` hears that something was
+ * written to the book.
  */
 export function MatchDialog({
   open,
   onOpenChange,
-  detail,
+  libraryId,
+  path,
+  onMatched,
 }: {
   open: boolean;
   onOpenChange: (open: boolean) => void;
-  detail: AdminBookDetail;
+  libraryId: number;
+  path: string;
+  onMatched?: () => void;
 }) {
   const { t } = useTranslation();
   const [step, setStep] = useState<Step>('search');
@@ -94,18 +105,57 @@ export function MatchDialog({
         }
       >
         {open ? (
-          <MatchBody
-            detail={detail}
+          <MatchLoader
+            libraryId={libraryId}
+            path={path}
             step={step}
             onStep={setStep}
-            onDone={() => onOpenChange(false)}
+            onDone={() => {
+              onMatched?.();
+              onOpenChange(false);
+            }}
           />
         ) : null}
       </DialogContent>
     </Dialog>
   );
 }
 
+/** The book the dialog matches, loaded (or why it didn't), then the dialog's body. */
+function MatchLoader({
+  libraryId,
+  path,
+  ...body
+}: {
+  libraryId: number;
+  path: string;
+  step: Step;
+  onStep: (step: Step) => void;
+  onDone: () => void;
+}) {
+  const { t } = useTranslation();
+  const book = useAdminBook(libraryId, path);
+  if (book.data) return <MatchBody detail={book.data} {...body} />;
+  if (book.isError) {
+    return (
+      <DialogBody>
+        <QueryError
+          title={t('book.error')}
+          error={book.error}
+          onRetry={() => void book.refetch()}
+        />
+      </DialogBody>
+    );
+  }
+  return (
+    <DialogBody className="flex flex-col gap-2" role="status" aria-label={t('common.loading')}>
+      {[0, 1, 2].map((i) => (
+        <div key={i} className="skel h-16 rounded-[12px]" />
+      ))}
+    </DialogBody>
+  );
+}
+
 type Step = 'search' | 'compare';
 
 function MatchBody({
@@ -117,6 +167,7 @@ function MatchBody({
   detail: AdminBookDetail;
   step: Step;
   onStep: (step: Step) => void;
+  /** Closes the dialog once something was written to the book. */
   onDone: () => void;
 }) {
   const { t } = useTranslation();

admin-ui/src/features/health/bulk-match-model.test.ts

@@ -7,6 +7,7 @@ import {
   bulkPhase,
   isChosen,
   isPickable,
+  leaveOut,
   runFraction,
   togglePick,
 } from './bulk-match-model';
@@ -115,6 +116,16 @@ describe('picks', () => {
     expect(applyCount(r, { exclude: new Set([1, 2]), include: new Set([9]) })).toBe(2);
     expect(applyCount(r, { exclude: new Set([1, 2, 3, 4]), include: new Set() })).toBe(0);
   });
+
+  it('leaves a book matched by hand out of the apply, and never offers one matched since', () => {
+    const picks = leaveOut(leaveOut(togglePick(NO_PICKS, doubtful), doubtful), confident);
+    expect(isChosen(confident, picks)).toBe(false);
+    expect(isChosen(doubtful, picks)).toBe(false);
+    expect(applyRequest('fill', picks)).toEqual({ scope: 'fill', exclude: [1], include: [] });
+    expect(isPickable(item({ matched_since: true }))).toBe(false);
+    // One already applied isn't pending, so leaving it out doesn't lower the count.
+    expect(leaveOut(NO_PICKS, item({ applied: 'applied' }))).toBe(NO_PICKS);
+  });
 });
 
 describe('regions', () => {

admin-ui/src/features/health/bulk-match-model.ts

@@ -44,30 +44,53 @@ export interface Picks {
 
 export const NO_PICKS: Picks = { exclude: new Set(), include: new Set() };
 
+const withId = (set: ReadonlySet<number>, id: number) => new Set(set).add(id);
+const withoutId = (set: ReadonlySet<number>, id: number) => {
+  const next = new Set(set);
+  next.delete(id);
+  return next;
+};
+
 /** Whether an apply takes the item: confident unless left out, a review one only if put in. */
 export function isChosen(item: MatchRunItem, picks: Picks): boolean {
   if (!isPickable(item)) return false;
   return item.outcome === 'auto' ? !picks.exclude.has(item.id) : picks.include.has(item.id);
 }
 
-/** Whether the item has a checkbox at all: a candidate, not applied yet, its book still there. */
+/**
+ * Whether the item has a checkbox at all: a candidate, not applied yet, its book
+ * still there and not matched since the run (an apply leaves that one alone).
+ */
 export function isPickable(item: MatchRunItem): boolean {
-  return (item.outcome === 'auto' || item.outcome === 'review') && !item.applied && !item.gone;
+  return (
+    (item.outcome === 'auto' || item.outcome === 'review') &&
+    !item.applied &&
+    !item.gone &&
+    !item.matched_since
+  );
 }
 
 /** Flips one item's pick. */
 export function togglePick(picks: Picks, item: MatchRunItem): Picks {
-  const flip = (set: ReadonlySet<number>) => {
-    const next = new Set(set);
-    if (next.has(item.id)) next.delete(item.id);
-    else next.add(item.id);
-    return next;
-  };
+  const flip = (set: ReadonlySet<number>) =>
+    set.has(item.id) ? withoutId(set, item.id) : withId(set, item.id);
   return item.outcome === 'auto'
     ? { ...picks, exclude: flip(picks.exclude) }
     : { ...picks, include: flip(picks.include) };
 }
 
+/**
+ * Leaves an item out of the apply, as a book matched by hand from the review is
+ * (the server leaves it alone anyway: this keeps the apply's count true). One
+ * already applied is no pick at all (applyCount counts only pending ones).
+ */
+export function leaveOut(picks: Picks, item: MatchRunItem): Picks {
+  if (item.applied) return picks;
+  return item.outcome === 'auto'
+    ? { ...picks, exclude: withId(picks.exclude, item.id) }
+    : { ...picks, include: withoutId(picks.include, item.id) };
+}
+
 /** How many books an apply writes to: the confident ones still pending, less left out, plus put in. */
 export function applyCount(run: MatchRun, picks: Picks): number {
   return Math.max(0, run.counts.pending - picks.exclude.size) + picks.include.size;

admin-ui/src/features/health/bulk-match.test.tsx

@@ -4,6 +4,7 @@ import { setToken } from '@/api/token';
 import type { MatchRun, MatchRunItem } from '@/api/types';
 import { mockFetch, type MockRoute } from '@/test/fetch-mock';
 import { issuesSummary } from '@/test/fixtures';
+import { bookDetail, books } from '@/test/library-fixtures';
 import { renderApp } from '@/test/render-app';
 import { signedInRoutes } from '@/test/routes';
 
@@ -301,6 +302,116 @@ describe('bulk matching', () => {
     await waitFor(() => expect(issuesAsked()).toBeGreaterThan(before));
   });
 
+  // The match dialog over the review: one candidate, no recordings (so no cover).
+  const byHandRoutes: Record<string, MockRoute> = {
+    'GET /admin/libraries/1/book/match': {
+      body: {
+        candidates: [
+          {
+            work_id: 'the-martian',
+            title: 'The Martian',
+            authors: [{ id: 'a1', name: 'Andy Weir' }],
+            first_published: '2011',
+            series: [],
+            web_url: 'https://meta.audiosilo.app/works/the-martian',
+            recordings: [],
+            score: 100,
+          },
+        ],
+      },
+    },
+    'POST /admin/meta/covers': { body: { covers: [''] } },
+  };
+
+  /** Matches the review's confident book by hand, accepting the default ticks. */
+  async function matchByHand(user: ReturnType<typeof userEvent.setup>, row: HTMLElement) {
+    await user.click(within(row).getByRole('button', { name: 'Match The Martian by hand' }));
+    const match = await screen.findByRole('dialog', { name: 'Match with community metadata' });
+    await within(match).findByRole('radio', { name: /The Martian/ });
+    await user.click(within(match).getByRole('button', { name: 'Compare fields' }));
+    const compare = await screen.findByRole('dialog', { name: 'Compare with community metadata' });
+    await user.click(within(compare).getByRole('button', { name: /^Accept \d+ field/ }));
+    await waitFor(() =>
+      expect(screen.queryByRole('dialog', { name: 'Compare with community metadata' })).toBeNull(),
+    );
+  }
+
+  it('matches a book by hand over the review, which keeps its place and leaves it out', async () => {
+    const detail = bookDetail();
+    const calls = mockFetch(
+      routes([run()], {
+        'GET /admin/libraries/1/book': { body: detail },
+        'PATCH /admin/libraries/1/book': { body: detail },
+        ...byHandRoutes,
+      }),
+    );
+    const user = userEvent.setup();
+    renderApp('/health?issue=unmatched');
+    await user.click(await screen.findByRole('button', { name: 'Review and apply' }));
+    const review = await screen.findByRole('dialog', { name: 'Review matches' });
+    const row = (await within(review).findByText('B0UK000001 · UK')).closest('li')!;
+    // The book page is a new tab away, for a closer look without leaving the review.
+    expect(
+      within(row).getByRole('link', { name: 'Open the book page for The Martian in a new tab' }),
+    ).toHaveAttribute('target', '_blank');
+
+    // Escape closes the match, not the review under it.
+    await user.click(within(row).getByRole('button', { name: 'Match The Martian by hand' }));
+    expect(
+      await screen.findByRole('dialog', { name: 'Match with community metadata' }),
+    ).toBeInTheDocument();
+    await user.keyboard('{Escape}');
+    await waitFor(() =>
+      expect(screen.queryByRole('dialog', { name: 'Match with community metadata' })).toBeNull(),
+    );
+    expect(screen.getByRole('dialog', { name: 'Review matches' })).toBeInTheDocument();
+
+    await matchByHand(user, row);
+    expect(calls.some((c) => c.method === 'PATCH' && c.path === '/admin/libraries/1/book')).toBe(
+      true,
+    );
+
+    // Back on the review: the book reads as matched since the run, and the apply leaves it out.
+    const after = (await within(review).findByText('B0UK000001 · UK')).closest('li')!;
+    expect(
+      within(after).getByText('Matched since this run: an apply leaves it as it is'),
+    ).toBeInTheDocument();
+    expect(within(after).queryByRole('checkbox')).toBeNull();
+    expect(within(review).getByRole('button', { name: 'Apply to 0 books' })).toBeDisabled();
+  });
+
+  it('asks the server again for a book matched by hand that took no ASIN or ISBN', async () => {
+    const unmatched = bookDetail({ book: { ...books[0], asin: '' } });
+    const calls = mockFetch(
+      routes([run()], {
+        'GET /admin/libraries/1/book': { body: unmatched },
+        'PATCH /admin/libraries/1/book': { body: unmatched },
+        ...byHandRoutes,
+      }),
+    );
+    const autoAsked = () =>
+      calls.filter(
+        (c) => c.path === '/admin/match-runs/5/items' && c.query.get('outcome') === 'auto',
+      ).length;
+    const issuesAsked = () => calls.filter((c) => c.path === '/admin/issues').length;
+    const user = userEvent.setup();
+    renderApp('/health?issue=unmatched');
+    await user.click(await screen.findByRole('button', { name: 'Review and apply' }));
+    const review = await screen.findByRole('dialog', { name: 'Review matches' });
+    const row = (await within(review).findByText('B0UK000001 · UK')).closest('li')!;
+    const [items, issues] = [autoAsked(), issuesAsked()];
+    await matchByHand(user, row);
+
+    // Still unmatched to the server, so not "matched since": the review asks again,
+    // the Health page under it too, and the book stays left out of the apply.
+    await waitFor(() => expect(autoAsked()).toBeGreaterThan(items));
+    await waitFor(() => expect(issuesAsked()).toBeGreaterThan(issues));
+    const after = (await within(review).findByText('B0UK000001 · UK')).closest('li')!;
+    expect(within(after).queryByText(/Matched since this run/)).toBeNull();
+    expect(within(after).getByRole('checkbox')).not.toBeChecked();
+    expect(within(review).getByRole('button', { name: 'Apply to 0 books' })).toBeDisabled();
+  });
+
   it("shows a gone book's path once, as its title", async () => {
     const gone: MatchRunItem = {
       ...confident,