fix(correctness): pagination tiebreakers, dedup race, duration gating (0.87.6)

Three medium correctness fixes:

1. **Pagination ORDER BY missing tiebreakers** — four admin list queries
   sorted by a timestamp or `chapters.number` with no stable secondary
   sort. Add `id` tiebreakers across `repo::admin_view`,
   `repo::admin_audit`, `repo::bookmark`.

2. **`enqueue_pages` NOT EXISTS read-then-insert race** — concurrent
   admin clicks could land duplicate analyze_page jobs. Migration 0031
   adds a partial unique index mirroring 0014; query relies on
   `ON CONFLICT DO NOTHING`.

3. **`record_duration` overwrote a prior done row's duration on a
   non-terminal failed retry of a force-re-analyze.** Gate the call
   on "did this attempt actually write a row?".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

backend/Cargo.lock

@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
 
 [[package]]
 name = "mangalord"
-version = "0.87.5"
+version = "0.87.6"
 dependencies = [
  "anyhow",
  "argon2",

backend/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mangalord"
-version = "0.87.5"
+version = "0.87.6"
 edition = "2021"
 default-run = "mangalord"
 

backend/migrations/0031_crawler_jobs_analyze_page_dedup.sql

@@ -0,0 +1,17 @@
+-- Dedup analyze_page jobs in flight (mirrors 0014 for sync_chapter_content).
+--
+-- Without this, `repo::page_analysis::enqueue_pages`' `NOT EXISTS(... pending
+-- | running ...)` read-then-insert race let concurrent admin POSTs land
+-- duplicate jobs for the same page — visible as duplicate "now analyzing"
+-- SSE events and inflated `pending` counters in the admin dashboard, even
+-- though the worker's `skip-if-done` net (analysis/daemon.rs:210-217)
+-- prevented the actual duplicate vision call.
+--
+-- The partial unique index lets enqueue_pages drop the NOT EXISTS subquery
+-- and rely on `ON CONFLICT DO NOTHING` instead. At most one (pending|running)
+-- job per page_id can exist; the slot frees the moment the job transitions
+-- to done/failed/dead, so a force re-analyze still re-enqueues cleanly.
+CREATE UNIQUE INDEX crawler_jobs_analyze_page_dedup_idx
+    ON crawler_jobs ((payload->>'page_id'))
+ WHERE state IN ('pending', 'running')
+   AND payload->>'kind' = 'analyze_page';

backend/src/analysis/daemon.rs

@@ -313,6 +313,17 @@ impl WorkerContext {
             self.events.publish(event);
         }
 
+        // Stamp how long the vision dispatch took — but ONLY when a row
+        // was actually written this attempt. Otherwise a non-terminal
+        // failure on a force-re-analyze (page already has a `done` row)
+        // would overwrite the prior duration with the duration of a
+        // failed retry that wrote nothing new.
+        let wrote_row = match &result {
+            Ok(()) => true,
+            Err(_) if lease.attempts >= lease.max_attempts => true, // mark_failed wrote below
+            Err(_) => false,
+        };
+
         match result {
             Ok(()) => {
                 let _ = jobs::ack_done(&self.pool, lease.id).await;
@@ -341,10 +352,9 @@ impl WorkerContext {
             }
         }
 
-        // Stamp how long the vision dispatch took. Best-effort and ordered
+        if wrote_row {
-        // after the row is written (persist on success / mark_failed on
+            let _ = repo::page_analysis::record_duration(&self.pool, page_id, elapsed_ms).await;
-        // terminal failure); a transient failure with no row updates nothing.
+        }
-        let _ = repo::page_analysis::record_duration(&self.pool, page_id, elapsed_ms).await;
     }
 }
 

backend/src/repo/admin_audit.rs

@@ -60,7 +60,11 @@ pub async fn list(
            AND ($2::text IS NULL OR a.target_kind = $2)
            AND ($3::uuid IS NULL OR a.actor_user_id = $3)
            AND ($4::timestamptz IS NULL OR a.at >= $4)
-         ORDER BY a.at DESC
+         -- `id` tiebreaker for stable pagination: a single admin action
+         -- can write multiple audit rows in the same statement_timestamp
+         -- (e.g. bulk requeue) and ties on the timestamp would otherwise
+         -- skip or repeat rows across pages.
+         ORDER BY a.at DESC, a.id DESC
          LIMIT $5 OFFSET $6
         "#,
     )

backend/src/repo/admin_view.rs

@@ -239,7 +239,11 @@ pub async fn list_mangas_with_sync_state(
         )
         SELECT * FROM classified
          WHERE ($2::text IS NULL OR sync_state = $2)
-         ORDER BY updated_at DESC
+         -- `id` tiebreaker stabilises pagination across mangas that share an
+         -- `updated_at` (multi-row crawl-tick bulk updates land within the
+         -- same statement_timestamp() and tie on Postgres' default ts
+         -- resolution). Without it, page boundaries can skip or repeat rows.
+         ORDER BY updated_at DESC, id DESC
          LIMIT $3 OFFSET $4
         "#,
         case = MANGA_SYNC_STATE_CASE
@@ -332,7 +336,12 @@ pub async fn list_chapters_with_sync_state(
               WHERE cs.chapter_id = c.id AND cs.dropped_at IS NULL) AS latest_seen_at
           FROM chapters c
          WHERE c.manga_id = $1
-         ORDER BY c.number ASC
+         -- `id` tiebreaker: migration 0013 dropped the (manga_id, number)
+         -- UNIQUE constraint to allow variants ("Ch.14: PH" / "Ch.14:
+         -- Official") and non-numeric entries (all parse to 0). Without a
+         -- tiebreaker, pagination over those tied rows can skip or repeat
+         -- depending on Postgres' chosen sort order.
+         ORDER BY c.number ASC, c.id ASC
          LIMIT $2 OFFSET $3
         "#,
     )

backend/src/repo/bookmark.rs

@@ -63,7 +63,11 @@ pub async fn list_for_user(
         INNER JOIN mangas m ON m.id = b.manga_id
         LEFT JOIN chapters c ON c.id = b.chapter_id
         WHERE b.user_id = $1
-        ORDER BY b.created_at DESC
+        -- `manga_id` tiebreaker: the (user_id, manga_id) PK guarantees no
+        -- two bookmarks share both, so this is a deterministic ordering.
+        -- Without it, multiple bookmarks added in the same `created_at`
+        -- tick (timestamptz resolution: 1µs) can skip/repeat across pages.
+        ORDER BY b.created_at DESC, b.manga_id DESC
         LIMIT $2 OFFSET $3
         "#,
     )

backend/src/repo/page_analysis.rs

@@ -86,6 +86,14 @@ pub async fn enqueue_pages(
         }
         ReenqueueScope::Chapter(_) => "AND p.chapter_id = $2",
     };
+    // The `NOT EXISTS(... pending|running ...)` pre-check used to live in
+    // this query, but it races with concurrent admin clicks (two requests
+    // both see "no in-flight job" and both INSERT). The partial unique
+    // index `crawler_jobs_analyze_page_dedup_idx` (migration 0031) covers
+    // (payload->>'page_id') WHERE state IN ('pending', 'running') AND
+    // kind = 'analyze_page', so `ON CONFLICT DO NOTHING` is now the
+    // atomic primitive — duplicates can't land and concurrent enqueue
+    // calls are both observably correct.
     let sql = format!(
         r#"
         INSERT INTO crawler_jobs (payload)
@@ -95,11 +103,7 @@ pub async fn enqueue_pages(
                    SELECT 1 FROM page_analysis pa
                     WHERE pa.page_id = p.id AND pa.status = 'done'))
            {scope_clause}
-           AND NOT EXISTS (
+        ON CONFLICT DO NOTHING
-                   SELECT 1 FROM crawler_jobs j
-                    WHERE j.payload->>'kind' = 'analyze_page'
-                      AND j.payload->>'page_id' = p.id::text
-                      AND j.state IN ('pending', 'running'))
         "#
     );
     let query = sqlx::query(&sql).bind(only_unanalyzed);

frontend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "mangalord-frontend",
-  "version": "0.87.5",
+  "version": "0.87.6",
   "private": true,
   "type": "module",
   "scripts": {