fix(books): enforce read status lifecycle rules and threshold handling

server/src/db/schema/reader.ts

@@ -1,4 +1,5 @@
 import { date, index, integer, jsonb, pgTable, primaryKey, real, serial, text, timestamp, uniqueIndex, varchar } from 'drizzle-orm/pg-core';
+import type { ReadStatus, ReadStatusSource } from '@projectx/types';
 
 import { bookFiles, books } from './books';
 import { libraries } from './libraries';
@@ -14,9 +15,9 @@ export const userBookStatus = pgTable(
       .notNull()
       .references(() => books.id, { onDelete: 'cascade' }),
     // 'unread' | 'want_to_read' | 'reading' | 'on_hold' | 'rereading' | 'read' | 'skimmed' | 'abandoned'
-    status: varchar('status', { length: 20 }).notNull().default('unread'),
+    status: varchar('status', { length: 20 }).$type<ReadStatus>().notNull().default('unread'),
     // 'auto' (derived from progress) | 'manual' (user-set; never auto-overridden)
-    source: varchar('source', { length: 10 }).notNull().default('auto'),
+    source: varchar('source', { length: 10 }).$type<ReadStatusSource>().notNull().default('auto'),
     startedAt: timestamp('started_at'),
     finishedAt: timestamp('finished_at'),
     updatedAt: timestamp('updated_at')

server/src/modules/user-book-status/dto/set-status.dto.ts

@@ -1,7 +1,8 @@
 import { IsIn } from 'class-validator';
 import type { ReadStatus } from '@projectx/types';
+import { READ_STATUSES } from '../user-book-status.constants';
 
 export class SetStatusDto {
-  @IsIn(['unread', 'want_to_read', 'reading', 'on_hold', 'rereading', 'read', 'skimmed', 'abandoned'])
+  @IsIn(READ_STATUSES)
   status!: ReadStatus;
 }

server/src/modules/user-book-status/user-book-status.constants.ts

@@ -0,0 +1,16 @@
+import type { ReadStatus, ReadStatusSource } from '@projectx/types';
+
+export const READ_STATUSES: readonly ReadStatus[] = ['unread', 'want_to_read', 'reading', 'on_hold', 'rereading', 'read', 'skimmed', 'abandoned'];
+
+export const READ_STATUS_SOURCES: readonly ReadStatusSource[] = ['auto', 'manual'];
+
+const READ_STATUS_SET = new Set<ReadStatus>(READ_STATUSES);
+const READ_STATUS_SOURCE_SET = new Set<ReadStatusSource>(READ_STATUS_SOURCES);
+
+export function isReadStatus(value: string): value is ReadStatus {
+  return READ_STATUS_SET.has(value as ReadStatus);
+}
+
+export function isReadStatusSource(value: string): value is ReadStatusSource {
+  return READ_STATUS_SOURCE_SET.has(value as ReadStatusSource);
+}

server/src/modules/user-book-status/user-book-status.repository.test.ts

@@ -0,0 +1,53 @@
+import { describe, expect, it } from 'vitest';
+
+import type { UserBookStatusRow } from '../../db/schema';
+import { deriveLifecycle } from './user-book-status.repository';
+
+function makeRow(overrides: Partial<UserBookStatusRow> = {}): UserBookStatusRow {
+  return {
+    userId: 1,
+    bookId: 10,
+    status: 'reading',
+    source: 'auto',
+    startedAt: new Date('2024-01-01T00:00:00.000Z'),
+    finishedAt: new Date('2024-02-01T00:00:00.000Z'),
+    updatedAt: new Date('2024-03-01T00:00:00.000Z'),
+    ...overrides,
+  };
+}
+
+describe('deriveLifecycle', () => {
+  const now = new Date('2025-01-15T12:00:00.000Z');
+  const existingStarted = new Date('2024-04-01T10:00:00.000Z');
+  const existingFinished = new Date('2024-08-01T10:00:00.000Z');
+  const existing = makeRow({ startedAt: existingStarted, finishedAt: existingFinished, status: 'read' });
+
+  it('clears both timestamps for unread and want_to_read', () => {
+    expect(deriveLifecycle('unread', now, existing)).toEqual({ startedAt: null, finishedAt: null });
+    expect(deriveLifecycle('want_to_read', now, existing)).toEqual({ startedAt: null, finishedAt: null });
+  });
+
+  it('initializes startedAt and clears finishedAt for active statuses', () => {
+    for (const status of ['reading', 'on_hold', 'rereading'] as const) {
+      expect(deriveLifecycle(status, now, null)).toEqual({ startedAt: now, finishedAt: null });
+      expect(deriveLifecycle(status, now, existing)).toEqual({ startedAt: existingStarted, finishedAt: null });
+    }
+  });
+
+  it('sets finishedAt to now for read and keeps existing startedAt', () => {
+    expect(deriveLifecycle('read', now, null)).toEqual({ startedAt: now, finishedAt: now });
+    expect(deriveLifecycle('read', now, existing)).toEqual({ startedAt: existingStarted, finishedAt: now });
+  });
+
+  it('clears stale finishedAt for skimmed and abandoned', () => {
+    expect(deriveLifecycle('skimmed', now, existing)).toEqual({ startedAt: existingStarted, finishedAt: null });
+    expect(deriveLifecycle('abandoned', now, existing)).toEqual({ startedAt: existingStarted, finishedAt: null });
+  });
+
+  it('initializes startedAt for skimmed and abandoned when missing', () => {
+    const withoutStarted = makeRow({ startedAt: null, finishedAt: existingFinished, status: 'reading' });
+
+    expect(deriveLifecycle('skimmed', now, withoutStarted)).toEqual({ startedAt: now, finishedAt: null });
+    expect(deriveLifecycle('abandoned', now, withoutStarted)).toEqual({ startedAt: now, finishedAt: null });
+  });
+});

server/src/modules/user-book-status/user-book-status.repository.ts

@@ -6,10 +6,27 @@ import { DB } from '../../db';
 import * as schema from '../../db/schema';
 import { userBookStatus } from '../../db/schema';
 import type { ReadStatus, ReadStatusSource } from '@projectx/types';
+import type { UserBookStatusRow } from '../../db/schema';
 
 type Db = NodePgDatabase<typeof schema>;
-const STARTED_FROM_ACTIVE_STATUSES: ReadStatus[] = ['reading', 'on_hold', 'rereading'];
-const RESET_FINISHED_AT_STATUSES: ReadStatus[] = ['unread', 'want_to_read', 'reading', 'on_hold', 'rereading', 'skimmed'];
+
+export type UserBookStatusLifecycle = Pick<UserBookStatusRow, 'startedAt' | 'finishedAt'>;
+
+export function deriveLifecycle(status: ReadStatus, now: Date, existing: UserBookStatusRow | null): UserBookStatusLifecycle {
+  switch (status) {
+    case 'unread':
+    case 'want_to_read':
+      return { startedAt: null, finishedAt: null };
+    case 'reading':
+    case 'on_hold':
+    case 'rereading':
+    case 'skimmed':
+    case 'abandoned':
+      return { startedAt: existing?.startedAt ?? now, finishedAt: null };
+    case 'read':
+      return { startedAt: existing?.startedAt ?? now, finishedAt: now };
+  }
+}
 
 @Injectable()
 export class UserBookStatusRepository {
@@ -41,9 +58,7 @@ export class UserBookStatusRepository {
     existing?: Awaited<ReturnType<typeof this.findOne>>,
   ): Promise<void> {
     const row = existing !== undefined ? existing : await this.findOne(userId, bookId);
-
-    const startedAt = row?.startedAt ?? (STARTED_FROM_ACTIVE_STATUSES.includes(status) ? now : null);
-    const finishedAt = status === 'read' ? now : RESET_FINISHED_AT_STATUSES.includes(status) ? null : (row?.finishedAt ?? null);
+    const { startedAt, finishedAt } = deriveLifecycle(status, now, row);
 
     await this.db
       .insert(userBookStatus)

server/src/modules/user-book-status/user-book-status.service.test.ts

@@ -1,6 +1,10 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest';
-import { UserBookStatusService } from './user-book-status.service';
+import { InternalServerErrorException } from '@nestjs/common';
+import type { ReadStatus, ReadStatusSource } from '@projectx/types';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
 import type { UserBookStatusRow } from '../../db/schema';
+import { UserBookStatusRepository } from './user-book-status.repository';
+import { UserBookStatusService } from './user-book-status.service';
 
 function makeRow(overrides: Partial<UserBookStatusRow> = {}): UserBookStatusRow {
   return {
@@ -16,140 +20,155 @@ function makeRow(overrides: Partial<UserBookStatusRow> = {}): UserBookStatusRow
 }
 
 const mockRepo = {
-  findOne: vi.fn(),
-  findByBookIds: vi.fn(),
-  upsert: vi.fn(),
+  findOne: vi.fn<(...args: [number, number]) => Promise<UserBookStatusRow | null>>(),
+  findByBookIds: vi.fn<(...args: [number, number[]]) => Promise<UserBookStatusRow[]>>(),
+  upsert: vi.fn<(...args: [number, number, ReadStatus, ReadStatusSource, Date, (UserBookStatusRow | null)?]) => Promise<void>>(),
 };
 
 let service: UserBookStatusService;
 
 beforeEach(() => {
   vi.clearAllMocks();
+  mockRepo.findOne.mockResolvedValue(null);
+  mockRepo.findByBookIds.mockResolvedValue([]);
   mockRepo.upsert.mockResolvedValue(undefined);
-  service = new UserBookStatusService(mockRepo as any);
+  service = new UserBookStatusService(mockRepo as unknown as UserBookStatusRepository);
 });
 
 describe('setManual', () => {
-  it('calls upsert with source manual and correct args', async () => {
+  it('calls upsert with manual source', async () => {
     await service.setManual(1, 10, 'reading');
+
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    const [userId, bookId, status, source] = mockRepo.upsert.mock.calls[0];
+    const [userId, bookId, status, source, now] = mockRepo.upsert.mock.calls[0];
     expect(userId).toBe(1);
     expect(bookId).toBe(10);
     expect(status).toBe('reading');
     expect(source).toBe('manual');
-  });
-
-  it('passes a Date instance for now', async () => {
-    await service.setManual(1, 10, 'read');
-    const now = mockRepo.upsert.mock.calls[0][4];
     expect(now).toBeInstanceOf(Date);
   });
 });
 
-describe('autoUpdate — default thresholds', () => {
-  it('does not call upsert when no existing record and percentage is below reading threshold', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 0.1);
-    expect(mockRepo.upsert).not.toHaveBeenCalled();
-  });
+describe('autoUpdate with default thresholds', () => {
+  it.each([
+    { percentage: 0.1, expectedStatus: null },
+    { percentage: 0.25, expectedStatus: 'reading' },
+    { percentage: 50, expectedStatus: 'reading' },
+    { percentage: 98, expectedStatus: 'read' },
+    { percentage: 100, expectedStatus: 'read' },
+  ])('derives expected status for percentage=$percentage', async ({ percentage, expectedStatus }) => {
+    await service.autoUpdate(1, 10, percentage);
+
+    if (expectedStatus === null) {
+      expect(mockRepo.upsert).not.toHaveBeenCalled();
+      return;
+    }
 
-  it('calls upsert with reading when no existing record and percentage equals reading threshold', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 0.25);
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading');
+    expect(mockRepo.upsert.mock.calls[0][2]).toBe(expectedStatus);
   });
+});
 
-  it('calls upsert with reading when no existing record and percentage is above reading but below finish', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 50);
-    expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading');
-  });
+describe('autoUpdate with custom thresholds', () => {
+  it.each([
+    { percentage: 0.5, readingThreshold: 1, finishThreshold: 90, existingStatus: 'reading', expectedStatus: 'unread' },
+    { percentage: 1, readingThreshold: 1, finishThreshold: 90, existingStatus: 'unread', expectedStatus: 'reading' },
+    { percentage: 90, readingThreshold: 1, finishThreshold: 90, existingStatus: 'reading', expectedStatus: 'read' },
+  ])(
+    'derives expected status for percentage=$percentage',
+    async ({ percentage, readingThreshold, finishThreshold, existingStatus, expectedStatus }) => {
+      mockRepo.findOne.mockResolvedValue(makeRow({ status: existingStatus, source: 'auto' }));
 
-  it('calls upsert with read when no existing record and percentage equals finish threshold', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 98);
-    expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('read');
-  });
+      await service.autoUpdate(1, 10, percentage, readingThreshold, finishThreshold);
+
+      expect(mockRepo.upsert).toHaveBeenCalledOnce();
+      expect(mockRepo.upsert.mock.calls[0][2]).toBe(expectedStatus);
+    },
+  );
+
+  it('falls back to defaults for null and undefined thresholds', async () => {
+    await service.autoUpdate(1, 10, 98, null, undefined);
 
-  it('calls upsert with read when no existing record and percentage is above finish threshold', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 100);
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
     expect(mockRepo.upsert.mock.calls[0][2]).toBe('read');
   });
 });
 
-describe('autoUpdate — custom thresholds', () => {
-  it('derives unread when percentage is below custom reading threshold', async () => {
+describe('autoUpdate normalization and guard behavior', () => {
+  it('does not override manual statuses', async () => {
+    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'manual' }));
+
+    await service.autoUpdate(1, 10, 100);
+
+    expect(mockRepo.upsert).not.toHaveBeenCalled();
+  });
+
+  it('skips updates when derived status is unchanged', async () => {
     mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' }));
-    await service.autoUpdate(1, 10, 0.5, 1, 90);
+
+    await service.autoUpdate(1, 10, 50);
+
+    expect(mockRepo.upsert).not.toHaveBeenCalled();
+  });
+
+  it('passes existing row to upsert for lifecycle derivation', async () => {
+    const existing = makeRow({ status: 'reading', source: 'auto' });
+    mockRepo.findOne.mockResolvedValue(existing);
+
+    await service.autoUpdate(1, 10, 99);
+
+    expect(mockRepo.upsert).toHaveBeenCalledOnce();
+    expect(mockRepo.upsert.mock.calls[0][5]).toBe(existing);
+  });
+
+  it('clamps percentage values outside 0..100', async () => {
+    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' }));
+
+    await service.autoUpdate(1, 10, 120);
+    expect(mockRepo.upsert).toHaveBeenCalledOnce();
+    expect(mockRepo.upsert.mock.calls[0][2]).toBe('read');
+
+    vi.clearAllMocks();
+    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'read', source: 'auto' }));
+    mockRepo.upsert.mockResolvedValue(undefined);
+
+    await service.autoUpdate(1, 10, -12);
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
     expect(mockRepo.upsert.mock.calls[0][2]).toBe('unread');
   });
 
-  it('derives reading when percentage equals custom reading threshold', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 1.0, 1, 90);
+  it('treats non-finite percentages as 0', async () => {
+    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' }));
+
+    await service.autoUpdate(1, 10, Number.NaN);
+
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading');
+    expect(mockRepo.upsert.mock.calls[0][2]).toBe('unread');
   });
 
-  it('derives read when percentage equals custom finish threshold', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 90, 1, 90);
-    expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('read');
-  });
-
-  it('falls back to default thresholds when null is passed', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 0.25, null, null);
-    expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading');
-  });
-
-  it('falls back to default thresholds when undefined is passed', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
-    await service.autoUpdate(1, 10, 98, undefined, undefined);
-    expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('read');
-  });
-});
-
-describe('autoUpdate — existing record logic', () => {
-  it('calls upsert when existing auto record is unread and derived is reading', async () => {
+  it('falls back to default thresholds for non-finite values', async () => {
     mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'auto' }));
-    await service.autoUpdate(1, 10, 50);
+
+    await service.autoUpdate(1, 10, 50, Number.NaN, Number.POSITIVE_INFINITY);
+
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
     expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading');
   });
 
-  it('does not call upsert when existing auto record matches derived status', async () => {
-    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' }));
-    await service.autoUpdate(1, 10, 50);
-    expect(mockRepo.upsert).not.toHaveBeenCalled();
-  });
+  it('clamps custom thresholds into the 0..100 range', async () => {
+    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'auto' }));
+
+    await service.autoUpdate(1, 10, 50, -5, 150);
 
