Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion tests/browser/lobby.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,10 @@ lobbyTest(
const title = await page
.locator(`[data-problem="${first.card}"] .problem-title`)
.textContent();
assert.equal(first.note, `Selected problem: ${title}.`);
assert.equal(
first.note,
`Selected problem: ${title} (${(await cardInfo(page, first.card)).level}).`,
);
assert.equal(first.card, eligible[1]);
assert.deepEqual(first.levels, ["Medium", "Hard"]);
assert.equal(first.duration, "60");
Expand Down Expand Up @@ -1894,6 +1897,28 @@ lobbyTest(
},
);

lobbyTest(
"random selection and difficulty changes escape overdue reviews",
async (page) => {
reports = [savedAttempt(EASY[0]), savedAttempt(EASY[1])];
const initial = await lobby(page);
assert.match(initial.note, /Review due/);
await page.evaluate(() => {
Math.random = () => 0;
});
await page.click("#random-problem");
const drawn = await snapshot(page);
assert.equal((await cardInfo(page, drawn.card)).level, "Medium");
assert.doesNotMatch(drawn.note, /Review due/);
await restore(page);
await awaitReady(page);
assert.equal((await snapshot(page)).card, drawn.card);
await setLevel(page, "Hard", true);
await setLevel(page, "Medium", false);
assert.equal((await cardInfo(page, (await snapshot(page)).card)).level, "Hard");
Comment on lines +1916 to +1918

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difficulty half of this test only runs after the Random click has already set pickerMode = "random", so deleting the assignment in the level handler still passes. It also checks only the level, not that the note lacks Review due. Exercise the level change from a fresh lobby with overdue reviews, as a separate test, and assert both.

},
);

