From a047cdd517efe7693ccd41162f9267f48cd67955 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Wed, 4 Feb 2026 22:31:13 +0900 Subject: feat(study): enforce newCardsPerDay limit in study API Split due card fetching into new cards and review cards, applying the deck's newCardsPerDay limit to new cards while leaving review cards unrestricted. New cards are placed before review cards in the response. Co-Authored-By: Claude Opus 4.5 --- src/server/repositories/card.test.ts | 2 + src/server/repositories/card.ts | 163 +++++++++++++++++++++------------- src/server/repositories/review-log.ts | 21 ++++- src/server/repositories/types.ts | 11 +++ src/server/routes/cards.test.ts | 2 + src/server/routes/decks.test.ts | 2 + src/server/routes/study.test.ts | 104 ++++++++++++++++++++-- src/server/routes/study.ts | 16 +++- 8 files changed, 251 insertions(+), 70 deletions(-) diff --git a/src/server/repositories/card.test.ts b/src/server/repositories/card.test.ts index b492fd7..1ed31c7 100644 --- a/src/server/repositories/card.test.ts +++ b/src/server/repositories/card.test.ts @@ -114,6 +114,8 @@ function createMockCardRepo(): CardRepository { countDueCards: vi.fn(), findDueCardsWithNoteData: vi.fn(), findDueCardsForStudy: vi.fn(), + findDueNewCardsForStudy: vi.fn(), + findDueReviewCardsForStudy: vi.fn(), updateFSRSFields: vi.fn(), }; } diff --git a/src/server/repositories/card.ts b/src/server/repositories/card.ts index 2f14149..6202267 100644 --- a/src/server/repositories/card.ts +++ b/src/server/repositories/card.ts @@ -1,4 +1,4 @@ -import { and, eq, isNull, lt, sql } from "drizzle-orm"; +import { and, eq, isNull, lt, lte, ne, sql } from "drizzle-orm"; import { getEndOfStudyDayBoundary } from "../../shared/date.js"; import { db } from "../db/index.js"; import { @@ -263,69 +263,49 @@ export const cardRepository: CardRepository = { limit: number, ): Promise { const dueCards = await this.findDueCards(deckId, now, limit); + return enrichCardsForStudy(dueCards); + }, - const cardsForStudy: CardForStudy[] = []; - - for (const card of dueCards) { - // Fetch note to get noteTypeId - const noteResult = await db - .select() - .from(notes) - .where(and(eq(notes.id, card.noteId), isNull(notes.deletedAt))); - - const note = noteResult[0]; - if (!note) { - // Note was deleted, skip this card - continue; - } - - // Fetch note type for templates - const noteTypeResult = await db - .select({ - frontTemplate: noteTypes.frontTemplate, - backTemplate: noteTypes.backTemplate, - }) - .from(noteTypes) - .where( - and(eq(noteTypes.id, note.noteTypeId), isNull(noteTypes.deletedAt)), - ); - - const noteType = noteTypeResult[0]; - if (!noteType) { - // Note type was deleted, skip this card - continue; - } - - // Fetch field values with their field names - const fieldValuesWithNames = await db - .select({ - fieldName: noteFieldTypes.name, - value: noteFieldValues.value, - }) - .from(noteFieldValues) - .innerJoin( - noteFieldTypes, - eq(noteFieldValues.noteFieldTypeId, noteFieldTypes.id), - ) - .where(eq(noteFieldValues.noteId, card.noteId)); - - // Convert to name-value map - const fieldValuesMap: Record = {}; - for (const fv of fieldValuesWithNames) { - fieldValuesMap[fv.fieldName] = fv.value; - } - - cardsForStudy.push({ - ...card, - noteType: { - frontTemplate: noteType.frontTemplate, - backTemplate: noteType.backTemplate, - }, - fieldValuesMap, - }); - } + async findDueNewCardsForStudy( + deckId: string, + now: Date, + limit: number, + ): Promise { + const result = await db + .select() + .from(cards) + .where( + and( + eq(cards.deckId, deckId), + isNull(cards.deletedAt), + lte(cards.due, now), + eq(cards.state, CardState.New), + ), + ) + .orderBy(cards.due) + .limit(limit); + return enrichCardsForStudy(result); + }, - return cardsForStudy; + async findDueReviewCardsForStudy( + deckId: string, + now: Date, + limit: number, + ): Promise { + const result = await db + .select() + .from(cards) + .where( + and( + eq(cards.deckId, deckId), + isNull(cards.deletedAt), + lte(cards.due, now), + ne(cards.state, CardState.New), + ), + ) + .orderBy(cards.due) + .limit(limit); + return enrichCardsForStudy(result); }, async updateFSRSFields( @@ -369,3 +349,62 @@ export const cardRepository: CardRepository = { return result[0]; }, }; + +async function enrichCardsForStudy(dueCards: Card[]): Promise { + const cardsForStudy: CardForStudy[] = []; + + for (const card of dueCards) { + const noteResult = await db + .select() + .from(notes) + .where(and(eq(notes.id, card.noteId), isNull(notes.deletedAt))); + + const note = noteResult[0]; + if (!note) { + continue; + } + + const noteTypeResult = await db + .select({ + frontTemplate: noteTypes.frontTemplate, + backTemplate: noteTypes.backTemplate, + }) + .from(noteTypes) + .where( + and(eq(noteTypes.id, note.noteTypeId), isNull(noteTypes.deletedAt)), + ); + + const noteType = noteTypeResult[0]; + if (!noteType) { + continue; + } + + const fieldValuesWithNames = await db + .select({ + fieldName: noteFieldTypes.name, + value: noteFieldValues.value, + }) + .from(noteFieldValues) + .innerJoin( + noteFieldTypes, + eq(noteFieldValues.noteFieldTypeId, noteFieldTypes.id), + ) + .where(eq(noteFieldValues.noteId, card.noteId)); + + const fieldValuesMap: Record = {}; + for (const fv of fieldValuesWithNames) { + fieldValuesMap[fv.fieldName] = fv.value; + } + + cardsForStudy.push({ + ...card, + noteType: { + frontTemplate: noteType.frontTemplate, + backTemplate: noteType.backTemplate, + }, + fieldValuesMap, + }); + } + + return cardsForStudy; +} diff --git a/src/server/repositories/review-log.ts b/src/server/repositories/review-log.ts index c8950d6..591c647 100644 --- a/src/server/repositories/review-log.ts +++ b/src/server/repositories/review-log.ts @@ -1,5 +1,6 @@ +import { and, eq, gte, sql } from "drizzle-orm"; import { db } from "../db/index.js"; -import { reviewLogs } from "../db/schema.js"; +import { CardState, cards, reviewLogs } from "../db/schema.js"; import type { ReviewLog, ReviewLogRepository } from "./types.js"; export const reviewLogRepository: ReviewLogRepository = { @@ -29,4 +30,22 @@ export const reviewLogRepository: ReviewLogRepository = { } return reviewLog; }, + + async countTodayNewCardReviews(deckId: string, now: Date): Promise { + const startOfDay = new Date(now); + startOfDay.setHours(0, 0, 0, 0); + + const result = await db + .select({ count: sql`count(distinct ${reviewLogs.cardId})::int` }) + .from(reviewLogs) + .innerJoin(cards, eq(reviewLogs.cardId, cards.id)) + .where( + and( + eq(cards.deckId, deckId), + eq(reviewLogs.state, CardState.New), + gte(reviewLogs.reviewedAt, startOfDay), + ), + ); + return result[0]?.count ?? 0; + }, }; diff --git a/src/server/repositories/types.ts b/src/server/repositories/types.ts index cb3a287..47fb68f 100644 --- a/src/server/repositories/types.ts +++ b/src/server/repositories/types.ts @@ -158,6 +158,16 @@ export interface CardRepository { now: Date, limit: number, ): Promise; + findDueNewCardsForStudy( + deckId: string, + now: Date, + limit: number, + ): Promise; + findDueReviewCardsForStudy( + deckId: string, + now: Date, + limit: number, + ): Promise; updateFSRSFields( id: string, deckId: string, @@ -198,6 +208,7 @@ export interface ReviewLogRepository { elapsedDays: number; durationMs?: number | null; }): Promise; + countTodayNewCardReviews(deckId: string, now: Date): Promise; } export interface NoteType { diff --git a/src/server/routes/cards.test.ts b/src/server/routes/cards.test.ts index e5fb0d4..2179720 100644 --- a/src/server/routes/cards.test.ts +++ b/src/server/routes/cards.test.ts @@ -28,6 +28,8 @@ function createMockCardRepo(): CardRepository { countDueCards: vi.fn(), findDueCardsWithNoteData: vi.fn(), findDueCardsForStudy: vi.fn(), + findDueNewCardsForStudy: vi.fn(), + findDueReviewCardsForStudy: vi.fn(), updateFSRSFields: vi.fn(), }; } diff --git a/src/server/routes/decks.test.ts b/src/server/routes/decks.test.ts index 55aca2d..d48e494 100644 --- a/src/server/routes/decks.test.ts +++ b/src/server/routes/decks.test.ts @@ -33,6 +33,8 @@ function createMockCardRepo(): CardRepository { countDueCards: vi.fn().mockResolvedValue(0), findDueCardsWithNoteData: vi.fn(), findDueCardsForStudy: vi.fn(), + findDueNewCardsForStudy: vi.fn(), + findDueReviewCardsForStudy: vi.fn(), updateFSRSFields: vi.fn(), }; } diff --git a/src/server/routes/study.test.ts b/src/server/routes/study.test.ts index a5ac817..aeba46b 100644 --- a/src/server/routes/study.test.ts +++ b/src/server/routes/study.test.ts @@ -28,6 +28,8 @@ function createMockCardRepo(): CardRepository { countDueCards: vi.fn(), findDueCardsWithNoteData: vi.fn(), findDueCardsForStudy: vi.fn(), + findDueNewCardsForStudy: vi.fn(), + findDueReviewCardsForStudy: vi.fn(), updateFSRSFields: vi.fn(), }; } @@ -45,6 +47,7 @@ function createMockDeckRepo(): DeckRepository { function createMockReviewLogRepo(): ReviewLogRepository { return { create: vi.fn(), + countTodayNewCardReviews: vi.fn().mockResolvedValue(0), }; } @@ -170,7 +173,8 @@ describe("GET /api/decks/:deckId/study", () => { vi.mocked(mockDeckRepo.findById).mockResolvedValue( createMockDeck({ id: DECK_ID }), ); - vi.mocked(mockCardRepo.findDueCardsForStudy).mockResolvedValue([]); + vi.mocked(mockCardRepo.findDueNewCardsForStudy).mockResolvedValue([]); + vi.mocked(mockCardRepo.findDueReviewCardsForStudy).mockResolvedValue([]); const res = await app.request(`/api/decks/${DECK_ID}/study`, { method: "GET", @@ -184,7 +188,12 @@ describe("GET /api/decks/:deckId/study", () => { DECK_ID, "user-uuid-123", ); - expect(mockCardRepo.findDueCardsForStudy).toHaveBeenCalledWith( + expect(mockCardRepo.findDueNewCardsForStudy).toHaveBeenCalledWith( + DECK_ID, + expect.any(Date), + 20, + ); + expect(mockCardRepo.findDueReviewCardsForStudy).toHaveBeenCalledWith( DECK_ID, expect.any(Date), 100, @@ -192,24 +201,31 @@ describe("GET /api/decks/:deckId/study", () => { }); it("returns due cards", async () => { - const mockCards = [ + const newCards = [ createMockCardForStudy({ id: "card-1", front: "Q1", back: "A1", + state: CardState.New, fieldValuesMap: {}, }), + ]; + const reviewCards = [ createMockCardForStudy({ id: "card-2", front: "Q2", back: "A2", + state: CardState.Review, fieldValuesMap: {}, }), ]; vi.mocked(mockDeckRepo.findById).mockResolvedValue( createMockDeck({ id: DECK_ID }), ); - vi.mocked(mockCardRepo.findDueCardsForStudy).mockResolvedValue(mockCards); + vi.mocked(mockCardRepo.findDueNewCardsForStudy).mockResolvedValue(newCards); + vi.mocked(mockCardRepo.findDueReviewCardsForStudy).mockResolvedValue( + reviewCards, + ); const res = await app.request(`/api/decks/${DECK_ID}/study`, { method: "GET", @@ -241,7 +257,10 @@ describe("GET /api/decks/:deckId/study", () => { vi.mocked(mockDeckRepo.findById).mockResolvedValue( createMockDeck({ id: DECK_ID }), ); - vi.mocked(mockCardRepo.findDueCardsForStudy).mockResolvedValue(mockCards); + vi.mocked(mockCardRepo.findDueNewCardsForStudy).mockResolvedValue( + mockCards, + ); + vi.mocked(mockCardRepo.findDueReviewCardsForStudy).mockResolvedValue([]); const res = await app.request(`/api/decks/${DECK_ID}/study`, { method: "GET", @@ -255,6 +274,81 @@ describe("GET /api/decks/:deckId/study", () => { expect(body.cards?.[0]?.fieldValuesMap?.Front).toBe("Question"); }); + it("limits new cards based on newCardsPerDay", async () => { + vi.mocked(mockDeckRepo.findById).mockResolvedValue( + createMockDeck({ id: DECK_ID, newCardsPerDay: 5 }), + ); + vi.mocked(mockReviewLogRepo.countTodayNewCardReviews).mockResolvedValue(3); + vi.mocked(mockCardRepo.findDueNewCardsForStudy).mockResolvedValue([]); + vi.mocked(mockCardRepo.findDueReviewCardsForStudy).mockResolvedValue([]); + + await app.request(`/api/decks/${DECK_ID}/study`, { + method: "GET", + headers: { Authorization: `Bearer ${authToken}` }, + }); + + expect(mockCardRepo.findDueNewCardsForStudy).toHaveBeenCalledWith( + DECK_ID, + expect.any(Date), + 2, + ); + }); + + it("returns 0 new cards when daily limit is reached", async () => { + vi.mocked(mockDeckRepo.findById).mockResolvedValue( + createMockDeck({ id: DECK_ID, newCardsPerDay: 5 }), + ); + vi.mocked(mockReviewLogRepo.countTodayNewCardReviews).mockResolvedValue(5); + vi.mocked(mockCardRepo.findDueNewCardsForStudy).mockResolvedValue([]); + vi.mocked(mockCardRepo.findDueReviewCardsForStudy).mockResolvedValue([]); + + await app.request(`/api/decks/${DECK_ID}/study`, { + method: "GET", + headers: { Authorization: `Bearer ${authToken}` }, + }); + + expect(mockCardRepo.findDueNewCardsForStudy).toHaveBeenCalledWith( + DECK_ID, + expect.any(Date), + 0, + ); + }); + + it("places new cards before review cards in response", async () => { + const newCards = [ + createMockCardForStudy({ + id: "new-1", + state: CardState.New, + fieldValuesMap: {}, + }), + ]; + const reviewCards = [ + createMockCardForStudy({ + id: "review-1", + state: CardState.Review, + fieldValuesMap: {}, + }), + ]; + vi.mocked(mockDeckRepo.findById).mockResolvedValue( + createMockDeck({ id: DECK_ID }), + ); + vi.mocked(mockCardRepo.findDueNewCardsForStudy).mockResolvedValue(newCards); + vi.mocked(mockCardRepo.findDueReviewCardsForStudy).mockResolvedValue( + reviewCards, + ); + + const res = await app.request(`/api/decks/${DECK_ID}/study`, { + method: "GET", + headers: { Authorization: `Bearer ${authToken}` }, + }); + + expect(res.status).toBe(200); + const body = (await res.json()) as StudyResponse; + expect(body.cards).toHaveLength(2); + expect(body.cards?.[0]?.id).toBe("new-1"); + expect(body.cards?.[1]?.id).toBe("review-1"); + }); + it("returns 404 for non-existent deck", async () => { vi.mocked(mockDeckRepo.findById).mockResolvedValue(undefined); diff --git a/src/server/routes/study.ts b/src/server/routes/study.ts index d978a6a..efd450c 100644 --- a/src/server/routes/study.ts +++ b/src/server/routes/study.ts @@ -51,9 +51,21 @@ export function createStudyRouter(deps: StudyDependencies) { } const now = new Date(); - const dueCards = await cardRepo.findDueCardsForStudy(deckId, now, 100); - return c.json({ cards: dueCards }, 200); + // Calculate new card budget based on today's already-reviewed new cards + const reviewedNewCards = await reviewLogRepo.countTodayNewCardReviews( + deckId, + now, + ); + const newCardBudget = Math.max(0, deck.newCardsPerDay - reviewedNewCards); + + // Fetch new cards (limited) and review cards separately + const [newCards, reviewCards] = await Promise.all([ + cardRepo.findDueNewCardsForStudy(deckId, now, newCardBudget), + cardRepo.findDueReviewCardsForStudy(deckId, now, 100), + ]); + + return c.json({ cards: [...newCards, ...reviewCards] }, 200); }) .post( "/:cardId", -- cgit v1.3-1-g0d28