From f7e1bddda68fd8370ac7d332c0b4250dd7a27166 Mon Sep 17 00:00:00 2001
From: Huang Xin <chrox.huang@gmail.com>
Date: Sat, 20 Jun 2026 11:17:27 +0800
Subject: [PATCH] fix(sync): stop re-pinning statusless books to the top of the
 library after every sync (#4677)

The sync POST handler rewrote books.updated_at = now() whenever a pushed
book's resolved reading_status differed from the server row's. A book that
was imported locally and never given a status sends reading_status:
undefined, while the server stores null, so `undefined !== null` reported a
spurious status change. The 1-day re-sync window re-pushes every recently
touched book on each sync, so the server stamped those books with a fresh,
batch-identical timestamp every cycle, floating them to the top of the
date-sorted library (and above a book the user had just read).

Normalize nullish reading_status values before comparing so a statusless
book never registers as a status change. Verified on-device via CDP:
PUSH_SENT carried the old timestamp while PUSH_RETURNED came back with a
fresh now() for exactly the statusless books.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../pages/api/sync-reading-status.test.ts     | 26 ++++++++++++++++++-
 apps/readest-app/src/pages/api/sync.ts        | 18 ++++++++++++-
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts b/apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts
index 66f0a02b..43979b5d 100644
--- a/apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts
+++ b/apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts
@@ -1,8 +1,32 @@
 import { describe, expect, it } from 'vitest';
-import { resolveReadingStatusMerge } from '@/pages/api/sync';
+import { readingStatusChanged, resolveReadingStatusMerge } from '@/pages/api/sync';
 
 const iso = (ms: number) => new Date(ms).toISOString();
 
+describe('readingStatusChanged', () => {
+  // The bug: a locally-imported book the client never gave a status to sends
+  // `reading_status: undefined`, while the server row stores `null`. Comparing
+  // them with `!==` reports a spurious change, so the server rewrites
+  // `updated_at = now()` on every push and the book floats to the top of the
+  // date-sorted library after every sync.
+  it('treats client undefined and server null as the same (no change)', () => {
+    expect(readingStatusChanged(undefined, null)).toBe(false);
+    expect(readingStatusChanged(null, undefined)).toBe(false);
+    expect(readingStatusChanged(null, null)).toBe(false);
+    expect(readingStatusChanged(undefined, undefined)).toBe(false);
+  });
+
+  it('reports a real status change', () => {
+    expect(readingStatusChanged('finished', null)).toBe(true);
+    expect(readingStatusChanged(undefined, 'reading')).toBe(true);
+    expect(readingStatusChanged('reading', 'finished')).toBe(true);
+  });
+
+  it('reports no change when both sides hold the same status', () => {
+    expect(readingStatusChanged('finished', 'finished')).toBe(false);
+  });
+});
+
 describe('resolveReadingStatusMerge', () => {
   it('keeps the client status when its status timestamp is newer', () => {
     const out = resolveReadingStatusMerge(
diff --git a/apps/readest-app/src/pages/api/sync.ts b/apps/readest-app/src/pages/api/sync.ts
index 46e21c77..a80fcb7c 100644
--- a/apps/readest-app/src/pages/api/sync.ts
+++ b/apps/readest-app/src/pages/api/sync.ts
@@ -44,6 +44,19 @@ export function pickWinningPages(
  * decided the other way by updated_at (which page-turn progress dominates) —
  * issue #4634.
  */
+/**
+ * `undefined` (the client omitted reading_status entirely — e.g. a locally
+ * imported book that never had a status set) and `null` (the DB default) both
+ * mean "no reading status". Collapse them so a statusless book never registers
+ * as a status change. Without this, the `statusChanged` branch below rewrites
+ * `updated_at = now()` on every push for such books, and since the 1-day
+ * re-sync window re-pushes recently-touched books each cycle, they get a fresh
+ * timestamp every sync and pin themselves to the top of the date-sorted
+ * library.
+ */
+export const readingStatusChanged = (client?: string | null, server?: string | null): boolean =>
+  (client ?? null) !== (server ?? null);
+
 export function resolveReadingStatusMerge(
   client: Pick<DBBook, 'reading_status' | 'reading_status_updated_at'>,
   server: Pick<DBBook, 'reading_status' | 'reading_status_updated_at'>,
@@ -419,7 +432,10 @@ export async function POST(req: NextRequest) {
               // Only rewrite when the resolved status VALUE differs from the
               // server's — a timestamp-only difference on the same value is a
               // no-op, and rewriting it would churn updated_at + re-propagate.
-              const statusChanged = status.reading_status !== serverBook.reading_status;
+              const statusChanged = readingStatusChanged(
+                status.reading_status,
+                serverBook.reading_status,
+              );
               if (statusChanged) {
                 // Server wins the row, but the client's status is newer. Write
                 // server's row with the fresher status and bump updated_at so