lobbyTest(
"two passes move the candidate up a level, and the lobby says why",
async (page) => {
Expand Down
54 changes: 54 additions & 0 deletions tests/browser/problem-picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,52 @@ const completed = (problemId, at) => ({
const first = () => 0;
const day = 24 * 60 * 60 * 1000;

test("explicit random draws reach every eligible problem despite overdue reviews", () => {
const problems = [
{ id: "due-a", difficulty: "Easy" },
{ id: "due-b", difficulty: "Easy" },
{ id: "unseen", difficulty: "Easy" },
{ id: "outside", difficulty: "Hard" },
];
const reports = ["due-a", "due-b", "outside"].map((id) => completed(id, 0));
for (const avoid of [undefined, "due-a", "due-b", "unseen"]) {
const expected = problems.filter(
(problem) => problem.difficulty === "Easy" && problem.id !== avoid,
);
for (let index = 0; index < expected.length; index++) {
const choice = pickProblem(
problems,
new Set(["Easy"]),
reports,
() => (index + 0.5) / expected.length,
10 * day,
avoid,
undefined,
"random",
);
assert.equal(choice.picked.id, expected[index].id);
assert.equal(choice.review, null);
assert.equal(choice.repeat, false);
}
}
});

test("explicit random draws retain a sole eligible problem and respect empty filters", () => {
const draw = (levels) =>
pickProblem(
bank,
new Set(levels),
[completed("passed", 0)],
first,
10 * day,
"medium",
undefined,
"random",
);
assert.equal(draw(["Medium"]).picked.id, "medium");
assert.equal(draw([]), null);
});

test("a problem already passed is not what gets recommended next", () => {
const choice = pickProblem(bank, new Set(["Easy"]), [hired("passed")], first);
assert.equal(choice.picked.id, "fresh");
Expand Down Expand Up @@ -614,3 +660,11 @@ test("the interview reads the focus from session storage, never from its address
assert.match(source, /practiceFocus: consumeSharedFocus\(tabStorage\)/);
assert.match(source, /const tabStorage = storageArea\("sessionStorage"\);/);
});

test("random draws respect narrowed practice while reviews keep their broader pool", () => {
const reports = [completed("passed", 0)];
const practice = bank.filter((problem) => problem.id === "fresh");
const draw = (mode) => pickProblem(practice, new Set(["Easy"]), reports, first, 10 * day, undefined, bank, mode);
assert.equal(draw("recommend").picked.id, "passed");
assert.equal(draw("random").picked.id, "fresh");
});
10 changes: 10 additions & 0 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ let durationCeiling = Infinity;
let roll = Math.random();
// Keep the exclusion with the roll so restoring the page repeats the same draw.
let avoidedProblem;
// Preserve the draw policy as well as its roll during history/page restores.
let pickerMode = "recommend";

const nodes = {
accountStatus: document.querySelector("#account-status"),
Expand Down Expand Up @@ -233,6 +235,7 @@ nodes.randomProblem.addEventListener("click", () => {
keptDraw = null;
roll = Math.random();
avoidedProblem = problem?.id;
pickerMode = "random";
applyDifficulties();
recommend();
});
Expand Down Expand Up @@ -263,6 +266,7 @@ for (const input of levels) {
keptDraw = null;
roll = Math.random();
avoidedProblem = undefined;
pickerMode = "random";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A level change is not an explicit random draw, but this switches it to uniform selection anyway. With a passed Hard problem and unseen Hard ones, checking Hard and unchecking the current level can now offer the passed problem first. That undoes "a problem already passed is not what gets recommended next", and the description does not mention it. If the goal is only to stop reviews crossing the selected levels, keep "recommend" here and restrict the review pool to the selected difficulties instead.

// Not before the reports are in. Recommending from an empty history here
// would offer a problem the candidate has already passed and then swap it
// when the fetch lands. `settle` makes the pick for this level instead, and
Expand Down Expand Up @@ -1021,6 +1025,7 @@ function recommend(note = "") {
undefined,
avoidedProblem,
cards,
pickerMode,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Topic-filter redraws in random mode can select the current problem again because filterSelectionChanged() clears avoidedProblem before recommend(). Preserve the current card as the exclusion on random-mode redraws.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At web/app.js, line 1028:

<comment>Topic-filter redraws in random mode can select the current problem again because `filterSelectionChanged()` clears `avoidedProblem` before `recommend()`. Preserve the current card as the exclusion on random-mode redraws.</comment>

<file context>
@@ -1021,6 +1025,7 @@ function recommend(note = "") {
     undefined,
     avoidedProblem,
     cards,
+    pickerMode,
   );
   // Nothing to offer is still an answer, and it has to go through `setProblem`
</file context>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pickerMode is set to "random" and never goes back to "recommend", so after one click every later recommend() draws uniformly: filterSelectionChanged(), sign-in, report deletion and every settle(). Concrete case: click Random and get X, finish X, press Back. The bfcache restore refetches reports that now record X as passed, yet roll, avoidedProblem and the pool are unchanged, so settle() offers X again (and silently drops the reason keptDraw is cleared there). Reset to "recommend" wherever avoidedProblem is reset to undefined, and in settle() whenever the reports differ from the ones the random draw was made from (track them the way keptDraw does).

);
// Nothing to offer is still an answer, and it has to go through `setProblem`
// like every other one. Returning here left whatever was picked for the
Expand All @@ -1032,6 +1037,11 @@ function recommend(note = "") {
return;
}
setProblem(choice.picked);
// Editing begins: When randomly selecting questions, display the difficulty level next to the question title.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Editing begins:" reads like an editor marker. A comment here should say why random mode skips the review sentence, e.g. that a random draw is never a review and the level is named because it may differ from the suggested one.

if (pickerMode === "random") {
nodes.recommendation.textContent = `${note}Selected problem: ${title(choice.picked)} (${choice.picked.difficulty}).`;
return;
}
if (choice.review) {
// A review can fall outside the levels now selected, so the card is
// unhidden and the level said out loud rather than silently ignored.
Expand Down
11 changes: 11 additions & 0 deletions web/problem-picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ function step(level, by) {
/// An explicit redraw avoids the current problem whenever another is available.
/// `reviewProblems` stays broader than `problems` when an opt-in filter narrows
/// new practice: a due review remains the first priority across those filters.
/// `random` mode draws uniformly from the selected levels instead of giving
/// overdue reviews or unseen problems priority.
export function pickProblem(
problems,
difficulties,
Expand All @@ -157,10 +159,19 @@ export function pickProblem(
now = Date.now(),
avoid,
reviewProblems = problems,
mode = "recommend",
) {
const eligible = problems.filter((problem) =>
difficulties.has(problem.difficulty),
);
// Explicit random draws give every problem at the selected levels a chance,
// regardless of its attempt history or review schedule.
if (mode === "random") {
const alternatives = eligible.filter((problem) => problem.id !== avoid);
const choices = alternatives.length ? alternatives : eligible;
const picked = choices[Math.floor(random() * choices.length)];
return picked ? { picked, repeat: false, review: null } : null;
}
const reportList = Array.isArray(reports) ? reports : [];
const reviews = reviewStatus(reportList, now);
const passed = new Set(
Expand Down
Loading