fix: deduplicate distractors, replace tautological test, add distractor failure test
This commit is contained in:
parent
3d16ab0fff
commit
a02f3b139d
3 changed files with 81 additions and 14 deletions
|
|
@ -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 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++) {
|
expect(optionTexts.filter((t) => t === "cane")).toHaveLength(1);
|
||||||
const question = session.questions[i]!;
|
expect(question.options).toHaveLength(4);
|
||||||
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);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("sets the prompt from the source text", async () => {
|
it("sets the prompt from the source text", async () => {
|
||||||
|
|
@ -141,6 +143,14 @@ describe("createGameSession", () => {
|
||||||
createGameSession(validRequest, store, "user-1"),
|
createGameSession(validRequest, store, "user-1"),
|
||||||
).rejects.toThrow("connection refused");
|
).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", () => {
|
describe("evaluateAnswer", () => {
|
||||||
|
|
|
||||||
|
|
@ -39,10 +39,13 @@ export const createGameSession = async (
|
||||||
request.target_language,
|
request.target_language,
|
||||||
request.pos,
|
request.pos,
|
||||||
request.difficulty,
|
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 shuffledTexts = shuffleArray(optionTexts);
|
||||||
const correctOptionId = shuffledTexts.indexOf(term.targetText);
|
const correctOptionId = shuffledTexts.indexOf(term.targetText);
|
||||||
|
|
||||||
|
|
|
||||||
54
documentation/tickets/t00008.md
Normal file
54
documentation/tickets/t00008.md
Normal file
|
|
@ -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`
|
||||||
Loading…
Add table
Add a link
Reference in a new issue