Uh oh!
There was an error while loading. Please reload this page.
fix(mastery): one set of tier thresholds, cited not copied (#557) - #570
Conversation
Three sets existed. `config.get_mastery_tier` said mastered >= 0.75 / learning >= 0.45 / struggling >= 0.1; the tutor's progress tool carried its own 0.7 / 0.4; flashcards drilled an ad-hoc < 0.4. So a concept at 0.72 read "learning" on the Tree while the tutor counted it mastered, and one at 0.42 read "struggling" on the Tree while the tutor counted it in-progress AND flashcards refused to drill it — three surfaces disagreeing about the same number in the same session. `config.py` now owns the thresholds as named constants plus two predicates, `is_mastered` / `is_weak`, and the other two sites call them. `is_weak` is "below the learning floor" — struggling OR unexplored — because that union is what every caller is actually asking for (which concepts need work), and splitting it would push the union back out to the call sites, which is where the drift came from. The tutor's local copy carried a comment claiming the duplication was deliberate, so "the agent's definitions can evolve independently". They did not evolve; they drifted. That rationale is replaced with the rule the issue asks for: a genuinely different cut gets a named constant in `config.py`, never a literal at the call site. Behaviour changes, all of them the point of the issue: * tutor `mastered_count` 0.7 -> 0.75 * tutor `weak_count` 0.4 -> 0.45 * flashcards' weak-concept picker 0.4 -> 0.45, so concepts in [0.4, 0.45) — "struggling" on the Tree — are now offered for practice. The surface whose whole job is drilling weak concepts had been skipping a slice. Tests pin the AGREEMENT, not the numbers, so the thresholds stay movable in one place: a sweep across every tier and boundary asserts the tutor's classification matches `get_mastery_tier` for the same score, driven through the real tool rather than its constants. Plus a guard that the local constants are gone, since the failure mode was three copies drifting, not one wrong number. Hermetic 2169 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 37e4c15 | Commit Preview URL Branch Preview URL | Aug 22 2026, 08:34 AM |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reached
Next review available in:54 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughMastery thresholds are centralized in ChangesMastery tier unification
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk:⚪ Minimal · up to This PR centralizes mastery thresholds and aligns tutor progress and flashcard selection with the canonical behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Two review findings, both about claims this PR made that weren't yet true. **A fourth copy survived, in TypeScript.** `Learn.tsx::tierForScore` re-declares 0.75/0.45/0.1 to classify a STREAMED mastery delta client-side, so the live Tree matches the refetch that follows. It cannot import from `config.py` — but the header I added asserted "every surface reads them from here", and the guard test only checked that two Python attribute names were absent. So moving a threshold would have painted a node one tier live and a different tier on the next refetch: #557's own bug, across the wire instead of across two modules. The mirror is now pinned by a test that reads the TSX and asserts the numbers match, and the header says the mirror exists. **Widening the floor made an unsorted cap unsafe.** `_get_weak_concepts` takes the first 15 qualifying rows in PostgREST order. That was survivable at `< 0.4`; at `< 0.45` the newly-admitted [0.4, 0.45) concepts can displace 0.0-0.1 ones on arbitrary row order, so the surface whose job is drilling the weakest concepts could drill the least-weak of the weak — a regression created by this PR's own widening. Sorted ascending before the cap, which also makes the truncation deterministic. Hermetic 2171 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AndresL230
commented
Aug 22, 2026
Review round — two findings, both fixed in |
Uh oh!
There was an error while loading. Please reload this page.
Closes#557. Workstream H5 of epic #537.
The problem
Three sets of mastery thresholds existed:
config.get_mastery_tier(canonical — the Tree, seeds, graph writes)agents/tools/chat_context.py(tutor progress)routes/flashcards.py(weak-concept picker)So in one session a concept at 0.72 read "learning" on the Tree while the tutor counted it mastered, and one at 0.42 read "struggling" on the Tree while the tutor counted it in progress and flashcards refused to drill it. Three surfaces disagreeing about the same number.
The fix
config.pyowns the thresholds as named constants plus two predicates,is_mastered/is_weak. The other two sites call them.is_weakmeans "below the learning floor" — struggling or unexplored — because that union is what every caller is actually asking ("which concepts need work"). Splitting it would push the union back out to the call sites, which is exactly where the drift came from.The tutor's copy carried a comment asserting the duplication was deliberate, so "the agent's definitions of 'mastered' / 'weak' can evolve independently". They didn't evolve, they drifted — so that rationale is replaced with the rule the issue asks for: a genuinely different cut gets a named constant in
config.py, never a literal at the call site.Behaviour changes
All three are the point of the issue, not incidental:
mastered_count: 0.7 → 0.75weak_count: 0.4 → 0.450.4was never a tier boundary in the first place; it was a third number nobody reconciled.Tests
They pin the agreement, not the numbers, so the thresholds stay movable in one place:
get_mastery_tierfor the same score, driven through the real tool rather than through its constants, so reintroducing a local threshold anywhere on that path fails;Verification
Hermetic 2169 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 47 passed (the single failure is #566, red on
main, fixed by #568).🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Tests