-  it('calls upsert when existing auto record is reading and derived is read', async () => {
-    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' }));
-    await service.autoUpdate(1, 10, 99);
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
-    expect(mockRepo.upsert.mock.calls[0][2]).toBe('read');
+    expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading');
   });
 
-  it('does not call upsert when existing record source is manual', async () => {
-    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'manual' }));
-    await service.autoUpdate(1, 10, 100);
-    expect(mockRepo.upsert).not.toHaveBeenCalled();
-  });
+  it('normalizes inverted thresholds to keep read threshold <= finish threshold', async () => {
+    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' }));
+
+    await service.autoUpdate(1, 10, 79, 90, 80);
 
-  it('calls upsert with unread when existing auto read record drops below reading threshold', async () => {
-    mockRepo.findOne.mockResolvedValue(makeRow({ status: 'read', source: 'auto' }));
-    await service.autoUpdate(1, 10, 0.1);
     expect(mockRepo.upsert).toHaveBeenCalledOnce();
     expect(mockRepo.upsert.mock.calls[0][2]).toBe('unread');
   });
@@ -157,17 +176,18 @@ describe('autoUpdate — existing record logic', () => {
 
 describe('findOne', () => {
   it('returns null when repo returns null', async () => {
-    mockRepo.findOne.mockResolvedValue(null);
     const result = await service.findOne(1, 10);
     expect(result).toBeNull();
   });
 
-  it('maps row to DTO with all fields as ISO strings', async () => {
+  it('maps valid rows into DTO shape', async () => {
     const started = new Date('2024-03-01T08:00:00.000Z');
     const finished = new Date('2024-06-01T12:00:00.000Z');
     const updated = new Date('2024-06-01T12:00:00.000Z');
     mockRepo.findOne.mockResolvedValue(makeRow({ status: 'read', source: 'manual', startedAt: started, finishedAt: finished, updatedAt: updated }));
+
     const result = await service.findOne(1, 10);
+
     expect(result).toEqual({
       status: 'read',
       source: 'manual',
@@ -177,45 +197,51 @@ describe('findOne', () => {
     });
   });
 
-  it('returns null startedAt and finishedAt when they are null on the row', async () => {
-    mockRepo.findOne.mockResolvedValue(makeRow({ startedAt: null, finishedAt: null }));
-    const result = await service.findOne(1, 10);
-    expect(result?.startedAt).toBeNull();
-    expect(result?.finishedAt).toBeNull();
+  it('throws when row status is invalid', async () => {
+    mockRepo.findOne.mockResolvedValue(
+      makeRow({
+        status: 'not_a_status' as unknown as UserBookStatusRow['status'],
+      }),
+    );
+
+    await expect(service.findOne(1, 10)).rejects.toThrowError(InternalServerErrorException);
+  });
+
+  it('throws when row source is invalid', async () => {
+    mockRepo.findOne.mockResolvedValue(
+      makeRow({
+        source: 'not_a_source' as unknown as UserBookStatusRow['source'],
+      }),
+    );
+
+    await expect(service.findOne(1, 10)).rejects.toThrowError(InternalServerErrorException);
   });
 });
 
 describe('findByBookIds', () => {
-  it('returns empty Map when repo returns empty array', async () => {
-    mockRepo.findByBookIds.mockResolvedValue([]);
+  it('returns empty map when repo returns no rows', async () => {
     const result = await service.findByBookIds(1, []);
     expect(result.size).toBe(0);
   });
 
-  it('maps a single row correctly into the Map', async () => {
-    const updated = new Date('2024-05-01T00:00:00.000Z');
-    mockRepo.findByBookIds.mockResolvedValue([makeRow({ bookId: 10, status: 'reading', source: 'auto', updatedAt: updated })]);
-    const result = await service.findByBookIds(1, [10]);
-    expect(result.size).toBe(1);
-    expect(result.get(10)).toEqual({
-      status: 'reading',
-      source: 'auto',
-      startedAt: null,
-      finishedAt: null,
-      updatedAt: updated.toISOString(),
-    });
-  });
-
-  it('maps multiple rows keyed by bookId', async () => {
+  it('maps rows keyed by bookId', async () => {
     const updated1 = new Date('2024-05-01T00:00:00.000Z');
     const updated2 = new Date('2024-06-01T00:00:00.000Z');
     mockRepo.findByBookIds.mockResolvedValue([
       makeRow({ bookId: 10, status: 'reading', source: 'auto', updatedAt: updated1 }),
       makeRow({ bookId: 20, status: 'read', source: 'manual', updatedAt: updated2 }),
     ]);
+
     const result = await service.findByBookIds(1, [10, 20]);
+
     expect(result.size).toBe(2);
-    expect(result.get(10)?.status).toBe('reading');
+    expect(result.get(10)).toEqual({
+      status: 'reading',
+      source: 'auto',
+      startedAt: null,
+      finishedAt: null,
+      updatedAt: updated1.toISOString(),
+    });
     expect(result.get(20)?.status).toBe('read');
   });
 });

server/src/modules/user-book-status/user-book-status.service.ts

@@ -1,10 +1,13 @@
-import { Injectable } from '@nestjs/common';
+import { Injectable, InternalServerErrorException } from '@nestjs/common';
 import type { ReadStatus, UserBookStatus } from '@projectx/types';
 import { UserBookStatusRepository } from './user-book-status.repository';
 import type { UserBookStatusRow } from '../../db/schema';
+import { isReadStatus, isReadStatusSource } from './user-book-status.constants';
 
 const DEFAULT_FINISH_THRESHOLD = 98;
 const READING_THRESHOLD = 0.25;
+const MIN_PERCENTAGE = 0;
+const MAX_PERCENTAGE = 100;
 
 @Injectable()
 export class UserBookStatusService {
@@ -25,9 +28,10 @@ export class UserBookStatusService {
 
     if (existing?.source === 'manual') return;
 
-    const readTh = readingThreshold ?? READING_THRESHOLD;
-    const finishTh = finishThreshold ?? DEFAULT_FINISH_THRESHOLD;
-    const derived: ReadStatus = percentage >= finishTh ? 'read' : percentage >= readTh ? 'reading' : 'unread';
+    const normalizedPercentage = this.normalizePercentage(percentage);
+    const { readThreshold, finishThreshold: normalizedFinishThreshold } = this.normalizeThresholds(readingThreshold, finishThreshold);
+    const derived: ReadStatus =
+      normalizedPercentage >= normalizedFinishThreshold ? 'read' : normalizedPercentage >= readThreshold ? 'reading' : 'unread';
 
     if (!existing && derived === 'unread') return;
     if (existing?.status === derived) return;
@@ -50,12 +54,48 @@ export class UserBookStatusService {
   }
 
   private toDto(row: UserBookStatusRow): UserBookStatus {
+    const status = this.toReadStatus(row.status);
+    const source = this.toReadStatusSource(row.source);
+
     return {
-      status: row.status as ReadStatus,
-      source: row.source as 'auto' | 'manual',
+      status,
+      source,
       startedAt: row.startedAt?.toISOString() ?? null,
       finishedAt: row.finishedAt?.toISOString() ?? null,
       updatedAt: row.updatedAt.toISOString(),
     };
   }
+
+  private normalizePercentage(value: number): number {
+    if (!Number.isFinite(value)) return MIN_PERCENTAGE;
+    return Math.min(MAX_PERCENTAGE, Math.max(MIN_PERCENTAGE, value));
+  }
+
+  private normalizeThresholds(readingThreshold?: number | null, finishThreshold?: number | null) {
+    const normalizedReading = this.normalizeThreshold(readingThreshold, READING_THRESHOLD);
+    const normalizedFinish = this.normalizeThreshold(finishThreshold, DEFAULT_FINISH_THRESHOLD);
+    if (normalizedReading <= normalizedFinish) {
+      return { readThreshold: normalizedReading, finishThreshold: normalizedFinish };
+    }
+    return { readThreshold: normalizedFinish, finishThreshold: normalizedFinish };
+  }
+
+  private normalizeThreshold(value: number | null | undefined, fallback: number): number {
+    if (value == null || !Number.isFinite(value)) return fallback;
+    return Math.min(MAX_PERCENTAGE, Math.max(MIN_PERCENTAGE, value));
+  }
+
+  private toReadStatus(value: string): ReadStatus {
+    if (!isReadStatus(value)) {
+      throw new InternalServerErrorException(`Invalid read status value: ${value}`);
+    }
+    return value;
+  }
+
+  private toReadStatusSource(value: string): 'auto' | 'manual' {
+    if (!isReadStatusSource(value)) {
+      throw new InternalServerErrorException(`Invalid read status source value: ${value}`);
+    }
+    return value;
+  }
 }