Skip to content

Commit 4c34f50

Browse files
fix(learning-engine): league rollover catch-up, local-day streak nudge, FSRS lapsed backfill, interleaving gate, store races (#549)
Six correctness defects across the shipped learning-engine work (#389 FSRS, #393 interleaving, #394 leagues + store, #397 digest), none of which had test coverage, plus the test runner four of them needed. Stand up Vitest in mcp-server/ (no runner existed at all) and characterize gradeCard, cardFromRow, computeInterleavingPool, eloExpected and eloJitter against current behaviour before changing anything. §1 A single missed weekly rollover wiped every student's tier. The previous week was hardcoded as v_week - 7, so one skipped Monday tick left that join matching nothing and COALESCE(..., 1) reset the whole tenant to Bronze, while the genuinely-previous week kept final_rank NULL forever. The previous week is now resolved from the data and every unfinalized week is finalized, each over its own [w, w+7) XP window — carrying one window across a multi-week gap would rank a stale cohort over unrelated activity. §2 Promotion/relegation carried no information at the minimum cohort size: promote_count = demote_count = 5 against a cold-start floor of 10 meant the bands exactly tiled the cohort and 'stayed' was unreachable. Bands now scale with active cohort size and always leave at least one 'stayed'. Zero-XP members are excluded from that size and parked, so an inactive student can no longer promote over an active one on a UUID tiebreak. §3 The streak nudge was inverted for the LATAM market. isStreakAtRisk compared against the UTC day while the send decision and idempotency key are tenant-local, so at UTC-5 a student who practised that local afternoon was told their streak was ending hours after securing it, and the students who actually skipped got nothing. Comparison is now against the tenant-local day boundary, computed calendar-first so it survives DST. award_xp's own day boundary is deliberately unchanged — moving it would shift existing streaks — and the residual is documented at the call site. §4 The FSRS backfill skipped the hardest cards. Its predicate was repetitions > 0, but the SM-2 code it replaced reset repetitions to 0 on a lapse, so every card whose last rating was "Again" was excluded and then silently downgraded to createEmptyCard(). Widened to include lapsed rows, with difficulty derived from the retained ease column and state 3 (Relearning). §5 The interleaving gate was advisory. lms_record_practice_attempt accepted mode='mixed' with no gate and no kill-switch check, and its own description told the model to call it directly, so the bypass was the documented path — and the rows it wrote were the rows the gate later read back, letting a session certify its own topics. Both tools now share one gate decision. Separately, XP is awarded once per session rather than once per stored row, so a 3-topic mixed session no longer pays 45 XP and burns 3 of the 10 daily slots against 15 XP for the same work done focused. §6 spend-points double-spent coins and over-granted freezes via three unsynchronized read-modify-writes against a derived balance. Replaced with a single redeem_store_item() RPC holding SELECT ... FOR UPDATE on the profile row, which also refuses at the 5-freeze ceiling instead of charging for a grant that cannot land. Verified: 467/467 root unit tests, 35 mcp-server tests, typecheck and build clean, and seeded-DB assertions for §1/§2/§4/§5/§6 committed under tests/sql/issue-549/ — including a real 5-way concurrency race proving the store redemptions serialize. Closes #549 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RGCEptT7pgWwRiMdB3TUCM
1 parent d2cef7c commit 4c34f50

23 files changed

Lines changed: 1946 additions & 194 deletions

app/api/cron/league-rollover/route.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
* all handled in SQL. pg_cron ('league-weekly-rollover', Mondays 00:05) is the
77
* primary scheduler — this route is a manual trigger + Dokploy/Vercel fallback.
88
*
9+
* Missed ticks need no special handling and no _week_start argument: since
10+
* issue #549 the rollover resolves the previous week from the data and
11+
* finalizes every still-unfinalized week, so simply running it late is the
12+
* catch-up path. It used to reset every student to Bronze in that situation.
13+
*
914
* Secured by CRON_SECRET (Bearer token).
1015
*/
1116

lib/database.types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4793,6 +4793,7 @@ export type Database = {
47934793
mode: string
47944794
questions: Json
47954795
score: number
4796+
session_id: string
47964797
source: string
47974798
source_exercise_id: number | null
47984799
tenant_id: string
@@ -4810,6 +4811,7 @@ export type Database = {
48104811
mode?: string
48114812
questions: Json
48124813
score: number
4814+
session_id?: string
48134815
source?: string
48144816
source_exercise_id?: number | null
48154817
tenant_id: string
@@ -4827,6 +4829,7 @@ export type Database = {
48274829
mode?: string
48284830
questions?: Json
48294831
score?: number
4832+
session_id?: string
48304833
source?: string
48314834
source_exercise_id?: number | null
48324835
tenant_id?: string
@@ -6517,6 +6520,10 @@ export type Database = {
65176520
Returns: undefined
65186521
}
65196522
publish_scheduled_lessons: { Args: never; Returns: undefined }
6523+
redeem_store_item: {
6524+
Args: { _item_id: string; _tenant_id: string; _user_id: string }
6525+
Returns: Json
6526+
}
65206527
refresh_leaderboard_cache: { Args: never; Returns: undefined }
65216528
register_push_token: {
65226529
Args: { _device_name?: string; _platform: string; _token: string }

lib/notifications/daily-digest.ts

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,45 @@ export function localDateStr(now: Date, timezone: string): string {
107107
}
108108
}
109109

110-
/** UTC yesterday as YYYY-MM-DD — matches award_xp()'s CURRENT_DATE streak math. */
111-
export function utcYesterday(now: Date): string {
112-
const d = new Date(now.getTime() - 24 * 60 * 60 * 1000)
113-
return d.toISOString().slice(0, 10)
110+
/**
111+
* YYYY-MM-DD for the day before `now` in the given IANA timezone.
112+
*
113+
* Takes the local calendar date first, then steps back one calendar day on that
114+
* date alone. Subtracting 24h from the instant would land on the wrong date
115+
* across a DST transition.
116+
*/
117+
export function localYesterday(now: Date, timezone: string): string {
118+
const [y, m, d] = localDateStr(now, timezone).split('-').map(Number)
119+
return new Date(Date.UTC(y, m - 1, d - 1)).toISOString().slice(0, 10)
114120
}
115121

116-
/** Streak survives only if the student acts today: last activity was exactly yesterday. */
117-
export function isStreakAtRisk(lastActivityDate: string | null, streak: number, now: Date, min: number): boolean {
118-
return streak >= min && lastActivityDate === utcYesterday(now)
122+
/**
123+
* Streak survives only if the student acts today: last activity was exactly
124+
* yesterday *in the tenant's timezone*.
125+
*
126+
* This compared against UTC yesterday until issue #549. Everything else about
127+
* the send is tenant-local — `localHour` decides whether to send at all, and
128+
* `localDateStr` keys idempotency — so at UTC-5 (Bogotá, Lima) the 20:00 nudge
129+
* runs at 01:00 UTC the *next* UTC day, and a student who practised that same
130+
* local afternoon was told their streak was ending hours after they had secured
131+
* it, while the students who actually skipped the local day were silently
132+
* skipped. It misfired whenever `nudgeHour + |utc_offset| >= 24`.
133+
*
134+
* Known residual: `last_activity_date` is still written by award_xp() from
135+
* CURRENT_DATE, i.e. the UTC day, so a student whose only activity fell in the
136+
* local evening after the UTC day rolled over is still measured against the
137+
* wrong day. Fixing that means changing award_xp()'s day boundary, which would
138+
* shift every existing streak — a deliberate migration, not a side effect of
139+
* this one (issue #549 §3).
140+
*/
141+
export function isStreakAtRisk(
142+
lastActivityDate: string | null,
143+
streak: number,
144+
now: Date,
145+
min: number,
146+
timezone: string
147+
): boolean {
148+
return streak >= min && lastActivityDate === localYesterday(now, timezone)
119149
}
120150

121151
const SUMMARY_COPY = {
@@ -290,8 +320,8 @@ export async function runDailyDigest(admin: SupabaseClient, now: Date = new Date
290320
kind === 'daily_digest'
291321
? c.due_cards > 0 ||
292322
c.goals_pending > 0 ||
293-
isStreakAtRisk(c.last_activity_date, c.current_streak, now, DIGEST_STREAK_MIN)
294-
: isStreakAtRisk(c.last_activity_date, c.current_streak, now, NUDGE_STREAK_MIN)
323+
isStreakAtRisk(c.last_activity_date, c.current_streak, now, DIGEST_STREAK_MIN, settings.timezone)
324+
: isStreakAtRisk(c.last_activity_date, c.current_streak, now, NUDGE_STREAK_MIN, settings.timezone)
295325
)
296326
if (recipients.length === 0) continue
297327

@@ -321,7 +351,8 @@ export async function runDailyDigest(admin: SupabaseClient, now: Date = new Date
321351
candidate.last_activity_date,
322352
candidate.current_streak,
323353
now,
324-
kind === 'daily_digest' ? DIGEST_STREAK_MIN : NUDGE_STREAK_MIN
354+
kind === 'daily_digest' ? DIGEST_STREAK_MIN : NUDGE_STREAK_MIN,
355+
settings.timezone
325356
)
326357
const summary = buildSummary(
327358
{

mcp-server/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
"dev": "mcp-use dev",
2626
"start": "mcp-use start",
2727
"deploy": "mcp-use deploy",
28+
"test": "vitest run",
29+
"test:watch": "vitest",
2830
"postinstall": "mcp-use generate-types || exit 0"
2931
},
3032
"dependencies": {
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import { describe, it, expect } from "vitest";
2+
import { gradeCard, cardFromRow } from "./flashcards.js";
3+
4+
/**
5+
* Characterization tests (issue #549) — these capture current behaviour of
6+
* `cardFromRow`/`gradeCard` in mcp-server/src/tools/flashcards.ts so a later
7+
* refactor shows up as a visible test diff. Do NOT change source behaviour
8+
* to make these pass; if source is wrong, the test documents the bug.
9+
*/
10+
11+
const NOW = new Date("2026-07-26T12:00:00.000Z");
12+
13+
type FsrsRow = Parameters<typeof cardFromRow>[0];
14+
15+
/** A row that has never been through FSRS — the SM-2/pre-migration shape. */
16+
function freshRow(overrides: Partial<FsrsRow> = {}): FsrsRow {
17+
return {
18+
interval_days: 0,
19+
repetitions: 0,
20+
due_at: NOW.toISOString(),
21+
last_reviewed_at: null,
22+
stability: null,
23+
difficulty: null,
24+
fsrs_state: 0,
25+
lapses: 0,
26+
learning_steps: 0,
27+
elapsed_days: 0,
28+
...overrides,
29+
};
30+
}
31+
32+
/** A fully FSRS-populated row (as a migration-seeded / previously-graded row would be). */
33+
function populatedRow(overrides: Partial<FsrsRow> = {}): FsrsRow {
34+
return {
35+
interval_days: 6,
36+
repetitions: 3,
37+
due_at: "2026-07-30T00:00:00.000Z",
38+
last_reviewed_at: "2026-07-24T00:00:00.000Z",
39+
stability: 12.34,
40+
difficulty: 5.67,
41+
fsrs_state: 2, // Review
42+
lapses: 1,
43+
learning_steps: 0,
44+
elapsed_days: 6,
45+
...overrides,
46+
};
47+
}
48+
49+
describe("cardFromRow", () => {
50+
it("returns a fresh empty card when stability is null", () => {
51+
const row = freshRow({ stability: null, difficulty: 5 });
52+
const card = cardFromRow(row, NOW);
53+
expect(card.state).toBe(0); // State.New
54+
expect(card.stability).toBe(0);
55+
expect(card.difficulty).toBe(0);
56+
expect(card.reps).toBe(0);
57+
expect(card.lapses).toBe(0);
58+
expect(card.due).toEqual(NOW);
59+
expect(card.last_review).toBeUndefined();
60+
});
61+
62+
it("returns a fresh empty card when difficulty is null", () => {
63+
const row = freshRow({ stability: 5, difficulty: null });
64+
const card = cardFromRow(row, NOW);
65+
expect(card.state).toBe(0); // State.New
66+
expect(card.stability).toBe(0);
67+
expect(card.difficulty).toBe(0);
68+
expect(card.due).toEqual(NOW);
69+
expect(card.last_review).toBeUndefined();
70+
});
71+
72+
it("returns a fresh empty card when both stability and difficulty are null", () => {
73+
const row = freshRow();
74+
const card = cardFromRow(row, NOW);
75+
expect(card.state).toBe(0);
76+
expect(card.due).toEqual(NOW);
77+
});
78+
79+
it("round-trips a fully populated row onto the Card", () => {
80+
const row = populatedRow();
81+
const card = cardFromRow(row, NOW);
82+
expect(card.stability).toBe(row.stability);
83+
expect(card.difficulty).toBe(row.difficulty);
84+
expect(card.reps).toBe(row.repetitions);
85+
expect(card.lapses).toBe(row.lapses);
86+
expect(card.state).toBe(row.fsrs_state);
87+
expect(card.scheduled_days).toBe(row.interval_days);
88+
expect(card.elapsed_days).toBe(row.elapsed_days);
89+
expect(card.learning_steps).toBe(row.learning_steps);
90+
expect(card.due).toEqual(new Date(row.due_at));
91+
expect(card.last_review).toEqual(new Date(row.last_reviewed_at as string));
92+
});
93+
94+
it("falls back to `now` for last_review when last_reviewed_at is null", () => {
95+
const row = populatedRow({ last_reviewed_at: null });
96+
const card = cardFromRow(row, NOW);
97+
expect(card.last_review).toEqual(NOW);
98+
});
99+
});
100+
101+
describe("gradeCard", () => {
102+
const ratings = ["again", "hard", "good", "easy"] as const;
103+
104+
it.each(ratings)("returns the full persisted-row shape for rating '%s' on a New card", (rating) => {
105+
const row = freshRow();
106+
const result = gradeCard(row, rating, NOW);
107+
108+
expect(result).toMatchObject({
109+
stability: expect.any(Number),
110+
difficulty: expect.any(Number),
111+
fsrs_state: expect.any(Number),
112+
lapses: expect.any(Number),
113+
learning_steps: expect.any(Number),
114+
elapsed_days: expect.any(Number),
115+
interval_days: expect.any(Number),
116+
repetitions: expect.any(Number),
117+
due_at: expect.any(String),
118+
last_reviewed_at: expect.any(String),
119+
});
120+
121+
// due_at / last_reviewed_at are ISO strings.
122+
expect(new Date(result.due_at).toISOString()).toBe(result.due_at);
123+
expect(new Date(result.last_reviewed_at).toISOString()).toBe(result.last_reviewed_at);
124+
expect(result.last_reviewed_at).toBe(NOW.toISOString());
125+
});
126+
127+
it("orders scheduling further out as rating improves: again < hard < good < easy", () => {
128+
const row = freshRow();
129+
const again = gradeCard(row, "again", NOW);
130+
const hard = gradeCard(row, "hard", NOW);
131+
const good = gradeCard(row, "good", NOW);
132+
const easy = gradeCard(row, "easy", NOW);
133+
134+
const t = (r: ReturnType<typeof gradeCard>) => new Date(r.due_at).getTime();
135+
136+
expect(t(again)).toBeLessThanOrEqual(t(hard));
137+
expect(t(hard)).toBeLessThanOrEqual(t(good));
138+
expect(t(good)).toBeLessThan(t(easy));
139+
});
140+
141+
it("rating 'again' on an established Review-state card increments lapses", () => {
142+
const row = populatedRow({ fsrs_state: 2, lapses: 1 });
143+
const result = gradeCard(row, "again", NOW);
144+
expect(result.lapses).toBe(2);
145+
});
146+
147+
it("BUG (#549 §4): a 'lapsed' row (repetitions/interval reset but stability/difficulty null) currently grades as a brand-new card, discarding history", () => {
148+
// This row represents history that was reset to null stability/difficulty
149+
// (e.g. a lapse-tracking path) while repetitions/interval_days/last_reviewed_at
150+
// still carry prior state. Because cardFromRow() only checks
151+
// stability===null || difficulty===null, this row is treated as a
152+
// never-seen card: FSRS starts a brand-new learning card, and prior
153+
// repetitions/interval history is silently discarded. Asserting today's
154+
// (buggy) behaviour here — do NOT "fix" the source to make this pass.
155+
const lapsedRow = freshRow({
156+
repetitions: 0,
157+
interval_days: 0,
158+
last_reviewed_at: "2026-07-20T00:00:00.000Z",
159+
stability: null,
160+
difficulty: null,
161+
});
162+
163+
const card = cardFromRow(lapsedRow, NOW);
164+
// Discards history: comes back as a brand-new createEmptyCard(now) — the
165+
// null-stability/difficulty check short-circuits BEFORE the
166+
// last_reviewed_at fallback logic even runs, so last_review is left
167+
// `undefined` (createEmptyCard's default), not `now` and not the row's
168+
// real last_reviewed_at.
169+
expect(card.state).toBe(0); // State.New
170+
expect(card.reps).toBe(0);
171+
expect(card.last_review).toBeUndefined();
172+
173+
const result = gradeCard(lapsedRow, "good", NOW);
174+
// A brand-new "good" grade produces the same result whether or not the
175+
// row had prior repetitions — because cardFromRow() throws that history away.
176+
const freshResult = gradeCard(freshRow(), "good", NOW);
177+
expect(result).toEqual(freshResult);
178+
});
179+
});

0 commit comments

Comments
 (0)