diff --git a/apps/api/src/services/gameService.test.ts b/apps/api/src/services/gameService.test.ts index e922bd4..d5b56de 100644 --- a/apps/api/src/services/gameService.test.ts +++ b/apps/api/src/services/gameService.test.ts @@ -85,20 +85,22 @@ describe("createGameSession", () => { } }); - it("distractors are never the correct answer", async () => { + it("correct answer appears exactly once even if getDistractors returns a duplicate", async () => { + mockGetDistractors.mockResolvedValueOnce([ + "cane", + "wrong2", + "wrong3", + "wrong4", + "wrong5", + "wrong6", + ]); + const session = await createGameSession(validRequest, store, "user-1"); + const question = session.questions[0]!; + const optionTexts = question.options.map((o) => o.text); - for (let i = 0; i < session.questions.length; i++) { - const question = session.questions[i]!; - const correctText = fakeTerms[i]!.targetText; - const distractorTexts = question.options - .map((o) => o.text) - .filter((t) => t !== correctText); - - for (const text of distractorTexts) { - expect(text).not.toBe(correctText); - } - } + expect(optionTexts.filter((t) => t === "cane")).toHaveLength(1); + expect(question.options).toHaveLength(4); }); it("sets the prompt from the source text", async () => { @@ -141,6 +143,14 @@ describe("createGameSession", () => { createGameSession(validRequest, store, "user-1"), ).rejects.toThrow("connection refused"); }); + + it("propagates getDistractors failure", async () => { + mockGetDistractors.mockRejectedValue(new Error("db timeout")); + + await expect( + createGameSession(validRequest, store, "user-1"), + ).rejects.toThrow("db timeout"); + }); }); describe("evaluateAnswer", () => { diff --git a/apps/api/src/services/gameService.ts b/apps/api/src/services/gameService.ts index ddb2d4d..a64b4e7 100644 --- a/apps/api/src/services/gameService.ts +++ b/apps/api/src/services/gameService.ts @@ -39,10 +39,13 @@ export const createGameSession = async ( request.target_language, request.pos, request.difficulty, - 3, + 6, ); - const optionTexts = [term.targetText, ...distractorTexts]; + const uniqueDistractors = distractorTexts.filter( + (t) => t !== term.targetText, + ); + const optionTexts = [term.targetText, ...uniqueDistractors.slice(0, 3)]; const shuffledTexts = shuffleArray(optionTexts); const correctOptionId = shuffledTexts.indexOf(term.targetText); diff --git a/documentation/tickets/t00008.md b/documentation/tickets/t00008.md new file mode 100644 index 0000000..b171abc --- /dev/null +++ b/documentation/tickets/t00008.md @@ -0,0 +1,54 @@ +# fix: deduplicate distractors, replace tautological test + +## Problem + +Two issues in `createGameSession` and its test suite: + +1. If `getDistractors` returned the correct answer as one of the distractors, `createGameSession` would include it in the options array without filtering it out. `indexOf` would then find the first occurrence, which might not be the one intended as the correct answer — producing a question where the correct answer appears twice and the stored `correctOptionId` is wrong. + +2. The test `"distractors are never the correct answer"` was tautological — it filtered the correct answer out of the options array, then asserted the remaining items were not the correct answer. It was testing that `Array.filter()` works. It could never fail. + +## Options considered + +### Option A — Filter duplicates after fetching, request extra distractors as buffer ✅ + +Filter out any distractor that matches the correct answer after fetching. Request 6 distractors instead of 3 to ensure enough remain after deduplication. Take the first 3 valid ones with `slice(0, 3)`. + +Chosen because: deduplication at the service layer is the right place — `getDistractors` shouldn't need to know what the correct answer is. Requesting extra provides a buffer against collisions. + +### Option B — Fix `getDistractors` to never return the correct answer + +Add a NOT filter in the database query. + +Not chosen for this ticket — the database query is in `@lila/db` and is a separate concern. The service layer should be defensive regardless of what the model layer returns. + +## Solution + +- Filter distractors against the correct answer before building options: + + ```ts + const uniqueDistractors = distractorTexts.filter((t) => t !== term.targetText); + const optionTexts = [term.targetText, ...uniqueDistractors.slice(0, 3)]; + ``` + +- Request 6 distractors instead of 3 to account for potential duplicates +- Replaced tautological test with a test that actually exercises the duplicate case: + + ```ts + it("correct answer appears exactly once even if getDistractors returns a duplicate", ...) + ``` + +- Added distractor failure propagation test: + + ```ts + it("propagates getDistractors failure", ...) + ``` + +## Files changed + +- `apps/api/src/services/gameService.ts` — deduplication logic, distractor count increased to 6 +- `apps/api/src/services/gameService.test.ts` — tautological test replaced, failure test added + +## Commit + +`fix: deduplicate distractors, replace tautological test, add distractor failure test`