mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
refactor(books): rework recommendation scoring and session validation
This commit is contained in:
@@ -10,7 +10,7 @@ export class ReadingSessionController {
|
||||
constructor(private readonly service: ReadingSessionService) {}
|
||||
|
||||
@Post(':fileId/sessions')
|
||||
@HttpCode(202)
|
||||
@HttpCode(204)
|
||||
async saveSession(@Param('fileId', ParseIntPipe) fileId: number, @Body() dto: SaveReadingSessionDto, @CurrentUser() user: RequestUser) {
|
||||
await this.service.save(fileId, dto, user);
|
||||
}
|
||||
|
||||
@@ -9,6 +9,5 @@ import { ReadingSessionService } from './reading-session.service';
|
||||
imports: [BookModule],
|
||||
controllers: [ReadingSessionController],
|
||||
providers: [ReadingSessionService, ReadingSessionRepository],
|
||||
exports: [ReadingSessionRepository],
|
||||
})
|
||||
export class ReadingSessionModule {}
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { readingSessions, userReadingDailyStats } from '../../db/schema';
|
||||
import { ReadingSessionRepository } from './reading-session.repository';
|
||||
|
||||
function makeDbHarness(options?: { fileLibraryId?: number | null; insertedIds?: Array<{ id: number }> }) {
|
||||
const fileLibraryId = options?.fileLibraryId === undefined ? 11 : options.fileLibraryId;
|
||||
const insertedIds = options?.insertedIds ?? [{ id: 1 }];
|
||||
|
||||
const limit = vi.fn().mockResolvedValue(fileLibraryId == null ? [] : [{ libraryId: fileLibraryId }]);
|
||||
const where = vi.fn().mockReturnValue({ limit });
|
||||
const innerJoin = vi.fn().mockReturnValue({ where });
|
||||
const from = vi.fn().mockReturnValue({ innerJoin });
|
||||
const select = vi.fn().mockReturnValue({ from });
|
||||
|
||||
const sessionReturning = vi.fn().mockResolvedValue(insertedIds);
|
||||
const sessionConflict = vi.fn().mockReturnValue({ returning: sessionReturning });
|
||||
const sessionValues = vi.fn().mockReturnValue({ onConflictDoNothing: sessionConflict });
|
||||
|
||||
const dailyConflictUpdate = vi.fn().mockResolvedValue(undefined);
|
||||
const dailyValues = vi.fn().mockReturnValue({ onConflictDoUpdate: dailyConflictUpdate });
|
||||
|
||||
const tx = {
|
||||
insert: vi.fn((table: unknown) => {
|
||||
if (table === readingSessions) return { values: sessionValues };
|
||||
if (table === userReadingDailyStats) return { values: dailyValues };
|
||||
throw new Error('Unexpected table in insert');
|
||||
}),
|
||||
};
|
||||
|
||||
const transaction = vi.fn(async (callback: (trx: typeof tx) => Promise<unknown>) => callback(tx));
|
||||
|
||||
const db = { select, transaction };
|
||||
const repo = new ReadingSessionRepository(db as never);
|
||||
|
||||
return {
|
||||
repo,
|
||||
select,
|
||||
transaction,
|
||||
sessionValues,
|
||||
sessionConflict,
|
||||
sessionReturning,
|
||||
dailyValues,
|
||||
dailyConflictUpdate,
|
||||
};
|
||||
}
|
||||
|
||||
describe('ReadingSessionRepository', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('skips when duration is below the minimum threshold', async () => {
|
||||
const { repo, select, transaction } = makeDbHarness();
|
||||
|
||||
const result = await repo.saveSession(1, 2, 'short', new Date('2026-04-15T10:00:00.000Z'), new Date('2026-04-15T10:00:09.000Z'), 9, null, null);
|
||||
|
||||
expect(result).toEqual({ kind: 'skipped', reason: 'duration_below_minimum' });
|
||||
expect(select).not.toHaveBeenCalled();
|
||||
expect(transaction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips when no book file row is found', async () => {
|
||||
const { repo, transaction } = makeDbHarness({ fileLibraryId: null });
|
||||
|
||||
const result = await repo.saveSession(
|
||||
1,
|
||||
9999,
|
||||
'missing-file',
|
||||
new Date('2026-04-15T10:00:00.000Z'),
|
||||
new Date('2026-04-15T10:00:20.000Z'),
|
||||
20,
|
||||
null,
|
||||
null,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ kind: 'skipped', reason: 'book_file_not_found' });
|
||||
expect(transaction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips when session id already exists (idempotent duplicate)', async () => {
|
||||
const { repo, dailyValues } = makeDbHarness({ insertedIds: [] });
|
||||
|
||||
const result = await repo.saveSession(
|
||||
5,
|
||||
8,
|
||||
'duplicate-id',
|
||||
new Date('2026-04-15T10:00:00.000Z'),
|
||||
new Date('2026-04-15T10:01:00.000Z'),
|
||||
60,
|
||||
3.2,
|
||||
12.5,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ kind: 'skipped', reason: 'duplicate_session_id' });
|
||||
expect(dailyValues).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('persists session and upserts daily stats when insert succeeds', async () => {
|
||||
const { repo, sessionValues, dailyValues, dailyConflictUpdate } = makeDbHarness({ fileLibraryId: 3, insertedIds: [{ id: 99 }] });
|
||||
|
||||
const result = await repo.saveSession(
|
||||
2,
|
||||
4,
|
||||
'new-session',
|
||||
new Date('2026-04-15T10:00:00.000Z'),
|
||||
new Date('2026-04-15T10:01:00.000Z'),
|
||||
60,
|
||||
null,
|
||||
42.5,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ kind: 'saved' });
|
||||
expect(sessionValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
userId: 2,
|
||||
bookFileId: 4,
|
||||
sessionId: 'new-session',
|
||||
durationSeconds: 60,
|
||||
progressDelta: null,
|
||||
endProgress: 42.5,
|
||||
}),
|
||||
);
|
||||
expect(dailyValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
userId: 2,
|
||||
libraryId: 3,
|
||||
readingSeconds: 60,
|
||||
progressDelta: 0,
|
||||
sessionsCount: 1,
|
||||
updatedAt: expect.any(Date),
|
||||
}),
|
||||
);
|
||||
expect(dailyValues.mock.calls[0]?.[0]).toHaveProperty('day');
|
||||
expect(dailyConflictUpdate).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
@@ -8,7 +8,14 @@ import { bookFiles, books, readingSessions, userReadingDailyStats } from '../../
|
||||
|
||||
type Db = NodePgDatabase<typeof schema>;
|
||||
|
||||
const MIN_SESSION_SECONDS = 10;
|
||||
const MIN_READING_SESSION_SECONDS = 10;
|
||||
|
||||
export type SaveReadingSessionResult =
|
||||
| { kind: 'saved' }
|
||||
| {
|
||||
kind: 'skipped';
|
||||
reason: 'duration_below_minimum' | 'book_file_not_found' | 'duplicate_session_id';
|
||||
};
|
||||
|
||||
@Injectable()
|
||||
export class ReadingSessionRepository {
|
||||
@@ -23,8 +30,10 @@ export class ReadingSessionRepository {
|
||||
durationSeconds: number,
|
||||
progressDelta: number | null,
|
||||
endProgress: number | null,
|
||||
): Promise<void> {
|
||||
if (durationSeconds < MIN_SESSION_SECONDS) return;
|
||||
): Promise<SaveReadingSessionResult> {
|
||||
if (durationSeconds < MIN_READING_SESSION_SECONDS) {
|
||||
return { kind: 'skipped', reason: 'duration_below_minimum' };
|
||||
}
|
||||
|
||||
const [fileRow] = await this.db
|
||||
.select({ libraryId: books.libraryId })
|
||||
@@ -33,26 +42,29 @@ export class ReadingSessionRepository {
|
||||
.where(eq(bookFiles.id, bookFileId))
|
||||
.limit(1);
|
||||
|
||||
if (!fileRow) return;
|
||||
if (!fileRow) {
|
||||
return { kind: 'skipped', reason: 'book_file_not_found' };
|
||||
}
|
||||
|
||||
const { libraryId } = fileRow;
|
||||
const day = startedAt.toISOString().slice(0, 10);
|
||||
|
||||
await this.db.transaction(async (tx) => {
|
||||
return this.db.transaction(async (tx): Promise<SaveReadingSessionResult> => {
|
||||
const inserted = await tx
|
||||
.insert(readingSessions)
|
||||
.values({ userId, bookFileId, sessionId, startedAt, endedAt, durationSeconds, progressDelta, endProgress })
|
||||
.onConflictDoNothing({ target: [readingSessions.sessionId] })
|
||||
.returning({ id: readingSessions.id });
|
||||
|
||||
if (inserted.length === 0) return;
|
||||
if (inserted.length === 0) {
|
||||
return { kind: 'skipped', reason: 'duplicate_session_id' };
|
||||
}
|
||||
|
||||
await tx
|
||||
.insert(userReadingDailyStats)
|
||||
.values({
|
||||
userId,
|
||||
libraryId,
|
||||
day,
|
||||
day: sql<string>`date_trunc('day', ${startedAt}::timestamp)::date`,
|
||||
readingSeconds: durationSeconds,
|
||||
progressDelta: progressDelta ?? 0,
|
||||
sessionsCount: 1,
|
||||
@@ -67,41 +79,8 @@ export class ReadingSessionRepository {
|
||||
updatedAt: new Date(),
|
||||
},
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
async recomputeRecentDailyStats(since: Date): Promise<{ deleted: number; inserted: number }> {
|
||||
const sinceDay = since.toISOString().slice(0, 10);
|
||||
|
||||
return this.db.transaction(async (tx) => {
|
||||
const deleteResult = await tx.execute(sql`delete from user_reading_daily_stats where day >= ${sinceDay}::date`);
|
||||
|
||||
const insertResult = await tx.execute(sql`
|
||||
insert into user_reading_daily_stats (user_id, library_id, day, reading_seconds, progress_delta, sessions_count, updated_at)
|
||||
select
|
||||
rs.user_id,
|
||||
b.library_id,
|
||||
date_trunc('day', rs.started_at)::date as day,
|
||||
coalesce(sum(rs.duration_seconds), 0)::int as reading_seconds,
|
||||
coalesce(sum(rs.progress_delta), 0)::real as progress_delta,
|
||||
count(*)::int as sessions_count,
|
||||
now() as updated_at
|
||||
from reading_sessions rs
|
||||
inner join book_files bf on bf.id = rs.book_file_id
|
||||
inner join books b on b.id = bf.book_id
|
||||
where rs.started_at >= ${since}
|
||||
group by rs.user_id, b.library_id, date_trunc('day', rs.started_at)::date
|
||||
on conflict (user_id, library_id, day) do update set
|
||||
reading_seconds = excluded.reading_seconds,
|
||||
progress_delta = excluded.progress_delta,
|
||||
sessions_count = excluded.sessions_count,
|
||||
updated_at = excluded.updated_at
|
||||
`);
|
||||
|
||||
return {
|
||||
deleted: Number((deleteResult as { rowCount?: number }).rowCount ?? 0),
|
||||
inserted: Number((insertResult as { rowCount?: number }).rowCount ?? 0),
|
||||
};
|
||||
return { kind: 'saved' };
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,169 @@
|
||||
import { BadRequestException, ForbiddenException, Logger } from '@nestjs/common';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { BookService } from '../book/book.service';
|
||||
import { ReadingSessionRepository, type SaveReadingSessionResult } from './reading-session.repository';
|
||||
import { ReadingSessionService } from './reading-session.service';
|
||||
|
||||
function makeUser(overrides?: Partial<RequestUser>): RequestUser {
|
||||
return {
|
||||
id: 7,
|
||||
username: 'reader',
|
||||
name: 'Reader',
|
||||
email: null,
|
||||
active: true,
|
||||
isSuperuser: false,
|
||||
isDefaultPassword: false,
|
||||
tokenVersion: 1,
|
||||
settings: {},
|
||||
avatarUrl: null,
|
||||
provisioningMethod: 'local',
|
||||
permissions: [],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
const mockRepo = {
|
||||
saveSession: vi.fn<(...args: [number, number, string, Date, Date, number, number | null, number | null]) => Promise<SaveReadingSessionResult>>(),
|
||||
};
|
||||
|
||||
const mockBookService = {
|
||||
verifyFileAccess: vi.fn<(...args: [number, RequestUser]) => Promise<void>>(),
|
||||
};
|
||||
|
||||
describe('ReadingSessionService', () => {
|
||||
let service: ReadingSessionService;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockRepo.saveSession.mockResolvedValue({ kind: 'saved' });
|
||||
mockBookService.verifyFileAccess.mockResolvedValue(undefined);
|
||||
vi.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined);
|
||||
vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
|
||||
service = new ReadingSessionService(mockRepo as unknown as ReadingSessionRepository, mockBookService as unknown as BookService);
|
||||
});
|
||||
|
||||
it('verifies access and persists a session with wall-clock clamped duration', async () => {
|
||||
await service.save(
|
||||
42,
|
||||
{
|
||||
sessionId: 'session-1',
|
||||
startedAt: '2026-04-15T10:00:00.000Z',
|
||||
endedAt: '2026-04-15T10:02:00.000Z',
|
||||
durationSeconds: 999,
|
||||
progressDelta: 2.5,
|
||||
endProgress: 10,
|
||||
},
|
||||
makeUser({ id: 12 }),
|
||||
);
|
||||
|
||||
expect(mockBookService.verifyFileAccess).toHaveBeenCalledWith(42, expect.objectContaining({ id: 12 }));
|
||||
expect(mockRepo.saveSession).toHaveBeenCalledWith(
|
||||
12,
|
||||
42,
|
||||
'session-1',
|
||||
new Date('2026-04-15T10:00:00.000Z'),
|
||||
new Date('2026-04-15T10:02:00.000Z'),
|
||||
120,
|
||||
2.5,
|
||||
10,
|
||||
);
|
||||
});
|
||||
|
||||
it('passes nullable progress values through as null', async () => {
|
||||
await service.save(
|
||||
42,
|
||||
{
|
||||
sessionId: 'session-2',
|
||||
startedAt: '2026-04-15T10:00:00.000Z',
|
||||
endedAt: '2026-04-15T10:00:30.000Z',
|
||||
durationSeconds: 30,
|
||||
progressDelta: null,
|
||||
endProgress: null,
|
||||
},
|
||||
makeUser({ id: 21 }),
|
||||
);
|
||||
|
||||
expect(mockRepo.saveSession).toHaveBeenCalledWith(
|
||||
21,
|
||||
42,
|
||||
'session-2',
|
||||
new Date('2026-04-15T10:00:00.000Z'),
|
||||
new Date('2026-04-15T10:00:30.000Z'),
|
||||
30,
|
||||
null,
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects sessions where endedAt is before startedAt', async () => {
|
||||
await expect(
|
||||
service.save(
|
||||
42,
|
||||
{
|
||||
sessionId: 'bad-order',
|
||||
startedAt: '2026-04-15T10:00:30.000Z',
|
||||
endedAt: '2026-04-15T10:00:00.000Z',
|
||||
durationSeconds: 10,
|
||||
},
|
||||
makeUser(),
|
||||
),
|
||||
).rejects.toThrow(BadRequestException);
|
||||
|
||||
expect(mockRepo.saveSession).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects invalid timestamp payloads', async () => {
|
||||
await expect(
|
||||
service.save(
|
||||
42,
|
||||
{
|
||||
sessionId: 'bad-date',
|
||||
startedAt: 'not-a-date',
|
||||
endedAt: '2026-04-15T10:00:00.000Z',
|
||||
durationSeconds: 10,
|
||||
},
|
||||
makeUser(),
|
||||
),
|
||||
).rejects.toThrow(BadRequestException);
|
||||
|
||||
expect(mockRepo.saveSession).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not persist when access verification fails', async () => {
|
||||
mockBookService.verifyFileAccess.mockRejectedValueOnce(new ForbiddenException());
|
||||
|
||||
await expect(
|
||||
service.save(
|
||||
42,
|
||||
{
|
||||
sessionId: 'forbidden',
|
||||
startedAt: '2026-04-15T10:00:00.000Z',
|
||||
endedAt: '2026-04-15T10:00:20.000Z',
|
||||
durationSeconds: 20,
|
||||
},
|
||||
makeUser(),
|
||||
),
|
||||
).rejects.toThrow(ForbiddenException);
|
||||
|
||||
expect(mockRepo.saveSession).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not throw when repository reports skipped outcomes', async () => {
|
||||
mockRepo.saveSession.mockResolvedValueOnce({ kind: 'skipped', reason: 'duration_below_minimum' });
|
||||
|
||||
await expect(
|
||||
service.save(
|
||||
42,
|
||||
{
|
||||
sessionId: 'skip',
|
||||
startedAt: '2026-04-15T10:00:00.000Z',
|
||||
endedAt: '2026-04-15T10:00:05.000Z',
|
||||
durationSeconds: 5,
|
||||
},
|
||||
makeUser(),
|
||||
),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { BadRequestException, Injectable, Logger } from '@nestjs/common';
|
||||
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { BookService } from '../book/book.service';
|
||||
@@ -7,31 +7,59 @@ import { ReadingSessionRepository } from './reading-session.repository';
|
||||
|
||||
@Injectable()
|
||||
export class ReadingSessionService {
|
||||
private readonly logger = new Logger(ReadingSessionService.name);
|
||||
|
||||
constructor(
|
||||
private readonly repo: ReadingSessionRepository,
|
||||
private readonly bookService: BookService,
|
||||
) {}
|
||||
|
||||
async save(fileId: number, dto: SaveReadingSessionDto, user: RequestUser): Promise<void> {
|
||||
await this.bookService.verifyFileAccess(fileId, user);
|
||||
|
||||
const startedAt = new Date(dto.startedAt);
|
||||
const endedAt = new Date(dto.endedAt);
|
||||
const wallClockSeconds = Math.max(0, Math.floor((endedAt.getTime() - startedAt.getTime()) / 1000));
|
||||
|
||||
// Client sends active reading time (excludes idle/hidden periods); cap it at the wall-clock
|
||||
// span to prevent the client from reporting more time than physically elapsed.
|
||||
const durationSeconds = Math.min(dto.durationSeconds, wallClockSeconds);
|
||||
|
||||
await this.repo.saveSession(
|
||||
user.id,
|
||||
fileId,
|
||||
dto.sessionId,
|
||||
startedAt,
|
||||
endedAt,
|
||||
durationSeconds,
|
||||
dto.progressDelta ?? null,
|
||||
dto.endProgress ?? null,
|
||||
const event = 'reading_session.save';
|
||||
const startedAtMs = Date.now();
|
||||
this.logger.log(
|
||||
`[${event}] [start] fileId=${fileId} userId=${user.id} sessionId=${dto.sessionId} clientDurationSeconds=${dto.durationSeconds} - reading session save started`,
|
||||
);
|
||||
|
||||
try {
|
||||
await this.bookService.verifyFileAccess(fileId, user);
|
||||
|
||||
const startedAt = new Date(dto.startedAt);
|
||||
const endedAt = new Date(dto.endedAt);
|
||||
if (Number.isNaN(startedAt.getTime()) || Number.isNaN(endedAt.getTime())) {
|
||||
throw new BadRequestException('Invalid reading session timestamps');
|
||||
}
|
||||
if (endedAt.getTime() < startedAt.getTime()) {
|
||||
throw new BadRequestException('endedAt must be greater than or equal to startedAt');
|
||||
}
|
||||
|
||||
const wallClockSeconds = Math.floor((endedAt.getTime() - startedAt.getTime()) / 1000);
|
||||
|
||||
// Client sends active reading time (excludes idle/hidden periods); cap it at the wall-clock
|
||||
// span to prevent the client from reporting more time than physically elapsed.
|
||||
const durationSeconds = Math.min(dto.durationSeconds, wallClockSeconds);
|
||||
|
||||
const result = await this.repo.saveSession(
|
||||
user.id,
|
||||
fileId,
|
||||
dto.sessionId,
|
||||
startedAt,
|
||||
endedAt,
|
||||
durationSeconds,
|
||||
dto.progressDelta ?? null,
|
||||
dto.endProgress ?? null,
|
||||
);
|
||||
|
||||
this.logger.log(
|
||||
`[${event}] [end] fileId=${fileId} userId=${user.id} sessionId=${dto.sessionId} durationMs=${Date.now() - startedAtMs} outcome=${result.kind}${result.kind === 'skipped' ? ` reason=${result.reason}` : ''} - reading session save completed`,
|
||||
);
|
||||
} catch (error) {
|
||||
const errorClass = error instanceof Error ? error.constructor.name : 'UnknownError';
|
||||
const errorMessage = (error instanceof Error ? error.message : 'unknown error').replaceAll('"', "'");
|
||||
this.logger.warn(
|
||||
`[${event}] [fail] fileId=${fileId} userId=${user.id} sessionId=${dto.sessionId} durationMs=${Date.now() - startedAtMs} errorClass=${errorClass} error="${errorMessage}" - reading session save failed`,
|
||||
);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,9 +56,21 @@ describe('RecommendationRepository', () => {
|
||||
it('combines metadata, authors, genres, and tags for target book data', async () => {
|
||||
const { db, select } = makeDb([
|
||||
{ terminal: 'limit', result: [{ embedding: [0.1, 0.2], seriesName: 'Saga', rating: 4.5 }] },
|
||||
{ terminal: 'where', result: [{ name: 'Author A' }, { name: 'Author B' }] },
|
||||
{ terminal: 'where', result: [{ name: 'Fantasy' }] },
|
||||
{ terminal: 'where', result: [{ name: 'Epic' }, { name: 'Classic' }] },
|
||||
{
|
||||
terminal: 'where',
|
||||
result: [
|
||||
{ bookId: 7, name: 'Author A' },
|
||||
{ bookId: 7, name: 'Author B' },
|
||||
],
|
||||
},
|
||||
{ terminal: 'where', result: [{ bookId: 7, name: 'Fantasy' }] },
|
||||
{
|
||||
terminal: 'where',
|
||||
result: [
|
||||
{ bookId: 7, name: 'Epic' },
|
||||
{ bookId: 7, name: 'Classic' },
|
||||
],
|
||||
},
|
||||
]);
|
||||
const repo = new RecommendationRepository(db);
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import * as schema from '../../db/schema';
|
||||
import { authors, bookAuthors, bookGenres, bookMetadata, bookTags, books, genres, tags } from '../../db/schema';
|
||||
|
||||
type Db = NodePgDatabase<typeof schema>;
|
||||
const ANN_CANDIDATE_FETCH_LIMIT = 100;
|
||||
|
||||
export interface AnnCandidate {
|
||||
bookId: number;
|
||||
@@ -46,26 +47,14 @@ export class RecommendationRepository {
|
||||
|
||||
if (!meta) return null;
|
||||
|
||||
const [authorRows, genreRows, tagRows] = await Promise.all([
|
||||
this.db
|
||||
.select({ name: authors.name })
|
||||
.from(bookAuthors)
|
||||
.innerJoin(authors, eq(authors.id, bookAuthors.authorId))
|
||||
.where(eq(bookAuthors.bookId, bookId)),
|
||||
this.db
|
||||
.select({ name: genres.name })
|
||||
.from(bookGenres)
|
||||
.innerJoin(genres, eq(genres.id, bookGenres.genreId))
|
||||
.where(eq(bookGenres.bookId, bookId)),
|
||||
this.db.select({ name: tags.name }).from(bookTags).innerJoin(tags, eq(tags.id, bookTags.tagId)).where(eq(bookTags.bookId, bookId)),
|
||||
]);
|
||||
const [candidateMetadata] = await this.getCandidateMetadata([bookId]);
|
||||
|
||||
return {
|
||||
embedding: meta.embedding,
|
||||
seriesName: meta.seriesName,
|
||||
rating: meta.rating,
|
||||
authorNames: authorRows.map((r) => r.name),
|
||||
genreTagNames: [...genreRows.map((r) => r.name), ...tagRows.map((r) => r.name)],
|
||||
authorNames: candidateMetadata?.authorNames ?? [],
|
||||
genreTagNames: candidateMetadata?.genreTagNames ?? [],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -85,7 +74,7 @@ export class RecommendationRepository {
|
||||
.innerJoin(books, eq(books.id, bookMetadata.bookId))
|
||||
.where(and(inArray(books.libraryId, libraryIds), ne(bookMetadata.bookId, targetBookId), isNotNull(bookMetadata.embedding)))
|
||||
.orderBy(sql`${bookMetadata.embedding} <=> ${vecStr}::vector`)
|
||||
.limit(100);
|
||||
.limit(ANN_CANDIDATE_FETCH_LIMIT);
|
||||
}
|
||||
|
||||
async getCandidateMetadata(bookIds: number[]): Promise<CandidateMetadata[]> {
|
||||
@@ -109,19 +98,8 @@ export class RecommendationRepository {
|
||||
.where(inArray(bookTags.bookId, bookIds)),
|
||||
]);
|
||||
|
||||
const authorsByBook = new Map<number, string[]>();
|
||||
for (const r of authorRows) {
|
||||
const list = authorsByBook.get(r.bookId) ?? [];
|
||||
list.push(r.name);
|
||||
authorsByBook.set(r.bookId, list);
|
||||
}
|
||||
|
||||
const genreTagsByBook = new Map<number, string[]>();
|
||||
for (const r of [...genreRows, ...tagRows]) {
|
||||
const list = genreTagsByBook.get(r.bookId) ?? [];
|
||||
list.push(r.name);
|
||||
genreTagsByBook.set(r.bookId, list);
|
||||
}
|
||||
const authorsByBook = this.groupNamesByBook(authorRows);
|
||||
const genreTagsByBook = this.groupNamesByBook([...genreRows, ...tagRows]);
|
||||
|
||||
return bookIds.map((id) => ({
|
||||
bookId: id,
|
||||
@@ -129,4 +107,14 @@ export class RecommendationRepository {
|
||||
genreTagNames: genreTagsByBook.get(id) ?? [],
|
||||
}));
|
||||
}
|
||||
|
||||
private groupNamesByBook(rows: Array<{ bookId: number; name: string }>): Map<number, string[]> {
|
||||
const grouped = new Map<number, string[]>();
|
||||
for (const row of rows) {
|
||||
const names = grouped.get(row.bookId) ?? [];
|
||||
names.push(row.name);
|
||||
grouped.set(row.bookId, names);
|
||||
}
|
||||
return grouped;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -67,14 +67,20 @@ describe('RecommendationService', () => {
|
||||
expect(libraryService.verifyUserAccess).toHaveBeenCalledWith(user.id, 21, true);
|
||||
});
|
||||
|
||||
it('returns empty recommendations when the target book has no metadata row', async () => {
|
||||
const { service, bookRepo, recRepo, embedder } = makeService();
|
||||
it('uses fallback embedding when the target book has no metadata row', async () => {
|
||||
const { service, bookRepo, recRepo, embedder, libraryService } = makeService();
|
||||
|
||||
bookRepo.findLibraryIdByBookId.mockResolvedValue(21);
|
||||
recRepo.getTargetBookData.mockResolvedValue(null);
|
||||
embedder.embedBook.mockResolvedValue([0.4, 0.6]);
|
||||
libraryService.findAll.mockResolvedValue([{ id: 7 }, { id: 9 }]);
|
||||
recRepo.findAnnCandidates.mockResolvedValue([{ bookId: 91, cosineSim: 0.78, seriesName: null, rating: null }]);
|
||||
recRepo.getCandidateMetadata.mockResolvedValue([{ bookId: 91, authorNames: [], genreTagNames: [] }]);
|
||||
bookRepo.findRecommendationTitlesByBookIds.mockResolvedValue([{ id: 91, title: 'Fallback Match' }]);
|
||||
|
||||
await expect(service.getRecommendations(55, makeUser())).resolves.toEqual([]);
|
||||
expect(embedder.embedBook).not.toHaveBeenCalled();
|
||||
await expect(service.getRecommendations(55, makeUser())).resolves.toEqual([{ id: 91, title: 'Fallback Match' }]);
|
||||
expect(embedder.embedBook).toHaveBeenCalledWith(55);
|
||||
expect(recRepo.findAnnCandidates).toHaveBeenCalledWith([0.4, 0.6], 55, [7, 9]);
|
||||
});
|
||||
|
||||
it('returns empty recommendations when fallback embedding is invalid', async () => {
|
||||
@@ -94,6 +100,17 @@ describe('RecommendationService', () => {
|
||||
expect(recRepo.findAnnCandidates).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns empty recommendations when metadata row is missing and generated embedding is invalid', async () => {
|
||||
const { service, bookRepo, recRepo, embedder } = makeService();
|
||||
|
||||
bookRepo.findLibraryIdByBookId.mockResolvedValue(6);
|
||||
recRepo.getTargetBookData.mockResolvedValue(null);
|
||||
embedder.embedBook.mockResolvedValue([Number.NaN]);
|
||||
|
||||
await expect(service.getRecommendations(6, makeUser())).resolves.toEqual([]);
|
||||
expect(recRepo.findAnnCandidates).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rescales inconsistent provider values and keeps ranking deterministic', async () => {
|
||||
const { service, recRepo, bookRepo } = makeService();
|
||||
|
||||
@@ -125,6 +142,37 @@ describe('RecommendationService', () => {
|
||||
expect(result[1]).toEqual({ id: 200, title: 'Second' });
|
||||
});
|
||||
|
||||
it('normalizes author and genre-tag metadata before similarity scoring', async () => {
|
||||
const { service, recRepo, bookRepo } = makeService();
|
||||
|
||||
bookRepo.findLibraryIdByBookId.mockResolvedValue(13);
|
||||
recRepo.getTargetBookData.mockResolvedValue({
|
||||
embedding: [0.4, 0.2],
|
||||
seriesName: null,
|
||||
rating: null,
|
||||
authorNames: [' Frank Herbert '],
|
||||
genreTagNames: [' Sci-Fi '],
|
||||
});
|
||||
recRepo.findAnnCandidates.mockResolvedValue([
|
||||
{ bookId: 1, cosineSim: 0.7, seriesName: null, rating: null },
|
||||
{ bookId: 2, cosineSim: 0.85, seriesName: null, rating: null },
|
||||
]);
|
||||
recRepo.getCandidateMetadata.mockResolvedValue([
|
||||
{ bookId: 1, authorNames: ['frank herbert'], genreTagNames: ['sci-fi'] },
|
||||
{ bookId: 2, authorNames: [], genreTagNames: [] },
|
||||
]);
|
||||
bookRepo.findRecommendationTitlesByBookIds.mockResolvedValue([
|
||||
{ id: 1, title: 'Token Match' },
|
||||
{ id: 2, title: 'Cosine Only' },
|
||||
]);
|
||||
|
||||
const result = await service.getRecommendations(13, makeUser());
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toEqual({ id: 1, title: 'Token Match' });
|
||||
expect(result[1]).toEqual({ id: 2, title: 'Cosine Only' });
|
||||
});
|
||||
|
||||
it('filters out ANN results that cannot be mapped to cards', async () => {
|
||||
const { service, recRepo, bookRepo } = makeService();
|
||||
|
||||
@@ -152,6 +200,25 @@ describe('RecommendationService', () => {
|
||||
expect(result[0]).toEqual({ id: 11, title: 'Only Card' });
|
||||
});
|
||||
|
||||
it('returns empty recommendations when user has no accessible libraries with ANN candidates', async () => {
|
||||
const { service, recRepo, bookRepo, libraryService } = makeService();
|
||||
|
||||
bookRepo.findLibraryIdByBookId.mockResolvedValue(15);
|
||||
recRepo.getTargetBookData.mockResolvedValue({
|
||||
embedding: [0.2],
|
||||
seriesName: null,
|
||||
rating: null,
|
||||
authorNames: [],
|
||||
genreTagNames: [],
|
||||
});
|
||||
libraryService.findAll.mockResolvedValue([]);
|
||||
recRepo.findAnnCandidates.mockResolvedValue([]);
|
||||
|
||||
await expect(service.getRecommendations(15, makeUser())).resolves.toEqual([]);
|
||||
expect(recRepo.getCandidateMetadata).not.toHaveBeenCalled();
|
||||
expect(bookRepo.findRecommendationTitlesByBookIds).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('limits rescored output to 25 candidates before loading cards', async () => {
|
||||
const { service, recRepo, bookRepo } = makeService();
|
||||
|
||||
|
||||
@@ -7,6 +7,18 @@ import { BookReadService } from '../book/book-read.service';
|
||||
import { LibraryService } from '../library/library.service';
|
||||
import { AnnCandidate, CandidateMetadata, RecommendationRepository, TargetBookData } from './recommendation.repository';
|
||||
|
||||
const RECOMMENDATION_EVENT = 'book.recommendations';
|
||||
const MAX_RECOMMENDATIONS = 25;
|
||||
const DEFAULT_RATING_PROXIMITY = 0.5;
|
||||
const RATING_PROXIMITY_RANGE = 4;
|
||||
const SCORE_WEIGHTS = {
|
||||
cosineSim: 0.5,
|
||||
authorSim: 0.1,
|
||||
genreTagSim: 0.25,
|
||||
seriesBonus: 0.1,
|
||||
ratingProximity: 0.05,
|
||||
} as const;
|
||||
|
||||
@Injectable()
|
||||
export class RecommendationService {
|
||||
private readonly logger = new Logger(RecommendationService.name);
|
||||
@@ -19,57 +31,97 @@ export class RecommendationService {
|
||||
) {}
|
||||
|
||||
async getRecommendations(bookId: number, user: RequestUser): Promise<BookRecommendation[]> {
|
||||
const libraryId = await this.bookReadService.findLibraryIdByBookId(bookId);
|
||||
if (libraryId === null) throw new NotFoundException(`Book ${bookId} not found`);
|
||||
await this.libraryService.verifyUserAccess(user.id, libraryId, this.isSuperuser(user));
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(
|
||||
`[${RECOMMENDATION_EVENT}] [start] bookId=${bookId} userId=${user.id} isSuperuser=${user.isSuperuser} - recommendation lookup started`,
|
||||
);
|
||||
|
||||
const target = await this.recRepo.getTargetBookData(bookId);
|
||||
if (!target) return [];
|
||||
try {
|
||||
const libraryId = await this.bookReadService.findLibraryIdByBookId(bookId);
|
||||
if (libraryId === null) throw new NotFoundException(`Book ${bookId} not found`);
|
||||
await this.libraryService.verifyUserAccess(user.id, libraryId, user.isSuperuser);
|
||||
|
||||
let embedding = target.embedding;
|
||||
if (!embedding) {
|
||||
embedding = await this.embedder.embedBook(bookId);
|
||||
const target = (await this.recRepo.getTargetBookData(bookId)) ?? this.createFallbackTarget();
|
||||
const embedding = target.embedding ?? (await this.embedder.embedBook(bookId));
|
||||
if (!this.isValidEmbedding(embedding)) {
|
||||
this.logger.log(
|
||||
`[${RECOMMENDATION_EVENT}] [end] bookId=${bookId} userId=${user.id} libraryId=${libraryId} durationMs=${Date.now() - startedAt} reason=invalid_embedding - recommendation lookup completed`,
|
||||
);
|
||||
return [];
|
||||
}
|
||||
|
||||
const accessibleLibraries = await this.libraryService.findAll(user);
|
||||
const accessibleLibraryIds = accessibleLibraries.map((library) => library.id);
|
||||
|
||||
const candidates = await this.recRepo.findAnnCandidates(embedding, bookId, accessibleLibraryIds);
|
||||
if (candidates.length === 0) {
|
||||
this.logger.log(
|
||||
`[${RECOMMENDATION_EVENT}] [end] bookId=${bookId} userId=${user.id} libraryId=${libraryId} durationMs=${Date.now() - startedAt} accessibleLibraryCount=${accessibleLibraryIds.length} candidateCount=0 resultCount=0 - recommendation lookup completed`,
|
||||
);
|
||||
return [];
|
||||
}
|
||||
|
||||
const candidateMetadata = await this.recRepo.getCandidateMetadata(candidates.map((c) => c.bookId));
|
||||
const metaMap = new Map(candidateMetadata.map((m) => [m.bookId, m]));
|
||||
|
||||
const rescored = candidates
|
||||
.map((candidate) => ({
|
||||
bookId: candidate.bookId,
|
||||
score: this.rescore(candidate, target, metaMap.get(candidate.bookId) ?? null),
|
||||
}))
|
||||
.sort((a, b) => b.score - a.score)
|
||||
.slice(0, MAX_RECOMMENDATIONS);
|
||||
|
||||
if (rescored.length === 0) {
|
||||
this.logger.log(
|
||||
`[${RECOMMENDATION_EVENT}] [end] bookId=${bookId} userId=${user.id} libraryId=${libraryId} durationMs=${Date.now() - startedAt} accessibleLibraryCount=${accessibleLibraryIds.length} candidateCount=${candidates.length} rescoredCount=0 resultCount=0 - recommendation lookup completed`,
|
||||
);
|
||||
return [];
|
||||
}
|
||||
|
||||
const topIds = rescored.map((row) => row.bookId);
|
||||
const rows = await this.bookReadService.findRecommendationTitlesByBookIds(topIds);
|
||||
const rowMap = new Map(rows.map((row) => [row.id, row]));
|
||||
const recommendations = rescored
|
||||
.map((rescoredCandidate) => rowMap.get(rescoredCandidate.bookId))
|
||||
.filter((row): row is { id: number; title: string | null } => row != null);
|
||||
|
||||
this.logger.log(
|
||||
`[${RECOMMENDATION_EVENT}] [end] bookId=${bookId} userId=${user.id} libraryId=${libraryId} durationMs=${Date.now() - startedAt} accessibleLibraryCount=${accessibleLibraryIds.length} candidateCount=${candidates.length} rescoredCount=${rescored.length} resultCount=${recommendations.length} - recommendation lookup completed`,
|
||||
);
|
||||
|
||||
return recommendations;
|
||||
} catch (err) {
|
||||
const { errorClass, errorMessage } = this.parseError(err);
|
||||
this.logger.error(
|
||||
`[${RECOMMENDATION_EVENT}] [fail] bookId=${bookId} userId=${user.id} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - recommendation lookup failed`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
if (!this.isValidEmbedding(embedding)) return [];
|
||||
|
||||
const libs = await this.libraryService.findAll(user);
|
||||
const accessibleLibraryIds = libs.map((l) => l.id);
|
||||
|
||||
const candidates = await this.recRepo.findAnnCandidates(embedding, bookId, accessibleLibraryIds);
|
||||
if (candidates.length === 0) return [];
|
||||
|
||||
const candidateMetadata = await this.recRepo.getCandidateMetadata(candidates.map((c) => c.bookId));
|
||||
const metaMap = new Map(candidateMetadata.map((m) => [m.bookId, m]));
|
||||
|
||||
const rescored = candidates
|
||||
.map((c) => ({ bookId: c.bookId, score: this.rescore(c, target, metaMap.get(c.bookId) ?? null) }))
|
||||
.sort((a, b) => b.score - a.score)
|
||||
.slice(0, 25);
|
||||
|
||||
if (rescored.length === 0) return [];
|
||||
|
||||
const topIds = rescored.map((r) => r.bookId);
|
||||
const rows = await this.bookReadService.findRecommendationTitlesByBookIds(topIds);
|
||||
const rowMap = new Map(rows.map((row) => [row.id, row]));
|
||||
return rescored.map((r) => rowMap.get(r.bookId)).filter((row): row is { id: number; title: string | null } => row != null);
|
||||
}
|
||||
|
||||
private rescore(candidate: AnnCandidate, target: TargetBookData, meta: CandidateMetadata | null): number {
|
||||
const cosineSim = Math.max(0, Math.min(1, candidate.cosineSim));
|
||||
const cosineSim = this.clamp01(candidate.cosineSim);
|
||||
|
||||
const authorSim = meta ? this.jaccard(new Set(target.authorNames), new Set(meta.authorNames)) : 0;
|
||||
const genreTagSim = meta ? this.jaccard(new Set(target.genreTagNames), new Set(meta.genreTagNames)) : 0;
|
||||
const authorSim = meta ? this.jaccard(this.toNormalizedSet(target.authorNames), this.toNormalizedSet(meta.authorNames)) : 0;
|
||||
const genreTagSim = meta ? this.jaccard(this.toNormalizedSet(target.genreTagNames), this.toNormalizedSet(meta.genreTagNames)) : 0;
|
||||
|
||||
const targetSeries = this.normalizeSeries(target.seriesName);
|
||||
const candidateSeries = this.normalizeSeries(candidate.seriesName);
|
||||
const seriesBonus = targetSeries && candidateSeries && targetSeries === candidateSeries ? 1.0 : 0.0;
|
||||
|
||||
let ratingProximity = 0.5;
|
||||
let ratingProximity = DEFAULT_RATING_PROXIMITY;
|
||||
if (target.rating != null && candidate.rating != null) {
|
||||
ratingProximity = Math.max(0, Math.min(1, 1 - Math.abs(target.rating - candidate.rating) / 4));
|
||||
ratingProximity = this.clamp01(1 - Math.abs(target.rating - candidate.rating) / RATING_PROXIMITY_RANGE);
|
||||
}
|
||||
|
||||
return 0.5 * cosineSim + 0.1 * authorSim + 0.25 * genreTagSim + 0.1 * seriesBonus + 0.05 * ratingProximity;
|
||||
return (
|
||||
SCORE_WEIGHTS.cosineSim * cosineSim +
|
||||
SCORE_WEIGHTS.authorSim * authorSim +
|
||||
SCORE_WEIGHTS.genreTagSim * genreTagSim +
|
||||
SCORE_WEIGHTS.seriesBonus * seriesBonus +
|
||||
SCORE_WEIGHTS.ratingProximity * ratingProximity
|
||||
);
|
||||
}
|
||||
|
||||
private jaccard(a: Set<string>, b: Set<string>): number {
|
||||
@@ -79,17 +131,42 @@ export class RecommendationService {
|
||||
return intersection / (a.size + b.size - intersection);
|
||||
}
|
||||
|
||||
private isSuperuser(user: RequestUser): boolean {
|
||||
return user.isSuperuser;
|
||||
}
|
||||
|
||||
private isValidEmbedding(embedding: number[] | null): embedding is number[] {
|
||||
return Array.isArray(embedding) && embedding.length > 0 && embedding.every((v) => Number.isFinite(v));
|
||||
}
|
||||
|
||||
private toNormalizedSet(values: string[]): Set<string> {
|
||||
return new Set(values.map((value) => value.trim().toLowerCase()).filter((value) => value.length > 0));
|
||||
}
|
||||
|
||||
private createFallbackTarget(): TargetBookData {
|
||||
return {
|
||||
embedding: null,
|
||||
seriesName: null,
|
||||
rating: null,
|
||||
authorNames: [],
|
||||
genreTagNames: [],
|
||||
};
|
||||
}
|
||||
|
||||
private clamp01(value: number): number {
|
||||
return Math.max(0, Math.min(1, value));
|
||||
}
|
||||
|
||||
private normalizeSeries(seriesName: string | null): string | null {
|
||||
if (!seriesName) return null;
|
||||
const normalized = seriesName.trim().toLowerCase();
|
||||
return normalized.length > 0 ? normalized : null;
|
||||
}
|
||||
|
||||
private parseError(err: unknown): { errorClass: string; errorMessage: string } {
|
||||
if (err instanceof Error) {
|
||||
return { errorClass: err.constructor.name, errorMessage: this.sanitizeErrorMessage(err.message) };
|
||||
}
|
||||
return { errorClass: 'UnknownError', errorMessage: this.sanitizeErrorMessage(String(err)) };
|
||||
}
|
||||
|
||||
private sanitizeErrorMessage(message: string): string {
|
||||
return message.replace(/[\r\n"]/g, ' ').slice(0, 200);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user