fix(compress): compressing works in a real library, and says how it went

CLAUDE.md

@@ -291,6 +291,16 @@ notes**, which is the one place somebody wrote down what happened in a clip.
 Every non-trivial operation is a class in `src/main/actions/` extending `BaseAction<TInput, TOutput>`
 with a single `execute(input)`. **Add new business logic as an Action**, not inline in a route.
 
+**Anything that sends a file to the Recycle Bin goes through `MoveFileToTrashAction`**, which
+normalises the path. Rows store forward slashes (fast-glob's spelling) and `shell.trashItem` refuses
+those with "Failed to parse path"; compressing a clip called it directly and failed, after the encode,
+on every clip in a real library. `compress-check.mjs`'s shell stub now refuses a forward slash too.
+
+**A second row for one file is retired by the scan when it holds nothing of its own**
+(`services/duplicateRows.ts`). Older builds indexed some files under both slash spellings; the scan
+stopped making new ones but never removed an old one, so the clip showed twice. Only the row goes,
+never the file, and a duplicate that carries anything its twin lacks is left and logged.
+
 `ScanAndSyncClipsAction` has a **prune guard**: it refuses to delete rows when the videos folder looks
 empty or when one scan would remove more than half the library. A clip row carries the only copy of
 its tags, notes, display name, collections and stars, and an unmounted drive makes every file look

scripts/compress-check.mjs

@@ -109,14 +109,17 @@ const isGreen = ({ rgb: [r, g, b] }) => g > 60 && g - r > 40 && g - b > 40;
 /* ------------------------------------------------------------ the source */
 
 announceRoot(ROOT);
-const dir = join(ROOT, game);
+// A file instead of a game folder runs the bench on that exact recording,
+// which is how a clip somebody reports as not compressing gets reproduced.
+const givenFile = VIDEO.test(game) && existsSync(game) ? game : null;
+const dir = givenFile ? join(givenFile, '..') : join(ROOT, game);
 if (!existsSync(dir)) {
   console.error(`no such folder: ${dir}`);
   process.exit(2);
 }
 
-let source = null;
-for (const name of readdirSync(dir).filter((f) => VIDEO.test(f) && !f.startsWith('.'))) {
+let source = givenFile;
+for (const name of givenFile ? [] : readdirSync(dir).filter((f) => VIDEO.test(f) && !f.startsWith('.'))) {
   const candidate = join(dir, name);
   if ((await seconds(candidate)) >= MIN_SOURCE_SEC) {
     source = candidate;
@@ -133,10 +136,11 @@ mkdirSync(TMP, { recursive: true });
 
 const library = join(tmpdir(), `goodbit-compress-check-${Date.now()}`);
 const profile = join(library, '.profile');
-mkdirSync(join(library, game), { recursive: true });
+const folder = givenFile ? 'Clips' : game;
+mkdirSync(join(library, folder), { recursive: true });
 mkdirSync(profile, { recursive: true });
 
-const working = join(library, game, source.split(/[\\/]/).pop());
+const working = join(library, folder, source.split(/[\\/]/).pop());
 copyFileSync(source, working);
 
 /*
@@ -176,7 +180,9 @@ writeFileSync(
     // The real one moves a file to the Recycle Bin. There is no bin to check
     // here and leaving the original in place would make the rename fail, so it
     // is deleted, and the bench asserts the sequence rather than the bin.
-    'export const shell = { trashItem: (p) => rm(p, { force: true }) };',
+    // Refuses a forward-slash path exactly as the real one does on Windows, which is
+    // how every compression in a real library failed while this bench passed.
+    'export const shell = { trashItem: async (p) => { if (process.platform === "win32" && p.includes("/")) throw new Error("Failed to parse path"); await rm(p, { force: true }); } };',
     "export const screen = { getPrimaryDisplay: () => ({ id: 0, label: '' }), getAllDisplays: () => [] };",
     'export default { app, shell, screen };',
     '',
@@ -220,11 +226,12 @@ const before = {
 
 const clip = await repo.save(
   repo.create({
-    filePath: working,
+    // Stored the way a real library stores it: fast-glob hands back forward slashes.
+    filePath: working.split(String.fromCharCode(92)).join("/"),
     relPath: working.slice(library.length + 1),
     filename: working.split(/[\\/]/).pop(),
     extension: 'mp4',
-    game,
+    game: folder,
     sizeBytes: before.bytes,
     durationSec: before.seconds,
     fileModifiedAt: before.mtime,
@@ -309,7 +316,7 @@ ok('and the picture has colour in it', middle.saturation > 3, middle.saturation.
 /* ---------------------------------------- a clip that is already small */
 
 console.log('\na clip already smaller than the preset aims for');
-const small = join(library, game, 'already-small.mp4');
+const small = join(library, folder, 'already-small.mp4');
 await execFileAsync(ffmpeg, [
   '-v', 'error', '-y',
   '-f', 'lavfi', '-i', 'testsrc=size=640x360:rate=30:duration=4',
@@ -324,7 +331,7 @@ const smallRow = await repo.save(
     relPath: small.slice(library.length + 1),
     filename: 'already-small.mp4',
     extension: 'mp4',
-    game,
+    game: folder,
     sizeBytes: statSync(small).size,
     durationSec: 4,
     fileModifiedAt: statSync(small).mtime,

src/main/actions/CompressClipAction.ts

@@ -1,6 +1,6 @@
+import { MoveFileToTrashAction } from './MoveFileToTrashAction.js';
 import path from 'node:path';
 import fsPromises from 'node:fs/promises';
-import { shell } from 'electron';
 import { BaseAction } from './BaseAction.js';
 import { AppDataSource } from '../data-source.js';
 import { Clip } from '../entity/Clip.js';
@@ -160,8 +160,11 @@ export class CompressClipAction extends BaseAction<CompressClipInput, CompressCl
 
       await cancelSource(clip.filePath);
       // The bin rather than an overwrite, so a compression somebody regrets is
-      // recoverable outside this app. Never `unlink`.
-      await shell.trashItem(clip.filePath);
+      // recoverable outside this app. Never `unlink`. Through the one helper
+      // that normalises the path: rows store forward slashes, and
+      // `shell.trashItem` refuses those with "Failed to parse path", which
+      // left every compression in a real library failing after the encode.
+      await new MoveFileToTrashAction().execute({ filePath: clip.filePath });
       await fsPromises.rename(staged, clip.filePath);
 
       const now = await fsPromises.stat(clip.filePath);

src/main/actions/ScanAndSyncClipsAction.ts

@@ -1,3 +1,4 @@
+import { planDuplicateRows } from '../services/duplicateRows.js';
 import path from 'node:path';
 import fs from 'node:fs/promises';
 import fg from 'fast-glob';
@@ -192,6 +193,8 @@ export class ScanAndSyncClipsAction extends BaseAction<void, ScanResult> {
       }
     }
 
+    await this.retireDuplicateRows();
+
     const allClips = await clipRepo.find();
     const missing = allClips.filter((clip) => !nowOnDisk.has(samePath(clip.filePath)));
 
@@ -253,6 +256,45 @@ export class ScanAndSyncClipsAction extends BaseAction<void, ScanResult> {
 
     return { added, updated, removed, total, hidden: Number(hiddenRow?.n ?? 0), pruneSkipped };
   }
-}
 
+  /**
+   * Remove a second row for a file that already has one, when it holds nothing
+   * of its own. See `services/duplicateRows.ts` for the rule; this is the
+   * reading and the deleting. Rows only: the file is never touched.
+   */
+  private async retireDuplicateRows(): Promise<void> {
+    const rows: Array<Record<string, unknown>> = await AppDataSource.query(`
+      SELECT c.id, c.filePath, c.displayName, c.notes, c.starred, c.published, c.openCount,
+        (SELECT COUNT(*) FROM clip_tags_tag t WHERE t.clipId = c.id) AS tags,
+        (SELECT COUNT(*) FROM good_bit g WHERE g.clipId = c.id) AS marks,
+        (SELECT COUNT(*) FROM collection_clips_clip k WHERE k.clipId = c.id) AS collections
+      FROM clip c
+      WHERE REPLACE(LOWER(c.filePath), char(92), '/') IN (
+        SELECT REPLACE(LOWER(filePath), char(92), '/') FROM clip
+        GROUP BY REPLACE(LOWER(filePath), char(92), '/') HAVING COUNT(*) > 1
+      )`);
+    if (!rows.length) return;
 
+    const plan = planDuplicateRows(
+      rows.map((row) => ({
+        id: Number(row.id),
+        filePath: String(row.filePath),
+        displayName: (row.displayName as string | null) ?? null,
+        notes: (row.notes as string | null) ?? null,
+        starred: Boolean(row.starred),
+        published: Boolean(row.published),
+        openCount: Number(row.openCount ?? 0),
+        tags: Number(row.tags),
+        marks: Number(row.marks),
+        collections: Number(row.collections),
+      })),
+    );
+    for (const group of plan.kept) {
+      console.warn(`[scan] ${group.path} has ${group.ids.length} rows that each hold something; left as they are`);
+    }
+    if (plan.remove.length) {
+      await AppDataSource.getRepository(Clip).delete(plan.remove);
+      console.log(`[scan] removed ${plan.remove.length} duplicate row(s) for files that already had one: ${plan.remove.join(', ')}`);
+    }
+  }
+}

src/main/routes/clips.ts

@@ -800,8 +800,20 @@ clipsRouter.post('/:id/compress', asyncHandler(async (req, res) => {
       // The row is the answer worth waiting for: the tile has to redraw with
       // the new size, and a refusal has to redraw with the old one.
       const after = await AppDataSource.getRepository(Clip).findOneBy({ id: clipId });
+      // A refusal is an answer, not a failure, and the job says which in words
+      // the toast can show: before this it was a log line, so a clip that was
+      // left alone looked exactly like one that had been compressed.
       if (!result.replaced) {
         console.log(`[compress] ${clip.filename}: ${result.reason}, left alone`);
+        setJobProgress(
+          jobId,
+          100,
+          result.reason === 'not-smaller'
+            ? 'Already about as small as a compression would make it, so it was left alone.'
+            : 'The compressed copy came out the wrong length, so the original was kept.',
+        );
+      } else {
+        setJobProgress(jobId, 100, 'Compressed');
       }
       completeJob(jobId, after ? ClipDTO.fromEntity(after) : undefined);
     })

src/main/services/duplicateRows.ts

@@ -0,0 +1,86 @@
+/**
+ * Two rows for one file, and which of them may go.
+ *
+ * Before the scan learned that `D:/Clips/x.mp4` and `D:\Clips\x.mp4` are the
+ * same file, a clip indexed by one path spelling and found again by the other
+ * got a second row. The scan stops making new ones, but it never removed an
+ * old one: the file exists, so neither row looks orphaned, and the library
+ * showed the clip twice, once with its publish state and once without.
+ *
+ * **Only a row that carries nothing is removed.** A clip row is the only copy
+ * of its tags, notes, name, stars, marks and collections, so a duplicate is
+ * retired only when everything it holds is already on the row that stays.
+ * Anything else is left, twice, and logged: a clip shown twice is an
+ * annoyance, and a clip whose notes were thrown away is a loss.
+ *
+ * The row only. The file is the same file either way and is never touched.
+ */
+export interface DuplicateCandidate {
+  id: number;
+  filePath: string;
+  displayName: string | null;
+  notes: string | null;
+  starred: boolean;
+  published: boolean;
+  openCount: number;
+  tags: number;
+  marks: number;
+  collections: number;
+}
+
+export interface DuplicatePlan {
+  /** Rows safe to delete, because their twin holds everything they do. */
+  remove: number[];
+  /** Groups left alone because more than one row carries something, for the log. */
+  kept: Array<{ path: string; ids: number[] }>;
+}
+
+const BACKSLASH = String.fromCharCode(92);
+const samePath = (value: string): string => value.split(BACKSLASH).join('/').toLowerCase();
+
+/** How much a row holds that nothing else does. Higher stays. */
+function weight(row: DuplicateCandidate): number {
+  return (
+    row.tags * 1000 +
+    row.marks * 1000 +
+    row.collections * 1000 +
+    (row.notes?.trim() ? 1000 : 0) +
+    (row.starred ? 100 : 0) +
+    (row.published ? 100 : 0) +
+    row.openCount
+  );
+}
+
+/** Whether `extra` holds anything `keeper` does not. */
+function carriesMore(extra: DuplicateCandidate, keeper: DuplicateCandidate): boolean {
+  if (extra.tags > 0 || extra.marks > 0 || extra.collections > 0) return true;
+  if (extra.notes?.trim() && extra.notes !== keeper.notes) return true;
+  if (extra.starred && !keeper.starred) return true;
+  if (extra.published && !keeper.published) return true;
+  const name = extra.displayName?.trim();
+  if (name && name !== keeper.displayName?.trim()) return true;
+  return false;
+}
+
+export function planDuplicateRows(rows: readonly DuplicateCandidate[]): DuplicatePlan {
+  const groups = new Map<string, DuplicateCandidate[]>();
+  for (const row of rows) {
+    const key = samePath(row.filePath);
+    const group = groups.get(key);
+    if (group) group.push(row);
+    else groups.set(key, [row]);
+  }
+
+  const plan: DuplicatePlan = { remove: [], kept: [] };
+  for (const [path, group] of groups) {
+    if (group.length < 2) continue;
+    // The richest row stays; the oldest breaks a tie, since it is the one
+    // anything outside the database is most likely to know about.
+    const ordered = [...group].sort((a, b) => weight(b) - weight(a) || a.id - b.id);
+    const [keeper, ...rest] = ordered;
+    const removable = rest.filter((row) => !carriesMore(row, keeper));
+    plan.remove.push(...removable.map((row) => row.id));
+    if (removable.length < rest.length) plan.kept.push({ path, ids: ordered.map((row) => row.id) });
+  }
+  return plan;
+}

src/renderer/src/App.vue

@@ -43,6 +43,7 @@ onUnmounted(() => {
     :confirm-label="pending?.confirmLabel"
     :tone="pending?.tone"
     :icon="pending?.icon"
+    :emphasis="pending?.emphasis"
     @confirm="accept"
     @cancel="cancel"
   />

src/renderer/src/components/Base/BaseConfirmDialog.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { nextTick, onBeforeUnmount, onMounted, ref, watch } from 'vue';
+import { computed, nextTick, onBeforeUnmount, onMounted, ref, watch } from 'vue';
 import { Icon } from '@iconify/vue';
 
 /**
@@ -27,6 +27,8 @@ interface Props {
   /** `danger` paints the action red. Anything that deletes or replaces. */
   tone?: 'danger' | 'normal';
   icon?: string;
+  /** A part of the description to set in bold, the first place it appears. */
+  emphasis?: string;
 }
 
 interface Emits {
@@ -40,6 +42,17 @@ const props = withDefaults(defineProps<Props>(), {
   icon: 'material-symbols:warning-rounded',
 });
 
+/** The description around its emphasised part. Plain text either side, never HTML. */
+const descriptionParts = computed(() => {
+  const at = props.emphasis ? props.description.indexOf(props.emphasis) : -1;
+  if (at < 0 || !props.emphasis) return { before: '', bold: '', after: '' };
+  return {
+    before: props.description.slice(0, at),
+    bold: props.emphasis,
+    after: props.description.slice(at + props.emphasis.length),
+  };
+});
+
 const emit = defineEmits<Emits>();
 
 /*
@@ -167,7 +180,12 @@ onBeforeUnmount(() => window.removeEventListener('keydown', onEscape, true));
                   {{ title }}
                 </h2>
                 <p class="mt-1 text-sm text-muted-500 whitespace-pre-line">
-                  {{ description }}
+                  <template v-if="descriptionParts.bold"
+                    >{{ descriptionParts.before
+                    }}<strong class="font-semibold text-foreground">{{ descriptionParts.bold }}</strong
+                    >{{ descriptionParts.after }}</template
+                  >
+                  <template v-else>{{ description }}</template>
                 </p>
               </div>
             </div>

src/renderer/src/composables/clips/useCompressionResult.ts

@@ -0,0 +1,46 @@
+import { pollJob } from '@renderer/services/jobs';
+import { useToastStore } from '@renderer/stores/toast';
+import { useClipsStore } from '@renderer/stores/clips';
+import { useFormat } from '@renderer/composables/ui/useFormat';
+import type { Clip } from '@renderer/types/clip';
+
+/**
+ * Follow a compression to its end, and say what happened.
+ *
+ * It used to say "It will finish in the background" and then nothing, so a
+ * clip that was compressed, one that was left alone because it was already
+ * small, and one whose compression failed all looked the same: the card kept
+ * its old size and nobody knew why. Now the job is watched, the card is given
+ * the new row the moment it lands, and one toast says which of the three it
+ * was, with the sizes when it worked.
+ */
+export function useCompressionResult() {
+  const toast = useToastStore();
+  const clips = useClipsStore();
+  const { formatBytes } = useFormat();
+
+  async function follow(jobId: string, before: Clip): Promise<{ outcome: 'done' | 'left' | 'failed'; clip: Clip | null }> {
+    const title = before.displayName?.trim() || before.filename;
+    try {
+      const view = await pollJob(jobId);
+      if (view.status === 'done') {
+        if (view.clip) clips.updateClip(view.clip);
+        const after = view.clip?.sizeBytes ?? null;
+        if (after !== null && after < (before.sizeBytes ?? Infinity)) {
+          toast.success(`${title}: ${formatBytes(before.sizeBytes)} to ${formatBytes(after)}`, 'Compressed');
+          return { outcome: 'done', clip: view.clip };
+        }
+        toast.info(view.message || 'It was left as it was.', title);
+        return { outcome: 'left', clip: view.clip };
+      }
+      if (view.status === 'cancelled') return { outcome: 'left', clip: null };
+      toast.error(view.error || 'The compression stopped partway. The original is untouched.', `Could not compress ${title}`);
+      return { outcome: 'failed', clip: null };
+    } catch (error) {
+      toast.error(error instanceof Error ? error.message : String(error), `Could not compress ${title}`);
+      return { outcome: 'failed', clip: null };
+    }
+  }
+
+  return { follow };
+}